diff --git a/.changeset/calm-noop-finalization.md b/.changeset/calm-noop-finalization.md new file mode 100644 index 0000000000..23605cece7 --- /dev/null +++ b/.changeset/calm-noop-finalization.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Stop verified no-op tasks from repeatedly bouncing between lifecycle states. +category: fix +dev: Trust verified intentional skips and preserve durable merger blockers during graph unwind. diff --git a/packages/core/src/__tests__/no-commits-finalize-guard.test.ts b/packages/core/src/__tests__/no-commits-finalize-guard.test.ts index 971c5c46cb..f779f292ca 100644 --- a/packages/core/src/__tests__/no-commits-finalize-guard.test.ts +++ b/packages/core/src/__tests__/no-commits-finalize-guard.test.ts @@ -40,6 +40,34 @@ describe("evaluateNoCommitsNoOpFinalize", () => { })).toEqual({ blocked: false, doneCount: 5, incompleteCount: 1 }); }); + it("allows intentional no-op tasks when all remaining steps are done", () => { + expect(evaluateNoCommitsNoOpFinalize({ + noCommitsExpected: true, + steps: namedSteps([ + ["Preflight", "done"], + ["Restore the invariant if needed", "skipped"], + ["Apply the invariant everywhere", "skipped"], + ["Add regressions if needed", "skipped"], + ["Testing & Verification", "done"], + ["Documentation & Delivery", "done"], + ]), + })).toEqual({ blocked: false, doneCount: 3, incompleteCount: 3 }); + }); + + it("still blocks an equal done/skipped split without completed verification", () => { + expect(evaluateNoCommitsNoOpFinalize({ + noCommitsExpected: true, + steps: namedSteps([ + ["Preflight", "done"], + ["Apply", "done"], + ["Document", "done"], + ["Deploy", "skipped"], + ["Announce", "skipped"], + ["Follow up", "skipped"], + ]), + })).toMatchObject({ blocked: true, doneCount: 3, incompleteCount: 3 }); + }); + it("blocks pending or in-progress work on no-commits tasks", () => { expect(evaluateNoCommitsNoOpFinalize({ noCommitsExpected: true, diff --git a/packages/core/src/merge/no-commits-finalize-guard.ts b/packages/core/src/merge/no-commits-finalize-guard.ts index 94f8ec938f..d6e7798b24 100644 --- a/packages/core/src/merge/no-commits-finalize-guard.ts +++ b/packages/core/src/merge/no-commits-finalize-guard.ts @@ -28,6 +28,9 @@ export function evaluateNoCommitsNoOpFinalize( const noCommitsExpected = task.noCommitsExpected === true; const skippedSteps = steps.filter((step) => step.status === "skipped"); + const hasCompletedVerification = steps.some((step) => + step.status === "done" && VERIFICATION_STEP_NAME.test(step.name ?? ""), + ); // FN-8141: skipped step + empty diff. Applies to ALL tasks regardless of `noCommitsExpected`. if (skippedSteps.length > 0) { @@ -65,13 +68,20 @@ export function evaluateNoCommitsNoOpFinalize( } // Legacy FN-6461 rule: no-commits ops tasks whose incomplete work (incl. pending/in-progress) - // ties or outweighs completed work must not finalize on step evidence alone. + // ties or outweighs completed work must not finalize on step evidence alone. A verified + // no-op is the exception: skipped implementation steps are intentional when every other + // step is done and a verification/review step positively confirmed there was no work to land. + const verifiedIntentionalNoOp = + skippedSteps.length > 0 && + skippedSteps.length === incompleteCount && + hasCompletedVerification; if ( noCommitsExpected && steps.length > 0 && incompleteCount > 0 && - // Equal counts still block: requeueing is recoverable, but silently dropping ops work is not. - incompleteCount >= doneCount + // Equal counts still block unless positive verification proves the skips were intentional. + incompleteCount >= doneCount && + !verifiedIntentionalNoOp ) { return { blocked: true, diff --git a/packages/engine/src/__tests__/merger-ai.test.ts b/packages/engine/src/__tests__/merger-ai.test.ts index 00e7971cd7..6f178e9cc6 100644 --- a/packages/engine/src/__tests__/merger-ai.test.ts +++ b/packages/engine/src/__tests__/merger-ai.test.ts @@ -741,6 +741,40 @@ describe("runAiMerge", () => { expect(store.moveTask).toHaveBeenCalledWith("FN-1", "done", expect.objectContaining({ moveSource: "engine", preserveProgress: true })); }); + it("finalizes a verified intentional no-op instead of bouncing it back to todo", async () => { + const { dir } = initRepoWithBranch({ branch: "fusion/fn-1" }); + git(dir, "merge -q fusion/fn-1"); + const { store, task } = makeStore(dir, { + noCommitsExpected: true, + steps: [ + { name: "Preflight", status: "done" }, + { name: "Restore the invariant if needed", status: "skipped" }, + { name: "Apply the invariant everywhere", status: "skipped" }, + { name: "Add regressions if needed", status: "skipped" }, + { name: "Testing & Verification", status: "done" }, + { name: "Documentation & Delivery", status: "done" }, + ], + }); + + const result = await runAiMerge(store, dir, "FN-1", { manual: true }, { + mergeAgent: vi.fn(async () => { /* nothing to do */ }), + reviewAgent: vi.fn(async () => "REVIEW_VERDICT: approve"), + }); + + expect(result).toMatchObject({ noOp: true, merged: false, ok: true }); + expect(task.column).toBe("done"); + expect(store.moveTask).toHaveBeenCalledWith( + "FN-1", + "done", + expect.objectContaining({ moveSource: "engine", preserveProgress: true }), + ); + expect(store.moveTask).not.toHaveBeenCalledWith( + "FN-1", + "todo", + expect.anything(), + ); + }); + /* * FN-8141 regression: the AI empty-merge lane laundered a task whose branch was empty ONLY because * the executor reverted its own work. A commit-expected empty branch must not finalize `done` without diff --git a/packages/engine/src/__tests__/reliability-interactions/merge-node-paused-abort-retryable.test.ts b/packages/engine/src/__tests__/reliability-interactions/merge-node-paused-abort-retryable.test.ts index 2b5110bc73..a4d1d6fe36 100644 --- a/packages/engine/src/__tests__/reliability-interactions/merge-node-paused-abort-retryable.test.ts +++ b/packages/engine/src/__tests__/reliability-interactions/merge-node-paused-abort-retryable.test.ts @@ -194,6 +194,34 @@ describe("merge-node paused-abort retry classification (FN-6735)", () => { expect(store.updateTask).not.toHaveBeenCalled(); }); + it.each([ + "merge", + "requestMerge", + "merge-gate", + "merge-attempt", + "manual-merge-hold", + "merge-manual-hold", + "retry-backoff", + "merge-retry", + ] as const)("honors a durable merger blocker at node %s after the task rebounded", async (nodeId) => { + const blocker = "no-commits task has incomplete work with no net branch changes"; + const { store, task, executor, mergeRequester } = makeHarness({ + column: "todo", + status: null, + error: blocker, + paused: false, + }); + + await invokeGraphFailure(executor, task, nodeId, blocker); + + expect(mergeRequester).not.toHaveBeenCalled(); + expect(store.updateTask).not.toHaveBeenCalled(); + expect(store.moveTask).not.toHaveBeenCalled(); + const messages = logText(store); + expect(messages).toContain("honoring park, not retrying or resuming merge"); + expect(messages).not.toContain("routed to bounded auto-merge retry"); + }); + it.each([ "merge", "requestMerge", diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index c533c6cc1b..85d0dc5591 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -11435,6 +11435,28 @@ export class TaskExecutor { return; } /* + FNXC:WorkflowMerge 2026-08-06-14:41: + A merge requester can deliberately reject finalization, persist the blocker in `error`, and + rebound the task to its workflow hold column. The graph then unwinds as a merge-node failure. + Retrying or resuming that stale graph overrides the merger's durable decision and creates an + unbounded hold -> merge -> hold loop. Honor the fresh parked row before any retry router; an + operator retry can clear the error and start a new graph run explicitly. + */ + const parkedMergeNode = result.visitedNodeIds[result.visitedNodeIds.length - 1]; + if ( + live.error != null && + live.column === failureLanes.hold && + this.isMergeGraphFailure(parkedMergeNode) + ) { + this.clearPausedAborted(task.id); + this.activeWorktrees.delete(task.id); + const mergerParkHonored = `Workflow graph run ended after merger parked task with blocker (${live.error}) — honoring park, not retrying or resuming merge`; + executorLog.log(`${task.id}: ${mergerParkHonored}`); + await this.store.logEntry(task.id, mergerParkHonored, undefined, this.getRunContextFor(task.id)); + await this.persistTokenUsage(task.id); + return; + } + /* FNXC:WorkflowIrPin 2026-07-19-21:10 (KTD-3 drift park, PR #2342): A graph run that exited on the drift guard carries WORKFLOW_DRIFT_PARK_CONTEXT_KEY and visited no nodes. Before this branch existed the result fell through to the