Repository navigation
Description
The defect
A runtime-authored flow that reuses a packaged flow's name silently and
non-deterministically replaces the packaged flow in the automation engine, with no
diagnostic, no receipt, and no way for an admin to tell which one is armed.
This is reachable today: the metadata registry declares flow with
allowRuntimeCreate: true (packages/spec/src/kernel/metadata-plugin.zod.ts:827), so
authoring a runtime flow named opportunity_approval while a package ships one is an
ordinary, accepted write.
Mechanism — three facts that are individually correct and collide
-
The SchemaRegistry stores both, deliberately. Items are keyed
packageId:name(packages/objectql/src/registry.ts:1148), and ADR-0048 section 3.4
coexistence is pinned inpackages/objectql/src/registry-cross-package-collision.test.ts:20-40
— two packages shipping one bare name legitimately coexist, reachable via
package-scopedgetItem. -
listItemsreturns both, with no dedup and no precedence.
packages/objectql/src/registry.ts:3394-3414:const items = Array.from(this.metadata.get(type)?.values() || []) as T[];It filters by
packageIdonly when one is passed, and filters out disabled packages.
Nothing collapses two entries that share a bare name. -
The automation engine keys flows by bare name.
packages/services/service-automation/src/engine.ts:1252:private flows = new Map<string, FlowParsed>();and the boot pull (
packages/services/service-automation/src/plugin.ts:870-880) is:const flows = ql?.registry?.listItems?.('flow') ?? []; for (const f of flows) { ... this.engine.registerFlow(def.name, def as never); ... }registerFlowends inthis.flows.set(name, parsed)(engine.ts:2527).
⇒ Both entries are pulled, both are registered under the same key, and the last one in
Map iteration order wins. The survivor is decided by registration order into the
registry, which is boot load order — not by any declared precedence.
Why it is worth a card rather than a note
- It is silent in both directions. Neither
registerFlownor the flow pull warns on a
name that is already present; the pull's only log is a count
(plugin.ts:891Pulled N flow(s) from ObjectQL registry), and N counts the
registrations, not the distinct names. listFlows/getFlowRuntimeStatescannot show it — they enumerate
this.flows.keys()(engine.ts:2600, 2638), which by then holds one entry.- The trigger binding follows the survivor, so the observable symptom is a packaged
flow that "stopped working" or a runtime flow that "does something it was not authored
to do", depending on which won. - The precedent for the neighbouring type went the other way deliberately: for permission
sets, a save targeting a package-declared name is refused loudly with a message
naming the sanctioned path (packages/plugins/plugin-security/src/packaged-permission-set-lock.ts,
maintainer ruling 2026-08-24, "lock the base, clone to customize"). Flows have no
equivalent guard.
Not measured
I did not run this end to end — this is a static read of the three code paths above,
found while measuring the trigger-dispatch path for the packaged-flow customization design
work. The registration-order claim in particular deserves a runtime confirmation before
anyone prices a fix.
Possible directions (not a recommendation — this needs triage first)
- refuse the runtime write when a package declares the name (the permission-set shape); or
- make the engine's flow map package-aware, which is a much larger change; or
- at minimum, warn at pull time when one bare name is registered twice, so the
condition stops being invisible.
Provenance
Found while measuring the trigger-dispatch path for the packaged-flow clone-to-customize
design (#11665, section 2.2 of the design document posted there). Filed separately because
it is a live defect on main independent of whether that design is accepted, and because
it is not in that card's scope.
Read at origin/main d63b01436035f0866df8c22156e26ab2a064580d. Unassigned, not claimed.
Generated by Claude Code
Activity
claude commentedon Aug 25, 2026
Concentrated triage batch (final tail):
finding→pm:queue, Bug (domain:services+bugstand) — declared≠enforced: the SchemaRegistry deliberately coexistspackageId:name(ADR-0048 §3.4, pinned) while the automation engine collapses flows by bare name with Map order deciding the winner — silent, non-deterministic replacement of a packaged flow by a same-named runtime flow, reachable through an ordinary accepted write. Premise-first at claim: read ADR-0048 §3.4's precedence semantics — if the ADR already answers who wins (runtime-overlay precedence is the likely ruled shape), implement that deterministically WITH a loud diagnostic and an admin-visible receipt; if the ADR is silent on precedence, STOP and report the fork — shadowing semantics is then a contract decision, not a dev choice. Clause-②: yes if collision behaviour changes on a served surface — declare at claim, contract-review tier.Generated by Claude Code
claude commentedon Aug 25, 2026
{ "issue": 11997, "status": "done", "branch": "claude/issue-11997-packaged-flow-name-shadow", "pr": "https://github.com/objectstack-ai/objectstack/pull/12026", "premise_still_valid": true, "summary": "ADR-0005 states the precedence direction the card's premise-first condition needed, twice: its resolution order lists `sys_metadata ... <- overlay (wins)` above `SchemaRegistry ... <- artifact default`, and its Collision-warning section calls the runtime overlay shadowing the artifact 'correct ADR-0005 behavior'. So triage's implement branch, not its escalate branch. The boot pull now collapses same-named flow definitions before anything is armed via a new resolveFlowPrecedence: runtime overlay beats packaged artifact, and two packages sharing a bare name resolve by lexicographic packageId, so Map iteration order decides nothing. It warns once per colliding name (bare name, every contender, which one is armed) at pull time and again at kernel:bootstrapped, and leaves an admin receipt: new getShadowedFlows() plus armedFrom/shadowed on getFlowRuntimeStates() rows, which previously could not show the displaced definition at all. The engine's flow map is still keyed by bare name -- the receipt is a side map that never affects dispatch. isCodeArtifactBody is exported from @objectstack/objectql rather than re-derived, since a bare _packageId check misreads both the sys_metadata rehydration sentinel and a tenant overlay bound to a real package id. Two extras worth the PM's eye: the pre-fix engine could actively CONTRADICT the registry's own [Registry] Collision warning (in the one order where that warning fires, it promised the runtime row would win while the engine armed the packaged body), and the 'Pulled N flow(s)' line now counts distinct names rather than registrations, which the card flagged.", "tests": "All gates ran against b5ca58cec2 (branch head, clean tree); gates derived by `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack`, which reported the list as derived from this repo at that same commit. BEFORE-STATE, measured on unmodified origin/main (227c1788d5) with a real SchemaRegistry + real AutomationEngine, since the card was explicitly a static read: registering the package first armed the RUNTIME body, registering the runtime row first armed the PACKAGED body -- `Tests 3 passed (3)` on the probe asserting exactly that, plus listFlows() == ['opportunity_approval'] and getFlowRuntimeStates().length == 1 in BOTH orders. A first probe run failed with `TypeError: this.logger.info is not a function` -- my harness bug, counted as NOT MEASURED and re-run, never as a disproof. GATES (each quoting its own verdict line): service-automation `Test Files 89 passed (89)` / `Tests 1051 passed (1051)`; service-automation build (this package has NO typecheck script -- `pnpm --filter ... typecheck` first returned ERR_PNPM_RECURSIVE_RUN_NO_SCRIPT, i.e. NOT MEASURED, and its build is the type check) `DTS Build success in 6329ms`; objectql typecheck `tsc --noEmit && tsc --noEmit -p tsconfig.scripts.json` exit 0; 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.` (first attempt was killed by the ~10min foreground cap mid-re-measure = NOT MEASURED, re-run with the closure cached); check:type-check-coverage `OK -- 65/78 workspace packages type-checked`; check:engine-double-contract `OK -- 405 pinned, 133 in the DEBT ledger, 2 exempt.`; check:where-matcher `297 matcher(s) discovered, 297 answer the combinator battery correctly or refuse it loudly`; check:query-options-erasure `ratchet holds: 67 unswept non-test site(s) in 17 file(s), none new`; check:cross-package-test-inputs `OK: 16 package(s) read outside themselves, all declared`; check:test-source-alias `OK -- 72 packages with tests scanned`; check:type-source-resolution `OK -- 93 tsc program(s) across 77 packages scanned`; check:published-files `69 publishable package(s) of 78 workspace member(s)`; check:nul-bytes `self-test: 75 assertions over a temp git repo`; check-engine-split-ratio `ratio: 97.6%` (first run REFUSED on a shallow clone = NOT MEASURED, re-run after `git fetch --shallow-since=2026-05-20`). DECLARED NARROWING: round narrowed to the ratchet family plus service-automation per dispatch; repo-wide `pnpm lint` replaced by a targeted run whose narrowing is measured on all three counts -- population read from eslint.config.mjs's own statement that this repo 'runs one eslint.config.mjs, which never enables type-aware linting (no parserOptions.project, no typed @typescript-eslint rules) for ANY file'; file count read from eslint's `--format json` (7 files linted, 0 errors, 0 warnings); invariance therefore follows, since with no type-aware linting this diff cannot move the verdict on any untouched file. ABLATION: predicted in writing BEFORE running -- strip the grouping from resolveFlowPrecedence so each item is its own winner; predicted 7 failures (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) and 4 passes (registry-returns-both, arms-runtime-over-packaged as an accidental pass, single-definition-silent, sentinel-classification). Observed `Tests 7 failed | 4 passed (11)` -- the same seven, test for test. Mutation proved ON DISK before the run (injected marker grep count 1, `git diff --stat` +7 lines), never inferred from an editor exit code. NO REBUILD was needed for this leg and that is load-bearing, not an omission: the test reaches flow-precedence.ts through a RELATIVE import that vitest resolves from source, while the unaliased @objectstack/objectql import resolves to dist/ (confirmed in scripts/check-test-source-alias.mjs's KNOWN_UNALIASED_TEST_IMPORTS) and was built beforehand and left unmutated -- `isCodeArtifactBody` verified present in dist/index.js (9 occurrences) and dist/index.d.ts. The ablation script carried `trap '<restore>' EXIT INT TERM`. RESTORE leg verified byte-for-byte: marker absent, disk-vs-HEAD / disk-vs-index / index-vs-HEAD diffs all empty, and `git hash-object` == `git rev-parse HEAD:<path>` (07ed8e6e...), whole tree clean.", "open_questions": [], "out_of_scope_findings": [ "filed as #12027: [Registry] Collision warning is order-asymmetric -- its `packageId && collection.has(bareKey)` guard means it fires ONLY when the package registers second, so in the ordinary cold-boot order (artifact first, then loadMetaFromDb) the artifact-vs-DB overlay is entirely silent at the registry level, for every metadata type. Measured 0 warnings vs 1 across the two orders.", "filed as #12028 (label `finding`, Blocked-by #11997): the CLI startup banner's collectAutomationSummary already reads getFlowRuntimeStates() and getTriggerBindingAudit() but surfaces neither the new armedFrom/shadowed fields nor getShadowedFlows(), so shadowing misses the one channel that function's own docblock calls reliable when the boot-quiet stdout window swallows warn/info logs." ] }Generated by Claude Code
fix(automation): resolve same-named flow definitions deterministically (
test(service-automation): pin precedence against an explicit loader's…
fix(service-automation): boot-time flow precedence classifies contend…
fix(service-automation, metadata-protocol): a shipped flow name arms …