Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions .changeset/29414-commit-revert-own-package-row.md
Original file line number Diff line number Diff line change
@@ -0,0 +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, 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 a `<base>.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` 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.

<!-- 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). Its surface and reason now name the commit doors as further addresses of the rollback it closes (text only, no new id). No stored row is rewritten or moved, so `objectstack migrate meta` has nothing new to do. -->

**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 `<base>.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 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.custom/commits/cmt_123/revert
```

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
```

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 `<base>.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/<id>/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.
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,15 @@ async function attempt<T>(run: () => Promise<T>): 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',
Expand Down Expand Up @@ -457,7 +463,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' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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' });
Expand All @@ -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' });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -476,4 +476,119 @@ 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 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([]);

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([]);
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' }]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,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. */
Expand All @@ -191,6 +193,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<string>();
const rows = new Map<string, Row>();
const historyRows: HistoryRow[] = [];
const commitRows: any[] = [];
Expand Down Expand Up @@ -244,6 +248,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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,11 @@ function matchesWhere(r: Row, where: Record<string, unknown>): 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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,11 @@ function matchesWhere(r: Row, where: Record<string, unknown>): 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',
Expand Down
Loading
Loading