diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx index 710deee8..044302ce 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx @@ -2182,6 +2182,39 @@ describe("SchemaForm multiline strings (#2042)", () => { expect(onChange).toHaveBeenCalledWith({ count: 3 }); }); + it("clears an in-progress draft when the branch changes", async () => { + const user = userEvent.setup(); + const schema: InspectorFormSchema = { + type: "object", + anyOf: [ + { + type: "object", + title: "A", + properties: { value: { type: "number", title: "Value" } }, + }, + { + type: "object", + title: "B", + properties: { value: { type: "number", title: "Value" } }, + }, + ], + }; + renderWithMantine( + , + ); + const before = screen.getByLabelText(/Value/) as HTMLInputElement; + // A lone `-` parses to nothing, so the parent value stays `undefined` on + // both sides of the switch — the field's own key is what has to change. + await user.type(before, "-"); + expect(before.value).toBe("-"); + + await user.click(screen.getByRole("textbox", { name: /Variant/ })); + await user.click(screen.getByRole("option", { name: "B" })); + expect((screen.getByLabelText(/Value/) as HTMLInputElement).value).toBe( + "", + ); + }); + it("renders no picker for a single-branch union but still shows its fields", () => { const schema: InspectorFormSchema = { type: "object", diff --git a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx index 46053d13..8ef66f11 100644 --- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx +++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx @@ -606,6 +606,16 @@ export function SchemaForm({ const properties = effectiveSchema.properties ?? {}; const requiredFields = effectiveSchema.required ?? []; + // The key the draft-holding fields are remounted by. Switching branches is a + // reset for them too: two alternatives may declare the same name with the + // same widget, and a half-typed `-` or an unparsed JSON draft would otherwise + // survive into a field the switch was supposed to clear — with both parent + // values `undefined`, nothing else tells them the entity changed. + const draftKey = + activeBranch === null + ? resetKey + : `${resetKey ?? ""}#${branches.indexOf(activeBranch)}`; + // The names of fields currently holding unsendable text. Held here rather // than in each field because only the form sees them all, and only the form // knows when the last one cleared. @@ -867,7 +877,7 @@ export function SchemaForm({ { expect(branches).toEqual([]); }); + it("declines a const paired with an assertion it cannot evaluate", () => { + // `minimum: 10` beside `const: 1` is as unsatisfiable as a type mismatch, + // and proving the conjunction safe is the requirement here. + const { branches } = resolveRootUnion({ + type: "object", + properties: { x: { type: "number", minimum: 10 } }, + anyOf: [ + { type: "object", properties: { x: { const: 1 } } }, + { type: "object", properties: { other: { type: "string" } } }, + ] as unknown[], + }); + expect(branches).toEqual([]); + }); + it("accepts a const its root type and enum admit", () => { const { branches } = resolveRootUnion({ type: "object", diff --git a/clients/web/src/utils/toolUtils.test.ts b/clients/web/src/utils/toolUtils.test.ts index a524ae97..07c6446f 100644 --- a/clients/web/src/utils/toolUtils.test.ts +++ b/clients/web/src/utils/toolUtils.test.ts @@ -134,6 +134,17 @@ describe("hasInputFields with root composition (#2123)", () => { ).toBe(true); }); + it("counts a $ref member, whose shape is unknown rather than empty", () => { + expect( + hasInputFields( + tool({ + type: "object", + anyOf: [{ $ref: "#/$defs/Email" }, { $ref: "#/$defs/Sms" }], + }), + ), + ).toBe(true); + }); + it("still reports no fields for a bare object schema", () => { expect(hasInputFields(tool({ type: "object" }))).toBe(false); }); diff --git a/core/json/rootUnion.ts b/core/json/rootUnion.ts index 07b5786e..a9c31680 100644 --- a/core/json/rootUnion.ts +++ b/core/json/rootUnion.ts @@ -258,10 +258,26 @@ function jsonTypeOf(value: unknown): string { * it leaves nothing satisfiable. An `integer` const satisfies a `number` type, * which is the one direction JSON Schema widens. */ +const CONST_CHECKABLE = new Set(["const", "type", "enum"]); + function constSatisfiesSiblings(schema: Record): boolean { if (!("const" in schema)) return true; const value = schema.const; const { type, enum: allowed } = schema; + // A `const` can be contradicted by any assertion, and only `type` and `enum` + // are evaluated here — `minimum: 10` beside `const: 1` is as unsatisfiable as + // a type mismatch. Rather than partially evaluate JSON Schema, a merged + // declaration pairing a `const` with an assertion this cannot check is + // declined: proving the conjunction safe is the requirement, not disproving + // it. Annotations are exempt, since they assert nothing. + if ( + Object.keys(schema).some( + (keyword) => + !CONST_CHECKABLE.has(keyword) && !ANNOTATION_KEYWORDS.has(keyword), + ) + ) { + return false; + } const actual = jsonTypeOf(value); const admits = (name: unknown) => name === actual || (name === "number" && actual === "integer"); @@ -420,6 +436,10 @@ export function declaresAnyFields( ): boolean { if (schema === undefined) return false; if (Object.keys(propertiesOf(schema) ?? {}).length > 0) return true; + // A `$ref`'s shape is unknown rather than empty, so it counts. Reporting "no + // fields" for `anyOf: [{ $ref: … }, { $ref: … }]` would auto-invoke an App + // tool with `{}` on the strength of something never read. + if (schema.$ref !== undefined) return true; const members = [ ...(schema.allOf ?? []), ...(schema.anyOf ?? []),