diff --git a/.changeset/revert-commit-registry-refresh.md b/.changeset/revert-commit-registry-refresh.md new file mode 100644 index 0000000000..61e68b8e02 --- /dev/null +++ b/.changeset/revert-commit-registry-refresh.md @@ -0,0 +1,48 @@ +--- +"@objectstack/metadata-protocol": patch +--- + +fix(metadata-protocol): a successful `revertCommit` refreshes the SchemaRegistry (#6621) + +`revertCommit` persisted its change and left the running process serving the +body it had just reverted away. The single-item revert `rollbackMetaItem` has +ended its restore with a registry write-through since #4521 — "a rollback is a +live write like any other: the restored body must be the one the runtime +dispatches on immediately, not after someone lists the type" — and the batch +path over the same repository call had no equivalent on either limb. + +Measured before the fix, real `SysMetadataRepository`, an `object` saved twice +(v2 adds a `due_date` field) and then reverted: + +``` +revertCommit -> { success: true, revertedCount: 1, failed: [] } +stored sys_metadata row fields -> ["name","amount"] # reverted +SchemaRegistry.getObject(...) fields -> [...,"name","amount","due_date"] # NOT reverted +``` + +So the undo reported success while data CRUD kept dispatching the pre-revert +schema, healing only at the next restart. It is type-agnostic and older than +the `object` support that made it loud: an overlay `view` showed the same split +(stored `Cases`, registry still `Renamed`). `rollbackToPackageCommit` reverts +through the same loop, so a whole-package rollback could report success and +change nothing the running process could see. + +Both limbs now refresh the registry, each reusing the seam its single-item +sibling already uses: + +- **Restore limb** — writes the restored body through under the row's OWN + ownership key, read from the row before the restore (#4636; stated as the + `sys_metadata` sentinel instead, `registerObject` throws `already owned by + package "app."` into a best-effort warning and the stale body survives). + The row's own organization is passed per item, so an org-scoped row inherits + ADR-0005's rule that only env-wide rows enter the process-wide registry. +- **Soft-remove limb** — runs the same three-tier heal `deleteMetaItem` runs + after its own repository delete: an overlay that shadows a packaged artifact + falls back to the artifact rather than vanishing, and only a name no layer + serves at all is retired. A flat unregister would have deleted names a code + package still ships. This heal is gated to env-wide reverts: an org-scoped row + never entered the shared registry, so healing on its behalf would retire the + entry every other organization reads. + +No contract change — ADR-0067 already defines what a revert leaves behind; this +makes the runtime agree with it without waiting for a restart. diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 782de83794..b8ecf64926 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -10382,6 +10382,18 @@ export class ObjectStackProtocolImplementation implements * artifact is restored to its pre-commit `prevVersion`. The revert is itself * recorded as a NEW commit (operation='revert'), so history stays * append-only and the revert is itself revertible. + * + * [#6621] BOTH limbs refresh the SchemaRegistry, so a revert that answers + * `success: true` is one the running process has already acted on. The + * restore limb writes the restored body through ({@link + * applyRegistryWriteThrough}, the #4521 rule the sibling + * {@link rollbackMetaItem} has always carried); the soft-remove limb runs + * the same three-tier heal the sibling {@link deleteMetaItem} runs after + * its own `repo.delete` ({@link restoreArtifactRegistryView}). Before + * this, a batch revert persisted its change and left the runtime + * dispatching the reverted-away body until restart — + * {@link rollbackToPackageCommit} inherited it, so a whole-package + * rollback could report success and change nothing the process could see. */ async revertCommit(request: { commitId: string; @@ -10451,9 +10463,8 @@ export class ObjectStackProtocolImplementation implements // Sibling limb: #6563 (PR #6642) did the same for the // restore branch below, where the intent was UNSTATED and // fell through to `restoreVersion`'s `?? 'override-artifact'` - // default. Still not addressed here, filed with its own - // measurement: neither limb refreshes the SchemaRegistry the - // way `rollbackMetaItem` does (#6621). + // default. The registry half of both limbs is #6621, fixed + // here and below. const intent: 'override-artifact' | 'runtime-only' = this.isArtifactBacked(it.type, it.name) ? 'override-artifact' : 'runtime-only'; if (current) { @@ -10465,6 +10476,50 @@ export class ObjectStackProtocolImplementation implements state: 'active', }); } + // [#6621] The registry must stop serving what the revert + // just removed — the #4521 rule on the DELETE side of it. + // + // Measured on `origin/main` before this line existed: a + // first-build undo of a created `object` answered + // `success: true`, left `sys_metadata` with zero rows for + // the name, and `SchemaRegistry` kept serving the body — + // the same split the restore limb below showed, one limb + // over. Same for an overlay `view` on a control-plane + // kernel, where the plain-key entry `saveMetaItem`'s + // write-through had put there simply stayed. + // + // WHICH heal, and why not a bare unregister: this is the + // #6687 three-tier walk the sibling delete caller + // {@link deleteMetaItem} runs after its own `repo.delete`, + // and the tiers are the point. A soft-removed overlay that + // shadows a packaged artifact must fall BACK to the + // artifact (tier 1, ADR-0005 reset), not vanish; only when + // no layer serves the name at all is the plain-key entry + // retired (tier 3, #5079). A flat `removeOverlayEntry` + // here would delete names a code package still ships. Both + // delete/revert callers now run the same walk, exactly as + // both now derive the same per-item intent. + // + // Run for the no-row case too, deliberately: that is the + // self-heal branch `deleteMetaItem` documents — a stale + // shadow can outlive the row it came from, and this limb's + // contract is "this artifact is not here after the revert", + // not "a row was deleted". + // + // [#6602] ORG GATE, and it is asymmetric ON PURPOSE. Only + // an env-wide revert may mutate the process-wide registry: + // an org-scoped row never entered it (ADR-0005, the rule + // {@link hydrateOverlayIntoRegistry} owns), so healing on + // its behalf would un-shadow or retire an entry that + // belongs to the env-wide row every other org reads. The + // write-through's object branch is deliberately NOT + // org-gated, and that carve-out does not transfer here: it + // is argued from `assertObjectRegistered` failing CLOSED, + // which licenses registering broadly and never retiring + // broadly. Register wide, retire narrow. + if (orgId === null) { + await this.restoreArtifactRegistryView(it.type, it.name); + } 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. @@ -10489,18 +10544,65 @@ export class ObjectStackProtocolImplementation implements // // The soft-remove limb above stated the same intent as a // CONSTANT and was fixed the same way (#6620), so both limbs now - // derive it. One neighbour is still open, filed with its own - // measurement: neither limb refreshes the SchemaRegistry the way - // `rollbackMetaItem` does, so a restored body is persisted but not - // yet dispatched on (#6621). + // derive it. The registry half of both is #6621, below. const intent: 'override-artifact' | 'runtime-only' = this.isArtifactBacked(it.type, it.name) ? 'override-artifact' : 'runtime-only'; - await repo.restoreVersion(ref, it.prevVersion, { + // [#6621 / #4636] The ownership key the write-through needs, + // read from the ROW rather than from the request — the sibling + // revert caller {@link rollbackMetaItem} reads it exactly this + // way, and for the same reason: `revertCommit` has no + // `packageId` parameter either, and inventing one would let a + // caller re-key an artifact it does not own. Left unpassed, a + // row bound to `app.` re-registers under the + // `'sys_metadata'` sentinel and `registerObject` throws + // `already owned by package "app."` into a best-effort + // `console.warn` — a revert that reports success while the + // registry keeps the body it was supposed to revert. + // + // Read BEFORE the restore, deliberately (#4636 again): the row + // exists at this point and a read failure still fails this ITEM + // cleanly into `failed[]`. Read afterwards it would be a + // fallible query downstream of a write that already succeeded — + // the shape that ends in a `catch {}` swallowing a real outage + // (#4867). Per ITEM, because a batch mixes bindings. + const restorePackageId = await this.resolveOverlayPackageBinding(it.type, it.name, orgId); + const restored = await repo.restoreVersion(ref, it.prevVersion, { actor, source: 'protocol.revertCommit', message: `revert commit ${request.commitId}`, intent, }); + // [#6621] #4521 — a revert is a live write like any other: the + // restored body must be the one the runtime dispatches on + // immediately, not after someone lists the type. + // + // Measured on `origin/main` before this call existed, with the + // real `SysMetadataRepository`: an `object` saved twice and then + // reverted answered `{ success: true, revertedCount: 1, + // failed: [] }`, the stored row came back to `["name","amount"]` + // — and `SchemaRegistry.getObject(...)` still carried + // `due_date`. `success: true` while CRUD dispatches on the body + // the operator just reverted away, healing only at the next + // restart. Type-agnostic: an overlay `view` on a control-plane + // kernel showed the same split (`stored 'Cases'` vs + // `registry 'Renamed'`). + // + // The registry key is the SINGULAR type — the spelling + // `saveMetaItem`'s own write-through registered under — while + // the repo-facing reads above keep `it.type`, which is the + // spelling the row is stored with. Two different keys, on + // purpose. + this.applyRegistryWriteThrough({ + type: PLURAL_TO_SINGULAR[it.type] ?? it.type, + name: it.name, + item: restored.item.body, + packageId: restorePackageId, + // [#6602] The row's OWN scope, per item. An org-scoped row + // is refused by {@link hydrateOverlayIntoRegistry} and never + // reaches the registry every org in this process shares — + // inherited, not re-decided here. + organizationId: orgId, + }); reverted.push({ type: it.type, name: it.name, action: 'restored' }); } } catch (e: any) { diff --git a/packages/objectql/src/protocol-commit-history.test.ts b/packages/objectql/src/protocol-commit-history.test.ts index 3b84faa20c..2891944b86 100644 --- a/packages/objectql/src/protocol-commit-history.test.ts +++ b/packages/objectql/src/protocol-commit-history.test.ts @@ -21,6 +21,17 @@ import { assertEngineUpdateDispatch } from './engine-update-dispatch.js'; function makeFakeEngine(seedCommits: any[] = []) { const commits: any[] = [...seedCommits]; const engine: any = { + // [#6621] `revertCommit` now touches `engine.registry` on BOTH limbs (the + // restore write-through and the soft-remove heal), so the double carries + // the real registry rather than nothing. A bare `{}` engine would make + // every plan assertion below fail for a reason that has nothing to do with + // the plan — which is exactly the false red a stubbed double is supposed + // to avoid. + registry: (() => { + const r = new SchemaRegistry({ multiTenant: false }); + (r as any).logLevel = 'silent'; + return r; + })(), insert: vi.fn(async (table: string, data: any) => { if (table === 'sys_metadata_commit') commits.push(data); }), @@ -97,7 +108,15 @@ describe('ADR-0067 — revertCommit', () => { const { engine } = makeFakeEngine([ applyCommit({ id: 'cmt_2', items: [{ type: 'object', name: 'course', existedBefore: true, prevVersion: 3 }], created_at: '2026-06-24T00:00:00.000Z' }), ]); - const restoreVersion = vi.fn(async () => ({})); + // [#6621] `{}` was not what `SysMetadataRepository.restoreVersion` returns: + // its declared `PutResult` carries the committed item, and the restore limb + // now writes that body through to the registry. A double that answers less + // than the contract makes the caller look broken. + const restoreVersion = vi.fn(async () => ({ + version: 'h3', + seq: 2, + item: { body: { name: 'course', label: 'Course', fields: { name: { name: 'name', type: 'text' } } } }, + })); const repo = { get: vi.fn(async () => ({ hash: 'h2' })), delete: vi.fn(), restoreVersion }; const p = makeProtocol(engine, repo); @@ -245,7 +264,14 @@ const rowKey = (w: Record) => * at all), `sys_metadata_history`, `sys_metadata_commit`. Both write verbs are * pinned to ObjectQL's own dispatch predicates. */ -function makeRealRepoHarness(seedCommits: any[] = []) { +function makeRealRepoHarness(seedCommits: any[] = [], opts: { controlPlane?: boolean } = {}) { + // [#6621] The kernel scope is a FLAG, never an `environmentId` parameter with + // an `'env_test'` default: passing `undefined` explicitly to a defaulted + // parameter re-applies the default, so a "control-plane" harness would have + // silently stayed project-scoped and the overlay-type write-through — which + // is gated on `environmentId === undefined` — would have been measured as + // "never registers anything at all". + const environmentId: string | undefined = opts.controlPlane === true ? undefined : 'env_test'; const registry = new SchemaRegistry({ multiTenant: false }); (registry as any).logLevel = 'silent'; const rows = new Map(); @@ -305,7 +331,7 @@ function makeRealRepoHarness(seedCommits: any[] = []) { async syncObjectSchema() { /* no physical storage in this double */ }, }; - const protocol = new ObjectStackProtocolImplementation(engine, undefined, 'env_test'); + const protocol = new ObjectStackProtocolImplementation(engine, undefined, environmentId); return { protocol, engine, rows, historyRows, commits, registry }; } @@ -831,6 +857,338 @@ describe('#6620 — revertCommit soft-removes a runtime-CREATED `object`', () => * while the created object was never removed. The line that goes red pre-fix is * the STORED ROW. */ +/** + * #6621 — a successful `revertCommit` REFRESHES the SchemaRegistry. + * + * Everything above pins what `revertCommit` writes to `sys_metadata`. Nothing + * above looks at what the RUNTIME then dispatches on, and that is where this + * defect lived: the restore limb awaited `repo.restoreVersion(...)`, pushed + * `{ action: 'restored' }` and moved on, while the sibling single-item revert + * `rollbackMetaItem` had ended the same repository call with a registry + * write-through since #4521 ("a rollback is a live write like any other"). One + * seam over, the rule was simply missing. + * + * Measured on `origin/main` (this file's own harness, real + * `SysMetadataRepository`), an `object` saved twice then reverted: + * + * revertCommit -> { success: true, revertedCount: 1, failed: [] } + * stored sys_metadata row fields -> ["name","amount"] # reverted + * SchemaRegistry.getObject(...) fields -> [..., "name","amount","due_date"] # NOT reverted + * + * `success: true` while CRUD keeps dispatching the body the operator just + * reverted away, healing only at the next restart — and + * `rollbackToPackageCommit` reverts through the same loop, so a whole-package + * rollback could report success and change nothing the running process sees. + * + * Type-agnostic and older than #6563: an overlay `view` on a control-plane + * kernel showed the same split (stored `Cases`, registry still `Renamed`). + * `object` merely makes it loud, because the registry copy is what data CRUD + * dispatches on. + */ + +/** The registry copy — the thing the runtime dispatches on, not the row. */ +const registryObjectFields = (registry: SchemaRegistry, name: string) => + Object.keys(((registry.getObject(name) as any)?.fields ?? {})); + +const registryViewLabel = (registry: SchemaRegistry, name: string) => + (registry.getItem('view', name) as any)?.label ?? null; + +describe('#6621 — revertCommit RESTORE limb refreshes the registry', () => { + it('an object revert moves the registry copy too, not just the stored row', async () => { + const { protocol, rows, registry } = makeRealRepoHarness([objectCommit({ + id: 'cmt_reg_obj', + items: [{ type: 'object', name: 'myapp_invoice', existedBefore: true, prevVersion: 1 }], + })]); + await seedObjectEdit(protocol, 'myapp_invoice', APP_PKG); + // The edit really is live in the registry before the revert — otherwise + // "reverted" below could be true for the empty reason. + expect(registryObjectFields(registry, 'myapp_invoice')).toContain('due_date'); + + const res = await protocol.revertCommit({ commitId: 'cmt_reg_obj' }); + + expect(res.failed).toEqual([]); + expect(res.revertedCount).toBe(1); + expect(storedFields(rows, 'myapp_invoice').fields).not.toContain('due_date'); + // THE LINE THAT WAS RED: pre-fix this still contained `due_date`. + expect(registryObjectFields(registry, 'myapp_invoice')).not.toContain('due_date'); + expect(registryObjectFields(registry, 'myapp_invoice')).toEqual( + expect.arrayContaining(['name', 'amount']), + ); + }); + + /** + * #4636 — the write-through re-registers under the ROW'S OWN ownership key. + * Stated as the `'sys_metadata'` sentinel instead, `registerObject` throws + * `already owned by package "app.myapp"` into a best-effort `console.warn` + * and the registry keeps the pre-revert body — a revert that reports success + * and changed nothing, which is the very failure this block exists to close. + * So ownership is asserted as a SURVIVING fact, not a changed one. + */ + it('ownership survives the refresh: same owner package, still org-provenanced, no clash', async () => { + const { protocol, registry } = makeRealRepoHarness([objectCommit({ + id: 'cmt_reg_owner', + items: [{ type: 'object', name: 'myapp_invoice', existedBefore: true, prevVersion: 1 }], + })]); + await seedObjectEdit(protocol, 'myapp_invoice', APP_PKG); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const res = await protocol.revertCommit({ commitId: 'cmt_reg_owner' }); + expect(res.failed).toEqual([]); + expect(registry.getObjectOwner('myapp_invoice')?.packageId).toBe(APP_PKG); + expect((registry.getObject('myapp_invoice') as any)?._provenance).toBe('org'); + expect( + warn.mock.calls.map((c) => String(c[0])).filter((m) => m.includes('already owned by package')), + ).toEqual([]); + } finally { + warn.mockRestore(); + } + }); + + /** + * The overlay half. `applyRegistryWriteThrough`'s non-object branch is gated + * on `environmentId === undefined`, so this one needs a control-plane kernel + * — which is also the kernel on which an overlay type could ALWAYS reach this + * limb, long before #6563 let `object` in. The defect is older than the issue + * that made it visible. + */ + it('an overlay-type revert moves the registry copy too (control-plane kernel)', async () => { + const { protocol, rows, registry } = makeRealRepoHarness([applyCommit({ + id: 'cmt_reg_view', + package_id: APP_PKG, + items: [{ type: 'view', name: 'myapp_case_grid', existedBefore: true, prevVersion: 1 }], + created_at: '2026-08-08T00:00:02.000Z', + })], { controlPlane: true }); + await seedPackageBoundEdit(protocol); + expect(registryViewLabel(registry, 'myapp_case_grid')).toBe('Renamed'); + + const res = await protocol.revertCommit({ commitId: 'cmt_reg_view' }); + + expect(res.failed).toEqual([]); + const stored = Array.from(rows.values()).filter((r) => r.name === 'myapp_case_grid'); + expect(JSON.parse(stored[0].metadata).label).toBe('Cases'); + // THE LINE THAT WAS RED: pre-fix the registry still served 'Renamed'. + expect(registryViewLabel(registry, 'myapp_case_grid')).toBe('Cases'); + }); + + /** + * [#6602] The org dimension, INHERITED rather than re-decided here. ADR-0005: + * only env-wide rows enter the process-wide SchemaRegistry, and PR #6779 made + * `organizationId` a REQUIRED argument of the write-through so no caller can + * forget to say which it is. This limb passes the row's own org, so an + * org-scoped revert persists and stays out of the shared registry. + * + * Direction note (measured, not assumed): this case is green BEFORE the fix + * as well — pre-fix nothing was written through at all, so "the shared + * registry is untouched" was true for the wrong reason. It cannot go red by + * removing the write-through; what it goes red on is the write-through + * passing anything other than the row's own org, which is the mistake the + * required parameter exists to prevent. + */ + it('an ORG-scoped revert persists and still never reaches the process-wide registry', async () => { + const { protocol, rows, registry } = makeRealRepoHarness([applyCommit({ + id: 'cmt_reg_org', + package_id: APP_PKG, + organization_id: 'org_a', + items: [{ type: 'view', name: 'myapp_case_grid', existedBefore: true, prevVersion: 1 }], + created_at: '2026-08-08T00:00:02.000Z', + })], { controlPlane: true }); + await protocol.saveMetaItem({ + type: 'view', name: 'myapp_case_grid', organizationId: 'org_a', packageId: APP_PKG, item: gridBody('Cases'), + }); + await protocol.saveMetaItem({ + type: 'view', name: 'myapp_case_grid', organizationId: 'org_a', packageId: APP_PKG, item: gridBody('Renamed'), + }); + expect(registryViewLabel(registry, 'myapp_case_grid')).toBeNull(); + + const res = await protocol.revertCommit({ commitId: 'cmt_reg_org', organizationId: 'org_a' }); + + expect(res.failed).toEqual([]); + expect(res.revertedCount).toBe(1); + const stored = Array.from(rows.values()).filter((r) => r.name === 'myapp_case_grid'); + expect(stored[0].organization_id).toBe('org_a'); + expect(JSON.parse(stored[0].metadata).label).toBe('Cases'); + expect(registryViewLabel(registry, 'myapp_case_grid')).toBeNull(); + }); +}); + +/** + * #6621 — the SOFT-REMOVE limb's symmetric decision, and why it is `reuse the + * heal` rather than `nothing` or `unregister`. + * + * Measured on `origin/main`: a first-build undo of a created `object` answered + * `success: true`, left zero `sys_metadata` rows for the name, and the registry + * kept serving it. Same for a created overlay `view` on a control-plane kernel. + * So "do nothing" was not a neutral option — it is the measured defect. + * + * The heal reused is the one the sibling DELETE caller `deleteMetaItem` runs + * after its own `repo.delete` (`restoreArtifactRegistryView`, the #6687 + * three-tier walk), and the tiers are the reason a flat unregister was + * rejected: an overlay that shadows a packaged artifact must fall BACK to the + * artifact (tier 1), and only a name no layer serves is retired (tier 3, + * #5079). Both cases are pinned below. + * + * These assert PARITY with `deleteMetaItem` rather than a literal registry + * state, deliberately. The two callers perform the same repository delete and + * should leave the same runtime view; stating it as parity also keeps the pin + * honest about a gap it does NOT close — `restoreArtifactRegistryView` reaches + * the `metadata` map but not `objectContributors`, so `getObject` still serves + * a soft-removed runtime object. That gap is `deleteMetaItem`'s too (there is + * no per-name object unregister in `SchemaRegistry` at all, only + * `unregisterObjectsByPackage`), it is not introduced here, and a parity + * assertion stays green when it is fixed for both. + */ + +/** The registry facts a soft-remove is allowed to change, as one comparable value. */ +const registryShapeFor = (registry: SchemaRegistry, type: string, name: string) => ({ + plainKeyEntry: Array.from( + ((registry as any).metadata as Map>).get(type)?.keys() ?? [], + ).includes(name), + itemLabel: (registry.getItem(type, name) as any)?.label ?? null, + objectServed: registry.getObject(name) !== undefined, +}); + +describe('#6621 — revertCommit SOFT-REMOVE limb heals the registry, like deleteMetaItem', () => { + it('a created overlay item stops being served — and matches what deleteMetaItem leaves', async () => { + const viaRevert = makeRealRepoHarness([applyCommit({ + id: 'cmt_reg_new_view', + package_id: APP_PKG, + items: [{ type: 'view', name: 'myapp_case_grid', existedBefore: false, prevVersion: null }], + created_at: '2026-08-08T00:00:02.000Z', + })], { controlPlane: true }); + await viaRevert.protocol.saveMetaItem({ + type: 'view', name: 'myapp_case_grid', packageId: APP_PKG, item: gridBody('Cases'), + }); + expect(registryViewLabel(viaRevert.registry, 'myapp_case_grid')).toBe('Cases'); + + const res = await viaRevert.protocol.revertCommit({ commitId: 'cmt_reg_new_view' }); + expect(res.failed).toEqual([]); + expect(storedRows(viaRevert.rows, 'myapp_case_grid')).toHaveLength(0); + // THE LINE THAT WAS RED: pre-fix the registry still served 'Cases'. + expect(registryViewLabel(viaRevert.registry, 'myapp_case_grid')).toBeNull(); + + // …and it is the SAME view the single-item delete leaves behind. + const viaDelete = makeRealRepoHarness([], { controlPlane: true }); + await viaDelete.protocol.saveMetaItem({ + type: 'view', name: 'myapp_case_grid', packageId: APP_PKG, item: gridBody('Cases'), + }); + await viaDelete.protocol.deleteMetaItem({ type: 'view', name: 'myapp_case_grid' }); + expect(registryShapeFor(viaRevert.registry, 'view', 'myapp_case_grid')) + .toEqual(registryShapeFor(viaDelete.registry, 'view', 'myapp_case_grid')); + }); + + /** + * Tier 1, and the reason a flat `removeOverlayEntry` was the WRONG answer: a + * packaged artifact sits under the reverted overlay, so the revert must leave + * the artifact serving the name — not leave the name unresolvable. + */ + it('falls BACK to the packaged artifact when one is underneath, never retiring the name', async () => { + const { protocol, registry } = makeRealRepoHarness([applyCommit({ + id: 'cmt_reg_new_shadow', + package_id: APP_PKG, + items: [{ type: 'view', name: 'myapp_case_grid', existedBefore: false, prevVersion: null }], + created_at: '2026-08-08T00:00:02.000Z', + })], { controlPlane: true }); + // The code package's own artifact, registered under its composite key. + (registry as any).registerItem('view', gridBody('Packaged'), 'name', APP_PKG); + await protocol.saveMetaItem({ + type: 'view', name: 'myapp_case_grid', packageId: APP_PKG, item: gridBody('Overlay'), + }); + expect(registryViewLabel(registry, 'myapp_case_grid')).toBe('Overlay'); + + await protocol.revertCommit({ commitId: 'cmt_reg_new_shadow' }); + + // Not `null` — the artifact default is back (ADR-0005 reset semantics). + expect(registryViewLabel(registry, 'myapp_case_grid')).toBe('Packaged'); + }); + + it('an object soft-remove leaves exactly the registry view deleteMetaItem leaves', async () => { + const viaRevert = makeRealRepoHarness([objectCommit({ + id: 'cmt_reg_new_obj', + items: [createdItem('myapp_invoice')], + })], { controlPlane: true }); + await seedCreatedObject(viaRevert.protocol, 'myapp_invoice', APP_PKG); + expect(registryShapeFor(viaRevert.registry, 'object', 'myapp_invoice').plainKeyEntry).toBe(true); + + const res = await viaRevert.protocol.revertCommit({ commitId: 'cmt_reg_new_obj' }); + expect(res.failed).toEqual([]); + expect(storedRows(viaRevert.rows, 'myapp_invoice')).toHaveLength(0); + // THE LINE THAT WAS RED: pre-fix the plain-key entry stayed for the life of + // the process, so `GET /meta/object` kept enumerating a reverted-away item. + expect(registryShapeFor(viaRevert.registry, 'object', 'myapp_invoice').plainKeyEntry).toBe(false); + + const viaDelete = makeRealRepoHarness([], { controlPlane: true }); + await seedCreatedObject(viaDelete.protocol, 'myapp_invoice', APP_PKG); + await viaDelete.protocol.deleteMetaItem({ type: 'object', name: 'myapp_invoice' }); + expect(registryShapeFor(viaRevert.registry, 'object', 'myapp_invoice')) + .toEqual(registryShapeFor(viaDelete.registry, 'object', 'myapp_invoice')); + }); + + /** + * [#6602] The org gate on this limb is ASYMMETRIC with the write-through's + * object branch, on purpose: only an env-wide revert may mutate the registry + * every org in this process shares. An org-scoped row never entered it, so + * healing on its behalf would retire or un-shadow the ENV-WIDE row's entry — + * a per-org undo breaking every other org. Register wide, retire narrow. + */ + it('an ORG-scoped soft-remove leaves the env-wide registry entry alone', async () => { + const { protocol, rows, registry } = makeRealRepoHarness([applyCommit({ + id: 'cmt_reg_new_org', + package_id: APP_PKG, + organization_id: 'org_a', + items: [{ type: 'view', name: 'myapp_case_grid', existedBefore: false, prevVersion: null }], + created_at: '2026-08-08T00:00:02.000Z', + })], { controlPlane: true }); + // The env-wide row is what the shared registry holds (ADR-0005). + await protocol.saveMetaItem({ + type: 'view', name: 'myapp_case_grid', packageId: APP_PKG, item: gridBody('EnvWide'), + }); + // …and org A authored its own overlay of the same name. + await protocol.saveMetaItem({ + type: 'view', name: 'myapp_case_grid', organizationId: 'org_a', packageId: APP_PKG, item: gridBody('OrgA'), + }); + expect(registryViewLabel(registry, 'myapp_case_grid')).toBe('EnvWide'); + + const res = await protocol.revertCommit({ commitId: 'cmt_reg_new_org', organizationId: 'org_a' }); + + expect(res.failed).toEqual([]); + // Org A's row really went away… + expect(Array.from(rows.values()).filter((r) => r.name === 'myapp_case_grid' && r.organization_id === 'org_a')) + .toHaveLength(0); + // …and the env-wide entry every other org reads is untouched. + expect(registryViewLabel(registry, 'myapp_case_grid')).toBe('EnvWide'); + }); +}); + +/** + * #6621 — the inheritance. `rollbackToPackageCommit` reverts through the SAME + * loop, so a whole-package rollback answered `success: true` while the running + * process kept dispatching every reverted-away body. As in #6563's and #6620's + * inheritance pins, the status cannot show it: the line that was red is the + * REGISTRY copy, not the result shape. + */ +describe('#6621 — rollbackToPackageCommit inherits the registry refresh', () => { + it('rolls an object edit back through the loop — and the registry copy really moved', async () => { + const { protocol, rows, registry } = 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); + expect(registryObjectFields(registry, 'myapp_invoice')).toContain('due_date'); + + const res = await protocol.rollbackToPackageCommit({ commitId: 'cmt_base' }); + + expect(res.revertedCommits).toEqual(['cmt_edit']); + expect(res.failed).toEqual([]); + expect(storedFields(rows, 'myapp_invoice').fields).not.toContain('due_date'); + // `success: true` was ALREADY true pre-fix — this is the line that was not. + expect(registryObjectFields(registry, 'myapp_invoice')).not.toContain('due_date'); + }); +}); + describe('#6620 — rollbackToPackageCommit inherits the per-item soft-remove intent', () => { it('rolls a first build back through the loop — and the created row really went away', async () => { const { protocol, rows } = makeRealRepoHarness([