diff --git a/.changeset/fn-8826-restart-partial-progress.md b/.changeset/fn-8826-restart-partial-progress.md new file mode 100644 index 0000000000..11e5a540a2 --- /dev/null +++ b/.changeset/fn-8826-restart-partial-progress.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Resume partially completed tasks after restart without reporting a false failure. +category: fix +dev: Extends bounded unknown-node recovery to resumable partial steps and ignores recovered tool errors in failure attribution. diff --git a/docs/architecture.md b/docs/architecture.md index 4bffd86e16..e492012ada 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1369,7 +1369,7 @@ The columns/traits track moved *board* policy (transitions, capacity, hold, merg - A `parse-steps` node reads a workflow-declared **artifact** (PROMPT.md is just the default workflow's declared `step-source` artifact) and runs a registry **parser** (`step-headings`, `json-steps`, or a plugin-contributed parser) to write `Task.steps[]`. It is the only graph-side step-list writer and must dominate any `foreach`. Parsers fail closed to a routable `outcome:parse-error`. - A `foreach(source:"task-steps")` node instantiates an inline template subgraph once per planned step, with `mode` (sequential/parallel) and `isolation` (shared/worktree) as explicit axes and per-instance run-state pinned + persisted for crash-safe resume. -- Resume-limbo graph failures are retried only through a narrow persisted counter (`Task.graphResumeRetryCount`, max 2). The executor classifies a failure as transient only when it happens immediately after the engine restart/unpause resume log marker, reports no graph `reason`, has no completed step progress, and the task has no durable `lastError`/`failureReason`; it clears transient `status`/`error`, logs the auto-retry, and schedules one more graph execution. Any explicit graph reason, completed step progress, durable task error, missing resume marker, or exhausted counter remains a genuine `status:"failed"` disposition and goes to review handoff, preserving the FN-5704 anti-loop contract. +- Resume-limbo graph failures are retried only through a narrow persisted counter (`Task.graphResumeRetryCount`, max 2). The executor classifies a failure as transient only when it happens immediately after the engine restart/unpause resume log marker, reports no graph `reason`, still has resumable nonterminal step progress (or no steps yet), and the task has no durable `lastError`/`failureReason`; it clears transient `status`/`error`, logs the auto-retry, and schedules one more graph execution. Any explicit graph reason, fully completed step list, durable task error, missing resume marker, or exhausted counter remains a genuine `status:"failed"` disposition and goes to review handoff, preserving the FN-5704 anti-loop contract. - FN-7996 uses the separate durable `Task.consecutiveToolFailureRetryCount` budget for same-model retries after threshold consecutive `tool_error` completions; it never consumes `graphResumeRetryCount`, and exhaustion falls through to the unchanged terminal graph-failure park. - Paused graph exits are benign only while the task is still in `in-progress`; that is the user-pause/engine-pause state where preserving the pause without requeueing is intentional. If the graph reports a pause/abort exit after the task has already advanced to another live column (for example `in-review` after an unpause/resume race), `TaskExecutor.handleGraphFailure()` surfaces the boundary as operator-actionable failure evidence (`status:"failed"`/`error` when no failure is already present, plus a task-log entry) and does **not** move, rewind, or auto-merge the task unless the graph result carries the typed interrupted-node marker. The exceptions are typed in-flight node pause aborts (FN-7214), completed/no-commit finalize-to-review teardown (FN-6625/FN-6644/FN-6647), and benign merge-seam pause/resume aborts (FN-6735). For FN-7214 node aborts, `hard-cancel` and lifted `global-pause` provenance can re-enter the interrupted node through the bounded `graphResumeRetryCount` path; explicit `userPaused`, active global pause, merge/finalize provenance, genuine node failures, `autoMerge:false` human-gated review rows, retry-exhausted tasks, and already-confirmed merges still use the protected operator-action path. For completed finalize handoff, once the persisted task row proves a completed finalize handoff (non-`in-progress`, all steps done/skipped, no live pause/status/error, and the finalize-to-review log entry), a trailing graph abort resolves as an already-advanced benign graph exit even if volatile completion markers were cleared by teardown/restart and later abort provenance was re-marked from `completion-finalize` to `hard-cancel`. For merge-seam aborts, `in-review` tasks with no persisted status/error and no confirmed merge may re-enter bounded auto-merge retry only when the failed graph node is a merge/request-merge seam, the graph value is not conflict/contamination/foreign/retry-exhaustion evidence, project settings allow auto-merge processing (or the task is a shared-branch local integration member), and the merge retry budget is not exhausted. `done` and `archived` remain terminal and keep their column/status, while existing failure details are preserved. - A `step-review` node surfaces reviewer verdicts (APPROVE/REVISE/RETHINK/UNAVAILABLE) as outcome edges; `rework` edges (the only legal graph cycles, bounded per instance) route REVISE/RETHINK back to `step-execute`, with RETHINK traversal triggering the reset seam. diff --git a/packages/dashboard/app/components/TaskDetailModal.tsx b/packages/dashboard/app/components/TaskDetailModal.tsx index 5c05cd252c..2f4ee607d2 100644 --- a/packages/dashboard/app/components/TaskDetailModal.tsx +++ b/packages/dashboard/app/components/TaskDetailModal.tsx @@ -4098,12 +4098,19 @@ export function TaskDetailContent({ without mounting an empty error-message shell. The default banner fetches agent logs independently of the Raw Logs segment because FN-7995 persists bounded `tool_error` detail there; the Raw-Logs-gated display list is not a diagnostic data source. + + FNXC:TaskFailedBanner 2026-08-07-23:36: + Only the latest tool completion can supply failure detail. A later `tool_result` or a blank latest `tool_error` prevents an older recovered error from being attributed to the current failure. */ const shouldShowTaskFailureAlert = Boolean(task.status === "failed" && !hasPendingRecovery && !isPlannerChatExpanded); const taskFailureReason = task.error?.trim() || t("taskDetail.error.genericFailureReason", "The task failed before it could complete."); const taskFailureToolDetail = useMemo(() => { - const lastToolError = [...agentLogEntries].reverse().find((entry) => entry.type === "tool_error" && entry.detail?.trim()); - return lastToolError?.detail?.trim().slice(0, 1024); + const lastToolCompletion = agentLogEntries.findLast( + (entry) => entry.type === "tool_result" || entry.type === "tool_error", + ); + return lastToolCompletion?.type === "tool_error" + ? lastToolCompletion.detail?.trim().slice(0, 1024) || undefined + : undefined; }, [agentLogEntries]); const taskFailureHint = /workflow graph terminated|step-execute|no files? (were )?modified/i.test(`${task.error ?? ""}\n${taskFailureToolDetail ?? ""}`) ? t("taskDetail.error.retryHint", "Consider retrying with a different model or node.") diff --git a/packages/dashboard/app/components/__tests__/TaskDetailModal.test.tsx b/packages/dashboard/app/components/__tests__/TaskDetailModal.test.tsx index 050aac3417..60e33eb14f 100644 --- a/packages/dashboard/app/components/__tests__/TaskDetailModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/TaskDetailModal.test.tsx @@ -2,7 +2,7 @@ FNXC:TaskDetailTabs 2026-06-17-08:20: FN-7324 keeps the stable internal `chat` tab as Activity for explicit legacy links, but the omitted non-done default is now planner Chat. Tests that assert Definition-only sections must opt into `initialTab="definition"` so they verify the intended surface instead of the Chat landing state. */ -import { describe, it, expect, vi } from "vitest"; +import { afterEach, describe, it, expect, vi } from "vitest"; import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; import React, { type ComponentProps } from "react"; import userEvent from "@testing-library/user-event"; @@ -140,6 +140,11 @@ describe("TaskDetailModal reset confirmations", () => { }); describe("TaskDetailModal planner Chat tab", () => { + afterEach(async () => { + const { useAgentLogs } = await import("../../hooks/useAgentLogs"); + vi.mocked(useAgentLogs).mockReturnValue({ entries: [], loading: false, clear: vi.fn(), loadMore: vi.fn(async () => {}), hasMore: false, total: null, loadingMore: false }); + }); + function renderTask(column: any = "in-progress", initialTab?: ComponentProps["initialTab"]) { return render( { await waitFor(() => expect(updateTask).toHaveBeenCalledWith("FN-099", { modelProvider: "anthropic", modelId: "claude-alternate" }, undefined)); await waitFor(() => expect(onRetryTask).toHaveBeenCalledWith("FN-099")); - vi.mocked(useAgentLogs).mockReturnValue({ entries: [], loading: false, clear: vi.fn(), loadMore: vi.fn(async () => {}), hasMore: false, total: null, loadingMore: false }); + }); + + it("does not attribute a recovered historical tool error to an unknown graph failure", async () => { + const { useAgentLogs } = await import("../../hooks/useAgentLogs"); + vi.mocked(useAgentLogs).mockReturnValue({ + entries: [ + { timestamp: "2026-08-07T16:00:00Z", taskId: "FN-099", text: "edit", type: "tool_error", detail: "oldText was not unique" }, + { timestamp: "2026-08-07T16:01:00Z", taskId: "FN-099", text: "edit", type: "tool_result", detail: "updated" }, + { timestamp: "2026-08-07T16:02:00Z", taskId: "FN-099", text: "continued after correction", type: "text" }, + ], + loading: false, + clear: vi.fn(), + loadMore: vi.fn(async () => {}), + hasMore: false, + total: 3, + loadingMore: false, + }); + + render( + , + ); + + expect(screen.getByText("Workflow graph terminated with failure at node 'unknown'")).toBeInTheDocument(); + expect(document.querySelector(".detail-error-detail")).toBeNull(); + }); + + it("does not fall back to an older tool error when the latest tool error detail is blank", async () => { + const { useAgentLogs } = await import("../../hooks/useAgentLogs"); + vi.mocked(useAgentLogs).mockReturnValue({ + entries: [ + { timestamp: "2026-08-07T16:00:00Z", taskId: "FN-099", text: "edit", type: "tool_error", detail: "oldText was not unique" }, + { timestamp: "2026-08-07T16:01:00Z", taskId: "FN-099", text: "verify", type: "tool_error", detail: " " }, + ], + loading: false, + clear: vi.fn(), + loadMore: vi.fn(async () => {}), + hasMore: false, + total: 2, + loadingMore: false, + }); + + render( + , + ); + + expect(screen.queryByText("oldText was not unique")).not.toBeInTheDocument(); + expect(document.querySelector(".detail-error-detail")).toBeNull(); }); }); 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 b4fa193c00..27bf84a42b 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 @@ -88,6 +88,203 @@ describe("pause-abort benign requeue-to-todo (FN-6782)", () => { vi.useRealTimers(); }); + it.each([ + { action: "Resumed after engine restart", visitedNodeIds: [] }, + { action: "Resumed after engine restart", visitedNodeIds: ["execute"] }, + { action: "Resuming execution after unpause", visitedNodeIds: [] }, + { action: "Resuming execution after unpause", visitedNodeIds: ["execute"] }, + ])("auto-retries a reasonless $visitedNodeIds resume with partial step progress after '$action'", async ({ action, visitedNodeIds }) => { + const { store, task, executor } = makeHarness({ + column: "in-progress", + steps: [ + { name: "Implement", status: "done" }, + { name: "Verify", status: "in-progress" }, + ], + currentStep: 1, + log: [{ action, timestamp: now }], + graphResumeRetryCount: 0, + }); + (executor as any).clearPausedAborted(task.id); + const executeSpy = vi.spyOn(executor as any, "execute").mockResolvedValue(undefined); + + await invokeGraphFailure(executor, task, { + visitedNodeIds, + context: {}, + }); + + expect(store.updateTask).toHaveBeenCalledWith(task.id, { + graphResumeRetryCount: 1, + status: null, + error: null, + }, undefined); + expect(store.updateTask).not.toHaveBeenCalledWith( + task.id, + expect.objectContaining({ status: "failed" }), + expect.anything(), + ); + await flushScheduledRetry(); + expect(executeSpy).toHaveBeenCalledWith(expect.objectContaining({ + id: task.id, + graphResumeRetryCount: 1, + })); + }); + + it.each([ + { + label: "fully terminal steps", + taskOverrides: { + steps: [ + { name: "Implement", status: "done" }, + { name: "Verify", status: "skipped" }, + ], + }, + resultOverrides: {}, + }, + { + label: "explicit graph reason", + taskOverrides: {}, + resultOverrides: { reason: "provider failed" }, + }, + { + label: "durable lastError", + taskOverrides: { lastError: "session failed" }, + resultOverrides: {}, + }, + { + label: "durable failureReason", + taskOverrides: { failureReason: "workflow rejected" }, + resultOverrides: {}, + }, + { + label: "exhausted retry budget", + taskOverrides: { graphResumeRetryCount: 2 }, + resultOverrides: {}, + }, + ])("does not transiently retry partial progress with $label", async ({ taskOverrides, resultOverrides }) => { + const { store, task, executor } = makeHarness({ + column: "in-progress", + steps: [ + { name: "Implement", status: "done" }, + { name: "Verify", status: "in-progress" }, + ], + currentStep: 1, + log: [{ action: "Resumed after engine restart", timestamp: now }], + graphResumeRetryCount: 0, + ...taskOverrides, + } as Partial); + (executor as any).clearPausedAborted(task.id); + const executeSpy = vi.spyOn(executor as any, "execute").mockResolvedValue(undefined); + + await invokeGraphFailure(executor, task, { + visitedNodeIds: [], + context: {}, + ...resultOverrides, + }); + await flushScheduledRetry(); + + expect(store.updateTask).not.toHaveBeenCalledWith( + task.id, + expect.objectContaining({ graphResumeRetryCount: 1 }), + expect.anything(), + ); + expect(store.updateTask).toHaveBeenCalledWith( + task.id, + expect.objectContaining({ status: "failed" }), + undefined, + ); + expect(executeSpy).not.toHaveBeenCalled(); + }); + + it.each([ + { label: "deleted", patch: { deletedAt: "2026-08-07T23:36:00.000Z" } }, + { label: "paused", patch: { paused: true } }, + { label: "user-paused", patch: { userPaused: true } }, + { label: "moved out of WIP", patch: { column: "todo" } }, + { label: "moved to a terminal column", patch: { column: "done" } }, + { label: "new failed status", patch: { status: "failed" } }, + { label: "new persisted error", patch: { error: "new failure" } }, + { label: "new durable lastError", patch: { lastError: "new session failure" } }, + { label: "new durable failureReason", patch: { failureReason: "new workflow failure" } }, + ])("fire-time guard: skips a transient graph retry when the task became $label", async ({ patch }) => { + const { store, task, executor } = makeHarness({ + column: "in-progress", + steps: [ + { name: "Implement", status: "done" }, + { name: "Verify", status: "in-progress" }, + ], + currentStep: 1, + log: [{ action: "Resumed after engine restart", timestamp: now }], + }); + (executor as any).clearPausedAborted(task.id); + const executeSpy = vi.spyOn(executor as any, "execute").mockResolvedValue(undefined); + + await invokeGraphFailure(executor, task, { visitedNodeIds: [], context: {} }); + await store.updateTask(task.id, patch); + await flushScheduledRetry(); + + expect(executeSpy).not.toHaveBeenCalled(); + }); + + it("fire-time guard: skips a transient graph retry when the task was canceled", async () => { + const { store, task, executor } = makeHarness({ + column: "in-progress", + steps: [ + { name: "Implement", status: "done" }, + { name: "Verify", status: "in-progress" }, + ], + currentStep: 1, + log: [{ action: "Resumed after engine restart", timestamp: now }], + }); + (executor as any).clearPausedAborted(task.id); + const executeSpy = vi.spyOn(executor as any, "execute").mockResolvedValue(undefined); + + await invokeGraphFailure(executor, task, { visitedNodeIds: [], context: {} }); + (executor as any).userCanceledTaskIds.add(task.id); + await flushScheduledRetry(); + + expect(executeSpy).not.toHaveBeenCalled(); + }); + + it("fire-time guard: skips a transient graph retry when another run is active", async () => { + const { task, executor } = makeHarness({ + column: "in-progress", + steps: [ + { name: "Implement", status: "done" }, + { name: "Verify", status: "in-progress" }, + ], + currentStep: 1, + log: [{ action: "Resumed after engine restart", timestamp: now }], + }); + (executor as any).clearPausedAborted(task.id); + const executeSpy = vi.spyOn(executor as any, "execute").mockResolvedValue(undefined); + + await invokeGraphFailure(executor, task, { visitedNodeIds: [], context: {} }); + (executor as any).activeSessions.set(task.id, {}); + await flushScheduledRetry(); + + expect(executeSpy).not.toHaveBeenCalled(); + }); + + it("fire-time guard: skips a transient graph retry when the task disappeared", async () => { + const { store, task, executor } = makeHarness({ + column: "in-progress", + steps: [ + { name: "Implement", status: "done" }, + { name: "Verify", status: "in-progress" }, + ], + currentStep: 1, + log: [{ action: "Resumed after engine restart", timestamp: now }], + }); + (executor as any).clearPausedAborted(task.id); + const executeSpy = vi.spyOn(executor as any, "execute").mockResolvedValue(undefined); + + await invokeGraphFailure(executor, task, { visitedNodeIds: [], context: {} }); + store.getTask.mockRejectedValueOnce(new Error("Task not found")); + await flushScheduledRetry(); + + expect(executeSpy).not.toHaveBeenCalled(); + }); + 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 diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 56a0b89792..8bc6263a84 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -10837,7 +10837,11 @@ export class TaskExecutor { const failedNode = result.visitedNodeIds[result.visitedNodeIds.length - 1]; if (failedNode !== undefined && failedNode !== "execute") return false; - if (live.steps.some((step) => step.status === "done")) return false; + /* + FNXC:GraphRestartRecovery 2026-08-07-23:36: + Completed earlier steps are resumable progress when a later step is still active. Only a fully terminal step list fences this bounded retry path. + */ + if (live.steps.length > 0 && !hasNonTerminalWorkflowSteps(live)) return false; const failureState = live as Task & { lastError?: unknown; failureReason?: unknown }; if (failureState.lastError != null || failureState.failureReason != null) return false; @@ -12839,9 +12843,38 @@ export class TaskExecutor { error: null, }, this.getRunContextFor(task.id)); const scheduleRetry = () => { - this.execute(live).catch((err) => - executorLog.error(`Failed transient graph resume retry for ${task.id}:`, err), - ); + void (async () => { + try { + const resumeTask = await this.store.getTask(task.id); + const resumeFailureState = resumeTask as Task & { lastError?: unknown; failureReason?: unknown }; + if ( + resumeTask.deletedAt + || resumeTask.paused + || resumeTask.userPaused + || this.userCanceledTaskIds.has(task.id) + || resumeTask.status != null + || resumeTask.error != null + || resumeFailureState.lastError != null + || resumeFailureState.failureReason != null + || resumeTask.column !== failureLanes.wip + || (await resolveTerminalColumnsFor(this.store, resumeTask.id)).includes(resumeTask.column) + || this.executing.has(task.id) + || this.activeSessions.has(task.id) + || this.activeStepExecutors.has(task.id) + || this.activeWorkflowStepSessions.has(task.id) + || this.activeCliTaskSessions.has(task.id) + || this.activeWorkflowGraphAbortControllers.has(task.id) + || this.resumingUnpaused.has(task.id) + || TaskExecutor.processWideGraphRouting.has(task.id) + ) { + executorLog.debug(`${task.id}: skipping transient graph resume retry — task is no longer in a safe WIP resume state`); + return; + } + await this.execute(resumeTask); + } catch (err) { + executorLog.error(`Failed transient graph resume retry for ${task.id}:`, err); + } + })(); }; if (TRANSIENT_GRAPH_RESUME_RETRY_BACKOFF_MS > 0) { const handle = setTimeout(scheduleRetry, TRANSIENT_GRAPH_RESUME_RETRY_BACKOFF_MS);