fix: render nullable and union argument fields correctly in the web form and the TUI

[cliffhall]

Closes #1928
Closes #2015
Closes #2007

The bug

A tool argument that is optional and explicitly nullable — Zod's .nullish() / .nullable().optional() — compiles to

{ "anyOf": [{ "type": "string", "enum": ["envio", "recebimento"] }, { "type": "null" }] }

Every branch of the tool-call form's field dispatcher (SchemaForm.renderField) tests a single type string, so a field shaped like that matches nothing and falls through to the raw-JSON JsonInput fallback. That fallback conflates display and storage: its onChange does JSON.parse(text) and, on failure, stores the raw unparsed text, which the next render re-JSON.stringifys — adding a layer of escaping per keystroke until the value is unusable.

The fix

normalizeUnionType already existed in utils/schemaUtils.ts for the FastMCP string|null shape, but it fell short on both counts:

• it copied only the surviving branch's type and dropped everything else, so the enum never reached the dispatcher — a nullable enum normalized to a bare { type: "string" } and rendered as a text box;
• nothing called it on a tool input schema. Its only production caller was resolveRefsInMessage, i.e. elicitation requests.

So:

1. The collapse lives in core/json/nullableUnion.ts as normalizeNullableUnion, and hoists the non-null branch's own keywords (enum, enumNames, items, properties, minimum, …) onto the flattened schema, matching what v1.x did. This also collapses nine near-identical blocks into two, and picks up the object / array cases the type: [X, "null"] form was missing.
2. SchemaForm.renderField normalizes each field before dispatching, so tool, elicitation, and MCP App forms all benefit rather than just elicitation. (The web-only schemaUtils.normalizeUnionType is gone — both its callers now use the core function.)
3. A nullable enum Select gets a clear affordance, so a chosen value can be set back to null. A non-nullable enum keeps none — clearing emits null, which such a schema would reject.

Nothing else changes: a union of two real types, a three-member anyOf, or a branch with no renderable type still falls through to the JSON input, which is the honest representation of a shape the form cannot model.

Screenshots

Driven against the new nullable-fields-http.json showcase server (below), whose record_shipment tool declares all four nullable scalar shapes.

Before — every nullable field is a raw JSON textarea, including the enum:

Before, after typing envio into direction — the compounding escape from the issue:

After — direction is a real Select, reference a text input, quantity a number input (with the branch's minimum/maximum hoisted), express a checkbox:

Test server

Adds a record_shipment preset and a nullable-fields-http.json showcase config (README row + section), so the shape is reproducible by hand instead of needing a hand-rolled server.

The TUI had the same defect — fixed here, not deferred

clients/tui/src/utils/schemaToForm.ts dispatches the same way, and its local JsonSchemaProperty declared only type?: string — no anyOf, no array-valued type — so a .nullish() argument fell to the switch default and lost its select. Milder than the web bug (a plain text field, not a corrupting JSON textarea), but the same class.

Fixing it in a second place is the argument for not having the logic in a first place, so rather than duplicate it the collapse moved into core/json/ (point 1 above) and both form builders now share one copy. normalizeNullableUnion is generic over a minimal NullableUnionSchema — the least a caller must expose to be recognized as a nullable union — so neither client has to adopt the other's schema type.

Verified by driving the built TUI under a pty against the same showcase server. Before, every argument of record_shipment renders as a plain text field:

 ╚[1] Parameters ═════════════════════════════════════════════════════════╝
   │ direction:                                                         │
   ╰─reference: ────────────────────────────────────────────────────────╯
   │ quantity:                                                          │
   ╰─express: ──────────────────────────────────────────────────────────╯

After — direction is a select (No value), express a boolean ([Not set]):

 ╚[1] Parameters ═════════════════════════════════════════════════════════╝
   │ direction: No value                                                │
   ╰─reference: ────────────────────────────────────────────────────────╯
   │ quantity:                                                          │
   ╰─express: [Not set]─────────────────────────────────────────────────╯

(No image: Ink needs a real TTY, which is also why this repo's own smoke:tui is local-only. These are the captured terminal frames.)

#2007 comes along, because this PR widens it

z.array(z.union([z.object(…), z.object(…)])) produces an items.anyOf whose branches have no top-level const, so every MultiSelect option was String(item.const ?? "") — the empty string, twice. Mantine throws on duplicate option values, greying out the whole tool panel (#2007).

That bug predates this branch, but the collapse gives it a new way in: a nullable array of object unions had no top-level type before, so it fell to the safe JSON fallback; now it collapses to type: "array" with items.anyOf and lands straight on the crash. Shipping the collapse without this fix would widen an existing crash, so they belong together. Verified both shapes crash on the intermediate commit, and both are covered by tests now.

toConstOptions returns null unless every branch yields a distinct, non-empty value, and the field then falls through to the JSON editor — the honest widget for a union of object shapes anyway. The string + oneOf path had the identical ?? "" and the same duplicate-throw waiting on it, so it is gated the same way.

Testing

• clients/web/src/test/core/nullableUnion.test.ts — the collapse's own suite (the web project is what gates core/): cases for enum/items/properties hoisting, the typeless-enum branch, branch order, the object/array type-array forms, and the three shapes that must stay untouched.
• clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx — a SchemaForm nullable unions block covering the Select (and its clear-to-null), the absence of a clear button on a plain enum, and the text/number/checkbox/MultiSelect/nested-object/JSON-fallback paths.
• clients/tui/__tests__/schemaToForm.test.ts — a nullable unions block covering the select, the remaining nullable scalars (with the branch's minimum/maximum hoisted), the nullable array-of-enum, the type: [T, "null"] encoding, and the real-union case that must stay a plain field.
• SchemaForm.test.tsx also covers SchemaForm crashes with Mantine "Duplicate options … value """ when array items use anyOf of object schemas #2007: the plain and nullable array-of-object-unions, a duplicate-const pair, a const-less oneOf, and the good case that must still render a MultiSelect.
• npm run ci passes locally. (Two pre-existing flakes surfaced under full-suite load and pass in isolation and on re-run — the TUI OAuth step-up poll test and the AppRenderer theme-flip test; neither touches this change.)

🤖 Generated with Claude Code

https://claude.ai/code/session_01SKGRe3bER3rxfDYU8K4Dyh

[Copilot]

Pull request overview

Fixes nullable JSON Schema fields across web and TUI forms by sharing normalization logic in core/json.

Changes:

• Adds shared nullable-union normalization.
• Updates web and TUI field dispatchers with tests.
• Adds a reproducible nullable-fields test-server preset and documentation.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
core/json/nullableUnion.ts	Adds shared union normalization.
clients/web/src/components/groups/SchemaForm/SchemaForm.tsx	Normalizes fields and enables nullable enum clearing.
clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx	Tests nullable web controls.
clients/web/src/utils/schemaUtils.ts	Replaces the web-only normalizer.
clients/web/src/utils/schemaUtils.test.ts	Removes superseded tests.
clients/web/src/test/core/nullableUnion.test.ts	Tests shared normalization behavior.
clients/tui/src/utils/schemaToForm.ts	Normalizes TUI field schemas.
clients/tui/__tests__/schemaToForm.test.ts	Tests nullable TUI controls.
test-servers/src/test-server-fixtures.ts	Adds the nullable-fields tool.
test-servers/src/preset-registry.ts	Registers the new preset.
test-servers/configs/nullable-fields-http.json	Adds a showcase server configuration.
README.md	Documents reproduction and expected behavior.
AGENTS.md	Documents the new core utility.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[cliffhall]

Copilot review round 1 — all four addressed (e4e16bb)

Mirroring the inline replies here, since those threads go hidden once the fix is pushed. All four findings were valid; two of them (2 and 3) were real bugs, not just type hygiene.

#	Finding	Resolution
1	Returning T is unsound — type and anyOf both change	Return type is now T | NormalizedNullableUnion<T>
2	A typeless enum does not imply strings	Inference gated on an all-string, non-empty enum
3	Required + nullable + cleared → submit stuck disabled	null counts as present when the schema admits it
4	collectSchemaDefaults reads the raw union	It normalizes each property first, like the renderer

1 — unsound T. The normalized arm is Omit<T, "type" | "anyOf"> & { type?: RenderableType; anyOf?: undefined; nullable?: boolean }, naming exactly the two fields the collapse changes. I used RenderableType (the six names it can emit) over a conditional PlainType<T["type"]>: both are sound, but the conditional left an unresolved Omit TS could not compare at the construction site, and it is weaker anyway — the six literals are a subset of what either caller declares, so the result stays assignable to InspectorFormSchema and the recursive SchemaForm call still typechecks. One assertion remains, isolated in a collapsed() helper with a comment.

What this does not reach: the hoist cannot be made sound, because an anyOf branch is unknown and any keyword lifted off it is whatever the server sent, however T declares it. That is why finding 2 matters — enum is the one hoisted keyword the renderers dereference as a typed array, and it is now validated at runtime.

2 — enum inference. A bug I introduced. { enum: [1, 2] } would have been read as a string enum, handing numbers to a Mantine Select declared string[] and making the TUI submit "1" where the server expects 1. Now only an all-string, non-empty enum earns the inference. An explicit type stays authoritative ({ type: "number", enum: [1, 2] } still collapses — the members are the server's declaration, not my guess), and enum: [] is treated as unknown rather than passing every vacuously.

3 — required nullable fields. A bug my own clear button created: before this PR nothing could produce a null, so the "null means missing" shortcut was moot. required constrains presence, not content, so null now counts as present exactly when normalizeNullableUnion(fieldSchema).nullable === true. Deliberately unchanged: a non-nullable required field still rejects null; undefined/"" are still missing regardless (the empty-string heuristic is a separate concern); and a required field the schema does not describe still rejects null, since nothing says it is permitted.

4 — defaults. The sharp edge here is that the form displays a hoisted default while the seeded values omit it, so the field silently submits empty — wrong data, not cosmetics. I normalized per-consumer rather than once up front: the two walk the schema differently (SchemaForm recurses through nested SchemaForm instances, collectSchemaDefaults through its own recursion), so a single pre-pass would have to deep-normalize the whole tree eagerly and both would still need to agree on where that happened. The collapse is idempotent, so normalizing at each read is cheap and local.

npm run ci is green (4,972 web tests, all smokes, Storybook).

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

clients/web/src/utils/jsonUtils.ts:163

• normalizeNullableUnion is intentionally narrower than “admits null”: it leaves a three-member union unchanged. Consequently, a required field such as anyOf: [{ type: "string" }, { type: "number" }, { type: "null" }] renders the JSON fallback and parses an entered null, but this check still marks it missing and disables App/elicitation submission even though the schema accepts it. Determine null admission independently of renderer normalization (including null branches in larger unions).

    if (value === null) {
      const fieldSchema = properties[field];
      return fieldSchema === undefined
        ? true
        : normalizeNullableUnion(fieldSchema).nullable !== true;

test-servers/src/preset-registry.ts:97

• The new preset path is not exercised by an automated test: the added web/TUI tests construct schemas directly, so neither record_shipment/nullable_fields resolution nor the Zod-generated showcase schema is verified. Add a resolver/config test that loads this preset and asserts the nullable schema shape; otherwise a registry typo or Zod output change can silently break the documented reproduction server.

    case "record_shipment":
    case "nullable_fields":
      return createNullableFieldsTool();

[cliffhall]

Copilot review round 3 — all three addressed (34ba825)

One inline comment plus two in the suppressed block, which are easy to miss and were both real. All three valid.

1 — non-string const in toConstOptions (inline). Same class as round 2's enum finding: items: { anyOf: [{ const: 1 }, { const: 2 }] } submitted ["1"] where the schema says [1]. Only a string const is selectable now; anything else falls to the JSON editor, where the value keeps its type. I declined the "preserve and map back" alternative — it means threading a parallel value↔const map through MultiSelect's string-only value/onChange, and an inspector reporting ["1"] when it sent [1] is the worse failure to leave in place. The guard now does double duty: a const-less object branch fails the same check, which is what keeps #2007 from crashing.

2 — null admission was derived from the renderer (suppressed). The sharper half of the review. hasMissingRequiredFields asked normalizeNullableUnion(...).nullable, but that collapse is narrow on purpose — two-member unions only. So anyOf: [string, number, null] renders through the JSON fallback, where null is perfectly typeable, and was still marked missing: submit disabled on a value the schema accepts.

The underlying mistake was conflating two questions. "Can this become one widget?" is a rendering question with good reasons to be conservative. "Does this permit null?" is a validity question with no such limit. They now have separate functions: admitsNull in core recognizes nullable: true, type: "null", type: [..., "null"], and a null branch anywhere in an anyOf/oneOf of any size.

3 — the preset had no automated coverage (suppressed). The web and TUI tests build the anyOf shape by hand. That verifies the renderers but assumes the premise the entire fix rests on: that Zod's .nullish() emits anyOf: [<branch>, { type: "null" }] with the enum on the branch. Nothing pinned it, so a Zod output change or a registry typo would leave every unit test green while the documented reproduction server quietly stopped reproducing the bug.

Added src/test/integration/mcp/nullable-fields.test.ts: boots the preset over real HTTP, asserts the wire shape (direction has no top-level type or enum; its anyOf has two members, one of them { type: "null" }), then runs each property back through the collapse and checks the four widget types. This follows duplicate-tool-names.test.ts, which exists for exactly this reason (#1957) — covering the server fixture a manual repro depends on.

npm run ci green: 4,986 web tests, 321 files, all smokes, Storybook.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (2)

core/json/nullableUnion.ts:219

• The type: [T, "null"] path leaves top-level enum values untouched. A valid nullable enum is commonly encoded as { type: ["string", "null"], enum: ["envio", "recebimento", null] }; after this collapse, the web dispatcher passes the remaining null to Mantine Select as option data, while the TUI's isStringEnum check rejects the enum and renders a plain text field. Handle the enum's null sentinel when collapsing or in both dispatchers so this second supported nullable encoding also produces the intended select without invalid option data.

  if (
    Array.isArray(schema.type) &&
    schema.type.length === 2 &&
    schema.type.includes("null")
  ) {
    const type = schema.type.find((member) => member !== "null");
    if (isRenderableType(type)) {
      return collapsed(schema, undefined, type);

clients/web/src/test/integration/mcp/nullable-fields.test.ts:55

• This test bypasses both new integration points by constructing createNullableFieldsTool() directly. Consequently, the preset registry could misspell record_shipment, or nullable-fields-http.json could reference an invalid preset, and this purported end-to-end test would still pass—the exact regression its header says it pins. Load and resolve the checked-in config here (as duplicate-tool-names.test.ts does) before starting the server so the new registry/config wiring is actually covered.

    const started = createTestServerHttp({
      serverInfo: createTestServerInfo("nullable-fields-test", "1.0.0"),
      tools: [createNullableFieldsTool()],

[cliffhall]

Copilot review round 4 — both addressed (8356033)

No new visible comments this round; both findings were in the suppressed block, and both were real.

1 — the enum's null sentinel survived the type: [T, "null"] collapse. A genuine hole in the second supported encoding, and a good catch precisely because it fails differently in each client. That encoding keeps its keywords at the top level, so a nullable enum written that way carries the null inside the list:

{ "type": ["string", "null"], "enum": ["envio", "recebimento", null] }

Collapsing type while leaving the list alone gave the web dispatcher a null to hand Mantine as option data, and made the TUI's all-strings check reject the entire enum and fall back to a plain text field. Two different wrong answers from one omission — exactly the divergence centralizing the collapse was meant to prevent, which makes this the right layer to fix it at.

The collapse now strips a null member whenever it flattens, since that fact has already moved onto nullable. An enum of nothing but null drops out entirely rather than becoming an empty select: it offers no value a dropdown could show, so the plain widget serves the field better. Covered in nullableUnion.test.ts and end-to-end in both dispatchers.

2 — the new integration test did not test what its header claimed. Fair, and the more useful of the two. I wrote that the test pins the registry and config wiring, then built the server with createNullableFieldsTool() directly — so a misspelt case "record_shipment" or a config naming a preset that no longer exists would have left it green while the documented reproduction silently broke. That is the exact regression the header promised to catch.

It now goes through loadConfig → resolveConfig on the checked-in nullable-fields-http.json, matching duplicate-tool-names.test.ts, and asserts the resolved tool names before booting. The port still comes from the harness rather than the config, so it cannot collide with a showcase server someone is running by hand.

npm run ci green: 4,992 web tests across 321 files, all smokes, Storybook.

[cliffhall]

Copilot review round 7 — both addressed (987279b)

One visible comment, one suppressed. They are the same principle at two levels: JSON Schema siblings are conjunctive, and this code was treating them as alternatives.

1 — the hoist dropped wrapper constraints. The merge is a spread, and a spread is replacement. A wrapper enum: ["a"] around a branch enum: ["a", "b"] means "a" (both must hold), but the spread produced ["a", "b"] — a dropdown offering, and submitting, a value the schema rejects. type, bounds, and the object keywords fail identically.

Worth being clear about the provenance: "the branch wins on any shared key" came from v1.x, and I carried it over and documented it as intentional. It is only safe because the wrapper in practice holds title/description/default and nothing else. It was never safe in general, and I asserted otherwise.

The collapse now declines when wrapper and branch hold different values for the same validation keyword, leaving the field on the JSON editor with its schema intact. I chose declining over intersection deliberately: intersecting means handling enum×enum, type, bounds, pattern, items, properties, required — a much larger surface where a subtly wrong intersection is worse than none, because it renders a plausible widget for the wrong constraint.

Two things keep the guard from over-firing: identical values are not a conflict (so the ordinary redundant-type shape still collapses), and metadata — title, description, default, enumNames — is exempt, since nothing validates against it. Comparison is canonical JSON, so key-order differences read as a conflict: conservative in the safe direction.

2 — an explicit type was outvoted by a null branch. { type: "string", anyOf: [{ type: "string", enum: ["a"] }, { type: "null" }] } rejects null, because the value must satisfy the type and the union. admitsNull fell through to the branch scan and said yes, which meant a clear button emitting null and required-field gating accepting it. An explicit type now decides.

Together with round 6's sibling-enum fix, nullability is now derived from the whole schema rather than from whichever keyword happened to be checked first.

npm run ci green: 5,015 web tests across 322 files, all smokes, Storybook.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

core/json/nullableUnion.ts:141

• This conflict whitelist omits validation applicators such as oneOf, allOf, and not. If both the wrapper and surviving branch define oneOf, the branch spread replaces the wrapper constraint; for example, a wrapper allowing only "a" can normalize to a Select offering branch value "b", which the original schema rejects. Please block conflicting non-annotation keys conservatively (or cover the full validation/applicator set) rather than relying on this incomplete whitelist.

const VALIDATION_KEYWORDS = [
  "type",
  "enum",
  "const",
  "items",

clients/web/src/components/groups/SchemaForm/SchemaForm.tsx:250

• Invalid draft text is propagated as undefined, but no validity state reaches the submit controls. Optional invalid fields therefore leave Open/Submit enabled and are silently omitted; in ToolDetailPanel, Execute is gated only by isExecuting, so even a required invalid JSON field can be sent without the argument. The visible error does not prevent the incorrect request. Expose draft validity to all SchemaForm callers and disable their action while any draft is invalid.

      onChange={(text) => {
        setDraft(text);
        onChange(parseJsonDraft(text));

[cliffhall]

All three false positives are real — fixed in 2fdbd19, and I took the "conservatively decline" option you offered rather than trying to evaluate them.

• { type: "null", const: "x" } is unsatisfiable: the branch check now runs the same sibling test on the branch itself, so naming null is no longer sufficient.
• not and allOf at the top level can rule null out; both now return false outright.
• oneOf requires exactly one branch to match, so a null branch does not by itself mean null validates. Dropped from the scan entirely rather than half-evaluated.

The asymmetry is what decides the direction: under-claiming nullability costs a clear button and treats a valid null as missing; over-claiming lets the form emit a value the schema rejects. For an inspector the second is the worse failure, so every shape this module cannot evaluate now answers false.

This round also made me change the shape of the other guard rather than extend it — see the suppressed comment about VALIDATION_KEYWORDS. You were right that it was an incomplete whitelist, and more importantly it was incomplete in the failing-open direction. It is now an allowlist of annotation keywords, so an unrecognized keyword declines instead of being dropped. That is the third round in a row on conjunctive semantics, and the pattern was that each fix was a new special case; inverting both defaults to fail-closed is the answer that does not need a fourth.

[cliffhall]

Copilot review round 8 — two fixed, one deferred with a correction (2fdbd19)

Rounds 5–8 have all been the same conjunctive-semantics seam opened one level deeper each time, and each fix was a new special case. So this round changes the shape of both guards rather than adding another: they now fail closed. Anything this module cannot evaluate sends the field to the JSON editor instead of to a widget built from a partial reading.

1 — the conflict whitelist was incomplete in the failing-open direction. It enumerated validation keywords and declined when two disagreed, which meant any keyword nobody listed — oneOf, allOf, not, $ref, a future vocabulary — was silently safe to drop. Replaced with an allowlist of annotation keywords (title, description, default, enumNames, …): the collapse declines whenever the wrapper carries anything beyond annotations and the union itself.

Stricter than the old equality test — a wrapper merely restating its branch's type now declines too. That costs a dropdown on a rare redundant shape and buys a rule with no gap, which is the right trade for code that cannot evaluate JSON Schema.

2 — admitsNull claimed shapes it had not evaluated. All three of the reported false positives were real: { type: "null", const: "x" } is unsatisfiable, top-level not / allOf can rule null out, and oneOf requires exactly one branch to match so a null branch does not by itself mean null validates. All now answer false.

The asymmetry decides the direction: under-claiming nullability costs a clear button and treats a valid null as missing; over-claiming lets the form emit a value the schema rejects. For an inspector, the second is the worse failure.

---

3 — deferred, and a correction to what I said in round 6

The suppressed comment about draft validity not reaching the submit controls is not fixed here, and it is worse than I described.

I claimed a required field was already gated, because its value is undefined and hasMissingRequiredFields treats that as missing. That holds for AppDetailPanel and ElicitationFormPanel. It does not hold for ToolDetailPanel — the Tools tab, the primary surface for #1928 — which never calls hasMissingRequiredFields at all and gates Execute on isExecuting alone (ToolDetailPanel.tsx:344-345). So a required JSON field holding invalid text really can be executed with the argument simply absent, exactly as the comment says.

Two distinct gaps, both now recorded on #2020 with the corrected framing:

1. ToolDetailPanel has no required-field gating at all — a smaller, more urgent fix using a function that already exists, and not specific to the JSON editor (an empty required text field has the same problem today).
2. Draft validity does not leave SchemaForm, so no caller can gate on it. SchemaNumberInput reports undefined the same way, so the channel should be general.

Both are behavior changes to the submit path across four components, which is beyond what a fix for #1928/#2007/#2015 should carry — but the corrected severity is on the issue rather than left as I first wrote it.

npm run ci green: 5,019 web tests across 322 files, all smokes, Storybook.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (4)

core/json/nullableUnion.ts:306

• The documentation says oneOf null branches are recognized, but the implementation deliberately returns false whenever oneOf is present. Document the conservative behavior so callers do not rely on null admission that the function never reports.

 * Recognizes every encoding the collapse does, plus the ones it declines:
 * `nullable: true`, `type: "null"`, `type: [..., "null"]`, and a `"null"`
 * branch anywhere in an `anyOf` or `oneOf` of any size.

core/json/nullableUnion.ts:335

• An explicit type that includes null does not by itself prove the whole schema accepts null; sibling keywords remain conjunctive. { type: ["string", "null"], anyOf: [{ type: "string" }] } rejects null, but this returns true, so required-field gating accepts an invalid null. If anyOf is present, verify it rather than returning immediately, and update the test that currently expects this false positive.

  if (schema.type !== undefined) {
    return typeNamesNull(schema.type);
  }

core/json/nullableUnion.ts:351

• A null-typed branch can still be unsatisfiable because of its own nested anyOf. For example, { type: "null", anyOf: [{ type: "string" }] } currently passes this check, making a normalized Select clearable even though the original schema rejects null. Treat nested anyOf like the other unevaluated applicators and add a regression test.

      return (
        !nullExcludedBySiblings(branchSchema) &&
        branchSchema.not === undefined &&
        branchSchema.allOf === undefined &&
        branchSchema.oneOf === undefined
      );

core/json/nullableUnion.ts:412

• The type: [T, "null"] path can also carry an independent sibling anyOf, but collapsed() always clears anyOf. For example, { type: ["string", "null"], anyOf: [{ const: "a" }] } is widened to an unconstrained string field. Conservatively leave this compound schema unnormalized instead of dropping its constraint, with a regression test.

  if (
    Array.isArray(schema.type) &&
    schema.type.length === 2 &&
    schema.type.includes("null")
  ) {

[cliffhall]

Copilot review round 9 — all four addressed (274a328)

No visible comments; four in the suppressed block. All valid, and all of them the same defect: round 8's fail-closed inversion was the right idea enforced only at the positions I happened to be editing. The rule now holds everywhere a schema is inspected.

1 — an explicit type returned early, ignoring a sibling anyOf. They are conjunctive, so { type: ["string", "null"], anyOf: [{ type: "string" }] } admits no null at all. A sibling union now withholds the claim.

2 — a null-typed branch can be unsatisfiable via its own nested anyOf. The branch check tested not/allOf/oneOf but not anyOf, so { type: "null", anyOf: [{ type: "string" }] } passed. A nested union is now equally opaque.

3 — the type: [T, "null"] collapse path had no applicator guard at all. I added the guard to the anyOf path in round 7 and never to this one. Since collapsed clears anyOf unconditionally and the renderers ignore the other applicators, { type: ["string", "null"], anyOf: [{ const: "a" }] } was widened into an unconstrained string field. It declines now, like its sibling path.

4 — the admitsNull docstring advertised oneOf support that round 8 removed. A stale doc is worse than no doc when the subject is what a function refuses to guarantee, so it is rewritten around exactly that: what is recognized, what is declined and why, and the asymmetry that settles the direction.

The three code fixes now share one hasOpaqueApplicator helper rather than repeating the condition, so the next position that needs it cannot drift out of step — which is what happened here three times.

On the shape of this review

Rounds 5–9 have all worked the same seam: this module inspects JSON Schema without evaluating it, and every guard is a place that can be incomplete. Two things are worth recording rather than leaving implicit.

First, the fixes have converged. Rounds 5–7 were new special cases each time; round 8 changed the defaults to fail closed; round 9 is that same rule applied at the three positions I missed, sharing a helper so there is no fourth set of positions to miss. That is a different trajectory from whack-a-mole.

Second, the remaining risk is bounded in the safe direction. Every unresolved case now renders through the JSON editor with its constraints intact rather than through a widget built from a partial reading. The cost is a dropdown the user does not get on an exotic schema; the thing avoided is an inspector that misreports what it sends.

npm run ci green: 5,023 web tests across 322 files, all smokes, Storybook.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (1)

core/json/nullableUnion.ts:427

• A non-null branch can itself contain anyOf, but this guard still collapses it. In collapsed(), the branch's nested anyOf overwrites the outer one during the spread and is then set to undefined, widening (for example) { anyOf: [{ type: "string", anyOf: [{ const: "a" }] }, { type: "null" }] } into an unconstrained nullable string field. Conservatively decline normalization when the surviving branch has its own anyOf, just as the type-array path already does for sibling applicators.

    if (nullBranch && branch && !wrapperCarriesConstraints(schema)) {

[cliffhall]

Copilot review round 10 — addressed (562709a)

One suppressed finding, valid: the non-null branch can carry its own anyOf, and the hoist drops it — the branch's anyOf overwrites the wrapper's in the spread and is then cleared, so { anyOf: [{ type: "string", anyOf: [{ const: "a" }] }, { type: "null" }] } widened into an unconstrained nullable string.

Rather than add a fourth inline condition, the four sites that ask "does this schema compose something I cannot evaluate?" now share one predicate, hasUnevaluatedComposition (an opaque applicator, or a nested anyOf):

• the surviving branch in the anyOf collapse — this round's finding,
• the null branch in admitsNull — round 9,
• the type: [T, "null"] collapse path — round 9,
• and, through hasOpaqueApplicator, the top of admitsNull — round 8.

Every one of those was missed independently, for the same reason each time: the guard existed but was written inline at each site, so fixing one did not fix the others. Sharing the predicate is the part that matters more than this round's individual fix.

Where this leaves the module

Ten rounds, 22 findings, all valid. The arc is worth stating plainly because it is not the same thing repeating:

• Rounds 1–4 — distinct defects across the change (unsound return type, enum inference, required-field gating, defaults, SchemaForm crashes with Mantine "Duplicate options … value """ when array items use anyOf of object schemas #2007, preset coverage).
• Rounds 5–7 — three variations on conjunctive semantics, each fixed as its own case.
• Round 8 — the realization that case-by-case was the wrong shape; both guards inverted to fail closed.
• Rounds 9–10 — that rule applied at the four positions it had not reached, now factored so there is no fifth.

What remains unhandled is unhandled deliberately and visibly: this module recognizes two nullable-union encodings and refuses everything else, and "refuses" means the field renders through the JSON editor with its constraints intact. The failure mode left is a user not getting a dropdown on an exotic schema. The failure mode avoided is an inspector that shows a widget for a constraint the server did not send.

Two items remain open and are tracked rather than carried here: #2020 (draft validity never reaches the submit controls, plus ToolDetailPanel having no required-field gating at all) and #2015's TUI array-of-enum path, noted inline on that round's reply.

npm run ci green: 5,025 web tests across 322 files, all smokes, Storybook.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (3)

core/json/nullableUnion.ts:270

• An empty enum also needs to stop normalization. In anyOf: [{ type: "string", enum: [] }, { type: "null" }], the original schema permits only null, but this path returns unchanged, producing a clearable Select with no options. A required field then has no interaction that can write the explicit null needed to satisfy it. Conservatively decline the collapse for an empty enum just as for [null].

  if (kept.length === members.length) {
    return { kind: "unchanged" };

core/json/nullableUnion.ts:281

• When enumNames was already length-mismatched, preserving it can accidentally make it look aligned after null entries are removed. For example, enum: [null, "b"] with enumNames: ["None"] becomes enum: ["b"] and enumNames: ["None"], so both renderers now label b as “None” instead of ignoring the malformed labels. Set enumNames to undefined unless it was parallel before filtering.

  const aligned =
    Array.isArray(names) && names.length === members.length
      ? kept.map((entry) => names[entry.index])
      : names;

core/json/nullableUnion.ts:206

• $ref is another unevaluated composition keyword, but this predicate does not reject it. A branch such as { type: "string", $ref: "#/$defs/integerOnly" } can be unsatisfiable, yet it is flattened into a nullable text field; similarly, admitsNull can accept null despite a sibling $ref ruling it out, enabling submission of a schema-invalid required value. Treat $ref as opaque anywhere this helper is used.

function hasOpaqueApplicator(schema: NullableUnionSchema): boolean {
  return (
    schema.not !== undefined ||
    schema.allOf !== undefined ||
    schema.oneOf !== undefined
  );

[cliffhall]

Round 11 findings addressed in 18053d4. No round 12 requested — the review loop is stopped at the maintainer's direction, so this is the final state of the branch from my side.

Two of the three were the enum-strip logic this PR introduced misbehaving on degenerate input, which is worth naming rather than burying:

• Empty enum — it permits nothing at all, but the strip reported "unchanged" (nothing was removed) and the collapse built a Select with no options. A required field then had no interaction capable of supplying a value. It now reaches the same conclusion as [null], and the outcome is renamed unselectable since "only-null" no longer describes both routes to it.
• enumNames — filtering shortens enum, which could make an already mismatched enumNames come out the same length: enum: [null, "b"] with enumNames: ["None"] began labelling b as "None", turning labels both renderers had correctly ignored into a confident mislabel. A list that was not parallel before filtering is now dropped.
• $ref — joins the opaque set. Never resolved here, applies alongside its siblings, so { type: "string", $ref: "#/$defs/intOnly" } can be unsatisfiable. The wrapper path already declined on it via the annotation allowlist; this extends the rule to branches, the type: [T, "null"] path, and admitsNull.

For the record, since I claimed otherwise after round 10: I said rounds 9–10 had closed the source of repeat findings. Round 11 disproved that — three more, two of them in code this PR added. The fail-closed direction has held (nothing found since round 8 has been a case of the form rendering something the schema forbids by default; they have been positions the rule had not reached), but "converged" was my inference, not an observation, and it was wrong.

Known-open items are tracked, not hidden: #2020 (draft validity never reaches the submit controls, and ToolDetailPanel has no required-field gating at all) and the TUI array-of-enum path noted on round 2.

npm run ci green: 5,028 web tests across 322 files, all smokes, Storybook.