diff --git a/packages/metadata-protocol/src/protocol.container-issue-descent.test.ts b/packages/metadata-protocol/src/protocol.container-issue-descent.test.ts index f20f4b6563..acd8651eaf 100644 --- a/packages/metadata-protocol/src/protocol.container-issue-descent.test.ts +++ b/packages/metadata-protocol/src/protocol.container-issue-descent.test.ts @@ -73,17 +73,29 @@ const keyOf = (w: Record) => /** The engine surface the repository write path touches (as #5364's harness). */ function makeProtocol() { - const rows = new Map(); + // ⚠️ Keyed BY TABLE. `find`/`findOne` below answer nothing, so this harness + // cannot serve a `sys_metadata_history` row as a `sys_metadata` row the way + // #16223 measured — but one flat map still made `rows.size` the total of + // every table one save writes. `rows` is the store table these tests assert + // on; the journals the protocol also writes get their own. + const tables = new Map>(); + const tableOf = (table: string): Map => { + const existing = tables.get(table); + if (existing) return existing; + const created = new Map(); + tables.set(table, created); + return created; + }; + const rows = tableOf('sys_metadata'); let nextId = 0; const engine: any = { async findOne(object: string, query?: EngineFindOneQueryInput) { assertEngineFindOnePredicate(object, query); return null; }, async find() { return []; }, async insert(table: string, data: Record) { - if (table === 'sys_metadata_audit') return { id: 'audit_skip' }; nextId += 1; const row = { id: `r_${nextId}`, ...(data as any) } as Row; - rows.set(keyOf(data), row); + tableOf(table).set(keyOf(data), row); return { id: row.id }; }, async update(_t: string, data: Record, opts?: Record) { diff --git a/packages/metadata-protocol/src/protocol.dashboard-dataset-publish-gate.test.ts b/packages/metadata-protocol/src/protocol.dashboard-dataset-publish-gate.test.ts index 0fbe201a80..accd5ad132 100644 --- a/packages/metadata-protocol/src/protocol.dashboard-dataset-publish-gate.test.ts +++ b/packages/metadata-protocol/src/protocol.dashboard-dataset-publish-gate.test.ts @@ -111,14 +111,30 @@ const keyOf = (w: Record) => `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}`; function makeStubEngine() { - const rows = new Map(); + // ⚠️ Keyed BY TABLE, and that is a correctness property of this harness + // rather than tidiness. One flat row map answers a read of `sys_metadata` + // with rows the protocol wrote to `sys_metadata_history` and + // `sys_metadata_commit`: a DRAFT save appends a history row carrying no + // `state`, the declared `defaultValue: 'active'` modelled below fills it + // in, and the draft comes back as an ACTIVE metadata row. Measured in + // #16223, where one assertion's polarity was the only thing that caught it. + const tables = new Map>(); + const tableOf = (table: string): Map => { + const existing = tables.get(table); + if (existing) return existing; + const created = new Map(); + tables.set(table, created); + return created; + }; + /** The store table these tests assert on; the journals get their own. */ + const rows = tableOf('sys_metadata'); let nextId = 0; - const findRow = (w: Record): { key: string; row: Row } | null => { + const findRow = (table: string, w: Record): { key: string; row: Row } | null => { if (w.id !== undefined) { - for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + for (const [k, r] of tableOf(table)) if (r.id === w.id) return { key: k, row: r }; return null; } - for (const [k, r] of rows) { + for (const [k, r] of tableOf(table)) { if (w.type !== undefined && r.type !== w.type) continue; if (w.name !== undefined && r.name !== w.name) continue; if (w.organization_id !== undefined && r.organization_id !== w.organization_id) continue; @@ -128,12 +144,12 @@ function makeStubEngine() { return null; }; const engine: any = { - async findOne(_t: string, opts: { where: Record }) { - assertEngineFindOnePredicate(_t, opts); - return findRow(opts.where)?.row ?? null; + async findOne(table: string, opts: { where: Record }) { + assertEngineFindOnePredicate(table, opts); + return findRow(table, opts.where)?.row ?? null; }, - async find(_t: string, opts: { where: Record }) { - return Array.from(rows.values()).filter((r) => { + async find(table: string, opts: { where: Record }) { + return Array.from(tableOf(table).values()).filter((r) => { if (opts.where.type && r.type !== opts.where.type) return false; if (opts.where.organization_id !== undefined && r.organization_id !== opts.where.organization_id) return false; @@ -141,25 +157,24 @@ function makeStubEngine() { return true; }); }, - async insert(_t: string, data: Record) { - if (_t === 'sys_metadata_audit') return { id: 'audit_skip' }; + async insert(table: string, data: Record) { nextId += 1; const row = { id: `r_${nextId}`, ...(data as any) } as Row; - rows.set(keyOf(data), row); + tableOf(table).set(keyOf(data), row); return { id: row.id }; }, - async update(_t: string, data: Record, opts: { where: Record }) { + async update(table: string, data: Record, opts: { where: Record }) { assertEngineUpdateDispatch(data, opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { id: null }; - rows.set(found.key, { ...found.row, ...(data as any) }); + tableOf(table).set(found.key, { ...found.row, ...(data as any) }); return { id: found.row.id }; }, - async delete(_t: string, opts: { where: Record }) { + async delete(table: string, opts: { where: Record }) { assertEngineDeleteDispatch(opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { deleted: 0 }; - rows.delete(found.key); + tableOf(table).delete(found.key); return { deleted: 1 }; }, registry: { @@ -196,14 +211,14 @@ function makeStubEngine() { getItem: () => undefined, }, }; - return { engine, rows }; + return { engine, rows, tableOf }; } /** A protocol on the ordinary tenant posture (environment id, default channel). */ function makeProtocol() { - const { engine, rows } = makeStubEngine(); + const { engine, rows, tableOf } = makeStubEngine(); const protocol = new ObjectStackProtocolImplementation(engine, () => new Map(), 'env_test'); - return { protocol: protocol as any, rows }; + return { protocol: protocol as any, engine, rows, tableOf }; } const dashboardRows = (rows: Map) => @@ -345,3 +360,79 @@ describe('dashboard dataset bindings at the publish door (#7529)', () => { expect(dashboardRows(rows).length).toBeGreaterThan(0); }); }); + +// ───────────────────────────────────────────────────────────────────────────── +// #16225 — the stub answers the table it was asked about, and only that one +// ───────────────────────────────────────────────────────────────────────────── +// +// This block is what makes the table-keying above a MEASUREMENT rather than a +// rename. Every other test in this file passes identically with the tables +// merged back into one map, because none of them reads `sys_metadata` as a +// table — which is exactly how the shape survived in eight harnesses. +// +// The incident it pins is #16223's: one save writes `sys_metadata`, +// `sys_metadata_history` and `sys_metadata_audit`, all three addressed to the +// same `(type, name)`, and the two journal rows carry no `state` of their own. +// A flat map hands them back to a `sys_metadata` read; a harness that also +// models `sys_metadata.state`'s declared `defaultValue: 'active'` — correctly, +// which is what made it convincing — serves a DRAFT-only artifact back as an +// ACTIVE metadata row. +describe('#16225 a `sys_metadata` read is not answered from the journal tables', () => { + let warn: ReturnType; + beforeEach(() => { warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); }); + afterEach(() => { warn.mockRestore(); }); + + it('serves the store row only, never the history/audit rows the same save wrote', async () => { + const { protocol, engine, tableOf } = makeProtocol(); + + const saved = await save(protocol, legitBoard(), { mode: 'draft' }); + expect(saved.success).toBe(true); + expect(saved.state, 'the artifact exists as a DRAFT and nothing else').toBe('draft'); + + // The firing control. If one save ever stops writing the journals, the + // read below is measuring an empty universe rather than a separation, + // and this line says so instead of going quietly green. It is read + // through `tableOf`, so it stays satisfied under a re-merge — the pin + // and not the control is what a re-merge is meant to break. + const journal = [ + ...tableOf('sys_metadata_history').values(), + ...tableOf('sys_metadata_audit').values(), + // `Partial` and not `Record`: `Row` declares no + // index signature, so that widening is a TS2352, and `Partial` is + // the honest type anyway — a journal row carries `type` and `name` + // and does NOT carry the `state` the assertions below look for. + ] as Partial[]; + expect( + journal.length, + 'the firing control: one save must WRITE the journal tables, or the read below ' + + 'proves nothing about which table answered it', + ).toBeGreaterThan(0); + + // ── The pin ────────────────────────────────────────────────────────── + // Asserted on a STORE-ONLY column rather than on the row count, because + // a journal row is addressed to the same `(type, name, organization_id)` + // as the store row and can therefore COLLIDE with it under `keyOf` — + // a merged map can hold the audit row in the store row's place and + // still answer with one row of the right name. `checksum` and `state` + // are written by the store leg alone, so this fails either way. + const stored = await engine.find('sys_metadata', { where: { type: 'dashboard' } }); + expect( + stored.map((r: Row) => r.state), + 'a `sys_metadata` read must answer with the store row and nothing else', + ).toEqual(['draft']); + expect( + stored.map((r: Row) => typeof r.checksum), + 'and the row it answers with must be a STORE row, not a journal row wearing ' + + 'the same `(type, name)`', + ).toEqual(['string']); + + // The mechanics of the incident, recorded once the separation holds: + // every journal row is addressed to the same `(type, name)` as the + // store row, and none of them declares a `state` — so a harness + // modelling `sys_metadata.state`'s declared `defaultValue: 'active'` + // hands a DRAFT-only artifact back as an ACTIVE metadata row (#16223). + expect(journal.map((r) => `${r.type}|${r.name}`)) + .toEqual(new Array(journal.length).fill('dashboard|ops_board')); + expect(journal.some((r) => 'state' in r)).toBe(false); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.graft-folded-form-sections.test.ts b/packages/metadata-protocol/src/protocol.graft-folded-form-sections.test.ts index 2aa35014e0..49667dd8e3 100644 --- a/packages/metadata-protocol/src/protocol.graft-folded-form-sections.test.ts +++ b/packages/metadata-protocol/src/protocol.graft-folded-form-sections.test.ts @@ -54,14 +54,30 @@ const keyOf = (w: Record) => * INSIDE `saveMetaItem` cannot be tested through a harness that mocks it. */ function makeProtocol() { - const rows = new Map(); + // ⚠️ Keyed BY TABLE, and that is a correctness property of this harness + // rather than tidiness. One flat row map answers a read of `sys_metadata` + // with rows the protocol wrote to `sys_metadata_history` and + // `sys_metadata_commit`: a DRAFT save appends a history row carrying no + // `state`, the declared `defaultValue: 'active'` modelled below fills it + // in, and the draft comes back as an ACTIVE metadata row. Measured in + // #16223, where one assertion's polarity was the only thing that caught it. + const tables = new Map>(); + const tableOf = (table: string): Map => { + const existing = tables.get(table); + if (existing) return existing; + const created = new Map(); + tables.set(table, created); + return created; + }; + /** The store table these tests assert on; the journals get their own. */ + const rows = tableOf('sys_metadata'); let nextId = 0; - const findRow = (w: Record): { key: string; row: Row } | null => { + const findRow = (table: string, w: Record): { key: string; row: Row } | null => { if (w.id !== undefined) { - for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + for (const [k, r] of tableOf(table)) if (r.id === w.id) return { key: k, row: r }; return null; } - for (const [k, r] of rows) { + for (const [k, r] of tableOf(table)) { if (w.type !== undefined && r.type !== w.type) continue; if (w.name !== undefined && r.name !== w.name) continue; if (w.organization_id !== undefined && r.organization_id !== w.organization_id) continue; @@ -71,12 +87,12 @@ function makeProtocol() { return null; }; const engine: any = { - async findOne(_t: string, opts: { where: Record }) { - assertEngineFindOnePredicate(_t, opts); - return findRow(opts.where)?.row ?? null; + async findOne(table: string, opts: { where: Record }) { + assertEngineFindOnePredicate(table, opts); + return findRow(table, opts.where)?.row ?? null; }, - async find(_t: string, opts: { where: Record }) { - return Array.from(rows.values()).filter((r) => { + async find(table: string, opts: { where: Record }) { + return Array.from(tableOf(table).values()).filter((r) => { if (opts.where.type && r.type !== opts.where.type) return false; if (opts.where.organization_id !== undefined && r.organization_id !== opts.where.organization_id) return false; @@ -84,25 +100,24 @@ function makeProtocol() { return true; }); }, - async insert(_t: string, data: Record) { - if (_t === 'sys_metadata_audit') return { id: 'audit_skip' }; + async insert(table: string, data: Record) { nextId += 1; const row = { id: `r_${nextId}`, ...(data as any) } as Row; - rows.set(keyOf(data), row); + tableOf(table).set(keyOf(data), row); return { id: row.id }; }, - async update(_t: string, data: Record, opts: { where: Record }) { + async update(table: string, data: Record, opts: { where: Record }) { assertEngineUpdateDispatch(data, opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { id: null }; - rows.set(found.key, { ...found.row, ...(data as any) }); + tableOf(table).set(found.key, { ...found.row, ...(data as any) }); return { id: found.row.id }; }, - async delete(_t: string, opts: { where: Record }) { + async delete(table: string, opts: { where: Record }) { assertEngineDeleteDispatch(opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { deleted: 0 }; - rows.delete(found.key); + tableOf(table).delete(found.key); return { deleted: 1 }; }, registry: { registerItem: () => {}, registerObject: () => {} }, diff --git a/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts b/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts index db63dec3aa..e61cb4b9a7 100644 --- a/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts +++ b/packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts @@ -98,17 +98,29 @@ const keyOf = (w: Record) => `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}`; function makeProtocol() { - const rows = new Map(); + // ⚠️ Keyed BY TABLE. `find`/`findOne` below answer nothing, so this harness + // cannot serve a `sys_metadata_history` row as a `sys_metadata` row the way + // #16223 measured — but one flat map still made `rows.size` the total of + // every table one save writes. `rows` is the store table these tests assert + // on; the journals the protocol also writes get their own. + const tables = new Map>(); + const tableOf = (table: string): Map => { + const existing = tables.get(table); + if (existing) return existing; + const created = new Map(); + tables.set(table, created); + return created; + }; + const rows = tableOf('sys_metadata'); let nextId = 0; const engine: any = { async findOne(object: string, query?: EngineFindOneQueryInput) { assertEngineFindOnePredicate(object, query); return null; }, async find() { return []; }, async insert(table: string, data: Record) { - if (table === 'sys_metadata_audit') return { id: 'audit_skip' }; nextId += 1; const row = { id: `r_${nextId}`, ...(data as any) } as Row; - rows.set(keyOf(data), row); + tableOf(table).set(keyOf(data), row); return { id: row.id }; }, async update(_t: string, data: Record, opts?: Record) { diff --git a/packages/metadata-protocol/src/protocol.package-closure-gate.test.ts b/packages/metadata-protocol/src/protocol.package-closure-gate.test.ts index 0ef81d25c6..f4c5a03d0d 100644 --- a/packages/metadata-protocol/src/protocol.package-closure-gate.test.ts +++ b/packages/metadata-protocol/src/protocol.package-closure-gate.test.ts @@ -81,14 +81,30 @@ const flowOn = (name: string, objectName: string) => ({ }); function makeStubEngine() { - const rows = new Map(); + // ⚠️ Keyed BY TABLE, and that is a correctness property of this harness + // rather than tidiness. One flat row map answers a read of `sys_metadata` + // with rows the protocol wrote to `sys_metadata_history` and + // `sys_metadata_commit`: a DRAFT save appends a history row carrying no + // `state`, the declared `defaultValue: 'active'` modelled below fills it + // in, and the draft comes back as an ACTIVE metadata row. Measured in + // #16223, where one assertion's polarity was the only thing that caught it. + const tables = new Map>(); + const tableOf = (table: string): Map => { + const existing = tables.get(table); + if (existing) return existing; + const created = new Map(); + tables.set(table, created); + return created; + }; + /** The store table these tests assert on; the journals get their own. */ + const rows = tableOf('sys_metadata'); let nextId = 0; - const findRow = (w: Record): { key: string; row: Row } | null => { + const findRow = (table: string, w: Record): { key: string; row: Row } | null => { if (w.id !== undefined) { - for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + for (const [k, r] of tableOf(table)) if (r.id === w.id) return { key: k, row: r }; return null; } - for (const [k, r] of rows) { + for (const [k, r] of tableOf(table)) { if (w.type !== undefined && r.type !== w.type) continue; if (w.name !== undefined && r.name !== w.name) continue; if (w.organization_id !== undefined && r.organization_id !== w.organization_id) continue; @@ -104,12 +120,12 @@ function makeStubEngine() { [STRANGER]: { manifest: { name: STRANGER, version: '1.0.0' } }, }; const engine: any = { - async findOne(_t: string, opts: { where: Record }) { - assertEngineFindOnePredicate(_t, opts); - return findRow(opts.where)?.row ?? null; + async findOne(table: string, opts: { where: Record }) { + assertEngineFindOnePredicate(table, opts); + return findRow(table, opts.where)?.row ?? null; }, - async find(_t: string, opts: { where: Record }) { - return Array.from(rows.values()).filter((r) => { + async find(table: string, opts: { where: Record }) { + return Array.from(tableOf(table).values()).filter((r) => { if (opts.where.type && r.type !== opts.where.type) return false; if (opts.where.organization_id !== undefined && r.organization_id !== opts.where.organization_id) return false; @@ -117,25 +133,24 @@ function makeStubEngine() { return true; }); }, - async insert(_t: string, data: Record) { - if (_t === 'sys_metadata_audit') return { id: 'audit_skip' }; + async insert(table: string, data: Record) { nextId += 1; const row = { id: `r_${nextId}`, ...(data as any) } as Row; - rows.set(keyOf(data), row); + tableOf(table).set(keyOf(data), row); return { id: row.id }; }, - async update(_t: string, data: Record, opts: { where: Record }) { + async update(table: string, data: Record, opts: { where: Record }) { assertEngineUpdateDispatch(data, opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { id: null }; - rows.set(found.key, { ...found.row, ...(data as any) }); + tableOf(table).set(found.key, { ...found.row, ...(data as any) }); return { id: found.row.id }; }, - async delete(_t: string, opts: { where: Record }) { + async delete(table: string, opts: { where: Record }) { assertEngineDeleteDispatch(opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { deleted: 0 }; - rows.delete(found.key); + tableOf(table).delete(found.key); return { deleted: 1 }; }, registry: { diff --git a/packages/metadata-protocol/src/protocol.platform-schedule-org-gate.test.ts b/packages/metadata-protocol/src/protocol.platform-schedule-org-gate.test.ts index 4d58b887f6..3a3dbe82d6 100644 --- a/packages/metadata-protocol/src/protocol.platform-schedule-org-gate.test.ts +++ b/packages/metadata-protocol/src/protocol.platform-schedule-org-gate.test.ts @@ -326,14 +326,30 @@ const keyOf = (w: Record) => `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}`; function makeStubEngine() { - const rows = new Map(); + // ⚠️ Keyed BY TABLE, and that is a correctness property of this harness + // rather than tidiness. One flat row map answers a read of `sys_metadata` + // with rows the protocol wrote to `sys_metadata_history` and + // `sys_metadata_commit`: a DRAFT save appends a history row carrying no + // `state`, the declared `defaultValue: 'active'` modelled below fills it + // in, and the draft comes back as an ACTIVE metadata row. Measured in + // #16223, where one assertion's polarity was the only thing that caught it. + const tables = new Map>(); + const tableOf = (table: string): Map => { + const existing = tables.get(table); + if (existing) return existing; + const created = new Map(); + tables.set(table, created); + return created; + }; + /** The store table these tests assert on; the journals get their own. */ + const rows = tableOf('sys_metadata'); let nextId = 0; - const findRow = (w: Record): { key: string; row: Row } | null => { + const findRow = (table: string, w: Record): { key: string; row: Row } | null => { if (w.id !== undefined) { - for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + for (const [k, r] of tableOf(table)) if (r.id === w.id) return { key: k, row: r }; return null; } - for (const [k, r] of rows) { + for (const [k, r] of tableOf(table)) { if (w.type !== undefined && r.type !== w.type) continue; if (w.name !== undefined && r.name !== w.name) continue; if (w.organization_id !== undefined && r.organization_id !== w.organization_id) continue; @@ -343,12 +359,12 @@ function makeStubEngine() { return null; }; const engine: any = { - async findOne(_t: string, opts: { where: Record }) { - assertEngineFindOnePredicate(_t, opts); - return findRow(opts.where)?.row ?? null; + async findOne(table: string, opts: { where: Record }) { + assertEngineFindOnePredicate(table, opts); + return findRow(table, opts.where)?.row ?? null; }, - async find(_t: string, opts: { where: Record }) { - return Array.from(rows.values()).filter((r) => { + async find(table: string, opts: { where: Record }) { + return Array.from(tableOf(table).values()).filter((r) => { if (opts.where.type && r.type !== opts.where.type) return false; if (opts.where.organization_id !== undefined && r.organization_id !== opts.where.organization_id) return false; @@ -356,25 +372,24 @@ function makeStubEngine() { return true; }); }, - async insert(_t: string, data: Record) { - if (_t === 'sys_metadata_audit') return { id: 'audit_skip' }; + async insert(table: string, data: Record) { nextId += 1; const row = { id: `r_${nextId}`, ...(data as any) } as Row; - rows.set(keyOf(data), row); + tableOf(table).set(keyOf(data), row); return { id: row.id }; }, - async update(_t: string, data: Record, opts: { where: Record }) { + async update(table: string, data: Record, opts: { where: Record }) { assertEngineUpdateDispatch(data, opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { id: null }; - rows.set(found.key, { ...found.row, ...(data as any) }); + tableOf(table).set(found.key, { ...found.row, ...(data as any) }); return { id: found.row.id }; }, - async delete(_t: string, opts: { where: Record }) { + async delete(table: string, opts: { where: Record }) { assertEngineDeleteDispatch(opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { deleted: 0 }; - rows.delete(found.key); + tableOf(table).delete(found.key); return { deleted: 1 }; }, registry: { diff --git a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts index 2ec5ee7b8c..4463447560 100644 --- a/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts +++ b/packages/metadata-protocol/src/protocol.runtime-authoring-gate.test.ts @@ -81,14 +81,30 @@ const keyOf = (w: Record) => `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}`; function makeStubEngine() { - const rows = new Map(); + // ⚠️ Keyed BY TABLE, and that is a correctness property of this harness + // rather than tidiness. One flat row map answers a read of `sys_metadata` + // with rows the protocol wrote to `sys_metadata_history` and + // `sys_metadata_commit`: a DRAFT save appends a history row carrying no + // `state`, the declared `defaultValue: 'active'` modelled below fills it + // in, and the draft comes back as an ACTIVE metadata row. Measured in + // #16223, where one assertion's polarity was the only thing that caught it. + const tables = new Map>(); + const tableOf = (table: string): Map => { + const existing = tables.get(table); + if (existing) return existing; + const created = new Map(); + tables.set(table, created); + return created; + }; + /** The store table these tests assert on; the journals get their own. */ + const rows = tableOf('sys_metadata'); let nextId = 0; - const findRow = (w: Record): { key: string; row: Row } | null => { + const findRow = (table: string, w: Record): { key: string; row: Row } | null => { if (w.id !== undefined) { - for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + for (const [k, r] of tableOf(table)) if (r.id === w.id) return { key: k, row: r }; return null; } - for (const [k, r] of rows) { + for (const [k, r] of tableOf(table)) { if (w.type !== undefined && r.type !== w.type) continue; if (w.name !== undefined && r.name !== w.name) continue; if (w.organization_id !== undefined && r.organization_id !== w.organization_id) continue; @@ -98,12 +114,12 @@ function makeStubEngine() { return null; }; const engine: any = { - async findOne(_t: string, opts: { where: Record }) { - assertEngineFindOnePredicate(_t, opts); - return findRow(opts.where)?.row ?? null; + async findOne(table: string, opts: { where: Record }) { + assertEngineFindOnePredicate(table, opts); + return findRow(table, opts.where)?.row ?? null; }, - async find(_t: string, opts: { where: Record }) { - return Array.from(rows.values()).filter((r) => { + async find(table: string, opts: { where: Record }) { + return Array.from(tableOf(table).values()).filter((r) => { if (opts.where.type && r.type !== opts.where.type) return false; if (opts.where.organization_id !== undefined && r.organization_id !== opts.where.organization_id) return false; @@ -111,25 +127,24 @@ function makeStubEngine() { return true; }); }, - async insert(_t: string, data: Record) { - if (_t === 'sys_metadata_audit') return { id: 'audit_skip' }; + async insert(table: string, data: Record) { nextId += 1; const row = { id: `r_${nextId}`, ...(data as any) } as Row; - rows.set(keyOf(data), row); + tableOf(table).set(keyOf(data), row); return { id: row.id }; }, - async update(_t: string, data: Record, opts: { where: Record }) { + async update(table: string, data: Record, opts: { where: Record }) { assertEngineUpdateDispatch(data, opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { id: null }; - rows.set(found.key, { ...found.row, ...(data as any) }); + tableOf(table).set(found.key, { ...found.row, ...(data as any) }); return { id: found.row.id }; }, - async delete(_t: string, opts: { where: Record }) { + async delete(table: string, opts: { where: Record }) { assertEngineDeleteDispatch(opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { deleted: 0 }; - rows.delete(found.key); + tableOf(table).delete(found.key); return { deleted: 1 }; }, registry: { diff --git a/packages/metadata-protocol/src/protocol.save-flow-canonicalization.test.ts b/packages/metadata-protocol/src/protocol.save-flow-canonicalization.test.ts index 8dd0454287..183ed68678 100644 --- a/packages/metadata-protocol/src/protocol.save-flow-canonicalization.test.ts +++ b/packages/metadata-protocol/src/protocol.save-flow-canonicalization.test.ts @@ -82,14 +82,30 @@ function keyOf(w: Record) { /** The engine surface the repository write path touches. */ function makeStubEngine() { - const rows = new Map(); + // ⚠️ Keyed BY TABLE, and that is a correctness property of this harness + // rather than tidiness. One flat row map answers a read of `sys_metadata` + // with rows the protocol wrote to `sys_metadata_history` and + // `sys_metadata_commit`: a DRAFT save appends a history row carrying no + // `state`, the declared `defaultValue: 'active'` modelled below fills it + // in, and the draft comes back as an ACTIVE metadata row. Measured in + // #16223, where one assertion's polarity was the only thing that caught it. + const tables = new Map>(); + const tableOf = (table: string): Map => { + const existing = tables.get(table); + if (existing) return existing; + const created = new Map(); + tables.set(table, created); + return created; + }; + /** The store table these tests assert on; the journals get their own. */ + const rows = tableOf('sys_metadata'); let nextId = 0; - const findRow = (w: Record): { key: string; row: Row } | null => { + const findRow = (table: string, w: Record): { key: string; row: Row } | null => { if (w.id !== undefined) { - for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + for (const [k, r] of tableOf(table)) if (r.id === w.id) return { key: k, row: r }; return null; } - for (const [k, r] of rows) { + for (const [k, r] of tableOf(table)) { if (w.type !== undefined && r.type !== w.type) continue; if (w.name !== undefined && r.name !== w.name) continue; if (w.organization_id !== undefined && r.organization_id !== w.organization_id) continue; @@ -99,12 +115,12 @@ function makeStubEngine() { return null; }; const engine: any = { - async findOne(_t: string, opts: { where: Record }) { - assertEngineFindOnePredicate(_t, opts); - return findRow(opts.where)?.row ?? null; + async findOne(table: string, opts: { where: Record }) { + assertEngineFindOnePredicate(table, opts); + return findRow(table, opts.where)?.row ?? null; }, - async find(_t: string, opts: { where: Record }) { - return Array.from(rows.values()).filter((r) => { + async find(table: string, opts: { where: Record }) { + return Array.from(tableOf(table).values()).filter((r) => { if (opts.where.type && r.type !== opts.where.type) return false; if (opts.where.organization_id !== undefined && r.organization_id !== opts.where.organization_id) return false; @@ -112,25 +128,24 @@ function makeStubEngine() { return true; }); }, - async insert(_t: string, data: Record) { - if (_t === 'sys_metadata_audit') return { id: 'audit_skip' }; + async insert(table: string, data: Record) { nextId += 1; const row = { id: `r_${nextId}`, ...(data as any) } as Row; - rows.set(keyOf(data), row); + tableOf(table).set(keyOf(data), row); return { id: row.id }; }, - async update(_t: string, data: Record, opts: { where: Record }) { + async update(table: string, data: Record, opts: { where: Record }) { assertEngineUpdateDispatch(data, opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { id: null }; - rows.set(found.key, { ...found.row, ...(data as any) }); + tableOf(table).set(found.key, { ...found.row, ...(data as any) }); return { id: found.row.id }; }, - async delete(_t: string, opts: { where: Record }) { + async delete(table: string, opts: { where: Record }) { assertEngineDeleteDispatch(opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { deleted: 0 }; - rows.delete(found.key); + tableOf(table).delete(found.key); return { deleted: 1 }; }, registry: { diff --git a/packages/metadata-protocol/src/protocol.save-union-issues.test.ts b/packages/metadata-protocol/src/protocol.save-union-issues.test.ts index ea2d86a544..08b171c83a 100644 --- a/packages/metadata-protocol/src/protocol.save-union-issues.test.ts +++ b/packages/metadata-protocol/src/protocol.save-union-issues.test.ts @@ -48,17 +48,29 @@ const keyOf = (w: Record) => /** The engine surface the repository write path touches. */ function makeProtocol() { - const rows = new Map(); + // ⚠️ Keyed BY TABLE. `find`/`findOne` below answer nothing, so this harness + // cannot serve a `sys_metadata_history` row as a `sys_metadata` row the way + // #16223 measured — but one flat map still made `rows.size` the total of + // every table one save writes. `rows` is the store table these tests assert + // on; the journals the protocol also writes get their own. + const tables = new Map>(); + const tableOf = (table: string): Map => { + const existing = tables.get(table); + if (existing) return existing; + const created = new Map(); + tables.set(table, created); + return created; + }; + const rows = tableOf('sys_metadata'); let nextId = 0; const engine: any = { async findOne(object: string, query?: EngineFindOneQueryInput) { assertEngineFindOnePredicate(object, query); return null; }, async find() { return []; }, async insert(table: string, data: Record) { - if (table === 'sys_metadata_audit') return { id: 'audit_skip' }; nextId += 1; const row = { id: `r_${nextId}`, ...(data as any) } as Row; - rows.set(keyOf(data), row); + tableOf(table).set(keyOf(data), row); return { id: row.id }; }, async update(_t: string, data: Record, opts?: Record) { diff --git a/packages/objectql/src/protocol-save-meta-repo-path.test.ts b/packages/objectql/src/protocol-save-meta-repo-path.test.ts index 0571087d04..21d16f2509 100644 --- a/packages/objectql/src/protocol-save-meta-repo-path.test.ts +++ b/packages/objectql/src/protocol-save-meta-repo-path.test.ts @@ -35,29 +35,45 @@ function keyOf(w: Record) { } function makeStubEngine() { - const rows = new Map(); + // ⚠️ Keyed BY TABLE, and that is a correctness property of this harness + // rather than tidiness. One flat row map answers a read of `sys_metadata` + // with rows the protocol wrote to `sys_metadata_history` and + // `sys_metadata_commit`: a DRAFT save appends a history row carrying no + // `state`, the declared `defaultValue: 'active'` modelled below fills it + // in, and the draft comes back as an ACTIVE metadata row. Measured in + // #16223, where one assertion's polarity was the only thing that caught it. + const tables = new Map>(); + const tableOf = (table: string): Map => { + const existing = tables.get(table); + if (existing) return existing; + const created = new Map(); + tables.set(table, created); + return created; + }; + /** The store table these tests assert on; the journals get their own. */ + const rows = tableOf('sys_metadata'); let nextId = 0; - const findRow = (w: Record): { key: string; row: Row } | null => { + const findRow = (table: string, w: Record): { key: string; row: Row } | null => { if (w.id !== undefined) { - for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + for (const [k, r] of tableOf(table)) if (r.id === w.id) return { key: k, row: r }; return null; } const k = keyOf(w); - const r = rows.get(k); + const r = tableOf(table).get(k); return r ? { key: k, row: r } : null; }; const engine: any = { - async findOne(_t: string, opts: { where: Record }) { + async findOne(table: string, opts: { where: Record }) { // [#11957] Pinned to ObjectQL.findOne's OWN #4419 predicate: `findOne` // applies limit: 1, so a query naming no record returns an ARBITRARY row // and the engine REFUSES it. A double that answers it anyway is how // #11767 shipped a bootstrap bypass that was inert on every real // deployment while a 641-line unit matrix stayed green. - assertEngineFindOnePredicate(_t, opts); - return findRow(opts.where)?.row ?? null; + assertEngineFindOnePredicate(table, opts); + return findRow(table, opts.where)?.row ?? null; }, - async find(_t: string, opts: { where: Record }) { - return Array.from(rows.values()).filter((r) => { + async find(table: string, opts: { where: Record }) { + return Array.from(tableOf(table).values()).filter((r) => { if (opts.where.type && r.type !== opts.where.type) return false; if (opts.where.organization_id !== undefined && r.organization_id !== opts.where.organization_id) return false; @@ -65,32 +81,31 @@ function makeStubEngine() { return true; }); }, - async insert(_t: string, data: Record) { - if (_t === 'sys_metadata_audit') return { id: 'audit_skip' }; + async insert(table: string, data: Record) { nextId += 1; const row = { id: `r_${nextId}`, ...(data as any) } as Row; - rows.set(keyOf(data), row); + tableOf(table).set(keyOf(data), row); return { id: row.id }; }, - async update(_t: string, data: Record, opts: { where: Record }) { + async update(table: string, data: Record, opts: { where: Record }) { // [#5480] Pinned to ObjectQL.update's OWN dispatch predicate, the // twin of the delete pin below and on the same argument: this file // could bind one write verb to the producer and not the other only // because `update` had no shared predicate to bind to. assertEngineUpdateDispatch(data, opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { id: null }; - rows.set(found.key, { ...found.row, ...(data as any) }); + tableOf(table).set(found.key, { ...found.row, ...(data as any) }); return { id: found.row.id }; }, - async delete(_t: string, opts: { where: Record }) { + async delete(table: string, opts: { where: Record }) { // [#4550] Pinned to ObjectQL.delete's OWN dispatch predicate. A double // looser than the engine it stands in for is how #4434 shipped a REST // route that answered 500 to every caller with its suite green. assertEngineDeleteDispatch(opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { deleted: 0 }; - rows.delete(found.key); + tableOf(table).delete(found.key); return { deleted: 1 }; }, registry: { @@ -98,7 +113,7 @@ function makeStubEngine() { registerObject: () => {}, }, }; - return { engine, rows }; + return { engine, rows, tableOf }; } describe('saveMetaItem — repository write path (post PR-10d.6)', () => { @@ -392,3 +407,68 @@ describe('saveMetaItem — repository write path (post PR-10d.6)', () => { expect(create!.package_id).toBe('app.objectstack.hotcrm'); }); }); + +// ----------------------------------------------------------------------------- +// #16225 — the stub answers the table it was asked about, and only that one +// ----------------------------------------------------------------------------- +// +// The objectql half of the pin that makes the table-keying above a MEASUREMENT +// and not a rename: every other test in this file passes identically with the +// maps merged back into one, because none of them reads `sys_metadata` as a +// table. One `saveMetaItem` writes `sys_metadata`, `sys_metadata_history` and +// `sys_metadata_audit`, all three addressed to the same `(type, name)`, and the +// two journal rows carry no `state` of their own — so a flat map serves them +// back to a `sys_metadata` read as rows of the store (#16223). +describe('#16225 a `sys_metadata` read is not answered from the journal tables', () => { + it('serves the store row only, never the history/audit rows the same save wrote', async () => { + const { engine, tableOf } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + const result = await protocol.saveMetaItem({ + type: 'view', + name: 'case_grid', + organizationId: 'org_alpha', + item: { + name: 'case_grid', type: 'grid', label: 'Cases', + columns: ['id', 'title'], object: 'case', viewKind: 'list', + }, + }); + expect(result.success).toBe(true); + + const journal = [ + ...tableOf('sys_metadata_history').values(), + ...tableOf('sys_metadata_commit').values(), + ...tableOf('sys_metadata_audit').values(), + ] as Partial[]; + + // The firing control. Without it a green here would be consistent with + // this save having stopped writing the journals altogether, which + // measures nothing about which table answered the read below. + expect( + journal.length, + 'the firing control: one save must WRITE the journal tables, or the read below ' + + 'proves nothing about which table answered it', + ).toBeGreaterThan(0); + + // ── The pin ────────────────────────────────────────────────────── + // Asserted on STORE-ONLY columns rather than on the row count. A + // journal row is addressed to the same `(type, name, organization_id)` + // as the store row, which is this file's whole `keyOf`, so under a + // merged map the audit row simply OVERWRITES the store row and a + // count- or name-based assertion still reads one row called + // `case_grid`. `checksum` and `state` are written by the store leg + // alone; a journal row carries neither. + const stored = await engine.find('sys_metadata', { + where: { type: 'view', organization_id: 'org_alpha' }, + }); + expect( + stored.map((r: Row) => r.name), + 'a `sys_metadata` read must answer with the store row and nothing else', + ).toEqual(['case_grid']); + expect( + stored.map((r: Row) => [typeof r.checksum, r.state]), + 'and the row it answers with must be a STORE row, not a journal row wearing ' + + 'the same `(type, name, organization_id)`', + ).toEqual([['string', 'active']]); + }); +}); diff --git a/packages/objectql/src/protocol-view-identity-overlay.test.ts b/packages/objectql/src/protocol-view-identity-overlay.test.ts index b07bf6bca9..416fd758ef 100644 --- a/packages/objectql/src/protocol-view-identity-overlay.test.ts +++ b/packages/objectql/src/protocol-view-identity-overlay.test.ts @@ -53,61 +53,76 @@ interface Row { } function makeStubEngine(registryViews: Record = {}) { - const rows = new Map(); + // ⚠️ Keyed BY TABLE, and that is a correctness property of this harness + // rather than tidiness. One flat row map answers a read of `sys_metadata` + // with rows the protocol wrote to `sys_metadata_history` and + // `sys_metadata_commit`: a DRAFT save appends a history row carrying no + // `state`, the declared `defaultValue: 'active'` modelled below fills it + // in, and the draft comes back as an ACTIVE metadata row. Measured in + // #16223, where one assertion's polarity was the only thing that caught it. + const tables = new Map>(); + const tableOf = (table: string): Map => { + const existing = tables.get(table); + if (existing) return existing; + const created = new Map(); + tables.set(table, created); + return created; + }; + /** The store table these tests assert on; the journals get their own. */ + const rows = tableOf('sys_metadata'); let nextId = 0; const keyOf = (w: Record) => `${w.type}|${w.name}|${w.organization_id ?? '__env__'}`; - const findRow = (w: Record) => { + const findRow = (table: string, w: Record) => { if (w.id !== undefined) { - for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + for (const [k, r] of tableOf(table)) if (r.id === w.id) return { key: k, row: r }; return null; } - const r = rows.get(keyOf(w)); + const r = tableOf(table).get(keyOf(w)); return r ? { key: keyOf(w), row: r } : null; }; const engine: any = { - async findOne(_t: string, opts: { where: Record }) { + async findOne(table: string, opts: { where: Record }) { // [#11957] Pinned to ObjectQL.findOne's OWN #4419 predicate: `findOne` // applies limit: 1, so a query naming no record returns an ARBITRARY row // and the engine REFUSES it. A double that answers it anyway is how // #11767 shipped a bootstrap bypass that was inert on every real // deployment while a 641-line unit matrix stayed green. - assertEngineFindOnePredicate(_t, opts); - return findRow(opts.where)?.row ?? null; + assertEngineFindOnePredicate(table, opts); + return findRow(table, opts.where)?.row ?? null; }, - async find(_t: string, opts: { where: Record }) { - return Array.from(rows.values()).filter((r) => { + async find(table: string, opts: { where: Record }) { + return Array.from(tableOf(table).values()).filter((r) => { if (opts.where.type && r.type !== opts.where.type) return false; if (opts.where.organization_id !== undefined && r.organization_id !== opts.where.organization_id) return false; if (opts.where.state && r.state !== opts.where.state) return false; return true; }); }, - async insert(_t: string, data: Record) { - if (_t === 'sys_metadata_audit') return { id: 'audit_skip' }; + async insert(table: string, data: Record) { nextId += 1; const row = { id: `r_${nextId}`, ...(data as any) } as Row; - rows.set(keyOf(data), row); + tableOf(table).set(keyOf(data), row); return { id: row.id }; }, - async update(_t: string, data: Record, opts: { where: Record }) { + async update(table: string, data: Record, opts: { where: Record }) { // [#5480] Pinned to ObjectQL.update's OWN dispatch predicate, the // twin of the delete pin below and on the same argument: this file // could bind one write verb to the producer and not the other only // because `update` had no shared predicate to bind to. assertEngineUpdateDispatch(data, opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { id: null }; - rows.set(found.key, { ...found.row, ...(data as any) }); + tableOf(table).set(found.key, { ...found.row, ...(data as any) }); return { id: found.row.id }; }, - async delete(_t: string, opts: { where: Record }) { + async delete(table: string, opts: { where: Record }) { // [#4550] Pinned to ObjectQL.delete's OWN dispatch predicate. A double // looser than the engine it stands in for is how #4434 shipped a REST // route that answered 500 to every caller with its suite green. assertEngineDeleteDispatch(opts); - const found = findRow(opts.where); + const found = findRow(table, opts.where); if (!found) return { deleted: 0 }; - rows.delete(found.key); + tableOf(table).delete(found.key); return { deleted: 1 }; }, registry: {