fix(objectql): report a bound action named like one its target already lists, and registerAction replacing another owner's handler

.changeset/24472-bound-action-collision-report.md

@@ -0,0 +1,14 @@
+---
+'@objectstack/objectql': patch
+---
+
+fix(objectql): a bound action named like an action its target object already lists, with a different declaration, is reported at boot naming both packages; `registerAction` reports replacing another owner's handler
+
+Clause-②: no (a warning only: no accept set narrows, no door answer moves, and no published surface gains a key; owes the seat's review of record before the queue)
+
+- **The listing.** A top-level action bound by `objectName` to an object that already lists an action of the same name is still not attached: the object keeps serving its own declaration, as the action router does. When the two declarations differ, the boot now warns once per pair, at `kernel:ready`, naming the object, the action, the package whose declaration is served and the package whose declaration is not listed. Two packages binding different declarations of one name to a third package's object are reported the same way (the first one attached is served).
+- **Not a collision.** One declaration carried twice is compared without its provenance stamps (`_packageId`, `_provenance`, the `_lock` envelope, `protection`) and without `objectName`, and is not reported: a same-stack bound action merged onto its object, or the same declaration shipped by two packages.
+- **Later artifacts.** An artifact registered after the boot (a marketplace install, a ledger rehydrate) is judged once it has registered, for the pairs naming one of its packages; a pair the boot already reported is not reported again.
+- **The handler.** `ObjectQL.registerAction` keys handlers by `OBJECT:ACTION`. A registration under a key another owner holds still replaces it (the last registration runs), and now warns, naming both owners: the action door runs the new owner's handler whichever declaration the object lists, and withdrawing that owner leaves the action with no handler. The same owner re-registering is silent, and so is the authored-action resync, which tears its set down first. A registration with no package is its own owner.
+- **Unchanged.** Nothing is refused and no boot fails on it; the served `actions`, the router's precedence and which handler runs are as before. Measured on stock CRM, showcase and todo boots: no new warning.
+- **Fix.** Rename one of the two actions, or drop the duplicate declaration.

packages/objectql/src/bound-action-collision-report.test.ts

@@ -0,0 +1,246 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#24472] A top-level action bound by `objectName` to an object that already
+ * lists an action of the same name is not attached — the object keeps serving
+ * its own declaration (the router's rung 1). When the two declarations DIFFER,
+ * that pair is reported, never refused: one warning per pair, naming the
+ * object, the action and both packages, at `kernel:ready` and for each artifact
+ * that arrives later. A pair that is one declaration carried twice is not a
+ * collision.
+ *
+ * The handler face of the same pair: the engine keys handlers by
+ * `OBJECT:ACTION`, so the second package's `registerAction` replaces the
+ * first's. It still does — the last registration runs — but it is warned,
+ * naming both owners; the same owner re-registering is not.
+ */
+
+import { describe, it, expect, afterEach, vi } from 'vitest';
+import { ObjectKernel, type PluginContext } from '@objectstack/core';
+import { defineStack } from '@objectstack/spec';
+import { ObjectQL } from './engine.js';
+import { ObjectQLPlugin } from './plugin.js';
+import { FIND_BOUND_ACTION_COLLISIONS, SchemaRegistry } from './registry.js';
+
+/** The package that OWNS the target object, with an action of its own. */
+const OWNER = {
+    id: 'com.test.owner', name: 'owner',
+    objects: [{
+        name: 'bc_org', label: 'Org',
+        fields: { name: { name: 'name', type: 'text', label: 'Name' } },
+        actions: [{ name: 'rename', label: 'Rename', type: 'url', target: 'https://example.com/own', locations: ['record_header'] }],
+    }],
+};
+
+/** Another package binding an action of the same NAME to the owner's object — a different declaration. */
+const TWIN = {
+    id: 'com.test.twin', name: 'twin',
+    actions: [{ name: 'rename', label: 'Rename (twin)', objectName: 'bc_org', type: 'url', target: 'https://example.com/twin', locations: ['record_header'] }],
+};
+
+/** Another package carrying the owner's declaration itself, bound by `objectName` — one declaration, two packages. */
+const CARRIER = {
+    id: 'com.test.carrier', name: 'carrier',
+    actions: [{ ...OWNER.objects[0].actions[0], objectName: 'bc_org' }],
+};
+
+/** A package that brings an unrelated object — the late-artifact control. */
+const BYSTANDER = {
+    id: 'com.test.bystander', name: 'bystander',
+    objects: [{ name: 'bc_other', label: 'Other', fields: { name: { name: 'name', type: 'text', label: 'Name' } } }],
+};
+
+async function engineWith(packages: any[]) {
+    const engine = new ObjectQL();
+    await engine.init();
+    for (const pkg of packages) engine.registerApp(structuredClone(pkg));
+    return engine;
+}
+
+const servedRename = (engine: ObjectQL): any[] =>
+    ((engine.registry.getObject('bc_org') as any)?.actions ?? []).filter((a: any) => a?.name === 'rename');
+
+describe('the registry\'s bound-action collision audit', () => {
+    for (const [order, packages] of [
+        ['owner first', [OWNER, TWIN]],
+        ['twin first', [TWIN, OWNER]],
+    ] as const) {
+        it(`${order}: a different declaration under the object's own action name is one pair, naming both packages`, async () => {
+            const engine = await engineWith([...packages]);
+            expect(engine.registry[FIND_BOUND_ACTION_COLLISIONS]()).toEqual([
+                { objectName: 'bc_org', action: 'rename', servedPackageId: 'com.test.owner', shadowedPackageId: 'com.test.twin' },
+            ]);
+            // Reported, not acted on: the object still serves its own declaration.
+            expect(servedRename(engine).map((a) => a.target)).toEqual(['https://example.com/own']);
+        });
+    }
+
+    it('one declaration carried by two packages is not a collision', async () => {
+        const engine = await engineWith([OWNER, CARRIER]);
+        expect(engine.registry[FIND_BOUND_ACTION_COLLISIONS]()).toEqual([]);
+    });
+
+    it('a same-stack bound action — merged onto its object and kept top-level — is not a collision', async () => {
+        const stack = defineStack({
+            manifest: { id: 'com.test.same', name: 'same', version: '1.0.0', type: 'app' },
+            objects: [{ name: 'bc_same', label: 'Same', fields: { name: { name: 'name', type: 'text', label: 'Name' } } }],
+            actions: [{ name: 'same_action', label: 'Same', objectName: 'bc_same', type: 'url', target: 'https://example.com', locations: ['record_header'] }],
+        } as any);
+        // The JSON round trip is the compiled artifact's shape: two copies, not one reference.
+        const engine = await engineWith([JSON.parse(JSON.stringify({ ...stack, ...stack.manifest }))]);
+        expect((engine.registry.getObject('bc_same') as any)?.actions?.map((a: any) => a.name)).toEqual(['same_action']);
+        expect(engine.registry[FIND_BOUND_ACTION_COLLISIONS]()).toEqual([]);
+    });
+
+    it('two packages binding different declarations of one name to a third package\'s object: the first attached is served, the second reported', async () => {
+        const owner = { ...OWNER, objects: [{ ...OWNER.objects[0], actions: [] }] };
+        const second = { ...TWIN, id: 'com.test.second', name: 'second', actions: [{ ...TWIN.actions[0], label: 'Rename (second)', target: 'https://example.com/second' }] };
+        const engine = await engineWith([owner, TWIN, second]);
+        expect(servedRename(engine).map((a) => a._packageId)).toEqual(['com.test.twin']);
+        expect(engine.registry[FIND_BOUND_ACTION_COLLISIONS]()).toEqual([
+            { objectName: 'bc_org', action: 'rename', servedPackageId: 'com.test.twin', shadowedPackageId: 'com.test.second' },
+        ]);
+    });
+
+    it('is no nameable member of the published face: keyed on a symbol neither package entry exports', async () => {
+        const entry = await import('./index.js');
+        const core = await import('./core.js');
+        expect('FIND_BOUND_ACTION_COLLISIONS' in entry).toBe(false);
+        expect('FIND_BOUND_ACTION_COLLISIONS' in core).toBe(false);
+        expect(Object.values(entry)).not.toContain(FIND_BOUND_ACTION_COLLISIONS);
+        expect(Object.getOwnPropertyNames(SchemaRegistry.prototype)).not.toContain('findBoundActionCollisions');
+        expect(Object.getOwnPropertySymbols(SchemaRegistry.prototype)).toContain(FIND_BOUND_ACTION_COLLISIONS);
+    });
+
+    it('narrowed to a set of packages, it answers only the pairs naming one of them', async () => {
+        const engine = await engineWith([OWNER, TWIN, BYSTANDER]);
+        expect(engine.registry[FIND_BOUND_ACTION_COLLISIONS](new Set(['com.test.bystander']))).toEqual([]);
+        expect(engine.registry[FIND_BOUND_ACTION_COLLISIONS](new Set(['com.test.owner']))).toHaveLength(1);
+        expect(engine.registry[FIND_BOUND_ACTION_COLLISIONS](new Set(['com.test.twin']))).toHaveLength(1);
+    });
+});
+
+// ── The report: at kernel:ready, and for each later artifact ───────────────
+
+interface ManifestService {
+    register(artifact: unknown): Promise<void> | void;
+}
+
+const kernels: ObjectKernel[] = [];
+afterEach(async () => {
+    while (kernels.length) {
+        const kernel = kernels.pop()!;
+        try { if (kernel.getState() === 'running') await kernel.shutdown(); } catch { /* noop */ }
+    }
+});
+
+function registering(name: string, artifacts: unknown[]) {
+    return {
+        name,
+        dependencies: ['com.objectstack.engine.objectql'],
+        init: async (ctx: PluginContext) => {
+            const manifest = ctx.getService<ManifestService>('manifest');
+            for (const artifact of artifacts) await manifest.register(structuredClone(artifact));
+        },
+    };
+}
+
+/** A kernel whose logger's `warn` is spied: the plugin's `ctx.logger` is the kernel's own. */
+function kernelWith(...plugins: any[]) {
+    const kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false });
+    kernels.push(kernel);
+    const warn = vi.spyOn((kernel as any).logger, 'warn');
+    const collisionWarnings = () => warn.mock.calls.filter(([, meta]: any[]) => meta && 'shadowedPackageId' in meta);
+    return { kernel, collisionWarnings, use: async () => { for (const p of plugins) await kernel.use(p); } };
+}
+
+describe('the bound-action collision report', () => {
+    it('warns once at kernel:ready, naming the object, the action and both packages — and the boot goes on', async () => {
+        const { kernel, collisionWarnings, use } = kernelWith(new ObjectQLPlugin(), registering('owner', [OWNER]), registering('twin', [TWIN]));
+        await use();
+        await kernel.bootstrap();
+        const warned = collisionWarnings();
+        expect(warned).toHaveLength(1);
+        const [message, meta] = warned[0] as [string, any];
+        expect(meta).toEqual({ objectName: 'bc_org', action: 'rename', servedPackageId: 'com.test.owner', shadowedPackageId: 'com.test.twin' });
+        for (const subject of ["'rename'", "'bc_org'", "'com.test.owner'", "'com.test.twin'"]) expect(message).toContain(subject);
+        expect(servedRename(kernel.getService<ObjectQL>('objectql')).map((a) => a.target)).toEqual(['https://example.com/own']);
+    });
+
+    it('control: one declaration carried by two packages boots with no warning', async () => {
+        const { kernel, collisionWarnings, use } = kernelWith(new ObjectQLPlugin(), registering('owner', [OWNER]), registering('carrier', [CARRIER]));
+        await use();
+        await kernel.bootstrap();
+        expect(collisionWarnings()).toEqual([]);
+    });
+
+    it('an artifact arriving after the boot is judged once it registers, and nothing of it is refused', async () => {
+        const { kernel, collisionWarnings, use } = kernelWith(new ObjectQLPlugin(), registering('owner', [OWNER]));
+        await use();
+        await kernel.bootstrap();
+        expect(collisionWarnings()).toEqual([]);
+        await kernel.getService<ManifestService>('manifest').register(structuredClone(TWIN));
+        expect(collisionWarnings().map(([, meta]) => meta)).toEqual([
+            { objectName: 'bc_org', action: 'rename', servedPackageId: 'com.test.owner', shadowedPackageId: 'com.test.twin' },
+        ]);
+        expect(kernel.getService<ObjectQL>('objectql').registry.getItem('action', 'rename')).toBeDefined();
+    });
+
+    it('a pair the boot reported is not reported again for an unrelated late artifact', async () => {
+        const { kernel, collisionWarnings, use } = kernelWith(new ObjectQLPlugin(), registering('owner', [OWNER]), registering('twin', [TWIN]));
+        await use();
+        await kernel.bootstrap();
+        await kernel.getService<ManifestService>('manifest').register(structuredClone(BYSTANDER));
+        expect(collisionWarnings()).toHaveLength(1);
+    });
+});
+
+// ── The handler face: registerAction's OBJECT:ACTION key ────────────────────
+
+describe('ObjectQL.registerAction on a key another owner holds', () => {
+    async function engineSpied() {
+        const engine = new ObjectQL();
+        const warn = vi.spyOn((engine as any).logger, 'warn');
+        const replacements = () => warn.mock.calls.filter(([, meta]: any[]) => meta && 'replacedPackage' in meta);
+        return { engine, replacements };
+    }
+
+    it('replaces the handler — the last registration runs — and warns naming both owners', async () => {
+        const { engine, replacements } = await engineSpied();
+        engine.registerAction('bc_org', 'rename', () => 'owner', 'app:com.test.owner');
+        engine.registerAction('bc_org', 'rename', () => 'twin', 'app:com.test.twin');
+        expect(await engine.executeAction('bc_org', 'rename', {})).toBe('twin');
+        expect(replacements()).toHaveLength(1);
+        const [message, meta] = replacements()[0] as [string, any];
+        expect(meta).toEqual({ objectName: 'bc_org', actionName: 'rename', package: 'app:com.test.twin', replacedPackage: 'app:com.test.owner' });
+        for (const subject of ["'bc_org:rename'", "'app:com.test.owner'", "'app:com.test.twin'"]) expect(message).toContain(subject);
+    });
+
+    it('withdrawing the replacing owner leaves the key with no handler — the consequence the warning names', async () => {
+        const { engine } = await engineSpied();
+        engine.registerAction('bc_org', 'rename', () => 'owner', 'app:com.test.owner');
+        engine.registerAction('bc_org', 'rename', () => 'twin', 'app:com.test.twin');
+        engine.removeActionsByPackage('app:com.test.twin');
+        await expect(engine.executeAction('bc_org', 'rename', {})).rejects.toThrow(/not found/);
+    });
+
+    it('a registration with no package and a package\'s are two owners', async () => {
+        const { engine, replacements } = await engineSpied();
+        engine.registerAction('bc_org', 'rename', () => 'owner', 'app:com.test.owner');
+        engine.registerAction('bc_org', 'rename', () => 'imperative');
+        expect(replacements().map(([, meta]) => (meta as any).replacedPackage)).toEqual(['app:com.test.owner']);
+    });
+
+    it('the same owner re-registering, and the tear-down-then-register resync, are silent', async () => {
+        const { engine, replacements } = await engineSpied();
+        engine.registerAction('bc_org', 'rename', () => 1, 'app:com.test.owner');
+        engine.registerAction('bc_org', 'rename', () => 2, 'app:com.test.owner');
+        engine.registerAction('bc_org', 'unowned', () => 1);
+        engine.registerAction('bc_org', 'unowned', () => 2);
+        engine.registerAction('bc_org', 'authored', () => 1, 'metadata-service');
+        engine.removeActionsByPackage('metadata-service');
+        engine.registerAction('bc_org', 'authored', () => 2, 'metadata-service');
+        expect(replacements()).toEqual([]);
+        expect(await engine.executeAction('bc_org', 'rename', {})).toBe(2);
+    });
+});

packages/objectql/src/engine.ts

@@ -5560,9 +5560,28 @@ export class ObjectQL implements IObjectQLEngine {
    *   action's declared param contract at dispatch before the handler runs.
    *   The seam itself stays untyped so existing untyped handlers keep working.
    * @param packageName Optional package owner (for cleanup)
+   *
+   * [#24472] A key another package already holds is still replaced — the last
+   * registration runs, as before — but no longer silently: the replacement is
+   * warned, naming both owners. The same owner re-registering (a re-bind, a
+   * reload, the authored-action resync, which tears its set down first) is
+   * not. A registration with no `packageName` is its own owner here, so it
+   * replacing a package's handler, or the reverse, is warned too.
    */
   registerAction(objectName: string, actionName: string, handler: (ctx: any) => Promise<any> | any, packageName?: string): void {
     const key = `${objectName}:${actionName}`;
+    const replaced = this.actions.get(key);
+    if (replaced !== undefined && replaced.package !== packageName) {
+      const by = (name: string | undefined) => (name ? `by package '${name}'` : 'with no package');
+      this.logger.warn(
+        `Action handler '${key}' registered ${by(packageName)} replaces the one registered ${by(replaced.package)}: `
+        + `the action door now runs the handler registered ${by(packageName)}, whichever declaration object `
+        + `'${objectName}' lists${packageName ? `, and withdrawing package '${packageName}' leaves the action with no handler` : ''}. `
+        + `Two owners share one handler key only when both declare action '${actionName}' on object `
+        + `'${objectName}' — rename one of them.`,
+        { objectName, actionName, package: packageName, replacedPackage: replaced.package },
+      );
+    }
     this.actions.set(key, { handler, package: packageName });
     this.logger.debug('Registered action', { objectName, actionName, package: packageName });
   }