From f23619c2d4e674fe68b224f44c04bfee00985dcf Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 12 Jul 2026 09:08:42 -0700 Subject: [PATCH] fix: preserve user pause across executor pause teardown (FN-7851 pause-bounce loop) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pausing an in-progress task never stuck: the pause teardown re-queued the row to todo with a plain engine move, and the reopen block wiped paused/pausedByAgentId/pausedReason. The graph-failure classifier then saw an unpaused row, misread the hard-cancel as an engine-internal abort, and auto-continued the session (graphResumeRetryCount 1/2, 2/2); once the budget was exhausted the benign re-queue left the row dispatchable and the scheduler re-dispatched it seconds later — an indefinite pause/resume bounce, burning a fresh worktree + pnpm install per cycle. - store: new moveTask option `preservePause` keeps the pause park across a reopen-to-todo/triage move (flag-ON trait hook + flag-OFF legacy inline, kept in sync). It never SETS a pause, only prevents clearing one. - executor teardown: when the pause that caused the abort is still in force, move with preservePause so the row lands in todo still parked (scheduler skips paused/userPaused rows until explicit unpause). - classifier: a live task pause is labeled operator intent, never "engine abort during pause/resume"; the benign log now says "parked … awaiting explicit unpause" instead of the contradictory "cleared for normal scheduling" for parked rows. Surfaces covered by tests: flag-ON hook (preserve + never-set + default clear), classifier no-auto-continue for task-pause/user-pause/global-pause rows in todo, provenance labels. Co-Authored-By: Claude Fable 5 --- .changeset/pause-survives-teardown.md | 7 ++++ .../__tests__/default-workflow-hooks.test.ts | 37 +++++++++++++++++ packages/core/src/default-workflow-hooks.ts | 18 +++++++-- packages/core/src/store.ts | 26 ++++++++++-- .../executor-paused-abort-todo-benign.test.ts | 39 ++++++++++++++---- packages/engine/src/executor.ts | 40 ++++++++++++++++--- 6 files changed, 146 insertions(+), 21 deletions(-) create mode 100644 .changeset/pause-survives-teardown.md diff --git a/.changeset/pause-survives-teardown.md b/.changeset/pause-survives-teardown.md new file mode 100644 index 0000000000..0cc739be36 --- /dev/null +++ b/.changeset/pause-survives-teardown.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Pausing an in-progress task now sticks — the pause survives session teardown instead of auto-resuming. +category: fix +dev: New `preservePause` moveTask option; the executor pause teardown passes it so the todo re-queue keeps `paused`/`pausedByAgentId`/`pausedReason`. The graph-failure classifier now labels a preserved task pause as operator intent (never "engine abort during pause/resume" auto-continue), and the benign re-queue log says "parked … awaiting explicit unpause" for paused rows. diff --git a/packages/core/src/__tests__/default-workflow-hooks.test.ts b/packages/core/src/__tests__/default-workflow-hooks.test.ts index c650a89a78..e46d4fa6ce 100644 --- a/packages/core/src/__tests__/default-workflow-hooks.test.ts +++ b/packages/core/src/__tests__/default-workflow-hooks.test.ts @@ -84,4 +84,41 @@ describe("default-workflow-hooks registry wiring", () => { applyDefaultWorkflowMoveEffects(engineCtx); expect(engineCtx.task.userPaused).toBeUndefined(); }); + + // FN-7851 pause-bounce regression: the executor's pause teardown re-queues a + // user-paused in-progress task to todo. Without preservePause the reopen + // block wiped the pause flags, leaving the row dispatchable — the scheduler + // re-dispatched it seconds after the user paused it. + it("preservePause keeps the pause park across an engine reopen to todo", () => { + registerDefaultWorkflowHooks(); + const ctx = makeCtx({ fromColumn: "in-progress", toColumn: "todo", moveSource: "engine", options: { preservePause: true } }); + ctx.task.paused = true; + ctx.task.pausedByAgentId = "agent-1"; + ctx.task.pausedReason = "operator pause"; + ctx.task.userPaused = true; + applyDefaultWorkflowMoveEffects(ctx); + expect(ctx.task.paused).toBe(true); + expect(ctx.task.pausedByAgentId).toBe("agent-1"); + expect(ctx.task.pausedReason).toBe("operator pause"); + expect(ctx.task.userPaused).toBe(true); + }); + + it("preservePause never SETS a pause on an unpaused reopen, and default reopen still clears one", () => { + registerDefaultWorkflowHooks(); + // preservePause on an unpaused task: nothing appears. + const unpausedCtx = makeCtx({ fromColumn: "in-progress", toColumn: "todo", moveSource: "engine", options: { preservePause: true } }); + applyDefaultWorkflowMoveEffects(unpausedCtx); + expect(unpausedCtx.task.paused).toBeUndefined(); + expect(unpausedCtx.task.userPaused).toBeUndefined(); + + // Default (no preservePause) engine reopen still clears an existing pause. + const defaultCtx = makeCtx({ fromColumn: "in-progress", toColumn: "todo", moveSource: "engine" }); + defaultCtx.task.paused = true; + defaultCtx.task.pausedByAgentId = "agent-1"; + defaultCtx.task.pausedReason = "operator pause"; + applyDefaultWorkflowMoveEffects(defaultCtx); + expect(defaultCtx.task.paused).toBeUndefined(); + expect(defaultCtx.task.pausedByAgentId).toBeUndefined(); + expect(defaultCtx.task.pausedReason).toBeUndefined(); + }); }); diff --git a/packages/core/src/default-workflow-hooks.ts b/packages/core/src/default-workflow-hooks.ts index 2b6c1936da..3f63225c77 100644 --- a/packages/core/src/default-workflow-hooks.ts +++ b/packages/core/src/default-workflow-hooks.ts @@ -86,6 +86,7 @@ export interface DefaultWorkflowMoveContext { preserveResumeState?: boolean; preserveProgress?: boolean; preserveWorktree?: boolean; + preservePause?: boolean; }; /** Reset all steps to pending + currentStep 0 (store owns the impl). */ resetSteps: () => void; @@ -134,19 +135,28 @@ export function applyResetOnEntryEffects(ctx: DefaultWorkflowMoveContext): void (toColumn === "todo" || toColumn === "triage"); if (!isReopenToTodoOrTriage) return; + /* + FNXC:WorkflowLifecycle 2026-07-12-09:05: + Pause-bounce loop (observed on FN-7851, 2026-07-12): a user pause of an in-progress task hard-cancels the session and the executor teardown re-queues the row to todo. This reopen block unconditionally wiped `paused`/`pausedByAgentId`/`pausedReason`, so the pause NEVER survived its own teardown — the graph-failure classifier then saw an unpaused row, misread the abort as engine-internal, and auto-continued the session (and after the retry budget, the scheduler re-dispatched the unpaused todo row). `preservePause` lets the pause-caused teardown move keep the park; the scheduler skips paused/userPaused todo rows until an explicit unpause. + `userPaused` promotion for user-source moves is unchanged; preservePause only prevents CLEARING an existing park, never sets one. + */ if (!options.preserveStatus) { task.status = undefined; task.error = undefined; - task.pausedReason = undefined; + if (!options.preservePause) { + task.pausedReason = undefined; + } } task.blockedBy = undefined; task.overlapBlockedBy = undefined; - task.paused = undefined; - task.pausedByAgentId = undefined; + if (!options.preservePause) { + task.paused = undefined; + task.pausedByAgentId = undefined; + } // abort-on-exit userPaused: only for user-source moves to todo (KTD-9). if (moveSource === "user" && toColumn === "todo") { task.userPaused = true; - } else { + } else if (!options.preservePause) { task.userPaused = undefined; } diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index db48db914c..1d33ec772c 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -1436,6 +1436,15 @@ interface MoveTaskOptions { preserveProgress?: boolean; preserveWorktree?: boolean; preserveStatus?: boolean; + /** + * FNXC:WorkflowLifecycle 2026-07-12-09:05: + * Keep `paused`/`pausedByAgentId`/`pausedReason` (and any existing + * `userPaused`) across a reopen-to-todo/triage move. Used by the executor's + * pause teardown so a user pause survives its own hard-cancel re-queue and + * the row stays parked until an explicit unpause (FN-7851 pause-bounce loop). + * Never SETS a pause — only prevents the reopen block from clearing one. + */ + preservePause?: boolean; allocateWorktree?: (reservedNames: Set) => string | null; moveSource?: "user" | "engine" | "scheduler"; workflowMoveActor?: WorkflowMovePolicyInput["actor"]; @@ -7885,6 +7894,7 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS} preserveResumeState: options?.preserveResumeState, preserveProgress: options?.preserveProgress, preserveWorktree: options?.preserveWorktree, + preservePause: options?.preservePause, }, resetSteps: () => this.resetAllStepsToPending(task), }; @@ -7946,18 +7956,26 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS} && (toColumn === "todo" || toColumn === "triage"); if (isReopenToTodoOrTriage) { + // FNXC:WorkflowLifecycle 2026-07-12-09:05: keep this flag-OFF inline + // block in sync with applyResetOnEntryEffects (default-workflow-hooks.ts) + // — `preservePause` keeps a pause-caused teardown move from clearing the + // user's park (FN-7851 pause-bounce loop). if (!options?.preserveStatus) { task.status = undefined; task.error = undefined; - task.pausedReason = undefined; + if (!options?.preservePause) { + task.pausedReason = undefined; + } } task.blockedBy = undefined; task.overlapBlockedBy = undefined; - task.paused = undefined; - task.pausedByAgentId = undefined; + if (!options?.preservePause) { + task.paused = undefined; + task.pausedByAgentId = undefined; + } if (moveSource === "user" && toColumn === "todo") { task.userPaused = true; - } else { + } else if (!options?.preservePause) { task.userPaused = undefined; } 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 1d68a0cca2..270e96ac5f 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 @@ -193,22 +193,47 @@ describe("pause-abort benign requeue-to-todo (FN-6782)", () => { }); it.each([ - { label: "explicit user pause", overrides: { column: "todo", userPaused: true }, provenance: "hard-cancel" as const }, - { label: "global pause", overrides: { column: "todo" }, provenance: "global-pause" as const }, + { + label: "explicit user pause", + overrides: { column: "todo", userPaused: true }, + provenance: "hard-cancel" as const, + expectedBenign: "benign, paused awaiting explicit unpause", + expectedProvenance: "explicit user pause", + }, + { + label: "global pause", + overrides: { column: "todo" }, + provenance: "global-pause" as const, + expectedBenign: "benign, cleared for normal scheduling", + expectedProvenance: "global pause", + }, + { + // FN-7851 pause-bounce regression: a pause-button pause that survived the + // executor teardown via preservePause lands in todo with `paused: true`. + // It must be classified as a task pause (NOT an engine-internal abort), + // stay parked, and never auto-continue the session. + label: "task pause (pause button, preserved across teardown)", + overrides: { column: "todo", paused: true }, + provenance: "hard-cancel" as const, + expectedBenign: "benign, paused awaiting explicit unpause", + expectedProvenance: "task pause", + }, ])( "does NOT auto-resume a $label that landed in todo", - async ({ overrides, provenance }) => { + async ({ overrides, provenance, expectedBenign, expectedProvenance }) => { // 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. + // provenance. A genuine operator pause (userPaused OR a preserved + // pause-button `paused`) 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. const { store, task, executor } = makeHarness(overrides, provenance); (executor as any).addActiveWorktree(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)).toContain(expectedBenign); + expect(logText(store)).toContain(`during ${expectedProvenance} with task`); expect(logText(store)).not.toContain("auto-continuing the agent session"); await flushScheduledRetry(); expect(executeSpy).not.toHaveBeenCalled(); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 113bbe6cb1..87f00891ff 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -8739,13 +8739,20 @@ export class TaskExecutor { FNXC:WorkflowLifecycle 2026-06-17-23:32: FN-6625: completion-finalize aborts are teardown artifacts after a completed/no-commit execution has already advanced to in-review. Without excluding that provenance, the FN-6614 execute-node tail failure was mislabeled as an operator-action pause abort and re-parked failed. */ + // FNXC:WorkflowLifecycle 2026-07-12-09:05: check `live.paused` BEFORE the + // bare pausedAborted marker — a task-pause park that survived teardown + // (preservePause, FN-7851) is operator intent, not an engine-internal + // abort, and must be labeled as such so the benign re-queue log below + // does not misreport it as engine churn. const pauseProvenance = live.userPaused ? "explicit user pause" : abortProvenance === "global-pause" ? "global pause" - : pausedAborted - ? "engine abort during pause/resume" - : "task pause"; + : live.paused + ? "task pause" + : 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 @@ -8864,7 +8871,13 @@ export class TaskExecutor { // shared budget and falls back to plain todo re-queueing. 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`; + // FNXC:WorkflowLifecycle 2026-07-12-09:05: a row still carrying a + // pause park (paused/userPaused) is NOT "cleared for normal + // scheduling" — the scheduler skips it until an explicit unpause. + // Say so, or the log contradicts the board (FN-7851 misdiagnosis). + const todoBenign = live.paused || live.userPaused + ? `Workflow graph run ended during ${pauseProvenance} with task parked in todo — benign, paused awaiting explicit unpause` + : `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)); // FNXC:WorkflowLifecycle 2026-06-20-19:58: reconcile a stale @@ -11588,13 +11601,28 @@ export class TaskExecutor { const hasResumableProgress = (progressSource.currentStep ?? 0) > 0 || (progressSource.steps?.some((step) => step.status === "done" || step.status === "in-progress") ?? false); + /* + FNXC:WorkflowLifecycle 2026-07-12-09:05: + Pause-bounce loop (observed on FN-7851): this teardown runs BECAUSE the user paused the task, but the plain move-to-todo below wiped the pause flags (store reopen block), leaving an unpaused dispatchable todo row. The graph-failure classifier then read `paused=false, userPaused=false`, misclassified the abort as engine-internal, and auto-continued the session; once the shared graphResumeRetryCount budget was exhausted the scheduler simply re-dispatched the row seconds later — so pausing an in-progress task could never stick. When the pause that caused this abort is still in force at teardown time, move with `preservePause` so the row lands in todo still parked (`paused` kept; scheduler skips paused/userPaused todo rows) and the classifier sees the pause and routes benignly. An unpause during the teardown window leaves `paused` unset and restores the old requeue-for-normal-scheduling behavior. + */ + const pauseStillInForce = latestTask?.paused === true; 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.logEntry( + task.id, + pauseStillInForce + ? "Execution paused — agent terminated, parked in todo (pause preserved, awaiting explicit unpause)" + : "Execution paused — agent terminated, moved to todo", + undefined, + this.getRunContextFor(task.id), + ); this.markGraphExecuteSelfRequeued(task.id); - await this.store.moveTask(task.id, "todo", hasResumableProgress ? { preserveResumeState: true } : undefined); + await this.store.moveTask(task.id, "todo", { + ...(hasResumableProgress ? { preserveResumeState: true } : {}), + ...(pauseStillInForce ? { preservePause: true } : {}), + }); } } else if (this.stuckAborted.has(task.id)) { // Task was killed by stuck task detector — defer requeue to finally block