Merge pull request #1707 from Runfusion/gsxdsm/engine-worklow
fix(engine): preserve task progress when a single-session run is hard-cancelled (FN-6722)
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,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();
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user