Skip to content

fix(publication): stop retrying verified stale review artifacts - #1250

Draft
vincentkoc wants to merge 1 commit into
mainfrom
fix/superseded-publication-tuple-proof-20260825
Draft

fix(publication): stop retrying verified stale review artifacts#1250
vincentkoc wants to merge 1 commit into
mainfrom
fix/superseded-publication-tuple-proof-20260825

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

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_sync result whose canonical reason contains safe positive durable lease comment IDs with live > 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 reports completion_kind=superseded with reason_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 superseded terminal 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.md is release-owned for normal repair work.

Evidence

  • Signed head: 25565772df3c5dd6a414ed5132cff5a4bc6580da
  • Focused proof: 21/21 event-apply-proof and stale-event-disposition tests passed.
  • Affected review set: 194/194 build, workflow, apply, batch, and repair tests passed.
  • Static proof: main, repair, and dashboard TypeScript builds; scoped oxlint; scoped oxfmt; git diff --check; privacy scan.
  • Containment retry: 31 relevant assertions passed serially with 4 platform skips after the broad repair target at concurrency 8 hit a subprocess timeout under host contention.
  • Codex review: the dirty-tree review found and fixed a guarded-open regression; the committed-range review found no remaining issue.
  • Production delta: +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.js runtime, 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=module harness 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.

@clawsweeper

clawsweeper Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 26, 2026
@clawsweeper

clawsweeper Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 25, 2026, 8:32 PM ET / August 26, 2026, 00:32 UTC.

ClawSweeper review

What this changes

The 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 readiness

⚠️ Ready for maintainer review - 1 item remains

Keep 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
Reviewed head: 25565772df3c5dd6a414ed5132cff5a4bc6580da

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, fail-closed repair with convincing runtime evidence and no discrete correctness finding.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (live_output): The PR body provides after-fix built-publisher live output for four isolated fixtures, including the superseded result and zero GitHub mutations.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The PR body provides after-fix built-publisher live output for four isolated fixtures, including the superseded result and zero GitHub mutations.
Evidence reviewed 6 items Strict terminal classification: The patch recognizes only one exact stale-comment action whose canonical reason has positive, safe-integer lease IDs with the live ID strictly greater than the report ID; all other strings remain unproven.
Terminal path precedes publication: A superseded disposition writes the batch result and completion reason, emits the summary, and returns before batch mutation planning or snapshot publication.
Canonical reason producer: Current main emits the lease-based stale reason only when the live and report revisions match and the live durable lease is valid; the new consumer adds a stricter greater-than check than the producer’s wording branch.
Findings None None.
Security None None.

Live Verification

Command: pnpm run build:repair && pnpm run repair:publish-event-result

Result: FAIL (failed) — step 1 run pnpm run build:repair: previous terminal command was still running after 30 seconds: "pnpm run build:repair && pnpm run repair:publish-event-result"

$ tsc -p tsconfig.repair.json
$ node dist/repair/publish-event-result.js
[apply-artifacts] applied=0 skipped_closed=0
[action-ledger] finalized 3 immutable workflow shards
Skipping stale event apply for openclaw/clawsweeperthis PR: the event produced no record tuple

Assertions:

  • FAIL expect_output: TARGET_REPO is required

How this fits together

The 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]
Loading

Before merge

  • Resolve merge risk (P1) - The producer-to-consumer protocol is an exact reason string; a future wording change fails closed but can turn verified stale artifacts into permanent failures until both sides are updated.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta production +58/-20; tests +69/-2 The added logic is concentrated in the disposition parser and terminal publisher branch, with focused coverage for valid, legacy, malformed, and mixed results.

Merge-risk options

Maintainer options:

  1. Merge the strict fail-closed contract (recommended)
    Accept the narrow automation risk because only the existing canonical tuple reason with strictly ordered safe lease IDs terminalizes the artifact.
  2. Strengthen producer-to-publisher coverage
    Before merge, add a cross-module fixture that carries the producer’s actual stale-tuple result through the publisher completion path.

Technical review

Best 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.

Labels

Label changes:

  • add P2: This repairs retry behavior in a bounded publication workflow without evidence of a current user-facing outage.
  • add merge-risk: 🚨 automation: The PR changes exact-review publication completion and queue lifecycle behavior, where an incorrect classification could alter retry handling.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-fix built-publisher live output for four isolated fixtures, including the superseded result and zero GitHub mutations.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides after-fix built-publisher live output for four isolated fixtures, including the superseded result and zero GitHub mutations.

Label justifications:

  • P2: This repairs retry behavior in a bounded publication workflow without evidence of a current user-facing outage.
  • merge-risk: 🚨 automation: The PR changes exact-review publication completion and queue lifecycle behavior, where an incorrect classification could alter retry handling.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body provides after-fix built-publisher live output for four isolated fixtures, including the superseded result and zero GitHub mutations.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides after-fix built-publisher live output for four isolated fixtures, including the superseded result and zero GitHub mutations.

Evidence

What I checked:

  • Strict terminal classification: The patch recognizes only one exact stale-comment action whose canonical reason has positive, safe-integer lease IDs with the live ID strictly greater than the report ID; all other strings remain unproven. (src/repair/event-apply-proof.ts:169, 25565772df3c)
  • Terminal path precedes publication: A superseded disposition writes the batch result and completion reason, emits the summary, and returns before batch mutation planning or snapshot publication. (src/repair/publish-event-result.ts:254, 25565772df3c)
  • Canonical reason producer: Current main emits the lease-based stale reason only when the live and report revisions match and the live durable lease is valid; the new consumer adds a stricter greater-than check than the producer’s wording branch. (src/clawsweeper-review-comment-state.ts:492, afe976209aa5)
  • Workflow accepts the new terminal result: The publication workflow already accepts superseded/remote_newer_tuple as a successful terminal lifecycle result. (.github/workflows/sweep.yml:3079, afe976209aa5)
  • Feature-history provenance: Blame attributes the current main tuple producer and existing apply-proof path to the current baseline commit; older publisher history includes the central publication refactor. (src/repair/event-apply-proof.ts:114, 115abf8548e0)
  • After-fix runtime evidence: The PR body records a built-publisher four-fixture run: the verified newer tuple completed as superseded with remote_newer_tuple, while legacy, mixed, and guarded paths retained their intended outcomes and reported zero GitHub mutations. (test/repair/event-apply-proof.test.ts:406, 25565772df3c)

Likely related people:

  • Martin Cleary: Current-main blame attributes the durable review tuple producer and apply-proof baseline to this commit. (role: current main feature-history contributor; confidence: medium; commits: 115abf8548e0; files: src/clawsweeper-review-comment-state.ts, src/repair/event-apply-proof.ts)
  • Peter Steinberger: Earlier history for the publisher includes the central publication refactor, making this a relevant routing contact for lifecycle behavior. (role: publisher history contributor; confidence: medium; commits: f32082eb7d3e, 32e1a069db2d; files: src/repair/publish-event-result.ts)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

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

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant