From ba66a6bdc3800e1c53d006e93aacf9f87f025270 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 18:19:59 -0400 Subject: [PATCH] Skip in_review for autonomous top-level completions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In `ai_ceo` mode the operator opted out of human review, but `processRunCompletion` still advanced every successful run through `in_review`. The top-level CEO has no manager to walk up to and no human gate, so the task became a dead letter: the heartbeat candidate filter only picks `todo / in_progress / blocked`, so the CEO never saw its own stuck rows and looped on taskless heartbeats every two minutes producing nothing. The previous workaround was even uglier: `advanceToInReview` unconditionally force-flagged `requires_user_review = true` on every top-level completion just to bypass `guardInReviewHasReviewer`. The guard's own comment said "the right root fix is to reject at source"; the workaround did the opposite. `decideTerminalStatus` is the new single source of truth: it returns `done` only when the assignee is top-level AND the company is in `ai_ceo` mode AND `reviewDeliverables` is off. Every other shape — non-top-level, ai_cofounder, or operator-opted-in deliverable review — still routes to `in_review` so existing manager-review and human-review flows are unchanged. `advanceSuccessfulRun` is the single entry point both successful branches call so the deliverable and non-deliverable paths cannot diverge again. --- .../automation/automation-run-completion.ts | 98 +++++++++++++++++-- .../document-run-provenance.spec.ts | 5 +- 2 files changed, 94 insertions(+), 9 deletions(-) diff --git a/src/main/automation/automation-run-completion.ts b/src/main/automation/automation-run-completion.ts index 73574412..252e6f0b 100644 --- a/src/main/automation/automation-run-completion.ts +++ b/src/main/automation/automation-run-completion.ts @@ -207,6 +207,51 @@ function createDeliverableReviewApproval( return approvalId; } +/** + * Decide the terminal status a successful run should leave the task in. + * + * `in_review` only makes sense when there is a reachable reviewer: + * - a non-top-level assignee always has a manager who reviews; + * - in `ai_cofounder` mode the human is the implicit reviewer; + * - in `ai_ceo` mode the operator opted out of human review, but + * they can re-opt-in per company via `reviewDeliverables`. + * + * Top-level agents in `ai_ceo` mode without `reviewDeliverables` have + * no reviewer — neither a manager to walk up to nor a human gate. + * Routing them through `in_review` strands the task forever (the + * heartbeat candidate filter only picks `todo / in_progress / blocked` + * so the CEO never sees its own stuck rows) and the codebase's old + * workaround was to force-flag `requires_user_review` so the + * `guardInReviewHasReviewer` would let the transition through. That + * was paper over the symptom; the right answer is to skip review + * entirely and advance straight to `done`. + */ +function decideTerminalStatus( + db: RunCompletionDeps["db"], + agent: ReturnType, + companyId: string, +): "in_review" | "done" { + if (agent.reportsTo !== null) return "in_review"; + const mode = db.getCompanyExecutionMode(companyId); + if (mode !== "ai_ceo") return "in_review"; + const settings = db.getCompanyApprovalSettings(companyId); + return settings?.reviewDeliverables ? "in_review" : "done"; +} + +/** Auto-resolves the task as a successful autonomous completion. */ +function advanceToDone( + deps: RunCompletionDeps, + callbacks: RunCompletionCallbacks, + run: RunRecord, + taskId: string, + statusMessage: string, +): void { + const { db, publishDomainChanged } = deps; + db.advanceTaskStatus(taskId, "done", statusMessage); + callbacks.handleTaskStatusChange(taskId, run.companyId, "in_progress", "done"); + publishDomainChanged(); +} + /** Advances a task to in_review and optionally flags deliverables for user review. */ function advanceToInReview( deps: RunCompletionDeps, @@ -218,9 +263,11 @@ function advanceToInReview( const { db, logger, publishDomainChanged } = deps; // The tasks-status guard refuses an `in_review` transition for a // top-level assignee (reports_to IS NULL) unless the task is - // flagged for user review. Auto-completed runs owned by the CEO - // routinely trip that path — pre-flag them so the transition - // succeeds and the human becomes the reviewer. + // flagged for user review. The caller (`processRunCompletion`) only + // routes top-level runs through this helper when human review is + // expected (`ai_cofounder` mode or `reviewDeliverables = true`), so + // pre-flagging here is the right semantics — the human IS the + // reviewer in those configurations. const agent = db.getAgent(run.agentId); if (agent.reportsTo === null) { db.setTaskRequiresUserReview(taskId, true); @@ -245,6 +292,29 @@ function advanceToInReview( publishDomainChanged(); } +/** + * Pick the right terminal-status helper for this run. `processRunCompletion` + * uses it after every successful "produced something or didn't need to" + * branch; `decideTerminalStatus` is the single source of truth that says + * `done` (no reviewer needed) vs `in_review` (reviewer expected). + */ +function advanceSuccessfulRun( + deps: RunCompletionDeps, + callbacks: RunCompletionCallbacks, + run: RunRecord, + taskId: string, + statusMessage: string, +): "in_review" | "done" { + const agent = deps.db.getAgent(run.agentId); + const target = decideTerminalStatus(deps.db, agent, run.companyId); + if (target === "done") { + advanceToDone(deps, callbacks, run, taskId, statusMessage); + } else { + advanceToInReview(deps, callbacks, run, taskId, statusMessage); + } + return target; +} + export function processRunCompletion( deps: RunCompletionDeps, callbacks: RunCompletionCallbacks, @@ -298,13 +368,25 @@ export function processRunCompletion( publishDomainChanged(); // Skip the normal in_review advance } else { - advanceToInReview(deps, callbacks, run, task.id, "Run completed with deliverable. Awaiting review."); - effectiveStatus = "in_review"; + effectiveStatus = advanceSuccessfulRun( + deps, + callbacks, + run, + task.id, + "Run completed with deliverable.", + ); } } else { - // Code/management/general tasks advance normally - advanceToInReview(deps, callbacks, run, task.id, "Run completed successfully. Awaiting review."); - effectiveStatus = "in_review"; + // Code/management/general tasks: route through the same selector + // so ai_ceo top-level completions land in `done` instead of + // stalling in the `in_review` dead-letter. + effectiveStatus = advanceSuccessfulRun( + deps, + callbacks, + run, + task.id, + "Run completed successfully.", + ); } } diff --git a/tests/integration/document-run-provenance.spec.ts b/tests/integration/document-run-provenance.spec.ts index cd2b860b..8f853ef2 100644 --- a/tests/integration/document-run-provenance.spec.ts +++ b/tests/integration/document-run-provenance.spec.ts @@ -169,8 +169,11 @@ describe("document run provenance", () => { errorMessage: null, }, () => false); + // ai_ceo + top-level CEO without `reviewDeliverables` advances + // straight to `done` — there is no reviewer in that configuration + // so the prior `in_review` state was a dead letter. const task = harness.db.getTask(harness.taskId); - expect(task.status).toBe("in_review"); + expect(task.status).toBe("done"); } finally { harness.server.stop(); harness.db.close();