fix(metadata-protocol)!: a commit revert or package rollback that would restore a row bound to the read-only base is refused per item, naming the .custom pair

.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 (`<base>`) itself, on an item of the five overlay types that package ships, where the customization can be written into the base's paired `<base>.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.
+
+<!-- adr-0087: not-required (already-registered packageless-overlay-write-closed) That entry, unreleased and shipping in the same release, already prescribes this change's TO for every customization of an installed package: write it into the base's paired `<base>.custom` with `PUT /api/v1/meta/<type>/<name>?package=<base>.custom` and the `overrides` declaration (its replacement), because the customization lives there (its reason, ADR-0146 D1, D2, D11). This change closes the commit doors' restore onto a row bound to the read-only base, the third write that still landed such a row after the save, publish and per-item rollback naming the base were closed, onto the same TO. No stored row is rewritten or moved, so `objectstack migrate meta` has nothing new to do. The entry names both meta-door addresses (the write naming no package, and the save, publish or rollback naming the read-only package, a rollback of a row bound to it included) but not yet the commit doors (`POST /api/v1/packages/<base>/commits/<commitId>/revert` and `POST /api/v1/packages/<base>/rollback`); that text widening is a `packages/spec` edit this change does not make, and it rides #29414. -->
+
+**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/<base>/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).

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<PackagelessWriteOperation, 'delete'>;
 

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<string, unknown>, beforeClose = false): Promise<string> {
+        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');
+    });
+});

packages/metadata-protocol/src/protocol.ts

@@ -19888,6 +19888,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;
@@ -20241,6 +20250,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',