Skip to content

fix(metadata-protocol): a package rollback carries every item a revert step refused, and answers success: false - #27957

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-27868-rollback-carries-item-failures
Oct 30, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-27868-rollback-carries-item-failures

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

domain:server#2 · os-sales · session_01L6bhgUCizoYWQVJWjhdR6F

Requested by Sales · project thread

Fixes #27868

Clause-②: no
Metadata-protocol: none
Skills-docs: none — the answer's declared shape and its success contract do not change, and no skill or doc describes the rollback's failed[] population

维护者速读

  • 改了什么:rollbackToPackageCommit 现在把每一步 revertCommit 返回的逐项拒绝(例如非规范存储类型的恢复被拒)写进回滚自己的 failed[],有任何一项被拒就答 success: false。
  • 为什么改:之前只有抛错才进 failed[],逐项拒绝被丢弃,管理员回滚一个包时被告知成功,实际上有的条目原样未动,包处于半回滚状态且毫无提示。
  • 风险与回滚:只改 rollbackToPackageCommit 一个方法内部,答复形状不变(仍是 { commitId, error });回滚继续越过被拒条目的行为不变。出问题直接 revert 本 PR 即可。
  • 先看:packages/metadata-protocol/src/protocol.ts(rollbackToPackageCommit 循环里新增的 for (const item of step.failed)),然后 packages/metadata-protocol/src/protocol.rollback-carries-item-failures.test.ts。
  • 两分钟验证:pnpm --filter @objectstack/metadata-protocol exec vitest run src/protocol.rollback-carries-item-failures.test.ts,预期 3 个用例全部通过。
  • 需要你做的:无,席位落地。

What changes

rollbackToPackageCommit (POST /packages/:id/rollback) runs revertCommit once per newer apply commit. revertCommit reports an item it refuses in its returned per-item failed[] and does not throw; the rollback read only thrown errors, so a package left half rolled back answered success: true with an empty failed[].

Now each returned per-item failure becomes one entry in the rollback's own failed[]:

  • commitId: the commit the item belongs to;
  • error: Item 'TYPE/NAME' was not reverted: followed by the step's own failed[].error, which is already client-facing (the P11 withhold in revertCommit has run on it).

success stays failed.length === 0, so any refused item makes it false.

Shape. The declared answer, RollbackToPackageCommitResponseSchema (packages/spec/src/api/package-lifecycle.zod.ts), types each failed[] entry as { commitId, error }, with no item fields. So the item is named inside error, and no spec edit is made. The step's per-item code (for example STORED_TYPE_NOT_CANONICAL) has no field in that shape and is not carried. See Acceptance notes.

What stays the same.

  • A refused item stops neither its commit nor the rollback. The step still reverts the item's neighbours and records its revert commit, so that commit is also listed in revertedCommits. A partly reverted commit therefore appears in both lists.
  • A step that throws still lands as one { commitId, error } entry, and the rollback goes on to the next commit.
  • The HTTP route is unchanged: it still answers 200 with the result as data.

Clause-②: no, and why it is not a narrowing. The answer's shape is unchanged. What changes is success becoming false for a request that used to read true. The published contract already says this is the right answer: RollbackToPackageCommitResponseSchema.success is described as "True exactly when nothing failed", and the sibling RevertPackageCommitResponseSchema already answers success: false for the same refused item. The change brings the implementation back to its declared contract. It does not narrow an accept set or a published shape: every request accepted before is still accepted and gets the same shape. Hence plain no, with no (narrowing) arm, no ! and no BREAKING note.

Tests

New packages/metadata-protocol/src/protocol.rollback-carries-item-failures.test.ts. It runs through the real protocol and the real SysMetadataRepository, with an engine double that also serves the commit ledger, so listCommits plans the rollback itself. It is the same harness shape as protocol.revert-stored-type-canonical.test.ts. The package history is: target c0; c1 creates a view; c2 edits view/grid, edits fields/a.b (stored under the non-canonical type, which revertCommit refuses with STORED_TYPE_NOT_CANONICAL) and creates a view.

  1. Refused item. success: false. failed has exactly one entry with keys commitId and error: commit c2, an error that names fields/a.b and quotes the step's refusal text. revertedCommits is c1 and c2. Both created views are removed, view/grid is back to V1, and fields/a.b stays at V2.
  2. Control, all clean. { success: true, revertedCommits: [c1, c2], failed: [] }.
  3. Control, thrown step. c2 is listed but cannot be read. The result is failed: [{ commitId: 'c2', error: "No commit 'c2'." }], and c1 is still reverted.

The test imports ./protocol.js from src, not through a package exports path, so no dist rebuild sits between the mutation and the run.

Ablation. Through scripts/ablation-replace.mjs on packages/metadata-protocol/src/protocol.ts, the anchor for (const item of step.failed) { is replaced by for (const item of [] as typeof step.failed) {, which drops the carried failures and gives the pre-fix behaviour. The predicted result was case 1 red and both controls green. That is what happened.

Ablation output
ablation-replace: anchor   "for (const item of step.failed) {"  x1 (before)
ablation-replace:   anchor hits 1 time(s), as declared
ablation-replace: anchor   x1 -> x0
ablation-replace: replace  "for (const item of [] as typeof step.failed) {"  x0 -> x1
ablation-replace: blob     f0181b90b1fa28b115a4ea59a4b8e4821a7d1f0b -> 9780b8b5cb0d132a5936d879d9eb7fc7ff9fe1d4
ablation-replace: ok mutation landed: anchor 1 -> 0, blob f0181b90b1fa -> 9780b8b5cb0d
     x answers `success: false` and names the commit, the item and the refusal text 30ms
AssertionError: expected true to be false // Object.is equality
 Test Files  1 failed (1)
      Tests  1 failed | 2 passed (3)
ablation-replace: command exited 1
ablation-replace:   blob after restore  f0181b90b1fa28b115a4ea59a4b8e4821a7d1f0b
ablation-replace:   blob at HEAD        f0181b90b1fa28b115a4ea59a4b8e4821a7d1f0b
ablation-replace:   git diff HEAD       empty
ablation-replace: ok restored: blob == HEAD (f0181b90b1fa) and `git diff HEAD` is empty
Package suite, typecheck, lint
pnpm --filter @objectstack/metadata-protocol test           (at c47a8170)  exit 0
  Test Files  283 passed | 4 skipped (287)
       Tests  21846 passed | 26 skipped (21872)
vitest run on the new pin + batch-verb-driver-text + commit-timeline-instant-order (at the final test-file content, 233b1f0e)  exit 0
  Test Files  3 passed (3)
       Tests  34 passed (34)
pnpm --filter @objectstack/metadata-protocol typecheck      (at 233b1f0e)  exit 0
  tsc --listFiles names the new test file (1 hit), so it is compiled
pnpm exec eslint packages/metadata-protocol --no-inline-config            exit 0

The commit after c47a817 changes only the new test's engine double (it holds limit and refuses unknown combinators) and the engine-double ledger. The three affected test files and the typecheck were run again on that content.

Acceptance notes

  • The declared item shape is coarser than the step's. RollbackToPackageCommitResponseSchema.failed[] has no type, name or code, so a client reads the refused item from error text and loses the step's machine code. The schema's failed description, "Every commit the rollback could not revert.", now also covers a partly reverted commit's refused items. Giving the rollback entry structured type / name / code fields would be a packages/spec change (a widening, so Clause-②: yes) and is out of this card's surface. carrier: none.
  • check:engine-double-contract ledger. scripts/engine-double-contract.pinned.json gains the new test's engine double, written by node scripts/check-engine-double-contract.mjs --write as the gate prescribes. The new double holds the caller's limit (check:objectql-double-limit) and refuses any combinator other than $or (check:where-matcher).
  • In-flight intersection. No open PR touches packages/metadata-protocol/src/protocol.ts. All 27 open PRs were read, including all 721 files of the Version Packages PR. The one in-flight branch in the same package, claude/issue-27829-searchfields-credential-refused (no PR yet), does not touch protocol.ts. git merge-tree --write-tree HEAD against it exits 0.

Verification

Head 233b1f0e, merged with origin/main (0a370917).

The union was run after the final commit, at 233b1f0e: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derives 69 families. All 69 were run, each exit code captured before any pipe, and every one exited 0. Reconciliation: dispatch-gates --ran answers "69 derived famil(ies) accounted for — 69 run, 0 NOT-MEASURED (a DERIVED zero — all 69 recorded an exit code and none of them is 3)".

At c47a8170 the first union had four reds: check:engine-double-contract (ledger not updated), check:objectql-double-limit (the double was limit-blind), check:where-matcher (the double had no combinator refusal), and check:dual-build-cjs-loads / check:lean-entry-closure (exit 3, prerequisite not met: dist was missing). The first three were fixed in 233b1f0e. The last two were measured after the build and are green.

The CI families are cited from this PR's check-runs once they report.


Generated by Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_01L6bhgUCizoYWQVJWjhdR6F

… step's refused items

Claude-Session: https://claude.ai/code/session_01L6bhgUCizoYWQVJWjhdR6F
Co-authored-by: Claude <noreply@anthropic.com>
…t step refused, and answers success: false

Claude-Session: https://claude.ai/code/session_01L6bhgUCizoYWQVJWjhdR6F
Co-authored-by: Claude <noreply@anthropic.com>
…es refused items

Claude-Session: https://claude.ai/code/session_01L6bhgUCizoYWQVJWjhdR6F
Co-authored-by: Claude <noreply@anthropic.com>
…ound, refuses combinators, and is in the engine-double ledger

Claude-Session: https://claude.ai/code/session_01L6bhgUCizoYWQVJWjhdR6F
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol, touching 2 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/concepts/metadata-lifecycle.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))

⛔ 2 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))
  • content/docs/releases/v17/17-0.mdx (via ObjectStackProtocolImplementation (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 58 of 210 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 10 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json ed0e71db94a73acc02512ff2ee0655106d02e585 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 42f73054112a33d05b805fb3602fdeb921d3f0d8 — the merge of head 233b1f0eb9c66199967072c6c18c20453e6a7fb9 into base ed0e71db94a73acc02512ff2ee0655106d02e585, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 42f73054112a33d05b805fb3602fdeb921d3f0d8 && git checkout 42f73054112a33d05b805fb3602fdeb921d3f0d8
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ed0e71db94a73acc02512ff2ee0655106d02e585 233b1f0eb9c66199967072c6c18c20453e6a7fb9 && git checkout -B drift-repro ed0e71db94a73acc02512ff2ee0655106d02e585 && git merge --no-ff 233b1f0eb9c66199967072c6c18c20453e6a7fb9

node scripts/docs-audit/affected-docs.mjs --json ed0e71db94a73acc02512ff2ee0655106d02e585

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs ed0e71db94a73acc02512ff2ee0655106d02e585 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-sales · session_01L6bhgUCizoYWQVJWjhdR6F

维护者速读(席位终稿)

  • 改了什么:按提交回滚整个包(POST /packages/:id/rollback)时,某一步拒收的条目现在会逐条进 failed[],error 里点明是哪个条目;只要有拒收,success 就是 false。
  • 为什么改:每一步的 revertCommit 把拒收条目放在它自己的返回值里,不抛错。回滚只看抛错,所以包回滚了一半,仍然回 success: true、failed[] 为空。
  • 风险与回滚:答复形状不变。只有“部分回滚”的答复从 success: true 变成 false,这正是声明里写的“没有任何失败时才为 true”。回滚过程照旧:越过拒收条目继续执行,部分回滚的提交仍列在 revertedCommits 里。回滚 = revert 本 PR。
  • 先看:packages/metadata-protocol/src/protocol.ts 的 rollbackToPackageCommit(+18/−1)。
  • 两分钟验证:pnpm --filter @objectstack/metadata-protocol exec vitest run src/protocol.rollback-carries-item-failures.test.ts。
  • 需要你做的:无。席位落地。

席位意见: ACCEPT。实现与已声明的契约一致,改动小,且只在一个函数内。

席位核对

ACCEPT · head 233b1f0eb9c66199967072c6c18c20453e6a7fb9 · 2026-10-30T05:36Z

  • 形态: 4 files (+330/−1), with production code +18/−1, all inside rollbackToPackageCommit and its JSDoc. The rest is a new test, the changeset and engine-double ledger rows. No packages/spec. Tier S.
  • Clause-②: no, Metadata-protocol: none. The declared answer (RollbackToPackageCommitResponseSchema.success, "True exactly when nothing failed") already promised what this head returns, so this is conformance, not a narrowing: no request is refused that was accepted before.
  • 前提实测: at origin/main, revertCommit returns failed: Array<{ type, name, error, code? }> and does not throw. The rollback loop discarded that value. The seat read this.
  • 抽查读数: the seat read the diff. Each step.failed item is pushed as { commitId, error: "Item 'TYPE/NAME' was not reverted: …" }, and the return is success: failed.length === 0. The runtime route (domains/packages.ts) passes the result through unchanged. No ui/ or CLI reader branches on revertedCommits. @objectstack/client only types the answer.
  • 门禁 check-runs: 52 on this head: 37 success, 15 skipped, 0 other (read 2026-10-30T05:36Z); mergeable_state: clean. Derived gates 69/69 exit 0, 0 NOT-MEASURED.
  • 接受的偏差: scripts/engine-double-contract.pinned.json added to the file surface (claim amendment 6515484009; rows for the new test only). Red-first rests on the ablation leg: 1 red, 2 controls green, blob restored. The full package suite ran at c47a8170; the final commit changes only the test double and the ledger, and its affected files, typecheck, eslint and the gate union were rerun.
  • 范围外去向: the dev's class (b) note: failed[] declares only { commitId, error }, so a refused item's type/name/code travel only as text. Carrying them as fields would be a spec widening, and nothing asks for it today. Not filed; recorded here.
  • 散文面: the changeset and the JSDoc, read by the seat.
  • 落地条款: relay pr_ready + automerge_enable through the merge queue.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 30, 2026 05:37
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 30, 2026 05:38
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 30, 2026
Merged via the queue into main with commit d5951c8 Oct 30, 2026
53 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-27868-rollback-carries-item-failures branch October 30, 2026 06:18
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.

metadata-protocol: rollbackToPackageCommit drops revertCommit's per-item failed[], so an item the restore refuses is reported as a successful rollback

1 participant