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.

CLAUDE.md

@@ -62,6 +62,10 @@ nix/                     # Home Manager module (`services.maestro`)
 
 See `docs/INSTALL.md` for end-user install instructions.
 
+`docs/WORKTREES.md` is the contract for session worktrees — placement,
+collisions, what blocks a kill, what `--force` retains, and what reclaim
+touches. Read it before changing anything that creates or removes one.
+
 `nix run .#dev` (or `just dev`) brings up the whole local stack under
 process-compose: a debug daemon on 127.0.0.1:7071, then Vite on :5173 once the
 daemon answers `/api/info`. The SPA proxies `/api/*` back to the daemon, so only

crates/maestro-cli/src/sessions.rs

@@ -153,6 +153,7 @@ mod tests {
             command: command.into(),
             env: HashMap::new(),
             metadata: HashMap::new(),
+            worktree: None,
             status: SessionStatus::Running,
             created_at: Utc.with_ymd_and_hms(2026, 5, 25, 12, 0, 0).unwrap(),
             foreground: None,

crates/maestro-daemon/src/projects.rs

@@ -262,10 +262,16 @@ impl ProjectScanner {
 
     /// Cached snapshot, rescanning (off-thread) when the TTL has lapsed.
     pub async fn repos(&self) -> std::sync::Arc<Vec<DiscoveredRepo>> {
+        self.repos_inner(false).await
+    }
+
+    async fn repos_inner(&self, force: bool) -> std::sync::Arc<Vec<DiscoveredRepo>> {
         let mut cache = self.cache.lock().await;
-        if let Some((at, repos)) = cache.as_ref() {
-            if at.elapsed() < self.ttl {
-                return repos.clone();
+        if !force {
+            if let Some((at, repos)) = cache.as_ref() {
+                if at.elapsed() < self.ttl {
+                    return repos.clone();
+                }
             }
         }
         let roots = self.roots.clone();
@@ -281,9 +287,11 @@ impl ProjectScanner {
         repos
     }
 
-    /// Rescan now, ignoring the TTL. Called by the background timer.
+    /// Rescan now, ignoring the TTL. Called by the background timer — going
+    /// through `repos()` would hit the cache and make most ticks no-ops,
+    /// leaving requests to pay for the rescan instead (#54 review).
     async fn refresh(&self) {
-        self.repos().await;
+        self.repos_inner(true).await;
     }
 
     /// Spawn the slow background refresh task. Drop the returned handle to

crates/maestro-daemon/src/session.rs

@@ -226,6 +226,9 @@ impl SessionManager {
                 let WorktreeSpec {
                     base_repo, branch, ..
                 } = spec;
+                // 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 {
@@ -290,6 +293,12 @@ impl SessionManager {
             command: req.command,
             env: req.env,
             metadata: req.metadata,
+            worktree: worktree_path.as_ref().zip(worktree_base_repo.as_ref()).map(
+                |(path, base_repo)| maestro_protocol::SessionWorktree {
+                    path: path.clone(),
+                    base_repo: base_repo.clone(),
+                },
+            ),
             status: SessionStatus::Running,
             created_at: Utc::now(),
             // A freshly-spawned session is producing its initial output; the
@@ -430,6 +439,7 @@ impl SessionManager {
                     command: String::new(),
                     env: Default::default(),
                     metadata: Default::default(),
+                    worktree: None,
                     status: SessionStatus::Running,
                     created_at: Utc::now(),
                     // Activity is unknown on adoption; the watcher fills it in.
@@ -457,8 +467,11 @@ impl SessionManager {
                 SessionRecord {
                     session: session.clone(),
                     fifo_path,
-                    worktree_path: None,
-                    worktree_base_repo: None,
+                    // Restored from the sidecar: without it the recovered
+                    // session cleans up nothing on kill, and the leftover
+                    // directory blocks that branch forever (#54 review).
+                    worktree_path: session.worktree.as_ref().map(|w| w.path.clone()),
+                    worktree_base_repo: session.worktree.as_ref().map(|w| w.base_repo.clone()),
                     bridge,
                 },
             );

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,23 +47,36 @@ 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`.
+    /// spawning the same branch name target one directory.
     fn target_path(&self, base_repo: &Path, branch: &str) -> PathBuf {
-        let slug = sanitize_branch_for_path(branch);
+        // An unusable branch name falls back to a digest rather than escaping
+        // the root; `run_add` surfaces git's own error for it anyway.
+        let slug = branch_relpath(branch).unwrap_or_else(|| PathBuf::from(short_hash(branch)));
         let main = main_checkout(base_repo);
         match &self.root_override {
             Some(root) => {
-                let repo = main
+                // Basename alone collides for two checkouts of the same name
+                // (`~/a/maestro` and `~/b/maestro`), so disambiguate with a
+                // digest of the full path (#54 review).
+                let base = main
                     .file_name()
                     .map(|n| n.to_string_lossy().into_owned())
                     .unwrap_or_else(|| "repo".into());
+                let repo = format!("{base}-{}", short_hash(&main.to_string_lossy()));
                 root.join(repo).join(slug)
             }
             None => main.join("worktrees").join(slug),
         }
     }
 
+    /// [`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> {
@@ -86,12 +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() {
-            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.
@@ -180,26 +195,60 @@ fn nested_worktree(path: &Path) -> Option<PathBuf> {
         .args(["worktree", "list", "--porcelain"])
         .output()
         .ok()?;
+    // `worktree list` prints fully-resolved paths while ours comes from
+    // `--git-common-dir` and may still hold symlinks. Comparing the two forms
+    // directly makes the guard silently pass on any symlinked repo path — and
+    // this guard is what stands between a kill and deleting an inner
+    // worktree's uncommitted work (#54 review).
+    let needle = path.canonicalize().unwrap_or_else(|_| path.to_path_buf());
     let text = String::from_utf8_lossy(&out.stdout);
     text.lines()
         .filter_map(|l| l.strip_prefix("worktree "))
         .map(PathBuf::from)
-        .find(|p| p != path && p.starts_with(path))
+        .find(|p| {
+            let real = p.canonicalize().unwrap_or_else(|_| p.clone());
+            real != needle && real.starts_with(&needle)
+        })
 }
 
-fn sanitize_branch_for_path(branch: &str) -> String {
-    let mut out = String::with_capacity(branch.len());
-    for c in branch.chars() {
-        if c.is_ascii_alphanumeric() || c == '-' || c == '_' {
-            out.push(c);
-        } else if c == '/' || c == '.' {
-            out.push('-');
-        }
+/// Short digest used to disambiguate override roots.
+///
+/// Hand-rolled FNV-1a rather than `DefaultHasher`: this value becomes a
+/// directory name, and `DefaultHasher`'s algorithm is explicitly not
+/// guaranteed stable across Rust releases — a toolchain bump would remap every
+/// repo to a new directory and orphan the worktrees under the old one
+/// (#67 review).
+fn short_hash(s: &str) -> String {
+    let mut h: u32 = 0x811c_9dc5;
+    for b in s.as_bytes() {
+        h ^= u32::from(*b);
+        h = h.wrapping_mul(0x0100_0193);
     }
-    if out.is_empty() {
-        out.push_str("wt");
+    format!("{h:08x}")
+}
+
+/// The relative directory a branch's worktree lives in.
+///
+/// A git branch name is already a valid relative path — git stores it as
+/// `.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 (#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
+/// traversal and absolute components are rejected.
+fn branch_relpath(branch: &str) -> Option<PathBuf> {
+    use std::path::Component;
+    let candidate = Path::new(branch);
+    if branch.is_empty()
+        || !candidate
+            .components()
+            .all(|c| matches!(c, Component::Normal(_)))
+    {
+        return None;
     }
-    out
+    Some(candidate.to_path_buf())
 }
 
 /// `Some(reason)` when deleting `path` would destroy work.

crates/maestro-daemon/tests/session_manager.rs

@@ -370,7 +370,7 @@ async fn kill_removes_worktree_even_when_spec_says_keep() {
     let fx = DaemonFixture::new();
     let repo = TempDir::new().unwrap();
     init_repo(repo.path());
-    let wt_path = repo.path().join("worktrees").join("feat-keep");
+    let wt_path = repo.path().join("worktrees").join("feat").join("keep");
 
     let s = fx
         .manager
@@ -401,7 +401,7 @@ async fn kill_leaves_worktree_with_unsaved_work_on_disk() {
     let fx = DaemonFixture::new();
     let repo = TempDir::new().unwrap();
     init_repo(repo.path());
-    let wt_path = repo.path().join("worktrees").join("feat-guarded");
+    let wt_path = repo.path().join("worktrees").join("feat").join("guarded");
 
     let s = fx
         .manager

crates/maestro-daemon/tests/sidecar.rs

@@ -19,6 +19,7 @@ fn sample(id: Uuid) -> Session {
         command: "bash".into(),
         env: HashMap::from([("FOO".into(), "bar".into())]),
         metadata: HashMap::from([("name".into(), Value::String("maestro-impl".into()))]),
+        worktree: None,
         status: SessionStatus::Running,
         created_at: DateTime::parse_from_rfc3339("2026-05-24T12:00:00Z")
             .unwrap()

crates/maestro-daemon/tests/worktree.rs

@@ -53,7 +53,7 @@ fn add_places_worktree_inside_the_base_repo_by_default() {
     let mgr = WorktreeManager::new(None);
 
     let path = mgr.add_new(&repo, "feat/example").expect("add worktree");
-    assert_eq!(path, repo.join("worktrees").join("feat-example"));
+    assert_eq!(path, repo.join("worktrees").join("feat").join("example"));
     assert!(path.exists());
     assert!(path.join("README").exists());
 
@@ -74,31 +74,47 @@ fn worktree_root_override_redirects_placement() {
     // repos spawning the same branch name don't target one directory.
     let repo_name = repo.file_name().unwrap();
     let path = mgr.add_new(&repo, "feat/example").expect("add worktree");
-    assert_eq!(path, wt_root.path().join(repo_name).join("feat-example"));
+    // The override root disambiguates same-named checkouts with a digest of
+    // the full repo path, and the branch keeps its own shape.
+    // <root>/<repo basename>-<digest>/feat/example
+    let prefix = format!("{}-", repo_name.to_string_lossy());
+    let repo_dir_name = path
+        .strip_prefix(wt_root.path())
+        .unwrap()
+        .components()
+        .next()
+        .unwrap();
+    assert!(
+        repo_dir_name
+            .as_os_str()
+            .to_string_lossy()
+            .starts_with(&prefix),
+        "{path:?}"
+    );
+    assert!(path.ends_with("feat/example"), "{path:?}");
     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-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-collide")
-                .to_str()
-                .unwrap()
-        ),
-        "{msg}"
+        squatter.join("operator-notes.txt").exists(),
+        "the occupant is untouched"
     );
 }
 
@@ -312,10 +328,9 @@ fn placement_resolves_a_linked_worktree_to_the_main_checkout() {
         !inner.starts_with(&outer),
         "must not nest: {inner:?} inside {outer:?}"
     );
-    assert_eq!(
-        inner.parent().and_then(|p| p.parent()),
-        Some(repo.canonicalize().unwrap().as_path()),
-        "placed against the main checkout"
+    assert!(
+        inner.starts_with(repo.canonicalize().unwrap().join("worktrees")),
+        "placed against the main checkout: {inner:?}"
     );
 }
 
@@ -363,3 +378,74 @@ fn override_root_keeps_repos_apart() {
     let b = mgr.add_new(b_dir.path(), "feat/shared").expect("repo b");
     assert_ne!(a, b, "same branch name in two repos must not collide");
 }
+
+/// 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();
+    let repo = repo_dir.path().to_path_buf();
+    init_repo(&repo);
+
+    let mgr = WorktreeManager::new(None);
+    let a = mgr.add_new(&repo, "feat/x").expect("feat/x");
+    let b = mgr.add_new(&repo, "feat.x").expect("feat.x");
+    let c = mgr.add_new(&repo, "feat-x").expect("feat-x");
+    assert_ne!(a, b);
+    assert_ne!(b, c);
+    assert_ne!(a, c);
+    assert!(a.ends_with("feat/x"), "{a:?}");
+}
+
+/// 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_never_adopted() {
+    let repo_dir = TempDir::new().unwrap();
+    let repo = repo_dir.path().to_path_buf();
+    init_repo(&repo);
+
+    let mgr = WorktreeManager::new(None);
+    let first = mgr.add_new(&repo, "feat/resume").expect("add");
+    std::fs::write(first.join("notes.txt"), b"unsaved\n").unwrap();
+
+    // The guard refuses, leaving it on disk.
+    mgr.remove(&repo, &first)
+        .expect_err("dirty tree is guarded");
+
+    let err = mgr
+        .add_existing(&repo, "feat/resume")
+        .expect_err("a branch already checked out cannot be spawned twice");
+    assert!(format!("{err}").contains("git exited"), "{err}");
+    assert!(
+        first.join("notes.txt").exists(),
+        "the leftover's unsaved work is untouched"
+    );
+}
+
+/// The `worktree_root` override gets the same collision suffix (#77).
+#[test]
+fn the_override_root_suffixes_collisions_too() {
+    let repo_dir = TempDir::new().unwrap();
+    let repo = repo_dir.path().to_path_buf();
+    init_repo(&repo);
+
+    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:?}");
+}

crates/maestro-e2e/tests/e2e.rs

@@ -156,7 +156,7 @@ async fn kill_removes_clean_worktree() {
     assert!(
         worktrees
             .iter()
-            .any(|w| w.contains("worktrees/feature-e2e")),
+            .any(|w| w.contains("worktrees/feature/e2e")),
         "expected a worktree for feature/e2e under <repo>/worktrees, got: {worktrees:?}"
     );
 
@@ -186,7 +186,11 @@ async fn kill_spares_worktree_with_uncommitted_changes() {
 
     let base = TempDir::new().unwrap();
     init_repo(base.path());
-    let wt_path = base.path().join("worktrees").join("feature-unsaved");
+    let wt_path = base
+        .path()
+        .join("worktrees")
+        .join("feature")
+        .join("unsaved");
 
     let req = CreateSessionRequest {
         cwd: base.path().to_path_buf(),
@@ -278,7 +282,7 @@ async fn spawn_new_branch_creates_worktree_and_runs_in_it() {
 
     let base = TempDir::new().unwrap();
     init_repo(base.path());
-    let wt_path = base.path().join("worktrees").join("feature-fresh");
+    let wt_path = base.path().join("worktrees").join("feature").join("fresh");
 
     let s = client
         .spawn_session(&worktree_req(base.path(), "feature/fresh", true))
@@ -343,13 +347,20 @@ async fn second_spawn_on_same_branch_is_rejected() {
     }
 
     // Exactly one registration, one directory, still usable.
+    // Count branch records only: the worktree path now contains the branch
+    // name too (`worktrees/feature/dup`), so a substring match over every line
+    // would count the same tree twice.
     let holders = git_worktree_list(base.path())
         .iter()
-        .filter(|w| w.contains("feature/dup"))
+        .filter(|w| w.as_str() == "branch refs/heads/feature/dup")
         .count();
     assert_eq!(holders, 1, "duplicate spawn must not register another tree");
     assert!(
-        base.path().join("worktrees").join("feature-dup").is_dir(),
+        base.path()
+            .join("worktrees")
+            .join("feature")
+            .join("dup")
+            .is_dir(),
         "original worktree must be untouched"
     );
 
@@ -384,7 +395,7 @@ async fn spawn_from_linked_worktree_places_in_main_checkout() {
         .await
         .expect("spawn from linked worktree");
 
-    let want = base.path().join("worktrees").join("feature-nested");
+    let want = base.path().join("worktrees").join("feature").join("nested");
     assert_eq!(
         s.cwd, want,
         "worktree must be placed under the main checkout"

crates/maestro-protocol/src/lib.rs

@@ -53,6 +53,14 @@ pub struct Session {
     pub env: HashMap<String, String>,
     #[serde(default)]
     pub metadata: HashMap<String, Value>,
+    /// The worktree this session owns, when it was spawned with one.
+    ///
+    /// Persisted rather than kept in memory only: after a daemon restart the
+    /// recovered session must still know what to clean up, and with
+    /// deterministic worktree paths a forgotten association leaves a directory
+    /// that blocks that branch from ever being spawned again (#54 review).
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub worktree: Option<SessionWorktree>,
     pub status: SessionStatus,
     pub created_at: DateTime<Utc>,
     /// Current foreground command in the session's pane (e.g. `claude`,
@@ -127,6 +135,14 @@ fn default_true() -> bool {
     true
 }
 
+/// The worktree a running session owns, carried on [`Session`] so it survives
+/// a daemon restart.
+#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
+pub struct SessionWorktree {
+    pub path: PathBuf,
+    pub base_repo: PathBuf,
+}
+
 /// HTTP `POST /api/sessions` body.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub struct CreateSessionRequest {
@@ -410,6 +426,7 @@ mod tests {
             command: "claude".into(),
             env: HashMap::from([("FOO".into(), "bar".into())]),
             metadata: HashMap::from([("name".into(), Value::String("maestro-impl".into()))]),
+            worktree: None,
             status: SessionStatus::Running,
             created_at: DateTime::parse_from_rfc3339("2026-05-24T12:00:00Z")
                 .unwrap()

crates/maestro-testing/src/lib.rs

@@ -36,6 +36,7 @@ pub fn session(id: Uuid, cwd: &str, command: &str) -> Session {
         command: command.into(),
         env: HashMap::new(),
         metadata: HashMap::new(),
+        worktree: None,
         status: SessionStatus::Running,
         created_at: Utc::now(),
         foreground: None,

crates/maestro-tui/src/app.rs

@@ -55,6 +55,14 @@ pub struct App {
     help: HelpOverlay,
     chord: ChordState,
     daemon: DaemonClient,
+    /// Results of draft data loads, delivered back to the event loop.
+    ///
+    /// These are network calls — `/api/projects` on a cold cache walks all of
+    /// `$HOME` — and awaiting them inside `perform` froze the whole TUI: no
+    /// keys, no frames, attached panes stalled, for up to the request timeout
+    /// (#67 review). They run as detached tasks and post here instead.
+    draft_tx: tokio::sync::mpsc::UnboundedSender<DraftUpdate>,
+    draft_rx: tokio::sync::mpsc::UnboundedReceiver<DraftUpdate>,
     /// Direct HTTP for the one endpoint [`DaemonClient`] doesn't cover yet:
     /// `GET /api/names` (#64).
     http: reqwest::Client,
@@ -105,6 +113,7 @@ pub enum Effect {
 
 impl App {
     pub fn new(cfg: TuiConfig, catalog: CatalogStore, daemon: DaemonClient) -> Self {
+        let (draft_tx, draft_rx) = tokio::sync::mpsc::unbounded_channel();
         let chord_timeout = cfg.chord_timeout;
         let help = HelpOverlay {
             lines: render_help_lines(&cfg),
@@ -123,6 +132,8 @@ impl App {
             chord: ChordState::new(chord_timeout),
             daemon,
             http,
+            draft_tx,
+            draft_rx,
             region: RegionMap::default(),
             last_sent_size: HashMap::new(),
             clicks: ClickTracker::default(),
@@ -144,6 +155,9 @@ impl App {
                         return Ok(());
                     }
                 }
+                Some(update) = self.draft_rx.recv() => {
+                    self.apply_draft_update(update).await;
+                }
                 _ = self.catalog.changed() => {
                     // Render on next loop iteration.
                 }
@@ -204,13 +218,13 @@ impl App {
                 Effect::Attach(id) => self.attach(id),
                 Effect::Spawn(req) => self.submit_spawn(req).await,
                 Effect::Kill(id) => self.submit_kill(id).await,
-                Effect::LoadDraft(tab_idx) => self.load_draft_data(tab_idx).await,
-                Effect::FastForward => self.fast_forward_spawn().await,
-                Effect::RefillNames(tab_idx) => self.refill_names(tab_idx).await,
+                Effect::LoadDraft(tab_idx) => self.spawn_load_draft(tab_idx),
+                Effect::FastForward => self.spawn_fast_forward(),
+                Effect::RefillNames(tab_idx) => self.spawn_refill_names(tab_idx),
                 Effect::ProbeGitRoot { tab, seq, path } => {
-                    self.probe_git_root(tab, seq, path).await
+                    self.spawn_probe_git_root(tab, seq, path)
                 }
-                Effect::LoadBranches(tab) => self.load_branches(tab).await,
+                Effect::LoadBranches(tab) => self.spawn_load_branches(tab),
                 Effect::Forward { id, bytes } => {
                     if let Some(handle) = self.emulators.get(id) {
                         let _ = handle.tx.send(SessionInput::Bytes(bytes)).await;
@@ -381,38 +395,136 @@ impl App {
         effects
     }
 
-    /// One `/api/names` batch. [`DaemonClient`] has no typed call for this
-    /// route yet, so it goes through plain reqwest against the same base URL
-    /// and query contract (`routes::NAMES` + `routes::COUNT_QUERY`).
-    async fn fetch_names(&self, count: usize) -> Vec<String> {
-        let url = format!(
-            "{}{}",
-            maestro_protocol::routes::normalize_base(self.daemon.base()),
-            routes::NAMES
-        );
-        match self
-            .http
-            .get(url)
-            .query(&[(routes::COUNT_QUERY, count)])
-            .send()
-            .await
-        {
-            Ok(resp) => resp
-                .json::<NameBatch>()
-                .await
-                .map(|batch| batch.names)
-                .unwrap_or_default(),
-            Err(_) => Vec::new(),
-        }
-    }
-
     /// Populate a freshly opened draft: presets from `/api/info` (falling back
     /// to the built-ins already in place), projects from `/api/projects`, and
     /// a first name batch. Applies only if that tab still shows the draft —
     /// a slow response must never clobber what replaced it.
-    async fn load_draft_data(&mut self, tab_idx: usize) {
-        let (info, projects) = tokio::join!(self.daemon.daemon_info(), self.daemon.list_projects());
-        let names = self.fetch_names(crate::draft::NAME_BATCH_SIZE).await;
+    /// Kick off the draft's initial data load without blocking the loop.
+    fn spawn_load_draft(&self, tab: usize) {
+        let Some(draft) = self.draft_id(tab) else {
+            return;
+        };
+        let (daemon, http, base, tx) = self.loader_handles();
+        tokio::spawn(async move {
+            let (info, projects) = tokio::join!(daemon.daemon_info(), daemon.list_projects());
+            let names = fetch_names(&http, &base, crate::draft::NAME_BATCH_SIZE).await;
+            let _ = tx.send(DraftUpdate::Data {
+                draft,
+                info: info.ok(),
+                projects: projects.ok(),
+                names,
+            });
+        });
+    }
+
+    fn spawn_refill_names(&self, tab: usize) {
+        let Some(draft) = self.draft_id(tab) else {
+            return;
+        };
+        let (_, http, base, tx) = self.loader_handles();
+        tokio::spawn(async move {
+            let names = fetch_names(&http, &base, crate::draft::NAME_BATCH_SIZE).await;
+            let _ = tx.send(DraftUpdate::Names { draft, names });
+        });
+    }
+
+    fn spawn_probe_git_root(&self, tab: usize, seq: u32, path: PathBuf) {
+        let Some(draft) = self.draft_id(tab) else {
+            return;
+        };
+        let (daemon, _, _, tx) = self.loader_handles();
+        tokio::spawn(async move {
+            let root = daemon
+                .git_root(&path.to_string_lossy())
+                .await
+                .ok()
+                .and_then(|g| g.root);
+            let _ = tx.send(DraftUpdate::GitRoot { draft, seq, root });
+        });
+    }
+
+    fn spawn_load_branches(&self, tab: usize) {
+        let Some(draft) = self.draft_id(tab) else {
+            return;
+        };
+        let Some(cwd) = self
+            .state
+            .tabs
+            .get(tab)
+            .and_then(|t| t.pane.as_ref())
+            .and_then(|p| match p {
+                PaneRef::Draft(d) => d.cwd(),
+                _ => None,
+            })
+        else {
+            return;
+        };
+        let (_, http, base, tx) = self.loader_handles();
+        tokio::spawn(async move {
+            let branches = fetch_branches(&http, &base, &cwd).await;
+            let _ = tx.send(DraftUpdate::Branches {
+                draft,
+                cwd,
+                branches,
+            });
+        });
+    }
+
+    /// The draft pane living in `tab`, by identity.
+    fn draft_id(&self, tab: usize) -> Option<u64> {
+        match self.state.tabs.get(tab).and_then(|t| t.pane.as_ref()) {
+            Some(PaneRef::Draft(d)) => Some(d.id),
+            _ => None,
+        }
+    }
+
+    fn loader_handles(
+        &self,
+    ) -> (
+        DaemonClient,
+        reqwest::Client,
+        String,
+        tokio::sync::mpsc::UnboundedSender<DraftUpdate>,
+    ) {
+        (
+            self.daemon.clone(),
+            self.http.clone(),
+            self.daemon.base().to_string(),
+            self.draft_tx.clone(),
+        )
+    }
+
+    /// Fold a finished background load into the draft that asked for it.
+    async fn apply_draft_update(&mut self, update: DraftUpdate) {
+        if let DraftUpdate::FastForward {
+            info,
+            projects,
+            names,
+        } = update
+        {
+            self.fast_forward_spawn(info, projects, names).await;
+            return;
+        }
+        let want = match &update {
+            DraftUpdate::Data { draft, .. }
+            | DraftUpdate::Names { draft, .. }
+            | DraftUpdate::GitRoot { draft, .. }
+            | DraftUpdate::Branches { draft, .. } => *draft,
+            // Handled above; it targets no particular draft.
+            DraftUpdate::FastForward { .. } => unreachable!(),
+        };
+        // Find it by identity: closing a tab shifts every index to its right,
+        // so an in-flight load addressed by index would be dropped or land on
+        // whatever pane inherited the slot (#67 review).
+        let Some(tab_idx) = self
+            .state
+            .tabs
+            .iter()
+            .position(|t| matches!(&t.pane, Some(PaneRef::Draft(d)) if d.id == want))
+        else {
+            // The draft went away while the request was in flight.
+            return;
+        };
         let Some(PaneRef::Draft(draft)) = self
             .state
             .tabs
@@ -421,17 +533,43 @@ impl App {
         else {
             return;
         };
-        if let Ok(info) = info {
-            draft.home_dir.clone_from(&info.home_dir);
-            if !info.presets.is_empty() {
-                draft.set_presets(info.presets);
+        let mut follow_up_branches = false;
+        match update {
+            DraftUpdate::Data {
+                info,
+                projects,
+                names,
+                ..
+            } => {
+                if let Some(info) = info {
+                    draft.home_dir.clone_from(&info.home_dir);
+                    if !info.presets.is_empty() {
+                        draft.set_presets(info.presets);
+                    }
+                }
+                if let Some(projects) = projects {
+                    draft.set_projects(projects, Utc::now());
+                }
+                if !names.is_empty() {
+                    draft.seed_names(names);
+                }
             }
+            DraftUpdate::Names { names, .. } => {
+                if !names.is_empty() {
+                    draft.extend_names(names);
+                }
+            }
+            DraftUpdate::GitRoot { seq, root, .. } => {
+                draft.resolve_gitroot(seq, root);
+                follow_up_branches = draft.take_branch_load().is_some();
+            }
+            DraftUpdate::Branches { cwd, branches, .. } => {
+                draft.set_branches(&cwd, branches);
+            }
+            DraftUpdate::FastForward { .. } => unreachable!(),
         }
-        if let Ok(projects) = projects {
-            draft.set_projects(projects, Utc::now());
-        }
-        if !names.is_empty() {
-            draft.seed_names(names);
+        if follow_up_branches {
+            self.spawn_load_branches(tab_idx);
         }
     }
 
@@ -441,11 +579,38 @@ impl App {
     /// branch named after that name. Anything unresolvable — no projects
     /// known yet, no command, no name — downgrades to a draft pane seeded
     /// with whatever did resolve.
-    async fn fast_forward_spawn(&mut self) {
-        let (info, projects) = tokio::join!(self.daemon.daemon_info(), self.daemon.list_projects());
-        let names = self.fetch_names(crate::draft::NAME_BATCH_SIZE).await;
+    /// Load what the fast path needs without blocking the loop — the same
+    /// reason the draft loaders were moved off it (#67 review): a cold
+    /// `/api/projects` walks all of `$HOME`, and pressing the key must not
+    /// freeze keys, frames and attached panes until it returns.
+    fn spawn_fast_forward(&self) {
+        let (daemon, http, base, tx) = self.loader_handles();
+        tokio::spawn(async move {
+            let (info, projects) = tokio::join!(daemon.daemon_info(), daemon.list_projects());
+            let names = fetch_names(&http, &base, crate::draft::NAME_BATCH_SIZE).await;
+            let _ = tx.send(DraftUpdate::FastForward {
+                info: info.ok(),
+                projects: projects.ok(),
+                names,
+            });
+        });
+    }
+
+    async fn fast_forward_spawn(
+        &mut self,
+        info: Option<maestro_protocol::DaemonInfo>,
+        projects: Option<Vec<maestro_protocol::Project>>,
+        names: Vec<String>,
+    ) {
+        // Keep the home dir: without it the pre-filled draft renders
+        // unabbreviated paths and "Other directory…" seeds `/` instead of
+        // home, so the two draft entry points disagreed (#67 review).
+        let home_dir = info
+            .as_ref()
+            .map(|i| i.home_dir.clone())
+            .unwrap_or_default();
         let presets = match info {
-            Ok(info) if !info.presets.is_empty() => info.presets,
+            Some(info) if !info.presets.is_empty() => info.presets,
             _ => Vec::new(),
         };
         let discovered = projects.unwrap_or_default();
@@ -465,6 +630,7 @@ impl App {
         let Some(PaneRef::Draft(draft)) = self.state.active_tab_mut().pane.as_mut() else {
             return;
         };
+        draft.home_dir = home_dir;
         if !presets.is_empty() {
             draft.set_presets(presets);
         }
@@ -475,88 +641,6 @@ impl App {
         draft.seed_names(names);
     }
 
-    /// Fetch a fresh name batch after a reroll drained the local queue.
-    async fn refill_names(&mut self, tab_idx: usize) {
-        let names = self.fetch_names(crate::draft::NAME_BATCH_SIZE).await;
-        let Some(PaneRef::Draft(draft)) = self
-            .state
-            .tabs
-            .get_mut(tab_idx)
-            .and_then(|t| t.pane.as_mut())
-        else {
-            return;
-        };
-        if !names.is_empty() {
-            draft.extend_names(names);
-        }
-    }
-
-    /// Resolve the draft target's enclosing repository via `/api/gitroot`
-    /// (#65). A miss unticks the worktree toggle inside `resolve_gitroot`;
-    /// a hit may schedule a branch load (Existing mode), served inline.
-    async fn probe_git_root(&mut self, tab_idx: usize, seq: u32, path: PathBuf) {
-        let root = self
-            .daemon
-            .git_root(&path.to_string_lossy())
-            .await
-            .ok()
-            .and_then(|g| g.root);
-        let Some(PaneRef::Draft(draft)) = self
-            .state
-            .tabs
-            .get_mut(tab_idx)
-            .and_then(|t| t.pane.as_mut())
-        else {
-            return;
-        };
-        draft.resolve_gitroot(seq, root);
-        if draft.take_branch_load().is_some() {
-            self.load_branches(tab_idx).await;
-        }
-    }
-
-    /// One `/api/branches` batch for the draft's current cwd (#65). Same
-    /// reqwest route as [`Self::fetch_names`]: no typed client call exists.
-    async fn load_branches(&mut self, tab_idx: usize) {
-        let Some(path) = self
-            .state
-            .tabs
-            .get(tab_idx)
-            .and_then(|t| t.pane.as_ref())
-            .and_then(|p| match p {
-                PaneRef::Draft(d) => d.cwd(),
-                _ => None,
-            })
-        else {
-            return;
-        };
-        let url = format!(
-            "{}{}",
-            maestro_protocol::routes::normalize_base(self.daemon.base()),
-            routes::BRANCHES
-        );
-        let resp = self
-            .http
-            .get(url)
-            .query(&[(routes::PATH_QUERY, path.to_string_lossy().as_ref())])
-            .send()
-            .await;
-        // On failure the picker stays empty; "new" mode still works.
-        let branches = match resp {
-            Ok(resp) => resp.json::<Vec<BranchInfo>>().await.unwrap_or_default(),
-            Err(_) => return,
-        };
-        let Some(PaneRef::Draft(draft)) = self
-            .state
-            .tabs
-            .get_mut(tab_idx)
-            .and_then(|t| t.pane.as_mut())
-        else {
-            return;
-        };
-        draft.set_branches(&path, branches);
-    }
-
     async fn submit_spawn(&mut self, req: CreateSessionRequest) {
         match self.daemon.create_session(&req).await {
             Ok(session) => {
@@ -744,6 +828,82 @@ fn render_help_lines(cfg: &TuiConfig) -> Vec<String> {
     lines
 }
 
+/// A completed background load, applied on the event loop.
+#[derive(Debug)]
+enum DraftUpdate {
+    Data {
+        draft: u64,
+        info: Option<maestro_protocol::DaemonInfo>,
+        projects: Option<Vec<maestro_protocol::Project>>,
+        names: Vec<String>,
+    },
+    Names {
+        draft: u64,
+        names: Vec<String>,
+    },
+    GitRoot {
+        draft: u64,
+        seq: u32,
+        root: Option<PathBuf>,
+    },
+    Branches {
+        draft: u64,
+        cwd: PathBuf,
+        branches: Vec<BranchInfo>,
+    },
+    /// A fast-forward's data arrived; resolution and spawn happen on the loop.
+    FastForward {
+        info: Option<maestro_protocol::DaemonInfo>,
+        projects: Option<Vec<maestro_protocol::Project>>,
+        names: Vec<String>,
+    },
+}
+
+/// `GET /api/names?count=N`. Free function: the spawned loaders have no `self`.
+async fn fetch_names(http: &reqwest::Client, base: &str, count: usize) -> Vec<String> {
+    let url = format!(
+        "{}{}",
+        maestro_protocol::routes::normalize_base(base),
+        routes::NAMES
+    );
+    match http
+        .get(url)
+        .query(&[(routes::COUNT_QUERY, count)])
+        .send()
+        .await
+    {
+        Ok(resp) => resp
+            .json::<NameBatch>()
+            .await
+            .map(|batch| batch.names)
+            .unwrap_or_default(),
+        Err(_) => Vec::new(),
+    }
+}
+
+/// `GET /api/branches?path=`. On failure the picker stays empty and "new
+/// branch" mode still works.
+async fn fetch_branches(
+    http: &reqwest::Client,
+    base: &str,
+    path: &std::path::Path,
+) -> Vec<BranchInfo> {
+    let url = format!(
+        "{}{}",
+        maestro_protocol::routes::normalize_base(base),
+        routes::BRANCHES
+    );
+    match http
+        .get(url)
+        .query(&[(routes::PATH_QUERY, path.to_string_lossy().as_ref())])
+        .send()
+        .await
+    {
+        Ok(resp) => resp.json::<Vec<BranchInfo>>().await.unwrap_or_default(),
+        Err(_) => Vec::new(),
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -830,6 +990,7 @@ mod tests {
                 cwd: std::path::PathBuf::from("/"),
                 env: Default::default(),
                 metadata: Default::default(),
+                worktree: None,
                 status: SessionStatus::Running,
                 created_at: Utc::now(),
                 foreground: None,
@@ -957,7 +1118,14 @@ mod tests {
 
         let mut app = make_app_at(&base);
         let effects = app.dispatch(Action::FastForwardSpawn);
+        // The fast path loads off the event loop now; the run loop drains the
+        // channel, so do that by hand.
         app.perform(effects).await;
+        let update = tokio::time::timeout(Duration::from_secs(5), app.draft_rx.recv())
+            .await
+            .expect("load completed")
+            .expect("channel open");
+        app.apply_draft_update(update).await;
 
         let posts = posted.lock().unwrap();
         assert_eq!(posts.len(), 1, "one immediate POST, no form shown");
@@ -1001,7 +1169,14 @@ mod tests {
 
         let mut app = make_app_at(&base);
         let effects = app.dispatch(Action::FastForwardSpawn);
+        // The fast path loads off the event loop now; the run loop drains the
+        // channel, so do that by hand.
         app.perform(effects).await;
+        let update = tokio::time::timeout(Duration::from_secs(5), app.draft_rx.recv())
+            .await
+            .expect("load completed")
+            .expect("channel open");
+        app.apply_draft_update(update).await;
 
         assert!(
             matches!(app.state.active_tab().pane, Some(PaneRef::Draft(_))),
@@ -1220,7 +1395,15 @@ mod tests {
         let mut app = make_app_at(&base);
         let effects = app.dispatch(Action::NewDraftPane);
         assert!(matches!(effects.as_slice(), [Effect::LoadDraft(0)]));
+        // `perform` only spawns the load — it must not block the event loop on
+        // the network (#67 review). The result arrives on the draft channel,
+        // which the run loop drains; do that by hand here.
         app.perform(effects).await;
+        let update = tokio::time::timeout(Duration::from_secs(5), app.draft_rx.recv())
+            .await
+            .expect("load completed")
+            .expect("channel open");
+        app.apply_draft_update(update).await;
 
         let PaneRef::Draft(draft) = app.state.active_tab().pane.as_ref().unwrap() else {
             panic!("expected a draft pane");
@@ -1313,6 +1496,7 @@ mod tests {
                 cwd: std::path::PathBuf::from("/"),
                 env: Default::default(),
                 metadata: Default::default(),
+                worktree: None,
                 status: SessionStatus::Running,
                 created_at: Utc::now(),
                 foreground: None,

crates/maestro-tui/src/catalog.rs

@@ -188,6 +188,7 @@ mod tests {
             cwd: std::path::PathBuf::from("/tmp"),
             env: Default::default(),
             metadata: Default::default(),
+            worktree: None,
             status: SessionStatus::Running,
             created_at: Utc::now(),
             foreground: None,

crates/maestro-tui/src/draft.rs

@@ -114,6 +114,11 @@ pub enum DraftInputResult {
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct DraftPane {
+    /// Identity for routing async load results back to *this* draft.
+    ///
+    /// Tab indices shift when a tab closes, so an in-flight load addressed by
+    /// index could be dropped or land on the wrong pane (#67 review).
+    pub id: u64,
     /// Includes the implicit custom tile (empty `command`) last.
     pub presets: Vec<SpawnPreset>,
     pub selected_preset: usize,
@@ -164,6 +169,9 @@ pub struct DraftPane {
     pub home_dir: String,
 }
 
+/// Source of [`DraftPane::id`].
+static NEXT_DRAFT_ID: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1);
+
 impl Default for DraftPane {
     fn default() -> Self {
         // Built-ins until `/api/info` answers — the form must be usable even
@@ -171,6 +179,7 @@ impl Default for DraftPane {
         let mut presets = builtin_presets();
         presets.push(custom_preset());
         Self {
+            id: NEXT_DRAFT_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed),
             presets,
             selected_preset: 0,
             custom_command: crate::text_input::TextInput::default(),
@@ -256,10 +265,16 @@ impl DraftPane {
 
     /// Append a refill batch; the first name replaces the field (this only
     /// happens after an explicit reroll on an empty batch).
-    pub fn extend_names(&mut self, names: Vec<String>) {
-        if let Some(first) = names.first() {
-            self.set_name(first.clone());
+    ///
+    /// The shown name is *consumed*, not left at the head — queueing it too
+    /// made the next reroll produce the same name and the key look dead
+    /// (#67 review).
+    pub fn extend_names(&mut self, mut names: Vec<String>) {
+        if names.is_empty() {
+            return;
         }
+        let first = names.remove(0);
+        self.set_name(first);
         self.name_queue.extend(names);
     }
 
@@ -382,7 +397,7 @@ impl DraftPane {
         self.branches_for = Some(cwd.to_path_buf());
         self.branches = branches;
         self.selected_branch = (0..self.branches.len())
-            .find(|&i| self.branches[i].worktree.is_none())
+            .find(|&i| Self::branch_is_available(&self.branches[i]))
             .unwrap_or(0);
     }
 
@@ -424,8 +439,17 @@ impl DraftPane {
         self.schedule_branch_load();
     }
 
+    /// A branch is pickable only when nothing has it checked out. That means
+    /// no linked worktree *and* not the main checkout's HEAD — git refuses a
+    /// second checkout either way, and HEAD is commonly `main`, so treating it
+    /// as available made it the default pick and every spawn failed with
+    /// "already used by worktree" (#54 review).
+    fn branch_is_available(b: &BranchInfo) -> bool {
+        b.worktree.is_none() && !b.is_head
+    }
+
     fn available_branch(&self, i: usize) -> bool {
-        self.branches.get(i).is_some_and(|b| b.worktree.is_none())
+        self.branches.get(i).is_some_and(Self::branch_is_available)
     }
 
     /// Move the existing-branch highlight, skipping branches another worktree
@@ -445,11 +469,11 @@ impl DraftPane {
         self.selected_branch = available[((cur as i32) + delta).rem_euclid(len) as usize];
     }
 
-    /// The highlighted branch, only when no other worktree holds it.
+    /// The highlighted branch, only when nothing has it checked out.
     pub fn picked_branch(&self) -> Option<&BranchInfo> {
         self.branches
             .get(self.selected_branch)
-            .filter(|b| b.worktree.is_none())
+            .filter(|b| Self::branch_is_available(b))
     }
 
     pub fn is_custom(&self) -> bool {
@@ -1781,15 +1805,16 @@ mod tests {
             ],
         );
         assert_eq!(
-            draft.branches[draft.selected_branch].name, "main",
-            "selection lands on an available branch"
+            draft.branches[draft.selected_branch].name, "feature",
+            "selection skips `main`: the main checkout has it, so git would \
+             refuse a second checkout"
         );
 
         draft.focused_field = DraftField::BranchExisting;
         draft.on_input(&InputKey::Down);
         assert_eq!(
             draft.branches[draft.selected_branch].name, "feature",
-            "moving skips branches another worktree holds"
+            "the only pickable branch stays selected"
         );
 
         let wt = draft.build_request().unwrap().worktree.unwrap();

crates/maestro-tui/src/sidebar.rs

@@ -92,6 +92,7 @@ mod tests {
             cwd: std::path::PathBuf::from("/tmp"),
             env: Default::default(),
             metadata,
+            worktree: None,
             status: SessionStatus::Running,
             created_at: Utc::now(),
             foreground: None,
@@ -179,6 +180,7 @@ mod tests {
             cwd: std::path::PathBuf::from("/tmp"),
             env: Default::default(),
             metadata: HashMap::new(),
+            worktree: None,
             status: SessionStatus::Running,
             created_at: Utc::now(),
             foreground: None,

docs/WORKTREES.md

@@ -0,0 +1,127 @@
+# Worktrees
+
+Maestro can create a git worktree for a session and is responsible for it from
+that moment until the worktree is removed. This document is the contract for
+that lifetime: where a worktree goes, what ends a session, what survives, and
+what the operator has to do by hand.
+
+It exists because the rules here are not guessable from the code. Every rule
+below was decided once, re-litigated in review, and decided the same way again.
+The reasoning is written down alongside each rule so the next reader does not
+have to reconstruct it.
+
+## Nothing is ever adopted
+
+The daemon only ever removes a worktree it created in this session's lifetime or
+recorded in its retained store. It never takes ownership of a directory it finds
+on disk, whatever that directory looks like.
+
+There is no reliable way to ask "is this worktree mine?". A git worktree carries
+no marker distinguishing one a daemon made from one a human made ten minutes
+earlier, and any heuristic that answers the question will eventually answer it
+wrong — in the direction of deleting somebody's work.
+
+So the daemon does not ask. If the target path is occupied by anything at all, it
+picks a different path.
+
+## Placement
+
+A worktree for branch `<branch>` of repository `<repo>` goes to
+`<repo>/worktrees/<branch>`, where `<repo>` is the main checkout — not whichever
+worktree the request happened to come from.
+
+The branch name is used verbatim as a relative path, so `feature/login` becomes
+`worktrees/feature/login`. A branch name that cannot be a relative path falls
+back to a short hash of the branch.
+
+If that path exists, the daemon appends `-2`, then `-3`, and so on until it finds
+a free one. The suffix names the **directory only**. The branch is whatever was
+requested; a second worktree of the same branch is not what happens here, because
+git refuses to check the same branch out twice and the branch picker already
+marks branches that are checked out elsewhere.
+
+### The `worktree_root` override
+
+`[daemon].worktree_root` moves every worktree out of the repository and under one
+root. The per-repository directory is the repository's basename plus a short
+digest of its full path — the digest is what keeps two repositories with the same
+basename apart. Inside it the layout and the collision suffix are the same.
+
+## What ends a session
+
+`DELETE /api/sessions/:id` is **refused** when the session's worktree holds
+unsaved work, meaning any of:
+
+- uncommitted changes to tracked files
+- untracked files that are not ignored
+- commits on the branch reachable from no remote-tracking ref
+
+Ignored files never block. Every Rust worktree has a `target/`; if ignored files
+counted, no session would ever be killable.
+
+The refusal is a structured error naming what blocked it and how much of it there
+is, so a client can say "3 uncommitted files, 2 unpushed commits". The session
+keeps running and the worktree is untouched.
+
+The point of refusing rather than cleaning up is that the session stays as the
+operator's handle on their own unfinished work. A session that is killed while
+its worktree still matters leaves a directory nobody is looking at.
+
+A consequence, accepted deliberately: a feature branch almost always has unpushed
+commits, so `--force` is the ordinary way to end a session, not an exception.
+That is only safe because force retains.
+
+### `--force`
+
+`--force` overrides the refusal and nothing else. It ends the session and
+**retains** the worktree, which enters the retained store with reason
+`force-killed`. Force never removes a tree that would have blocked.
+
+Ending a session and destroying its work are two separate destructive acts. Each
+is chosen on its own, and the second one — reclaim — is chosen while looking at
+exactly what would be lost.
+
+### Exiting on its own
+
+A session that ends by itself — the command finished, the operator typed `exit` —
+never reaches the kill path. When the watcher notices the tmux session is gone it
+applies the same rule: a pristine worktree is removed, anything else is retained
+with reason `exited-with-unsaved-work`.
+
+Nobody is waiting for an answer here, so there is nothing to refuse; the choice is
+only between removing and retaining. It uses the same predicate as the kill, from
+the same place, so the two paths cannot drift apart.
+
+## The retained store
+
+A `state_dir`-backed record of worktrees the daemon created that no longer have a
+session. For each one: the path, the base repository, the branch, why it was
+retained, and when.
+
+It survives a daemon restart. It is the recovery path for everything above — a
+store that is lost on restart is worse than no store, because the worktrees are
+still on disk with nothing pointing at them.
+
+Entries leave when the worktree is reclaimed, and when the daemon notices the
+directory is gone because somebody removed it by hand.
+
+**It is not an ownership record.** It must never be consulted to decide whether a
+path may be adopted — see the first rule. Its only job is to remember what the
+daemon is holding so that it can be listed and reclaimed.
+
+## Reclaim
+
+`GET /api/worktrees` lists the store: path, base repository, branch, reason, when,
+and what is currently in the way. The in-the-way breakdown is recomputed on read,
+not stored, so a tree whose work has since been pushed says so.
+
+`DELETE /api/worktrees/<id>` reclaims one. It removes the directory and drops the
+store entry. It does **not** consult the guard — reclaim is the deliberate
+destructive act, and the operator chose it while looking at what was in the way.
+
+It never deletes the branch. `git worktree remove` leaves the ref, and reclaiming
+a tree must not throw away commits on it. This is worth saying out loud in every
+client, because "reclaim" otherwise sounds like it takes the commits too.
+
+Nothing is reclaimed automatically. There is no age-based expiry and no bulk
+reclaim.

web/src/fastForward.test.ts

@@ -85,9 +85,12 @@ describe("resolveFastForward (#67)", () => {
     });
   });
 
-  it("seeds worktree off when not even a project resolved", () => {
+  it("leaves the worktree default alone when not even a project resolved", () => {
+    // Not `false`: the form defaults the toggle on and a false would stick,
+    // so the fallback draft would disagree with a normally-opened draft and
+    // with the TUI's fast path.
     const seed = fastForwardSeed(resolveFastForward([], presets));
-    expect(seed.worktreeEnabled).toBe(false);
+    expect(seed.worktreeEnabled).toBeUndefined();
     expect(seed.cwd).toBe("");
   });
 });

web/src/fastForward.ts

@@ -55,7 +55,11 @@ export function fastForwardSeed(res: FastForwardResolution) {
     cwd: res.project?.path ?? "",
     command: res.command ?? "",
     name: res.name ?? "",
-    worktreeEnabled: Boolean(res.project),
+    // Left undefined rather than false when nothing resolved: the form
+    // defaults the toggle on, and `false` would stick, so the fallback draft
+    // would disagree with both a normally-opened draft and the TUI's fast path
+    // (#67 review).
+    worktreeEnabled: res.project ? true : undefined,
     worktreeBranch:
       res.project && res.name ? res.name : undefined,
   };

web/src/main.ts

@@ -303,7 +303,12 @@ async function requestFastForward(): Promise<void> {
   const req = buildFastForwardRequest(resolved);
   if (req) {
     try {
-      await createSession(req);
+      const session = await createSession(req);
+      // Mount it, the way the TUI's fast path attaches. Without this the
+      // keystroke creates a session and leaves the UI untouched, so it reads
+      // as a dead key (#67 review).
+      mv.openSessionBeside(session.id);
+      updateShellRoute();
       return;
     } catch {
       // A failed POST degrades to the same pre-filled draft as an

web/src/multiview.ts

@@ -49,6 +49,7 @@ export interface MultiView {
   /** Bring a session into the layout: focus it if already open, else fill the
    *  focused pane (or open the first pane in an empty tab). */
   openSession: (sessionId: string) => void;
+  openSessionBeside: (sessionId: string) => void;
   /** Drop the pane showing `sessionId` (the session was disconnected/killed). */
   closeSession: (sessionId: string) => void;
   /** Every session currently shown in some pane, across all tabs. */
@@ -378,6 +379,39 @@ export function createMultiView(opts: MultiViewOptions): MultiView {
     render();
   }
 
+  /// Mount a session without evicting one.
+  ///
+  /// `openSession` deliberately replaces the focused pane (VS Code style) for
+  /// a sidebar click, but the fast path must never do that: pressing the key
+  /// while attached would drop the running session out of the layout
+  /// entirely. Follows `openDraft`'s rule instead — reuse an empty pane, else
+  /// split beside the focused one (#67 review).
+  function openSessionBeside(sessionId: string): void {
+    const loc = locateSession(layout, sessionId);
+    if (loc) {
+      layout.activeTabId = loc.tab.id;
+      loc.tab.focusedLeafId = loc.leaf.id;
+      render();
+      return;
+    }
+    const tab = activeTab(layout);
+    if (!tab.root || !tab.focusedLeafId || !findLeaf(tab.root, tab.focusedLeafId)) {
+      tab.focusedLeafId = splitLeaf(tab, "", sessionId, "row", true);
+      render();
+      opts.onOpenSessionsChange?.();
+      return;
+    }
+    const target = findLeaf(tab.root, tab.focusedLeafId)!;
+    if (!target.sessionId) {
+      target.sessionId = sessionId;
+      target.draft = null;
+    } else {
+      tab.focusedLeafId = splitLeaf(tab, target.id, sessionId, "row", true);
+    }
+    render();
+    opts.onOpenSessionsChange?.();
+  }
+
   function openDraft(seed?: Partial<Draft>): void {
     const tab = activeTab(layout);
     const draft = (): Draft => ({ command: "", cwd: "", name: "", ...seed });
@@ -693,6 +727,7 @@ export function createMultiView(opts: MultiViewOptions): MultiView {
   return {
     el,
     openSession,
+    openSessionBeside,
     openDraft,
     closeSession,
     openSessions: () => openSessions(layout),

web/src/spawnForm.test.ts

@@ -9,6 +9,7 @@ const base: SpawnFormValues = {
   worktreeEnabled: false,
   worktreeBranch: "",
   worktreeBranchMode: "new",
+  worktreeBranchEdited: false,
 };
 
 describe("buildSpawnRequest", () => {

web/src/spawnForm.ts

@@ -25,6 +25,10 @@ export interface SpawnFormSeed {
   name?: string;
   worktreeEnabled?: boolean;
   worktreeBranch?: string;
+  /** Whether the operator typed the branch themselves, rather than it having
+   *  been auto-synced from the session name. Carried separately because the
+   *  value alone cannot distinguish the two. */
+  worktreeBranchEdited?: boolean;
   worktreeBranchMode?: WorktreeBranchMode;
   cwdRawVisible?: boolean;
 }
@@ -37,6 +41,7 @@ export interface SpawnFormValues {
   name: string;
   worktreeEnabled: boolean;
   worktreeBranch: string;
+  worktreeBranchEdited: boolean;
   /** "new" creates the branch; "existing" checks out a picked one (#61). */
   worktreeBranchMode: WorktreeBranchMode;
   /** Whether the cwd lives in the raw text input (vs a picked recent row). */
@@ -230,6 +235,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
       name: nameInput.value,
       worktreeEnabled: wtEnabled.checked,
       worktreeBranch: branchValue(),
+      worktreeBranchEdited: branchEdited,
       worktreeBranchMode: branchMode,
       cwdRawVisible: !cwdRawWrap.hidden,
     });
@@ -239,7 +245,11 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
   // The worktree branch defaults to the session name — including a clone's
   // prefilled name — and tracks it on reroll/edit until the user types their
   // own branch. Only in "new" mode: an existing-branch pick is not a name.
-  let branchEdited = Boolean(prefill.worktreeBranch);
+  // True only once the operator types their own branch. It cannot be inferred
+  // from the seed: in "new" mode `emit()` publishes the auto-synced session
+  // name, so seeding from it froze the branch after any form rebuild
+  // (#67 review). `worktreeBranchEdited` is carried explicitly instead.
+  let branchEdited = Boolean(prefill.worktreeBranchEdited);
   const syncBranchToName = () => {
     if (!branchEdited && branchMode === "new") branchInput.value = nameInput.value;
   };
@@ -540,6 +550,16 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
   /** Decide how the worktree section describes the current target. Called
    * once the initial load has settled `cwd`. */
   function resolveBaseRepo(): void {
+    // Nothing to resolve when the operator turned the worktree off — the
+    // section would otherwise rebuild expanded under an unchecked box, and
+    // fire a `/api/gitroot` probe for a worktree nobody asked for
+    // (#67 review).
+    if (!wtEnabled.checked) {
+      wtBody.hidden = true;
+      wtHint.hidden = true;
+      inferredBaseRepo = null;
+      return;
+    }
     if (!cwd) {
       inferredBaseRepo = null;
       wtBody.hidden = true;
@@ -634,11 +654,14 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
       return;
     }
     for (const b of branches) {
-      // A branch another worktree holds stays visible but unpickable — git
-      // would refuse to check it out twice; the path says who holds it.
-      const o = opt(b.worktree ? `${b.name} — held by ${b.worktree}` : b.name, b.name);
-      o.disabled = Boolean(b.worktree);
-      o.title = b.worktree ?? "";
+      // A branch that is already checked out anywhere stays visible but
+      // unpickable — git refuses a second checkout. That includes the main
+      // checkout's own HEAD, which is commonly `main` and would otherwise be
+      // the default pick, making every existing-branch spawn fail (#54 review).
+      const held = b.worktree ?? (b.is_head ? "the main checkout" : null);
+      const o = opt(held ? `${b.name} — held by ${held}` : b.name, b.name);
+      o.disabled = held !== null;
+      o.title = held ?? "";
       branchSelect.appendChild(o);
     }
   }
@@ -685,6 +708,7 @@ export function createSpawnForm(opts: SpawnFormOptions): SpawnFormHandle {
         name: nameInput.value,
         worktreeEnabled: wtEnabled.checked,
         worktreeBranch: branchValue(),
+        worktreeBranchEdited: branchEdited,
         worktreeBranchMode: branchMode,
       },
       inferredBaseRepo,