feat(maestro): worktree specs can check out an existing branch
This commit is contained in:
@@ -44,6 +44,7 @@ pub async fn run_spawn(client: &DaemonClient, args: SpawnArgs) -> Result<Session
|
||||
base_repo: repo.clone(),
|
||||
branch: branch.clone(),
|
||||
cleanup_on_kill: args.cleanup_on_kill,
|
||||
create_branch: true,
|
||||
}),
|
||||
_ => None,
|
||||
};
|
||||
|
||||
@@ -213,14 +213,16 @@ impl SessionManager {
|
||||
/// Spawn a new session. On success returns the freshly-created `Session`.
|
||||
pub async fn create(&self, req: CreateSessionRequest) -> Result<Session, SessionError> {
|
||||
let (effective_cwd, worktree_path, worktree_base_repo) = match &req.worktree {
|
||||
Some(WorktreeSpec {
|
||||
base_repo, branch, ..
|
||||
}) => {
|
||||
let path = self
|
||||
.inner
|
||||
.worktree_mgr
|
||||
.add(base_repo, branch)
|
||||
.map_err(SessionError::Worktree)?;
|
||||
Some(spec) => {
|
||||
let WorktreeSpec {
|
||||
base_repo, branch, ..
|
||||
} = spec;
|
||||
let path = if spec.create_branch {
|
||||
self.inner.worktree_mgr.add_new(base_repo, branch)
|
||||
} else {
|
||||
self.inner.worktree_mgr.add_existing(base_repo, branch)
|
||||
}
|
||||
.map_err(SessionError::Worktree)?;
|
||||
(path.clone(), Some(path), Some(base_repo.clone()))
|
||||
}
|
||||
None => (req.cwd.clone(), None, None),
|
||||
|
||||
@@ -31,17 +31,35 @@ impl WorktreeManager {
|
||||
&self.root
|
||||
}
|
||||
|
||||
/// `git -C <base_repo> worktree add -b <branch> <generated-path>`.
|
||||
/// Returns the path of the new worktree on success.
|
||||
pub fn add(&self, base_repo: &Path, branch: &str) -> Result<PathBuf, WorktreeError> {
|
||||
/// `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> {
|
||||
self.run_add(base_repo, branch, true)
|
||||
}
|
||||
|
||||
/// `git -C <base_repo> worktree add <path> <branch>` — checks out a branch
|
||||
/// that must already exist (#56).
|
||||
pub fn add_existing(&self, base_repo: &Path, branch: &str) -> Result<PathBuf, WorktreeError> {
|
||||
self.run_add(base_repo, branch, false)
|
||||
}
|
||||
|
||||
fn run_add(
|
||||
&self,
|
||||
base_repo: &Path,
|
||||
branch: &str,
|
||||
create_branch: bool,
|
||||
) -> Result<PathBuf, WorktreeError> {
|
||||
std::fs::create_dir_all(&self.root)?;
|
||||
let path = self.generate_path(branch);
|
||||
let out = Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(base_repo)
|
||||
.args(["worktree", "add", "-b", branch])
|
||||
.arg(&path)
|
||||
.output()?;
|
||||
let mut cmd = Command::new("git");
|
||||
cmd.arg("-C").arg(base_repo).args(["worktree", "add"]);
|
||||
if create_branch {
|
||||
cmd.arg("-b").arg(branch).arg(&path);
|
||||
} else {
|
||||
// Existing branch: `worktree add <path> <branch>` with no `-b`.
|
||||
cmd.arg(&path).arg(branch);
|
||||
}
|
||||
let out = cmd.output()?;
|
||||
check(out)?;
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
@@ -33,7 +33,7 @@ fn add_creates_worktree_on_new_branch() {
|
||||
let wt_root = TempDir::new().unwrap();
|
||||
let mgr = WorktreeManager::new(wt_root.path());
|
||||
|
||||
let path = mgr.add(&repo, "feat/example").expect("add worktree");
|
||||
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()));
|
||||
@@ -51,8 +51,8 @@ fn add_twice_generates_distinct_paths() {
|
||||
let wt_root = TempDir::new().unwrap();
|
||||
let mgr = WorktreeManager::new(wt_root.path());
|
||||
|
||||
let p1 = mgr.add(&repo, "feat/a").expect("add a");
|
||||
let p2 = mgr.add(&repo, "feat/b").expect("add b");
|
||||
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());
|
||||
@@ -67,7 +67,7 @@ fn remove_prunes_worktree() {
|
||||
let wt_root = TempDir::new().unwrap();
|
||||
let mgr = WorktreeManager::new(wt_root.path());
|
||||
|
||||
let path = mgr.add(&repo, "feat/gone").expect("add");
|
||||
let path = mgr.add_new(&repo, "feat/gone").expect("add");
|
||||
assert!(path.exists());
|
||||
|
||||
mgr.remove(&repo, &path).expect("remove");
|
||||
@@ -79,6 +79,62 @@ fn remove_prunes_worktree() {
|
||||
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(wt_root.path());
|
||||
|
||||
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 add_fails_when_branch_already_exists() {
|
||||
let repo_dir = TempDir::new().unwrap();
|
||||
@@ -97,7 +153,7 @@ fn add_fails_when_branch_already_exists() {
|
||||
let wt_root = TempDir::new().unwrap();
|
||||
let mgr = WorktreeManager::new(wt_root.path());
|
||||
|
||||
let err = mgr.add(&repo, "feat/dup").unwrap_err();
|
||||
let err = mgr.add_new(&repo, "feat/dup").unwrap_err();
|
||||
let msg = format!("{err}");
|
||||
assert!(msg.contains("git exited"), "{msg}");
|
||||
}
|
||||
|
||||
@@ -129,6 +129,7 @@ async fn cleanup_on_kill_removes_worktree() {
|
||||
base_repo: base.path().to_path_buf(),
|
||||
branch: "feature/e2e".into(),
|
||||
cleanup_on_kill: true,
|
||||
create_branch: true,
|
||||
}),
|
||||
};
|
||||
|
||||
@@ -160,6 +161,40 @@ async fn cleanup_on_kill_removes_worktree() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn spawn_on_existing_branch_checks_it_out() {
|
||||
let (_daemon, client) = bring_up().await;
|
||||
|
||||
let base = TempDir::new().unwrap();
|
||||
init_repo(base.path());
|
||||
run_git(&["branch", "feature/existing"], base.path());
|
||||
|
||||
let req = CreateSessionRequest {
|
||||
cwd: base.path().to_path_buf(),
|
||||
command: "sleep 30".into(),
|
||||
env: HashMap::new(),
|
||||
metadata: HashMap::new(),
|
||||
worktree: Some(WorktreeSpec {
|
||||
base_repo: base.path().to_path_buf(),
|
||||
branch: "feature/existing".into(),
|
||||
cleanup_on_kill: true,
|
||||
create_branch: false,
|
||||
}),
|
||||
};
|
||||
|
||||
let s = client
|
||||
.spawn_session(&req)
|
||||
.await
|
||||
.expect("spawn on existing branch");
|
||||
let worktrees = git_worktree_list(base.path());
|
||||
assert!(
|
||||
worktrees.iter().any(|w| w.contains("feature/existing")),
|
||||
"expected a worktree checked out at feature/existing, got: {worktrees:?}"
|
||||
);
|
||||
|
||||
client.kill_session(s.id).await.expect("kill");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn daemon_restart_recovers_session() {
|
||||
let (mut daemon, client) = bring_up().await;
|
||||
|
||||
@@ -113,6 +113,15 @@ pub struct WorktreeSpec {
|
||||
pub branch: String,
|
||||
#[serde(default)]
|
||||
pub cleanup_on_kill: bool,
|
||||
/// When false, `branch` must already exist and is checked out as-is
|
||||
/// (#56); when true (the default, and what every payload meant before
|
||||
/// the field existed) the daemon creates it with `worktree add -b`.
|
||||
#[serde(default = "default_true")]
|
||||
pub create_branch: bool,
|
||||
}
|
||||
|
||||
fn default_true() -> bool {
|
||||
true
|
||||
}
|
||||
|
||||
/// HTTP `POST /api/sessions` body.
|
||||
@@ -393,6 +402,7 @@ mod tests {
|
||||
base_repo: PathBuf::from("/home/naps62/projects/maestro"),
|
||||
branch: "feat/foo".into(),
|
||||
cleanup_on_kill: true,
|
||||
create_branch: true,
|
||||
}),
|
||||
};
|
||||
assert_eq!(req, round_trip(&req));
|
||||
@@ -410,6 +420,31 @@ mod tests {
|
||||
assert!(req.metadata.is_empty());
|
||||
}
|
||||
|
||||
/// A client that predates the new/existing discriminator sends no
|
||||
/// `create_branch`; the daemon must read that as "create a new branch" —
|
||||
/// exactly what every payload meant before the field existed (#56).
|
||||
#[test]
|
||||
fn worktree_spec_defaults_create_branch_true_on_legacy_payload() {
|
||||
let json = serde_json::json!({
|
||||
"base_repo": "/home/naps62/projects/maestro",
|
||||
"branch": "feat/foo",
|
||||
"cleanup_on_kill": true,
|
||||
});
|
||||
let spec: WorktreeSpec = serde_json::from_value(json).unwrap();
|
||||
assert!(spec.create_branch);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn worktree_spec_round_trips_existing_branch_mode() {
|
||||
let spec = WorktreeSpec {
|
||||
base_repo: PathBuf::from("/work"),
|
||||
branch: "feat/foo".into(),
|
||||
cleanup_on_kill: true,
|
||||
create_branch: false,
|
||||
};
|
||||
assert_eq!(spec, round_trip(&spec));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn catalog_event_round_trips_and_tags() {
|
||||
let session = sample_session();
|
||||
|
||||
Reference in New Issue
Block a user