fix(web): reset a tool form by row identity, not by tool name
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JTHVxSu8AUgHRLvo1ntZ8H Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
parent
121ce4f051
commit
d47071d535
3 changed files with 52 additions and 4 deletions
|
|
@ -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}
|
||||
/>
|
||||
|
||||
|
|
|
|||
|
|
@ -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(<ControlledToolsScreen tools={duplicated} />);
|
||||
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
|
||||
|
|
|
|||
|
|
@ -258,6 +258,9 @@ export function ToolsScreen({
|
|||
<ContentCard>
|
||||
<ToolDetailPanel
|
||||
tool={selectedTool}
|
||||
// The row key, not the name — a server may list the same tool
|
||||
// name twice (#1957), and the panel is reused across selections.
|
||||
resetKey={selectedToolKey}
|
||||
formValues={formValues}
|
||||
isExecuting={isExecuting}
|
||||
progress={callState?.progress}
|
||||
|
|
|
|||
Loading…
Reference in a new issue