fix(client): use one tool definition for mirroring and output validation

.changeset/tool-call-definition-snapshot.md

@@ -0,0 +1,5 @@
+---
+'@modelcontextprotocol/client': patch
+---
+
+Keep tool-call header mirroring and output validation on the same cached tool definition when a catalog refresh overlaps a call.

packages/client/src/client/client.ts

@@ -536,14 +536,13 @@ export class Client extends Protocol<ClientContext> {
     /**
      * The response-cache substrate. Owns the backing store, the per-method
      * eviction-generation counter, the user-supplied/default flag, and the
-     * stamp-memoized derived `name → Tool` / `name → output-validator`
+     * stamp-memoized derived `name → Tool` / `name → output-metadata`
      * indices — the cache-coordination state that used to live as separate
      * private fields here. The internal aggregating walk writes one entry per
      * list verb; `list_changed` evicts the matching method;
      * `_resetConnectionState` resets the lot. {@linkcode callTool}'s
-     * output-schema validation reads the derived `outputValidator` index (the
-     * substrate's first production caller); the stacked SEP-2243 PR wires
-     * `Mcp-Param-*` mirroring through `toolDefinition` on top.
+     * header mirroring and output-schema validation share the source Tool and
+     * compiled validator held together in the `outputValidator` index.
      */
     private readonly _cache: ClientResponseCache;
     private readonly _defaultCacheTtlMs: number;
@@ -1849,24 +1848,6 @@ export class Client extends Protocol<ClientContext> {
         }
     }
 
-    /**
-     * Resolve the SEP-2243 `x-mcp-header` declaration scan for a tool name.
-     *
-     * The caller-supplied `toolDefinition` escape hatch wins; otherwise the
-     * cached `tools/list` entry (via the cache's `toolDefinition`) is the
-     * source. Freshness is the response cache's lifecycle: `list_changed`
-     * evicts, otherwise the held schema is the best information available
-     * regardless of age, and a stale schema is recovered through the
-     * `HEADER_MISMATCH` → evict-refetch-retry path in {@linkcode callTool}.
-     * On a miss the call proceeds without `Mcp-Param-*` headers (the spec's
-     * "client SHOULD send without custom headers" guidance) and relies on the
-     * same recovery.
-     */
-    private async _resolveXMcpHeaderScan(name: string, override: Tool | undefined): Promise<XMcpHeaderScanResult | undefined> {
-        const tool = override ?? (await this._cache.toolDefinition(name));
-        return tool === undefined ? undefined : scanXMcpHeaderDeclarations(tool.inputSchema);
-    }
-
     /**
      * Reads the contents of a resource by URI.
      *
@@ -2343,25 +2324,21 @@ export class Client extends Protocol<ClientContext> {
         // of running SEP-2243 from a browser; pass `null` for the designated
         // argument or supply `options.toolDefinition` against a relaxed server.
         const mirroringActive = this.getProtocolEra() === 'modern' && detectProbeEnvironment() !== 'browser';
-        // Mirroring (and output-schema validation below) read the cached
-        // `tools/list` entry directly via `_cache.toolDefinition` /
-        // `_cache.outputValidator`. `callTool` never issues a `tools/list`
+        // Mirroring and output-schema validation share one cached tool
+        // definition per attempt, including across an asynchronous store read.
+        // `callTool` never issues a `tools/list`
         // itself — the cache is populated by the caller's own
         // {@linkcode listTools | listTools()} (which now auto-aggregates) and
         // by the `HEADER_MISMATCH` recovery path below. A cold cache means
         // the call proceeds without `Mcp-Param-*` headers (the spec's
         // "client SHOULD send without custom headers" guidance) and without
         // output-schema validation (the v1.x opportunistic behaviour, kept so
         // a legacy/stdio `callTool` still issues zero extra requests).
-        const buildSendOptions = async (): Promise<CallToolRequestOptions | undefined> => {
-            if (!mirroringActive) return options;
-            // A custom store's `get()` may reject (the documented store
-            // contract); route that to `onerror` and degrade to sending
-            // without `Mcp-Param-*` headers — same posture as a cold cache —
-            // rather than aborting the call before it reaches the wire.
+        const buildSendOptions = (): CallToolRequestOptions | undefined => {
+            if (!mirroringActive || metadata === undefined) return options;
             let scan: XMcpHeaderScanResult | undefined;
             try {
-                scan = await this._resolveXMcpHeaderScan(params.name, options?.toolDefinition);
+                scan = scanXMcpHeaderDeclarations(metadata.tool.inputSchema);
             } catch (error) {
                 this._reportStoreError(error);
             }
@@ -2378,13 +2355,18 @@ export class Client extends Protocol<ClientContext> {
         // derived views must agree — and is compiled in isolation (never written to the cache). The
         // cache read is guarded: a custom store whose `get()` rejects routes to `onerror` and
         // degrades to skipping validation (same outcome as a cold cache).
-        let compiled =
+        // Memoize the source definition together with its compiled validator:
+        // a concurrent catalog refresh must not mix one generation's output
+        // schema with another generation's header declarations.
+        const resolveMetadata = () =>
             options?.toolDefinition === undefined
-                ? await this._cache
-                      .outputValidator(params.name, tool => this._compileOutputValidator(tool))
+                ? this._cache
+                      .outputValidator(params.name, tool => ({ tool, compiled: this._compileOutputValidator(tool) }))
                       .catch(error => void this._reportStoreError(error))
-                : this._compileOutputValidator(options.toolDefinition);
+                : { tool: options.toolDefinition, compiled: this._compileOutputValidator(options.toolDefinition) };
+        let metadata = await resolveMetadata();
         const assertCompiled = (): void => {
+            const compiled = metadata?.compiled;
             if (compiled === undefined || compiled.ok) return;
             const err = compiled.compileError;
             const message = (err instanceof Error ? err.message : String(err)).slice(0, 200);
@@ -2397,7 +2379,7 @@ export class Client extends Protocol<ClientContext> {
         // map there is no wider union to narrow away.
         let result: CallToolResult;
         try {
-            result = await this.request({ method: 'tools/call', params }, await buildSendOptions());
+            result = await this.request({ method: 'tools/call', params }, buildSendOptions());
         } catch (error) {
             // SEP-2243 one-refresh-on-miss: a `HEADER_MISMATCH` rejection on a
             // modern connection means the server enforced an `Mcp-Param-*`
@@ -2429,18 +2411,17 @@ export class Client extends Protocol<ClientContext> {
             // `HEADER_MISMATCH` — the refetch failure is observable only
             // through `onerror`.
             await this.listTools(undefined, refreshOptions).catch(error_ => this._reportStoreError(error_));
-            // Re-resolve the output validator against the freshly-fetched entry — the pre-flight
-            // `compiled` was resolved from the now-evicted cache and may be stale (different
-            // outputSchema) or absent (cold cache on the first attempt). The recovery path is only
+            // Re-resolve both metadata views against the freshly-fetched entry — the pre-flight
+            // snapshot came from the now-evicted cache and may be stale (different input/output
+            // schemas) or absent (cold cache on the first attempt). The recovery path is only
             // entered when `options.toolDefinition` is undefined, so the cache is the sole source.
             // Re-run the same fail-fast compile-error check before issuing the retry.
-            compiled = await this._cache
-                .outputValidator(params.name, tool => this._compileOutputValidator(tool))
-                .catch(error_ => void this._reportStoreError(error_));
+            metadata = await resolveMetadata();
             assertCompiled();
-            result = await this.request({ method: 'tools/call', params }, await buildSendOptions());
+            result = await this.request({ method: 'tools/call', params }, buildSendOptions());
         }
 
+        const compiled = metadata?.compiled;
         const validator = compiled !== undefined && compiled.ok ? compiled.validator : undefined;
         if (validator) {
             // If tool has outputSchema, it MUST return structuredContent (unless it's an error).

packages/client/src/client/responseCache.ts

@@ -291,7 +291,7 @@ export class ClientResponseCache {
      */
     private _toolIndex?: { stamp: number; byName: Map<string, Tool> };
     /**
-     * `name → compiled output-schema validator` derived from the cached
+     * `name → caller-defined output metadata` derived from the cached
      * `tools/list` entry; same stamp-keyed memoization as `_toolIndex`. Typed
      * `unknown` so this class stays free of any validator-provider dependency
      * — the compile callback supplied to {@linkcode outputValidator} owns the
@@ -605,8 +605,9 @@ export class ClientResponseCache {
      * Returns `undefined` only when no `tools/list` response is held at all,
      * or the held list does not contain `name`.
      *
-     * Consumed by `callTool()`'s SEP-2243 `_resolveXMcpHeaderScan` (mirroring)
-     * and, via {@linkcode outputValidator}, its output-schema validation.
+     * For consumers needing only the descriptor. `Client.callTool()` uses
+     * {@linkcode outputValidator} to retain the source Tool alongside its
+     * compiled output validator in one metadata snapshot.
      */
     async toolDefinition(name: string): Promise<Tool | undefined> {
         const entry = await this._probe('tools/list');
@@ -627,16 +628,17 @@ export class ClientResponseCache {
     }
 
     /**
-     * The compiled output-schema validator for tool `name`, derived from the
+     * Caller-defined output metadata for tool `name`, derived from the
      * cached `tools/list` entry — same source and same stamp-keyed
-     * memoization as {@linkcode toolDefinition}. The `name → validator` index
+     * memoization as {@linkcode toolDefinition}. The `name → metadata` index
      * re-derives only when the backing entry's stamp changes (a refetched
      * `tools/list` recompiles; a `list_changed` eviction drops it). Returns
-     * `undefined` when no `tools/list` is held, the tool is absent, or it has
-     * no `outputSchema`.
+     * `undefined` when no `tools/list` is held, the tool is absent, or the
+     * callback returns `undefined` for it.
      *
-     * `compile` is the caller-supplied validator-compile callback (the
-     * `Client` passes its `_jsonSchemaValidator` wrapper) so this
+     * `compile` is the caller-supplied callback. `Client` retains the source
+     * Tool alongside its compiled validator, including Tools without an
+     * `outputSchema`, so mirroring and validation share one snapshot. This
      * class carries no validator-provider dependency. One tool's uncompilable
      * `outputSchema` (e.g. an invalid `pattern` regex or unresolvable `$ref`)
      * must not poison every other tool's `callTool` — the callback isolates

packages/client/test/client/mcpParamMirroring.test.ts

@@ -12,7 +12,8 @@ import {
     encodeMcpParamValue,
     HEADER_MISMATCH_ERROR_CODE,
     InMemoryTransport,
-    PROTOCOL_VERSION_META_KEY
+    PROTOCOL_VERSION_META_KEY,
+    ProtocolErrorCode
 } from '@modelcontextprotocol/core-internal';
 import { describe, expect, it, vi } from 'vitest';
 
@@ -125,6 +126,107 @@ function modernClient(store?: InMemoryResponseCacheStore): Client {
 }
 
 describe('SEP-2243 Mcp-Param-* mirroring (modern era)', () => {
+    it.each(['New', 'Old'])(
+        'keeps mirroring and output validation on the same catalog generation during a refresh (%s result)',
+        async resultGeneration => {
+            const versionedTool = (generation: string): Tool => ({
+                name: 'route',
+                inputSchema: {
+                    type: 'object',
+                    properties: { region: { type: 'string', 'x-mcp-header': `${generation}-Region` } }
+                },
+                outputSchema: {
+                    type: 'object',
+                    properties: { generation: { const: generation } },
+                    required: ['generation']
+                }
+            });
+            const oldTool = versionedTool('Old');
+            const newTool = versionedTool('New');
+            const store = new InMemoryResponseCacheStore();
+            const { clientTx, serverTx, callHeaders, listCount } = await scriptedModernServer([[newTool]]);
+            const originalHandler = serverTx.onmessage!;
+            let executions = 0;
+            serverTx.onmessage = (message, extra) => {
+                const request = message as JSONRPCRequest;
+                if (request.method !== 'tools/call') {
+                    originalHandler(message, extra);
+                    return;
+                }
+                // A rejected stale header does not execute the handler. Choose the
+                // response generation to exercise validation after recovery.
+                if (callHeaders.at(-1)?.['Mcp-Param-New-Region'] !== 'eu') {
+                    void serverTx.send({
+                        jsonrpc: '2.0',
+                        id: request.id,
+                        error: { code: HEADER_MISMATCH_ERROR_CODE, message: 'stale header definition' }
+                    });
+                    return;
+                }
+                executions++;
+                void serverTx.send({
+                    jsonrpc: '2.0',
+                    id: request.id,
+                    result: { resultType: 'complete', content: [], structuredContent: { generation: resultGeneration } }
+                });
+            };
+            const client = modernClient(store);
+            await client.connect(clientTx);
+            store.set({ method: 'tools/list', partition: PART }, { value: JSON.stringify({ tools: [oldTool] }) });
+
+            // Pause one async store read after it captured the old entry, then
+            // complete a real listTools refresh before allowing that read to finish.
+            // No timers or private client/cache methods are involved.
+            const get = store.get.bind(store);
+            let captured!: () => void;
+            const readCaptured = new Promise<void>(resolve => {
+                captured = resolve;
+            });
+            let release!: () => void;
+            const readReleased = new Promise<void>(resolve => {
+                release = resolve;
+            });
+            let pause = true;
+            (store as ResponseCacheStore).get = key => {
+                const entry = get(key);
+                if (pause && key.method === 'tools/list' && key.partition === PART) {
+                    pause = false;
+                    captured();
+                    return readReleased.then(() => entry);
+                }
+                return entry;
+            };
+
+            try {
+                const outcome = client.callTool({ name: 'route', arguments: { region: 'eu' } }).then(
+                    result => ({ result }),
+                    error => ({ error })
+                );
+                await readCaptured;
+                await client.listTools(undefined, { cacheMode: 'refresh' });
+                release();
+                const observed = await outcome;
+                if (resultGeneration === 'New') {
+                    expect(observed).toEqual({ result: { content: [], structuredContent: { generation: 'New' } } });
+                } else {
+                    expect(observed).toMatchObject({
+                        error: {
+                            code: ProtocolErrorCode.InvalidParams,
+                            message: expect.stringContaining("does not match the tool's output schema")
+                        }
+                    });
+                }
+                expect(executions).toBe(1);
+                expect(callHeaders).toEqual([{ 'Mcp-Param-Old-Region': 'eu' }, { 'Mcp-Param-New-Region': 'eu' }]);
+                expect(listCount()).toBe(2);
+            } finally {
+                release();
+                await client.close();
+                await serverTx.close();
+            }
+        }
+    );
+
     it('listTools() and the cached tools/list entry exclude constraint-violating x-mcp-header tools and warn', async () => {
         const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
         const store = new InMemoryResponseCacheStore();
@@ -285,18 +387,14 @@ describe('SEP-2243 Mcp-Param-* mirroring (modern era)', () => {
         client.onerror = e => errors.push(e);
         await client.connect(clientTx);
 
-        // The pre-send mirroring read AND the post-success validator read both
-        // hit a rejecting `get()`. Neither aborts the call: the request goes
-        // out without `Mcp-Param-*` headers (cold-cache posture), the
-        // server-side result is returned, and both store failures surface via
-        // `onerror`. The post-success guard is the critical one — a store
-        // failure after the server has executed the call must never surface
-        // as a `callTool()` rejection (duplicate-execution hazard on retry).
+        // One metadata read supplies both mirroring and validation. Its store
+        // failure is reported once, and the successful call remains available
+        // to the caller without a second read after server execution.
         const result = await client.callTool({ name: 'route', arguments: { region: 'ap' } });
         expect(result.content?.[0]).toEqual({ type: 'text', text: 'ok' });
         expect(callHeaders).toEqual([undefined]);
         expect(listCount()).toBe(0);
-        expect(errors.map(e => e.message)).toEqual(['redis down', 'redis down']);
+        expect(errors.map(e => e.message)).toEqual(['redis down']);
     });
 
     it('a paginating server: the cached aggregate holds every page and a page-2 x-mcp-header tool mirrors on the first call', async () => {