From 6b22fa9f0ecf31e3c761351788998f53a74ffb4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sun, 19 Apr 2026 21:09:16 -0400 Subject: [PATCH] Free tasks from the checkout_race_lost deadlock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs combined to park successful-but-deliverable-less tasks in an unrecoverable state. Operator saw Alex Chen loop on `checkout_race_lost` every two minutes for 16+ minutes in the field log after completing its first task. 1. `processRunCompletion` in automation-run-completion.ts fired a no-op status transition (`in_progress` → `in_progress`) when a run succeeded without producing the expected deliverable. advanceTaskStatus no-ops on identical prev/next, and handleTaskStatusChange logged its `caller should gate this` warning and returned. finishRun had already released active_run_id and checkout_run_id, so the task sat at `in_progress` with no lock, no run — the exact orphan pattern that checkoutTask's `status IN ('todo','backlog')` clause refuses. Transition the row to `blocked` with the diagnostic comment attached so the manager's blocker queue surfaces it, the assignee stops spinning, and effectiveStatus carries the real state downstream. 2. `checkoutTask` itself refused to reclaim any orphan `in_progress` row, even when active_run_id was already null. That refusal was the right invariant against concurrent re-entry but wrong for the orphan case a crashed or status-unchanged run leaves behind. Extend the CAS predicate to accept `status = 'in_progress' AND active_run_id IS NULL` — heartbeat candidate selection already filters out tasks whose active_run_id points at a live run, so the guard preserves the original mutual-exclusion property for the busy case while allowing the orphan reclaim the scheduler actually needs. --- .../automation/automation-run-completion.ts | 25 ++++++++++++++++--- src/main/database/tasks-status.ts | 19 +++++++++++++- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/main/automation/automation-run-completion.ts b/src/main/automation/automation-run-completion.ts index 252e6f0b..012f5579 100644 --- a/src/main/automation/automation-run-completion.ts +++ b/src/main/automation/automation-run-completion.ts @@ -349,10 +349,26 @@ export function processRunCompletion( if (needsDeliverableCheck(taskType)) { const hasDeliverable = checkRunProducedDeliverable(db, run.id, run.companyId, task.id, taskType); if (!hasDeliverable) { - // Don't auto-advance -- flag for manager review - db.advanceTaskStatus(task.id, "in_progress", - `${taskType} task run completed but no deliverable artifact was detected. Flagging for manager review.`); - handleTaskStatusChange(task.id, run.companyId, "in_progress", "in_progress"); + // Don't auto-advance to in_review: the run succeeded but did not + // produce the required deliverable, so it's not ready for + // manager sign-off. + // + // Park the task as `blocked` rather than leaving it at + // `in_progress`. finishRun has already cleared the run's + // checkout_run_id and active_run_id, so a task still tagged + // `in_progress` has no lock, no run, and no path back to + // `todo` — heartbeat picks it up, checkoutTask rejects it + // (status ∉ todo/backlog), and the assignee is stuck in a + // permanent `checkout_race_lost` loop. Blocking instead puts + // the task in the manager's blocker queue with the diagnostic + // comment attached; the manager patches it back to `todo` (or + // `cancelled`) after deciding how to proceed. + db.advanceTaskStatus( + task.id, + "blocked", + `${taskType} task run completed but no deliverable artifact was detected. Flagging for manager review.`, + ); + handleTaskStatusChange(task.id, run.companyId, "in_progress", "blocked"); // Wake manager for review try { const agent = db.getAgent(run.agentId); @@ -366,6 +382,7 @@ export function processRunCompletion( ); } publishDomainChanged(); + effectiveStatus = "blocked"; // Skip the normal in_review advance } else { effectiveStatus = advanceSuccessfulRun( diff --git a/src/main/database/tasks-status.ts b/src/main/database/tasks-status.ts index f0697cec..8f0d9884 100644 --- a/src/main/database/tasks-status.ts +++ b/src/main/database/tasks-status.ts @@ -159,13 +159,30 @@ export function buildTaskStatusMethods( checkoutTask(taskId: string, runId: string): boolean { const ts = nowIso(); + // Primary path is `status IN ('todo','backlog')`. The extra + // `in_progress AND active_run_id IS NULL` branch recovers from a + // specific orphan: finishRun has cleared active_run_id and + // checkout_run_id for the previous run but the status transition + // to done/in_review never fired (e.g. the run succeeded without a + // deliverable and automation parked it at in_progress, or a + // crashed run left the row behind). Without the reclaim, heartbeat + // picks the task (activeRunId=null passes the candidate filter) + // and then loops on `checkout_race_lost` forever because status + // never flips back to todo on its own. + // + // The `active_run_id IS NULL` guard preserves the original CAS + // contract for the busy case: a task whose active_run_id still + // points at a live run is refused here, exactly as before. const result = getDb() .prepare( `UPDATE tasks SET checkout_run_id = ?, execution_locked_at = ?, status = 'in_progress', updated_at = ? WHERE id = ? AND checkout_run_id IS NULL - AND status IN ('todo', 'backlog')`, + AND ( + status IN ('todo', 'backlog') + OR (status = 'in_progress' AND active_run_id IS NULL) + )`, ) .run(runId, ts, ts, taskId); return result.changes > 0;