From 23d5553bc38404362708d2aa2b765d1138eebc7f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 00:29:33 +0000 Subject: [PATCH 1/8] test(dogfood): red-first cases for a pair commit's plan and revert binding, and a deleted base-bound row's revert Claude-Session: https://claude.ai/code/session_01JQwxbZ5jYNfPGLvPV8LYRe Co-authored-by: Claude --- ...commit-revert-path-package.dogfood.test.ts | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/packages/qa/dogfood/test/package-commit-revert-path-package.dogfood.test.ts b/packages/qa/dogfood/test/package-commit-revert-path-package.dogfood.test.ts index 96ea5f93c8f..bb821d7555b 100644 --- a/packages/qa/dogfood/test/package-commit-revert-path-package.dogfood.test.ts +++ b/packages/qa/dogfood/test/package-commit-revert-path-package.dogfood.test.ts @@ -286,4 +286,89 @@ describe('dogfood: package commit revert and rollback act only on a commit of th }); expect(await served()).toBe(shipped); }); + + // [#29414] The commit a package records names its OWN rows: the plan reads the + // pre-publish state of the publishing package's row, and the revert removes or + // restores that row and no other package's. Before, both read any package's + // active row, so the pair's first publish beside a base-bound row was recorded + // as an edit of the base row's version, and its revert addressed the base row. + const view = (label: string) => ({ + name: OVERLAY, object: 'showcase_task', viewKind: 'list', label, + config: { type: 'grid', columns: [{ field: 'title' }] }, + }); + const servedLabel = async () => (await call('GET', `/meta/view/${OVERLAY}`)).json?.item?.label; + const publishBaseBound = async (label: string): Promise => { + const protocol = (stack.kernel as any).getService('protocol'); + protocol.namedBaseOverlayWriteRefusal = () => null; + try { + const saved = await call('PUT', `/meta/view/${OVERLAY}?mode=draft&package=${SHOWCASE}`, view(label)); + expect(saved.status, JSON.stringify(saved.json)).toBe(200); + const published = await call('POST', `/packages/${SHOWCASE}/publish-drafts`, {}); + expect(published.status, JSON.stringify(published.json)).toBe(200); + return published.json?.data?.commitId as string; + } finally { + delete protocol.namedBaseOverlayWriteRefusal; + } + }; + + it('[#29414] a pair commit beside a base-bound row records the pair\'s own row, and its revert removes the pair row and serves the base row again', async () => { + const shipped = await servedLabel(); + await publishBaseBound('Base-bound row'); + expect(await servedLabel()).toBe('Base-bound row'); + + const pair = await ensurePair(stack, admin, SHOWCASE); + const saved = await call('PUT', inPair(`/meta/view/${OVERLAY}?mode=draft`, SHOWCASE), declaredOverride(view('Pair override'), SHOWCASE, '0.1.0')); + expect(saved.status, JSON.stringify(saved.json)).toBe(200); + const published = await call('POST', `/packages/${pair}/publish-drafts`, {}); + expect(published.status, JSON.stringify(published.json)).toBe(200); + const commitId = published.json?.data?.commitId as string; + expect(await servedLabel()).toBe('Pair override'); + + const commit = (await commitsOf(pair)).find((c) => c.id === commitId) as any; + expect(commit?.items, JSON.stringify(commit)).toEqual([ + expect.objectContaining({ type: 'view', name: OVERLAY, existedBefore: false, prevVersion: null }), + ]); + + const res = await call('POST', `/packages/${pair}/commits/${commitId}/revert`, {}); + expect(res.status, JSON.stringify(res.json)).toBe(200); + expect(res.json?.data).toMatchObject({ + success: true, revertedCount: 1, failedCount: 0, reverted: [{ type: 'view', name: OVERLAY, action: 'removed' }], + }); + expect(await servedLabel()).toBe('Base-bound row'); + + // The base-bound row is cleared the one way the close leaves open: a delete naming the base. + const cleared = await call('DELETE', `/meta/view/${OVERLAY}?package=${SHOWCASE}`); + expect(cleared.status, JSON.stringify(cleared.json)).toBe(200); + expect(await servedLabel()).toBe(shipped); + }); + + it('[#29414] a commit that edited a base-bound row since deleted is refused by the package-less close, as the per-item rollback is, and writes nothing', async () => { + const shipped = await servedLabel(); + await publishBaseBound('Deleted later 1'); + const edited = await publishBaseBound('Deleted later 2'); + const cleared = await call('DELETE', `/meta/view/${OVERLAY}?package=${SHOWCASE}`); + expect(cleared.status, JSON.stringify(cleared.json)).toBe(200); + expect(await servedLabel()).toBe(shipped); + + const res = await call('POST', `/packages/${SHOWCASE}/commits/${edited}/revert`, {}); + expect(res.status, JSON.stringify(res.json)).toBe(200); + expect(res.json?.data).toMatchObject({ success: false, revertedCount: 0, failedCount: 1 }); + expect(res.json?.data?.failed?.[0]).toMatchObject({ type: 'view', name: OVERLAY, code: 'NOT_OVERRIDABLE' }); + expect(res.json.data.failed[0].error).toContain('a rollback of its package-less row is closed'); + expect(res.json.data.failed[0].error).toContain(`?package=${SHOWCASE}.custom`); + expect(await servedLabel()).toBe(shipped); + }); + + it('[#29414] control: a writable package\'s edit commit restores its own row in place', async () => { + await publishObject(A, 'cmtpa_edited', 'A edited 1'); + await publishObject(A, 'cmtpa_edited', 'A edited 2'); + const edit = (await commitsOf(A))[0] as any; + expect(edit.items).toEqual([expect.objectContaining({ type: 'object', name: 'cmtpa_edited', existedBefore: true })]); + const res = await call('POST', `/packages/${A}/commits/${edit.id}/revert`, {}); + expect(res.status, JSON.stringify(res.json)).toBe(200); + expect(res.json?.data).toMatchObject({ + success: true, revertedCount: 1, reverted: [{ type: 'object', name: 'cmtpa_edited', action: 'restored' }], + }); + expect((await call('GET', '/meta/object/cmtpa_edited')).json?.item?.label).toBe('A edited 1'); + }); }); From 167d683d5f384a55341bddfddef37b70d31b443e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 00:34:23 +0000 Subject: [PATCH 2/8] fix(metadata-protocol): a package commit's plan and revert address the committing package's own row Claude-Session: https://claude.ai/code/session_01JQwxbZ5jYNfPGLvPV8LYRe Co-authored-by: Claude --- packages/metadata-protocol/src/protocol.ts | 54 +++++++++++++++++-- .../src/sys-metadata-repository.ts | 18 +++++-- 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index acae72bf506..931f7e5b379 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -17548,9 +17548,17 @@ export class ObjectStackProtocolImplementation implements // [#22877] …and the draft's own bundle member: another // member's active row is not this member's pre-publish state, // and the revert plan records the member it describes. + // [#29414] …and the publishing package's own row: every draft + // listed here is bound to `request.packageId` and is promoted + // into that package's row, so another package's active row of + // the same name (a row bound to the read-only base beside its + // `.custom` pair) is not this item's pre-publish state. Read + // package-agnostically, the pair's first publish was recorded + // as an edit of the base row's version, and its revert + // addressed the base row instead of removing the pair's. const activeRow = (await this.findEnvironmentRow('sys_metadata', { where: { - type: d.type, name: d.name, state: 'active', + type: d.type, name: d.name, state: 'active', package_id: request.packageId, ...overlayMemberWhere(d.type, d.member), }, context: { isSystem: true }, @@ -19947,6 +19955,14 @@ export class ObjectStackProtocolImplementation implements * rollback sentence naming the base's `.custom` pair) with nothing written, * and {@link rollbackToPackageCommit} reports it per item and continues. * The soft-remove limb stays open: removing such a row writes nothing. + * + * [#29414, ADR-0048] Both limbs address the committing package's own row + * (`package_id` = the commit's), as the plan {@link publishPackageDrafts} + * records describes it: a pair commit beside a row bound to its read-only + * base removes or restores the pair's row and leaves the base's served. + * A restore whose row is gone writes the package-less row, so it asks the + * package-less close ({@link packagelessOverlayWriteRefusal}) on that + * `null` binding exactly as {@link rollbackMetaItem} does. */ async revertCommit(request: { commitId: string; @@ -19971,6 +19987,9 @@ export class ObjectStackProtocolImplementation implements ); await this.ensureOverlayIndex(); const row = await this.readRevertableCommit(request.commitId, request.packageId); + // [#29414] The package whose rows this commit's plan describes: every + // item's rows are read, removed and restored under it below. + const commitPackageId: string = row.package_id; const items = this.parseCommitItems(row.items); // #4556 — threaded into repo.put/delete → `recorded_by`; NULL when the // revert carries no human actor. @@ -20117,7 +20136,14 @@ export class ObjectStackProtocolImplementation implements // Absent (the canonical member) on every other item, as before. const member = it.member; try { - const current = await repo.get(ref, { state: 'active', ...memberOption(member) }); + // [#29414] The committing package's own row, and no other + // package's: the plan describes the rows this package's publish + // promoted into, so every read, removal and restore below names + // `commitPackageId`. Read package-agnostically, a pair commit + // beside a row bound to its read-only base addressed whichever + // row the driver returned first — the base row, refused below + // or restored in the pair's name. + const current = await repo.get(ref, { state: 'active', packageId: commitPackageId, ...memberOption(member) }); if (!it.existedBefore) { // Created by this commit → soft-remove (metadata only; table stays). // @@ -20160,6 +20186,7 @@ export class ObjectStackProtocolImplementation implements source: 'protocol.revertCommit', intent, state: 'active', + packageId: commitPackageId, ...memberOption(member), }); } @@ -20299,7 +20326,26 @@ export class ObjectStackProtocolImplementation implements // fallible query downstream of a write that already succeeded — // the shape that ends in a `catch {}` swallowing a real outage // (#4867). Per ITEM, because a batch mixes bindings. - const restorePackageId = await this.resolveOverlayPackageBinding(it.type, it.name, member); + // + // [#29414] The binding is the committing package's own row's + // (`current`, read under `commitPackageId` above), not any + // package's: `commitPackageId` when that row is stored, `null` + // when it is not (deleted since the commit), which is what + // `restoreVersion` then writes — the package-unbound row, as + // the per-item rollback writes it. + const restorePackageId: string | null = current ? commitPackageId : null; + // [#29414, ADR-0146 D11] …so a restore whose row is gone + // writes the package-less row, and is asked the package-less + // close exactly as {@link rollbackMetaItem} asks it on the + // same `null` binding ({@link packagelessOverlayWriteRefusal}, + // its rollback sentence): an overlay type's item the + // read-only base ships is refused into `failed[]` with + // nothing written. A binding naming a package leaves it open. + const closed = await this.packagelessOverlayWriteRefusal( + { type: PLURAL_TO_SINGULAR[it.type] ?? it.type, name: it.name, packageId: restorePackageId, state: 'active', ...memberOption(member) }, + 'rollback', + ); + if (closed) throw closed; // [#29390, ADR-0146 D1, D11] A restore writes the row this // binding names, so a restore into a row bound to the // read-only base itself (a commit recorded before #28905 @@ -20317,6 +20363,8 @@ export class ObjectStackProtocolImplementation implements const restored = await repo.restoreVersion(ref, restoreToVersion, { actor, source: 'protocol.revertCommit', + // [#29414] The row restored is the committing package's. + packageId: commitPackageId, message: `revert commit ${request.commitId}`, intent, // [#21470] The divergent `name` refusal, [ADR-0086 D3] diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index 83fb10a21d8..e3e022502ae 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -1683,6 +1683,15 @@ export class SysMetadataRepository implements MetadataRepository { * of ANOTHER member of the bundle is not this member's version. */ member?: string | null; + /** + * [#29414] The package whose active row is restored. Stated → the + * active row (its parent hash and its binding) is resolved under the + * ADR-0048 key `(org, type, name, package_id)`, so another package's + * row of the same name is never the one restored; no such row → the + * unbound row, as below. Omitted → the "match any package" resolution, + * as before. A LOCAL extension, as on {@link delete}. + */ + packageId?: string | null; }, ): Promise<{ version: string; seq: number; item: MetadataItem }> { this.assertOpen(); @@ -1739,8 +1748,9 @@ export class SysMetadataRepository implements MetadataRepository { // A missing active row (deleted, or never published) yields `null` — the // unbound row, which is the only defined answer: `sys_metadata_history` // carries no `package_id` column, so a vanished binding is not recoverable. + // [#29414] …under the package the caller names, when it names one. const activeRow = await this.findEnvironmentRow({ - where: this.whereFor(ref, 'active', undefined, member), + where: this.whereFor(ref, 'active', 'packageId' in opts ? (opts.packageId ?? null) : undefined, member), context: { isSystem: true }, }); const activePackageId = @@ -2649,9 +2659,9 @@ export class SysMetadataRepository implements MetadataRepository { // package A promoted package B's pending draft, drained B's row, recorded // it under A's commit, and left A's own edit pending while answering // `success: true`. A caller that KNOWS the binding must state it — see - // {@link promoteDraft}'s `packageId`. The same question is open for the - // `delete` and `restoreVersion` resolving reads (#6215); they are - // deliberately unchanged here. + // {@link promoteDraft}'s `packageId`. The `delete` and `restoreVersion` + // resolving reads take the same option (#6215); [#29414] the commit revert + // states it on both. if (packageId !== undefined) where.package_id = packageId; // string → eq; null → IS NULL if (member !== undefined) where[OVERLAY_MEMBER_COLUMN] = member; // string → eq; null → IS NULL return where; From 9179b36a8af9a5410982e89650e01b51fc12f908 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 00:35:54 +0000 Subject: [PATCH 3/8] test(metadata-protocol): pin the pair commit's plan and revert binding and the deleted-row package-less close Claude-Session: https://claude.ai/code/session_01JQwxbZ5jYNfPGLvPV8LYRe Co-authored-by: Claude --- .../protocol.named-base-overlay-write.test.ts | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/packages/metadata-protocol/src/protocol.named-base-overlay-write.test.ts b/packages/metadata-protocol/src/protocol.named-base-overlay-write.test.ts index 76f04303ec6..cf5723491c2 100644 --- a/packages/metadata-protocol/src/protocol.named-base-overlay-write.test.ts +++ b/packages/metadata-protocol/src/protocol.named-base-overlay-write.test.ts @@ -474,4 +474,66 @@ describe('the commit doors: restoring a row bound to the base is closed, removin expect(rows(VIEW.type, VIEW.name)).toEqual([{ package_id: PAIR, state: 'active', label: 'PairFirst' }]); expect((await protocol.getMetaItem({ type: VIEW.type, name: VIEW.name })).item.label).toBe('PairFirst'); }); + + // [#29414] The plan and both revert limbs address the committing package's + // own row. Before, each read any package's active row, so the pair's + // commit beside a row bound to the base addressed the base row. + const planOf = async (protocol: any, packageId: string, commitId: string) => + ((await protocol.listCommits({ packageId })) as Array<{ id: string; items: unknown[] }>).find((c) => c.id === commitId)?.items; + + it('[#29414] the pair\'s first commit beside a row bound to the base records a create, and its revert removes the pair row; the base row is served again', async () => { + const { protocol, rows } = makeProtocol(); + await intoBaseBeforeClose(protocol, 'BaseRow'); + const created = await publishCommit(protocol, PAIR, declared('PairFirst')); + expect(await planOf(protocol, PAIR, created)).toEqual([ + expect.objectContaining({ type: VIEW.type, name: VIEW.name, existedBefore: false, prevVersion: null }), + ]); + + tick(); + const result = await protocol.revertCommit({ commitId: created, packageId: PAIR, actor: 'admin' }); + + expect(result).toMatchObject({ success: true, revertedCount: 1, failed: [], reverted: [{ type: VIEW.type, name: VIEW.name, action: 'removed' }] }); + expect(rows(VIEW.type, VIEW.name)).toEqual([{ package_id: BASE, state: 'active', label: 'BaseRow' }]); + expect((await protocol.getMetaItem({ type: VIEW.type, name: VIEW.name })).item.label).toBe('BaseRow'); + }); + + it('[#29414] the pair\'s edit commit beside a row bound to the base restores the pair\'s own row; the base row is untouched', async () => { + const { protocol, rows } = makeProtocol(); + await intoBaseBeforeClose(protocol, 'BaseRow'); + await publishCommit(protocol, PAIR, declared('PairFirst')); + const edit = await publishCommit(protocol, PAIR, declared('PairSecond')); + expect(await planOf(protocol, PAIR, edit)).toEqual([ + expect.objectContaining({ type: VIEW.type, name: VIEW.name, existedBefore: true }), + ]); + + tick(); + const result = await protocol.revertCommit({ commitId: edit, packageId: PAIR, actor: 'admin' }); + + expect(result).toMatchObject({ success: true, revertedCount: 1, failed: [], reverted: [{ type: VIEW.type, name: VIEW.name, action: 'restored' }] }); + expect(rows(VIEW.type, VIEW.name)).toEqual([ + { package_id: BASE, state: 'active', label: 'BaseRow' }, + { package_id: PAIR, state: 'active', label: 'PairFirst' }, + ]); + }); + + it('[#29414] a revert of the base\'s edit commit whose row was since deleted is refused by the package-less close, as the per-item rollback is; nothing is written', async () => { + const { protocol, rows } = makeProtocol(); + await intoBaseBeforeClose(protocol, 'First'); + const edit = await intoBaseBeforeClose(protocol, 'Second'); + expect(await protocol.deleteMetaItem({ type: VIEW.type, name: VIEW.name, packageId: BASE })).toMatchObject({ success: true }); + expect(rows(VIEW.type, VIEW.name)).toEqual([]); + // The per-item rollback on the same `null` binding: the one predicate, its rollback sentence. + const rollback = await thrown(() => protocol.rollbackMetaItem({ type: VIEW.type, name: VIEW.name, toVersion: 1 })); + expect(resolveThrownHttpError(rollback)).toMatchObject({ status: 403, code: 'NOT_OVERRIDABLE' }); + + tick(); + const result = await protocol.revertCommit({ commitId: edit, packageId: BASE, actor: 'admin' }); + + expect(result).toMatchObject({ success: false, revertedCount: 0, failedCount: 1, reverted: [] }); + expect(result.failed).toEqual([expect.objectContaining({ type: VIEW.type, name: VIEW.name, code: 'NOT_OVERRIDABLE' })]); + expect(result.failed[0].error).toBe(rollback.message); + expect(result.failed[0].error).toContain('a rollback of its package-less row is closed'); + expect(rows(VIEW.type, VIEW.name)).toEqual([]); + expect((await protocol.getMetaItem({ type: VIEW.type, name: VIEW.name })).item.label).toBe(PACKAGED); + }); }); From ab76fc6bd359fffc6103dd05df983816452da1e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 00:36:51 +0000 Subject: [PATCH 4/8] chore: changeset and dogfood header for the commit doors' own-package addressing Claude-Session: https://claude.ai/code/session_01JQwxbZ5jYNfPGLvPV8LYRe Co-authored-by: Claude --- .../29414-commit-revert-own-package-row.md | 35 +++++++++++++++++++ ...commit-revert-path-package.dogfood.test.ts | 7 ++++ 2 files changed, 42 insertions(+) create mode 100644 .changeset/29414-commit-revert-own-package-row.md diff --git a/.changeset/29414-commit-revert-own-package-row.md b/.changeset/29414-commit-revert-own-package-row.md new file mode 100644 index 00000000000..132ad6f12b5 --- /dev/null +++ b/.changeset/29414-commit-revert-own-package-row.md @@ -0,0 +1,35 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +fix(metadata-protocol)!: a package commit's revert plan and its revert address the committing package's own row; a revert whose row was deleted asks the package-less close, as the per-item rollback does + +Clause-②: no (narrowing) — the commit revert (`POST /api/v1/packages/:id/commits/:commitId/revert`) and the package rollback (`POST /api/v1/packages/:id/rollback`) no longer restore a stored version into the package-unbound row when the committing package's own row of the item is gone, on an item of the five overlay types one installed read-only package ships, where `os migrate packageless-overlays` would move such a row into the base's paired `.custom`; they inserted that package-less row before. The per-item rollback on the same package-less address has refused it since `packageless-overlay-write-closed`. + +Metadata-protocol: none + +Skills-docs: none needed — `skills/objectstack-platform/SKILL.md` already says the package-less save, reset and rollback of a movable row are refused once movable (D11), and places every customization of an installed base in its paired `.custom`; nothing it says becomes false. + + + +**BREAKING** write narrowing at the commit doors (`@objectstack/metadata-protocol`), shipped as `patch` (`Metadata-protocol: none`). + +The plan a package publish records (`POST /api/v1/packages/:id/publish-drafts`, the commit's `existedBefore` and `prevVersion` per item) now reads the publishing package's own active row, and the revert removes or restores that row only. Before, both read any package's active row of the name: the first publish of a `.custom` pair's declared override beside a row bound to the base (stored before the write naming the base was closed) was recorded as an edit of the base row's version, and its revert addressed the base row, refused there instead of removing the pair's row. It now removes the pair's row, and the row bound to the base is served again. Commits recorded before this change keep the plan they recorded. + +### Migration: FROM → TO + +FROM, undoing a commit of the installed package `com.acme.crm` (recorded before the write naming it was closed) that edited a customization whose row was deleted since: + +```http +POST /api/v1/packages/com.acme.crm/commits/cmt_123/revert +``` + +It now answers `200` with `success: false`, and the item in `failed[]` with `code: NOT_OVERRIDABLE` and the sentence the per-item rollback gives; nothing is written. TO, write the version you want back into the base's paired `.custom`, declaring what it overrides: + +```http +PUT /api/v1/meta/view/account_list?package=com.acme.crm.custom +{ "name": "account_list", "label": "My accounts", "overrides": { "package": "com.acme.crm", "basedOn": "1.4.0" } } +``` + +- **Now refused, per item.** The restore of a commit item whose row in the committing package is gone, of a `view`, `dashboard`, `report`, `translation` or `email_template` one read-only package ships, where the package-less close applies (the row would be one `os migrate packageless-overlays` moves). The commit's other items still revert, and `POST /api/v1/packages//rollback` reports the item in its `failed[]` and continues. +- **Still open.** Every restore into a row the committing package holds (a writable package's in place, the pair's in place), every removal, and every package-less restore the close leaves open (a name no read-only package ships, or one several ship). diff --git a/packages/qa/dogfood/test/package-commit-revert-path-package.dogfood.test.ts b/packages/qa/dogfood/test/package-commit-revert-path-package.dogfood.test.ts index bb821d7555b..b4248d4fc50 100644 --- a/packages/qa/dogfood/test/package-commit-revert-path-package.dogfood.test.ts +++ b/packages/qa/dogfood/test/package-commit-revert-path-package.dogfood.test.ts @@ -51,6 +51,13 @@ // restoring it would write the row bound to the read-only base again. The // last case pins both on such a commit, recorded with the close shadowed on // the booted instance only (what a build before the close did). +// +// [#29414] A commit names its own package's rows. The pair's first publish +// beside such a base-bound row is recorded as a create (its plan used to read +// the base row's version), so its revert removes the pair's row and the base +// row is served again; a revert whose row was deleted since asks the +// package-less close, as the per-item rollback does. Control: a writable +// package's edit commit restores its own row in place. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { type VerifyStack } from '@objectstack/verify'; From 55837b787f4005ce0ec8179b1e2af73c37f8ddb7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 04:26:45 +0000 Subject: [PATCH 5/8] test(metadata-protocol): bind the commit-revert fixtures' rows to the commit's own package Six suites seeded a package's commit over package-less rows, or served a commit row with no package_id: a shape no publish produces. A revert now addresses the committing package's own rows, so each fixture states that package. The publish audit harness arms its rival write on the second package-scoped read of the name, since the plan scan now names the package. Claude-Session: https://claude.ai/code/session_01JQwxbZ5jYNfPGLvPV8LYRe Co-authored-by: Claude --- .../src/protocol.extension-field-suffix.test.ts | 10 ++++++++-- .../src/protocol.item-name-every-door.test.ts | 12 +++++++++--- .../src/protocol.package-publish-audit-rows.test.ts | 12 ++++++++++-- .../protocol.recovery-doors-emit-mutation.test.ts | 2 ++ ...rotocol.recovery-doors-mutation-projector.test.ts | 2 ++ .../protocol.revert-stored-type-canonical.test.ts | 7 +++++-- .../protocol.rollback-carries-item-failures.test.ts | 9 +++++++-- 7 files changed, 43 insertions(+), 11 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.extension-field-suffix.test.ts b/packages/metadata-protocol/src/protocol.extension-field-suffix.test.ts index 32eaeb7cb95..5db53d50b30 100644 --- a/packages/metadata-protocol/src/protocol.extension-field-suffix.test.ts +++ b/packages/metadata-protocol/src/protocol.extension-field-suffix.test.ts @@ -304,9 +304,15 @@ async function attempt(run: () => Promise): Promise<{ result: T | null; er afterEach(() => vi.restoreAllMocks()); -const commitRow = (items: unknown[]) => ({ +/** + * [#29414] `packageId` — the package the commit is recorded for, which a + * revert addresses the rows of; omitted, no `package_id` (the package-less + * rows those cases seed). + */ +const commitRow = (items: unknown[], packageId?: string) => ({ id: 'c1', commit_id: 'c1', + ...(packageId !== undefined ? { package_id: packageId } : {}), organization_id: null, operation: 'apply', message: 'the commit under revert', @@ -455,7 +461,7 @@ describe('[ADR-0146 D10] the restore doors refuse a stored version carrying a `_ it('revertCommit\'s restore limb reports it in failed[] with the envelope\'s code; nothing is written', async () => { const h = makeProtocol(); const before = await threeVersionsWithFieldResidueAtV2(h, BOUND); - h.serveCommit(commitRow([{ type: 'object', name: 'probe_ticket', existedBefore: true, prevVersion: 2 }])); + h.serveCommit(commitRow([{ type: 'object', name: 'probe_ticket', existedBefore: true, prevVersion: 2 }], BOUND)); vi.spyOn(console, 'warn').mockImplementation(() => {}); const res = await h.protocol.revertCommit({ commitId: 'c1' }); diff --git a/packages/metadata-protocol/src/protocol.item-name-every-door.test.ts b/packages/metadata-protocol/src/protocol.item-name-every-door.test.ts index d859bbfa757..c60b0c2b7ce 100644 --- a/packages/metadata-protocol/src/protocol.item-name-every-door.test.ts +++ b/packages/metadata-protocol/src/protocol.item-name-every-door.test.ts @@ -391,9 +391,15 @@ async function threeVersionsWithResidueAtV2(h: Harness) { return { historyBefore: h.historyRows.length, activeBefore: JSON.stringify(stored(h, 'dashboard')) }; } -const commitRow = (items: unknown[]) => ({ +/** + * [#29414] `packageId` — the package the commit is recorded for, which a + * revert addresses the rows of; omitted, no `package_id` (the package-less + * rows those cases seed). + */ +const commitRow = (items: unknown[], packageId?: string) => ({ id: 'c1', commit_id: 'c1', + ...(packageId !== undefined ? { package_id: packageId } : {}), organization_id: null, operation: 'apply', message: 'the commit under revert', @@ -794,7 +800,7 @@ describe('[ADR-0086 D3] the restore doors refuse a stored version whose body `pa it('revertCommit\'s restore limb reports a disagreeing version in failed[] with the envelope\'s code; nothing is written', async () => { const h = makeProtocol(); const before = await threeSetVersionsWithOwnerResidueAtV2(h, BOUND, OTHER); - h.serveCommit(commitRow([{ type: 'permission', name: 'probe_member', existedBefore: true, prevVersion: 2 }])); + h.serveCommit(commitRow([{ type: 'permission', name: 'probe_member', existedBefore: true, prevVersion: 2 }], BOUND)); vi.spyOn(console, 'warn').mockImplementation(() => {}); const res = await h.protocol.revertCommit({ commitId: 'c1' }); @@ -812,7 +818,7 @@ describe('[ADR-0086 D3] the restore doors refuse a stored version whose body `pa it('CONTROL: revertCommit\'s restore limb restores a version whose body agrees with the binding', async () => { const h = makeProtocol(); await threeSetVersionsWithOwnerResidueAtV2(h, BOUND, BOUND); - h.serveCommit(commitRow([{ type: 'permission', name: 'probe_member', existedBefore: true, prevVersion: 2 }])); + h.serveCommit(commitRow([{ type: 'permission', name: 'probe_member', existedBefore: true, prevVersion: 2 }], BOUND)); const res = await h.protocol.revertCommit({ commitId: 'c1' }); diff --git a/packages/metadata-protocol/src/protocol.package-publish-audit-rows.test.ts b/packages/metadata-protocol/src/protocol.package-publish-audit-rows.test.ts index aec2d407b77..e8f6bd09cae 100644 --- a/packages/metadata-protocol/src/protocol.package-publish-audit-rows.test.ts +++ b/packages/metadata-protocol/src/protocol.package-publish-audit-rows.test.ts @@ -173,8 +173,10 @@ type Harness = { * between the two reads `promoteDraft` makes (`get` for the parent version, * then `put`'s own read inside its transaction), which is exactly the * 409 `METADATA_CONFLICT` the repository raises. Scoped to package-scoped - * active reads so the pre-flight `commitItems` scan — which reads active - * rows WITHOUT a `package_id` — does not consume the one-shot. + * active reads, and armed on the SECOND one of the name: [#29414] the + * pre-flight `commitItems` scan reads the publishing package's active row + * too (it read any package's before), and it runs before the promotion, + * so it must not consume the one-shot. */ faults: { failMetadataWriteFor?: string; advanceActiveOnRead?: string }; /** Audit rows that LANDED and were not rolled back. */ @@ -189,6 +191,8 @@ type Harness = { function makeStubEngine(opts: { failAudit?: boolean } = {}): Harness { const faults: { failMetadataWriteFor?: string; advanceActiveOnRead?: string } = {}; + /** [#29414] The names whose pre-flight plan read has passed (see `advanceActiveOnRead`). */ + const planReadPassed = new Set(); const rows = new Map(); const historyRows: HistoryRow[] = []; const commitRows: any[] = []; @@ -242,6 +246,10 @@ function makeStubEngine(opts: { failAudit?: boolean } = {}): Harness { if (faults.advanceActiveOnRead === hit.row.name && opts2.where?.state === 'active' && 'package_id' in (opts2.where ?? {})) { + if (!planReadPassed.has(hit.row.name)) { + planReadPassed.add(hit.row.name); + return hit.row; + } delete faults.advanceActiveOnRead; const asItStood = { ...hit.row }; hit.row.checksum = 'sha256:advanced_by_a_rival'; diff --git a/packages/metadata-protocol/src/protocol.recovery-doors-emit-mutation.test.ts b/packages/metadata-protocol/src/protocol.recovery-doors-emit-mutation.test.ts index 7a295a23bf8..13b829df149 100644 --- a/packages/metadata-protocol/src/protocol.recovery-doors-emit-mutation.test.ts +++ b/packages/metadata-protocol/src/protocol.recovery-doors-emit-mutation.test.ts @@ -161,9 +161,11 @@ function matchesWhere(r: Row, where: Record): boolean { } /** One `sys_metadata_commit` row in the driver's snake_case wire shape. */ +/** [#29414] Recorded for `PKG`, the package the rows below are bound to: a commit's revert addresses its own package's rows. */ const commitRow = (items: unknown[]) => ({ id: 'c1', commit_id: 'c1', + package_id: PKG, organization_id: null, operation: 'apply', message: 'the commit under revert', diff --git a/packages/metadata-protocol/src/protocol.recovery-doors-mutation-projector.test.ts b/packages/metadata-protocol/src/protocol.recovery-doors-mutation-projector.test.ts index ced797802ef..7b70ace95c2 100644 --- a/packages/metadata-protocol/src/protocol.recovery-doors-mutation-projector.test.ts +++ b/packages/metadata-protocol/src/protocol.recovery-doors-mutation-projector.test.ts @@ -126,9 +126,11 @@ function matchesWhere(r: Row, where: Record): boolean { } /** One `sys_metadata_commit` row in the driver's snake_case wire shape. */ +/** [#29414] Recorded for `PKG`, the package the rows below are bound to: a commit's revert addresses its own package's rows. */ const commitRow = (items: unknown[]) => ({ id: 'c1', commit_id: 'c1', + package_id: PKG, organization_id: null, operation: 'apply', message: 'the commit under revert', diff --git a/packages/metadata-protocol/src/protocol.revert-stored-type-canonical.test.ts b/packages/metadata-protocol/src/protocol.revert-stored-type-canonical.test.ts index 0457d14984f..71955a7772b 100644 --- a/packages/metadata-protocol/src/protocol.revert-stored-type-canonical.test.ts +++ b/packages/metadata-protocol/src/protocol.revert-stored-type-canonical.test.ts @@ -305,14 +305,17 @@ async function seedRowVerbatim( const repo = protocol.getOverlayRepo(null); const ref = { type: args.type, name: args.name, org: 'env' }; // Successive writes to one identity build the lineage a revert restores - // from, so each must present the CURRENT head as its parent. - const existing = await repo.get(ref, { state: 'active' }); + // from, so each must present the CURRENT head as its parent. [#29414] + // Bound to the package the commit under revert is recorded for + // (`commitRow`): a commit's revert addresses that package's own rows. + const existing = await repo.get(ref, { state: 'active', packageId: 'app.demo' }); await repo.put(ref, args.body, { parentVersion: existing?.hash ?? null, actor: null, source: 'test.at-rest-residue', intent: 'runtime-only', state: 'active', + packageId: 'app.demo', }); } diff --git a/packages/metadata-protocol/src/protocol.rollback-carries-item-failures.test.ts b/packages/metadata-protocol/src/protocol.rollback-carries-item-failures.test.ts index 9f73c0cf758..2b8c9781b56 100644 --- a/packages/metadata-protocol/src/protocol.rollback-carries-item-failures.test.ts +++ b/packages/metadata-protocol/src/protocol.rollback-carries-item-failures.test.ts @@ -173,18 +173,23 @@ function makeProtocol(opts: { unreadableCommits?: readonly string[] } = {}) { return { protocol, rows, commits }; } -/** Write a row through the repository, which stores `type` exactly as given. */ +/** + * Write a row through the repository, which stores `type` exactly as given, + * bound to the package the commits are recorded for: [#29414] a commit's + * revert addresses the committing package's own rows, as its publish wrote them. + */ async function seed(protocol: any, type: string, name: string, label: string): Promise { await protocol.ensureOverlayIndex(); const repo = protocol.getOverlayRepo(null); const ref = { type, name, org: 'env' }; - const existing = await repo.get(ref, { state: 'active' }); + const existing = await repo.get(ref, { state: 'active', packageId: PKG }); await repo.put(ref, { name, label }, { parentVersion: existing?.hash ?? null, actor: null, source: 'test.rollback-item-failures', intent: 'runtime-only', state: 'active', + packageId: PKG, }); } From 7f036633a2933cd079a318f4efb1e79d500cae0d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 05:24:42 +0000 Subject: [PATCH 6/8] fix(metadata-protocol): a commit revert's restore judges a pair row by the override write gate and writes the committing package's row when it is gone Claude-Session: https://claude.ai/code/session_01JQwxbZ5jYNfPGLvPV8LYRe Co-authored-by: Claude --- .../protocol.named-base-overlay-write.test.ts | 65 +++++++++- packages/metadata-protocol/src/protocol.ts | 116 +++++++++++------- .../src/sys-metadata-repository.ts | 10 +- 3 files changed, 139 insertions(+), 52 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.named-base-overlay-write.test.ts b/packages/metadata-protocol/src/protocol.named-base-overlay-write.test.ts index cf5723491c2..d6c6aacc0b8 100644 --- a/packages/metadata-protocol/src/protocol.named-base-overlay-write.test.ts +++ b/packages/metadata-protocol/src/protocol.named-base-overlay-write.test.ts @@ -516,24 +516,77 @@ describe('the commit doors: restoring a row bound to the base is closed, removin ]); }); - it('[#29414] a revert of the base\'s edit commit whose row was since deleted is refused by the package-less close, as the per-item rollback is; nothing is written', async () => { + it('[#29414] a revert of the base\'s edit commit whose row was since deleted restores into the base\'s own row, so the named-base close refuses it; nothing is written', async () => { const { protocol, rows } = makeProtocol(); await intoBaseBeforeClose(protocol, 'First'); const edit = await intoBaseBeforeClose(protocol, 'Second'); expect(await protocol.deleteMetaItem({ type: VIEW.type, name: VIEW.name, packageId: BASE })).toMatchObject({ success: true }); expect(rows(VIEW.type, VIEW.name)).toEqual([]); - // The per-item rollback on the same `null` binding: the one predicate, its rollback sentence. - const rollback = await thrown(() => protocol.rollbackMetaItem({ type: VIEW.type, name: VIEW.name, toVersion: 1 })); - expect(resolveThrownHttpError(rollback)).toMatchObject({ status: 403, code: 'NOT_OVERRIDABLE' }); tick(); const result = await protocol.revertCommit({ commitId: edit, packageId: BASE, actor: 'admin' }); expect(result).toMatchObject({ success: false, revertedCount: 0, failedCount: 1, reverted: [] }); expect(result.failed).toEqual([expect.objectContaining({ type: VIEW.type, name: VIEW.name, code: 'NOT_OVERRIDABLE' })]); - expect(result.failed[0].error).toBe(rollback.message); - expect(result.failed[0].error).toContain('a rollback of its package-less row is closed'); + expectClosedSentence(result.failed[0].error, VIEW.type, VIEW.name, 'rollback'); expect(rows(VIEW.type, VIEW.name)).toEqual([]); expect((await protocol.getMetaItem({ type: VIEW.type, name: VIEW.name })).item.label).toBe(PACKAGED); }); + + it('[#29414] a pair commit recorded before #29414 beside a row bound to the base names the base row\'s version: its revert is refused by the override write gate, and the pair\'s declared row is unchanged', async () => { + const { protocol, rows } = makeProtocol(); + await intoBaseBeforeClose(protocol, 'BaseRow'); + const created = await publishCommit(protocol, PAIR, declared('PairFirst')); + // The plan such a commit recorded before #29414: the base row's version, read off any package's row. + const engine = protocol.engine; + const baseVersion = (await engine.findOne('sys_metadata', { where: { type: VIEW.type, name: VIEW.name, state: 'active', package_id: BASE } })).version; + expect(typeof baseVersion).toBe('number'); + await engine.update('sys_metadata_commit', { + items: JSON.stringify([{ type: VIEW.type, name: VIEW.name, existedBefore: true, prevVersion: baseVersion }]), + }, { where: { id: created } }); + expect(await planOf(protocol, PAIR, created)).toEqual([ + expect.objectContaining({ existedBefore: true, prevVersion: baseVersion }), + ]); + + tick(); + const result = await protocol.revertCommit({ commitId: created, packageId: PAIR, actor: 'admin' }); + + expect(result).toMatchObject({ success: false, revertedCount: 0, failedCount: 1, reverted: [] }); + expect(result.failed).toEqual([expect.objectContaining({ type: VIEW.type, name: VIEW.name, code: 'NOT_OVERRIDABLE' })]); + expect(rows(VIEW.type, VIEW.name)).toEqual([ + { package_id: BASE, state: 'active', label: 'BaseRow' }, + { package_id: PAIR, state: 'active', label: 'PairFirst' }, + ]); + const pairRow = await engine.findOne('sys_metadata', { where: { type: VIEW.type, name: VIEW.name, state: 'active', package_id: PAIR } }); + expect(JSON.parse(pairRow.metadata).overrides).toEqual({ package: BASE, basedOn: BASE_VERSION }); + expect((await protocol.getMetaItem({ type: VIEW.type, name: VIEW.name })).item.label).toBe('PairFirst'); + }); + + it('[#29414] CONTROL: the pair\'s edit commit whose row was since deleted restores the pair\'s declared row again', async () => { + const { protocol, rows } = makeProtocol(); + await publishCommit(protocol, PAIR, declared('PairFirst')); + const edit = await publishCommit(protocol, PAIR, declared('PairSecond')); + expect(await protocol.deleteMetaItem({ type: VIEW.type, name: VIEW.name, packageId: PAIR })).toMatchObject({ success: true }); + expect(rows(VIEW.type, VIEW.name)).toEqual([]); + + tick(); + const result = await protocol.revertCommit({ commitId: edit, packageId: PAIR, actor: 'admin' }); + + expect(result).toMatchObject({ success: true, revertedCount: 1, failed: [], reverted: [{ type: VIEW.type, name: VIEW.name, action: 'restored' }] }); + expect(rows(VIEW.type, VIEW.name)).toEqual([{ package_id: PAIR, state: 'active', label: 'PairFirst' }]); + expect((await protocol.getMetaItem({ type: VIEW.type, name: VIEW.name })).item.label).toBe('PairFirst'); + }); + + it('[#29414] CONTROL: a writable package\'s edit commit whose row was since deleted restores into that package\'s row, never a package-less one', async () => { + const { protocol, rows } = makeProtocol(); + await publishCommit(protocol, WRITABLE, VIEW.body('First')); + const edit = await publishCommit(protocol, WRITABLE, VIEW.body('Second')); + expect(await protocol.deleteMetaItem({ type: VIEW.type, name: VIEW.name, packageId: WRITABLE })).toMatchObject({ success: true }); + + tick(); + const result = await protocol.revertCommit({ commitId: edit, packageId: WRITABLE, actor: 'admin' }); + + expect(result).toMatchObject({ success: true, revertedCount: 1, failed: [], reverted: [{ type: VIEW.type, name: VIEW.name, action: 'restored' }] }); + expect(rows(VIEW.type, VIEW.name)).toEqual([{ package_id: WRITABLE, state: 'active', label: 'First' }]); + }); }); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 75c175010dc..15c1ebe32ad 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -1512,6 +1512,45 @@ export class ObjectStackProtocolImplementation implements return restoredBodyWriter.call(host, type, name, binding, source); } + /** + * [ADR-0146 D2] The write intent and the body writer of a restore into + * the row `binding` names — the per-item rollback's ({@link rollbackMetaItem}) + * and [#29414] a commit revert's restore limb ({@link revertCommit}), one + * derivation for both. + * + * A restore into the base's paired `.custom` row is the pair's declared + * override, as its save is: it edits no managed item, so the store is told + * a runtime-only write, and the restored body is judged by the override + * write gate into the pair ({@link overrideWriteVerdict}). A version that + * declares no override (a body the base row stored, which a commit plan + * recorded before #29414 can name) is refused as the save is + * (`overrideWriteRefusalError`, `403 NOT_OVERRIDABLE`), thrown from inside + * `deriveRestoredBody`, so nothing is written. Any other binding: the + * caller's `artifactBacked` decides the intent, and the body writer is + * {@link restoredBodyWriter}'s alone. + */ + private restoreWriteFor( + singularType: string, + name: string, + binding: string | null, + artifactBacked: boolean, + source: string, + ): { intent: 'override-artifact' | 'runtime-only'; deriveRestoredBody: (historyBody: unknown) => Promise } { + const owner = artifactBacked ? this.packagedArtifactOwner({ type: singularType, name }) : undefined; + const pairRow = owner !== undefined && isCustomizationPairOf(binding, owner); + const intent: 'override-artifact' | 'runtime-only' = artifactBacked && !pairRow ? 'override-artifact' : 'runtime-only'; + const writeRestored = this.restoredBodyWriter(singularType, name, binding, source); + const deriveRestoredBody = pairRow + ? async (historyBody: unknown): Promise => { + const body = await writeRestored(historyBody); + const verdict = this.overrideWriteVerdict({ type: singularType, name, packageId: binding, item: body }); + if (verdict.verdict === 'refuse') throw overrideWriteRefusalError(verdict); + return body; + } + : writeRestored; + return { intent, deriveRestoredBody }; + } + /** * [#25278] The save door's refusal family, asked of a history body a * restore is about to make the active row. @@ -19965,9 +20004,13 @@ export class ObjectStackProtocolImplementation implements * (`package_id` = the commit's), as the plan {@link publishPackageDrafts} * records describes it: a pair commit beside a row bound to its read-only * base removes or restores the pair's row and leaves the base's served. - * A restore whose row is gone writes the package-less row, so it asks the - * package-less close ({@link packagelessOverlayWriteRefusal}) on that - * `null` binding exactly as {@link rollbackMetaItem} does. + * The restore writes the committing package's row, in place or created + * again when it was deleted since, so this door never writes the + * package-less row; a commit of the read-only base itself is refused by + * the #29390 close. A restore into the base's paired `.custom` row is judged + * by the override write gate as {@link rollbackMetaItem} judges it + * ({@link restoreWriteFor}): a version declaring no override is refused + * into `failed[]`. */ async revertCommit(request: { commitId: string; @@ -20311,8 +20354,18 @@ export class ObjectStackProtocolImplementation implements // The soft-remove limb above stated the same intent as a // CONSTANT and was fixed the same way (#6620), so both limbs now // derive it. The registry half of both is #6621, below. - const intent: 'override-artifact' | 'runtime-only' = - this.isArtifactBacked(it.type, it.name) ? 'override-artifact' : 'runtime-only'; + // + // [#29414, ADR-0146 D2] …and a restore into the base's paired + // `.custom` row is the pair's declared override, judged by the + // override write gate exactly as {@link rollbackMetaItem} + // judges it ({@link restoreWriteFor}, one derivation): a + // version that declares no override — the base row's body a + // plan recorded before #29414 names as the pair's pre-commit + // version — is refused into `failed[]`, nothing written. + const { intent, deriveRestoredBody } = this.restoreWriteFor( + PLURAL_TO_SINGULAR[it.type] ?? it.type, it.name, commitPackageId, + this.isArtifactBacked(it.type, it.name), 'protocol.revertCommit', + ); // [#6621 / #4636] The ownership key the write-through needs, // read from the ROW rather than from the request — the sibling // revert caller {@link rollbackMetaItem} reads it exactly this @@ -20332,25 +20385,16 @@ export class ObjectStackProtocolImplementation implements // the shape that ends in a `catch {}` swallowing a real outage // (#4867). Per ITEM, because a batch mixes bindings. // - // [#29414] The binding is the committing package's own row's - // (`current`, read under `commitPackageId` above), not any - // package's: `commitPackageId` when that row is stored, `null` - // when it is not (deleted since the commit), which is what - // `restoreVersion` then writes — the package-unbound row, as - // the per-item rollback writes it. - const restorePackageId: string | null = current ? commitPackageId : null; - // [#29414, ADR-0146 D11] …so a restore whose row is gone - // writes the package-less row, and is asked the package-less - // close exactly as {@link rollbackMetaItem} asks it on the - // same `null` binding ({@link packagelessOverlayWriteRefusal}, - // its rollback sentence): an overlay type's item the - // read-only base ships is refused into `failed[]` with - // nothing written. A binding naming a package leaves it open. - const closed = await this.packagelessOverlayWriteRefusal( - { type: PLURAL_TO_SINGULAR[it.type] ?? it.type, name: it.name, packageId: restorePackageId, state: 'active', ...memberOption(member) }, - 'rollback', - ); - if (closed) throw closed; + // [#29414, ADR-0048, ADR-0067 D3] The binding is the + // committing package's, never another package's row's: the + // restore writes the committing package's own row, in place + // when it is stored and created again when it was deleted + // since (`restoreVersion` is handed `commitPackageId` and + // writes it either way). So this door never writes the + // package-less row, and the package-less close has nothing + // here to judge (every commit names its package); the write + // naming the read-only base itself is closed below. + const restorePackageId: string = commitPackageId; // [#29390, ADR-0146 D1, D11] A restore writes the row this // binding names, so a restore into a row bound to the // read-only base itself (a commit recorded before #28905 @@ -20380,9 +20424,8 @@ export class ObjectStackProtocolImplementation implements // in `failed[]` below, with nothing written. Judged under // the SINGULAR type, the spelling the write-through // registers under. See {@link restoredBodyWriter}. - deriveRestoredBody: this.restoredBodyWriter( - PLURAL_TO_SINGULAR[it.type] ?? it.type, it.name, restorePackageId, 'protocol.revertCommit', - ), + // [ADR-0146 D2] …and, for the pair's row, the override write gate. + deriveRestoredBody, ...memberOption(member), }); // [#6621] #4521 — a revert is a live write like any other: the @@ -20772,21 +20815,10 @@ export class ObjectStackProtocolImplementation implements // the override write gate as that save is ({@link overrideWriteVerdict} // into the pair): a version that declares no override is refused as // the save is, with nothing written. - const owner = artifactBacked ? this.packagedArtifactOwner({ type: singularType, name: request.name }) : undefined; - const pairRowRollback = owner !== undefined && isCustomizationPairOf(rollbackPackageId, owner); - const intent: 'override-artifact' | 'runtime-only' = artifactBacked && !pairRowRollback - ? 'override-artifact' : 'runtime-only'; - const writeRestored = this.restoredBodyWriter(singularType, request.name, rollbackPackageId, 'protocol.rollbackMetaItem'); - const deriveRestoredBody = pairRowRollback - ? async (historyBody: unknown): Promise => { - const body = await writeRestored(historyBody); - const verdict = this.overrideWriteVerdict({ - type: singularType, name: request.name, packageId: rollbackPackageId, item: body, - }); - if (verdict.verdict === 'refuse') throw overrideWriteRefusalError(verdict); - return body; - } - : writeRestored; + // [#29414] The derivation a commit revert's restore limb shares ({@link restoreWriteFor}). + const { intent, deriveRestoredBody } = this.restoreWriteFor( + singularType, request.name, rollbackPackageId, artifactBacked, 'protocol.rollbackMetaItem', + ); try { const result = await repo.restoreVersion(ref, request.toVersion, { // #4556 — NULL, not 'system', for an actor-less rollback. diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index e3e022502ae..dc9ba82f943 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -1687,9 +1687,10 @@ export class SysMetadataRepository implements MetadataRepository { * [#29414] The package whose active row is restored. Stated → the * active row (its parent hash and its binding) is resolved under the * ADR-0048 key `(org, type, name, package_id)`, so another package's - * row of the same name is never the one restored; no such row → the - * unbound row, as below. Omitted → the "match any package" resolution, - * as before. A LOCAL extension, as on {@link delete}. + * row of the same name is never the one restored, and the restore is + * written under it, creating that package's row again when it is gone. + * Omitted → the "match any package" resolution and, with no active row, + * the unbound row, as before. A LOCAL extension, as on {@link delete}. */ packageId?: string | null; }, @@ -1764,7 +1765,8 @@ export class SysMetadataRepository implements MetadataRepository { intent: opts.intent ?? 'override-artifact', state: 'active', opType: 'revert', - packageId: activePackageId, + // [#29414] The stated package's row, found or not. + packageId: 'packageId' in opts ? (opts.packageId ?? null) : activePackageId, }); } From a8758fbc9539bcdd986541dd96b07fd3236db8f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 05:29:21 +0000 Subject: [PATCH 7/8] fix(spec): the packageless-overlay-write-closed entry names the commit doors; changeset and dogfood case follow the review Claude-Session: https://claude.ai/code/session_01JQwxbZ5jYNfPGLvPV8LYRe Co-authored-by: Claude --- .../29414-commit-revert-own-package-row.md | 30 ++++++++++++------- ...commit-revert-path-package.dogfood.test.ts | 9 +++--- .../18.packageless-overlay-write-closed.ts | 13 ++++++-- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/.changeset/29414-commit-revert-own-package-row.md b/.changeset/29414-commit-revert-own-package-row.md index 132ad6f12b5..e3707fa8387 100644 --- a/.changeset/29414-commit-revert-own-package-row.md +++ b/.changeset/29414-commit-revert-own-package-row.md @@ -1,35 +1,43 @@ --- '@objectstack/metadata-protocol': patch +'@objectstack/spec': patch --- -fix(metadata-protocol)!: a package commit's revert plan and its revert address the committing package's own row; a revert whose row was deleted asks the package-less close, as the per-item rollback does +fix(metadata-protocol)!: a package commit's revert plan and its revert address the committing package's own row, and a restore into a `.custom` pair's row is judged by the override write gate -Clause-②: no (narrowing) — the commit revert (`POST /api/v1/packages/:id/commits/:commitId/revert`) and the package rollback (`POST /api/v1/packages/:id/rollback`) no longer restore a stored version into the package-unbound row when the committing package's own row of the item is gone, on an item of the five overlay types one installed read-only package ships, where `os migrate packageless-overlays` would move such a row into the base's paired `.custom`; they inserted that package-less row before. The per-item rollback on the same package-less address has refused it since `packageless-overlay-write-closed`. +Clause-②: no (narrowing) — the commit revert (`POST /api/v1/packages/:id/commits/:commitId/revert`) and the package rollback (`POST /api/v1/packages/:id/rollback`) no longer restore a stored version into a `.custom` pair's row unless the version declares the override the pair's save requires, and no longer restore into another package's row of the name, or into a package-less row, when the committing package's own row was deleted since: they restore into the committing package's row, so a commit of an installed read-only package itself whose row is gone is refused by the close naming the pair. They restored into whichever row of the name the store returned first, or inserted a package-less row, before. Metadata-protocol: none -Skills-docs: none needed — `skills/objectstack-platform/SKILL.md` already says the package-less save, reset and rollback of a movable row are refused once movable (D11), and places every customization of an installed base in its paired `.custom`; nothing it says becomes false. +Skills-docs: none needed — `skills/objectstack-platform/SKILL.md` places every customization of an installed base in its paired `.custom`, declaring `overrides`, and says a pair row is honoured only when declared; nothing it says becomes false. - + -**BREAKING** write narrowing at the commit doors (`@objectstack/metadata-protocol`), shipped as `patch` (`Metadata-protocol: none`). +**BREAKING** write narrowing at the commit doors (`@objectstack/metadata-protocol`), shipped as `patch` (`Metadata-protocol: none`). `@objectstack/spec` carries the text widening of the `packageless-overlay-write-closed` upgrade entry. -The plan a package publish records (`POST /api/v1/packages/:id/publish-drafts`, the commit's `existedBefore` and `prevVersion` per item) now reads the publishing package's own active row, and the revert removes or restores that row only. Before, both read any package's active row of the name: the first publish of a `.custom` pair's declared override beside a row bound to the base (stored before the write naming the base was closed) was recorded as an edit of the base row's version, and its revert addressed the base row, refused there instead of removing the pair's row. It now removes the pair's row, and the row bound to the base is served again. Commits recorded before this change keep the plan they recorded. +The plan a package publish records (`POST /api/v1/packages/:id/publish-drafts`, the commit's `existedBefore` and `prevVersion` per item) now reads the publishing package's own active row, and the revert removes or restores that row only. Before, both read any package's active row of the name: the first publish of a `.custom` pair's declared override beside a row bound to the base (stored before the write naming the base was closed) was recorded as an edit of the base row's version, and its revert addressed the base row and was refused there instead of removing the pair's row. It now removes the pair's row, and the row bound to the base is served again. + +A pair commit recorded before this change beside such a row still names the base row's version as the pair's pre-commit state. Its revert now restores into the pair's own row, so the restored body meets the override write gate exactly as the pair's per-item rollback does: the base row's body declares no override, so the item is refused per item (`403 NOT_OVERRIDABLE`, the save door's sentence) and the pair's declared row stays as it is. ### Migration: FROM → TO -FROM, undoing a commit of the installed package `com.acme.crm` (recorded before the write naming it was closed) that edited a customization whose row was deleted since: +FROM, undoing a pair commit recorded before this change beside a row bound to the installed package `com.acme.crm`: ```http -POST /api/v1/packages/com.acme.crm/commits/cmt_123/revert +POST /api/v1/packages/com.acme.crm.custom/commits/cmt_123/revert ``` -It now answers `200` with `success: false`, and the item in `failed[]` with `code: NOT_OVERRIDABLE` and the sentence the per-item rollback gives; nothing is written. TO, write the version you want back into the base's paired `.custom`, declaring what it overrides: +It answers `200` with `success: false`, and the item in `failed[]` with `code: NOT_OVERRIDABLE`; nothing is written. TO, write the version you want into the pair, declaring what it overrides, or remove the pair's row to serve what is under it: ```http PUT /api/v1/meta/view/account_list?package=com.acme.crm.custom { "name": "account_list", "label": "My accounts", "overrides": { "package": "com.acme.crm", "basedOn": "1.4.0" } } + +DELETE /api/v1/meta/view/account_list?package=com.acme.crm.custom ``` -- **Now refused, per item.** The restore of a commit item whose row in the committing package is gone, of a `view`, `dashboard`, `report`, `translation` or `email_template` one read-only package ships, where the package-less close applies (the row would be one `os migrate packageless-overlays` moves). The commit's other items still revert, and `POST /api/v1/packages//rollback` reports the item in its `failed[]` and continues. -- **Still open.** Every restore into a row the committing package holds (a writable package's in place, the pair's in place), every removal, and every package-less restore the close leaves open (a name no read-only package ships, or one several ship). +FROM, undoing a commit of `com.acme.crm` itself (recorded before the write naming it was closed) that edited a customization whose row was deleted since: the restore wrote a package-less row. It now restores into the row bound to `com.acme.crm`, which the close refuses (`NOT_OVERRIDABLE`, the rollback sentence naming the pair); TO, the same `PUT …?package=com.acme.crm.custom` with the `overrides` declaration. + +- **Now refused, per item.** A restore into a `.custom` pair's row of a version that does not declare the override the pair's save requires (the override write gate). A restore of a commit of a read-only package itself into its row bound to the package, now also when that row was deleted since (the close naming the pair). The commit's other items still revert, and `POST /api/v1/packages//rollback` reports each refused item in its `failed[]` and continues. +- **Now restored into the committing package's row.** A commit whose row was deleted since, of a writable package or of the pair (a declared version): the row is created again under that package, never as a package-less row. +- **Still open.** Every restore into a row the committing package holds that the gate admits (a writable package's in place, the pair's declared version in place), and every removal. diff --git a/packages/qa/dogfood/test/package-commit-revert-path-package.dogfood.test.ts b/packages/qa/dogfood/test/package-commit-revert-path-package.dogfood.test.ts index b4248d4fc50..ac2e220c592 100644 --- a/packages/qa/dogfood/test/package-commit-revert-path-package.dogfood.test.ts +++ b/packages/qa/dogfood/test/package-commit-revert-path-package.dogfood.test.ts @@ -55,8 +55,9 @@ // [#29414] A commit names its own package's rows. The pair's first publish // beside such a base-bound row is recorded as a create (its plan used to read // the base row's version), so its revert removes the pair's row and the base -// row is served again; a revert whose row was deleted since asks the -// package-less close, as the per-item rollback does. Control: a writable +// row is served again. A revert whose row was deleted since restores into the +// committing package's own row, never a package-less one, so the base's own +// commit is refused by the close naming the pair. Control: a writable // package's edit commit restores its own row in place. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -349,7 +350,7 @@ describe('dogfood: package commit revert and rollback act only on a commit of th expect(await servedLabel()).toBe(shipped); }); - it('[#29414] a commit that edited a base-bound row since deleted is refused by the package-less close, as the per-item rollback is, and writes nothing', async () => { + it('[#29414] a commit that edited a base-bound row since deleted restores into the base\'s own row, so it is refused by the close naming the pair, and writes nothing', async () => { const shipped = await servedLabel(); await publishBaseBound('Deleted later 1'); const edited = await publishBaseBound('Deleted later 2'); @@ -361,7 +362,7 @@ describe('dogfood: package commit revert and rollback act only on a commit of th expect(res.status, JSON.stringify(res.json)).toBe(200); expect(res.json?.data).toMatchObject({ success: false, revertedCount: 0, failedCount: 1 }); expect(res.json?.data?.failed?.[0]).toMatchObject({ type: 'view', name: OVERLAY, code: 'NOT_OVERRIDABLE' }); - expect(res.json.data.failed[0].error).toContain('a rollback of its package-less row is closed'); + expect(res.json.data.failed[0].error).toContain('a rollback of its row bound to the read-only base is closed'); expect(res.json.data.failed[0].error).toContain(`?package=${SHOWCASE}.custom`); expect(await servedLabel()).toBe(shipped); }); diff --git a/packages/spec/src/migrations/entries/semantic/18.packageless-overlay-write-closed.ts b/packages/spec/src/migrations/entries/semantic/18.packageless-overlay-write-closed.ts index f33a735b4c6..749a05d9631 100644 --- a/packages/spec/src/migrations/entries/semantic/18.packageless-overlay-write-closed.ts +++ b/packages/spec/src/migrations/entries/semantic/18.packageless-overlay-write-closed.ts @@ -9,7 +9,9 @@ import type { SemanticMigration } from '../../types.js'; // its row as a declared override (a row it moves as the pair's own member // stays open), and a save, publish or rollback naming the read-only package // itself onto an item it ships is refused where the pair can take the -// declared override (a delete naming the package stays open). +// declared override (a delete naming the package stays open; a commit revert +// or package rollback restoring a commit item into such a row is that +// rollback, refused per item). // Registered because an upgrading deployment can carry the rows those writes // made, package-less or bound to the read-only package itself, and the // operator step that moves both kinds into the base's `.custom` pair ships in @@ -28,7 +30,10 @@ export const entry: SemanticMigration = { + 'member, an email template locale the base does not ship, keeps the write); (2) naming the ' + 'read-only package itself (a save in draft or publish mode or a per-item publish under ' + '/api/v1/meta//?package=, POST /api/v1/packages//publish-drafts ' - + 'holding such a draft, and a rollback of a row bound to the package), where the pair can be ' + + 'holding such a draft, and a rollback of a row bound to the package: the per-item rollback, ' + + 'and the restore of a commit item into that row by POST /api/v1/packages//commits/' + + '/revert and POST /api/v1/packages//rollback, reported per item in their ' + + 'failed[]), where the pair can be ' + 'ensured, the base records a version and the override write gate admits the declared body ' + 'into the pair (a delete naming the package keeps the write); and the sys_metadata rows ' + 'those writes stored, package-less or bound to the read-only package itself', @@ -80,7 +85,9 @@ export const entry: SemanticMigration = { + 'member, keeps its package-less write. The close reaches only what the command moves as a ' + 'declared override (its own classification and its own move steps), so no customization is ' + 'left without a write door. The save, publish or rollback naming the read-only package ' - + 'itself is refused 403 NOT_OVERRIDABLE naming the pair and the `overrides` declaration ' + + 'itself (a commit revert or package rollback restoring a commit item into the row bound to ' + + 'it among them, which reports the item in its failed[] and reverts the rest) is refused 403 ' + + 'NOT_OVERRIDABLE naming the pair and the `overrides` declaration ' + 'wherever the pair can take the declared override (it can be ensured, the base records a ' + 'version and the override write gate admits the declared body); a delete naming the package ' + 'stays open, as the way to remove such a row, and so does every write the pair could not ' From 52e25699e3c294485da93554dae660cce05c740a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 09:10:40 +0000 Subject: [PATCH 8/8] test(objectql): the publish plan read names the publishing package (#29414) The two objectql capture-read pins held the plan read's old shape { type, name, state }; publishPackageDrafts now scopes it with package_id: request.packageId, so each read carries the draft's package. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01JQwxbZ5jYNfPGLvPV8LYRe --- .../src/protocol-publish-package-drafts.test.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/objectql/src/protocol-publish-package-drafts.test.ts b/packages/objectql/src/protocol-publish-package-drafts.test.ts index fe2a8b7608f..404293e2cf6 100644 --- a/packages/objectql/src/protocol-publish-package-drafts.test.ts +++ b/packages/objectql/src/protocol-publish-package-drafts.test.ts @@ -157,9 +157,9 @@ describe('protocol.publishPackageDrafts (ADR-0033 / ADR-0067 D2)', () => { // one per draft, in draft order, each in the draft's own scope. (Reads // after these belong to the ADR-0038 L3 probes, which run post-commit.) expect(captureReads.slice(0, 3)).toEqual([ - { type: 'object', name: 'course', state: 'active' }, - { type: 'object', name: 'student', state: 'active' }, - { type: 'view', name: 'course_list', state: 'active' }, + { type: 'object', name: 'course', state: 'active', package_id: 'app.edu' }, + { type: 'object', name: 'student', state: 'active', package_id: 'app.edu' }, + { type: 'view', name: 'course_list', state: 'active', package_id: 'app.edu' }, ]); expect(promote).toHaveBeenCalledTimes(3); expect((promote.mock.calls[0][0] as any)).toMatchObject({ type: 'object', name: 'course' }); @@ -205,10 +205,12 @@ describe('protocol.publishPackageDrafts (ADR-0033 / ADR-0067 D2)', () => { expect(res).toMatchObject({ success: true, publishedCount: 2 }); // The capture ran for both items, in their own scope — this is what makes - // the values below evidence rather than defaults. + // the values below evidence rather than defaults. [#29414] The scope names + // the publishing package: another package's row of the name is not the + // item's pre-publish state. expect(captureReads.slice(0, 2)).toEqual([ - { type: 'object', name: 'course', state: 'active' }, - { type: 'object', name: 'student', state: 'active' }, + { type: 'object', name: 'course', state: 'active', package_id: 'app.edu' }, + { type: 'object', name: 'student', state: 'active', package_id: 'app.edu' }, ]); // One commit, carrying one entry per promoted item, each with the state // that was actually READ.