feat: add closeSSEStream callback to RequestHandlerExtra #1166
+147
−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: |
felixweinberger
force-pushed
the
fweinberger/closeSSEStream-callback
branch
from
da2e98c to
3a58ea1
Compare
findleyr
reviewed
Nov 25, 2025
src/server/streamableHttp.ts
Outdated
| 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.
Contributor
|
What do you think about putting all these |
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.
Address review feedback: sending `retry: N\n\n` could trigger SSE event dispatch behavior in some client implementations. Using a single `\n` ensures the retry field is processed without potentially dispatching an empty event.
Remove the optional retryInterval parameter from closeSSEStream callback for simplicity. The transport's default retry interval (from priming events) is sufficient. This aligns with Python SDK.
felixweinberger
force-pushed
the
fweinberger/closeSSEStream-callback
branch
from
a37d53d to
47d0ad1
Compare
Contributor
Author
Probably a good idea 🤔 wary of refactoring right now for backwards compatibility, but could be something to put on the list for v2 what do you think? |
6 tasks
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