refactor(maestro): sidestep worktree path collisions with a numeric suffix

Anything occupying <repo>/worktrees/<branch> — one of ours, one the
operator made, an unrelated directory — is now bypassed by appending
-2, -3, ... to the directory; the branch stays as requested. The same
suffix applies under the [daemon].worktree_root override.

The daemon no longer asks whether an occupant is "ours": delete
registered_for(), the adoption branch in run_add and the
SessionManager::create live-session check, plus WorktreeError::PathExists.
A branch already checked out elsewhere is git's refusal to surface,
already marked by the branch picker's is_head/worktree flags.

crates/maestro-daemon/src/session.rs

@@ -226,25 +226,9 @@ impl SessionManager {
                 let WorktreeSpec {
                     base_repo, branch, ..
                 } = spec;
-                // `run_add` adopts a leftover tree for this branch — the
-                // dirty-work guard refuses to delete unsaved work and the path
-                // is deterministic, so erroring would burn the branch forever.
-                // A tree a *live* session is using is a different matter:
-                // two sessions must not share one checkout, so reject it here
-                // where the session records are visible (#67 review).
-                let planned = self.inner.worktree_mgr.planned_path(base_repo, branch);
-                if self
-                    .inner
-                    .sessions
-                    .lock()
-                    .await
-                    .values()
-                    .any(|r| r.worktree_path.as_deref() == Some(planned.as_path()))
-                {
-                    return Err(SessionError::Worktree(
-                        crate::worktree::WorktreeError::PathExists { path: planned },
-                    ));
-                }
+                // A path collision is sidestepped inside `run_add` (#77), and
+                // a branch already checked out elsewhere is git's refusal,
+                // not ours.
                 let path = if spec.create_branch {
                     self.inner.worktree_mgr.add_new(base_repo, branch)
                 } else {

crates/maestro-daemon/src/worktree.rs

@@ -10,10 +10,6 @@ pub enum WorktreeError {
     Spawn(#[from] std::io::Error),
     #[error("git exited with code {code}: {stderr}")]
     NonZero { code: i32, stderr: String },
-    /// A worktree already occupies the target path. The caller decides what
-    /// to do; the daemon never relocates to a different directory (#56).
-    #[error("worktree path already exists: {}", .path.display())]
-    PathExists { path: PathBuf },
     /// The worktree holds unsaved work; removal was refused (#56).
     #[error("left {} on disk: {}", .path.display(), .reason)]
     Guarded { path: PathBuf, reason: String },
@@ -39,9 +35,9 @@ impl WorktreeManager {
     }
 
     /// Where a worktree for `branch` will be created: the override root, or
-    /// `<main checkout>/worktrees/<sanitized branch>` by default. The path is
-    /// predictable (no UUID suffix), so an existing directory at the target
-    /// is an error rather than a silently different location.
+    /// `<main checkout>/worktrees/<sanitized branch>` by default. If the path
+    /// is occupied, [`Self::add_new`] / [`Self::add_existing`] sidestep to
+    /// `<path>-2`, `<path>-3`, ... instead.
     ///
     /// `base_repo` is whatever the client picked, which the project list makes
     /// easy to point at a *linked worktree* — placing the new tree inside it
@@ -51,14 +47,7 @@ impl WorktreeManager {
     /// uncommitted work (#54 review). Always place against the main checkout.
     ///
     /// The override root gets the repo's basename too: without it two repos
-    /// spawning the same branch name target one directory and the second
-    /// fails with `PathExists`.
-    /// Where [`Self::add_new`] / [`Self::add_existing`] would place this
-    /// branch's worktree. Callers use it to check ownership before creating.
-    pub fn planned_path(&self, base_repo: &Path, branch: &str) -> PathBuf {
-        self.target_path(base_repo, branch)
-    }
-
+    /// spawning the same branch name target one directory.
     fn target_path(&self, base_repo: &Path, branch: &str) -> PathBuf {
         // An unusable branch name falls back to a digest rather than escaping
         // the root; `run_add` surfaces git's own error for it anyway.
@@ -80,6 +69,14 @@ impl WorktreeManager {
         }
     }
 
+    /// [`Self::target_path`] with `-n` appended to the final component, the
+    /// collision suffix (#77).
+    fn target_path_suffixed(&self, base_repo: &Path, branch: &str, n: u32) -> PathBuf {
+        let mut name = self.target_path(base_repo, branch).into_os_string();
+        name.push(format!("-{n}"));
+        PathBuf::from(name)
+    }
+
     /// `git -C <base_repo> worktree add -b <branch> <path>` — creates the
     /// branch. Returns the path of the new worktree on success.
     pub fn add_new(&self, base_repo: &Path, branch: &str) -> Result<PathBuf, WorktreeError> {
@@ -98,22 +95,18 @@ impl WorktreeManager {
         branch: &str,
         create_branch: bool,
     ) -> Result<PathBuf, WorktreeError> {
-        let path = self.target_path(base_repo, branch);
-        // Clear registrations whose directory is gone before testing the
-        // path, so a previously-leaked entry doesn't block this branch.
+        // Clear registrations whose directory is gone first, so a
+        // previously-leaked entry doesn't push this branch to a suffix.
         prune(base_repo)?;
-        if path.symlink_metadata().is_ok() {
-            // A tree we left behind for this very branch — the dirty-work
-            // guard refuses to delete unsaved work, and the path is
-            // deterministic, so erroring here would burn the branch
-            // permanently: no spawn could ever produce a worktree for it
-            // again and the product offers no way to clear it. Adopt it
-            // instead; landing in the operator's own unsaved work on the
-            // branch they asked for is what they wanted (#67 review).
-            if registered_for(base_repo, &path, branch) {
-                return Ok(path);
-            }
-            return Err(WorktreeError::PathExists { path });
+        // Anything occupying the target — a tree we left behind, one the
+        // operator made, an unrelated directory — is sidestepped with a
+        // numeric suffix on the directory; the branch stays whatever was
+        // requested (#77). The daemon never adopts what it finds.
+        let mut path = self.target_path(base_repo, branch);
+        let mut n = 1;
+        while path.symlink_metadata().is_ok() {
+            n += 1;
+            path = self.target_path_suffixed(base_repo, branch, n);
         }
         // The parent (`<repo>/worktrees` or the override root) may not exist
         // yet; the target itself must not.
@@ -192,34 +185,6 @@ fn main_checkout(path: &Path) -> PathBuf {
         .unwrap_or_else(|| path.to_path_buf())
 }
 
-/// True when `path` is already a registered worktree of `base_repo` holding
-/// `branch` — i.e. one this daemon created for this branch and later declined
-/// to remove.
-fn registered_for(base_repo: &Path, path: &Path, branch: &str) -> bool {
-    let Ok(out) = Command::new("git")
-        .arg("-C")
-        .arg(base_repo)
-        .args(["worktree", "list", "--porcelain"])
-        .output()
-    else {
-        return false;
-    };
-    let want = path.canonicalize().unwrap_or_else(|_| path.to_path_buf());
-    let text = String::from_utf8_lossy(&out.stdout);
-    let mut here = false;
-    for line in text.lines() {
-        if let Some(p) = line.strip_prefix("worktree ") {
-            let p = PathBuf::from(p);
-            here = p.canonicalize().unwrap_or(p) == want;
-        } else if here {
-            if let Some(r) = line.strip_prefix("branch ") {
-                return r.trim() == format!("refs/heads/{branch}");
-            }
-        }
-    }
-    false
-}
-
 /// `Some(inner)` when a registered worktree lives under `path`. Git omits
 /// those directories from `git status`, so the dirty-work guard cannot see
 /// them and removal would delete them (#54 review).
@@ -268,9 +233,7 @@ fn short_hash(s: &str) -> String {
 /// `.git/refs/heads/feat/example` — so it is used verbatim and `feat/example`
 /// nests under `worktrees/feat/`. That keeps the path readable *and* keeps the
 /// mapping injective: flattening `/` and `.` to `-` made `feat/x`, `feat.x`
-/// and `feat-x` share one directory, and with deterministic paths plus
-/// `PathExists` as a hard error the losers could never get a worktree at all
-/// (#54 review).
+/// and `feat-x` share one directory (#54 review).
 ///
 /// Git already forbids holding both `feat` and `feat/x`, so the directory
 /// collision that nesting could cause cannot arise from valid refs. Only

crates/maestro-daemon/tests/worktree.rs

@@ -95,28 +95,26 @@ fn worktree_root_override_redirects_placement() {
     assert!(path.exists());
 }
 
+/// Anything at all occupying the target path — ours, the operator's, or an
+/// unrelated directory — is sidestepped, not adopted and not reported (#77).
 #[test]
-fn existing_directory_at_target_path_is_an_error_carrying_the_path() {
+fn an_occupied_target_path_gets_a_numeric_suffix() {
     let repo_dir = TempDir::new().unwrap();
     let repo = repo_dir.path().to_path_buf();
     init_repo(&repo);
 
-    // A predictable path means collisions are possible; they must be
-    // reported, never silently relocated.
-    std::fs::create_dir_all(repo.join("worktrees").join("feat").join("collide")).unwrap();
+    // A non-git directory sits exactly where the worktree would go.
+    let squatter = repo.join("worktrees").join("feat").join("collide");
+    std::fs::create_dir_all(&squatter).unwrap();
+    std::fs::write(squatter.join("operator-notes.txt"), b"mine\n").unwrap();
 
     let mgr = WorktreeManager::new(None);
-    let err = mgr.add_new(&repo, "feat/collide").unwrap_err();
-    let msg = format!("{err}");
+    let path = mgr.add_new(&repo, "feat/collide").expect("sidesteps");
+    assert_eq!(path, repo.join("worktrees").join("feat").join("collide-2"));
+    assert!(path.exists());
     assert!(
-        msg.contains(
-            repo.join("worktrees")
-                .join("feat")
-                .join("collide")
-                .to_str()
-                .unwrap()
-        ),
-        "{msg}"
+        squatter.join("operator-notes.txt").exists(),
+        "the occupant is untouched"
     );
 }
 
@@ -381,9 +379,8 @@ fn override_root_keeps_repos_apart() {
     assert_ne!(a, b, "same branch name in two repos must not collide");
 }
 
-/// Flattening `/` and `.` to `-` made these three share one directory, and a
-/// deterministic path plus a hard `PathExists` meant the losers could never
-/// get a worktree (#54 review).
+/// Flattening `/` and `.` to `-` made these three share one directory, so the
+/// losers could never get a worktree (#54 review).
 #[test]
 fn similar_branch_names_get_distinct_directories() {
     let repo_dir = TempDir::new().unwrap();
@@ -400,12 +397,11 @@ fn similar_branch_names_get_distinct_directories() {
     assert!(a.ends_with("feat/x"), "{a:?}");
 }
 
-/// The dead end the cleanup guard created: it refuses to delete unsaved work,
-/// the path is deterministic, and `PathExists` is a hard error — so one stray
-/// untracked file used to burn that branch permanently, with no way to clear
-/// it from the product. The leftover is adopted instead (#67 review).
+/// A guarded leftover stays on disk untouched, and is never adopted (#77).
+/// Re-spawning its branch is refused by git itself — the branch is still
+/// checked out inside the leftover — which the caller surfaces.
 #[test]
-fn a_guarded_leftover_is_adopted_rather_than_burning_the_branch() {
+fn a_guarded_leftover_is_never_adopted() {
     let repo_dir = TempDir::new().unwrap();
     let repo = repo_dir.path().to_path_buf();
     init_repo(&repo);
@@ -417,28 +413,39 @@ fn a_guarded_leftover_is_adopted_rather_than_burning_the_branch() {
     // The guard refuses, leaving it on disk.
     mgr.remove(&repo, &first)
         .expect_err("dirty tree is guarded");
-    assert!(first.exists());
 
-    // Spawning that branch again lands in the operator's own work, not an error.
-    let again = mgr
+    let err = mgr
         .add_existing(&repo, "feat/resume")
-        .expect("leftover is adopted");
-    assert_eq!(again, first);
+        .expect_err("a branch already checked out cannot be spawned twice");
+    assert!(format!("{err}").contains("git exited"), "{err}");
     assert!(
-        again.join("notes.txt").exists(),
-        "their work is still there"
+        first.join("notes.txt").exists(),
+        "the leftover's unsaved work is untouched"
     );
 }
 
-/// Adoption must not extend to a directory that merely sits in the way.
+/// The `worktree_root` override gets the same collision suffix (#77).
 #[test]
-fn an_unrelated_directory_at_the_target_path_is_still_an_error() {
+fn the_override_root_suffixes_collisions_too() {
     let repo_dir = TempDir::new().unwrap();
     let repo = repo_dir.path().to_path_buf();
     init_repo(&repo);
 
-    std::fs::create_dir_all(repo.join("worktrees").join("feat").join("squatter")).unwrap();
-    let mgr = WorktreeManager::new(None);
-    mgr.add_new(&repo, "feat/squatter")
-        .expect_err("a non-worktree directory must not be adopted");
+    let wt_root = TempDir::new().unwrap();
+    let mgr = WorktreeManager::new(Some(wt_root.path().to_path_buf()));
+
+    // First add settles `<root>/<repo basename>-<digest>/` so the squatter
+    // can be created at the exact target.
+    mgr.add_new(&repo, "feat/first").expect("add first");
+    let repo_dir_in_root = std::fs::read_dir(wt_root.path())
+        .unwrap()
+        .next()
+        .unwrap()
+        .unwrap()
+        .path();
+    let squatter = repo_dir_in_root.join("feat").join("collide");
+    std::fs::create_dir_all(&squatter).unwrap();
+
+    let path = mgr.add_new(&repo, "feat/collide").expect("sidesteps");
+    assert_eq!(path, squatter.with_file_name("collide-2"), "{path:?}");
 }