fix(service-automation): one renderer for the contested-flow phrase (#12563) (#12607)

One event — a flow name claimed by more than one definition — was described to
an operator in three places, each with its own private `const describe` beside
the log call. Nothing held them equal, and two axes had already drifted:
`flow-precedence.ts` rendered `package "crm"` while the other two rendered
`package 'crm'`, and the two engine copies interpolated a bare `undefined`
where the CLI copy rendered a real fallback.

`describeFlowContender` sat exactly where a shared renderer would live and read
like one, but it is a CLASSIFIER with no prose in it — so each caller wrote its
own sentence while an export that looked reusable sat beside them. That is the
durable part of the defect: a missing renderer is a gap someone fills, a
misleading one is a gap that keeps getting re-filled privately.

The two copies in this package now call one exported `renderFlowContender`, and
`describeFlowContender`'s own docs say what it is not. Both spellings were
decided on measurement:

- Single quotes, measured against this package rather than voted across the
  three copies: 203 single-quoted interpolations in operator prose under
  `service-automation/src` against 3 double-quoted, one of which was this
  phrase. The sentence already single-quotes the flow name beside it.
- A named fallback instead of `package 'undefined'`. This package's callers
  cannot reach that branch today (`isCodeArtifactBody` is false on a falsy
  `_packageId`) — but that is a property of today's callers, not of an
  exported function.

The third copy, in `@objectstack/cli`'s startup banner, deliberately keeps its
own structural rendering: that package takes no static value import of
`@objectstack/service-automation`, and its engine reads are feature-detected so
a host on an older automation package still boots its banner. It is held equal
by a test-only agreement pin instead, which is red in both directions.


Claude-Session: https://claude.ai/code/session_0194kbQJxUvv2yvsGRtuXpP5

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-litant

.changeset/flow-contender-one-renderer.md

@@ -0,0 +1,53 @@
+---
+"@objectstack/service-automation": minor
+---
+
+fix(service-automation): one renderer for the contested-flow phrase, and the two spellings it had drifted into (#12563)
+
+`minor`, not `patch`, and not empty: this adds a new export
+(`renderFlowContender`) to a published package's public API, and it changes
+**shipped operator-facing log text**. Both are real changes a consumer can
+observe.
+
+## What changed
+
+One event — a flow name claimed by more than one definition — was described to
+an operator in three places, each with its own private `const describe` beside
+the log call: `flow-precedence.ts`'s precedence warning, `plugin.ts`'s bootstrap
+audit, and (in `@objectstack/cli`) the startup banner. Nothing held them equal,
+and two axes had already drifted:
+
+- **Quoting.** `flow-precedence.ts` rendered `package "crm"`; the other two
+  rendered `package 'crm'`.
+- **Absent package id.** The two engine copies interpolated a bare `undefined`
+  into the sentence; the CLI copy rendered a real fallback.
+
+The two copies in this package are now one exported renderer. The choice on
+each axis was measured, not voted:
+
+- **Single quotes**, measured against this package rather than across the three
+  copies: of the interpolated identifiers in operator prose under
+  `service-automation/src`, 203 are single-quoted and 3 double-quoted — one of
+  those 3 being this phrase. The sentence already single-quotes the flow name
+  beside it.
+- **A named fallback** (`a code-shipped package (id unknown)`) instead of
+  `package 'undefined'`. This package's own callers cannot reach that branch
+  today, because `isCodeArtifactBody` is false on a falsy `_packageId` — but
+  that is a property of today's callers, not of an exported function.
+
+## Log text a consumer may be matching on
+
+`[Automation] Flow name collision: …` (the precedence warning) now renders a
+packaged contender as `package 'crm'` rather than `package "crm"`.
+`plugin.ts`'s bootstrap `[Automation] flow '<name>' is claimed by …` warning is
+byte-identical to before for every input its callers can produce; only its
+unreachable absent-id branch changed.
+
+## Why the CLI still renders its own
+
+`@objectstack/cli` deliberately keeps its own spelling and takes no value
+import of this package for the banner: its engine reads are structural and
+feature-detected so a host on an older automation package still boots. The
+third copy is held equal by a test-only agreement pin
+(`packages/cli/src/utils/format.flow-contender-agreement.test.ts`) that asserts
+the banner line through this renderer, so it goes red in both directions.

packages/cli/src/utils/format.flow-contender-agreement.test.ts

@@ -0,0 +1,143 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+//
+// [#12563] THE CLI's COPY OF THE CONTESTED-FLOW PHRASE, HELD EQUAL TO THE
+// ENGINE's — by agreement, not by a shared runtime import.
+//
+// ## What was wrong
+//
+// One event — a flow name claimed by more than one definition — was described
+// to an operator in three places, each with its own private `const describe`:
+// twice inside `@objectstack/service-automation` and once here, in the startup
+// banner. Nothing held them equal, and two axes had ALREADY drifted before
+// anyone noticed: the engine's precedence warning double-quoted the package id
+// while the other two single-quoted it, and the two engine copies interpolated
+// a bare `undefined` where this one renders a real fallback.
+//
+// The two engine copies are now one exported renderer, `renderFlowContender`.
+// This file is what holds the THIRD copy — the one in this package — to it.
+//
+// ## ⛔ Why this is a TEST-only import and `format.ts` still renders its own
+//
+// The obvious fix is for `format.ts` to import the renderer and call it. It is
+// the wrong fix here, and the reason is measured rather than stylistic:
+//
+//   - `packages/cli` takes ZERO static value imports of
+//     `@objectstack/service-automation` today. Its only value import of that
+//     package is a DYNAMIC `await import()` in `utils/data-migration-plugins.ts`,
+//     deferred behind `opts.automation === true`.
+//   - `collectAutomationSummary` reads the engine STRUCTURALLY and feature-
+//     detects every probe (#12028/#12562) precisely so a host running an OLDER
+//     automation package still boots its banner. A value import states a
+//     guarantee that runtime deliberately does not make.
+//   - `utils/format.ts` is imported by ~56 command modules in this package. A
+//     static import here would pull the whole automation package into the
+//     module graph of `os whoami`, `os init`, `os login` — every command.
+//
+// A TEST is not the shipped runtime path, and `@objectstack/service-automation`
+// is already a workspace dependency, so importing the renderer HERE costs none
+// of that. The banner keeps its own spelling; this file is what makes the two
+// spellings a fact rather than a coincidence.
+//
+// ⚠️ This specifier resolves through the package's `exports` to its **dist**,
+// not its source — it is already registered for this package in
+// `KNOWN_UNALIASED_TEST_IMPORTS['@objectstack/cli']` (that shrink-only ledger is
+// NOT widened by this file; the specifier was already reachable via the dynamic
+// import named above). `turbo.json` declares `@objectstack/cli#test`
+// `dependsOn: ["build"]`, so CI builds it first. Locally: build
+// `@objectstack/service-automation` before running this, or the named import
+// below fails to link and says so.
+//
+// ⛔ Deliberately NOT aliased to source in `vitest.config.ts`. Aliasing a dep to
+// source imports that dep's ENTIRE import surface into this package's
+// resolution domain (see `scripts/check-test-source-alias.mjs` and this
+// package's vitest config header) — a config change reaching all ~185 test
+// files here, to buy staleness-resistance on one three-branch pure function.
+
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import { renderFlowContender } from '@objectstack/service-automation';
+import { printServerReady, type ServerReadyOptions, type AutomationReadySummary } from './format.js';
+
+type Contender = { source: 'package' | 'runtime'; packageId?: string };
+
+const base: ServerReadyOptions = {
+  externalBaseOrigin: 'http://localhost:3000',
+  configFile: 'objectstack.config.ts',
+  isDev: true,
+  pluginCount: 1,
+};
+
+/**
+ * The banner summary shaped by hand rather than through
+ * `collectAutomationSummary`: this file is about the RENDERING of one contender,
+ * so the collection step is deliberately not in the loop. The end-to-end path
+ * from a fake engine through the collector is covered by
+ * `commands/serve-automation-shadowing.test.ts`.
+ */
+const summaryWith = (armed: Contender, shadowedCount = 1): AutomationReadySummary => ({
+  enabled: true,
+  declaredFlowCount: 1,
+  flowCount: 1,
+  boundCount: 1,
+  triggerTypes: ['record_change'],
+  unbound: [],
+  unknownObject: [],
+  draftCount: 0,
+  shadowed: [{ flowName: 'send-welcome', armed, shadowedCount }],
+});
+
+describe('#12563 — the banner phrase for a contested flow agrees with the engine renderer', () => {
+  let lines: string[];
+  let spy: ReturnType<typeof vi.spyOn>;
+
+  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 shadowLine = (armed: Contender) => {
+    printServerReady({ ...base, automation: summaryWith(armed) });
+    const shown = lines.filter((l) => l.includes('is claimed by'));
+    expect(shown).toHaveLength(1);
+    return shown[0];
+  };
+
+  // One row per branch of the renderer, so a drift on ONE axis fails the row
+  // for that axis instead of collapsing the file. The assertion is
+  // `toContain(renderFlowContender(c))`, which is red in BOTH directions: it
+  // fails if this package's `describeFlowBody` changes, and it fails if the
+  // engine's renderer changes underneath it.
+  it('spells a packaged contender exactly as the engine spells it', () => {
+    const armed: Contender = { source: 'package', packageId: 'crm' };
+    expect(shadowLine(armed)).toContain(renderFlowContender(armed));
+  });
+
+  it('spells a runtime overlay row exactly as the engine spells it', () => {
+    const armed: Contender = { source: 'runtime' };
+    expect(shadowLine(armed)).toContain(renderFlowContender(armed));
+  });
+
+  it('spells an absent package id exactly as the engine spells it — and never as `undefined`', () => {
+    const armed: Contender = { source: 'package' };
+    const line = shadowLine(armed);
+    expect(line).toContain(renderFlowContender(armed));
+    // The axis this pin exists for, stated independently of the renderer: if
+    // BOTH sides regressed to interpolation at once the agreement above would
+    // still hold, and this row would not.
+    expect(line).not.toContain('undefined');
+  });
+
+  // ── The instrument can say no ─────────────────────────────────────────────
+  it('would notice a disagreement — the comparison is not vacuous', () => {
+    const armed: Contender = { source: 'package', packageId: 'crm' };
+    const line = shadowLine(armed);
+    // The pre-#12563 engine spelling of the SAME contender. If `toContain`
+    // could not tell the two apart, every row above would pass no matter what
+    // either side rendered.
+    expect(line).not.toContain(`package "${armed.packageId}"`);
+    expect(renderFlowContender(armed)).not.toBe(`package "${armed.packageId}"`);
+  });
+});

packages/services/service-automation/src/flow-name-shadowing.test.ts

@@ -26,7 +26,7 @@
 import { describe, it, expect, vi } from 'vitest';
 import { SchemaRegistry } from '@objectstack/objectql';
 import { AutomationEngine } from './engine.js';
-import { resolveFlowPrecedence, describeFlowContender } from './flow-precedence.js';
+import { resolveFlowPrecedence, describeFlowContender, renderFlowContender } from './flow-precedence.js';
 
 const FLOW = 'opportunity_approval';
 const silentLogger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } as any;
@@ -121,7 +121,14 @@ describe('#11997 — packaged flow shadowed by a same-named runtime flow', () =>
         const [message, meta] = warn.mock.calls[0] as [string, any];
 
         expect(message).toContain(FLOW); // the bare name
-        expect(message).toContain('package "crm"'); // contender A
+        // ⚠️ Contender A is asserted THROUGH the renderer, not as a literal
+        // (#12563). This warning used to spell the phrase itself, in a private
+        // `const describe` two lines above the call — with double quotes, while
+        // the two other copies of the same sentence used single. Deriving the
+        // expectation here means re-introducing a private spelling at this call
+        // site fails THIS row; the renderer's own literal output is pinned
+        // separately below, so the two cannot drift together and stay green.
+        expect(message).toContain(renderFlowContender({ source: 'package', packageId: 'crm' })); // contender A
         expect(message).toContain('runtime-authored row'); // contender B
         expect(message).toContain('arming a runtime-authored row'); // which one wins
         expect(message).toContain('ADR-0005');
@@ -219,3 +226,48 @@ describe('#11997 — precedence is a total order, not an iteration order', () =>
         expect((resolved[0].definition as any).label).toBe('TENANT');
     });
 });
+
+/**
+ * [#12563] The phrase itself, pinned as literals.
+ *
+ * The call sites above and the CLI banner assert THROUGH `renderFlowContender`,
+ * so a caller that re-invents the phrase privately goes red. That alone is not
+ * enough: if the renderer's own spelling changed, every derived assertion would
+ * move with it and stay green. These three rows are the anchor that cannot
+ * move quietly — one per decision the renderer makes.
+ */
+describe('#12563 — renderFlowContender: one spelling for the contested-flow phrase', () => {
+    it('quotes a package id the way this package quotes identifiers — single, not double', () => {
+        // ⛔ Not a majority vote of the three copies that used to exist. Measured
+        // over `service-automation/src`: 203 single-quoted interpolations in
+        // operator prose against 3 double-quoted, one of which WAS this phrase.
+        // The sentence already single-quotes the flow name beside this.
+        expect(renderFlowContender({ source: 'package', packageId: 'crm' })).toBe("package 'crm'");
+        expect(renderFlowContender({ source: 'package', packageId: 'com.objectstack.platform-objects' })).toBe(
+            "package 'com.objectstack.platform-objects'",
+        );
+    });
+
+    it('names a runtime overlay by the table an admin would go edit', () => {
+        expect(renderFlowContender({ source: 'runtime' })).toBe('a runtime-authored row (sys_metadata)');
+        // A runtime row bound to a real package id is STILL a runtime row — the
+        // id is not part of this branch's phrase, and leaking it here would
+        // read as "a package shipped this", the opposite of what happened.
+        expect(renderFlowContender({ source: 'runtime', packageId: 'crm' })).toBe(
+            'a runtime-authored row (sys_metadata)',
+        );
+    });
+
+    it('never interpolates an absent package id into the sentence', () => {
+        // ⚠️ Unreachable from THIS package's callers today — `isCodeArtifactBody`
+        // is false on a falsy `_packageId`, so `describeFlowContender` never
+        // emits a package contender without one. That is a property of today's
+        // callers, not of the renderer, and this row is what keeps the renderer
+        // safe for the next one. It is a real branch of an exported function.
+        expect(renderFlowContender({ source: 'package' })).toBe('a code-shipped package (id unknown)');
+        expect(renderFlowContender({ source: 'package' })).not.toContain('undefined');
+        expect(renderFlowContender({ source: 'package', packageId: '' })).toBe(
+            'a code-shipped package (id unknown)',
+        );
+    });
+});

packages/services/service-automation/src/flow-precedence.ts

@@ -60,6 +60,14 @@ export interface FlowPrecedenceWinner {
 /**
  * Classify one registry body's provenance.
  *
+ * ⚠️ This is a CLASSIFIER, not a renderer. It answers "where did this body come
+ * from?" with a {@link FlowContender} and carries no prose at all — yet it is
+ * exported, and it sits exactly where a shared renderer would live. It has been
+ * mistaken for one: three separate callers each wrote their own private
+ * sentence about a contested flow name, because the export that looked
+ * reusable had nothing in it to reuse. For the operator-facing phrase, use
+ * {@link renderFlowContender} below.
+ *
  * Delegates to `isCodeArtifactBody` — the canonical ADR-0029 D9.6 test, which
  * exists precisely so callers cannot drift into a second answer to "does a code
  * package ship this name?". ⛔ Do not re-derive this from `_packageId`: that
@@ -78,6 +86,43 @@ export function describeFlowContender(item: unknown): FlowContender {
     };
 }
 
+/**
+ * Render one contender as the phrase an operator reads.
+ *
+ * ⛔ The ONLY place this phrase is spelled. Before it existed the same sentence
+ * was written three times from scratch — twice in this package (the pull
+ * warning below, and the plugin's bootstrap audit) and once in
+ * `@objectstack/cli`'s startup banner — and the copies had already drifted on
+ * TWO axes. A private `const describe = …` beside a log call is how each copy
+ * arrived; reach for this instead, and a fourth caller costs nothing.
+ *
+ * ## Both spellings are decisions, so they are recorded here
+ *
+ * **Single quotes** — measured against this package, ⛔ not voted across the
+ * copies. Of the interpolated identifiers in operator prose under
+ * `service-automation/src`, 203 are single-quoted and 3 double-quoted, and one
+ * of those 3 was this phrase. The sentence this phrase lands in already
+ * single-quotes the flow NAME, which is the more free-form of the two values,
+ * so single quotes here add no ambiguity the line does not already carry.
+ * `packageId` is an unconstrained `z.string()` in `packages/spec`, so neither
+ * spelling is provably safe against an adversarial id — this one is at least
+ * the house convention rather than a coin flip.
+ *
+ * **A named fallback, never an interpolated `undefined`.** `packageId` is
+ * optional on {@link FlowContender}, and `package 'undefined'` is the one
+ * rendering an operator cannot act on. This package's own callers cannot reach
+ * that branch today — `isCodeArtifactBody` is false on a falsy `_packageId`,
+ * so a `source: 'package'` contender always carries one — but that is a
+ * property of today's CALLERS, not of this function. A renderer that is safe
+ * only because of who happens to call it stops being safe at the next caller.
+ */
+export function renderFlowContender(contender: FlowContender): string {
+    if (contender.source !== 'package') return 'a runtime-authored row (sys_metadata)';
+    return contender.packageId
+        ? `package '${contender.packageId}'`
+        : 'a code-shipped package (id unknown)';
+}
+
 /**
  * Rank one contender for a bare name. LOWER wins.
  *
@@ -147,13 +192,10 @@ export function resolveFlowPrecedence(
 
         const armed = ranked[0];
         const shadowed = ranked.slice(1).map((entry) => entry.contender);
-        const describe = (c: FlowContender) =>
-            c.source === 'package' ? `package "${c.packageId}"` : 'a runtime-authored row (sys_metadata)';
-
         logger?.warn(
             `[Automation] Flow name collision: '${name}' is claimed by ${group.length} definitions ` +
-            `(${ranked.map((entry) => describe(entry.contender)).join(', ')}); ` +
-            `arming ${describe(armed.contender)} per ADR-0005 overlay precedence and shadowing ` +
+            `(${ranked.map((entry) => renderFlowContender(entry.contender)).join(', ')}); ` +
+            `arming ${renderFlowContender(armed.contender)} per ADR-0005 overlay precedence and shadowing ` +
             `${shadowed.length} other definition(s). Only the armed definition dispatches. ` +
             `Rename one, or remove the sys_metadata row if the package value should win.`,
             {

packages/services/service-automation/src/index.ts

@@ -40,7 +40,7 @@ export type {
 // pull applies this; exported so a host that assembles its own flow list (or a
 // test) collapses contenders the same deterministic way instead of inventing a
 // second precedence.
-export { resolveFlowPrecedence, describeFlowContender } from './flow-precedence.js';
+export { resolveFlowPrecedence, describeFlowContender, renderFlowContender } from './flow-precedence.js';
 export type { FlowPrecedenceWinner } from './flow-precedence.js';
 
 // Per-run summary (#4354): the fold that turns a run's step log into