feat(tracing): per-header verdicts for the mirrored Mcp-* headers (SEP-2243) - #3574
Conversation
…P-2243)
The MCP-headers block listed each mirrored header with its family
("routing", "protocol version") — a category restating what the header
name already says. It never showed the one thing 2026-07-28 added that a
reader cannot see for themselves: whether the header still agrees with
the body it was copied from, which is the entire point of SEP-2243 (a
router may forward on the header while the server executes on the body).
Same rows, same density; the third slot now carries the verdict:
mcp-method server/discover ✓ matches body
mcp-name get_weather ✕ body says get_forecast → -32020
mcp-name — not required here
New SDK export `evaluateMcpHeaders` computes it, era-gated exactly like
`findMcpHeaderIssues` (which now derives from it, output unchanged): no
verdict before 2026-07-28, where nothing is mirrored, and none for
`Mcp-Param-*`, whose body values are not captured. An absent header gets
an explicit row because "missing, required" and "correctly absent" are
otherwise indistinguishable — `Mcp-Name` is required only for
tools/call, resources/read and prompts/get.
Two subtractions come with it: the mirrored headers are filtered out of
the raw request-header map, which was printing mcp-method twice on one
screen, and the standing "-32020 HeaderMismatch" banner is gone — the
code now lands on the row that actually failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dosubot
Bot
added
the
size:L
GitHub Actions
Inactive
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_6f97b9fa-5d89-48c6-a6a7-83b5fc550731) |
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c2e36e11d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // A sentinel that will not decode is only a defect where the spec defines | ||
| // the sentinel at all — a legacy value that merely resembles it is not. | ||
| const undecodable = crossCheck && !!decoded.decodeError; |
There was a problem hiding this comment.
When a modern request contains an Mcp-Param-* value with a malformed base64 sentinel, this condition marks the row undecodable; HttpExchangeDetails then presents it as a definite -32020 failure. Parameter arguments and their x-mcp-header annotations are not captured, so this API cannot validate these rows and promises to report them as unchecked; restrict the undecodable verdict to the standard cross-checked families.
Useful? React with 👍 / 👎.
| } else if (!spec.required && body?.method !== undefined) { | ||
| // Absent AND not required. Said out loud, because a blank row reads | ||
| // identically to the `missing` case above. | ||
| out.push({ name: spec.header, family: spec.family, status: "not-required" }); |
There was a problem hiding this comment.
For tasks/get, tasks/update, and tasks/cancel, the repository's SEP-2663 transport explicitly requires Mcp-Name: <taskId>, but deriveMirroredBodyValues does not capture params.taskId and these methods are absent from NAME_REQUIRED_METHODS. Consequently, a captured task request missing Mcp-Name reaches this branch and is displayed as “not required here,” concealing the exact routing defect the tracing verdict is meant to expose; capture the task ID and treat these routed task methods as requiring the header.
Useful? React with 👍 / 👎.
Internal previewPreview URL: https://mcp-inspector-pr-3574.up.railway.app |
WalkthroughThe SDK adds Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
sdk/src/mcp-client-manager/http-exchange-log.tsOops! Something went wrong! :( ESLint: 8.57.1 YAMLException: Cannot read config file: /sdk/eslint.config.mjs 7 | ...tseslint.configs.recommended, sdk/src/mcp-client-manager/mcp-header-mirror.tsOops! Something went wrong! :( ESLint: 8.57.1 YAMLException: Cannot read config file: /sdk/eslint.config.mjs 7 | ...tseslint.configs.recommended, sdk/src/mcp-client-manager/transport-utils.tsOops! Something went wrong! :( ESLint: 8.57.1 YAMLException: Cannot read config file: /sdk/eslint.config.mjs 7 | ...tseslint.configs.recommended,
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
sdk/tests/mcp-header-mirror.test.ts (1)
402-480: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe version-scope block is thorough; one uncovered corner remains.
Every era path is pinned here, but nothing exercises a modern
tools/callwhose captured body lacksnamewhile the header is absent — the case where the evaluator emits no row at all (see the note onevaluateMcpHeaderslines 263-278). Pinning whatever behavior you settle on would stop that silence from drifting unnoticed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/tests/mcp-header-mirror.test.ts` around lines 402 - 480, Extend the “version scope” tests around evaluateMcpHeaders with a modern tools/call request whose captured body omits name and whose corresponding header is absent. Assert the intended behavior for this case, specifically whether the evaluator should emit a missing/not-required row or no row, so the current silent outcome is explicitly pinned.sdk/src/mcp-client-manager/mcp-header-mirror.ts (1)
263-278: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueA required-but-absent header with no captured body value vanishes from the list entirely.
The doc above promises that an absent header always earns an explicit row, yet when
spec.requiredholds andspec.bodyValueisundefined(saytools/callcaptured withoutparams.name), neither branch fires and the row is dropped. Silence there reads the same as "header not applicable" — the very ambiguity this evaluator exists to dispel. Anuncheckedrow would keep the promise.♻️ Optional: emit an
uncheckedrow instead of nothingif (!found) { if (spec.required && spec.bodyValue !== undefined) { out.push({ name: spec.header, family: spec.family, status: "missing", bodyValue: spec.bodyValue, bodyField: spec.bodyField, }); } else if (!spec.required && body?.method !== undefined) { // Absent AND not required. Said out loud, because a blank row reads // identically to the `missing` case above. out.push({ name: spec.header, family: spec.family, status: "not-required" }); + } else if (spec.required) { + // Required, absent, and no body value to judge it against. + out.push({ name: spec.header, family: spec.family, status: "unchecked" }); } continue; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdk/src/mcp-client-manager/mcp-header-mirror.ts` around lines 263 - 278, Update the absent-header handling in the mirror-building logic so every required missing header emits a row, including when spec.bodyValue is undefined. Preserve the existing missing row when a body value is available, and emit an unchecked status row otherwise; keep the not-required branch unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/src/mcp-client-manager/mcp-header-mirror.ts`:
- Around line 208-220: Update evaluateMcpHeaders to decode the selected
MCP-Protocol-Version value before calculating isModern and crossCheck. Apply the
existing sentinel-decoding utility once to the header/body-resolved version,
then use the decoded value for protocol validation and all cross-checks so
matching encapsulated versions follow the modern path.
---
Nitpick comments:
In `@sdk/src/mcp-client-manager/mcp-header-mirror.ts`:
- Around line 263-278: Update the absent-header handling in the mirror-building
logic so every required missing header emits a row, including when
spec.bodyValue is undefined. Preserve the existing missing row when a body value
is available, and emit an unchecked status row otherwise; keep the not-required
branch unchanged.
In `@sdk/tests/mcp-header-mirror.test.ts`:
- Around line 402-480: Extend the “version scope” tests around
evaluateMcpHeaders with a modern tools/call request whose captured body omits
name and whose corresponding header is absent. Assert the intended behavior for
this case, specifically whether the evaluator should emit a missing/not-required
row or no row, so the current silent outcome is explicitly pinned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e35d8fb8-3605-4717-a382-15fa259a1b10
📒 Files selected for processing (5)
.changeset/tracing-header-verdicts.mdmcpjam-inspector/client/src/components/tracing/HttpExchangeDetails.tsxsdk/src/browser.tssdk/src/mcp-client-manager/mcp-header-mirror.tssdk/tests/mcp-header-mirror.test.ts
| export function evaluateMcpHeaders( | ||
| headers: Record<string, string>, | ||
| body: MirroredBodyValues | undefined | ||
| ): McpHeaderIssue[] { | ||
| ): McpHeaderAssessment[] { | ||
| const lookup = new Map<string, { name: string; value: string }>(); | ||
| for (const [name, value] of Object.entries(headers)) { | ||
| lookup.set(name.toLowerCase(), { name, value }); | ||
| } | ||
|
|
||
| const versionHeader = lookup.get("mcp-protocol-version")?.value; | ||
| const version = versionHeader ?? body?.protocolVersion; | ||
| const version = lookup.get("mcp-protocol-version")?.value ?? body?.protocolVersion; | ||
| const isModern = | ||
| !!version && isKnownProtocolVersion(version) && isStatelessProtocolVersion(version); | ||
| if (!isModern || !body) return []; | ||
| const crossCheck = isModern && !!body; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
SEP-2243 Mcp-Protocol-Version header base64 sentinel encoding allowed
💡 Result:
Under SEP-2243, which standardized HTTP headers for the Model Context Protocol (MCP) Streamable HTTP transport, Base64 sentinel encoding is explicitly allowed and required for certain header values [1][2]. The encoding uses the format =?base64?{Base64EncodedValue}?= [1][3]. This format must be used for values in Mcp-Name and Mcp-Param-{Name} headers if the value contains non-ASCII characters, control characters, or leading/trailing whitespace that cannot be safely represented as plain ASCII [2][3][4]. Crucially, to avoid ambiguity, clients must also apply this Base64 encoding to any plain-ASCII value that happens to match the sentinel pattern (i.e., starts with =?base64? and ends with ?=) [1][5][3]. The sentinel markers are case-sensitive and must appear exactly as shown in lowercase [1][2][3]. Servers and intermediaries are required to decode these values before performing comparisons or policy enforcement [1][3][4].
Citations:
- 1: https://modelcontextprotocol.io/seps/2243-http-standardization
- 2: https://mcp.mintlify.app/seps/2243-http-standardization
- 3: https://modelcontextprotocol.io/specification/2026-07-28/basic/transports/streamable-http
- 4: https://docs.quarkiverse.io/quarkus-mcp-server/dev/concepts-transports.html
- 5: docs: SEP-2243 clarifications for x-mcp-header modelcontextprotocol/modelcontextprotocol#2772
🌐 Web query:
SEP-2243 Mcp-Protocol-Version header value base64 sentinel encoding
💡 Result:
SEP-2243 (HTTP Header Standardization for Streamable HTTP Transport) introduces a requirement for Base64 sentinel encoding for MCP HTTP header values that cannot be represented as plain ASCII [1][2]. When a value (such as Mcp-Name or Mcp-Param-{Name}) contains non-ASCII characters, control characters, or has leading/trailing whitespace, clients must encode the UTF-8 representation of that value using the following format [2][3]: =?base64?{Base64EncodedValue}?= The key requirements for this encoding are: 1. Sentinel Markers: The prefix =?base64? and suffix ?= are case-sensitive and must be provided exactly as lowercase [2][3]. 2. Ambiguity Prevention: To avoid ambiguity, clients must also apply this Base64 sentinel encoding to any plain-ASCII value that happens to match this specific pattern (i.e., starts with =?base64? and ends with ?= [2][3]). 3. Decoding: Servers and intermediaries must decode these values before performing any validation or inspection (such as comparing the header value to the corresponding request body value) [2][3]. This encoding is part of the broader standard for MCP HTTP header standardization, which enables infrastructure like proxies and load balancers to inspect request metadata without parsing the JSON-RPC body [1][4]. The protocol uses version gating, where header enforcement—including these encoding rules—applies when the MCP-Protocol-Version header indicates a version that requires header–body validation (e.g., DRAFT-2026-v1 or later) [4][2].
Citations:
- 1: https://modelcontextprotocol.io/seps/2243-http-standardization
- 2: https://modelcontextprotocol.io/specification/2026-07-28/basic/transports/streamable-http
- 3: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/02dd8f61/docs/seps/2243-http-standardization.mdx
- 4: Implementation for SEP-2243 Http Standardization modelcontextprotocol/csharp-sdk#1553
🌐 Web query:
site:github.com/modelcontextprotocol modelcontextprotocol docs/seps/2243 Mcp-Protocol-Version
💡 Result:
SEP-2243 (HTTP Header Standardization) is a finalized specification for the Model Context Protocol (MCP) that enhances the Streamable HTTP transport by mirroring key JSON-RPC metadata into standard HTTP headers [1][2]. This allows network intermediaries—such as load balancers, proxies, and observability tools—to route, trace, and prioritize MCP traffic without performing deep packet inspection of the request body [1][3]. Key technical aspects include: 1. Protocol Versioning and Gating: The SEP introduces a "version-gating" mechanism for intermediaries [4]. When the MCP-Protocol-Version header indicates a version that requires header-body validation (such as DRAFT-2026-v1), intermediaries can reliably enforce policy or routing decisions based on the mirrored headers [4][3]. If a client sends an unsupported or invalid MCP-Protocol-Version header, the server must respond with a 400 Bad Request error [5]. 2. Header Mirroring: Clients are required to include specific headers on all Streamable HTTP POST requests, including Mcp-Method (corresponding to the JSON-RPC method field) and Mcp-Name (corresponding to params.name) [1][2]. 3. Dynamic Parameter Mirroring: The specification defines an x-mcp-header annotation for tool parameters [1][4]. When a tool parameter is so annotated, clients must mirror the parameter's value into a custom Mcp-Param-{Name} HTTP header [1][3]. 4. Implementation and Validation: - Implementations are expected to handle encoding (e.g., Base64 wrapping for non-ASCII characters) and validation of these headers [2][3]. - Validation failures (e.g., mismatch between a header and the actual JSON-RPC body) result in a specific JSON-RPC error, McpErrorCode.HeaderMismatch (code -32001) [3]. - Clarifications for this SEP, such as those addressed in pull request #2772, refined the syntax for header names to align with RFC 9110 and clarified that header filtering/validation is scoped specifically to the Streamable HTTP transport [4][6]. The Mcp-Protocol-Version header itself is a standard requirement for HTTP-based MCP communication, serving as the primary mechanism for clients and servers to negotiate and declare the protocol version being used for each request [5][7]. Every request must declare the protocol version in its _meta field, which should align with the value provided in the MCP-Protocol-Version HTTP header [7][8].
Citations:
- 1: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/02dd8f61/seps/2243-http-standardization.md
- 2: SEP-2243 HTTP Standardization modelcontextprotocol/modelcontextprotocol#2243
- 3: Implementation for SEP-2243 Http Standardization modelcontextprotocol/csharp-sdk#1553
- 4: docs: SEP-2243 clarifications for x-mcp-header modelcontextprotocol/modelcontextprotocol#2772
- 5: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/04c6e1f0ea6544c7df307fb2d7c637efe34f58d3/docs/specification/draft/basic/transports.mdx
- 6: SEP-2243 Clarifications modelcontextprotocol/modelcontextprotocol#2762
- 7: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/02dd8f61/docs/specification/draft/basic/versioning.mdx
- 8: https://github.com/modelcontextprotocol/python-sdk/blob/main/src/mcp/types/_types.py
🌐 Web query:
MCP Protocol Version header Mcp-Protocol-Version date sentinel base64
💡 Result:
The Model Context Protocol (MCP) utilizes the Mcp-Protocol-Version header to manage protocol versioning, specifically when using HTTP-based transports [1][2]. Mcp-Protocol-Version Header The Mcp-Protocol-Version header is a required HTTP header in requests sent to an MCP server [2][3]. It allows the server to identify and respond according to the specific MCP protocol version being used [3][4]. Key aspects include: - Negotiation: The version specified in this header should match the version negotiated during the initialization phase [3][5]. - Validation: Servers must reject requests with an invalid or unsupported protocol version with a 400 Bad Request error [3][4]. - Synchronization: The header value must match the io.modelcontextprotocol/protocolVersion field provided in the request body's _meta field [2]. - Backward Compatibility: Servers may assume a default version (such as 2025-03-26) if the header is absent and no other versioning information is available, though modern specifications increasingly require explicit versioning [2][3][4]. Base64 Encoding In the context of the MCP protocol, base64 encoding is primarily used for the transmission of binary data, such as images or other non-textual resources [6][7]. When a resource contains binary data, it is typically represented as a base64-encoded string within the blob field or data field of the content object [6][8][9]. The specification uses the @format byte annotation in its schema definitions to indicate that these strings contain base64-encoded binary data [8][10].
Citations:
- 1: https://modelcontextprotocol.io/specification/2026-07-28/basic/versioning
- 2: https://mcp-staging.mintlify.app/specification/draft/basic/transports/streamable-http
- 3: https://modelcontextprotocol.io/specification/2025-06-18/basic/transports
- 4: https://modelcontextprotocol.io/specification/2025-11-25/basic/transports
- 5: https://mcp.mintlify.app/specification/2025-11-25/basic/transports
- 6: https://modelcontextprotocol.io/specification/2026-07-28/server/tools
- 7: https://github.com/modelcontextprotocol/docs/blob/573dc60c/docs/concepts/resources.mdx
- 8: https://github.com/modelcontextprotocol/specification/blob/main/schema/2024-11-05/schema.ts
- 9: https://modelcontextprotocol.io/specification/2024-11-05/server/resources
- 10: https://github.com/modelcontextprotocol/typescript-sdk/blob/7d7e62cc/packages/core/src/types/spec.types.ts
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate relevant files:"
fd -a 'mcp-header-mirror.ts|.*specs|.*spec' . \
| sed 's#^\./##' \
| head -80
echo
echo "Candidate file outline:"
ast-grep outline sdk/src/mcp-client-manager/mcp-header-mirror.ts --view expanded || true
echo
echo "Relevant code slices:"
sed -n '1,260p' sdk/src/mcp-client-manager/mcp-header-mirror.ts | cat -n
echo
echo "Search for sentinel decode helpers/usages:"
rg -n "base64|sentinel|decode|HeaderMismatch|isKnownProtocolVersion|Mcp-Protocol-Version|MCP-Protocol-Version|protocolVersion" sdk/src -SRepository: MCPJam/inspector
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '260,380p' sdk/src/mcp-client-manager/mcp-header-mirror.ts | cat -n
echo
echo "Protocol version guard implementation:"
sed -n '1,180p' sdk/src/mcp-client-manager/mcp-protocol-version.ts | cat -n
echo
echo "Behavioral probe for sentinel-encoded protocol version:"
node - <<'JS'
const MCP_HEADER_SENTINEL_PREFIX = "=?base64?";
const MCP_HEADER_SENTINEL_SUFFIX = "?=";
function base64ToUtf8(encoded) {
const binary = atob(encoded);
const bytes = new Uint8Array(binary.length);
for (let i = 0; i < binary.length; i += 1) bytes[i] = binary.charCodeAt(i);
return new TextDecoder("utf-8", { fatal: true }).decode(bytes);
}
function isKnownProtocolVersion(v) {
return /^(\d{4}-\d{2}-\d{2}|2026-01-26|2026-02-26)$/.test(v ?? "");
}
function isStatelessProtocolVersion(v) {
return !(v && (v.startsWith("2026-03-26") || v.startsWith("2026-06-26")));
}
function currentProbe(lookup, body) {
const version = lookup.get("mcp-protocol-version")?.value ?? body?.protocolVersion;
return { version, isModern: !!version && isKnownProtocolVersion(version) && isStatelessProtocolVersion(version) };
}
function decodedProbe(lookup, body) {
const raw = lookup.get("mcp-protocol-version")?.value ?? body?.protocolVersion;
const decoded = raw && raw.startsWith(MCP_HEADER_SENTINEL_PREFIX) && raw.endsWith(MCP_HEADER_SENTINEL_SUFFIX)
? base64ToUtf8(raw.slice(MCP_HEADER_SENTINEL_PREFIX.length, raw.length - MCP_HEADER_SENTINEL_SUFFIX.length))
: raw;
return { decoded, isModern: !!decoded && isKnownProtocolVersion(decoded) && isStatelessProtocolVersion(decoded) };
}
const raw = Buffer.from("2026-01-26", "utf8").toString("base64");
console.log(currentProbe(new Map([["mcp-protocol-version", `=?base64?${raw}?=`]]]), {}));
console.log(decodedProbe(new Map([["mcp-protocol-version", `=?base64?${raw}?=`]]]), {}));
JSRepository: MCPJam/inspector
Length of output: 8729
Decode MCP-Protocol-Version before the modern/gating check.
SEP-2243 applies sentinel decoding across MCP HTTP header values before validation. A sentinel-encapsulated version like =?base64?MjAyNi0wMS0yNg==?=, even when matching the body, is treated as unknown and sends the flow back to unchecked rows. Decode once and use that value for both the protocol-mode probe and cross-checks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdk/src/mcp-client-manager/mcp-header-mirror.ts` around lines 208 - 220,
Update evaluateMcpHeaders to decode the selected MCP-Protocol-Version value
before calculating isModern and crossCheck. Apply the existing sentinel-decoding
utility once to the header/body-resolved version, then use the decoded value for
protocol validation and all cross-checks so matching encapsulated versions
follow the modern path.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…dable verdict
Two review findings on the per-header verdicts, both real.
SEP-2663 ("Streamable HTTP: Routing Headers") makes `Mcp-Name: <taskId>`
a MUST for tasks/get, tasks/update and tasks/cancel, and
`wrapFetchForTaskRouting` already sends it — but the required-name set
covered only the three SEP-2243 core methods, so a routed task request
that omitted the header rendered as "not required here". The new row
turned a silent omission into an affirmative wrong claim about exactly
the routing defect it exists to expose. `TASK_ROUTED_METHODS` moves to
`mcp-header-mirror` (the pure leaf module) and is now read by both the
send side and the judge side so the two cannot drift, and
`deriveMirroredBodyValues` reads `params.taskId` for those three methods
— and no others, so a method merely carrying a taskId is not
cross-checked against a field it never mirrored.
The `undecodable` verdict also applied to any mirrored family on a
modern request, so a sentinel-looking `Mcp-Session-Id` or `Last-Event-ID`
rendered as `✕ does not decode → -32020`. Neither header has an encoded
form in any version. It now applies to `Mcp-Param-*` only, where servers
MUST reject a recognized param header carrying invalid characters.
Declined: decoding `MCP-Protocol-Version` through the sentinel before the
era probe. The spec defines the sentinel for `Mcp-Name` and
`Mcp-Param-{Name}` only; a protocol version is a plain date token, so
accepting an encoded one would invent a wire form.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
dosubot
Bot
added
size:XL
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f561af81-bfa3-474d-a2c8-e8221f6ae9fe) |
Review triage — 3 findings after dedupeCodex and cubic reported the same two issues; CodeRabbit added a third. Both P2s were real and are fixed in 643aed4. 1.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdk/src/mcp-client-manager/http-exchange-log.ts`:
- Around line 94-106: The name selection in the HTTP exchange logging flow must
prioritize routedTaskId for methods in TASK_ROUTED_METHODS, preventing
params.name or params.uri from overriding the task ID used for Mcp-Name
validation. Update the name expression near routedTaskId while preserving
existing fallback behavior for non-routed methods, and add a regression test
covering a routed request containing both taskId and name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9a76229f-7333-4fcd-a25e-b34b48a813a1
📒 Files selected for processing (6)
.changeset/tracing-header-verdicts.mdsdk/src/mcp-client-manager/http-exchange-log.tssdk/src/mcp-client-manager/mcp-header-mirror.tssdk/src/mcp-client-manager/transport-utils.tssdk/tests/http-exchange-log.test.tssdk/tests/mcp-header-mirror.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/tests/mcp-header-mirror.test.ts
| taskId?: unknown; | ||
| _meta?: Record<string, unknown>; | ||
| }; | ||
| }; | ||
| const name = params?.name ?? params?.uri; | ||
| // `params.taskId` is the `Mcp-Name` source for the SEP-2663 routed task | ||
| // methods and for nothing else, so it is read only for those — otherwise a | ||
| // method that merely carries a taskId would be cross-checked against the | ||
| // wrong field. | ||
| const routedTaskId = | ||
| typeof method === "string" && TASK_ROUTED_METHODS.has(method) | ||
| ? params?.taskId | ||
| : undefined; | ||
| const name = params?.name ?? params?.uri ?? routedTaskId; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make taskId authoritative for routed methods.
transport-utils.ts injects Mcp-Name from params.taskId, and mcp-header-mirror.ts validates it against the task ID. However, this expression lets params.name or params.uri override routedTaskId, so a routed request containing both fields can be reported as a false mismatch.
- const name = params?.name ?? params?.uri ?? routedTaskId;
+ const name =
+ typeof method === "string" && TASK_ROUTED_METHODS.has(method)
+ ? routedTaskId
+ : params?.name ?? params?.uri;Add a regression test containing both taskId and name.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| taskId?: unknown; | |
| _meta?: Record<string, unknown>; | |
| }; | |
| }; | |
| const name = params?.name ?? params?.uri; | |
| // `params.taskId` is the `Mcp-Name` source for the SEP-2663 routed task | |
| // methods and for nothing else, so it is read only for those — otherwise a | |
| // method that merely carries a taskId would be cross-checked against the | |
| // wrong field. | |
| const routedTaskId = | |
| typeof method === "string" && TASK_ROUTED_METHODS.has(method) | |
| ? params?.taskId | |
| : undefined; | |
| const name = params?.name ?? params?.uri ?? routedTaskId; | |
| taskId?: unknown; | |
| _meta?: Record<string, unknown>; | |
| }; | |
| }; | |
| // `params.taskId` is the `Mcp-Name` source for the SEP-2663 routed task | |
| // methods and for nothing else, so it is read only for those — otherwise a | |
| // method that merely carries a taskId would be cross-checked against the | |
| // wrong field. | |
| const routedTaskId = | |
| typeof method === "string" && TASK_ROUTED_METHODS.has(method) | |
| ? params?.taskId | |
| : undefined; | |
| const name = | |
| typeof method === "string" && TASK_ROUTED_METHODS.has(method) | |
| ? routedTaskId | |
| : params?.name ?? params?.uri; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sdk/src/mcp-client-manager/http-exchange-log.ts` around lines 94 - 106, The
name selection in the HTTP exchange logging flow must prioritize routedTaskId
for methods in TASK_ROUTED_METHODS, preventing params.name or params.uri from
overriding the task ID used for Mcp-Name validation. Update the name expression
near routedTaskId while preserving existing fallback behavior for non-routed
methods, and add a regression test covering a routed request containing both
taskId and name.
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="sdk/src/mcp-client-manager/http-exchange-log.ts">
<violation number="1" location="sdk/src/mcp-client-manager/http-exchange-log.ts:106">
P2: For SEP-2663 routed task methods (tasks/get, tasks/update, tasks/cancel), `routedTaskId` should be the authoritative source for `Mcp-Name`, but the current fallback chain `params?.name ?? params?.uri ?? routedTaskId` lets an incidental `params.name` or `params.uri` field take precedence over the actual task id. If a routed request happens to carry both, the derived body value used for cross-checking will be wrong, producing a false mismatch verdict against the correctly-sent `Mcp-Name` header. Consider making the routed-task case exclusive: `typeof method === "string" && TASK_ROUTED_METHODS.has(method) ? routedTaskId : (params?.name ?? params?.uri)`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| typeof method === "string" && TASK_ROUTED_METHODS.has(method) | ||
| ? params?.taskId | ||
| : undefined; | ||
| const name = params?.name ?? params?.uri ?? routedTaskId; |
There was a problem hiding this comment.
P2: For SEP-2663 routed task methods (tasks/get, tasks/update, tasks/cancel), routedTaskId should be the authoritative source for Mcp-Name, but the current fallback chain params?.name ?? params?.uri ?? routedTaskId lets an incidental params.name or params.uri field take precedence over the actual task id. If a routed request happens to carry both, the derived body value used for cross-checking will be wrong, producing a false mismatch verdict against the correctly-sent Mcp-Name header. Consider making the routed-task case exclusive: typeof method === "string" && TASK_ROUTED_METHODS.has(method) ? routedTaskId : (params?.name ?? params?.uri).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At sdk/src/mcp-client-manager/http-exchange-log.ts, line 106:
<comment>For SEP-2663 routed task methods (tasks/get, tasks/update, tasks/cancel), `routedTaskId` should be the authoritative source for `Mcp-Name`, but the current fallback chain `params?.name ?? params?.uri ?? routedTaskId` lets an incidental `params.name` or `params.uri` field take precedence over the actual task id. If a routed request happens to carry both, the derived body value used for cross-checking will be wrong, producing a false mismatch verdict against the correctly-sent `Mcp-Name` header. Consider making the routed-task case exclusive: `typeof method === "string" && TASK_ROUTED_METHODS.has(method) ? routedTaskId : (params?.name ?? params?.uri)`.</comment>
<file context>
@@ -88,10 +91,19 @@ export function deriveMirroredBodyValues(
+ typeof method === "string" && TASK_ROUTED_METHODS.has(method)
+ ? params?.taskId
+ : undefined;
+ const name = params?.name ?? params?.uri ?? routedTaskId;
const protocolVersion = params?._meta?.[PROTOCOL_VERSION_META_KEY];
const derived: MirroredBodyValues = {
</file context>
| const name = params?.name ?? params?.uri ?? routedTaskId; | |
| const name = | |
| typeof method === "string" && TASK_ROUTED_METHODS.has(method) | |
| ? routedTaskId | |
| : params?.name ?? params?.uri; |
1 participant
Why
The MCP-headers block in Tracing lists each mirrored header with its family —
routing,protocol version— which restates what the header name already says.FAMILY_LABELeven mapped bothmethodandnameto the same string, so it never distinguished rows.What it never showed is the one thing
2026-07-28actually added: whether the header still agrees with the body it was copied from. That's the whole point of SEP-2243 — per the spec's Server Validation section, a load balancer may route on the header while the server executes on the body, so the disagreement is the defect. Today the body value only surfaces once something is already broken, which means the normal case gives you no way to tell it's right.What changes
Same rows, same density. Only the third slot changes:
New SDK export
evaluateMcpHeaderscomputes the per-row verdict.findMcpHeaderIssuesnow derives from it; its output shape and version scoping are unchanged (all 12 of its existing tests pass untouched).Two things fall out, both subtractions:
mcp-methodandmcp-protocol-versiontwice on one screen;-32020 HeaderMismatchbanner is gone — it explained the error class whether or not one occurred. The code now lands on the row that failed.Version scope
Era-gated identically to the existing validator:
2026-07-28standard three≤ 2025-11-25unchecked— nothing is mirrored before draft, soMcp-Method/Mcp-Nameare never demanded, and a value merely resembling the base64 sentinel is not a defectMcp-Param-*unchecked— the captured body carries no arguments, so no verdict is reachableMcp-Session-Id/Last-Event-IDAn absent header gets an explicit row, because "required and missing" and "correctly absent" are otherwise indistinguishable: per the Standard Request Headers table,
Mcp-Nameis required only fortools/call,resources/readandprompts/get. On theserver/discoverrequest that motivated this, the honest answer is not required here — not a blank.decodedis now set only when a sentinel decodes successfully, so an undecodable value no longer renders asraw → raw(a failed decode that looks like it round-tripped).Not in scope
Mcp-Param-*verification needs the tool'sinputSchemax-mcp-headerannotations plus the spec's three-row omit/include table. Shown asuncheckedrather than implying a check that didn't run.42.0==42). Only reachable once params are verified, since the standard three are all strings.MCP-Session-Idon the initialize response, and the collector still only runs over request headers.Verification
sdk/tests/mcp-header-mirror.test.ts— 31 pass (12 pre-existing + 19 new: match/mismatch/missing/not-required/undecodable,params.urisource forresources/read, sentinel decode, and five version-scope cases incl. "no undecodable defect on legacy" and "no body captured")sdk/tests/http-exchange-log.test.ts— 9 passreview-surface-snapshots+rpc-log-bus— 12 passtsc --noEmitclean for both@mcpjam/sdkand the client🤖 Generated with Claude Code
Note
Medium Risk
Ships a minor SDK API plus a behavior change: missing
Mcp-Nameon routed task RPCs now surfaces as defects where validation was previously silent; impact is limited to protocol debugging/tracing paths and is covered by new tests.Overview
Adds
evaluateMcpHeadersto@mcpjam/sdk/browser, returning a per-header verdict (match, mismatch, missing, not-required, undecodable, unchecked) with optional body field/value context—so Tracing can show whether each mirrored header agrees with the captured body, not only a defect list when something breaks.The Tracing HTTP exchange detail switches from
findMcpHeaderIssues+ family labels to row-level verdict text (including explicit rows for absent optional vs requiredMcp-Name), drops the global-32020banner, and hides mirrored headers from the raw “Other request headers” JSON to avoid duplicates.findMcpHeaderIssuesis refactored to filter defects fromevaluateMcpHeaders; shape and era gating stay the same, butMcp-Nameis now required fortasks/get,tasks/update, andtasks/cancel(SEP-2663), withTASK_ROUTED_METHODSshared between send (transport-utils) and judge logic.deriveMirroredBodyValuesmapsparams.taskIdto the mirrored name for those methods only.Reviewed by Cursor Bugbot for commit 643aed4. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds per-header verdicts for mirrored
Mcp-*request headers (SEP-2243) and updates the tracing inspector to show if each header matches the body. Also requiresMcp-Namefor routed task methods (SEP-2663) and limits undecodable verdicts toMcp-Param-*.New Features
evaluateMcpHeadersin@mcpjam/sdk/browser; inspector shows a verdict per header and removes the global HeaderMismatch banner. Mirrored headers are hidden from “Other request headers.”resources/readcomparesMcp-Nameto.params.uri; routed tasks use.params.taskId.Bug Fixes
Mcp-Nameis required fortasks/get,tasks/update, andtasks/cancel(SEP-2663);findMcpHeaderIssuesnow reports a missing defect for these. Send and judge shareTASK_ROUTED_METHODS;deriveMirroredBodyValuesreads.params.taskIdfor those methods.undecodableapplies only toMcp-Param-*; session/resumption headers are never decoded. Legacy eras and uncaptured bodies returnunchecked.Written for commit 643aed4. Summary will update on new commits.