fix(worktree): detach kept worktree so branch_per=run survives a kept failure (#138)

CHANGELOG.md

@@ -145,6 +145,18 @@ PATH)`, the TUI notifies `multiplexer backend unavailable — launch/attach disa
 
 ### Fixed
 
+- **`branch_per=run` + `keep_failed` no longer poisons a multi-story run after the first kept
+  failure (#138).** The first story to end deferred under `keep_failed=true` left its worktree
+  checked out on the single shared run branch, so every subsequent story's `git worktree add`
+  collided ("branch already checked out") and insta-deferred with zero dev activity — one kept
+  failure turned an N-story run into a 1-story run. A kept worktree under `branch_per=run` now
+  detaches its HEAD (`git checkout --detach`), freeing the shared branch name for the next story
+  while preserving the working tree, uncommitted changes, and the branch ref (still at the kept
+  commit) for inspection; subsequent stories mount the run branch normally and get genuine
+  attempts. Best effort — if the detach ever fails, the existing `worktree-open-failed` defer
+  still surfaces the collision (no regression). The escalate-and-pause path was already safe: it
+  halts the run rather than continuing, and resume frees the kept worktree before any sibling mounts.
+
 - **Dev/review sessions can no longer livelock on their own wake nudges (#149).** The idle
   wake nudge is delivered as a submitted turn, so a session that merely _answers_ it ends in
   another result-less Stop — which refilled the nudge budget, re-armed the grace window, and

src/bmad_loop/engine.py

@@ -561,6 +561,7 @@ def _integrate_unit(self, task: StoryTask, unit: UnitWorkspace) -> None:
                 run_dir=self.run_dir,
                 unit_key=task.story_key,
                 delete_branch=scm.delete_branch,
+                detach_kept=scm.branch_per == "run",
                 diff_max_file_bytes=self._failed_diff_max_bytes(),
             )
             self.journal.append(

src/bmad_loop/verify.py

@@ -647,6 +647,17 @@ def checkout_branch(repo: Path, name: str) -> None:
         raise GitError(f"git checkout {name} failed in {repo}: {out}")
 
 
+def checkout_detach(repo: Path) -> None:
+    """Detach HEAD at its current commit, leaving working tree + index untouched.
+
+    Frees a shared branch name held by a kept worktree so a sibling worktree can
+    check that branch out (git refuses a branch checked out in another worktree).
+    """
+    rc, out = _git(repo, "checkout", "--detach")
+    if rc != 0:
+        raise GitError(f"git checkout --detach failed in {repo}: {out}")
+
+
 def worktree_remove(repo: Path, path: Path, force: bool = False) -> None:
     args = ["worktree", "remove"]
     if force:

src/bmad_loop/workspace.py

@@ -119,6 +119,7 @@ def close_unit_workspace(
     run_dir: Path,
     unit_key: str,
     delete_branch: bool = True,
+    detach_kept: bool = False,
     diff_max_file_bytes: int | None = None,
 ) -> Path | None:
     """Tear down (or preserve) a unit's worktree.
@@ -130,6 +131,11 @@ def close_unit_workspace(
     and, if delete_branch, the branch deleted. Returns the patch path it wrote,
     or None.
 
+    detach_kept (branch_per=run only): when keep_failed preserves the worktree,
+    detach its HEAD so the shared run branch it holds is freed for the next unit
+    to mount — otherwise every later unit's `git worktree add` collides on the
+    already-checked-out branch. Best effort; see the keep_failed branch below.
+
     diff_max_file_bytes caps the per-untracked-file size in that forensic patch
     (None = no cap); see verify.capture_diff.
     """
@@ -148,7 +154,19 @@ def close_unit_workspace(
             patch.parent.mkdir(parents=True, exist_ok=True)
             patch.write_text(diff, encoding="utf-8")
         if keep_failed:
-            return patch  # leave the worktree + branch mounted
+            if detach_kept:
+                # branch_per=run shares one branch across the run; a kept worktree
+                # left checked out on it blocks every later unit's `git worktree
+                # add` (git refuses a branch checked out elsewhere). Detach HEAD to
+                # free the shared branch name while preserving the working tree,
+                # uncommitted changes, and the branch ref (still at the kept commit)
+                # for inspection. Best effort: on failure the later unit still
+                # surfaces the collision via the worktree-open-failed defer path.
+                try:
+                    verify.checkout_detach(unit.path)
+                except verify.GitError:
+                    pass
+            return patch  # leave the worktree mounted (branch detached if shared)
 
     # success, or a failure we are not keeping: remove the worktree. A failed
     # tree is dirty, so force; a successful unit was committed + merged, so its

tests/test_engine_worktree.py

@@ -342,17 +342,31 @@ def test_worktree_defer_then_next_story_succeeds(project):
     assert "change for 1-1-a" not in (project.project / "src.txt").read_text()
 
 
-def test_branch_per_run_kept_failure_defers_next_unit_gracefully(project):
-    """branch_per=run shares one branch; a kept-failed unit holds it, so the
-    next unit can't mount it. That degrades to a deferral, never a crash."""
+def test_branch_per_run_kept_failure_detaches_so_next_unit_runs(project):
+    """branch_per=run shares one branch; keeping a kept-failed unit's worktree
+    checked out on it would block every later unit's mount and cascade the whole
+    run into never-attempted deferrals. close_unit_workspace detaches the kept
+    worktree's HEAD, freeing the shared branch so the next unit gets a genuine
+    attempt instead of insta-deferring on a collision (issue #138)."""
     commit_sprint(project, {"1-1-a": "ready-for-dev", "1-2-b": "ready-for-dev"})
-    script = _defer_script(project, "1-1-a") + [wt_dev_effect(project, "1-2-b")]
+    script = _defer_script(project, "1-1-a") + [
+        wt_dev_effect(project, "1-2-b"),
+        wt_review_effect(project, "1-2-b", clean=True),
+    ]
     engine, _ = make_engine(project, script, policy=wt_policy(branch_per="run", limits=_NO_DAMP))
     summary = engine.run()
 
-    assert summary.deferred == 2 and summary.done == 0 and not summary.paused
-    assert "could not open worktree" in engine.state.tasks["1-2-b"].defer_reason
-    assert "worktree-open-failed" in journal_kinds(engine)
+    # 1-1-a defers (kept), but 1-2-b actually runs and lands — no collision cascade
+    assert summary.deferred == 1 and summary.done == 1 and not summary.paused
+    assert "worktree-open-failed" not in journal_kinds(engine)
+    assert engine.state.tasks["1-2-b"].phase == Phase.DONE
+    assert not engine.state.tasks["1-2-b"].defer_reason
+    assert "change for 1-2-b" in (project.project / "src.txt").read_text()
+    # the kept 1-1-a worktree is detached (freeing the shared run branch), while
+    # the branch ref itself survives for inspection
+    assert branch_exists(project.project, "bmad-loop/test-run")
+    kept = [p for p in worktree_list(project.project) if p.resolve() != project.project.resolve()]
+    assert len(kept) == 1 and current_branch(kept[0]) == "HEAD"
 
 
 def test_worktree_followup_damped_commits_and_integrates(project):
@@ -442,6 +456,48 @@ def diverging_open(*a, **k):
     assert branch_exists(project.project, "bmad-loop/test-run/1-1-a")
 
 
+def test_branch_per_run_escalation_pauses_without_dispatching_next_unit(project):
+    """Issue #138 scoping guard: the shared-branch collision cascade is a property
+    of the DEFER path, which *returns* and lets the loop dispatch the next unit
+    into the held branch. A merge-conflict escalation instead *pauses* the run
+    (RunPaused), so under branch_per=run no sibling is ever dispatched while the
+    kept worktree holds the shared branch — there is nothing to detach here, and
+    on resume the re-armed unit's worktree is freed by the resume-restart discard
+    (see test_worktree_crash_restart_discards_stale_worktree) before any mount."""
+    commit_sprint(project, {"1-1-a": "ready-for-dev", "1-2-b": "ready-for-dev"})
+    engine, _ = make_engine(
+        project,
+        [wt_dev_effect(project, "1-1-a"), wt_review_effect(project, "1-1-a", clean=True)],
+        policy=wt_policy(branch_per="run", merge_strategy="ff"),
+    )
+    # diverge the target right after the (shared) worktree is cut so ff-only merge
+    # of 1-1-a cannot fast-forward → escalate + pause
+    import bmad_loop.engine as eng
+
+    real_open = eng.open_unit_workspace
+
+    def diverging_open(*a, **k):
+        unit = real_open(*a, **k)
+        if not (project.project / "diverge.txt").exists():
+            (project.project / "diverge.txt").write_text("target moved\n")
+            git(project.project, "add", "-A")
+            git(project.project, "commit", "-q", "-m", "target diverges")
+        return unit
+
+    eng.open_unit_workspace = diverging_open
+    try:
+        summary = engine.run()
+    finally:
+        eng.open_unit_workspace = real_open
+
+    assert summary.paused and summary.escalated == 1
+    assert engine.state.tasks["1-1-a"].phase == Phase.ESCALATED
+    # the run halted at the escalation: 1-2-b was never dispatched, so the
+    # shared-branch collision that cascades the DEFER path cannot arise here
+    assert "1-2-b" not in engine.state.tasks
+    assert "worktree-open-failed" not in journal_kinds(engine)
+
+
 # ----------------------------------------------------------------- resume
 
 

tests/test_verify_worktree.py

@@ -82,6 +82,31 @@ def test_worktree_remove_dirty_needs_force(project, tmp_path):
     assert not wt.exists()
 
 
+def test_checkout_detach_frees_branch(project, tmp_path):
+    """A worktree checked out on a branch holds that branch — git refuses to mount
+    it elsewhere. Detaching the worktree's HEAD frees the branch name for a sibling
+    worktree while preserving the branch ref, the working tree, and uncommitted
+    changes (issue #138)."""
+    repo = project.project
+    wt = tmp_path / "wt"
+    verify.worktree_add(repo, wt, "feat", "main")
+    (wt / "dirty.txt").write_text("uncommitted\n")  # local edit that must survive
+
+    # while 'feat' is checked out in wt, a sibling mount of it is refused
+    wt2 = tmp_path / "wt2"
+    with pytest.raises(verify.GitError):
+        verify.worktree_add(repo, wt2, "feat", create=False)
+
+    verify.checkout_detach(wt)
+
+    assert verify.current_branch(wt) == "HEAD"  # detached
+    assert verify.branch_exists(repo, "feat")  # branch ref preserved
+    assert (wt / "dirty.txt").read_text() == "uncommitted\n"  # working tree preserved
+    # branch name is now free → the sibling mount succeeds
+    verify.worktree_add(repo, wt2, "feat", create=False)
+    assert wt2.is_dir()
+
+
 # ---------------------------------------------------------------- merge