Skip to content

Accept Realtime client + channelName in sessions#89

Open
zknill wants to merge 3 commits into
mainfrom
zak/ait-718/accept-realtime-client
Open

Accept Realtime client + channelName in sessions#89
zknill wants to merge 3 commits into
mainfrom
zak/ait-718/accept-realtime-client

Conversation

@zknill
Copy link
Copy Markdown
Contributor

@zknill zknill commented May 6, 2026

Replace channel: Ably.RealtimeChannel on createClientSession and createAgentSession (core + Vercel) with { client: Ably.Realtime, channelName: string }. The session resolves the channel itself via client.channels.get(channelName) and registers an ai-transport-js agent identifier on client.options.agents for SDK usage tracking, mirroring the pattern used by ably-chat-js. The agent registration is idempotent so multiple sessions sharing one client are safe.

This shifts boilerplate (client.channels.get(...)) out of every caller and gives the backend a way to attribute usage to this SDK. The caller still owns the client's lifecycle — session.close() does not release the channel or close the client.

The React ClientSessionProvider drops Ably's ChannelProvider / useChannel plumbing and now reads the client from the surrounding <AblyProvider> via useAbly(). The Vercel ChatTransportProvider inherits the simplification.

Spec points AIT-ST1 and AIT-CT1 are amended; new sub-points AIT-ST1a / AIT-CT1a describe the agent registration and channel ownership contract. The /release skill is updated to bump src/version.ts alongside package.json.

Demos and the custom-codec example are updated to the new shape. Tests use a new shared createMockClient(channel) helper.

@github-actions github-actions Bot temporarily deployed to staging/pull/89/typedoc May 6, 2026 12:54 Inactive
@github-actions

This comment was marked as low quality.

Replace `channel: Ably.RealtimeChannel` on `createClientSession` and
`createAgentSession` (core + Vercel) with `{ client: Ably.Realtime,
channelName: string }`. The session resolves the channel itself via
`client.channels.get(channelName)` and registers an `ai-transport-js`
agent identifier on `client.options.agents` for SDK usage tracking,
mirroring the pattern used by ably-chat-js. The agent registration
is idempotent so multiple sessions sharing one client are safe.

This shifts boilerplate (`client.channels.get(...)`) out of every
caller and gives the backend a way to attribute usage to this SDK.
The caller still owns the client's lifecycle — `session.close()`
does not release the channel or close the client.

The React `ClientSessionProvider` drops Ably's `ChannelProvider` /
`useChannel` plumbing and now reads the client from the surrounding
`<AblyProvider>` via `useAbly()`. The Vercel `ChatTransportProvider`
inherits the simplification.

Spec points `AIT-ST1` and `AIT-CT1` are amended; new sub-points
`AIT-ST1a` / `AIT-CT1a` describe the agent registration and channel
ownership contract. The `/release` skill is updated to bump
`src/version.ts` alongside `package.json`.

Demos and the custom-codec example are updated to the new shape.
Tests use a new shared `createMockClient(channel)` helper.
@zknill zknill force-pushed the zak/ait-718/accept-realtime-client branch from fc3bc5b to c021968 Compare May 6, 2026 13:12
@zknill zknill requested review from a team and lmars and removed request for a team May 6, 2026 13:12
@zknill zknill marked this pull request as ready for review May 6, 2026 13:12
@zknill
Copy link
Copy Markdown
Contributor Author

zknill commented May 6, 2026

@github-actions github-actions Bot temporarily deployed to staging/pull/89/typedoc May 6, 2026 13:13 Inactive
@zknill zknill requested review from VeskeR and lawrence-forooghian and removed request for VeskeR and lmars May 7, 2026 09:29
Comment thread src/core/agent.ts
@lawrence-forooghian
Copy link
Copy Markdown
Contributor

lawrence-forooghian commented May 7, 2026

It seems to me like there are two separate pieces of work here:

  1. Making the session accept a client instead of a channel
  2. Injecting the AIT agent identifier into the client

Whilst 1 seems necessary to enable 2, it also seems from the PR description like you believe that 1 is a valid standalone change ("shifts boilerplate"). Would this PR benefit from multiple commits in that case?

Comment thread src/core/transport/agent-session.ts Outdated
Comment thread src/core/transport/client-session.ts Outdated
Comment thread src/core/transport/client-session.ts Outdated
Comment thread docs/get-started/vercel-use-chat.md Outdated
Comment thread src/core/transport/types.ts Outdated
@lawrence-forooghian
Copy link
Copy Markdown
Contributor

I've looked at the tracking side of things but not the React side; would appreciate a look at that from @ttypic or @VeskeR

Comment thread src/core/transport/agent-session.ts Outdated
zknill added 2 commits May 8, 2026 15:12
Sessions now also carry the `ai-transport-js` agent on the channel
ATTACH via `client.channels.get(name, { params: { agent } })`, in
addition to the existing `options.agents` mutation. The agents-map
path only attaches the identifier to the initial WebSocket connect;
when the supplied client is already CONNECTING/CONNECTED it gets
missed. Mirroring chat-js's two-path approach closes that gap.

`registerAgent` now returns the channel options the caller passes
to `channels.get`, so each session constructor configures the
client and resolves the channel in one go.

Public JSDoc on `AgentSessionOptions`, `ClientSessionOptions`, the
two factory functions, the React provider, and the React hooks
loses its leaked-implementation prose ("calls channels.get
internally", "registers the agent for usage tracking", "via
useAbly()"). It is replaced with an ownership contract: the caller
owns the client; the session owns the channel — don't resolve the
same channel name elsewhere with conflicting options. Three docs
files get the same sweep.

New tests in test/core/transport/{agent,client}-session.test.ts
assert that constructing a session both writes
options.agents['ai-transport-js'] and forwards
params.agent: 'ai-transport-js/<VERSION>' to channels.get, and
that pre-seeded unrelated agents survive across multiple sessions
sharing one client.
@zknill
Copy link
Copy Markdown
Contributor Author

zknill commented May 8, 2026

Would this PR benefit from multiple commits in that case?

Probably, but if it's ok I will leave them in one. It would have been better to open as 2 commits, but now we are here 🤷

@zknill zknill requested a review from lawrence-forooghian May 8, 2026 14:25
Copy link
Copy Markdown
Contributor

@lawrence-forooghian lawrence-forooghian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy with the tracking changes but I'd appreciate eyes on the React changes from someone who knows React

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants