Merge pull request #1961 from modelcontextprotocol/v2/fix/tool-row-keys

fix: key tool rows by source position so duplicate names filter correctly

README.md

@@ -139,6 +139,7 @@ Each config below is a ready-made server for exercising one feature by hand. Loa
 | `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) |
 | `structured-output-http.json`             | Tools tab: a result's `structuredContent` section  | [#1908](https://github.com/modelcontextprotocol/inspector/issues/1908) |
+| `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) |
@@ -213,6 +214,14 @@ Turn on **"Fetch Lists One Page at a Time"** (Server Settings — the `paginated
 
 Run `list_items` from the Tools tab: the result panel shows the `content[]` text summary ("Found 2 items.") **and** a collapsible **Structured Output** section rendering the schema-validated payload as pretty-printed, copyable JSON. That section is what v2 was dropping — a tool declaring an `outputSchema` returns its real data there, and the text block usually only summarizes it. Run `echo` to confirm the section is absent when a result carries no `structuredContent`.
 
+#### 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`.

clients/web/src/components/groups/ToolControls/ToolControls.test.tsx

@@ -204,6 +204,129 @@ describe("ToolControls", () => {
     expect(onRefreshList).toHaveBeenCalledTimes(1);
   });
 
+  // A server may legitimately return the same tool name twice; keying rows by
+  // name alone collides, and React then reuses the stale row instead of
+  // unmounting it when the filter narrows (#1957).
+  const duplicateNameTools: Tool[] = [
+    {
+      name: "get_record",
+      title: "Get Record First",
+      inputSchema: { type: "object" },
+    },
+    {
+      name: "duplicate_tool",
+      title: "First Duplicate",
+      inputSchema: { type: "object" },
+    },
+    {
+      name: "unrelated_tool",
+      title: "Unrelated Tool First",
+      inputSchema: { type: "object" },
+    },
+    {
+      name: "duplicate_tool",
+      title: "Second Duplicate",
+      inputSchema: { type: "object" },
+    },
+    {
+      name: "get_record",
+      title: "Get Record Second",
+      inputSchema: { type: "object" },
+    },
+    {
+      name: "unrelated_tool",
+      title: "Unrelated Tool Second",
+      inputSchema: { type: "object" },
+    },
+  ];
+
+  it("removes every non-matching row when tool names repeat (#1957)", async () => {
+    const user = userEvent.setup();
+    renderWithMantine(<ControlledToolControls tools={duplicateNameTools} />);
+
+    await user.type(screen.getByPlaceholderText("Search tools..."), "get");
+
+    expect(screen.getByText("Get Record First")).toBeInTheDocument();
+    expect(screen.getByText("Get Record Second")).toBeInTheDocument();
+    for (const stale of [
+      "First Duplicate",
+      "Second Duplicate",
+      "Unrelated Tool First",
+      "Unrelated Tool Second",
+    ]) {
+      expect(screen.queryByText(stale)).not.toBeInTheDocument();
+    }
+  });
+
+  // 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(
+      <ControlledToolControls tools={appendedDuplicateTools} />,
+    );
+
+    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 = [
+      {
+        tool: { name: "get_thing", inputSchema: { type: "object" as const } },
+        reason: "a",
+      },
+      {
+        tool: { name: "dupe", inputSchema: { type: "object" as const } },
+        reason: "b",
+      },
+      {
+        tool: { name: "dupe", inputSchema: { type: "object" as const } },
+        reason: "c",
+      },
+      {
+        tool: { name: "get_other", inputSchema: { type: "object" as const } },
+        reason: "d",
+      },
+    ];
+    renderWithMantine(
+      <ControlledToolControls tools={[]} excludedTools={duplicateExcluded} />,
+    );
+
+    await user.type(screen.getByPlaceholderText("Search tools..."), "get");
+
+    expect(screen.getByText("get_thing")).toBeInTheDocument();
+    expect(screen.getByText("get_other")).toBeInTheDocument();
+    expect(screen.queryByText("dupe")).not.toBeInTheDocument();
+  });
+
   it("renders no load error by default", () => {
     renderWithMantine(<ToolControls {...baseProps} />);
     expect(screen.queryByText(/Couldn't load/)).not.toBeInTheDocument();

clients/web/src/components/groups/ToolControls/ToolControls.tsx

@@ -108,6 +108,18 @@ const ExcludedTooltip = Tooltip.withProps({
   position: "right",
 });
 
+// A server may return the same tool name more than once, so the name alone is
+// not a unique React key — colliding keys let a filtered-out row survive
+// reconciliation instead of unmounting (#1957). The tool's position in the
+// unfiltered list disambiguates duplicates and stays stable while the search
+// narrows, since it is captured before filtering.
+const rowKey = (name: string, sourceIndex: number) => `${sourceIndex}:${name}`;
+
+/** Matches a tool against the (already lower-cased) search query by name or title. */
+const matchesQuery = (tool: Tool, query: string) =>
+  tool.name.toLowerCase().includes(query) ||
+  (tool.title?.toLowerCase().includes(query) ?? false);
+
 export function ToolControls({
   tools,
   excludedTools = [],
@@ -122,22 +134,19 @@ export function ToolControls({
 }: ToolControlsProps) {
   const viewportRef = useScrollMemory("tools-sidebar");
   const query = searchText.toLowerCase();
-  const filteredTools = searchText
-    ? tools.filter(
-        (tool) =>
-          tool.name.toLowerCase().includes(query) ||
-          (tool.title?.toLowerCase().includes(query) ?? false),
-      )
-    : tools;
+  // Stamp each row's source position before filtering, so the key survives the
+  // list narrowing (#1957).
+  const filteredTools = tools
+    .map((tool, sourceIndex) => ({ tool, key: rowKey(tool.name, sourceIndex) }))
+    .filter(({ tool }) => !searchText || matchesQuery(tool, query));
   // Excluded tools are searchable too, matching name AND title like the main
   // list above, so a filtered view stays consistent.
-  const filteredExcluded = searchText
-    ? excludedTools.filter(
-        ({ tool }) =>
-          tool.name.toLowerCase().includes(query) ||
-          (tool.title?.toLowerCase().includes(query) ?? false),
-      )
-    : excludedTools;
+  const filteredExcluded = excludedTools
+    .map((excluded, sourceIndex) => ({
+      ...excluded,
+      key: rowKey(excluded.tool.name, sourceIndex),
+    }))
+    .filter(({ tool }) => !searchText || matchesQuery(tool, query));
 
   return (
     <SidebarStack>
@@ -156,9 +165,9 @@ export function ToolControls({
       <ListLoadError error={loadError} what="tools" onRetry={onRefreshList} />
       <SidebarScroll viewportRef={viewportRef}>
         <Stack gap="xs">
-          {filteredTools.map((tool) => (
+          {filteredTools.map(({ tool, key }) => (
             <ToolListItem
-              key={tool.name}
+              key={key}
               tool={tool}
               selected={tool.name === selectedName}
               onClick={() => {
@@ -169,8 +178,8 @@ export function ToolControls({
           {filteredExcluded.length > 0 && (
             <>
               <ExcludedDivider />
-              {filteredExcluded.map(({ tool, reason }) => (
-                <ExcludedTooltip key={tool.name} label={reason}>
+              {filteredExcluded.map(({ tool, reason, key }) => (
+                <ExcludedTooltip key={key} label={reason}>
                   <ExcludedRow>
                     <ExcludedWarningIcon>
                       <RiErrorWarningLine />

clients/web/src/test/integration/mcp/duplicate-tool-names.test.ts

@@ -0,0 +1,171 @@
+import { describe, it, expect, afterEach } from "vitest";
+import path from "node:path";
+import { fileURLToPath } from "node:url";
+import { InspectorClient } from "@inspector/core/mcp/inspectorClient.js";
+import { createTransportNode } from "@inspector/core/mcp/node/transport.js";
+import {
+  createTestServerHttp,
+  type TestServerHttp,
+  createTestServerInfo,
+  createEchoTool,
+  createGetWeatherTool,
+  loadConfig,
+  resolveConfig,
+} from "@modelcontextprotocol/inspector-test-server";
+import type { ServerConfig } from "@modelcontextprotocol/inspector-test-server";
+
+/**
+ * Live coverage of `ServerConfig.duplicateToolNames` (#1957) — the only way this
+ * repo can serve a `tools/list` that repeats a name, since every preset
+ * registers a unique one and the SDK's `registerTool` rejects a repeat.
+ *
+ * The Tools sidebar keyed its rows by `tool.name`, so duplicates collided and
+ * filtering left an unrelated row mounted. The component-level regressions live
+ * in `ToolControls.test.tsx`; this file covers the server option those
+ * screenshots and the manual repro depend on — the wire shape, the ordering
+ * that makes the defect observable, and the config plumbing.
+ */
+describe("duplicate tool names in tools/list (#1957)", () => {
+  let client: InspectorClient | null = null;
+  let server: TestServerHttp | null = null;
+
+  afterEach(async () => {
+    if (client) {
+      try {
+        await client.disconnect();
+      } catch {
+        // ignore
+      }
+      client = null;
+    }
+    if (server) {
+      try {
+        await server.stop();
+      } catch {
+        // ignore
+      }
+      server = null;
+    }
+  });
+
+  async function start(config: Partial<ServerConfig>): Promise<TestServerHttp> {
+    const started = createTestServerHttp({
+      serverInfo: createTestServerInfo("duplicate-tool-names-test", "1.0.0"),
+      tools: [createEchoTool(), createGetWeatherTool()],
+      ...config,
+    });
+    await started.start();
+    server = started;
+    return started;
+  }
+
+  async function connect(url: string): Promise<InspectorClient> {
+    const connected = new InspectorClient(
+      { type: "streamable-http", url },
+      { environment: { transport: createTransportNode } },
+    );
+    await connected.connect();
+    client = connected;
+    return connected;
+  }
+
+  it("emits the named tools twice, repeats appended, second copy titled", async () => {
+    const started = await start({ duplicateToolNames: ["echo"] });
+    const connected = await connect(started.url);
+
+    const { tools } = await connected.listAllTools();
+
+    // Repeats go at the END, not beside their twin. That ordering is
+    // load-bearing: React matches a leading run of same-key children first, so
+    // an adjacent duplicate lines up and the defect hides. Asserting the exact
+    // sequence keeps a future "tidy-up" from silently defanging the fixture.
+    expect(tools.map((t) => t.name)).toEqual(["echo", "get_weather", "echo"]);
+    // These presets carry no title, so the marker falls back to the name —
+    // which is what keeps the two rows distinguishable on screen.
+    expect(tools.at(-1)?.title).toBe("echo (duplicate)");
+    // Only the appended copy is marked; the originals are passed through as-is.
+    expect(tools[0]?.title).toBeUndefined();
+    expect(tools[1]?.title).toBeUndefined();
+  });
+
+  it("leaves the list alone when no names are given", async () => {
+    const started = await start({ duplicateToolNames: [] });
+    const connected = await connect(started.url);
+
+    const { tools } = await connected.listAllTools();
+    expect(tools.map((t) => t.name)).toEqual(["echo", "get_weather"]);
+  });
+
+  it("ignores a name that is not registered", async () => {
+    const started = await start({ duplicateToolNames: ["not_a_tool"] });
+    const connected = await connect(started.url);
+
+    const { tools } = await connected.listAllTools();
+    expect(tools.map((t) => t.name)).toEqual(["echo", "get_weather"]);
+  });
+
+  it("duplicates before paginating, so a pair straddles a page boundary", async () => {
+    const started = await start({
+      duplicateToolNames: ["echo", "get_weather"],
+      maxPageSize: { tools: 2 },
+    });
+    const connected = await connect(started.url);
+
+    // Four tools at a page size of two: the duplicated copies land on page 2,
+    // which only holds if duplication runs before the slice.
+    const firstPage = await connected.listTools();
+    expect(firstPage.tools.map((t) => t.name)).toEqual(["echo", "get_weather"]);
+    expect(firstPage.nextCursor).toBeDefined();
+
+    const { tools } = await connected.listAllTools();
+    expect(tools.map((t) => t.name)).toEqual([
+      "echo",
+      "get_weather",
+      "echo",
+      "get_weather",
+    ]);
+    expect(tools.slice(2).map((t) => t.title)).toEqual([
+      "echo (duplicate)",
+      "get_weather (duplicate)",
+    ]);
+  });
+
+  it("serves the shape the showcase config declares", async () => {
+    // Covers the JSON → ConfigFile → ServerConfig plumbing, not just the
+    // in-process option: a config file is how the manual repro and the
+    // screenshots in #1957 are produced.
+    const configPath = path.resolve(
+      path.dirname(fileURLToPath(import.meta.url)),
+      "../../../../../../test-servers/configs/duplicate-tool-names-http.json",
+    );
+    const resolved = resolveConfig(loadConfig(configPath));
+    expect(resolved.duplicateToolNames).toEqual(["get_weather", "echo"]);
+
+    // Let the harness pick the port instead of the config's fixed one, so this
+    // test can't collide with a manually-running showcase server.
+    const started = await start({
+      tools: resolved.tools,
+      duplicateToolNames: resolved.duplicateToolNames,
+    });
+    const connected = await connect(started.url);
+
+    const { tools } = await connected.listAllTools();
+    expect(tools.map((t) => t.name)).toEqual([
+      "get_weather",
+      "get_temp",
+      "echo",
+      "add",
+      "get_weather",
+      "echo",
+    ]);
+
+    // The whole point of the fixture: filtering by "get" must be able to drop
+    // every non-matching row, duplicates included.
+    const matching = tools.filter(
+      (t) =>
+        t.name.includes("get") ||
+        (t.title?.toLowerCase().includes("get") ?? false),
+    );
+    expect(matching).toHaveLength(3);
+  });
+});

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
+  }
+}

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<string, unknown> = {
@@ -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;

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

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")