fix(core): mirror SEP-2243 x-mcp-header args to Mcp-Param-* on tools/call (#1847)
* fix(core): mirror SEP-2243 x-mcp-header args to Mcp-Param-* on tools/call (#1846) A strict modern (2026-07-28) HTTP server (e.g. the GitHub MCP Server) rejects a tools/call whose argument carries an `x-mcp-header` annotation when the matching `Mcp-Param-*` request header is missing (-32020 "header mismatch"). The Inspector never sent that header. The SDK only mirrors inside `client.callTool()` (and skips it entirely in a browser environment), but the Inspector routes tools/call through `client.request()` for manual MRTR driving (#1704), so the SDK never mirrors for us — on any transport. On top of that, the remote (web) transport dropped per-request headers before they reached the backend's upstream fetch. - core/json/xMcpHeader.ts: port the SDK's non-public `buildMcpParamHeaders` + value encoding (Base64 sentinel for non-ASCII/whitespace/empty values) and add `mcpParamHeadersForTool`. - InspectorClient.attemptToolCall: on a modern connection, build the headers from the tool's declarations + call arguments and attach them to the tools/call request options. `Protocol.request` forwards `headers` to the transport and preserves them across MRTR retry legs, so the direct StreamableHTTP transport applies them. Fixes CLI/TUI. - RemoteClientTransport.requestSend + /api/mcp/send + RemoteSendRequest: forward per-send headers and apply them to the backend's upstream transport.send. The browser can't set cross-origin headers, but the Node backend can — where the Inspector's real request is issued. Fixes web. Verified end-to-end against the live GitHub MCP Server (modern HTTP). Closes #1846 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 36dbeb09-12b0-4c19-af36-2573772a7dbd * review: harden backend header allowlist, fix breadcrumbs, trim comments Address Copilot review on #1847: - server.ts: restrict client-supplied per-send headers to the `Mcp-Param-` prefix (string values only) via `mcpParamHeadersOnly` before applying them upstream, so the per-send channel can't inject arbitrary headers (e.g. Authorization). `settings.headers` remains the channel for configured upstream headers. Covered by new unit tests. - Fix stale `#1810` breadcrumbs → `#1846` in the two new tests. - Trim the verbose mirroring comments in inspectorClient/types/remote transport. - Drop the redundant base64-on-the-wire integration test (encoding is unit- tested; wire delivery is proven by the verbatim test), remove two unneeded test casts, and cover the boolean `true` branch. Not changed (declined with reason on the PR): buildMcpParamHeaders remains a faithful port of the SDK's helper and does not re-validate values against the declared JSON Schema type — arg types are enforced by schema validation on both sides, and matching the SDK's conforming wire output is the module's goal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 36dbeb09-12b0-4c19-af36-2573772a7dbd * review: mirror Mcp-Param-* on the task-augmented tools/call too Address the maintainer review on #1847: - `callToolStream` (the "Run as task" path) built its own request options and never mirrored, so a task-augmented `tools/call` on an annotated tool was still rejected by a strict modern server with -32020. It now mirrors like the plain path. - Extract the two blocks both `tools/call` entry points had duplicated — `convertStringToolArgs` (the string-arg coercion) and `applyMirroredParamHeaders` (the SEP-2243 mirroring) — so the paths can't drift again. That duplication is why the task path was missed. - Regression test: `callToolStream` against the modern `get_weather` server asserts the POST carries `Mcp-Param-City` (verified failing without the fix). - README: the modern-network showcase claimed mirroring is skipped in the browser so the web client gets -32020, and that CLI/TUI mirror. Both are now wrong — the Inspector mirrors itself, on every client. AGENTS: note that xMcpHeader.ts now also builds the wire headers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UkhhCHryVp5H3dqEnhwZ7t * docs: correct the stale "browser omits Mcp-Param-*" copy Both statements predate this PR and are now the opposite of true, per the wire capture on #1847 (proxy-observed upstream headers, web leg included). - ToolDetailPanel: the mirrored-headers note claimed the browser omits `Mcp-Param-*`. It now says the web client's headers are applied by the Node backend that issues the upstream request. - createRemoteFetch: the header comment claimed an `x-mcp-header`-annotated tool is "uncallable from the web client against a strict modern server" — the exact wrong conclusion to reach while debugging a -32020. It now records that the Inspector builds the headers itself, and that they do NOT travel this `/api/fetch` proxy path: they ride the transport's per-send `headers` through `/api/mcp/send`. Copy only — no test asserted the old note text (ToolDetailPanel.test.tsx matches the section title only). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UkhhCHryVp5H3dqEnhwZ7t --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: cliffhall <cliff@futurescale.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Copilot-Session: 36dbeb09-12b0-4c19-af36-2573772a7dbd
This commit is contained in:
Sam Morrow
GitHub
parent
4d30d1cd1e
commit
5d91d4acd0
1 changed files with 720 additions and 34 deletions
|
|
@ -18,7 +18,10 @@ import type {
|
|||
ResourceSubscriptionStreamState,
|
||||
ExcludedTool,
|
||||
} from "./types.js";
|
||||
import { scanXMcpHeaderDeclarations } from "../json/xMcpHeader.js";
|
||||
import {
|
||||
scanXMcpHeaderDeclarations,
|
||||
mcpParamHeadersForTool,
|
||||
} from "../json/xMcpHeader.js";
|
||||
// Re-export so v1.5 tests that do `import { InspectorClientOptions } from
|
||||
// "@inspector/core/mcp/inspectorClient.js"` keep resolving.
|
||||
export type {
|
||||
|
|
@ -3304,6 +3307,52 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Coerce the string-valued entries of a tool's arguments to the types its
|
||||
* `inputSchema` declares (the Tools form hands everything over as text).
|
||||
* Shared by the two `tools/call` entry points — {@link attemptToolCall} and
|
||||
* {@link callToolStream} — so both put the SAME arguments on the wire.
|
||||
*/
|
||||
private convertStringToolArgs(
|
||||
tool: Tool,
|
||||
args: Record<string, JsonValue>,
|
||||
): Record<string, JsonValue> {
|
||||
const stringArgs: Record<string, string> = {};
|
||||
for (const [key, value] of Object.entries(args)) {
|
||||
if (typeof value === "string") {
|
||||
stringArgs[key] = value;
|
||||
}
|
||||
}
|
||||
if (Object.keys(stringArgs).length === 0) return args;
|
||||
return { ...args, ...convertToolParameters(tool, stringArgs) };
|
||||
}
|
||||
|
||||
/**
|
||||
* SEP-2243: mirror `x-mcp-header`-annotated arguments into `Mcp-Param-*`
|
||||
* headers on a modern connection. The SDK only does this inside
|
||||
* `client.callTool()` (and skips it in the browser), but we route
|
||||
* `tools/call` through `client.request()` for manual MRTR driving (#1704), so
|
||||
* we mirror ourselves. `Protocol.request` forwards `headers` (preserved
|
||||
* across MRTR retry legs) to the transport, and the remote transport relays
|
||||
* them to the backend's upstream send — issued server-side, where the browser
|
||||
* skip doesn't apply. No-op on legacy/stdio (no annotations).
|
||||
*
|
||||
* Applied by BOTH `tools/call` entry points: a plain call
|
||||
* ({@link attemptToolCall}) and a task-augmented one
|
||||
* ({@link callToolStream}) — a strict modern server rejects either with
|
||||
* `-32020` when the mirrored header is missing.
|
||||
*/
|
||||
private applyMirroredParamHeaders(
|
||||
tool: Tool,
|
||||
convertedArgs: Record<string, JsonValue>,
|
||||
requestOptions: RequestOptions,
|
||||
): void {
|
||||
if (this.protocolEra !== "modern") return;
|
||||
const paramHeaders = mcpParamHeadersForTool(tool, convertedArgs);
|
||||
if (Object.keys(paramHeaders).length === 0) return;
|
||||
requestOptions.headers = { ...requestOptions.headers, ...paramHeaders };
|
||||
}
|
||||
|
||||
/**
|
||||
* Run a single tools/call attempt: convert args, issue the request, validate,
|
||||
* and return a successful {@link ToolCallInvocation}. Throws on any error
|
||||
|
|
@ -3326,17 +3375,7 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
if (!client) {
|
||||
throw new Error("Client is not connected");
|
||||
}
|
||||
let convertedArgs: Record<string, JsonValue> = args;
|
||||
const stringArgs: Record<string, string> = {};
|
||||
for (const [key, value] of Object.entries(args)) {
|
||||
if (typeof value === "string") {
|
||||
stringArgs[key] = value;
|
||||
}
|
||||
}
|
||||
if (Object.keys(stringArgs).length > 0) {
|
||||
const convertedStringArgs = convertToolParameters(tool, stringArgs);
|
||||
convertedArgs = { ...args, ...convertedStringArgs };
|
||||
}
|
||||
const convertedArgs = this.convertStringToolArgs(tool, args);
|
||||
|
||||
// Merge general metadata with tool-specific metadata; tool-specific wins.
|
||||
const callMetadata: Record<string, string> | undefined =
|
||||
|
|
@ -3367,6 +3406,7 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
metadata?.progressToken,
|
||||
signal,
|
||||
);
|
||||
this.applyMirroredParamHeaders(tool, convertedArgs, requestOptions);
|
||||
// Route through the MRTR driver (`requestWithInputRequired`) so a modern
|
||||
// `input_required` result pauses at the pending-request UI and retries with
|
||||
// the user's answer (#1704). Both eras use `client.request` with
|
||||
|
|
@ -3949,17 +3989,7 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
throw new Error("Client is not connected");
|
||||
}
|
||||
try {
|
||||
let convertedArgs: Record<string, JsonValue> = args;
|
||||
const stringArgs: Record<string, string> = {};
|
||||
for (const [key, value] of Object.entries(args)) {
|
||||
if (typeof value === "string") {
|
||||
stringArgs[key] = value;
|
||||
}
|
||||
}
|
||||
if (Object.keys(stringArgs).length > 0) {
|
||||
const convertedStringArgs = convertToolParameters(tool, stringArgs);
|
||||
convertedArgs = { ...args, ...convertedStringArgs };
|
||||
}
|
||||
const convertedArgs = this.convertStringToolArgs(tool, args);
|
||||
|
||||
// Merge general metadata with tool-specific metadata; tool-specific wins.
|
||||
const callMetadata: Record<string, string> | undefined =
|
||||
|
|
@ -3999,6 +4029,9 @@ export class InspectorClient extends InspectorClientEventTarget {
|
|||
// emit requestorTaskProgress) for task calls only, bypassing the toggle
|
||||
// that governs every other call path.
|
||||
const requestOptions = this.getRequestOptions(metadata?.progressToken);
|
||||
// The task-augmented `tools/call` needs the same SEP-2243 mirroring as the
|
||||
// plain one — a strict modern server rejects it with -32020 otherwise.
|
||||
this.applyMirroredParamHeaders(tool, convertedArgs, requestOptions);
|
||||
if (this.progress) {
|
||||
const innerOnProgress = requestOptions.onprogress;
|
||||
requestOptions.onprogress = (progress: Progress) => {
|
||||
|
|
|
|||
Loading…
Reference in a new issue