fix: do not advertise 2024-11-05 from Streamable HTTP transport providers - #1088
Open
senor14 wants to merge 1 commit into
Open
fix: do not advertise 2024-11-05 from Streamable HTTP transport providers#1088senor14 wants to merge 1 commit into
senor14 wants to merge 1 commit into
Conversation
McpStreamableServerTransportProvider inherited the default protocolVersions() from McpServerTransportProviderBase, which includes 2024-11-05. Streamable HTTP was introduced in 2025-03-26, so a provider of this transport advertised a version whose transport it cannot serve. Override protocolVersions() on the interface rather than on the servlet implementation so every current and future Streamable HTTP provider is covered, mirroring the existing McpStatelessServerTransport default. Fixes modelcontextprotocolgh-750
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.
Fixes #750
McpStreamableServerTransportProviderinherits the defaultprotocolVersions()fromMcpServerTransportProviderBase, which includes2024-11-05. Streamable HTTP was introduced in2025-03-26, so a Streamable HTTP provider advertises a version whose transport it cannot serve — the symptom reported in #750 beingdoGetonHttpServletStreamableServerTransportProviderbeing unable to initialize a session without a prior POST.Following the direction agreed in the issue, this overrides
protocolVersions()on theMcpStreamableServerTransportProviderinterface rather than on the servlet implementation, so every current and future Streamable HTTP provider is covered. It mirrors the existingMcpStatelessServerTransportdefault, which already excludes2024-11-05for the same reason.HttpServletSseServerTransportProviderkeeps its2024-11-05-only override, and transports without the Streamable HTTP constraint (stdio) keep the full range.Behavioural consequence
A client that requests
2024-11-05against a Streamable HTTP server previously had that version echoed back, and then failed later in confusing ways. With this changeMcpAsyncServertakes theelsebranch and responds with the highest supported version (2025-11-25), logging the mismatch — which is what the spec comment in that method already prescribes. So the change turns a silent late failure into an explicit negotiation result at initialize time.Tests
Added
ServerTransportProtocolVersionsTestsasserting the Streamable HTTP and stateless defaults exclude2024-11-05while the base default keeps the full range, so a future transport can't silently regain the unsupported version../mvnw testresults:mcp-core: 358 tests, all passing (including the 3 new ones)mcp-test: the 109HttpServletStreamable*tests are unaffectedFor transparency, the full
mcp-testmodule currently has 1 failure and 12 errors (ToolInputValidationIntegrationTests,HttpClientStreamableHttpTransportTest) on my machine. I verified these are pre-existing by running the whole suite on a clean checkout with this change stashed — identical counts before and after, so they are unrelated to this PR.