From d47071d535c93f9cdf75da82869d03d4e4e37238 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 24 Aug 2026 10:18:55 -0400 Subject: [PATCH] fix(web): reset a tool form by row identity, not by tool name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review round 4 on #2109. A `tools/list` may repeat a name (#1957) and ToolsScreen already disambiguates those rows with a key of its own, but the detail panel handed SchemaForm only the name — so switching between two same-named tools looked like the same entity and left the first one's enlarged fields (and its in-progress number/JSON text, which predates this PR) in place. ToolDetailPanel now takes an optional `resetKey`, defaulting to the name for callers whose list cannot repeat one, and ToolsScreen passes the row key. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JTHVxSu8AUgHRLvo1ntZ8H Signed-off-by: cliffhall --- .../ToolDetailPanel/ToolDetailPanel.tsx | 18 +++++++--- .../screens/ToolsScreen/ToolsScreen.test.tsx | 35 +++++++++++++++++++ .../screens/ToolsScreen/ToolsScreen.tsx | 3 ++ 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.tsx b/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.tsx index 46b92cfa..1984cac3 100644 --- a/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.tsx +++ b/clients/web/src/components/groups/ToolDetailPanel/ToolDetailPanel.tsx @@ -55,6 +55,15 @@ export interface ToolDetailPanelProps { /** Receives the effective run-as-task decision for this execution. */ onExecute: (runAsTask: boolean) => void; onCancel: () => void; + /** + * Identity of the selected *row*, which is not the tool name: a server may + * repeat a name, and `ToolsScreen` disambiguates those rows with a key of its + * own (#1957). Passed straight to `SchemaFormProps.resetKey`, so switching + * between two same-named tools drops the first one's in-progress field text + * and enlarged fields. Falls back to the name, which is the right answer for + * any caller whose list cannot repeat one. + */ + resetKey?: string; } // Outer column: title/annotations pin at top, the Execute footer pins at the @@ -201,6 +210,7 @@ export function ToolDetailPanel({ onFormChange, onExecute, onCancel, + resetKey, }: ToolDetailPanelProps) { const { name, title, description, icons, annotations, inputSchema } = tool; // Narrow the SDK protocol schema to the form renderer's schema type. @@ -333,10 +343,10 @@ export function ToolDetailPanel({ values={formValues} onChange={onFormChange} disabled={isExecuting} - // This panel is reused across tool selections rather than remounted, - // so the form needs the tool name to drop another tool's - // in-progress field text. See SchemaFormProps.resetKey. - resetKey={name} + // This panel is reused across tool selections rather than + // remounted, so the form needs a per-selection key to drop another + // tool's in-progress field text. See SchemaFormProps.resetKey. + resetKey={resetKey ?? name} onValidityChange={setHasInvalidDraft} /> diff --git a/clients/web/src/components/screens/ToolsScreen/ToolsScreen.test.tsx b/clients/web/src/components/screens/ToolsScreen/ToolsScreen.test.tsx index bdf9f978..fa37c28a 100644 --- a/clients/web/src/components/screens/ToolsScreen/ToolsScreen.test.tsx +++ b/clients/web/src/components/screens/ToolsScreen/ToolsScreen.test.tsx @@ -82,6 +82,41 @@ describe("ToolsScreen", () => { ).not.toBeInTheDocument(); }); + it("drops an enlarged field when switching between duplicated tool names", async () => { + // The panel is reused across selections, so SchemaForm resets per-field + // state on `resetKey`. That key must be the ROW identity: two rows sharing + // a name would otherwise look like the same entity, and the first copy's + // enlarged field would stay enlarged over the second copy's (#2042/#1957). + const user = userEvent.setup(); + const duplicated: Tool[] = [ + { + name: "get_weather", + inputSchema: { + type: "object", + properties: { city: { type: "string", title: "City" } }, + }, + }, + { + name: "get_weather", + inputSchema: { + type: "object", + properties: { city: { type: "string", title: "City" } }, + }, + }, + ]; + renderWithMantine(); + const rows = screen.getAllByText("get_weather"); + + await user.click(rows[0] as HTMLElement); + await user.click(screen.getByRole("button", { name: "Enlarge City" })); + expect(screen.getByRole("textbox", { name: /City/ }).tagName).toBe( + "TEXTAREA", + ); + + await user.click(rows[1] as HTMLElement); + expect(screen.getByRole("textbox", { name: /City/ }).tagName).toBe("INPUT"); + }); + 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 diff --git a/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx b/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx index 811e052e..e7bb6084 100644 --- a/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx +++ b/clients/web/src/components/screens/ToolsScreen/ToolsScreen.tsx @@ -258,6 +258,9 @@ export function ToolsScreen({