fix(maestro): third-review findings — nested worktrees, staleness, placement

The one that loses data: the project picker lists linked worktrees as
pickable rows, so `base_repo` was easily a worktree rather than the main
checkout, and the new placement then nested the tree inside it. Git hides
a registered worktree's contents from the outer `git status`, so the
outer tree read clean and removing it deleted the inner one's uncommitted
work — while that session could still be running. Placement now resolves
to the main checkout via `--git-common-dir`, and removal refuses outright
when `worktree list` reports anything registered underneath.

Also:

- The `worktree_root` override dropped the old UUID without replacing it,
  so two repos spawning the same branch name targeted one directory and
  the second failed. The repo basename now sits between root and slug.
- Project staleness read `.git/HEAD`'s mtime, which only moves on
  checkout. A repo committed to daily from one long-lived branch reported
  its last branch switch and got dimmed. Prefer `logs/HEAD`.
- The projects cache TTL (5s) was shorter than its own background refresh
  (60s), so nearly every request rescanned all of $HOME under the scanner
  lock — and draft panes re-request on unrelated re-renders. Now 180s.
- onSuccess resolved the leaf against the active tab only, so switching
  tabs mid-POST orphaned the session. Searches every tab.

crates/maestro-daemon/src/projects.rs

@@ -29,9 +29,14 @@ pub const DEFAULT_HOME_IGNORE: &[&str] =
     &["node_modules", "Downloads", "Library", "go", "snap", "opt"];
 
 /// How long a discovery snapshot is served as-is before the next request
-/// triggers a rescan. Short: a scan is a bounded filesystem walk plus one
-/// `git worktree list` per hit.
-pub const CACHE_TTL: Duration = Duration::from_secs(5);
+/// triggers a rescan.
+///
+/// A scan walks the roots and runs one `git worktree list` per hit, holding
+/// the scanner lock throughout, and clients re-request whenever a draft pane
+/// re-mounts — which `multiview.render()` does on unrelated events. Keep this
+/// comfortably longer than [`REFRESH_INTERVAL`] so the background timer is
+/// what refreshes, and a request only rescans when that timer has failed to.
+pub const CACHE_TTL: Duration = Duration::from_secs(180);
 /// Background refresh cadence. Repositories appear rarely; this keeps the
 /// served list eventually fresh without anyone asking.
 pub const REFRESH_INTERVAL: Duration = Duration::from_secs(60);
@@ -154,9 +159,15 @@ fn list_worktrees(repo: &Path) -> Option<Vec<PathBuf>> {
     (!paths.is_empty()).then_some(paths)
 }
 
-/// Mtime of the checkout's HEAD file. For a linked worktree `.git` is a file
-/// holding a `gitdir:` pointer; resolve it so worktrees report their own HEAD
-/// rather than nothing.
+/// Mtime of the checkout's reflog, falling back to `HEAD` itself.
+///
+/// `HEAD` is only rewritten on checkout — committing updates
+/// `refs/heads/<branch>` and appends to `logs/HEAD`, leaving `HEAD` alone. A
+/// repo worked on daily from one long-lived branch would otherwise report its
+/// last *branch switch* as its activity and get dimmed as stale (#54 review).
+///
+/// For a linked worktree `.git` is a file holding a `gitdir:` pointer;
+/// resolve it so worktrees report their own reflog rather than nothing.
 fn head_mtime(repo: &Path) -> Option<DateTime<Utc>> {
     let dot_git = repo.join(".git");
     let head = if dot_git.is_dir() {
@@ -173,6 +184,9 @@ fn head_mtime(repo: &Path) -> Option<DateTime<Utc>> {
         };
         p.join("HEAD")
     };
+    // Prefer the reflog next to whichever HEAD we resolved.
+    let reflog = head.with_file_name("logs").join("HEAD");
+    let head = if reflog.exists() { reflog } else { head };
     let mtime = std::fs::metadata(head).ok()?.modified().ok()?;
     Some(DateTime::<Utc>::from(mtime))
 }

crates/maestro-daemon/src/worktree.rs

@@ -39,14 +39,32 @@ impl WorktreeManager {
     }
 
     /// Where a worktree for `branch` will be created: the override root, or
-    /// `<base_repo>/worktrees/<sanitized branch>` by default. The path is
+    /// `<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.
+    ///
+    /// `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
+    /// would nest one worktree in another, and git hides a registered
+    /// worktree's contents from the outer `git status`, so the outer tree
+    /// would later look clean and its removal would delete the inner one's
+    /// 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`.
     fn target_path(&self, base_repo: &Path, branch: &str) -> PathBuf {
         let slug = sanitize_branch_for_path(branch);
+        let main = main_checkout(base_repo);
         match &self.root_override {
-            Some(root) => root.join(slug),
-            None => base_repo.join("worktrees").join(slug),
+            Some(root) => {
+                let repo = main
+                    .file_name()
+                    .map(|n| n.to_string_lossy().into_owned())
+                    .unwrap_or_else(|| "repo".into());
+                root.join(repo).join(slug)
+            }
+            None => main.join("worktrees").join(slug),
         }
     }
 
@@ -108,6 +126,12 @@ impl WorktreeManager {
         if !path.exists() {
             return prune(base_repo);
         }
+        if let Some(inner) = nested_worktree(path) {
+            return Err(WorktreeError::Guarded {
+                path: path.to_path_buf(),
+                reason: format!("holds a nested worktree at {}", inner.display()),
+            });
+        }
         if let Some(reason) = unsafe_to_remove(path)? {
             return Err(WorktreeError::Guarded {
                 path: path.to_path_buf(),
@@ -124,6 +148,45 @@ impl WorktreeManager {
     }
 }
 
+/// The repository's main checkout. `--git-common-dir` points at the shared
+/// `.git` for every linked worktree, so its parent is the main tree. Falls
+/// back to the input when git can't answer.
+fn main_checkout(path: &Path) -> PathBuf {
+    let out = Command::new("git")
+        .arg("-C")
+        .arg(path)
+        .args(["rev-parse", "--path-format=absolute", "--git-common-dir"])
+        .output();
+    let Ok(out) = out else {
+        return path.to_path_buf();
+    };
+    if !out.status.success() {
+        return path.to_path_buf();
+    }
+    let common = PathBuf::from(String::from_utf8_lossy(&out.stdout).trim().to_string());
+    common
+        .parent()
+        .map(Path::to_path_buf)
+        .unwrap_or_else(|| path.to_path_buf())
+}
+
+/// `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).
+fn nested_worktree(path: &Path) -> Option<PathBuf> {
+    let out = Command::new("git")
+        .arg("-C")
+        .arg(path)
+        .args(["worktree", "list", "--porcelain"])
+        .output()
+        .ok()?;
+    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))
+}
+
 fn sanitize_branch_for_path(branch: &str) -> String {
     let mut out = String::with_capacity(branch.len());
     for c in branch.chars() {

crates/maestro-daemon/tests/worktree.rs

@@ -70,8 +70,11 @@ fn worktree_root_override_redirects_placement() {
     let wt_root = TempDir::new().unwrap();
     let mgr = WorktreeManager::new(Some(wt_root.path().to_path_buf()));
 
+    // The repo's basename sits between the root and the branch slug, so two
+    // 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("feat-example"));
+    assert_eq!(path, wt_root.path().join(repo_name).join("feat-example"));
     assert!(path.exists());
 }
 
@@ -288,3 +291,75 @@ fn add_fails_when_branch_already_exists() {
     let msg = format!("{err}");
     assert!(msg.contains("git exited"), "{msg}");
 }
+
+/// The project picker lists linked worktrees as pickable rows, so `base_repo`
+/// is easily a worktree rather than the main checkout. Placing inside it would
+/// nest one worktree in another — and git hides a registered worktree's
+/// contents from the outer `git status`, so the outer tree reads clean and
+/// removing it deletes the inner one's uncommitted work (#54 review).
+#[test]
+fn placement_resolves_a_linked_worktree_to_the_main_checkout() {
+    let repo_dir = TempDir::new().unwrap();
+    let repo = repo_dir.path().to_path_buf();
+    init_repo(&repo);
+
+    let mgr = WorktreeManager::new(None);
+    let outer = mgr.add_new(&repo, "feat/outer").expect("add outer");
+
+    // Spawn again with the *worktree* as the base, the way the picker would.
+    let inner = mgr.add_new(&outer, "feat/inner").expect("add inner");
+    assert!(
+        !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"
+    );
+}
+
+/// Belt and braces for a nesting that predates this daemon: refuse to remove
+/// a worktree that has another registered underneath it.
+#[test]
+fn remove_refuses_a_worktree_holding_a_nested_one() {
+    let repo_dir = TempDir::new().unwrap();
+    let repo = repo_dir.path().to_path_buf();
+    init_repo(&repo);
+
+    let mgr = WorktreeManager::new(None);
+    let outer = mgr.add_new(&repo, "feat/outer").expect("add outer");
+    // Register one by hand inside `outer`, bypassing our placement.
+    let nested = outer.join("worktrees").join("hand-rolled");
+    run_git(
+        &repo,
+        &[
+            "worktree",
+            "add",
+            "-b",
+            "feat/hand",
+            nested.to_str().unwrap(),
+        ],
+    );
+    std::fs::write(nested.join("notes.txt"), b"unsaved\n").unwrap();
+
+    mgr.remove(&repo, &outer)
+        .expect_err("a worktree holding a nested one must be refused");
+    assert!(nested.join("notes.txt").exists(), "inner work survives");
+}
+
+/// Two repos spawning the same branch name under one override root must not
+/// collide (#54 review).
+#[test]
+fn override_root_keeps_repos_apart() {
+    let root_dir = TempDir::new().unwrap();
+    let a_dir = TempDir::new().unwrap();
+    let b_dir = TempDir::new().unwrap();
+    init_repo(a_dir.path());
+    init_repo(b_dir.path());
+
+    let mgr = WorktreeManager::new(Some(root_dir.path().to_path_buf()));
+    let a = mgr.add_new(a_dir.path(), "feat/shared").expect("repo a");
+    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");
+}

web/src/multiview.ts

@@ -154,13 +154,14 @@ export function createMultiView(opts: MultiViewOptions): MultiView {
         onSuccess: (session) => {
           // The pane can be closed while the POST is in flight. Mounting the
           // session onto a leaf no longer in the tree would strand it in no
-          // pane at all, so only claim the leaf while it is still there.
-          const t = activeTab(layout);
-          const attached = leaves(t.root).includes(leaf);
-          if (attached) {
+          // pane at all, so only claim the leaf while it is still there —
+          // searching every tab, since the operator may have switched tabs
+          // while the request was in flight.
+          const host = layout.tabs.find((t) => leaves(t.root).includes(leaf));
+          if (host) {
             leaf.sessionId = session.id;
             leaf.draft = null;
-            t.focusedLeafId = leaf.id;
+            host.focusedLeafId = leaf.id;
           }
           render();
           opts.onOpenSessionsChange?.();