diff --git a/AGENTS.md b/AGENTS.md index 00b81809..9b4fa79d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -211,10 +211,13 @@ v2/main/ │ │ # intersections: a union whose members are not ALL │ │ # field-carrying objects (or whose member `type` │ │ # rules objects out), a branch restating a -│ │ # constraint the root states differently, an allOf -│ │ # carrying a member it cannot fold in (`false`, a -│ │ # $ref) — dropping the keyword there would turn an -│ │ # UNSATISFIABLE schema into a fillable form — and a +│ │ # constraint the root states differently, ANY member +│ │ # stating more than the merge applies (only +│ │ # type/properties/required — a `false`, a $ref, a +│ │ # nested applicator would have its constraint erased +│ │ # with the keyword, turning an UNSATISFIABLE schema +│ │ # into a fillable form; allOf members are checked +│ │ # against the ACCUMULATED merge, not the root), and a │ │ # schema carrying BOTH oneOf and anyOf. Does not │ │ # interpret `not` at all. Declining changes what │ │ # RENDERS, never whether the tool takes arguments: diff --git a/README.md b/README.md index 463e0ad7..1b34448f 100644 --- a/README.md +++ b/README.md @@ -312,7 +312,7 @@ What it declines to flatten is as deliberate as what it flattens, and every case - **A union whose members are not all field-carrying object schemas** — including one whose member `type` rules objects out, since tool arguments are a JSON object and such a member can never match. A picker whose options render nothing is no better than no picker. - **A branch that restates a constraint the root already states.** The two are conjunctive, so root `minimum: 10` under branch `minimum: 0` is still 10, disjoint `enum`s leave nothing satisfiable, and `type: "string"` under `type: "number"` describes a value that cannot exist — rendering either side would accept what the schema rejects. A property both declare *compatibly* is merged rather than replaced, so a root's `minimum` survives a branch's `maximum`, and a disagreement about `title`/`description` is not a conflict at all. -- **An `allOf` with a member it cannot fold in** — JSON Schema's boolean form (`allOf: [false, …]` admits nothing) or a `$ref`, whose constraints are unknown rather than absent. Merging the rest and dropping the keyword would turn an unsatisfiable schema into a fillable form. +- **A composition member stating anything the merge cannot apply.** Only `type`, `properties` and `required` are folded in, so a member carrying a nested `allOf`/`anyOf`, a `not`, an `additionalProperties`, or a `$ref` would have that constraint erased along with the keyword — turning an unsatisfiable schema (`allOf: [false, …]` admits nothing) into a fillable form. `allOf` members are checked against the accumulated merge rather than the root alone, so two of them contradicting each other is caught even when neither contradicts the root. - **A schema carrying both `oneOf` and `anyOf`** — independent keywords a value satisfies *together*, not two spellings of one union, so reading one and dropping the other omits real constraints while looking complete. Satisfying both honestly means the cross product of their alternatives, which no real schema has yet asked for. - **`not`**, which is not interpreted at all: there is no faithful form for "anything except this". diff --git a/clients/web/src/test/core/nullableUnion.test.ts b/clients/web/src/test/core/nullableUnion.test.ts index eb22ab2e..4b44aa02 100644 --- a/clients/web/src/test/core/nullableUnion.test.ts +++ b/clients/web/src/test/core/nullableUnion.test.ts @@ -731,4 +731,23 @@ describe("admitsNull", () => { }), ).toBe(true); }); + + describe("a const-null schema (#2123)", () => { + it("admits null when nothing else rules it out", () => { + expect(admitsNull({ const: null })).toBe(true); + expect(admitsNull({ type: "null", const: null })).toBe(true); + expect(admitsNull({ type: ["string", "null"], const: null })).toBe(true); + }); + + it("does not override a sibling that rejects null", () => { + // `const` is conjunctive with its siblings, not an override: both of + // these reject every value, so claiming nullability would let the + // required-field gate accept a `null` the schema forbids. + expect(admitsNull({ type: "string", const: null })).toBe(false); + expect(admitsNull({ const: null, anyOf: [{ type: "string" }] })).toBe( + false, + ); + expect(admitsNull({ const: null, not: { type: "null" } })).toBe(false); + }); + }); }); diff --git a/clients/web/src/test/core/rootUnion.test.ts b/clients/web/src/test/core/rootUnion.test.ts index e1a9acf7..8a3f2a83 100644 --- a/clients/web/src/test/core/rootUnion.test.ts +++ b/clients/web/src/test/core/rootUnion.test.ts @@ -397,6 +397,24 @@ describe("resolveRootUnion", () => { ).toEqual([]); }); + it("declines a member carrying a constraint the merge cannot apply", () => { + // Same faithfulness test the `allOf` fold applies — otherwise a branch + // whose nested `allOf: [false]` makes it unsatisfiable is offered as a + // callable alternative. + const { branches } = resolveRootUnion({ + type: "object", + anyOf: [ + EMAIL, + { + type: "object", + properties: { x: { type: "string" } }, + allOf: [false as unknown], + }, + ], + }); + expect(branches).toEqual([]); + }); + it("declines an empty union", () => { expect(resolveRootUnion({ type: "object", anyOf: [] }).branches).toEqual( [], @@ -450,6 +468,33 @@ describe("resolveRootUnion", () => { expect(selectBranchIndex(branches, { kind: "sms" })).toBe(1); }); + it("identifies a branch from the constants that were supplied", () => { + // Both branches pin `version` as well; a deep link naming only `kind` + // must still find its branch — the unsupplied constant is one this + // identification exists to seed. + const versioned = resolveRootUnion({ + type: "object", + anyOf: [ + { + type: "object", + properties: { + version: { const: "1" }, + kind: { const: "email" }, + }, + }, + { + type: "object", + properties: { version: { const: "1" }, kind: { const: "sms" } }, + }, + ], + }).branches; + expect(selectBranchIndex(versioned, { kind: "sms" })).toBe(1); + // A supplied constant that disagrees is still evidence against. + expect(selectBranchIndex(versioned, { version: "2", kind: "sms" })).toBe( + null, + ); + }); + it("reports none when the values identify nothing", () => { expect(selectBranchIndex(branches, {})).toBeNull(); expect(selectBranchIndex(branches, { kind: "other" })).toBeNull(); diff --git a/core/json/nullableUnion.ts b/core/json/nullableUnion.ts index 9dad6f8e..580c0e8f 100644 --- a/core/json/nullableUnion.ts +++ b/core/json/nullableUnion.ts @@ -368,17 +368,15 @@ function collapsed( * an optimistic guess, and a schema this module cannot read renders through the * JSON editor with its constraints intact. */ +/** Whether an absent or null-naming `type` leaves null on the table. */ +function typeAdmitsNull(type: string | string[] | undefined): boolean { + return type === undefined || typeNamesNull(type); +} + export function admitsNull(schema: NullableUnionSchema): boolean { if (nullExcludedBySiblings(schema)) { return false; } - // `const: null` admits null and nothing else, whatever the schema says (or - // omits) about `type`. Without this a required field pinned to null is seeded - // with the only value it accepts and then reported as missing, leaving submit - // permanently disabled on a form the user cannot change (#2123). - if (schema.const === null) { - return true; - } // Applicators this module does not evaluate. `not: { type: "null" }` rules // null out; `allOf` can add a member that does; and `oneOf` requires // **exactly one** branch to match, so a null branch does not by itself mean @@ -388,6 +386,20 @@ export function admitsNull(schema: NullableUnionSchema): boolean { if (hasOpaqueApplicator(schema)) { return false; } + // `const: null` admits null and nothing else — the case a required field + // pinned to null needs, or it is seeded with the only value it accepts and + // then reported missing, leaving submit permanently disabled on a field the + // user cannot change (#2123). + // + // Claimed only where the siblings agree, since `const` is conjunctive with + // them rather than an override: `{ type: "string", const: null }` and + // `{ const: null, anyOf: [...] }` reject every value, and saying otherwise + // would let the gate accept a `null` the schema forbids. The opaque + // applicators (`not`, `allOf`, `oneOf`) are already refused above. + if (schema.const === null) { + return schema.anyOf === undefined && typeAdmitsNull(schema.type); + } + if (schema.nullable === true) { return true; } diff --git a/core/json/rootUnion.ts b/core/json/rootUnion.ts index fff5c7c7..75953e2d 100644 --- a/core/json/rootUnion.ts +++ b/core/json/rootUnion.ts @@ -430,6 +430,11 @@ export function resolveRootUnion( (branch) => branch === null || !isOfferable(branch) || + // The same faithfulness test the `allOf` fold applies: a member + // carrying a constraint the merge does not copy — a nested `allOf`, a + // `not`, a `$ref` — would have it erased along with the union keyword, + // which can present an unsatisfiable branch as a callable one. + !isFlattenable(branch) || conflictsWithBase(base, branch), ) ) { @@ -476,9 +481,14 @@ export function selectBranchIndex( ] as const, ) .filter(([, constValue]) => constValue !== undefined); + // Only the pinned names the caller actually supplied are evidence. A + // constant it did not supply is one this identification exists to *seed* — + // requiring it would mean a deep link naming `kind` alone matched no branch + // whenever the branches also pin, say, a `version`. + const supplied = pinned.filter(([name]) => values[name] !== undefined); if ( - pinned.length > 0 && - pinned.every(([name, constValue]) => values[name] === constValue) + supplied.length > 0 && + supplied.every(([name, constValue]) => values[name] === constValue) ) { matches.push(index); }