fix(metadata-protocol): make the destructive-change remedy clause face-aware — stop prescribing ?force=true on the duplicate door (#11015)

.changeset/duplicate-destructive-remedy-face-aware.md

@@ -0,0 +1,53 @@
+---
+"@objectstack/metadata-protocol": patch
+---
+
+fix(metadata-protocol): stop prescribing `?force=true` on the duplicate door, which accepts no `force` (#11015)
+
+`saveMetaItem`'s Phase 3a-destructive refusal ended every message with
+`— re-submit with ?force=true to proceed.` The refusal is raised in one place
+and quoted onto whatever response the caller's catch builds, so that one
+sentence went out on every face that reaches the gate — including
+`POST /packages/:id/duplicate`, which has no `force` to set.
+
+Measured: the duplicate route accepts `targetPackageId`, `targetName`,
+`targetNamespace`, `organizationId` and `actor` — no `force` in the query
+string or the body — and `duplicatePackage`'s own request type has no `force`
+field either, so its internal `saveMetaItem` call cannot carry one. The gate is
+reached on the ordinary duplicate-**again** workflow, where the target
+namespace already holds the renamed object from an earlier duplicate; the copy
+is refused and the refusal is reported as data on a `200`:
+
+```
+"error": "[destructive_change] object/crm2_task would drop or transform existing
+          data: Field 'b' removed — … — re-submit with ?force=true to proceed."
+```
+
+A caller who does what that sentence says gets the identical refusal back. The
+remedies that do exist on that face — duplicate into a target namespace that is
+free, or reconcile the colliding object first — were never stated.
+
+The clause is now rendered per face. The duplicate door says:
+
+```
+… — this copy cannot be forced: the duplicate door accepts no `force`.
+Duplicate into a target namespace that does not already hold 'crm2_task', or
+reconcile that item with the source first.
+```
+
+Three narrowings, each pinned:
+
+- **The clause is repaired, not the door.** No `force` parameter is added to
+  `POST /packages/:id/duplicate`; that would widen a public surface and is a
+  contract decision, not a message fix. Which face is being served is stated by
+  the server on the internal call, exactly as `source` already is — a caller
+  cannot smuggle one in.
+- **Nothing else in the message moved.** #10886 measured that
+  `duplicatePackage`'s `failed[].error` is the sole carrier of the per-field
+  destructive findings, so the findings prose stays verbatim. Only the trailing
+  remedy sentence is face-dependent.
+- **No accept/reject behaviour changed.** The copy is still refused, still
+  reported as `failed[]` data on the `200`, still counted. Faces that state no
+  door — the single-segment REST `PUT /api/v1/meta/:type/:name`, where
+  `?force=true` is a real query parameter the route threads — keep the previous
+  wording byte for byte.

packages/metadata-protocol/src/protocol.destructive-409-face-inventory.test.ts

@@ -81,6 +81,23 @@
  * the ablation therefore needs no rebuild, and its RED result is what rules
  * out the stale-artifact false green.
  *
+ * ## [#11015] The same inventory, read one column further left
+ *
+ * The `force` column above is not decoration: it says which faces can lift
+ * this refusal, and only ROW 1 can. Rows 2, 3 and 6 all reach the gate with no
+ * way to set `force` — rows 2 and 3 because their routes never thread the
+ * parameter, row 6 because `duplicatePackage` has no `force` field at all —
+ * yet every one of them used to be handed the sentence `re-submit with
+ * ?force=true to proceed.` A caller who does what it says gets the identical
+ * refusal back.
+ *
+ * #11015 repairs the clause on ROW 6, where a genuinely different remedy
+ * exists to prescribe (a free target namespace, or reconciling the collision).
+ * Rows 2 and 3 are left as measured and filed as #11095: the honest repair for
+ * a `PUT` that cannot acknowledge a risk may be to thread `force` on those
+ * routes, which is a contract decision and not a message fix. Section 4 pins
+ * row 6; section 1's remedy guard pins that row 1's wording is untouched.
+ *
  * ⛔ Never a bare `toThrow()` here. `duplicatePackage` does not throw, it
  * REPORTS, and what the report says IS the defect; and for the throw itself
  * the minimum assertion is `code` + `status` (ADR-0112 envelope), with the
@@ -206,8 +223,19 @@ async function destructiveRefusal(): Promise<any> {
     throw new Error('expected saveMetaItem to refuse the destructive change');
 }
 
-/** The remedy sentence that must survive ANY future trim (#10886 non-effect). */
-const REMEDY = 're-submit with ?force=true to proceed.';
+/**
+ * The remedy sentence that must survive ANY future trim (#10886 non-effect),
+ * as the ordinary REST `PUT` door renders it. `?force=true` is a real query
+ * parameter THERE — the route reads it and threads it into the request.
+ */
+const PUT_REMEDY = 're-submit with ?force=true to proceed.';
+/**
+ * [#11015] …and as the DUPLICATE door renders it, which is a different
+ * sentence because `?force=true` is not a thing a caller can set on that face.
+ * See section 4 — the remedy stays, the mechanism it names becomes one that
+ * exists.
+ */
+const DUPLICATE_REMEDY_HEAD = 'this copy cannot be forced';
 /** One finding's prose, as `detectDestructiveObjectChanges` words it. */
 const FINDING_PROSE = "Field 'b' removed — existing data in this column will become inaccessible.";
 
@@ -243,7 +271,10 @@ describe('[#10886] the 409 renders its findings into the message AND attaches th
         // refusal, this is a risk-ACKNOWLEDGEMENT flow: the remedy is the
         // whole point, it is not one of the `issues`, and nothing else on any
         // face carries it.
-        expect(err.message).toContain(REMEDY);
+        // No `writeFace` on this request — the ordinary REST/Studio save, the
+        // one door where `?force=true` is real. [#11015] made this clause
+        // face-aware; this default is byte-identical to what it always said.
+        expect(err.message).toContain(PUT_REMEDY);
         const wire = JSON.stringify(err.issues);
         expect(wire).not.toContain('force=true');
     });
@@ -317,18 +348,119 @@ describe('[#10886] [GUARD] `duplicatePackage`’s `failed[].error` is the SOLE c
         expect(entry.issues).toBeUndefined();
     });
 
-    it('⛔ carries the `?force=true` remedy, on a response with no other channel for it', async () => {
+    it('⛔ carries the remedy, on a response with no other channel for it', async () => {
         const { protocol } = duplicateIntoOccupiedNamespace();
 
         const r = await protocol.duplicatePackage({
             sourcePackageId: PKG, targetPackageId: TARGET_PKG,
         });
 
-        expect(r.failed[0].error).toContain(REMEDY);
+        // ⚠️ [#11015] This assertion USED to read `toContain(REMEDY)` with
+        // REMEDY = the `?force=true` sentence, and it passed — because the
+        // producer rendered that sentence on every face. It was pinning the
+        // defect: this door accepts no `force`, so the prescription it quoted
+        // was unactionable. Replaced rather than re-spelled, because what it
+        // asserted stopped being true of a correct producer. What #10886 put
+        // it here to protect is unchanged and still asserted: SOME remedy
+        // reaches the caller through this string and through nothing else.
+        expect(r.failed[0].error).toContain(DUPLICATE_REMEDY_HEAD);
         // The whole response, not just the entry: nothing anywhere else on it
         // states the remedy or the findings.
         const wire = JSON.stringify({ ...r, failed: r.failed.map((f: any) => ({ ...f, error: '' })) });
-        expect(wire).not.toContain('force=true');
+        expect(wire).not.toContain('cannot be forced');
         expect(wire).not.toContain('inaccessible');
     });
 });
+
+// ═══════════════════════════════════════════════════════════════════════════
+// 4. [#11015] [GUARD] The remedy names a mechanism THIS face actually has
+// ═══════════════════════════════════════════════════════════════════════════
+
+describe('[#11015] [GUARD] the destructive remedy clause is face-aware', () => {
+    /** Same reachability fixture as section 3 — the duplicate-AGAIN workflow. */
+    const duplicateIntoOccupiedNamespace = () => makeKernel({
+        seed: [
+            objectRow('crm_task', ['a']),
+            objectRow('crm2_task', ['a', 'b', 'c', 'd'], TARGET_PKG),
+        ],
+    });
+
+    const duplicateFailure = async (extra: Record<string, unknown> = {}) => {
+        const { protocol } = duplicateIntoOccupiedNamespace();
+        const r = await protocol.duplicatePackage({
+            sourcePackageId: PKG, targetPackageId: TARGET_PKG, ...extra,
+        });
+        return r;
+    };
+
+    it('⛔ the duplicate face never prescribes `force` — the door accepts none', async () => {
+        const r = await duplicateFailure();
+
+        // The defect, stated as the assertion that would have failed before
+        // the fix. Not `not.toContain(PUT_REMEDY)` alone: the substring that
+        // must be gone is the MECHANISM NAME, because a caller reading it goes
+        // looking for a parameter that does not exist on this door.
+        expect(r.failed[0].error).not.toContain('force=true');
+        expect(r.failed[0].error).not.toContain(PUT_REMEDY);
+    });
+
+    it('prescribes the remedies that DO exist on this face, and names the collision', async () => {
+        const r = await duplicateFailure();
+        const error: string = r.failed[0].error;
+
+        // Both real remedies, in the caller's own vocabulary — `targetNamespace`
+        // is a parameter this door genuinely accepts.
+        expect(error).toContain('target namespace');
+        expect(error).toContain('reconcile');
+        // …and WHICH item collides, which is the copy's re-namespaced name
+        // (`crm_task` → `crm2_task`), not the source row's.
+        expect(error).toContain('crm2_task');
+    });
+
+    it('[#10886 non-effect] the per-field findings prose is still there, untrimmed', async () => {
+        const r = await duplicateFailure();
+
+        // ⛔ This card repaired the remedy clause ONLY. #10886's verdict — the
+        // findings prose stays, because `failed[].error` is its sole carrier on
+        // this face — is untouched, and this is the assertion that says so.
+        expect(r.failed[0].error).toContain(FINDING_PROSE);
+        expect(r.failed[0].error).toContain('[destructive_change]');
+    });
+
+    it('the refusal still REFUSES — this is a message repair, not a behaviour one', async () => {
+        const r = await duplicateFailure();
+
+        // Clause-② line: no accept/reject behaviour moved. The copy is still
+        // rejected, still reported as data on the 200, still counted.
+        expect(r.success).toBe(false);
+        expect(r.copiedCount).toBe(0);
+        expect(r.failedCount).toBe(1);
+        expect(r.copied).toEqual([]);
+    });
+
+    it('⛔ the face is stated by the SERVER — a caller cannot smuggle one in', async () => {
+        // The duplicate route builds `duplicatePackage`'s request field by
+        // field and this method hard-codes the face on its internal
+        // `saveMetaItem` call, so neither a `force` nor a `writeFace` on the
+        // caller's request can reach the gate. Asserted from the OUTSIDE
+        // rather than by reading the type, because the type is what a future
+        // edit would widen: if adding `force` to this door ever becomes the
+        // decision, this test is the one that has to be rewritten deliberately
+        // instead of quietly starting to pass.
+        const smuggled = await duplicateFailure({ force: true, writeFace: undefined });
+
+        expect(smuggled.failedCount).toBe(1);
+        expect(smuggled.failed[0].error).toContain(DUPLICATE_REMEDY_HEAD);
+        expect(smuggled.failed[0].error).not.toContain('force=true');
+    });
+
+    it('the OTHER faces keep the `?force=true` wording — a switch, not a global delete', async () => {
+        // Row 1 of the inventory, driven at the producer with no face stated.
+        const err = await destructiveRefusal();
+
+        expect(err.code).toBe('DESTRUCTIVE_CHANGE');
+        expect(err.status).toBe(409);
+        expect(err.message).toContain(PUT_REMEDY);
+        expect(err.message).not.toContain(DUPLICATE_REMEDY_HEAD);
+    });
+});

packages/metadata-protocol/src/protocol.ts

@@ -3202,6 +3202,64 @@ function detectDestructiveObjectChanges(prev: any, next: any): Array<{
     return issues;
 }
 
+/**
+ * [#11015] The remedy clause the Phase 3a-destructive refusal ends with — one
+ * sentence per FACE, because the mechanism that lifts the refusal is not the
+ * same on every door that raises it.
+ *
+ * The refusal is raised in ONE place ({@link ObjectStackProtocolImplementation.saveMetaItem}'s
+ * Phase 3a-destructive gate) and is quoted onto whatever response the caller's
+ * catch builds, so the clause used to read `re-submit with ?force=true to
+ * proceed.` on every face. That is correct prose for exactly one of them —
+ * `PUT /api/v1/meta/:type/:name`, whose route reads `?force` and threads it
+ * into the request — and a caller on any OTHER face who does what the sentence
+ * says gets the identical refusal back.
+ *
+ * MEASURED on the face this card was filed on: `POST /packages/:id/duplicate`
+ * accepts `targetPackageId`, `targetName`, `targetNamespace`, `organizationId`
+ * and `actor` — no `force`, in the query string or the body — and
+ * {@link ObjectStackProtocolImplementation.duplicatePackage}'s own request type
+ * has no `force` field either, so its internal `saveMetaItem` call cannot carry
+ * one. There is nothing on that door for the caller to set.
+ *
+ * ⛔ What is repaired is the CLAUSE, not the door. Giving the duplicate route a
+ * `force` would widen a public surface and is a contract decision, deliberately
+ * NOT taken here. Nor is the clause deleted on that face: #10886 measured that
+ * `duplicatePackage`'s `failed[].error` is the SOLE carrier of this
+ * prescription, so deleting the remedy there deletes it from the wire outright.
+ * Each face therefore states the remedy it actually has.
+ *
+ * ⚠️ An absent `face` renders the `?force=true` wording, byte-identical to what
+ * every face carried before. That default is right on the single-segment REST
+ * `PUT` — but `protocol.destructive-409-face-inventory.test.ts` inventories two
+ * further doors that reach this gate and never thread `force` either
+ * (`@objectstack/rest`'s compound-name `PUT /meta/:type/:a/:b`, and
+ * `@objectstack/runtime`'s dispatcher `PUT /meta`). Those are wrong for the
+ * same reason and are deliberately NOT repaired here: unlike the duplicate
+ * gesture, which has a genuine collision-free alternative to prescribe, the
+ * right repair for a `PUT` that cannot acknowledge a risk may well be to thread
+ * `force` on those routes — a contract question, filed as #11095 rather than
+ * guessed at. Adding a face value here is one of the two candidate repairs it
+ * weighs.
+ */
+function destructiveChangeRemedy(
+    face: 'package-duplicate' | undefined,
+    name: string,
+): string {
+    switch (face) {
+        case 'package-duplicate':
+            // The duplicate-AGAIN workflow: the target namespace already holds
+            // the renamed item this copy is about to overwrite. Both remedies
+            // are things the caller can actually do on THIS door — choose a
+            // free target namespace, or make the collision non-destructive.
+            return `this copy cannot be forced: the duplicate door accepts no \`force\`. `
+                + `Duplicate into a target namespace that does not already hold '${name}', `
+                + `or reconcile that item with the source first.`;
+        default:
+            return 're-submit with ?force=true to proceed.';
+    }
+}
+
 /**
  * Result of projecting a published metadata body into its data-plane
  * representation. `success:false` with an `error` is the surfaced-not-thrown
@@ -12847,7 +12905,7 @@ export class ObjectStackProtocolImplementation implements
         }
     }
 
-    async saveMetaItem(request: { type: string, name: string, item?: any, organizationId?: string, parentVersion?: string | null, actor?: string, force?: boolean, mode?: 'draft' | 'publish', packageId?: string | null, source?: string }) {
+    async saveMetaItem(request: { type: string, name: string, item?: any, organizationId?: string, parentVersion?: string | null, actor?: string, force?: boolean, mode?: 'draft' | 'publish', packageId?: string | null, source?: string, writeFace?: 'package-duplicate' }) {
         // [#8818] The ADR-0112 envelope this refusal always owed. Every OTHER
         // refusal in this method declares `code` AND `status`
         // (`NOT_OVERRIDABLE`/403, `NOT_CREATABLE`/403, `ITEM_LOCKED`/403,
@@ -13198,16 +13256,22 @@ export class ObjectStackProtocolImplementation implements
                         // `protocol.destructive-409-face-inventory.test.ts`
                         // carries the whole inventory and pins this face.
                         //
-                        // ⛔ Whatever else a future trim does, the `?force=true`
-                        // remedy below must survive it: this is a
+                        // ⛔ Whatever else a future trim does, the remedy
+                        // clause below must survive it: this is a
                         // risk-acknowledgement refusal, not a validation one,
                         // and no structured channel on any face carries the
                         // remedy.
+                        //
+                        // [#11015] Which remedy that IS depends on the face —
+                        // `?force=true` names a query parameter only the
+                        // single-segment REST `PUT` reads, and prescribing it
+                        // to a caller who has no way to set it sends them in a
+                        // circle. See {@link destructiveChangeRemedy}.
                         const summary = issues.slice(0, 3).map((i) => i.message).join('; ');
                         const err = new Error(
                             `[destructive_change] ${request.type}/${request.name} would drop or transform existing data: ${summary}`
                             + (issues.length > 3 ? ` (+${issues.length - 3} more)` : '')
-                            + ` — re-submit with ?force=true to proceed.`
+                            + ` — ${destructiveChangeRemedy(request.writeFace, request.name)}`
                         );
                         (err as any).code = 'DESTRUCTIVE_CHANGE';
                         (err as any).status = 409;
@@ -16847,6 +16911,14 @@ export class ObjectStackProtocolImplementation implements
                     item: rewritten,
                     mode: 'publish',
                     packageId: request.targetPackageId,
+                    // [#11015] Which door the refusal below will be prescribing
+                    // a remedy FOR. Stated by the server, never by the caller —
+                    // `duplicatePackage`'s own request type has no such field,
+                    // exactly as `source` is server-stated one gate down. The
+                    // Phase 3a-destructive gate reaches this call on the
+                    // duplicate-AGAIN workflow, and its `?force=true` default
+                    // would name a parameter this door does not accept.
+                    writeFace: 'package-duplicate',
                     ...(copyOrgId ? { organizationId: copyOrgId } : {}),
                     ...(request.actor ? { actor: request.actor } : {}),
                 });