fix(metadata-protocol): revertCommit states its write intent per item (#6563)

.changeset/revert-commit-write-intent.md

@@ -0,0 +1,41 @@
+---
+'@objectstack/metadata-protocol': patch
+---
+
+fix(metadata-protocol): `revertCommit` states its write intent per item, so an `object` overlay can be reverted at all (#6563)
+
+`ObjectStackProtocolImplementation.revertCommit` restored an edited artifact
+through `repo.restoreVersion(ref, prevVersion, { actor, source, message })` — with
+no `intent`. `SysMetadataRepository.restoreVersion` therefore fell back to its
+`?? 'override-artifact'` default, `put` opened with
+`assertAllowed(ref.type, opts.intent)`, and that gate refuses every type whose
+registry entry is not `allowOrgOverride`. `object` is exactly such a type, so
+every `object` item of a reverted commit came back in `failed[]`:
+
+```
+[NOT_OVERRIDABLE] 'object' is not allowOrgOverride in the registry.
+Overlay-allowed: view, page, dashboard, app, action, report, dataset, ...
+```
+
+The package-commit undo (ADR-0067) therefore could not revert the metadata type
+Studio and AI-built apps create most, while the same edit reverted fine one
+artifact at a time through the version-history revert — the two user-facing
+revert paths disagreed about what is revertable. The failure was per item, so
+the call still answered `success` overall with a populated `failed[]`, which
+reads as a flaky revert rather than a systematic refusal.
+`rollbackToPackageCommit` reverts through the same loop and inherited it, and
+there the symptom was quieter still: a per-item refusal never throws, so the
+rollback recorded the commit as reverted and answered `success: true` while the
+object was untouched.
+
+`revertCommit` now derives the intent from the artifact the way its sibling
+`rollbackMetaItem` already does — `isArtifactBacked` gives `'override-artifact'`,
+otherwise `'runtime-only'` — and does it **per item**, because a commit is a
+batch that routinely mixes a runtime-created object with an overlay on a
+packaged view.
+
+The repository's default is deliberately unchanged: it is right for callers that
+genuinely mean "override a packaged artifact", and the defect was this caller
+never saying which of the two cases it is. So the gate is not widened — an
+object a code package really ships still resolves to `'override-artifact'` and
+is still refused with `NOT_OVERRIDABLE`, which is pinned alongside the fix.

packages/metadata-protocol/src/protocol.ts

@@ -10163,10 +10163,38 @@ export class ObjectStackProtocolImplementation implements
                     reverted.push({ type: it.type, name: it.name, action: 'removed' });
                 } else if (it.prevVersion !== null && it.prevVersion !== undefined) {
                     // Edited an existing artifact → restore the pre-commit body.
+                    //
+                    // [#6563] The write INTENT is derived per item, exactly as the
+                    // sibling caller {@link rollbackMetaItem} derives it. Left
+                    // unstated, `SysMetadataRepository.restoreVersion` defaults to
+                    // `'override-artifact'` and `put`'s `assertAllowed` refuses every
+                    // type that is not `allowOrgOverride` — `object` among them — so
+                    // each `object` item of a reverted commit came back in `failed[]`
+                    // as `NOT_OVERRIDABLE` while the same edit reverted fine one
+                    // artifact at a time through the version-history revert. The
+                    // repository's default is right for callers that genuinely mean
+                    // "override a packaged artifact"; the defect was this caller never
+                    // saying which of the two cases it is.
+                    //
+                    // Per ITEM, not per call: `revertCommit` reverts a batch, and a
+                    // commit routinely mixes a runtime-created object with an overlay
+                    // on a packaged view. A genuinely artifact-backed item still
+                    // resolves to `'override-artifact'` and is still refused — the
+                    // derivation states the case, it does not widen the gate.
+                    //
+                    // Two neighbours are deliberately NOT changed here, each filed
+                    // with its own measurement: the soft-remove limb above states the
+                    // same intent as a CONSTANT, so a commit that CREATED an object
+                    // still cannot be reverted (#6620); and neither limb refreshes the
+                    // SchemaRegistry the way `rollbackMetaItem` does, so a restored
+                    // body is persisted but not yet dispatched on (#6621).
+                    const intent: 'override-artifact' | 'runtime-only' =
+                        this.isArtifactBacked(it.type, it.name) ? 'override-artifact' : 'runtime-only';
                     await repo.restoreVersion(ref, it.prevVersion, {
                         actor,
                         source: 'protocol.revertCommit',
                         message: `revert commit ${request.commitId}`,
+                        intent,
                     });
                     reverted.push({ type: it.type, name: it.name, action: 'restored' });
                 }

packages/objectql/src/protocol-commit-history.test.ts

@@ -313,11 +313,11 @@ function makeRealRepoHarness(seedCommits: any[] = []) {
  * The reverted artifact is a `view`, not an `object`, and deliberately: the
  * repository's `assertAllowed` refuses `override-artifact` on `object` (not
  * `allowOrgOverride`), and `revertCommit` — unlike `rollbackMetaItem`, which
- * derives `runtime-only` for a runtime-created artifact — passes no intent, so
- * an object item fails that gate BEFORE reaching the scoping this file pins.
- * That is a separate defect of the same family, filed as #6563; using an
- * overlay-allowed type keeps this pin measuring one thing. The repository line
- * under test is type-agnostic.
+ * derives `runtime-only` for a runtime-created artifact — passed no intent, so
+ * an object item failed that gate BEFORE reaching the scoping this file pins.
+ * That was a separate defect of the same family, fixed as #6563 (whose own pins
+ * are the last block in this file); using an overlay-allowed type keeps this pin
+ * measuring one thing. The repository line under test is type-agnostic.
  */
 const gridBody = (label: string) => ({
   name: 'myapp_case_grid', type: 'grid', label, columns: ['id', 'title'],
@@ -431,3 +431,204 @@ describe('#6215 — revertCommit restores a PACKAGE-BOUND overlay row', () => {
     expect(JSON.parse(stored[0].metadata).label).toBe('Renamed');
   });
 });
+
+/**
+ * #6563 — `revertCommit` states its write INTENT, per item.
+ *
+ * The block above could only be written about a `view`: `revertCommit` passed
+ * no `intent`, so `SysMetadataRepository.restoreVersion` fell back to its
+ * `?? 'override-artifact'` default, `put` opened with
+ * `assertAllowed(ref.type, opts.intent)`, and every type that is not
+ * `allowOrgOverride` was refused — `object` among them. So the metadata type
+ * Studio creates most could not be reverted through the package-commit undo AT
+ * ALL, while the same edit reverted fine one artifact at a time through
+ * `rollbackMetaItem`, which derives the intent instead of defaulting it. The
+ * two user-facing revert paths disagreed about what is revertable.
+ *
+ * The repository default is unchanged and correct — it is right for callers
+ * that genuinely mean "override a packaged artifact". What was missing is this
+ * caller saying which of the two cases each item is, which is why the fix is a
+ * per-item `isArtifactBacked` derivation in `revertCommit` and not a looser
+ * gate: the artifact-backed refusal below is the half that must NOT move.
+ */
+
+const invoiceBody = (name: string, extra?: Record<string, unknown>) => ({
+  name,
+  label: 'Invoice',
+  fields: {
+    name: { name: 'name', type: 'text', label: 'Name' },
+    amount: { name: 'amount', type: 'number', label: 'Amount' },
+  },
+  ...extra,
+});
+
+/** v2 of the same object: the commit's edit added a field. */
+const evolvedInvoiceBody = (name: string) => {
+  const body = invoiceBody(name);
+  (body.fields as Record<string, unknown>).due_date = { name: 'due_date', type: 'date', label: 'Due' };
+  return body;
+};
+
+/** The exact measured repro: saved twice through `saveMetaItem`, then reverted. */
+async function seedObjectEdit(protocol: any, name: string, packageId?: string) {
+  const pkg = packageId ? { packageId } : {};
+  await protocol.saveMetaItem({ type: 'object', name, ...pkg, item: invoiceBody(name) });
+  await protocol.saveMetaItem({ type: 'object', name, ...pkg, item: evolvedInvoiceBody(name) });
+}
+
+const storedFields = (rows: Map<string, any>, name: string) => {
+  const stored = Array.from(rows.values()).filter((r) => r.name === name);
+  expect(stored).toHaveLength(1);
+  return { row: stored[0], fields: Object.keys(JSON.parse(stored[0].metadata).fields) };
+};
+
+const objectCommit = (over: Record<string, unknown> & { id: string; items: any[] }) => applyCommit({
+  package_id: APP_PKG,
+  created_at: '2026-08-08T00:00:02.000Z',
+  ...over,
+} as any);
+
+describe('#6563 — revertCommit restores a runtime-created `object`', () => {
+  it('a package-bound object reverts: revertedCount 1, failed [], pre-commit body back', async () => {
+    const { protocol, rows } = makeRealRepoHarness([objectCommit({
+      id: 'cmt_obj',
+      items: [{ type: 'object', name: 'myapp_invoice', existedBefore: true, prevVersion: 1 }],
+    })]);
+    await seedObjectEdit(protocol, 'myapp_invoice', APP_PKG);
+
+    const res = await protocol.revertCommit({ commitId: 'cmt_obj' });
+
+    // Pre-fix, verbatim: revertedCount 0 / failedCount 1 carrying
+    // "[NOT_OVERRIDABLE] 'object' is not allowOrgOverride in the registry."
+    expect(res.failed).toEqual([]);
+    expect(res.revertedCount).toBe(1);
+    expect(res.reverted[0]).toMatchObject({ type: 'object', name: 'myapp_invoice', action: 'restored' });
+    // The restore also still lands IN PLACE on the bound row (#6215's half, now
+    // exercised on the type that could not reach it).
+    const { row, fields } = storedFields(rows, 'myapp_invoice');
+    expect(row.package_id).toBe(APP_PKG);
+    expect(fields).not.toContain('due_date');
+  });
+
+  it('a package-LESS object reverts identically — the binding was never the cause', async () => {
+    const { protocol, rows } = makeRealRepoHarness([objectCommit({
+      id: 'cmt_obj_global',
+      package_id: null,
+      items: [{ type: 'object', name: 'global_invoice', existedBefore: true, prevVersion: 1 }],
+    })]);
+    await seedObjectEdit(protocol, 'global_invoice');
+
+    const res = await protocol.revertCommit({ commitId: 'cmt_obj_global' });
+
+    expect(res.failed).toEqual([]);
+    expect(res.revertedCount).toBe(1);
+    const { row, fields } = storedFields(rows, 'global_invoice');
+    expect(row.package_id).toBeNull();
+    expect(fields).not.toContain('due_date');
+  });
+
+  /**
+   * The refusal that must SURVIVE the fix. Deriving the intent is the caller
+   * stating its case, not a wider gate: an object a code package really ships
+   * resolves to `'override-artifact'` and is refused exactly as before.
+   *
+   * Staging it needs the ordering a real deployment has anyway — the overlay
+   * rows are authored while the name is runtime-only, and the artifact arrives
+   * with the package that later claims it. `registerObject(body, pkg)` with no
+   * `_provenance` is the shape `applyProtection` stamps as `'package'`, which
+   * is what `getArtifactItem` reads and `isArtifactBacked` answers on (the same
+   * lever #4636's B-minimal counter-example pulls).
+   *
+   * Envelope note (ADR-0112): `revertCommit` converts a per-item throw into a
+   * `failed[]` record whose DECLARED shape is `{ type, name, error, code? }` —
+   * no `status`. So `code` is asserted here together with the condition's own
+   * first sentence, and the full `{ code, status }` pair is asserted at the
+   * throwing surface in `protocol-writepath-object-ownership.test.ts`.
+   */
+  it('still REFUSES an artifact-backed object: NOT_OVERRIDABLE, nothing written', async () => {
+    const { protocol, registry, rows } = makeRealRepoHarness([objectCommit({
+      id: 'cmt_obj_artifact',
+      items: [{ type: 'object', name: 'myapp_invoice', existedBefore: true, prevVersion: 1 }],
+    })]);
+    await seedObjectEdit(protocol, 'myapp_invoice', APP_PKG);
+    registry.registerObject(invoiceBody('myapp_invoice') as never, APP_PKG);
+
+    const res = await protocol.revertCommit({ commitId: 'cmt_obj_artifact' });
+
+    expect(res.revertedCount).toBe(0);
+    expect(res.failedCount).toBe(1);
+    expect(res.failed[0]).toMatchObject({
+      type: 'object',
+      name: 'myapp_invoice',
+      code: 'NOT_OVERRIDABLE',
+    });
+    expect(res.failed[0].error).toContain(
+      `[NOT_OVERRIDABLE] 'object' is not allowOrgOverride in the registry.`,
+    );
+    // Refused means refused: the edit the commit made is still the live body.
+    expect(storedFields(rows, 'myapp_invoice').fields).toContain('due_date');
+  });
+
+  /**
+   * PER ITEM, not per call — the half a single-item fixture cannot see. One
+   * commit, two objects, opposite verdicts: a `for` loop that hoisted one
+   * intent for the batch would have to pick one and be wrong about the other.
+   */
+  it('derives the intent PER ITEM: one object restored, its artifact-backed neighbour refused', async () => {
+    const { protocol, registry, rows } = makeRealRepoHarness([objectCommit({
+      id: 'cmt_obj_mixed',
+      items: [
+        { type: 'object', name: 'myapp_invoice', existedBefore: true, prevVersion: 1 },
+        { type: 'object', name: 'myapp_quote', existedBefore: true, prevVersion: 1 },
+      ],
+    })]);
+    await seedObjectEdit(protocol, 'myapp_invoice', APP_PKG);
+    await seedObjectEdit(protocol, 'myapp_quote', APP_PKG);
+    // Only the quote is claimed by a code artifact.
+    registry.registerObject(invoiceBody('myapp_quote') as never, APP_PKG);
+
+    const res = await protocol.revertCommit({ commitId: 'cmt_obj_mixed' });
+
+    expect(res.reverted).toEqual([
+      { type: 'object', name: 'myapp_invoice', action: 'restored' },
+    ]);
+    expect(res.failed).toHaveLength(1);
+    expect(res.failed[0]).toMatchObject({ name: 'myapp_quote', code: 'NOT_OVERRIDABLE' });
+    expect(storedFields(rows, 'myapp_invoice').fields).not.toContain('due_date');
+    expect(storedFields(rows, 'myapp_quote').fields).toContain('due_date');
+  });
+});
+
+/**
+ * #6563 — the inheritance. `rollbackToPackageCommit` reverts through the SAME
+ * loop, one `revertCommit` per apply commit newer than the target.
+ *
+ * Its own return shape cannot show this defect: `revertCommit` converts a
+ * per-item refusal into `failed[]` rather than throwing, so the rollback
+ * recorded the commit as reverted and answered `success: true` while the object
+ * was untouched. The assertion that goes red pre-fix is therefore the STORED
+ * BODY, not the status — asserting `success` alone would have been green on the
+ * defect.
+ */
+describe('#6563 — rollbackToPackageCommit inherits the per-item intent', () => {
+  it('rolls an object edit back through the loop — and the stored body really moved', async () => {
+    const { protocol, rows } = makeRealRepoHarness([
+      objectCommit({ id: 'cmt_base', items: [], created_at: '2026-08-08T00:00:01.000Z' }),
+      objectCommit({
+        id: 'cmt_edit',
+        items: [{ type: 'object', name: 'myapp_invoice', existedBefore: true, prevVersion: 1 }],
+        created_at: '2026-08-08T00:00:02.000Z',
+      }),
+    ]);
+    await seedObjectEdit(protocol, 'myapp_invoice', APP_PKG);
+
+    const res = await protocol.rollbackToPackageCommit({ commitId: 'cmt_base' });
+
+    expect(res.revertedCommits).toEqual(['cmt_edit']);
+    expect(res.failed).toEqual([]);
+    // `success: true` was ALREADY true pre-fix — this is the line that was not.
+    const { row, fields } = storedFields(rows, 'myapp_invoice');
+    expect(row.package_id).toBe(APP_PKG);
+    expect(fields).not.toContain('due_date');
+  });
+});