fix(cards): stop finishRunForLane overwriting a run's stalled status

[anagnorisis2peripeteia]

What Problem This Solves

When a run finishes for a lane (advance to Done / Human Review / Backlog), finishRunForLane overwrote
the run's status unconditionally. If a reconciler had already marked that run stalled — a
heartbeat timeout or crash detected during the caller's read→write window — its terminal provenance
was silently replaced with completed / review / canceled, and a false "run <status>" event
was recorded. The stalled fact (and the preserved workspace it implies) was lost.

This is the same check-then-act race the two siblings on this table already guard against: stall()
and reconcileStalledRuns() both fence their run_attempts UPDATE with WHERE status IN (activeRunStatuses) and only emit their event when a row actually changed. finishRunForLane was the
one writer that didn't.

The change

Fence the transition on the run still being pre-terminal, and commit the transition and its audit
event in one atomic DB.batch — the same guarded-update + conditional-insert pattern claimRun
already uses on this table:

UPDATE run_attempts SET status = ?, ended_at = ?, updated_at = ?, control_intent = ?
  WHERE id = ? AND status IN (<active statuses [+ 'review' only when target = 'completed']>);
INSERT INTO events (card_id, actor, message, created_at)
  SELECT card_id, ?, ?, ? FROM run_attempts
  WHERE id = ? AND status = ? AND (SELECT changes()) = 1;               -- only if THIS batch's UPDATE transitioned the row

'review' is allowed in the precondition only for the Done transition (review → completed).
For the Human Review / Backlog transitions the run must still be active, so a run already in review
is never re-transitioned — a repeated or concurrent Human Review completion cannot re-fire the audit
event or rewrite the run's terminal timing.

A run a reconciler moved to stalled (or any terminal status) no longer matches the UPDATE, so the
transition is skipped. The event insert is gated on (SELECT changes()) = 1 — the row count of the
immediately-preceding UPDATE — so it belongs strictly to the transition this batch performed: a
stalled/terminal run gets no false event, an already-terminal run is not re-announced, and two finish
calls sharing the same now cannot each emit the event (only the one whose UPDATE changed the row
does — this closes the duplicate-audit-event risk raised on the previous revision). Sharing one batch
means the status write and its event commit or roll back together — they can never persist separately.

Real-behaviour proof (before → after, real D1)

The same interleaving, driven against a real Miniflare D1 (SQLite) so the actual compiled SQL runs:

BASE origin/main — unconditional UPDATE, no status guard (real D1)
  after finishRunForLane(Done): run status=completed, run-finished events=1 ("run completed")
  => STOMPED: stalled overwritten to 'completed' + false 'run completed' event

HEAD (this PR) — guarded UPDATE + conditional event, one atomic batch (real D1)
  after finishRunForLane(Done): run status=stalled, run-finished events=0
  => PRESERVED: stalled provenance intact, no false 'run completed' event

Reproducible transcript: https://gist.github.com/anagnorisis2peripeteia/1c6b581b8cd3236008ee787582a7040e

Testing

pnpm check (tsc + oxlint + oxfmt), pnpm build, pnpm exec wrangler deploy --dry-run, pnpm test
all green. tests/card-run-finish-guard.test.ts runs against a real Miniflare D1 (not a mock):
a live run transitions and records exactly one event; a stalled run is left untouched with no event
(the race fix); an already-terminal run is not re-announced; two finish calls sharing the same
now emit exactly one event (pins the changes()-gated conditional insert against duplicate audit
events); a review run promotes to Done; a repeated Human Review completion re-fires no event and
does not rewrite the run's timing (pins the review-only-for-Done restriction); and a non-terminal
lane cancels with control_intent. Mutation testing of the changed range scores 100%. A concurrency
lint of the diff is clean.

Scope: this fixes only the run-provenance overwrite. The card's own lane move (moveCard) is a
separate, pre-existing, user-initiated step and is intentionally left as-is.

Related: FINDING 2 of #88.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed July 17, 2026, 6:47 PM ET / 22:47 UTC.

Summary
The PR replaces the unconditional finishRunForLane status update with a guarded atomic D1 update-plus-event batch and adds real-Miniflare-D1 regression tests for stalled and terminal runs.

Reproducibility: yes. The PR supplies a focused real-Miniflare-D1 path that seeds a stalled run, invokes finishRunForLane, and verifies that the persisted status and event log remain unchanged.

Review metrics: 2 noteworthy metrics.

• Persistence coverage: 1 production file changed, 1 test file added; 203 additions, 18 deletions. The patch is tightly scoped to one run-state transition and its D1-backed regression coverage.
• Terminal-path cases: 6 focused persistence behaviors covered. Coverage includes stalled, terminal, duplicate-finish, review promotion, repeated review, and cancellation behavior.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

• [P1] Merging changes persisted run terminal-state and audit-event semantics, so maintainers should retain the real-D1 regression coverage and verify the behavior on the exact merge head.

Maintainer options:

1. Merge with the guarded D1 contract (recommended)
   Accept the focused fix after confirming the exact merge head retains the atomic guarded update and real-D1 regression coverage for terminal run states.

Next step before merge

• [P2] No mechanical repair remains; the PR is ready for ordinary maintainer review of the exact merge head and its persisted-state semantics.

Security
Cleared: The diff is limited to D1 lifecycle-state writes and a local Miniflare test; it adds no dependency, secret, permission, workflow, or supply-chain surface.

Review details

Best possible solution:

Merge this narrow guarded-transition repair after ordinary maintainer review, preserving the atomic update/event batch and its D1 regression cases as the contract for run terminal provenance.

Do we have a high-confidence way to reproduce the issue?

Yes. The PR supplies a focused real-Miniflare-D1 path that seeds a stalled run, invokes finishRunForLane, and verifies that the persisted status and event log remain unchanged.

Is this the best way to solve the issue?

Yes. Guarding the persisted transition and conditionally inserting its audit event in the same D1 batch is the narrowest maintainable fix for the stale read-to-write race without changing the card-lane operation.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 656c3514fcd7.

Label changes

Label justifications:

• P2: This is a bounded reliability repair for persisted run lifecycle state with limited but meaningful user impact.
• merge-risk: 🚨 session-state: The PR changes how persisted run statuses and terminal audit events are committed under concurrent lifecycle operations.
• rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The PR body provides a before/after real-Miniflare-D1 transcript showing that base overwrites a stalled run while the PR preserves it and emits no false event; the added test exercises the same persisted SQL path.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body provides a before/after real-Miniflare-D1 transcript showing that base overwrites a stalled run while the PR preserves it and emits no false event; the added test exercises the same persisted SQL path.

Evidence reviewed

What I checked:

• PR head and prior review continuity: The current head 6ef99a0009cd5998ece1c5587abe4e250844727b is the same head accepted in the latest completed ClawSweeper cycle; its three earlier blockers—atomic write/event behavior, unique event gating, and review-only-for-Done behavior—are all explicitly covered by the revised implementation and tests. (src/worker/card-repository.ts:477, 6ef99a0009cd)
• Guarded state transition: The PR changes finishRunForLane so only active runs, plus review when advancing to Done, can transition; a concurrently stalled or otherwise terminal row no longer matches the update and cannot receive a false terminal event. (src/worker/card-repository.ts:477, 6ef99a0009cd)
• Real persistence coverage: The added Miniflare D1 test covers the stalled-run no-op, already-terminal no-op, same-time duplicate-finish case, review-to-completed path, repeated Human Review no-op, and cancellation control intent against SQLite-backed D1 rather than a mock. (tests/card-run-finish-guard.test.ts:1, 6ef99a0009cd)
• Related umbrella report: The linked umbrella report identifies this as its bounded run-status-stomp finding and keeps the broader lifecycle audit open; this PR is a focused candidate fix rather than a duplicate of the umbrella work. (6ef99a0009cd)

Likely related people:

• anagnorisis2peripeteia: Authored the linked lifecycle-concurrency report and the focused repair with real-D1 proof; no current-main ownership trail was available from the read-only execution environment. (role: focused concurrency investigator; confidence: low; commits: 6ef99a0009cd; files: src/worker/card-repository.ts, tests/card-run-finish-guard.test.ts)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

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

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

Review history (4 earlier review cycles)

• reviewed 2026-07-17T19:48:33.327Z sha bed54f6 :: needs real behavior proof before merge. :: [P2] Keep the status update and event insertion atomic
• reviewed 2026-07-17T21:27:29.880Z sha 845c52d :: needs changes before merge. :: [P2] Use a unique marker for the conditional audit event
• reviewed 2026-07-17T21:53:51.838Z sha ec8f6b4 :: needs changes before merge. :: [P2] Restrict review to the Done transition
• reviewed 2026-07-17T22:05:47.247Z sha 6ef99a0 :: needs maintainer review before merge. :: none

[anagnorisis2peripeteia]

@clawsweeper re-review

Addressed both prior findings: (1) the guarded transition and its audit event now commit in one atomic DB.batch (guarded UPDATE + conditional INSERT…SELECT…WHERE updated_at=now, the same pattern as claimRun) — no split-write, and no false event for an already-terminal run. (2) Proof is now a real Miniflare D1 before/after (base STOMPS a stalled run, HEAD no-ops) with committed real-D1 regression tests; transcript in the linked gist.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The durable review comment was published and its verdict was queued for target-serialized routing.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/29616200873
• Updated: 2026-07-17T21:57:04.519Z

[anagnorisis2peripeteia]

@clawsweeper re-review

Addressed the [P1] duplicate-audit-event risk: the conditional event insert is now gated on the UPDATE's own row count — (SELECT changes()) = 1 — instead of the updated_at timestamp, so the event belongs strictly to the transition this batch performed. Two finish calls sharing the same now can no longer each emit the event. Added the requested real-Miniflare-D1 regression: two same-now finish calls now produce exactly one run completed event.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The durable review comment was published and its verdict was queued for target-serialized routing.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/29616831410
• Updated: 2026-07-17T22:09:42.424Z

[anagnorisis2peripeteia]

@clawsweeper re-review

Addressed the [P1]: review is now allowed in the guard only for the Done transition (review → completed). For Human Review / Backlog the run must still be active, so a run already in review is never re-transitioned — a repeated or concurrent Human Review completion can no longer emit a duplicate audit event or rewrite terminal timing. Added the requested real-Miniflare-D1 regression (repeated Human Review completion → exactly one run review event, updated_at/ended_at unchanged). Mutation on the changed range is now 100%.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

• State: Complete
• Detail: The durable review comment was published and its verdict was queued for target-serialized routing.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/29618930840
• Updated: 2026-07-17T22:50:57.220Z