fix(publication): stop retrying verified stale review artifacts - #1250
fix(publication): stop retrying verified stale review artifacts#1250vincentkoc 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 PR terminalizes a stale exact-review artifact only when a newer durable review lease is strictly proven, preventing an obsolete artifact from being retried or published. Merge readinessKeep open for normal maintainer review: this active MEMBER-authored PR has a narrow, source-backed repair, strong local runtime proof, and no actionable correctness finding. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — step 1 Assertions:
How this fits togetherThe repair publication lane runs apply results against an exact review artifact, then either plans publication or records a terminal lifecycle outcome. This change consumes the apply result and sends a verified stale artifact to the queue’s superseded completion path before any new mutation is planned. flowchart LR
A[Exact review artifact] --> B[Apply result]
B --> C[Durable tuple validation]
C -->|Strictly newer live lease| D[Superseded completion]
C -->|Legacy or ambiguous result| E[Existing retry or failure path]
D --> F[Batch lifecycle record]
E --> G[Publication planning]
F --> H[No GitHub mutation]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep terminal completion limited to the strict positive, ordered durable-lease proof and preserve the existing retry or failure paths for every legacy or ambiguous result. Do we have a high-confidence way to reproduce the issue? Yes, by source inspection: current main treats the verified stale-comment action as neither applied nor requeueable unless it is legacy tupleless, leading the publisher to throw; the PR body also supplies an after-fix built-runtime fixture run. Is this the best way to solve the issue? Yes. Restricting terminal completion to one strict, ordered durable-lease proof is narrower and safer than treating every stale-comment result as superseded. 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:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
What Problem This Solves
Resolves a problem where the publication worker would retry an obsolete exact-review artifact after apply-time verification found that a strictly newer durable review tuple was already live.
Why This Change Was Made
The publisher now terminalizes only one exact
skipped_stale_review_comment_syncresult whose canonical reason contains safe positive durable lease comment IDs withlive > report. Legacy tupleless artifacts still requeue the latest revision. Equal, reversed, malformed, mixed, or otherwise unverified results fail closed.The terminal path exits before mutation planning or publication, emits batch kind
superseded, and reportscompletion_kind=supersededwithreason_code=remote_newer_tuple.User Impact
Operators no longer spend publication retries on a review artifact that has been proven obsolete by a newer durable review. The newer live review remains authoritative, and no replacement review or GitHub mutation is started.
OpenClaw Bay Impact
Bay is unaffected. This change uses the existing
supersededterminal lifecycle and reason code; it does not add or change a Bay data contract, navigation surface, or action.Documentation Impact
No documentation change is required. The public workflow and operator contract are unchanged, and
CHANGELOG.mdis release-owned for normal repair work.Evidence
25565772df3c5dd6a414ed5132cff5a4bc6580daevent-apply-proofandstale-event-dispositiontests passed.git diff --check; privacy scan.+58/-20, net+38. The growth implements a fail-closed lifecycle invariant: strict durable tuple proof plus terminal completion before mutation planning. Tests are+69/-2.Real Behavior Proof
Claim: A verified strictly newer live durable tuple completes the stale artifact as superseded without GitHub mutation or a new review, while legacy tupleless, ambiguous/mixed, and guarded-open paths retain their existing behavior.
Exercised surface: The built
dist/repair/publish-event-result.jsruntime, including apply-artifact/apply-decision result ingestion and batch outcome/output emission.Scenario and environment: Node 26.7.0, isolated temporary record store, owned fake apply CLI, exact event publication enabled, batch mutation output enabled, no GitHub credentials or API calls.
Command: A bounded
node --input-type=moduleharness spawned the built publisher for four fixtures.Observed result:
{ "status": "passed", "superseded": { "exit": 0, "batch_kind": "superseded", "completion_kind": "superseded", "reason_code": "remote_newer_tuple", "apply_phases": ["apply-artifacts", "apply-decisions"] }, "legacy": { "exit": 0, "batch_kind": "eligible", "requeue_latest": true }, "mixed": { "exit": 1, "batch_kind": "permanent_failure", "reason_code": "unknown_failure" }, "guarded": { "exit": 0, "batch_kind": "eligible", "guarded_open_action": "skipped_protected_label" }, "github_mutations": 0, "new_reviews": 0 }Artifact or trace: The harness asserted the emitted batch JSON, GitHub output file, process exit status, and invoked apply phases, then removed its owned temporary fixtures.
Limits: This proves the built publisher and completion contract locally. It does not deploy the worker, mutate a live queue, or exercise live GitHub publication; exact-head hosted CI and ClawSweeper review cover the submitted branch.