Skip to content

feat(spec): composeStacks refuses two stacks whose actions resolve to one scope-qualified runtime key - #14854

Merged
os-sam merged 2 commits into
mainfrom
claude/issue-14662-compose-stacks-action-key-collision
Sep 3, 2026
Merged

feat(spec): composeStacks refuses two stacks whose actions resolve to one scope-qualified runtime key#14854
os-sam merged 2 commits into
mainfrom
claude/issue-14662-compose-stacks-action-key-collision

Conversation

@claude

@claude claude Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #14662

Summary

composeStacks concatenated actions across its inputs with no same-scope duplicate-key check. Two packages, each legal on its own, each declaring a global action named shared_refresh, composed into ["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: the dead button defineStack refuses within one stack (#14686), arriving one composition step later.

Maintainer ruling on the card (5520465813, director seat, 2026-09-03, verbatim): 「同意,然后执行契约复审」 — option A. composeStacks now runs the same scope-qualified key check over the composed action set; a collision throws, naming both source stacks by manifest id and where each declaration sits. ⛔ No actionConflict option. ⛔ objectConflict, devPlugins (#14722), ManifestSchema and formatZodError are untouched; #14124 is not widened.

Clause-②: yes — a published API's accept set narrows. This PR is a DRAFT and carries needs:contract-review; it is not flipped to ready by this seat. Contract review at CONTRACT_REVIEW_TIER per the pm-dispatch reference.

What changed (packages/spec/src/stack.zod.ts only, plus tests and a changeset)

  • mergeObjects now also returns actionsOwner — per composed object name, the index of the input stack whose actions array the composed object carries. Recorded beside the spread that decides it (first sighting and 'override' hand the whole object to stack i; 'merge' hands actions to the later object only when that object has the key itself), so the provenance is never re-derived from the strategy elsewhere.
  • New collectComposedActionKeyCollisions(stacks, composedObjects, actionsOwner): walks every input's standalone actions (they concatenate, attributed to the stack that wrote them) and every composed object's embedded actions (attributed to actionsOwner), keys each as objectName:name / global:name exactly as collectDuplicateActionKeyErrors does, and reports every key declared by two or more DISTINCT stacks.
  • New formatComposedActionKeyCollisions: defineStack's envelope shape under the composeStacks conflict: prefix; names no strategy option.
  • composeStacks step 6 runs the check after every collection is composed and BEFORE mergeActionsIntoObjects (see H1 below for why that order is load-bearing).
  • composeStacks JSDoc and the actions collection's describe text state the rule.

The exact refusal (the card's case, two legal stacks each declaring a global shared_refresh)

composeStacks conflict: cross-stack action key collision (1 issue):

  ✗ 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].

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.
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.

A manifest-less input is named by position (stack #0); a stack that declares a key at several sites lists them joined by + (stack.actions[0] + objects['shared'].actions[0]); N colliding keys are N lines under (N issues).

PM mechanism hypotheses — measured (all probes on origin/main f3ae441fa, defineStack outputs as inputs, source not dist)

# Hypothesis Measurement Verdict
H1 Reuse collectDuplicateActionKeyErrors over the composed set before mergeActionsIntoObjects; provenance is lost after 'concat' Falsified as written, and for a sharper reason than provenance. Every defineStack OUTPUT already carries each bound standalone action twice — in stack.actions and as the copy mergeActionsIntoObjects appended to its object on the way out. Running the site-counting walk over a composition of built stacks would refuse every bound action as a collision with itself (composeStacks([a, b]) with A binding dup_x to a_item and B unrelated: a_item:dup_x at stack.actions[0] AND objects['a_item'].actions[0], both from A). So the check counts DISTINCT STACKS per key, not sites; provenance for embedded actions comes from mergeObjects (actionsOwner), for standalone ones from the input index. The check still runs BEFORE mergeActionsIntoObjects so the composition's own echo is not counted either. Same error class (Error). Reuse the KEY RULE, not the walk
H2 Match composeStacks conflict: shape; no actionConflict in the remedy; check the error-code ledger composeSingleValue / mergeObjects throw plain Error with a composeStacks conflict: prefix and a Fix: tail — matched, with defineStack's (N issue(s)): + line envelope for aggregation. Message names no option (pinned: not.toMatch(/actionConflict|objectConflict/)). check:dispatcher-error-vocabulary (ADR-0112) governs HTTP dispatcher error.code envelopes — an authoring-time Error thrown by composeStacks is not on that surface; the gate ran green (#34 below) and check:error-code-casing is not in the derived family. No ledger entry needed. Confirmed
H3 Object-bound vs global same name do NOT collide; two object-bound on one merged object DO collide; measure mergeObjects per strategy 'merge' = { ...existing, ...obj, fields: { ...existing.fields, ...obj.fields } }: only fields merges; actions (like every other key) is the LATER object's whenever it carries the key. 'override' replaces the object wholesale. Measured base behaviour and the pinned verdicts: (P2) bound(A→a_item) + global(B) — two keys, ACCEPTED; (P6) same name bound to a_item (A) and b_item (B) — two keys, ACCEPTED; (P4) both stacks bind a standalone dup_s to a shared object, merge or override — both standalones concatenate, REFUSED naming A at stack.actions[0] and B at stack.actions[0] + objects['shared'].actions[0]; (P5b) B binds a standalone to shared, A (last) embeds the same name — A's object wins, A's embedded + B's standalone, REFUSED; (P5) the same pair with A first — B's object wins and A's embedded action is NOT carried, ACCEPTED (one handler reaches the key); (P3) both stacks EMBED the same name on one object, merge or override — the later array replaces the earlier, one entry, ACCEPTED. The rule is judged on what composition CARRIES: an embedded action the object strategy discarded cannot collide — that loss is the strategy's own semantics (filed as #14848, not addressed here). Confirmed and pinned per strategy
H4 examples/app-multi-package composes unchanged; re-run #14686's corpus pass #14686's corpus pass loaded the four examples through defineStack and counted declared actions (crm 1 / showcase 70 / todo 8 / multi-package 0); it is a PR-recorded run, not a checked-in script. app-multi-package is the only composeStacks caller in the repo (and hotcrm / objectui carry none — director measurement on the card). Run here through the BUILT spec (the example resolves @objectstack/spec to dist/) at this head: manifest.id=com.example.multi.core, packages=[com.example.multi.orders, com.example.multi.core], objects=[crm_order, crm_account], top actions 0, embedded actions 0 — composes unchanged. A mirror of that shape with one distinct action per package is pinned in the new suite. Confirmed

Derived judgments for the contract review (each one a decision this diff makes; the reviewer should confirm or refuse them)

  1. Distinct-stack counting. Only a key declared by two or more distinct input stacks is refused. A key one input repeats within itself is defineStack's door (strict: false opted out of it there by choice) and is NOT re-checked by composeStacks — pinned. The alternative (re-running the within-stack walk) is impossible without first solving the build echo in H1.
  2. Judged on what composition carries. Under objectConflict: 'override' / 'merge', an earlier stack's embedded action that the strategy did not carry into the composed object is not a collision party (P5 / P3 accepted). The alternative — counting every input's declarations regardless of strategy — would refuse the add-on-overrides-core case that 'override' exists for whenever both objects embed a same-named action.
  3. Accept set narrows for: two stacks each declaring global:NAME; two stacks each binding a standalone action to one object name; a standalone bound to an object the OTHER stack's composed object embeds under the same name. Semver: minor with a BREAKING banner, the same convention feat(spec): defineStack refuses two actions that resolve to one scope-qualified runtime key #14686 shipped under; ADR-0087 disposition not-required (no-migration-prescription) in the changeset (the marker line itself is in the changeset file; it is not reproduced here because the GitHub sanitizer eats comment-shaped fragments).
  4. Message shape (one condition, one wording) is pinned character-for-character in compose-stacks-action-key-collision.test.ts, including the full envelope for the card's case.

Changeset (.changeset/compose-stacks-action-key-collision.md, @objectstack/spec: minor)

Opens with the BREAKING banner (**BREAKING** accept-set narrowing, shipped as minor under the repo's launch-window convention for breaking changes), names the refused shape (two or more input stacks each declaring an action resolving to one objectName:name / global:name key), reproduces the envelope, states what the check judges (what composition carries; distinct stacks; the build echo is never a collision with itself), what stays accepted (the cross-scope pair; one name on two objects), the corpus fact (app-multi-package composes unchanged; no callers in objectui / hotcrm), the migration (rename within scope, rebind, or drop — an authoring decision), and the ADR-0087 disposition marker.

Verification record — every command below ran on the final head d3081ab88 (this branch has one commit; the tree was clean before and after every run)

  • pnpm --filter @objectstack/spec exec vitest run --maxWorkers=2 over the new suite plus compose-stacks.test.ts, stack-duplicate-action-key.test.ts, compose-stacks-manifest-preserve.test.ts, compose-stacks-key-loss.test.ts, compose-stacks-i18n-merge.test.ts, stack-artifact-packages.test.ts, stack.test.ts, stack-inline-action-crossref.test.ts (under scripts/pm/os-verify-lock.sh): Test Files 9 passed (9) / Tests 284 passed (284); lock VERDICT command-exit 0. The new suite is 22 tests.
  • Reverse verification (fix committed first): git restore --source=f3ae441fa -- packages/spec/src/stack.zod.ts (tree only), on-disk proof grep -c collectComposedActionKeyCollisions = 0 and git hash-object = the base blob 493e930…; the new suite then reads Tests 11 failed | 11 passed (22) — every refusal pin red, every acceptance pin green (measured direction: red, as predicted). Restored with git checkout HEAD -- PATH (absolute path, trap-guarded): marker count 4, git hash-object = HEAD blob 6af34ee…, git diff HEAD empty. The suite imports ./stack.zod from source (no dist resolution, no vitest alias), so no rebuild is part of this leg.
  • pnpm --filter @objectstack/spec typecheck (tsc + check:scripts-typecheck + check:test-typecheck, under the lock): exit 0, check:test-typecheck self-test line printed green.
  • pnpm --filter @objectstack/spec build (under the lock, 147 s): check-dts-emitted: 34/34. Then pnpm --filter @objectstack/spec check:generated: ✓ All 15 generated artifacts are up to date — the describe-text edit produced no artifact diff (the tree stayed clean after gen:schema), so no regeneration commit exists.
  • node scripts/pm/dispatch-gates.mjs --commands (no paths; the script derived from the merge-base itself, header line names this repo at d3081ab88): 53 commands — 47 of the PM's family plus the changeset / new-test additions. Each ran with its exit code captured before any pipe (bash -c CMD > LOG 2>&1; EXIT=$?). 48 exited 0. The other five, read from their own verdict lines:
    • Migrate from npm to pnpm #6 check-dev-prereqs: The workspace is not built — 1 unmet precondition, not a list of problems (66 of 67 dists absent; a pnpm dev precondition, not a reading of this diff) — NOT MEASURED locally, CI builds.
    • Add comprehensive test suite for Zod schema validation #12 check-test-completeness: PREREQUISITE NOT MET — this gate grades a saved turbo run test log, and no log was named (exit 3; the script itself says to record it as NOT MEASURED when run from the derived family) — NOT MEASURED locally.
    • [WIP] Add support for default language configuration #18 check:doc-formula-expressions: PREREQUISITE NOT MET — the workspace package @objectstack/formula is not built (exit 3); after turbo run build --filter=@objectstack/formula the re-run reports the next unbuilt link, @objectstack/lint (exit 3 again). Its input is the built lint package, not this diff — NOT MEASURED locally, CI builds the closure.
    • Fix homepage font color and add blog section with in-depth analytical articles using fumadocs #36 check:dual-build-cjs-loads: PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/ (85 packages) — NOT MEASURED locally.
    • [WIP] Fix issues in action run step #51 check:type-check-debt: --re-measure cannot run: 54 workspace dependencies of the ledgered packages have no built type entry pointNOT MEASURED locally (the coverage half printed OK — 69/79 type-checked).
      Named-family reads of note: check:adr-0087-registration 0, check:changeset-no-major 0, check:api-surface 0, check:docs 0, check:dispatcher-error-vocabulary 0, check:stack-collection-maps 0, check:cross-package-test-inputs 0 (the new suite reads nothing outside its package).
  • pnpm check:nul-bytes: check-nul-bytes: OK (scanned 8072 text file(s) … no raw ASCII control bytes).
  • Corpus (H4) through dist/ at this head: app-multi-package composes; output above.
  • Repo-wide pnpm lint is CI's run and was not run here (declared narrowing; no eslint measurement is claimed).

Out of scope — filed unassigned, neither is addressed here

Dedupe before filing: REST list of all 481 open issues plus 1,200 recently-closed ones, grepped locally for composeStacks / mergeActionsIntoObjects / objectConflict; #14512 (artifact serialized twice) and #14599 (attribution) are the nearest neighbours and are different defects.

Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_017RbbUMnxkUnWhE4j94v8FE


Generated by Claude Code

… one scope-qualified runtime key

`composeStacks` concatenated `actions` across its inputs with no same-scope
duplicate-key check, so two packages each legal on their own and each
declaring `global:shared_refresh` composed into one collapsed handler key —
the dead button `defineStack` refuses within one stack, one composition
step later. The composed set is now walked with the same scope-qualified
key rule, judged on what the composition carries, and a collision throws
naming both source stacks by manifest id and where each declaration sits.
No `actionConflict` option (maintainer ruling 2026-09-03).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017RbbUMnxkUnWhE4j94v8FE
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/spec, touching 4 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/getting-started/examples.mdx (via composeStacks (symbol, a top-level function))
  • content/docs/getting-started/glossary.mdx (via composeStacks (symbol, a top-level function))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx (via composeStacks (symbol, a top-level function))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 128 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 2263ca4d679026335f559184c0bed4e76d35a242packageMentionDocs.

Which tree this was computed on

This run read content/docs from d1e36c64763f20a09fe34dfcc598969e032c9405 — the merge of head 3887008447bfc66b3a7323052d1496090f8c9051 into base 2263ca4d679026335f559184c0bed4e76d35a242, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin d1e36c64763f20a09fe34dfcc598969e032c9405 && git checkout d1e36c64763f20a09fe34dfcc598969e032c9405
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2263ca4d679026335f559184c0bed4e76d35a242 3887008447bfc66b3a7323052d1496090f8c9051 && git checkout -B drift-repro 2263ca4d679026335f559184c0bed4e76d35a242 && git merge --no-ff 3887008447bfc66b3a7323052d1496090f8c9051

node scripts/docs-audit/affected-docs.mjs --json 2263ca4d679026335f559184c0bed4e76d35a242

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 2263ca4d679026335f559184c0bed4e76d35a242 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claude

claude Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Contract review — FAIL (patch round 1)domain:spec seat, session_017RbbUMnxkUnWhE4j94v8FE, 2026-09-03T05:53Z. Full verdict (isolated claude-fable-5-1 reviewer, transcript-verified, adopted verbatim): #14662 comment 5521181451. needs:contract-review stays on the PR and the card until the patched head passes re-review.

Everything substantive passed — ruling A faithful, both dev boundary flags CONFIRMED as shipped (distinct-stack counting; judged on what composition carries), semver/changeset precedent-identical, scope fences intact, actionsOwner provenance correct. The FAIL is two one-liners, verbatim from the verdict:

Required patches before landing:

  1. packages/spec/src/stack.zod.ts:2808 — change action.objectName ?? GLOBAL_ACTION_SCOPE to action.objectName || GLOBAL_ACTION_SCOPE so an empty-string objectName keys as global, exactly as collectDuplicateActionKeyErrors (@f3ae441fa:1603-1608), mergeActionsIntoObjects (:2091) and standaloneActionOwnerKey (objectql/action-governance.ts:92) resolve it; add one pin in compose-stacks-action-key-collision.test.ts: two strict:false stacks, A act('dup_g', { objectName: '' }), B act('dup_g'), refused as Action key 'global:dup_g' is declared by 2 stacks: ….
  2. packages/spec/src/compose-stacks-action-key-collision.test.ts:298 — either annotate the line with // #14847: the bound action appears twice — measured shape today, not the contract or assert keysOf(out).embedded.crm_order with toContain('ship_order/BOUND') and crm_account with toEqual(['archive_account/EMB']), so the composeStacks re-merges bound standalone actions that defineStack already copied into their objects — every bound action appears twice in the composed object's actions #14847 fix does not read as a composition regression.

Recommended, non-blocking: stack.zod.ts:395 — "runs the same key rule across its input stacks (counting distinct stacks, not sites) and names both source stacks on a collision."

Dev: apply exactly these two on the same branch (claude/issue-14662-compose-stacks-action-key-collision), plus the non-blocking wording at stack.zod.ts:395 if you agree it is more accurate; re-run the new suite + compose-stacks.test.ts + typecheck; push; post an os-dev-report addendum (files touched, gates, head sha) on the card. Nothing else changes in this round.


Generated by Claude Code

…cks collision walk; pin it; loosen the multi-package mirror pin

Contract review patch round 1 (isolated reviewer verdict on the card):
the standalone branch keyed `objectName` by nullish coalescing where the
sibling walk, `mergeActionsIntoObjects` and objectql's owner-key ladder
all resolve an empty string by truthiness — now `||`, with a pin. The
app-multi-package mirror pin no longer encodes the doubled bound entry
(#14847) as the contract. Describe text says "same key rule (counting
distinct stacks, not sites)".

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017RbbUMnxkUnWhE4j94v8FE

os-sam commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

CI on 388700844: Test Core (1/6) (job 100539526191, run 33720807222) is the one red of 36 checks — and it is not this PR's failure.

FAIL  integration  test/run-dev-unbuilt-workspace.e2e.test.ts > the mirror direction: a reader that is never coming back > gives up and exits instead of waiting forever
AssertionError: the harness SIGKILLed the child — it was still alive at the ceiling. cap 180000 ms (RUN_TIMEOUT_MS, constant and load-independent by design); this child ran 180103 ms; case 1 measured the same child at 7643 ms on this runner minutes earlier: expected 'SIGKILL' to be null
Test Files  1 failed | 235 passed (236)   ·   Tests  1 failed | 2734 passed (2735)
check-test-completeness: OK (10 of 10 scheduled package(s) reported … 5260 test(s) declared and all accounted for)

That is #14832 — the os dev hang in an unbuilt workspace, a packages/cli product defect (pm:dispatched to the domain:cli seat; no fix merged yet, so there is nothing to port). This PR's diff is packages/spec/src/stack.zod.ts, its new test file and one changeset; it does not touch packages/cli, and every @objectstack/spec shard passed. The same signature has ejected 26 PRs from the merge queue in 24 h (merge-queue-triage on #14735, 06:01Z).

Failed jobs re-run once (MCP rerun_failed_jobs, requested in the same stroke as this note) — the one re-run the seat's rule allows, as the discriminator. Green ⇒ the landing path continues (trial merge is already clean vs 2263ca4d6, NOT governed 0/3). Red again on the same line ⇒ no second re-run: card #14662 goes pm:blocked on #14832 with Unlock-action: re-check PR #14854, as #14337 and #13881 already are. Contract review is complete (PASS 5521296835; gate cleared on both carriers; carriers check exit 0) — nothing on this branch needs to change.


Generated by Claude Code

os-sam commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Landing provenance (seat, 06:44Z) — ready + auto-merge (squash) from here.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants