fix(client): isolate per-tool outputSchema compile failures (PR #2249 review)

Addresses claude-bot review on PR #2249: assertSchemaSafeToCompile (SEP-2106
SSRF / composition-DoS guard) throws inside getValidator(), and
cacheToolMetadata() eagerly compiled every advertised tool's outputSchema with
no per-tool error handling. A single tool advertising a non-local $ref or an
over-budget schema therefore rejected the entire listTools() call, leaving the
client unable to list or call ANY tool from that server.

- catch compilation per-tool in cacheToolMetadata(); store the error in a new
  _toolOutputValidatorErrors map instead of letting it propagate.
- surface the scoped error from callTool() only when the offending tool is
  actually called (clear, descriptive ProtocolError).
- integration test: one tool with a non-local $ref outputSchema does not break
  listTools() or the use of a sibling good tool; the bad tool errors only on call.

Note: the second review comment (experimental/tasks callToolStream truthiness)
is stale \u2014 the tasks feature was removed in c8d7401 before this branch, no
callToolStream exists, and no truthiness structuredContent checks remain.

Author: mattzcarey

packages/client/src/client/client.ts

@@ -218,6 +218,13 @@ export class Client extends Protocol<ClientContext> {
     private _instructions?: string;
     private _jsonSchemaValidator: jsonSchemaValidator;
     private _cachedToolOutputValidators: Map<string, JsonSchemaValidator<unknown>> = new Map();
+    /**
+     * Tools whose advertised `outputSchema` could not be compiled into a validator (e.g. it tripped
+     * the SEP-2106 safety guards — a non-local `$ref` or an over-budget schema). The error is stored
+     * per-tool and surfaced only when that tool is called, so one malformed tool definition does not
+     * break `listTools()` or the use of every other tool from the same server.
+     */
+    private _toolOutputValidatorErrors: Map<string, Error> = new Map();
     private _listChangedDebounceTimers: Map<string, ReturnType<typeof setTimeout>> = new Map();
     private _pendingListChangedConfig?: ListChangedHandlers;
     private _enforceStrictCapabilities: boolean;
@@ -790,6 +797,17 @@ export class Client extends Protocol<ClientContext> {
     async callTool(params: CallToolRequest['params'], options?: RequestOptions): Promise<CallToolResult> {
         const result = await this._requestWithSchema({ method: 'tools/call', params }, CallToolResultSchema, options);
 
+        // If the tool advertised an outputSchema that failed to compile (e.g. a SEP-2106 safety-guard
+        // rejection), surface that error now — scoped to this tool — rather than silently skipping
+        // output validation.
+        const validatorError = this._toolOutputValidatorErrors.get(params.name);
+        if (validatorError) {
+            throw new ProtocolError(
+                ProtocolErrorCode.InvalidParams,
+                `Tool ${params.name} has an output schema that could not be compiled: ${validatorError.message}`
+            );
+        }
+
         // Check if the tool has an outputSchema
         const validator = this.getToolOutputValidator(params.name);
         if (validator) {
@@ -836,12 +854,19 @@ export class Client extends Protocol<ClientContext> {
      */
     private cacheToolMetadata(tools: Tool[]): void {
         this._cachedToolOutputValidators.clear();
+        this._toolOutputValidatorErrors.clear();
 
         for (const tool of tools) {
-            // If the tool has an outputSchema, create and cache the validator
+            // If the tool has an outputSchema, create and cache the validator. Compilation can throw
+            // (invalid schema, or a SEP-2106 safety-guard rejection); scope that failure to the
+            // offending tool rather than letting it reject the whole listTools() call.
             if (tool.outputSchema) {
-                const toolValidator = this._jsonSchemaValidator.getValidator(tool.outputSchema as JsonSchemaType);
-                this._cachedToolOutputValidators.set(tool.name, toolValidator);
+                try {
+                    const toolValidator = this._jsonSchemaValidator.getValidator(tool.outputSchema as JsonSchemaType);
+                    this._cachedToolOutputValidators.set(tool.name, toolValidator);
+                } catch (error) {
+                    this._toolOutputValidatorErrors.set(tool.name, error instanceof Error ? error : new Error(String(error)));
+                }
             }
         }
     }

test/integration/test/client/client.test.ts

@@ -1940,6 +1940,60 @@ describe('outputSchema validation', () => {
         );
     });
 
+    /***
+     * Test: A single tool with an uncompilable outputSchema does not break listTools()
+     * or the use of other tools (SEP-2106 safety-guard isolation).
+     */
+    test('isolates a tool whose outputSchema fails to compile from the rest of the server', async () => {
+        const server = new Server({ name: 'test-server', version: '1.0.0' }, { capabilities: { tools: {} } });
+
+        server.setRequestHandler('initialize', async request => ({
+            protocolVersion: request.params.protocolVersion,
+            capabilities: { tools: {} },
+            serverInfo: { name: 'test-server', version: '1.0.0' }
+        }));
+
+        server.setRequestHandler('tools/list', async () => ({
+            tools: [
+                {
+                    name: 'bad-tool',
+                    description: 'advertises a non-local $ref the SEP-2106 guard rejects',
+                    inputSchema: { type: 'object', properties: {} },
+                    // Non-same-document $ref: assertSchemaSafeToCompile throws when this is compiled.
+                    outputSchema: { $ref: 'https://evil.example/schema.json' }
+                },
+                {
+                    name: 'good-tool',
+                    description: 'a normal tool that must remain usable',
+                    inputSchema: { type: 'object', properties: {} },
+                    outputSchema: { type: 'object', properties: { ok: { type: 'boolean' } }, required: ['ok'] }
+                }
+            ]
+        }));
+
+        server.setRequestHandler('tools/call', async request => {
+            if (request.params.name === 'good-tool') {
+                return { content: [], structuredContent: { ok: true } };
+            }
+            return { content: [], structuredContent: { irrelevant: true } };
+        });
+
+        const client = new Client({ name: 'test-client', version: '1.0.0' });
+        const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
+        await Promise.all([client.connect(clientTransport), server.connect(serverTransport)]);
+
+        // listTools() must NOT reject just because one tool's schema is uncompilable.
+        const listed = await client.listTools();
+        expect(listed.tools.map(t => t.name).toSorted()).toEqual(['bad-tool', 'good-tool']);
+
+        // The good tool is fully usable.
+        const good = await client.callTool({ name: 'good-tool' });
+        expect(good.structuredContent).toEqual({ ok: true });
+
+        // The bad tool surfaces a scoped, descriptive error only when it is called.
+        await expect(client.callTool({ name: 'bad-tool' })).rejects.toThrow(/output schema that could not be compiled/i);
+    });
+
     /***
      * Test: Handle Tools Without outputSchema Normally
      */