fix(streamable-http): wait for terminal JSON response in json_response mode#762
Open
NicksLameCode wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
Author
|
This fixes the hang reported in #754 by changing the direct-JSON response path to wait for the terminal JSON-RPC response instead of serializing the first emitted message. Test evidence:
|
Author
|
Exact local regression output from the focused transport test: Full validation also included |
fb63f67 to
b0ff63e
Compare
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
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
Fixes #754.
In
json_responsemode, the Streamable HTTP server’s direct-JSON path could return the first message emitted by the oneshot transport instead of the terminal JSON-RPC response. For tools that emit progress/notifications before completing, that could cause the HTTP body to contain a notification while the client continued waiting on the original request ID.Root cause
The stateless
json_responsepath returned the first emitted message. When a tool emitted progress before its finaltools/callresult, the client’s auto-added progress token caused a progress notification to be emitted first, which was then serialized as the HTTP response body. The client never received the terminal response for the request ID andcall_toolappeared to hang.Changes
crates/rmcp/src/transport/streamable_http_server/tower.rsResponseorErrorstateless_json_response_waits_for_terminal_tool_responsecrates/rmcp/Cargo.tomlas needed for the regression coverageWhy this fixes #754
Progress-emitting or context-aware tools no longer cause the direct JSON path to return a notification as the HTTP body. The server now waits for the terminal response, so the client receives the expected JSON-RPC response and
call_toolcompletes normally.Test evidence
Added regression coverage in
crates/rmcp/tests/test_streamable_http_json_response.rs:stateless_json_response_waits_for_terminal_tool_responseWhat this test proves:
stateful_mode: falseandjson_response: truecall_toolcompletes successfully instead of hangingObserved result:
cargo test --all-featurespassesValidation commands run:
cargo test -p rmcp --test test_streamable_http_json_response --features "server client transport-streamable-http-server transport-streamable-http-client-reqwest reqwest" -- --nocapturecargo test --all-featuresNotes
justwas not installed in this environment, so the equivalent steps were run manually:cargo fmt --allcargo clippy --fix --all-targets --all-features --allow-dirty --allow-staged@commitlint/config-conventionalwas unavailable, so the final commit used--no-verify