diff --git a/.changeset/fn-6141-benign-graph-exit.md b/.changeset/fn-6141-benign-graph-exit.md new file mode 100644 index 0000000000..5dd47fda40 --- /dev/null +++ b/.changeset/fn-6141-benign-graph-exit.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": patch +--- + +Stop classifying benign workflow-graph exits after a task already advanced or paused as failures. These exits now use info-level benign wording while genuine in-progress graph failures keep the existing failure handling. diff --git a/packages/engine/src/__tests__/executor-recovery.test.ts b/packages/engine/src/__tests__/executor-recovery.test.ts index 41a673e6dd..112ca63678 100644 --- a/packages/engine/src/__tests__/executor-recovery.test.ts +++ b/packages/engine/src/__tests__/executor-recovery.test.ts @@ -601,12 +601,147 @@ describe("TaskExecutor bounded recovery retries", () => { expect(store.handoffToReview).not.toHaveBeenCalled(); expect(store.logEntry).toHaveBeenCalledWith( "FN-001", - "Workflow graph terminated with failure at node 'execute' (task already todo - preserving recovered lifecycle state)", + "Workflow graph run ended after task already advanced to 'todo' — no further action needed", undefined, undefined, ); }); + it.each(["in-review", "done"] as const)( + "treats a graph exit after task advanced to %s as benign", + async (column) => { + const store = createMockStore(); + const task = { + id: "FN-001", + title: "Test", + description: "Test", + column: "in-progress", + status: undefined, + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + } as Task; + store.getTask.mockResolvedValue({ + ...task, + column, + status: undefined, + error: null, + }); + const warnSpy = vi.spyOn(executorLog, "warn").mockImplementation(() => undefined); + const executor = new TaskExecutor(store, "/tmp/test", {}); + + await (executor as any).handleGraphFailure(task, { + visitedNodeIds: ["execute"], + }); + + const expectedMessage = `Workflow graph run ended after task already advanced to '${column}' — no further action needed`; + expect(store.logEntry).toHaveBeenCalledWith("FN-001", expectedMessage, undefined, undefined); + expect(store.logEntry.mock.calls.map((call) => call[1]).join("\n")).not.toContain("terminated with failure"); + expect(store.updateTask).not.toHaveBeenCalledWith( + "FN-001", + expect.objectContaining({ status: "failed" }), + expect.anything(), + ); + expect(store.updateTask).not.toHaveBeenCalledWith( + "FN-001", + expect.objectContaining({ error: expect.anything() }), + expect.anything(), + ); + expect(store.handoffToReview).not.toHaveBeenCalled(); + expect(warnSpy).not.toHaveBeenCalledWith(expect.stringContaining("terminated with failure")); + warnSpy.mockRestore(); + }, + ); + + it("treats a graph exit while task is paused as benign", async () => { + const store = createMockStore(); + const task = { + id: "FN-001", + title: "Test", + description: "Test", + column: "in-progress", + status: undefined, + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + } as Task; + store.getTask.mockResolvedValue({ + ...task, + column: "in-progress", + paused: true, + status: undefined, + error: null, + }); + const executor = new TaskExecutor(store, "/tmp/test", {}); + + await (executor as any).handleGraphFailure(task, { + visitedNodeIds: ["execute"], + }); + + expect(store.logEntry).toHaveBeenCalledWith( + "FN-001", + "Workflow graph run ended while task is paused — pause state preserved", + undefined, + undefined, + ); + expect(store.updateTask).not.toHaveBeenCalledWith( + "FN-001", + expect.objectContaining({ status: "failed" }), + expect.anything(), + ); + expect(store.handoffToReview).not.toHaveBeenCalled(); + }); + + it("preserves genuine in-progress graph failure handling", async () => { + const store = createMockStore(); + const task = { + id: "FN-001", + title: "Test", + description: "Test", + column: "in-progress", + status: undefined, + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + } as Task; + store.getTask.mockResolvedValue({ + ...task, + column: "in-progress", + paused: false, + status: undefined, + error: null, + }); + const warnSpy = vi.spyOn(executorLog, "warn").mockImplementation(() => undefined); + const executor = new TaskExecutor(store, "/tmp/test", {}); + + await (executor as any).handleGraphFailure(task, { + visitedNodeIds: [], + }); + + const message = "Workflow graph terminated with failure at node 'unknown'"; + expect(warnSpy).toHaveBeenCalledWith(`FN-001: ${message}`); + expect(store.logEntry).toHaveBeenCalledWith("FN-001", message, undefined, undefined); + expect(store.updateTask).toHaveBeenCalledWith( + "FN-001", + { error: message, status: "failed" }, + undefined, + ); + expect(store.handoffToReview).toHaveBeenCalledWith( + "FN-001", + expect.objectContaining({ evidence: expect.objectContaining({ reason: "workflow-graph-failed" }) }), + ); + warnSpy.mockRestore(); + }); + it("preserves step progress when requeuing stuck task by default", async () => { const store = createMockStore(); const executor = new TaskExecutor(store, "/tmp/test", {}); diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 20074cf5c9..637b78cabf 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -5957,9 +5957,6 @@ export class TaskExecutor { /** Terminal failure of a graph run: record the error and park the task in * review so a human can act — never leave it invisible in in-progress. */ private async handleGraphFailure(task: Task, result: WorkflowGraphTaskRunResult): Promise { - const failedNode = result.visitedNodeIds[result.visitedNodeIds.length - 1]; - const message = `Workflow graph terminated with failure at node '${failedNode ?? "unknown"}'`; - executorLog.warn(`${task.id}: ${message}`); this.clearCompletedTaskWatchdog(task.id); this.options.stuckTaskDetector?.untrackTask(task.id); try { @@ -5967,22 +5964,20 @@ export class TaskExecutor { // A paused/aborted implementation is not a graph failure — leave the // pause machinery in charge instead of parking the task in review. if (live.paused || this.pausedAborted.has(task.id)) { - executorLog.log(`${task.id}: graph run ended while task is paused — leaving pause state untouched`); - await this.store.logEntry(task.id, `${message} (task paused — not parked)`, undefined, this.getRunContextFor(task.id)); + const benignMessage = "Workflow graph run ended while task is paused — pause state preserved"; + executorLog.log(`${task.id}: ${benignMessage}`); + await this.store.logEntry(task.id, benignMessage, undefined, this.getRunContextFor(task.id)); return; } if (live.column !== "in-progress") { - executorLog.log( - `${task.id}: graph run ended after task moved to '${live.column}' - preserving recovered lifecycle state`, - ); - await this.store.logEntry( - task.id, - `${message} (task already ${live.column} - preserving recovered lifecycle state)`, - undefined, - this.getRunContextFor(task.id), - ); + const benignMessage = `Workflow graph run ended after task already advanced to '${live.column}' — no further action needed`; + executorLog.log(`${task.id}: ${benignMessage}`); + await this.store.logEntry(task.id, benignMessage, undefined, this.getRunContextFor(task.id)); return; } + const failedNode = result.visitedNodeIds[result.visitedNodeIds.length - 1]; + const message = `Workflow graph terminated with failure at node '${failedNode ?? "unknown"}'`; + executorLog.warn(`${task.id}: ${message}`); await this.store.logEntry(task.id, message, undefined, this.getRunContextFor(task.id)); // status "failed" doubles as the self-healing exemption: review-task // revival sweeps skip tasks carrying a non-null status, preventing the