fix(service-automation): the two-package flow collision line says neither flow is armed

.changeset/31754-flow-collision-two-packages-neither-armed.md

@@ -0,0 +1,14 @@
+---
+'@objectstack/service-automation': patch
+---
+
+fix(service-automation): the boot log's flow name collision line says no flow is armed when the engine refuses the name
+
+Clause-②: no — log text only; what the engine arms, refuses or runs does not change.
+Metadata-protocol: none
+
+When a second package ships a flow of the same name, the engine's package-scoped resolver refuses the bare name for a caller with no package, and `registerFlow` arms neither flow, naming both packages (ADR-0150 D3). The boot pull's collision warning still read "arming package 'FIRST' by package id", "arming package 'FIRST' because a managed package's flow is sealed" or "arming the declared override …", depending on which other definitions sat beside the two packages, and its log meta named an armed contender.
+
+The warning now asks the same resolver the engine asks. When it refuses the name, every such line reads "arming neither: the engine refuses this bare name for a caller with no package (ADR-0150 D3), so no definition dispatches; the 'failed to register flow' warning for 'NAME' names every holder. Rename one of them." ("arming none of them" for three or more definitions), and its meta carries `refused: true` and the contenders instead of `armed`/`shadowed`.
+
+A name the resolver serves keeps its line byte for byte, and so does every line when no resolver is attached. `resolveFlowPrecedence` takes the reader as an optional fifth argument (`FlowNameRefusalReader`, exported as a type); the automation plugin passes it.

packages/cli/src/utils/format.boot-warning-classes.test.ts

@@ -15,6 +15,19 @@ import {
 import { BootLogCapture, isVerboseBootLevel } from './boot-log-capture.js';
 import { collectAutomationSummary } from '../commands/serve.js';
 
+// Pay the first load of what `serve.ts` reaches through dynamic `import()`s at
+// MODULE LOAD, outside any clocked case (`pnpm check:test-source-alias`).
+import '@objectstack/metadata-protocol';
+import '@objectstack/metadata';
+import '@objectstack/plugin-hono-server';
+import '@objectstack/rest';
+import '@objectstack/service-settings';
+import '@objectstack/service-datasource';
+import '@objectstack/driver-turso';
+import '@objectstack/driver-sql';
+import '@objectstack/platform-objects/plugin';
+import '@objectstack/plugin-security';
+
 /**
  * #22073 — one boot line per warning class, and each warning printed once.
  *
@@ -491,8 +504,9 @@ describe('print-once: Boot diagnostics withholds what the banner restated (forma
     );
     const collision = record(
       "[Automation] Flow name collision: 'dup_flow' is claimed by 2 definitions (package 'crm', a runtime-authored " +
-        "row (sys_metadata)); arming package 'crm' by package id, and shadowing 1 other definition(s). " +
-        'Only the armed definition dispatches. Rename one of them.',
+        "row (sys_metadata)); arming package 'crm' because a managed package's flow is sealed (ADR-0126 §2), and " +
+        'shadowing 1 other definition(s). Only the armed definition dispatches. To change what runs, declare it ' +
+        "as an override in 'crm.custom' (overrides: { package: 'crm', basedOn }), or save it under a new name.",
     );
     printServerReady({
       ...BASE,

packages/services/service-automation/src/flow-declared-override-first-boot-pull.integration.test.ts

@@ -93,6 +93,17 @@ const shippingStack = () =>
         { strict: false },
     ) as Record<string, unknown>;
 
+/** [#31754] A second package shipping FLOW's bare name — a clash sealed before ADR-0150 phase 1. */
+const OTHER = 'com.acme.other';
+const otherStack = () =>
+    defineStack(
+        { manifest: { ...pluginManifest(OTHER), namespace: 'other', name: 'other' }, flows: [ticketFlow('OTHER')] } as never,
+        { strict: false },
+    ) as Record<string, unknown>;
+
+/** Whether, and before or after the shipping package, OTHER is registered. */
+type Second = 'none' | 'before' | 'after';
+
 type Row = 'override' | 'legacy';
 
 const storedRow = (row: Row) =>
@@ -153,7 +164,7 @@ afterEach(async () => {
  * Boot the composition on `file`. `start` runs in a plugin that starts after
  * the automation plugin: after the boot pull, before `kernel:ready`.
  */
-async function boot(file: string, start: (ql: ObjectQL, automation: AutomationEngine) => Promise<void>) {
+async function boot(file: string, start: (ql: ObjectQL, automation: AutomationEngine) => Promise<void>, second: Second = 'none') {
     const kernel = new ObjectKernel({ logger: { level: 'fatal' }, gracefulShutdown: false } as never);
     kernels.push(kernel);
     const driver = new SqlDriver({ client: 'better-sqlite3', connection: { filename: file }, useNullAsDefault: true });
@@ -174,9 +185,11 @@ async function boot(file: string, start: (ql: ObjectQL, automation: AutomationEn
         version: '1.0.0',
         dependencies: ['com.objectstack.engine.objectql', 'com.objectstack.service-automation'],
         async init(ctx: PluginContext) {
-            const stack = shippingStack();
-            await ctx.getService<{ register(m: unknown): Promise<void> }>('manifest')
-                .register({ ...(stack.manifest as object), ...stack });
+            const manifests = ctx.getService<{ register(m: unknown): Promise<void> }>('manifest');
+            const registerStack = (stack: Record<string, unknown>) => manifests.register({ ...(stack.manifest as object), ...stack });
+            if (second === 'before') await registerStack(otherStack());
+            await registerStack(shippingStack());
+            if (second === 'after') await registerStack(otherStack());
             const ql = () => ctx.getService<ObjectQL>('objectql');
             const automation = () => ctx.getService<AutomationEngine>('automation');
             automation().registerTrigger(objectqlRecordTrigger(ql, automation, fired));
@@ -192,7 +205,11 @@ async function boot(file: string, start: (ql: ObjectQL, automation: AutomationEn
 }
 
 /** The first boot stores `rows`, in this order; the second, cold, is measured. */
-async function measuredBoot(rows: readonly Row[], start: (ql: ObjectQL, automation: AutomationEngine) => Promise<void>) {
+async function measuredBoot(
+    rows: readonly Row[],
+    start: (ql: ObjectQL, automation: AutomationEngine) => Promise<void>,
+    second: Second = 'none',
+) {
     const dir = mkdtempSync(join(tmpdir(), 'qa-28717-'));
     dirs.push(dir);
     const file = join(dir, 'store.db');
@@ -202,11 +219,16 @@ async function measuredBoot(rows: readonly Row[], start: (ql: ObjectQL, automati
     await first.kernel.shutdown();
     kernels.splice(kernels.indexOf(first.kernel), 1);
     const warn = vi.spyOn(ObjectLogger.prototype, 'warn');
-    const measured = await boot(file, start);
-    const collisions = warn.mock.calls
-        .map(([message]) => String(message))
-        .filter((message) => message.includes('Flow name collision') && message.includes(`'${FLOW}'`));
-    return { ...measured, collisions };
+    const measured = await boot(file, start, second);
+    const collisionCalls = warn.mock.calls
+        .filter(([message]) => String(message).includes('Flow name collision') && String(message).includes(`'${FLOW}'`));
+    const collisions = collisionCalls.map(([message]) => String(message));
+    // [#31754] The engine's own answer for FLOW: each refusal it logged while arming.
+    const refusals = warn.mock.calls
+        .filter(([message, meta]) =>
+            String(message).endsWith('failed to register flow') && (meta as { flow?: unknown } | undefined)?.flow === FLOW)
+        .map(([, meta]) => String((meta as { error?: unknown }).error));
+    return { ...measured, collisions, collisionMeta: collisionCalls.map(([, meta]) => meta), refusals };
 }
 
 const insertTicket = (ql: ObjectQL, title: string) => ql.insert(TICKET, { title }, { context: SYS } as never);
@@ -273,3 +295,69 @@ describe('[#28717] a declared .custom flow override is armed from the first boot
         expect(collisions).toEqual([]);
     });
 });
+
+/**
+ * [#31754, ADR-0150 D3] The collision line agrees with the engine on a real
+ * cold boot. A second package shipping FLOW's bare name (a clash sealed before
+ * phase 1) makes the engine's resolver refuse the name for a caller with no
+ * package, so `registerFlow` arms nothing — in each of the three shapes the
+ * pull ranks differently (the two packages alone; the declared override beside
+ * them; a package-less stored row beside them). Every collision line for the
+ * name then reads "arming neither"/"arming none of them", its meta names no
+ * armed contender, and the engine's refusal names both packages. The line asks
+ * the resolver the engine asks (`refusalOf`), never a guess from the shapes.
+ */
+describe('[#31754] a name a second package also ships: the collision line says no definition is armed, as the engine does', () => {
+    const SHAPES: ReadonlyArray<readonly [string, readonly Row[]]> = [
+        ['two packages', []],
+        ['the declared override beside a second package', ['override']],
+        ['two packages and a package-less stored row', ['legacy']],
+    ];
+    for (const [shape, rows] of SHAPES) {
+        for (const second of ['before', 'after'] as const) {
+            it(`${shape}, the second package registered ${second} the shipping one`, async () => {
+                let armedAtPull: unknown = 'unread';
+                const { automation, collisions, collisionMeta, refusals } = await measuredBoot(rows, async (_ql, engine) => {
+                    armedAtPull = await engine.getFlow(FLOW);
+                }, second);
+
+                // The engine: refused at every arming step, naming both packages; nothing armed.
+                expect(refusals.length, 'the engine refused the name').toBeGreaterThan(0);
+                for (const refusal of refusals) {
+                    expect(refusal).toContain('held by 2 packages in this environment');
+                    expect(refusal).toContain(`package "${BASE}"`);
+                    expect(refusal).toContain(`package "${OTHER}"`);
+                }
+                expect(armedAtPull).toBeNull();
+                expect(await automation.getFlow(FLOW)).toBeNull();
+                expect(automation.getShadowedFlows()).toEqual([]);
+
+                // The line: every collision warning for the name agrees.
+                expect(collisions.length, 'the pull warned about the collision').toBeGreaterThan(0);
+                for (const line of collisions) {
+                    expect(line).toMatch(/; arming (neither|none of them): the engine refuses this bare name for a caller with no package \(ADR-0150 D3\), so no definition dispatches; /);
+                    expect(line).toContain(`the 'failed to register flow' warning for '${FLOW}' names every holder. Rename one of them.`);
+                    expect(line).not.toMatch(/arming (package|the declared override|a runtime-authored)/);
+                    expect(line).not.toContain('Only the armed definition dispatches');
+                }
+                for (const meta of collisionMeta) {
+                    expect(meta).toMatchObject({ flow: FLOW, refused: true });
+                    expect(meta).not.toHaveProperty('armed');
+                }
+            });
+        }
+    }
+
+    it('CONTROL: a name the resolver serves (one package and a package-less row) keeps its line byte for byte', async () => {
+        const { automation, collisions, collisionMeta, refusals } = await measuredBoot(['legacy'], async () => {});
+        expect(refusals).toEqual([]);
+        expect(((await automation.getFlow(FLOW)) as { label?: string } | null)?.label).toBe('BASE');
+        expect(collisions).toEqual([
+            `[Automation] Flow name collision: '${FLOW}' is claimed by 2 definitions (package '${BASE}', a runtime-authored ` +
+                `row (sys_metadata)); arming package '${BASE}' because a managed package's flow is sealed (ADR-0126 §2), and ` +
+                'shadowing 1 other definition(s). Only the armed definition dispatches. To change what runs, declare it as ' +
+                `an override in '${PAIR}' (overrides: { package: '${BASE}', basedOn }), or save it under a new name.`,
+        ]);
+        expect(collisionMeta).toEqual([{ flow: FLOW, armed: { source: 'package', packageId: BASE }, shadowed: [{ source: 'runtime' }] }]);
+    });
+});

packages/services/service-automation/src/flow-package-scoped-resolution.test.ts

@@ -24,6 +24,9 @@ import { installBuiltinNodes } from './builtin/index.js';
 import { flowNameResolver } from './plugin.js';
 import { InMemoryFlowActivationStore } from './flow-activation-store.js';
 import { FlowActivationProjection } from './flow-activation-projection.js';
+import { resolveFlowPrecedence } from './flow-precedence.js';
+import { bindStoredRow } from '@objectstack/metadata-protocol';
+import { installedManifestsOf } from '@objectstack/metadata-core';
 
 const P = 'com.acme.caller';
 const A = 'com.acme.alpha';
@@ -364,3 +367,131 @@ describe('FlowActivationProjection', () => {
         expect(projection.has('notify')).toBe(true);
     });
 });
+
+/**
+ * [#31754, ADR-0150 D3] The boot pull's flow name collision line asks the
+ * resolver the engine asks (`refusalOf`, built here as the plugin builds it,
+ * from `flowNameResolver`). For a name two packages hold, precedence still
+ * hands the lexicographically first body on and `registerFlow` refuses it
+ * (red line 2 above), so the line says NEITHER is armed and points at that
+ * refusal. The three shapes on a real cold boot are pinned in
+ * `flow-declared-override-first-boot-pull.integration.test.ts`. CONTROL: with
+ * no resolver handed in, every arm's line is byte for byte main's.
+ */
+describe('[#31754] the collision line for a name two packages hold says neither is armed', () => {
+    const NOTHING_INSTALLED = installedManifestsOf(undefined);
+    /** The boot pull's two steps over the registry: precedence (its one warning), then registration. */
+    function pull(): { line: string; refusal: unknown } {
+        const warn = vi.fn();
+        const resolver = flowNameResolver({ getService: () => ({ registry }) } as never);
+        const [winner] = resolveFlowPrecedence(
+            registry.listItems('flow') as unknown[],
+            { warn },
+            (name) => engine.packagedFlowOwner(name),
+            NOTHING_INSTALLED,
+            (name) => {
+                const resolution = resolver(name, null);
+                return resolution?.status === 'refused' ? resolution.message : undefined;
+            },
+        );
+        expect(warn).toHaveBeenCalledTimes(1);
+        let refusal: unknown;
+        try {
+            engine.registerFlow(winner.name, winner.definition as never);
+        } catch (e) {
+            refusal = e;
+        }
+        return { line: String(warn.mock.calls[0][0]), refusal };
+    }
+    /** One collision line from precedence alone. */
+    function lineOf(listed: unknown[], owner: Record<string, string>): string {
+        const warn = vi.fn();
+        resolveFlowPrecedence(listed, { warn }, (name) => owner[name], NOTHING_INSTALLED);
+        expect(warn).toHaveBeenCalledTimes(1);
+        return String(warn.mock.calls[0][0]);
+    }
+
+    for (const order of [[A, Q], [Q, A]] as const) {
+        it(`two packages, registered ${order[0] === A ? 'alpha' : 'stranger'} first: the line arms neither, and the engine refuses`, async () => {
+            install(A);
+            install(Q);
+            for (const id of order) registry.registerItem('flow', { ...leaf('notify'), _packageId: id }, 'name' as never, id);
+            loaderSet.notify = A;
+
+            const { line, refusal } = pull();
+            expect(line).toBe(
+                "[Automation] Flow name collision: 'notify' is claimed by 2 definitions (package 'com.acme.alpha', " +
+                    "package 'com.acme.stranger'); arming neither: the engine refuses this bare name for a caller with " +
+                    "no package (ADR-0150 D3), so no definition dispatches; the 'failed to register flow' warning for " +
+                    "'notify' names every holder. Rename one of them.",
+            );
+            expect(line).not.toContain('arming package');
+            expect(line).not.toContain('by package id');
+            expect(line).not.toContain('Only the armed definition dispatches');
+            expect(line).not.toContain('\n');
+            // What the line points at: the refusal, naming both, and nothing armed.
+            const refused = String((refusal as Error)?.message);
+            expect(refused).toContain('held by 2 packages in this environment');
+            expect(refused).toContain('package "com.acme.alpha"');
+            expect(refused).toContain('package "com.acme.stranger"');
+            expect(await engine.getFlow('notify')).toBeNull();
+        });
+    }
+
+    it('CONTROL: with no resolver, two packages keep main\'s line byte for byte (by package id)', () => {
+        const line = lineOf(
+            [{ ...leaf('notify'), _packageId: Q, _provenance: 'package' }, { ...leaf('notify'), _packageId: A, _provenance: 'package' }],
+            { notify: A },
+        );
+        expect(line).toBe(
+            "[Automation] Flow name collision: 'notify' is claimed by 2 definitions (package 'com.acme.alpha', " +
+                "package 'com.acme.stranger'); arming package 'com.acme.alpha' by package id, and shadowing 1 other " +
+                'definition(s). Only the armed definition dispatches. Rename one of them.',
+        );
+    });
+
+    it('CONTROL: a package beside an environment row keeps its line byte for byte (ADR-0126 §2)', () => {
+        const line = lineOf(
+            [{ ...leaf('notify'), _packageId: A, _provenance: 'package' }, { ...leaf('notify'), _provenance: 'org' }],
+            { notify: A },
+        );
+        expect(line).toBe(
+            "[Automation] Flow name collision: 'notify' is claimed by 2 definitions (package 'com.acme.alpha', " +
+                "a runtime-authored row (sys_metadata)); arming package 'com.acme.alpha' because a managed package's " +
+                'flow is sealed (ADR-0126 §2), and shadowing 1 other definition(s). Only the armed definition ' +
+                "dispatches. To change what runs, declare it as an override in 'com.acme.alpha.custom' (overrides: " +
+                "{ package: 'com.acme.alpha', basedOn }), or save it under a new name.",
+        );
+    });
+
+    it('CONTROL: the declared override beside a stored row keeps its line byte for byte (ADR-0146 D9)', () => {
+        const pair = `${A}.custom`;
+        const declared = { overrides: { package: A, basedOn: '1.0.0' }, _provenance: 'org' };
+        const line = lineOf(
+            [
+                bindStoredRow({ ...leaf('notify'), _provenance: 'org' }, null),
+                { ...leaf('notify'), _packageId: A, _provenance: 'package' },
+                bindStoredRow({ ...leaf('notify'), ...declared }, pair),
+            ],
+            { notify: A },
+        );
+        expect(line).toBe(
+            "[Automation] Flow name collision: 'notify' is claimed by 3 definitions (the declared override in " +
+                "'com.acme.alpha.custom' (sys_metadata), package 'com.acme.alpha', a runtime-authored row " +
+                "(sys_metadata)); arming the declared override in 'com.acme.alpha.custom' (sys_metadata) because it " +
+                "is the declared override of 'com.acme.alpha' (ADR-0146 D9), and shadowing 2 other definition(s). " +
+                "Only the armed definition dispatches. To change what runs, declare it as an override in " +
+                "'com.acme.alpha.custom' (overrides: { package: 'com.acme.alpha', basedOn }), or save it under a new name.",
+        );
+    });
+
+    it('CONTROL: two environment rows of a name no package ships keep their line byte for byte (arrival order)', () => {
+        const line = lineOf([{ ...leaf('notify'), _provenance: 'org' }, { ...leaf('notify'), _provenance: 'org' }], {});
+        expect(line).toBe(
+            "[Automation] Flow name collision: 'notify' is claimed by 2 definitions (a runtime-authored row " +
+                '(sys_metadata), a runtime-authored row (sys_metadata)); arming a runtime-authored row (sys_metadata) ' +
+                'by arrival order, and shadowing 1 other definition(s). Only the armed definition dispatches. ' +
+                'Rename one of them.',
+        );
+    });
+});