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
This commit is contained in:
@@ -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.
|
||||||
@@ -869,6 +869,68 @@ describe("In-progress task resume after restart", () => {
|
|||||||
// Must NOT silently finalize to review as a success.
|
// Must NOT silently finalize to review as a success.
|
||||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-963", "in-review");
|
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 ────────────────────────────────
|
// ── Step 3: In-review merge re-queue tests ────────────────────────────────
|
||||||
|
|||||||
@@ -3720,6 +3720,36 @@ export class TaskExecutor {
|
|||||||
executorLog.log(`${task.id}: skipping recoverCompletedTask — task has active execution in flight`);
|
executorLog.log(`${task.id}: skipping recoverCompletedTask — task has active execution in flight`);
|
||||||
return false;
|
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();
|
const settings = await this.store.getSettings();
|
||||||
if (settings.globalPause || settings.enginePaused) {
|
if (settings.globalPause || settings.enginePaused) {
|
||||||
executorLog.log(
|
executorLog.log(
|
||||||
|
|||||||
Reference in New Issue
Block a user