From 05bbfe5074968c03e062115f0bc853df3c4ad700 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 17 Aug 2026 01:25:58 -0400 Subject: [PATCH] fix: read the completions map by own property, not through the prototype Addresses Copilot's round-17 review on #2035. A real crash, and a class the earlier prototype work missed rather than a narrower version of it. `toString`, `constructor` and `__proto__` are valid RFC 6570 variable names, and `completions` starts as `{}`. So for such a name: completions["toString"] ?? [] => the prototype's FUNCTION `??` catches only null and undefined, so that function went to Mantine's Autocomplete as its `data`, and the field crashed on first render with `TypeError: data.map is not a function` -- reproduced by reverting the fix under the new test. The earlier rounds fixed this for the *values* map, which is seeded with every declared variable and so always has the key as an own property. The completions map is the one name-keyed map this component reads WITHOUT having seeded it, which is what made it the surviving instance. Both reads now go through `Object.hasOwn` -- the render, and the stale-dropdown check that clears a variable's options on the next keystroke. Only the autocomplete branch could reach it, so the regression drives that branch: completions supported, a `{toString}` template, type and submit. npm run ci passes. Signed-off-by: cliffhall --- .../ResourceTemplatePanel.test.tsx | 24 +++++++++++++++++++ .../ResourceTemplatePanel.tsx | 23 ++++++++++++++++-- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx index b7d6c934..083a11c6 100644 --- a/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx +++ b/clients/web/src/components/groups/ResourceTemplatePanel/ResourceTemplatePanel.test.tsx @@ -318,6 +318,30 @@ describe("ResourceTemplatePanel", () => { expect(screen.getByText("Still needed: a")).toBeInTheDocument(); }); + it("renders an autocomplete field for a name that collides with Object.prototype", async () => { + // `toString` is a valid RFC 6570 varname and `completions` starts empty, + // so a bare `completions[varName] ?? []` handed Mantine the prototype's + // *function* as its options array (`??` catches only null/undefined) and + // the field crashed on first render. + const user = userEvent.setup(); + const onCompleteArgument = vi.fn().mockResolvedValue(["a", "b"]); + const onReadResource = vi.fn(); + renderWithMantine( + , + ); + // By role, not label: "toString" also appears in the "Still needed" line. + const input = screen.getByRole("textbox", { name: "toString" }); + expect(input).toBeInTheDocument(); + await user.type(input, "v"); + await user.click(screen.getByRole("button", { name: "Read Resource" })); + expect(onReadResource).toHaveBeenCalledWith("x://v"); + }); + it("shows the malformed template unexpanded in the preview", () => { renderWithMantine( , + varName: string, +): string[] { + return Object.hasOwn(completions, varName) ? completions[varName] : []; +} + const HeaderRow = Group.withProps({ justify: "space-between", wrap: "nowrap", @@ -206,7 +224,8 @@ export function ResourceTemplatePanel({ // show ghost suggestions from the old keystroke while the new // request is in flight (300ms debounce + network latency). setCompletions((prev) => { - if (prev[varName] === undefined) return prev; + // Own-property, as above: an inherited member is not a stale dropdown. + if (!Object.hasOwn(prev, varName)) return prev; const next = { ...prev }; delete next[varName]; return next; @@ -328,7 +347,7 @@ export function ResourceTemplatePanel({ description={description} placeholder={`Enter ${varName}`} value={fieldValue} - data={completions[varName] ?? []} + data={completionsFor(completions, varName)} // The server already filtered the values for the typed // prefix; passing options through verbatim avoids hiding // valid suggestions when the input is empty or doesn't