From 9e4773d6d69633163a50da9fba36620bf3dd4f18 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 14:26:04 +0000 Subject: [PATCH 1/3] fix(metadata-protocol)!: a commit revert restoring a row bound to the read-only base is refused per item, naming the pair revertCommit's restore limb (and rollbackToPackageCommit through it) now asks the named-base close on the binding the restore writes, as rollbackMetaItem does, with the same rollback sentence. The item lands in failed[] (NOT_OVERRIDABLE) with nothing written; the soft-remove limb stays open. Claude-Session: https://claude.ai/code/session_01ADHkmTKKCrTrtA7H6oSj28 Co-authored-by: Claude --- .../29390-named-base-commit-revert-closed.md | 35 ++++++ .../src/protocol-packaged-base-refusal.ts | 13 +- .../protocol.named-base-overlay-write.test.ts | 115 +++++++++++++++++- packages/metadata-protocol/src/protocol.ts | 23 ++++ ...commit-revert-path-package.dogfood.test.ts | 71 ++++++++++- 5 files changed, 245 insertions(+), 12 deletions(-) create mode 100644 .changeset/29390-named-base-commit-revert-closed.md diff --git a/.changeset/29390-named-base-commit-revert-closed.md b/.changeset/29390-named-base-commit-revert-closed.md new file mode 100644 index 00000000000..d9bd3c58d80 --- /dev/null +++ b/.changeset/29390-named-base-commit-revert-closed.md @@ -0,0 +1,35 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +fix(metadata-protocol)!: a commit revert or package rollback that would restore a row bound to an installed read-only package onto its view, dashboard, report, translation or email template is refused per item, naming the base's `.custom` pair + +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 row bound to an installed read-only package (``) itself, on an item of the five overlay types that package ships, where the customization can be written into the base's paired `.custom` instead; they rewrote that row before. The per-item rollback of the same row has refused it since `named-base-overlay-write-closed`. + +Metadata-protocol: none + +Skills-docs: none needed — `skills/objectstack-platform/SKILL.md` places every customization of an installed base in its paired `.custom`, declaring `overrides`, and says nothing about reverting a commit recorded against the base itself; nothing it says becomes false. + + + +**BREAKING** write narrowing at the commit doors (`@objectstack/metadata-protocol`), shipped as `patch` (`Metadata-protocol: none`). + +### Migration: FROM → TO + +FROM, undoing a commit that the installed package `com.acme.crm` recorded before the write naming it was closed (a customization published into the package itself), where the commit edited the customization: + +```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 (the pair's declared override is served over the row bound to the base): + +```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" } } +``` + +Or remove the row bound to the base (`DELETE /api/v1/meta/view/account_list?package=com.acme.crm`) to serve the package's own document again. + +- **Now refused, per item.** The restore of a commit item into a row bound to the read-only base itself, of a `view`, `dashboard`, `report`, `translation` or `email_template` that the base ships, where the pair can be ensured (the base has a namespace and a SemVer version) and the override write gate admits the declared body into it. The commit's other items still revert, and the revert is still recorded. `POST /api/v1/packages//rollback` reports each refused item in its `failed[]` (`success: false`) and continues through the remaining commits. +- **Still open.** The revert of an item the commit created: it removes the row bound to the base, which writes nothing and is the one way to clear such a row (as a delete naming the base is). Every revert whose restore lands in a row of the pair, a writable package or no package, and every restore the pair could not take (a base whose pair cannot be ensured, a base with no version, an `email_template` locale the base does not ship). diff --git a/packages/metadata-protocol/src/protocol-packaged-base-refusal.ts b/packages/metadata-protocol/src/protocol-packaged-base-refusal.ts index afbe987568e..410bf525f92 100644 --- a/packages/metadata-protocol/src/protocol-packaged-base-refusal.ts +++ b/packages/metadata-protocol/src/protocol-packaged-base-refusal.ts @@ -295,11 +295,14 @@ function closedOverlayWriteError(message: string): Error { /** * The writes {@link namedBaseOverlayWriteRefusal} judges: the three that land - * a row bound to the base. ⛔ Not `delete`: removing a row bound to the base - * writes nothing, and it is the one way to clear such a row stored before the - * close — `os migrate packageless-overlays` reads package-less rows only, so - * it never moves one. The removal serves the base's own document again, or - * the pair's declared override where one exists (it is served over the row). + * a row bound to the base. `rollback` is every restore of a stored version: + * the per-item rollback and [#29390] a commit revert's restore limb (and the + * package rollback through it), one sentence for both. ⛔ Not `delete`: + * removing a row bound to the base writes nothing, and it is the one way to + * clear such a row stored before the close — `os migrate packageless-overlays` + * reads package-less rows only, so it never moves one. The removal serves the + * base's own document again, or the pair's declared override where one exists + * (it is served over the row). */ export type NamedBaseWriteOperation = Exclude; 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 68ad160302b..76f04303ec6 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 @@ -35,7 +35,7 @@ * Refusals are asserted as the envelope an HTTP boundary answers with * (`resolveThrownHttpError`). */ -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { assertEngineDeleteDispatch, assertEngineFindOnePredicate, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; import { resolveThrownHttpError } from '@objectstack/types'; import { ObjectStackProtocolImplementation } from './protocol.js'; @@ -206,6 +206,7 @@ beforeEach(() => { afterEach(() => { ObjectStackProtocolImplementation.resetEnvWritableCache(); resetEnvWritableMetadataTypes(); + vi.useRealTimers(); }); describe('a write naming the read-only base itself is closed: its customization lands in the pair (ADR-0146 D1, D11)', () => { @@ -362,3 +363,115 @@ describe('CONTROLS: the writes the close leaves open, as on main', () => { expect(rows(VIEW.type, VIEW.name)).toEqual([]); }); }); + +/** + * [#29390] The commit doors. A commit the base recorded before the close (a + * draft naming the base, published by `publishPackageDrafts` with the close + * shadowed on this instance only, which is what a build before #28905 did) + * can carry an item that EDITED the row bound to the base. Its revert restores + * a stored version into that row, so it is refused per item, as the per-item + * rollback of the same row is: the restore limb asks the same close on the + * same key (the binding the restore writes) with the same sentence. An item + * the commit CREATED is reverted by removing the row, which writes nothing and + * stays open. + */ +describe('the commit doors: restoring a row bound to the base is closed, removing one stays open (#29390)', () => { + /** Each commit a second apart, so the package rollback's "strictly newer" reads an order. */ + let clock = Date.parse('2026-11-02T00:00:00.000Z'); + const tick = () => { clock += 1000; vi.setSystemTime(clock); }; + + /** Publish `item` into `packageId` as one commit; `beforeClose` shadows the close on this instance only. */ + async function publishCommit(protocol: any, packageId: string, item: Record, beforeClose = false): Promise { + tick(); + if (beforeClose) protocol.namedBaseOverlayWriteRefusal = () => null; + try { + const saved = await protocol.saveMetaItem({ type: VIEW.type, name: VIEW.name, item, packageId, mode: 'draft' }); + expect(saved).toMatchObject({ success: true }); + const published = await protocol.publishPackageDrafts({ packageId }); + expect(published, JSON.stringify(published)).toMatchObject({ success: true }); + expect(typeof published.commitId).toBe('string'); + return published.commitId; + } finally { + if (beforeClose) delete protocol.namedBaseOverlayWriteRefusal; + } + } + const intoBaseBeforeClose = (protocol: any, label: string) => publishCommit(protocol, BASE, VIEW.body(label), true); + const declared = (label: string) => ({ ...VIEW.body(label), overrides: { package: BASE, basedOn: BASE_VERSION } }); + + beforeEach(() => { + vi.useFakeTimers({ toFake: ['Date'] }); + }); + + it('a revert of the base\'s commit that edited its row is refused per item naming the pair, and nothing is written', async () => { + const { protocol, rows } = makeProtocol(); + await intoBaseBeforeClose(protocol, 'First'); + const edit = await intoBaseBeforeClose(protocol, 'Second'); + // The close is live again on this instance: the save naming the base is refused. + expectClosed(await thrown(() => protocol.saveMetaItem({ type: VIEW.type, name: VIEW.name, item: VIEW.body('After'), packageId: BASE })), VIEW.type, VIEW.name, 'save'); + + 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' })]); + expectClosedSentence(result.failed[0].error, VIEW.type, VIEW.name, 'rollback'); + expect(rows(VIEW.type, VIEW.name)).toEqual([{ package_id: BASE, state: 'active', label: 'Second' }]); + expect((await protocol.getMetaItem({ type: VIEW.type, name: VIEW.name })).item.label).toBe('Second'); + }); + + it('a package rollback through those commits reports each refused item and continues; the row is unchanged', async () => { + const { protocol, rows } = makeProtocol(); + const created = await intoBaseBeforeClose(protocol, 'First'); + const second = await intoBaseBeforeClose(protocol, 'Second'); + const third = await intoBaseBeforeClose(protocol, 'Third'); + + tick(); + const result = await protocol.rollbackToPackageCommit({ commitId: created, packageId: BASE, actor: 'admin' }); + + expect(result.success).toBe(false); + expect(result.revertedCommits).toEqual([third, second]); + expect(result.failed.map((f: { commitId: string }) => f.commitId)).toEqual([third, second]); + for (const f of result.failed) { + expect(f.error).toContain(`Item '${VIEW.type}/${VIEW.name}' was not reverted: `); + expectClosedSentence(f.error.slice(f.error.indexOf(': ') + 2), VIEW.type, VIEW.name, 'rollback'); + } + expect(rows(VIEW.type, VIEW.name)).toEqual([{ package_id: BASE, state: 'active', label: 'Third' }]); + }); + + it('CONTROL: a revert of the base\'s commit that created its row removes it (the delete stays open); the base is served again', async () => { + const { protocol, rows } = makeProtocol(); + const created = await intoBaseBeforeClose(protocol, 'First'); + + tick(); + const result = await protocol.revertCommit({ commitId: created, packageId: BASE, 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([]); + expect((await protocol.getMetaItem({ type: VIEW.type, name: VIEW.name })).item.label).toBe(PACKAGED); + }); + + it('CONTROL: a writable package\'s commit of the same item reverts its row in place', async () => { + const { protocol, rows } = makeProtocol(); + await publishCommit(protocol, WRITABLE, VIEW.body('First')); + const edit = await publishCommit(protocol, WRITABLE, VIEW.body('Second')); + + 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' }]); + }); + + it('CONTROL: the pair\'s own commit (no row bound to the base) reverts its row in place', async () => { + const { protocol, rows } = makeProtocol(); + await publishCommit(protocol, PAIR, declared('PairFirst')); + const edit = await publishCommit(protocol, PAIR, declared('PairSecond')); + + 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'); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index a49c188c559..19bf7c8f525 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -22083,6 +22083,15 @@ export class ObjectStackProtocolImplementation implements * performs its promise exactly, and the removal is the one action that * reduces the residue. The measurement and the refuse-vs-decline reasoning * are stated at the gate itself. + * + * [#29390, ADR-0146 D1, D11] The restore limb asks the #28905 close + * ({@link namedBaseOverlayWriteRefusal}) on the binding it writes, as + * {@link rollbackMetaItem} does: an item whose restore would land in a row + * bound to the read-only base itself — a commit the base recorded before + * the write naming it closed — goes into `failed[]` (`NOT_OVERRIDABLE`, the + * 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. */ async revertCommit(request: { commitId: string; @@ -22436,6 +22445,20 @@ export class ObjectStackProtocolImplementation implements // 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); + // [#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 + // closed the write naming the base) is refused exactly as + // {@link rollbackMetaItem} refuses it, on the same key and + // with the same sentence ({@link namedBaseOverlayWriteRefusal}), + // into `failed[]` with nothing written. The commit's other + // items still revert. The soft-remove limb above stays open: + // removing a row bound to the base writes nothing. + const namedBase = this.namedBaseOverlayWriteRefusal( + { type: PLURAL_TO_SINGULAR[it.type] ?? it.type, name: it.name, packageId: restorePackageId, ...memberOption(member) }, + 'rollback', + ); + if (namedBase) throw namedBase; const restored = await repo.restoreVersion(ref, restoreToVersion, { actor, source: 'protocol.revertCommit', 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 71f76137d2c..eb1c036b133 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 @@ -22,7 +22,7 @@ // still answer 200 and record their commits; a member without // `manage_metadata` is still refused 403 first. // -// ## Read-only packages carry commits too, and keep their revert +// ## Read-only packages carry commits too, and what their revert may still do // // Measured on main: an environment overlay draft on a showcase item, bound to // `com.example.showcase` (a booted code package, read-only) and published with @@ -34,11 +34,19 @@ // // [#28905, ADR-0146 D1] That draft naming the read-only showcase itself is // refused now (`403 NOT_OVERRIDABLE`, naming its `.custom` pair), so a fresh -// boot can no longer give the showcase such a commit; one recorded before -// keeps its revert, as the commit doors still take no read-only guard. The -// last case pins the refusal, then the same customization written where D1 -// puts it, the showcase's pair: its commit reverts through the pair's own -// path, and through no other. +// boot can no longer give the showcase such a commit. The commit doors still +// take no package-level read-only guard. The second-to-last case pins the +// refusal, then the same customization written where D1 puts it, the +// showcase's pair: its commit reverts through the pair's own path, and +// through no other. +// +// [#29390] A commit the showcase recorded before that close keeps its revert +// only where the revert REMOVES the row (an item the commit created). An item +// it edited is refused per item (`failed[]`, `NOT_OVERRIDABLE`, the rollback +// sentence naming the pair), as the per-item rollback of that row is: +// 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). import { describe, it, expect, beforeAll, afterAll } from 'vitest'; import { type VerifyStack } from '@objectstack/verify'; @@ -192,4 +200,55 @@ describe('dogfood: package commit revert and rollback act only on a commit of th }); expect(await commitsOf(pair)).toHaveLength(before.length + 1); }); + + it('[#29390] the showcase\'s commits recorded before the close: an edited item\'s revert is refused naming the pair, a created one\'s still removes it', async () => { + const protocol = (stack.kernel as any).getService('protocol'); + const served = async () => (await call('GET', `/meta/view/${OVERLAY}`)).json?.data?.item?.label; + const shipped = await served(); + const view = (label: string) => ({ + name: OVERLAY, object: 'showcase_task', viewKind: 'list', label, + config: { type: 'grid', columns: [{ field: 'title' }] }, + }); + const publishBeforeClose = async (label: string): Promise => { + 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; + } + }; + const created = await publishBeforeClose('Before close 1'); + const edited = await publishBeforeClose('Before close 2'); + expect((await call('PUT', `/meta/view/${OVERLAY}?mode=draft&package=${SHOWCASE}`, view('After close'))).status).toBe(403); + expect(await served()).toBe('Before close 2'); + const expectRefused = (error: string) => { + expect(error).toContain('a rollback of its row bound to the read-only base is closed'); + expect(error).toContain(`?package=${SHOWCASE}.custom`); + }; + + const revert = await call('POST', `/packages/${SHOWCASE}/commits/${edited}/revert`, {}); + expect(revert.status, JSON.stringify(revert.json)).toBe(200); + expect(revert.json?.data).toMatchObject({ success: false, revertedCount: 0, failedCount: 1 }); + expect(revert.json?.data?.failed?.[0]).toMatchObject({ type: 'view', name: OVERLAY, code: 'NOT_OVERRIDABLE' }); + expectRefused(revert.json.data.failed[0].error); + expect(await served()).toBe('Before close 2'); + + const rollback = await call('POST', `/packages/${SHOWCASE}/rollback`, { commitId: created }); + expect(rollback.status, JSON.stringify(rollback.json)).toBe(200); + expect(rollback.json?.data).toMatchObject({ success: false, revertedCommits: [edited] }); + expect(rollback.json?.data?.failed?.[0]?.commitId).toBe(edited); + expectRefused(rollback.json.data.failed[0].error); + expect(await served()).toBe('Before close 2'); + + const removal = await call('POST', `/packages/${SHOWCASE}/commits/${created}/revert`, {}); + expect(removal.status, JSON.stringify(removal.json)).toBe(200); + expect(removal.json?.data).toMatchObject({ + success: true, revertedCount: 1, reverted: [{ type: 'view', name: OVERLAY, action: 'removed' }], + }); + expect(await served()).toBe(shipped); + }); }); From 340e070f637642d8a9eb1fdab96233fdbbbed30d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Nov 2026 14:29:23 +0000 Subject: [PATCH 2/3] test(dogfood): read the served view label from the /meta item envelope Claude-Session: https://claude.ai/code/session_01ADHkmTKKCrTrtA7H6oSj28 Co-authored-by: Claude --- .../test/package-commit-revert-path-package.dogfood.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 eb1c036b133..3398157e0cd 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 @@ -203,8 +203,9 @@ describe('dogfood: package commit revert and rollback act only on a commit of th it('[#29390] the showcase\'s commits recorded before the close: an edited item\'s revert is refused naming the pair, a created one\'s still removes it', async () => { const protocol = (stack.kernel as any).getService('protocol'); - const served = async () => (await call('GET', `/meta/view/${OVERLAY}`)).json?.data?.item?.label; + const served = async () => (await call('GET', `/meta/view/${OVERLAY}`)).json?.item?.label; const shipped = await served(); + expect(typeof shipped).toBe('string'); const view = (label: string) => ({ name: OVERLAY, object: 'showcase_task', viewKind: 'list', label, config: { type: 'grid', columns: [{ field: 'title' }] }, From 85261a810dff62b530193fbe92e7b607158a76d3 Mon Sep 17 00:00:00 2001 From: "objectstack-fleet[bot]" <332303061+objectstack-fleet[bot]@users.noreply.github.com> Date: Mon, 2 Nov 2026 23:05:56 +0000 Subject: [PATCH 3/3] docs(changeset): the commit-revert close's adr-0087 marker says the entry names both meta-door addresses, not yet the commit doors The sentence carried over from the named-base close said the packageless-overlay-write-closed entry names the package-less write only; 1bdf674e widened it to both meta-door addresses. The commit doors are what it does not name yet; that text widening rides the any-package lookup card. Category and id unchanged. Claude-Session: https://claude.ai/code/session_01ADHkmTKKCrTrtA7H6oSj28 Co-authored-by: Claude --- .changeset/29390-named-base-commit-revert-closed.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/29390-named-base-commit-revert-closed.md b/.changeset/29390-named-base-commit-revert-closed.md index d9bd3c58d80..e6b8cdaf48a 100644 --- a/.changeset/29390-named-base-commit-revert-closed.md +++ b/.changeset/29390-named-base-commit-revert-closed.md @@ -10,7 +10,7 @@ Metadata-protocol: none Skills-docs: none needed — `skills/objectstack-platform/SKILL.md` places every customization of an installed base in its paired `.custom`, declaring `overrides`, and says nothing about reverting a commit recorded against the base itself; nothing it says becomes false. - + **BREAKING** write narrowing at the commit doors (`@objectstack/metadata-protocol`), shipped as `patch` (`Metadata-protocol: none`).