diff --git a/README.md b/README.md index 13792764..f650dcdf 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,7 @@ Each config below is a ready-made server for exercising one feature by hand. Loa | `modern-network-http.json` | Network tab: `Mcp-*` headers + error taxonomy | [#1628](https://github.com/modelcontextprotocol/inspector/issues/1628) | | `xmcpheader-modern-http.json` | Tools tab: `x-mcp-header` mirroring and exclusions | [#1632](https://github.com/modelcontextprotocol/inspector/issues/1632) | | `pagination-http.json` | Page-by-page list fetching | [#1721](https://github.com/modelcontextprotocol/inspector/issues/1721) | +| `duplicate-tool-names-http.json` | A `tools/list` that repeats a tool name | [#1957](https://github.com/modelcontextprotocol/inspector/issues/1957) | | `advertised-extensions-http.json` | Tool registration gated on advertised extensions | [#1739](https://github.com/modelcontextprotocol/inspector/issues/1739) | | `logging-{legacy,modern}-http.json` | Logging, both eras | [#1629](https://github.com/modelcontextprotocol/inspector/issues/1629) | | `subscriptions-{legacy,modern}-http.json` | Resource subscriptions, both eras | [#1630](https://github.com/modelcontextprotocol/inspector/issues/1630) | @@ -206,6 +207,14 @@ Under SDK v2 a `tools/call` rejecting with `-32602` renders as a distinct error Turn on **"Fetch Lists One Page at a Time"** (Server Settings — the `paginatedLists` setting, or the **Paginated** switch in a list sidebar) and the lists load page 1 only (4 items) with a **Load next page** control and an _N pages loaded_ status. Each click fetches the next 4 and appends them; Refresh resets to page 1. With the switch off (the default), the same lists auto-aggregate all three pages on connect. +#### Duplicate tool names + +`duplicate-tool-names-http.json` serves `get_weather`, `get_temp`, `echo`, and `add`, then repeats `get_weather` and `echo` at the end of `tools/list` with the same `name` and a `(duplicate)` title (`duplicateToolNames`). No preset can produce this shape — the SDK's `registerTool` rejects a repeated name — but a real server can and does, and the Inspector has to render it faithfully. + +Connect (default legacy era), open the Tools tab, and type `get` into **Search tools**: the list must narrow to exactly the three `get_*` rows. On the broken build it kept a stale `echo` row, because the sidebar keyed rows by `tool.name` alone and the colliding keys orphaned a child during reconciliation ([#1957](https://github.com/modelcontextprotocol/inspector/issues/1957)). + +The duplicated copies are appended rather than placed beside their twin on purpose. React matches a leading run of same-key children first, so a head-adjacent duplicate happens to line up and the defect hides; separating the pair is what makes it observable — and it is also the realistic shape, two tool sources concatenated. + #### Advertised extensions `advertised-extensions-http.json` serves `echo` (always) and a `get_weather` tool **gated on the `io.modelcontextprotocol/tasks` extension** (`extensionGatedTools`): the tool is registered but starts disabled, and the server enables it on `notifications/initialized` only when the client declared that extension in its `capabilities.extensions`. diff --git a/clients/web/src/components/groups/ToolControls/ToolControls.test.tsx b/clients/web/src/components/groups/ToolControls/ToolControls.test.tsx index ef3d0210..b6e379e8 100644 --- a/clients/web/src/components/groups/ToolControls/ToolControls.test.tsx +++ b/clients/web/src/components/groups/ToolControls/ToolControls.test.tsx @@ -258,6 +258,44 @@ describe("ToolControls", () => { } }); + // The shape `test-servers/configs/duplicate-tool-names-http.json` serves: the + // repeats are appended after the whole list rather than sitting beside their + // twin. Distinct from the fixture above and worth its own case — React + // matches a leading run of same-key children first, so where the duplicates + // sit decides which row gets orphaned (#1957). + const appendedDuplicateTools: Tool[] = [ + { name: "get_weather", inputSchema: { type: "object" } }, + { name: "get_temp", inputSchema: { type: "object" } }, + { name: "echo", inputSchema: { type: "object" } }, + { name: "add", inputSchema: { type: "object" } }, + { + name: "get_weather", + title: "get_weather (duplicate)", + inputSchema: { type: "object" }, + }, + { + name: "echo", + title: "echo (duplicate)", + inputSchema: { type: "object" }, + }, + ]; + + it("removes every non-matching row when repeats are appended (#1957)", async () => { + const user = userEvent.setup(); + renderWithMantine( + , + ); + + await user.type(screen.getByPlaceholderText("Search tools..."), "get"); + + expect(screen.getByText("get_temp")).toBeInTheDocument(); + expect(screen.getByText("get_weather (duplicate)")).toBeInTheDocument(); + expect(screen.queryByText("add")).not.toBeInTheDocument(); + // The row the collision orphaned on the broken build. + expect(screen.queryByText("echo")).not.toBeInTheDocument(); + expect(screen.queryByText("echo (duplicate)")).not.toBeInTheDocument(); + }); + it("removes every non-matching excluded row when names repeat (#1957)", async () => { const user = userEvent.setup(); const duplicateExcluded = [ diff --git a/test-servers/configs/duplicate-tool-names-http.json b/test-servers/configs/duplicate-tool-names-http.json new file mode 100644 index 00000000..af1699be --- /dev/null +++ b/test-servers/configs/duplicate-tool-names-http.json @@ -0,0 +1,17 @@ +{ + "serverInfo": { + "name": "duplicate-tool-names", + "version": "1.0.0" + }, + "tools": [ + { "preset": "get_weather" }, + { "preset": "get_temp" }, + { "preset": "echo" }, + { "preset": "add" } + ], + "duplicateToolNames": ["get_weather", "echo"], + "transport": { + "type": "streamable-http", + "port": 3142 + } +} diff --git a/test-servers/src/composable-test-server.ts b/test-servers/src/composable-test-server.ts index 4c836e4b..648fad35 100644 --- a/test-servers/src/composable-test-server.ts +++ b/test-servers/src/composable-test-server.ts @@ -472,6 +472,22 @@ export interface ServerConfig { resourceTemplates?: number; prompts?: number; }; + /** + * Emit the named registered tools **twice** in `tools/list`, with the same + * `name` on both entries and " (duplicate)" appended to the second's title. + * + * Nothing else in this repo can produce this shape: every preset registers a + * unique name, and the SDK's `registerTool` rejects a repeat. But a real + * server can and does return duplicates, and the Inspector has to render that + * faithfully rather than break on it (#1957 — duplicate names collided in the + * Tools sidebar's React keys, so filtering left unrelated rows on screen). + * + * Deliberately a list of names rather than a blanket flag, so a config can + * duplicate part of its tool set and leave the rest alone — that mix is what + * makes a search filter's behavior legible. A name that isn't registered is + * ignored. + */ + duplicateToolNames?: string[]; /** * Gate a tool's visibility in `tools/list` on a client-declared extension * (SEP-2133 `capabilities.extensions`). Maps extension id → tool name (the @@ -1107,16 +1123,45 @@ export function createMcpServer(config: ServerConfig): McpServer { // Set up pagination handlers if maxPageSize is configured const maxPageSize = config.maxPageSize || {}; - // Tools pagination - if (capabilities.tools && maxPageSize.tools !== undefined) { + // Emit each named tool a second time, same `name`, title marked so the two + // rows are told apart on screen. See ServerConfig.duplicateToolNames (#1957). + // + // The second copies go **after** the whole list rather than beside their + // twin, which is both how a real server produces duplicates (two tool sources + // concatenated) and what makes the defect observable: React's child + // reconciliation walks a matching prefix first, so head-adjacent duplicates + // happen to line up and survive. It is the *separated* pair that collides in + // the keyed map and orphans a row. + const duplicateToolNames = new Set(config.duplicateToolNames ?? []); + const withDuplicates = (tools: Tool[]): Tool[] => + duplicateToolNames.size === 0 + ? tools + : [ + ...tools, + ...tools + .filter((tool) => duplicateToolNames.has(tool.name)) + .map((tool) => ({ + ...tool, + title: `${tool.title ?? tool.name} (duplicate)`, + })), + ]; + + // Tools pagination, and/or the duplicate-name override — both need the same + // hand-built list, so the handler is installed when either is configured. + if ( + capabilities.tools && + (maxPageSize.tools !== undefined || duplicateToolNames.size > 0) + ) { mcpServer.server.setRequestHandler("tools/list", async (request) => { const cursor = request.params?.cursor; - const pageSize = maxPageSize.tools!; + // No pagination configured: one page holding everything, so the duplicate + // override can share this handler without inventing a page size. + const pageSize = maxPageSize.tools ?? Number.MAX_SAFE_INTEGER; // Convert registered tools to Tool format, mirroring the SDK's tools/list. // The input-schema JSON comes from the SDK's memoised converter; the // output-schema JSON is the value the SDK cached at registration. - const allTools: Tool[] = []; + const registeredTools: Tool[] = []; for (const [name, registered] of state.registeredTools.entries()) { if (registered.enabled) { const toolDefinition: Record = { @@ -1134,9 +1179,12 @@ export function createMcpServer(config: ServerConfig): McpServer { toolDefinition.outputSchema = registered.outputSchemaJson; } - allTools.push(toolDefinition as Tool); + registeredTools.push(toolDefinition as Tool); } } + // Duplicate before paginating, so a duplicated pair can straddle a page + // boundary exactly as a real server's would. + const allTools = withDuplicates(registeredTools); const startIndex = cursor ? parseInt(cursor, 10) : 0; const endIndex = startIndex + pageSize; diff --git a/test-servers/src/load-config.ts b/test-servers/src/load-config.ts index 8e225200..7ee77a84 100644 --- a/test-servers/src/load-config.ts +++ b/test-servers/src/load-config.ts @@ -66,6 +66,12 @@ export interface ConfigFile { resourceTemplates?: number; prompts?: number; }; + /** + * Names of registered tools to emit **twice** in `tools/list` (same `name`, + * the second's title marked "(duplicate)") — the nonconforming-but-real shape + * no preset can produce. See {@link ServerConfig.duplicateToolNames} (#1957). + */ + duplicateToolNames?: string[]; /** * Gate a tool's `tools/list` visibility on a client-declared extension. Maps * extension id → tool name; the tool appears only when the connected client diff --git a/test-servers/src/resolve-config.ts b/test-servers/src/resolve-config.ts index 491bfed6..bd229c94 100644 --- a/test-servers/src/resolve-config.ts +++ b/test-servers/src/resolve-config.ts @@ -91,6 +91,7 @@ export function resolveConfig(config: ConfigFile): ServerConfig { tasks: config.tasks, tasksExtension: config.tasksExtension, maxPageSize: config.maxPageSize, + duplicateToolNames: config.duplicateToolNames, extensionGatedTools: config.extensionGatedTools, serverType: isHttp ? (transport.type as "sse" | "streamable-http")