diff --git a/.changeset/rollback-addresses-served-row.md b/.changeset/rollback-addresses-served-row.md new file mode 100644 index 00000000000..19de3041cf6 --- /dev/null +++ b/.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. diff --git a/packages/metadata-protocol/src/protocol-package-commits.ts b/packages/metadata-protocol/src/protocol-package-commits.ts index 8d91365b37c..35ffb243b5c 100644 --- a/packages/metadata-protocol/src/protocol-package-commits.ts +++ b/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, request: RollbackMetaItemRequest): Promise<{ + static async rollbackMetaItemUnaudited(host: Pick, 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 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 5ea6dbf4b8e..a3adc31ca08 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 @@ -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 { + 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' }]); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 8ea5370d2cf..9f94644dca6 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/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. diff --git a/packages/objectql/src/protocol-packageless-overlay-migration.test.ts b/packages/objectql/src/protocol-packageless-overlay-migration.test.ts index 457a94a5bdb..4e0f22bc56b 100644 --- a/packages/objectql/src/protocol-packageless-overlay-migration.test.ts +++ b/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 () => {