fix(engine): keep the task worktree across replan bounces
`moveTask`'s reopen-to-todo/triage block clears `task.worktree` but leaves `task.branch` intact, and `moveTaskToReplanColumn` called it with no options. A replan bounce therefore left the row split-brained: no worktree pointer, but still owning `fusion/<id>`, which was still checked out in the worktree it had just orphaned. The next planning acquisition skipped its resume branch (gated on `task.worktree`), re-created the same branch, collided, and fell into `cleanupConflictingWorktree` — force-remove + `git branch -D` + fresh `git worktree add` + init command, on every bounce. Observed on FN-8603: two Plan Review REVISE bounces burned two full teardown/rebuild cycles for nothing, since planning writes its spec to the task store, not the worktree. Pass `preserveWorktree: true` at the shared seam, so this covers every replan mover — Plan Review REVISE, required-artifact recovery, and the executor and scheduler spec-staleness and filesystem-validation rebounds. Acquisition still re-validates, so a preserved pointer to a removed checkout self-heals as before; the rest of the replan contract (steps reset, status/error cleared) is unchanged. Regression coverage asserts the invariant across both replan-column shapes (triage and plan-in-place todo) and all three reopen origins. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
7
.changeset/replan-bounce-preserves-worktree.md
Normal file
7
.changeset/replan-bounce-preserves-worktree.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Replan bounces now keep the task worktree instead of tearing it down and re-cutting the branch.
|
||||
category: fix
|
||||
dev: `moveTaskToReplanColumn` passes `preserveWorktree: true`. `moveTask`'s reopen-to-todo/triage block cleared `task.worktree` while leaving `task.branch`, so the next planning acquisition could not resume, re-created the same `fusion/<id>` branch, collided with the orphaned worktree, and fell into `cleanupConflictingWorktree` (force-remove + `git branch -D` + fresh `git worktree add` + init command) on every bounce. Covers all replan movers: Plan Review REVISE, required-artifact recovery, and the executor/scheduler spec-staleness and filesystem-validation rebounds.
|
||||
@@ -292,7 +292,7 @@ describe("moveTaskToReplanColumn", () => {
|
||||
const store = storeWithSelection("builtin:coding-ideas");
|
||||
const target = await moveTaskToReplanColumn(store, { id: "FN-1", column: "in-progress" });
|
||||
expect(target).toBe("todo");
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-1", "todo");
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-1", "todo", { preserveWorktree: true });
|
||||
});
|
||||
|
||||
it("skips the move when the card is already in the replan column (plan-in-place)", async () => {
|
||||
@@ -302,3 +302,58 @@ describe("moveTaskToReplanColumn", () => {
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowReplan 2026-07-26-11:05:
|
||||
Symptom (FN-8603): two Plan Review REVISE bounces each logged "Removed conflicting worktree /
|
||||
Deleted branch / Cleaned up conflicting worktree, retrying" and rebuilt the checkout from scratch
|
||||
(~10s init each). `moveTask`'s reopen block clears `worktree` but keeps `branch`, so the replan
|
||||
row lost its checkout while still owning `fusion/<id>` — the next planning acquisition could not
|
||||
resume, re-created the same branch, and collided with the worktree it had just orphaned.
|
||||
|
||||
Surface enumeration — every replan-bounce mover routes through `moveTaskToReplanColumn`, so the
|
||||
invariant is asserted at that seam for ALL of them, not just the Plan Review repro:
|
||||
- Plan Review REVISE -> automatic replan (executor.ts, the reported case)
|
||||
- required-workflow-artifact planning recovery (executor.ts)
|
||||
- spec-staleness rebound inside execute() (executor.ts)
|
||||
- scheduler filesystem-validation and spec-staleness rebounds, legacy loop + workflow sweep
|
||||
Both replan-column shapes are covered (default Coding "triage" and plan-in-place Coding (Ideas)
|
||||
"todo"), as is every reopen origin column that `moveTask` treats as a reopen (in-progress,
|
||||
in-review, done). The already-in-column no-op case cannot strand a worktree because it never
|
||||
moves.
|
||||
*/
|
||||
describe("replan bounces preserve the task worktree (FN-8603)", () => {
|
||||
const REPLAN_BOUNCE_ORIGINS = ["in-progress", "in-review", "done"] as const;
|
||||
const REPLAN_COLUMN_SHAPES = [
|
||||
{ workflowId: undefined, expected: "triage", label: "default Coding (triage replan column)" },
|
||||
{ workflowId: "builtin:coding-ideas", expected: "todo", label: "Coding (Ideas) (plan-in-place todo)" },
|
||||
] as const;
|
||||
|
||||
for (const shape of REPLAN_COLUMN_SHAPES) {
|
||||
for (const from of REPLAN_BOUNCE_ORIGINS) {
|
||||
it(`preserves the worktree bouncing ${from} -> ${shape.expected} — ${shape.label}`, async () => {
|
||||
const store = storeWithSelection(shape.workflowId);
|
||||
const target = await moveTaskToReplanColumn(store, { id: "FN-8603", column: from });
|
||||
expect(target).toBe(shape.expected);
|
||||
expect(store.moveTask).toHaveBeenCalledWith(
|
||||
"FN-8603",
|
||||
shape.expected,
|
||||
expect.objectContaining({ preserveWorktree: true }),
|
||||
);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
it("preserves the worktree when the caller pre-resolved the replan column", async () => {
|
||||
// The Plan Review REVISE handler resolves the column first so it can log it, then passes
|
||||
// it in — that overload must carry the same option as the self-resolving one.
|
||||
const store = storeWithSelection(undefined);
|
||||
const target = await moveTaskToReplanColumn(store, { id: "FN-8603", column: "in-progress" }, "triage");
|
||||
expect(target).toBe("triage");
|
||||
expect(store.moveTask).toHaveBeenCalledWith(
|
||||
"FN-8603",
|
||||
"triage",
|
||||
expect.objectContaining({ preserveWorktree: true }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -68,7 +68,7 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
});
|
||||
|
||||
expect(scheduled).toBe(true);
|
||||
expect(store.moveTask).toHaveBeenCalledWith(liveTask.id, "triage");
|
||||
expect(store.moveTask).toHaveBeenCalledWith(liveTask.id, "triage", { preserveWorktree: true });
|
||||
expect(store.updateTask).toHaveBeenCalledWith(liveTask.id, expect.objectContaining({
|
||||
status: "needs-replan",
|
||||
recoveryRetryCount: 1,
|
||||
@@ -319,7 +319,7 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
expect.stringContaining("PROMPT.md is missing the new workflow-order requirement"),
|
||||
undefined,
|
||||
);
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-7066", "triage");
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-7066", "triage", { preserveWorktree: true });
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-7066", { postReviewFixCount: 1 }, undefined);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-7066", {
|
||||
status: "needs-replan",
|
||||
@@ -361,7 +361,7 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
nodeId: "plan-review",
|
||||
});
|
||||
|
||||
expect(store.moveTask).toHaveBeenCalledWith(liveTask.id, "triage");
|
||||
expect(store.moveTask).toHaveBeenCalledWith(liveTask.id, "triage", { preserveWorktree: true });
|
||||
expect(abortSpy).not.toHaveBeenCalled();
|
||||
expect((executor as any).pausedAborted.has(liveTask.id)).toBe(false);
|
||||
} finally {
|
||||
@@ -447,7 +447,7 @@ describe("TaskExecutor pre-merge optional-step fix seam", () => {
|
||||
maxRevisions: "unbounded",
|
||||
})).resolves.toBe(true);
|
||||
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-7066", "triage");
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-7066", "triage", { preserveWorktree: true });
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-7066",
|
||||
"Plan Review failed — moved to triage for automatic replan (attempt 15/unbounded)",
|
||||
|
||||
@@ -199,6 +199,27 @@ export async function resolveReplanTargetColumn(store: TaskStore, taskId: string
|
||||
* Move `task` to its workflow-aware replan column unless it is already there.
|
||||
* Pass `target` when the caller already resolved it (e.g. to log the target
|
||||
* first) so the resolve/compare/move contract still lives in one place.
|
||||
*
|
||||
* FNXC:WorkflowReplan 2026-07-26-11:05:
|
||||
* A replan bounce KEEPS the task worktree (`preserveWorktree: true`). `moveTask`'s
|
||||
* reopen-to-todo/triage block clears `task.worktree` but deliberately leaves `task.branch`
|
||||
* intact, so an unguarded replan move produced a split-brain row: no worktree pointer, but
|
||||
* still owning `fusion/<id>`, which is still checked out in the worktree that was just
|
||||
* orphaned. The next planning entry (`ensureTaskWorktreeForPlanning` ->
|
||||
* `ensureGraphCustomNodeWorktree` -> `acquireTaskWorktree`) therefore skipped its resume
|
||||
* branch (gated on `task.worktree`), tried to create the SAME branch fresh, collided, and
|
||||
* fell into `cleanupConflictingWorktree` — force-removing the previous worktree and
|
||||
* `git branch -D`-ing `fusion/<id>` before re-cutting it off the integration branch.
|
||||
* Observed on FN-8603: two Plan Review REVISE bounces burned two full teardown +
|
||||
* `git worktree add` + init-command cycles (~10s each) and two branch delete/recreate rounds
|
||||
* for zero benefit — planning writes its spec to the task store, not the worktree, so the
|
||||
* tree it is handed is the tree it should keep.
|
||||
*
|
||||
* Preserving is safe for every caller because acquisition still re-validates: a preserved
|
||||
* pointer to a removed or unusable checkout is caught by `classifyTaskWorktree` in
|
||||
* `acquireTaskWorktree`, which clears the metadata and creates a fresh worktree. The rest of
|
||||
* the replan contract (steps reset to pending, status/error cleared, `executionStartedAt`
|
||||
* dropped) is unchanged — only the checkout survives.
|
||||
*/
|
||||
export async function moveTaskToReplanColumn(
|
||||
store: TaskStore,
|
||||
@@ -207,7 +228,7 @@ export async function moveTaskToReplanColumn(
|
||||
): Promise<string> {
|
||||
const replanColumn = target ?? await resolveReplanTargetColumn(store, task.id);
|
||||
if (task.column !== replanColumn) {
|
||||
await store.moveTask(task.id, replanColumn);
|
||||
await store.moveTask(task.id, replanColumn, { preserveWorktree: true });
|
||||
}
|
||||
return replanColumn;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user