fix(service-automation): one renderer for the contested-flow phrase, and the two spellings it had drifted into

.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