client.callTool validates structuredContent against outputSchema even when isError is true (asymmetric with server)

[bokelley]

Summary

The client-side callTool (src/client/index.ts, current dist/esm/client/index.js around lines 496–519) runs outputSchema validation against result.structuredContent whenever structuredContent is present, regardless of result.isError. The server-side validateToolOutput correctly short-circuits when isError is true. The asymmetry makes any tool whose error envelope carries structuredContent unusable with outputSchema.

Reproduction

const server = new McpServer({ name: 'x', version: '1.0.0' });
server.registerTool(
  'get_things',
  {
    inputSchema: {},
    outputSchema: { things: z.array(z.string()) },
  },
  async () => ({
    content: [{ type: 'text', text: JSON.stringify({ err: 'NOT_FOUND' }) }],
    structuredContent: { err: { code: 'NOT_FOUND', message: 'nope' } },
    isError: true,
  }),
);

Server-side: validateToolOutput returns early because result.isError is true.
Client-side: callTool sees structuredContent, validates against the things schema, throws McpError(-32602, 'Structured content does not match...'). The buyer can never reach the error payload.

Current behaviour in client/index.js

if (validator) {
  if (!result.structuredContent && !result.isError) {
    throw new McpError(
      ErrorCode.InvalidRequest,
      `Tool ${params.name} has an output schema but did not return structured content`,
    );
  }
  // Only validate structured content if present (not when there's an error)
  if (result.structuredContent) {   // <-- missing !result.isError guard
    // ...validate...
  }
}

The comment above the second if ("not when there's an error") matches the intent, but the guard itself is missing.

Server-side reference

server/mcp.js validateToolOutput:

if (result.isError) {
  return;
}

This is the behaviour that should also apply client-side.

Proposed fix

-  if (result.structuredContent) {
+  if (result.structuredContent && !result.isError) {
     // validate...
   }

Two lines. Matches the existing comment and matches the server-side behaviour.

Impact

Any protocol layering structured error envelopes on top of MCP (AdCP's adcp_error, and likely others that rely on structuredContent for programmatic error extraction) is forced to either:

• skip outputSchema entirely (losing tools/list discoverability and client-side success validation),
• or inflate outputSchema into a permissive union that accepts both success and error shapes (defeats the point).

The two-line fix removes that tradeoff and makes client/server output validation consistent.

Happy to open a PR if the fix is welcome.

Environment

• @modelcontextprotocol/sdk 1.29.0 (checked against current main; behaviour unchanged)
• Found while migrating @adcp/client off the deprecated server.tool() overload to registerTool.

[truffle-dev]

Verified against current main at bdfd7f0. The bug is real at packages/client/src/client/client.ts:893, same comment/code mismatch you flagged, guard still missing:

// Only validate structured content if present (not when there's an error)
if (result.structuredContent) {
    // ...validate...
}

One addition to flag: the same pattern exists in packages/client/src/experimental/tasks/client.ts:139, identical comment and identical missing !result.isError guard. A PR that fixes both closes the asymmetry completely; fixing only the main client location would leave tasks-mode clients with the same bug.

Server-side reference for contrast is at packages/server/src/server/mcp.ts:276 (if (result.isError) { return; } short-circuits before validation), which matches your stated intended behavior.

[Zelys-DFKH]

The comment on line 760 (// Only validate structured content if present (not when there's an error)) already says the right thing — the !result.isError guard is just missing. Same gap in src/experimental/tasks/client.ts line 125 for the streaming path.

Two one-line changes: if (result.structuredContent && !result.isError) in both places. I'll send a PR.

[truffle-dev]

Heads-up: #1945 already covers this (opened this morning). Same two-site guard on client.ts and experimental/tasks/client.ts, with tests on both paths using structuredContent that intentionally violates outputSchema. A second review pass there would be more useful than a duplicate.

[bokelley]

Superseded by #1945, which has the two-site guard (client.ts + experimental/tasks/client.ts) plus integration tests for both paths. Closing in favor of that PR.