fix(FN-1256): auto-recover tasks with all steps done but no task_done call
When context overflow or compaction causes an agent to lose awareness of the task_done tool, the executor now checks if all steps are complete before failing — treating it as an implicit task_done. Also adds self-healing recovery for tasks that already slipped through as misclassified failures in in-review. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -1279,6 +1279,19 @@ export class TaskExecutor {
|
||||
return;
|
||||
}
|
||||
|
||||
// If the agent didn't explicitly call task_done, check whether
|
||||
// all steps are already complete — treat as implicit done to avoid
|
||||
// unnecessary retry sessions for context-overflow / compaction cases.
|
||||
if (!taskDone) {
|
||||
const implicitCheck = await this.store.getTask(task.id);
|
||||
if (implicitCheck.steps.length > 0 &&
|
||||
implicitCheck.steps.every((s) => s.status === "done" || s.status === "skipped")) {
|
||||
taskDone = true;
|
||||
executorLog.log(`${task.id} all steps done — treating as implicit task_done`);
|
||||
await this.store.logEntry(task.id, "All steps complete — implicit task_done (agent did not call tool explicitly)");
|
||||
}
|
||||
}
|
||||
|
||||
if (taskDone) {
|
||||
// Capture modified files before running workflow steps
|
||||
const updatedTask = await this.store.getTask(task.id);
|
||||
@@ -1357,6 +1370,20 @@ export class TaskExecutor {
|
||||
await promptWithFallback(retrySession, retryPrompt);
|
||||
checkSessionError(retrySession);
|
||||
|
||||
// If the agent didn't explicitly call task_done, check whether
|
||||
// all steps are already complete — if so, treat as implicit done.
|
||||
// This handles context-overflow / compaction scenarios where the
|
||||
// agent lost awareness of the task_done tool but finished the work.
|
||||
if (!taskDone) {
|
||||
const implicitCheck = await this.store.getTask(task.id);
|
||||
if (implicitCheck.steps.length > 0 &&
|
||||
implicitCheck.steps.every((s) => s.status === "done" || s.status === "skipped")) {
|
||||
taskDone = true;
|
||||
executorLog.log(`${task.id} all steps done — treating as implicit task_done`);
|
||||
await this.store.logEntry(task.id, "All steps complete — implicit task_done (agent did not call tool explicitly)");
|
||||
}
|
||||
}
|
||||
|
||||
if (taskDone) {
|
||||
const updatedTask = await this.store.getTask(task.id);
|
||||
const modifiedFiles = this.captureModifiedFiles(worktreePath, updatedTask.baseCommitSha);
|
||||
|
||||
@@ -596,6 +596,86 @@ describe("SelfHealingManager", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("recoverMisclassifiedFailures", () => {
|
||||
it("clears failed status when all steps are done and error is no-task_done", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
});
|
||||
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-300",
|
||||
column: "in-review",
|
||||
status: "failed",
|
||||
error: "Agent finished without calling task_done (after retry)",
|
||||
steps: [{ status: "done" }, { status: "done" }, { status: "skipped" }],
|
||||
log: [],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverMisclassifiedFailures();
|
||||
|
||||
expect(result).toBe(1);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-300", {
|
||||
status: null,
|
||||
error: null,
|
||||
});
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-300",
|
||||
expect.stringContaining("Auto-recovered"),
|
||||
);
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("skips tasks where steps are not all done", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
});
|
||||
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-301",
|
||||
column: "in-review",
|
||||
status: "failed",
|
||||
error: "Agent finished without calling task_done (after retry)",
|
||||
steps: [{ status: "done" }, { status: "in-progress" }],
|
||||
log: [],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverMisclassifiedFailures();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("skips tasks with different error messages", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
});
|
||||
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-302",
|
||||
column: "in-review",
|
||||
status: "failed",
|
||||
error: "Workflow step failed",
|
||||
steps: [{ status: "done" }, { status: "done" }],
|
||||
log: [],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverMisclassifiedFailures();
|
||||
|
||||
expect(result).toBe(0);
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
});
|
||||
|
||||
describe("recoverOrphanedExecutions", () => {
|
||||
it("requeues in-progress tasks whose reserved worktree is missing", async () => {
|
||||
const getExecuting = vi.fn().mockReturnValue(new Set<string>());
|
||||
|
||||
@@ -265,6 +265,7 @@ export class SelfHealingManager {
|
||||
this.checkpointWal();
|
||||
await this.enforceWorktreeCap();
|
||||
await this.recoverCompletedTasks();
|
||||
await this.recoverMisclassifiedFailures();
|
||||
await this.recoverOrphanedExecutions();
|
||||
await this.recoverApprovedTriageTasks();
|
||||
|
||||
@@ -324,6 +325,62 @@ export class SelfHealingManager {
|
||||
}
|
||||
}
|
||||
|
||||
// ── Misclassified failure recovery ───────────────────────────────
|
||||
|
||||
/**
|
||||
* Recover tasks in `in-review` marked as `failed` where all steps are
|
||||
* actually done. This catches the case where an agent completed all work
|
||||
* but the session ended without calling `task_done` (e.g., context
|
||||
* overflow, compaction losing tool awareness). The executor marks these
|
||||
* as failed, but the work is complete — clear the error so the normal
|
||||
* review flow can proceed.
|
||||
*
|
||||
* @returns Number of tasks recovered
|
||||
*/
|
||||
async recoverMisclassifiedFailures(): Promise<number> {
|
||||
try {
|
||||
const tasks = await this.store.listTasks();
|
||||
|
||||
const misclassified = tasks.filter((t) =>
|
||||
t.column === "in-review" &&
|
||||
t.status === "failed" &&
|
||||
t.error?.includes("without calling task_done") &&
|
||||
t.steps.length > 0 &&
|
||||
t.steps.every((s) => s.status === "done" || s.status === "skipped"),
|
||||
);
|
||||
|
||||
if (misclassified.length === 0) return 0;
|
||||
|
||||
log.warn(`Found ${misclassified.length} misclassified failure(s) with all steps done`);
|
||||
|
||||
let recovered = 0;
|
||||
for (const task of misclassified) {
|
||||
try {
|
||||
await this.store.updateTask(task.id, {
|
||||
status: null,
|
||||
error: null,
|
||||
});
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
"Auto-recovered: all steps complete despite 'no task_done' failure — cleared error for normal review",
|
||||
);
|
||||
log.log(`Recovered misclassified failure ${task.id}: ${task.title || task.description?.slice(0, 60) || "(untitled)"}`);
|
||||
recovered++;
|
||||
} catch (err: any) {
|
||||
log.error(`Failed to recover misclassified failure ${task.id}: ${err.message}`);
|
||||
}
|
||||
}
|
||||
|
||||
if (recovered > 0) {
|
||||
log.log(`Recovered ${recovered} misclassified failure(s) → cleared for review`);
|
||||
}
|
||||
return recovered;
|
||||
} catch (err: any) {
|
||||
log.error(`Misclassified failure recovery failed: ${err.message}`);
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Recover executor tasks stranded in `in-progress` before a real session was
|
||||
* established, typically when the scheduler reserved a worktree path but the
|
||||
|
||||
Reference in New Issue
Block a user