root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(engine): stuck-requeue no longer clobbers concurrently-recovered tasks

Browse Source 
When SelfHealingManager.recoverCompletedTasks moved a task from
in-progress to in-review, the executor's stuck-kill cleanup running in
execute()'s finally block could fire 20s later, see a stale captured
task.column = "in-progress", and overwrite the recovery by tearing down
the worktree and moving the task back to todo with all step progress
reset. Both the outer-finally and step-session requeue blocks (and the
force-requeue setTimeout in markStuckAborted) now re-read the latest
column and skip cleanup entirely if the task has moved past
in-progress/todo.

Adds a new preserveProgressOnStuckRequeue setting (default: true,
toggle in Settings near the Stuck Task Timeout) so stuck-requeue passes
{ preserveProgress: true } to moveTask. Completed step statuses now
survive the bounce so the agent resumes from where it left off instead
of restarting every step from pending.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-05-06 09:07:52 -07:00
parent d9a3241f19
commit 0d1591665a
6 changed files with 241 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
.changeset/stuck-requeue-recovery-race.md Normal file
View File

@@ -0,0 +1,18 @@
---
"@runfusion/fusion": patch
---
Fix a race in the stuck-task requeue path that could clobber a task back to
`todo` (with all step progress reset and worktree torn down) immediately
after `SelfHealingManager.recoverCompletedTasks` had already moved it to
`in-review`. The executor's stuck-kill cleanup ran in `execute()`'s
`finally` block and used a stale captured `task.column` snapshot, so it
would happily overwrite a fresh recovery. The cleanup now re-reads the
latest column and skips entirely when the task has moved past
`in-progress`/`todo`.
Also adds a new setting `preserveProgressOnStuckRequeue` (default: `true`,
toggle in Settings → Engine, near "Stuck Task Timeout"). When enabled, the
stuck detector's requeue passes `{ preserveProgress: true }` to `moveTask`
so completed step statuses survive the bounce and the agent can resume
from where it left off instead of restarting every step from pending.

1
packages/core/src/settings-schema.ts
View File

@@ -216,6 +216,7 @@ export const DEFAULT_PROJECT_SETTINGS = {
autoUnpauseBaseDelayMs: 300_000,
autoUnpauseMaxDelayMs: 3_600_000,
maxStuckKills: 6,
preserveProgressOnStuckRequeue: true,
maxPostReviewFixes: 1,
maxSpawnedAgentsPerParent: 5,
maxSpawnedAgentsGlobal: 20,

6
packages/core/src/types.ts
View File

@@ -1971,6 +1971,12 @@ export interface ProjectSettings {
/** Maximum number of times the stuck-task detector can kill and re-queue a task
* before it is marked as permanently failed. Default: 6. */
maxStuckKills?: number;
/** When the stuck-task detector kills and re-queues a task, preserve the
* task's step progress (step statuses + currentStep) instead of resetting
* every step to `pending`. The worktree and branch are still cleared so
* the retry gets a fresh checkout, but completed steps stay completed so
* the agent can resume from where it left off. Default: true. */
preserveProgressOnStuckRequeue?: boolean;
/** Maximum number of times the self-healing manager may auto-revive a task parked
* in `in-review` with a failed pre-merge workflow step. Each revival injects the
* failure feedback into `PROMPT.md`, resets steps, and sends the task back through

14
packages/dashboard/app/components/SettingsModal.tsx
View File

@@ -2904,6 +2904,20 @@ export function SettingsModal({
/>
<small>Timeout in minutes for detecting stuck tasks. When a task&apos;s agent session shows no activity for longer than this duration, the task is terminated and retried. Leave empty to disable. Suggested: 10.</small>
</div>
<div className="form-group">
<label htmlFor="preserveProgressOnStuckRequeue" className="checkbox-label">
<input
id="preserveProgressOnStuckRequeue"
type="checkbox"
checked={form.preserveProgressOnStuckRequeue !== false}
onChange={(e) =>
setForm((f) => ({ ...f, preserveProgressOnStuckRequeue: e.target.checked }))
}
/>
Preserve step progress on stuck-task requeue
</label>
<small>When the stuck detector kills and re-queues a task, keep completed step statuses so the agent can resume from where it left off. Disable to reset every step to pending on each stuck retry. Default: enabled.</small>
</div>
<div className="form-group">
<label htmlFor="specStalenessEnabled" className="checkbox-label">
<input

110
packages/engine/src/__tests__/executor.test.ts
View File

@@ -6424,7 +6424,7 @@ describe("TaskExecutor bounded recovery retries", () => {
expect(store.moveTask).not.toHaveBeenCalledWith("FN-001", "in-review");
// Executor now handles the requeue in its finally block
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: "stuck-killed", worktree: null, branch: null });
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo");
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo", { preserveProgress: true });
});
it("does not requeue when stuck-kill budget is exhausted", async () => {
@@ -6464,14 +6464,26 @@ describe("TaskExecutor bounded recovery retries", () => {
);
});
it("skips moveTask when task is already in todo at execute start", async () => {
it("skips stuck-requeue cleanup when task was concurrently recovered to in-review", async () => {
const store = createMockStore();
// Self-healing already moved the task to in-review while execute() was unwinding.
store.getTask.mockResolvedValue({
id: "FN-001",
title: "Test",
description: "Test",
column: "in-review",
dependencies: [],
steps: [{ name: "step", status: "done" }],
currentStep: 1,
log: [],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
});
const executor = new TaskExecutor(store, "/tmp/test", {});
mockedCreateFnAgent.mockImplementation(async () => ({
session: {
prompt: vi.fn(async () => {
// Simulate stuck kill
executor.markStuckAborted("FN-001", true);
throw new Error("Stuck task");
}),
@@ -6484,7 +6496,44 @@ describe("TaskExecutor bounded recovery retries", () => {
id: "FN-001",
title: "Test",
description: "Test",
column: "todo", // Task was already in todo when execute started
column: "in-progress",
dependencies: [],
steps: [{ name: "step", status: "done" }],
currentStep: 1,
log: [],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
});
// Must NOT undo the recovery: no move, no stuck-killed status, no worktree clearing.
expect(store.moveTask).not.toHaveBeenCalledWith("FN-001", "todo");
expect(store.moveTask).not.toHaveBeenCalledWith("FN-001", "todo", expect.anything());
expect(store.updateTask).not.toHaveBeenCalledWith(
"FN-001",
{ status: "stuck-killed", worktree: null, branch: null },
);
});
it("preserves step progress when requeuing stuck task by default", async () => {
const store = createMockStore();
const executor = new TaskExecutor(store, "/tmp/test", {});
const resetSpy = vi.spyOn(executor as any, "resetStepsIfWorkLost").mockResolvedValue(undefined);
mockedCreateFnAgent.mockImplementation(async () => ({
session: {
prompt: vi.fn(async () => {
executor.markStuckAborted("FN-001", true);
}),
dispose: vi.fn(),
state: {},
},
}) as any);
await executor.execute({
id: "FN-001",
title: "Test",
description: "Test",
column: "in-progress",
dependencies: [],
steps: [],
currentStep: 0,
@@ -6493,10 +6542,52 @@ describe("TaskExecutor bounded recovery retries", () => {
updatedAt: new Date().toISOString(),
});
// Should NOT call moveTask because task is already in todo
expect(store.moveTask).not.toHaveBeenCalledWith("FN-001", "todo");
// Should still clean up and mark as stuck-killed
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: "stuck-killed", worktree: null, branch: null });
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo", { preserveProgress: true });
// resetStepsIfWorkLost MUST be skipped when preserveProgress is on, otherwise
// the requeue would silently drop committed step status before moveTask preserves it.
expect(resetSpy).not.toHaveBeenCalled();
});
it("resets step progress when preserveProgressOnStuckRequeue is disabled", async () => {
const store = createMockStore();
store.getSettings.mockResolvedValue({
maxConcurrent: 2,
maxWorktrees: 4,
pollIntervalMs: 15000,
groupOverlappingFiles: false,
autoMerge: false,
worktreeInitCommand: undefined,
preserveProgressOnStuckRequeue: false,
});
const executor = new TaskExecutor(store, "/tmp/test", {});
const resetSpy = vi.spyOn(executor as any, "resetStepsIfWorkLost").mockResolvedValue(undefined);
mockedCreateFnAgent.mockImplementation(async () => ({
session: {
prompt: vi.fn(async () => {
executor.markStuckAborted("FN-001", true);
}),
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(),
});
// No options arg → moveTask defaults to resetting steps
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo", undefined);
expect(resetSpy).toHaveBeenCalledTimes(1);
});
it("clears recovery metadata after successful run completes", async () => {
@@ -12490,7 +12581,7 @@ describe("StepSessionExecutor integration", () => {
worktree: null,
branch: null,
}));
expect(store.moveTask).toHaveBeenCalledWith("FN-200", "todo");
expect(store.moveTask).toHaveBeenCalledWith("FN-200", "todo", { preserveProgress: true });
});
it("REGRESSION: stuck-kill with exhausted budget does not requeue step-session task", async () => {
@@ -12516,6 +12607,7 @@ describe("StepSessionExecutor integration", () => {
// Should NOT move to todo or mark as stuck-killed
expect(store.moveTask).not.toHaveBeenCalledWith("FN-200", "todo");
expect(store.moveTask).not.toHaveBeenCalledWith("FN-200", "todo", expect.anything());
expect(store.updateTask).not.toHaveBeenCalledWith("FN-200", expect.objectContaining({
status: "stuck-killed",
}));

141
packages/engine/src/executor.ts
View File

@@ -2739,22 +2739,37 @@ export class TaskExecutor {
// Stuck-requeue: clean up worktree and move to todo
if (stuckRequeue === true) {
try {
// Reset steps whose work was never committed before destroying the worktree
// Re-read latest task state. Self-healing may have already moved
// the task out of in-progress while this step-session execution
// was unwinding; continuing the cleanup would clobber a valid
// recovery (see the analogous block in the outer finally for the
// full reasoning).
const latestTask = await this.store.getTask(task.id);
await this.resetStepsIfWorkLost(latestTask);
if (latestTask.column !== "in-progress" && latestTask.column !== "todo") {
executorLog.log(
`${task.id} stuck-requeue skipped — task is now in '${latestTask.column}' (recovered concurrently)`,
);
} else {
const settings = await this.store.getSettings();
const preserveProgress = settings.preserveProgressOnStuckRequeue !== false;
if (worktreePath && existsSync(worktreePath)) {
try {
await execAsync(`git worktree remove "${worktreePath}" --force`, { cwd: this.rootDir });
} catch (wtErr: unknown) {
const msg = wtErr instanceof Error ? wtErr.message : String(wtErr);
executorLog.warn(`${task.id}: worktree removal failed during stuck-requeue cleanup (${worktreePath}): ${msg}`);
if (!preserveProgress) {
await this.resetStepsIfWorkLost(latestTask);
}
if (worktreePath && existsSync(worktreePath)) {
try {
await execAsync(`git worktree remove "${worktreePath}" --force`, { cwd: this.rootDir });
} catch (wtErr: unknown) {
const msg = wtErr instanceof Error ? wtErr.message : String(wtErr);
executorLog.warn(`${task.id}: worktree removal failed during stuck-requeue cleanup (${worktreePath}): ${msg}`);
}
}
await this.store.updateTask(task.id, { status: "stuck-killed", worktree: null, branch: null });
if (latestTask.column !== "todo") {
await this.store.moveTask(task.id, "todo", preserveProgress ? { preserveProgress: true } : undefined);
executorLog.log(`${task.id} moved to todo for retry after stuck kill${preserveProgress ? " (progress preserved)" : ""}`);
}
}
await this.store.updateTask(task.id, { status: "stuck-killed", worktree: null, branch: null });
if (task.column !== "todo") {
await this.store.moveTask(task.id, "todo");
executorLog.log(`${task.id} moved to todo for retry after stuck kill`);
}
} catch (err: unknown) {
const errorMessage = err instanceof Error ? err.message : String(err);
@@ -3688,33 +3703,55 @@ export class TaskExecutor {
// task in "in-progress" with no active session or worktree.
if (stuckRequeue === true) {
try {
// Reset steps whose work was never committed before destroying the worktree
// Re-read latest task state. While this execute() invocation was
// unwinding, self-healing (e.g. recoverCompletedTasks) may have
// already transitioned the task to in-review or done. Continuing
// the stuck-requeue cleanup in that case would destroy the worktree
// the recovery now relies on and clobber the task back to todo with
// all step progress reset, undoing valid completion. Skip the
// entire cleanup if the column has moved on past in-progress/todo.
const latestTask = await this.store.getTask(task.id);
await this.resetStepsIfWorkLost(latestTask);
// Clean up the old worktree so the retry gets a fresh one
if (worktreePath && existsSync(worktreePath)) {
try {
await execAsync(`git worktree remove "${worktreePath}" --force`, { cwd: this.rootDir });
executorLog.log(`Removed old worktree for stuck-killed retry: ${worktreePath}`);
// Audit trail: record worktree removal (FN-1404)
await audit.git({ type: "worktree:remove", target: worktreePath });
} catch (cleanupErr: unknown) {
const cleanupErrMessage = cleanupErr instanceof Error ? cleanupErr.message : String(cleanupErr);
executorLog.warn(`Failed to remove old worktree ${worktreePath}: ${cleanupErrMessage}`);
}
}
await this.store.updateTask(task.id, { status: "stuck-killed", worktree: null, branch: null });
// Only move to todo if not already there. The task.column check uses the
// captured task object from execute() start — if the task was already in "todo"
// when execute() started (e.g., resumed orphan), we skip the redundant move.
if (task.column !== "todo") {
await this.store.moveTask(task.id, "todo");
// Audit trail: record task move (FN-1404)
await audit.database({ type: "task:move", target: task.id, metadata: { to: "todo" } });
executorLog.log(`${task.id} moved to todo for retry after stuck kill`);
if (latestTask.column !== "in-progress" && latestTask.column !== "todo") {
executorLog.log(
`${task.id} stuck-requeue skipped — task is now in '${latestTask.column}' (recovered concurrently)`,
);
} else {
executorLog.log(`${task.id} already in todo — skipping redundant move`);
const settings = await this.store.getSettings();
const preserveProgress = settings.preserveProgressOnStuckRequeue !== false;
// Reset steps whose work was never committed before destroying
// the worktree. Skipped when preserveProgress is on — the
// setting's whole point is to keep step status across the
// requeue so the agent can resume from where it left off.
if (!preserveProgress) {
await this.resetStepsIfWorkLost(latestTask);
}
// Clean up the old worktree so the retry gets a fresh one
if (worktreePath && existsSync(worktreePath)) {
try {
await execAsync(`git worktree remove "${worktreePath}" --force`, { cwd: this.rootDir });
executorLog.log(`Removed old worktree for stuck-killed retry: ${worktreePath}`);
// Audit trail: record worktree removal (FN-1404)
await audit.git({ type: "worktree:remove", target: worktreePath });
} catch (cleanupErr: unknown) {
const cleanupErrMessage = cleanupErr instanceof Error ? cleanupErr.message : String(cleanupErr);
executorLog.warn(`Failed to remove old worktree ${worktreePath}: ${cleanupErrMessage}`);
}
}
await this.store.updateTask(task.id, { status: "stuck-killed", worktree: null, branch: null });
// Only move to todo if not already there. Use the freshly-read
// latestTask.column rather than the stale captured task.column —
// the captured snapshot can be hours old and would race against
// any concurrent recovery (see comment above).
if (latestTask.column !== "todo") {
await this.store.moveTask(task.id, "todo", preserveProgress ? { preserveProgress: true } : undefined);
// Audit trail: record task move (FN-1404)
await audit.database({ type: "task:move", target: task.id, metadata: { to: "todo" } });
executorLog.log(`${task.id} moved to todo for retry after stuck kill${preserveProgress ? " (progress preserved)" : ""}`);
} else {
executorLog.log(`${task.id} already in todo — skipping redundant move`);
}
}
} catch (err: unknown) {
const errorMessage = err instanceof Error ? err.message : String(err);
@@ -6570,17 +6607,41 @@ and show an appropriate message to the user.\`
const FORCE_REQUEUE_GRACE_MS = 60_000; // 60 s — generous, but bounded
setTimeout(async () => {
if (!this.executing.has(taskId)) return; // executor unwound normally — nothing to do
// Re-check the latest column: self-healing may have already moved the
// task out of in-progress (e.g. recoverCompletedTasks → in-review).
// Force-requeueing in that case would clobber a valid recovery, undo
// the worktree/branch state that recovery now relies on, and reset
// step progress.
let latestColumn: string | undefined;
try {
const latestTask = await this.store.getTask(taskId);
latestColumn = latestTask.column;
} catch (err: unknown) {
executorLog.warn(
`${taskId} force-requeue could not read latest task state: ${err instanceof Error ? err.message : String(err)}`,
);
}
if (latestColumn && latestColumn !== "in-progress") {
executorLog.log(
`${taskId} force-requeue skipped — task is now in '${latestColumn}' (recovered concurrently)`,
);
this.executing.delete(taskId);
this.stuckAborted.delete(taskId);
return;
}
executorLog.warn(
`${taskId} still executing ${FORCE_REQUEUE_GRACE_MS / 1000}s after stuck-kill signal ` +
`(likely a hung subprocess) — force-requeueing`,
);
try {
const settings = await this.store.getSettings();
const preserveProgress = settings.preserveProgressOnStuckRequeue !== false;
await this.store.logEntry(
taskId,
`Force-requeued after stuck-kill: executor did not unwind within ${FORCE_REQUEUE_GRACE_MS / 1000}s (hung subprocess)`,
`Force-requeued after stuck-kill: executor did not unwind within ${FORCE_REQUEUE_GRACE_MS / 1000}s (hung subprocess)${preserveProgress ? " — progress preserved" : ""}`,
);
await this.store.updateTask(taskId, { status: "stuck-killed", worktree: null, branch: null });
await this.store.moveTask(taskId, "todo");
await this.store.moveTask(taskId, "todo", preserveProgress ? { preserveProgress: true } : undefined);
// Remove from executing so the scheduler can re-dispatch normally.
// The old Promise is still running but the executing guard is cleared so
// a fresh execute() call won't be blocked.