Merge pull request #2041 from modelcontextprotocol/v2/fix/2001-duplicate-tool-selection

fix(web): key tool selection by row identity so duplicate tool names are individually inspectable
This commit is contained in:
Cliff Hall 2026-08-17 08:19:42 -04:00 • committed by GitHub
commit 5dd038eb21
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 270 additions and 56 deletions

View file

@ -363,7 +363,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({
InspectorView: (props: { InspectorView: (props: {
toolCallState?: { status?: string }; toolCallState?: { status?: string };
toolsUi?: { toolsUi?: {
selectedToolName?: string; selectedToolKey?: string;
formValues: Record<string, unknown>; formValues: Record<string, unknown>;
search: string; search: string;
}; };
@ -384,7 +384,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({
onConnectionInfo: () => void; onConnectionInfo: () => void;
onToggleConnection: (id: string) => void; onToggleConnection: (id: string) => void;
onToolsUiChange: (next: { onToolsUiChange: (next: {
selectedToolName?: string; selectedToolKey?: string;
formValues: Record<string, unknown>; formValues: Record<string, unknown>;
search: string; search: string;
}) => void; }) => void;
@ -433,7 +433,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({
{Object.keys(props.progressByTaskId ?? {}).join(",") || "none"} {Object.keys(props.progressByTaskId ?? {}).join(",") || "none"}
</span> </span>
<span data-testid="selected-tool"> <span data-testid="selected-tool">
{props.toolsUi?.selectedToolName ?? "none"} {props.toolsUi?.selectedToolKey ?? "none"}
</span> </span>
<span data-testid="tool-search">{props.toolsUi?.search || "none"}</span> <span data-testid="tool-search">{props.toolsUi?.search || "none"}</span>
<span data-testid="selected-prompt"> <span data-testid="selected-prompt">
@ -469,7 +469,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({
formValues: {}, formValues: {},
search: "", search: "",
...props.toolsUi, ...props.toolsUi,
selectedToolName: "get_acts", selectedToolKey: "0:get_acts",
}) })
} }
> >
@ -481,7 +481,7 @@ vi.mock("./components/views/InspectorView/InspectorView", () => ({
formValues: {}, formValues: {},
search: "", search: "",
...props.toolsUi, ...props.toolsUi,
selectedToolName: "other_tool", selectedToolKey: "1:other_tool",
}) })
} }
> >
@ -869,7 +869,7 @@ describe("App session-scoped state reset on disconnect", () => {
expect(screen.getByTestId("tool-status")).toHaveTextContent("ok"), expect(screen.getByTestId("tool-status")).toHaveTextContent("ok"),
); );
// A search keystroke leaves `selectedToolName` unchanged, so the result // A search keystroke leaves `selectedToolKey` unchanged, so the result
// stays put. // stays put.
await user.click(screen.getByText("set-tool-search")); await user.click(screen.getByText("set-tool-search"));
await waitFor(() => await waitFor(() =>

View file

@ -3160,17 +3160,17 @@ function App() {
// drops the previous tool's result — the result panel renders `toolCallState` // drops the previous tool's result — the result panel renders `toolCallState`
// regardless of selection, so without this a stale result would linger under // regardless of selection, so without this a stale result would linger under
// the newly-selected tool (which has no result of its own yet). Search and // the newly-selected tool (which has no result of its own yet). Search and
// form edits keep `selectedToolName` unchanged, so they leave the result be. // form edits keep `selectedToolKey` unchanged, so they leave the result be.
// Depends on `selectedToolName` only (not the whole `toolsUi`), so a search // Depends on `selectedToolKey` only (not the whole `toolsUi`), so a search
// keystroke doesn't churn the callback identity. // keystroke doesn't churn the callback identity.
const onToolsUiChange = useCallback( const onToolsUiChange = useCallback(
(next: ToolsUiState) => { (next: ToolsUiState) => {
if (next.selectedToolName !== toolsUi.selectedToolName) { if (next.selectedToolKey !== toolsUi.selectedToolKey) {
setToolCallState(undefined); setToolCallState(undefined);
} }
setToolsUi(next); setToolsUi(next);
}, },
[toolsUi.selectedToolName], [toolsUi.selectedToolKey],
); );
// --- MCP Apps handlers. Unlike onCallTool (which feeds the Tools panel), // --- MCP Apps handlers. Unlike onCallTool (which feeds the Tools panel),

View file

@ -45,7 +45,8 @@ export const Default: Story = {
export const WithSelection: Story = { export const WithSelection: Story = {
args: { args: {
tools: longToolList, tools: longToolList,
selectedName: "query_database", // Row identity is `index:name`, so the selection names the position too.
selectedKey: "1:query_database",
}, },
}; };

View file

@ -83,7 +83,7 @@ describe("ToolControls", () => {
<ToolControls {...baseProps} onSelectTool={onSelectTool} />, <ToolControls {...baseProps} onSelectTool={onSelectTool} />,
); );
await user.click(screen.getByText("git_status")); await user.click(screen.getByText("git_status"));
expect(onSelectTool).toHaveBeenCalledWith("git_status"); expect(onSelectTool).toHaveBeenCalledWith("2:git_status");
}); });
it("does not invoke onSelectTool when the already-selected tool is clicked", async () => { it("does not invoke onSelectTool when the already-selected tool is clicked", async () => {
@ -92,7 +92,7 @@ describe("ToolControls", () => {
renderWithMantine( renderWithMantine(
<ToolControls <ToolControls
{...baseProps} {...baseProps}
selectedName="git_status" selectedKey="2:git_status"
onSelectTool={onSelectTool} onSelectTool={onSelectTool}
/>, />,
); );
@ -100,6 +100,40 @@ describe("ToolControls", () => {
expect(onSelectTool).not.toHaveBeenCalled(); expect(onSelectTool).not.toHaveBeenCalled();
}); });
it("selects each copy of a duplicated tool name independently", async () => {
// A `tools/list` may repeat a name. Selection is keyed by row identity, so
// selecting the first copy must leave the second clickable and unhighlighted
// (#2001 — with a name-keyed selection both highlighted and neither of the
// later copies could be opened).
const user = userEvent.setup();
const onSelectTool = vi.fn();
const duplicated: Tool[] = [
{ name: "get_weather", inputSchema: { type: "object" } },
{ name: "echo", inputSchema: { type: "object" } },
{ name: "get_weather", inputSchema: { type: "object" } },
];
renderWithMantine(
<ToolControls
{...baseProps}
tools={duplicated}
selectedKey="0:get_weather"
onSelectTool={onSelectTool}
/>,
);
const rows = screen.getAllByText("get_weather");
expect(rows).toHaveLength(2);
// Only the selected row carries the highlight background.
const highlighted = rows
.map((row) => row.closest("button"))
.filter((button) => button?.style.background !== "");
expect(highlighted).toHaveLength(1);
expect(highlighted[0]).toBe(rows[0]?.closest("button"));
// The second copy is still clickable, and reports its own row key.
await user.click(rows[1] as HTMLElement);
expect(onSelectTool).toHaveBeenCalledWith("2:get_weather");
});
it("does not show the list-changed indicator when listChanged is false", () => { it("does not show the list-changed indicator when listChanged is false", () => {
renderWithMantine(<ToolControls {...baseProps} />); renderWithMantine(<ToolControls {...baseProps} />);
expect(screen.queryByText("List updated")).not.toBeInTheDocument(); expect(screen.queryByText("List updated")).not.toBeInTheDocument();

View file

@ -23,13 +23,19 @@ import {
} from "../../elements/ListPaginationControls/ListPaginationControls"; } from "../../elements/ListPaginationControls/ListPaginationControls";
import { ToolListItem } from "../ToolListItem/ToolListItem"; import { ToolListItem } from "../ToolListItem/ToolListItem";
import { useScrollMemory } from "../../../hooks/useScrollMemory"; import { useScrollMemory } from "../../../hooks/useScrollMemory";
import { toolRowKey } from "../../../utils/toolUtils";
export interface ToolControlsProps { export interface ToolControlsProps {
tools: Tool[]; tools: Tool[];
/** Tools the SDK excluded from `tools/list` for invalid `x-mcp-header` /** Tools the SDK excluded from `tools/list` for invalid `x-mcp-header`
* annotations (SEP-2243), shown below the list with the reason (#1632). */ * annotations (SEP-2243), shown below the list with the reason (#1632). */
excludedTools?: ExcludedTool[]; excludedTools?: ExcludedTool[];
selectedName?: string; /**
* The selected row's {@link toolRowKey}, not its name — a `tools/list` may
* repeat a name, and a name-keyed selection would highlight every copy at
* once and make the others unclickable (#2001).
*/
selectedKey?: string;
// Search text is controlled by the parent (App, via ToolsScreen) so it // Search text is controlled by the parent (App, via ToolsScreen) so it
// persists across tab navigation within a live session — see #1417. // persists across tab navigation within a live session — see #1417.
searchText?: string; searchText?: string;
@ -49,7 +55,8 @@ export interface ToolControlsProps {
/** Pagination controls (#1721). */ /** Pagination controls (#1721). */
pagination: ListPaginationControlsProps; pagination: ListPaginationControlsProps;
onSearchChange: (value: string) => void; onSearchChange: (value: string) => void;
onSelectTool: (name: string) => void; /** Emits the clicked row's {@link toolRowKey} (see `selectedKey`). */
onSelectTool: (key: string) => void;
} }
// One excluded tool: a warning icon, the tool name (struck through, since it is // One excluded tool: a warning icon, the tool name (struck through, since it is
@ -116,12 +123,12 @@ const ExcludedTooltip = Tooltip.withProps({
position: "right", position: "right",
}); });
// A server may return the same tool name more than once, so the name alone is // Excluded rows render into the same `Stack` as the list rows above them, so
// not a unique React key — colliding keys let a filtered-out row survive // their keys share one namespace — and an excluded tool's index is counted
// reconciliation instead of unmounting (#1957). The tool's position in the // within its own list, which would collide with a listed tool of the same name
// unfiltered list disambiguates duplicates and stays stable while the search // at the same index. The prefix keeps the two spaces apart.
// narrows, since it is captured before filtering. const excludedRowKey = (name: string, sourceIndex: number) =>
const rowKey = (name: string, sourceIndex: number) => `${sourceIndex}:${name}`; `excluded:${toolRowKey(name, sourceIndex)}`;
/** Matches a tool against the (already lower-cased) search query by name or title. */ /** Matches a tool against the (already lower-cased) search query by name or title. */
const matchesQuery = (tool: Tool, query: string) => const matchesQuery = (tool: Tool, query: string) =>
@ -131,7 +138,7 @@ const matchesQuery = (tool: Tool, query: string) =>
export function ToolControls({ export function ToolControls({
tools, tools,
excludedTools = [], excludedTools = [],
selectedName, selectedKey,
searchText = "", searchText = "",
listChanged, listChanged,
onRefreshList, onRefreshList,
@ -146,14 +153,17 @@ export function ToolControls({
// Stamp each row's source position before filtering, so the key survives the // Stamp each row's source position before filtering, so the key survives the
// list narrowing (#1957). // list narrowing (#1957).
const filteredTools = tools const filteredTools = tools
.map((tool, sourceIndex) => ({ tool, key: rowKey(tool.name, sourceIndex) })) .map((tool, sourceIndex) => ({
tool,
key: toolRowKey(tool.name, sourceIndex),
}))
.filter(({ tool }) => !searchText || matchesQuery(tool, query)); .filter(({ tool }) => !searchText || matchesQuery(tool, query));
// Excluded tools are searchable too, matching name AND title like the main // Excluded tools are searchable too, matching name AND title like the main
// list above, so a filtered view stays consistent. // list above, so a filtered view stays consistent.
const filteredExcluded = excludedTools const filteredExcluded = excludedTools
.map((excluded, sourceIndex) => ({ .map((excluded, sourceIndex) => ({
...excluded, ...excluded,
key: rowKey(excluded.tool.name, sourceIndex), key: excludedRowKey(excluded.tool.name, sourceIndex),
})) }))
.filter(({ tool }) => !searchText || matchesQuery(tool, query)); .filter(({ tool }) => !searchText || matchesQuery(tool, query));
@ -183,9 +193,9 @@ export function ToolControls({
<ToolListItem <ToolListItem
key={key} key={key}
tool={tool} tool={tool}
selected={tool.name === selectedName} selected={key === selectedKey}
onClick={() => { onClick={() => {
if (tool.name !== selectedName) onSelectTool(tool.name); if (key !== selectedKey) onSelectTool(key);
}} }}
/> />
))} ))}

View file

@ -82,6 +82,46 @@ describe("ToolsScreen", () => {
).not.toBeInTheDocument(); ).not.toBeInTheDocument();
}); });
it("opens the second copy of a duplicated tool name and calls it by its protocol name", async () => {
// A `tools/list` may repeat a name with a different schema. Selection is
// keyed by row identity, so the later copy is reachable and the detail
// panel shows *its* schema — a name-based lookup always resolved the first
// (#2001). The wire identity stays the duplicated name.
const user = userEvent.setup();
const onCallTool = vi.fn();
const duplicated: Tool[] = [
{
name: "get_weather",
inputSchema: {
type: "object",
properties: { city: { type: "string" } },
},
},
{
name: "get_weather",
inputSchema: {
type: "object",
properties: { zip: { type: "string", default: "94103" } },
},
},
];
renderWithMantine(
<ControlledToolsScreen tools={duplicated} onCallTool={onCallTool} />,
);
const rows = screen.getAllByText("get_weather");
await user.click(rows[1] as HTMLElement);
// The second copy's own field renders — the first copy's does not.
expect(screen.getByLabelText(/zip/i)).toBeInTheDocument();
expect(screen.queryByLabelText(/city/i)).toBeNull();
await user.click(screen.getByRole("button", { name: /Execute/ }));
expect(onCallTool).toHaveBeenCalledWith(
"get_weather",
{ zip: "94103" },
false,
);
});
it("filters the sidebar list as the search text changes", async () => { it("filters the sidebar list as the search text changes", async () => {
const user = userEvent.setup(); const user = userEvent.setup();
renderWithMantine(<ControlledToolsScreen />); renderWithMantine(<ControlledToolsScreen />);
@ -111,7 +151,7 @@ describe("ToolsScreen", () => {
renderWithMantine( renderWithMantine(
<ToolsScreen <ToolsScreen
{...baseProps} {...baseProps}
ui={{ ...EMPTY_TOOLS_UI, selectedToolName: "alpha" }} ui={{ ...EMPTY_TOOLS_UI, selectedToolKey: "0:alpha" }}
callState={{ callState={{
status: "ok", status: "ok",
result: { content: [{ type: "text", text: "ok" }] }, result: { content: [{ type: "text", text: "ok" }] },
@ -134,7 +174,7 @@ describe("ToolsScreen", () => {
function Host() { function Host() {
const [ui, setUi] = useState<ToolsUiState>({ const [ui, setUi] = useState<ToolsUiState>({
...EMPTY_TOOLS_UI, ...EMPTY_TOOLS_UI,
selectedToolName: "alpha", selectedToolKey: "0:alpha",
}); });
const [callState, setCallState] = useState<ToolsScreenProps["callState"]>( const [callState, setCallState] = useState<ToolsScreenProps["callState"]>(
{ {
@ -264,7 +304,7 @@ describe("ToolsScreen", () => {
renderWithMantine( renderWithMantine(
<ToolsScreen <ToolsScreen
{...baseProps} {...baseProps}
ui={{ ...EMPTY_TOOLS_UI, selectedToolName: "alpha" }} ui={{ ...EMPTY_TOOLS_UI, selectedToolKey: "0:alpha" }}
callState={{ status: "pending" }} callState={{ status: "pending" }}
/>, />,
); );
@ -277,7 +317,7 @@ describe("ToolsScreen", () => {
renderWithMantine( renderWithMantine(
<ToolsScreen <ToolsScreen
{...baseProps} {...baseProps}
ui={{ ...EMPTY_TOOLS_UI, selectedToolName: "alpha" }} ui={{ ...EMPTY_TOOLS_UI, selectedToolKey: "0:alpha" }}
onCancelCall={onCancelCall} onCancelCall={onCancelCall}
callState={{ status: "pending" }} callState={{ status: "pending" }}
/>, />,

View file

@ -16,6 +16,7 @@ import { ToolResultPanel } from "../../groups/ToolResultPanel/ToolResultPanel";
import { ToolCallErrorPanel } from "../../groups/ToolResultPanel/ToolCallErrorPanel"; import { ToolCallErrorPanel } from "../../groups/ToolResultPanel/ToolCallErrorPanel";
import { resultHasResourceLinks } from "../../groups/ToolResultPanel/toolResultUtils"; import { resultHasResourceLinks } from "../../groups/ToolResultPanel/toolResultUtils";
import { collectSchemaDefaults, toFormSchema } from "../../../utils/jsonUtils"; import { collectSchemaDefaults, toFormSchema } from "../../../utils/jsonUtils";
import { findToolByRowKey } from "../../../utils/toolUtils";
export interface ToolCallState { export interface ToolCallState {
status: "idle" | "pending" | "ok" | "error"; status: "idle" | "pending" | "ok" | "error";
@ -35,7 +36,14 @@ export interface ToolCallState {
// as one object so they persist across tab navigation within a live session; // as one object so they persist across tab navigation within a live session;
// the screen unmounts on tab switch, so local state would be lost (#1414/#1417). // the screen unmounts on tab switch, so local state would be lost (#1414/#1417).
export interface ToolsUiState { export interface ToolsUiState {
selectedToolName?: string; /**
* The selected row's `toolRowKey` — its position in the list plus its name,
* not the name alone. A `tools/list` may repeat a name, and identifying the
* selection by name highlighted every copy while the detail panel always
* resolved the first, so the later copies could not be inspected (#2001).
* The protocol name a `tools/call` sends still comes from the resolved tool.
*/
selectedToolKey?: string;
formValues: Record<string, unknown>; formValues: Record<string, unknown>;
search: string; search: string;
// Screen-level "Run as task" toggle, shared across tools (not per-tool): // Screen-level "Run as task" toggle, shared across tools (not per-tool):
@ -168,27 +176,25 @@ export function ToolsScreen({
onClearResult, onClearResult,
onReadResource, onReadResource,
}: ToolsScreenProps) { }: ToolsScreenProps) {
const { selectedToolName, formValues, search } = ui; const { selectedToolKey, formValues, search } = ui;
const selectedTool = selectedToolName const selectedTool = findToolByRowKey(tools, selectedToolKey);
? tools.find((t) => t.name === selectedToolName)
: undefined;
const isExecuting = callState?.status === "pending"; const isExecuting = callState?.status === "pending";
const handleSelectTool = (name: string) => { const handleSelectTool = (key: string) => {
// Seed the form with the tool's schema defaults so default-only fields the // Seed the form with the tool's schema defaults so default-only fields the
// user never edits are still sent on execute (the form shows defaults via // user never edits are still sent on execute (the form shows defaults via
// resolveValue, but onChange only writes edited fields). // resolveValue, but onChange only writes edited fields).
const tool = tools.find((t) => t.name === name); const tool = findToolByRowKey(tools, key);
// `name` always comes from the rendered tools list (ToolControls only emits // `key` always comes from the rendered tools list (ToolControls only emits
// names it was given), so the lookup never misses; the empty-object fallback // keys it computed from the tools it was given), so the lookup never misses;
// is an unreachable defensive default. // the empty-object fallback is an unreachable defensive default.
let nextFormValues: Record<string, unknown> = {}; let nextFormValues: Record<string, unknown> = {};
/* v8 ignore next -- unreachable: onSelectTool always names a tool in the list */ /* v8 ignore next -- unreachable: onSelectTool always keys a tool in the list */
if (tool) if (tool)
nextFormValues = collectSchemaDefaults( nextFormValues = collectSchemaDefaults(
toFormSchema(tool.inputSchema) ?? {}, toFormSchema(tool.inputSchema) ?? {},
); );
onUiChange({ ...ui, selectedToolName: name, formValues: nextFormValues }); onUiChange({ ...ui, selectedToolKey: key, formValues: nextFormValues });
}; };
return ( return (
@ -199,7 +205,7 @@ export function ToolsScreen({
tools={tools} tools={tools}
excludedTools={excludedTools} excludedTools={excludedTools}
malformedListItems={malformedListItems} malformedListItems={malformedListItems}
selectedName={selectedToolName} selectedKey={selectedToolKey}
searchText={search} searchText={search}
listChanged={listChanged} listChanged={listChanged}
onRefreshList={onRefreshList} onRefreshList={onRefreshList}

View file

@ -16,7 +16,7 @@ import { ALL_LEVELS_VISIBLE } from "./LoggingScreen/logLevels";
import { ALL_CATEGORIES_VISIBLE } from "./NetworkScreen/fetchCategories"; import { ALL_CATEGORIES_VISIBLE } from "./NetworkScreen/fetchCategories";
export const EMPTY_TOOLS_UI: ToolsUiState = { export const EMPTY_TOOLS_UI: ToolsUiState = {
selectedToolName: undefined, selectedToolKey: undefined,
formValues: {}, formValues: {},
search: "", search: "",
runAsTask: false, runAsTask: false,

View file

@ -80,7 +80,7 @@ describe("oauthResume", () => {
tabUi: { tabUi: {
Tools: { Tools: {
...EMPTY_TOOLS_UI, ...EMPTY_TOOLS_UI,
selectedToolName: "echo", selectedToolKey: "0:echo",
formValues: { message: "hi" }, formValues: { message: "hi" },
}, },
}, },
@ -96,7 +96,7 @@ describe("oauthResume", () => {
it("builds and restores tab ui snapshots", () => { it("builds and restores tab ui snapshots", () => {
const toolsUi = { const toolsUi = {
...EMPTY_TOOLS_UI, ...EMPTY_TOOLS_UI,
selectedToolName: "get_temp", selectedToolKey: "1:get_temp",
formValues: { city: "NYC" }, formValues: { city: "NYC" },
}; };
const tabUi = buildTabUiSnapshot({ const tabUi = buildTabUiSnapshot({
@ -123,6 +123,40 @@ describe("oauthResume", () => {
expect(setToolsUi).toHaveBeenCalledWith(toolsUi); expect(setToolsUi).toHaveBeenCalledWith(toolsUi);
}); });
it("drops a pre-#2001 selectedToolName from a legacy snapshot, keeping the rest", () => {
// A redirect begun on a build that keyed tool selection by name, restored
// on one that keys it by row identity (the app upgraded mid-redirect).
// The name can't be mapped to a row key here — the tools list is fetched
// after reconnect — so the selection is dropped, not carried through as a
// stray field, and everything else survives.
const setToolsUi = vi.fn();
restoreTabUiFromSnapshot(
{
Tools: {
formValues: { city: "NYC" },
search: "get",
runAsTask: false,
selectedToolName: "get_temp",
},
},
{
setToolsUi,
setPromptsUi: vi.fn(),
setResourcesUi: vi.fn(),
setAppsUi: vi.fn(),
setTasksUi: vi.fn(),
setLogsUi: vi.fn(),
setProtocolUi: vi.fn(),
setNetworkUi: vi.fn(),
},
);
expect(setToolsUi).toHaveBeenCalledWith({
formValues: { city: "NYC" },
search: "get",
runAsTask: false,
});
});
it("falls back to legacy pending server key", () => { it("falls back to legacy pending server key", () => {
storage.set(OAUTH_PENDING_SERVER_KEY, "legacy-srv"); storage.set(OAUTH_PENDING_SERVER_KEY, "legacy-srv");
const snapshot = readOAuthResumeSnapshot(); const snapshot = readOAuthResumeSnapshot();
@ -186,7 +220,7 @@ describe("oauthResume", () => {
it("applyOAuthResumeUi restores tab ui, active tab, and clears in-flight panels", () => { it("applyOAuthResumeUi restores tab ui, active tab, and clears in-flight panels", () => {
const toolsUi = { const toolsUi = {
...EMPTY_TOOLS_UI, ...EMPTY_TOOLS_UI,
selectedToolName: "get_temp", selectedToolKey: "1:get_temp",
formValues: { city: "NYC" }, formValues: { city: "NYC" },
}; };
const snapshot: OAuthResumeSnapshot = { const snapshot: OAuthResumeSnapshot = {

View file

@ -96,6 +96,23 @@ export function buildTabUiSnapshot(
}; };
} }
/**
* A snapshot written before #2001 carries `selectedToolName` (a tool's name)
* where {@link ToolsUiState} now expects `selectedToolKey` (its `index:name`
* row identity). A snapshot only lives for the length of one OAuth redirect,
* so this matters exactly when the app is upgraded mid-redirect — and the name
* cannot be mapped to a row key here, since the tools list is fetched after
* reconnect, long after restore. So the stale selection is dropped rather than
* carried as a stray field that would be re-serialized on the next redirect;
* the rest of the tab state (search text, form values) is restored intact and
* the screen opens on its "select a tool" placeholder.
*/
function normalizeToolsUi(value: unknown): ToolsUiState {
const ui = { ...((value as ToolsUiState | undefined) ?? EMPTY_TOOLS_UI) };
delete (ui as ToolsUiState & { selectedToolName?: string }).selectedToolName;
return ui;
}
export function restoreTabUiFromSnapshot( export function restoreTabUiFromSnapshot(
tabUi: Partial<Record<InspectorTabId, unknown>> | undefined, tabUi: Partial<Record<InspectorTabId, unknown>> | undefined,
setters: TabUiSetters, setters: TabUiSetters,
@ -110,9 +127,7 @@ export function restoreTabUiFromSnapshot(
const value = tabUi[tabId]; const value = tabUi[tabId];
switch (tabId) { switch (tabId) {
case "Tools": case "Tools":
setters.setToolsUi( setters.setToolsUi(normalizeToolsUi(value));
(value as ToolsUiState | undefined) ?? EMPTY_TOOLS_UI,
);
break; break;
case "Prompts": case "Prompts":
setters.setPromptsUi( setters.setPromptsUi(

View file

@ -1,6 +1,11 @@
import { describe, it, expect } from "vitest"; import { describe, it, expect } from "vitest";
import type { Tool } from "@modelcontextprotocol/client"; import type { Tool } from "@modelcontextprotocol/client";
import { hasInputFields, resolveDisplayLabel } from "./toolUtils"; import {
findToolByRowKey,
hasInputFields,
resolveDisplayLabel,
toolRowKey,
} from "./toolUtils";
describe("resolveDisplayLabel", () => { describe("resolveDisplayLabel", () => {
it("returns the title when provided", () => { it("returns the title when provided", () => {
@ -47,3 +52,37 @@ describe("hasInputFields", () => {
).toBe(true); ).toBe(true);
}); });
}); });
describe("toolRowKey / findToolByRowKey", () => {
const tool = (name: string): Tool => ({
name,
inputSchema: { type: "object" },
});
// A `tools/list` may legitimately repeat a name (#1957/#2001).
const tools: Tool[] = [
tool("get_weather"),
tool("echo"),
tool("get_weather"),
];
it("distinguishes two tools that share a name", () => {
expect(toolRowKey("get_weather", 0)).not.toBe(toolRowKey("get_weather", 2));
});
it("resolves each duplicate to its own entry", () => {
expect(findToolByRowKey(tools, "0:get_weather")).toBe(tools[0]);
// The second copy — the one a name-based lookup could never reach.
expect(findToolByRowKey(tools, "2:get_weather")).toBe(tools[2]);
});
it("returns undefined for no key", () => {
expect(findToolByRowKey(tools, undefined)).toBeUndefined();
});
it("returns undefined for a key naming no current row", () => {
// A stale selection after the list changed underneath it.
expect(findToolByRowKey(tools, "9:gone")).toBeUndefined();
// Right name, wrong position: still not a match.
expect(findToolByRowKey(tools, "1:get_weather")).toBeUndefined();
});
});

View file

@ -19,3 +19,35 @@ export function resolveDisplayLabel(name: string, title?: string): string {
export function hasInputFields(tool: Tool): boolean { export function hasInputFields(tool: Tool): boolean {
return Object.keys(tool.inputSchema.properties ?? {}).length > 0; return Object.keys(tool.inputSchema.properties ?? {}).length > 0;
} }
/**
* A tool's stable per-row identity in the Tools sidebar.
*
* A server may return the same tool name more than once, so the name alone
* identifies neither a React child nor a selection: colliding keys let a
* filtered-out row survive reconciliation (#1957), and a name-keyed selection
* highlights both copies at once while the detail panel can only ever resolve
* the first (#2001). The tool's position in the *unfiltered* list disambiguates
* duplicates and stays stable while the search narrows, since it is captured
* before filtering.
*
* This is a UI identity only — the wire identity is still `tool.name`, which is
* what a `tools/call` must send.
*/
export function toolRowKey(name: string, sourceIndex: number): string {
return `${sourceIndex}:${name}`;
}
/**
* Resolves the tool a {@link toolRowKey} refers to, or `undefined` when the key
* names no row in the current list (e.g. the list changed under a stale
* selection). Compares computed keys rather than parsing the key, so the two
* stay in lockstep if the format ever changes.
*/
export function findToolByRowKey(
tools: Tool[],
key: string | undefined,
): Tool | undefined {
if (key === undefined) return undefined;
return tools.find((tool, index) => toolRowKey(tool.name, index) === key);
}

View file

@ -304,7 +304,9 @@ interface LogsDisplayActions {
```typescript ```typescript
interface ExecutionFormState { interface ExecutionFormState {
// Tools // Tools
selectedToolName: string | null; // The selected row's `toolRowKey` (`index:name`), not the tool's name — a
// `tools/list` may repeat a name (#2001).
selectedToolKey: string | null;
toolFormValues: Record<string, unknown>; toolFormValues: Record<string, unknown>;
lastToolResult: ToolResult | null; lastToolResult: ToolResult | null;

View file

@ -646,7 +646,8 @@ will close out as part of that work.
- **Purpose**: Searchable tool list sidebar with list-changed indicator. - **Purpose**: Searchable tool list sidebar with list-changed indicator.
- **Current props**: `tools: ToolListItemProps[]`, `listChanged`, `onRefreshList`, `onSelectTool`. - **Current props**: `tools: ToolListItemProps[]`, `listChanged`, `onRefreshList`, `onSelectTool`.
- **MCP schema touch points**: `Tool`, `ListToolsResult`, `ToolListChangedNotification`. - **MCP schema touch points**: `Tool`, `ListToolsResult`, `ToolListChangedNotification`.
- **Target props**: `tools: Tool[]`, `selectedName?`, `listChanged`, `onRefreshList`, `onSelectTool`. - **Target props**: `tools: Tool[]`, `selectedKey?`, `listChanged`, `onRefreshList`, `onSelectTool`.
- **Row identity**: a `tools/list` may repeat a name, so rows are keyed — for React reconciliation *and* for selection — by `toolRowKey(name, sourceIndex)` (`utils/toolUtils.ts`), not by `tool.name`. `selectedKey` and `onSelectTool` both carry that key (#1957/#2001).
- **Callbacks → core hook**: `useManagedTools`. - **Callbacks → core hook**: `useManagedTools`.
- **Internal refactors**: Consume schema `Tool[]` directly. - **Internal refactors**: Consume schema `Tool[]` directly.
@ -716,14 +717,14 @@ themselves — every callback passed down must originate in a core hook.
- **MCP schema touch points**: `Tool`, `ListToolsResult`, `CallToolRequest.params`, `CallToolResult`, `ToolListChangedNotification`. - **MCP schema touch points**: `Tool`, `ListToolsResult`, `CallToolRequest.params`, `CallToolResult`, `ToolListChangedNotification`.
- **Target props**: - **Target props**:
- `tools: Tool[]` — raw MCP `Tool[]` from `ListToolsResult.tools`. - `tools: Tool[]` — raw MCP `Tool[]` from `ListToolsResult.tools`.
- `selectedToolName?: string` — selection is screen-local; detail derived from `tools`. - `selectedToolKey?: string` — selection is screen-local; detail derived from `tools`. This is the row's `toolRowKey`, not its name: duplicate names would otherwise select every copy at once and resolve only the first (#2001).
- `callState?: { status: 'idle' | 'pending' | 'ok' | 'error'; request?: CallToolRequest['params']; result?: CallToolResult; error?: string }`. - `callState?: { status: 'idle' | 'pending' | 'ok' | 'error'; request?: CallToolRequest['params']; result?: CallToolResult; error?: string }`.
- `listChanged: boolean` — driven by `notifications/tools/list_changed`. - `listChanged: boolean` — driven by `notifications/tools/list_changed`.
- `onRefreshList: () => void`, `onSelectTool: (name: string) => void`, `onCallTool: (name: string, args: Record<string, unknown>) => void`. - `onRefreshList: () => void`, `onSelectTool: (key: string) => void`, `onCallTool: (name: string, args: Record<string, unknown>) => void` — note `onSelectTool` carries the row key (UI identity) while `onCallTool` carries the protocol name (wire identity).
- **Callbacks → core hook**: `useManagedTools` provides `tools`, `listChanged`, `refreshTools`, `callTool`, `lastCallResult`; backed by `useInspectorClient` for the underlying `client.request`. - **Callbacks → core hook**: `useManagedTools` provides `tools`, `listChanged`, `refreshTools`, `callTool`, `lastCallResult`; backed by `useInspectorClient` for the underlying `client.request`.
- **Internal refactors**: - **Internal refactors**:
- Stop flattening `Tool` into `ToolListItemProps` at the screen boundary — pass `Tool` down and let `ToolListItem` project fields. - Stop flattening `Tool` into `ToolListItemProps` at the screen boundary — pass `Tool` down and let `ToolListItem` project fields.
- Derive the selected tool from `tools.find(t => t.name === selectedToolName)` instead of receiving a pre-built `ToolDetailPanelProps`. - Derive the selected tool from `findToolByRowKey(tools, selectedToolKey)` instead of receiving a pre-built `ToolDetailPanelProps`.
- Replace the separate `result` prop with a unified `callState` so pending/error UI can be rendered by `ToolResultPanel`. - Replace the separate `result` prop with a unified `callState` so pending/error UI can be rendered by `ToolResultPanel`.
### PromptsScreen ### PromptsScreen