fix: address Copilot review round 36 — re-derive the branch when the union changes underneath

`branchIndex` is a position, and a tool refreshed in place keeps its
`resetKey` — so a union reordered or rewritten with the same number of
branches left the index pointing at a different shape than the one the values
describe: the picker showed SMS while Execute submitted the email arguments.
The selection is now re-derived from the values whenever the alternatives
themselves change, keyed by their labels and declared fields.

Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-08-26 14:25:07 -04:00
commit f78b541ce1
2 changed files with 48 additions and 0 deletions

View file

@ -2355,6 +2355,42 @@ describe("SchemaForm multiline strings (#2042)", () => {
);
});
it("re-derives the selection when the branches are reordered in place", () => {
// Same `resetKey`, same branch count — only the order changed, so a
// numeric index would now point at the other shape and the picker would
// show SMS while `values` describe email.
const reversed: InspectorFormSchema = {
...UNION_SCHEMA,
anyOf: [...(UNION_SCHEMA.anyOf ?? [])].reverse(),
};
const values = { note: "hi", kind: "email", address: "a@b.c" };
const { rerender } = renderWithMantine(
<SchemaForm
schema={UNION_SCHEMA}
values={values}
onChange={vi.fn()}
resetKey="same-tool"
/>,
);
expect(screen.getByRole("textbox", { name: /Address/ })).toBeTruthy();
rerender(
<SchemaForm
schema={reversed}
values={values}
onChange={vi.fn()}
resetKey="same-tool"
/>,
);
// Still the email branch — the one the values describe — not whatever
// now sits at the old index.
expect(
(screen.getByRole("textbox", { name: /Variant/ }) as HTMLInputElement)
.value,
).toBe("email");
expect(screen.getByRole("textbox", { name: /Address/ })).toBeTruthy();
});
it("renders no picker for a single-branch union but still shows its fields", () => {
const schema: InspectorFormSchema = {
type: "object",

View file

@ -623,6 +623,18 @@ export function SchemaForm({
const [branchIndex, setBranchIndex] = useState(
() => selectBranchIndex(branches, values) ?? 0,
);
// The alternatives themselves, as a stable key. A tool refreshed in place
// keeps its `resetKey`, so nothing else notices that the union underneath was
// reordered or rewritten — and a numeric index then points at a different
// branch than the one whose values are held, showing SMS while submitting
// email. Re-derived from the values, which is where the answer actually is.
const branchesKey = branches
.map((branch) => `${branch.label}:${branch.declaredFields.join(",")}`)
.join("|");
useValueChange(branchesKey, () =>
setBranchIndex(selectBranchIndex(branches, values) ?? 0),
);
// A form reused for another entity can be handed a shorter union, so the
// index is clamped rather than trusted — `resetKey` resets it below, but a
// caller that omits it (the elicitation panels mount fresh) supplies none.