fix: recover interrupted command dispatches - #407
Conversation
Co-authored-by: Alex <alexgild@gmail.com>
Co-authored-by: Alex <alexgild@gmail.com>
|
Codex review: needs changes before merge. Reviewed July 4, 2026, 10:09 AM ET / 14:09 UTC. Summary Reproducibility: yes. source inspection gives a high-confidence reproduction path: append two executed repair_loop_label_sweep entries for the same idempotency key and appendLedger collapses them into one ledger record. autoRepairBlockReason then reads ledger.commands and can undercount previous repair dispatches. Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Preserve claim-to-executed coalescing for a single synthetic sweep attempt while assigning later legitimate repair-loop sweeps distinct ledger keys, with regression coverage proving autoRepairBlockReason still counts multiple attempts. Do we have a high-confidence way to reproduce the issue? Yes, source inspection gives a high-confidence reproduction path: append two executed repair_loop_label_sweep entries for the same idempotency key and appendLedger collapses them into one ledger record. autoRepairBlockReason then reads ledger.commands and can undercount previous repair dispatches. Is this the best way to solve the issue? No, not yet. Recoverable dispatch receipts are the right direction, but the implementation needs safer ledger identity for synthetic sweep attempts before it is the narrowest maintainable fix. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against a52b852c3fc2. Label changesLabel changes:
Label justifications:
Evidence reviewedAcceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
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. How this review workflow works
Review history (12 earlier review cycles; latest 8 shown)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bb104c118
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const expectedTitle = repairRunNameForJob( | ||
| command.target.job_path, | ||
| automergeRunNamePrefix, | ||
| dispatchKey, | ||
| ); |
There was a problem hiding this comment.
For implement_issue repair jobs, command.target.job_path is jobs/<owner>/inbox/issue-...md, but .github/workflows/repair-cluster-worker.yml names those runs issue implementation <job> [<dispatch_key>]. repairRunNameForJob() still falls back to repair cluster <job> [<dispatch_key>] for non-automerge jobs, so claimedDispatchState() never matches an existing issue-implementation receipt; after the grace period the router redispatches instead of recovering the prior dispatch claim.
Useful? React with 👍 / 👎.
| function priorDispatchClaim(command: LooseRecord) { | ||
| return priorDispatchClaims.get(commentVersionKey(command) ?? "") ?? null; |
There was a problem hiding this comment.
This lookup only uses commentVersionKey, but synthetic commands from listRepairLoopSweepCommands() have comment_version_key: null and a comment_updated_at based on the current router start time. After an interrupted label-sweep dispatch, the next run generates a different version key and cannot find the prior claimed ledger entry, even though dispatchReceiptKey() is stable via idempotency_key, so the router can redispatch during the recovery window instead of waiting/recovering.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6203048cde
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function dispatchReceiptKey(command: LooseRecord) { | ||
| return `router-${createHash("sha256") | ||
| .update(String(command.idempotency_key ?? command.comment_version_key ?? "unknown")) | ||
| .digest("hex") |
There was a problem hiding this comment.
For repair-loop label sweeps, idempotency_key is stable for the same repo/intent/PR, so hashing only this value makes every future automerge/autofix worker for that PR reuse the same dispatch_key and therefore the same repair run title. The new receipt jobs skip when any older exact title exists, so a later legitimate sweep after a new head or resume can be dispatched and then skipped as a duplicate while the router records it as executed. Include an attempt-specific value for synthetic sweeps, or bound the receiver dedupe to the current claim window.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
proof: sufficient
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
Align issue-implementation receipts, recover synthetic commands by idempotency key, ignore failed workflow receipts, and stabilize instrumented coverage tests.
|
@clawsweeper re-review Please review exact head |
|
🦞🧹 I asked ClawSweeper to review this item again. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e5d648f02
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const successfulRun = matchingRuns.find( | ||
| (run) => String(run.conclusion ?? "").toLowerCase() === "success", | ||
| ); | ||
| if (successfulRun) return { action: "recover", run: successfulRun }; |
There was a problem hiding this comment.
With the new receipt jobs in assist.yml and repair-cluster-worker.yml, a duplicate run that sees an older active run writes proceed=false and exits successfully, so that workflow run can have conclusion success even though the assist/cluster worker never ran. Because this branch recovers on any matching successful run, if the older run later fails, a subsequent router retry will recover the skipped duplicate and mark the dispatch executed instead of redispatching.
Useful? React with 👍 / 👎.
| const claims = commands | ||
| .filter(commandNeedsDurableDispatchClaim) | ||
| .filter((command) => !priorDispatchClaim(command)) | ||
| .map((command) => ({ |
There was a problem hiding this comment.
When a prior claim is stale and no receipt is visible, this filter prevents writing a fresh claim for the retry attempt. If the router crashes again after redispatching but before the run becomes visible or the final ledger append happens, the next run still compares against the old processed_at that is already past the grace window and immediately dispatches again instead of waiting another visibility window, allowing repeated duplicate dispatches during the crash-recovery path this claim is meant to protect.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
@clawsweeper re-review Please review exact head |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
@clawsweeper re-review Exact head: The two exact-review findings are repaired:
Local proof: 189 focused tests, 600 unit tests, 617 repair tests, full check and both coverage modes green, formatting green, AutoReview clean. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c7e05b135
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| expected_title="Assist ${TARGET_REPO}#${ITEM_NUMBER} [${DISPATCH_KEY}]" | ||
| if gh api --method GET "repos/${GITHUB_REPOSITORY}/actions/workflows/assist.yml/runs?per_page=100" \ | ||
| | jq -e --arg title "$expected_title" --arg current "$GITHUB_RUN_ID" \ | ||
| 'any(.workflow_runs[]; .display_title == $title and .id < ($current | tonumber) and (.status == "queued" or .status == "in_progress" or .status == "waiting" or .status == "pending" or .status == "requested" or .conclusion == "success"))' >/dev/null; then |
There was a problem hiding this comment.
When a same-key duplicate is dispatched while the older assist/repair run is still queued or in progress, this receipt job exits 0 with proceed=false; GitHub records that receipt-only run as a successful workflow even though the real assist job (and the analogous Plan and review cluster job in .github/workflows/repair-cluster-worker.yml) was skipped. Because this predicate treats any older .conclusion == "success" as a duplicate, if the original run later fails, every router retry with the same dispatch key is skipped by the receipt-only success while the router's new job verification refuses to recover it, leaving the command in a claimed/retry loop.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
![openclaw-clawsweeper[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
clawsweeper
Bot
added
rating: 🦪 silver shellfish
clawsweeper
Bot
added
proof: sufficient
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca335bc305
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| !entry.comment_version_key && | ||
| entry.automation_source === "repair_loop_label_sweep" && | ||
| entry.idempotency_key | ||
| ) { | ||
| return `idempotency:${entry.idempotency_key}`; |
There was a problem hiding this comment.
For repair-loop label sweeps, comment_version_key is always null and idempotency_key is stable for the same repo/intent/PR, so this key collapses every executed sweep for that PR into a single ledger entry. autoRepairBlockReason() relies on the ledger entries with executed dispatch_repair actions to enforce maxAutoRepairsPerPr/maxAutoRepairsPerHead; after the second scheduled repair the older dispatch history is overwritten, allowing the sweep lane to undercount prior repairs and exceed the intended safety caps. Collapse only the claimed→executed record for the same attempt, or include attempt-specific material for executed sweep entries.
Useful? React with 👍 / 👎.
|
@clawsweeper re-review Exact head: The compatibility finding is fixed, and current-head live proof is now attached: https://github.com/openclaw/clawsweeper/actions/runs/28708493789 completed successfully through keyed The PR body also records the full 604 unit / 617 repair / 1,221 aggregate test proof and the clean final AutoReview. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
rating: 🧂 unranked krab
1 participant
What changed
This keeps the useful pre-dispatch claim from #380, but makes the claim recoverable instead of terminal.
workflow_dispatchfallback. The fallback carries the same receipt key and produces the same recoverable run title asrepository_dispatch.The contributor branch could not be updated through the available workflow-scoped OAuth authorization, so this replacement preserves and extends @ag-linden's work with co-author credit.
Proof
Exact head:
ca335bc305181cbfe61c60af91480f937e3e8eecworkflow_dispatch, renderedReview event item openclaw/clawsweeper#407 [router-live-ca335bc3], planned exactly PR fix: recover interrupted command dispatches #407, completed the review shard, published artifacts, and completed recovery handling.Risk
Medium. This changes command-dispatch recovery and three receiving workflows. Recovery is deliberately conservative: claims wait five minutes before stale recovery, worker execution must be proven for assist/repair, failed receipts remain retryable, the fallback preserves the previous transport, and receiver-side ownership prevents duplicate execution without losing every copy.
Replaces #380.