diff --git a/.changeset/preserve-progress-on-pause-abort.md b/.changeset/preserve-progress-on-pause-abort.md new file mode 100644 index 0000000000..6aaae53271 --- /dev/null +++ b/.changeset/preserve-progress-on-pause-abort.md @@ -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. diff --git a/packages/engine/src/__tests__/executor-pause.test.ts b/packages/engine/src/__tests__/executor-pause.test.ts index 3239a763ae..491dffefae 100644 --- a/packages/engine/src/__tests__/executor-pause.test.ts +++ b/packages/engine/src/__tests__/executor-pause.test.ts @@ -2466,6 +2466,144 @@ 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 (FN-6722): pause-abort preserves progress committed mid-session by a freshly-dispatched task", async () => { + // The dispatch-time `task` snapshot is frozen — a task dispatched fresh + // (currentStep 0, all steps pending) that commits step progress to the store + // mid-session shows that progress ONLY via the store read (`latestTask`), + // never via `task`. The teardown must read `latestTask` so this first-run + // case is preserved too; reading the stale `task` here would clear the branch + // and reset steps — the same FN-6722 failure mode. + const store = createMockStore(); + + // Dispatch-time snapshot: no progress yet. + const task = createTaskWithSteps({ + description: "# test\n## Steps\n### Step 0: Preflight\n- [ ] check", + steps: [ + { name: "Step 0", status: "pending" }, + { name: "Step 1", status: "pending" }, + ], + currentStep: 0, + }); + + store.getSettings.mockResolvedValue({ + maxConcurrent: 2, + maxWorktrees: 4, + pollIntervalMs: 15000, + groupOverlappingFiles: false, + autoMerge: false, + runStepsInNewSessions: false, + }); + // Store snapshot the teardown re-reads: the agent committed Step 0 + a branch + // during the session. column stays in-progress so the 9227 early-return guard + // (which needs column === "todo") does not fire and we reach the teardown. + store.getTask.mockResolvedValue({ + ...task, + column: "in-progress", + currentStep: 1, + branch: "fusion/fn-200", + steps: [ + { name: "Step 0", status: "done" }, + { name: "Step 1", status: "pending" }, + ], + } as any); + + 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", {}); + (executor as any).pausedAborted.add("FN-200"); + await executor.execute(task); + + // Progress visible only via latestTask must still be preserved. + expect(store.moveTask).toHaveBeenCalledWith("FN-200", "todo", { preserveResumeState: true }); + 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(); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index c9bb610a65..c9a997913f 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -9281,9 +9281,42 @@ export class TaskExecutor { executorLog.warn(`Failed to remove old worktree ${worktreePath}: ${cleanupErrMessage}`); } } - await this.store.updateTask(task.id, { worktree: undefined, branch: undefined }); + // FNXC:WorkflowLifecycle 2026-06-21-00:00: 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 the + // 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. + // + // Read progress from `latestTask` (the store snapshot fetched at ~9226), + // NOT the `task` parameter: `task` is frozen at dispatch time and never + // mutated mid-run, so a fresh task (currentStep 0, all steps pending at + // dispatch) whose agent committed step progress to the store during this + // session would otherwise look progress-less here and hit the destructive + // reset — the exact FN-6722 failure mode. Fall back to `task` when the + // store read came back empty. + const progressSource = latestTask ?? task; + const hasResumableProgress = + (progressSource.currentStep ?? 0) > 0 + || (progressSource.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