test(engine): pin the PR-conflict sweep's worktree-owner index (20th resolver, after two discarded attempts) (#3202)

`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 -->
This commit is contained in:
gsxdsm
2026-07-31 10:15:21 -07:00
committed by GitHub
parent 5f97fbcb06
commit aef88a2976

View File

@@ -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);