fix(FN-8817): stop no-op lifecycle bounce
Honor verified intentional no-ops and preserve durable merger parks during workflow graph unwind. Fusion-Task-Id: FN-8817
This commit is contained in:
7
.changeset/calm-noop-finalization.md
Normal file
7
.changeset/calm-noop-finalization.md
Normal file
@@ -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.
|
||||||
@@ -40,6 +40,34 @@ describe("evaluateNoCommitsNoOpFinalize", () => {
|
|||||||
})).toEqual({ blocked: false, doneCount: 5, incompleteCount: 1 });
|
})).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", () => {
|
it("blocks pending or in-progress work on no-commits tasks", () => {
|
||||||
expect(evaluateNoCommitsNoOpFinalize({
|
expect(evaluateNoCommitsNoOpFinalize({
|
||||||
noCommitsExpected: true,
|
noCommitsExpected: true,
|
||||||
|
|||||||
@@ -28,6 +28,9 @@ export function evaluateNoCommitsNoOpFinalize(
|
|||||||
const noCommitsExpected = task.noCommitsExpected === true;
|
const noCommitsExpected = task.noCommitsExpected === true;
|
||||||
|
|
||||||
const skippedSteps = steps.filter((step) => step.status === "skipped");
|
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`.
|
// FN-8141: skipped step + empty diff. Applies to ALL tasks regardless of `noCommitsExpected`.
|
||||||
if (skippedSteps.length > 0) {
|
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)
|
// 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 (
|
if (
|
||||||
noCommitsExpected &&
|
noCommitsExpected &&
|
||||||
steps.length > 0 &&
|
steps.length > 0 &&
|
||||||
incompleteCount > 0 &&
|
incompleteCount > 0 &&
|
||||||
// Equal counts still block: requeueing is recoverable, but silently dropping ops work is not.
|
// Equal counts still block unless positive verification proves the skips were intentional.
|
||||||
incompleteCount >= doneCount
|
incompleteCount >= doneCount &&
|
||||||
|
!verifiedIntentionalNoOp
|
||||||
) {
|
) {
|
||||||
return {
|
return {
|
||||||
blocked: true,
|
blocked: true,
|
||||||
|
|||||||
@@ -741,6 +741,40 @@ describe("runAiMerge", () => {
|
|||||||
expect(store.moveTask).toHaveBeenCalledWith("FN-1", "done", expect.objectContaining({ moveSource: "engine", preserveProgress: true }));
|
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
|
* 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
|
* the executor reverted its own work. A commit-expected empty branch must not finalize `done` without
|
||||||
|
|||||||
@@ -194,6 +194,34 @@ describe("merge-node paused-abort retry classification (FN-6735)", () => {
|
|||||||
expect(store.updateTask).not.toHaveBeenCalled();
|
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([
|
it.each([
|
||||||
"merge",
|
"merge",
|
||||||
"requestMerge",
|
"requestMerge",
|
||||||
|
|||||||
@@ -11435,6 +11435,28 @@ export class TaskExecutor {
|
|||||||
return;
|
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):
|
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
|
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
|
and visited no nodes. Before this branch existed the result fell through to the
|
||||||
|
|||||||
Reference in New Issue
Block a user