diff --git a/.changeset/compose-stacks-action-key-collision.md b/.changeset/compose-stacks-action-key-collision.md new file mode 100644 index 0000000000..107761e127 --- /dev/null +++ b/.changeset/compose-stacks-action-key-collision.md @@ -0,0 +1,17 @@ +--- +'@objectstack/spec': minor +--- + +`composeStacks` now refuses two input stacks whose action declarations resolve to one scope-qualified runtime key — **BREAKING** accept-set narrowing, shipped as `minor` under the repo's launch-window convention for breaking changes. + +**The refused shape:** two (or more) input stacks that each declare an action resolving to one runtime key — `objectName:name`, or `global:name` for an object-less action. The canonical case is two packages, each legal on its own, each declaring a global action named `shared_refresh`: `composeStacks([a, b])` used to accept the pair and emit `["global:shared_refresh", "global:shared_refresh"]`, and the runtime — which registers and dispatches every action under that one exact-string key — collapsed both handlers to one registration: whichever registered second won, and the other package's action stayed a live, declared, permission-gated button whose handler was unreachable. `defineStack` refuses exactly this collision within one stack (#14686); composition was the second door in the same file that let it through. + +The refusal carries `composeStacks`' conflict prefix and `defineStack`'s envelope shape — `composeStacks conflict: cross-stack action key collision (N issue(s)):`, one `✗` line per colliding key — and every line names the key, **both source stacks by manifest id** (`'com.example.a' (stack #0)`; a manifest-less input is named by position), and where each declaration sits (`stack.actions[i]`, or `objects['OBJECT'].actions[j]` for an embedded one). The fix is the one the message names: rename one of the colliding actions within its scope, bind one of them to a different object, or remove the duplicate from one of the stacks. ⛔ There is no `actionConflict` option and none is coming: `composeStacks` does not pick a winner for actions (maintainer ruling, 2026-09-03). + +What the check judges is what composition **carries**: standalone actions from every stack (they concatenate), and each composed object's embedded actions attributed to the one stack whose object `objectConflict` handed the `actions` array to. So an embedded action that `'override'` or `'merge'` did not carry into the composed object cannot collide — that loss is the object strategy's own semantics — while a standalone action bound to an object from one stack does collide with an embedded action of the same name the composed object carries from another. Only a key declared by two or more **distinct** stacks is reported: a key an input repeats within itself is `defineStack`'s door (`strict: false` opts out there by choice), and an input built by `defineStack` legitimately carries each bound standalone action twice — as the copy the build appended to its object — which is never a collision with itself. + +Deliberately unchanged, as in `defineStack`: one global and one object-bound action may share a name across stacks (two keys), and one name bound to two different objects is two keys. The shipped composer, `examples/app-multi-package`, declares no colliding key and composes unchanged; no `composeStacks` caller exists in objectui or hotcrm. + +**Migration.** A composition refused by the new check must resolve the collision in one of the two packages — rename the action within its scope, bind it to a different object, or drop the duplicate. Which package keeps the name is an authoring decision the metadata cannot make for you. + + diff --git a/packages/spec/src/compose-stacks-action-key-collision.test.ts b/packages/spec/src/compose-stacks-action-key-collision.test.ts new file mode 100644 index 0000000000..04ba926d1f --- /dev/null +++ b/packages/spec/src/compose-stacks-action-key-collision.test.ts @@ -0,0 +1,317 @@ +/** + * `composeStacks` refuses two INPUT STACKS whose action declarations resolve + * to one scope-qualified runtime key — and only those (#14662). + * + * `defineStack` refuses the same collision within one stack + * (`stack-duplicate-action-key.test.ts`); the runtime keys a composed artifact + * the same way (`objectName:name`, `global:name` for an object-less action), + * so two packages that are each legal on their own and both declare + * `global:shared_refresh` composed into one collapsed handler key — the same + * dead button, arriving one composition step later. + * + * Measured on `main` @ `f3ae441fa` before the check existed, `defineStack` + * outputs as inputs (the shape `examples/app-multi-package` composes): + * + * ``` + * P1 global(A) + global(B) : ACCEPTED actions=["global:shared_refresh","global:shared_refresh"] + * P1 … with manifest: 'preserve' : ACCEPTED same + * P4 bound(A→shared) + bound(B→shared), merge : ACCEPTED shared.actions=[dup_s/BOUND ×3] + * P5b bound(B→shared) + embedded(A on shared), merge : ACCEPTED shared.actions=[dup_m/EMB, dup_m/BOUND] + * P2 bound(A→a_item) + global(B) : ACCEPTED two keys — stays accepted + * P3 embedded(A on shared) + embedded(B on shared) : merge/override ACCEPTED — B's array REPLACES A's + * ``` + * + * 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 + * is broken" (`objectConflict: 'error'` throws on several of these fixtures + * for a different reason). + * + * The inputs are BUILT stacks on purpose: `defineStack` copies each bound + * standalone action into its object's `actions` on the way out, so a built + * input already carries one declaration in two sites. That echo must never + * read as a collision — the check counts distinct stacks per key, not sites. + */ +import { describe, it, expect } from 'vitest'; +import { composeStacks, defineStack, type ObjectStackDefinition } from './stack.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, actions?: ReturnType[]) => ({ + name, + label: name, + fields: { title: { type: 'text' as const } }, + ...(actions ? { actions } : {}), +}); + +/** The runtime keys a composed stack would register, per position. */ +const keysOf = (s: ObjectStackDefinition) => ({ + top: (s.actions ?? []).map((a) => `${a.objectName ?? 'global'}:${a.name}`), + embedded: Object.fromEntries( + (s.objects ?? []).map((o) => [o.name, (o.actions ?? []).map((a) => `${a.name}/${a.objectName ? 'BOUND' : 'EMB'}`)]), + ), +}); + +/** 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 ENVELOPE_ONE = 'composeStacks conflict: cross-stack action key collision (1 issue):'; +const WHY = + "The runtime registers and dispatches every action under one exact-string key — the owning object's name " + + "(or 'global' for an object-less action), a colon, then the action name — so only one of these handlers " + + 'would be reachable in the composed artifact and the other declaration is a dead button: the collision ' + + 'defineStack refuses within one stack, arriving one composition step later. Each stack is legal on its own; ' + + 'the collision is between them.'; +const FIX = + 'Fix: rename one of the colliding actions within its scope, bind one of them to a different object, or ' + + 'remove the duplicate from one of the stacks. composeStacks does not pick a winner for actions.'; + +// Two legal packages, each declaring one global `shared_refresh` — the card's case. +const globalA = () => defineStack({ manifest: mf('com.example.a'), objects: [obj('a_item')], actions: [act('shared_refresh')] }); +const globalB = () => defineStack({ manifest: mf('com.example.b'), objects: [obj('b_item')], actions: [act('shared_refresh')] }); + +describe('composeStacks - two stacks declaring one global action key', () => { + it('refuses two legal stacks each declaring global:shared_refresh, naming both manifest ids and both sites', () => { + const a = globalA(); + const b = globalB(); + const msg = refusal(() => composeStacks([a, b])); + expect(msg).toBe( + `${ENVELOPE_ONE}\n\n` + + " ✗ Action key 'global:shared_refresh' is declared by 2 stacks: " + + "'com.example.a' (stack #0) at stack.actions[0] and 'com.example.b' (stack #1) at stack.actions[0].\n\n" + + `${WHY}\n${FIX}`, + ); + }); + + it("refuses the same pair under manifest: 'preserve' — the shape app-multi-package composes", () => { + const msg = refusal(() => composeStacks([globalA(), globalB()], { manifest: 'preserve' })); + expect(msg).toContain(ENVELOPE_ONE); + expect(msg).toContain( + " ✗ Action key 'global:shared_refresh' is declared by 2 stacks: " + + "'com.example.a' (stack #0) at stack.actions[0] and 'com.example.b' (stack #1) at stack.actions[0].", + ); + }); + + it('names no strategy option — there is none for actions by ruling', () => { + const msg = refusal(() => composeStacks([globalA(), globalB()])); + expect(msg).not.toMatch(/actionConflict|objectConflict/); + }); + + it('lists all three stacks when three declare the key, still as one issue', () => { + const c = defineStack({ manifest: mf('com.example.c'), actions: [act('shared_refresh')] }); + const msg = refusal(() => composeStacks([globalA(), globalB(), c])); + expect(msg).toContain(ENVELOPE_ONE); + expect(msg).toContain( + " ✗ Action key 'global:shared_refresh' is declared by 3 stacks: " + + "'com.example.a' (stack #0) at stack.actions[0], 'com.example.b' (stack #1) at stack.actions[0] " + + "and 'com.example.c' (stack #2) at stack.actions[0].", + ); + }); + + it('reports two colliding keys as two issues, one line each', () => { + const a = defineStack({ manifest: mf('com.example.a'), actions: [act('refresh'), act('export')] }); + const b = defineStack({ manifest: mf('com.example.b'), actions: [act('export'), act('refresh')] }); + const msg = refusal(() => composeStacks([a, b])); + expect(msg).toContain('composeStacks conflict: cross-stack action key collision (2 issues):'); + expect(msg).toContain( + " ✗ Action key 'global:refresh' is declared by 2 stacks: " + + "'com.example.a' (stack #0) at stack.actions[0] and 'com.example.b' (stack #1) at stack.actions[1].", + ); + expect(msg).toContain( + " ✗ Action key 'global:export' is declared by 2 stacks: " + + "'com.example.a' (stack #0) at stack.actions[1] and 'com.example.b' (stack #1) at stack.actions[0].", + ); + }); + + it("keys an empty-string objectName as global — the sibling walk, the merge and the runtime ladder all resolve '' by truthiness", () => { + // Type-legal, refused by ActionSchema's regex only under a strict parse, + // so reachable through `strict: false`; `??` would have keyed it as ':dup_g'. + const a = defineStack({ manifest: mf('com.example.a'), actions: [act('dup_g', { objectName: '' })] }, { strict: false }); + const b = defineStack({ manifest: mf('com.example.b'), actions: [act('dup_g')] }, { strict: false }); + const msg = refusal(() => composeStacks([a, b])); + expect(msg).toContain(ENVELOPE_ONE); + expect(msg).toContain( + " ✗ Action key 'global:dup_g' is declared by 2 stacks: " + + "'com.example.a' (stack #0) at stack.actions[0] and 'com.example.b' (stack #1) at stack.actions[0].", + ); + }); + + it('names a manifest-less input by position', () => { + const a = defineStack({ actions: [act('dup_n')] }, { strict: false }); + const b = defineStack({ actions: [act('dup_n')] }, { strict: false }); + const msg = refusal(() => composeStacks([a, b])); + expect(msg).toContain( + " ✗ Action key 'global:dup_n' is declared by 2 stacks: stack #0 at stack.actions[0] and stack #1 at stack.actions[0].", + ); + }); +}); + +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. + 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' })] }); + + it.each(['merge', 'override'] as const)( + 'refuses two stacks each binding a standalone action to the same object (objectConflict: %s)', + (objectConflict) => { + const msg = refusal(() => composeStacks([boundA(), boundB()], { objectConflict })); + expect(msg).toContain(ENVELOPE_ONE); + expect(msg).toContain( + " ✗ Action key 'shared:dup_s' is declared by 2 stacks: " + + "'com.example.a' (stack #0) at stack.actions[0] and " + + "'com.example.b' (stack #1) at stack.actions[0] + objects['shared'].actions[0].", + ); + }, + ); + + it("under objectConflict: 'error' the object conflict is what throws — the fixtures are legal on their own", () => { + const msg = refusal(() => composeStacks([boundA(), boundB()])); + expect(msg).toContain("composeStacks conflict: object 'shared' is defined in multiple stacks."); + expect(msg).not.toContain('action key'); + }); + + // A embeds `dup_m` on `shared`; B binds a standalone `dup_m` to `shared`. + 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); + 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(['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.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)", + (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')])] }); + const out = composeStacks([a, b], { objectConflict }); + expect(keysOf(out).embedded).toEqual({ shared: ['dup_e/EMB'] }); + expect(keysOf(out).top).toEqual([]); + }, + ); + + it('refuses a standalone bound to an object the OTHER stack owns when that object embeds the same name', () => { + // An add-on package (strict: false — it does not declare `core_item` + // itself) binds `approve` to the core package's object, which already + // embeds an `approve`. Composition merges the add-on's action into the + // core object at `mergeActionsIntoObjects`: two stacks, one key. + const core = defineStack({ manifest: mf('com.example.core'), objects: [obj('core_item', [act('approve')])] }); + const addon = defineStack({ manifest: mf('com.example.addon'), actions: [act('approve', { objectName: 'core_item' })] }, { strict: false }); + const msg = refusal(() => composeStacks([core, addon])); + expect(msg).toContain(ENVELOPE_ONE); + expect(msg).toContain( + " ✗ Action key 'core_item:approve' is declared by 2 stacks: " + + "'com.example.core' (stack #0) at objects['core_item'].actions[0] and " + + "'com.example.addon' (stack #1) at stack.actions[0].", + ); + }); +}); + +describe('composeStacks - what stays accepted', () => { + it('accepts the cross-scope pair: one stack binds a name to its object, the other declares it global — two keys', () => { + const a = defineStack({ manifest: mf('com.example.a'), objects: [obj('a_item')], actions: [act('dup_x', { objectName: 'a_item' })] }); + const b = defineStack({ manifest: mf('com.example.b'), objects: [obj('b_item')], actions: [act('dup_x')] }); + const out = composeStacks([a, b]); + expect(keysOf(out).top).toEqual(['a_item:dup_x', 'global:dup_x']); + }); + + it('accepts one name bound to two different objects from two stacks — two keys', () => { + const a = defineStack({ manifest: mf('com.example.a'), objects: [obj('a_item')], actions: [act('dup_o', { objectName: 'a_item' })] }); + const b = defineStack({ manifest: mf('com.example.b'), objects: [obj('b_item')], actions: [act('dup_o', { objectName: 'b_item' })] }); + const out = composeStacks([a, b]); + expect(keysOf(out).top).toEqual(['a_item:dup_o', 'b_item:dup_o']); + }); + + it("does not read a built input's own echo (a bound standalone plus the copy defineStack put on its object) as a collision", () => { + const a = defineStack({ manifest: mf('com.example.a'), objects: [obj('a_item')], actions: [act('bound_one', { objectName: 'a_item' })] }); + // The build already carries the declaration twice — one stack, one key. + expect(keysOf(a).embedded).toEqual({ a_item: ['bound_one/BOUND'] }); + const b = defineStack({ manifest: mf('com.example.b'), objects: [obj('b_item')], actions: [act('other')] }); + expect(refusal(() => composeStacks([a, b]))).toBeNull(); + expect(refusal(() => composeStacks([a, b], { manifest: 'preserve' }))).toBeNull(); + }); + + it("leaves a key one input repeats WITHIN itself to defineStack's door — composeStacks reports cross-stack collisions only", () => { + // `strict: false` opted out of defineStack's walk; composition does not + // re-run it, and the repeat is one stack, not two. + const a = defineStack({ manifest: mf('com.example.a'), actions: [act('dup_n'), act('dup_n')] }, { strict: false }); + const b = defineStack({ manifest: mf('com.example.b'), actions: [act('other')] }); + expect(refusal(() => composeStacks([a, b]))).toBeNull(); + }); + + it('accepts distinct keys in every position across stacks', () => { + const a = defineStack({ manifest: mf('com.example.a'), objects: [obj('a_item', [act('embedded_a')])], actions: [act('global_a'), act('bound_a', { objectName: 'a_item' })] }); + const b = defineStack({ manifest: mf('com.example.b'), objects: [obj('b_item', [act('embedded_b')])], actions: [act('global_b'), act('bound_b', { objectName: 'b_item' })] }); + const out = composeStacks([a, b]); + expect(keysOf(out).top).toEqual(['global_a', 'a_item:bound_a', 'global_b', 'b_item:bound_b'].map((k) => (k.includes(':') ? k : `global:${k}`))); + }); + + it("composes the app-multi-package shape unchanged — an App and a Module sharing a namespace, no action key in common, manifest: 'preserve'", () => { + // A mirror of `examples/app-multi-package` (the one shipped composer): the + // example itself declares no action at all, so it is composed as-is by the + // corpus run recorded on the PR; this mirror gives each package one + // distinct action so the check has something to walk. + const core = defineStack({ + manifest: { ...mf('com.example.multi.core'), namespace: 'crm' }, + objects: [obj('crm_account', [act('archive_account')])], + apps: [{ name: 'crm_app', label: 'CRM', navigation: [{ id: 'nav_accounts', type: 'object' as const, objectName: 'crm_account', label: 'Accounts' }] }], + }); + const orders = defineStack({ + manifest: { ...mf('com.example.multi.orders'), namespace: 'crm', type: 'module' as const, dependencies: { 'com.example.multi.core': '^1.0.0' } }, + objects: [obj('crm_order')], + actions: [act('ship_order', { objectName: 'crm_order' })], + }); + const out = composeStacks([orders, core], { manifest: 'preserve' }); + expect(out.packages).toHaveLength(2); + expect(keysOf(out).top).toEqual(['crm_order:ship_order']); + // #14847: the bound action appears twice on `crm_order` today — the measured + // shape, not the contract — so the pin asks only that it is carried, and + // that the other package's embedded action is carried exactly once. + expect(keysOf(out).embedded.crm_order).toContain('ship_order/BOUND'); + expect(keysOf(out).embedded.crm_account).toEqual(['archive_account/EMB']); + }); +}); diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index 493e930707..311458ea92 100644 --- a/packages/spec/src/stack.zod.ts +++ b/packages/spec/src/stack.zod.ts @@ -391,7 +391,9 @@ const STACK_DEFINITION_COLLECTIONS_SHAPE = { + "object's actions, or one in each position, identical twins included (an embedded action is keyed " + 'by the object it is written on, not by its own objectName). ' + "One global and one object-bound action may share " - + "a name; on that object's route the object's own actions take precedence for by-name readers.", + + "a name; on that object's route the object's own actions take precedence for by-name readers. " + + 'composeStacks runs the same key rule across its input stacks (counting distinct stacks, not sites) ' + + 'and names both source stacks on a collision.', ), // `themes` was REMOVED in 17.1 (#10485, ADR-0049 enforce-or-remove — ruled // 退役授权面, 2026-08-21). The pipeline was live from authoring gate through @@ -2681,23 +2683,37 @@ function warnUncomposedStackKey(key: string, rule: ComposeDisposition): void { /** * Merge objects from multiple stacks according to the chosen conflict strategy. + * + * 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 + * tell a cross-stack collision from one stack's own declarations. It is + * recorded here, beside the spread that decides it, rather than re-derived + * 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. * @internal */ function mergeObjects( stacks: ObjectStackDefinition[], strategy: ConflictStrategy, -): ObjectStackDefinition['objects'] { +): { objects: ObjectStackDefinition['objects']; actionsOwner: Map } { type Obj = NonNullable[number]; const map = new Map(); const result: Obj[] = []; + const actionsOwner = new Map(); - for (const stack of stacks) { + for (const [i, stack] of stacks.entries()) { if (!stack.objects) continue; for (const obj of stack.objects) { const existing = map.get(obj.name); if (!existing) { map.set(obj.name, obj); result.push(obj); + actionsOwner.set(obj.name, i); continue; } @@ -2712,6 +2728,7 @@ function mergeObjects( const idx = result.indexOf(existing); result[idx] = obj; map.set(obj.name, obj); + actionsOwner.set(obj.name, i); break; } case 'merge': { @@ -2719,13 +2736,133 @@ function mergeObjects( 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); break; } } } } - return result.length > 0 ? result : undefined; + return { objects: result.length > 0 ? result : undefined, actionsOwner }; +} + +/** + * Cross-stack duplicate action keys over the COMPOSED action set (#14662). + * + * `defineStack` refuses two declarations that resolve to one scope-qualified + * runtime key within ONE stack ({@link collectDuplicateActionKeyErrors}), and + * the runtime keys a composed artifact exactly the same way — so two input + * stacks, each legal on its own, each declaring `global:shared_refresh`, + * composed into one collapsed handler key: the same dead button, arriving one + * composition step later. This is that walk over the composed set, with every + * declaration attributed to the stack it came from. + * + * Judged on what composition CARRIES, not on what each input declared: + * + * - A standalone action reaches the output from every stack (`actions` is a + * `'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. + * - 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 + * each bound standalone action legitimately, as the copy + * `mergeActionsIntoObjects` appended to its object on the way out. Counting + * sites instead of stacks would refuse every composition of built stacks that + * binds an action to an object. + * + * Deliberately NOT refused, as in `defineStack`: the cross-scope pair (a global + * and an object-bound action sharing a name — two keys) and one name bound to + * two different objects. + * + * Runs BEFORE `mergeActionsIntoObjects` so the composed output's own echo is + * not counted either. Returns one line per colliding key, in first-seen order. + * @internal + */ +function collectComposedActionKeyCollisions( + stacks: ObjectStackDefinition[], + composedObjects: ObjectStackDefinition['objects'], + actionsOwner: ReadonlyMap, +): string[] { + type Action = NonNullable[number]; + // key → stack index → where that stack declares it + const sitesByKey = new Map>(); + const note = (scope: string, name: string, stackIndex: number, site: string): void => { + const key = `${scope}:${name}`; + const byStack = sitesByKey.get(key) ?? new Map(); + const sites = byStack.get(stackIndex) ?? []; + sites.push(site); + byStack.set(stackIndex, sites); + sitesByKey.set(key, byStack); + }; + + for (const [i, stack] of stacks.entries()) { + // A non-array `actions` never reaches the output: the concat pass drops it + // (and warns), so it declares nothing here either. + const declared = (stack as Record).actions; + if (!Array.isArray(declared)) continue; + for (const [j, action] of (declared as Action[]).entries()) { + // Truthiness, not nullish: an empty-string `objectName` (type-legal; + // refused by ActionSchema's regex only under strict parse) keys as + // global here exactly as `collectDuplicateActionKeyErrors`, + // `mergeActionsIntoObjects` and objectql's `standaloneActionOwnerKey` + // resolve it. + note(action.objectName || GLOBAL_ACTION_SCOPE, action.name, i, `stack.actions[${j}]`); + } + } + for (const obj of composedObjects ?? []) { + const owner = actionsOwner.get(obj.name); + if (owner === undefined) { + // Every composed object is recorded by `mergeObjects` the moment it is + // first seen; a miss is an edit to that function that forgot the map, + // and skipping would hide exactly the collisions this walk exists for. + throw new Error(`composeStacks internal error: no source stack recorded for composed object '${obj.name}'.`); + } + for (const [j, action] of (obj.actions ?? []).entries()) { + note(obj.name, action.name, owner, `objects['${obj.name}'].actions[${j}]`); + } + } + + const errors: string[] = []; + for (const [key, byStack] of sitesByKey) { + if (byStack.size < 2) continue; + // Stack order, not note order — standalone declarations are walked before + // embedded ones, so a later stack's standalone would otherwise be named + // ahead of an earlier stack's embedded action. + const parties = [...byStack.entries()] + .sort(([a], [b]) => a - b) + .map(([index, sites]) => `${stackLabel(stacks[index], index)} at ${sites.join(' + ')}`); + errors.push(`Action key '${key}' is declared by ${byStack.size} stacks: ${joinDeclarationOrigins(parties)}.`); + } + return errors; +} + +/** + * The refusal `composeStacks` throws for {@link collectComposedActionKeyCollisions} + * findings — `defineStack`'s envelope shape (a count, one `✗` line per key) + * under the `composeStacks conflict:` prefix every other composition refusal + * carries. Names no strategy option: there is none for actions by ruling. + * @internal + */ +function formatComposedActionKeyCollisions(lines: readonly string[]): string { + const header = + `composeStacks conflict: cross-stack action key collision ` + + `(${lines.length} issue${lines.length === 1 ? '' : 's'}):`; + const body = lines.map((line) => ` ✗ ${line}`).join('\n'); + const why = + `The runtime registers and dispatches every action under one exact-string key — the owning ` + + `object's name (or '${GLOBAL_ACTION_SCOPE}' for an object-less action), a colon, then the action ` + + `name — so only one of these handlers would be reachable in the composed artifact and the ` + + `other declaration is a dead button: the collision defineStack refuses within one stack, ` + + `arriving one composition step later. Each stack is legal on its own; the collision is between them.`; + const fix = + `Fix: rename one of the colliding actions within its scope, bind one of them to a different ` + + `object, or remove the duplicate from one of the stacks. composeStacks does not pick a winner for actions.`; + return `${header}\n\n${body}\n\n${why}\n${fix}`; } /** @@ -2856,6 +2993,17 @@ function assemblePackageBody(stack: ObjectStackDefinition): AssembledPackageBody * neither overridden nor merged: identical declarations pass through, and two * stacks declaring *different* values throw an error naming both stacks * (#5005; `i18n` joined them in #5051). + * **Actions** concatenate like every other collection, and the composed set is + * then checked the way `defineStack` checks one stack (#14662): two input + * stacks whose declarations resolve to one scope-qualified runtime key + * (`objectName:name`, or `global:name` for an object-less action) throw, and + * the error names both stacks by manifest id and where each declaration sits. + * There is no strategy option for actions — rename one, bind it to a different + * 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. * * @param stacks - Stack definitions to compose (order matters for conflict resolution) * @param options - Composition options (conflict strategy, manifest selection, etc.) @@ -2868,7 +3016,8 @@ function assemblePackageBody(stack: ObjectStackDefinition): AssembledPackageBody * const crm = defineStack({ ... }); * const todo = defineStack({ ... }); * - * // Simple composition — throws on duplicate objects + * // Simple composition — throws on duplicate objects, and on two stacks + * // declaring one scope-qualified action key * const combined = composeStacks([crm, todo]); * * // Override strategy — later stacks win @@ -2902,8 +3051,9 @@ export function composeStacks( // and no consumer reading `composed.manifest` loses a key. composed.manifest = selectManifest(stacks, opts.manifest === 'preserve' ? 'last' : opts.manifest); - // 2. Objects — use conflict strategy - const objects = mergeObjects(stacks, opts.objectConflict); + // 2. Objects — use conflict strategy (and remember, per composed object, + // which stack's `actions` array it carries — step 6 reads that). + const { objects, actionsOwner } = mergeObjects(stacks, opts.objectConflict); if (objects) { composed.objects = objects; } @@ -2987,5 +3137,16 @@ export function composeStacks( if (single.declared) composed[key] = single.value; } + // 6. Cross-stack action key collisions (#14662) — the check `defineStack` + // runs within one stack, over what composition actually carries. AFTER + // every collection is composed, and BEFORE `mergeActionsIntoObjects` + // copies each bound standalone action into its object: that copy is the + // echo an input built by `defineStack` already carries, and counting it + // would make every bound action collide with itself. + const actionCollisions = collectComposedActionKeyCollisions(stacks, objects, actionsOwner); + if (actionCollisions.length > 0) { + throw new Error(formatComposedActionKeyCollisions(actionCollisions)); + } + return mergeActionsIntoObjects(composed as ObjectStackDefinition); }