fix(client,core,server): #2337 review — stale compile-error cache, typeless-root stamping, result mutation, dead-surface JSDoc

Author: felixweinberger

packages/client/src/client/client.ts

@@ -413,6 +413,19 @@ interface ListenStateEntry {
     settle: (outcome: { ack: SubscriptionFilter } | { cause: 'local' | 'remote'; error?: Error }) => void;
 }
 
+/**
+ * Per-tool result of compiling an `outputSchema` (SEP-2106). Stored on the
+ * response-cache substrate's stamp-keyed `name → validator` index so it
+ * inherits that substrate's invalidation lifecycle (`list_changed` evicts,
+ * a refetched `tools/list` re-derives, `resetForReconnect` clears) — no
+ * parallel map to keep in sync.
+ *
+ * @internal
+ */
+type OutputSchemaCompileResult =
+    | { ok: true; validator: JsonSchemaValidator<unknown> }
+    | { ok: false; validator?: undefined; compileError: unknown };
+
 /**
  * An MCP client on top of a pluggable transport.
  *
@@ -453,13 +466,6 @@ export class Client extends Protocol<ClientContext> {
     private _capabilities: ClientCapabilities;
     private _instructions?: string;
     private _jsonSchemaValidator: jsonSchemaValidator;
-    /**
-     * Per-tool compile errors for `outputSchema`, captured by {@linkcode _compileOutputValidator}
-     * (the response-cache substrate's compile callback) so one bad schema does not poison the whole
-     * list (SEP-2106; baseline-bug #14). Surfaced by {@linkcode callTool} as a typed
-     * `InvalidParams` error before the request is sent.
-     */
-    private _cachedToolOutputCompileErrors: Map<string, unknown> = new Map();
     /**
      * The response-cache substrate. Owns the backing store, the per-method
      * eviction-generation counter, the user-supplied/default flag, and the
@@ -531,7 +537,6 @@ export class Client extends Protocol<ClientContext> {
             clearTimeout(timer);
         }
         this._listChangedDebounceTimers.clear();
-        this._cachedToolOutputCompileErrors.clear();
         // A user-supplied store is NOT cleared on reconnect/close — that would
         // defeat the only reason to supply one. The per-instance default IS
         // cleared (it is connection-scoped); derived indices and the
@@ -1649,25 +1654,28 @@ export class Client extends Protocol<ClientContext> {
     }
 
     /**
-     * Compile a single tool's `outputSchema` (or `undefined` when absent /
-     * uncompilable) — the caller-supplied-definition path of
-     * {@linkcode callTool} so an explicit `options.toolDefinition` is the
-     * source for BOTH mirroring AND output validation. Also passed as the
-     * compile callback to {@linkcode ClientResponseCache.outputValidator} so
-     * the cache class stays free of any validator-provider dependency.
+     * Compile a single tool's `outputSchema`. Passed as the compile callback to
+     * {@linkcode ClientResponseCache.outputValidator} so the cache class stays
+     * free of any validator-provider dependency, and called directly for the
+     * `options.toolDefinition` path of {@linkcode callTool} (a one-off
+     * caller-supplied definition is compiled in isolation and never enters the
+     * cache, so it cannot poison the listed tool of the same name).
+     *
+     * Returns `undefined` when the tool has no `outputSchema`, or a
+     * discriminated `{ok}` result otherwise. SEP-2106: ANY throw — from the
+     * ref/bounds/dialect guard or from the underlying engine — is captured as
+     * `{ok: false, compileError}` so one bad schema does not poison the rest
+     * of the listing; `callTool()` surfaces it as a typed `InvalidParams`
+     * error before the request. The `{ok}` discriminator (not
+     * `compileError !== undefined`) means a custom provider that does
+     * `throw undefined` is still treated as a captured failure.
      */
-    private _compileOutputValidator(tool: Tool): JsonSchemaValidator<unknown> | undefined {
+    private _compileOutputValidator(tool: Tool): OutputSchemaCompileResult | undefined {
         if (!tool.outputSchema) return undefined;
         try {
-            const validator = this._jsonSchemaValidator.getValidator(tool.outputSchema as JsonSchemaType);
-            this._cachedToolOutputCompileErrors.delete(tool.name);
-            return validator;
+            return { ok: true, validator: this._jsonSchemaValidator.getValidator(tool.outputSchema as JsonSchemaType) };
         } catch (error) {
-            // SEP-2106: ANY throw — from the ref/bounds/dialect guard or from the underlying
-            // engine — is captured per-tool so one bad schema does not poison the rest of the
-            // listing; callTool() surfaces it as a typed InvalidParams error before the request.
-            this._cachedToolOutputCompileErrors.set(tool.name, error);
-            return undefined;
+            return { ok: false, compileError: error };
         }
     }
 
@@ -2171,23 +2179,23 @@ export class Client extends Protocol<ClientContext> {
         // surfaced here, per-tool, without a wasted network round-trip and server-side handler
         // execution. When the caller supplied `toolDefinition`, that definition is the source for
         // BOTH the `Mcp-Param-*` mirroring above AND output validation — the two derived views
-        // must agree. 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).
-        const validator =
+        // 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).
+        const compiled =
             options?.toolDefinition === undefined
                 ? await this._cache
                       .outputValidator(params.name, tool => this._compileOutputValidator(tool))
                       .catch(error => void this._reportStoreError(error))
                 : this._compileOutputValidator(options.toolDefinition);
-        // `.has()` (not `.get()!==undefined`) so a custom provider that does `throw undefined` is
-        // still treated as a captured failure.
-        if (this._cachedToolOutputCompileErrors.has(params.name)) {
-            const compileError = this._cachedToolOutputCompileErrors.get(params.name);
+        if (compiled !== undefined && !compiled.ok) {
+            const compileError = compiled.compileError;
             const message = (compileError instanceof Error ? compileError.message : String(compileError)).slice(0, 200);
             throw new ProtocolError(ProtocolErrorCode.InvalidParams, `Tool '${params.name}' has an invalid outputSchema: ${message}`, {
                 reason: compileError instanceof SchemaCompileError ? compileError.reason.kind : 'invalid-schema'
             });
         }
+        const validator = compiled?.validator;
 
         // The method-keyed request() path validates the era registry's plain
         // CallToolResult schema — with the result map aligned to the typed

packages/client/src/client/responseCache.ts

@@ -612,9 +612,14 @@ export class ClientResponseCache {
      * `Client` passes its `_jsonSchemaValidator` wrapper) so 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 returns
-     * `undefined` (and warns naming the offender) for the bad one and the
-     * index simply omits it.
+     * must not poison every other tool's `callTool` — the callback isolates
+     * that compile error per tool by returning a per-tool error variant which
+     * the index stores alongside the good ones, and `callTool` surfaces it as
+     * a typed `InvalidParams` only for that name. Because the error is held on
+     * this stamp-keyed substrate (not a parallel map), it inherits the
+     * substrate's invalidation lifecycle: a `list_changed` eviction drops it,
+     * a refetched `tools/list` re-derives it, and `resetForReconnect` clears
+     * the lot.
      */
     async outputValidator<V>(name: string, compile: (tool: Tool) => V | undefined): Promise<V | undefined> {
         const entry = await this._probe('tools/list');
@@ -625,10 +630,8 @@ export class ClientResponseCache {
         if (this._toolOutputValidatorIndex?.stamp !== entry.stamp) {
             const byName = new Map<string, unknown>();
             for (const tool of (entry.value as ListToolsResult).tools) {
-                if (tool.outputSchema) {
-                    const validator = compile(tool);
-                    if (validator !== undefined) byName.set(tool.name, validator);
-                }
+                const compiled = compile(tool);
+                if (compiled !== undefined) byName.set(tool.name, compiled);
             }
             this._toolOutputValidatorIndex = { stamp: entry.stamp, byName };
         }

packages/client/test/client/jsonSchemaValidatorOverride.test.ts

@@ -114,6 +114,88 @@ describe('client JSON Schema validator overrides', () => {
         await serverTransport.close();
     });
 
+    describe('outputSchema compile-error lifecycle (substrate-held; no parallel map)', () => {
+        // SEP-2106 §invalid-outputSchema: a tool whose outputSchema fails to compile is
+        // surfaced as a typed InvalidParams BEFORE the request is sent. The compile error is
+        // held on the response-cache substrate's stamp-keyed `name → validator` index, so it
+        // inherits that substrate's invalidation lifecycle — a refetched `tools/list` re-derives
+        // it from scratch (no stale-entry bug when the server fixes the tool by removing the
+        // schema). The caller-supplied `toolDefinition` path is compiled in isolation and never
+        // touches the cache, so a one-off bad definition cannot poison the listed tool.
+        async function connectMutableToolsClient(getTools: () => unknown[]) {
+            const client = new Client({ name: 'test-client', version: '1.0.0' }, { capabilities: {} });
+            const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
+            serverTransport.onmessage = async message => {
+                if (!('method' in message) || !('id' in message)) return;
+                if (message.method === 'initialize') {
+                    await serverTransport.send({
+                        jsonrpc: '2.0',
+                        id: message.id,
+                        result: {
+                            protocolVersion: LATEST_PROTOCOL_VERSION,
+                            capabilities: { tools: {} },
+                            serverInfo: { name: 'test-server', version: '1.0.0' }
+                        }
+                    });
+                } else if (message.method === 'tools/list') {
+                    await serverTransport.send({
+                        jsonrpc: '2.0',
+                        id: message.id,
+                        result: { tools: getTools() }
+                    } satisfies JSONRPCMessage);
+                } else if (message.method === 'tools/call') {
+                    await serverTransport.send({
+                        jsonrpc: '2.0',
+                        id: message.id,
+                        result: { content: [{ type: 'text', text: 'ok' }], structuredContent: { count: 1 } }
+                    } satisfies JSONRPCMessage);
+                }
+            };
+            await Promise.all([client.connect(clientTransport), serverTransport.start()]);
+            return { client, close: () => Promise.all([client.close(), clientTransport.close(), serverTransport.close()]) };
+        }
+
+        // A network `$ref` is rejected at compile time by the SDK's built-in ref guard.
+        const BAD_SCHEMA = { type: 'object', $ref: 'https://example.invalid/schema.json' } as const;
+        const GOOD_SCHEMA = { type: 'object', properties: { count: { type: 'number' } } } as const;
+
+        test('re-advertising a tool WITHOUT the bad outputSchema clears the captured failure', async () => {
+            let tools: unknown[] = [{ name: 't', inputSchema: { type: 'object' }, outputSchema: BAD_SCHEMA }];
+            const { client, close } = await connectMutableToolsClient(() => tools);
+
+            await client.listTools();
+            await expect(client.callTool({ name: 't' })).rejects.toThrow(/invalid outputSchema/);
+
+            // Server fixes the tool by removing outputSchema entirely; refetched `tools/list`
+            // re-derives the index from scratch — no stale compile-error entry survives.
+            tools = [{ name: 't', inputSchema: { type: 'object' } }];
+            await client.listTools();
+            await expect(client.callTool({ name: 't' })).resolves.toMatchObject({
+                content: [{ type: 'text', text: 'ok' }]
+            });
+
+            await close();
+        });
+
+        test('a one-off `toolDefinition` with a bad outputSchema does not poison the listed tool', async () => {
+            const tools: unknown[] = [{ name: 't', inputSchema: { type: 'object' }, outputSchema: GOOD_SCHEMA }];
+            const { client, close } = await connectMutableToolsClient(() => tools);
+
+            await client.listTools();
+            await expect(
+                client.callTool({ name: 't' }, { toolDefinition: { name: 't', inputSchema: { type: 'object' }, outputSchema: BAD_SCHEMA } })
+            ).rejects.toThrow(/invalid outputSchema/);
+
+            // Subsequent plain callTool of the same name (against the cached, valid listed
+            // schema) succeeds — the one-off definition never entered the cache.
+            await expect(client.callTool({ name: 't' })).resolves.toMatchObject({
+                structuredContent: { count: 1 }
+            });
+
+            await close();
+        });
+    });
+
     test('fromJsonSchema uses an explicitly supplied custom validator', async () => {
         const validator = new RecordingValidator();
         const schema: JsonSchemaType = {

packages/core/src/util/standardSchema.ts

@@ -206,10 +206,18 @@ export function standardSchemaToJsonSchema(schema: StandardJSONSchemaV1, io: 'in
     }
     if (io === 'output') {
         // SEP-2106: outputSchema may have any JSON Schema root type (array, string, …) — no
-        // non-object throw. A typeless root keeps the pre-SEP `type:'object'` default so the
-        // 2025-era wire bytes are unchanged for object/typeless schemas; an explicit non-object
-        // `type` is returned as-is (and gated by the legacy projection drop).
-        return result.type === undefined ? { type: 'object', ...result } : result;
+        // non-object throw. An explicit `type` (object or not) is returned as-is. A typeless
+        // root is returned as-is when it is NOT provably object-shaped — e.g. a `z.union(...)`
+        // emits `{anyOf:[…]}`, and stamping `type:'object'` there would be self-contradictory.
+        // Only when the root is typeless AND object-shaped (carries `properties` /
+        // `patternProperties` / `additionalProperties` / `required`) do we default
+        // `type:'object'` so the 2025-era wire bytes are unchanged for the pre-SEP object
+        // schemas. The legacy projection drop gates anything that does not end up
+        // `type:'object'`.
+        if (result.type !== undefined) return result;
+        const isObjectShaped =
+            'properties' in result || 'patternProperties' in result || 'additionalProperties' in result || 'required' in result;
+        return isObjectShaped ? { type: 'object', ...result } : result;
     }
     if (result.type !== undefined && result.type !== 'object') {
         throw new Error(

packages/core/src/validators/ajvProvider.ts

@@ -89,10 +89,16 @@ export class AjvJsonSchemaValidator implements jsonSchemaValidator {
         this._ajv = ajv ?? createDefaultAjvInstance();
     }
 
-    /** Supported `$schema` dialect identifiers. */
+    /**
+     * JSON Schema dialects this provider can validate, as short identifiers. Advisory/diagnostic
+     * only — the SDK does not branch on it.
+     */
     readonly supportedDialects = ['2020-12'] as const;
 
-    /** @inheritdoc */
+    /**
+     * The dialect this provider applies when a schema declares no `$schema`.
+     * Advisory/diagnostic only — the SDK does not branch on it.
+     */
     readonly defaultDialect = '2020-12';
 
     getValidator<T>(schema: JsonSchemaType): JsonSchemaValidator<T> {

packages/core/src/validators/cfWorkerProvider.ts

@@ -68,13 +68,17 @@ export class CfWorkerJsonSchemaValidator implements jsonSchemaValidator {
         this.draft = options?.draft;
     }
 
-    /** @inheritdoc */
+    /**
+     * JSON Schema dialects this provider can validate, as short identifiers. Advisory/diagnostic
+     * only — the SDK does not branch on it.
+     */
     readonly supportedDialects = ['2020-12'] as const;
 
-    /** @inheritdoc */
-    get defaultDialect(): string {
-        return this.draft ?? '2020-12';
-    }
+    /**
+     * The dialect this provider applies when a schema declares no `$schema` and no `{draft}` is
+     * forced. Advisory/diagnostic only — the SDK does not branch on it.
+     */
+    readonly defaultDialect = '2020-12';
 
     /**
      * Create a validator for the given JSON Schema

packages/core/src/validators/schemaBounds.ts

@@ -29,9 +29,10 @@ export const DEFAULT_MAX_SCHEMA_DEPTH = 64;
 export const DEFAULT_MAX_SUBSCHEMA_COUNT = 10_000;
 
 /**
- * JSON Schema 2020-12 applicator keywords whose value is one or more subschemas (plus the draft-07
- * spellings `definitions` / `additionalItems`). Exported for consumers that need to descend through
- * schema-valued positions only — notably the SEP-2243 `x-mcp-header` scan.
+ * Vocabulary list of JSON Schema keywords whose values are themselves schemas. Currently consumed
+ * only within this module's tests; intended as the single source for the SEP-2243 `x-mcp-header`
+ * reachability scan in `mcpParamHeaders.ts` (which today carries its own private list —
+ * consolidation is a follow-up).
  *
  * NOTE: {@link assertSchemaSafeToCompile} does **not** consult this list; its walk is intentionally
  * exhaustive (every key) so a future or vendor keyword cannot smuggle a `$ref` past the guard.

packages/core/src/validators/types.ts

@@ -60,18 +60,4 @@ export interface jsonSchemaValidator {
      * @returns A validator function that can be called multiple times
      */
     getValidator<T>(schema: JsonSchemaType): JsonSchemaValidator<T>;
-
-    /**
-     * JSON Schema dialects this provider can validate, as short identifiers (e.g. `'2020-12'`,
-     * `'draft-07'`). When present, the SDK uses this to surface a graceful
-     * {@link SchemaCompileError} for an unrecognised `$schema` declaration; when absent the SDK
-     * does not dialect-check on the provider's behalf.
-     */
-    readonly supportedDialects?: readonly string[];
-
-    /**
-     * The dialect (from {@link supportedDialects}) this provider applies when a schema declares no
-     * `$schema`. Advisory — surfaced for diagnostics; the SDK does not branch on it.
-     */
-    readonly defaultDialect?: string;
 }

packages/core/test/util/standardSchema.test.ts

@@ -39,4 +39,24 @@ describe('standardSchemaToJsonSchema', () => {
         expect(keys.filter(k => k === 'type')).toHaveLength(1);
         expect(result.type).toBe('object');
     });
+
+    describe("io='output' (SEP-2106)", () => {
+        test('union of primitives is returned as-is without type:object stamping', () => {
+            const result = standardSchemaToJsonSchema(z.union([z.string(), z.number()]), 'output');
+            // zod emits a typeless `{anyOf:[…]}` — stamping `type:'object'` here would be
+            // self-contradictory; the legacy projection drop gates it instead.
+            expect(result.type).toBeUndefined();
+            expect(result.anyOf).toBeDefined();
+        });
+
+        test('nullable object is not force-stamped type:object', () => {
+            const result = standardSchemaToJsonSchema(z.object({}).nullable(), 'output');
+            expect(result.type).not.toBe('object');
+        });
+
+        test('object schemas still get type:object', () => {
+            const result = standardSchemaToJsonSchema(z.object({ a: z.string() }), 'output');
+            expect(result.type).toBe('object');
+        });
+    });
 });