🔌 refactor: Drive the Resumable Stream Through the ChatTransport - #16378
Merged
Merged
Conversation
Add reconnectToStream to ChatTransport and the sse.js transport: a GET attach with request headers, a token refresh on every 401, a cancel the caller did not issue reported as a dropped connection (status 0), and a closed flag for the foreground check. useResumableSSE now consumes normalized ChatEvents instead of opening sse.js and parsing frames itself; resume, steer and queue bookkeeping are unchanged.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Refresh the token once per attachment and report a second 401, so the hook's reconnect budget bounds a stream that keeps answering 401. A server-written error event that is not JSON keeps its raw text instead of reading as a dropped connection.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The resumable stream is the path every non-Assistants chat takes, but
useResumableSSEstill openedsse.jsitself and parsed frames inline, so the app's real streaming path bypassed theChatTransportthat #16335 and #16336 introduced. This routes it through the transport, behaviour-preserving, so stream state can move out of the hooks next.ChatTransportgainsreconnectToStream, the counterpart of the AI SDK method of that name, implemented on thesse.jstransport. It attaches with a GET, refreshes the token once per attachment on a 401 (a second 401 goes to the hook's bounded reconnect ladder), reports a cancel the caller did not issue (a backgrounded or frozen tab) as a dropped connection with status 0, and exposesclosedfor the foreground reattach check.useResumableSSEnow consumes normalizedChatEvents. Resume cursors, job ids, steer and queue bookkeeping, abort and cleanup stay where they were, and the step handler is untouched. The hook shrinks by 48 lines.Type of change
Testing
Tested environments/configuration:
lcbackend and frontend on this branch, Anthropic endpoint (claude-sonnet-4-6)__killNetworkdropped the stream and it reattached withresume=true; the backend logged the disconnect and a resumed subscription one second later, and events kept arriving.pendingSteerson FINAL, converted to a queued turn and sent.Automated tests:
cd client && npx jest hooks/SSE data-provider/SSE: 16 suites, 482 tests pass (all existing specs unchanged, 13 new)reconnectToStreamtests insse.spec.ts: GET headers, resume snapshot framing, server error vs HTTP status, user-agent cancel, caller abort, body end, 401 refresh, second 401 and failed refresh, plain-text server error, abort during refreshcd client && npm run typecheck,cd packages/data-provider && npx tsc --noEmit: cleannpx eslint,npx prettier --check,node scripts/sort-imports.mts --checkon touched files: cleanScreenshots / recordings
No user-facing change
Risk / compatibility
No wire format change. The resume path previously refreshed the token on every 401 without a cap; it now refreshes once per attachment and lets the reconnect ladder bound the rest. A body that ends without a terminal frame is still only recovered on the next foreground, as before; that is tracked separately in berry-13#144.
Checklist