feat: add multi-server chatbot client example#1691
feat: add multi-server chatbot client example#1691travisbreaks wants to merge 1 commit intomodelcontextprotocol:mainfrom
Conversation
Adds a new standalone example that connects to multiple MCP servers simultaneously, aggregates their tools, and routes tool calls to the correct server. This is the TypeScript equivalent of the Python SDK's simple-chatbot example, addressing issue modelcontextprotocol#740. The example follows the client-quickstart pattern: minimal dependencies (Anthropic SDK + MCP client), standalone package, simple CLI interface. It uses a JSON config file (same format as Claude Desktop) to define servers and includes an agentic loop for multi-step tool use. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@modelcontextprotocol/client
@modelcontextprotocol/server
@modelcontextprotocol/express
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
|
@claude review |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward new example following the existing client-quickstart pattern with only minor nits.
Extended reasoning...
Overview
This PR adds a new examples/client-multi-server/ directory containing a ~170-line CLI chatbot that connects to multiple MCP servers, aggregates tools, and routes tool calls. It also adds the expected config entries (.changeset/config.json ignore list, .prettierignore). No library code is modified — only new example files and the lockfile.
Security risks
None. This is a standalone CLI example that reads a local JSON config and calls the Anthropic API. No auth, crypto, or permissions code is involved. The env passthrough in transport config mirrors the existing quickstart pattern.
Level of scrutiny
Low scrutiny is appropriate. This is self-contained example code that closely mirrors the existing client-quickstart example in structure, dependencies, and patterns. It does not affect any published packages (added to changeset ignore list). The examples/ directory is not covered by CODEOWNERS.
Other factors
All three bug reports are nits on example code with no runtime impact: (1) a type guard narrowing to the wrong SDK type (works due to structural compatibility), (2) no warning on tool name collisions across servers, and (3) a theoretical client leak if listTools fails after connect. These are worth noting as inline comments for the author to consider but do not block merging. The PR fixes an open issue (#740) and the author confirms all tests, typecheck, and lint pass.
| const resultText = result.content | ||
| .filter((c): c is Anthropic.TextBlock => c.type === 'text') | ||
| .map((c) => c.text) | ||
| .join('\n'); |
There was a problem hiding this comment.
🟡 Nit: The type guard on line 115 narrows MCP ContentBlock to Anthropic.TextBlock, which is the wrong type — client.callTool() returns MCP types, not Anthropic SDK types. It works at runtime due to structural compatibility, but consider using a plain .filter((c) => c.type === "text") like the client-quickstart example does (line 93).
Extended reasoning...
What the bug is
On line 115, result.content from client.callTool() returns MCP ContentBlock[] (which is TextContent | ImageContent | AudioContent | ResourceLink | EmbeddedResource). The filter uses a type predicate (c): c is Anthropic.TextBlock => c.type === "text" to narrow these MCP content blocks to Anthropic.TextBlock — a type from the Anthropic SDK, not the MCP SDK.
Why it works today
TypeScript uses structural typing, so as long as the shapes are compatible, the type assertion compiles and behaves correctly at runtime. Both Anthropic.TextBlock and MCP TextContent share { type: "text"; text: string }, and only .text is accessed after the filter, so there is no runtime issue.
Why it should still be fixed
The type predicate is semantically incorrect — it asserts that an MCP content block is an Anthropic type, which is misleading. If either type evolves independently (e.g., MCP adds fields to TextContent that don't exist on Anthropic.TextBlock, or vice versa), the narrowing could silently hide available properties or introduce type errors.
Concrete example
The client-quickstart example handles the same pattern correctly on line 93:
const toolResultText = result.content
.filter((block) => block.type === "text")
.map((block) => block.text)
.join("\n");Here TypeScript narrows the type via the discriminant check without asserting a foreign type, which is both simpler and correct.
Suggested fix
Replace the explicit type predicate with a plain discriminant filter:
const resultText = result.content
.filter((c) => c.type === "text")
.map((c) => c.text)
.join("\n");This is a one-line change that makes the example consistent with client-quickstart and eliminates the cross-package type assertion.
Impact
This is example code with no production impact. The runtime behavior is correct today. This is purely a type-correctness nit to keep the example accurate and maintainable.
| for (const tool of toolsResult.tools) { | ||
| this.toolToServer.set(tool.name, name); | ||
| this.tools.push({ | ||
| name: tool.name, | ||
| description: tool.description ?? '', | ||
| input_schema: tool.inputSchema as Anthropic.Tool.InputSchema, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🟡 Nit: If two servers expose a tool with the same name, toolToServer.set(tool.name, name) silently overwrites the previous server mapping while this.tools.push() still adds both definitions — resulting in duplicate tool entries sent to Claude but all calls routed to only the last-registered server. Consider adding a console.warn when a collision is detected, or prefixing tool names with the server name.
Extended reasoning...
What the bug is
The connectToServers method iterates over each server's tools and populates two data structures: toolToServer (a Map<string, string>) and this.tools (an Anthropic.Tool[] array). When two servers expose a tool with the same name, Map.set() on line 50 overwrites the previous entry, losing the first server's mapping. Meanwhile, Array.push() on lines 51-55 always appends, creating duplicate entries in the tools array.
How it manifests
Consider a concrete scenario: Server A and Server B both expose a tool named "search". After connectToServers completes:
toolToServermaps"search"→ Server B (Server A's mapping is silently lost)this.toolscontains two entries for"search"(one from each server)- Claude sees duplicate tool definitions in the API request
- When Claude calls
"search", it is always routed to Server B; Server A's"search"tool becomes unreachable
Why existing code doesn't prevent it
There is no collision detection anywhere in the tool registration loop. The code unconditionally sets the map entry and pushes to the array without checking whether a tool with that name has already been registered from a different server.
Impact
Since this is a ~170-line example application (not library code), and tool name collisions across different MCP servers are uncommon in practice, the real-world impact is low. However, this example specifically demonstrates multi-server tool routing — its core value proposition — so silent misrouting is a surprising behavior that could confuse users who extend this example with their own servers.
Suggested fix
A simple console.warn when a collision is detected would suffice for example code:
if (this.toolToServer.has(tool.name)) {
console.warn(` Warning: tool "${tool.name}" from "${name}" collides with server "${this.toolToServer.get(tool.name)}" — last server wins`);
}Alternatively, tool names could be prefixed with the server name (e.g., "everything:search") to guarantee uniqueness, though that changes the tool names Claude sees.
| await client.connect(transport); | ||
|
|
||
| // Discover tools from this server | ||
| const toolsResult = await client.listTools(); | ||
| for (const tool of toolsResult.tools) { | ||
| this.toolToServer.set(tool.name, name); | ||
| this.tools.push({ | ||
| name: tool.name, | ||
| description: tool.description ?? '', | ||
| input_schema: tool.inputSchema as Anthropic.Tool.InputSchema, | ||
| }); | ||
| } | ||
|
|
||
| this.servers.set(name, client); | ||
| console.log( | ||
| ` Connected to ${name} with tools: ${toolsResult.tools.map((t) => t.name).join(', ')}` | ||
| ); | ||
| } catch (e) { | ||
| console.error(` Failed to connect to ${name}:`, e); | ||
| throw e; | ||
| } |
There was a problem hiding this comment.
🟡 Nit: If client.listTools() (line 48) throws after client.connect(transport) (line 45), the connected client is never stored in this.servers (line 58), so cleanup() cannot close it. Since this is example code and process.exit() handles cleanup on failure, the practical impact is negligible. A simple fix would be to move this.servers.set(name, client) immediately after connect(), or add await client.close() in the catch block.
Extended reasoning...
Bug Analysis
In connectToServers, the code connects a client on line 45 (await client.connect(transport)) but does not register it in this.servers until line 58, after listTools() and the tool registration loop have completed. If anything in lines 46-57 throws — most likely client.listTools() on line 48 — the client remains connected (with a spawned subprocess) but is never tracked.
Code Path
Here is the concrete scenario:
client.connect(transport)succeeds — a child process is spawned and the MCP session is established.client.listTools()throws (e.g., the server returns an error, or the connection drops mid-request).- Execution jumps to the
catchblock (line 62), which logs the error and re-throws. - The re-thrown error propagates to
main(), which callsprocess.exit(1)in the catch block. cleanup()in thefinallyblock iterates overthis.servers, but the failed client was never added to the map, so it is skipped.
Why Existing Code Doesn't Prevent It
The catch block on line 62 only logs and re-throws — it does not call client.close(). And cleanup() only iterates this.servers, which never received the orphaned client. There is no other mechanism to track or close the leaked client.
Practical Impact
The practical impact is negligible for this example. The main() function calls process.exit(1) on error, which terminates the Node.js process. The OS then reclaims all resources, closes file descriptors, and terminates child processes whose stdin pipes have closed. Additionally, this is demo/example code (~170 lines), not library code, and the failure scenario (listTools throwing after a successful connect) is unusual.
Addressing the Refutation
The refutation correctly points out that process.exit() and OS-level cleanup make this a theoretical rather than practical leak. This is why the severity is "nit" rather than a blocking issue. However, for pedagogical purposes — since this is example code that users will copy and adapt — it is worth noting the pattern so that users building production clients based on this example do not carry the same gap into code where process.exit() is not the fallback.
Suggested Fix
Move this.servers.set(name, client) immediately after await client.connect(transport) (before listTools()). This way, even if tool discovery fails, cleanup() will still close the connected client. Alternatively, add await client.close() in the catch block before re-throwing.
felixweinberger
left a comment
felixweinberger
left a comment
There was a problem hiding this comment.
Thanks for this — the mcpServers config parsing and agentic loop are solid. A few changes before merge:
1. Prefix tool names with server name
The flat Map<toolName, serverName> means if two servers both expose a tool with the same name, the last one silently overwrites the first. Prefix tool names to avoid this:
const prefixedName = `${serverName}__${tool.name}`;
this.toolToServer.set(prefixedName, { serverName, originalName: tool.name });
this.tools.push({ name: prefixedName, ... });Then strip the prefix before calling the server.
2. Default servers.json should have 2+ servers
A multi-server example with one server in the default config doesn't demonstrate the routing. Add @modelcontextprotocol/server-time as a second entry — it works via npx with no extra setup.
3. Fix the content type predicate
filter((c): c is Anthropic.TextBlock => c.type === 'text') — MCP CallToolResult content blocks aren't Anthropic.TextBlock. Use TextContent from @modelcontextprotocol/core.
2 participants
Summary
Adds a
client-multi-serverexample that connects to multiple MCP servers simultaneously and routes tool calls to the correct server. TypeScript equivalent of the Python SDK's simple-chatbot example.Fixes #740
What it does
servers.json(same format as Claude Desktop'smcpServersblock)Implementation
client-quickstartexample pattern (package structure, tsconfig, dependencies)servers.jsonpointing toserver-everythingfor easy testingTest plan