fix(engine): auto-continue agent session after engine-internal pause/resume abort
When the engine hard-cancels in-flight work during a pause/resume cycle and the workflow graph run ends with the task re-queued to `todo`, the executor used to leave it for a fresh scheduler dispatch and fire a spurious failure notification. It now continues the agent session in place via a bounded internal retry (reusing the graph-resume retry budget + backoff), falling back to the benign todo re-queue only after retries are exhausted. - Scoped strictly to engine-internal aborts via a typed `isEngineInternalAbort` discriminant; genuine user/global/task pauses are never auto-resumed. - Re-checks the task at retry fire time and aborts the auto-continue if it was paused, moved, or deleted during the backoff window. - Clears any stale `failed` status and emits an `Auto-recovered:` log so the deferred failure notification is suppressed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
5
.changeset/auto-continue-engine-pause-abort.md
Normal file
5
.changeset/auto-continue-engine-pause-abort.md
Normal file
@@ -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.
|
||||
@@ -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> = {}): TaskDetail {
|
||||
} as TaskDetail;
|
||||
}
|
||||
|
||||
function makeHarness(taskOverrides: Partial<TaskDetail> = {}) {
|
||||
type AbortProvenance = "hard-cancel" | "global-pause" | "merge-seam" | "completion-finalize";
|
||||
|
||||
function makeHarness(
|
||||
taskOverrides: Partial<TaskDetail> = {},
|
||||
provenance: AbortProvenance = "hard-cancel",
|
||||
) {
|
||||
const store = createMockStore();
|
||||
const task = makeTask(taskOverrides);
|
||||
store.getTask.mockResolvedValue(task);
|
||||
@@ -43,7 +48,7 @@ function makeHarness(taskOverrides: Partial<TaskDetail> = {}) {
|
||||
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<typeof createMockStore>): string {
|
||||
return store.logEntry.mock.calls.map((call: unknown[]) => call[1]).join("\n");
|
||||
}
|
||||
@@ -63,50 +75,65 @@ function logText(store: ReturnType<typeof createMockStore>): 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 () => {
|
||||
|
||||
@@ -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));
|
||||
|
||||
Reference in New Issue
Block a user