Skip to content

fix(automation): resolve same-named flow definitions deterministically per ADR-0005 - #12026

Merged
os-trump merged 1 commit into
mainfrom
claude/issue-11997-packaged-flow-name-shadow
Aug 25, 2026
Merged

fix(automation): resolve same-named flow definitions deterministically per ADR-0005#12026
os-trump merged 1 commit into
mainfrom
claude/issue-11997-packaged-flow-name-shadow

Conversation

@os-sam

Copy link
Copy Markdown
Collaborator

Fixes#11997

Clause-② declared: YES. This changes collision behaviour on a served surface — a flow. Today the armed flow is decided by Map iteration order, so making the winner deterministic changes which flow fires on some boots. needs:contract-review is carried on this PR as the second half of the dual carrier (the card holds the first). ⛔ Not to be cleared, flipped ready, or auto-merged from here.

The defect, confirmed at runtime before anything was changed

The card was an explicit static read ("I did not run this end to end … the registration-order claim in particular deserves a runtime confirmation"). It does hold. Measured with a real SchemaRegistry and a real AutomationEngine on unmodified origin/main:

registration order into the registryflow the engine armed
packaged first, then the runtime rowRUNTIME
the runtime row first, then packagedPACKAGED

Same two definitions, opposite outcomes, decided by nothing but Map iteration order. listFlows() returned exactly ['opportunity_approval'] in both cases and getFlowRuntimeStates() had one entry — so nothing observable distinguished the two boots.

One extra finding the card did not have: Registry.registerItem's existing [Registry] Collision warning fires in only one of the two orders (packageId && collection.has(bareKey) — i.e. a runtime row exists and then a package ships the name). In that very order the warning promises "The runtime row will shadow the package value" while the engine armed the PACKAGED body. The one diagnostic that did fire was actively contradicted by the thing it warned about.

The direction is ADR-0005's, not a choice made here

Triage's premise-first condition routed through ADR-0048, which does not restate the direction. ADR-0005 does, twice:

  • §Decision, resolution order: 1. sys_metadata … ← overlay (wins) above 2. SchemaRegistry / MetadataService … ← artifact default
  • §Collision warning: "the runtime overlay layer silently shadows the artifact value (correct ADR-0005 behavior)"

and ADR-0048 §1.5 lists the runtime/DB overlay as "the sanctioned override path" under what is not a collision. So the runtime overlay wins; the packaged artifact is the default it overlays. This PR makes the engine agree with that deterministically instead of agreeing with whatever Map order produced.

What changed

  1. Deterministic precedence. The boot pull collapses same-named definitions before anything is armed, via a new resolveFlowPrecedence. The order is total and never reads iteration order: runtime before package, then lexicographic packageId. That second rule covers ADR-0048 §3.4's legitimate two-packages-one-name case, which the engine's bare-name map cannot represent but still has to answer deterministically.
  2. The loud diagnostic ADR-0048 already expected — one warning per colliding name, naming the name, every contender, and which one is armed, on a single line (bug(service-automation): flow 绑定失败的告警把 Zod issue 数组塞进单行日志,读者只拿到一个孤零零的 [ #5048), with the facts also in meta. Repeated at kernel:bootstrapped beside the other automation audits.
  3. An admin-visible receipt.getShadowedFlows() lists each contested name with its armed and shadowed definitions, and getFlowRuntimeStates() rows carry armedFrom / shadowed for contested names. Previously the displaced definition was invisible by construction. The Pulled N flow(s) line now counts distinct names, not registrations — the card noted N counted the latter.

The engine's flow map is still keyed by bare name. Making it package-aware is the larger change the card names and the ADR does not require; the receipt is a side map that never affects dispatch.

isCodeArtifactBody is exported from @objectstack/objectql rather than re-derived in the automation plugin. That test exists precisely so callers cannot drift into a second answer to "does a code package ship this name?", and a bare _packageId check gets it wrong twice — the sys_metadata rehydration sentinel, and a tenant overlay bound to a real package id (cloud#970). Both are pinned.

Verification

All gates below ran against b5ca58cec2, the head of this branch, with a clean tree.

  • pnpm --filter @objectstack/service-automation testTest Files 89 passed (89) / Tests 1051 passed (1051)
  • pnpm --filter @objectstack/service-automation build (this package has no typecheck script; its build is the type check) — DTS ⚡️ Build success in 6329ms
  • pnpm --filter @objectstack/objectql typechecktsc --noEmit && tsc --noEmit -p tsconfig.scripts.json, exit 0
  • pnpm check:type-check-debt--re-measure: OK — 32 ledger entr(ies) re-measured in 216.8s, 1898 raw tsc error(s) total, none above its recorded number.
  • pnpm check:type-check-coverageOK — 65/78 workspace packages type-checked (plus the root), 13 in the DEBT ledger
  • pnpm check:engine-double-contractOK — 405 pinned, 133 in the DEBT ledger, 2 exempt.
  • pnpm check:where-matcher✓ 297 matcher(s) discovered, 297 answer the combinator battery correctly or refuse it loudly
  • pnpm check:query-options-erasure✓ ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new
  • pnpm check:cross-package-test-inputsOK: 16 package(s) read outside themselves, all declared
  • pnpm check:test-source-aliasOK — 72 packages with tests scanned
  • pnpm check:type-source-resolutionOK — 93 tsc program(s) across 77 packages scanned
  • pnpm check:published-files✓ 69 publishable package(s) of 78 workspace member(s)
  • pnpm check:nul-bytes✓ self-test: 75 assertions over a temp git repo
  • node scripts/check-engine-split-ratio.mjsratio: 97.6%, deferred per ADR-0076 OQ#5

Gates derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, which reported the list as derived from this repo at b5ca58cec2. Two gates first came back NOT MEASURED and were re-run rather than counted: service-automation typecheck (ERR_PNPM_RECURSIVE_RUN_NO_SCRIPT — no such script; the build is the type check) and check-engine-split-ratio (refused on a shallow clone, re-run after --shallow-since).

Declared narrowing: the round was narrowed to the ratchet family plus service-automation, per dispatch. Repo-wide pnpm lint was replaced with a targeted run, and the narrowing is measured, not assumed: eslint's own --format json reports 7 files linted, 0 errors, 0 warnings, and eslint.config.mjs states this repo "runs one eslint.config.mjs, which never enables type-aware linting (no parserOptions.project, no typed @typescript-eslint rules) for ANY file" — so this diff cannot move the verdict on any file it does not touch. CI runs the full farm regardless.

Ablation

The failing assertions were predicted in writing first, then observed exactly. Ablation: strip the grouping from resolveFlowPrecedence so every item becomes its own winner — the pre-fix pull.

Predicted to fail (7): arms-same-definition-regardless-of-order · one-flow-per-bare-name · warns-loudly · admin-receipt · two-packages-by-packageId · first-seen-order · tenant-overlay-beats-package. Predicted to pass (4): registry-returns-both · arms-runtime-over-packaged (accidental — package-first leaves RUNTIME last) · single-definition-silent · sentinel-classification.

Observed: Tests 7 failed | 4 passed (11) — the same seven, test for test.

The mutation was proved on disk before the run (injected marker present, git diff --stat +7 lines), not inferred from an editor exit code. No rebuild was needed for this leg and that is load-bearing: the test reaches flow-precedence.ts through a relative import, which vitest resolves from source, while the unaliased @objectstack/objectql import resolves to dist/ and was built and left unmutated. The script carried trap '<restore>' EXIT INT TERM. Restore was verified byte-for-byte — marker absent, disk/index/HEAD diffs all empty, and git hash-object equal to git rev-parse HEAD:<path> (07ed8e6e…).

Generated by Claude Code


Generated by Claude Code

A runtime-authored flow reusing a packaged flow's name silently and
non-deterministically replaced it. The registry coexists both by design
(ADR-0048 §3.4) and listItems returns both with no precedence, while the
engine keys flows by bare name — so the boot pull registered both under one
key and Map iteration order decided the survivor.
Apply the ADR-0005 overlay precedence ADR-0048 §3.4 routes this case to
(runtime overlay wins over the packaged artifact), warn per colliding name,
and leave an admin-visible receipt for the shadowed definition.
Fixes#11997
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/objectql, @objectstack/service-automation, touching 19 documentable anchor(s). ⚠️2 changed file(s) yielded no anchor (packages/objectql/src/index.ts, packages/services/service-automation/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • content/docs/api/client-sdk.mdx(via getBookTree (sdk), meta.getBookTree (sdk), meta.publishItem (sdk), publishItem (sdk))
  • content/docs/kernel/contracts/metadata-service.mdx(via packageId (symbol), /:type/:name/publish (route))
  • content/docs/permissions/authorization.mdx(via packageId (symbol))
  • content/docs/permissions/capabilities.mdx(via packageId (symbol))
  • content/docs/permissions/permission-sets.mdx(via packageId (symbol))
  • content/docs/protocol/kernel/plugin-spec.mdx(via packageId (symbol))
  • content/docs/ui/doc-pages.mdx(via packageId (symbol))
  • content/docs/ui/setup-app.mdx(via packageId (symbol))

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

  • content/docs/releases/v14.mdx(via /book/:name/tree (route))
  • content/docs/releases/v16.mdx(via AutomationEngine (symbol))
  • content/docs/releases/v17.mdx(via AutomationEngine (symbol), /:type/:name/publish (route))

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
  • 2 changed file(s) yielded no anchor (packages/objectql/src/index.ts, packages/services/service-automation/src/index.ts) — pages documenting those are invisible to this run
  • 5 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 222 client-bound route-ledger rows — the other 177 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: 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 — 16 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 1e79aa4f812708d5d097ca33d9b7040e02b958c6packageMentionDocs.

Which tree this was computed on

This run read content/docs from 3db178eecc98d2ce3571a626af7437a9061ddb0f — the merge of head b5ca58cec2d575c096d7d2aabf0b252c5cd88420 into base 1e79aa4f812708d5d097ca33d9b7040e02b958c6, 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 3db178eecc98d2ce3571a626af7437a9061ddb0f && git checkout 3db178eecc98d2ce3571a626af7437a9061ddb0f
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1e79aa4f812708d5d097ca33d9b7040e02b958c6 b5ca58cec2d575c096d7d2aabf0b252c5cd88420 && git checkout -B drift-repro 1e79aa4f812708d5d097ca33d9b7040e02b958c6 && git merge --no-ff b5ca58cec2d575c096d7d2aabf0b252c5cd88420
node scripts/docs-audit/affected-docs.mjs --json 1e79aa4f812708d5d097ca33d9b7040e02b958c6

⚠️ 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 1e79aa4f812708d5d097ca33d9b7040e02b958c6 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-trumpClaude

Copy link
Copy Markdown
Collaborator

Contract review PASS at CONTRACT_REVIEW_TIER (verdict + readings on the card: #11997). Label cleared on both carriers; flipping ready and entering the merge queue.


Generated by Claude Code

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

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

3 participants

@os-sam@os-trump@claude