-
-
Notifications
You must be signed in to change notification settings - Fork 260
feat(tracing): per-header verdicts for the mirrored Mcp-* headers (SEP-2243) #3574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| --- | ||
| "@mcpjam/sdk": minor | ||
| --- | ||
|
|
||
| Add `evaluateMcpHeaders` (exported from `@mcpjam/sdk/browser`) — per-header | ||
| verdicts for the SEP-2243 mirrored `Mcp-*` headers, alongside the existing | ||
| defect-list form `findMcpHeaderIssues`. | ||
|
|
||
| A defect list answers "what is broken"; a debugger also has to answer "is this | ||
| right", which needs a row per header carrying the body field it was checked | ||
| against. Two cases only a verdict list can express: a conforming header (no | ||
| defect, but nothing said either) and an ABSENT one — `Mcp-Name` is required | ||
| only for `tools/call`, `resources/read`, `prompts/get` and the SEP-2663 routed | ||
| task methods, so a blank cannot distinguish a `-32020` from correct behavior. | ||
|
|
||
| Era-gated identically: before `2026-07-28` nothing is mirrored, so every header | ||
| comes back `unchecked` rather than judged by rules its version never had. A | ||
| `Mcp-Param-*` value cannot be cross-checked either — the captured body values | ||
| carry no arguments — though a malformed base64 sentinel in one is still | ||
| reported, since servers MUST reject a recognized `Mcp-Param-{Name}` carrying | ||
| invalid characters. `Mcp-Session-Id` / `Last-Event-ID` have no encoded form in | ||
| any version and are never judged. | ||
|
|
||
| Two behavior changes to shipped code: | ||
|
|
||
| - `Mcp-Name` is now required for `tasks/get`, `tasks/update` and `tasks/cancel` | ||
| (SEP-2663 "Streamable HTTP: Routing Headers" makes it a MUST, and | ||
| `wrapFetchForTaskRouting` already sends it) — so `findMcpHeaderIssues` reports | ||
| a `missing` defect for a routed task request that omits it, where it was | ||
| previously silent. `TASK_ROUTED_METHODS` now lives with the header logic and | ||
| is read by both the send and judge halves so they cannot drift. | ||
| - `deriveMirroredBodyValues` reads `params.taskId` as the `Mcp-Name` source for | ||
| those three methods, and for no others. | ||
|
|
||
| `findMcpHeaderIssues` otherwise derives from the same evaluation; its output | ||
| shape and version scoping are unchanged. |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,7 +19,10 @@ | |||||||||||
| * so the cross-check can run later without retaining the body. | ||||||||||||
| */ | ||||||||||||
|
|
||||||||||||
| import type { MirroredBodyValues } from "./mcp-header-mirror.js"; | ||||||||||||
| import { | ||||||||||||
| TASK_ROUTED_METHODS, | ||||||||||||
| type MirroredBodyValues, | ||||||||||||
| } from "./mcp-header-mirror.js"; | ||||||||||||
|
|
||||||||||||
| /** `_meta` key carrying the per-request protocol version in the modern era. */ | ||||||||||||
| const PROTOCOL_VERSION_META_KEY = "io.modelcontextprotocol/protocolVersion"; | ||||||||||||
|
|
@@ -88,10 +91,19 @@ export function deriveMirroredBodyValues( | |||||||||||
| params?: { | ||||||||||||
| name?: unknown; | ||||||||||||
| uri?: unknown; | ||||||||||||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: For SEP-2663 routed task methods (tasks/get, tasks/update, tasks/cancel), Prompt for AI agents
Suggested change
|
||||||||||||
| const protocolVersion = params?._meta?.[PROTOCOL_VERSION_META_KEY]; | ||||||||||||
| const derived: MirroredBodyValues = { | ||||||||||||
| method: typeof method === "string" ? method : undefined, | ||||||||||||
|
|
||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make
taskIdauthoritative for routed methods.transport-utils.tsinjectsMcp-Namefromparams.taskId, andmcp-header-mirror.tsvalidates it against the task ID. However, this expression letsparams.nameorparams.urioverrideroutedTaskId, so a routed request containing both fields can be reported as a false mismatch.Add a regression test containing both
taskIdandname.📝 Committable suggestion
🤖 Prompt for AI Agents