diff --git a/crates/agent_ui/src/agent_panel.rs b/crates/agent_ui/src/agent_panel.rs index 2ff4cd18a78fd53c5d540e66670d6e6c9e51aa47..0becd8efb4ecc1e0edb98255608f6ad463641979 100644 --- a/crates/agent_ui/src/agent_panel.rs +++ b/crates/agent_ui/src/agent_panel.rs @@ -2740,6 +2740,8 @@ impl AgentPanel { fn start_worktree_creations( git_repos: &[Entity], worktree_name: Option, + existing_worktree_names: &[String], + existing_worktree_paths: &HashSet, branch_name: &str, use_existing_branch: bool, start_point: Option, @@ -2756,12 +2758,21 @@ impl AgentPanel { let mut creation_infos = Vec::new(); let mut path_remapping = Vec::new(); - let worktree_name = worktree_name.unwrap_or_else(|| branch_name.to_string()); + let worktree_name = worktree_name.unwrap_or_else(|| { + let existing_refs: Vec<&str> = + existing_worktree_names.iter().map(|s| s.as_str()).collect(); + let mut rng = rand::rng(); + crate::branch_names::generate_branch_name(&existing_refs, &mut rng) + .unwrap_or_else(|| branch_name.to_string()) + }); for repo in git_repos { let (work_dir, new_path, receiver) = repo.update(cx, |repo, _cx| { let new_path = repo.path_for_new_linked_worktree(&worktree_name, worktree_directory_setting)?; + 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, @@ -2827,7 +2838,8 @@ impl AgentPanel { return Ok(created_paths); }; - // Rollback all attempted worktrees (both successful and failed) + // Rollback all attempted worktrees (both successful and failed, + // since a failed creation may have left an orphan directory). let mut rollback_futures = Vec::new(); for (rollback_repo, rollback_path) in &repos_and_paths { let receiver = cx @@ -3037,6 +3049,8 @@ impl AgentPanel { } 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)) => { @@ -3044,6 +3058,15 @@ impl AgentPanel { if let Some(branch_name) = worktree.branch_name() { occupied_branches.insert(branch_name.to_string()); } + if let Some(name) = worktree + .path + .parent() + .and_then(|p| p.file_name()) + .and_then(|n| n.to_str()) + { + existing_worktree_names.push(name.to_string()); + } + existing_worktree_paths.insert(worktree.path.clone()); } } Ok(Err(err)) => { @@ -3077,6 +3100,8 @@ impl AgentPanel { Self::start_worktree_creations( &git_repos, worktree_name, + &existing_worktree_names, + &existing_worktree_paths, &branch_name, use_existing_branch, start_point, @@ -6165,6 +6190,151 @@ mod tests { assert!(!existing_branches.contains(&resolved.0)); } + #[gpui::test] + async fn test_worktree_dir_name_is_random_when_using_existing_branch(cx: &mut TestAppContext) { + init_test(cx); + + let app_state = cx.update(|cx| { + agent::ThreadStore::init_global(cx); + language_model::LanguageModelRegistry::test(cx); + + let app_state = workspace::AppState::test(cx); + workspace::init(app_state.clone(), cx); + app_state + }); + + let fs = app_state.fs.as_fake(); + fs.insert_tree( + "/project", + json!({ + ".git": {}, + "src": { + "main.rs": "fn main() {}" + } + }), + ) + .await; + // Put the main worktree on "develop" so that "main" is NOT + // occupied by any worktree. + fs.set_branch_name(Path::new("/project/.git"), Some("develop")); + fs.insert_branches(Path::new("/project/.git"), &["main", "develop"]); + + let project = Project::test(app_state.fs.clone(), [Path::new("/project")], cx).await; + + let multi_workspace = + cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + multi_workspace + .update(cx, |multi_workspace, _, cx| { + multi_workspace.open_sidebar(cx); + }) + .unwrap(); + + let workspace = multi_workspace + .read_with(cx, |multi_workspace, _cx| { + multi_workspace.workspace().clone() + }) + .unwrap(); + + workspace.update(cx, |workspace, _cx| { + workspace.set_random_database_id(); + }); + + cx.update(|cx| { + cx.observe_new( + |workspace: &mut Workspace, + window: Option<&mut Window>, + cx: &mut Context| { + if let Some(window) = window { + let panel = cx.new(|cx| AgentPanel::new(workspace, None, window, cx)); + workspace.add_panel(panel, window, cx); + } + }, + ) + .detach(); + }); + + let cx = &mut VisualTestContext::from_window(multi_workspace.into(), cx); + cx.run_until_parked(); + + let panel = workspace.update_in(cx, |workspace, window, cx| { + let panel = cx.new(|cx| AgentPanel::new(workspace, None, window, cx)); + workspace.add_panel(panel.clone(), window, cx); + panel + }); + + cx.run_until_parked(); + + panel.update_in(cx, |panel, window, cx| { + panel.open_external_thread_with_server( + Rc::new(StubAgentServer::default_response()), + window, + cx, + ); + }); + + cx.run_until_parked(); + + // Select "main" as an existing branch — this should NOT make the + // worktree directory named "main"; it should get a random name. + let content = vec![acp::ContentBlock::Text(acp::TextContent::new( + "Hello from test", + ))]; + panel.update_in(cx, |panel, window, cx| { + panel.handle_worktree_requested( + content, + WorktreeCreationArgs::New { + worktree_name: None, + branch_target: NewWorktreeBranchTarget::ExistingBranch { + name: "main".to_string(), + }, + }, + window, + cx, + ); + }); + + cx.run_until_parked(); + + // Find the new workspace and check its worktree path. + let new_worktree_path = multi_workspace + .read_with(cx, |multi_workspace, cx| { + let new_workspace = multi_workspace + .workspaces() + .find(|ws| ws.entity_id() != workspace.entity_id()) + .expect("a new workspace should have been created"); + + let new_project = new_workspace.read(cx).project().clone(); + let worktree = new_project + .read(cx) + .visible_worktrees(cx) + .next() + .expect("new workspace should have a worktree"); + worktree.read(cx).abs_path().to_path_buf() + }) + .unwrap(); + + // The worktree directory path should contain a random adjective-noun + // name, NOT the branch name "main". + let path_str = new_worktree_path.to_string_lossy(); + assert!( + !path_str.contains("/main/"), + "worktree directory should use a random name, not the branch name. \ + Got path: {path_str}", + ); + // Verify it looks like an adjective-noun pair (contains a hyphen in + // the directory component above the project name). + let parent = new_worktree_path + .parent() + .and_then(|p| p.file_name()) + .and_then(|n| n.to_str()) + .expect("should have a parent directory name"); + assert!( + parent.contains('-'), + "worktree parent directory should be an adjective-noun pair (e.g. 'swift-falcon'), \ + got: {parent}", + ); + } + #[gpui::test] async fn test_worktree_creation_preserves_selected_agent(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/agent_ui/src/branch_names.rs b/crates/agent_ui/src/branch_names.rs index 74a934e95d6219382312bdb49e7e28501a6a2517..460be9bb85c50ab7c4b326d3ad1485f95df3ec94 100644 --- a/crates/agent_ui/src/branch_names.rs +++ b/crates/agent_ui/src/branch_names.rs @@ -61,7 +61,7 @@ const NOUNS: &[&str] = &[ pub fn generate_branch_name(existing_branches: &[&str], rng: &mut impl Rng) -> Option { let existing: HashSet<&str> = existing_branches.iter().copied().collect(); - for _ in 0..100 { + for _ in 0..10 { let adjective = ADJECTIVES[rng.random_range(0..ADJECTIVES.len())]; let noun = NOUNS[rng.random_range(0..NOUNS.len())]; let name = format!("{adjective}-{noun}");