From fac75563214451b8004a1e8fc2d364d383d3677d Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 28 Jun 2026 19:22:06 -0700 Subject: [PATCH] fix(FN-7210): stop merge-retry loop starving executor remediation pass recoverCompletedTask refused workflow-graph re-entry when the live task has incomplete plan steps or a remediation bounce is already scheduled. A pre-merge optional/advisory REVISE reopens steps and schedules a sendTaskBackForFix bounce; a competing graph re-entry re-passed the advisory step (budget exhausted), advanced to merge, and looped forever on the 'task has incomplete steps' gate. Defer to the bounce / stale-incomplete-review recovery so the executor finishes the steps. Fusion-Task-Id: FN-7210 --- ...ecover-completed-incomplete-steps-guard.md | 7 +++ .../src/__tests__/restart.integration.test.ts | 62 +++++++++++++++++++ packages/engine/src/executor.ts | 30 +++++++++ 3 files changed, 99 insertions(+) create mode 100644 .changeset/fn-7210-recover-completed-incomplete-steps-guard.md diff --git a/.changeset/fn-7210-recover-completed-incomplete-steps-guard.md b/.changeset/fn-7210-recover-completed-incomplete-steps-guard.md new file mode 100644 index 0000000000..af3b7ff847 --- /dev/null +++ b/.changeset/fn-7210-recover-completed-incomplete-steps-guard.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix tasks stuck in review after a code-review revision by stopping the merge-retry loop from starving the executor's fix pass. +category: fix +dev: recoverCompletedTask now refuses workflow-graph re-entry when the live task has incomplete steps or a remediation bounce (sendTaskBackForFix → scheduleWorkflowRerun) is already scheduled, so a pre-merge optional/advisory REVISE that reopens plan steps lets the executor finish them instead of re-passing the advisory step (budget exhausted) and looping on the "task has incomplete steps" merge gate. Regression: restart.integration.test.ts. diff --git a/packages/engine/src/__tests__/restart.integration.test.ts b/packages/engine/src/__tests__/restart.integration.test.ts index 6ed69d79c8..3c315e75b7 100644 --- a/packages/engine/src/__tests__/restart.integration.test.ts +++ b/packages/engine/src/__tests__/restart.integration.test.ts @@ -869,6 +869,68 @@ describe("In-progress task resume after restart", () => { // Must NOT silently finalize to review as a success. expect(store.moveTask).not.toHaveBeenCalledWith("FN-963", "in-review"); }); + + // FNXC:WorkflowOptionalStepFix 2026-06-28-12:00 (FN-7210 regression): + // A pre-merge optional/advisory REVISE reopens plan steps to `pending` and + // schedules a remediation bounce so the executor can finish them. recoverCompletedTask + // must NOT re-enter the workflow graph (and thus the merge node) while the live task + // still has incomplete steps — otherwise the re-run re-passes the advisory step + // (fix budget exhausted), advances to merge, and the merge gate refuses forever with + // "task has incomplete steps". + it("recoverCompletedTask() refuses graph re-entry when the live task has incomplete steps", async () => { + const store = createMockStore({ + getTask: vi.fn().mockResolvedValue(makeTaskDetail("FN-7210", "in-review", { + worktree: "/tmp/wt/FN-7210", + steps: makeSteps("done", "done", "pending"), + })), + }); + const task = makeTask("FN-7210", "in-review", { + worktree: "/tmp/wt/FN-7210", + steps: makeSteps("done", "done", "pending"), + }); + + const executor = new TaskExecutor(store, "/tmp/test"); + vi.spyOn(executor as any, "captureModifiedFiles").mockResolvedValue([]); + const graphEntry = vi + .spyOn(executor as any, "maybeExecuteWorkflowGraph") + .mockResolvedValue(true); + + const recovered = await executor.recoverCompletedTask(task); + + expect(recovered).toBe(false); + expect(graphEntry).not.toHaveBeenCalled(); + }); + + // FNXC:WorkflowOptionalStepFix 2026-06-28-12:00 (FN-7210 regression): + // While a remediation bounce is scheduled (scheduleWorkflowRerun registers a + // rerun watchdog synchronously), recoverCompletedTask must yield to it rather than + // racing a competing graph re-entry to the merge node. + it("recoverCompletedTask() yields to a scheduled workflow remediation bounce", async () => { + const store = createMockStore({ + getTask: vi.fn().mockResolvedValue(makeTaskDetail("FN-7211", "in-review", { + worktree: "/tmp/wt/FN-7211", + steps: makeSteps("done"), + })), + }); + const task = makeTask("FN-7211", "in-review", { + worktree: "/tmp/wt/FN-7211", + steps: makeSteps("done"), + }); + + const executor = new TaskExecutor(store, "/tmp/test"); + vi.spyOn(executor as any, "captureModifiedFiles").mockResolvedValue([]); + const graphEntry = vi + .spyOn(executor as any, "maybeExecuteWorkflowGraph") + .mockResolvedValue(true); + // Simulate a bounce already scheduled for this task. + (executor as any).workflowRerunWatchdogs.set("FN-7211", setTimeout(() => {}, 0)); + + const recovered = await executor.recoverCompletedTask(task); + + expect(recovered).toBe(false); + expect(graphEntry).not.toHaveBeenCalled(); + clearTimeout((executor as any).workflowRerunWatchdogs.get("FN-7211")); + }); }); // ── Step 3: In-review merge re-queue tests ──────────────────────────────── diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index 64dda6cd7a..22878ae075 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -3720,6 +3720,36 @@ export class TaskExecutor { executorLog.log(`${task.id}: skipping recoverCompletedTask — task has active execution in flight`); return false; } + + /* + FNXC:WorkflowOptionalStepFix 2026-06-28-12:00: + A pre-merge optional/advisory step REVISE (Code Review / Browser Verification) reopens + plan steps to `pending` and schedules a remediation bounce (sendTaskBackForFix → + scheduleWorkflowRerun) that moves the task in-review → todo → in-progress so the executor + can finish the reopened steps. Re-entering the workflow graph here while that bounce is + still scheduled — or while the live task already carries incomplete plan steps — preempts + the executor's single fix cycle: the re-run re-passes the advisory step (its fix budget is + now exhausted), advances to the `merge` node, and the merge gate refuses with + "task has incomplete steps" forever (observed on FN-7210; the FN-7122 bounce fix handled + the column race but not this competing graph re-entry). recoverCompletedTask only owns + tasks whose work is genuinely COMPLETE, so refuse re-entry when a remediation bounce is in + flight or the live task has non-terminal steps, and let the bounce / stale-incomplete-review + recovery re-launch execution instead. + */ + if (this.workflowRerunWatchdogs.has(task.id) || this.workflowRerunPending.has(task.id)) { + executorLog.log(`${task.id}: skipping recoverCompletedTask — workflow remediation bounce already scheduled`); + return false; + } + const liveForCompletenessCheck = await this.store.getTask(task.id).catch(() => task); + if ( + liveForCompletenessCheck + && (liveForCompletenessCheck.steps?.length ?? 0) > 0 + && !this.isTaskWorkComplete(liveForCompletenessCheck) + ) { + executorLog.log(`${task.id}: skipping recoverCompletedTask — task has incomplete steps awaiting executor remediation`); + return false; + } + const settings = await this.store.getSettings(); if (settings.globalPause || settings.enginePaused) { executorLog.log(