fix(FN-3305): reset mergeRetries when dispatching tasks to in-progress
A task whose previous run exhausted its merge budget (mergeRetries=MAX) could land back in in-review with status=null, where the merger refused it (canMergeTask false) and the ghost-review fallback bounced it back to todo every taskStuckTimeoutMs (10 min) — beating the 30 min merge cooldown reset. Each fresh execution now starts with mergeRetries=0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
5
.changeset/fn-3305-reset-merge-retries-on-dispatch.md
Normal file
5
.changeset/fn-3305-reset-merge-retries-on-dispatch.md
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Fix infinite todo↔in-review loop on tasks whose previous run exhausted their merge budget. The scheduler now resets `mergeRetries` to 0 when dispatching a task to in-progress, so each fresh execution gets a fresh merge budget. Without this, a task with `mergeRetries=MAX` and `status=null` would land back in in-review, the merger would refuse it (`canMergeTask` false), and the ghost-review fallback would bounce it to todo every 10 minutes — before the 30-minute merge-cooldown could elapse.
|
||||||
@@ -247,6 +247,44 @@ describe("Scheduler", () => {
|
|||||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "in-progress");
|
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "in-progress");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("resets mergeRetries when dispatching a task to in-progress", async () => {
|
||||||
|
// Regression: a task whose previous run exhausted its merge budget
|
||||||
|
// (mergeRetries = MAX) would, after status was cleared, land back in
|
||||||
|
// in-review with the merger refusing it (canMergeTask false) and the
|
||||||
|
// ghost-review fallback bouncing it back every taskStuckTimeoutMs —
|
||||||
|
// infinite loop. Each fresh execution must get a fresh merge budget.
|
||||||
|
vi.mocked(existsSync).mockReturnValue(true);
|
||||||
|
vi.mocked(readFile).mockResolvedValue("# Task\nDo something");
|
||||||
|
|
||||||
|
const listTasksMock = vi.fn()
|
||||||
|
.mockResolvedValueOnce([])
|
||||||
|
.mockResolvedValue([
|
||||||
|
createMockTask({ id: "FN-001", column: "todo", dependencies: [], mergeRetries: 3 }),
|
||||||
|
]);
|
||||||
|
|
||||||
|
const store = createMockStore({
|
||||||
|
listTasks: listTasksMock,
|
||||||
|
getSettings: vi.fn().mockResolvedValue({ maxConcurrent: 2, maxWorktrees: 4 }),
|
||||||
|
updateTask: vi.fn().mockResolvedValue(undefined),
|
||||||
|
moveTask: vi.fn().mockResolvedValue(undefined),
|
||||||
|
});
|
||||||
|
|
||||||
|
const scheduler = new Scheduler(store);
|
||||||
|
scheduler.start();
|
||||||
|
await flushAsyncWork();
|
||||||
|
|
||||||
|
const onCalls = (store.on as any).mock.calls;
|
||||||
|
const createdHandler = onCalls.find((call: any) => call[0] === "task:created")?.[1];
|
||||||
|
await createdHandler(createMockTask({ id: "FN-001", column: "todo", mergeRetries: 3 }));
|
||||||
|
await flushAsyncWork();
|
||||||
|
|
||||||
|
expect(store.updateTask).toHaveBeenCalledWith(
|
||||||
|
"FN-001",
|
||||||
|
expect.objectContaining({ mergeRetries: 0 }),
|
||||||
|
);
|
||||||
|
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "in-progress");
|
||||||
|
});
|
||||||
|
|
||||||
it("registers task:moved event listener", () => {
|
it("registers task:moved event listener", () => {
|
||||||
const store = createMockStore();
|
const store = createMockStore();
|
||||||
new Scheduler(store);
|
new Scheduler(store);
|
||||||
@@ -820,6 +858,7 @@ describe("Scheduler", () => {
|
|||||||
worktree: "/test/project/.worktrees/fn-010",
|
worktree: "/test/project/.worktrees/fn-010",
|
||||||
effectiveNodeId: null,
|
effectiveNodeId: null,
|
||||||
effectiveNodeSource: "local",
|
effectiveNodeSource: "local",
|
||||||
|
mergeRetries: 0,
|
||||||
});
|
});
|
||||||
expect(moveTask).toHaveBeenCalledWith("FN-010", "in-progress");
|
expect(moveTask).toHaveBeenCalledWith("FN-010", "in-progress");
|
||||||
expect(updateTask.mock.invocationCallOrder[0]).toBeLessThan(moveTask.mock.invocationCallOrder[0]);
|
expect(updateTask.mock.invocationCallOrder[0]).toBeLessThan(moveTask.mock.invocationCallOrder[0]);
|
||||||
@@ -858,6 +897,7 @@ describe("Scheduler", () => {
|
|||||||
worktree: "/test/project/.worktrees/amber-aspen",
|
worktree: "/test/project/.worktrees/amber-aspen",
|
||||||
effectiveNodeId: null,
|
effectiveNodeId: null,
|
||||||
effectiveNodeSource: "local",
|
effectiveNodeSource: "local",
|
||||||
|
mergeRetries: 0,
|
||||||
});
|
});
|
||||||
expect(updateTask).toHaveBeenNthCalledWith(2, "FN-012", {
|
expect(updateTask).toHaveBeenNthCalledWith(2, "FN-012", {
|
||||||
status: null,
|
status: null,
|
||||||
@@ -866,6 +906,7 @@ describe("Scheduler", () => {
|
|||||||
worktree: "/test/project/.worktrees/amber-aspen-2",
|
worktree: "/test/project/.worktrees/amber-aspen-2",
|
||||||
effectiveNodeId: null,
|
effectiveNodeId: null,
|
||||||
effectiveNodeSource: "local",
|
effectiveNodeSource: "local",
|
||||||
|
mergeRetries: 0,
|
||||||
});
|
});
|
||||||
|
|
||||||
randomSpy.mockRestore();
|
randomSpy.mockRestore();
|
||||||
|
|||||||
@@ -824,7 +824,13 @@ export class Scheduler {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Clear status, reserve worktree path, and then move to in-progress
|
// Clear status, reserve worktree path, and then move to in-progress.
|
||||||
|
// Reset mergeRetries so a fresh execution gets a fresh merge budget —
|
||||||
|
// otherwise a task whose previous run exhausted its 3 retries (e.g.
|
||||||
|
// verification failure that was later cleared) lands back in in-review
|
||||||
|
// with mergeRetries=MAX, the merger refuses it (canMergeTask false),
|
||||||
|
// and the ghost-review fallback bounces it back to todo every 10 min
|
||||||
|
// before the 30-min cooldown can elapse — infinite loop. See FN-3305.
|
||||||
schedulerLog.log(`Starting ${task.id}: ${task.title || task.id} (deps satisfied)`);
|
schedulerLog.log(`Starting ${task.id}: ${task.title || task.id} (deps satisfied)`);
|
||||||
await this.store.updateTask(task.id, {
|
await this.store.updateTask(task.id, {
|
||||||
status: null,
|
status: null,
|
||||||
@@ -833,6 +839,7 @@ export class Scheduler {
|
|||||||
worktree: plannedWorktree,
|
worktree: plannedWorktree,
|
||||||
effectiveNodeId: effectiveNode.nodeId ?? null,
|
effectiveNodeId: effectiveNode.nodeId ?? null,
|
||||||
effectiveNodeSource: effectiveNode.source,
|
effectiveNodeSource: effectiveNode.source,
|
||||||
|
mergeRetries: 0,
|
||||||
});
|
});
|
||||||
await this.store.moveTask(task.id, "in-progress");
|
await this.store.moveTask(task.id, "in-progress");
|
||||||
this.wasNodeBlocked.delete(task.id);
|
this.wasNodeBlocked.delete(task.id);
|
||||||
|
|||||||
Reference in New Issue
Block a user