fix(engine): preserve task progress when a single-session run is hard-cancelled
When the engine aborted in-flight work mid-execution and bounced the task back to `todo`, the single-session teardown cleared the task `branch` and re-queued without `preserveResumeState` — resetting every step to `pending` (store.moveTaskInternal resetAllStepsToPending) and dropping the pointer to the commits already on the task branch. The next dispatch then re-planned from Step 0 and the committed work was stranded, observed as FN-6722 losing all its progress and getting stuck in todo. The teardown now keeps the branch and moves with `preserveResumeState` whenever the task has resumable step progress, matching the sibling step-session (executor ~8065) and pause-park (executor ~1826) paths, so execute() resumes onto the existing branch from the first incomplete step. The worktree is still removed to free its concurrency slot (FN-6782) — only the durable pointers (branch + step state) are kept. Adds a regression test driving the exact single-session catch teardown. Fusion-Task-Id: FN-6722 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
5
.changeset/preserve-progress-on-pause-abort.md
Normal file
5
.changeset/preserve-progress-on-pause-abort.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
Preserve task progress when a single-session run is hard-cancelled mid-execution. When the engine aborted in-flight work and bounced the task back to `todo`, the single-session teardown cleared the task `branch` and re-queued without `preserveResumeState` — resetting every step to `pending` and dropping the pointer to commits already on the task branch, so the next dispatch re-planned from Step 0 and the committed work was stranded (observed as a task that "lost all progress" and got stuck). The teardown now keeps the branch and moves with `preserveResumeState` whenever the task has resumable step progress, matching the step-session and pause-park paths, so execution resumes onto the existing branch from the first incomplete step. The worktree is still removed to free its concurrency slot — only the durable pointers (branch + step state) are kept.
|
||||
@@ -2466,6 +2466,75 @@ describe("StepSessionExecutor integration", () => {
|
||||
}));
|
||||
});
|
||||
|
||||
it("REGRESSION (FN-6722): pause-abort with step progress preserves branch + resume state on requeue to todo", async () => {
|
||||
// A mid-run abort on a task that already completed steps (with commits on its
|
||||
// branch) must not reset progress or drop the branch when bounced to todo —
|
||||
// otherwise the next dispatch re-plans from Step 0 and the committed work is
|
||||
// stranded (observed as FN-6722 "lost all progress / stuck"). The teardown
|
||||
// must move with preserveResumeState and keep the branch pointer.
|
||||
// Single-session mode — this is the teardown that logs the exact FN-6722
|
||||
// string "Execution paused — agent terminated, moved to todo" (executor.ts
|
||||
// ~9280). Reject the session work with pausedAborted set so execute() enters
|
||||
// that catch-block teardown, exactly as the FN-6722 hard-cancel did.
|
||||
const store = createMockStore();
|
||||
const taskState = createTaskWithSteps({
|
||||
description: "# test\n## Steps\n### Step 0: Preflight\n- [ ] check",
|
||||
steps: [
|
||||
{ name: "Step 0", status: "done" },
|
||||
{ name: "Step 1", status: "pending" },
|
||||
],
|
||||
currentStep: 1,
|
||||
branch: "fusion/fn-200",
|
||||
});
|
||||
|
||||
store.getSettings.mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 15000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: false,
|
||||
runStepsInNewSessions: false,
|
||||
});
|
||||
store.getTask.mockImplementation(async () => ({ ...taskState }));
|
||||
|
||||
const session = {
|
||||
prompt: vi.fn().mockRejectedValue(new Error("aborted by hard-cancel")),
|
||||
dispose: vi.fn(),
|
||||
subscribe: vi.fn(),
|
||||
on: vi.fn(),
|
||||
abortBash: vi.fn(),
|
||||
state: {},
|
||||
sessionManager: { getLeafId: vi.fn().mockReturnValue("leaf-1") },
|
||||
getSessionStats: vi.fn().mockReturnValue({ tokens: {} }),
|
||||
};
|
||||
mockedCreateFnAgent.mockResolvedValue({ session } as any);
|
||||
|
||||
const executor = new TaskExecutor(store, "/tmp/test", {});
|
||||
// Mark the in-flight run hard-cancelled so the rejection routes through the
|
||||
// pausedAborted teardown branch rather than the generic failure sink.
|
||||
(executor as any).pausedAborted.add("FN-200");
|
||||
await executor.execute(taskState);
|
||||
|
||||
// Confirm we exercised the single-session catch-block teardown.
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-200",
|
||||
"Execution paused — agent terminated, moved to todo",
|
||||
undefined,
|
||||
expect.anything(),
|
||||
);
|
||||
// Resume state preserved (steps NOT reset to pending)...
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-200", "todo", { preserveResumeState: true });
|
||||
// ...and the branch pointer to the committed work is NOT cleared.
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
"FN-200",
|
||||
expect.objectContaining({ branch: undefined }),
|
||||
);
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith(
|
||||
"FN-200",
|
||||
expect.objectContaining({ branch: null }),
|
||||
);
|
||||
});
|
||||
|
||||
it("REGRESSION: untrackTask called with bare task ID during pause in step-session mode", async () => {
|
||||
const store = createStepSessionStore();
|
||||
|
||||
|
||||
@@ -9276,9 +9276,32 @@ export class TaskExecutor {
|
||||
executorLog.warn(`Failed to remove old worktree ${worktreePath}: ${cleanupErrMessage}`);
|
||||
}
|
||||
}
|
||||
await this.store.updateTask(task.id, { worktree: undefined, branch: undefined });
|
||||
// FN-6722: a mid-run abort on a task that already has real step
|
||||
// progress must not discard that progress on the bounce to todo. The
|
||||
// sibling pause-park path (parkTaskAfterWorkflowStepPause, ~1826) moves
|
||||
// with preserveResumeState; this teardown branch historically did not —
|
||||
// it cleared `branch` AND moved without preservation, which reset every
|
||||
// step to pending (store.moveTaskInternal ~7322 resetAllStepsToPending)
|
||||
// and dropped the pointer to the commits already on the task branch.
|
||||
// The next dispatch then re-planned from Step 0 even though the work was
|
||||
// committed on `task.branch` — observably a "lost all progress / stuck"
|
||||
// failure. Preserve the branch + resume state when there is resumable
|
||||
// progress so execute() resumes onto the existing branch (the
|
||||
// `acquisition.isResume && task.branch` reconciliation ~7679) from the
|
||||
// first incomplete step. The worktree is still removed above and its
|
||||
// binding cleared below to free the concurrency slot (FN-6782) — only the
|
||||
// durable pointers (branch + step state) are kept. The 9227 guard above
|
||||
// covers the same intent but is race-contingent on the move having
|
||||
// already landed; this makes the fall-through path safe regardless.
|
||||
const hasResumableProgress =
|
||||
(task.currentStep ?? 0) > 0
|
||||
|| (task.steps?.some((step) => step.status === "done" || step.status === "in-progress") ?? false);
|
||||
await this.store.updateTask(
|
||||
task.id,
|
||||
hasResumableProgress ? { worktree: undefined } : { worktree: undefined, branch: undefined },
|
||||
);
|
||||
await this.store.logEntry(task.id, "Execution paused — agent terminated, moved to todo", undefined, this.getRunContextFor(task.id));
|
||||
await this.store.moveTask(task.id, "todo");
|
||||
await this.store.moveTask(task.id, "todo", hasResumableProgress ? { preserveResumeState: true } : undefined);
|
||||
}
|
||||
} else if (this.stuckAborted.has(task.id)) {
|
||||
// Task was killed by stuck task detector — defer requeue to finally block
|
||||
|
||||
Reference in New Issue
Block a user