diff --git a/README.md b/README.md
index 1b34448f..1682f2a9 100644
--- a/README.md
+++ b/README.md
@@ -313,6 +313,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.
- **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 union that adds fields under a restrictive root `additionalProperties`.** That keyword constrains whatever its *sibling* `properties` does not name, so a root `additionalProperties: false` rejects every field the branches add — flattening would move them beside the keyword, where they read as allowed.
- **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/components/groups/SchemaForm/SchemaForm.test.tsx b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx
index 3c0e6f7a..710deee8 100644
--- a/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx
+++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.test.tsx
@@ -2149,6 +2149,39 @@ describe("SchemaForm multiline strings (#2042)", () => {
expect(onChange).toHaveBeenCalledWith({});
});
+ it("keeps a root value the incoming branch merely inherits", async () => {
+ const user = userEvent.setup();
+ const onChange = vi.fn();
+ const schema: InspectorFormSchema = {
+ type: "object",
+ properties: { count: {} },
+ anyOf: [
+ {
+ type: "object",
+ title: "A",
+ properties: { count: { type: "number", title: "Count" } },
+ },
+ {
+ type: "object",
+ title: "B",
+ properties: { other: { type: "string", title: "Other" } },
+ },
+ ],
+ };
+ renderWithMantine(
+ ,
+ );
+ await user.click(screen.getByRole("textbox", { name: /Variant/ }));
+ await user.click(screen.getByRole("option", { name: "B" }));
+ // Branch B does not redeclare `count`, so it is a root argument there —
+ // dropping it would erase a value the schema still accepts.
+ expect(onChange).toHaveBeenCalledWith({ count: 3 });
+ });
+
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 5974fa4c..5b01de8d 100644
--- a/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx
+++ b/clients/web/src/components/groups/SchemaForm/SchemaForm.tsx
@@ -694,20 +694,23 @@ export function SchemaForm({
if (!nextBranch) return;
setBranchIndex(nextIndex);
const nextProperties = nextBranch.schema.properties ?? {};
- // Only what the *base* contributed is carried across — a value the outgoing
- // branch declared belongs to that branch's shape, and a name the two
- // branches type differently would arrive as the wrong type entirely (a `3`
- // typed into branch A's number field landing in branch B's checkbox). A
- // field the incoming branch pins to a `const` is likewise not carried: the
- // branches of a discriminated union share the discriminator's *name* and
- // disagree about its value.
+ // A value is carried unless it belonged to the outgoing branch's own shape:
+ // a name **both** branches declare may be typed differently by each, so a
+ // `3` typed into branch A's number field must not arrive in branch B's
+ // checkbox. A name only the outgoing branch specialized still survives when
+ // the incoming branch merely inherits the root's declaration — it is a root
+ // argument there, and dropping it would erase a valid value. A field the
+ // incoming branch pins to a `const` is never carried: the branches of a
+ // discriminated union share the discriminator's *name* and disagree about
+ // its value.
const outgoing = new Set(activeBranch?.declaredFields ?? []);
+ const incoming = new Set(nextBranch.declaredFields);
const carried: Record = {};
for (const [name, fieldSchema] of Object.entries(nextProperties)) {
if (
values[name] !== undefined &&
fieldSchema.const === undefined &&
- !outgoing.has(name)
+ !(outgoing.has(name) && incoming.has(name))
) {
carried[name] = values[name];
}
diff --git a/clients/web/src/test/core/rootUnion.test.ts b/clients/web/src/test/core/rootUnion.test.ts
index 8a3f2a83..bc363652 100644
--- a/clients/web/src/test/core/rootUnion.test.ts
+++ b/clients/web/src/test/core/rootUnion.test.ts
@@ -260,6 +260,83 @@ describe("resolveRootUnion", () => {
expect(branches).toEqual([]);
});
+ it("tolerates a branch suggesting a different default", () => {
+ // `default` constrains nothing, so two declarations suggesting different
+ // initial values still accept the same values.
+ const { branches } = resolveRootUnion({
+ type: "object",
+ properties: { count: { default: 1 } },
+ anyOf: [
+ {
+ type: "object",
+ properties: { count: { type: "number", default: 2 } },
+ },
+ { type: "object", properties: { other: { type: "string" } } },
+ ],
+ });
+ expect(branches).toHaveLength(2);
+ });
+
+ it("does not mistake an inherited object property for a root declaration", () => {
+ // `constructor` is a legal argument name; finding it on `Object.prototype`
+ // would report a conflict the root never declared.
+ const anyOf: unknown[] = [
+ { type: "object", properties: { constructor: { type: "string" } } },
+ { type: "object", properties: { other: { type: "string" } } },
+ ];
+ const { branches } = resolveRootUnion({
+ type: "object",
+ properties: {},
+ anyOf,
+ });
+ expect(branches).toHaveLength(2);
+ expect(branches[0].schema.properties?.constructor).toEqual({
+ type: "string",
+ });
+ });
+
+ it("keeps a branch property named __proto__", () => {
+ // Assigning it would invoke the legacy prototype setter rather than create
+ // an own property, losing a renderable field.
+ const { branches } = resolveRootUnion({
+ type: "object",
+ properties: { keep: { type: "string" } },
+ // A computed key: `__proto__:` in an object literal is the prototype
+ // setter, so the literal form would not even create the property.
+ anyOf: [
+ { type: "object", properties: { ["__proto__"]: { type: "string" } } },
+ { type: "object", properties: { other: { type: "string" } } },
+ ] as unknown[],
+ });
+ expect(Object.keys(branches[0].schema.properties ?? {})).toEqual([
+ "keep",
+ "__proto__",
+ ]);
+ });
+
+ it("declines a union that adds fields under a restrictive additionalProperties", () => {
+ // `additionalProperties` constrains what its SIBLING `properties` does not
+ // name, so the original schema admits none of the branch fields — moving
+ // them beside the keyword would make them read as allowed.
+ const { branches } = resolveRootUnion({
+ type: "object",
+ properties: { known: { type: "string" } },
+ additionalProperties: false,
+ anyOf: [EMAIL, SMS],
+ });
+ expect(branches).toEqual([]);
+ });
+
+ it("allows a restrictive additionalProperties the branches stay within", () => {
+ const { branches } = resolveRootUnion({
+ type: "object",
+ properties: { kind: {}, address: {}, phone: {} },
+ additionalProperties: false,
+ anyOf: [EMAIL, SMS],
+ });
+ expect(branches).toHaveLength(2);
+ });
+
it("declines a union whose branch contradicts the root's type for a field", () => {
// `string` under a base `number` describes a value that cannot exist, so
// flattening it would render one type and accept what the schema rejects.
diff --git a/core/json/jsonUtils.ts b/core/json/jsonUtils.ts
index 27ed6690..32d53166 100644
--- a/core/json/jsonUtils.ts
+++ b/core/json/jsonUtils.ts
@@ -165,7 +165,12 @@ function coercionProperties(
return { ...matching[0].schema.properties };
}
- const properties: Record = { ...base.properties };
+ // `hasOwn`/`fromEntries` rather than `in`/assignment throughout: `properties`
+ // is a JSON record, so `constructor` and `__proto__` are legal argument names
+ // that the prototype chain and the legacy setter would otherwise mishandle.
+ const properties: Record = Object.fromEntries(
+ Object.entries(base.properties ?? {}),
+ );
for (const name of new Set(branches.flatMap((b) => b.declaredFields))) {
// Only the branches that *declare* the name have an opinion about it — a
// branch that merely inherited the root's declaration is not a second,
@@ -176,7 +181,12 @@ function coercionProperties(
.map((branch) => branch.schema.properties?.[name]);
const types = new Set(declarations.map((schema) => typeNameOf(schema)));
if (types.size === 1) {
- properties[name] = declarations[0];
+ Object.defineProperty(properties, name, {
+ value: declarations[0],
+ writable: true,
+ enumerable: true,
+ configurable: true,
+ });
} else {
delete properties[name];
}
diff --git a/core/json/rootUnion.ts b/core/json/rootUnion.ts
index 75953e2d..d44fcce3 100644
--- a/core/json/rootUnion.ts
+++ b/core/json/rootUnion.ts
@@ -42,6 +42,12 @@ export interface RootUnionSchema {
* own repro does). Read only to *label* a branch — never to validate.
*/
discriminator?: { propertyName?: string };
+ /**
+ * Read only to *decline*: `additionalProperties` constrains the names its
+ * **sibling** `properties` does not list, so a restrictive one at the root
+ * rejects every field a branch adds. See {@link resolveRootUnion}.
+ */
+ additionalProperties?: unknown;
/**
* Read only to *decline* a member: its referent is not resolved here, so a
* `$ref` member's constraints are unknown rather than absent.
@@ -160,6 +166,9 @@ const ANNOTATION_KEYWORDS = new Set([
"readOnly",
"writeOnly",
"$comment",
+ // An annotation too: a suggested initial value constrains nothing, so two
+ // declarations suggesting different ones still accept the same values.
+ "default",
]);
/** Structural equality, via canonical JSON — enough for schema keyword values. */
@@ -254,7 +263,11 @@ function conflictsWithBase(
const branchProperties = propertiesOf(branch) ?? {};
return Object.entries(branchProperties).some(
([name, branchProperty]) =>
- name in baseProperties && conflicts(baseProperties[name], branchProperty),
+ // `hasOwn`, not `in`: `properties` is a JSON record, so `constructor` and
+ // `toString` are legal argument names that `in` would find on
+ // `Object.prototype` and report as collisions the root never declared.
+ Object.hasOwn(baseProperties, name) &&
+ conflicts(baseProperties[name], branchProperty),
);
}
@@ -297,13 +310,19 @@ function mergeBranch(
): ResolvedSchema {
const baseProperties = propertiesOf(base) ?? {};
const branchProperties = propertiesOf(branch) ?? {};
- const properties: Record = { ...baseProperties };
- for (const [name, branchProperty] of Object.entries(branchProperties)) {
- properties[name] =
- name in baseProperties
+ // Built through `fromEntries` rather than by assignment: a property named
+ // `__proto__` is a legal argument name, and assigning it would invoke the
+ // legacy prototype setter instead of creating an own property — losing the
+ // field entirely. `hasOwn` for the same reason `conflictsWithBase` uses it.
+ const properties: Record = Object.fromEntries([
+ ...Object.entries(baseProperties),
+ ...Object.entries(branchProperties).map(([name, branchProperty]) => [
+ name,
+ Object.hasOwn(baseProperties, name)
? mergeProperty(baseProperties[name], branchProperty)
- : branchProperty;
- }
+ : branchProperty,
+ ]),
+ ]);
const required = [
...(base.required ?? []),
...(branch.required ?? []).filter(
@@ -441,6 +460,27 @@ export function resolveRootUnion(
return { base, branches: [] };
}
+ // `additionalProperties` applies to whatever its **sibling** `properties`
+ // does not name, so a restrictive one at the root rejects every field the
+ // branches add — the original schema admits none of them. Flattening moves
+ // those fields *beside* the keyword, where they would read as allowed, so a
+ // form built from it would submit what the schema forbids.
+ const additional = base.additionalProperties;
+ const restrictsAdditional =
+ additional === false ||
+ (typeof additional === "object" && additional !== null);
+ const baseNames = propertiesOf(base) ?? {};
+ if (
+ restrictsAdditional &&
+ branches.some((branch) =>
+ Object.keys(propertiesOf(branch as RootUnionSchema) ?? {}).some(
+ (name) => !Object.hasOwn(baseNames, name),
+ ),
+ )
+ ) {
+ return { base, branches: [] };
+ }
+
const discriminatorProperty = schema.discriminator?.propertyName;
return {
base,