Skip to content

revertCommit's soft-remove limb hard-codes intent: 'override-artifact', so a commit that CREATED an object cannot be reverted either #6620

Closed

Description

@baozhoutao

Found while implementing #6563 (the RESTORE half of the same loop). Recording only, per Prime Directive #10 — a different line with a different cause (a hard-coded literal, not an unstated default), and outside #6563's ruled file surface.

Evidence

ObjectStackProtocolImplementation.revertCommit (packages/metadata-protocol/src/protocol.ts) has two limbs. #6563 fixes the existedBefore one. The OTHER limb — an artifact the commit CREATED, which the revert soft-removes — states its intent as a constant:

if (!it.existedBefore) {
    // Created by this commit -> soft-remove (metadata only; table stays).
    if (current) {
        await repo.delete(ref, {
            parentVersion: current.hash,
            actor,
            source: 'protocol.revertCommit',
            intent: 'override-artifact',
            state: 'active',
        });
    }
    reverted.push({ type: it.type, name: it.name, action: 'removed' });
}

SysMetadataRepository.delete opens with this.assertAllowed(ref.type, opts.intent) — the same gate put uses — which refuses any type whose registry entry is not allowOrgOverride. object is exactly such a type, so a commit that created an object cannot be undone.

Measured on origin/main @ e39dd66 against a real SysMetadataRepository over an in-memory sys_metadata / sys_metadata_history / sys_metadata_commit double (the #6215 harness in packages/objectql/src/protocol-commit-history.test.ts): an object created once through saveMetaItem under a Studio package workspace, recorded in a commit as existedBefore: false, then reverted through revertCommit:

{
 "success": false, "revertedCount": 0, "failedCount": 1, "reverted": [],
 "failed": [{ "type": "object", "name": "myapp_invoice",
   "error": "[NOT_OVERRIDABLE] 'object' is not allowOrgOverride in the registry. ...",
   "code": "NOT_OVERRIDABLE" }]
}

…and the sys_metadata row is still there afterwards (1 row remaining). The same measurement re-run on #6563's branch is byte-identical: fixing the restore limb does not touch this one.

Why it matters

This is the first-build revert — the AI/Studio flow that publishes a brand-new app and then undoes it. Every object the commit created stays behind, success is false with a populated failed[], and the package is left half-reverted (its overlay-allowed items removed, its objects not). rollbackToPackageCommit reverts through the same loop, so it inherits this.

Suggested direction (not a ruling)

Same shape as #6563's ruling, applied to the other limb: derive the intent from the artifact per item (isArtifactBacked -> 'override-artifact' : 'runtime-only'), exactly as the sibling caller rollbackMetaItem does, rather than stating a constant. A genuinely artifact-backed item must still be refused. The repository's gate is right; the caller is what never says which of the two cases it is.

Note the delete-limb equivalent of #6563's premise check is worth re-running before implementing: deleteMetaItem (the per-item delete path) should be read first to see which intent it derives, so the two delete callers agree.

Related: #6563 (the restore limb, same loop), ADR-0067, ADR-0005.

Activity

  1. added
    pm:queueReady for the PM dispatch loop
    target:v17Release blocker for v17 — on the board until fixed, dropped as no longer valid, or accepted for GA
    on Aug 8, 2026
  2. os-zhuang commented on Aug 8, 2026

    @os-zhuang
    Contributor

    Triage: pm:queue + domain:metadata + target:v17.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  3. self-assigned this
    on Aug 8, 2026
  4. os-zhuang commented on Aug 8, 2026

    @os-zhuang
    Contributor

    Claim: PM loop round 2 (domain:metadata seat, sticker #6367)
    Session: session_01W6bLax4KMrSfnE1ydFU8Dw
    Branch: claude/issue-6620-revert-soft-remove-intent
    Worktree: objectstack-issue-6620
    Domain: domain:metadata
    File surface: packages/metadata-protocol/src/protocol.ts (revertCommit soft-remove limb only, ~:10159) + the #6215/#6642 pin suite (packages/objectql/src/protocol-commit-history.test.ts, extend in place) + one .changeset/*.md
    Serial constraints cleared: sibling limb landed as PR #6642 (MERGED, same function — base already carries it); #6621 targets the same revertCommit region and is deliberately held for a later round (same-file/same-region serialization); parallel round-2 cards #6602 (:7582/getMetaItems) / #6418 (:2519 ensureOverlayIndex) are disjoint regions, and rounds-1 PRs still landing (#6687/#6700/#6708/#6709) touch other regions. Premise-check note from the card carried verbatim: read deleteMetaItem's intent derivation first so the two delete callers agree.


    Generated by Claude Code

  5. os-zhuang commented on Aug 8, 2026

    @os-zhuang
    Contributor

    Review verdict: ACCEPT — PM domain:metadata seat, session session_01W6bLax4KMrSfnE1ydFU8Dw.

    PR #6768: the soft-remove limb now derives intent per item from isArtifactBacked, so all four delete/revert callers (deleteMetaItem, rollbackMetaItem, both revertCommit limbs) agree; the repository gate is untouched and an artifact-backed item is still refused with NOT_OVERRIDABLE (pinned, with #6563's envelope split — code in failed[], full pair at the throwing surface). Highlights accepted on review:

    • Premise check 2 answered by measurement: deleteMetaItem/rollbackMetaItem already derive identically; the one deliberate difference (raw it.type, no re-canonicalization) is documented.
    • Reverse verification in BOTH threatening directions: unfixed constant → predicted 5 red / measured 5 red verbatim; wrong-fix constant ('runtime-only') → the refusal and per-item pins go red. The honest note that the artifact-refusal pin cannot go before-red (the old constant was accidentally right there) is exactly the right shape of reporting.
    • The rollback-inheritance pin asserts the STORED ROW, not success — which was already true on the defect.
    • fix(metadata-protocol): revertCommit states its write intent per item (#6563) #6642's now-false neighbour note in the restore limb is rewritten in the same region.

    #6621 pricing recorded on its card. Next: CI convergence, then ready + merge queue.


    Generated by Claude Code

  6. os-zhuang commented on Aug 8, 2026

    @os-zhuang
    Contributor

    Landing confirmed — PR #6768 merged as 75e6871bf (verified on origin/main). The first-build undo now reverts created objects; all four delete/revert callers derive the write intent per item. This clears the target:v17 entry this card carried. #6621 is now dispatchable on a base that includes this commit (its pricing note stands). — domain:metadata PM, session session_01W6bLax4KMrSfnE1ydFU8Dw


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

pm:dispatchedDispatched to a dev agent by /pm-dispatchpm:queueReady for the PM dispatch looptarget:v17Release blocker for v17 — on the board until fixed, dropped as no longer valid, or accepted for GA

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions