Skip to content

Commit 75e6871

Browse files
os-zhuangclaude
andauthored
fix(metadata-protocol): revertCommit's soft-remove limb states its write intent per item (#6620) (#6768)
The limb that undoes an artifact a commit CREATED stated its intent as the constant 'override-artifact'. SysMetadataRepository.delete opens with assertAllowed(ref.type, opts.intent), which refuses every type that is not allowOrgOverride — 'object' among them — so a commit that created an object could not be reverted at all, and the first-build undo left the package half-reverted with success: false. The intent is now derived PER ITEM from isArtifactBacked, exactly as the sibling delete caller deleteMetaItem and the sibling revert caller rollbackMetaItem already derive it, so all three agree. The repository's gate is untouched: a genuinely artifact-backed item still resolves to 'override-artifact' and is still refused with NOT_OVERRIDABLE. Claude-Session: https://claude.ai/code/session_01W6bLax4KMrSfnE1ydFU8Dw Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6029cc1 commit 75e6871

3 files changed

Lines changed: 308 additions & 7 deletions

File tree

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): `revertCommit`'s soft-remove limb states its write intent per item, so a commit that CREATED an object can be reverted (#6620)
6+
7+
`ObjectStackProtocolImplementation.revertCommit` has two limbs. #6563 (PR #6642)
8+
fixed the one that RESTORES an edited artifact, where the intent was unstated and
9+
fell through to `restoreVersion`'s `?? 'override-artifact'` default. The other
10+
limb — an artifact the commit CREATED, which the revert soft-removes — stated the
11+
same intent as a literal constant:
12+
13+
```
14+
intent: 'override-artifact',
15+
```
16+
17+
`SysMetadataRepository.delete` opens with `this.assertAllowed(ref.type, opts.intent)`,
18+
the same gate `put` uses, and it refuses every type whose registry entry is not
19+
`allowOrgOverride`. `object` is exactly such a type, so every created object of a
20+
reverted commit came back in `failed[]`:
21+
22+
```
23+
[NOT_OVERRIDABLE] 'object' is not allowOrgOverride in the registry.
24+
Overlay-allowed: view, page, dashboard, app, action, report, dataset, ...
25+
```
26+
27+
This is the FIRST-BUILD undo — the Studio / AI flow that publishes a brand-new app
28+
and then undoes it. Every object the commit created stayed behind, the call
29+
answered `success: false` with a populated `failed[]`, and the package was left
30+
half-reverted: its overlay-allowed items removed, its objects not.
31+
`rollbackToPackageCommit` reverts through the same loop and inherited it, and
32+
there the symptom was quieter still — a per-item refusal never throws, so the
33+
rollback recorded the commit as reverted and answered `success: true` while the
34+
created object was untouched.
35+
36+
The limb now derives the intent from the artifact the way the sibling DELETE
37+
caller `deleteMetaItem` already does — `isArtifactBacked` gives
38+
`'override-artifact'`, otherwise `'runtime-only'` — and does it **per item**,
39+
because one first-build commit routinely creates a runtime object beside a
40+
packaged-artifact name. All three delete/revert callers (`deleteMetaItem`,
41+
`rollbackMetaItem`, both `revertCommit` limbs) now derive the same fact the same
42+
way.
43+
44+
The repository's gate is deliberately unchanged: it is right for callers that
45+
genuinely mean "override a packaged artifact", and the defect was this caller
46+
never saying which of the two cases each item is. An object a code package really
47+
ships still resolves to `'override-artifact'` and is still refused with
48+
`NOT_OVERRIDABLE`, which is pinned alongside the fix.

packages/metadata-protocol/src/protocol.ts

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10370,12 +10370,46 @@ export class ObjectStackProtocolImplementation implements
1037010370
const current = await repo.get(ref, { state: 'active' });
1037110371
if (!it.existedBefore) {
1037210372
// Created by this commit → soft-remove (metadata only; table stays).
10373+
//
10374+
// [#6620] The write INTENT is derived per item, exactly as
10375+
// the sibling DELETE caller {@link deleteMetaItem} derives
10376+
// it (and as the sibling revert caller
10377+
// {@link rollbackMetaItem} derives its own) — all three now
10378+
// agree. Stated as the CONSTANT `'override-artifact'` this
10379+
// limb used to carry, `SysMetadataRepository.delete` opened
10380+
// with `assertAllowed(ref.type, opts.intent)` — the same
10381+
// gate `put` uses — which refuses every type that is not
10382+
// `allowOrgOverride`, `object` among them. So a commit that
10383+
// CREATED an object could not be reverted at all: the
10384+
// first-build undo (publish a brand-new app, then undo it)
10385+
// left every created object behind, answered `success:
10386+
// false` with a populated `failed[]`, and left the package
10387+
// half-reverted — its overlay-allowed items removed, its
10388+
// objects not.
10389+
//
10390+
// Per ITEM, not per call: one first-build commit routinely
10391+
// creates a runtime object beside a packaged-artifact name,
10392+
// so a hoisted intent has to pick one and be wrong about
10393+
// the other. A genuinely artifact-backed item still
10394+
// resolves to `'override-artifact'` and is still refused
10395+
// with `NOT_OVERRIDABLE` — the derivation states the
10396+
// caller's case, it does not widen the repository's gate,
10397+
// which is unchanged and right.
10398+
//
10399+
// Sibling limb: #6563 (PR #6642) did the same for the
10400+
// restore branch below, where the intent was UNSTATED and
10401+
// fell through to `restoreVersion`'s `?? 'override-artifact'`
10402+
// default. Still not addressed here, filed with its own
10403+
// measurement: neither limb refreshes the SchemaRegistry the
10404+
// way `rollbackMetaItem` does (#6621).
10405+
const intent: 'override-artifact' | 'runtime-only' =
10406+
this.isArtifactBacked(it.type, it.name) ? 'override-artifact' : 'runtime-only';
1037310407
if (current) {
1037410408
await repo.delete(ref, {
1037510409
parentVersion: current.hash,
1037610410
actor,
1037710411
source: 'protocol.revertCommit',
10378-
intent: 'override-artifact',
10412+
intent,
1037910413
state: 'active',
1038010414
});
1038110415
}
@@ -10401,12 +10435,12 @@ export class ObjectStackProtocolImplementation implements
1040110435
// resolves to `'override-artifact'` and is still refused — the
1040210436
// derivation states the case, it does not widen the gate.
1040310437
//
10404-
// Two neighbours are deliberately NOT changed here, each filed
10405-
// with its own measurement: the soft-remove limb above states the
10406-
// same intent as a CONSTANT, so a commit that CREATED an object
10407-
// still cannot be reverted (#6620); and neither limb refreshes the
10408-
// SchemaRegistry the way `rollbackMetaItem` does, so a restored
10409-
// body is persisted but not yet dispatched on (#6621).
10438+
// The soft-remove limb above stated the same intent as a
10439+
// CONSTANT and was fixed the same way (#6620), so both limbs now
10440+
// derive it. One neighbour is still open, filed with its own
10441+
// measurement: neither limb refreshes the SchemaRegistry the way
10442+
// `rollbackMetaItem` does, so a restored body is persisted but not
10443+
// yet dispatched on (#6621).
1041010444
const intent: 'override-artifact' | 'runtime-only' =
1041110445
this.isArtifactBacked(it.type, it.name) ? 'override-artifact' : 'runtime-only';
1041210446
await repo.restoreVersion(ref, it.prevVersion, {

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

Lines changed: 219 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,3 +632,222 @@ describe('#6563 — rollbackToPackageCommit inherits the per-item intent', () =>
632632
expect(fields).not.toContain('due_date');
633633
});
634634
});
635+
636+
/**
637+
* #6620 — the OTHER limb of the same loop: SOFT-REMOVE states its intent too.
638+
*
639+
* `revertCommit` has two limbs, and #6563 (above) only fixed the restore one.
640+
* The limb that undoes an artifact the commit CREATED stated its intent as a
641+
* CONSTANT — `intent: 'override-artifact'`, written into the `repo.delete(...)`
642+
* call — and `SysMetadataRepository.delete` opens with the same
643+
* `assertAllowed(ref.type, opts.intent)` gate `put` uses. So `object`, which is
644+
* not `allowOrgOverride`, was refused on the delete path exactly as it had been
645+
* on the restore path, and a commit that CREATED an object could not be
646+
* reverted either.
647+
*
648+
* That is the FIRST-BUILD undo — publish a brand-new app, then undo it — which
649+
* is the flow Studio and AI authoring produce most. Every object the commit
650+
* created stayed behind, `success` came back `false` with a populated
651+
* `failed[]`, and the package was left half-reverted: its overlay-allowed items
652+
* removed, its objects not.
653+
*
654+
* The two causes are different even though the symptom rhymes: #6563 was an
655+
* UNSTATED intent falling through to the repository's `?? 'override-artifact'`
656+
* default, this one is a literal the caller wrote down. The fix is the same
657+
* family shape — derive it per item from `isArtifactBacked`, the way the
658+
* sibling delete caller `deleteMetaItem` and the sibling revert caller
659+
* `rollbackMetaItem` both already do — so all three delete/revert callers now
660+
* agree, and the repository's gate is untouched.
661+
*/
662+
663+
/** The commit item shape for an artifact this commit CREATED (ADR-0067). */
664+
const createdItem = (name: string) => ({
665+
type: 'object', name, existedBefore: false, prevVersion: null,
666+
});
667+
668+
/** The first-build shape: authored ONCE, never edited — nothing to restore to. */
669+
async function seedCreatedObject(protocol: any, name: string, packageId?: string) {
670+
await protocol.saveMetaItem({
671+
type: 'object', name, ...(packageId ? { packageId } : {}), item: invoiceBody(name),
672+
});
673+
}
674+
675+
const storedRows = (rows: Map<string, any>, name: string) =>
676+
Array.from(rows.values()).filter((r) => r.name === name);
677+
678+
describe('#6620 — revertCommit soft-removes a runtime-CREATED `object`', () => {
679+
it('a package-bound created object reverts: revertedCount 1, failed [], row gone', async () => {
680+
const { protocol, rows, historyRows } = makeRealRepoHarness([objectCommit({
681+
id: 'cmt_new',
682+
items: [createdItem('myapp_invoice')],
683+
})]);
684+
await seedCreatedObject(protocol, 'myapp_invoice', APP_PKG);
685+
expect(storedRows(rows, 'myapp_invoice')).toHaveLength(1);
686+
687+
const res = await protocol.revertCommit({ commitId: 'cmt_new' });
688+
689+
// Pre-fix, verbatim (the issue's measurement): success false, revertedCount
690+
// 0, failedCount 1 carrying "[NOT_OVERRIDABLE] 'object' is not
691+
// allowOrgOverride in the registry.", and the row still standing.
692+
expect(res.failed).toEqual([]);
693+
expect(res.success).toBe(true);
694+
expect(res.revertedCount).toBe(1);
695+
expect(res.reverted[0]).toMatchObject({ type: 'object', name: 'myapp_invoice', action: 'removed' });
696+
expect(storedRows(rows, 'myapp_invoice')).toHaveLength(0);
697+
// Soft, not hard: ADR-0067 §5 keeps the removal recoverable, so the delete
698+
// is an append-only tombstone in history rather than a vanished lineage.
699+
const tombstone = historyRows.filter(
700+
(h) => h.name === 'myapp_invoice' && h.operation_type === 'delete',
701+
);
702+
expect(tombstone).toHaveLength(1);
703+
expect(tombstone[0].metadata).toBeNull();
704+
});
705+
706+
it('a package-LESS created object reverts identically — the binding was never the cause', async () => {
707+
const { protocol, rows } = makeRealRepoHarness([objectCommit({
708+
id: 'cmt_new_global',
709+
package_id: null,
710+
items: [createdItem('global_invoice')],
711+
})]);
712+
await seedCreatedObject(protocol, 'global_invoice');
713+
714+
const res = await protocol.revertCommit({ commitId: 'cmt_new_global' });
715+
716+
expect(res.failed).toEqual([]);
717+
expect(res.revertedCount).toBe(1);
718+
expect(storedRows(rows, 'global_invoice')).toHaveLength(0);
719+
});
720+
721+
/**
722+
* The refusal that must SURVIVE the fix — and the one case the constant got
723+
* right by accident, which is why its direction is INVERTED: it was green
724+
* before the change and is green after. It cannot go red by removing the fix,
725+
* because removing the fix refuses EVERYTHING. What it does go red on is the
726+
* wrong fix — hard-coding `'runtime-only'` in place of the old
727+
* `'override-artifact'` — which is the mistake a one-line "just make objects
728+
* work" edit would make, and which would let a revert tombstone an artifact a
729+
* code package genuinely ships.
730+
*
731+
* Staged the way a real deployment stages it (as in #6563's block): the
732+
* overlay row is authored while the name is runtime-only, and the artifact
733+
* arrives with the package that later claims it. `registerObject(body, pkg)`
734+
* with no `_provenance` is the shape `applyProtection` stamps as `'package'`,
735+
* which is what `getArtifactItem` reads and `isArtifactBacked` answers on.
736+
*
737+
* Envelope note (ADR-0112): `revertCommit` converts a per-item throw into a
738+
* `failed[]` record whose DECLARED shape is `{ type, name, error, code? }` —
739+
* no `status`. So `code` is asserted here together with the condition's own
740+
* first sentence, and the full `{ code, status }` pair belongs to the
741+
* throwing surface (`protocol-writepath-object-ownership.test.ts`), exactly
742+
* as #6563 split it.
743+
*/
744+
it('still REFUSES soft-removing an artifact-backed object: NOT_OVERRIDABLE, row kept', async () => {
745+
const { protocol, registry, rows } = makeRealRepoHarness([objectCommit({
746+
id: 'cmt_new_artifact',
747+
items: [createdItem('myapp_invoice')],
748+
})]);
749+
await seedCreatedObject(protocol, 'myapp_invoice', APP_PKG);
750+
registry.registerObject(invoiceBody('myapp_invoice') as never, APP_PKG);
751+
752+
const res = await protocol.revertCommit({ commitId: 'cmt_new_artifact' });
753+
754+
expect(res.revertedCount).toBe(0);
755+
expect(res.failedCount).toBe(1);
756+
expect(res.failed[0]).toMatchObject({
757+
type: 'object',
758+
name: 'myapp_invoice',
759+
code: 'NOT_OVERRIDABLE',
760+
});
761+
expect(res.failed[0].error).toContain(
762+
`[NOT_OVERRIDABLE] 'object' is not allowOrgOverride in the registry.`,
763+
);
764+
// Refused means refused: the artifact-backed row is still there.
765+
expect(storedRows(rows, 'myapp_invoice')).toHaveLength(1);
766+
});
767+
768+
/**
769+
* PER ITEM, not per call — the half a single-item fixture cannot see, on the
770+
* soft-remove limb this time. One commit, two created objects, opposite
771+
* verdicts: a loop that hoisted one intent for the batch (which is precisely
772+
* what the constant did) has to pick one and be wrong about the other.
773+
*/
774+
it('derives the intent PER ITEM: one created object removed, its artifact-backed neighbour refused', async () => {
775+
const { protocol, registry, rows } = makeRealRepoHarness([objectCommit({
776+
id: 'cmt_new_mixed',
777+
items: [createdItem('myapp_invoice'), createdItem('myapp_quote')],
778+
})]);
779+
await seedCreatedObject(protocol, 'myapp_invoice', APP_PKG);
780+
await seedCreatedObject(protocol, 'myapp_quote', APP_PKG);
781+
// Only the quote is claimed by a code artifact.
782+
registry.registerObject(invoiceBody('myapp_quote') as never, APP_PKG);
783+
784+
const res = await protocol.revertCommit({ commitId: 'cmt_new_mixed' });
785+
786+
expect(res.reverted).toEqual([
787+
{ type: 'object', name: 'myapp_invoice', action: 'removed' },
788+
]);
789+
expect(res.failed).toHaveLength(1);
790+
expect(res.failed[0]).toMatchObject({ name: 'myapp_quote', code: 'NOT_OVERRIDABLE' });
791+
expect(storedRows(rows, 'myapp_invoice')).toHaveLength(0);
792+
expect(storedRows(rows, 'myapp_quote')).toHaveLength(1);
793+
});
794+
795+
/**
796+
* A commit that created BOTH an overlay-allowed item and an object is the
797+
* half-reverted package the issue describes: pre-fix the view came out and
798+
* the object stayed, so `success` was `false` and the package sat in a state
799+
* neither before nor after the commit.
800+
*/
801+
it('reverts a mixed-TYPE first build whole: the view and the object both come out', async () => {
802+
const { protocol, rows } = makeRealRepoHarness([objectCommit({
803+
id: 'cmt_new_build',
804+
items: [
805+
createdItem('myapp_invoice'),
806+
{ type: 'view', name: 'myapp_case_grid', existedBefore: false, prevVersion: null },
807+
],
808+
})]);
809+
await seedCreatedObject(protocol, 'myapp_invoice', APP_PKG);
810+
await protocol.saveMetaItem({
811+
type: 'view', name: 'myapp_case_grid', packageId: APP_PKG, item: gridBody('Cases'),
812+
});
813+
814+
const res = await protocol.revertCommit({ commitId: 'cmt_new_build' });
815+
816+
expect(res.failed).toEqual([]);
817+
expect(res.success).toBe(true);
818+
expect(res.revertedCount).toBe(2);
819+
expect(storedRows(rows, 'myapp_invoice')).toHaveLength(0);
820+
expect(storedRows(rows, 'myapp_case_grid')).toHaveLength(0);
821+
});
822+
});
823+
824+
/**
825+
* #6620 — the inheritance, on the soft-remove limb. `rollbackToPackageCommit`
826+
* reverts through the SAME loop, so it carried the same constant.
827+
*
828+
* As in #6563's inheritance pin, the status cannot show the defect:
829+
* `revertCommit` turns a per-item refusal into `failed[]` instead of throwing,
830+
* so the rollback recorded the commit as reverted and answered `success: true`
831+
* while the created object was never removed. The line that goes red pre-fix is
832+
* the STORED ROW.
833+
*/
834+
describe('#6620 — rollbackToPackageCommit inherits the per-item soft-remove intent', () => {
835+
it('rolls a first build back through the loop — and the created row really went away', async () => {
836+
const { protocol, rows } = makeRealRepoHarness([
837+
objectCommit({ id: 'cmt_base', items: [], created_at: '2026-08-08T00:00:01.000Z' }),
838+
objectCommit({
839+
id: 'cmt_build',
840+
items: [createdItem('myapp_invoice')],
841+
created_at: '2026-08-08T00:00:02.000Z',
842+
}),
843+
]);
844+
await seedCreatedObject(protocol, 'myapp_invoice', APP_PKG);
845+
846+
const res = await protocol.rollbackToPackageCommit({ commitId: 'cmt_base' });
847+
848+
expect(res.revertedCommits).toEqual(['cmt_build']);
849+
expect(res.failed).toEqual([]);
850+
// `success: true` was ALREADY true pre-fix — this is the line that was not.
851+
expect(storedRows(rows, 'myapp_invoice')).toHaveLength(0);
852+
});
853+
});

0 commit comments

Comments
 (0)