fix(sdk): watch-mode chat subscriptions survive quiet windows - #4548
fix(sdk): watch-mode chat subscriptions survive quiet windows#4548kathiekiwi wants to merge 24 commits into
Conversation
🦋 Changeset detectedLatest commit: 5810c80 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}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…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.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…at/agent-storybook-gallery
| cancel() { | ||
| internalAbort.abort(); | ||
| }, |
There was a problem hiding this comment.
🔍 Aborting an in-flight stream still deletes the successor's abort controller
subscribeToSessionStream's finally (packages/trigger-sdk/src/v3/chat.ts:2055-2059) calls this.activeStreams.delete(chatId) unconditionally, but the map may already hold a newer controller. sendMessages aborts the old stream and then synchronously registers the new one (packages/trigger-sdk/src/v3/chat.ts:867-880 → chat.ts:1638) with no await in between, so the aborted stream's teardown runs afterwards on a microtask and removes the live stream's entry. Consequences: a later sendMessages/stopGeneration finds no active stream to abort (two concurrent readers both writing state.lastEventId, the exact hazard called out at chat.ts:1254-1257), and reconnectToStream's activeStreams.has() guard stops protecting against a duplicate subscription. This is pre-existing, but the new cancel() hook (which aborts internalAbort from a consumer cancel) plus watch-mode subscriptions that now outlive every turn make the window much easier to hit. Guarding the delete with if (this.activeStreams.get(chatId) === internalAbort) would fix it.
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.
🔍 In a watch-mode transport, a stream opened by sendMessages now never closes after turn-complete
resumeAfterEof no longer stops on state.isStreaming when this.watchMode is set, and that flag is a property of the transport, not of the individual subscription. A watch-mode transport that also calls sendMessages/sendAction gets a stream that skips controller.close() on turn-complete (chat.ts:2009) and now also never closes at the following EOF — previously it closed within one long-poll window. For AI SDK consumers that means status stays "streaming" forever after the reply finished. Watch mode is documented as a read-only viewer mode (chat.ts:583-590), so this may be out of scope, but it is worth confirming that no consumer both watches and sends on the same transport instance.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
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.