Skip to content

Commit 271cee1

Browse files
os-zhuangclaude
os-zhuang
and
claude
authored
fix(metadata-protocol): a successful revertCommit refreshes the SchemaRegistry, on both limbs (#6621) (#6807)
* wip(#6621): revertCommit refreshes the SchemaRegistry on both limbs * test(#6621): pin the registry refresh on both revertCommit limbs * chore(#6621): changeset --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8825a06 commit 271cee1

3 files changed

Lines changed: 519 additions & 11 deletions

File tree

‎.changeset/revert-commit-registry-refresh.md‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
fix(metadata-protocol): a successful `revertCommit` refreshes the SchemaRegistry (#6621)
6+
7+
`revertCommit` persisted its change and left the running process serving the
8+
body it had just reverted away. The single-item revert `rollbackMetaItem` has
9+
ended its restore with a registry write-through since #4521 — "a rollback is a
10+
live write like any other: the restored body must be the one the runtime
11+
dispatches on immediately, not after someone lists the type" — and the batch
12+
path over the same repository call had no equivalent on either limb.
13+
14+
Measured before the fix, real `SysMetadataRepository`, an `object` saved twice
15+
(v2 adds a `due_date` field) and then reverted:
16+
17+
```
18+
revertCommit -> { success: true, revertedCount: 1, failed: [] }
19+
stored sys_metadata row fields -> ["name","amount"] # reverted
20+
SchemaRegistry.getObject(...) fields -> [...,"name","amount","due_date"] # NOT reverted
21+
```
22+
23+
So the undo reported success while data CRUD kept dispatching the pre-revert
24+
schema, healing only at the next restart. It is type-agnostic and older than
25+
the `object` support that made it loud: an overlay `view` showed the same split
26+
(stored `Cases`, registry still `Renamed`). `rollbackToPackageCommit` reverts
27+
through the same loop, so a whole-package rollback could report success and
28+
change nothing the running process could see.
29+
30+
Both limbs now refresh the registry, each reusing the seam its single-item
31+
sibling already uses:
32+
33+
- **Restore limb** — writes the restored body through under the row's OWN
34+
ownership key, read from the row before the restore (#4636; stated as the
35+
`sys_metadata` sentinel instead, `registerObject` throws `already owned by
36+
package "app.<slug>"` into a best-effort warning and the stale body survives).
37+
The row's own organization is passed per item, so an org-scoped row inherits
38+
ADR-0005's rule that only env-wide rows enter the process-wide registry.
39+
- **Soft-remove limb** — runs the same three-tier heal `deleteMetaItem` runs
40+
after its own repository delete: an overlay that shadows a packaged artifact
41+
falls back to the artifact rather than vanishing, and only a name no layer
42+
serves at all is retired. A flat unregister would have deleted names a code
43+
package still ships. This heal is gated to env-wide reverts: an org-scoped row
44+
never entered the shared registry, so healing on its behalf would retire the
45+
entry every other organization reads.
46+
47+
No contract change — ADR-0067 already defines what a revert leaves behind; this
48+
makes the runtime agree with it without waiting for a restart.

‎packages/metadata-protocol/src/protocol.ts‎

Lines changed: 110 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10382,6 +10382,18 @@ export class ObjectStackProtocolImplementation implements
1038210382
* artifact is restored to its pre-commit `prevVersion`. The revert is itself
1038310383
* recorded as a NEW commit (operation='revert'), so history stays
1038410384
* append-only and the revert is itself revertible.
10385+
*
10386+
* [#6621] BOTH limbs refresh the SchemaRegistry, so a revert that answers
10387+
* `success: true` is one the running process has already acted on. The
10388+
* restore limb writes the restored body through ({@link
10389+
* applyRegistryWriteThrough}, the #4521 rule the sibling
10390+
* {@link rollbackMetaItem} has always carried); the soft-remove limb runs
10391+
* the same three-tier heal the sibling {@link deleteMetaItem} runs after
10392+
* its own `repo.delete` ({@link restoreArtifactRegistryView}). Before
10393+
* this, a batch revert persisted its change and left the runtime
10394+
* dispatching the reverted-away body until restart —
10395+
* {@link rollbackToPackageCommit} inherited it, so a whole-package
10396+
* rollback could report success and change nothing the process could see.
1038510397
*/
1038610398
async revertCommit(request: {
1038710399
commitId: string;
@@ -10451,9 +10463,8 @@ export class ObjectStackProtocolImplementation implements
1045110463
// Sibling limb: #6563 (PR #6642) did the same for the
1045210464
// restore branch below, where the intent was UNSTATED and
1045310465
// fell through to `restoreVersion`'s `?? 'override-artifact'`
10454-
// default. Still not addressed here, filed with its own
10455-
// measurement: neither limb refreshes the SchemaRegistry the
10456-
// way `rollbackMetaItem` does (#6621).
10466+
// default. The registry half of both limbs is #6621, fixed
10467+
// here and below.
1045710468
const intent: 'override-artifact' | 'runtime-only' =
1045810469
this.isArtifactBacked(it.type, it.name) ? 'override-artifact' : 'runtime-only';
1045910470
if (current) {
@@ -10465,6 +10476,50 @@ export class ObjectStackProtocolImplementation implements
1046510476
state: 'active',
1046610477
});
1046710478
}
10479+
// [#6621] The registry must stop serving what the revert
10480+
// just removed — the #4521 rule on the DELETE side of it.
10481+
//
10482+
// Measured on `origin/main` before this line existed: a
10483+
// first-build undo of a created `object` answered
10484+
// `success: true`, left `sys_metadata` with zero rows for
10485+
// the name, and `SchemaRegistry` kept serving the body —
10486+
// the same split the restore limb below showed, one limb
10487+
// over. Same for an overlay `view` on a control-plane
10488+
// kernel, where the plain-key entry `saveMetaItem`'s
10489+
// write-through had put there simply stayed.
10490+
//
10491+
// WHICH heal, and why not a bare unregister: this is the
10492+
// #6687 three-tier walk the sibling delete caller
10493+
// {@link deleteMetaItem} runs after its own `repo.delete`,
10494+
// and the tiers are the point. A soft-removed overlay that
10495+
// shadows a packaged artifact must fall BACK to the
10496+
// artifact (tier 1, ADR-0005 reset), not vanish; only when
10497+
// no layer serves the name at all is the plain-key entry
10498+
// retired (tier 3, #5079). A flat `removeOverlayEntry`
10499+
// here would delete names a code package still ships. Both
10500+
// delete/revert callers now run the same walk, exactly as
10501+
// both now derive the same per-item intent.
10502+
//
10503+
// Run for the no-row case too, deliberately: that is the
10504+
// self-heal branch `deleteMetaItem` documents — a stale
10505+
// shadow can outlive the row it came from, and this limb's
10506+
// contract is "this artifact is not here after the revert",
10507+
// not "a row was deleted".
10508+
//
10509+
// [#6602] ORG GATE, and it is asymmetric ON PURPOSE. Only
10510+
// an env-wide revert may mutate the process-wide registry:
10511+
// an org-scoped row never entered it (ADR-0005, the rule
10512+
// {@link hydrateOverlayIntoRegistry} owns), so healing on
10513+
// its behalf would un-shadow or retire an entry that
10514+
// belongs to the env-wide row every other org reads. The
10515+
// write-through's object branch is deliberately NOT
10516+
// org-gated, and that carve-out does not transfer here: it
10517+
// is argued from `assertObjectRegistered` failing CLOSED,
10518+
// which licenses registering broadly and never retiring
10519+
// broadly. Register wide, retire narrow.
10520+
if (orgId === null) {
10521+
await this.restoreArtifactRegistryView(it.type, it.name);
10522+
}
1046810523
reverted.push({ type: it.type, name: it.name, action: 'removed' });
1046910524
} else if (it.prevVersion !== null && it.prevVersion !== undefined) {
1047010525
// Edited an existing artifact → restore the pre-commit body.
@@ -10489,18 +10544,65 @@ export class ObjectStackProtocolImplementation implements
1048910544
//
1049010545
// The soft-remove limb above stated the same intent as a
1049110546
// CONSTANT and was fixed the same way (#6620), so both limbs now
10492-
// derive it. One neighbour is still open, filed with its own
10493-
// measurement: neither limb refreshes the SchemaRegistry the way
10494-
// `rollbackMetaItem` does, so a restored body is persisted but not
10495-
// yet dispatched on (#6621).
10547+
// derive it. The registry half of both is #6621, below.
1049610548
const intent: 'override-artifact' | 'runtime-only' =
1049710549
this.isArtifactBacked(it.type, it.name) ? 'override-artifact' : 'runtime-only';
10498-
await repo.restoreVersion(ref, it.prevVersion, {
10550+
// [#6621 / #4636] The ownership key the write-through needs,
10551+
// read from the ROW rather than from the request — the sibling
10552+
// revert caller {@link rollbackMetaItem} reads it exactly this
10553+
// way, and for the same reason: `revertCommit` has no
10554+
// `packageId` parameter either, and inventing one would let a
10555+
// caller re-key an artifact it does not own. Left unpassed, a
10556+
// row bound to `app.<slug>` re-registers under the
10557+
// `'sys_metadata'` sentinel and `registerObject` throws
10558+
// `already owned by package "app.<slug>"` into a best-effort
10559+
// `console.warn` — a revert that reports success while the
10560+
// registry keeps the body it was supposed to revert.
10561+
//
10562+
// Read BEFORE the restore, deliberately (#4636 again): the row
10563+
// exists at this point and a read failure still fails this ITEM
10564+
// cleanly into `failed[]`. Read afterwards it would be a
10565+
// fallible query downstream of a write that already succeeded —
10566+
// the shape that ends in a `catch {}` swallowing a real outage
10567+
// (#4867). Per ITEM, because a batch mixes bindings.
10568+
const restorePackageId = await this.resolveOverlayPackageBinding(it.type, it.name, orgId);
10569+
const restored = await repo.restoreVersion(ref, it.prevVersion, {
1049910570
actor,
1050010571
source: 'protocol.revertCommit',
1050110572
message: `revert commit ${request.commitId}`,
1050210573
intent,
1050310574
});
10575+
// [#6621] #4521 — a revert is a live write like any other: the
10576+
// restored body must be the one the runtime dispatches on
10577+
// immediately, not after someone lists the type.
10578+
//
10579+
// Measured on `origin/main` before this call existed, with the
10580+
// real `SysMetadataRepository`: an `object` saved twice and then
10581+
// reverted answered `{ success: true, revertedCount: 1,
10582+
// failed: [] }`, the stored row came back to `["name","amount"]`
10583+
// — and `SchemaRegistry.getObject(...)` still carried
10584+
// `due_date`. `success: true` while CRUD dispatches on the body
10585+
// the operator just reverted away, healing only at the next
10586+
// restart. Type-agnostic: an overlay `view` on a control-plane
10587+
// kernel showed the same split (`stored 'Cases'` vs
10588+
// `registry 'Renamed'`).
10589+
//
10590+
// The registry key is the SINGULAR type — the spelling
10591+
// `saveMetaItem`'s own write-through registered under — while
10592+
// the repo-facing reads above keep `it.type`, which is the
10593+
// spelling the row is stored with. Two different keys, on
10594+
// purpose.
10595+
this.applyRegistryWriteThrough({
10596+
type: PLURAL_TO_SINGULAR[it.type] ?? it.type,
10597+
name: it.name,
10598+
item: restored.item.body,
10599+
packageId: restorePackageId,
10600+
// [#6602] The row's OWN scope, per item. An org-scoped row
10601+
// is refused by {@link hydrateOverlayIntoRegistry} and never
10602+
// reaches the registry every org in this process shares —
10603+
// inherited, not re-decided here.
10604+
organizationId: orgId,
10605+
});
1050410606
reverted.push({ type: it.type, name: it.name, action: 'restored' });
1050510607
}
1050610608
} catch (e: any) {

0 commit comments

Comments
 (0)