Client.listTools() corrupts its cached tool metadata when an output schema fails to compile

[sebthom]

Description

Client.listTools() clears its existing output-validator and task-metadata caches before every schema in the replacement catalog has compiled successfully.

If compilation of a later output schema throws, listTools() rejects as expected, but the previously valid metadata has already been erased or partially replaced. Subsequent callTool() operations may
therefore skip output validation, and cached task-support information may also be lost.

A failed catalog refresh should leave the previous complete metadata generation unchanged.

This is separate from the concurrent callTool()/listTools() validator-generation race reported in: #2612

Reproduction

Tested with @modelcontextprotocol/sdk@1.30.0.

import assert from "node:assert/strict";
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
import { ListToolsResultSchema } from "@modelcontextprotocol/sdk/types.js";

const client = new Client({
  name: "metadata-cache-repro",
  version: "1.0.0",
});

const validCatalog = ListToolsResultSchema.parse({
  tools: [{
    name: "versioned",
    inputSchema: {
      type: "object",
      additionalProperties: false,
    },
    outputSchema: {
      type: "object",
      properties: {
        generation: { const: "old" },
      },
      required: ["generation"],
      additionalProperties: false,
    },
  }],
});

// The MCP result schema accepts this catalog because the output schema has a
// valid object root. AJV later rejects the invalid nested `type`.
const invalidCatalog = ListToolsResultSchema.parse({
  tools: [{
    name: "invalid",
    inputSchema: {
      type: "object",
      additionalProperties: false,
    },
    outputSchema: {
      type: "object",
      properties: {
        value: { type: "not-a-json-schema-type" },
      },
    },
  }],
});

const catalogs = [validCatalog, invalidCatalog];

client.request = async ({ method }) => {
  if (method === "tools/list") {
    return catalogs.shift();
  }

  if (method === "tools/call") {
    return {
      content: [{ type: "text", text: "new" }],
      structuredContent: { generation: "new" },
      isError: false,
    };
  }

  throw new Error(`Unexpected method: ${method}`);
};

// Installs the validator requiring generation === "old".
await client.listTools();

// Compilation throws, which is expected for the invalid schema.
await assert.rejects(() => client.listTools());

// This should still use the validator from the last successful catalog and
// reject generation === "new". Instead, it resolves because that validator
// was cleared before the failed replacement compiled.
await assert.rejects(
  () => client.callTool({
    name: "versioned",
    arguments: {},
  }),
  /does not match the tool's output schema/,
);

The final assertion fails with:

AssertionError: Missing expected rejection

Expected behavior

Metadata replacement should be failure-atomic:

1. Compile all output validators and collect all task metadata into temporary collections.
2. Publish the new collections only after the entire catalog succeeds.
3. If any schema compilation throws, preserve the previous complete collections.

Actual behavior

cacheToolMetadata() clears the current collections before compilation begins:

this._cachedToolOutputValidators.clear();
this._cachedKnownTaskTools.clear();
this._cachedRequiredTaskTools.clear();

A later compilation error therefore leaves the client with empty or partially replaced metadata.

Suggested fix

One possible failure-atomic fix is to build replacement Map and Set instances locally, then publish them only after every tool has been processed and every output schema has compiled successfully.

diff --git a/src/client/index.ts b/src/client/index.ts
--- a/src/client/index.ts
+++ b/src/client/index.ts
@@
 private cacheToolMetadata(tools: Tool[]): void {
-    this._cachedToolOutputValidators.clear();
-    this._cachedKnownTaskTools.clear();
-    this._cachedRequiredTaskTools.clear();
+    // Compile the complete replacement before publishing it so a late schema
+    // failure leaves the previous successful metadata generation intact.
+    const toolOutputValidators = new Map<string, JsonSchemaValidator<unknown>>();
+    const knownTaskTools = new Set<string>();
+    const requiredTaskTools = new Set<string>();

     for (const tool of tools) {
         // If the tool has an outputSchema, create and cache the validator
         if (tool.outputSchema) {
             const toolValidator = this._jsonSchemaValidator.getValidator(
                 tool.outputSchema as JsonSchemaType
             );
-            this._cachedToolOutputValidators.set(tool.name, toolValidator);
+            toolOutputValidators.set(tool.name, toolValidator);
         }

         // If the tool supports task-based execution, cache that information
         const taskSupport = tool.execution?.taskSupport;
         if (taskSupport === 'required' || taskSupport === 'optional') {
-            this._cachedKnownTaskTools.add(tool.name);
+            knownTaskTools.add(tool.name);
         }

         if (taskSupport === 'required') {
-            this._cachedRequiredTaskTools.add(tool.name);
+            requiredTaskTools.add(tool.name);
         }
     }
+
+    this._cachedToolOutputValidators = toolOutputValidators;
+    this._cachedKnownTaskTools = knownTaskTools;
+    this._cachedRequiredTaskTools = requiredTaskTools;
 }

[kinKingen]

I'd like to take this if maintainers are happy to assign it.

I verified that the current v1.x branch still clears all three metadata caches before output-schema validator compilation in cacheToolMetadata(). Since this report targets @modelcontextprotocol/sdk@1.30.0 and the main v2 client now uses a different cache architecture, I would target v1.x.

Proposed focused change:

• add a regression test that installs valid output/task metadata, makes the next listTools() refresh fail during schema compilation, then verifies the previous output validator and task-support metadata are still active;
• build replacement Map/Set collections locally and publish them only after every tool compiles successfully;
• run the focused client tests plus the v1.x typecheck/lint gates.

I will wait for assignment before starting implementation, per the contribution guide.

AI disclosure: I used an AI coding assistant to help inspect the current branches and formulate this plan; I personally reviewed the repository rules, code path, and conclusions above.