From 66ac781e65e253ec143d82fd9512ae0c0b3b4b52 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Mon, 13 Apr 2026 20:35:28 -0400 Subject: [PATCH] Start new git worktrees in detached HEAD state (#53840) Create worktrees in detached HEAD state, then attempt to check out the requested branch. If the checkout fails (e.g. the branch is already in use by another worktree), log a warning and stay in detached HEAD state instead of showing an error to the user. This simplifies the worktree creation flow by: - Removing the occupied-branch fallback logic that would generate random branch names - Always using `CreateWorktreeTarget::Detached` for worktree creation - Attempting branch checkout as a best-effort post-creation step - Updating the branch picker UI to reflect the new behavior Release Notes: - Threads now start git worktrees in detached HEAD state if branch is in use or unspecified, instead of generating a random branch name. --------- Co-authored-by: Eric Holk --- crates/agent_ui/src/agent_panel.rs | 326 +++++++++--------- crates/agent_ui/src/agent_ui.rs | 2 +- crates/agent_ui/src/thread_branch_picker.rs | 9 +- .../{branch_names.rs => worktree_names.rs} | 22 +- crates/fs/src/fake_git_repo.rs | 9 + crates/git/src/repository.rs | 33 ++ crates/project/src/git_store.rs | 26 ++ 7 files changed, 248 insertions(+), 179 deletions(-) rename crates/agent_ui/src/{branch_names.rs => worktree_names.rs} (90%) diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index 73380d4ca4ce7d9025353e933c22b3634e992287..ffb13b4d7074ec57d320defb535016a5c8f09b12 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -1,5 +1,5 @@ use std::{ - path::PathBuf, + path::{Path, PathBuf}, rc::Rc, sync::{ Arc, @@ -755,9 +755,9 @@ impl StartThreadIn { fn branch_trigger_label(&self, project: &Project, cx: &App) -> Option { match self { Self::NewWorktree { branch_target, .. } => { - let (branch_name, is_occupied) = match branch_target { + let label: SharedString = match branch_target { NewWorktreeBranchTarget::CurrentBranch => { - let name: SharedString = if project.repositories(cx).len() > 1 { + if project.repositories(cx).len() > 1 { "current branches".into() } else { project @@ -769,49 +769,25 @@ impl StartThreadIn { .map(|branch| SharedString::from(branch.name().to_string())) }) .unwrap_or_else(|| "HEAD".into()) - }; - (name, false) - } - NewWorktreeBranchTarget::ExistingBranch { name } => { - let occupied = Self::is_branch_occupied(name, project, cx); - (name.clone().into(), occupied) + } } + NewWorktreeBranchTarget::ExistingBranch { name } => name.clone().into(), NewWorktreeBranchTarget::CreateBranch { from_ref: Some(from_ref), .. - } => { - let occupied = Self::is_branch_occupied(from_ref, project, cx); - (from_ref.clone().into(), occupied) - } - NewWorktreeBranchTarget::CreateBranch { name, .. } => { - (name.clone().into(), false) - } - }; - - let prefix = if is_occupied { - Some("New From:".into()) - } else { - None + } => from_ref.clone().into(), + NewWorktreeBranchTarget::CreateBranch { name, .. } => name.clone().into(), }; Some(StartThreadInLabel { - prefix, - label: branch_name, + prefix: None, + label, suffix: None, }) } _ => None, } } - - fn is_branch_occupied(branch_name: &str, project: &Project, cx: &App) -> bool { - project.repositories(cx).values().any(|repo| { - repo.read(cx) - .linked_worktrees - .iter() - .any(|wt| wt.branch_name() == Some(branch_name)) - }) - } } #[derive(Clone, Debug)] @@ -3147,28 +3123,15 @@ impl AgentPanel { fn resolve_worktree_branch_target( branch_target: &NewWorktreeBranchTarget, - existing_branches: &HashSet, - occupied_branches: &HashSet, - rng: &mut impl rand::Rng, - ) -> Result<(String, bool, Option)> { - let mut generate_branch_name = || { - let refs: Vec<&str> = existing_branches.iter().map(|s| s.as_str()).collect(); - crate::branch_names::generate_branch_name(&refs, rng) - .ok_or_else(|| anyhow!("Failed to generate a unique branch name")) - }; - + ) -> (Option, Option) { match branch_target { - NewWorktreeBranchTarget::CreateBranch { name, from_ref } => { - Ok((name.clone(), false, from_ref.clone())) - } + NewWorktreeBranchTarget::CurrentBranch => (None, None), NewWorktreeBranchTarget::ExistingBranch { name } => { - if occupied_branches.contains(name) { - Ok((generate_branch_name()?, false, Some(name.clone()))) - } else { - Ok((name.clone(), true, None)) - } + (Some(name.clone()), Some(name.clone())) + } + NewWorktreeBranchTarget::CreateBranch { name, from_ref } => { + (Some(name.clone()), from_ref.clone()) } - NewWorktreeBranchTarget::CurrentBranch => Ok((generate_branch_name()?, false, None)), } } @@ -3183,9 +3146,7 @@ impl AgentPanel { worktree_name: Option, existing_worktree_names: &[String], existing_worktree_paths: &HashSet, - branch_name: &str, - use_existing_branch: bool, - start_point: Option, + base_ref: Option, worktree_directory_setting: &str, rng: &mut impl rand::Rng, cx: &mut Context, @@ -3203,8 +3164,8 @@ impl AgentPanel { let worktree_name = worktree_name.unwrap_or_else(|| { let existing_refs: Vec<&str> = existing_worktree_names.iter().map(|s| s.as_str()).collect(); - crate::branch_names::generate_branch_name(&existing_refs, rng) - .unwrap_or_else(|| branch_name.to_string()) + crate::worktree_names::generate_worktree_name(&existing_refs, rng) + .unwrap_or_else(|| "worktree".to_string()) }); for repo in git_repos { @@ -3214,19 +3175,8 @@ impl AgentPanel { if existing_worktree_paths.contains(&new_path) { anyhow::bail!("A worktree already exists at {}", new_path.display()); } - let target = if use_existing_branch { - debug_assert!( - git_repos.len() == 1, - "use_existing_branch should only be true for a single repo" - ); - git::repository::CreateWorktreeTarget::ExistingBranch { - branch_name: branch_name.to_string(), - } - } else { - git::repository::CreateWorktreeTarget::NewBranch { - branch_name: branch_name.to_string(), - base_sha: start_point.clone(), - } + let target = git::repository::CreateWorktreeTarget::Detached { + base_sha: base_ref.clone(), }; let receiver = repo.create_worktree(target, new_path.clone()); let work_dir = repo.work_directory_abs_path.clone(); @@ -3351,6 +3301,103 @@ impl AgentPanel { Err(anyhow!(error_message)) } + /// Attempts to check out a branch in a newly created worktree. + /// First tries checking out an existing branch, then tries creating a new + /// branch. If both fail, the worktree stays in detached HEAD state. + async fn try_checkout_branch_in_worktree( + repo: &Entity, + branch_name: &str, + worktree_path: &Path, + cx: &mut AsyncWindowContext, + ) { + // First, try checking out the branch (it may already exist). + let Ok(receiver) = cx.update(|_, cx| { + repo.update(cx, |repo, _cx| { + repo.checkout_branch_in_worktree( + branch_name.to_string(), + worktree_path.to_path_buf(), + false, + ) + }) + }) else { + log::warn!( + "Failed to check out branch {branch_name} for worktree at {}. \ + Staying in detached HEAD state.", + worktree_path.display(), + ); + + return; + }; + + let Ok(result) = receiver.await else { + log::warn!( + "Branch checkout was canceled for worktree at {}. \ + Staying in detached HEAD state.", + worktree_path.display() + ); + + return; + }; + + if let Err(err) = result { + log::info!( + "Failed to check out branch '{branch_name}' in worktree at {}, \ + will try creating it: {err}", + worktree_path.display() + ); + } else { + log::info!( + "Checked out branch '{branch_name}' in worktree at {}", + worktree_path.display() + ); + + return; + } + + // Checkout failed, so try creating the branch. + let create_result = cx.update(|_, cx| { + repo.update(cx, |repo, _cx| { + repo.checkout_branch_in_worktree( + branch_name.to_string(), + worktree_path.to_path_buf(), + true, + ) + }) + }); + + match create_result { + Ok(receiver) => match receiver.await { + Ok(Ok(())) => { + log::info!( + "Created and checked out branch '{branch_name}' in worktree at {}", + worktree_path.display() + ); + } + Ok(Err(err)) => { + log::warn!( + "Failed to create branch '{branch_name}' in worktree at {}: {err}. \ + Staying in detached HEAD state.", + worktree_path.display() + ); + } + Err(_) => { + log::warn!( + "Branch creation was canceled for worktree at {}. \ + Staying in detached HEAD state.", + worktree_path.display() + ); + } + }, + Err(err) => { + log::warn!( + "Failed to dispatch branch creation for worktree at {}: {err}. \ + Staying in detached HEAD state.", + worktree_path.display(), + ); + } + } + } + fn set_worktree_creation_error( &mut self, message: SharedString, @@ -3400,15 +3447,9 @@ impl AgentPanel { return; } - let (branch_receivers, worktree_receivers, worktree_directory_setting) = + let (worktree_receivers, worktree_directory_setting) = if matches!(args, WorktreeCreationArgs::New { .. }) { ( - Some( - git_repos - .iter() - .map(|repo| repo.update(cx, |repo, _cx| repo.branches())) - .collect::>(), - ), Some( git_repos .iter() @@ -3423,7 +3464,7 @@ impl AgentPanel { ), ) } else { - (None, None, None) + (None, None) }; let active_file_path = self.workspace.upgrade().and_then(|workspace| { @@ -3467,38 +3508,17 @@ impl AgentPanel { worktree_name, branch_target, } => { - let branch_receivers = branch_receivers - .expect("branch receivers must be prepared for new worktree creation"); let worktree_receivers = worktree_receivers .expect("worktree receivers must be prepared for new worktree creation"); let worktree_directory_setting = worktree_directory_setting .expect("worktree directory must be prepared for new worktree creation"); - let mut existing_branches = HashSet::default(); - for result in futures::future::join_all(branch_receivers).await { - match result { - Ok(Ok(branches)) => { - for branch in branches { - existing_branches.insert(branch.name().to_string()); - } - } - Ok(Err(err)) => { - Err::<(), _>(err).log_err(); - } - Err(_) => {} - } - } - - let mut occupied_branches = HashSet::default(); let mut existing_worktree_names = Vec::new(); let mut existing_worktree_paths = HashSet::default(); for result in futures::future::join_all(worktree_receivers).await { match result { Ok(Ok(worktrees)) => { for worktree in worktrees { - if let Some(branch_name) = worktree.branch_name() { - occupied_branches.insert(branch_name.to_string()); - } if let Some(name) = worktree .path .parent() @@ -3519,25 +3539,8 @@ impl AgentPanel { let mut rng = rand::rng(); - let (branch_name, use_existing_branch, start_point) = - match Self::resolve_worktree_branch_target( - &branch_target, - &existing_branches, - &occupied_branches, - &mut rng, - ) { - Ok(target) => target, - Err(err) => { - this.update_in(cx, |this, window, cx| { - this.set_worktree_creation_error( - err.to_string().into(), - window, - cx, - ); - })?; - return anyhow::Ok(()); - } - }; + let (branch_to_checkout, base_ref) = + Self::resolve_worktree_branch_target(&branch_target); let (creation_infos, path_remapping) = match this.update_in(cx, |_this, _window, cx| { @@ -3546,9 +3549,7 @@ impl AgentPanel { worktree_name, &existing_worktree_names, &existing_worktree_paths, - &branch_name, - use_existing_branch, - start_point, + base_ref, &worktree_directory_setting, &mut rng, cx, @@ -3569,6 +3570,12 @@ impl AgentPanel { } }; + let repo_paths: Vec<(Entity, PathBuf)> = + creation_infos + .iter() + .map(|(repo, path, _)| (repo.clone(), path.clone())) + .collect(); + let fs = cx.update(|_, cx| ::global(cx))?; let created_paths = @@ -3586,6 +3593,18 @@ impl AgentPanel { } }; + if let Some(ref branch_name) = branch_to_checkout { + for (repo, worktree_path) in &repo_paths { + Self::try_checkout_branch_in_worktree( + repo, + branch_name, + worktree_path, + cx, + ) + .await; + } + } + let mut all_paths = created_paths; let has_non_git = !non_git_paths.is_empty(); all_paths.extend(non_git_paths.iter().cloned()); @@ -5379,7 +5398,7 @@ mod tests { use fs::FakeFs; use gpui::{TestAppContext, VisualTestContext}; use project::Project; - use rand::rngs::StdRng; + use serde_json::json; use std::path::Path; use std::time::Instant; @@ -6709,54 +6728,37 @@ mod tests { ); } - #[gpui::test(iterations = 10)] - fn test_resolve_worktree_branch_target(mut rng: StdRng) { - let existing_branches = HashSet::from_iter([ - "main".to_string(), - "feature".to_string(), - "origin/main".to_string(), - ]); - - let resolved = AgentPanel::resolve_worktree_branch_target( - &NewWorktreeBranchTarget::CreateBranch { + #[gpui::test] + fn test_resolve_worktree_branch_target() { + let resolved = + AgentPanel::resolve_worktree_branch_target(&NewWorktreeBranchTarget::CreateBranch { name: "new-branch".to_string(), from_ref: Some("main".to_string()), - }, - &existing_branches, - &HashSet::from_iter(["main".to_string()]), - &mut rng, - ) - .unwrap(); + }); assert_eq!( resolved, - ("new-branch".to_string(), false, Some("main".to_string())) + (Some("new-branch".to_string()), Some("main".to_string())) ); - let resolved = AgentPanel::resolve_worktree_branch_target( - &NewWorktreeBranchTarget::ExistingBranch { + let resolved = + AgentPanel::resolve_worktree_branch_target(&NewWorktreeBranchTarget::CreateBranch { + name: "new-branch".to_string(), + from_ref: None, + }); + assert_eq!(resolved, (Some("new-branch".to_string()), None)); + + let resolved = + AgentPanel::resolve_worktree_branch_target(&NewWorktreeBranchTarget::ExistingBranch { name: "feature".to_string(), - }, - &existing_branches, - &HashSet::default(), - &mut rng, - ) - .unwrap(); - assert_eq!(resolved, ("feature".to_string(), true, None)); + }); + assert_eq!( + resolved, + (Some("feature".to_string()), Some("feature".to_string())) + ); - let resolved = AgentPanel::resolve_worktree_branch_target( - &NewWorktreeBranchTarget::ExistingBranch { - name: "main".to_string(), - }, - &existing_branches, - &HashSet::from_iter(["main".to_string()]), - &mut rng, - ) - .unwrap(); - assert_eq!(resolved.1, false); - assert_eq!(resolved.2, Some("main".to_string())); - assert_ne!(resolved.0, "main"); - assert!(existing_branches.contains("main")); - assert!(!existing_branches.contains(&resolved.0)); + let resolved = + AgentPanel::resolve_worktree_branch_target(&NewWorktreeBranchTarget::CurrentBranch); + assert_eq!(resolved, (None, None)); } #[gpui::test] diff --git a/crates/agent_ui/src/agent_ui.rs b/crates/agent_ui/src/agent_ui.rs index 18075e4e318cf68f0e441cd5f3f06c2c13343c69..d5883f9247dd7a1ec6aae9393f820294bcc71047 100644 --- a/crates/agent_ui/src/agent_ui.rs +++ b/crates/agent_ui/src/agent_ui.rs @@ -4,7 +4,6 @@ mod agent_diff; mod agent_model_selector; mod agent_panel; mod agent_registry_ui; -mod branch_names; mod buffer_codegen; mod completion_provider; mod config_options; @@ -37,6 +36,7 @@ pub mod thread_worktree_archive; mod thread_worktree_picker; pub mod threads_archive_view; mod ui; +mod worktree_names; use std::path::PathBuf; use std::rc::Rc; diff --git a/crates/agent_ui/src/thread_branch_picker.rs b/crates/agent_ui/src/thread_branch_picker.rs index e38f96e403b4de374b99e620a800b97869b89ba9..b2961624332fa0c71927a24f58cb3908a25e4e48 100644 --- a/crates/agent_ui/src/thread_branch_picker.rs +++ b/crates/agent_ui/src/thread_branch_picker.rs @@ -320,11 +320,9 @@ impl ThreadBranchPickerDelegate { fn branch_aside_text(&self, branch_name: &str, is_remote: bool) -> Option { if self.is_branch_occupied(branch_name) { Some( - format!( - "This branch is already checked out in another worktree. \ - A new branch will be created from {branch_name}." - ) - .into(), + "This branch is already checked out in another worktree. \ + The new worktree will start in detached HEAD state." + .into(), ) } else if is_remote { Some("A new local branch will be created from this remote branch.".into()) @@ -726,6 +724,7 @@ impl PickerDelegate for ThreadBranchPickerDelegate { .default_branch_name .as_ref() .filter(|name| *name != &self.current_branch_name)?; + let is_occupied = self.is_branch_occupied(default_branch_name); let item = ListItem::new("default-branch") diff --git a/crates/agent_ui/src/branch_names.rs b/crates/agent_ui/src/worktree_names.rs similarity index 90% rename from crates/agent_ui/src/branch_names.rs rename to crates/agent_ui/src/worktree_names.rs index 460be9bb85c50ab7c4b326d3ad1485f95df3ec94..68be75cb4c66ef43ab6644fdc23e407716726e72 100644 --- a/crates/agent_ui/src/branch_names.rs +++ b/crates/agent_ui/src/worktree_names.rs @@ -54,12 +54,12 @@ const NOUNS: &[&str] = &[ "vole", "walrus", "warbler", "willow", "wolf", "wren", "yew", "zenith", ]; -/// Generates a branch name in `"adjective-noun"` format (e.g. `"swift-falcon"`). +/// Generates a worktree name in `"adjective-noun"` format (e.g. `"swift-falcon"`). /// -/// Tries up to 100 random combinations, skipping any name that already appears -/// in `existing_branches`. Returns `None` if no unused name is found. -pub fn generate_branch_name(existing_branches: &[&str], rng: &mut impl Rng) -> Option { - let existing: HashSet<&str> = existing_branches.iter().copied().collect(); +/// Tries up to 10 random combinations, skipping any name that already appears +/// in `existing_names`. Returns `None` if no unused name is found. +pub fn generate_worktree_name(existing_names: &[&str], rng: &mut impl Rng) -> Option { + let existing: HashSet<&str> = existing_names.iter().copied().collect(); for _ in 0..10 { let adjective = ADJECTIVES[rng.random_range(0..ADJECTIVES.len())]; @@ -80,8 +80,8 @@ mod tests { use rand::rngs::StdRng; #[gpui::test(iterations = 10)] - fn test_generate_branch_name_format(mut rng: StdRng) { - let name = generate_branch_name(&[], &mut rng).unwrap(); + fn test_generate_worktree_name_format(mut rng: StdRng) { + let name = generate_worktree_name(&[], &mut rng).unwrap(); let (adjective, noun) = name.split_once('-').expect("name should contain a hyphen"); assert!( ADJECTIVES.contains(&adjective), @@ -91,9 +91,9 @@ mod tests { } #[gpui::test(iterations = 100)] - fn test_generate_branch_name_avoids_existing(mut rng: StdRng) { + fn test_generate_worktree_name_avoids_existing(mut rng: StdRng) { let existing = &["swift-falcon", "calm-river", "bold-cedar"]; - let name = generate_branch_name(existing, &mut rng).unwrap(); + let name = generate_worktree_name(existing, &mut rng).unwrap(); for &branch in existing { assert_ne!( name, branch, @@ -103,13 +103,13 @@ mod tests { } #[gpui::test] - fn test_generate_branch_name_returns_none_when_stuck(mut rng: StdRng) { + fn test_generate_worktree_name_returns_none_when_stuck(mut rng: StdRng) { let all_names: Vec = ADJECTIVES .iter() .flat_map(|adj| NOUNS.iter().map(move |noun| format!("{adj}-{noun}"))) .collect(); let refs: Vec<&str> = all_names.iter().map(|s| s.as_str()).collect(); - let result = generate_branch_name(&refs, &mut rng); + let result = generate_worktree_name(&refs, &mut rng); assert!(result.is_none()); } diff --git a/crates/fs/src/fake_git_repo.rs b/crates/fs/src/fake_git_repo.rs index 1b4e89102f942c3b4e5526b914c67c271a47ee2e..f224a13bbf3558d8fb45a37f2a975d99b187af8a 100644 --- a/crates/fs/src/fake_git_repo.rs +++ b/crates/fs/src/fake_git_repo.rs @@ -783,6 +783,15 @@ impl GitRepository for FakeGitRepository { .boxed() } + fn checkout_branch_in_worktree( + &self, + _branch_name: String, + _worktree_path: PathBuf, + _create: bool, + ) -> BoxFuture<'_, Result<()>> { + async { Ok(()) }.boxed() + } + fn change_branch(&self, name: String) -> BoxFuture<'_, Result<()>> { self.with_state_async(true, |state| { state.current_branch_name = Some(name); diff --git a/crates/git/src/repository.rs b/crates/git/src/repository.rs index faf973505af6cde1b2e736a0bfb630fa18c3647c..809081d4f0b6ab53be300fa9b04c537cb47d5096 100644 --- a/crates/git/src/repository.rs +++ b/crates/git/src/repository.rs @@ -757,6 +757,13 @@ pub trait GitRepository: Send + Sync { path: PathBuf, ) -> BoxFuture<'_, Result<()>>; + fn checkout_branch_in_worktree( + &self, + branch_name: String, + worktree_path: PathBuf, + create: bool, + ) -> BoxFuture<'_, Result<()>>; + fn remove_worktree(&self, path: PathBuf, force: bool) -> BoxFuture<'_, Result<()>>; fn rename_worktree(&self, old_path: PathBuf, new_path: PathBuf) -> BoxFuture<'_, Result<()>>; @@ -1799,6 +1806,32 @@ impl GitRepository for RealGitRepository { .boxed() } + fn checkout_branch_in_worktree( + &self, + branch_name: String, + worktree_path: PathBuf, + create: bool, + ) -> BoxFuture<'_, Result<()>> { + let git_binary = GitBinary::new( + self.any_git_binary_path.clone(), + worktree_path, + self.path(), + self.executor.clone(), + self.is_trusted(), + ); + + self.executor + .spawn(async move { + if create { + git_binary.run(&["checkout", "-b", &branch_name]).await?; + } else { + git_binary.run(&["checkout", &branch_name]).await?; + } + anyhow::Ok(()) + }) + .boxed() + } + fn change_branch(&self, name: String) -> BoxFuture<'_, Result<()>> { let repo = self.repository.clone(); let git_binary = self.git_binary(); diff --git a/crates/project/src/git_store.rs b/crates/project/src/git_store.rs index c760203dcdb8436bce9353b45f7d8ea20fa1eb78..27c84239816c905c87b37e6e5fc6a765ffa96150 100644 --- a/crates/project/src/git_store.rs +++ b/crates/project/src/git_store.rs @@ -6098,6 +6098,32 @@ impl Repository { ) } + pub fn checkout_branch_in_worktree( + &mut self, + branch_name: String, + worktree_path: PathBuf, + create: bool, + ) -> oneshot::Receiver> { + let description = if create { + format!("git checkout -b {branch_name}") + } else { + format!("git checkout {branch_name}") + }; + self.send_job(Some(description.into()), move |repo, _cx| async move { + match repo { + RepositoryState::Local(LocalRepositoryState { backend, .. }) => { + backend + .checkout_branch_in_worktree(branch_name, worktree_path, create) + .await + } + RepositoryState::Remote(_) => { + log::warn!("checkout_branch_in_worktree not supported for remote repositories"); + Ok(()) + } + } + }) + } + pub fn head_sha(&mut self) -> oneshot::Receiver>> { let id = self.id; self.send_job(None, move |repo, _cx| async move {