This repository has been archived on 2026-10-08. You can view files and clone it. You cannot open issues or pull requests or push a commit.
Files
maestro/crates/maestro-daemon/tests/worktree.rs
T
Miguel Palhas 8b3c5d73b6 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.
2026-08-23 21:36:29 +01:00

366 lines
12 KiB
Rust

//! Worktree-manager integration tests against a real `git` binary and a
//! tmpdir-backed source repo.
mod common;
use std::path::PathBuf;
use std::process::Command;
use common::init_repo;
use maestro_daemon::worktree::WorktreeManager;
use tempfile::TempDir;
fn worktree_branches(repo: &PathBuf) -> Vec<String> {
let out = Command::new("git")
.arg("-C")
.arg(repo)
.args(["worktree", "list", "--porcelain"])
.output()
.expect("git worktree list");
assert!(out.status.success());
let s = String::from_utf8_lossy(&out.stdout);
s.lines()
.filter_map(|l| l.strip_prefix("branch refs/heads/").map(String::from))
.collect()
}
#[test]
fn add_creates_worktree_on_new_branch() {
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()));
let path = mgr.add_new(&repo, "feat/example").expect("add worktree");
assert!(path.exists());
assert!(path.join("README").exists());
assert!(path.starts_with(wt_root.path()));
let branches = worktree_branches(&repo);
assert!(branches.iter().any(|b| b == "feat/example"), "{branches:?}");
}
#[test]
fn add_places_worktree_inside_the_base_repo_by_default() {
let repo_dir = TempDir::new().unwrap();
let repo = repo_dir.path().to_path_buf();
init_repo(&repo);
// No override: the worktree must land where the repository keeps its
// own worktrees (#56), not under some daemon state dir.
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!(path.exists());
assert!(path.join("README").exists());
let branches = worktree_branches(&repo);
assert!(branches.iter().any(|b| b == "feat/example"), "{branches:?}");
}
#[test]
fn worktree_root_override_redirects_placement() {
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()));
// 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(repo_name).join("feat-example"));
assert!(path.exists());
}
#[test]
fn existing_directory_at_target_path_is_an_error_carrying_the_path() {
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();
let mgr = WorktreeManager::new(None);
let err = mgr.add_new(&repo, "feat/collide").unwrap_err();
let msg = format!("{err}");
assert!(
msg.contains(
repo.join("worktrees")
.join("feat-collide")
.to_str()
.unwrap()
),
"{msg}"
);
}
#[test]
fn add_twice_generates_distinct_paths() {
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()));
let p1 = mgr.add_new(&repo, "feat/a").expect("add a");
let p2 = mgr.add_new(&repo, "feat/b").expect("add b");
assert_ne!(p1, p2);
assert!(p1.exists());
assert!(p2.exists());
}
#[test]
fn remove_prunes_worktree() {
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()));
let path = mgr.add_new(&repo, "feat/gone").expect("add");
assert!(path.exists());
mgr.remove(&repo, &path).expect("remove");
assert!(!path.exists());
let branches = worktree_branches(&repo);
// The branch itself still exists, but the worktree should be gone — only
// the base repo's main branch should show in `worktree list`.
assert_eq!(branches, vec!["main".to_string()]);
}
#[test]
fn add_existing_checks_out_branch_without_creating_it() {
let repo_dir = TempDir::new().unwrap();
let repo = repo_dir.path().to_path_buf();
init_repo(&repo);
// Pre-create the branch with its own commit so we can tell a checkout
// from an accidental `-b`.
let created = Command::new("git")
.arg("-C")
.arg(&repo)
.args(["checkout", "-q", "-b", "feat/dup"])
.status()
.unwrap();
assert!(created.success());
std::fs::write(repo.join("FEATURE"), b"dup\n").unwrap();
let staged = Command::new("git")
.arg("-C")
.arg(&repo)
.args(["add", "."])
.status()
.unwrap();
assert!(staged.success());
let committed = Command::new("git")
.arg("-C")
.arg(&repo)
.args(["commit", "-q", "-am", "wip"])
.status()
.unwrap();
assert!(committed.success());
let back = Command::new("git")
.arg("-C")
.arg(&repo)
.args(["checkout", "-q", "main"])
.status()
.unwrap();
assert!(back.success());
let wt_root = TempDir::new().unwrap();
let mgr = WorktreeManager::new(Some(wt_root.path().to_path_buf()));
let path = mgr.add_existing(&repo, "feat/dup").expect("add existing");
assert!(path.exists());
assert!(path.join("README").exists());
// The branch set is unchanged: `feat/dup` was checked out, not recreated.
let branches = worktree_branches(&repo);
let mut sorted = branches.clone();
sorted.sort();
assert_eq!(
sorted,
vec!["feat/dup".to_string(), "main".to_string()],
"{branches:?}"
);
}
#[test]
fn remove_refuses_worktree_with_uncommitted_changes() {
let repo_dir = TempDir::new().unwrap();
let repo = repo_dir.path().to_path_buf();
init_repo(&repo);
let mgr = WorktreeManager::new(None);
let path = mgr.add_new(&repo, "feat/dirty").expect("add");
std::fs::write(path.join("README"), b"unsaved edits\n").unwrap();
mgr.remove(&repo, &path)
.expect_err("dirty worktree must be refused");
assert!(path.exists(), "guarded worktree must stay on disk");
// Committing it makes removal safe: `worktree remove` keeps
// `refs/heads/feat/dirty`, so the work is still reachable. Refusing here
// instead would make every unpushed branch a one-shot, since the path is
// deterministic and the leftover tree blocks the next spawn (#56 review).
run_git(&path, &["add", "."]);
run_git(&path, &["commit", "-q", "-m", "saved"]);
mgr.remove(&repo, &path).expect("committed work can go");
assert!(!path.exists());
assert!(
worktree_branches(&repo).contains(&"main".to_string()),
"the branch survives removal"
);
let branches = String::from_utf8(
Command::new("git")
.arg("-C")
.arg(&repo)
.args(["for-each-ref", "--format=%(refname:short)", "refs/heads"])
.output()
.unwrap()
.stdout,
)
.unwrap();
assert!(branches.contains("feat/dirty"), "{branches}");
}
/// A worktree whose directory vanished must be pruned, not misread as holding
/// unsaved work — otherwise the registration is stranded and, because paths
/// are deterministic, that branch can never be spawned again (#56 review).
#[test]
fn remove_prunes_a_worktree_whose_directory_vanished() {
let repo_dir = TempDir::new().unwrap();
let repo = repo_dir.path().to_path_buf();
init_repo(&repo);
let mgr = WorktreeManager::new(None);
let path = mgr.add_new(&repo, "feat/gone").expect("add");
std::fs::remove_dir_all(&path).unwrap();
mgr.remove(&repo, &path)
.expect("a vanished worktree prunes");
// The registration is gone too, so the same branch can be spawned again.
mgr.add_existing(&repo, "feat/gone")
.expect("branch is spawnable again");
}
fn run_git(dir: &std::path::Path, args: &[&str]) {
let out = Command::new("git")
.arg("-C")
.arg(dir)
.args(args)
.output()
.expect("git spawn");
assert!(
out.status.success(),
"git {args:?} failed: {}",
String::from_utf8_lossy(&out.stderr)
);
}
#[test]
fn add_fails_when_branch_already_exists() {
let repo_dir = TempDir::new().unwrap();
let repo = repo_dir.path().to_path_buf();
init_repo(&repo);
// pre-create the branch
let status = Command::new("git")
.arg("-C")
.arg(&repo)
.args(["branch", "feat/dup"])
.status()
.unwrap();
assert!(status.success());
let wt_root = TempDir::new().unwrap();
let mgr = WorktreeManager::new(Some(wt_root.path().to_path_buf()));
let err = mgr.add_new(&repo, "feat/dup").unwrap_err();
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");
}