What the bug is

In processQuery(), the try block calls client.callTool() and constructs a ToolResultBlockParam from the result. The catch block correctly sets is_error: true when an exception is thrown (e.g., transport/protocol failure). However, the MCP protocol also allows servers to report tool-level failures in-band by returning { isError: true, content: [...] } without throwing. The code never checks result.isError and therefore never sets is_error: true for in-band errors.

The specific code path

Lines 120-129 of examples/client-multi-server/src/index.ts:

const resultText = result.content
  .filter((c) => c.type === "text")
  .map((c) => c.text)
  .join("

");

toolResults.push({
  type: "tool_result",
  tool_use_id: block.id,
  content: resultText,   // no is_error: true even when result.isError === true
});

The is_error field is only set inside the catch block, never in the success path.

Why existing code does not prevent it

The outer try/catch catches exceptions (network errors, JSON parse failures, etc.) but does not observe the return value of callTool(). The result.isError field is a separate, in-band signal that sits in the return value and is simply never read.

Impact

The MCP spec (packages/core/src/types/spec.types.ts:1563) explicitly states: "Any errors that originate from the tool SHOULD be reported inside the result object, with isError set to true, not as an MCP protocol-level error response. Otherwise, the LLM would not be able to see that an error occurred and self-correct." By omitting is_error: true, Claude receives the error text as a normal successful result and may continue reasoning incorrectly (e.g., treating "File not found" as actual file content). The example also teaches incorrect patterns to developers who copy it.

Step-by-step proof

1. An MCP server returns { isError: true, content: [{ type: "text", text: "File not found" }] } from a callTool request.
2. The code enters the try block (no exception is thrown).
3. result.content.filter/map/join produces "File not found".
4. toolResults.push({ type: "tool_result", tool_use_id: block.id, content: "File not found" }) -- no is_error: true.
5. Claude receives a tool_result block with content: "File not found" and no error flag; it interprets this as successful output and continues reasoning incorrectly.

How to fix it

Propagate result.isError to the block param:

toolResults.push({
  type: "tool_result",
  tool_use_id: block.id,
  content: resultText,
  ...(result.isError && { is_error: true }),
});

What the bug is

In main() (lines 193-210 of examples/client-multi-server/src/index.ts), the error handling uses process.exit(1) inside the catch block. In Node.js, process.exit() terminates the process synchronously and immediately — it halts the event loop without unwinding the JavaScript call stack. This means finally blocks do not execute after process.exit() is called.

The specific code path

try {
  await mcpClient.connectToServers(configPath);
  await mcpClient.chatLoop();
} catch (e) {
  console.error('Error:', e);
  process.exit(1);  // process terminates HERE — finally never runs
} finally {
  await mcpClient.cleanup();  // NEVER REACHED on error
  process.exit(0);
}

When connectToServers() or chatLoop() throws, the catch block calls process.exit(1), terminating the process before the finally block can execute mcpClient.cleanup().

Why existing code does not prevent it

This is a fundamental Node.js behavior — try/finally only ensures the finally block runs when control leaves the try block via normal flow, exceptions, or return/break. It does not protect against process.exit(), which terminates the process unconditionally without stack unwinding.

Step-by-step proof

1. connectToServers() successfully connects to servers A and B, then server C fails and throws.
2. The catch block runs: console.error('Error:', e) logs the error.
3. process.exit(1) is called — the Node.js event loop halts immediately.
4. The finally block never executes. mcpClient.cleanup() is never called.
5. The stdio transports for servers A and B are not gracefully closed via the MCP protocol. While the OS will eventually clean up file descriptors and child processes will receive EOF/SIGPIPE on stdin, there is no graceful MCP shutdown.

Impact

For this specific CLI example, the practical impact is limited — the OS reclaims resources on process exit, and child processes typically exit once their stdin pipe closes. However, this is example code that developers will copy into production applications. Using process.exit() inside a catch alongside a finally is a pedagogically incorrect pattern that defeats the purpose of finally and could cause real resource leaks in production services derived from this example.

How to fix

Replace process.exit(1) in the catch block with process.exitCode = 1. This sets the exit code without terminating immediately, allowing control to fall through to the finally block:

} catch (e) {
  console.error('Error:', e);
  process.exitCode = 1;  // set exit code, do not terminate yet
} finally {
  await mcpClient.cleanup();  // now runs on both success and error
  process.exit();  // exits with the code set above
}

What the bug is

In main(), connectToServers(configPath) is called unconditionally before the ANTHROPIC_API_KEY environment variable is read (lines 185 and 188 respectively in the final file). Only after all server connections and tool discovery complete does the code check for the key and print a friendly error message.

The specific code path

async function main() {
  const configPath = process.argv[2] ?? servers.json;
  const mcpClient = new MultiServerClient();
  try {
    await mcpClient.connectToServers(configPath);  // spawns processes, downloads packages

    const apiKey = process.env.ANTHROPIC_API_KEY;  // checked only after
    if (!apiKey) {
      console.log(\nNo ANTHROPIC_API_KEY found...);
      return;  // wasted work
    }
    await mcpClient.chatLoop();
  } ...
}

Why existing code does not prevent it

There is no early validation of required prerequisites before entering the expensive setup phase. The API key check is deferred until after connectToServers() completes, which for the default servers.json triggers npx -y @modelcontextprotocol/server-everything — downloading and installing the npm package on first run before establishing the MCP connection and discovering all tools.

Addressing the refutation

The refutation argues this is intentional, allowing users to verify MCP server connectivity without Anthropic credentials. However, this argument does not hold up: the code immediately returns after printing the "no API key" message without offering any tool exploration functionality. If the intent were to support a "browse tools without chatting" mode, there would be a listTools() display or similar. The message itself says "To chat with these tools via Claude" — signalling that chatting is the only intended purpose. The README also lists the API key as a prerequisite before running.

Step-by-step proof

1. User clones the repo and runs npx tsx src/index.ts without setting ANTHROPIC_API_KEY.
2. connectToServers() starts. It reads servers.json and finds the everything entry using npx -y @modelcontextprotocol/server-everything.
3. On first run, npx downloads and installs @modelcontextprotocol/server-everything (~seconds to a minute depending on network).
4. The server starts via stdio, MCP handshake completes, listTools() is called and all tools are discovered.
5. connectToServers() returns. Only now is process.env.ANTHROPIC_API_KEY read.
6. The key is missing. A friendly message is printed and the function returns.
7. cleanup() in finally iterates all connected servers and calls client.close() on each — closing connections that were opened purely to be immediately thrown away.
8. The user waited significant time (npm download + install + server boot) just to learn they need to set an env var.

How to fix

Move the API key check to the top of main(), before connectToServers():

async function main() {
  const apiKey = process.env.ANTHROPIC_API_KEY;
  if (!apiKey) {
    console.error(Error: ANTHROPIC_API_KEY is not set.);
    console.error(  export ANTHROPIC_API_KEY=your-api-key-here);
    process.exit(1);
  }
  // ... rest of setup
}

This is a one-line change that provides immediate feedback without touching any functionality.

What the bug is

The collision detection block on lines 47-51 checks whether toolToServer already contains prefixedName, where prefixedName = name + "__" + tool.name. The intent, based on the warning message, is to catch situations where two different servers expose a tool with the same name. However, because the server name is baked into the prefix, tools from different servers can never share the same prefixed key.

The specific code path

In the tool registration loop, for each server name and each tool from that server, the code computes prefixedName as the concatenation of the server name, a double-underscore separator, and the tool name. It then checks if this key already exists in the map. Since JSON object keys are guaranteed unique, name is distinct for every iteration of the outer loop, so this prefixed key is unique per server+tool pair by construction.

Why existing code does not prevent the issue

The prefixing scheme was added (correctly) to solve cross-server name collision, and it does solve it. But the collision-detection if block was left in place without being updated to reflect the new invariant. Nobody removed or rewrote it to match the new key format, so it now tests for an impossible condition.

Impact

There is no incorrect runtime behavior — the prefixing correctly routes all tool calls. The impact is purely one of code clarity and misleading diagnostics. A developer reading this example could incorrectly believe the if block guards against cross-server collisions. If a broken MCP server ever did return duplicate tool names, the warning would confusingly say "collides with an existing tool" with no indication that it is a same-server duplicate.

How to fix it

Two options: (1) Remove the check entirely — cross-server collisions are impossible with the prefix scheme, and same-server duplicates are an MCP protocol violation that does not need a warning in example code. (2) Update the warning to clarify the actual scenario, e.g. 'Warning: server X reported duplicate tool name Y — later entry overwrites earlier.'

Step-by-step proof

Suppose two servers 'weather' and 'news' both expose a tool named 'fetch':

• Iteration 1 (name = 'weather'): prefixedName = 'weather__fetch'. Map is empty, no collision. Map now contains 'weather__fetch'.
• Iteration 2 (name = 'news'): prefixedName = 'news__fetch'. Map contains 'weather__fetch' only. has('news__fetch') is false — no collision fires.

The warning can never fire for this cross-server case. The only way it could fire is if the same server returned 'fetch' twice in a single listTools() response, which is an MCP protocol violation that should never happen in practice.