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
20 changes: 20 additions & 0 deletions .changeset/package-commit-revert-path-package.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
'@objectstack/runtime': patch
'@objectstack/metadata-protocol': patch
---

fix(runtime)!: package commit revert and rollback act only on a commit of the package their path names

Clause-②: yes (narrowing)
Metadata-protocol: none

<!-- adr-0087: not-required (no-migration-prescription) No metadata key, schema, accepted value, export or error code is removed or renamed. Two REST doors now refuse a request whose path package does not own the commit it names, with the existing COMMIT_NOT_FOUND code and the package-not-found 404 the sibling package doors already give, so no stored metadata changes and there is nothing for objectstack migrate meta to rewrite. The protocol's two request types gain an OPTIONAL member, which no existing caller has to send. -->

**BREAKING** for one call shape only: a request that named one package in the path and a commit of another. `POST /api/v1/packages/:id/commits/:commitId/revert` and `POST /api/v1/packages/:id/rollback` (body `{ commitId }`) used to ignore `:id`: a commit of package A, sent through package B's path, answered 200, reverted A's items and recorded a revert commit in A's history, even after A was uninstalled. Such a request now answers `404 COMMIT_NOT_FOUND` ("No commit 'ID' in package 'B'.") and writes nothing. An unknown `:id`, which includes a package that has been uninstalled, answers `404 Package 'ID' not found`, as the draft doors do, so an uninstalled package's history no longer takes new revert commits through its own path either. A commit sent through its own package's path reverts and rolls back exactly as before, with the same response body.

- **`@objectstack/runtime`**: both doors hand the path's package to the protocol, after the `manage_metadata`, walled-catalog and query refusals, which keep their order.
- **`@objectstack/metadata-protocol`**: `revertCommit` and `rollbackToPackageCommit` take an optional `packageId`. When it is sent, a commit of any other package is refused `COMMIT_NOT_FOUND` (404) before anything is written. An in-process caller that sends none reads any commit, as before.

A read-only package keeps its commit revert. Its only commits are the publishes of its environment overlays (ADR-0131 D6), and `POST /packages/:id/revert` answers a read-only package's stored rows too.

What to do: nothing, unless a script sends commit ids through another package's path. Send each commit through the package that `GET /packages/:id/commits` lists it under.
31 changes: 28 additions & 3 deletions packages/metadata-protocol/src/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27774,8 +27774,15 @@ export class ObjectStackProtocolImplementation implements
* used to read it through {@link findEnvironmentRow}, which leaves the
* legacy commit out, and answered `COMMIT_NOT_FOUND` for a commit that is
* there.
*
* [ADR-0067] With `packageId`, a commit of ANOTHER package is not found
* either: the package a caller names (the REST doors' path id) decides
* which commits it can act on, so a commit id copied from the wrong
* package is answered `COMMIT_NOT_FOUND` naming that package, before
* anything is written. Without it (an in-process caller that names no
* package) any commit is read, as before.
*/
private async readRevertableCommit(commitId: string): Promise<any> {
private async readRevertableCommit(commitId: string, packageId?: string): Promise<any> {
// [#21908, ADR-0096 D5] The explicit system opt-in: the commit ledger is a
// platform store the door already authorized; the protocol scopes the
// `where` by organization itself. Principal-less, the engine refuses it.
Expand All @@ -27786,6 +27793,12 @@ export class ObjectStackProtocolImplementation implements
err.status = 404;
throw err;
}
if (packageId !== undefined && String(row.package_id) !== packageId) {
const err: any = new Error(`No commit '${commitId}' in package '${packageId}'.`);
err.code = 'COMMIT_NOT_FOUND';
err.status = 404;
throw err;
}
const legacyCommits = await this.legacyOrganizationRowsOf('sys_metadata_commit');
const legacy = ObjectStackProtocolImplementation.organizationScopedWriteRefusal(
`Commit '${commitId}'`,
Expand Down Expand Up @@ -27828,6 +27841,12 @@ export class ObjectStackProtocolImplementation implements
*/
async revertCommit(request: {
commitId: string;
/**
* The package the commit must belong to (ADR-0067): a commit of any
* other package is refused `COMMIT_NOT_FOUND` (404) and nothing is
* written. Optional for in-process callers that name no package.
*/
packageId?: string;
actor?: string;
}): Promise<{
success: boolean;
Expand All @@ -27842,7 +27861,7 @@ export class ObjectStackProtocolImplementation implements
`Commit '${request.commitId}'`, request,
);
await this.ensureOverlayIndex();
const row = await this.readRevertableCommit(request.commitId);
const row = await this.readRevertableCommit(request.commitId, request.packageId);
const items = this.parseCommitItems(row.items);
// #4556 — threaded into repo.put/delete → `recorded_by`; NULL when the
// revert carries no human actor.
Expand Down Expand Up @@ -28331,6 +28350,12 @@ export class ObjectStackProtocolImplementation implements
*/
async rollbackToPackageCommit(request: {
commitId: string;
/**
* The package being rolled back (ADR-0067): a target commit of any
* other package is refused `COMMIT_NOT_FOUND` (404) and nothing is
* written. Optional for in-process callers that name no package.
*/
packageId?: string;
actor?: string;
}): Promise<{
success: boolean;
Expand All @@ -28341,7 +28366,7 @@ export class ObjectStackProtocolImplementation implements
ObjectStackProtocolImplementation.refuseOrganizationScopedWrite(
`Commit '${request.commitId}'`, request,
);
const target = await this.readRevertableCommit(request.commitId);
const target = await this.readRevertableCommit(request.commitId, request.packageId);
const all = await this.listCommits({ packageId: target.package_id });
// listCommits is newest-first; revert every `apply` commit strictly newer
// than the target (by created_at). Revert commits are skipped (their
Expand Down
43 changes: 43 additions & 0 deletions packages/objectql/src/protocol-commit-history.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,28 @@ describe('ADR-0067 — revertCommit', () => {
await expect(p.revertCommit({ commitId: 'nope' })).rejects.toMatchObject({ code: 'COMMIT_NOT_FOUND', status: 404 });
});

it('refuses a commit of ANOTHER package with commit_not_found naming the package, and writes nothing', async () => {
const { engine, commits } = makeFakeEngine([
applyCommit({ id: 'cmt_a', items: [{ type: 'object', name: 'course', existedBefore: false, prevVersion: null }], created_at: '2026-06-24T00:00:00.000Z' }),
]);
const repo = { get: vi.fn(async () => ({ hash: 'h1' })), delete: vi.fn(async () => {}), restoreVersion: vi.fn() };
const p = makeProtocol(engine, repo);

await expect(p.revertCommit({ commitId: 'cmt_a', packageId: 'app.other' })).rejects.toMatchObject({
code: 'COMMIT_NOT_FOUND',
status: 404,
message: "No commit 'cmt_a' in package 'app.other'.",
});
expect(repo.delete).not.toHaveBeenCalled();
expect(repo.restoreVersion).not.toHaveBeenCalled();
expect(commits).toHaveLength(1);

// Control: the commit's own package reverts it.
const res = await p.revertCommit({ commitId: 'cmt_a', packageId: 'app.edu' });
expect(res).toMatchObject({ success: true, revertedCount: 1 });
expect(commits.map((c) => c.operation)).toEqual(['apply', 'revert']);
});

it('reverts dependent artifacts in REVERSE apply order (view before its object)', async () => {
const { engine } = makeFakeEngine([
applyCommit({
Expand Down Expand Up @@ -188,6 +210,27 @@ describe('ADR-0067 — rollbackToPackageCommit', () => {
const p = makeProtocol(engine, {});
await expect(p.rollbackToPackageCommit({ commitId: 'ghost' })).rejects.toMatchObject({ code: 'COMMIT_NOT_FOUND' });
});

it('refuses a target commit of ANOTHER package with commit_not_found, and reverts nothing', async () => {
const { engine, commits } = makeFakeEngine([
applyCommit({ id: 'c1', items: [], created_at: '2026-06-24T00:00:01.000Z' }),
applyCommit({ id: 'c2', items: [{ type: 'object', name: 'x', existedBefore: false, prevVersion: null }], created_at: '2026-06-24T00:00:02.000Z' }),
]);
const repo = { get: vi.fn(async () => ({ hash: 'h' })), delete: vi.fn(async () => {}), restoreVersion: vi.fn() };
const p = makeProtocol(engine, repo);

await expect(p.rollbackToPackageCommit({ commitId: 'c1', packageId: 'app.other' })).rejects.toMatchObject({
code: 'COMMIT_NOT_FOUND',
status: 404,
message: "No commit 'c1' in package 'app.other'.",
});
expect(repo.delete).not.toHaveBeenCalled();
expect(commits).toHaveLength(2);

// Control: the package's own rollback reverts c2.
const res = await p.rollbackToPackageCommit({ commitId: 'c1', packageId: 'app.edu' });
expect(res).toMatchObject({ success: true, revertedCommits: ['c2'] });
});
});

describe('ADR-0067 — publishPackageDrafts records a commit', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
//
// ADR-0067 — the per-commit revert and the rollback act only on a commit of
// the package their path names, over the real showcase composition and
// through the REST doors.
//
// ## What was broken
//
// `POST /api/v1/packages/B/commits/COMMIT_OF_A/revert` read the commit id from
// the path and never read the package id beside it: it answered 200, reverted
// package A's items, and recorded a "Revert: …" commit in A's history.
// `POST /api/v1/packages/B/rollback` with `{ commitId: COMMIT_OF_A }` did the
// same through A's whole timeline.
//
// ## What these cases pin
//
// A commit of another package answers `404 COMMIT_NOT_FOUND` naming the
// commit as not in the path's package, and writes nothing: the items are
// untouched and the commit's own package keeps the history it had. An
// unknown package id answers the sibling doors' `404 Package 'ID' not found`.
// Controls: the same revert and rollback through the commit's own package
// 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
//
// 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
// `publish-drafts`, records an `apply` commit in the showcase's history, and
// reverting it through the showcase's own path removes the overlay. That is
// the undo of an ADR-0131 D6 customization, which `POST /packages/:id/revert`
// also answers for a read-only package (stored rows first), so the commit
// doors take no read-only guard; the last case pins it.

import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { type VerifyStack } from '@objectstack/verify';
import { bootShowcase } from './showcase-boot.js';

const SHOWCASE = 'com.example.showcase';
const OVERLAY = 'showcase_task.default';
const A = 'app.cmtprobe-a';
const B = 'app.cmtprobe-b';

const object = (name: string, label: string) => ({
name,
label,
sharingModel: 'public_read_write',
fields: { name: { type: 'text', label: 'Name' } },
});

interface Commit { id: string; operation: string; message?: string }

describe('dogfood: package commit revert and rollback act only on a commit of the path\'s package (ADR-0067)', () => {
let stack: VerifyStack;
let admin: string;
let member: string;

const call = async (method: string, path: string, body?: unknown, token = admin) => {
const res = await stack.apiAs(token, method, path, body);
const json: any = await res.json().catch(() => ({}));
return { status: res.status, json };
};
const commitsOf = async (pkg: string): Promise<Commit[]> => {
const res = await call('GET', `/packages/${pkg}/commits`);
expect(res.status, JSON.stringify(res.json)).toBe(200);
return (res.json?.data?.commits ?? []) as Commit[];
};
const publishObject = async (pkg: string, name: string, label: string) => {
const saved = await call('PUT', `/meta/object/${name}?mode=draft&package=${pkg}`, object(name, label));
expect(saved.status, JSON.stringify(saved.json)).toBe(200);
const published = await call('POST', `/packages/${pkg}/publish-drafts`, {});
expect(published.status, JSON.stringify(published.json)).toBe(200);
};
const isServed = async (name: string) => (await call('GET', `/meta/object/${name}`)).status === 200;

beforeAll(async () => {
stack = await bootShowcase();
admin = await stack.signIn();
member = await stack.signUp('member.commit-revert@verify.test');
for (const [id, ns] of [[A, 'cmtpa'], [B, 'cmtpb']] as const) {
const created = await call('POST', '/packages', {
manifest: { id, name: `Commit probe ${ns}`, version: '0.1.0', type: 'app', namespace: ns },
});
expect(created.status, JSON.stringify(created.json)).toBe(201);
}
await publishObject(A, 'cmtpa_first', 'A first');
await publishObject(A, 'cmtpa_second', 'A second');
await publishObject(B, 'cmtpb_first', 'B first');
}, 180_000);

afterAll(async () => {
await stack?.stop();
});

it('the card: a commit of A reverted through B\'s path is 404 and writes nothing', async () => {
const before = await commitsOf(A);
const target = before.find((c) => c.operation === 'apply')!;
const res = await call('POST', `/packages/${B}/commits/${target.id}/revert`, {});
expect(res.status, JSON.stringify(res.json)).toBe(404);
expect(JSON.stringify(res.json)).toContain('COMMIT_NOT_FOUND');
expect(await commitsOf(A)).toEqual(before);
expect(await isServed('cmtpa_first')).toBe(true);
expect(await isServed('cmtpa_second')).toBe(true);
});

it('a rollback of B to a commit of A is 404 and writes nothing', async () => {
const before = await commitsOf(A);
const oldest = before[before.length - 1];
const res = await call('POST', `/packages/${B}/rollback`, { commitId: oldest.id });
expect(res.status, JSON.stringify(res.json)).toBe(404);
expect(JSON.stringify(res.json)).toContain('COMMIT_NOT_FOUND');
expect(await commitsOf(A)).toEqual(before);
expect(await commitsOf(B)).toHaveLength(1);
expect(await isServed('cmtpa_second')).toBe(true);
});

it('an unknown package id answers the sibling doors\' package-not-found 404', async () => {
const target = (await commitsOf(A))[0];
const revert = await call('POST', `/packages/app.cmtprobe-nosuch/commits/${target.id}/revert`, {});
expect(revert.status, JSON.stringify(revert.json)).toBe(404);
expect(JSON.stringify(revert.json)).toContain("Package 'app.cmtprobe-nosuch' not found");
const rollback = await call('POST', '/packages/app.cmtprobe-nosuch/rollback', { commitId: target.id });
expect(rollback.status, JSON.stringify(rollback.json)).toBe(404);
expect(JSON.stringify(rollback.json)).toContain("Package 'app.cmtprobe-nosuch' not found");
});

it('control: a member without manage_metadata is refused 403 first', async () => {
const target = (await commitsOf(A))[0];
expect((await call('POST', `/packages/${B}/commits/${target.id}/revert`, {}, member)).status).toBe(403);
expect((await call('POST', `/packages/${B}/rollback`, { commitId: target.id }, member)).status).toBe(403);
});

it('control: A\'s newest commit reverts through A\'s own path, as before', async () => {
const before = await commitsOf(A);
const newest = before[0];
const res = await call('POST', `/packages/${A}/commits/${newest.id}/revert`, {});
expect(res.status, JSON.stringify(res.json)).toBe(200);
expect(res.json?.data).toMatchObject({ success: true, revertedCount: 1, failedCount: 0 });
const after = await commitsOf(A);
expect(after).toHaveLength(before.length + 1);
expect(after[0]).toMatchObject({ operation: 'revert', id: res.json.data.revertCommitId });
expect(await isServed('cmtpa_second')).toBe(false);
});

it('control: B rolls back to its own first commit through B\'s path, as before', async () => {
await publishObject(B, 'cmtpb_second', 'B second');
const before = await commitsOf(B);
const oldest = before[before.length - 1];
const res = await call('POST', `/packages/${B}/rollback`, { commitId: oldest.id });
expect(res.status, JSON.stringify(res.json)).toBe(200);
expect(res.json?.data).toMatchObject({ success: true, revertedCommits: [before[0].id], failed: [] });
expect(await commitsOf(B)).toHaveLength(before.length + 1);
expect(await isServed('cmtpb_second')).toBe(false);
expect(await isServed('cmtpb_first')).toBe(true);
});

it('control: a read-only package\'s overlay commit reverts through its own path, and through no other', async () => {
const saved = await call('PUT', `/meta/view/${OVERLAY}?mode=draft&package=${SHOWCASE}`, {
name: OVERLAY, object: 'showcase_task', viewKind: 'list', label: 'Overlay probe',
config: { type: 'grid', columns: [{ field: 'title' }] },
});
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);
const before = await commitsOf(SHOWCASE);
expect(before[0]).toMatchObject({ operation: 'apply', id: published.json?.data?.commitId });

const cross = await call('POST', `/packages/${B}/commits/${before[0].id}/revert`, {});
expect(cross.status, JSON.stringify(cross.json)).toBe(404);
expect(await commitsOf(SHOWCASE)).toEqual(before);

const res = await call('POST', `/packages/${SHOWCASE}/commits/${before[0].id}/revert`, {});
expect(res.status, JSON.stringify(res.json)).toBe(200);
expect(res.json?.data).toMatchObject({
success: true, revertedCount: 1, reverted: [{ type: 'view', name: OVERLAY, action: 'removed' }],
});
expect(await commitsOf(SHOWCASE)).toHaveLength(before.length + 1);
});
});
Loading
Loading