Merge pull request #6031 from nesquena/stage/6028-solo

Release exp: worktree unlock-before-remove (#6028, #6023)

CHANGELOG.md

@@ -13,6 +13,8 @@
 
 - **Deep-link straight to a profile with `?profile=<name>`.** Opening the WebUI with a `?profile=NAME` query parameter now switches to that profile on boot. The name is validated against the existing profile allowlist (a nonexistent profile boots normally, path-traversal / cross-profile attempts are rejected), the switch goes through the same access-enforced `switchToProfile`, and boot fails safe on anything invalid. Thanks @rodboev. (#5730, #5682)
 
+- **WebUI-created git worktrees can now be cleaned up (no more orphan accumulation).** The agent locks every worktree it creates, and git refuses to remove a locked worktree with a single `--force`, so WebUI-created worktrees were piling up unremovable. Session-worktree removal now runs a fail-soft `git worktree unlock` right before the remove (mirroring the agent's own cleanup ordering); the existing dirty/uncommitted-changes guard still runs first, so user data stays protected. Thanks @ayxuerui. (#6028, #6023)
+
 ### Fixed
 
 - **Gateway provider errors are reported accurately, and your prompt survives a failed turn.** On gateway-backed sessions, a terminal provider error (bad model id, exhausted credentials, etc.) mid-turn is now classified and shown as the real error instead of a generic empty turn, the error card is bound to the correct session, and the in-flight user prompt plus any partial output is persisted so it survives a reload — even when you'd just re-sent an identical prompt (e.g. "continue"). Thanks @rodboev. (#5969, #5940)

api/worktrees.py

@@ -265,6 +265,18 @@ def remove_worktree_for_session(session, *, force: bool = False) -> dict:
     repo_root = getattr(session, "worktree_repo_root", None)
     if not repo_root:
         raise ValueError("Session missing worktree_repo_root")
+
+    # Unlock the creation-time bookkeeping lock before removing (fail-soft).
+    # The agent locks every worktree it creates (cli._setup_worktree), and git
+    # refuses to remove a locked worktree with a single --force, so without
+    # this every WebUI-created worktree is unremovable.  The dirty/untracked/
+    # unpushed/stream/terminal guards above are the real safety layer, not
+    # git's lock — mirrors the agent's own _cleanup_worktree ordering.
+    try:
+        _run_git(["worktree", "unlock", str(worktree_path)], str(repo_root), timeout=5)
+    except (OSError, subprocess.TimeoutExpired):
+        pass  # already unlocked / never locked — non-fatal
+
     try:
         remove_args = ["worktree", "remove"]
         if force:

tests/test_worktree_remove.py

@@ -78,6 +78,80 @@ def test_remove_clean_worktree_succeeds(tmp_path):
     assert not wt_path.exists()
 
 
+def test_remove_locked_worktree_succeeds_without_force(tmp_path):
+    """Issue #6023: WebUI-created worktrees are locked at creation by the
+    agent (cli._setup_worktree).  Removal must unlock first — otherwise git
+    refuses with \"use 'remove -f -f' to override or unlock first\" and every
+    WebUI-created worktree is unremovable from the UI."""
+    import subprocess
+    from api.models import Session
+
+    main = _make_minimal_git_repo(tmp_path)
+    wt_path = tmp_path / "wt_locked"
+    subprocess.run(
+        ["git", "-C", str(main), "worktree", "add", str(wt_path), "-b", "hermes/testlocked"],
+        check=True, capture_output=True,
+    )
+    # Reproduce the agent's creation-time lock.
+    subprocess.run(
+        ["git", "-C", str(main), "worktree", "lock", "--reason", "hermes pid=12345", str(wt_path)],
+        check=True, capture_output=True,
+    )
+
+    s = Session(
+        session_id="testlocked",
+        title="Locked",
+        workspace=str(wt_path),
+        worktree_path=str(wt_path),
+        worktree_branch="hermes/testlocked",
+        worktree_repo_root=str(main),
+    )
+
+    result = worktrees.remove_worktree_for_session(s, force=False)
+    assert result["ok"] is True
+    assert result["removed_path"] == str(wt_path.resolve())
+    assert not wt_path.exists()
+    listed = subprocess.run(
+        ["git", "-C", str(main), "worktree", "list", "--porcelain"],
+        check=True, capture_output=True, text=True,
+    ).stdout
+    assert str(wt_path.resolve()) not in listed
+
+
+def test_remove_never_locked_worktree_unlock_is_fail_soft(tmp_path):
+    """The pre-remove unlock must be fail-soft: removing a worktree that was
+    never locked still succeeds (unlock exits non-zero, _run_git is
+    check=False, remove proceeds)."""
+    import subprocess
+    from api.models import Session
+
+    main = _make_minimal_git_repo(tmp_path)
+    wt_path = tmp_path / "wt_neverlocked"
+    subprocess.run(
+        ["git", "-C", str(main), "worktree", "add", str(wt_path), "-b", "hermes/testneverlocked"],
+        check=True, capture_output=True,
+    )
+    # Sanity: not locked — `git worktree unlock` on it would fail.
+    probe = subprocess.run(
+        ["git", "-C", str(main), "worktree", "unlock", str(wt_path)],
+        capture_output=True, text=True,
+    )
+    assert probe.returncode != 0
+
+    s = Session(
+        session_id="testneverlocked",
+        title="Never locked",
+        workspace=str(wt_path),
+        worktree_path=str(wt_path),
+        worktree_branch="hermes/testneverlocked",
+        worktree_repo_root=str(main),
+    )
+
+    result = worktrees.remove_worktree_for_session(s, force=False)
+    assert result["ok"] is True
+    assert not wt_path.exists()
+
+
 def test_remove_clean_worktree_does_not_force(tmp_path, monkeypatch):
     from api.models import Session
 
@@ -111,7 +185,9 @@ def test_remove_clean_worktree_does_not_force(tmp_path, monkeypatch):
 
     result = worktrees.remove_worktree_for_session(s, force=False)
     assert result["ok"] is True
-    assert calls[0] == ["worktree", "remove", str(worktree_path.resolve())]
+    # Unlock (fail-soft) runs first, then the non-force remove.
+    assert calls[0] == ["worktree", "unlock", str(worktree_path.resolve())]
+    assert calls[1] == ["worktree", "remove", str(worktree_path.resolve())]
 
 
 def test_remove_dirty_worktree_without_force_is_rejected(tmp_path, monkeypatch):
@@ -234,7 +310,9 @@ def test_remove_force_warns_and_uses_git_force(tmp_path, monkeypatch):
 
     result = worktrees.remove_worktree_for_session(s, force=True)
     assert result["ok"] is True
-    assert calls[0] == ["worktree", "remove", "--force", str(worktree_path.resolve())]
+    # Unlock (fail-soft) runs first, then the forced remove.
+    assert calls[0] == ["worktree", "unlock", str(worktree_path.resolve())]
+    assert calls[1] == ["worktree", "remove", "--force", str(worktree_path.resolve())]
     assert "untracked file" in " ".join(result["warnings"])
     assert "unpushed commit" in " ".join(result["warnings"])