fix(metadata-protocol, cli, cloud-connection): a declared action override moved to another object is told which object the base ships the action on

.changeset/30623-moved-action-override-refusal.md

@@ -0,0 +1,12 @@
+---
+'@objectstack/metadata-protocol': patch
+'@objectstack/cli': patch
+'@objectstack/cloud-connection': patch
+---
+
+fix(metadata-protocol, cli, cloud-connection): a declared action override moved to another object is told which object the base ships the action on
+
+Clause-②: no
+Metadata-protocol: none
+
+The `declared_package_ships_no_such_item` refusal (`403 NOT_OVERRIDABLE`) for a `.custom` action override whose `objectName` differs from the base action's used to say the base "ships no action named 'X'" and to "correct the name", though the name was right. It now names both objects and tells the author to keep the base action's `objectName`, at all three doors (the save and publish door, `os validate`, the managed install), from one sentence builder in `judgeOverrideWrite`. A name the base does not ship at all reads as before. The rule, code and status are unchanged.

packages/cli/src/utils/override-declarations.test.ts

@@ -145,6 +145,29 @@ describe('#28410 — findOverrideDeclarationRefusals walks the artifact under th
     ]);
   });
 
+  it('[#30623] an action override moved to another object is told both objects and to keep the base action\'s objectName; CONTROL: an unshipped name keeps the name sentence', () => {
+    const overrides = { package: BASE, basedOn: '1.0.0' };
+    const rows = findOverrideDeclarationRefusals({
+      manifest: manifest('com.acme.bundle'),
+      packages: [base, pkg(PAIR, [BASE], {
+        actions: [
+          { name: 'log_call', objectName: 'crm_lead', label: 'Ours', overrides },
+          { name: 'no_such_call', objectName: 'crm_account', label: 'Ours', overrides },
+        ],
+      })],
+    });
+    expect(rows.map((r) => [r.path, r.code, r.rule])).toEqual([
+      ['packages[1].manifest.actions[0]', 'NOT_OVERRIDABLE', 'declared_package_ships_no_such_item'],
+      ['packages[1].manifest.actions[1]', 'NOT_OVERRIDABLE', 'declared_package_ships_no_such_item'],
+    ]);
+    expect(rows[0].message.split('. ')[0]).toBe(
+      `'action/log_call' declares an override of '${BASE}' on object 'crm_lead', but '${BASE}' ships its action 'log_call' on object 'crm_account'`);
+    expect(rows[0].message).toContain("keep the base action's `objectName: 'crm_account'` in the override");
+    expect(rows[1].message.split('. ')[0]).toBe(
+      `'action/no_such_call' declares an override of '${BASE}', which ships no action named 'no_such_call'`);
+    expect(rows[1].message).toContain('or correct the name');
+  });
+
   it('a pair that does not depend on its base is told to declare the dependency', () => {
     const rows = findOverrideDeclarationRefusals({
       manifest: manifest('com.acme.bundle'),

packages/cli/src/utils/override-declarations.ts

@@ -126,9 +126,14 @@ export function findOverrideDeclarationRefusals(parsed: Record<string, unknown>)
       // carries the base: what a declared action override is judged against.
       const baseItemOf = (baseId: string): unknown =>
         byId.get(baseId)?.items.find((i) => `${i.type}\u0000${i.identity}` === key)?.item;
+      // [#30623] The base's items of this type, when this artifact carries
+      // the base: what the ships-no-such-item refusal names an action's
+      // object from.
+      const baseItemsOf = (baseId: string): unknown[] =>
+        (byId.get(baseId)?.items ?? []).filter((i) => i.type === walked.type).map((i) => i.item);
       const verdict = judgeOverrideWrite(
         { type: walked.type, name: walked.name, packageId: pkg.id, item: walked.item },
-        { dependencies, shippers, baseItemOf },
+        { dependencies, shippers, baseItemOf, baseItemsOf },
       );
       if (verdict.verdict !== 'refuse') continue;
       rows.push({ path: walked.path, code: 'NOT_OVERRIDABLE', rule: verdict.rule, packageId: pkg.id, message: verdict.message });

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

@@ -341,3 +341,36 @@ describe('[#30542] the install door judges a declared action override against th
         expect([saved, ledger]).toEqual([[], []]);
     });
 });
+
+/**
+ * [#30623] An `action` is keyed `<objectName>:<name>`: a pair's declared
+ * override of an action its base ships on another object is refused
+ * `declared_package_ships_no_such_item`, and the refusal names both objects
+ * and the base action's `objectName` to keep, in the judge's one sentence.
+ */
+describe('[#30623] the install door names the object a moved action override left', () => {
+    const overrides = { package: BASE, basedOn: '1.0.0' };
+    const holdings: Holdings = { [BASE]: { action: [{ name: 'crm_close', objectName: 'crm_account', label: 'Close' }] } };
+    const pairOf = (action: Record<string, unknown>) =>
+        ({ id: PAIR, version: '1.0.0', namespace: 'crm_custom', dependencies: deps(BASE), actions: [action] });
+    const messageOf = (res: any) => res.payload?.error?.details?.refusals?.[0]?.message as string;
+
+    it('an override of crm_account:crm_close declared on crm_lead names both objects and keeps objectName: crm_account', async () => {
+        const { res, saved, ledger } = await install(pairOf({ name: 'crm_close', objectName: 'crm_lead', label: 'Ours', overrides }), [{ id: BASE }], holdings);
+        expect(res.status, JSON.stringify(res.payload)).toBe(403);
+        expect(res.payload?.error?.code).toBe('NOT_OVERRIDABLE');
+        expect(refusedOf(res)).toEqual([['action', 'crm_close', 'declared_package_ships_no_such_item']]);
+        expect(messageOf(res).split('. ')[0]).toBe(
+            `'action/crm_close' declares an override of '${BASE}' on object 'crm_lead', but '${BASE}' ships its action 'crm_close' on object 'crm_account'`);
+        expect(messageOf(res)).toContain("keep the base action's `objectName: 'crm_account'` in the override");
+        expect([saved, ledger]).toEqual([[], []]);
+    });
+
+    it('CONTROL: a name the base does not ship at all keeps the name sentence', async () => {
+        const { res } = await install(pairOf({ name: 'crm_reopen', objectName: 'crm_account', label: 'Ours', overrides }), [{ id: BASE }], holdings);
+        expect(res.status, JSON.stringify(res.payload)).toBe(403);
+        expect(refusedOf(res)).toEqual([['action', 'crm_reopen', 'declared_package_ships_no_such_item']]);
+        expect(messageOf(res).split('. ')[0]).toBe(`'action/crm_reopen' declares an override of '${BASE}', which ships no action named 'crm_reopen'`);
+        expect(messageOf(res)).toContain('or correct the name');
+    });
+});

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

@@ -3717,16 +3717,21 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
                 // [#30065] …and each shipper's own item, what a declared
                 // action override is judged against.
                 const shipped = new Map<string, unknown>();
+                // [#30623] …and each base's items of this type, what the
+                // ships-no-such-item refusal names an action's object from.
+                const listed = new Map<string, unknown[]>();
                 for (const base of bases) {
-                    const held = (await heldBy(item.type, base)).get(item.identity);
+                    const byIdentity = await heldBy(item.type, base);
+                    listed.set(base, [...byIdentity.values()]);
+                    const held = byIdentity.get(item.identity);
                     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 },
-                    { dependencies: closure, shippers, baseItemOf: (base) => shipped.get(base) },
+                    { dependencies: closure, shippers, baseItemOf: (base) => shipped.get(base), baseItemsOf: (base) => listed.get(base) ?? [] },
                 );
                 if (verdict.verdict === 'refuse') {
                     refusals.push({

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

@@ -816,6 +816,17 @@ export interface OverrideWriteClosure {
      * holds it.
      */
     readonly baseItemOf?: (basePackageId: string) => unknown;
+    /**
+     * [#30623] The items of this type a base ships, at any object — read by
+     * the door that holds them (the protocol's artifact lookup, the artifact
+     * `os validate` walks, the install door's listing). The gate reads it for
+     * one sentence only: when a declared `action` override names a base that
+     * ships no action of that name on the override's object, the
+     * `declared_package_ships_no_such_item` refusal names the object(s) the
+     * base does ship it on ({@link shipsNoSuchItemMessage}). Absent, or
+     * answering none of that name, the refusal reads as before.
+     */
+    readonly baseItemsOf?: (basePackageId: string) => readonly unknown[];
 }
 
 /** [#28834] The item door an `object_extension` is saved through (#28574), as the refusals name it. */
@@ -944,6 +955,49 @@ function excludedTypeRemedy(type: string, base: string | undefined): string {
     return `Remove \`overrides\`: ${articleFor(type)} ${type} is defined only in package source, so it changes only with a new version of ${shipper}.`;
 }
 
+/**
+ * [#30623] The `declared_package_ships_no_such_item` sentence, the one every
+ * door renders (they all ask {@link judgeOverrideWrite}). An `action` is keyed
+ * `<objectName>:<name>`, so when `base` ships an action of that name on
+ * ANOTHER object (`baseItems`, {@link OverrideWriteClosure.baseItemsOf}) the
+ * name is right and the object is what is wrong: the sentence names both
+ * objects and tells the author to keep the base action's `objectName`.
+ * Otherwise — another type, no such name at all, or no items in hand — it
+ * says the base ships no such item and to correct the name.
+ */
+function shipsNoSuchItemMessage(
+    type: string,
+    name: string,
+    base: string,
+    packageId: string | undefined,
+    overrideItem: unknown,
+    baseItems: readonly unknown[] | undefined,
+): string {
+    const item = `'${type}/${name}'`;
+    if (type === 'action') {
+        const bare = (n: unknown): unknown => (typeof n === 'string' ? n.slice(n.lastIndexOf(':') + 1) : n);
+        const object = overrideWriteActionObjectOf(overrideItem);
+        const others = Array.from(new Set((baseItems ?? [])
+            .filter((a) => isPlainRecord(a) && bare(a.name) === bare(name))
+            .map(overrideWriteActionObjectOf)
+            .filter((o) => o !== object))).sort();
+        if (others.length > 0) {
+            const objectPhrase = (o: string): string => (o === '' ? 'no object' : `object '${o}'`);
+            const fix = others.length === 1 && others[0] !== ''
+                ? `keep the base action's \`objectName: '${others[0]}'\` in the override`
+                : 'keep the `objectName` of the base action it replaces';
+            return `${item} declares an override of '${base}' on ${objectPhrase(object)}, but '${base}' ships its action `
+                + `'${bare(name)}' on ${others.map(objectPhrase).join(' and ')}. An override replaces the base's action on that `
+                + `action's own object: ${fix}, or remove \`overrides\` to keep it as '${packageId}''s own item (ADR-0146 D2).`;
+        }
+    }
+    // [#28746] A pair-identified type names the member it addressed.
+    const member = overrideWriteMemberOf(type, overrideItem);
+    const ofMember = member === undefined ? '' : ` with ${ITEM_KEY_DISCRIMINATORS[type]?.field} '${member}'`;
+    return `${item} declares an override of '${base}', which ships no ${type} named '${name}'${ofMember}. `
+        + `Remove \`overrides\` to keep it as '${packageId}''s own item, or correct the name (ADR-0146 D2).`;
+}
+
 /** The indefinite article for a type name, by its first letter: "an api", "a permission". */
 function articleFor(type: string): 'a' | 'an' {
     return /^[aeiou]/.test(type) ? 'an' : 'a';
@@ -1034,12 +1088,8 @@ export function judgeOverrideWrite(
                 + `Declare a dependency on '${base}' in the \`dependencies\` of '${packageId}''s manifest, or remove \`overrides\` (ADR-0146 D2).`);
         }
         if (!closure.shippers.includes(base)) {
-            // [#28746] A pair-identified type names the member it addressed.
-            const member = overrideWriteMemberOf(type, subject.item);
-            const ofMember = member === undefined ? '' : ` with ${ITEM_KEY_DISCRIMINATORS[type]?.field} '${member}'`;
             return refuse('declared_package_ships_no_such_item',
-                `${item} declares an override of '${base}', which ships no ${type} named '${name}'${ofMember}. `
-                + `Remove \`overrides\` to keep it as '${packageId}''s own item, or correct the name (ADR-0146 D2).`);
+                shipsNoSuchItemMessage(type, name, base, packageId, subject.item, closure.baseItemsOf?.(base)));
         }
         // [#30065] A declared action override replaces the whole action, so
         // it could drop the gates that decide who may run it — the base's
@@ -1208,7 +1258,12 @@ export function overrideWriteVerdict(
     const baseItemOf = (basePackageId: string): unknown => this.shippedArtifactsOf(request.type, request.name, undefined, member)
         .find((a) => overrideWriteMemberOf(request.type, a) === member && onItsObject(a)
             && (a as { _packageId?: unknown } | null | undefined)?._packageId === basePackageId);
-    return judgeOverrideWrite(request, { dependencies, shippers, baseItemOf });
+    // [#30623] The base's items of this name at every object: what the
+    // ships-no-such-item refusal names when the override moved the action.
+    const baseItemsOf = (basePackageId: string): unknown[] => this.shippedArtifactsOf(request.type, request.name, undefined, member)
+        .filter((a) => overrideWriteMemberOf(request.type, a) === member
+            && (a as { _packageId?: unknown } | null | undefined)?._packageId === basePackageId);
+    return judgeOverrideWrite(request, { dependencies, shippers, baseItemOf, baseItemsOf });
 }
 
 

packages/objectql/src/protocol-flow-custom-override.test.ts

@@ -524,6 +524,29 @@ describe('[#30065] a declared .custom action override is the packaged action, at
             .toMatchObject({ verdict: 'admit' });
     });
 
+    it('[#30623] the moved override\'s refusal names both objects and tells the author to keep the base action\'s objectName', async () => {
+        const { protocol } = await boot();
+        await install(protocol, PAIR);
+        const moved = actionBody('MOVED', { ...declared, objectName: 'showcase_other' });
+        const refusal = await protocol.saveMetaItem({
+            type: 'action', name: ACTION, item: moved, mode: 'draft', packageId: PAIR,
+        } as any).then(() => undefined, (e: any) => e);
+        expect(refusal).toMatchObject({ code: 'NOT_OVERRIDABLE', status: 403 });
+        expect(refusal.message).toBe(
+            `'action/${ACTION}' declares an override of '${BASE}' on object 'showcase_other', but '${BASE}' ships its action `
+            + `'${ACTION}' on object 'showcase_task'. An override replaces the base's action on that action's own object: `
+            + `keep the base action's \`objectName: 'showcase_task'\` in the override, or remove \`overrides\` to keep it as `
+            + `'${PAIR}''s own item (ADR-0146 D2).`);
+        // CONTROL: a name the base does not ship at all keeps the name sentence.
+        const unknown = { ...actionBody('NONE', declared), name: 'showcase_task_nope' };
+        expect((protocol as any).overrideWriteVerdict({ type: 'action', name: 'showcase_task_nope', packageId: PAIR, item: unknown }))
+            .toEqual({
+                verdict: 'refuse', rule: 'declared_package_ships_no_such_item',
+                message: `'action/showcase_task_nope' declares an override of '${BASE}', which ships no action named 'showcase_task_nope'. `
+                    + `Remove \`overrides\` to keep it as '${PAIR}''s own item, or correct the name (ADR-0146 D2).`,
+            });
+    });
+
     it('the pair\'s undeclared save is refused, and the remedy names the declaration, never a clone', async () => {
         const { protocol } = await boot();
         await install(protocol, PAIR);