Skip to content

Commit 809e612

Browse files
Elon Muskclaude
andauthored
test(metadata-protocol): inventory the DESTRUCTIVE_CHANGE 409's faces and pin the sole carrier (#11016)
#10886. `saveMetaItem`'s Phase 3a-destructive 409 renders its findings into the message AND attaches the same array as `err.issues`, so a console that renders both channels shows every finding twice — the render-then-attach shape #10524 trimmed on the publish refusals. The card's own first step is the face inventory, and the inventory says DO NOT TRIM. Enumerated from every caller of `saveMetaItem` in the repo, then filtered by the gate's own predicate (`!force`, folded type `object`/`field`, an existing item under the target name, a non-empty diff), four of the seven callers cannot reach the gate at all — three pass a literal non-object `type`, one passes `force: true`. Of the three that can: - the two `@objectstack/rest` `PUT /meta` doors and the `@objectstack/runtime` dispatcher door all resolve through `resolveThrownHttpError`, so `issues` reaches the wire structurally (a top-level `issues` on the REST body, `details.issues` on the dispatcher envelope) — a message trim would lose nothing there; - `duplicatePackage`'s `failed[].error` is a SOLE CARRIER. It reports per-item failures as DATA on a 200 (`POST /packages/:id/duplicate`), so no HTTP boundary is involved and `details.issues` never exists; the array is typed inline as `{ type, name, error }`; and unlike `publishPackageDrafts` — whose `failed[]` #10895 could extend because it has a response schema — `duplicatePackage` has none in `packages/spec` at all. So the declare half of declare-then-trim is not done for this refusal, and the trim is refused. Declaring a channel on that face is a `packages/spec` change and is deliberately out of this card's scope; reported to the PM instead. Reaching that face was measured, not argued, and the obvious attempt misleads: a duplicate re-namespaces objects, so the target name usually does not exist, the gate is skipped, and the copy fails the author-time gate instead. The gate fires on the duplicate-again workflow, where the target namespace already holds the renamed object. No behaviour change: one new pin file plus a comment recording the verdict at the throw site, so the next author is told which measurement they are standing on. Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5359a9b commit 809e612

2 files changed

Lines changed: 371 additions & 0 deletions

File tree

Lines changed: 334 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,334 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* #10886 — the face inventory for `saveMetaItem`'s Phase 3a-destructive
5+
* `409 DESTRUCTIVE_CHANGE`, and the pins that hold its conclusion.
6+
*
7+
* ## The duplication that raised the card
8+
*
9+
* The refusal renders its own findings into the message
10+
* (`issues.slice(0, 3).map((i) => i.message).join('; ')` plus a `(+N more)`
11+
* tail) AND attaches the same array as `err.issues`. A console that renders
12+
* both channels shows every finding twice — the render-then-attach shape
13+
* #10524 trimmed on the publish refusals.
14+
*
15+
* ## The conclusion: DO NOT TRIM. The message is a SOLE CARRIER.
16+
*
17+
* #10524 established the order — **declare a structured channel on every face
18+
* that quotes the message, and only then trim the message**. This file is the
19+
* measurement that says the first half is not done here, so the second half
20+
* must not happen. It is the same verdict, reached the same way, as the
21+
* sibling `INVALID_METADATA` message one gate down (which was trial-trimmed
22+
* during #10524 and REVERTED).
23+
*
24+
* ## The inventory, and how it was enumerated
25+
*
26+
* The 409 is raised in ONE place ({@link ObjectStackProtocolImplementation.saveMetaItem},
27+
* Phase 3a-destructive). A *face* is therefore any place a caller's catch puts
28+
* the thrown value's `.message` onto a response. So the enumeration is: every
29+
* caller of `saveMetaItem` in the repo, then — for each — can it reach the
30+
* gate at all, and if so what does its catch emit.
31+
*
32+
* The gate fires only when ALL of: `!request.force`, the folded type is
33+
* `object` or `field`, an item already exists under the target name, and the
34+
* diff is non-empty. That predicate is what eliminates four of the seven.
35+
*
36+
* | # | caller | type | `force` | reaches gate | face | `issues` structurally |
37+
* |:--|:--|:--|:--|:--|:--|:--|
38+
* | 1 | `@objectstack/rest` `PUT /meta/:type/:name` | any | `?force` | **yes** | `handleRouteError` 409 body | **yes** — top-level `issues` |
39+
* | 2 | `@objectstack/rest` `PUT /meta/:type/:a/:b` | any | never | **yes** | the same `handleRouteError` body | **yes** (same face as #1) |
40+
* | 3 | `@objectstack/runtime` dispatcher `PUT /meta` | any | never | **yes** | `errorFromThrown` → `details.issues` | **yes** |
41+
* | 4 | `@objectstack/runtime` ADR-0045 visibility flip | `'app'` | no | no — type | (`unhideError`) | n/a |
42+
* | 5 | `migrateStoredMetadata` (this file's protocol) | any | **true** | no — `force` | (`rows[].reason`) | n/a |
43+
* | 6 | {@link ObjectStackProtocolImplementation.duplicatePackage} | `row.type` incl. `object` | no | **yes** | `failed[].error` on a **200** | ⛔ **NO — sole carrier** |
44+
* | 7 | `plugin-security` permission-set projection ×4 | `'permission'` | no | no — type | n/a | n/a |
45+
*
46+
* Rows 1-3 and 6 are pinned below. Rows 4, 5 and 7 are eliminated by a
47+
* constant in the call itself (a literal `type`, or `force: true`), which is
48+
* why they are argued rather than pinned: there is no runtime state that could
49+
* make them reach the gate.
50+
*
51+
* ## Why row 6 is the one that forbids the trim
52+
*
53+
* `duplicatePackage` reports a per-item failure as **response DATA on a 200**
54+
* (`POST /packages/:id/duplicate`), so no HTTP boundary is involved and
55+
* `details.issues` never exists. And unlike `publishPackageDrafts` — whose
56+
* `failed[]` #10895 could extend because `PublishPackageDraftsResponseSchema`
57+
* exists — `duplicatePackage` has **no response schema in `packages/spec` at
58+
* all**; its `failed[]` is typed inline as
59+
* `Array<{ type: string; name: string; error: string }>` and the push adds no
60+
* `issues` key. Declaring a structured channel there is a `packages/spec`
61+
* change and is deliberately NOT part of this card.
62+
*
63+
* ⚠️ Row 6's reachability was MEASURED, not argued, and the obvious first
64+
* attempt says the wrong thing: a plain duplicate re-namespaces every object
65+
* (`com.acme.crm` → `com.acme.crm2` maps `crm_task` → `crm2_task`), so the
66+
* target name usually does not exist yet, `prev` is null, and the gate is
67+
* skipped — the copy fails the author-time gate instead. The gate is reached
68+
* on the ordinary *duplicate-again* workflow, where the target namespace
69+
* already holds the renamed object. That is the case pinned below.
70+
*
71+
* ## Reverse verification — direction predicted BEFORE running
72+
*
73+
* Predicted with the message trimmed to a headline (the trim this card
74+
* declines): section 3's two prose assertions go RED, because the per-field
75+
* prose has no other channel on that face; sections 1 and 2 stay GREEN,
76+
* because the structured channel is untouched by a message trim. Measured:
77+
* exactly that. See the PR body for the run.
78+
*
79+
* The tests import `./protocol.js` — a RELATIVE source specifier — so vitest
80+
* resolves the subject to `src/protocol.ts` and no `dist/` is on the path;
81+
* the ablation therefore needs no rebuild, and its RED result is what rules
82+
* out the stale-artifact false green.
83+
*
84+
* ⛔ Never a bare `toThrow()` here. `duplicatePackage` does not throw, it
85+
* REPORTS, and what the report says IS the defect; and for the throw itself
86+
* the minimum assertion is `code` + `status` (ADR-0112 envelope), with the
87+
* message text asserted on top because the message text is the contract this
88+
* file exists to protect.
89+
*/
90+
import { describe, expect, it } from 'vitest';
91+
// The ONE rule both HTTP doors read (`@objectstack/types`). Asserting against
92+
// the shared resolver rather than re-implementing either door is what makes
93+
// rows 1-3 of the inventory one measurement instead of three guesses.
94+
import { resolveThrownHttpError } from '@objectstack/types';
95+
import { ObjectStackProtocolImplementation } from './protocol.js';
96+
97+
// ---------------------------------------------------------------------------
98+
// Harness — the `sys_metadata`-backed kernel the #8333 batch-verb suite uses,
99+
// minus its fault injection (nothing here is about driver text).
100+
// ---------------------------------------------------------------------------
101+
102+
interface Row {
103+
id: string;
104+
type: string;
105+
name: string;
106+
organization_id: string | null;
107+
package_id: string | null;
108+
state: string;
109+
metadata: string;
110+
checksum: string;
111+
version?: number;
112+
}
113+
114+
const PKG = 'com.acme.crm';
115+
const TARGET_PKG = 'com.acme.crm2';
116+
117+
const row = (o: Partial<Row> & { type: string; name: string }): Row => ({
118+
id: `row_${o.type}_${o.name}_${o.state ?? 'active'}`,
119+
organization_id: null,
120+
package_id: PKG,
121+
state: 'active',
122+
metadata: JSON.stringify({ name: o.name, label: 'seeded' }),
123+
checksum: 'sha256_10886_fixture',
124+
version: 1,
125+
...o,
126+
});
127+
128+
/** An `object` body with the given fields — the only type the gate can act on. */
129+
const objectRow = (name: string, fields: readonly string[], pkg = PKG): Row => row({
130+
type: 'object',
131+
name,
132+
package_id: pkg,
133+
metadata: JSON.stringify({
134+
name,
135+
label: name,
136+
fields: Object.fromEntries(fields.map((f) => [f, { name: f, type: 'text' }])),
137+
}),
138+
});
139+
140+
function makeKernel(opts: { seed?: Row[] } = {}) {
141+
const rows = new Map<string, Row>();
142+
for (const r of opts.seed ?? []) rows.set(r.id, r);
143+
144+
const match = (r: Row, where: Record<string, unknown>): boolean =>
145+
Object.entries(where ?? {}).every(([k, v]) => {
146+
if (k === '$or') return (v as Array<Record<string, unknown>>).some((c) => match(r, c));
147+
return v === null || v === undefined
148+
? (r as any)[k] === null || (r as any)[k] === undefined
149+
: (r as any)[k] === v;
150+
});
151+
152+
const engine: any = {
153+
async find(table: string, o?: { where?: Record<string, unknown> }) {
154+
if (table !== 'sys_metadata') return [];
155+
return Array.from(rows.values()).filter((r) => match(r, o?.where ?? {}));
156+
},
157+
async findOne(table: string, o: { where: Record<string, unknown> }) {
158+
if (table !== 'sys_metadata') return null;
159+
for (const r of rows.values()) if (match(r, o?.where ?? {})) return r;
160+
return null;
161+
},
162+
async insert(table: string, data: Record<string, unknown>) {
163+
if (table === 'sys_metadata') {
164+
const r = { ...(data as any) } as Row;
165+
r.id = String(data.id ?? `r_${rows.size}`);
166+
rows.set(r.id, r);
167+
}
168+
return { id: String(data.id ?? 'r_new') };
169+
},
170+
// ⚠️ NO `update` / `delete` on this double, deliberately. Every case in
171+
// this file drives a REFUSAL — the save is rejected at the Phase
172+
// 3a-destructive gate before anything is persisted — so the write
173+
// verbs are never called, and a double that implements a verb its
174+
// subject never reaches is dead code that also has to be pinned.
175+
// ⛔ Adding a case here that actually PERSISTS means adding those two
176+
// verbs back, and they must then route through
177+
// `assertEngineUpdateDispatch` / `assertEngineDeleteDispatch`
178+
// (`@objectstack/metadata-core`, #5480 / #4550) so this double cannot
179+
// accept a call `ObjectQL` itself refuses. Never hand-mirror those
180+
// checks — `check:engine-double-contract` exists for exactly that.
181+
registry: {
182+
registerItem: () => {}, registerObject: () => {}, listItems: () => [],
183+
getItem: () => undefined, getArtifactItem: () => undefined,
184+
removeRuntimeShadow: () => false, removeOverlayEntry: () => {}, uninstallPackage: () => {},
185+
},
186+
};
187+
188+
const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any;
189+
return { protocol, engine, rows };
190+
}
191+
192+
/** The refusal this whole file is about, raised by the real producer. */
193+
async function destructiveRefusal(): Promise<any> {
194+
const { protocol } = makeKernel({
195+
seed: [objectRow('crm_task', ['a', 'b', 'c', 'd'])],
196+
});
197+
try {
198+
await protocol.saveMetaItem({
199+
type: 'object',
200+
name: 'crm_task',
201+
item: { name: 'crm_task', label: 'crm_task', fields: { a: { name: 'a', type: 'text' } } },
202+
});
203+
} catch (e: any) {
204+
return e;
205+
}
206+
throw new Error('expected saveMetaItem to refuse the destructive change');
207+
}
208+
209+
/** The remedy sentence that must survive ANY future trim (#10886 non-effect). */
210+
const REMEDY = 're-submit with ?force=true to proceed.';
211+
/** One finding's prose, as `detectDestructiveObjectChanges` words it. */
212+
const FINDING_PROSE = "Field 'b' removed — existing data in this column will become inaccessible.";
213+
214+
// ═══════════════════════════════════════════════════════════════════════════
215+
// 1. The duplication is real — both channels carry the same findings
216+
// ═══════════════════════════════════════════════════════════════════════════
217+
218+
describe('[#10886] the 409 renders its findings into the message AND attaches them', () => {
219+
it('declares the ADR-0112 envelope and attaches the structured findings', async () => {
220+
const err = await destructiveRefusal();
221+
222+
// Minimum assertion set for a refusal: code + status, never a bare throw.
223+
expect(err.code).toBe('DESTRUCTIVE_CHANGE');
224+
expect(err.status).toBe(409);
225+
expect(err.issues).toEqual(expect.arrayContaining([
226+
expect.objectContaining({ code: 'field_removed', field: 'b', message: FINDING_PROSE }),
227+
]));
228+
});
229+
230+
it('the message restates the SAME prose the `issues` array carries', async () => {
231+
const err = await destructiveRefusal();
232+
233+
// This is the duplication itself. A console rendering both channels
234+
// shows this sentence twice.
235+
expect(err.message).toContain(FINDING_PROSE);
236+
expect(err.issues.map((i: { message: string }) => i.message)).toContain(FINDING_PROSE);
237+
});
238+
239+
it('[GUARD] the message ends with the actionable remedy — no structural channel carries it', async () => {
240+
const err = await destructiveRefusal();
241+
242+
// ⭐ The expected NON-effect of any future trim. Unlike a validation
243+
// refusal, this is a risk-ACKNOWLEDGEMENT flow: the remedy is the
244+
// whole point, it is not one of the `issues`, and nothing else on any
245+
// face carries it.
246+
expect(err.message).toContain(REMEDY);
247+
const wire = JSON.stringify(err.issues);
248+
expect(wire).not.toContain('force=true');
249+
});
250+
});
251+
252+
// ═══════════════════════════════════════════════════════════════════════════
253+
// 2. Inventory rows 1-3 — the HTTP faces DO carry `issues` structurally
254+
// ═══════════════════════════════════════════════════════════════════════════
255+
256+
describe('[#10886] the HTTP doors are not sole carriers — `issues` reaches them structurally', () => {
257+
it('the shared boundary resolver threads `issues` into `details`', async () => {
258+
const err = await destructiveRefusal();
259+
260+
// Rows 1-3 of the inventory all resolve through this one function:
261+
// `@objectstack/rest`'s `handleRouteError` reads `error.issues` onto a
262+
// top-level `issues`, and the dispatcher's `errorFromThrown` puts
263+
// `thrown.details` on the envelope. Either way the findings survive a
264+
// message trim — which is exactly why those faces do NOT block one.
265+
const thrown = resolveThrownHttpError(err, 400);
266+
267+
expect(thrown.status).toBe(409);
268+
expect(thrown.code).toBe('DESTRUCTIVE_CHANGE');
269+
expect(thrown.details?.issues).toEqual(err.issues);
270+
});
271+
});
272+
273+
// ═══════════════════════════════════════════════════════════════════════════
274+
// 3. [GUARD] Inventory row 6 — the SOLE CARRIER. ⛔ This is what forbids the trim.
275+
// ═══════════════════════════════════════════════════════════════════════════
276+
277+
describe('[#10886] [GUARD] `duplicatePackage`’s `failed[].error` is the SOLE carrier of the destructive prescription', () => {
278+
/**
279+
* The reachability case. Source package `com.acme.crm` (namespace `crm`)
280+
* holds `crm_task` with one field; the target namespace `crm2` ALREADY
281+
* holds `crm2_task` with four — the state left by an earlier duplicate.
282+
* The copy would drop three columns, so the gate fires on the copy.
283+
*/
284+
const duplicateIntoOccupiedNamespace = () => makeKernel({
285+
seed: [
286+
objectRow('crm_task', ['a']),
287+
objectRow('crm2_task', ['a', 'b', 'c', 'd'], TARGET_PKG),
288+
],
289+
});
290+
291+
it('reaches the gate at all — the copy is refused with the destructive change', async () => {
292+
const { protocol } = duplicateIntoOccupiedNamespace();
293+
294+
const r = await protocol.duplicatePackage({
295+
sourcePackageId: PKG, targetPackageId: TARGET_PKG,
296+
});
297+
298+
expect(r.failedCount).toBe(1);
299+
expect(r.failed[0]).toMatchObject({ type: 'object', name: 'crm_task' });
300+
expect(r.failed[0].error).toContain('[destructive_change]');
301+
});
302+
303+
it('⛔ carries the per-field prose with NO structured channel beside it', async () => {
304+
const { protocol } = duplicateIntoOccupiedNamespace();
305+
306+
const r = await protocol.duplicatePackage({
307+
sourcePackageId: PKG, targetPackageId: TARGET_PKG,
308+
});
309+
const entry = r.failed[0];
310+
311+
// The prose reaches the caller ONLY through this string …
312+
expect(entry.error).toContain(FINDING_PROSE);
313+
// … and there is no `issues` beside it. Not "an empty array" — the key
314+
// is absent, and the array's own type has no slot for it. Trimming the
315+
// message would delete these findings from the wire outright.
316+
expect('issues' in entry).toBe(false);
317+
expect(entry.issues).toBeUndefined();
318+
});
319+
320+
it('⛔ carries the `?force=true` remedy, on a response with no other channel for it', async () => {
321+
const { protocol } = duplicateIntoOccupiedNamespace();
322+
323+
const r = await protocol.duplicatePackage({
324+
sourcePackageId: PKG, targetPackageId: TARGET_PKG,
325+
});
326+
327+
expect(r.failed[0].error).toContain(REMEDY);
328+
// The whole response, not just the entry: nothing anywhere else on it
329+
// states the remedy or the findings.
330+
const wire = JSON.stringify({ ...r, failed: r.failed.map((f: any) => ({ ...f, error: '' })) });
331+
expect(wire).not.toContain('force=true');
332+
expect(wire).not.toContain('inaccessible');
333+
});
334+
});

0 commit comments

Comments
 (0)