fix(metadata-protocol): the per-item rollback restores the row the read serves, so a .custom pair's override rolls back beside a base-bound row (#29848)

.changeset/rollback-addresses-served-row.md

@@ -0,0 +1,8 @@
+---
+'@objectstack/metadata-protocol': patch
+---
+
+Clause-②: no
+Metadata-protocol: none
+
+fix(metadata-protocol): the per-item rollback (`POST /api/v1/meta/:type/:name/rollback`) restores into the row the by-name read naming no package serves. A `.custom` pair's declared override stored beside a row bound to its read-only base (a row saved before the named-base close and not yet moved) is now rolled back in place, and the base row is left untouched; before, the door read a package-agnostic binding, addressed the base row, and was refused `403 NOT_OVERRIDABLE` naming a row the caller never meant. The door takes no new parameter. A rollback whose served row is the base's own is still refused by the named-base close.

packages/metadata-protocol/src/protocol-package-commits.ts

@@ -1906,7 +1906,7 @@ export class ProtocolPackageCommitsFacet {
     }
 
     /** [#28355] The body of {@link rollbackMetaItem} before its refusal audit; that method's docblock is the contract. */
-    static async rollbackMetaItemUnaudited(host: Pick<ProtocolHost, 'applyRegistryWriteThrough' | 'assertLockAllowsWrite' | 'emitMetadataMutation' | 'ensureOverlayIndex' | 'getOverlayRepo' | 'isArtifactBacked' | 'isOverlayAllowed' | 'isRuntimeCreateAllowed' | 'metadataConflictRefusal' | 'namedBaseOverlayWriteRefusal' | 'packagelessOverlayWriteRefusal' | 'receiptVersion' | 'recordMetadataAudit' | 'recordOptimisticConflictAudit' | 'refuseOrganizationScopedWrite' | 'resolveOverlayPackageBinding' | 'restoreWriteFor' | 'runMutationProjector'>, request: RollbackMetaItemRequest): Promise<{
+    static async rollbackMetaItemUnaudited(host: Pick<ProtocolHost, 'applyRegistryWriteThrough' | 'assertLockAllowsWrite' | 'emitMetadataMutation' | 'ensureOverlayIndex' | 'findServedOverlayRow' | 'getOverlayRepo' | 'isArtifactBacked' | 'isOverlayAllowed' | 'isRuntimeCreateAllowed' | 'metadataConflictRefusal' | 'namedBaseOverlayWriteRefusal' | 'packagelessOverlayWriteRefusal' | 'packagelessReadAddress' | 'receiptVersion' | 'recordMetadataAudit' | 'recordOptimisticConflictAudit' | 'refuseOrganizationScopedWrite' | 'resolveOverlayPackageBinding' | 'restoreWriteFor' | 'runMutationProjector'>, request: RollbackMetaItemRequest): Promise<{
             success: boolean;
             version: string;
             seq: number;
@@ -2025,7 +2025,31 @@ export class ProtocolPackageCommitsFacet {
         // it afterwards would put a fallible query downstream of a write that
         // already succeeded — the shape that ends in a `catch {}` swallowing a
         // real outage (#4867).
-        const rollbackPackageId = await host.resolveOverlayPackageBinding(singularType, request.name, member);
+        //
+        // [#29848, ADR-0146 D8 item 2] The row is the one the by-name read
+        // naming no package SERVES ({@link findServedOverlayRow} at
+        // {@link packagelessReadAddress}, its declared override first), so a
+        // rollback restores the item the caller sees. Beside a row bound to the
+        // read-only base (stored before #28905's close), the base's paired
+        // `.custom` row that declares the override is served, and its rollback
+        // restores it; the package-agnostic binding read answered whichever
+        // active row of the name the store returned first, so the pair's
+        // rollback addressed the base row and the named-base close below
+        // refused it, naming a row the caller never meant. No row served (a
+        // row only another package binds under the name): that row's binding,
+        // as before.
+        const servedPackage = await host.packagelessReadAddress(singularType, request.name, member);
+        const served = await host.findServedOverlayRow({
+            type: singularType,
+            name: request.name,
+            state: 'active',
+            ...(servedPackage !== undefined ? { packageId: servedPackage } : {}),
+            ...memberOption(member),
+            servedOverride: true,
+        });
+        const rollbackPackageId = served !== undefined
+            ? ((served.row as { package_id?: string | null } | null)?.package_id ?? null)
+            : await host.resolveOverlayPackageBinding(singularType, request.name, member);
         // [ADR-0146 D11, D1] A rollback writes the row its binding names, so
         // a rollback of a packaged item's package-less row (an overlay type's
         // row the ceremony has not moved, or none stored, whose restore writes
@@ -2081,6 +2105,9 @@ export class ProtocolPackageCommitsFacet {
                 deriveRestoredBody,
                 // [#22877] The member whose lineage `toVersion` is taken from.
                 ...memberOption(member),
+                // [#29848, #29414] The row the binding above names, and no other
+                // package's row of the name: its parent hash, and the write.
+                packageId: rollbackPackageId,
             });
             // #4521 — a rollback is a live write like any other: the restored
             // body must be the one the runtime dispatches on immediately, not

packages/metadata-protocol/src/protocol.named-base-overlay-write.test.ts

@@ -648,3 +648,84 @@ describe('the commit doors: restoring a row bound to the base is closed, removin
         expect(rows(VIEW.type, VIEW.name)).toEqual([{ package_id: WRITABLE, state: 'active', label: 'First' }]);
     });
 });
+
+/**
+ * [#29848, ADR-0146 D2, D8 item 2, D11] The per-item rollback
+ * (`POST /api/v1/meta/:type/:name/rollback`) beside a row bound to the base.
+ * The door names no package: it restores into the row the by-name read naming
+ * no package SERVES (the pair's declared override over a row bound to the
+ * base, `findServedOverlayRow`), never whichever active row of the name the
+ * store returns first. Before, the binding read and the restore's active-row
+ * read were both package-agnostic, so with the base row stored first the
+ * rollback of the pair's override addressed the base row and was refused by
+ * the named-base close, naming a row the caller never meant.
+ */
+describe('the per-item rollback addresses the row the read serves: the pair\'s declared override beside a row bound to the base (#29848)', () => {
+    const declared = (label: string) => ({ ...VIEW.body(label), overrides: { package: BASE, basedOn: BASE_VERSION } });
+
+    /** The history version whose body carries `label`. */
+    async function versionOf(protocol: any, label: string): Promise<number> {
+        const history = (await protocol.engine.find('sys_metadata_history', { where: { type: VIEW.type, name: VIEW.name } })) as Row[];
+        const hit = history.filter((h) => h.metadata != null && JSON.parse(h.metadata).label === label);
+        expect(hit, `one history row labelled ${label}`).toHaveLength(1);
+        return hit[0]!.version as number;
+    }
+
+    async function savePair(protocol: any, label: string) {
+        const saved = await protocol.saveMetaItem({ type: VIEW.type, name: VIEW.name, packageId: PAIR, item: declared(label) });
+        expect(saved, JSON.stringify(saved)).toMatchObject({ success: true });
+    }
+
+    it('the measured two-row state (the base row stored first): the pair\'s row rolls back, the base row is untouched', async () => {
+        const { protocol, rows, storeBaseRow } = makeProtocol();
+        await storeBaseRow(VIEW.type, VIEW.name, VIEW.body('BaseRow'));
+        await savePair(protocol, 'PairFirst');
+        await savePair(protocol, 'PairSecond');
+        expect((await protocol.getMetaItem({ type: VIEW.type, name: VIEW.name })).item.label).toBe('PairSecond');
+
+        const receipt = await protocol.rollbackMetaItem({
+            type: VIEW.type, name: VIEW.name, toVersion: await versionOf(protocol, 'PairFirst'), actor: 'admin',
+        });
+
+        expect(receipt).toMatchObject({ success: true });
+        expect(rows(VIEW.type, VIEW.name)).toEqual([
+            { package_id: BASE, state: 'active', label: 'BaseRow' },
+            { package_id: PAIR, state: 'active', label: 'PairFirst' },
+        ]);
+        const pairRow = await protocol.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');
+        expect((await protocol.getMetaItem({ type: VIEW.type, name: VIEW.name, packageId: BASE })).item.label).toBe('PairFirst');
+    });
+
+    it('the pair\'s rollback to the base row\'s version is the override write gate\'s refusal, not the named-base close; nothing is written', async () => {
+        const { protocol, rows, storeBaseRow } = makeProtocol();
+        await storeBaseRow(VIEW.type, VIEW.name, VIEW.body('BaseRow'));
+        await savePair(protocol, 'PairFirst');
+
+        const toVersion = await versionOf(protocol, 'BaseRow');
+        const refused = await thrown(() => protocol.rollbackMetaItem({ type: VIEW.type, name: VIEW.name, toVersion, actor: 'admin' }));
+
+        expect(resolveThrownHttpError(refused)).toMatchObject({ status: 403, code: 'NOT_OVERRIDABLE' });
+        expect(String(refused.message)).not.toContain('a rollback of its row bound to the read-only base is closed');
+        expect(rows(VIEW.type, VIEW.name)).toEqual([
+            { package_id: BASE, state: 'active', label: 'BaseRow' },
+            { package_id: PAIR, state: 'active', label: 'PairFirst' },
+        ]);
+    });
+
+    it('once the pair\'s row is deleted, the read serves the base row again and its rollback is refused by the named-base close', async () => {
+        const { protocol, rows, storeBaseRow } = makeProtocol();
+        await storeBaseRow(VIEW.type, VIEW.name, VIEW.body('BaseFirst'));
+        await storeBaseRow(VIEW.type, VIEW.name, VIEW.body('BaseSecond'));
+        await savePair(protocol, 'PairFirst');
+        expect(await protocol.deleteMetaItem({ type: VIEW.type, name: VIEW.name, packageId: PAIR })).toMatchObject({ success: true });
+        expect((await protocol.getMetaItem({ type: VIEW.type, name: VIEW.name })).item.label).toBe('BaseSecond');
+
+        const toVersion = await versionOf(protocol, 'BaseFirst');
+        const refused = await thrown(() => protocol.rollbackMetaItem({ type: VIEW.type, name: VIEW.name, toVersion, actor: 'admin' }));
+
+        expectClosed(refused, VIEW.type, VIEW.name, 'rollback');
+        expect(rows(VIEW.type, VIEW.name)).toEqual([{ package_id: BASE, state: 'active', label: 'BaseSecond' }]);
+    });
+});

packages/metadata-protocol/src/protocol.ts

@@ -13085,6 +13085,10 @@ export class ObjectStackProtocolImplementation implements
      * (`VERSION_NOT_FOUND`) when the target version doesn't exist;
      * 409 (`VERSION_NOT_RESTORABLE`) when the target is a delete
      * tombstone (no body to bring back).
+     *
+     * [#29848, ADR-0146 D8 item 2] The door names no package: it restores
+     * into the row the by-name read naming no package serves, a `.custom`
+     * pair's declared override over a row bound to its read-only base.
      */
     async rollbackMetaItem(request: RollbackMetaItemRequest) {
         // [#28355] A refusal that carries no row of its own leaves its `denied` row here.

packages/objectql/src/protocol-packageless-overlay-migration.test.ts

@@ -609,11 +609,17 @@ describe('[#28576] the close reaches only what the ceremony moves: every row it
         const pairRow = rowOf(VIEW, PAIR);
         expect(columnsOf(await read(protocol))).toEqual(['title']);
 
-        // The rollback's row is the first active row of the name in store
-        // order (`resolveOverlayPackageBinding`): here the package-less one,
-        // which is rolled back to its version 1.
-        expect(await refusalOf((protocol as any).rollbackMetaItem({ type: 'view', name: VIEW, toVersion: 1 }))).toBeUndefined();
-        expect(columnsOf(JSON.parse(String(rowOf(VIEW, null)!.metadata)))).toEqual(UNBOUND_COLUMNS);
+        // [#29848] The rollback's row is the one the read serves, the pair's
+        // declared row, never the first active row of the name in store order:
+        // version 1 is the package-less row's undeclared body, so the override
+        // write gate answers it there (D2), not the close, and nothing is written.
+        const packagelessRow = rowOf(VIEW, null);
+        const rolledBack = await refusalOf((protocol as any).rollbackMetaItem({ type: 'view', name: VIEW, toVersion: 1 }));
+        expect(rolledBack).toMatchObject({ status: 403, code: 'NOT_OVERRIDABLE' });
+        expect(rolledBack?.message).toContain('declares no override');
+        expect(rolledBack?.message).not.toContain('os migrate packageless-overlays');
+        expect(rowOf(VIEW, PAIR)).toEqual(pairRow);
+        expect(rowOf(VIEW, null)).toEqual(packagelessRow);
         // The package-less save and DELETE are not the close's to refuse:
         // they are addressed as before the close (`packagelessAddress`: the
         // one bound row of the name, the pair's), so the override write gate
@@ -627,7 +633,7 @@ describe('[#28576] the close reaches only what the ceremony moves: every row it
         expect(rows(VIEW)).toEqual([[null, 'active']]);
         // …which leaves the package-less row movable: the ceremony's remedy.
         expect(dispositions(await census(protocol))).toEqual([`${VIEW}@active movable`]);
-        expect(columnsOf(await read(protocol))).toEqual(UNBOUND_COLUMNS);
+        expect(columnsOf(await read(protocol))).toEqual(EDITED_COLUMNS);
     });
 
     it('CONTROL movable: the same stored row with no pair row beside it — its package-less rollback, save and DELETE are refused', async () => {