fix(server): replay request-scoped terminal response after closeSSEStream#2153
Open
adityasingh2400 wants to merge 1 commit into
Open
Conversation
…tream After closeSSEStream() removes the active stream, send() only stored the event while a controller was attached, so the terminal response produced during the reconnect window was never written to the event store and could not be replayed. With all responses ready and no live stream, send() then threw "No connection established for request ID". Store request-scoped SSE events whenever an event store is configured (regardless of controller presence) and write to the controller only when one is attached, mirroring the standalone GET SSE path. When the stream is gone but the response is persisted for replay, clean up the request mappings and return instead of throwing; keep throwing when no event store can deliver the response. Fixes modelcontextprotocol#2151
|
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
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.
Fixes #2151.
Problem
closeSSEStream(requestId)is meant to support the SEP-1699 flow where the server disconnects a request-scoped POST SSE stream, keeps working, and replays missed events after the client reconnects. Onmainthis breaks when the terminal response is produced while the stream is disconnected:closeSSEStreamremoves the stream from_streamMappingbut keeps_requestToStreamMapping.send(), request-scoped events are only stored insideif (!this._enableJsonResponse && stream?.controller && stream?.encoder), so once the controller is gone the result/error is never written to the event store.send()throwsNo connection established for request ID: ....So the documented "close SSE, reconnect, replay final result" path is not reliable for request-scoped POST SSE streams. The standalone GET SSE path already does the opposite (stores while disconnected, returns without throwing when no controller), so the two paths were asymmetric.
Fix
Mirror the standalone GET SSE behavior for request-scoped responses:
No behavior change when no event store is configured.
Testing
Added
should replay the terminal response after closeSSE and reconnecttopackages/middleware/node/test/streamableHttp.test.ts(the file that previously only asserted the stream closes, not that the result is replayable). It callsctx.http?.closeSSE?.(), completes the tool while disconnected, reconnects withLast-Event-ID, and asserts the terminaltools/callresult is replayed. The test fails onmain(reconnect never receives the result) and passes with this change.pnpm --filter @modelcontextprotocol/server test(65 passed) andpnpm --filter @modelcontextprotocol/node testfor the streamableHttp suite (75 passed) are green; typecheck and lint pass.