feat: add closeSSEStream callback to RequestHandlerExtra #1166
+241
−11
Conversation
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
9 tasks
commit: |
pcarleton
approved these changes
Nov 25, 2025
Comment on lines
+112
to
+114
| // Track transports by session ID for session reuse | ||
| const transports = new Map<string, StreamableHTTPServerTransport>(); | ||
|
|
Member
There was a problem hiding this comment.
Suggested change
| // Track transports by session ID for session reuse | |
| const transports = new Map<string, StreamableHTTPServerTransport>(); |
don't think you need this any more
Contributor
Author
There was a problem hiding this comment.
I thought so too at first, but we do still need this at line 120 below:
// Reuse existing transport or create new one
let transport = sessionId ? transports.get(sessionId) : undefined;
Address findleyr's feedback to decouple tool code from transport: - Add CloseSSEStreamOptions type for per-invocation retry intervals - Add closeSSEStream callback to MessageExtraInfo and RequestHandlerExtra - Callback only provided when eventStore is configured - Support custom retry interval via options.retryInterval - Update ssePollingExample to use the new callback API Tools can now close SSE streams directly via extra.closeSSEStream() without needing to track or access transports explicitly.
felixweinberger
force-pushed
the
fweinberger/closeSSEStream-callback
branch
from
da2e98c to
3a58ea1
Compare
findleyr
reviewed
Nov 25, 2025
| const stream = this._streamMapping.get(streamId); | ||
| if (stream) { | ||
| // If a custom retry interval is provided, send it before closing | ||
| if (retryInterval !== undefined) { |
There was a problem hiding this comment.
Doesn't this cause problems for the client?
If the "event" field is missing, the SSE spec says that it should be treated as a "message" event.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Adds
extra.closeSSEStreamcallback toRequestHandlerExtra, addressing findleyr's feedback on #1129.This decouples tool code from the transport - tools can now close SSE streams without needing to track or access transports explicitly.
Changes
CloseSSEStreamOptionstype with optionalretryIntervalfieldMessageExtraInfowith optionalcloseSSEStreamcallbackcloseSSEStreamtoRequestHandlerExtraeventStoreis configuredNew API
Dependencies
This PR depends on #1129 (SEP-1699 SSE polling) being merged first.
Test plan