fix(objectql, metadata-protocol, cloud-connection)!: a declared .custom action override is keyed and judged the same way at every door, and only its declared base's handler slots beneath it

.changeset/30542-action-override-doors-and-boot-order.md

@@ -0,0 +1,22 @@
+---
+'@objectstack/metadata-protocol': patch
+'@objectstack/objectql': patch
+'@objectstack/cloud-connection': patch
+---
+
+fix(objectql, metadata-protocol, cloud-connection)!: a declared `.custom` action override is keyed and judged the same way at every door, and only its declared base's handler slots beneath it
+
+Clause-②: yes (narrowing) — the protocol door (save and publish) now refuses a declared action override whose `objectName` is not the base action's, with `403 NOT_OVERRIDABLE` and rule `declared_package_ships_no_such_item`, the refusal `os validate` and the managed install already gave it; the save door used to admit it. The managed install now admits a re-install of the pair whose override relaxes an earlier tightening back to the base's own gates, which it used to refuse with `declared_action_widens_access` while the save door admitted it. Every other input answers as before.
+Metadata-protocol: none
+Skills-docs: follow-up #30505 — the declared action override's docs card; it owes one sentence that an override keeps the base action's `objectName`.
+
+<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped: the action schema and the `overrides` declaration keep their shapes. What moves is which door admits one declared override shape, so all three doors give one answer, and the refusal carries its own remedy; nothing at rest is rewritten. -->
+
+**BREAKING** accept-set narrowing at the protocol door (`@objectstack/metadata-protocol`, ADR-0146 D2/D9): a declared action override names its base's action by `<objectName>:<name>`, the identity `os validate` and the managed install key it on.
+
+- **Before.** The save and publish doors matched a declared action override's base by name alone, so an override in the base's `.custom` pair could move the base's action to another object. Its handler was not rebound there, and `os validate` and the install door refused the same body.
+- **Now.** The save door refuses it as the other two doors do: the base ships no action of that name on that object. Keep the base action's `objectName` in the override, or remove `overrides` and add the action under the pair's own name.
+
+`@objectstack/cloud-connection`: the managed install judges a declared action override against the installed base's code artifact (`SchemaRegistry#getArtifactItem`), as the save door does. The protocol's per-package listing it read before serves an armed override under the base's envelope, so a re-install of the pair was judged against its own previous version.
+
+`@objectstack/objectql`: `ObjectQL#registerActionOverride` takes the package the override declares (`overrides.package`) as an optional fifth argument, and the authored-action resync passes it. `registerAction` under a key such an override holds slots the handler beneath it only when the registry's code artifact of the action belongs to that package, on that object; any other registration, one with no package included, replaces the override and is warned like every other handler collision. Before, any other package's registration slotted beneath it, so a row of another base's pair bound before the real base registered kept running after the real base's handler arrived, while every read served the real base's action; and a registration with no package slotted beneath it at `debug`, where the method's docblock promises a warning.

packages/cloud-connection/src/marketplace-install-local-override-gate.test.ts

@@ -44,6 +44,8 @@ const deps = (...ids: string[]) => Object.fromEntries(ids.map((id) => [id, '*'])
 
 /** What each installed package holds, as the protocol's per-package listing answers it. */
 type Holdings = Record<string, Record<string, Array<Record<string, unknown>>>>;
+/** [#30542] The registry's code artifacts (`getArtifactItem`), each under its package's `_packageId`. */
+type Artifacts = Array<{ type: string; item: Record<string, unknown> }>;
 
 function makeRawApp() {
     const routes = new Map<string, Handler>();
@@ -59,7 +61,7 @@ function makeRawApp() {
  * A kernel whose registry holds `installed` (BASE read-only — a code or
  * marketplace package is in `manifests`) and whose protocol lists `holdings`.
  */
-function makeCtx(rawApp: any, installed: Array<{ id: string; dependencies?: Record<string, string> }>, holdings: Holdings) {
+function makeCtx(rawApp: any, installed: Array<{ id: string; dependencies?: Record<string, string> }>, holdings: Holdings, artifacts?: Artifacts) {
     const hooks = new Map<string, any>();
     const registered: any[] = [];
     const saved: Array<[string, string]> = [];
@@ -77,6 +79,11 @@ function makeCtx(rawApp: any, installed: Array<{ id: string; dependencies?: Reco
             registry: {
                 getAllPackages: () => [...records, ...registered],
                 getPackage: (id: string) => records.find((r) => r.manifest.id === id),
+                // [#30542] The registry's code artifacts, when the case gives them.
+                ...(artifacts ? {
+                    getArtifactItem: (type: string, name: string, packageId?: string) => artifacts
+                        .find((a) => a.type === type && a.item.name === name && a.item._packageId === packageId)?.item,
+                } : {}),
             },
         }),
         protocol: {
@@ -132,9 +139,10 @@ async function install(
     manifest: Record<string, unknown>,
     installed: Array<{ id: string; dependencies?: Record<string, string> }> = [{ id: BASE }],
     holdings: Holdings = BASE_HOLDINGS,
+    artifacts?: Artifacts,
 ) {
     const rawApp = makeRawApp();
-    const k = makeCtx(rawApp, installed, holdings);
+    const k = makeCtx(rawApp, installed, holdings, artifacts);
     const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir });
     await plugin.start(k.ctx as any);
     await k.fire();
@@ -299,3 +307,37 @@ describe('install-local 2d — the override write gate at the managed install (A
         expect(saved).toEqual([['email_template', 'crm_welcome'], ['email_template', 'crm_welcome']]);
     });
 });
+
+/**
+ * [#30542] A re-install of the pair is judged against the base's code
+ * artifact, as the protocol door judges the save. Once the pair's declared
+ * action override is armed, the protocol's listing of the base serves that
+ * override under the base's envelope (measured on the real engine and
+ * protocol); the registry still holds the base's own action.
+ */
+describe('[#30542] the install door judges a declared action override against the base\'s code artifact', () => {
+    const overrides = { package: BASE, basedOn: '1.0.0' };
+    const BASE_GATES = { requiredPermissions: ['crm_admin'], ai: { requiresConfirmation: true } };
+    // v1, armed: it added a capability to the base's.
+    const armed: Holdings = {
+        [BASE]: { action: [{ name: 'crm_call', label: 'V1', overrides, ...BASE_GATES, requiredPermissions: ['crm_admin', 'crm_auditor'] }] },
+    };
+    const artifacts: Artifacts = [{ type: 'action', item: { name: 'crm_call', label: 'Call', ...BASE_GATES, _packageId: BASE } }];
+    const pairOf = (action: Record<string, unknown>) =>
+        ({ id: PAIR, version: '2.0.0', namespace: 'crm_custom', dependencies: deps(BASE), actions: [action] });
+
+    it('a v2 that relaxes the v1 tightening back to the base\'s own gates installs', async () => {
+        const { res, saved } = await install(pairOf({ name: 'crm_call', label: 'V2', overrides, ...BASE_GATES }), [{ id: BASE }], armed, artifacts);
+        expect(res.status, JSON.stringify(res.payload)).toBe(200);
+        expect(saved).toEqual([['action', 'crm_call']]);
+    });
+
+    it('a v2 that drops a gate the base\'s own action carries is still refused 403, and nothing is written', async () => {
+        const wider = { name: 'crm_call', label: 'V2', overrides, ...BASE_GATES, ai: {} };
+        const { res, saved, ledger } = await install(pairOf(wider), [{ id: BASE }], armed, artifacts);
+        expect(res.status, JSON.stringify(res.payload)).toBe(403);
+        expect(res.payload?.error?.code).toBe('NOT_OVERRIDABLE');
+        expect(refusedOf(res)).toEqual([['action', 'crm_call', 'declared_action_widens_access']]);
+        expect([saved, ledger]).toEqual([[], []]);
+    });
+});

packages/cloud-connection/src/marketplace-install-local-plugin.ts

@@ -568,6 +568,8 @@ type NamespaceHoldingEngine = PackageWritabilityEngine & {
 type OverrideGateEngine = PackageWritabilityEngine & {
     registry?: PackageWritabilityEngine['registry'] & {
         getAllPackages?(): unknown;
+        /** [#30542] An installed package's code artifact (`SchemaRegistry#getArtifactItem`). */
+        getArtifactItem?(type: string, name: string, currentPackageId?: string): unknown;
     };
 };
 type OverrideGateLister = {
@@ -3681,6 +3683,20 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
             return read;
         };
         const isBase = (id: string): boolean => delivered.has(id) || !isWritablePackage(ql, id);
+        // [#30542] An installed base's own item is its code artifact, the one
+        // the protocol door judges a declared action override against
+        // (`overrideWriteVerdict`'s `baseItemOf`). The listing is not: once
+        // the base's declared override is armed, it serves the override under
+        // the base's envelope, so a re-install of the pair was judged against
+        // its own previous version, and a v2 that relaxes a v1 tightening
+        // back to the base's own gates was refused. A registry that answers
+        // no artifact leaves the listed item.
+        const artifactOf = (item: OverrideWriteItem, base: string): unknown => {
+            const artifact = registry.getArtifactItem?.(item.type, item.name, base);
+            return isPlainObject(artifact) && artifact._packageId === base && listedIdentity(item.type, artifact) === item.identity
+                ? artifact
+                : undefined;
+        };
 
         const refusals: OverrideInstallRefusal[] = [];
         for (const [packageId, pkg] of delivered) {
@@ -3693,7 +3709,10 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
                 const shipped = new Map<string, unknown>();
                 for (const base of bases) {
                     const held = (await heldBy(item.type, base)).get(item.identity);
-                    if (held !== undefined) { shippers.push(base); shipped.set(base, held); }
+                    if (held !== undefined) {
+                        shippers.push(base);
+                        shipped.set(base, (delivered.has(base) ? undefined : artifactOf(item, base)) ?? held);
+                    }
                 }
                 const verdict = judgeOverrideWrite(
                     { type: item.type, name: item.name, packageId, item: item.item },

packages/metadata-protocol/src/protocol-packaged-base-refusal.ts

@@ -766,6 +766,17 @@ export function overrideWriteIdentityOf(type: string, identity: string, item: un
     return member === undefined ? identity : `${identity}\u0000${member}`;
 }
 
+/**
+ * [#30542] The object an `action` item is keyed on under the override write
+ * gate — its `objectName`, `''` when it names none: the first half of the
+ * `<objectName>:<name>` identity {@link overrideWriteItemsOf} gives it, and
+ * what {@link overrideWriteVerdict} matches a declared override's base on.
+ */
+function overrideWriteActionObjectOf(item: unknown): string {
+    const objectName = isPlainRecord(item) ? item.objectName : undefined;
+    return typeof objectName === 'string' ? objectName : '';
+}
+
 /** One item written into one package — what the override write gate judges. */
 export interface OverrideWriteSubject {
     /** The metadata type, either spelling (folded here). */
@@ -1112,9 +1123,8 @@ export function overrideWriteItemsOf(body: Record<string, unknown>, at = ''): Ov
                 }
                 continue;
             }
-            const identity = type === 'action' && typeof item.objectName === 'string' && item.objectName !== ''
-                ? `${item.objectName}:${declaredName}`
-                : declaredName;
+            const object = type === 'action' ? overrideWriteActionObjectOf(item) : '';
+            const identity = object !== '' ? `${object}:${declaredName}` : declaredName;
             out.push({ type, name: declaredName, identity: overrideWriteIdentityOf(type, identity, item), item, path });
         }
     }
@@ -1171,11 +1181,19 @@ export function overrideWriteVerdict(
     if (packageId !== undefined && scope === undefined) return NOT_CONCERNED;
     const dependencies = scope?.dependencies.filter((id) => id !== packageId);
     const member = overrideWriteMemberOf(request.type, request.item);
+    // [#30542] A declared action override names its base's action by the
+    // identity `os validate` and the install door key it on,
+    // `<objectName>:<name>` ({@link overrideWriteItemsOf}): the base's action
+    // of that name on another object is no shipper of it, so an override that
+    // moves the action to another object is refused here as it is there.
+    const onItsObject = (a: unknown): boolean => !declaration.present
+        || (PLURAL_TO_SINGULAR[request.type] ?? request.type) !== 'action'
+        || overrideWriteActionObjectOf(a) === overrideWriteActionObjectOf(request.item);
     const shippers = dependencies === undefined || dependencies.length === 0
         ? []
         : Array.from(new Set(
             this.shippedArtifactsOf(request.type, request.name, undefined, member)
-                .filter((a) => overrideWriteMemberOf(request.type, a) === member)
+                .filter((a) => overrideWriteMemberOf(request.type, a) === member && onItsObject(a))
                 .map((a) => (a as { _packageId?: unknown } | null | undefined)?._packageId)
                 // An override replaces an item of an INSTALLED, read-only base
                 // (ADR-0146 D1): a writable package's item is edited in place
@@ -1188,7 +1206,7 @@ export function overrideWriteVerdict(
     // [#30065] The base's own item, the one a declared action override is
     // judged against: the shipping artifact under that base's envelope.
     const baseItemOf = (basePackageId: string): unknown => this.shippedArtifactsOf(request.type, request.name, undefined, member)
-        .find((a) => overrideWriteMemberOf(request.type, a) === member
+        .find((a) => overrideWriteMemberOf(request.type, a) === member && onItsObject(a)
             && (a as { _packageId?: unknown } | null | undefined)?._packageId === basePackageId);
     return judgeOverrideWrite(request, { dependencies, shippers, baseItemOf });
 }