Skip to content

fix(metadata-protocol): revertCommit's soft-remove limb states its write intent per item, so a commit that CREATED an object can be reverted - #6768

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-6620-revert-soft-remove-intent
Aug 8, 2026
Merged

fix(metadata-protocol): revertCommit's soft-remove limb states its write intent per item, so a commit that CREATED an object can be reverted#6768
os-zhuang merged 2 commits into
mainfrom
claude/issue-6620-revert-soft-remove-intent

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes #6620

What was wrong

ObjectStackProtocolImplementation.revertCommit has two limbs. PR #6642 (#6563) fixed the one that RESTORES an edited artifact, where the intent was unstated and fell through to restoreVersion's ?? 'override-artifact' default. The other limb — an artifact the commit CREATED, which the revert soft-removes — stated the same intent as a literal constant:

await repo.delete(ref, {
    parentVersion: current.hash,
    actor,
    source: 'protocol.revertCommit',
    intent: 'override-artifact',   // <- a constant, not a derivation
    state: 'active',
});

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

[NOT_OVERRIDABLE] 'object' is not allowOrgOverride in the registry.
Overlay-allowed: view, page, dashboard, app, action, report, dataset, translation,
email_template, book, permission, position, tool, skill.

This is the first-build undo — the Studio / AI flow that publishes a brand-new app and then undoes it. Every object the commit created stayed behind, the call answered success: false with a populated failed[], and the package was left half-reverted: its overlay-allowed items removed, its objects not. 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 created object was untouched.

Two different causes, one rhyming symptom: #6563 was an unstated intent, #6620 is a literal the caller wrote down.

The fix

The limb derives the intent per item from isArtifactBacked, the way the sibling callers already do. The repository's gate and default are untouched.

Premise check 2 — which delete caller was mirrored, and why

The issue asked that deleteMetaItem's derivation be read first so the two delete callers agree. Read on the merge base:

caller line derivation
deleteMetaItem protocol.ts:10716 artifactBacked ? 'override-artifact' : 'runtime-only' (on singularTypeForRepo)
rollbackMetaItem protocol.ts:10395 artifactBacked ? 'override-artifact' : 'runtime-only' (on singularType)
revertCommit restore limb (#6642) protocol.ts:10245 this.isArtifactBacked(it.type, it.name) ? … : 'runtime-only'

The two delete callers already agreed with each other and with the restore limb — there was nothing to reconcile, so mirroring is unambiguous. The one deliberate difference from deleteMetaItem is the type spelling: deleteMetaItem folds through canonicalizeMetaRequestType / PLURAL_TO_SINGULAR because it takes a caller-supplied request.type (#4432), while revertCommit reads it.type off a stored commit item and already uses that same raw it.type to build ref. Introducing a second spelling for one of the two facts inside one loop iteration would be the actual hazard, so this follows the region's existing convention (identical to #6642's restore limb). Canonicalizing the whole loop is a separate question and is not in this PR's ruled file surface.

Also updated: the note #6642 left in the restore limb saying the soft-remove limb "still cannot be reverted (#6620)". That sentence is now false, so it is rewritten rather than left to mislead the next reader; the still-open #6621 half of that note is kept verbatim.

Tests

Extended packages/objectql/src/protocol-commit-history.test.ts in place (the #6215 / #6642 pin suite, same harness, same real SysMetadataRepository over the in-memory sys_metadata / sys_metadata_history / sys_metadata_commit double). Six new cases:

  1. a package-bound created object reverts — revertedCount 1, failed [], row gone, and an append-only operation_type: 'delete' tombstone with metadata: null (ADR-0067 §5: soft, recoverable, not a vanished lineage)
  2. a package-less created object reverts identically — the binding was never the cause
  3. an artifact-backed created item is STILL refused: code: 'NOT_OVERRIDABLE' plus the condition's own first sentence, row kept
  4. per item, not per call: one commit, two created objects, opposite verdicts
  5. a mixed-type first build (created object + created view) reverts whole — the half-reverted package the issue describes
  6. rollbackToPackageCommit inherits it through the same loop

Envelope split follows #6642: revertCommit converts a per-item throw into a failed[] record whose declared shape is { type, name, error, code? } with no status, so code + the message's first sentence are asserted here, and the full { code, status } pair belongs to the throwing surface.

Reverse verification — and one honest inversion

Directions were predicted before running, and one of them is not before-green/after-red. Reporting it as measured rather than forcing the template:

pin old constant 'override-artifact' wrong fix: constant 'runtime-only' this PR
1, 2, 4, 5, 6 RED green green
3 (artifact-backed refusal) green RED green

Pin 3 cannot go red by removing the fix, because removing the fix refuses everything — the constant was accidentally right for exactly this one case. Its real red direction is the wrong fix, i.e. the one-line "just make objects work" edit that swaps one constant for the other and would let a revert tombstone an artifact a code package genuinely ships. Both directions were run: 5 red / 18 pass on the old constant, 2 red / 21 pass on the inverted constant, 23/23 only on the per-item derivation. A pin that reads as coverage but cannot go red in any direction would not be coverage; this one goes red in the direction that actually threatens it.

Commands

pnpm --filter @objectstack/metadata-protocol --filter @objectstack/objectql test
  metadata-protocol   Test Files  60 passed (60)    Tests   671 passed (671)
  objectql            Test Files 149 passed (149)   Tests  2557 passed (2557)

pnpm --filter @objectstack/objectql typecheck   -> tsc --noEmit, clean
  (@objectstack/metadata-protocol has no `typecheck` script; it is a measured
   ledger entry, and `pnpm check:type-check-coverage` passes unchanged)

pnpm lint  -> clean

Every check:* enumerated from .github/workflows/lint.yml's ESLint job was run one by one, all pass: slot-lookup, query-options-erasure, verify-stand-in, nul-bytes, doc-authoring, docs-audit-scope, role-word, quick-reference-counts, adr-anchors, org-identifier, authz-resolver, service-providers, route-envelope, error-code-casing, wildcard-fallthrough, meta-type-normalized, init-service-contract, durability-log-level, startup-registry-verdict, objectui-changeset, release-notes, release-body, node-version, workflow-status-functions, shard-attestation, published-files, engine-double-contract, kernel-hook-pairs, resume-authority-declared, driver-memory-census, merge-driver, spec-parsed-alias. Plus from the type-check job: type-check-coverage, driver-conformance, stall-guard, skill-frame-sync, skill-compatibility, and spec check:generated --reconcile-only.

origin/main moved 12 commits during the work (including #6708 / #6709 on other regions of this same file); merged clean, reinstalled, rebuilt, and the numbers above are the post-merge run.

No new fake engine was added — the harness reuses the existing double, which already routes both write verbs through assertEngineDeleteDispatch / assertEngineUpdateDispatch.

Scope

packages/metadata-protocol/src/protocol.ts (revertCommit soft-remove limb only) + packages/objectql/src/protocol-commit-history.test.ts + one changeset. #6621 (registry refresh after revert) targets the same region and was deliberately not absorbed.


中文摘要

revertCommit 的两条支路里,#6642 修好了「恢复被编辑的产物」那条;另一条——「软删除本次 commit 新建的产物」——把写入 intent 写成了常量 'override-artifact'。仓库层 delete 开头就是 assertAllowed(ref.type, opts.intent),会拒绝一切非 allowOrgOverride 的类型,object 正是其一,于是新建了对象的 commit 根本无法回滚:这正是「发布一个全新应用再撤销」的首次构建撤销路径,对象全部留在原地,success: false,包处于半回滚状态;rollbackToPackageCommit 走同一循环,症状更隐蔽——它照样返回 success: true

现在按每一项isArtifactBacked 推导 intent,与 deleteMetaItemrollbackMetaItem 以及 #6642 的恢复支路完全一致(三处调用方口径统一)。仓库层的 gate 一行未改:真正由代码包提供的产物仍然解析为 'override-artifact',仍然被 NOT_OVERRIDABLE 拒绝,这一条也一并钉住。

反向验证有一条方向是反的,如实记录而非套模板:artifact-backed 拒绝那条用例修前修后都绿——因为修改前是「全都拒绝」,它恰好被常量蒙对了;它真正会变红的方向是错误的修法(把常量换成 'runtime-only')。两个方向都实测过:旧常量 5 红,反向常量 2 红,只有 per-item 推导 23/23 全绿。

🤖 Generated with Claude Code

https://claude.ai/code/session_01W6bLax4KMrSfnE1ydFU8Dw


Generated by Claude Code

claude added 2 commits August 8, 2026 14:21
…ite intent per item (#6620)

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.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01W6bLax4KMrSfnE1ydFU8Dw
@vercel

vercel Bot commented Aug 8, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 8, 2026 2:43pm

Request Review

@github-actions github-actions Bot added the size/m label Aug 8, 2026
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol.

4 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via @objectstack/metadata-protocol)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/metadata-protocol)
  • content/docs/protocol/kernel/http-protocol.mdx (via @objectstack/metadata-protocol)
  • content/docs/releases/v9.mdx (via @objectstack/metadata-protocol)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

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

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants