Skip to content

Commit de43524

Browse files
fix(objectql): report a bound action named like one its target already lists, and registerAction replacing another owner's handler (#24526)
Fixes #24472 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) Dev session `session_01PmE7dZP8v5iv3z1dgybAtT`, dispatched by the `domain:engine` seat 1 (`session_01JtSQQ3EWsbNPzyYMug1tXy`) under claim 6264504693. It follows the maintainer decision 6262054499 ("18.x, report first"): the collision is reported, and nothing is refused. ## What changes - **The listing face** (`packages/objectql/src/registry.ts`). The audit sits beside `findUnboundActionTargets()` on `SchemaRegistry`, keyed on `FIND_BOUND_ACTION_COLLISIONS`, a `unique symbol` declared in `registry.ts` and re-exported from neither `index.ts` nor `core.ts`, so the published face gains no nameable member (the shape of PR #24518). The plugin calls it through that key. It returns each bound top-level action that its target object does not list, because the object already lists a **different** declaration under the same name. That declaration is either the object's own (rung 1 before rung 2, so `attachBoundActions` skips the name) or another package's bound action that was attached first. Each pair names the object, the action, the package whose declaration is served, and the package whose declaration is not listed. `attachBoundActions` itself is unchanged. - **The sameness test.** `sameActionDeclaration` compares two declarations structurally. At the top level it leaves out the provenance keys: every `_`-prefixed stamp (`_packageId`, `_provenance`, `_packageVersion`, the `_lock` envelope), the `protection` block those stamps translate, and `objectName`, which an object's own action leaves to its position. A pair that is one declaration carried twice is not a collision. Two cases are pinned: a same-stack bound action merged onto its object and kept at the top level (in compiled-artifact shape, so two copies), and one declaration shipped by two packages. - **The report** (`packages/objectql/src/plugin.ts`). At `kernel:ready`, right after the unbound-target refusal seals, the boot logs one `warn` per pair through `describeBoundActionCollision`. The boot always continues. **Late artifacts are judged too.** An artifact registered through the `manifest` service after `kernel:ready` (the `boundActionTargetsSealed` path: a marketplace install, a ledger rehydrate) is judged **after** it registers, and only for pairs that name one of its packages on either side. A pair the boot already reported is not reported again. The judgment comes after registration because a warning has nothing to refuse beforehand. - **The handler face** (`packages/objectql/src/engine.ts`, `registerAction`). When a registration lands on an `OBJECT:ACTION` key that another owner holds, it still replaces that owner's handler, so the last registration runs as before. It now logs a `warn` naming both owners and the consequence: the action door runs the new owner's handler whatever declaration the object lists, and withdrawing that owner leaves the action with no handler. **The line drawn:** - These stay silent: the same owner registering again (a re-bind, a reload), and the authored-action resync (`removeActionsByPackage('metadata-service')` runs before it registers). - A registration with no `packageName` is treated as its own owner. - The handler face sees handlers, not declarations, so it cannot tell "same declaration" apart from "different". When two packages carry one declaration, the handler face warns and the listing face does not. That is still a true statement: withdrawing the replacing package removes the handler the other package's declaration relies on (pinned). Not built, by the ruling and the claim: no refusal, no new error code, no new key on the served object, and no change to the router's precedence. ## Measurement M (booted composition, before and after) **Composition.** `os dev --fresh`, using the CLI built from this tree. The host config composes two **independent** packages as two `new AppPlugin(...)` entries: - `com.m.core` owns `crm_account`, with an embedded `script` action `whoami` whose body returns `{ by: 'core' }`; - `com.m.binder` binds a top-level `script` action `whoami` (`objectName: 'crm_account'`, body `{ by: 'binder' }`) and extends the object with one field. Registration order is the plugin order. Each row is a fresh boot: `GET /api/v1/meta/object/crm_account`, `POST /api/v1/actions/crm_account/whoami`, then a grep of the boot log. | case | served declaration | handler that ran | boot log before | boot log after | |---|---|---|---|---| | different, core first | core's (`Who am I (core)`) | **binder's** (`{"by":"binder"}`) | nothing | 1 listing warning (served `com.m.core`, shadowed `com.m.binder`) + 1 handler warning (`app:com.m.binder` replaces `app:com.m.core`) | | different, binder first | core's | core's | nothing | 1 listing warning (same pair) + 1 handler warning (`app:com.m.core` replaces `app:com.m.binder`) | | control, same declaration, core first | core's | core's body | nothing | **0** listing warnings, 1 handler warning | | control, same declaration, binder first | core's | core's | nothing | **0** listing warnings, 1 handler warning | - After the change, the served declaration and the running handler are identical to before in every row. - In the core-first row they disagree: core's declaration is served and the binder's handler runs. That is pre-existing and is now reported, by both faces. - **Composed into one artifact instead:** `composeStacks` already refuses the same pair at build time, identical declarations included ("cross-stack action key collision"). So the runtime collision exists only between separately installed packages, and that is the population measured here. **The named producer's shape (#24318), live.** `sys_organization` with a late artifact. Marketplace install-local takes an inline manifest binding `change_slug` (`type: url`) to `sys_organization`. - Before: nothing is logged, `change_slug` is served from `com.objectstack.plugin-auth` (`type: api`), and the binder's declaration is listed nowhere. - After: one warning per arrival naming `com.objectstack.plugin-auth` and `com.m.late3`. It fired once when the ledger rehydrated at boot, through the sealed door, and once more on re-install. The served object is unchanged. **Volume on stock boots, after:** `examples/app-crm`, `examples/app-showcase` and `examples/app-todo` give 0 listing warnings and 0 handler warnings each. Across the whole `@objectstack/objectql` suite (447 files), the handler warning fires 3 times, all from this PR's own pins. ## Rider: stale attached copy after uninstall. Measured, not live, dropped The card's steps, run on a booted composition with one package binding `mrider_ping` to `sys_organization`: 1. `GET /api/v1/meta/object/sys_organization` lists `mrider_ping@com.m.rider`. 2. **Step 2 is refused.** `PUT /api/v1/meta/object/sys_organization` with the served body answers `NOT_OVERRIDABLE` ("provided by a managed package and is sealed"), so no stored overlay can be written back from a served read through the REST door. 3. **Step 3 is refused for a booted binder.** `DELETE /api/v1/packages/com.m.rider` and `PATCH .../disable` both answer `WRITABLE_PACKAGE_REQUIRED`. 4. **The one withdrawal door that exists**, marketplace install-local `DELETE`, was measured with two late binders: `com.m.late1` binds only, and `com.m.late2` binds and extends `sys_organization`. After both are uninstalled, `GET /api/v1/meta/object/sys_organization` lists neither action. - The binding-only package never rewrote the boot-bridged MetadataService copy, which gets the action attached at read time. - The extending package's copy is restored by `restoreExtendedObjectCopies` from the registry's current definition. The served answer is right on every reachable path, so nothing is built for the rider. The optional `boundCodeActions()` index rider was not taken. The boot audit walks the bound actions once, each against a cached fold, and this PR does not otherwise touch that function. ## Verification All on head `f679753aa` (after merging `origin/main` `f4755a68a`) unless stated. Fix round 1 (`bb02d9415`) re-ran the pin file, the neighbour, the ablations, the build, the typecheck, core `test:repo` and all 72 derived gates; the objectql full suite stands from `f679753aa`. - **Pins:** `packages/objectql/src/bound-action-collision-report.test.ts` has 15 tests (plus a face pin: the key is exported from neither entry, and `SchemaRegistry.prototype` names no `findBoundActionCollisions`). - Registry: both orders; same declaration across two packages; the same-stack copy; a foreign-against-foreign pair; the narrowed set. - Report: a boot warning carrying exact meta and naming its subjects; the control boot; the late artifact; no re-report. - Handler face: replace and warn; withdrawal leaves no handler; a registration with no package; the silent same-owner and resync cases. - The neighbouring `bound-action-foreign-object.test.ts` stays green unmoved, 14 tests. Its pin "an action the object already declares under the same name keeps the object's declaration" still holds: the served answer did not move. - **Pin sweep:** grepped the tests that pin `attachBoundActions`' same-name skip and `registerAction`'s replacement (`bound-action-foreign-object`, `plugin-authored-actions`, `action-governance`, `engine-repo-execute-elevation`, `action-activation`, and the runtime binder suites). None asserts that the replacement is silent, and none moves. - **Ablations**, mutated on disk through `scripts/ablation-replace.mjs`, each restored with `git diff HEAD` empty afterwards. The suite imports `src` by relative path, so no `dist` rebuild was needed. - Listing audit: the collision test is replaced by `continue`, and 7 of 15 tests go red. - Handler report: the replacement guard is replaced by `false`, and 2 of 15 go red. - Sameness: the top-level key set is replaced by a sentinel, and 3 of 15 go red (exactly the three "same declaration" controls). The first attempt at this one was refused by the tool as a no-op, because the replacement text contained the anchor. It was redone with a different anchor. - Face: `index.ts` re-exports the key, and 1 of 15 goes red (the face pin). - **Package checks:** - `pnpm --filter @objectstack/objectql test` exits 0: 447 files, 8406 tests. - `pnpm --filter @objectstack/objectql typecheck` exits 0, including `check:test-typecheck`. - `pnpm --filter @objectstack/core test:repo` exits 0: 6 files, 82 tests (measured on `b848c4a36`). - `pnpm --filter @objectstack/spec check:generated` exits 0 after the merge, because spec moved on main's side. - **Derived gates:** `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`, with no paths, prints 72 commands. All 72 were run on `f679753aa` and again on `bb02d9415`, every one with exit 0, and every exit code was captured before any pipe. Reconciling with `--ran` gives "72 derived, 72 run, 0 NOT-MEASURED, 0 UNRUN (a DERIVED zero)". `check:dual-build-cjs-loads` first exited 3 (PREREQUISITE NOT MET: 8 packages had no `dist`), so the 8 were built and the gate re-run, exit 0. The repo was deepened first (`--shallow-since=2026-07-13`) for `check-engine-split-ratio --days 90` and `check-plugin-teardown-shape`. ## Acceptance notes - **Owner strings differ between the two faces.** The handler face names the owner `registerAction` was given: `app:ID` from the runtime's artifact binder, or `metadata-service`. The listing face names the registry's package id, `ID`. Both are what each layer actually holds; no mapping is invented. - **Two copies of the comparison.** `sameActionDeclaration` is a local structural comparator. `@objectstack/spec`'s `deepEqualAuthored` answers the same "same declaration?" question, but it is `@internal` and not exported, and exporting it would widen spec's surface. This PR now holds a second copy of that comparison. It has a provenance-key filter at the top level, which spec's does not need. - **Who becomes the replacing owner.** After a cross-package replacement, `removeActionsByPackage(loser)` removes nothing and `removeActionsByPackage(winner)` removes the key. The handler warning states this consequence. Changing it would mean changing which handler runs, which is out of scope by the ruling. - **Measurement fixtures.** The scratch compositions lived under `examples/_m24472*` during the measurement and were deleted. The repository's tree is unchanged apart from the five files listed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01PmE7dZP8v5iv3z1dgybAtT --- _Generated by [Claude Code](https://claude.ai/code/session_01PmE7dZP8v5iv3z1dgybAtT)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent ba4afcc commit de43524

5 files changed

Lines changed: 429 additions & 3 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/objectql': patch
3+
---
4+
5+
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
6+
7+
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)
8+
9+
- **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).
10+
- **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.
11+
- **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.
12+
- **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.
13+
- **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.
14+
- **Fix.** Rename one of the two actions, or drop the duplicate declaration.
Lines changed: 246 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,246 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#24472] A top-level action bound by `objectName` to an object that already
5+
* lists an action of the same name is not attached — the object keeps serving
6+
* its own declaration (the router's rung 1). When the two declarations DIFFER,
7+
* that pair is reported, never refused: one warning per pair, naming the
8+
* object, the action and both packages, at `kernel:ready` and for each artifact
9+
* that arrives later. A pair that is one declaration carried twice is not a
10+
* collision.
11+
*
12+
* The handler face of the same pair: the engine keys handlers by
13+
* `OBJECT:ACTION`, so the second package's `registerAction` replaces the
14+
* first's. It still does — the last registration runs — but it is warned,
15+
* naming both owners; the same owner re-registering is not.
16+
*/
17+
18+
import { describe, it, expect, afterEach, vi } from 'vitest';
19+
import { ObjectKernel, type PluginContext } from '@objectstack/core';
20+
import { defineStack } from '@objectstack/spec';
21+
import { ObjectQL } from './engine.js';
22+
import { ObjectQLPlugin } from './plugin.js';
23+
import { FIND_BOUND_ACTION_COLLISIONS, SchemaRegistry } from './registry.js';
24+
25+
/** The package that OWNS the target object, with an action of its own. */
26+
const OWNER = {
27+
id: 'com.test.owner', name: 'owner',
28+
objects: [{
29+
name: 'bc_org', label: 'Org',
30+
fields: { name: { name: 'name', type: 'text', label: 'Name' } },
31+
actions: [{ name: 'rename', label: 'Rename', type: 'url', target: 'https://example.com/own', locations: ['record_header'] }],
32+
}],
33+
};
34+
35+
/** Another package binding an action of the same NAME to the owner's object — a different declaration. */
36+
const TWIN = {
37+
id: 'com.test.twin', name: 'twin',
38+
actions: [{ name: 'rename', label: 'Rename (twin)', objectName: 'bc_org', type: 'url', target: 'https://example.com/twin', locations: ['record_header'] }],
39+
};
40+
41+
/** Another package carrying the owner's declaration itself, bound by `objectName` — one declaration, two packages. */
42+
const CARRIER = {
43+
id: 'com.test.carrier', name: 'carrier',
44+
actions: [{ ...OWNER.objects[0].actions[0], objectName: 'bc_org' }],
45+
};
46+
47+
/** A package that brings an unrelated object — the late-artifact control. */
48+
const BYSTANDER = {
49+
id: 'com.test.bystander', name: 'bystander',
50+
objects: [{ name: 'bc_other', label: 'Other', fields: { name: { name: 'name', type: 'text', label: 'Name' } } }],
51+
};
52+
53+
async function engineWith(packages: any[]) {
54+
const engine = new ObjectQL();
55+
await engine.init();
56+
for (const pkg of packages) engine.registerApp(structuredClone(pkg));
57+
return engine;
58+
}
59+
60+
const servedRename = (engine: ObjectQL): any[] =>
61+
((engine.registry.getObject('bc_org') as any)?.actions ?? []).filter((a: any) => a?.name === 'rename');
62+
63+
describe('the registry\'s bound-action collision audit', () => {
64+
for (const [order, packages] of [
65+
['owner first', [OWNER, TWIN]],
66+
['twin first', [TWIN, OWNER]],
67+
] as const) {
68+
it(`${order}: a different declaration under the object's own action name is one pair, naming both packages`, async () => {
69+
const engine = await engineWith([...packages]);
70+
expect(engine.registry[FIND_BOUND_ACTION_COLLISIONS]()).toEqual([
71+
{ objectName: 'bc_org', action: 'rename', servedPackageId: 'com.test.owner', shadowedPackageId: 'com.test.twin' },
72+
]);
73+
// Reported, not acted on: the object still serves its own declaration.
74+
expect(servedRename(engine).map((a) => a.target)).toEqual(['https://example.com/own']);
75+
});
76+
}
77+
78+
it('one declaration carried by two packages is not a collision', async () => {
79+
const engine = await engineWith([OWNER, CARRIER]);
80+
expect(engine.registry[FIND_BOUND_ACTION_COLLISIONS]()).toEqual([]);
81+
});
82+
83+
it('a same-stack bound action — merged onto its object and kept top-level — is not a collision', async () => {
84+
const stack = defineStack({
85+
manifest: { id: 'com.test.same', name: 'same', version: '1.0.0', type: 'app' },
86+
objects: [{ name: 'bc_same', label: 'Same', fields: { name: { name: 'name', type: 'text', label: 'Name' } } }],
87+
actions: [{ name: 'same_action', label: 'Same', objectName: 'bc_same', type: 'url', target: 'https://example.com', locations: ['record_header'] }],
88+
} as any);
89+
// The JSON round trip is the compiled artifact's shape: two copies, not one reference.
90+
const engine = await engineWith([JSON.parse(JSON.stringify({ ...stack, ...stack.manifest }))]);
91+
expect((engine.registry.getObject('bc_same') as any)?.actions?.map((a: any) => a.name)).toEqual(['same_action']);
92+
expect(engine.registry[FIND_BOUND_ACTION_COLLISIONS]()).toEqual([]);
93+
});
94+
95+
it('two packages binding different declarations of one name to a third package\'s object: the first attached is served, the second reported', async () => {
96+
const owner = { ...OWNER, objects: [{ ...OWNER.objects[0], actions: [] }] };
97+
const second = { ...TWIN, id: 'com.test.second', name: 'second', actions: [{ ...TWIN.actions[0], label: 'Rename (second)', target: 'https://example.com/second' }] };
98+
const engine = await engineWith([owner, TWIN, second]);
99+
expect(servedRename(engine).map((a) => a._packageId)).toEqual(['com.test.twin']);
100+
expect(engine.registry[FIND_BOUND_ACTION_COLLISIONS]()).toEqual([
101+
{ objectName: 'bc_org', action: 'rename', servedPackageId: 'com.test.twin', shadowedPackageId: 'com.test.second' },
102+
]);
103+
});
104+
105+
it('is no nameable member of the published face: keyed on a symbol neither package entry exports', async () => {
106+
const entry = await import('./index.js');
107+
const core = await import('./core.js');
108+
expect('FIND_BOUND_ACTION_COLLISIONS' in entry).toBe(false);
109+
expect('FIND_BOUND_ACTION_COLLISIONS' in core).toBe(false);
110+
expect(Object.values(entry)).not.toContain(FIND_BOUND_ACTION_COLLISIONS);
111+
expect(Object.getOwnPropertyNames(SchemaRegistry.prototype)).not.toContain('findBoundActionCollisions');
112+
expect(Object.getOwnPropertySymbols(SchemaRegistry.prototype)).toContain(FIND_BOUND_ACTION_COLLISIONS);
113+
});
114+
115+
it('narrowed to a set of packages, it answers only the pairs naming one of them', async () => {
116+
const engine = await engineWith([OWNER, TWIN, BYSTANDER]);
117+
expect(engine.registry[FIND_BOUND_ACTION_COLLISIONS](new Set(['com.test.bystander']))).toEqual([]);
118+
expect(engine.registry[FIND_BOUND_ACTION_COLLISIONS](new Set(['com.test.owner']))).toHaveLength(1);
119+
expect(engine.registry[FIND_BOUND_ACTION_COLLISIONS](new Set(['com.test.twin']))).toHaveLength(1);
120+
});
121+
});
122+
123+
// ── The report: at kernel:ready, and for each later artifact ───────────────
124+
125+
interface ManifestService {
126+
register(artifact: unknown): Promise<void> | void;
127+
}
128+
129+
const kernels: ObjectKernel[] = [];
130+
afterEach(async () => {
131+
while (kernels.length) {
132+
const kernel = kernels.pop()!;
133+
try { if (kernel.getState() === 'running') await kernel.shutdown(); } catch { /* noop */ }
134+
}
135+
});
136+
137+
function registering(name: string, artifacts: unknown[]) {
138+
return {
139+
name,
140+
dependencies: ['com.objectstack.engine.objectql'],
141+
init: async (ctx: PluginContext) => {
142+
const manifest = ctx.getService<ManifestService>('manifest');
143+
for (const artifact of artifacts) await manifest.register(structuredClone(artifact));
144+
},
145+
};
146+
}
147+
148+
/** A kernel whose logger's `warn` is spied: the plugin's `ctx.logger` is the kernel's own. */
149+
function kernelWith(...plugins: any[]) {
150+
const kernel = new ObjectKernel({ logger: { level: 'silent' }, gracefulShutdown: false });
151+
kernels.push(kernel);
152+
const warn = vi.spyOn((kernel as any).logger, 'warn');
153+
const collisionWarnings = () => warn.mock.calls.filter(([, meta]: any[]) => meta && 'shadowedPackageId' in meta);
154+
return { kernel, collisionWarnings, use: async () => { for (const p of plugins) await kernel.use(p); } };
155+
}
156+
157+
describe('the bound-action collision report', () => {
158+
it('warns once at kernel:ready, naming the object, the action and both packages — and the boot goes on', async () => {
159+
const { kernel, collisionWarnings, use } = kernelWith(new ObjectQLPlugin(), registering('owner', [OWNER]), registering('twin', [TWIN]));
160+
await use();
161+
await kernel.bootstrap();
162+
const warned = collisionWarnings();
163+
expect(warned).toHaveLength(1);
164+
const [message, meta] = warned[0] as [string, any];
165+
expect(meta).toEqual({ objectName: 'bc_org', action: 'rename', servedPackageId: 'com.test.owner', shadowedPackageId: 'com.test.twin' });
166+
for (const subject of ["'rename'", "'bc_org'", "'com.test.owner'", "'com.test.twin'"]) expect(message).toContain(subject);
167+
expect(servedRename(kernel.getService<ObjectQL>('objectql')).map((a) => a.target)).toEqual(['https://example.com/own']);
168+
});
169+
170+
it('control: one declaration carried by two packages boots with no warning', async () => {
171+
const { kernel, collisionWarnings, use } = kernelWith(new ObjectQLPlugin(), registering('owner', [OWNER]), registering('carrier', [CARRIER]));
172+
await use();
173+
await kernel.bootstrap();
174+
expect(collisionWarnings()).toEqual([]);
175+
});
176+
177+
it('an artifact arriving after the boot is judged once it registers, and nothing of it is refused', async () => {
178+
const { kernel, collisionWarnings, use } = kernelWith(new ObjectQLPlugin(), registering('owner', [OWNER]));
179+
await use();
180+
await kernel.bootstrap();
181+
expect(collisionWarnings()).toEqual([]);
182+
await kernel.getService<ManifestService>('manifest').register(structuredClone(TWIN));
183+
expect(collisionWarnings().map(([, meta]) => meta)).toEqual([
184+
{ objectName: 'bc_org', action: 'rename', servedPackageId: 'com.test.owner', shadowedPackageId: 'com.test.twin' },
185+
]);
186+
expect(kernel.getService<ObjectQL>('objectql').registry.getItem('action', 'rename')).toBeDefined();
187+
});
188+
189+
it('a pair the boot reported is not reported again for an unrelated late artifact', async () => {
190+
const { kernel, collisionWarnings, use } = kernelWith(new ObjectQLPlugin(), registering('owner', [OWNER]), registering('twin', [TWIN]));
191+
await use();
192+
await kernel.bootstrap();
193+
await kernel.getService<ManifestService>('manifest').register(structuredClone(BYSTANDER));
194+
expect(collisionWarnings()).toHaveLength(1);
195+
});
196+
});
197+
198+
// ── The handler face: registerAction's OBJECT:ACTION key ────────────────────
199+
200+
describe('ObjectQL.registerAction on a key another owner holds', () => {
201+
async function engineSpied() {
202+
const engine = new ObjectQL();
203+
const warn = vi.spyOn((engine as any).logger, 'warn');
204+
const replacements = () => warn.mock.calls.filter(([, meta]: any[]) => meta && 'replacedPackage' in meta);
205+
return { engine, replacements };
206+
}
207+
208+
it('replaces the handler — the last registration runs — and warns naming both owners', async () => {
209+
const { engine, replacements } = await engineSpied();
210+
engine.registerAction('bc_org', 'rename', () => 'owner', 'app:com.test.owner');
211+
engine.registerAction('bc_org', 'rename', () => 'twin', 'app:com.test.twin');
212+
expect(await engine.executeAction('bc_org', 'rename', {})).toBe('twin');
213+
expect(replacements()).toHaveLength(1);
214+
const [message, meta] = replacements()[0] as [string, any];
215+
expect(meta).toEqual({ objectName: 'bc_org', actionName: 'rename', package: 'app:com.test.twin', replacedPackage: 'app:com.test.owner' });
216+
for (const subject of ["'bc_org:rename'", "'app:com.test.owner'", "'app:com.test.twin'"]) expect(message).toContain(subject);
217+
});
218+
219+
it('withdrawing the replacing owner leaves the key with no handler — the consequence the warning names', async () => {
220+
const { engine } = await engineSpied();
221+
engine.registerAction('bc_org', 'rename', () => 'owner', 'app:com.test.owner');
222+
engine.registerAction('bc_org', 'rename', () => 'twin', 'app:com.test.twin');
223+
engine.removeActionsByPackage('app:com.test.twin');
224+
await expect(engine.executeAction('bc_org', 'rename', {})).rejects.toThrow(/not found/);
225+
});
226+
227+
it('a registration with no package and a package\'s are two owners', async () => {
228+
const { engine, replacements } = await engineSpied();
229+
engine.registerAction('bc_org', 'rename', () => 'owner', 'app:com.test.owner');
230+
engine.registerAction('bc_org', 'rename', () => 'imperative');
231+
expect(replacements().map(([, meta]) => (meta as any).replacedPackage)).toEqual(['app:com.test.owner']);
232+
});
233+
234+
it('the same owner re-registering, and the tear-down-then-register resync, are silent', async () => {
235+
const { engine, replacements } = await engineSpied();
236+
engine.registerAction('bc_org', 'rename', () => 1, 'app:com.test.owner');
237+
engine.registerAction('bc_org', 'rename', () => 2, 'app:com.test.owner');
238+
engine.registerAction('bc_org', 'unowned', () => 1);
239+
engine.registerAction('bc_org', 'unowned', () => 2);
240+
engine.registerAction('bc_org', 'authored', () => 1, 'metadata-service');
241+
engine.removeActionsByPackage('metadata-service');
242+
engine.registerAction('bc_org', 'authored', () => 2, 'metadata-service');
243+
expect(replacements()).toEqual([]);
244+
expect(await engine.executeAction('bc_org', 'rename', {})).toBe(2);
245+
});
246+
});

‎packages/objectql/src/engine.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5563,9 +5563,28 @@ export class ObjectQL implements IObjectQLEngine {
55635563
* action's declared param contract at dispatch before the handler runs.
55645564
* The seam itself stays untyped so existing untyped handlers keep working.
55655565
* @param packageName Optional package owner (for cleanup)
5566+
*
5567+
* [#24472] A key another package already holds is still replaced — the last
5568+
* registration runs, as before — but no longer silently: the replacement is
5569+
* warned, naming both owners. The same owner re-registering (a re-bind, a
5570+
* reload, the authored-action resync, which tears its set down first) is
5571+
* not. A registration with no `packageName` is its own owner here, so it
5572+
* replacing a package's handler, or the reverse, is warned too.
55665573
*/
55675574
registerAction(objectName: string, actionName: string, handler: (ctx: any) => Promise<any> | any, packageName?: string): void {
55685575
const key = `${objectName}:${actionName}`;
5576+
const replaced = this.actions.get(key);
5577+
if (replaced !== undefined && replaced.package !== packageName) {
5578+
const by = (name: string | undefined) => (name ? `by package '${name}'` : 'with no package');
5579+
this.logger.warn(
5580+
`Action handler '${key}' registered ${by(packageName)} replaces the one registered ${by(replaced.package)}: `
5581+
+ `the action door now runs the handler registered ${by(packageName)}, whichever declaration object `
5582+
+ `'${objectName}' lists${packageName ? `, and withdrawing package '${packageName}' leaves the action with no handler` : ''}. `
5583+
+ `Two owners share one handler key only when both declare action '${actionName}' on object `
5584+
+ `'${objectName}' — rename one of them.`,
5585+
{ objectName, actionName, package: packageName, replacedPackage: replaced.package },
5586+
);
5587+
}
55695588
this.actions.set(key, { handler, package: packageName });
55705589
this.logger.debug('Registered action', { objectName, actionName, package: packageName });
55715590
}

0 commit comments

Comments
 (0)