fix(FN-1473): recover no-progress task_done failures
This commit is contained in:
@@ -606,6 +606,12 @@ export class TaskExecutor {
|
||||
return task.steps.every((s) => s.status === "done" || s.status === "skipped");
|
||||
}
|
||||
|
||||
private isNoProgressNoTaskDoneFailure(task: Task): boolean {
|
||||
return task.status === "failed" &&
|
||||
task.error?.includes("without calling task_done") === true &&
|
||||
task.steps.every((step) => step.status === "pending");
|
||||
}
|
||||
|
||||
private async clearResumeFailureState(task: Task): Promise<void> {
|
||||
if (task.status === "failed" || task.error) {
|
||||
await this.store.updateTask(task.id, { status: null, error: null });
|
||||
@@ -736,6 +742,11 @@ export class TaskExecutor {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (this.isNoProgressNoTaskDoneFailure(task)) {
|
||||
executorLog.log(`${task.id} failed without task_done and has no step progress — leaving for self-healing requeue`);
|
||||
continue;
|
||||
}
|
||||
|
||||
executorLog.log(`Resuming ${task.id}: ${task.title || task.description.slice(0, 60)}`);
|
||||
try {
|
||||
await this.clearResumeFailureState(task);
|
||||
|
||||
@@ -427,6 +427,25 @@ describe("In-progress task resume after restart", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("resumeOrphaned() leaves no-progress no-task_done failures for self-healing", async () => {
|
||||
const store = createMockStore();
|
||||
const failedTask = makeTask("FN-1473", "in-progress", {
|
||||
status: "failed",
|
||||
error: "Agent finished without calling task_done (after retry)",
|
||||
steps: [],
|
||||
});
|
||||
store.listTasks.mockResolvedValue([failedTask]);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test");
|
||||
const executeSpy = vi.spyOn(executor, "execute");
|
||||
|
||||
await executor.resumeOrphaned();
|
||||
|
||||
expect(executeSpy).not.toHaveBeenCalled();
|
||||
expect(mockedCreateHaiAgent).not.toHaveBeenCalled();
|
||||
expect(store.logEntry).not.toHaveBeenCalledWith("FN-1473", "Resumed after engine restart");
|
||||
});
|
||||
|
||||
it("recoverCompletedTask() marks task failed then moves to in-review when workflow fails", async () => {
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue(makeTaskDetail("FN-963", "in-progress", {
|
||||
|
||||
@@ -11,12 +11,14 @@ const {
|
||||
mockSelfHealingStart,
|
||||
mockSelfHealingStop,
|
||||
mockSelfHealingCtor,
|
||||
mockRecoverNoProgressNoTaskDoneFailures,
|
||||
mockRunStartupRecovery,
|
||||
mockExecutorCtor,
|
||||
} = vi.hoisted(() => ({
|
||||
mockSelfHealingStart: vi.fn(),
|
||||
mockSelfHealingStop: vi.fn(),
|
||||
mockSelfHealingCtor: vi.fn(),
|
||||
mockRecoverNoProgressNoTaskDoneFailures: vi.fn().mockResolvedValue(0),
|
||||
mockRunStartupRecovery: vi.fn().mockResolvedValue(undefined),
|
||||
mockExecutorCtor: vi.fn(),
|
||||
}));
|
||||
@@ -97,6 +99,7 @@ vi.mock("../self-healing.js", async () => {
|
||||
return {
|
||||
start: mockSelfHealingStart,
|
||||
stop: mockSelfHealingStop,
|
||||
recoverNoProgressNoTaskDoneFailures: mockRecoverNoProgressNoTaskDoneFailures,
|
||||
runStartupRecovery: mockRunStartupRecovery,
|
||||
};
|
||||
}),
|
||||
@@ -207,6 +210,7 @@ describe("InProcessRuntime", () => {
|
||||
it("runs self-healing startup recovery immediately after orphan resume on startup", async () => {
|
||||
await runtime.start();
|
||||
|
||||
expect(mockRecoverNoProgressNoTaskDoneFailures).toHaveBeenCalledTimes(1);
|
||||
expect(mockRunStartupRecovery).toHaveBeenCalledTimes(1);
|
||||
}, 30000);
|
||||
|
||||
|
||||
@@ -417,7 +417,11 @@ export class InProcessRuntime
|
||||
// 8. Set up event forwarding from TaskStore
|
||||
this.setupEventForwarding();
|
||||
|
||||
// 9. Resume orphaned in-progress tasks
|
||||
// 9. Requeue no-progress no-task_done failures before resumeOrphaned
|
||||
// can restart them.
|
||||
await this.selfHealingManager.recoverNoProgressNoTaskDoneFailures();
|
||||
|
||||
// 10. Resume orphaned in-progress tasks
|
||||
await this.executor.resumeOrphaned();
|
||||
|
||||
// Some "stuck" tasks are already orphaned by the time the runtime boots:
|
||||
@@ -426,10 +430,10 @@ export class InProcessRuntime
|
||||
// SelfHealingManager so the policy lives in one place.
|
||||
await this.selfHealingManager.runStartupRecovery();
|
||||
|
||||
// 10. Start scheduler
|
||||
// 11. Start scheduler
|
||||
this.scheduler.start();
|
||||
|
||||
// 11. Start MissionExecutionLoop for validation cycle handling
|
||||
// 12. Start MissionExecutionLoop for validation cycle handling
|
||||
this.missionExecutionLoop = missionExecutionLoop;
|
||||
if (missionExecutionLoop) {
|
||||
missionExecutionLoop.start();
|
||||
@@ -446,7 +450,7 @@ export class InProcessRuntime
|
||||
void activeMissionAutopilot.recoverMissions(activeMissionStore);
|
||||
}
|
||||
|
||||
// 12. Reconcile feature status for all active missions (not just autopilot)
|
||||
// 13. Reconcile feature status for all active missions (not just autopilot)
|
||||
if (activeMissionStore) {
|
||||
void this.scheduler.reconcileAllMissionFeatures();
|
||||
}
|
||||
|
||||
@@ -29,6 +29,7 @@ import { existsSync } from "node:fs";
|
||||
import { scanOrphanedBranches } from "./worktree-pool.js";
|
||||
|
||||
const mockedExecSync = vi.mocked(execSync);
|
||||
const mockedExistsSync = vi.mocked(existsSync);
|
||||
const mockedScanOrphanedBranches = vi.mocked(scanOrphanedBranches);
|
||||
|
||||
// ── Mock helpers ────────────────────────────────────────────────────
|
||||
@@ -322,6 +323,7 @@ describe("SelfHealingManager", () => {
|
||||
});
|
||||
|
||||
it("runStartupRecovery invokes the startup recovery subset", async () => {
|
||||
const recoverNoProgressNoTaskDoneFailures = vi.spyOn(manager, "recoverNoProgressNoTaskDoneFailures").mockResolvedValue(1);
|
||||
const recoverCompletedTasks = vi.spyOn(manager, "recoverCompletedTasks").mockResolvedValue(1);
|
||||
const recoverMisclassifiedFailures = vi.spyOn(manager, "recoverMisclassifiedFailures").mockResolvedValue(1);
|
||||
const recoverOrphanedExecutions = vi.spyOn(manager, "recoverOrphanedExecutions").mockResolvedValue(1);
|
||||
@@ -329,6 +331,7 @@ describe("SelfHealingManager", () => {
|
||||
|
||||
await manager.runStartupRecovery();
|
||||
|
||||
expect(recoverNoProgressNoTaskDoneFailures).toHaveBeenCalledTimes(1);
|
||||
expect(recoverCompletedTasks).toHaveBeenCalledTimes(1);
|
||||
expect(recoverMisclassifiedFailures).toHaveBeenCalledTimes(1);
|
||||
expect(recoverOrphanedExecutions).toHaveBeenCalledTimes(1);
|
||||
@@ -336,6 +339,115 @@ describe("SelfHealingManager", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("recoverNoProgressNoTaskDoneFailures", () => {
|
||||
it("requeues clean in-progress no-task_done failures with no step progress", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
getExecutingTaskIds: () => new Set<string>(),
|
||||
});
|
||||
vi.spyOn(managerWithRecovery as any, "hasRecoverableGitWork").mockReturnValue(false);
|
||||
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-1473",
|
||||
column: "in-progress",
|
||||
status: "failed",
|
||||
error: "Agent finished without calling task_done (after retry)",
|
||||
paused: false,
|
||||
steps: [],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverNoProgressNoTaskDoneFailures();
|
||||
|
||||
expect(result).toBe(1);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-1473", {
|
||||
status: "stuck-killed",
|
||||
worktree: null,
|
||||
branch: null,
|
||||
});
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-1473",
|
||||
expect.stringContaining("no-progress no-task_done failure"),
|
||||
);
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-1473", "todo");
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("skips no-task_done failures with step progress", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
getExecutingTaskIds: () => new Set<string>(),
|
||||
});
|
||||
vi.spyOn(managerWithRecovery as any, "hasRecoverableGitWork").mockReturnValue(false);
|
||||
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-1473",
|
||||
column: "in-progress",
|
||||
status: "failed",
|
||||
error: "Agent finished without calling task_done (after retry)",
|
||||
paused: false,
|
||||
steps: [{ status: "done" }, { status: "pending" }],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverNoProgressNoTaskDoneFailures();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("FN-1473", expect.anything());
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-1473", "todo");
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("skips when git work should be preserved", async () => {
|
||||
const managerWithRecovery = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
getExecutingTaskIds: () => new Set<string>(),
|
||||
});
|
||||
vi.spyOn(managerWithRecovery as any, "hasRecoverableGitWork").mockReturnValue(true);
|
||||
|
||||
(store.listTasks as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||
{
|
||||
id: "FN-1473",
|
||||
column: "in-progress",
|
||||
status: "failed",
|
||||
error: "Agent finished without calling task_done (after retry)",
|
||||
paused: false,
|
||||
steps: [{ status: "pending" }],
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await managerWithRecovery.recoverNoProgressNoTaskDoneFailures();
|
||||
|
||||
expect(result).toBe(0);
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("FN-1473", expect.anything());
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-1473", "todo");
|
||||
|
||||
managerWithRecovery.stop();
|
||||
});
|
||||
|
||||
it("treats dirty worktrees as recoverable git work", () => {
|
||||
const task = {
|
||||
id: "FN-1473",
|
||||
worktree: "/tmp/test-project/.worktrees/fn-1473",
|
||||
branch: "fusion/fn-1473",
|
||||
} as Task;
|
||||
mockedExistsSync.mockReturnValue(true);
|
||||
mockedExecSync.mockImplementation((command) => {
|
||||
if (String(command) === "git status --porcelain") {
|
||||
return " M packages/engine/src/executor.ts\n" as any;
|
||||
}
|
||||
return "" as any;
|
||||
});
|
||||
|
||||
expect((manager as any).hasRecoverableGitWork(task)).toBe(true);
|
||||
mockedExecSync.mockClear();
|
||||
});
|
||||
});
|
||||
|
||||
// ── cleanupOrphanedBranches ────────────────────────────────────────
|
||||
|
||||
describe("cleanupOrphanedBranches", () => {
|
||||
|
||||
@@ -103,6 +103,7 @@ export class SelfHealingManager {
|
||||
* stale in-progress/specifying tasks that no longer have a live worker.
|
||||
*/
|
||||
async runStartupRecovery(): Promise<void> {
|
||||
await this.recoverNoProgressNoTaskDoneFailures();
|
||||
await this.recoverCompletedTasks();
|
||||
await this.recoverMisclassifiedFailures();
|
||||
await this.recoverOrphanedExecutions();
|
||||
@@ -342,6 +343,7 @@ export class SelfHealingManager {
|
||||
await this.recoverMergeableReviewTasks();
|
||||
await this.recoverMergedReviewTasks();
|
||||
await this.recoverMisclassifiedFailures();
|
||||
await this.recoverNoProgressNoTaskDoneFailures();
|
||||
await this.recoverOrphanedExecutions();
|
||||
await this.recoverApprovedTriageTasks();
|
||||
await this.archiveStaleDoneTasks();
|
||||
@@ -675,6 +677,109 @@ export class SelfHealingManager {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Recover `in-progress` tasks that failed only because the agent exited
|
||||
* without calling task_done, and where there is no sign of work to preserve.
|
||||
*
|
||||
* These are safe to requeue automatically when no steps progressed and git
|
||||
* has neither worktree changes nor branch commits. Cases with any evidence
|
||||
* of work are left alone for manual inspection or the normal orphan recovery
|
||||
* path.
|
||||
*/
|
||||
async recoverNoProgressNoTaskDoneFailures(): Promise<number> {
|
||||
try {
|
||||
const tasks = await this.store.listTasks({ slim: true, column: "in-progress" });
|
||||
const executingIds = this.options.getExecutingTaskIds?.() ?? new Set<string>();
|
||||
|
||||
const candidates = tasks.filter((task) =>
|
||||
task.column === "in-progress" &&
|
||||
task.status === "failed" &&
|
||||
isNoTaskDoneFailure(task) &&
|
||||
!task.paused &&
|
||||
!executingIds.has(task.id) &&
|
||||
!isTaskWorkComplete(task) &&
|
||||
!hasStepProgress(task),
|
||||
);
|
||||
|
||||
if (candidates.length === 0) return 0;
|
||||
|
||||
log.warn(`Found ${candidates.length} no-progress no-task_done failure(s) in in-progress`);
|
||||
|
||||
let recovered = 0;
|
||||
for (const task of candidates) {
|
||||
try {
|
||||
if (this.hasRecoverableGitWork(task)) {
|
||||
log.log(`${task.id} has recoverable git work — leaving in-progress for inspection`);
|
||||
continue;
|
||||
}
|
||||
|
||||
await this.store.updateTask(task.id, {
|
||||
status: "stuck-killed",
|
||||
worktree: null,
|
||||
branch: null,
|
||||
});
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
"Auto-recovered no-progress no-task_done failure — clean worktree, moved back to todo",
|
||||
);
|
||||
await this.store.moveTask(task.id, "todo");
|
||||
recovered++;
|
||||
} catch (err: any) {
|
||||
log.error(`Failed to recover no-progress no-task_done failure ${task.id}: ${err.message}`);
|
||||
}
|
||||
}
|
||||
|
||||
if (recovered > 0) {
|
||||
log.log(`Recovered ${recovered} no-progress no-task_done failure(s) → todo`);
|
||||
}
|
||||
return recovered;
|
||||
} catch (err: any) {
|
||||
log.error(`No-progress no-task_done recovery failed: ${err.message}`);
|
||||
return 0;
|
||||
}
|
||||
}
|
||||
|
||||
private hasRecoverableGitWork(task: Task): boolean {
|
||||
if (task.worktree && existsSync(task.worktree)) {
|
||||
try {
|
||||
const status = execSync("git status --porcelain", {
|
||||
cwd: task.worktree,
|
||||
stdio: "pipe",
|
||||
encoding: "utf-8",
|
||||
timeout: 30_000,
|
||||
}).trim();
|
||||
if (status.length > 0) return true;
|
||||
} catch {
|
||||
// If we cannot inspect an existing worktree, preserve it.
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
const branchName = task.branch || `fusion/${task.id.toLowerCase()}`;
|
||||
try {
|
||||
execSync(`git rev-parse --verify "${branchName}"`, {
|
||||
cwd: this.options.rootDir,
|
||||
stdio: "pipe",
|
||||
timeout: 30_000,
|
||||
});
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
|
||||
try {
|
||||
const uniqueCommits = execSync(`git rev-list --count HEAD.."${branchName}"`, {
|
||||
cwd: this.options.rootDir,
|
||||
stdio: "pipe",
|
||||
encoding: "utf-8",
|
||||
timeout: 30_000,
|
||||
}).trim();
|
||||
return Number.parseInt(uniqueCommits, 10) > 0;
|
||||
} catch {
|
||||
// If the branch exists but cannot be compared, preserve it.
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Recover triage tasks that already have an approved specification but were
|
||||
* left stuck in `status: "specifying"` without an active triage session.
|
||||
@@ -907,3 +1012,11 @@ function isTaskWorkComplete(task: Task): boolean {
|
||||
if (task.steps.length === 0) return false;
|
||||
return task.steps.every((step) => step.status === "done" || step.status === "skipped");
|
||||
}
|
||||
|
||||
function isNoTaskDoneFailure(task: Task): boolean {
|
||||
return task.error?.includes("without calling task_done") === true;
|
||||
}
|
||||
|
||||
function hasStepProgress(task: Task): boolean {
|
||||
return task.steps.some((step) => step.status !== "pending");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user