Skip to content

fix(cli): the startup banner names a contested flow name and says which definition is armed (#12028) - #12562

Merged
os-litant merged 2 commits into
mainfrom
claude/issue-12028-banner-flow-shadowing
Aug 26, 2026
Merged

fix(cli): the startup banner names a contested flow name and says which definition is armed (#12028)#12562
os-litant merged 2 commits into
mainfrom
claude/issue-12028-banner-flow-shadowing

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes#12028

What was wrong

collectAutomationSummary builds the os dev / os start banner by reading binding STATE off the live automation engine, because a flow that failed to arm emits no log line to go looking for and the boot-quiet stdout window swallows the engine's own warn narration. It was silent about the one failure it could not express as a count.

The engine's flow map is keyed by BARE name. When a packaged flow and a runtime-authored sys_metadata overlay both claim one name, ADR-0005 precedence arms one and the loser is not in the map — so it is not in listFlows(), not in getFlowRuntimeStates()'s rows, and therefore not in any number this banner prints. 3 flow(s), 3 bound to triggers was a true sentence about a set that did not contain the definition the operator had just edited.

#11997 gave the engine the receipt — AutomationEngine.getShadowedFlows() (packages/services/service-automation/src/engine.ts:2820) plus armedFrom / shadowed per runtime-state row (:2750). The automation plugin warns from it at kernel:bootstrapped, but that is a logger.warn, which is exactly the channel this banner exists to work around. serve.ts read neither.

What this does

collectAutomationSummary reads the receipt and the banner prints one line per contested name:

 ⚠ flow 'send-welcome' is claimed by 2 definitions — a runtime-authored row (sys_metadata) is ARMED, 1 shadowed (ADR-0005 overlay precedence; only the armed definition dispatches)

Naming which body is armed is the point. A line reporting only the count tells an admin something is wrong and withholds the answer they are standing there to get.

Silent on every healthy boot: no contested name, no line.

Measurements behind the shape

The sibling probe, measured before copying it. The unbound probe at serve.ts:5197 IS feature-detected — optional call, ?? [], try/catch carrying the /* older engine */ comment — and the function's docblock states the invariant for every probe in it. The new probe matches those three elements and adds nothing:

letshadowing: Array<{name: string;armed: Contender;shadowed: Contender[]}>=[];try{shadowing=automation.getShadowedFlows?.()??[];}catch{/* older engine */}

No cast past the states annotation. The local annotation now names armedFrom / shadowed alongside the pre-existing fields, so a later read of those rows is type-checked against the row's real shape instead of casting through any.

Where the receipt is read from, and why — a fork, declared rather than picked quietly. The two facts are available from two places: getShadowedFlows() and the states rows. They are information-equivalent except in one direction, which decided it: getFlowRuntimeStates() can only attach a receipt to a row it is already emitting (a name still in the flow map), while getShadowedFlows() returns every receipt the boot pull recorded. So the dedicated probe is the source, and the extended states annotation is carried as a declaration rather than as a second read — deliberately not a fallback, since a fallback would be tolerance the sibling probe does not have. Naming this explicitly because it is the one place the card's suggested shape and the "extend the annotation" direction do not describe the same implementation.

The type is structural, not imported.@objectstack/service-automation is a workspace dep of packages/cli and exports FlowContender / FlowShadowingRecord, so import type was available. It is deliberately not used: the probes here are feature-detected precisely so a host on an OLDER automation package still boots its banner, and typing these reads against the CURRENT package would state a guarantee the runtime does not make. Every other shape this function reads off the engine is annotated structurally for the same reason.

Benign-case filter. A receipt that displaced nothing is not a contested name, and collectAutomationSummary drops it. The engine already refuses to record one; the banner keeps its own end of that guarantee locally, where it is testable, rather than inheriting it.

Tests — pinned on what the banner RENDERS

packages/cli/src/commands/serve-automation-shadowing.test.ts (new, 13 pins). Every render pin drives a shadowing receipt through the real collectAutomationSummary and the real printServerReady and reads the stderr line an operator sees — the shape format.seed-summary.test.ts and serve-organizations-message-spelling.test.ts already use on this surface. Asserting only that a field was read would pass against a banner that prints nothing, which is the defect.

Three of the pins assert an ABSENCE: a healthy tree, a receipt that displaced nothing, and an engine that is not enabled at all must each produce no line, while the ordinary Flows: row still prints (so the absence is the line being withheld, not the banner being off).

Both directions were ablated at 6077c4b05, each leg with an EXIT INT TERM restore trap, absolute paths, and the mutation confirmed on disk by grep count and blob-hash comparison before any verdict was read:

  • Removing the render loop (is claimed by occurrences 1 to 0, hash changed): 5 failed / 8 passed. Exactly the five positive render pins died. The three absence pins stayed green — which is why the positive pins have to exist.
  • Making the line fire unconditionally (marker 0 to 1, hash changed): 4 failed / 9 passed — two absence pins plus the two count pins. Honest note: the "engine not enabled at all" absence pin stayed green under this particular mutation, because that branch returns before the loop; it still guards the disabled-engine construction path.

Restore verified by observation, not exit code: git diff HEAD empty and the working blob hash equal to the HEAD blob.

Verification — all at 6077c4b05

  • pnpm --filter @objectstack/cli exec vitest run --maxWorkers=2 over every test file that imports printServerReady or collectAutomationSummary (8 files): Test Files 8 passed (8), Tests 63 passed (63).
  • pnpm --filter @objectstack/cli typecheck: exit 0. tsc --listFiles confirms the new test file and format.ts are both in the program (1 hit each) — packages/cli/tsconfig.json includes src with no test exclusion, so "typecheck clean" really does cover the new tests.
  • pnpm lint (repo-wide eslint . --no-inline-config): exit 0. Not narrowed.
  • Gate union, exit codes captured before any pipe: check:nul-bytes, check:engine-double-contract, check:where-matcher, check:cross-package-test-inputs, check:query-options-erasure, check:test-source-alias, check:route-envelope, check:type-check-coverage, check:changeset-gate-self-tests, check:empty-changeset, check:objectui-changeset, check:i18n, check:i18n-coverage — all 0. check:i18n-coverage prints OK (12 config(s), 602 baselined untranslated string(s), none new).
  • Declared narrowing: the FULL packages/cli suite is NOT MEASURED here. It exceeded the container's ~10-minute foreground window and was SIGTERM-killed (exit 143) with no verdict written — recorded as unmeasured, never as a pass. The 8 files above are the complete consumer set of the changed surface (grep -rln for both symbols across packages/cli/src), and CI runs the whole suite.

Both automation package builds were done under the shared verify lock before any verdict was read, so nothing was judged against a stale dist.


Generated by Claude Code

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

17 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 5fbd58e0d6f100d1221fd2e3b04eb824144c7e88.

4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

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 — 23 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 5fbd58e0d6f100d1221fd2e3b04eb824144c7e88packageMentionDocs.

Which tree this was computed on

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

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 26, 2026
@os-litant
os-litant marked this pull request as ready for review August 26, 2026 10:53
@os-litant
os-litant added this pull request to the merge queueAug 26, 2026
Merged via the queue into main with commit 9bed0b0Aug 26, 2026
34 checks passed
@os-litant
os-litant deleted the claude/issue-12028-banner-flow-shadowing branch August 26, 2026 11:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI startup banner does not surface flow-name shadowing, though it already reads the rows that carry it

1 participant

@os-litant