diff --git a/.changeset/serve-banner-flow-name-shadowing.md b/.changeset/serve-banner-flow-name-shadowing.md new file mode 100644 index 0000000000..6977c2fa9f --- /dev/null +++ b/.changeset/serve-banner-flow-name-shadowing.md @@ -0,0 +1,39 @@ +--- +"@objectstack/cli": patch +--- + +fix(cli): the startup banner names a contested flow name and says which definition is armed (#12028) + +`os dev` / `os start` print an automation summary that reads binding STATE off the +live 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. That +summary 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 the 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, and nothing on the banner +said otherwise. #11997 gave the engine the receipt (`getShadowedFlows()`, plus +`armedFrom` / `shadowed` on each runtime-state row) and 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. + +`collectAutomationSummary` now reads that receipt through a probe feature-detected +exactly like the `getTriggerBindingAudit` one beside it, and the banner prints one +line per contested name carrying all three facts an operator needs: + +``` + ⚠ 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. This banner is read on +every start, and a warning that also fires when nothing is wrong is one readers +learn to skip. Both directions are pinned on what the banner RENDERS, absence +included. diff --git a/packages/cli/src/commands/serve-automation-shadowing.test.ts b/packages/cli/src/commands/serve-automation-shadowing.test.ts new file mode 100644 index 0000000000..ca853b32cc --- /dev/null +++ b/packages/cli/src/commands/serve-automation-shadowing.test.ts @@ -0,0 +1,264 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The startup banner names a contested flow name, and says WHICH body is armed + * (#12028). + * + * ── The hole this closes ───────────────────────────────────────────────── + * + * 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 of them and the loser is not in the map — so it is not in + * `listFlows()`, not in `getFlowRuntimeStates()`'s row set, and therefore not in + * ANY count this banner prints. `3 flow(s), 3 bound to triggers` is a true + * sentence about a set that does not contain the definition the operator just + * edited, and nothing on the banner contradicts it. + * + * #11997 gave the engine the receipt (`getShadowedFlows()`, plus `armedFrom` / + * `shadowed` on each runtime-state row). The automation plugin warns from it at + * `kernel:bootstrapped` — but that is a `logger.warn`, and the whole reason this + * banner reads engine STATE rather than scraping output is that the boot-quiet + * stdout window swallows exactly those lines. The banner was the reliable + * channel and it was silent. + * + * ── Why these pins read the RENDERED line ──────────────────────────────── + * + * Asserting that `collectAutomationSummary` "read the field" would pass with a + * banner that prints nothing, which is the defect. So every pin below 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. + * + * ── The instrument must be able to say no ──────────────────────────────── + * + * `prints no shadowing line …` is not filler. The banner is read on every + * `os dev` / `os start`; a warning that also appears when nothing is wrong is a + * warning readers learn to skip, and the next real one goes with it. An + * always-firing implementation passes every positive pin in this file, so the + * absence legs are what actually constrain it. + * + * ── The fakes ──────────────────────────────────────────────────────────── + * + * `getShadowedFlows()` returns `FlowShadowingRecord[]` — `{ name, armed, + * shadowed }`, where a contender is `{ source: 'package' | 'runtime'; + * packageId?: string }` (`packages/services/service-automation/src/engine.ts`, + * `FlowContender` / `FlowShadowingRecord`). Hand-rolled here rather than + * imported, matching the sibling `serve-automation-summary.test.ts`: the probe + * under test is feature-detected against an OLDER automation package, so typing + * these fakes against the current one would defeat the tolerance legs below. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { collectAutomationSummary } from './serve.js'; +import { printServerReady, type ServerReadyOptions } from '../utils/format.js'; + +type Contender = { source: 'package' | 'runtime'; packageId?: string }; +type ShadowRecord = { name: string; armed: Contender; shadowed: Contender[] }; + +type FlowState = { + name: string; + enabled: boolean; + bound: boolean; + status?: string; + triggerType?: string; + object?: string; +}; + +function fakeKernel(services: Record) { + return { + getService(name: string) { + if (!(name in services)) throw new Error(`Service '${name}' not found`); + return services[name]; + }, + }; +} + +/** An engine that knows about shadowing — i.e. anything from #11997 onwards. */ +function fakeAutomation(states: FlowState[], shadowing: ShadowRecord[] = []) { + return { + getFlowRuntimeStates: () => + states.map((s) => { + const record = shadowing.find((r) => r.name === s.name); + // The engine attaches the receipt to the row too, for a name still in + // the flow map. Mirrored here so the fake is not quietly narrower than + // the thing it stands in for. + return record ? { ...s, armedFrom: record.armed, shadowed: record.shadowed } : s; + }), + getRegisteredTriggerTypes: () => ['record_change'], + getTriggerBindingAudit: () => [], + getShadowedFlows: () => shadowing, + }; +} + +const armed = (states: FlowState[], shadowing: ShadowRecord[] = []) => + collectAutomationSummary(fakeKernel({ automation: fakeAutomation(states, shadowing) }), states.length); + +const flow = (name: string): FlowState => ({ + name, + enabled: true, + bound: true, + status: 'active', + triggerType: 'record_change', + object: 'lead', +}); + +/** + * The realistic pair, in the direction ADR-0005 actually resolves: a runtime + * overlay row OUTRANKS the packaged body, so the definition shipped in the + * package is the one that stopped running. + */ +const CONTESTED: ShadowRecord = { + name: 'send-welcome', + armed: { source: 'runtime' }, + shadowed: [{ source: 'package', packageId: 'crm' }], +}; + +describe('collectAutomationSummary — flow-name shadowing (#12028)', () => { + it('carries the contested name, the armed body and the displaced count', () => { + const summary = armed([flow('send-welcome'), flow('score-lead')], [CONTESTED])!; + expect(summary.shadowed).toEqual([ + { flowName: 'send-welcome', armed: { source: 'runtime' }, shadowedCount: 1 }, + ]); + }); + + it('is empty when no name is contested', () => { + expect(armed([flow('send-welcome')])!.shadowed).toEqual([]); + }); + + it('drops a receipt that displaced nothing — that is not a contested name', () => { + const summary = armed( + [flow('send-welcome')], + [{ name: 'send-welcome', armed: { source: 'runtime' }, shadowed: [] }], + )!; + expect(summary.shadowed).toEqual([]); + }); + + // The probe is feature-detected exactly like the `unbound` one beside it, and + // with nothing more. These two legs are what "exactly" means: an automation + // package predating #11997 has no `getShadowedFlows` at all, and the banner + // must degrade to its plain counts rather than take the whole boot down. + it('degrades on an engine that predates the receipt, without losing the banner', () => { + const older = { + getFlowRuntimeStates: () => [flow('send-welcome')], + getRegisteredTriggerTypes: () => ['record_change'], + getTriggerBindingAudit: () => [], + }; + const summary = collectAutomationSummary(fakeKernel({ automation: older }), 1)!; + expect(summary.shadowed).toEqual([]); + expect(summary.flowCount).toBe(1); + }); + + it('degrades when the receipt probe throws', () => { + const hostile = { + getFlowRuntimeStates: () => [flow('send-welcome')], + getRegisteredTriggerTypes: () => ['record_change'], + getTriggerBindingAudit: () => [], + getShadowedFlows: () => { + throw new Error('older engine'); + }, + }; + const summary = collectAutomationSummary(fakeKernel({ automation: hostile }), 1)!; + expect(summary.shadowed).toEqual([]); + expect(summary.flowCount).toBe(1); + }); +}); + +describe('startup banner — what an operator reads when a flow name is contested (#12028)', () => { + const base: ServerReadyOptions = { + externalBaseOrigin: 'http://localhost:3000', + configFile: 'objectstack.config.ts', + isDev: true, + pluginCount: 1, + }; + let lines: string[]; + let spy: ReturnType; + + beforeEach(() => { + lines = []; + // stderr, not stdout (#7915) — the whole banner is a diagnostic. + spy = vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { + lines.push(args.join(' ')); + }); + }); + afterEach(() => spy.mockRestore()); + + const render = (states: FlowState[], shadowing: ShadowRecord[] = []) => { + printServerReady({ ...base, automation: armed(states, shadowing) }); + return lines.filter((l) => l.includes('is claimed by')); + }; + + it('names the contested flow, WHICH definition is armed, and how many were shadowed', () => { + const shown = render([flow('send-welcome'), flow('score-lead')], [CONTESTED]); + expect(shown).toHaveLength(1); + // All three facts, in the one line the operator gets. The middle one is the + // point of the card: a line that reports the count and stops has told an + // admin something is wrong and withheld which body is running. + expect(shown[0]).toContain("flow 'send-welcome'"); // which name + expect(shown[0]).toContain('a runtime-authored row (sys_metadata) is ARMED'); // which body + expect(shown[0]).toContain('1 shadowed'); // how many lost + expect(shown[0]).toContain('is claimed by 2 definitions'); + expect(shown[0]).toContain('only the armed definition dispatches'); + }); + + it('names the package when the packaged body is the one that armed', () => { + const shown = render( + [flow('send-welcome')], + [{ name: 'send-welcome', armed: { source: 'package', packageId: 'crm' }, shadowed: [{ source: 'runtime' }] }], + ); + expect(shown[0]).toContain("package 'crm' is ARMED"); + }); + + it('never interpolates an absent package id into the sentence', () => { + const shown = render( + [flow('send-welcome')], + [{ name: 'send-welcome', armed: { source: 'package' }, shadowed: [{ source: 'runtime' }] }], + ); + expect(shown[0]).toContain('a code-shipped package (id unknown) is ARMED'); + expect(shown[0]).not.toContain('undefined'); + }); + + it('counts every displaced definition, not just the first', () => { + const shown = render( + [flow('send-welcome')], + [{ + name: 'send-welcome', + armed: { source: 'runtime' }, + shadowed: [{ source: 'package', packageId: 'crm' }, { source: 'package', packageId: 'marketing' }], + }], + ); + expect(shown[0]).toContain('is claimed by 3 definitions'); + expect(shown[0]).toContain('2 shadowed'); + }); + + it('reports one line per contested name', () => { + const shown = render( + [flow('send-welcome'), flow('score-lead')], + [ + CONTESTED, + { name: 'score-lead', armed: { source: 'runtime' }, shadowed: [{ source: 'package', packageId: 'crm' }] }, + ], + ); + expect(shown).toHaveLength(2); + expect(shown.join('\n')).toContain("flow 'score-lead'"); + }); + + // ── The instrument can say no ────────────────────────────────────────── + it('prints no shadowing line on a healthy boot, while still printing the counts', () => { + expect(render([flow('send-welcome'), flow('score-lead')])).toEqual([]); + // Not silent about everything — the ordinary Flows: row is still there, so + // the absence above is the line being withheld, not the banner being off. + expect(lines.some((l) => l.includes('Flows:') && l.includes('2 flow(s)'))).toBe(true); + }); + + it('prints no shadowing line for a receipt that displaced nothing', () => { + expect( + render([flow('send-welcome')], [{ name: 'send-welcome', armed: { source: 'runtime' }, shadowed: [] }]), + ).toEqual([]); + }); + + it('prints no shadowing line when the automation engine is not enabled at all', () => { + printServerReady({ ...base, automation: collectAutomationSummary(fakeKernel({}), 2) }); + expect(lines.filter((l) => l.includes('is claimed by'))).toEqual([]); + }); +}); diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 0cc49f8ae7..17e710912e 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -5160,9 +5160,9 @@ export function resolveBannerConfigRow(opts: { * banner reads the binding state off the engine instead). * * Every probe is feature-detected so an older `@objectstack/service-automation` - * (without `getTriggerBindingAudit` / extended runtime states) degrades to the - * plain count line instead of crashing the banner. Returns `undefined` when - * there is nothing automation-related to show at all. + * (without `getTriggerBindingAudit` / `getShadowedFlows` / extended runtime + * states) degrades to the plain count line instead of crashing the banner. + * Returns `undefined` when there is nothing automation-related to show at all. */ export function collectAutomationSummary( kernel: any, @@ -5181,12 +5181,40 @@ export function collectAutomationSummary( triggerTypes: [], unbound: [], unknownObject: [], + shadowed: [], draftCount: 0, } : undefined; } - let states: Array<{ name: string; enabled: boolean; bound: boolean; status?: string; triggerType?: string; object?: string }> = []; + /** + * One flow body's provenance, as the engine spells it (`FlowContender` in + * `@objectstack/service-automation`): a code-shipped artifact, or an + * ADR-0005 runtime overlay row in `sys_metadata`. + * + * Declared structurally, like every other shape this function reads off the + * engine. The probes below are feature-detected precisely so a host running + * an OLDER automation package still boots its banner, and a nominal import + * would type these reads against the CURRENT package while the runtime + * deliberately tolerates a previous one. + */ + type Contender = { source: 'package' | 'runtime'; packageId?: string }; + + let states: Array<{ + name: string; + enabled: boolean; + bound: boolean; + status?: string; + triggerType?: string; + object?: string; + // [#12028] `getFlowRuntimeStates()` has attached these two per row since + // #11997 whenever a bare name had more than one contender. Named here so a + // later read is type-checked against the row's real shape — casting past + // this annotation is how the field goes back to being declared and unread, + // which is the whole defect this banner line closes. + armedFrom?: Contender; + shadowed?: Contender[]; + }> = []; try { states = automation.getFlowRuntimeStates?.() ?? []; } catch { /* older engine */ } if (states.length === 0 && declaredFlowCount === 0) return undefined; @@ -5196,6 +5224,19 @@ export function collectAutomationSummary( let unbound: Array<{ flowName: string; triggerType: string; reason: string }> = []; try { unbound = automation.getTriggerBindingAudit?.() ?? []; } catch { /* older engine */ } + // [#12028] Same-named definitions: which body armed, and which lost. Read + // from the engine's dedicated receipt rather than from `states` above, for + // one measured reason — `getFlowRuntimeStates()` can only attach the receipt + // to a row it is already emitting, i.e. to a name still in the flow map, + // whereas `getShadowedFlows()` returns every receipt the boot pull recorded. + // A contested name is worth saying out loud either way. + // + // Feature-detected exactly like the `unbound` probe above and with nothing + // more: optional call, `?? []`, `catch` for an older engine. No extra + // tolerance — the neighbouring read is the standard here. + let shadowing: Array<{ name: string; armed: Contender; shadowed: Contender[] }> = []; + try { shadowing = automation.getShadowedFlows?.() ?? []; } catch { /* older engine */ } + // Dead bindings: a bound record-change flow whose target object nobody // registered — the hook is filtered to a name that never writes. const unknownObject: Array<{ flowName: string; object: string }> = []; @@ -5218,6 +5259,13 @@ export function collectAutomationSummary( triggerTypes, unbound, unknownObject, + // A receipt that displaced nothing is not a contested name. The engine + // already refuses to record one, and the banner keeps its own end of that + // guarantee here rather than inheriting it: a benign boot must print no + // shadowing line at all, and this is where "benign" is decided. + shadowed: shadowing + .filter((r) => r.shadowed.length > 0) + .map((r) => ({ flowName: r.name, armed: r.armed, shadowedCount: r.shadowed.length })), draftCount: states.filter((s) => s.enabled && (s.status ?? 'draft') === 'draft').length, }; } diff --git a/packages/cli/src/utils/format.ts b/packages/cli/src/utils/format.ts index 1dbe354f1f..eb214450f7 100644 --- a/packages/cli/src/utils/format.ts +++ b/packages/cli/src/utils/format.ts @@ -596,6 +596,28 @@ export interface AutomationReadySummary { unbound: Array<{ flowName: string; triggerType: string; reason: string }>; /** Bound record-change flows whose target object is not registered (dead binding). */ unknownObject: Array<{ flowName: string; object: string }>; + /** + * Bare flow names claimed by more than one definition, as the ADR-0005 + * overlay precedence resolved them (#12028). + * + * `armed` is the body that is actually in the engine's flow map and will + * dispatch; `shadowedCount` is how many same-named definitions it displaced. + * BOTH are carried on purpose. The engine's flow map is keyed by BARE name, + * so a displaced definition is invisible by construction: it is not in + * `flowCount`, not in `unbound`, not in `unknownObject`, and not in + * `listFlows()`. A count on its own would report that something is wrong and + * withhold the one fact the operator is standing there to get — *which* body + * is the one running. + * + * Empty on every healthy boot. No contested name, no line: this banner is + * read on every `os dev` / `os start`, and a warning that fires when nothing + * is wrong is a warning readers learn to skip. + */ + shadowed: Array<{ + flowName: string; + armed: { source: 'package' | 'runtime'; packageId?: string }; + shadowedCount: number; + }>; /** Enabled flows whose persisted status is 'draft' (they still fire). */ draftCount: number; } @@ -733,6 +755,20 @@ export function printBootDiagnostics(diagnostics: BootDiagnostics) { console.error(chalk.dim(' run with --log-level debug to watch the boot stream live')); } +/** + * Name one flow body the way an operator can act on it (#12028). + * + * Deliberately worded to match `@objectstack/service-automation`'s own + * bootstrap warning for the same event, so an operator who sees both at + * `--log-level info` reads one story rather than two. `packageId` is optional + * on the engine's contender shape, so both halves have a defined answer here + * instead of interpolating an absent id into the sentence. + */ +function describeFlowBody(c: { source: 'package' | 'runtime'; packageId?: string }): string { + if (c.source !== 'package') return 'a runtime-authored row (sys_metadata)'; + return c.packageId ? `package '${c.packageId}'` : 'a code-shipped package (id unknown)'; +} + /** * One-glance answer to "did my flows actually arm?" — the question the * boot-quiet stdout window otherwise makes unanswerable (the engine's own @@ -757,6 +793,19 @@ function printAutomationSummary(a: AutomationReadySummary) { if (a.draftCount > 0) parts.push(`· ${a.draftCount} draft`); console.error(chalk.dim(` Flows: ${parts.join(' ')}`)); + // #12028 — printed FIRST among the warnings because it re-reads every line + // above it: the counts describe the ARMED bodies only, so when a name is + // contested "3 flow(s), 3 bound" is true of a set that does not include the + // definition the operator just edited. + for (const s of a.shadowed) { + console.error( + chalk.yellow( + ` ⚠ flow '${s.flowName}' is claimed by ${s.shadowedCount + 1} definitions — ` + + `${describeFlowBody(s.armed)} is ARMED, ${s.shadowedCount} shadowed ` + + `(ADR-0005 overlay precedence; only the armed definition dispatches)`, + ), + ); + } for (const u of a.unbound) { console.error( chalk.yellow(` ⚠ flow '${u.flowName}' declares a '${u.triggerType}' trigger but is NOT bound — ${u.reason}`),