diff --git a/.changeset/auto-continue-engine-pause-abort.md b/.changeset/auto-continue-engine-pause-abort.md new file mode 100644 index 0000000000..3b4cea551b --- /dev/null +++ b/.changeset/auto-continue-engine-pause-abort.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": patch +--- + +Auto-continue the agent session after an engine-internal pause/resume abort instead of re-queueing the task to todo. When the engine tears down in-flight work (hard-cancel) and the workflow graph run ends with the task back in `todo`, the executor now retries the agent session in place — bounded by the existing graph-resume retry budget with backoff, falling back to a benign re-queue only after retries are exhausted. Before re-dispatching, it re-checks the task at fire time and aborts the auto-continue if the task was paused, moved, or deleted during the backoff window, so genuine user/global/task pauses are never resumed against the operator's intent. The transient reclassification clears any stale `failed` status and emits an `Auto-recovered:` log so no spurious failure notification fires. diff --git a/packages/engine/src/__tests__/executor-paused-abort-todo-benign.test.ts b/packages/engine/src/__tests__/executor-paused-abort-todo-benign.test.ts index 13a7486ead..b304058216 100644 --- a/packages/engine/src/__tests__/executor-paused-abort-todo-benign.test.ts +++ b/packages/engine/src/__tests__/executor-paused-abort-todo-benign.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import "./executor-test-helpers.js"; import { TaskExecutor } from "../executor.js"; import { createMockStore, resetExecutorMocks } from "./executor-test-helpers.js"; @@ -31,7 +31,12 @@ function makeTask(overrides: Partial = {}): TaskDetail { } as TaskDetail; } -function makeHarness(taskOverrides: Partial = {}) { +type AbortProvenance = "hard-cancel" | "global-pause" | "merge-seam" | "completion-finalize"; + +function makeHarness( + taskOverrides: Partial = {}, + provenance: AbortProvenance = "hard-cancel", +) { const store = createMockStore(); const task = makeTask(taskOverrides); store.getTask.mockResolvedValue(task); @@ -43,7 +48,7 @@ function makeHarness(taskOverrides: Partial = {}) { maxAutoMergeRetries: 3, }); const executor = new TaskExecutor(store, "/tmp/test", {}); - (executor as any).markPausedAborted(task.id, "hard-cancel"); + (executor as any).markPausedAborted(task.id, provenance); return { store, task, executor }; } @@ -56,6 +61,13 @@ async function invokeGraphFailure(executor: TaskExecutor, task: TaskDetail) { }); } +// Flush the unref'd setTimeout that schedules the in-place retry plus the async +// re-fetch + execute() chain inside it. Fake timers keep this deterministic +// (no real wall-clock wait) per the repo's no-slow-tests rule (FN-5048). +async function flushScheduledRetry() { + await vi.advanceTimersByTimeAsync(10); +} + function logText(store: ReturnType): string { return store.logEntry.mock.calls.map((call: unknown[]) => call[1]).join("\n"); } @@ -63,50 +75,65 @@ function logText(store: ReturnType): string { describe("pause-abort benign requeue-to-todo (FN-6782)", () => { beforeEach(() => { resetExecutorMocks(); + vi.useFakeTimers(); }); - it("does NOT park a todo-column pause-abort as failed (no retry storm)", async () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it("auto-continues the agent session for an engine-internal abort instead of re-queueing to todo", async () => { + // An "engine abort during pause/resume" (pausedAborted hard-cancel, no user/ + // global pause) is engine-internal churn, not an operator action — the + // executor must retry the agent session in place rather than bouncing the + // task through todo (and must not fire a failure notification). const { store, task, executor } = makeHarness({ column: "todo" }); (executor as any).activeWorktrees.set(task.id, task.worktree); + const executeSpy = vi + .spyOn(executor as any, "execute") + .mockResolvedValue(undefined); await invokeGraphFailure(executor, task); - // FNXC:WorkflowLifecycle a todo pause-abort must NOT write status:"failed" — that was the storm trigger. + // It must NOT park status:"failed" — that was the storm trigger. const parkedFailed = store.updateTask.mock.calls.some( (call: unknown[]) => (call[1] as { status?: string } | undefined)?.status === "failed", ); expect(parkedFailed).toBe(false); - // FNXC:WorkflowLifecycle the benign-clear log must surface for observability. - expect(logText(store)).toContain("benign, cleared for normal scheduling"); - // FNXC:WorkflowLifecycle the pausedAborted marker must be cleared so the next dispatch starts clean. - expect((executor as any).pausedAborted.has(task.id)).toBe(false); - // FNXC:WorkflowLifecycle the leaked worktree slot must be released to avoid board-wide concurrency blockage. - expect((executor as any).activeWorktrees.has(task.id)).toBe(false); - // FNXC:WorkflowLifecycle 2026-06-20-19:58 a clean todo row (no stale status/error) - // must NOT trigger the reconciliation write — the `live.status != null || - // live.error != null` guard skips it so the common benign re-queue stays a no-op. - const clearedClean = store.updateTask.mock.calls.some( + // It auto-continues instead of logging the benign re-queue line. + expect(logText(store)).toContain("auto-continuing the agent session (1/2)"); + expect(logText(store)).not.toContain("benign, cleared for normal scheduling"); + // The bounded retry budget is incremented and any stale failure cleared. + const bumpedRetry = store.updateTask.mock.calls.some( (call: unknown[]) => { - const patch = call[1] as { status?: unknown; error?: unknown } | undefined; - return patch?.status === null && patch?.error === null; + const patch = call[1] as { graphResumeRetryCount?: number; status?: unknown } | undefined; + return patch?.graphResumeRetryCount === 1 && patch?.status === null; }, ); - expect(clearedClean).toBe(false); + expect(bumpedRetry).toBe(true); + // An `Auto-recovered:`-prefixed log suppresses the failure notification. + expect(logText(store)).toContain("Auto-recovered: engine-internal pause/resume abort"); + // The pause-abort marker is cleared and the worktree slot released. + expect((executor as any).pausedAborted.has(task.id)).toBe(false); + expect((executor as any).activeWorktrees.has(task.id)).toBe(false); + // The agent session is re-executed in place after the backoff window. + await flushScheduledRetry(); + expect(executeSpy).toHaveBeenCalledTimes(1); }); - it("clears a stale failed status when reclassifying a todo pause-abort as benign (no lingering failure notification)", async () => { - // FNXC:WorkflowLifecycle 2026-06-20-19:58 a pause-abort parked status:"failed" - // on an earlier non-todo observation stays dispatchable (scheduler filters - // column+paused, not status) and re-enters this branch in todo. The benign - // reclassification must reconcile the row to status:null/error:null — - // otherwise the persisted failure survives, the board shows it failed, and - // the deferred failure notification fires despite the benign log. + it("clears a stale failed status when auto-continuing an engine-internal abort (no lingering failure notification)", async () => { + // A pause-abort parked status:"failed" on an earlier non-todo observation + // stays dispatchable (scheduler filters column+paused, not status) and + // re-enters this branch in todo. Auto-continue must reconcile the row to + // status:null/error:null — otherwise the persisted failure survives, the + // board shows it failed, and the deferred failure notification fires. const { store, task, executor } = makeHarness({ column: "todo", status: "failed", error: "Workflow graph failure surfaced after paused engine abort during pause/resume", }); (executor as any).activeWorktrees.set(task.id, task.worktree); + const executeSpy = vi.spyOn(executor as any, "execute").mockResolvedValue(undefined); await invokeGraphFailure(executor, task); @@ -121,12 +148,86 @@ describe("pause-abort benign requeue-to-todo (FN-6782)", () => { (call: unknown[]) => (call[1] as { status?: string } | undefined)?.status === "failed", ); expect(reParkedFailed).toBe(false); + expect(logText(store)).toContain("auto-continuing the agent session"); + expect(logText(store)).toContain("Auto-recovered: engine-internal pause/resume abort"); + await flushScheduledRetry(); + expect(executeSpy).toHaveBeenCalledTimes(1); + }); + + it("does NOT auto-resume a genuine user pause or global pause that landed in todo", async () => { + // The auto-continue is scoped strictly to the engine-internal abort + // provenance. A genuine operator pause (userPaused) or a global engine pause + // that ended up in todo must stay parked-benign and wait for explicit + // resume — auto-resuming it would override the operator's intent. + for (const harness of [ + makeHarness({ column: "todo", userPaused: true }), + makeHarness({ column: "todo" }, "global-pause"), + ]) { + const { store, task, executor } = harness; + (executor as any).activeWorktrees.set(task.id, task.worktree); + const executeSpy = vi.spyOn(executor as any, "execute").mockResolvedValue(undefined); + + await invokeGraphFailure(executor, task); + + expect(logText(store)).toContain("benign, cleared for normal scheduling"); + expect(logText(store)).not.toContain("auto-continuing the agent session"); + await flushScheduledRetry(); + expect(executeSpy).not.toHaveBeenCalled(); + } + }); + + it("falls back to a benign todo re-queue once internal retries are exhausted", async () => { + // After MAX_TRANSIENT_GRAPH_RESUME_RETRIES (2) internal retries, a still- + // wedged engine-internal abort must stop auto-continuing and fall through to + // the benign re-queue (no failure notification, no retry storm). + const { store, task, executor } = makeHarness({ + column: "todo", + graphResumeRetryCount: 2, + }); + (executor as any).activeWorktrees.set(task.id, task.worktree); + const executeSpy = vi + .spyOn(executor as any, "execute") + .mockResolvedValue(undefined); + + await invokeGraphFailure(executor, task); + expect(logText(store)).toContain("benign, cleared for normal scheduling"); - // FNXC:WorkflowLifecycle 2026-06-20-19:58 the clear path must emit an - // `Auto-recovered:`-prefixed log so NotificationService proactively cancels - // the pending failure timer (recoveredStatus path), not just suppress it at - // fire time. Prefix is the documented self-healing recovery contract. - expect(logText(store)).toContain("Auto-recovered: cleared stale pause-abort failure on todo re-queue"); + expect(logText(store)).not.toContain("auto-continuing the agent session"); + const parkedFailed = store.updateTask.mock.calls.some( + (call: unknown[]) => (call[1] as { status?: string } | undefined)?.status === "failed", + ); + expect(parkedFailed).toBe(false); + await flushScheduledRetry(); + expect(executeSpy).not.toHaveBeenCalled(); + }); + + it("clears a stale failed status on the retries-exhausted benign fallback", async () => { + // The retries-exhausted fallback shares the benign re-queue's stale-failure + // reconciliation: a row carrying status:"failed" from an earlier non-todo + // observation must be cleared and emit the `Auto-recovered:` log so the + // deferred failure notification is suppressed even when auto-continue is + // exhausted. + const { store, task, executor } = makeHarness({ + column: "todo", + graphResumeRetryCount: 2, + status: "failed", + error: "Workflow graph failure surfaced after paused engine abort during pause/resume", + }); + (executor as any).activeWorktrees.set(task.id, task.worktree); + + await invokeGraphFailure(executor, task); + + expect(logText(store)).toContain("benign, cleared for normal scheduling"); + expect(logText(store)).toContain( + "Auto-recovered: cleared stale pause-abort failure on todo re-queue", + ); + const clearedFailure = store.updateTask.mock.calls.some( + (call: unknown[]) => { + const patch = call[1] as { status?: unknown; error?: unknown } | undefined; + return patch?.status === null && patch?.error === null; + }, + ); + expect(clearedFailure).toBe(true); }); it("STILL parks a non-todo (in-review) pause-abort as operator-action failed", async () => { diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 090a2cd870..22cc45db5f 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -6675,6 +6675,13 @@ export class TaskExecutor { : pausedAborted ? "engine abort during pause/resume" : "task pause"; + // Typed discriminant for the engine-internal abort case (mirrors the + // `pauseProvenance === "engine abort during pause/resume"` arm above): + // a hard-cancel teardown that is NOT a user pause or global pause. Used + // to gate the auto-continue branch so the gate cannot silently drift if + // the human-readable provenance label is ever revised. + const isEngineInternalAbort = + pausedAborted && !live.userPaused && abortProvenance !== "global-pause"; if (live.column !== "in-progress") { // FN-6782: a pause/resume abort that has left the task back in `todo` // is benign — the work is simply re-queued for a fresh dispatch, not @@ -6698,6 +6705,84 @@ export class TaskExecutor { // Safe here: handleGraphFailure is terminal for this run (no seam // re-entry), and the next dispatch re-acquires a fresh worktree. this.activeWorktrees.delete(task.id); + // FNXC:WorkflowLifecycle 2026-06-20-22:42: FN-6782 follow-up — an + // "engine abort during pause/resume" is NOT an operator action: the + // engine tore down in-flight work (hard-cancel via + // abortInFlightTaskWork) while the workflow graph run was ending and + // the task got re-queued to todo. Bouncing it back through todo for + // a fresh scheduler dispatch is observable churn and used to fire a + // spurious failure notification. Instead, continue the agent session + // automatically by re-executing in place, bounded by the same + // graphResumeRetryCount budget + backoff as the transient-resume + // path (and reset to 0 on the next clean graph completion, executor + // ~4242) so a genuinely wedged task still falls through to the benign + // re-queue after MAX retries rather than looping with no backoff. + // Scoped strictly to the engine-internal abort provenance: an + // explicit user pause / global pause / task pause that landed in todo + // must still wait for an explicit resume (the benign re-queue below). + // The graphResumeRetryCount budget is deliberately SHARED with the + // transient-resume-after-restart path (executor ~6850): both are + // "the graph run ended transiently, re-run it" recoveries, and a + // single combined cap is the belt-and-suspenders guard the + // executor-retry-storm tests assert against. The count is reset to 0 + // only on a clean graph completion (~4242) — NOT on the benign + // fallback below, so a still-wedged task that exhausts the budget + // stops auto-continuing instead of looping (resetting here would + // reintroduce a slower storm). + if (isEngineInternalAbort) { + const priorRetries = live.graphResumeRetryCount ?? 0; + if (priorRetries < MAX_TRANSIENT_GRAPH_RESUME_RETRIES) { + const nextRetries = priorRetries + 1; + const retryMessage = `Workflow graph run ended during ${pauseProvenance} — auto-continuing the agent session (${nextRetries}/${MAX_TRANSIENT_GRAPH_RESUME_RETRIES}) instead of re-queueing to todo`; + executorLog.log(`${task.id}: ${retryMessage}`); + await this.store.logEntry(task.id, retryMessage, undefined, this.getRunContextFor(task.id)); + // Emit the Auto-recovered marker BEFORE clearing status so the + // status-clearing updateTask's task:updated event already carries + // the recovery log — NotificationService.maybeSuppressTransientFailedNotification + // (recoveredStatus path) then proactively cancels any pending + // failure timer rather than relying on the race-contingent + // fire-time re-check. + await this.store.logEntry(task.id, "Auto-recovered: engine-internal pause/resume abort — retrying agent session, failure notification suppressed", undefined, this.getRunContextFor(task.id)); + await this.store.updateTask(task.id, { graphResumeRetryCount: nextRetries, status: null, error: null }, this.getRunContextFor(task.id)); + await this.persistTokenUsage(task.id); + const scheduleRetry = () => { + // Re-fetch at fire time: the snapshot is up to + // TRANSIENT_GRAPH_RESUME_RETRY_BACKOFF_MS stale, and the direct + // execute() bypasses the scheduler's pause filter (we cleared + // pausedAborted at the top of this branch). If a user paused, + // moved, or deleted the task during the backoff window, abort + // the auto-continue and leave it to normal scheduling so we + // never resume work the user just parked. + void (async () => { + try { + const resumeTask = await this.store.getTask(task.id); + if ( + resumeTask.deletedAt + || resumeTask.paused + || resumeTask.userPaused + || resumeTask.column !== "todo" + ) { + executorLog.log( + `${task.id}: skipping pause-abort auto-continue — task is now ${resumeTask.deletedAt ? "deleted" : resumeTask.paused || resumeTask.userPaused ? "paused" : `in '${resumeTask.column}'`} at retry fire time`, + ); + return; + } + await this.execute(resumeTask); + } catch (err) { + executorLog.error(`Failed pause-abort internal retry for ${task.id}:`, err); + } + })(); + }; + if (TRANSIENT_GRAPH_RESUME_RETRY_BACKOFF_MS > 0) { + const handle = setTimeout(scheduleRetry, TRANSIENT_GRAPH_RESUME_RETRY_BACKOFF_MS); + handle.unref?.(); + } else { + setTimeout(scheduleRetry, 0).unref?.(); + } + return; + } + executorLog.warn(`${task.id}: engine abort during pause/resume exhausted ${MAX_TRANSIENT_GRAPH_RESUME_RETRIES} internal retries — falling back to benign todo re-queue`); + } const todoBenign = `Workflow graph run ended during ${pauseProvenance} with task re-queued to todo — benign, cleared for normal scheduling`; executorLog.log(`${task.id}: ${todoBenign}`); await this.store.logEntry(task.id, todoBenign, undefined, this.getRunContextFor(task.id));