Repository navigation
Conversation
github-actions
Bot
added
T-test
|
@bolinfest is on triage rotation this week and is the reviewer for this one. Automated classification did not run for this one, so it needs a full manual pass. Automated triage. Labels are a first pass and the assigned maintainer has the final say. |
| }; | ||
|
|
||
| let before: serde_json::Value = send_mismatch().await.json().await?; | ||
| assert_ne!( |
There was a problem hiding this comment.
I don't think the first request reaches the handler here, does it? From what I see it gets past the header check and then validate_required_protocol_meta rejects it with -32602 (no _meta). assert_ne! would also pass on a 500 so maybe we should assert 400 and -32602 here?
There was a problem hiding this comment.
Good catch, thanks! The -32602 error actually comes from validate_request_protocol_version_meta because _meta is missing from the 2026-07-28 request. validate_required_protocol_meta is disabled by default, but the effect is the same. I updated the test to check for a 400 status and -32602.
| let response = send_mismatch().await; | ||
| assert_eq!(response.status(), 400); | ||
| let body: serde_json::Value = response.json().await?; | ||
| assert_eq!(body["error"]["code"], -32020); |
There was a problem hiding this comment.
The body mentions two cases (a stale schema rejecting valid requests and a cached None skipping validation) but I think this test only covers the second one. I tried flipping annotated back to false at the end on this branch and I noticed main still returns -32020 there (the stale schema) and this branch doesn't, so it'd make a good regression test. Might be worth a second test that starts annotated?
There was a problem hiding this comment.
Good idea, @gocamille! Added skips_param_header_validation_once_tool_loses_annotation.
This branch has not been deployed
3 participants
Fixes #1348
Motivation and Context
For SEP-2243,
Mcp-Param-*validation now asksServerHandler::get_toolfor the tool's input schema on every request, instead of reading from a per-service cache that was never cleared.StreamableHttpServicecached each tool's schema by name for the entire lifetime of the service, includingNoneresults. When a server's tool list changed and it sentnotifications/tools/list_changed, header validation kept using the old schema. This caused two problems:x-mcp-headerannotation caused conforming clients to receive HTTP 400 /-32020.tools/callfor a name arrived before the tool existed, the cachedNonemeant itsMcp-Param-*headers were never checked again. An intermediary could then route or authorize a request based on a header value that the server never compares with the body.get_toolis already a synchronous lookup, so handlers that need caching can do it there and invalidate the cache correctly. A sharedHashMapin the transport can't do that, especially in stateless mode.How Has This Been Tested?
Add tests
Breaking Changes
None. SEP-2243 validation only applies to protocol versions
2026-07-28and later.Types of changes
Checklist