From aef88a2976f7bb25e9a26c75302d79e39128dbb0 Mon Sep 17 00:00:00 2001
From: gsxdsm <gsxdsm@users.noreply.github.com>
Date: Fri, 31 Jul 2026 10:15:21 -0700
Subject: [PATCH] test(engine): pin the PR-conflict sweep's worktree-owner
 index (20th resolver, after two discarded attempts) (#3202)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`prConflictWipColumns` builds the worktree-owner index behind
`ownedByOtherInProgressTask` — the guard that stops this sweep
**deleting a worktree another live task is executing in**. Keyed on the
id, that index is empty on a renamed board, so every worktree reads as
unowned.

## Two discarded attempts, and why they matter more than the fix

**1. Asserted `result.outcome !== "reclaimed"`.** It failed *with the
fix in place* — `reclaimed` is reachable through a second path this
guard does not gate. **An outcome assertion cannot isolate a guard in a
sweep with several routes to the same outcome.** That also explains my
earlier discard on `reclaimSelfOwnedBranchConflicts`, which has the same
shape.

**2. Asserted `removeWorktree` was not called — but overrode the task's
branch while leaving its id.** The reclaim path also requires
`branchOwnerTaskId === taskIdUpper`, so the branch was never reachable
and the case passed **blinded**: vacuous for a reason that had nothing
to do with lanes.

The shipped version asserts `removeWorktree`, which runs **only** on the
guarded branch and is the irreversible part, and keeps the default
id/branch pair so that branch is genuinely reachable.

## Measured

16 pass; blinding `prConflictWipColumns` fails exactly this case.

**20 of 26 pinned** across 19 merged PRs.

## Generalisation

For sweeps with multiple paths to one outcome, the discriminating
observable is a **path-specific side effect** — `removeWorktree`, a
`task:reconcile-*` audit type, a specific `reason` string — not the
return value. Every case I landed today that stuck used one; both
discards asserted a return value.

## Verification

`self-healing-pr-conflict` **16 passed** · `pnpm test:gate` 13 + 161 +
487 + 71 · lint — green.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved protection for active worktrees during pull request conflict
recovery, including tasks in renamed workflow lanes.

* **Tests**
* Added regression coverage to verify that worktrees owned by other
tasks are not removed incorrectly.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---
 .../self-healing-pr-conflict.test.ts          | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/packages/engine/src/__tests__/self-healing-pr-conflict.test.ts b/packages/engine/src/__tests__/self-healing-pr-conflict.test.ts
index 43b10b9193..c36ed2fb88 100644
--- a/packages/engine/src/__tests__/self-healing-pr-conflict.test.ts
+++ b/packages/engine/src/__tests__/self-healing-pr-conflict.test.ts
@@ -125,6 +125,44 @@ describe("SelfHealingManager.reclaimPrConflictForTask", () => {
     expect(result.outcome).toBe("reclaimed");
   });
 
+  /*
+  FNXC:WorkflowResolvedColumns 2026-07-31-23:20:
+  `prConflictWipColumns` builds the worktree-owner index behind `ownedByOtherInProgressTask` — the
+  guard that stops this sweep DELETING a worktree another live task is executing in. Keyed on the id
+  that index is empty on a renamed board, so every worktree reads as unowned.
+
+  ASSERTS ON `removeWorktree`, NOT ON `result.outcome`. My first attempt asserted the outcome and
+  failed while the fix was in place: `reclaimed` is reachable through a second path this guard does
+  not gate, so the outcome cannot isolate it. `removeWorktree` + `git branch -D` run ONLY on the
+  guarded branch, which makes them the observable that discriminates — and they are also the
+  irreversible part, which is what the guard exists to prevent.
+  */
+  it("does NOT delete a worktree owned by another task in a RENAMED wip lane", async () => {
+    /* Default id/branch pair is kept: the reclaim path also requires the branch to name THIS task
+       (branchOwnerTaskId === taskIdUpper), so overriding one of them alone makes the case vacuous. */
+    const task = makeTask();
+    const otherOwner = { ...makeTask({ id: "FN-OTHER" }), column: "building", worktree: task.worktree } as Task;
+    const store = makeStore(task);
+    const RENAMED_IR = {
+      version: "v2", id: "custom:renamed", nodes: [], edges: [],
+      columns: [{ id: "building", name: "building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] }],
+    };
+    (store as any).listWorkflowDefinitions = vi.fn(async () => [{ id: "custom:renamed", ir: RENAMED_IR }]);
+    (store as any).listTasks = vi.fn(async ({ column }: { column?: string } = {}) => (
+      column === "building" ? [otherOwner] : column ? [] : [task, otherOwner]
+    ));
+    vi.spyOn(branchConflicts, "inspectBranchConflict").mockResolvedValue({
+      kind: "fully-subsumed", livePath: task.worktree, tipSha: "abc123", taskAttributedCommitCount: 0, strandedCommits: [],
+    } as any);
+    const removeSpy = vi.spyOn(worktreePool, "removeWorktree").mockResolvedValue(undefined as never);
+    const manager = new SelfHealingManager(store as any, { rootDir: "/tmp/test" } as any);
+
+    await manager.reclaimPrConflictForTask(task.id);
+
+    /* The other task's checkout survives — deleting it is not recoverable. */
+    expect(removeSpy).not.toHaveBeenCalled();
+  });
+
   it("returns paused-unrecoverable when conflict is unrecoverable and dispatcher pauses", async () => {
     const task = makeTask();
     const store = makeStore(task);