From 93017a3c4a4f5cb81591243bd05e62bd9e66f41d Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 21 Jun 2026 09:41:51 -0700 Subject: [PATCH 1/2] fix(engine): preserve task progress when a single-session run is hard-cancelled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../preserve-progress-on-pause-abort.md | 5 ++ .../src/__tests__/executor-pause.test.ts | 69 +++++++++++++++++++ packages/engine/src/executor.ts | 27 +++++++- 3 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 .changeset/preserve-progress-on-pause-abort.md 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..e9d96372cf 100644 --- a/packages/engine/src/__tests__/executor-pause.test.ts +++ b/packages/engine/src/__tests__/executor-pause.test.ts @@ -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(); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index feb1810566..da34eea90b 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -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 From e6503ca872ad25b90737862924ff9ada22a560a3 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 21 Jun 2026 09:59:26 -0700 Subject: [PATCH 2/2] Address PR review feedback (#1707) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Read hasResumableProgress from latestTask (the store snapshot fetched at ~9226), not the frozen dispatch-time task param, so a freshly dispatched task that commits step progress mid-session is preserved on a hard-cancel teardown — not just the re-dispatch case (greptile P1) - Add companion regression test for the fresh-task-with-in-session-progress case, which fails against the stale-task snapshot (greptile P1) - Reformat the requirement comment block to the canonical FNXC heading (FNXC:WorkflowLifecycle yyyy-MM-dd-hh:mm:) convention (CodeRabbit) Fusion-Task-Id: FN-6722 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/__tests__/executor-pause.test.ts | 69 +++++++++++++++++++ packages/engine/src/executor.ts | 48 ++++++++----- 2 files changed, 98 insertions(+), 19 deletions(-) diff --git a/packages/engine/src/__tests__/executor-pause.test.ts b/packages/engine/src/__tests__/executor-pause.test.ts index e9d96372cf..491dffefae 100644 --- a/packages/engine/src/__tests__/executor-pause.test.ts +++ b/packages/engine/src/__tests__/executor-pause.test.ts @@ -2535,6 +2535,75 @@ describe("StepSessionExecutor integration", () => { ); }); + 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 da34eea90b..eb084e47a9 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -9276,26 +9276,36 @@ export class TaskExecutor { executorLog.warn(`Failed to remove old worktree ${worktreePath}: ${cleanupErrMessage}`); } } - // 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. + // 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 = - (task.currentStep ?? 0) > 0 - || (task.steps?.some((step) => step.status === "done" || step.status === "in-progress") ?? false); + (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 },