feat(sdk): chat.agent — runtime + browser transport (2/4)#3543
feat(sdk): chat.agent — runtime + browser transport (2/4)#3543ericallam wants to merge 6 commits into
Conversation
🦋 Changeset detectedLatest commit: 353fbf1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 31 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| */ | ||
| handoverResponse(result: StreamTextResult<any, any>): Response; | ||
| /** Manually dispatch the `handover` signal on `session.in`. */ | ||
| handover(args: { partialAssistantMessage: ModelMessage[] }): Promise<void>; |
There was a problem hiding this comment.
🔴 Public HeadStartSession.handover type omits required isFinal parameter, causing agent to always enter non-final path
The public HeadStartSession type declares handover(args: { partialAssistantMessage: ModelMessage[] }) at packages/trigger-sdk/src/v3/chat-server.ts:133, but the internal function it maps to (chat-server.ts:382-394) requires isFinal: boolean. When a customer calls handle.handover({ partialAssistantMessage: msgs }) through the chat.openSession() escape-hatch API, args.isFinal is undefined. In JSON.stringify at line 393, isFinal: undefined is omitted from the wire payload. The agent receiving this kind: "handover" chunk interprets the missing isFinal as falsy, so it always enters the non-final branch (runs streamText to execute tool-calls) — even when the customer intended a final handover (pure-text, no LLM call). There is no way for a customer using the manual handover() method to signal a final response.
| handover(args: { partialAssistantMessage: ModelMessage[] }): Promise<void>; | |
| handover(args: { partialAssistantMessage: ModelMessage[]; isFinal?: boolean; messageId?: string }): Promise<void>; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| void handoverWhenDone(result) | ||
| .finally(() => clearTimeout(idleTimer)) | ||
| .catch(() => {}); |
There was a problem hiding this comment.
🟡 Idle timer leak when handoverWhenDone is called outside handoverResponse
In openHandoverSession, an idleTimer is created at chat-server.ts:321 that aborts the session's AbortController after the timeout. The clearTimeout(idleTimer) call only exists inside handoverResponse at line 603 (chained on handoverWhenDone's .finally()). When a customer uses the lower-level chat.openSession() API and calls handle.tee() + handle.handoverWhenDone() directly — without going through handle.handoverResponse() — the timer is never cleared. After idleTimeoutInSeconds (default 60s), the timer fires and calls abortController.abort(), which may cancel in-progress operations or SSE subscriptions that the customer still needs.
Prompt for agents
The idleTimer created at line 321 is only cleared inside handoverResponse (line 603). The HeadStartSession handle exposes handoverWhenDone as a standalone public method (line 640), but if a customer calls it directly via the chat.openSession() escape hatch, the timer never gets cleared. Move the clearTimeout(idleTimer) into the handoverWhenDone function itself (e.g. in its finally block), so the timer is always cleared regardless of which code path invokes handoverWhenDone.
Was this helpful? React with 👍 or 👎 to provide feedback.
| async append(value, options) { | ||
| // Use a single-write writer so objects are serialized the same way | ||
| // as stream.writer() — the raw append API sends BodyInit which | ||
| // doesn't serialize objects correctly for SSE consumers. | ||
| const { waitUntilComplete } = writer(opts.id, { | ||
| ...options, | ||
| spanName: "streams.append()", | ||
| execute: ({ write }) => { | ||
| write(value); | ||
| }, | ||
| }); | ||
| await waitUntilComplete(); | ||
| }, |
There was a problem hiding this comment.
🚩 streams.define().append() changed from raw append to writer-based serialization
The define<TPart>().append() method at streams.ts:664-676 was changed from delegating to the raw append() function (which sends BodyInit) to using writer() with a single write() call. The comment explains this ensures objects are serialized the same way as stream.writer() — the raw append API sends BodyInit which doesn't serialize objects correctly for SSE consumers. This is a behavioral change to the RealtimeDefinedStream.append() method. Any existing code that relied on the old raw-append behavior (e.g., passing pre-stringified data) will now get double-serialized through the writer path. The change is intentional and an improvement, but callers should be aware of the semantic shift.
Was this helpful? React with 👍 or 👎 to provide feedback.
| "peerDependencies": { | ||
| "zod": "^3.0.0 || ^4.0.0", | ||
| "ai": "^4.2.0 || ^5.0.0 || ^6.0.0" | ||
| "ai": "^5.0.0 || ^6.0.0", |
There was a problem hiding this comment.
🚩 peerDependency range for 'ai' package narrowed — drops v4 support
In packages/trigger-sdk/package.json, the ai peer dependency range changed from ^4.2.0 || ^5.0.0 || ^6.0.0 to ^5.0.0 || ^6.0.0, dropping v4 support. The changeset mentions this is intentional (new features require AI SDK v5+). This is a breaking change for any existing users on ai@4.x — they'll see peer dependency warnings on install. The devDependency was also bumped from ^6.0.0 to ^6.0.116.
Was this helpful? React with 👍 or 👎 to provide feedback.
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
b84d537 to
ed7bf97
Compare
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
from
dbc0034 to
64929b7
Compare
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
ed7bf97 to
365e73b
Compare
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
from
64929b7 to
9d8b67b
Compare
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
365e73b to
b4a0986
Compare
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
from
9d8b67b to
f83a0e2
Compare
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
b4a0986 to
1712b59
Compare
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
from
f83a0e2 to
4cedf7c
Compare
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
1712b59 to
3721c34
Compare
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
from
4cedf7c to
63cb53e
Compare
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
3721c34 to
f240799
Compare
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
11 times, most recently
from
f694134 to
13447b8
Compare
ericallam
changed the title
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
from
13447b8 to
2e77f5f
Compare
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
9a09da4 to
84c717c
Compare
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
3 times, most recently
from
9af14ef to
b54c38e
Compare
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
2218110 to
5359eda
Compare
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
3 times, most recently
from
df753af to
950c0b5
Compare
| async *messages(): AsyncGenerator<UIMessage, void, unknown> { | ||
| for await (const message of readUIMessageStream({ stream: this._consumerStream })) { | ||
| this.lastAssistantMessage = message; | ||
| yield message; | ||
| } | ||
| if (this.lastAssistantMessage && this.onAssistantMessage) { | ||
| this.onAssistantMessage(this.lastAssistantMessage); | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 ChatStream calls onAssistantMessage callback twice when consumed via messages()
When ChatStream is constructed with an onAssistantMessage callback, the constructor tees the stream and starts a background collector IIFE that calls onAssistantMessage when the collector branch drains (chat-client.ts:142-144). If the consumer then iterates via messages(), that method reads the other tee branch and also calls this.onAssistantMessage at the end (chat-client.ts:183-184). Both branches process the same data and both invoke the callback, so onAssistantMessage fires twice with the same lastAssistantMessage. Currently no internal caller passes onAssistantMessage to the constructor (sendMessage and sendAction both call new ChatStream(rawStream) without it), so this only affects direct new ChatStream(stream, callback) + .messages() usage — but ChatStream is a public export.
Prompt for agents
The issue is in ChatStream (chat-client.ts). When onAssistantMessage is provided, the constructor tees the stream and starts a background IIFE on the collector branch that calls onAssistantMessage when done. The messages() method also calls onAssistantMessage after draining the consumer branch. Both fire the callback.
Fix options:
1. In messages(), skip the onAssistantMessage call when the tee collector is active (i.e., when _messageCollector is set). The collector IIFE handles the callback in that case.
2. Alternatively, don't tee at all when messages() is the intended consumption path — detect and short-circuit.
The simplest fix is option 1: in messages() at the end, guard the callback with something like:
if (this.lastAssistantMessage && this.onAssistantMessage && !this._messageCollector) {
this.onAssistantMessage(this.lastAssistantMessage);
}
This way, when the constructor set up the collector IIFE (meaning _messageCollector is truthy), messages() defers to the collector for the callback.
Was this helpful? React with 👍 or 👎 to provide feedback.
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
37ea386 to
32b0e42
Compare
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
from
c8b5507 to
8b6fcfd
Compare
| child.stdout?.on("data", (chunk: Buffer | string) => { | ||
| stdout += chunk.toString(); | ||
| }); | ||
| child.stderr?.on("data", (chunk: Buffer | string) => { | ||
| stderr += chunk.toString(); | ||
| }); | ||
| child.once("close", (code: number | null) => { | ||
| resolvePromise({ | ||
| exitCode: code, | ||
| stdout: truncate(stdout, DEFAULT_BASH_OUTPUT_BYTES), | ||
| stderr: truncate(stderr, DEFAULT_BASH_OUTPUT_BYTES), | ||
| }); |
There was a problem hiding this comment.
🟡 Unbounded stdout/stderr accumulation in runBashInSkill before truncation can cause OOM
The runBashInSkill function accumulates stdout and stderr into strings with no size cap during the child process execution (stdout += chunk.toString()). The truncate() call only fires after the process exits in the close event handler. If an LLM tool call generates a command that produces very large output (e.g., cat /dev/zero | head -c 2000000000), the in-memory string grows unbounded until the process completes or the container OOMs. The DEFAULT_BASH_OUTPUT_BYTES limit of 64 KiB only trims the string after it has already been fully accumulated in memory.
Accumulation without cap vs. post-hoc truncation
Lines 110–114 accumulate without limit:
child.stdout?.on("data", (chunk) => {
stdout += chunk.toString(); // grows unbounded
});Lines 117–121 truncate only on close:
child.once("close", (code) => {
resolvePromise({
stdout: truncate(stdout, DEFAULT_BASH_OUTPUT_BYTES), // too late
});
});Prompt for agents
In runBashInSkill (agentSkillsRuntime.ts), the stdout and stderr strings accumulate data from the child process data events without any size cap. The truncate() call at close time only trims after the entire output is already in memory. To fix this, cap the accumulation in the on(data) handlers: once stdout.length exceeds DEFAULT_BASH_OUTPUT_BYTES, stop appending new chunks (or discard them). Same for stderr. This prevents an LLM-generated command from producing enough output to OOM the worker process. A simple approach: check the current length before appending and skip/trim chunks that would push past the limit. You could also track a boolean like stdoutCapped to avoid repeated length checks after the cap is hit.
Was this helpful? React with 👍 or 👎 to provide feedback.
| function safeJoinInside(root: string, relative: string): string { | ||
| if (nodePath.isAbsolute(relative)) { | ||
| throw new Error(`Path must be relative to the skill directory: ${relative}`); | ||
| } | ||
| const resolved = nodePath.resolve(root, relative); | ||
| const normalized = nodePath.resolve(root) + nodePath.sep; | ||
| if (resolved !== nodePath.resolve(root) && !resolved.startsWith(normalized)) { | ||
| throw new Error(`Path escapes the skill directory: ${relative}`); | ||
| } | ||
| return resolved; | ||
| } |
There was a problem hiding this comment.
🚩 safeJoinInside path-traversal guard does not resolve symlinks
The safeJoinInside function uses nodePath.resolve() which normalizes .. segments but does NOT resolve filesystem symlinks. A symlink placed inside the skill directory (e.g., skills/demo/link -> /etc) would pass the prefix check since resolve(skillPath, 'link/passwd') produces {skillPath}/link/passwd which starts with the normalized root. The actual fs.readFile call would then follow the symlink to /etc/passwd. In practice this is low-risk because skill directories are developer-authored content bundled at build time — the developer controls what's there. However, if bash tool calls create symlinks at runtime (the bash tool runs with the skill dir as cwd), an LLM could exploit this. Using fs.realpath on the resolved path before the prefix check would close this gap.
Was this helpful? React with 👍 or 👎 to provide feedback.
Adds the chat.agent({...}) task definition (server runtime) and the
browser-side TriggerChatTransport + AgentChat that drives it from a
React or Next.js app. The runtime sits on top of the Sessions primitive
and handles the durable conversational task lifecycle.
Server runtime:
- chat.agent({...}) — session-aware task definition
- Lifecycle hooks: onChatStart, onTurnStart, onTurnComplete, onAction,
onValidateMessages, hydrateMessages
- chat.history read primitives for HITL flows
- chat.local, chat.headStart, chat.handover, oomMachine
- Delta-only wire + S3 snapshot reconstruction at run boot
- Actions are no longer turns
Browser transport:
- TriggerChatTransport (ai-sdk Transport): delta-only wire sends,
SSE reconnection with lastEventId resume, stop/abort cleanup,
dynamic accessToken refresh
- AgentChat: direct programmatic API
- useTriggerChatTransport (React hook)
- chat-tab-coordinator: cross-tab leader election
Includes the chat-agent, chat-agent-delta-wire-snapshots,
chat-history-read-primitives, chat-head-start, chat-actions-no-turn,
chat-session-attributes, agent-skills, and mock-chat-agent-test-harness
changesets.
…branch onChatStart's contract changes from 'fires on turn 0 of every run' to 'fires exactly once per chat, on the very first user message of the chat's lifetime.' Gated on `!couldHavePriorState`, so the hook also skips OOM-retry attempts — same outcome, the chat already started. The `continuation` and `previousRunId` fields on `ChatStartEvent` are now `@deprecated` (always false / undefined when the hook fires). Customers should drop any `if (continuation) return;` checks — they're no longer reachable. For per-turn setup that should run on continuation turns too, use `onTurnStart` (still fires on every turn including the first turn of a continuation run). Pairs with the server-side continuation-overrides fix that strips sticky boot-payload fields from basePayload on continuation runs. A new continuation-wait boot branch in the run loop kicks in when `payload.continuation === true && !payload.message` — the run waits silently on session.in for the next user message instead of running a phantom turn against the snapshot-seeded accumulator. `onPreload` does NOT fire on this path; `onChatStart` fires on the first real turn after the wait resolves. `ChatSuspendEvent` / `ChatResumeEvent` get a new discriminator `phase: "continuation"` for suspend/resume hooks during the wait. Mock harness: - new `mode: "continuation"` boots with trigger omitted + continuation: true (mirrors what the server produces on continuation runs) - `continuation: true` without explicit mode auto-selects 'continuation' - new `previousRunId` option Tests (3 new in mockChatAgent.test.ts): - onChatStart fires on a fresh first message (baseline) - onChatStart does NOT fire on a continuation run - onChatStart does NOT fire on an OOM-retry attempt (ctx.attempt.number > 1)
The TaskRunExecutionAttempt type only includes `number` and `startedAt`. The `status: "EXECUTING"` field I included was invalid. tsc passed on my local test runner (vitest uses esbuild, not tsc) but the typecheck CI step caught it. Drop the field, keeping the override to just the two supported keys.
The session.in version of waitWithIdleTimeout had the same shape
inconsistency as the input-stream version (fixed on se): the
skipSuspend branch returned { ok: false, error: undefined } instead
of a real WaitpointTimeoutError. Now matches the cold-phase wait()
result shape so callers can rely on a uniform { error: WaitpointTimeoutError }
on every !ok path.
…ecute subtasks
`chat.stream.writer({ target: "root" })` and the rest of the
`chat.stream` helpers all funnel through `getChatSession()`, which
looked up the session handle in run-scoped `locals`. That key is only
populated inside a `chat.agent` run — `ai.toolExecute` subtasks
running in a separate worker process don't inherit it, so any chat
helper used from a subtask threw `"chat.agent session handle is not
initialized"`.
Resolve the handle lazily: if it's not in `locals`, fall back to the
`chatId` already threaded through tool metadata under `METADATA_KEY`
and call `sessions.open(chatId)` to materialize it. Cache the result
back into `locals` so subsequent calls in the same subtask hit the
fast path.
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
from
8b6fcfd to
353fbf1
Compare
2 participants
Summary
Adds
chat.agent({...}), a durable conversational task runtime, plus the browser-sideTriggerChatTransport+AgentChatthat drive it from a React or Next.js app. Conversations survive page refreshes, network blips, idle suspend, and process restarts, with built-in tools, HITL approvals, multi-turn state, and stop-mid-stream cancellation. Builds on #3542.Design
Each
/in/appendrequest carries at most one new message. The agent reconstructs prior history at run boot from an object-store snapshot plus asession.outreplay tail, so conversation context lives server-side instead of bloating the wire. Awaited snapshot writes after everyonTurnCompletekeep the chain durable across idle suspend. RegisteringhydrateMessagesshort-circuits both paths for customers who own their own conversation store.Lifecycle hooks —
onChatStart,onTurnStart,onTurnComplete,onAction,onValidateMessages,hydrateMessages— cover validation, persistence, and post-turn work.chat.historyexposes read primitives (getPendingToolCalls,getResolvedToolCalls,extractNewToolResults,findMessage,all) for HITL flows.chat.localgives per-run typed state with Proxy access and dirty tracking.chat.headStartbridges first-turn TTFC via a customer HTTP handler.oomMachineopts a chat into one-shot OOM-retry on a larger machine.TriggerChatTransportis aTransportimplementation for Vercel's ai-sdkuseChat: delta-only wire sends, SSE reconnection withlastEventIdresume, stop/abort cleanup, dynamicaccessTokenrefresh,X-Peek-Settledfast-close.AgentChatis the direct programmatic equivalent. A cross-tab coordinator does leader election so multiple open tabs share a single SSE.Test plan
useChatagainst a local webappstop(), verify clean abortStack
Part of a 4-PR stack. Merge bottom-up.
main— Sessions dashboard + chat-ready hardeningOriginally split across #3543 and #3544. Folded into a single PR because the SDK package.json subpath exports for
./chatand./chat/reactmade the runtime and the browser transport install-coupled.This is part 4 of 5 in a stack made with GitButler: