fix(FN-4939): preserve worktree on contamination auto-recovery

The in-line branch-cross-contamination auto-recovery in executor.ts had
two related bugs that produced transient "no-worktree-no-merge-confirmed"
stall signals in the dashboard while a live worktree was still mapped
on disk:

1. autoRecoverCrossContamination was called with repoDir=this.rootDir.
   The recovery does: git checkout --detach <baseSha> → cherry-pick
   wanted commits → git update-ref → git checkout <branch>. When the
   branch is checked out in a worktree (the normal case), that final
   recheckout in rootDir is blocked by git with "branch already used
   by worktree at ...", so the in-line happy path silently failed for
   every contaminated task that had a real worktree. Pass the task's
   worktree as repoDir when available so the operations stay internal
   to the worktree and the recheckout succeeds.

2. The successful-recovery branch called
       moveTask(taskId, 'todo', { preserveResumeState: true })
   without preserveWorktree:true. moveTask defaults to nulling
   task.worktree on requeue. The worktree directory and git mapping
   were still live — the dashboard's in-review-stall classifier
   (no-worktree-no-merge-confirmed) and TaskChangesTab both keyed off
   task.worktree being null and lied about the worktree being gone.
   Sibling recovery paths (auto-recovery-handlers/contamination.ts,
   tryBootstrapMisbindingRecovery, self-healing.ts:1639) all already
   pass preserveWorktree:true; this site was inconsistent.

Updated the existing FN-4428 regression test and added a new
FN-4939 test asserting repoDir uses task.worktree (with fallback
to rootDir when the task has no worktree pointer).

Refs: packages/core/src/in-review-stall.ts:116
This commit is contained in:
gsxdsm
2026-05-17 12:03:44 -07:00
parent 93e49b062a
commit 14c5a17844
3 changed files with 64 additions and 4 deletions

View File

@@ -0,0 +1,8 @@
---
"@runfusion/fusion": patch
---
Fix contamination auto-recovery nulling `task.worktree` while leaving a live worktree mapped on disk, which triggered transient `no-worktree-no-merge-confirmed` stall signals in the dashboard. The in-line recovery in `executor.ts` now:
- Runs `autoRecoverCrossContamination` inside the task's worktree (when one exists) so the final `git checkout <branch>` doesn't collide with the branch already being checked out elsewhere — the previous `repoDir: this.rootDir` call would silently fail for any task that had a real worktree.
- Passes `preserveWorktree: true` when requeueing to `todo`, matching the sibling recovery paths in `auto-recovery-handlers/contamination.ts`, `tryBootstrapMisbindingRecovery`, and self-healing reclaim.

View File

@@ -131,7 +131,7 @@ describe("branch cross-contamination recovery (FN-4428/FN-4499)", () => {
mockedCreateFnAgent.mockRejectedValueOnce(contamination);
vi.spyOn(branchConflicts, "classifyBootstrapMisbinding").mockResolvedValueOnce({ isBootstrapMisbinding: false, ownCommitCount: 1, nonAttributedCount: 0 });
vi.spyOn(branchConflicts, "classifyForeignCommits").mockResolvedValueOnce({ alreadyUpstream: contamination.foreignCommits, unique: [] });
vi.spyOn(branchConflicts, "autoRecoverCrossContamination").mockResolvedValueOnce({
const recoverySpy = vi.spyOn(branchConflicts, "autoRecoverCrossContamination").mockResolvedValueOnce({
newTipSha: "2222222222222222222222222222222222222222",
droppedShas: ["1111111111111111111111111111111111111111"],
});
@@ -139,7 +139,41 @@ describe("branch cross-contamination recovery (FN-4428/FN-4499)", () => {
const executor = new TaskExecutor(store, "/tmp/test");
await executor.execute(makeTask());
expect(store.moveTask).toHaveBeenCalledWith("FN-4428", "todo", { preserveResumeState: true });
// FN-4939: contamination auto-recovery must preserve the worktree because the
// recovery operates inside it (re-anchors the branch, re-checks it out).
// Nulling task.worktree here triggered transient `no-worktree-no-merge-confirmed`
// stall signals while a live worktree remained mapped on disk.
expect(store.moveTask).toHaveBeenCalledWith("FN-4428", "todo", { preserveResumeState: true, preserveWorktree: true });
// FN-4939: recovery must run inside the task's worktree, not the repo root.
// Otherwise the final `git checkout <branch>` in rootDir collides with the
// branch already being checked out in the worktree and the recovery silently fails.
expect(recoverySpy).toHaveBeenCalledWith(expect.objectContaining({
repoDir: "/tmp/test/.worktrees/fn-4428",
}));
});
it("FN-4939: falls back to rootDir for recovery only when task has no worktree pointer", async () => {
const store = createMockStore();
const contamination = new branchConflicts.BranchCrossContaminationError({
branchName: "fusion/fn-4428",
baseSha: "abc123",
taskId: "FN-4428",
foreignCommits: [{ sha: "1111111111111111111111111111111111111111", subject: "feat(FN-4412): upstream", foreignTaskId: "FN-4412" }],
});
mockedCreateFnAgent.mockRejectedValueOnce(contamination);
vi.spyOn(branchConflicts, "classifyBootstrapMisbinding").mockResolvedValueOnce({ isBootstrapMisbinding: false, ownCommitCount: 1, nonAttributedCount: 0 });
vi.spyOn(branchConflicts, "classifyForeignCommits").mockResolvedValueOnce({ alreadyUpstream: contamination.foreignCommits, unique: [] });
const recoverySpy = vi.spyOn(branchConflicts, "autoRecoverCrossContamination").mockResolvedValueOnce({
newTipSha: "2222222222222222222222222222222222222222",
droppedShas: ["1111111111111111111111111111111111111111"],
});
const executor = new TaskExecutor(store, "/tmp/test");
await executor.execute({ ...makeTask(), worktree: undefined } as any);
expect(recoverySpy).toHaveBeenCalledWith(expect.objectContaining({ repoDir: "/tmp/test" }));
});
it("falls back to terminal contamination failure when bootstrap reanchor throws", async () => {

View File

@@ -4538,8 +4538,16 @@ export class TaskExecutor {
const alreadyAttemptedRecovery = (task.recoveryRetryCount ?? 0) > 0;
if (classified.unique.length === 0 && !alreadyAttemptedRecovery) {
// Run the recovery inside the worktree (when one exists) so the final
// `git checkout <branch>` step doesn't collide with the worktree's own
// checkout. If we operate from this.rootDir while the branch is checked
// out in a worktree, git refuses the recheckout with
// "branch already used by worktree" and the in-line happy path silently
// fails — every contaminated task would then fall through to the
// dispatcher pause path even when it could have auto-recovered.
const recoveryRepoDir = task.worktree ?? this.rootDir;
const recovery = await autoRecoverCrossContamination({
repoDir: this.rootDir,
repoDir: recoveryRepoDir,
branchName: err.branchName,
baseSha: err.baseSha,
taskId: task.id,
@@ -4560,7 +4568,17 @@ export class TaskExecutor {
pausedReason: null,
error: null,
});
await this.store.moveTask(task.id, "todo", { preserveResumeState: true });
// FN-4939: preserve the worktree across requeue. The recovery operated
// inside the worktree (re-anchored the branch and re-checked it out), so
// the worktree directory remains internally consistent and usable. Nulling
// task.worktree here was the root cause of transient
// `no-worktree-no-merge-confirmed` stall signals — a live mapped worktree
// would still exist on disk while task.worktree was null, and downstream
// classifiers (in-review-stall.ts, TaskChangesTab) cannot distinguish
// "worktree gone" from "pointer not yet repopulated". Matches sibling
// recovery paths in auto-recovery-handlers/contamination.ts,
// tryBootstrapMisbindingRecovery, and self-healing reclaim.
await this.store.moveTask(task.id, "todo", { preserveResumeState: true, preserveWorktree: true });
return;
}