From 902414619e47b542d71a782200c4b95068b901eb Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Sat, 22 Aug 2026 16:34:52 +0000 Subject: [PATCH 1/2] fix(metadata-protocol): make the destructive-change remedy clause face-aware (#11015) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 sentence went out on every face — including `POST /packages/:id/duplicate`, which accepts no `force` in the query string or the body, and whose `duplicatePackage` request type has no `force` field for the internal `saveMetaItem` call to carry. The clause is now rendered per face. `duplicatePackage` states the face on its internal call — server-stated, exactly as `source` already is — and gets the remedies that exist there: a free target namespace, or reconciling the collision first. Faces that state no door keep the previous wording byte for byte. Repairs the CLAUSE, not the door: no `force` is added to the duplicate route. #10886's verdict is untouched — the per-field findings prose stays, because `failed[].error` is its sole carrier on that face. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y --- ...duplicate-destructive-remedy-face-aware.md | 53 +++++++ ...col.destructive-409-face-inventory.test.ts | 144 +++++++++++++++++- packages/metadata-protocol/src/protocol.ts | 78 +++++++++- 3 files changed, 265 insertions(+), 10 deletions(-) create mode 100644 .changeset/duplicate-destructive-remedy-face-aware.md diff --git a/.changeset/duplicate-destructive-remedy-face-aware.md b/.changeset/duplicate-destructive-remedy-face-aware.md new file mode 100644 index 0000000000..e529bb1d32 --- /dev/null +++ b/.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. diff --git a/packages/metadata-protocol/src/protocol.destructive-409-face-inventory.test.ts b/packages/metadata-protocol/src/protocol.destructive-409-face-inventory.test.ts index 4aa6f8b69f..0bdd2f9f36 100644 --- a/packages/metadata-protocol/src/protocol.destructive-409-face-inventory.test.ts +++ b/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 separately: 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 { 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 = {}) => { + 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); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index d1a54eca01..49d219d95f 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -3202,6 +3202,62 @@ 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 rather than guessed at. + */ +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 +12903,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 +13254,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 +16909,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 } : {}), }); From c8b63f34f9604785764f7865bc7d340d8d723dac Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Sat, 22 Aug 2026 16:47:20 +0000 Subject: [PATCH 2/2] docs(metadata-protocol): name #11095 as the card holding the two unrepaired faces (#11015) The docblock and the face-inventory header both recorded that inventory rows 2 and 3 carry the same wrong remedy and were knowingly left. Point them at the issue that now holds that measurement. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y --- .../src/protocol.destructive-409-face-inventory.test.ts | 4 ++-- packages/metadata-protocol/src/protocol.ts | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.destructive-409-face-inventory.test.ts b/packages/metadata-protocol/src/protocol.destructive-409-face-inventory.test.ts index 0bdd2f9f36..4871dc8fe3 100644 --- a/packages/metadata-protocol/src/protocol.destructive-409-face-inventory.test.ts +++ b/packages/metadata-protocol/src/protocol.destructive-409-face-inventory.test.ts @@ -93,8 +93,8 @@ * * #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 separately: the honest repair - * for a `PUT` that cannot acknowledge a risk may be to thread `force` on those + * 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. * diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 49d219d95f..f30411eeb4 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -3238,7 +3238,9 @@ function detectDestructiveObjectChanges(prev: any, next: any): Array<{ * 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 rather than guessed at. + * `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,