From a3085e616825ecab278cbc574e0c971214dbe289 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 22:46:59 +0000 Subject: [PATCH 1/2] wip(spec): objectConflict 'merge' refuses colliding object-level collections (#14848 WIP) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01M59rPZZFzqhfMUPFqqZTkf --- ...ompose-stacks-action-key-collision.test.ts | 109 +++++--- ...se-stacks-merge-collection-refusal.test.ts | 260 ++++++++++++++++++ packages/spec/src/stack.zod.ts | 209 +++++++++++++- 3 files changed, 533 insertions(+), 45 deletions(-) create mode 100644 packages/spec/src/compose-stacks-merge-collection-refusal.test.ts diff --git a/packages/spec/src/compose-stacks-action-key-collision.test.ts b/packages/spec/src/compose-stacks-action-key-collision.test.ts index e39d66d85b..0ef9a8f91d 100644 --- a/packages/spec/src/compose-stacks-action-key-collision.test.ts +++ b/packages/spec/src/compose-stacks-action-key-collision.test.ts @@ -21,6 +21,16 @@ * P3 embedded(A on shared) + embedded(B on shared) : merge/override ACCEPTED — B's array REPLACES A's * ``` * + * Since #14848 `objectConflict: 'merge'` refuses two objects that declare + * DIFFERENT `actions` (any object-level collection) at `mergeObjects`, before + * this check runs — so the `'merge'` rows above that relied on B's array + * replacing A's (P3, P5b with A last, and its reverse) are refusals of THAT + * rule now, pinned here as such and in full in + * `compose-stacks-merge-collection-refusal.test.ts`; only `'override'` still + * hands the object wholesale to the later stack. IDENTICAL arrays — the built + * copies two stacks each binding one standalone to the same object carry (P4) + * — pass the object merge and reach this check as before. + * * Every refusal case pins the full line — the key, both manifest ids, and * where each declaration sits — rather than `toThrow()` alone: a bare throw * cannot tell "refused for the right reason" from "refused because the fixture @@ -160,9 +170,11 @@ describe('composeStacks - two stacks declaring one global action key', () => { describe('composeStacks - object-scoped keys across stacks, judged on what the composition carries', () => { // Both stacks declare object `shared` and bind a standalone `dup_s` to it. - // `objectConflict: 'merge'` / `'override'` keep the later stack's object - // (whose built copy of its own bound action is the echo in `objects[...]`), - // and both standalone declarations concatenate — two stacks, one key. + // `'override'` keeps the later stack's object (whose built copy of its own + // bound action is the echo in `objects[...]`); under `'merge'` the two built + // copies are IDENTICAL, so the object merge passes them (#14848) and the + // composed object carries one. Both standalone declarations concatenate + // either way — two stacks, one key, and this check is what refuses. const boundA = () => defineStack({ manifest: mf('com.example.a'), objects: [obj('shared')], actions: [act('dup_s', { objectName: 'shared' })] }); const boundB = () => defineStack({ manifest: mf('com.example.b'), objects: [obj('shared')], actions: [act('dup_s', { objectName: 'shared' })] }); @@ -189,42 +201,75 @@ describe('composeStacks - object-scoped keys across stacks, judged on what the c const embeddedA = () => defineStack({ manifest: mf('com.example.a'), objects: [obj('shared', [act('dup_m')])] }); const boundToSharedB = () => defineStack({ manifest: mf('com.example.b'), objects: [obj('shared')], actions: [act('dup_m', { objectName: 'shared' })] }); - it.each(['merge', 'override'] as const)( - "refuses an embedded action the composed object carries from one stack beside the other stack's standalone bound to it (objectConflict: %s, embedding stack last)", - (objectConflict) => { - // B first, A last: A's object wins the merge / override, so the composed - // `shared` carries A's embedded `dup_m` (B's built copy of its own bound - // action is NOT carried — B's object lost); B's standalone joins it at - // `mergeActionsIntoObjects` — two handlers, one key. - const msg = refusal(() => composeStacks([boundToSharedB(), embeddedA()], { objectConflict })); - expect(msg).toContain(ENVELOPE_ONE); + it("refuses an embedded action the composed object carries from one stack beside the other stack's standalone bound to it (objectConflict: 'override', embedding stack last)", () => { + // B first, A last: A's object wins the override, so the composed `shared` + // carries A's embedded `dup_m` (B's built copy of its own bound action is + // NOT carried — B's object lost); B's standalone joins it at + // `mergeActionsIntoObjects` — two handlers, one key. + const msg = refusal(() => composeStacks([boundToSharedB(), embeddedA()], { objectConflict: 'override' })); + expect(msg).toContain(ENVELOPE_ONE); + expect(msg).toContain( + " ✗ Action key 'shared:dup_m' is declared by 2 stacks: " + + "'com.example.b' (stack #0) at stack.actions[0] and " + + "'com.example.a' (stack #1) at objects['shared'].actions[0].", + ); + }); + + it.each([ + ['embedding stack last', () => [boundToSharedB(), embeddedA()], "'com.example.b' (stack #0)", "'com.example.a' (stack #1)"], + ['embedding stack first', () => [embeddedA(), boundToSharedB()], "'com.example.a' (stack #0)", "'com.example.b' (stack #1)"], + ] as const)( + "under 'merge' the same pair is refused one step earlier, at the object merge: both objects declare a DIFFERENT `actions` (%s) — #14848, not a collision", + (_order, stacks, holder, later) => { + // B's built object carries its bound copy (`dup_m/BOUND`), A's carries + // the embedded declaration (`dup_m/EMB`): two different arrays on one + // object, which `'merge'` no longer resolves by replacement. + const msg = refusal(() => composeStacks(stacks(), { objectConflict: 'merge' })); expect(msg).toContain( - " ✗ Action key 'shared:dup_m' is declared by 2 stacks: " + - "'com.example.b' (stack #0) at stack.actions[0] and " + - "'com.example.a' (stack #1) at objects['shared'].actions[0].", + "composeStacks conflict: object 'shared' is defined in multiple stacks and its 'actions' is declared " + + `with different values by ${holder} and ${later}.`, ); + expect(msg).not.toContain('action key'); }, ); - it.each(['merge', 'override'] as const)( - "accepts the same pair the other way round: the strategy hands `shared` to B, A's embedded action is not carried, one handler remains (objectConflict: %s)", - (objectConflict) => { - // A first, B last: B's built object carries `actions` (the echo of its - // own bound action), so both `'override'` and the `'merge'` spread hand - // the composed object's `actions` to B. A's embedded `dup_m` is not in - // the artifact — the object strategy's own loss, not a collision — so - // only B's handler reaches the runtime key. - const out = composeStacks([embeddedA(), boundToSharedB()], { objectConflict }); - const shared = (out.objects ?? []).find((o) => o.name === 'shared'); - expect(keysOf(out).top).toEqual(['shared:dup_m']); - // No embedded (object-less) entry survives: every carried declaration is B's bound one. - expect((shared?.actions ?? []).every((a) => a.objectName === 'shared')).toBe(true); - expect((shared?.actions ?? []).length).toBeGreaterThan(0); - }, - ); + it("accepts the same pair the other way round under 'override': the strategy hands `shared` to B, A's embedded action is not carried, one handler remains", () => { + // A first, B last: B's built object carries `actions` (the echo of its + // own bound action), so `'override'` hands the composed object's `actions` + // to B. A's embedded `dup_m` is not in the artifact — the object + // strategy's own loss, not a collision — so only B's handler reaches the + // runtime key. (`'merge'` refuses this pair — pinned above.) + const out = composeStacks([embeddedA(), boundToSharedB()], { objectConflict: 'override' }); + const shared = (out.objects ?? []).find((o) => o.name === 'shared'); + expect(keysOf(out).top).toEqual(['shared:dup_m']); + // No embedded (object-less) entry survives: every carried declaration is B's bound one. + expect((shared?.actions ?? []).every((a) => a.objectName === 'shared')).toBe(true); + expect((shared?.actions ?? []).length).toBeGreaterThan(0); + }); + + // P3 — two stacks each EMBEDDING the same name on one object, as two + // DIFFERENT declarations (the labels differ). + const embedA = () => defineStack({ manifest: mf('com.example.a'), objects: [obj('shared', [act('dup_e', { label: 'Dup E (a)' })])] }); + const embedB = () => defineStack({ manifest: mf('com.example.b'), objects: [obj('shared', [act('dup_e', { label: 'Dup E (b)' })])] }); + + it("accepts two stacks each EMBEDDING the same name on one object under 'override' — the later object's array replaces the earlier (measured), one handler reaches the artifact", () => { + const out = composeStacks([embedA(), embedB()], { objectConflict: 'override' }); + expect(keysOf(out).embedded).toEqual({ shared: ['dup_e/EMB'] }); + expect(keysOf(out).top).toEqual([]); + expect((out.objects ?? []).find((o) => o.name === 'shared')?.actions?.[0]?.label).toBe('Dup E (b)'); + }); + + it("refuses the same pair under 'merge' — two different `actions` arrays on one object are no longer resolved by replacement (#14848; formerly the P3 acceptance pin)", () => { + const msg = refusal(() => composeStacks([embedA(), embedB()], { objectConflict: 'merge' })); + expect(msg).toContain( + "composeStacks conflict: object 'shared' is defined in multiple stacks and its 'actions' is declared " + + "with different values by 'com.example.a' (stack #0) and 'com.example.b' (stack #1).", + ); + expect(msg).not.toContain('action key'); + }); it.each(['merge', 'override'] as const)( - "accepts two stacks each EMBEDDING the same name on one object — the later object's array replaces the earlier (measured), one handler reaches the artifact (objectConflict: %s)", + "accepts two stacks embedding the IDENTICAL declaration on one object — one declaration, carried once, one handler (objectConflict: %s)", (objectConflict) => { const a = defineStack({ manifest: mf('com.example.a'), objects: [obj('shared', [act('dup_e')])] }); const b = defineStack({ manifest: mf('com.example.b'), objects: [obj('shared', [act('dup_e')])] }); diff --git a/packages/spec/src/compose-stacks-merge-collection-refusal.test.ts b/packages/spec/src/compose-stacks-merge-collection-refusal.test.ts new file mode 100644 index 0000000000..f0ad94f87b --- /dev/null +++ b/packages/spec/src/compose-stacks-merge-collection-refusal.test.ts @@ -0,0 +1,260 @@ +/** + * `objectConflict: 'merge'` merges `fields` and REFUSES every other + * object-level collection two stacks declare differently (#14848). + * + * Measured on `main` @ `53cbad9f7` before the change, `'merge'` was + * `{ ...existing, ...obj, fields: { ...existing.fields, ...obj.fields } }`: + * `fields` was the one key merged, and every other key the later object + * carried — `actions`, `indexes`, `listViews`, `validations`, … — REPLACED the + * earlier package's value wholesale, with nothing at compose, build or boot + * saying so (the issue's own case: `[approve]` + `[archive]` composed to + * `[archive]`). Maintainer ruling (2026-09-04, option 4): refuse, in the shape + * `'error'` uses, naming the object, the colliding collection and both + * package ids; `fields` keeps its shallow merge; identical declarations pass + * (as `composeSingleValue` passes identical top-level values); scalars and + * fixed-shape config objects stay on later-wins. + * + * The refusal set is DERIVED from `ObjectSchema`'s shape (every array- or + * record-typed key but `fields`), so the last block pins it against the shape + * in BOTH directions with an independent walk: every collection key refuses, + * every other key composes. The literal list beside it is the reviewer's copy + * — a new collection key on the object schema joins the refusal without an + * edit to `stack.zod.ts`, and shows up here as a one-line diff. + */ +import { describe, it, expect } from 'vitest'; +import { composeStacks, defineStack, type ObjectStackDefinition } from './stack.zod'; +import { ObjectSchema } from './data/object.zod'; + +const mf = (id: string) => ({ id, name: id.split('.').pop()!, version: '1.0.0', type: 'app' as const }); + +const act = (name: string, extra: Record = {}) => + ({ name, label: name, type: 'script' as const, target: 'noop', ...extra }); + +// `as const` on the field type is load-bearing (see stack.test.ts): hoisted +// without it the literal widens to `string`, which the input type refuses. +const obj = (name: string, extra: Record = {}) => ({ + name, + label: name, + fields: { title: { type: 'text' as const } }, + ...extra, +}); + +/** The thrown message, or `null` when the composition is accepted. */ +function refusal(fn: () => unknown): string | null { + try { + fn(); + return null; + } catch (e) { + return (e as Error).message; + } +} + +const shared = (out: ObjectStackDefinition) => (out.objects ?? []).find((o) => o.name === 'shared'); + +/** + * The derived set, in the object shape's declaration order — the order the + * refusal prints it in. `fields` is absent by rule. + */ +const COLLECTION_KEYS_IN_SHAPE_ORDER = [ + 'indexes', + 'fieldGroups', + 'requiredPermissions', + 'validations', + 'activityMilestones', + 'highlightFields', + 'listViews', + 'searchableFields', + 'actions', +] as const; + +const REFUSED = (key: string, holder: string, later: string) => + `composeStacks conflict: object 'shared' is defined in multiple stacks and its '${key}' is declared with ` + + `different values by ${holder} and ${later}.`; +const WHY = (holder: string) => + "objectConflict: 'merge' shallow-merges 'fields' only. Any other object-level collection " + + `(${COLLECTION_KEYS_IN_SHAPE_ORDER.join(', ')}) is not merged: the later declaration would replace the ` + + `earlier one wholesale, silently dropping every entry ${holder} wrote.`; +const FIX = (key: string) => + `Fix: declare '${key}' on 'shared' in exactly one of the two stacks, make the two declarations identical, ` + + "or use { objectConflict: 'override' } to hand the whole object to the later stack."; + +const A0 = "'com.example.a' (stack #0)"; +const B1 = "'com.example.b' (stack #1)"; + +// The card's own case: two packages, one object, an embedded action each. +const approveA = () => defineStack({ manifest: mf('com.example.a'), objects: [obj('shared', { actions: [act('approve')] })] }); +const archiveB = () => defineStack({ manifest: mf('com.example.b'), objects: [obj('shared', { actions: [act('archive')] })] }); + +describe("composeStacks objectConflict: 'merge' — a collection both objects declare differently is refused", () => { + it("refuses [approve] + [archive] on one object, naming the object, 'actions' and both manifest ids — the full message", () => { + const msg = refusal(() => composeStacks([approveA(), archiveB()], { objectConflict: 'merge' })); + expect(msg).toBe(`${REFUSED('actions', A0, B1)}\n${WHY(A0)}\n${FIX('actions')}`); + }); + + it("refuses at compose — before the cross-stack action-key check, whose envelope never appears", () => { + const msg = refusal(() => composeStacks([approveA(), archiveB()], { objectConflict: 'merge' })); + expect(msg).not.toContain('action key'); + }); + + it("refuses two objects declaring different 'indexes' (strict-parsed inputs)", () => { + const a = defineStack({ manifest: mf('com.example.a'), objects: [obj('shared', { indexes: [{ fields: ['title'] }] })] }); + const b = defineStack({ manifest: mf('com.example.b'), objects: [obj('shared', { indexes: [{ fields: ['title'], unique: true }] })] }); + const msg = refusal(() => composeStacks([a, b], { objectConflict: 'merge' })); + expect(msg).toContain(REFUSED('indexes', A0, B1)); + expect(msg).toContain(FIX('indexes')); + }); + + // `strict: false` bypasses the parse: the fixture SHAPE is not the subject + // here, the composition is — each pair differs on exactly the named key. + it.each([ + ['validations', [{ type: 'script', name: 'v_a', condition: 'record.amount < 0' }], [{ type: 'script', name: 'v_b', condition: 'record.amount > 0' }]], + ['listViews', { all: { label: 'All' } }, { mine: { label: 'Mine' } }], + ['fieldGroups', [{ name: 'g_a', fields: ['title'] }], [{ name: 'g_b', fields: ['title'] }]], + ['searchableFields', ['title'], ['title', 'body']], + ['highlightFields', ['title'], ['body']], + ['activityMilestones', [{ name: 'm_a' }], [{ name: 'm_b' }]], + ['requiredPermissions', ['crm.read'], ['billing.read']], + ] as const)("refuses two objects declaring different '%s'", (key, left, right) => { + const a = defineStack({ manifest: mf('com.example.a'), objects: [obj('shared', { [key]: left })] }, { strict: false }); + const b = defineStack({ manifest: mf('com.example.b'), objects: [obj('shared', { [key]: right })] }, { strict: false }); + const msg = refusal(() => composeStacks([a, b], { objectConflict: 'merge' })); + expect(msg).toContain(REFUSED(key, A0, B1)); + expect(msg).toContain(WHY(A0)); + expect(msg).toContain(FIX(key)); + }); + + it('names the FIRST stack that declared the collection against the disagreeing later one (three stacks, two agreeing)', () => { + const idx = [{ fields: ['title'] }]; + const a = defineStack({ manifest: mf('com.example.a'), objects: [obj('shared', { indexes: idx })] }); + const b = defineStack({ manifest: mf('com.example.b'), objects: [obj('shared', { indexes: [{ fields: ['title'] }] })] }); + const c = defineStack({ manifest: mf('com.example.c'), objects: [obj('shared', { indexes: [{ fields: ['title'], unique: true }] })] }); + const msg = refusal(() => composeStacks([a, b, c], { objectConflict: 'merge' })); + expect(msg).toContain(REFUSED('indexes', A0, "'com.example.c' (stack #2)")); + }); + + it('names a manifest-less input by position', () => { + const a = defineStack({ objects: [obj('shared', { actions: [act('approve')] })] }, { strict: false }); + const b = defineStack({ objects: [obj('shared', { actions: [act('archive')] })] }, { strict: false }); + const msg = refusal(() => composeStacks([a, b], { objectConflict: 'merge' })); + expect(msg).toContain(REFUSED('actions', 'stack #0', 'stack #1')); + }); +}); + +describe("composeStacks objectConflict: 'merge' — what stays accepted", () => { + it('shallow-merges `fields` (later fields win, earlier fields kept) and lets a later scalar win', () => { + const a = defineStack({ + manifest: mf('com.example.a'), + objects: [{ name: 'shared', label: 'Shared v1', fields: { title: { type: 'text' as const }, industry: { type: 'text' as const }, status: { type: 'text' as const } } }], + }); + const b = defineStack({ + manifest: mf('com.example.b'), + objects: [{ name: 'shared', label: 'Shared v2', fields: { email: { type: 'email' as const }, status: { type: 'select' as const, options: [{ label: 'Active', value: 'active' }] } } }], + }); + const out = composeStacks([a, b], { objectConflict: 'merge' }); + const s = shared(out)!; + expect(Object.keys(s.fields).sort()).toEqual(['email', 'industry', 'status', 'title']); + expect(s.fields.status.type).toBe('select'); + expect(s.label).toBe('Shared v2'); + }); + + it('passes an IDENTICAL collection declared on both sides — carried once, nothing dropped', () => { + const a = defineStack({ manifest: mf('com.example.a'), objects: [obj('shared', { actions: [act('approve')], indexes: [{ fields: ['title'] }] })] }); + const b = defineStack({ manifest: mf('com.example.b'), objects: [obj('shared', { actions: [act('approve')], indexes: [{ fields: ['title'] }] })] }); + const out = composeStacks([a, b], { objectConflict: 'merge' }); + const s = shared(out)!; + expect((s.actions ?? []).map((x) => x.name)).toEqual(['approve']); + expect(s.indexes).toEqual([{ fields: ['title'] }]); + }); + + it("keeps the earlier stack's collection when the later object does not declare it", () => { + const b = defineStack({ manifest: mf('com.example.b'), objects: [obj('shared', { label: 'Shared v2' })] }); + const out = composeStacks([approveA(), b], { objectConflict: 'merge' }); + const s = shared(out)!; + expect((s.actions ?? []).map((x) => x.name)).toEqual(['approve']); + expect(s.label).toBe('Shared v2'); + }); + + it("reads an explicit `undefined` on the later object as no declaration — neither refused nor erased", () => { + // Zod v4 keeps an explicitly-undefined input key as an own property, so a + // built stack CAN carry `actions: undefined`; the bare spread used to let + // it erase the earlier array. + const b = defineStack({ manifest: mf('com.example.b'), objects: [{ ...obj('shared'), actions: undefined }] }); + const out = composeStacks([approveA(), b], { objectConflict: 'merge' }); + expect((shared(out)!.actions ?? []).map((x) => x.name)).toEqual(['approve']); + }); + + it("two built stacks each binding one standalone action to the same object carry identical copies — the object merge passes them and the action-key check is what refuses", () => { + const a = defineStack({ manifest: mf('com.example.a'), objects: [obj('shared')], actions: [act('dup_s', { objectName: 'shared' })] }); + const b = defineStack({ manifest: mf('com.example.b'), objects: [obj('shared')], actions: [act('dup_s', { objectName: 'shared' })] }); + const msg = refusal(() => composeStacks([a, b], { objectConflict: 'merge' })); + expect(msg).toContain('composeStacks conflict: cross-stack action key collision (1 issue):'); + expect(msg).not.toContain("its 'actions' is declared with different values"); + }); +}); + +describe('composeStacks — the other two strategies are unchanged', () => { + it("default 'error' still refuses the duplicate object with its own message", () => { + const msg = refusal(() => composeStacks([approveA(), archiveB()])); + expect(msg).toBe( + "composeStacks conflict: object 'shared' is defined in multiple stacks. " + + "Use { objectConflict: 'override' } or { objectConflict: 'merge' } to resolve.", + ); + }); + + it("'override' still hands the whole object to the later stack — the earlier collection is replaced, by choice", () => { + const out = composeStacks([approveA(), archiveB()], { objectConflict: 'override' }); + expect((shared(out)!.actions ?? []).map((x) => x.name)).toEqual(['archive']); + }); +}); + +describe('the refusal set is derived from ObjectSchema.shape — pinned in both directions', () => { + /** Independent walk: strip wrappers, read through lazy/pipe, any union member counts. */ + function isCollection(schema: unknown, depth = 0): boolean { + if (depth > 8) return false; + const def = (schema as { _zod?: { def?: Record } })._zod?.def; + const type = def?.type as string | undefined; + if (!type) return false; + if (type === 'array' || type === 'record') return true; + if (['optional', 'nullable', 'default', 'prefault', 'readonly', 'nonoptional', 'catch'].includes(type)) return isCollection(def!.innerType, depth + 1); + if (type === 'lazy') return isCollection((def!.getter as () => unknown)(), depth + 1); + if (type === 'pipe') return isCollection(def!.in, depth + 1); + if (type === 'union') return (def!.options as unknown[]).some((o) => isCollection(o, depth + 1)); + return false; + } + + const shapeKeys = Object.keys(ObjectSchema.shape); + const derived = shapeKeys.filter((k) => k !== 'fields' && isCollection((ObjectSchema.shape as Record)[k])); + + it('the literal list equals the shape walk, in shape order (a new collection key on the object schema lands here as a one-line diff)', () => { + expect(derived).toEqual([...COLLECTION_KEYS_IN_SHAPE_ORDER]); + }); + + it("'fields' is a record on the shape and is the one collection excluded by rule", () => { + expect(isCollection((ObjectSchema.shape as Record).fields)).toBe(true); + expect(derived).not.toContain('fields'); + }); + + // Dummy values by kind — `strict: false` inputs, so the shape of the value + // is irrelevant; only "differs on exactly this key" matters. + const valuesFor = (key: string): [unknown, unknown] => { + const s = (ObjectSchema.shape as Record)[key]; + if (!isCollection(s)) return ['x', 'y']; + return key === 'listViews' ? [{ x: {} }, { y: {} }] : [[{ name: 'x' }], [{ name: 'y' }]]; + }; + + it.each(shapeKeys.filter((k) => k !== 'name' && k !== 'fields'))( + "'%s': refused under 'merge' iff the shape declares it as a collection", + (key) => { + const [left, right] = valuesFor(key); + const a = defineStack({ manifest: mf('com.example.a'), objects: [obj('shared', { [key]: left })] }, { strict: false }); + const b = defineStack({ manifest: mf('com.example.b'), objects: [obj('shared', { [key]: right })] }, { strict: false }); + const msg = refusal(() => composeStacks([a, b], { objectConflict: 'merge' })); + if (derived.includes(key)) { + expect(msg).toContain(REFUSED(key, A0, B1)); + } else { + expect(msg).toBeNull(); + expect((shared(composeStacks([a, b], { objectConflict: 'merge' })) as Record)[key]).toEqual(right); + } + }, + ); +}); diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index dc669dcb9b..370b121877 100644 --- a/packages/spec/src/stack.zod.ts +++ b/packages/spec/src/stack.zod.ts @@ -2562,7 +2562,15 @@ export function defineStack( * * - `'error'` — Throw an error when a duplicate name is detected (default). * - `'override'` — Last stack wins; later definitions replace earlier ones. - * - `'merge'` — Shallow-merge items with the same name (later fields win). + * - `'merge'` — Shallow-merge `fields` of same-name objects (later fields + * win, earlier fields are kept). Every OTHER object-level + * collection (`actions`, `indexes`, `listViews`, + * `validations`, …) is not merged: when both objects declare + * one with different values the composition is REFUSED, + * naming the object, the collection and both stacks (#14848) + * — declare it in one stack only, or use `'override'`. + * Identical declarations pass through; a scalar or config + * object the later object declares replaces the earlier one. */ export const ConflictStrategySchema = lazySchema(() => z.enum(['error', 'override', 'merge'])); export type ConflictStrategy = z.input; @@ -2829,9 +2837,162 @@ function warnUncomposedStackKey(key: string, rule: ComposeDisposition): void { ); } +/** + * Does this schema declare a COLLECTION — an array, or a record of named + * members — once the optional/default/nullable wrappers are stripped, reading + * through a `lazy` or a `pipe` and into a union's members? (#14848) + * + * The one structural question {@link objectCollectionKeys} asks of each key + * on the object shape. A union counts when ANY member is a collection + * (`requiredPermissions` admits a `string[]` beside its object form): the + * author may have written the array form, and the loss the caller refuses is + * the same. A fixed-shape config object (`enable`, `access`, `protection`, …) + * is not a collection — its members are declared keys, not authored entries — + * and stays on the scalar rule. + * @internal + */ +function declaresCollection(schema: unknown, depth = 0): boolean { + if (depth > 8) return false; + const def = (schema as { + _zod?: { def?: { type?: string; innerType?: unknown; in?: unknown; options?: unknown[]; getter?: () => unknown } }; + })._zod?.def; + if (!def?.type) return false; + switch (def.type) { + case 'array': + case 'record': + return true; + case 'optional': + case 'nullable': + case 'default': + case 'prefault': + case 'readonly': + case 'nonoptional': + case 'catch': + return declaresCollection(def.innerType, depth + 1); + case 'lazy': + return declaresCollection(def.getter?.(), depth + 1); + case 'pipe': + return declaresCollection(def.in, depth + 1); + case 'union': + return (def.options ?? []).some((option) => declaresCollection(option, depth + 1)); + default: + return false; + } +} + +let objectCollectionKeysCache: ReadonlySet | undefined; + +/** + * The object-level keys `objectConflict: 'merge'` refuses to combine (#14848). + * + * DERIVED from `ObjectSchema`'s shape at first use, never transcribed: every + * key whose declared type is a collection ({@link declaresCollection}) is a + * member, except `fields` — the one collection `'merge'` merges, by its + * documented shallow spread. A hand-written list would be a second statement + * of the object shape (the drift ADR-0116 exists about) and would fail in the + * silent direction: a collection key added to the object schema tomorrow + * would fall back to the wholesale replacement this rule exists to refuse. + * Derived, it joins the refusal set the moment the shape declares it. A key + * the shape does not declare at all is no member either — the strict parse + * refuses it on every authored object before composition sees one. + * + * Resolved lazily rather than at module init: `ObjectSchema` is a lazy schema + * whose factory must not run while `stack.zod.ts` is still loading. + * @internal + */ +function objectCollectionKeys(): ReadonlySet { + if (objectCollectionKeysCache === undefined) { + const keys = new Set(); + for (const [key, schema] of Object.entries(ObjectSchema.shape)) { + if (key === 'fields') continue; + if (declaresCollection(schema)) keys.add(key); + } + objectCollectionKeysCache = keys; + } + return objectCollectionKeysCache; +} + +/** + * The collection keys `obj` declares — own, non-`undefined`, the reading + * {@link composeSingleValue} takes of a top-level declaration — each mapped + * to the declaring stack. Seeds {@link mergeObjects}' per-object ownership on + * a first sighting and on `'override'`. + * @internal + */ +function declaredCollections(obj: object, index: number): Map { + const owners = new Map(); + const record = obj as Record; + for (const key of objectCollectionKeys()) { + if (record[key] !== undefined) owners.set(key, index); + } + return owners; +} + +/** + * The `'merge'` refusal (#14848): the later object declares a collection the + * composed object already carries, with a DIFFERENT value. Records the later + * stack as owner of every collection it is the first to declare, so a third + * stack disagreeing with it is named against it. Identical declarations pass, + * the way {@link composeSingleValue} passes identical top-level values. + * @internal + */ +function refuseUnmergeableCollections( + stacks: ObjectStackDefinition[], + existing: object, + later: object, + owners: Map, + index: number, +): void { + const held = existing as Record; + const incoming = later as Record; + const name = (later as { name: string }).name; + for (const key of objectCollectionKeys()) { + const value = incoming[key]; + if (value === undefined) continue; + const holder = owners.get(key); + if (holder === undefined || held[key] === undefined) { + owners.set(key, index); + continue; + } + if (deepEqualAuthored(held[key], value)) continue; + + throw new Error( + `composeStacks conflict: object '${name}' is defined in multiple stacks and its '${key}' ` + + `is declared with different values by ${stackLabel(stacks[holder], holder)} and ` + + `${stackLabel(stacks[index], index)}.\n` + + `objectConflict: 'merge' shallow-merges 'fields' only. Any other object-level collection ` + + `(${[...objectCollectionKeys()].join(', ')}) is not merged: the later declaration would ` + + `replace the earlier one wholesale, silently dropping every entry ` + + `${stackLabel(stacks[holder], holder)} wrote.\n` + + `Fix: declare '${key}' on '${name}' in exactly one of the two stacks, make the two ` + + `declarations identical, or use { objectConflict: 'override' } to hand the whole object ` + + `to the later stack.`, + ); + } +} + /** * Merge objects from multiple stacks according to the chosen conflict strategy. * + * Under `'merge'` only `fields` is merged — the documented shallow spread, + * later fields winning, earlier fields kept. Every other object-level + * COLLECTION ({@link objectCollectionKeys}: `actions`, `indexes`, `listViews`, + * `validations`, …) is carried from exactly one stack: a later object that + * declares one the composed object already carries, with a different value, + * is refused (#14848, {@link refuseUnmergeableCollections}) — the refusal + * `'error'` uses, naming the object, the collection and both stacks — instead + * of the spread replacing the earlier stack's entries wholesale and in silence + * (the top-level key loss #5005 closed, one level down). Identical + * declarations pass through: two built stacks that each bind one standalone + * action to the same object carry the same copy of it, and nothing is + * dropped. A scalar or a fixed-shape config object (`label`, `sharingModel`, + * `enable`, `access`, …) the later object declares still replaces the earlier + * one — the ruling narrows collections only, and that half is stated here so + * the difference reads as the rule rather than as an oversight. An explicit + * `undefined` is not a declaration anywhere in this composer, and the spread + * agrees: it is dropped from the later object before spreading, so it neither + * counts as a differing value nor erases what the earlier stack declared. + * * Besides the merged list it reports, per composed object name, WHICH input * stack's `actions` array the composed object carries (`actionsOwner`, a stack * index) — the provenance {@link collectComposedActionKeyCollisions} needs to @@ -2840,9 +3001,12 @@ function warnUncomposedStackKey(key: string, rule: ComposeDisposition): void { * from the strategy elsewhere (a second statement of one rule is the drift * ADR-0116 exists about): a first sighting and `'override'` hand the whole * object to stack `i`; under `'merge'` the shallow spread hands `actions` to - * the LATER object only when that object carries the key itself — an absent - * key leaves the earlier stack's array in place — which is exactly what an - * own-property check reads. + * the LATER object only when that object declares the key itself — an absent + * (or explicitly `undefined`) key leaves the earlier stack's array in place — + * which is exactly what a defined-value check reads. The owner model stays + * per-object: a differing `actions` pair is refused before ownership could + * matter, and an identical pair is one declaration whichever stack it is + * attributed to. * @internal */ function mergeObjects( @@ -2853,6 +3017,9 @@ function mergeObjects( const map = new Map(); const result: Obj[] = []; const actionsOwner = new Map(); + // Per composed object, the stack that FIRST declared each collection key — + // the one a `'merge'` refusal names beside the disagreeing later stack. + const collectionOwner = new Map>(); for (const [i, stack] of stacks.entries()) { if (!stack.objects) continue; @@ -2862,6 +3029,7 @@ function mergeObjects( map.set(obj.name, obj); result.push(obj); actionsOwner.set(obj.name, i); + collectionOwner.set(obj.name, declaredCollections(obj, i)); continue; } @@ -2877,14 +3045,19 @@ function mergeObjects( result[idx] = obj; map.set(obj.name, obj); actionsOwner.set(obj.name, i); + collectionOwner.set(obj.name, declaredCollections(obj, i)); break; } case 'merge': { - const merged = { ...existing, ...obj, fields: { ...existing.fields, ...obj.fields } } as Obj; + refuseUnmergeableCollections(stacks, existing, obj, collectionOwner.get(obj.name)!, i); + const declared = Object.fromEntries( + Object.entries(obj).filter(([, value]) => value !== undefined), + ) as Partial; + const merged = { ...existing, ...declared, fields: { ...existing.fields, ...obj.fields } } as Obj; const idx = result.indexOf(existing); result[idx] = merged; map.set(obj.name, merged); - if (Object.prototype.hasOwnProperty.call(obj, 'actions')) actionsOwner.set(obj.name, i); + if (obj.actions !== undefined) actionsOwner.set(obj.name, i); break; } } @@ -2911,10 +3084,13 @@ function mergeObjects( * `'concat'` collection), attributed to the stack that wrote it. * - An embedded action reaches the output through exactly ONE stack's object — * the one `mergeObjects` handed the object's `actions` to (`actionsOwner`). - * Under `objectConflict: 'override'` / `'merge'` the other stacks' embedded - * declarations on that object are not in the artifact at all, so they cannot - * collide with anything; that loss is the object strategy's own semantics, - * not an action collision. + * Under `objectConflict: 'override'` the other stacks' embedded declarations + * on that object are not in the artifact at all, so they cannot collide with + * anything; that loss is the object strategy's own semantics, not an action + * collision. Under `'merge'` two objects declaring DIFFERENT `actions` are + * refused at `mergeObjects` before this walk runs (#14848), and identical + * ones are one declaration carried once — so the array attributed here never + * displaced another stack's entries. * - Only a key declared by TWO OR MORE DISTINCT stacks is reported. A key an * input repeats within itself is `defineStack`'s door (which `strict: false` * opts out of by choice) — and every input BUILT by `defineStack` repeats @@ -3150,8 +3326,14 @@ function assemblePackageBody(stack: ObjectStackDefinition): AssembledPackageBody * object, or drop it from one stack. One global and one object-bound action * may still share a name (two keys); a key an input repeats within itself is * `defineStack`'s door rather than this one; and an embedded action that - * `objectConflict: 'override'` / `'merge'` did not carry into the composed - * object cannot collide. + * `objectConflict: 'override'` did not carry into the composed object cannot + * collide. + * **`objectConflict: 'merge'`** shallow-merges `fields` only (#14848): two + * objects that both declare any other object-level collection (`actions`, + * `indexes`, `listViews`, `validations`, …) with different values are refused, + * naming the object, the collection and both stacks — nothing is dropped in + * silence. Identical declarations pass; a scalar the later object declares + * wins. * * @param stacks - Stack definitions to compose (order matters for conflict resolution) * @param options - Composition options (conflict strategy, manifest selection, etc.) @@ -3171,7 +3353,8 @@ function assemblePackageBody(stack: ObjectStackDefinition): AssembledPackageBody * // Override strategy — later stacks win * const combined = composeStacks([crm, todo], { objectConflict: 'override' }); * - * // Merge strategy — fields from later stacks are shallow-merged + * // Merge strategy — `fields` shallow-merged; a collection both objects + * // declare differently (actions, indexes, …) throws instead of being replaced * const combined = composeStacks([crm, todo], { objectConflict: 'merge' }); * * // Preserve — one artifact carrying BOTH packages, each assembled (ADR-0130 D4) From 387191336df6c9c667d6e008f488bd2967886b8b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 5 Sep 2026 22:54:04 +0000 Subject: [PATCH 2/2] wip(spec): pins + changeset for the 'merge' collection refusal (#14848 WIP) Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01M59rPZZFzqhfMUPFqqZTkf --- ...ompose-merge-refuses-object-collections.md | 65 +++++++++++++++++++ .../src/compose-stacks-action-echo.test.ts | 42 ++++++++---- ...se-stacks-merge-collection-refusal.test.ts | 4 +- 3 files changed, 98 insertions(+), 13 deletions(-) create mode 100644 .changeset/compose-merge-refuses-object-collections.md diff --git a/.changeset/compose-merge-refuses-object-collections.md b/.changeset/compose-merge-refuses-object-collections.md new file mode 100644 index 0000000000..278a7b137d --- /dev/null +++ b/.changeset/compose-merge-refuses-object-collections.md @@ -0,0 +1,65 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec)!: `composeStacks` `objectConflict: 'merge'` refuses object pairs whose object-level collections cannot be merged (#14848) + + + +**BREAKING** accept-set narrowing on `composeStacks({ objectConflict: 'merge' })` +— shipped as `minor` under the repo's launch-window convention for breaking +changes. Maintainer ruling 2026-09-04 on #14848 (director decision batch #38 +item 5, verbatim 「同意」): option 4, `'merge'` **refuses** what it cannot merge +instead of dropping it. + +**What changed.** `'merge'` was implemented as +`{ ...existing, ...obj, fields: { ...existing.fields, ...obj.fields } }`: +`fields` was the only key merged, and every other key the later object carried +— `actions`, `indexes`, `listViews`, `validations`, … — replaced the earlier +package's value wholesale, with nothing at compose, build or boot saying so. +Two packages each embedding an action on one shared object composed to the +later package's array alone; the earlier package's action was gone. + +Now, when both objects declare an object-level **collection** other than +`fields` with different values, `composeStacks` throws — the refusal shape +`'error'` uses — naming the object, the colliding collection and both stacks +by manifest id: + +``` +composeStacks conflict: object 'shared' is defined in multiple stacks and its 'actions' is declared with different values by 'com.example.a' (stack #0) and 'com.example.b' (stack #1). +objectConflict: 'merge' shallow-merges 'fields' only. Any other object-level collection (indexes, fieldGroups, requiredPermissions, validations, activityMilestones, highlightFields, listViews, searchableFields, actions) is not merged: the later declaration would replace the earlier one wholesale, silently dropping every entry 'com.example.a' (stack #0) wrote. +Fix: declare 'actions' on 'shared' in exactly one of the two stacks, make the two declarations identical, or use { objectConflict: 'override' } to hand the whole object to the later stack. +``` + +The refusal set is **derived from `ObjectSchema`'s shape** — every key whose +declared type is an array or a record (through optional/default wrappers and +into a union's members), except `fields` — not hand-listed, so a collection key +added to the object schema joins the refusal without an edit to the composer. +Today that set is `actions`, `activityMilestones`, `fieldGroups`, +`highlightFields`, `indexes`, `listViews`, `requiredPermissions`, +`searchableFields`, `validations`. + +**What did not change.** + +- `fields` keeps its documented shallow merge (later fields win, earlier + fields kept). +- **Identical** declarations on both sides pass through and are carried once + — the same reading `composeStacks` already gives identical top-level values + — so two built stacks that each bind one standalone action to the same + object (identical copies) still reach the cross-stack action-key check + (#14662) and are refused there, by name, as before. +- A scalar or fixed-shape config object the later object declares (`label`, + `sharingModel`, `enable`, `access`, …) still replaces the earlier one: the + ruling narrows collections only, and the docblock now says so. +- The default `'error'` and `'override'` are untouched, message for message. +- An explicit `undefined` on the later object is read as no declaration — it + neither counts as a differing value nor erases what the earlier stack + declared (the bare spread used to let it). + +**Who is affected.** Measured on `origin/main` @ `53cbad9f7`: **zero** non-test +call sites in `packages/**`, `examples/**`, `apps/**` pass `objectConflict` at +all — every real caller takes the default `'error'`. An external author who +opted into `'merge'` and relied on the later package's collection winning +silently now gets the refusal above; the fix is the one it names. + +The `ConflictStrategySchema` docblock for `'merge'` states the rule. diff --git a/packages/spec/src/compose-stacks-action-echo.test.ts b/packages/spec/src/compose-stacks-action-echo.test.ts index 0dfba800a2..37279a6143 100644 --- a/packages/spec/src/compose-stacks-action-echo.test.ts +++ b/packages/spec/src/compose-stacks-action-echo.test.ts @@ -20,6 +20,12 @@ * defineStack(a) (a lone built input) : REFUSED 'a_item:dup_x' is declared twice ← #14686's landed pin * ``` * + * The three-stack `merge` row above is a refusal since #14848: `'merge'` no + * longer hands `actions` to the later object when both declare it differently + * (the lost `emb1` / `emb2` were exactly that), so that arm pins the refusal + * and the echo-once reading stays on `'override'`, whose semantics did not + * move. + * * The discriminator is IDENTITY against the standalone list, not equality * (the triage ruling on the card): the only way an entry of `stack.actions` is * the very same object as an entry of `object.actions` is that a previous merge @@ -116,18 +122,30 @@ describe('composeStacks - a bound standalone action appears once in the composed actions: [act(`b${n}`, { objectName: 'shared' })], }); - it.each(['override', 'merge'] as const)( - "with three stacks under objectConflict: '%s', the surviving object carries the surviving stack's declared actions plus each concatenated standalone once", - (objectConflict) => { - const out = composeStacks([s(1), s(2), s(3)], { objectConflict }); - expect(keysOf(out).top).toEqual(['shared:b1', 'shared:b2', 'shared:b3']); - // s3's object survives with its built array as-is (embedded `emb3`, then - // the echo of its own `b3`); s1's and s2's bound actions join it once - // each, in concatenation order. The lost `emb1` / `emb2` are the object - // strategy's own semantics, not this merge's. Before: `b3/BOUND` twice. - expect(keysOf(out).embedded).toEqual({ shared: ['emb3/EMB', 'b3/BOUND', 'b1/BOUND', 'b2/BOUND'] }); - }, - ); + it("with three stacks under objectConflict: 'override', the surviving object carries the surviving stack's declared actions plus each concatenated standalone once", () => { + const out = composeStacks([s(1), s(2), s(3)], { objectConflict: 'override' }); + expect(keysOf(out).top).toEqual(['shared:b1', 'shared:b2', 'shared:b3']); + // s3's object survives with its built array as-is (embedded `emb3`, then + // the echo of its own `b3`); s1's and s2's bound actions join it once + // each, in concatenation order. The lost `emb1` / `emb2` are the object + // strategy's own semantics, not this merge's. Before: `b3/BOUND` twice. + expect(keysOf(out).embedded).toEqual({ shared: ['emb3/EMB', 'b3/BOUND', 'b1/BOUND', 'b2/BOUND'] }); + }); + + it("with three stacks under objectConflict: 'merge', the composition is refused at the object merge — each built object carries a different `actions` array (#14848)", () => { + // s1's `shared` carries [emb1, b1/BOUND], s2's [emb2, b2/BOUND]: two + // declarations `'merge'` used to resolve by replacement, dropping `emb1`. + let msg: string | null = null; + try { + composeStacks([s(1), s(2), s(3)], { objectConflict: 'merge' }); + } catch (e) { + msg = (e as Error).message; + } + expect(msg).toContain( + "composeStacks conflict: object 'shared' is defined in multiple stacks and its 'actions' is declared " + + "with different values by 'com.example.s1' (stack #0) and 'com.example.s2' (stack #1).", + ); + }); it("still binds another stack's standalone to an object it does not own — the add-on shape — once", () => { const core = defineStack({ manifest: mf('com.example.core'), objects: [obj('core_item', [act('archive')])] }); diff --git a/packages/spec/src/compose-stacks-merge-collection-refusal.test.ts b/packages/spec/src/compose-stacks-merge-collection-refusal.test.ts index f0ad94f87b..b1f65674e8 100644 --- a/packages/spec/src/compose-stacks-merge-collection-refusal.test.ts +++ b/packages/spec/src/compose-stacks-merge-collection-refusal.test.ts @@ -163,7 +163,9 @@ describe("composeStacks objectConflict: 'merge' — what stays accepted", () => const out = composeStacks([a, b], { objectConflict: 'merge' }); const s = shared(out)!; expect((s.actions ?? []).map((x) => x.name)).toEqual(['approve']); - expect(s.indexes).toEqual([{ fields: ['title'] }]); + // The strict parse fills `unique: false`; the subject is one entry, carried once. + expect(s.indexes).toHaveLength(1); + expect(s.indexes?.[0].fields).toEqual(['title']); }); it("keeps the earlier stack's collection when the later object does not declare it", () => {