Skip to content

Commit fc015bc

Browse files
os-muskclaude
andauthored
test(metadata-protocol,objectql): key the protocol stub engines by table (#16651)
* test(metadata-protocol,objectql): key the protocol stub engines by table Eleven protocol harnesses gave their stub engine one flat row map and told the tables apart in exactly one place — the `insert` early-return for `sys_metadata_audit`. `find`/`findOne` ignored the table argument entirely, so every other table one save writes (`sys_metadata_history`, `sys_metadata_commit`) landed in the map that answered reads of `sys_metadata`. Each stub now holds a map from table NAME to that table's rows, reached through a `tableOf(name)` accessor, copying the shape `protocol.runtime-gate-stored-universe.test.ts` established. `rows` stays bound to `sys_metadata`, so existing assertions read the table they name. The `audit_skip` early-return goes with it: with the tables separated it has no job left, and it was a standing trap for any future assertion about audit rows written in these files. Two pins make the change a measurement rather than a rename — one per package. A DRAFT save writes `sys_metadata`, `sys_metadata_history` and `sys_metadata_audit`, all three addressed to the same `(type, name)`, and the journal rows carry no `state`; each pin asserts a `sys_metadata` read answers with the store row and nothing else, behind a firing control that the journals really were written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * fix(test): type the journal rows in the #16225 pins as Partial<Row> `tsc --noEmit` refused the metadata-protocol pin with TS2352: `Row` declares no index signature, so asserting `Row[]` to `Record<string, unknown>[]` is not a legal widening. `Partial<Row>` is legal AND true of these rows — a journal row carries `type` and `name` and carries no `state`, which is exactly what the assertions beside it read. The objectql pin's `any[]` is spelled the same way for the same reason, replacing a cast that typechecked while saying nothing. Vitest never type-checks, so the suites were green over code tsc refuses; the error was only ever reachable through the package's own `typecheck` script, which this branch had last run before either pin existed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent cf74a11 commit fc015bc

11 files changed

Lines changed: 454 additions & 157 deletions

packages/metadata-protocol/src/protocol.container-issue-descent.test.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,17 +73,29 @@ const keyOf = (w: Record<string, unknown>) =>
7373

7474
/** The engine surface the repository write path touches (as #5364's harness). */
7575
function makeProtocol() {
76-
const rows = new Map<string, Row>();
76+
// ⚠️ Keyed BY TABLE. `find`/`findOne` below answer nothing, so this harness
77+
// cannot serve a `sys_metadata_history` row as a `sys_metadata` row the way
78+
// #16223 measured — but one flat map still made `rows.size` the total of
79+
// every table one save writes. `rows` is the store table these tests assert
80+
// on; the journals the protocol also writes get their own.
81+
const tables = new Map<string, Map<string, Row>>();
82+
const tableOf = (table: string): Map<string, Row> => {
83+
const existing = tables.get(table);
84+
if (existing) return existing;
85+
const created = new Map<string, Row>();
86+
tables.set(table, created);
87+
return created;
88+
};
89+
const rows = tableOf('sys_metadata');
7790
let nextId = 0;
7891
const engine: any = {
7992
async findOne(object: string, query?: EngineFindOneQueryInput) {
8093
assertEngineFindOnePredicate(object, query); return null; },
8194
async find() { return []; },
8295
async insert(table: string, data: Record<string, unknown>) {
83-
if (table === 'sys_metadata_audit') return { id: 'audit_skip' };
8496
nextId += 1;
8597
const row = { id: `r_${nextId}`, ...(data as any) } as Row;
86-
rows.set(keyOf(data), row);
98+
tableOf(table).set(keyOf(data), row);
8799
return { id: row.id };
88100
},
89101
async update(_t: string, data: Record<string, unknown>, opts?: Record<string, unknown>) {

packages/metadata-protocol/src/protocol.dashboard-dataset-publish-gate.test.ts

Lines changed: 112 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -111,14 +111,30 @@ const keyOf = (w: Record<string, unknown>) =>
111111
`${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}`;
112112

113113
function makeStubEngine() {
114-
const rows = new Map<string, Row>();
114+
// ⚠️ Keyed BY TABLE, and that is a correctness property of this harness
115+
// rather than tidiness. One flat row map answers a read of `sys_metadata`
116+
// with rows the protocol wrote to `sys_metadata_history` and
117+
// `sys_metadata_commit`: a DRAFT save appends a history row carrying no
118+
// `state`, the declared `defaultValue: 'active'` modelled below fills it
119+
// in, and the draft comes back as an ACTIVE metadata row. Measured in
120+
// #16223, where one assertion's polarity was the only thing that caught it.
121+
const tables = new Map<string, Map<string, Row>>();
122+
const tableOf = (table: string): Map<string, Row> => {
123+
const existing = tables.get(table);
124+
if (existing) return existing;
125+
const created = new Map<string, Row>();
126+
tables.set(table, created);
127+
return created;
128+
};
129+
/** The store table these tests assert on; the journals get their own. */
130+
const rows = tableOf('sys_metadata');
115131
let nextId = 0;
116-
const findRow = (w: Record<string, unknown>): { key: string; row: Row } | null => {
132+
const findRow = (table: string, w: Record<string, unknown>): { key: string; row: Row } | null => {
117133
if (w.id !== undefined) {
118-
for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r };
134+
for (const [k, r] of tableOf(table)) if (r.id === w.id) return { key: k, row: r };
119135
return null;
120136
}
121-
for (const [k, r] of rows) {
137+
for (const [k, r] of tableOf(table)) {
122138
if (w.type !== undefined && r.type !== w.type) continue;
123139
if (w.name !== undefined && r.name !== w.name) continue;
124140
if (w.organization_id !== undefined && r.organization_id !== w.organization_id) continue;
@@ -128,38 +144,37 @@ function makeStubEngine() {
128144
return null;
129145
};
130146
const engine: any = {
131-
async findOne(_t: string, opts: { where: Record<string, unknown> }) {
132-
assertEngineFindOnePredicate(_t, opts);
133-
return findRow(opts.where)?.row ?? null;
147+
async findOne(table: string, opts: { where: Record<string, unknown> }) {
148+
assertEngineFindOnePredicate(table, opts);
149+
return findRow(table, opts.where)?.row ?? null;
134150
},
135-
async find(_t: string, opts: { where: Record<string, unknown> }) {
136-
return Array.from(rows.values()).filter((r) => {
151+
async find(table: string, opts: { where: Record<string, unknown> }) {
152+
return Array.from(tableOf(table).values()).filter((r) => {
137153
if (opts.where.type && r.type !== opts.where.type) return false;
138154
if (opts.where.organization_id !== undefined
139155
&& r.organization_id !== opts.where.organization_id) return false;
140156
if (opts.where.state && r.state !== opts.where.state) return false;
141157
return true;
142158
});
143159
},
144-
async insert(_t: string, data: Record<string, unknown>) {
145-
if (_t === 'sys_metadata_audit') return { id: 'audit_skip' };
160+
async insert(table: string, data: Record<string, unknown>) {
146161
nextId += 1;
147162
const row = { id: `r_${nextId}`, ...(data as any) } as Row;
148-
rows.set(keyOf(data), row);
163+
tableOf(table).set(keyOf(data), row);
149164
return { id: row.id };
150165
},
151-
async update(_t: string, data: Record<string, unknown>, opts: { where: Record<string, unknown> }) {
166+
async update(table: string, data: Record<string, unknown>, opts: { where: Record<string, unknown> }) {
152167
assertEngineUpdateDispatch(data, opts);
153-
const found = findRow(opts.where);
168+
const found = findRow(table, opts.where);
154169
if (!found) return { id: null };
155-
rows.set(found.key, { ...found.row, ...(data as any) });
170+
tableOf(table).set(found.key, { ...found.row, ...(data as any) });
156171
return { id: found.row.id };
157172
},
158-
async delete(_t: string, opts: { where: Record<string, unknown> }) {
173+
async delete(table: string, opts: { where: Record<string, unknown> }) {
159174
assertEngineDeleteDispatch(opts);
160-
const found = findRow(opts.where);
175+
const found = findRow(table, opts.where);
161176
if (!found) return { deleted: 0 };
162-
rows.delete(found.key);
177+
tableOf(table).delete(found.key);
163178
return { deleted: 1 };
164179
},
165180
registry: {
@@ -196,14 +211,14 @@ function makeStubEngine() {
196211
getItem: () => undefined,
197212
},
198213
};
199-
return { engine, rows };
214+
return { engine, rows, tableOf };
200215
}
201216

202217
/** A protocol on the ordinary tenant posture (environment id, default channel). */
203218
function makeProtocol() {
204-
const { engine, rows } = makeStubEngine();
219+
const { engine, rows, tableOf } = makeStubEngine();
205220
const protocol = new ObjectStackProtocolImplementation(engine, () => new Map(), 'env_test');
206-
return { protocol: protocol as any, rows };
221+
return { protocol: protocol as any, engine, rows, tableOf };
207222
}
208223

209224
const dashboardRows = (rows: Map<string, Row>) =>
@@ -345,3 +360,79 @@ describe('dashboard dataset bindings at the publish door (#7529)', () => {
345360
expect(dashboardRows(rows).length).toBeGreaterThan(0);
346361
});
347362
});
363+
364+
// ─────────────────────────────────────────────────────────────────────────────
365+
// #16225 — the stub answers the table it was asked about, and only that one
366+
// ─────────────────────────────────────────────────────────────────────────────
367+
//
368+
// This block is what makes the table-keying above a MEASUREMENT rather than a
369+
// rename. Every other test in this file passes identically with the tables
370+
// merged back into one map, because none of them reads `sys_metadata` as a
371+
// table — which is exactly how the shape survived in eight harnesses.
372+
//
373+
// The incident it pins is #16223's: one save writes `sys_metadata`,
374+
// `sys_metadata_history` and `sys_metadata_audit`, all three addressed to the
375+
// same `(type, name)`, and the two journal rows carry no `state` of their own.
376+
// A flat map hands them back to a `sys_metadata` read; a harness that also
377+
// models `sys_metadata.state`'s declared `defaultValue: 'active'` — correctly,
378+
// which is what made it convincing — serves a DRAFT-only artifact back as an
379+
// ACTIVE metadata row.
380+
describe('#16225 a `sys_metadata` read is not answered from the journal tables', () => {
381+
let warn: ReturnType<typeof vi.spyOn>;
382+
beforeEach(() => { warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); });
383+
afterEach(() => { warn.mockRestore(); });
384+
385+
it('serves the store row only, never the history/audit rows the same save wrote', async () => {
386+
const { protocol, engine, tableOf } = makeProtocol();
387+
388+
const saved = await save(protocol, legitBoard(), { mode: 'draft' });
389+
expect(saved.success).toBe(true);
390+
expect(saved.state, 'the artifact exists as a DRAFT and nothing else').toBe('draft');
391+
392+
// The firing control. If one save ever stops writing the journals, the
393+
// read below is measuring an empty universe rather than a separation,
394+
// and this line says so instead of going quietly green. It is read
395+
// through `tableOf`, so it stays satisfied under a re-merge — the pin
396+
// and not the control is what a re-merge is meant to break.
397+
const journal = [
398+
...tableOf('sys_metadata_history').values(),
399+
...tableOf('sys_metadata_audit').values(),
400+
// `Partial<Row>` and not `Record<string, unknown>`: `Row` declares no
401+
// index signature, so that widening is a TS2352, and `Partial` is
402+
// the honest type anyway — a journal row carries `type` and `name`
403+
// and does NOT carry the `state` the assertions below look for.
404+
] as Partial<Row>[];
405+
expect(
406+
journal.length,
407+
'the firing control: one save must WRITE the journal tables, or the read below '
408+
+ 'proves nothing about which table answered it',
409+
).toBeGreaterThan(0);
410+
411+
// ── The pin ──────────────────────────────────────────────────────────
412+
// Asserted on a STORE-ONLY column rather than on the row count, because
413+
// a journal row is addressed to the same `(type, name, organization_id)`
414+
// as the store row and can therefore COLLIDE with it under `keyOf` —
415+
// a merged map can hold the audit row in the store row's place and
416+
// still answer with one row of the right name. `checksum` and `state`
417+
// are written by the store leg alone, so this fails either way.
418+
const stored = await engine.find('sys_metadata', { where: { type: 'dashboard' } });
419+
expect(
420+
stored.map((r: Row) => r.state),
421+
'a `sys_metadata` read must answer with the store row and nothing else',
422+
).toEqual(['draft']);
423+
expect(
424+
stored.map((r: Row) => typeof r.checksum),
425+
'and the row it answers with must be a STORE row, not a journal row wearing '
426+
+ 'the same `(type, name)`',
427+
).toEqual(['string']);
428+
429+
// The mechanics of the incident, recorded once the separation holds:
430+
// every journal row is addressed to the same `(type, name)` as the
431+
// store row, and none of them declares a `state` — so a harness
432+
// modelling `sys_metadata.state`'s declared `defaultValue: 'active'`
433+
// hands a DRAFT-only artifact back as an ACTIVE metadata row (#16223).
434+
expect(journal.map((r) => `${r.type}|${r.name}`))
435+
.toEqual(new Array(journal.length).fill('dashboard|ops_board'));
436+
expect(journal.some((r) => 'state' in r)).toBe(false);
437+
});
438+
});

packages/metadata-protocol/src/protocol.graft-folded-form-sections.test.ts

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,30 @@ const keyOf = (w: Record<string, unknown>) =>
5454
* INSIDE `saveMetaItem` cannot be tested through a harness that mocks it.
5555
*/
5656
function makeProtocol() {
57-
const rows = new Map<string, Row>();
57+
// ⚠️ Keyed BY TABLE, and that is a correctness property of this harness
58+
// rather than tidiness. One flat row map answers a read of `sys_metadata`
59+
// with rows the protocol wrote to `sys_metadata_history` and
60+
// `sys_metadata_commit`: a DRAFT save appends a history row carrying no
61+
// `state`, the declared `defaultValue: 'active'` modelled below fills it
62+
// in, and the draft comes back as an ACTIVE metadata row. Measured in
63+
// #16223, where one assertion's polarity was the only thing that caught it.
64+
const tables = new Map<string, Map<string, Row>>();
65+
const tableOf = (table: string): Map<string, Row> => {
66+
const existing = tables.get(table);
67+
if (existing) return existing;
68+
const created = new Map<string, Row>();
69+
tables.set(table, created);
70+
return created;
71+
};
72+
/** The store table these tests assert on; the journals get their own. */
73+
const rows = tableOf('sys_metadata');
5874
let nextId = 0;
59-
const findRow = (w: Record<string, unknown>): { key: string; row: Row } | null => {
75+
const findRow = (table: string, w: Record<string, unknown>): { key: string; row: Row } | null => {
6076
if (w.id !== undefined) {
61-
for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r };
77+
for (const [k, r] of tableOf(table)) if (r.id === w.id) return { key: k, row: r };
6278
return null;
6379
}
64-
for (const [k, r] of rows) {
80+
for (const [k, r] of tableOf(table)) {
6581
if (w.type !== undefined && r.type !== w.type) continue;
6682
if (w.name !== undefined && r.name !== w.name) continue;
6783
if (w.organization_id !== undefined && r.organization_id !== w.organization_id) continue;
@@ -71,38 +87,37 @@ function makeProtocol() {
7187
return null;
7288
};
7389
const engine: any = {
74-
async findOne(_t: string, opts: { where: Record<string, unknown> }) {
75-
assertEngineFindOnePredicate(_t, opts);
76-
return findRow(opts.where)?.row ?? null;
90+
async findOne(table: string, opts: { where: Record<string, unknown> }) {
91+
assertEngineFindOnePredicate(table, opts);
92+
return findRow(table, opts.where)?.row ?? null;
7793
},
78-
async find(_t: string, opts: { where: Record<string, unknown> }) {
79-
return Array.from(rows.values()).filter((r) => {
94+
async find(table: string, opts: { where: Record<string, unknown> }) {
95+
return Array.from(tableOf(table).values()).filter((r) => {
8096
if (opts.where.type && r.type !== opts.where.type) return false;
8197
if (opts.where.organization_id !== undefined
8298
&& r.organization_id !== opts.where.organization_id) return false;
8399
if (opts.where.state && r.state !== opts.where.state) return false;
84100
return true;
85101
});
86102
},
87-
async insert(_t: string, data: Record<string, unknown>) {
88-
if (_t === 'sys_metadata_audit') return { id: 'audit_skip' };
103+
async insert(table: string, data: Record<string, unknown>) {
89104
nextId += 1;
90105
const row = { id: `r_${nextId}`, ...(data as any) } as Row;
91-
rows.set(keyOf(data), row);
106+
tableOf(table).set(keyOf(data), row);
92107
return { id: row.id };
93108
},
94-
async update(_t: string, data: Record<string, unknown>, opts: { where: Record<string, unknown> }) {
109+
async update(table: string, data: Record<string, unknown>, opts: { where: Record<string, unknown> }) {
95110
assertEngineUpdateDispatch(data, opts);
96-
const found = findRow(opts.where);
111+
const found = findRow(table, opts.where);
97112
if (!found) return { id: null };
98-
rows.set(found.key, { ...found.row, ...(data as any) });
113+
tableOf(table).set(found.key, { ...found.row, ...(data as any) });
99114
return { id: found.row.id };
100115
},
101-
async delete(_t: string, opts: { where: Record<string, unknown> }) {
116+
async delete(table: string, opts: { where: Record<string, unknown> }) {
102117
assertEngineDeleteDispatch(opts);
103-
const found = findRow(opts.where);
118+
const found = findRow(table, opts.where);
104119
if (!found) return { deleted: 0 };
105-
rows.delete(found.key);
120+
tableOf(table).delete(found.key);
106121
return { deleted: 1 };
107122
},
108123
registry: { registerItem: () => {}, registerObject: () => {} },

packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,17 +98,29 @@ const keyOf = (w: Record<string, unknown>) =>
9898
`${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}`;
9999

100100
function makeProtocol() {
101-
const rows = new Map<string, Row>();
101+
// ⚠️ Keyed BY TABLE. `find`/`findOne` below answer nothing, so this harness
102+
// cannot serve a `sys_metadata_history` row as a `sys_metadata` row the way
103+
// #16223 measured — but one flat map still made `rows.size` the total of
104+
// every table one save writes. `rows` is the store table these tests assert
105+
// on; the journals the protocol also writes get their own.
106+
const tables = new Map<string, Map<string, Row>>();
107+
const tableOf = (table: string): Map<string, Row> => {
108+
const existing = tables.get(table);
109+
if (existing) return existing;
110+
const created = new Map<string, Row>();
111+
tables.set(table, created);
112+
return created;
113+
};
114+
const rows = tableOf('sys_metadata');
102115
let nextId = 0;
103116
const engine: any = {
104117
async findOne(object: string, query?: EngineFindOneQueryInput) {
105118
assertEngineFindOnePredicate(object, query); return null; },
106119
async find() { return []; },
107120
async insert(table: string, data: Record<string, unknown>) {
108-
if (table === 'sys_metadata_audit') return { id: 'audit_skip' };
109121
nextId += 1;
110122
const row = { id: `r_${nextId}`, ...(data as any) } as Row;
111-
rows.set(keyOf(data), row);
123+
tableOf(table).set(keyOf(data), row);
112124
return { id: row.id };
113125
},
114126
async update(_t: string, data: Record<string, unknown>, opts?: Record<string, unknown>) {

0 commit comments

Comments
 (0)