fix(sdk): watch-mode chat subscriptions survive quiet windows - #4548
fix(sdk): watch-mode chat subscriptions survive quiet windows#4548kathiekiwi wants to merge 14 commits into
Conversation
🦋 Changeset detectedLatest commit: 8c72806 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 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 Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughWatch mode now reconnects after completed turns and idle-window EOFs. It continues beyond the normal EOF resubscribe limit. The subscription stops when the session is settled or the operation is aborted. Tests cover reconnect behavior, settled-session termination, aborts during backoff, and updated SSE fixtures. A patch changeset documents the SDK change. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
… side effect (TRI-13070)
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
…to fix/watch-mode-keepalive-tri-13065
…to fix/watch-mode-keepalive-tri-13065
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| return this.subscribeToSessionStream(state, abortSignal, options.chatId, { | ||
| resumed: true, | ||
| sendStopOnAbort: !!options.abortSignal, | ||
| sendStopOnAbort: options.stopOnAbort ?? false, |
There was a problem hiding this comment.
📝 Info: stopOnAbort default flip mostly affects direct callers, not the AI SDK
ChatTransport.reconnectToStream in the AI SDK is invoked with { chatId } & ChatRequestOptions, which carries headers/body/metadata but no abortSignal, so !!options.abortSignal was already falsy on the AI-SDK-driven resume path. The behavioural change is therefore limited to code that calls reconnectToStream directly with a signal — those callers now need stopOnAbort: true to preserve the old "abort stops the turn" behaviour. No such caller exists in this repo (only tests), but external/embedding apps built on this SDK will silently lose the stop-on-unmount side effect.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // Watch mode is a standing subscription: it outlives turn-complete | ||
| // (which clears `isStreaming`) and idle windows EOF by design, so the | ||
| // give-up budget doesn't apply. Only abort or a settled session ends it. | ||
| while ( | ||
| state.isStreaming && | ||
| (this.watchMode || (state.isStreaming && eofResubscribes < MAX_EOF_RESUBSCRIBES)) && | ||
| !currentSubscription?.sessionSettled && | ||
| !combinedSignal.aborted && | ||
| eofResubscribes < MAX_EOF_RESUBSCRIBES | ||
| !combinedSignal.aborted |
There was a problem hiding this comment.
🔍 Watch subscription keeps running but persisted state says not streaming, so a remount can't resubscribe
In watch mode a turn-complete still sets state.isStreaming = false and persists it (packages/trigger-sdk/src/v3/chat.ts:2000-2001) while the loop keeps reconnecting. reconnectToStream bails early when state.isStreaming === false (packages/trigger-sdk/src/v3/chat.ts:1154), so once the first turn completes, any remount of the watch component (StrictMode double-mount, route re-entry, tab restore) can no longer open a subscription — nothing ever flips isStreaming back to true for a passive viewer. Pre-existing, but this PR makes the standing watch subscription the supported flow, so the resume-after-remount gap becomes more visible.
Was this helpful? React with 👍 or 👎 to provide feedback.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| // Watch mode is a standing subscription: it outlives turn-complete | ||
| // (which clears `isStreaming`) and idle windows EOF by design, so the | ||
| // give-up budget doesn't apply. Only abort or a settled session ends it. | ||
| while ( | ||
| state.isStreaming && | ||
| (this.watchMode || (state.isStreaming && eofResubscribes < MAX_EOF_RESUBSCRIBES)) && | ||
| !currentSubscription?.sessionSettled && | ||
| !combinedSignal.aborted && | ||
| eofResubscribes < MAX_EOF_RESUBSCRIBES | ||
| !combinedSignal.aborted | ||
| ) { |
There was a problem hiding this comment.
🔍 Watch mode's only documented clean exit (settled session) is unreachable in production
peekSettled: !this.watchMode means a watch-mode subscription never sends X-Peek-Settled: 1. On the server, X-Session-Settled: true is emitted only on the opt-in peek path (apps/webapp/app/services/realtime/s2realtimeStreams.server.ts:417-437, gated by the X-Peek-Settled header at apps/webapp/app/routes/realtime.v1.sessions.$session.$io.ts:184). Therefore currentSubscription.sessionSettled can never become true for a watch client in production, and the loop comment at packages/trigger-sdk/src/v3/chat.ts:1800-1802 ("Only abort or a settled session ends it") is only half true: abort is the sole exit. Practical consequence: a viewer opened on a chat whose session is long finished keeps re-issuing 120s long-polls forever (one standing server request per watcher) until the consumer aborts or cancels. The new tests exercise the settled exit only by synthesising the header in the mock, so they don't cover the production shape. Worth confirming this unbounded standing poll is the intended cost of the fix, or whether watch mode needs a separate terminal signal (e.g. session closed/expired).
Was this helpful? React with 👍 or 👎 to provide feedback.
…ate on give-up - reconnect no longer peek-settles in watch mode, so a settled peek between turns can't close the standing subscription before the next turn. - the returned stream now aborts its resubscribe loop when the reader is cancelled, instead of leaking it. - clear and persist isStreaming before the budget-exhaustion throw so a reload doesn't reopen a doomed subscription.
…stream-error A consumer cancelling the watch stream aborts the resubscribe loop, which reaches controller.close() on an already-closed controller. The resulting 'Invalid state' throw was surfaced as a bogus stream-error on every clean watch-viewer unmount. Wrap the remaining bare close sites to match the existing pattern.
| if ( | ||
| state.isStreaming && | ||
| !currentSubscription?.sessionSettled && | ||
| !combinedSignal.aborted | ||
| ) { | ||
| // Clear + persist before throwing so the surfaced error leaves | ||
| // consistent state — otherwise a reload sees isStreaming: true | ||
| // and reopens a doomed subscription. | ||
| state.isStreaming = false; | ||
| this.notifySessionChange(chatId, state); | ||
| throw new Error( | ||
| "Chat stream ended before the turn completed (reconnect budget exhausted)." | ||
| ); | ||
| } |
There was a problem hiding this comment.
🔍 Non-watch streams now hard-error after ~10 minutes of complete silence
The budget is 5 resubscribes and each window is streamTimeoutSeconds (default 120s, packages/trigger-sdk/src/v3/chat.ts:163), so a turn that emits no record at all for roughly 10 minutes now surfaces stream-error and errors the AI SDK stream instead of closing quietly — even though the agent may still be running. eofResubscribes is reset by any record (packages/trigger-sdk/src/v3/chat.ts:1930), so only fully silent stretches count; server keepalive comments do not produce records and therefore do not re-earn budget. Consider whether long tool calls / deep reasoning pauses can realistically exceed that window before this ships.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Two related fixes to the chat transport's watch/read-only subscription lifecycle.
TRI-13065 — In watch mode the chat stream died at the first long-poll window boundary after a turn completed: the EOF-reconnect path was gated on
isStreaming, which turn-complete clears, so a watcher stopped hearing later turns. Watch mode now keeps reconnecting across quiet windows and only stops on abort or a settled session. The bounded give-up budget still applies to normal (mid-turn) streams, but not to watch mode, where empty windows are expected.TRI-13070 —
reconnectToStreamderived mutation rights from mere signal presence (sendStopOnAbort: !!options.abortSignal), so a passive/read-only subscriber that passed an abortSignal would append a{kind:"stop"}to.inon unmount and could stop a turn it didn't own. Subscription lifecycle is not session ownership:reconnectToStreamnow takes an explicitstopOnAbortoption that defaults tofalse, and the owning turn paths (sendMessages,sendAction) passsendStopOnAbort: trueexplicitly. A read-only subscription ending never mutates the session; it still cancels its own request.