fix(publication): stop retrying reviews superseded by newer results - #1249
fix(publication): stop retrying reviews superseded by newer results#1249vincentkoc wants to merge 1 commit into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs maintainer review before merge. Reviewed August 25, 2026, 8:32 PM ET / August 26, 2026, 00:32 UTC. ClawSweeper reviewWhat this changesThe branch classifies an exact-review event as terminally superseded only when a newer durable review tuple for the same revision is fully verified, preventing stale publication retries and GitHub mutations. Merge readinessKeep open. This member-authored draft adds a narrowly scoped, fail-closed terminal path that current main does not contain; no blocking correctness defect was found, but the automation-path proof should be recorded in the repository-required container format before landing. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — step 1 Assertions:
How this fits togetherExact-review publication applies a review artifact, compares it with the live durable review comment, and then either publishes state or completes the queue item. The changed guard turns one verified stale-comment case into a terminal queue outcome so the publisher releases its lease without publishing obsolete state. flowchart LR
A[Exact review artifact] --> B[Apply-time review guard]
C[Live durable review comment] --> B
B --> D{Newer tuple fully verified?}
D -- Yes --> E[Superseded completion]
D -- No --> F[Existing apply or retry path]
E --> G[Batch lease release]
F --> H[Publication and state update]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain the strict same-revision, strictly newer lease-ID rule and land it only with a recorded Crabbox local-container trace showing both terminal supersession and fail-closed rejection of an incomplete or mixed tuple. Do we have a high-confidence way to reproduce the issue? No high-confidence live reproduction was established in this read-only review; source and focused fixtures define a controlled stale-versus-newer durable-tuple scenario with a clear expected terminal outcome. Is this the best way to solve the issue? Yes, conditionally: using the durable tuple rather than timestamps alone is the narrowest fail-closed solution, provided its production-path proof covers invalid and mixed tuple cases. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against afe976209aa5. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
What Problem This Solves
Fixes an issue where exact-review publishers would retry an older review artifact forever after a newer durable review for the same revision had already become authoritative.
Why This Change Was Made
The apply lane now proves the complete durable review tuple before classifying an event as superseded: trusted non-placeholder lease owners, positive lease comment IDs, the same reviewed revision, and a strictly newer live lease comment ID. Legacy tuple-less artifacts still requeue the latest revision, while incomplete, mixed, or otherwise unverified evidence fails closed.
Verified supersession exits before state publication or any new GitHub review mutation. It emits
completion_kind=supersededwithreason_code=remote_newer_tuple; the existing workflow then releases the publisher-owned review lease.User Impact
Operators should no longer see exact-review publication starvation when an older event loses a race to a newer durable review on the same revision. Ambiguous events remain retryable or fail closed rather than being silently discarded.
OpenClaw Bay Impact
OpenClaw Bay is unaffected. This change uses the existing
supersededlifecycle outcome andremote_newer_tuplereason already understood by the queue and Bay projections; no observer schema, route, or display contract changes.Documentation Impact
Reviewed
docs/limits.md, especially the exact-review publication lifecycle. Its existing contract already states that newer remote tuples complete as terminal superseded outcomes without GitHub mutation, so no documentation update is required.Evidence
b44bb38ceefb785bcc817fc6c97964588956bd3dorigin/main: no actionable regressions; focused typechecks, 66 tests, lint, formatting, and diff checks passed.completion_kind=superseded,reason_code=remote_newer_tuple, no publication command, and{ "kind": "superseded" }batch output./publication-batch-resultsrequest and terminalsupersededcompletion.registry.npmjs.orgwhile installingpnpm@10.33.0; changed-surface proof and all non-network gates passed.Real Behavior Proof
Claim: A publisher that observes one exact, verified newer durable review tuple terminates the stale event as superseded without publishing state or mutating GitHub.
Exercised surface:
apply-decisionsproof emission,publish-event-result, exact-review batch commit/completion, and existing lease-release routing.Scenario: An issue report with durable lease tuple
old-review-owner/9100is overtaken after apply preflight by a trusted live review tuplenew-review-owner/9200on the same source revision.Commands and environment:
The publisher proof runs the compiled
dist/repair/publish-event-result.jswithEXACT_EVENT_PUBLICATION=true, an isolated work root, a fake ClawSweeper CLI, and captured GitHub Actions outputs.Observable result: The apply lane emits
newerReviewTupleVerified: truewith zero external mutation calls. The publisher exits successfully withcompletion_kind=supersededandreason_code=remote_newer_tuple, invokes onlyapply-artifactsandapply-decisions, and does not enter state publication. Batch completion recordsterminal_outcome=supersededwithout posting a publication mutation.Artifact or trace: Assertions are durable in
test/apply-label-sync.test.ts,test/repair/event-apply-proof.test.ts,test/repair/publish-event-result-superseded.test.ts, andtest/repair/exact-review-batch-cli.test.ts.Limits: GitHub and the queue are controlled local doubles; the proof does not dispatch a live publisher recovery, deploy a Worker, or mutate a real repository.