root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(FN-2883): reset merge state when re-entering in-progress

Browse Source 
- Reset merge metadata, verification counters, and workflow results when tasks move from in-review/done back to in-progress
- Reopen verification-related steps (or the last step fallback) so re-verification runs from a pending state
- Add execute-time guard to clear stale mergeDetails on in-progress tasks before continuing
- Prevent resumeOrphaned fast-path recovery when completed in-progress tasks still carry merge metadata
- Add targeted executor and TaskForm tests covering FN-2883 regression paths
This commit is contained in:
Fusion
2026-04-28 10:09:59 -07:00
committed by gsxdsm
parent 2a26d61b2b
commit 78836b1969
4 changed files with 345 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

202
packages/engine/src/__tests__/executor.test.ts
View File

@@ -10324,6 +10324,127 @@ describe("TaskExecutor agent execution flow (FN-978)", () => {
expect(mockedCreateFnAgent).not.toHaveBeenCalled();
});
describe("merge-state reset when returning to in-progress (FN-2883)", () => {
it("resets merge state on in-review → in-progress move", async () => {
const store = createMockStore();
const executor = new TaskExecutor(store, "/tmp/test");
const executeSpy = vi.spyOn(executor, "execute").mockResolvedValue(undefined);
const movedTask = {
id: "FN-2883-A",
title: "Merge retry",
description: "desc",
column: "in-progress" as const,
dependencies: [],
steps: [
{ name: "Step 0: Preflight", status: "done" },
{ name: "Step 1: Implementation", status: "done" },
{ name: "Step 2: Testing & Verification", status: "done" },
{ name: "Step 3: Documentation & Delivery", status: "done" },
],
currentStep: 3,
log: [],
mergeDetails: { strategy: "manual" } as any,
mergeRetries: 2,
verificationFailureCount: 1,
workflowStepResults: [{ id: "wf-1", status: "passed" }],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
};
store.getTask.mockResolvedValue(movedTask);
store._trigger("task:moved", { task: movedTask, from: "in-review", to: "in-progress" });
await new Promise((resolve) => setTimeout(resolve, 20));
expect(store.updateTask).toHaveBeenCalledWith("FN-2883-A", expect.objectContaining({
mergeDetails: null,
mergeRetries: 0,
verificationFailureCount: 0,
workflowStepResults: [],
}));
expect(store.updateStep).toHaveBeenCalledWith("FN-2883-A", 3, "pending");
expect(store.logEntry).toHaveBeenCalledWith(
"FN-2883-A",
expect.stringContaining("Task returned to in-progress from in-review column"),
undefined,
undefined,
);
expect(executeSpy).toHaveBeenCalled();
});
it("resets merge state on done → in-progress move", async () => {
const store = createMockStore();
const executor = new TaskExecutor(store, "/tmp/test");
vi.spyOn(executor, "execute").mockResolvedValue(undefined);
const movedTask = {
id: "FN-2883-B",
title: "Done rollback",
description: "desc",
column: "in-progress" as const,
dependencies: [],
steps: [
{ name: "Step 0: Preflight", status: "done" },
{ name: "Step 1: Testing & Verification", status: "done" },
{ name: "Step 2: Documentation & Delivery", status: "done" },
],
currentStep: 2,
log: [],
mergeDetails: { strategy: "ours" } as any,
mergeRetries: 1,
verificationFailureCount: 2,
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
};
store.getTask.mockResolvedValue(movedTask);
store._trigger("task:moved", { task: movedTask, from: "done", to: "in-progress" });
await new Promise((resolve) => setTimeout(resolve, 20));
expect(store.updateTask).toHaveBeenCalledWith("FN-2883-B", expect.objectContaining({
mergeDetails: null,
mergeRetries: 0,
verificationFailureCount: 0,
workflowStepResults: [],
}));
expect(store.updateStep).toHaveBeenCalledWith("FN-2883-B", 2, "pending");
expect(store.logEntry).toHaveBeenCalledWith(
"FN-2883-B",
expect.stringContaining("Task returned to in-progress from done column"),
undefined,
undefined,
);
});
it("does not reset merge state on todo → in-progress move", async () => {
const store = createMockStore();
const executor = new TaskExecutor(store, "/tmp/test");
vi.spyOn(executor, "execute").mockResolvedValue(undefined);
const movedTask = {
id: "FN-2883-C",
title: "Fresh start",
description: "desc",
column: "in-progress" as const,
dependencies: [],
steps: [{ name: "Step 0: Preflight", status: "pending" }],
currentStep: 0,
log: [],
mergeDetails: null,
mergeRetries: 0,
verificationFailureCount: 0,
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
};
store._trigger("task:moved", { task: movedTask, from: "todo", to: "in-progress" });
await new Promise((resolve) => setTimeout(resolve, 20));
expect(store.updateTask).not.toHaveBeenCalledWith("FN-2883-C", expect.objectContaining({ mergeDetails: null }));
expect(store.updateStep).not.toHaveBeenCalled();
});
});
describe("when task is moved away from in-progress", () => {
it("terminates active session and removes from activeSessions map", async () => {
const store = createMockStore();
@@ -10743,6 +10864,87 @@ describe("TaskExecutor agent execution flow (FN-978)", () => {
});
});
describe("FN-2883 fast-path guards", () => {
beforeEach(() => {
vi.clearAllMocks();
mockedExistsSync.mockReturnValue(true);
});
it("execute() defensively clears stale mergeDetails for in-progress tasks", async () => {
const store = createMockStore();
const executor = new TaskExecutor(store, "/tmp/test");
const cleanupSpy = vi.spyOn(executor as any, "cleanupMergeStateForReverification")
.mockResolvedValue({
id: "FN-2883-D",
title: "stale merge",
description: "desc",
column: "in-progress",
dependencies: [],
steps: [{ name: "Step 0", status: "pending" }],
currentStep: 0,
log: [],
mergeDetails: null,
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
});
const task = {
id: "FN-2883-D",
title: "stale merge",
description: "desc",
column: "in-progress" as const,
dependencies: [],
steps: [{ name: "Step 0", status: "pending" }],
currentStep: 0,
log: [],
mergeDetails: { strategy: "theirs" } as any,
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
};
await executor.execute(task as any);
expect(cleanupSpy).toHaveBeenCalledWith(
expect.objectContaining({ id: "FN-2883-D" }),
expect.stringContaining("stale merge state"),
);
});
it("resumeOrphaned does not fast-path completed tasks that still have mergeDetails", async () => {
const store = createMockStore();
const executor = new TaskExecutor(store, "/tmp/test");
store.listTasks.mockResolvedValue([
{
id: "FN-2883-E",
title: "orphan",
description: "desc",
column: "in-progress",
paused: false,
dependencies: [],
steps: [
{ name: "Step 0", status: "done" },
{ name: "Step 1", status: "done" },
],
currentStep: 1,
log: [],
mergeDetails: { strategy: "manual" },
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
},
]);
const executeSpy = vi.spyOn(executor, "execute").mockResolvedValue(undefined);
const recoverSpy = vi.spyOn(executor, "recoverCompletedTask").mockResolvedValue(false);
await executor.resumeOrphaned();
expect(recoverSpy).not.toHaveBeenCalled();
expect(executeSpy).toHaveBeenCalledWith(expect.objectContaining({ id: "FN-2883-E" }));
});
});
// ── StepSessionExecutor integration tests ──────────────────────────────────
describe("StepSessionExecutor integration", () => {

80
packages/engine/src/executor.ts
View File

@@ -595,7 +595,10 @@ export class TaskExecutor {
executorLog.log(`[event:task:moved] ${task.id}: ${from} → ${to}`);
if (to === "in-progress") {
executorLog.log(`[event:task:moved] Initiating execute() for ${task.id}`);
this.execute(task).catch((err) =>
void (async () => {
const taskForExecution = await this.resetMergeStateIfNeeded(task, from);
await this.execute(taskForExecution);
})().catch((err) =>
executorLog.error(`Failed to start ${task.id}:`, err),
);
} else if (from === "in-progress") {
@@ -841,6 +844,71 @@ export class TaskExecutor {
return task.steps.every((s) => s.status === "done" || s.status === "skipped");
}
private async resetMergeStateIfNeeded(task: Task, from: Task["column"]): Promise<Task> {
if (from !== "in-review" && from !== "done") {
return task;
}
const hasMergeEvidence = Boolean(task.mergeDetails)
|| (task.mergeRetries ?? 0) > 0
|| (task.verificationFailureCount ?? 0) > 0
|| task.status === "merging"
|| task.status === "merging-pr";
if (!hasMergeEvidence) {
return task;
}
return this.cleanupMergeStateForReverification(
task,
`Task returned to in-progress from ${from} column — resetting verification steps and merge state for re-verification`,
);
}
private async cleanupMergeStateForReverification(task: Task, logMessage: string): Promise<Task> {
await this.store.updateTask(task.id, {
mergeDetails: null,
mergeRetries: 0,
verificationFailureCount: 0,
workflowStepResults: [],
});
const refreshedTask = await this.store.getTask(task.id);
const steps = refreshedTask.steps ?? [];
if (steps.length > 0) {
const allStepsComplete = this.isTaskWorkComplete(refreshedTask);
if (allStepsComplete) {
await this.reopenLastStepForRevision(task.id, refreshedTask);
} else {
const resetIndexes = new Set<number>();
for (let i = 0; i < steps.length; i++) {
const name = steps[i].name.toLowerCase();
if (/testing|verification/.test(name) || /documentation|delivery/.test(name)) {
resetIndexes.add(i);
}
}
if (resetIndexes.size === 0) {
const reopened = await this.reopenLastStepForRevision(task.id, refreshedTask);
if (reopened) {
resetIndexes.add(reopened.index);
}
} else {
for (const index of resetIndexes) {
if (steps[index].status !== "pending") {
await this.store.updateStep(task.id, index, "pending");
}
}
const earliestIndex = Math.min(...Array.from(resetIndexes));
await this.store.updateTask(task.id, { currentStep: earliestIndex });
}
}
}
await this.store.logEntry(task.id, logMessage, undefined, this.currentRunContext);
return this.store.getTask(task.id);
}
private isNoProgressNoTaskDoneFailure(task: Task): boolean {
return task.status === "failed" &&
task.error?.includes("without calling fn_task_done") === true &&
@@ -1156,7 +1224,7 @@ export class TaskExecutor {
for (const task of inProgress) {
// Fast-path: if the task already completed its work (all steps done),
// move it directly to in-review instead of re-executing from scratch.
if (this.isTaskWorkComplete(task)) {
if (this.isTaskWorkComplete(task) && !task.mergeDetails) {
if (this.recoveringCompleted.has(task.id)) {
executorLog.log(`${task.id} completed-task recovery already running - skipping duplicate startup recovery`);
continue;
@@ -1337,6 +1405,14 @@ export class TaskExecutor {
// executor can still recover by falling through to the fresh-worktree
// path below, but we emit a loud audit record so these states stop being
// silent.
if (task.column === "in-progress" && task.mergeDetails) {
executorLog.warn(`${task.id}: stale mergeDetails found while executing in-progress task — resetting merge state before continuing`);
task = await this.cleanupMergeStateForReverification(
task,
"Executor detected stale merge state while task was in-progress — reset verification steps and merge metadata before resuming",
);
}
if (task.column === "in-progress" && !task.worktree) {
executorLog.error(
`${task.id}: drift detected — task is in-progress with no worktree. ` +