Skip to content

Union branches with an undeclared required name are offered, then can never be submitted #2224

Description

@cliffhall

Found in the v2.5.0 milestone-merge review (#2215), in code that shipped on v2/main during the milestone (#2137). Filed here rather than fixed in the merge PR, whose tree is byte-identical to origin/v2/main.

The bug

core/json/rootUnion.ts's isOfferable decides whether a root-level anyOf/oneOf branch may be shown in the form's branch picker. It checks that the merged branch has at least one property and that every property value is readable — but it never checks that each name in required actually has a property to render.

So a branch like:

{ "properties": { "kind": { "const": "a" } }, "required": ["kind", "payload"] }

is offered. The form renders kind and nothing for payload, because there is no schema for it. missingRequiredFields (clients/tui/src/utils/schemaToForm.ts, called from ToolTestModal.tsx:135) then correctly reports payload missing at submit — permanently, since the user has no field in which to supply it. The picker offers an option that cannot be completed, and the only escape is to pick a different branch.

This is the gap the function already reasons about, one case short

The existing comment on isOfferable works through exactly this hazard for the JSON Schema boolean form:

JSON Schema's boolean form is legal, but only true is harmless — it constrains nothing and answers every keyword lookup with undefined, while false admits no value whatsoever, so a field declared with it can never be filled and a required one makes the whole branch unsatisfiable.

An undeclared required name has the identical outcome — a required field that can never be filled, so the branch is unsatisfiable — and is not checked. The reasoning is right; its coverage is one case short.

Fix

Add the check to isOfferable: every name in the merged required array must resolve to a renderable property in the merged properties. A branch that fails it is not offerable, for the same reason a false-schema required field is not.

Then make the fallback explicit rather than silent. Declining every branch of a union currently leaves the user with a picker and no valid options, which is a different dead end from the one being fixed. The form should fall back to raw JSON input for the whole argument when no branch is offerable — the raw-JSON toggle from #2151 is already there to be reached for, and #2171 / #2175 settled how raw values are handled without being retyped by the schema.

Note this affects both form builders — the shared core/json helper feeds the web and TUI schema forms — so the fix lands once but wants a test on each side. Cover: a branch with an undeclared required name is not offered; a union where every branch fails falls through to raw input rather than an empty picker; and a branch that declares required over names the root already provides stays offerable (the anyOf: [{ required: ["email"] }, …] case the function's comment calls out, which must not regress).

Reported by Copilot on #2215.

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingv2Issues and PRs for v2

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions