Skip in_review for autonomous top-level completions
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.
This commit is contained in:
1 parent
69a72b12a3
commit
ba66a6bdc3
2 files changed
+94
-9
No files matched your search
@@ -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<RunCompletionDeps["db"]["getAgent"]>,
|
||||
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.",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in new issue
Block a user