fix(FN-874): recover stuck-killed tasks reliably
This commit is contained in:
@@ -4563,6 +4563,41 @@ describe("TaskExecutor bounded recovery retries", () => {
|
||||
}));
|
||||
});
|
||||
|
||||
it("exits cleanly when a stuck-killed session resolves without throwing", async () => {
|
||||
const store = createMockStore();
|
||||
const executor = new TaskExecutor(store, "/tmp/test", {});
|
||||
|
||||
mockedCreateHaiAgent.mockImplementation(async () => ({
|
||||
session: {
|
||||
prompt: vi.fn(async () => {
|
||||
executor.markStuckAborted("FN-001");
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
state: {},
|
||||
},
|
||||
}) as any);
|
||||
|
||||
await executor.execute({
|
||||
id: "FN-001",
|
||||
title: "Test",
|
||||
description: "Test",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
});
|
||||
|
||||
expect(mockedCreateHaiAgent).toHaveBeenCalledTimes(1);
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
expect.objectContaining({ status: "failed" }),
|
||||
);
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-001", "in-review");
|
||||
});
|
||||
|
||||
it("clears recovery metadata after successful run completes", async () => {
|
||||
const store = createMockStore();
|
||||
|
||||
|
||||
@@ -668,6 +668,14 @@ export class TaskExecutor {
|
||||
return;
|
||||
}
|
||||
|
||||
// If the stuck task detector disposed the session and the agent exited
|
||||
// cleanly, stop here. The detector already handled recovery/re-queueing.
|
||||
if (this.stuckAborted.has(task.id)) {
|
||||
this.stuckAborted.delete(task.id);
|
||||
executorLog.log(`${task.id} terminated by stuck task detector (graceful session exit)`);
|
||||
return;
|
||||
}
|
||||
|
||||
if (taskDone) {
|
||||
// Capture modified files before running workflow steps
|
||||
const updatedTask = await this.store.getTask(task.id);
|
||||
|
||||
@@ -431,6 +431,30 @@ describe("StuckTaskDetector", () => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("marks the abort via onStuck before disposing the session", async () => {
|
||||
let onStuckCalled = false;
|
||||
const onStuck = vi.fn(() => {
|
||||
onStuckCalled = true;
|
||||
});
|
||||
const customDetector = new StuckTaskDetector(store, { onStuck });
|
||||
const session = {
|
||||
dispose: vi.fn(() => {
|
||||
expect(onStuckCalled).toBe(true);
|
||||
}),
|
||||
};
|
||||
|
||||
customDetector.trackTask("FN-001", session);
|
||||
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true });
|
||||
vi.advanceTimersByTime(61000);
|
||||
|
||||
await customDetector.killAndRetry("FN-001", 60000);
|
||||
|
||||
expect(onStuck).toHaveBeenCalledTimes(1);
|
||||
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("calls onStuck with loop reason and activity count", async () => {
|
||||
const onStuck = vi.fn();
|
||||
const customDetector = new StuckTaskDetector(store, { onStuck });
|
||||
|
||||
@@ -244,16 +244,6 @@ export class StuckTaskDetector {
|
||||
`${activitySinceProgress} events since last progress)`,
|
||||
);
|
||||
|
||||
// Dispose the agent session first
|
||||
try {
|
||||
entry.session.dispose();
|
||||
} catch (err) {
|
||||
stuckLog.error(`Failed to dispose session for ${taskId}:`, err);
|
||||
}
|
||||
|
||||
// Remove from tracking
|
||||
this.tracked.delete(taskId);
|
||||
|
||||
// Log the event to the task log
|
||||
try {
|
||||
await this.store.logEntry(
|
||||
@@ -276,6 +266,20 @@ export class StuckTaskDetector {
|
||||
activitySinceProgress,
|
||||
};
|
||||
|
||||
// Notify listeners before disposing the session so executor cleanup can
|
||||
// mark the abort as intentional before the disposed session unwinds.
|
||||
this.onStuck?.(event);
|
||||
|
||||
// Dispose the agent session after listeners have marked the abort.
|
||||
try {
|
||||
entry.session.dispose();
|
||||
} catch (err) {
|
||||
stuckLog.error(`Failed to dispose session for ${taskId}:`, err);
|
||||
}
|
||||
|
||||
// Remove from tracking
|
||||
this.tracked.delete(taskId);
|
||||
|
||||
// Check stuck kill budget before re-queuing (SelfHealingManager integration).
|
||||
// If beforeRequeue returns false, the task has been marked failed — skip re-queue.
|
||||
if (this.beforeRequeue) {
|
||||
@@ -283,7 +287,6 @@ export class StuckTaskDetector {
|
||||
const shouldRequeue = await this.beforeRequeue(taskId);
|
||||
if (!shouldRequeue) {
|
||||
stuckLog.log(`${taskId} exceeded stuck kill budget — not re-queuing`);
|
||||
this.onStuck?.(event);
|
||||
return;
|
||||
}
|
||||
} catch (err) {
|
||||
@@ -304,8 +307,6 @@ export class StuckTaskDetector {
|
||||
stuckLog.error(`Failed to move ${taskId} to todo:`, err);
|
||||
}
|
||||
|
||||
// Notify listeners
|
||||
this.onStuck?.(event);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user