fix(sep-2243): reject x-mcp-header on number-typed params - #444
Open
halter73 wants to merge 1 commit into
Open
Conversation
SEP-2243 permits `x-mcp-header` only on integer, string and boolean parameters — "Parameters with type `number` are not permitted" — but the harness never verified the `number` case and its requirement text still described `number` as permitted. PR modelcontextprotocol#371 fixed the positive `http-custom-headers` fixture (priority is now `integer`, `float_val` is served unannotated) but left the two remaining items from modelcontextprotocol#344: - `HttpInvalidToolHeadersScenario` serves invalid tools for every other x-mcp-header constraint but had no number-typed case, so nothing checked that a client excludes such a tool. - `sep-2243.yaml` claimed primitive types are "(number, string, or boolean)" and that "number values" get decimal-string encoding, both of which contradict the released 2026-07-28 spec. Add an `invalid_number_header` tool mapped to the existing `sep-2243-x-mcp-header-primitive-only` requirement (no new check IDs) and correct both requirement texts. traceability.json is updated in place since it embeds the yaml text verbatim; check IDs and statuses are unchanged. Verified against typescript-sdk@main: the new check fails there because the SDK deliberately allow-lists `number` to satisfy the old fixture — a workaround its own source notes as "tracked upstream" and can now drop. The positive scenario still passes 18/18. Closes modelcontextprotocol#344 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
Closes #344
Why this is still open after #371
SEP-2243 permits
x-mcp-headeronly on integer, string and boolean parameters. The released2026-07-28spec is explicit:and the value-encoding table in
basic/transports/streamable-http.mdxlists onlystring/integer/boolean.#371 fixed the positive fixture (
priorityis nowinteger,float_valis served unannotated and asserted not mirrored) but left the two items #344 listed as optional, which is why the issue stayed open. This finishes them.Changes
1. Negative case for the
numberrule.HttpInvalidToolHeadersScenarioserved invalid tools for every otherx-mcp-headerconstraint — empty / object / array / null / duplicate / space / colon / non-ascii / control-char — but had no number-typed one.sep-2243-x-mcp-header-primitive-onlywas only ever exercised via object/array/null, so nothing verified the rule the issue was actually about. Adds aninvalid_number_headertool withscore: { type: 'number', 'x-mcp-header': 'Score' }, mapped to the existing requirement.No check IDs added or removed —
INVALID_TOOL_DECLARED_CHECK_IDSis unchanged.2. Requirement text corrected.
sep-2243.yamlstill described the permitted set as "(number, string, or boolean)" and said "number values MUST be converted to their decimal string representation" — both contradict the released spec.traceability.jsonembeds this text verbatim, so its twotextfields are updated in place; check IDs andstatusare untouched.Verification
npm run typecheck,npm run lint,npm run buildclean;npm test505 passed / 43 files.INVALID_TOOL_CONSTRAINT_IDSmapping makes the new test fail (expected [ 'SUCCESS', 'SUCCESS', 'SUCCESS' ] to include 'FAILURE'), so it isn't vacuous.http-custom-headersstill 18/18.The new check finds a real SDK bug
http-invalid-tool-headersagainst typescript-sdk@main goes 11/12 —sep-2243-x-mcp-header-primitive-onlyfails oninvalid_number_header. The cause is in the SDK, not the harness:numberwas allow-listed there specifically to tolerate this fixture bug, with a comment noting the discrepancy is "tracked upstream." With #371 having fixed the fixture, that workaround can be dropped. Worth a follow-up on the SDK side.This won't red-light CI:
ci.ymlruns onlynpm ci/check/build/test. The typescript-sdk run lives intraceability.yml, which is manual/scheduled, uses|| true, and per AGENTS.md is not a PR gate.Out of scope
#354 (positive scenario's
ttlMs: 0vs. SEP-2549 stale-schema) is a separate defect owned by #358. The positive scenario's TTL is untouched here, and #358's scope note confirms it doesn't modify the invalid-tool scenario — so these shouldn't collide beyond a trivial rebase.