fix(web): key tool selection by row identity, not name

#1961 keyed the sidebar's React children by source position, but everything
downstream still identified a tool by `tool.name`. With a `tools/list` that
repeats a name, both copies highlighted at once, the click guard made the
other copy a no-op, and `tools.find((t) => t.name === …)` always resolved to
the first — so a later copy rendered but could never be opened.

Thread the row key through selection instead:

- `utils/toolUtils.ts` now owns `toolRowKey` (moved out of ToolControls) and
  adds `findToolByRowKey`, which compares computed keys rather than parsing
  one so the two can't drift.
- `ToolControls`: `selectedName` → `selectedKey`; the highlight and the click
  guard compare keys, and `onSelectTool` emits the row key.
- `ToolsScreen`: `ToolsUiState.selectedToolName` → `selectedToolKey`, with
  both lookups going through `findToolByRowKey`. Renamed through
  `screenUiState.ts` and `App.tsx`'s `onToolsUiChange`.

`onCallTool` still sends `selectedTool.name` — the duplicated name is the
genuine wire identity; only the UI identity needed to be unique.

Also prefixes the excluded (SEP-2243) rows' keys: they render into the same
`Stack` as the list rows but index within their own list, so an excluded tool
sharing a name and index with a listed one collided in the key space.

Closes #2001

Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-17 00:02:13 -04:00
commit ab40fb621a
3 changed files with 210 additions and 48 deletions

View file

@ -82,6 +82,46 @@ describe("ToolsScreen", () => {
).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 () => {
const user = userEvent.setup();
renderWithMantine(<ControlledToolsScreen />);
@ -111,7 +151,7 @@ describe("ToolsScreen", () => {
renderWithMantine(
<ToolsScreen
{...baseProps}
ui={{ ...EMPTY_TOOLS_UI, selectedToolName: "alpha" }}
ui={{ ...EMPTY_TOOLS_UI, selectedToolKey: "0:alpha" }}
callState={{
status: "ok",
result: { content: [{ type: "text", text: "ok" }] },
@ -134,7 +174,7 @@ describe("ToolsScreen", () => {
function Host() {
const [ui, setUi] = useState<ToolsUiState>({
...EMPTY_TOOLS_UI,
selectedToolName: "alpha",
selectedToolKey: "0:alpha",
});
const [callState, setCallState] = useState<ToolsScreenProps["callState"]>(
{
@ -264,7 +304,7 @@ describe("ToolsScreen", () => {
renderWithMantine(
<ToolsScreen
{...baseProps}
ui={{ ...EMPTY_TOOLS_UI, selectedToolName: "alpha" }}
ui={{ ...EMPTY_TOOLS_UI, selectedToolKey: "0:alpha" }}
callState={{ status: "pending" }}
/>,
);
@ -277,7 +317,7 @@ describe("ToolsScreen", () => {
renderWithMantine(
<ToolsScreen
{...baseProps}
ui={{ ...EMPTY_TOOLS_UI, selectedToolName: "alpha" }}
ui={{ ...EMPTY_TOOLS_UI, selectedToolKey: "0:alpha" }}
onCancelCall={onCancelCall}
callState={{ status: "pending" }}
/>,

View file

@ -16,6 +16,7 @@ import { ToolResultPanel } from "../../groups/ToolResultPanel/ToolResultPanel";
import { ToolCallErrorPanel } from "../../groups/ToolResultPanel/ToolCallErrorPanel";
import { resultHasResourceLinks } from "../../groups/ToolResultPanel/toolResultUtils";
import { collectSchemaDefaults, toFormSchema } from "../../../utils/jsonUtils";
import { findToolByRowKey } from "../../../utils/toolUtils";
export interface ToolCallState {
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;
// the screen unmounts on tab switch, so local state would be lost (#1414/#1417).
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>;
search: string;
// Screen-level "Run as task" toggle, shared across tools (not per-tool):
@ -168,27 +176,25 @@ export function ToolsScreen({
onClearResult,
onReadResource,
}: ToolsScreenProps) {
const { selectedToolName, formValues, search } = ui;
const selectedTool = selectedToolName
? tools.find((t) => t.name === selectedToolName)
: undefined;
const { selectedToolKey, formValues, search } = ui;
const selectedTool = findToolByRowKey(tools, selectedToolKey);
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
// user never edits are still sent on execute (the form shows defaults via
// resolveValue, but onChange only writes edited fields).
const tool = tools.find((t) => t.name === name);
// `name` always comes from the rendered tools list (ToolControls only emits
// names it was given), so the lookup never misses; the empty-object fallback
// is an unreachable defensive default.
const tool = findToolByRowKey(tools, key);
// `key` always comes from the rendered tools list (ToolControls only emits
// keys it computed from the tools it was given), so the lookup never misses;
// the empty-object fallback is an unreachable defensive default.
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)
nextFormValues = collectSchemaDefaults(
toFormSchema(tool.inputSchema) ?? {},
);
onUiChange({ ...ui, selectedToolName: name, formValues: nextFormValues });
onUiChange({ ...ui, selectedToolKey: key, formValues: nextFormValues });
};
return (
@ -199,7 +205,7 @@ export function ToolsScreen({
tools={tools}
excludedTools={excludedTools}
malformedListItems={malformedListItems}
selectedName={selectedToolName}
selectedKey={selectedToolKey}
searchText={search}
listChanged={listChanged}
onRefreshList={onRefreshList}

View file

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