fix(FN-1284): move terminal execution failures to in-review

- Move single-session and step-session executor failure paths to in-review after marking tasks failed
- Route exhausted transient recovery retries to in-review instead of leaving tasks outside review flow
- Move stuck-kill budget exhaustion failures in self-healing to in-review and update failure log wording
- Add regression tests in executor and self-healing suites to verify in-review transitions on these failure states
This commit is contained in:
gsxdsm
2026-04-08 11:01:04 -07:00
parent 4f48d2d186
commit 03ac3258a7
4 changed files with 41 additions and 7 deletions

View File

@@ -254,6 +254,7 @@ describe("TaskExecutor with semaphore", () => {
});
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: "failed", error: expect.any(String) });
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "in-review");
expect(onError).toHaveBeenCalled();
});
@@ -4909,7 +4910,7 @@ describe("TaskExecutor bounded recovery retries", () => {
expect(onError).not.toHaveBeenCalled();
});
it("escalates to failure when recovery retries are exhausted", async () => {
it("moves task to in-review when transient retries are exhausted (single-session)", async () => {
const store = createMockStore();
const onError = vi.fn();
@@ -4932,12 +4933,13 @@ describe("TaskExecutor bounded recovery retries", () => {
updatedAt: new Date().toISOString(),
});
expect(store.updateTask).toHaveBeenCalledWith("FN-001", expect.objectContaining({
expect(store.updateTask).toHaveBeenCalledWith("FN-001", {
status: "failed",
error: "socket hang up",
recoveryRetryCount: null,
nextRecoveryAt: null,
}));
});
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "in-review");
expect(store.moveTask).not.toHaveBeenCalledWith("FN-001", "todo");
expect(onError).toHaveBeenCalled();
});
@@ -8591,7 +8593,7 @@ describe("StepSessionExecutor integration", () => {
expect(onComplete).not.toHaveBeenCalled();
});
it("exception from executeAll marks task as failed", async () => {
it("moves task to in-review when step-session execution fails", async () => {
const store = createStepSessionStore();
mockExecuteAll.mockRejectedValue(new Error("Infrastructure failure"));
@@ -8605,6 +8607,28 @@ describe("StepSessionExecutor integration", () => {
status: "failed",
error: "Infrastructure failure",
}));
expect(store.moveTask).toHaveBeenCalledWith("FN-200", "in-review");
expect(onError).toHaveBeenCalled();
});
it("moves task to in-review when transient retries are exhausted (step-session)", async () => {
const store = createStepSessionStore();
mockExecuteAll.mockRejectedValue(new Error("socket hang up"));
const onError = vi.fn();
const executor = new TaskExecutor(store, "/tmp/test", { onError });
await executor.execute(createTaskWithSteps({ recoveryRetryCount: 3 }));
expect(store.updateTask).toHaveBeenCalledWith("FN-200", {
status: "failed",
error: "socket hang up",
recoveryRetryCount: null,
nextRecoveryAt: null,
});
expect(store.moveTask).toHaveBeenCalledWith("FN-200", "in-review");
expect(store.moveTask).not.toHaveBeenCalledWith("FN-200", "todo");
expect(onError).toHaveBeenCalled();
});

View File

@@ -975,11 +975,15 @@ export class TaskExecutor {
recoveryRetryCount: null,
nextRecoveryAt: null,
});
await this.store.moveTask(task.id, "in-review");
executorLog.log(`${task.id} transient retries exhausted → in-review`);
this.options.onError?.(task, err);
} else {
executorLog.error(`${task.id} step-session execution failed:`, err.message);
await this.store.logEntry(task.id, `Step-session execution failed: ${err.message}`);
await this.store.updateTask(task.id, { status: "failed", error: err.message });
await this.store.moveTask(task.id, "in-review");
executorLog.log(`${task.id} step-session execution failed → in-review`);
this.options.onError?.(task, err);
}
} finally {
@@ -1514,12 +1518,16 @@ export class TaskExecutor {
recoveryRetryCount: null,
nextRecoveryAt: null,
});
await this.store.moveTask(task.id, "in-review");
executorLog.log(`${task.id} transient retries exhausted → in-review`);
this.options.onError?.(task, err);
return;
}
executorLog.error(`${task.id} execution failed:`, err.message);
await this.store.logEntry(task.id, `Execution failed: ${err.message}`);
await this.store.updateTask(task.id, { status: "failed", error: err.message });
await this.store.moveTask(task.id, "in-review");
executorLog.log(`${task.id} execution failed → in-review`);
this.options.onError?.(task, err);
}
} finally {

View File

@@ -210,7 +210,7 @@ describe("SelfHealingManager", () => {
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { stuckKillCount: 3 });
});
it("returns false and marks failed when budget exceeded", async () => {
it("moves task to in-review when stuck-kill budget is exhausted", async () => {
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue({
id: "FN-001",
stuckKillCount: 6,
@@ -226,9 +226,10 @@ describe("SelfHealingManager", () => {
status: "failed",
error: expect.stringContaining("exceeded maximum of 6"),
});
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "in-review");
expect(store.logEntry).toHaveBeenCalledWith(
"FN-001",
expect.stringContaining("Permanently failed"),
expect.stringContaining("moved to in-review"),
);
});

View File

@@ -213,9 +213,10 @@ export class SelfHealingManager {
status: "failed",
error: `Task stuck ${newCount} times — exceeded maximum of ${maxKills} stuck kills`,
});
await this.store.moveTask(taskId, "in-review");
await this.store.logEntry(
taskId,
`Permanently failed: agent stuck ${newCount} times (max: ${maxKills})`,
`Permanently failed: agent stuck ${newCount} times (max: ${maxKills}) — moved to in-review`,
);
return false;
}