diff --git a/.changeset/fix-in-review-optional-step-fix-bounce.md b/.changeset/fix-in-review-optional-step-fix-bounce.md new file mode 100644 index 0000000000..c06bc6f218 --- /dev/null +++ b/.changeset/fix-in-review-optional-step-fix-bounce.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix tasks getting stuck in review forever after a pre-merge code-review revision. +category: fix +dev: performWorkflowRerunBounce now bounces an `in-review` task back to in-progress like `in-progress`/`todo`, instead of throwing "cannot bounce to in-progress". A pre-merge optional-step REVISE reopens the last plan step and schedules the bounce, but a completion race could land the task in-review first, stranding it with a pending step that the merge gate blocks on while self-healing only re-ran the graph. Regression covered in executor-step-session.test.ts (FN-7122). diff --git a/packages/engine/src/__tests__/executor-step-session.test.ts b/packages/engine/src/__tests__/executor-step-session.test.ts index fd8c3f9d91..bd103cd448 100644 --- a/packages/engine/src/__tests__/executor-step-session.test.ts +++ b/packages/engine/src/__tests__/executor-step-session.test.ts @@ -633,6 +633,67 @@ describe("Workflow Steps Execution", () => { injectSpy.mockRestore(); }); + // FNXC:WorkflowOptionalStepFix 2026-06-27-13:30: + // Regression for the FN-7122 deadlock: a pre-merge optional-step REVISE + // reopens the last plan step to `pending` and schedules a rerun bounce, but + // a completion race can land the task in `in-review` BEFORE the setTimeout(0) + // bounce fires. The bounce previously only handled `in-progress`/`todo` and + // THREW on `in-review`, stranding the task in-review with a `pending` step: + // the merge gate blocks forever and self-healing only re-runs the graph + // (re-passing the advisory step) without re-launching the executor. The + // invariant: performWorkflowRerunBounce must bounce an `in-review` task back + // to in-progress exactly like an `in-progress` task, so the reopened step is + // actually re-executed. + it("bounces an in-review task back to in-progress (FN-7122 deadlock)", async () => { + vi.useRealTimers(); + + const store = createMockStore(); + const mutableTask = { + id: "FN-7122", + title: "Test", + description: "Test task", + // The completion race left the task in in-review while the optional-step + // fix bounce was queued. + column: "in-review" as const, + dependencies: [] as string[], + steps: [ + { name: "Step 0", status: "done" as const }, + // Last step reopened by reopenLastStepForRevision — this is the step + // the merge gate blocks on until the executor re-runs. + { name: "Documentation & Delivery", status: "pending" as const }, + ], + currentStep: 1, + log: [] as any[], + worktree: "/tmp/test/worktree", + executionStartedAt: new Date().toISOString(), + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + }; + store.getTask.mockImplementation(async () => mutableTask); + + const onError = vi.fn(); + const executor = new TaskExecutor(store, "/tmp/test", { onError }); + + const outcome = await (executor as unknown as { + performWorkflowRerunBounce: ( + taskId: string, + worktreePath: string, + preserveResumeState?: boolean, + ) => Promise; + }).performWorkflowRerunBounce("FN-7122", mutableTask.worktree, true); + + // The bounce succeeds instead of throwing "cannot bounce to in-progress". + expect(outcome).toBe("bounced"); + // in-review → todo (preserving step progress + worktree) → in-progress, so + // the reopened step is re-executed rather than left stranded. + expect(store.moveTask).toHaveBeenCalledWith("FN-7122", "todo", { + preserveResumeState: true, + preserveWorktree: true, + }); + expect(store.moveTask).toHaveBeenCalledWith("FN-7122", "in-progress"); + expect(onError).not.toHaveBeenCalled(); + }); + }); describe("Real-time steering injection", () => { diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 56b0d799cb..9dd5587807 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -3175,9 +3175,24 @@ export class TaskExecutor { return "deferred-paused"; } - if (latestTask.column === "in-progress") { + /* + FNXC:WorkflowOptionalStepFix 2026-06-27-13:30: + A pre-merge optional step REVISE (Code Review / Browser Verification) schedules this + bounce via sendTaskBackForFix AFTER reopening the last plan step to `pending`. The + graph run that hosted that step reports `disposition: "completed"`, so the outer + completion flow can route the task to `in-review` BEFORE this setTimeout(0) bounce + runs. Previously the bounce only handled `in-progress`/`todo` and THREW on `in-review` + ("cannot bounce to in-progress"), leaving the task stranded in-review with a `pending` + step: the merge gate blocks forever on the incomplete step while self-healing only + re-runs the workflow graph (re-passing the advisory step) and never re-launches the + executor to finish the reopened step — a permanent deadlock (observed on FN-7122). + The bounce's ONLY caller is sendTaskBackForFix, which unconditionally intends to send + the task back for remediation, so `in-review` must bounce back exactly like + `in-progress` regardless of the column the completion race left it in. + */ + if (latestTask.column === "in-progress" || latestTask.column === "in-review") { const originalExecutionStartedAt = latestTask.executionStartedAt; - // Preserve step progress across the in-progress → todo hop: + // Preserve step progress across the in-progress/in-review → todo hop: // moveTask's default reopen-to-todo path resets every step to // pending and rewrites PROMPT.md checkboxes, which would discard // the partial progress this bounce is supposed to retry on top of.