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 f3b098cc15
commit ad0a1f489b
4 changed files with 345 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

111
packages/dashboard/app/components/SettingsModal.tsx
View File

@@ -147,8 +147,8 @@ function useGitHubStarCount(): number | null {
* The sidebar is organized into three groups:
* - Account: Scope-less sections (authentication)
* - Global: Global-scoped sections (appearance, notifications, node-sync, global-models)
* - Project: Project-scoped sections (project-models, general, scheduling, worktrees, commands,
* merge, memory, experimental, prompts, backups, plugins)
* - Project: Project-scoped sections (project-models, general, scheduling, node-routing,
* worktrees, commands, merge, memory, experimental, prompts, backups, plugins)
*
* To add a new section:
* 1. Add an entry to SETTINGS_SECTIONS with a unique id, label, and scope
@@ -190,6 +190,7 @@ const SETTINGS_SECTIONS: SettingsSection[] = [
{ id: "general", label: "General", scope: "project" },
{ id: "project-models", label: "Project Models", scope: "project" },
{ id: "scheduling", label: "Scheduling", scope: "project" },
{ id: "node-routing", label: "Node Routing", scope: "project" },
{ id: "worktrees", label: "Worktrees", scope: "project" },
{ id: "commands", label: "Commands", scope: "project" },
{ id: "merge", label: "Merge", scope: "project" },
@@ -2409,55 +2410,6 @@ export function SettingsModal({
/>
<small>Maximum concurrent planning agents</small>
</div>
<div className="form-group">
<label htmlFor="defaultNodeId">Default Execution Node</label>
<select
id="defaultNodeId"
className="select"
value={typeof form.defaultNodeId === "string" ? form.defaultNodeId : ""}
onChange={(e) => {
const val = e.target.value;
setForm((f) => ({ ...f, defaultNodeId: val || undefined } as SettingsFormState));
}}
>
<option value="">Local execution (no default node)</option>
{nodes.map((node) => (
<option key={node.id} value={node.id}>
{node.name} ({getNodeStatusLabel(node.status)})
</option>
))}
</select>
{(() => {
const selectedNode = nodes.find((node) => node.id === form.defaultNodeId);
if (!selectedNode) return null;
return (
<div className={`settings-node-status ${getNodeStatusClass(selectedNode.status)}`}>
<span className="settings-node-status__dot" aria-hidden="true" />
<span>{`Selected node: ${getNodeStatusLabel(selectedNode.status)}`}</span>
</div>
);
})()}
<small>Used when a task has no node override. Node status is shown for safer routing selection.</small>
</div>
<div className="form-group">
<label htmlFor="unavailableNodePolicy">Unavailable Node Policy</label>
<select
id="unavailableNodePolicy"
className="select"
value={
form.unavailableNodePolicy === "fallback-local" ? "fallback-local" : "block"
}
onChange={(e) =>
setForm((f) => ({
...f,
unavailableNodePolicy: e.target.value as "block" | "fallback-local",
} as SettingsFormState))
}
>
<option value="block">Block execution</option>
<option value="fallback-local">Fallback to local</option>
</select>
</div>
<div className="form-group">
<label htmlFor="pollIntervalMs">Poll Interval (ms)</label>
<input
@@ -2686,6 +2638,63 @@ export function SettingsModal({
</div>
</>
);
case "node-routing":
return (
<>
{renderScopeBanner()}
<h4 className="settings-section-heading">Node Routing</h4>
<div className="form-group">
<label htmlFor="defaultNodeId">Default Execution Node</label>
<select
id="defaultNodeId"
className="select"
value={typeof form.defaultNodeId === "string" ? form.defaultNodeId : ""}
onChange={(e) => {
const val = e.target.value;
setForm((f) => ({ ...f, defaultNodeId: val || undefined } as SettingsFormState));
}}
>
<option value="">Local execution (no default node)</option>
{nodes.map((node) => (
<option key={node.id} value={node.id}>
{node.name} ({getNodeStatusLabel(node.status)})
</option>
))}
</select>
{(() => {
const selectedNode = nodes.find((node) => node.id === form.defaultNodeId);
if (!selectedNode) return null;
return (
<div className={`settings-node-status ${getNodeStatusClass(selectedNode.status)}`}>
<span className="settings-node-status__dot" aria-hidden="true" />
<span>{`Selected node: ${getNodeStatusLabel(selectedNode.status)}`}</span>
</div>
);
})()}
<small>Used when a task has no node override. Node status is shown for safer routing selection.</small>
</div>
<div className="form-group">
<label htmlFor="unavailableNodePolicy">Unavailable Node Policy</label>
<select
id="unavailableNodePolicy"
className="select"
value={
form.unavailableNodePolicy === "fallback-local" ? "fallback-local" : "block"
}
onChange={(e) =>
setForm((f) => ({
...f,
unavailableNodePolicy: e.target.value as "block" | "fallback-local",
} as SettingsFormState))
}
>
<option value="block">Block execution</option>
<option value="fallback-local">Fallback to local</option>
</select>
</div>
</>
);
case "worktrees":
return (
<>

6
packages/dashboard/app/components/__tests__/TaskForm.test.tsx
View File

@@ -778,7 +778,11 @@ describe("TaskForm preset selection (FN-819)", () => {
expect(fetchSettings).toHaveBeenCalled();
});
const presetSelect = document.getElementById("model-preset") as HTMLSelectElement;
const presetSelect = await waitFor(() => {
const element = document.getElementById("model-preset") as HTMLSelectElement | null;
expect(element).toBeTruthy();
return element as HTMLSelectElement;
});
fireEvent.change(presetSelect, { target: { value: "default" } });
expect(onPresetModeChange).toHaveBeenCalledWith("default");

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. ` +