From 67bc6b03c544f413174acc47d25e2b0f1b80ce34 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 11:39:55 +0000 Subject: [PATCH 1/2] fix(metadata-protocol): put and delete accept the version a checksum-less row is served as SysMetadataRepository served a row with no `checksum` as the hash of its stored body (`rowToItem`), but `put` and `delete` judged the caller's parent against the raw column (`null`). Such a row could never be written or removed through the metadata door: every save and delete, an unpinned one included, answered 409 METADATA_CONFLICT. One helper (`servedVersion`) now names the version a row is served as, and the lock (`lockAccepts`) accepts it as the head of a checksum-less row. A row with a `checksum` is judged exactly as before, a `null` parent still matches a checksum-less row, and the next write stamps the row. A conflict on such a row reports its served version. No stored row is rewritten. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude --- .../src/sys-metadata-repository.ts | 104 ++++++++++++++++-- 1 file changed, 96 insertions(+), 8 deletions(-) diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index 80789431dc..dc58518587 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -33,7 +33,11 @@ * The watch() contract is scoped to the local repository instance. * Multi-replica deployments are not a supported topology for the * metadata overlay — see ADR-0008 §11. - * - hashSpec backfill for legacy rows missing `checksum` + * - hashSpec backfill for legacy rows missing `checksum`. Such a row is + * not rewritten at rest: it is served as the hash of its stored body, and + * `put` / `delete` accept that served version as its head + * ({@link SysMetadataRepository.servedVersion}, #21978). The next write + * stamps the row's `checksum` as usual. * * Schema mapping (ADR-0008 PR-10d.2): * Repository concept sys_metadata column @@ -180,6 +184,17 @@ function canonicalIsoInstant(value: unknown): string | undefined { return String(value); } +/** + * A `sys_metadata` row's stored body, parsed — the body {@link + * SysMetadataRepository.rowToItem} serves and {@link + * SysMetadataRepository.servedVersion} hashes, so the two read one value. + * + * @throws The `JSON.parse` error, unchanged, for stored bytes that do not parse. + */ +function storedRowBody(row: any): Record { + return typeof row.metadata === 'string' ? JSON.parse(row.metadata) : (row.metadata ?? {}); +} + /** * Overlay-row lifecycle state. * @@ -631,9 +646,14 @@ export class SysMetadataRepository implements MetadataRepository { context: { ...ctx, isSystem: true }, }); } + // The row's stored stamp, as written: the parent link this write records + // (`previous_checksum`, the event's `parentHash`) and the no-op check's + // input. `null` for a row stored without one. const existingHash: string | null = existing?.checksum ?? null; - if (opts.parentVersion !== existingHash) { - throw new ConflictError(this.fullRef(ref), opts.parentVersion, existingHash); + // [#21978] The lock judges the parent against the version the row is + // SERVED as — see {@link lockAccepts}. + if (!this.lockAccepts(ref, existing, opts.parentVersion)) { + throw new ConflictError(this.fullRef(ref), opts.parentVersion, this.lockHead(ref, existing)); } // No-op short-circuit: identical body → no write, no history row, @@ -812,9 +832,11 @@ export class SysMetadataRepository implements MetadataRepository { if (!existing) { throw new ConflictError(this.fullRef(ref), opts.parentVersion, null); } + // The stored stamp, as written: the tombstone's `previous_checksum`. const existingHash: string | null = existing.checksum ?? null; - if (opts.parentVersion !== existingHash) { - throw new ConflictError(this.fullRef(ref), opts.parentVersion, existingHash); + // [#21978] The same lock as `put`'s — see {@link lockAccepts}. + if (!this.lockAccepts(ref, existing, opts.parentVersion)) { + throw new ConflictError(this.fullRef(ref), opts.parentVersion, this.lockHead(ref, existing)); } const existingId = (existing as { id?: string }).id; @@ -2007,10 +2029,76 @@ export class SysMetadataRepository implements MetadataRepository { }; } + /** + * [#21978] The version a stored row is SERVED as — the one value this + * repository hands out for it and accepts back as its head. + * + * The stored `checksum` when the row carries one. A row stored without one + * is served as the hash of its stored body, computed exactly as `put` stamps + * a write (`hashSpec(body, type)`, hashed as its type). Every read serves it + * through {@link rowToItem}: `get`, `list`, and the parents `promoteDraft`, + * `restoreVersion` and the post-promotion drain pass to `put` / `delete`. + * Those two take it back through {@link lockAccepts}. + * + * ⛔ Not a backfill: nothing is written here. The next write stamps the row. + * + * @param body - The row's parsed body, when the caller already holds it. + * @throws The `JSON.parse` error for a row with no `checksum` whose stored + * bytes do not parse — the read cannot serve that row either. + */ + private servedVersion(ref: Pick, row: any, body?: Record): string { + return row.checksum ?? hashSpec(body ?? storedRowBody(row), ref.type); + } + + /** + * [#21978] The head `put` / `delete` report in a {@link ConflictError} for + * `row`: its {@link servedVersion}, so a conflict names the version a read + * hands out. `null` when there is no row, or when a row with no `checksum` + * has stored bytes that do not parse — no read serves a version for it, and + * a lock refusal must not turn into a parse failure. + */ + private lockHead(ref: Pick, row: any): string | null { + if (!row) return null; + if (row.checksum != null) return row.checksum as string; + let body: Record; + try { + body = storedRowBody(row); + } catch { + return null; + } + return this.servedVersion(ref, row, body); + } + + /** + * [#21978] Does `parent` name the head of `row` (`null` = no row)? The + * optimistic lock of `put` and `delete`. + * + * A row's head is the version it is SERVED as ({@link servedVersion}). The + * lock used to compare against the raw `checksum` column instead, so a row + * stored without one — the datasource admin door wrote such rows before it + * stamped them — was served one version and judged against `null`: every + * write and delete through the metadata door answered `409 + * METADATA_CONFLICT`, an unpinned (last-write-wins) one included, since the + * door takes the parent from the same read. + * + * Accepted, and nothing else: + * - the row's stored stamp — the old compare, unchanged. For a row with a + * `checksum` this is its served version, so such a row is judged exactly + * as before; for a row with none it is `null`, which some writers pass + * for such a row today (`migrateStoredMetadata` hands the raw column on), + * and that match stays; + * - for a row with no `checksum`, its served version. + */ + private lockAccepts(ref: Pick, row: any, parent: string | null): boolean { + const stamped: string | null = row?.checksum ?? null; + if (parent === stamped) return true; + return stamped === null && parent !== null && parent === this.lockHead(ref, row); + } + private rowToItem(ref: Pick, row: any): MetadataItem { - const body: Record = - typeof row.metadata === 'string' ? JSON.parse(row.metadata) : (row.metadata ?? {}); - const hash: string = row.checksum ?? hashSpec(body, ref.type); + const body = storedRowBody(row); + // [#21978] The one served version — the head `put` / `delete` accept. + const hash: string = this.servedVersion(ref, row, body); return { ref: this.fullRef(ref), body, From 5c4815a6ab94b4bee37a7971ae4f2573c3f7b063 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 11:46:54 +0000 Subject: [PATCH 2/2] test(metadata-protocol): pin the metadata door on a stored row with no checksum A checksum-less `sys_metadata` row is saved, deleted and published over through the version its read serves (with and without a pinned parent), a stale version is still refused with 409 METADATA_CONFLICT naming the served version, a null parent still matches it, the write stamps it, the post-promotion drain removes such a draft, and a row with a checksum is judged exactly as before. Adds the patch changeset. Claude-Session: https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi Co-authored-by: Claude --- .../21978-checksum-less-row-served-version.md | 12 + ...tocol-publish-drafts-package-scope.test.ts | 8 +- .../src/protocol.served-content-hash.test.ts | 231 ++++++++++++++++++ 3 files changed, 247 insertions(+), 4 deletions(-) create mode 100644 .changeset/21978-checksum-less-row-served-version.md diff --git a/.changeset/21978-checksum-less-row-served-version.md b/.changeset/21978-checksum-less-row-served-version.md new file mode 100644 index 0000000000..c6d515e495 --- /dev/null +++ b/.changeset/21978-checksum-less-row-served-version.md @@ -0,0 +1,12 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +A metadata row stored with no `checksum` can be edited and removed through the metadata door (#21978) + +Clause-②: no + +- `SysMetadataRepository` serves a `sys_metadata` row that carries no `checksum` as the hash of its stored body, but its `put` and `delete` compared the caller's parent with the raw column (`null`). So `PUT` and `DELETE /api/v1/meta/:type/:name` answered `409 METADATA_CONFLICT` ("Expected parent … but current is null") for every such row, with `If-Match` set to the version the door served and with no `If-Match` (last-write-wins) alike. A publish over such a row was refused the same way, as were the rollback and commit-revert doors, which take their parent from the same read. The datasource admin door stored such rows before it stamped them. +- `put` and `delete` now accept the version such a row is served as. A `null` parent still matches it, and a row with a `checksum` is judged exactly as before. A stale version is still refused with `409 METADATA_CONFLICT`, and the refusal now names the row's served version as the current one instead of `null`. +- The next write stamps the row's `checksum`, as every write does. Stored rows are not rewritten. +- Publishing a draft row stored with no `checksum` now also removes that draft row. Before, the post-promotion cleanup was refused by the same lock and the draft stayed pending, with nothing reported. diff --git a/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts b/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts index df942081fa..8ae5cd629f 100644 --- a/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts +++ b/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts @@ -621,10 +621,10 @@ describe('publishMetaItem — the scope probes ask the promote\'s question (#110 // must not come from `saveMetaItem(mode:'draft')` while PR #11139 is // changing that path's binding resolution. The shape mirrors what the // repository's `put` writes for a package-less org draft — `checksum` - // included: the post-promotion drain is an optimistic-lock delete - // keyed on it, and a checksum-less row makes the drain read as the - // benign "newer draft saved" race and survive (measured on this - // fixture's first run). + // included. (On this fixture's first run a checksum-less row made the + // post-promotion drain read as the benign "newer draft saved" race and + // survive; since #21978 the drain's lock accepts such a row's served + // version, and the stamp stays only so the row is the one `put` writes.) const noPackageBody = objectBody('shared_ticket', 'NO_PACKAGE'); await engine.insert('sys_metadata', { type: 'object', diff --git a/packages/metadata-protocol/src/protocol.served-content-hash.test.ts b/packages/metadata-protocol/src/protocol.served-content-hash.test.ts index 2a69d3c06c..aa2b6908a3 100644 --- a/packages/metadata-protocol/src/protocol.served-content-hash.test.ts +++ b/packages/metadata-protocol/src/protocol.served-content-hash.test.ts @@ -30,6 +30,11 @@ * own read and write shapes (the `protocol.lifecycle-audit-rows.test.ts` double, * plus the engine's `getKeyedDigest` accessor), so the stored values compared * against are the ones the real repository writes. + * + * [#21978] The last two blocks pin the same doors on a stored row with NO + * `checksum`: it is saved, deleted and published over through the version its + * read serves, a stale version is still refused, a `null` parent still matches + * it, the write stamps it, and a row WITH a `checksum` is judged as before. */ import { createHmac } from 'node:crypto'; @@ -38,9 +43,11 @@ import { assertEngineDeleteDispatch, assertEngineFindOnePredicate, assertEngineUpdateDispatch, + ConflictError, hashSpec, } from '@objectstack/metadata-core'; import { ObjectStackProtocolImplementation } from './protocol.js'; +import { SysMetadataRepository } from './sys-metadata-repository.js'; const TEST_KEY = 'served-content-hash-test-key'; /** A keyed digest with the provider contract's output shape, under a key this file holds. */ @@ -462,3 +469,227 @@ describe('[#21207] a change note that quotes a stored hash', () => { } }); }); + +// --------------------------------------------------------------------------- +// [#21978] A stored row with no `checksum` +// --------------------------------------------------------------------------- +// +// The datasource admin door stored its rows with no `checksum` before it +// stamped them, and such rows stay at rest (no backfill). The repository serves +// a row like that as the hash of its stored body; its `put` / `delete` used to +// judge the caller's parent against the raw column (`null`) instead, so every +// save and delete of such a row through the metadata door answered 409 — the +// unpinned (last-write-wins) ones included, since the door takes the parent +// from the same read. The lock is type-agnostic, so this file's `view` row +// stands in for the datasource one. + +const VIEW_REF = { type: 'view', name: 'case_grid', org: ORG } as const; + +/** Store the active row the way a writer that stamps no `checksum` did. */ +async function seedUnstamped(h: ReturnType, label = 'legacy', state = 'active'): Promise { + await h.engine.insert('sys_metadata', { + type: 'view', + name: 'case_grid', + organization_id: ORG, + package_id: null, + state, + metadata: JSON.stringify(viewBody(label)), + version: 1, + }); +} + +function caseGridRow(h: ReturnType, state = 'active'): Row | undefined { + return [...h.rows.values()].find((r) => r.name === 'case_grid' && r.state === state); +} + +function repoFor(h: ReturnType): SysMetadataRepository { + return new SysMetadataRepository({ engine: h.engine, organizationId: ORG, orgLabel: ORG }); +} + +/** The version the repository's own read serves for the row, keyed as a door hands it out. */ +async function servedToken(h: ReturnType): Promise { + const item = await repoFor(h).get(VIEW_REF as any); + expect(item).not.toBeNull(); + return keyedDigest(item!.hash); +} + +describe('[#21978] a stored row with no checksum is written through the version its read serves', () => { + it('save door: the served version is accepted as the parent, and the write stamps the row', async () => { + const h = makeEngine(); + const p = new ObjectStackProtocolImplementation(h.engine); + await seedUnstamped(h); + expect(caseGridRow(h)?.checksum).toBeUndefined(); + + const token = await servedToken(h); + // The read serves the hash of the stored body, as `put` would stamp it. + expect(token).toBe(await keyedDigest(hashSpec(viewBody('legacy'), 'view'))); + + const saved: any = await p.saveMetaItem({ ...ref, item: viewBody('edited'), parentVersion: token } as any); + expect(saved.success).toBe(true); + expect(JSON.parse(caseGridRow(h)!.metadata).label).toBe('edited'); + // The write stamped the row: it now carries the checksum of its new body. + expect(caseGridRow(h)!.checksum).toBe(hashSpec(viewBody('edited'), 'view')); + expect(saved.version).toBe(await keyedDigest(caseGridRow(h)!.checksum!)); + }); + + it('reset door: the served version is accepted as the parent, and the row is removed', async () => { + const h = makeEngine(); + const p = new ObjectStackProtocolImplementation(h.engine); + await seedUnstamped(h); + + const reset: any = await p.deleteMetaItem({ ...ref, parentVersion: await servedToken(h) } as any); + expect(reset.success).toBe(true); + expect(caseGridRow(h)).toBeUndefined(); + }); + + it('unpinned save and delete (last-write-wins) succeed; an identical re-save stamps the row too', async () => { + const saveSide = makeEngine(); + const p = new ObjectStackProtocolImplementation(saveSide.engine); + await seedUnstamped(saveSide); + const saved: any = await p.saveMetaItem({ ...ref, item: viewBody('legacy') } as any); + expect(saved.success).toBe(true); + expect(caseGridRow(saveSide)!.checksum).toBe(hashSpec(viewBody('legacy'), 'view')); + + const deleteSide = makeEngine(); + const q = new ObjectStackProtocolImplementation(deleteSide.engine); + await seedUnstamped(deleteSide); + const reset: any = await q.deleteMetaItem({ ...ref } as any); + expect(reset.success).toBe(true); + expect(caseGridRow(deleteSide)).toBeUndefined(); + }); + + it('a stale version is still refused (METADATA_CONFLICT / 409), the refusal names the served version, and that version is then accepted', async () => { + const h = makeEngine(); + const p = new ObjectStackProtocolImplementation(h.engine); + await seedUnstamped(h); + const served = await servedToken(h); + + const staleTokens = [ + await keyedDigest(hashSpec(viewBody('someone else'), 'view')), + // The served hash in stored (unkeyed) form stays refused at the door. + hashSpec(viewBody('legacy'), 'view'), + ]; + let saveRefusal: any; + for (const token of staleTokens) { + for (const [door, run] of [ + ['save', () => p.saveMetaItem({ ...ref, item: viewBody('lost'), parentVersion: token } as any)], + ['delete', () => p.deleteMetaItem({ ...ref, parentVersion: token } as any)], + ] as const) { + const refused = await rejection(run); + expect(refused.code, `${door} with ${token}`).toBe('METADATA_CONFLICT'); + expect(refused.status, `${door} with ${token}`).toBe(409); + expect(refused.actualHead, `${door} with ${token}`).toBe(served); + if (door === 'save') saveRefusal = refused; + } + } + // Nothing was written: the row is the one stored, still unstamped. + expect(JSON.parse(caseGridRow(h)!.metadata).label).toBe('legacy'); + expect(caseGridRow(h)!.checksum).toBeUndefined(); + + const after: any = await p.saveMetaItem({ ...ref, item: viewBody('edited'), parentVersion: saveRefusal.actualHead } as any); + expect(after.success).toBe(true); + expect(caseGridRow(h)!.checksum).toBe(hashSpec(viewBody('edited'), 'view')); + }); + + it('a writer passing null for such a row still succeeds (the stored-row migration hands the raw column on)', async () => { + const h = makeEngine(); + const p = new ObjectStackProtocolImplementation(h.engine); + await seedUnstamped(h); + + // `migrateStoredMetadata`'s in-process spelling: `row.checksum ?? null`. + const saved: any = await p.saveMetaItem({ + ...ref, + item: viewBody('migrated'), + storedParentVersion: caseGridRow(h)!.checksum ?? null, + } as any); + expect(saved.success).toBe(true); + expect(caseGridRow(h)!.checksum).toBe(hashSpec(viewBody('migrated'), 'view')); + }); + + it('publish over such a row: the promotion takes the active row\'s served version as its parent', async () => { + const h = makeEngine(); + const p = new ObjectStackProtocolImplementation(h.engine); + await seedUnstamped(h); + await p.saveMetaItem({ ...ref, item: viewBody('staged'), mode: 'draft' } as any); + + const published: any = await p.publishMetaItem({ ...ref } as any); + expect(published.version).toBe(await keyedDigest(hashSpec(viewBody('staged'), 'view'))); + expect(JSON.parse(caseGridRow(h)!.metadata).label).toBe('staged'); + expect(caseGridRow(h)!.checksum).toBe(hashSpec(viewBody('staged'), 'view')); + expect(caseGridRow(h, 'draft')).toBeUndefined(); + }); + + it('the post-promotion drain removes a draft row stored with no checksum', async () => { + const h = makeEngine(); + const p = new ObjectStackProtocolImplementation(h.engine); + await seedUnstamped(h, 'staged', 'draft'); + + await p.publishMetaItem({ ...ref } as any); + expect(JSON.parse(caseGridRow(h)!.metadata).label).toBe('staged'); + // The drain deletes by the draft's served version; judged against the raw + // column it read as the benign "newer draft saved" race and survived. + expect(caseGridRow(h, 'draft')).toBeUndefined(); + }); +}); + +describe('[#21978] the repository lock: a row with a checksum is judged exactly as before', () => { + it('its stamp is its head: the hash of its body and a null parent are refused when the stamp differs', async () => { + const h = makeEngine(); + const repo = repoFor(h); + // A stamp that is not the hash of the bytes beside it (a row stamped + // before its type's canonical form changed): the stamp, not the body, + // is the version that row is served as. + const stamp = hashSpec(viewBody('stamped earlier'), 'view'); + await h.engine.insert('sys_metadata', { + type: 'view', + name: 'case_grid', + organization_id: ORG, + package_id: null, + state: 'active', + metadata: JSON.stringify(viewBody('stamped')), + checksum: stamp, + }); + expect((await repo.get(VIEW_REF as any))!.hash).toBe(stamp); + + for (const parent of [hashSpec(viewBody('stamped'), 'view'), null]) { + const refused = await rejection(() => + repo.put(VIEW_REF as any, viewBody('next'), { parentVersion: parent, actor: null })); + expect(refused).toBeInstanceOf(ConflictError); + expect(refused.code).toBe('METADATA_CONFLICT'); + expect(refused.actualHead).toBe(stamp); + } + const refusedDelete = await rejection(() => + repo.delete(VIEW_REF as any, { parentVersion: hashSpec(viewBody('stamped'), 'view'), actor: null })); + expect(refusedDelete).toBeInstanceOf(ConflictError); + expect(refusedDelete.actualHead).toBe(stamp); + expect(caseGridRow(h)!.checksum).toBe(stamp); + + const written = await repo.put(VIEW_REF as any, viewBody('next'), { parentVersion: stamp, actor: null }); + expect(written.version).toBe(hashSpec(viewBody('next'), 'view')); + }); + + it('a row with no checksum: null and its served version are accepted, anything else is refused with the served version as head', async () => { + const h = makeEngine(); + const repo = repoFor(h); + await seedUnstamped(h); + const served = hashSpec(viewBody('legacy'), 'view'); + + const refused = await rejection(() => + repo.put(VIEW_REF as any, viewBody('next'), { parentVersion: hashSpec(viewBody('other'), 'view'), actor: null })); + expect(refused).toBeInstanceOf(ConflictError); + expect(refused.code).toBe('METADATA_CONFLICT'); + expect(refused.actualHead).toBe(served); + + const viaNull = await repo.put(VIEW_REF as any, viewBody('legacy'), { parentVersion: null, actor: null }); + // An identical body still writes: the row had no stamp, and now has one. + expect(viaNull.version).toBe(served); + expect(caseGridRow(h)!.checksum).toBe(served); + expect(h.historyRows).toHaveLength(1); + + const again = makeEngine(); + await seedUnstamped(again); + const removed = await repoFor(again).delete(VIEW_REF as any, { parentVersion: served, actor: null }); + expect(removed).toBeDefined(); + expect(caseGridRow(again)).toBeUndefined(); + }); +});