feat(sdk): chat.agent — runtime + browser transport (2/4)#3543
feat(sdk): chat.agent — runtime + browser transport (2/4)#3543ericallam wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 535245f 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 |
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (72)
✨ Finishing Touches📝 Generate docstrings
🧪 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.
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
from
8b6fcfd to
353fbf1
Compare
ericallam
force-pushed
the
feature/sessions-primitive
branch
from
219e550 to
be1a6cf
Compare
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
from
353fbf1 to
9a8b726
Compare
| console.log("[usePendingMessages] promote blocked — already promoted:", id); | ||
| return; | ||
| } | ||
| console.log("[usePendingMessages] promoting:", id); |
There was a problem hiding this comment.
🟡 Debug console.log statements left in production promoteToSteering callback
The promoteToSteering callback in usePendingMessages contains two console.log calls (lines 401 and 404) with the [usePendingMessages] prefix. These are clearly debug/development logs that will appear in end-users' browser consoles whenever they interact with the promote-to-steering feature. Production React hooks should not emit debug logs.
| console.log("[usePendingMessages] promote blocked — already promoted:", id); | |
| return; | |
| } | |
| console.log("[usePendingMessages] promoting:", id); | |
| if (promotedIdsRef.current.has(id)) { | |
| return; | |
| } | |
| promotedIdsRef.current.add(id); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| onTurnComplete?.({ | ||
| chatId, | ||
| lastEventId: state.lastEventId, | ||
| }); |
There was a problem hiding this comment.
🟡 Unhandled promise rejection from async onTurnComplete callback in AgentChat
In AgentChat.subscribeToSessionStream, the onTurnComplete callback (typed void | Promise<void>) is invoked fire-and-forget at chat-client.ts:745 without .catch() protection. If a customer passes an async onTurnComplete (e.g., persisting lastEventId to a database) and the operation throws, the returned Promise rejection is unhandled. In Node.js with --unhandled-rejections=throw (the default in recent versions), this crashes the process. The comparable browser-side transport (chat.ts:1207) uses synchronous notifySessionChange and doesn't have this issue.
Affected code
At line 745, onTurnComplete?.({...}) returns a potentially-rejected Promise that is never caught:
onTurnComplete?.({
chatId,
lastEventId: state.lastEventId,
});
internalAbort.abort();| onTurnComplete?.({ | |
| chatId, | |
| lastEventId: state.lastEventId, | |
| }); | |
| Promise.resolve(onTurnComplete?.({ | |
| chatId, | |
| lastEventId: state.lastEventId, | |
| })).catch(() => {}); |
Was this helpful? React with 👍 or 👎 to provide feedback.
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
from
9a8b726 to
7690836
Compare
…3542) ## Summary A `/sessions` dashboard for inspecting durable Sessions, an `AGENT` / `SCHEDULED` task-kind filter for the runs list, and the server-side hardening (rate-limit exemption for packets, retry-with-backoff on stream appends, typed too-large-chunk error) that the `chat.agent` runtime in #3543 needs. Builds on the Sessions primitive shipped in #3417. ## Design The Sessions list + detail routes mirror the run inspector pattern. `TaskTriggerSource` gains `AGENT` and `SCHEDULED` values, persisted on `BackgroundWorker.taskKind` and `TaskRun.taskKind` (plus a matching Clickhouse column), so the runs list can filter by kind. New `@trigger.dev/core` modules — `sessionStreams`, `inputStreams`, a `sessionStreamInstance` for realtime streams, and the `realtime-streams-api` / `session-streams-api` surfaces — expose the typed shapes that chat.agent will use to drive `session.out`. `ChatChunkTooLargeError` lets the runtime drop oversized chunks with a typed surface instead of failing the run. `s2Append` retries transient failures with exponential backoff. `/api/v[12]/packets/*` is exempt from customer rate limits so chat snapshot reads and writes don't get throttled under load. ## Stack Part of a 4-PR stack. Merge bottom-up. 1. **This PR** (#3542) → `main` 2. #3543 → #3542 — `chat.agent` runtime + browser transport 3. #3545 → #3543 — agent-view dashboard 4. #3546 → #3545 — ai-chat reference + MCP tooling Replaces #3173 (closed). <!-- GitButler Footer Boundary Top --> --- This is **part 5 of 5 in a stack** made with GitButler: - <kbd> 5 </kbd> #3612 - <kbd> 4 </kbd> #3546 - <kbd> 3 </kbd> #3545 - <kbd> 2 </kbd> #3543 - <kbd> 1 </kbd> #3542 👈 <!-- GitButler Footer Boundary Bottom -->
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.
ericallam
force-pushed
the
feature/chat-agent-runtime
branch
from
7690836 to
535245f
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: