test: add a duplicate-tool-names test server for the #1957 repro
Nothing in the repo could serve the shape this fix is about: every preset registers a unique name, and the SDK's `registerTool` rejects a repeat. So the only reproduction was a unit test — there was no way to smoke the real app against a server that returns duplicates. Adds `ServerConfig.duplicateToolNames` (a list of registered tool names) to the composable test server, plumbed through the JSON config, and a showcase config `duplicate-tool-names-http.json`. The named tools are emitted a second time in `tools/list` with the same `name` and a "(duplicate)" title. The repeats are appended after the whole list rather than placed beside their twin. That is both the realistic shape (two tool sources concatenated) and what makes the defect observable: React matches a leading run of same-key children first, so a head-adjacent duplicate happens to line up and the bug hides. Only a separated pair collides in the keyed map and orphans a row — verified both ways against the prod bundle. The `tools/list` override is shared with the existing pagination handler (which already hand-builds the list); with no `maxPageSize` it returns a single unbounded page. Duplication happens before pagination, so a duplicated pair can straddle a page boundary the way a real server's would. Also adds a ToolControls test for this appended ordering, distinct from the reporter's interleaved fixture, and documents the config in the README showcase table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SW1p8E2uiyyLx4RKwrwSrt
This commit is contained in:
parent
6f535356b4
commit
4340283f70
6 changed files with 124 additions and 5 deletions
|
|
@ -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`.
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
<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 = [
|
||||
|
|
|
|||
17
test-servers/configs/duplicate-tool-names-http.json
Normal file
17
test-servers/configs/duplicate-tool-names-http.json
Normal file
|
|
@ -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
|
||||
}
|
||||
}
|
||||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
Loading…
Reference in a new issue