Accept "type":"number" as a valid x-mcp-header primitive per SEP-2243#1647
Open
halter73 wants to merge 1 commit into
Open
Accept "type":"number" as a valid x-mcp-header primitive per SEP-2243#1647halter73 wants to merge 1 commit into
halter73 wants to merge 1 commit into
Conversation
Per the SEP-2243 spec text (specification/draft/server/tools.mdx, line 340): > MUST only be applied to parameters with primitive types (number, string, boolean) Our McpHeaderExtractor.ValidateToolSchema was rejecting tools whose x-mcp-header annotation targeted a property with "type":"number", contradicting the spec and causing the upstream conformance scenario `http-custom-headers` to fail (the schema declares `priority` and `float_val` as "type":"number" and expects them to be emitted as Mcp-Param-Priority / Mcp-Param-FloatVal headers). McpHeaderEncoder.ConvertToHeaderValue already handles JsonValueKind.Number by emitting element.GetRawText(), so number-typed values flow through the encoder unchanged — only the validator needed updating. Integer canonicalization (per the SEP's JavaScript safe integer rule) remains restricted to "type":"integer"; "type":"number" values are emitted using their raw JSON representation (e.g. 2.5, 3.14, -7), which is what the conformance suite expects. Updates: - Add "number" to IsAllowedPrimitiveTypeName. - Refresh doc comments + rejection message to match the spec wording. - Flip three previously-asserted-as-rejected test cases in McpHeaderExtractorValidationTests to assert acceptance, and add an ArrayTypeTool to retain a negative-case coverage point. - Add CallTool_NumberType_EmitsRawJsonNumberHeader in AddKnownToolsHeaderTests for end-to-end wire coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the MCP client’s x-mcp-header schema validation to treat JSON Schema "type": "number" as an allowed primitive (per SEP-2243), ensuring number-typed header parameters are not rejected and can be emitted as Mcp-Param-* headers.
Changes:
- Allow
"number"inMcpHeaderExtractor’s allowed primitive type set (and update related messaging/docs). - Update validation tests to accept number-typed
x-mcp-headerannotations and add a new negative case for non-primitive (array) types. - Add an end-to-end ASP.NET Core test asserting that
"type":"number"values are emitted as raw JSON numeric text in request headers.
Show a summary per file
| File | Description |
|---|---|
src/ModelContextProtocol.Core/Client/McpHeaderExtractor.cs |
Extends x-mcp-header validation to allow "number" and updates validation comments/error text accordingly. |
tests/ModelContextProtocol.Tests/Client/McpHeaderExtractorValidationTests.cs |
Flips number cases to acceptance and adds an array negative-case tool to preserve rejection coverage. |
tests/ModelContextProtocol.AspNetCore.Tests/AddKnownToolsHeaderTests.cs |
Adds an end-to-end test verifying raw JSON number header emission for "type":"number" parameters. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 1
Comment on lines
+407
to
+410
| Assert.NotNull(result); | ||
| var headers = _capturedHeaders.Values.First(); | ||
| Assert.Equal(expectedPriorityHeader, headers["Mcp-Param-Priority"]); | ||
| Assert.Equal(expectedRatioHeader, headers["Mcp-Param-Ratio"]); |
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
Accept
"type":"number"as a validx-mcp-headerprimitive per SEP-2243.Bug
The SEP-2243 spec (
docs/specification/draft/server/tools.mdx:340) defines the allowedx-mcp-headerprimitive types as:Our
McpHeaderExtractor.ValidateToolSchemawas rejecting tools whosex-mcp-headerannotation targeted a property with"type":"number", contradicting the spec. The upstream conformance scenariohttp-custom-headersdeclarespriorityandfloat_valas"type":"number"and expects them to be emitted asMcp-Param-Priority/Mcp-Param-FloatValheaders. Because the validator rejected those tools, the client never cached them, so when the test then called them by name the client sent the request with noMcp-Param-*headers at all — failing the conformance scenario.This was only visible after the
@modelcontextprotocol/conformancenpm pin is bumped past0.1.16(work being done on top ofmainin PR #1610) — on the currently-pinned suite the SEP-2243 scenarios are gated out.Fix
One-line behavioral change in
IsAllowedPrimitiveTypeName:McpHeaderEncoder.ConvertToHeaderValuealready handlesJsonValueKind.Numberby emittingelement.GetRawText(), so number-typed values flow through the encoder unchanged — only the validator needed updating. Integer canonicalization (per the SEP's JavaScript safe-integer rule) remains restricted to"type":"integer";"type":"number"values are emitted using their raw JSON representation (e.g.2.5,3.14,-7), which is what the conformance suite expects.Test changes
McpHeaderExtractorValidationTests:NumberTypeTool,NestedNumberTool,NumberUnionTool) flipped to assert acceptance.ArrayTypeToolretains a negative-case coverage point so the validator still rejects clearly-invalid non-primitive types.AddKnownToolsHeaderTests:CallTool_NumberType_EmitsRawJsonNumberHeadertheory drives an end-to-end Kestrel-backed scenario asserting integer and fractional"type":"number"values are emitted on the wire asMcp-Param-*headers using their raw JSON representation.Validation
dotnet test tests/ModelContextProtocol.Tests --filter "FullyQualifiedName~McpHeaderExtractorValidationTests"→ 12/12 passdotnet test tests/ModelContextProtocol.AspNetCore.Tests --filter "FullyQualifiedName~AddKnownToolsHeaderTests"→ 20/20 pass (including the 3 new theory rows)0.2.0-alpha.2pin bump tracked in PR Add draft protocol support: sessionless + handshake-less (SEP-2575 + SEP-2567) #1610)Related
Surfaced while validating PR #1610 against the upstream
@modelcontextprotocol/conformancecompat/conformance-draftsuite. The bug exists onmainbut is masked there because the suite version pinned onmain(0.1.16, March 2026) predates the SEP-2243 scenarios. This PR fixes it directly onmainso the fix is independent of and lands before PR #1610.