diff --git a/src-tauri/src/shared/workspaces_core/worktree.rs b/src-tauri/src/shared/workspaces_core/worktree.rs index bd22d5ad66..2221115961 100644 --- a/src-tauri/src/shared/workspaces_core/worktree.rs +++ b/src-tauri/src/shared/workspaces_core/worktree.rs @@ -346,7 +346,7 @@ pub(crate) async fn rename_worktree_core< data_dir: &PathBuf, workspaces: &Mutex>, sessions: &Mutex>>, - _app_settings: &Mutex, + app_settings: &Mutex, storage_path: &PathBuf, resolve_git_root: FResolveGitRoot, unique_branch_name: FUniqueBranch, @@ -406,9 +406,21 @@ where return Err("Branch name is unchanged.".to_string()); } - run_git_command(&parent_root, &["branch", "-m", &old_branch, &final_branch]).await?; - - let worktree_root = data_dir.join("worktrees").join(&parent.id); + // Use the same priority logic as add_worktree_core: + // per-workspace setting > global setting > default + let worktree_root = if let Some(custom_folder) = &parent.settings.worktrees_folder { + PathBuf::from(custom_folder) + } else { + let global_folder = { + let settings = app_settings.lock().await; + settings.global_worktrees_folder.clone() + }; + if let Some(global_folder) = global_folder { + PathBuf::from(global_folder).join(&parent.id) + } else { + data_dir.join("worktrees").join(&parent.id) + } + }; std::fs::create_dir_all(&worktree_root) .map_err(|err| format!("Failed to create worktree directory: {err}"))?; @@ -416,10 +428,15 @@ where let current_path = PathBuf::from(&entry.path); let next_path = unique_worktree_path_for_rename(&worktree_root, &safe_name, ¤t_path)?; let next_path_string = next_path.to_string_lossy().to_string(); - if next_path_string != entry.path { + let old_path_string = entry.path.clone(); + + run_git_command(&parent_root, &["branch", "-m", &old_branch, &final_branch]).await?; + + let mut moved_worktree = false; + if next_path_string != old_path_string { if let Err(error) = run_git_command( &parent_root, - &["worktree", "move", &entry.path, &next_path_string], + &["worktree", "move", &old_path_string, &next_path_string], ) .await { @@ -427,33 +444,64 @@ where run_git_command(&parent_root, &["branch", "-m", &final_branch, &old_branch]).await; return Err(error); } + moved_worktree = true; } - let (entry_snapshot, list) = { + let update_result: Result<(WorkspaceEntry, WorkspaceEntry, Vec), String> = { let mut workspaces = workspaces.lock().await; - let entry = match workspaces.get_mut(&id) { - Some(entry) => entry, - None => return Err("workspace not found".to_string()), - }; - if entry.name.trim() == old_branch { - entry.name = final_branch.clone(); - } - entry.path = next_path_string.clone(); - match entry.worktree.as_mut() { - Some(worktree) => { - worktree.branch = final_branch.clone(); + if let Some(entry) = workspaces.get_mut(&id) { + let old_snapshot = entry.clone(); + if entry.name.trim() == old_branch { + entry.name = final_branch.clone(); } - None => { - entry.worktree = Some(WorktreeInfo { - branch: final_branch.clone(), - }); + entry.path = next_path_string.clone(); + match entry.worktree.as_mut() { + Some(worktree) => { + worktree.branch = final_branch.clone(); + } + None => { + entry.worktree = Some(WorktreeInfo { + branch: final_branch.clone(), + }); + } } + let snapshot = entry.clone(); + let list: Vec<_> = workspaces.values().cloned().collect(); + Ok((old_snapshot, snapshot, list)) + } else { + Err("workspace not found".to_string()) } - let snapshot = entry.clone(); - let list: Vec<_> = workspaces.values().cloned().collect(); - (snapshot, list) }; - write_workspaces(storage_path, &list)?; + let (old_snapshot, entry_snapshot, list) = match update_result { + Ok(value) => value, + Err(error) => { + if moved_worktree { + let _ = run_git_command( + &parent_root, + &["worktree", "move", &next_path_string, &old_path_string], + ) + .await; + } + let _ = + run_git_command(&parent_root, &["branch", "-m", &final_branch, &old_branch]).await; + return Err(error); + } + }; + if let Err(error) = write_workspaces(storage_path, &list) { + if moved_worktree { + let _ = run_git_command( + &parent_root, + &["worktree", "move", &next_path_string, &old_path_string], + ) + .await; + } + let _ = run_git_command(&parent_root, &["branch", "-m", &final_branch, &old_branch]).await; + let mut workspaces = workspaces.lock().await; + if let Some(entry) = workspaces.get_mut(&id) { + *entry = old_snapshot; + } + return Err(error); + } if let Some(session) = sessions.lock().await.get(&entry_snapshot.id).cloned() { session diff --git a/src-tauri/src/workspaces/tests.rs b/src-tauri/src/workspaces/tests.rs index 04fe3ff190..01311ae70d 100644 --- a/src-tauri/src/workspaces/tests.rs +++ b/src-tauri/src/workspaces/tests.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use std::future::Future; use std::path::PathBuf; -use std::sync::Arc; +use std::sync::{Arc, Mutex as StdMutex}; use super::settings::{apply_workspace_settings_update, sort_workspaces}; use super::worktree::{ @@ -56,6 +56,7 @@ fn workspace_with_id_and_kind( launch_script: None, launch_scripts: None, worktree_setup_script: None, + worktrees_folder: None, }, } } @@ -390,6 +391,95 @@ fn rename_worktree_updates_name_when_unmodified() { }); } +#[test] +fn rename_worktree_validates_worktree_root_before_branch_rename() { + run_async(async { + let temp_dir = std::env::temp_dir().join(format!("codex-monitor-test-{}", Uuid::new_v4())); + let repo_path = temp_dir.join("repo"); + std::fs::create_dir_all(&repo_path).expect("create repo path"); + let worktree_path = temp_dir.join("worktrees").join("parent").join("old"); + std::fs::create_dir_all(&worktree_path).expect("create worktree path"); + + let invalid_root = temp_dir.join("not-a-directory"); + std::fs::write(&invalid_root, "x").expect("create invalid root file"); + + let mut parent_settings = WorkspaceSettings::default(); + parent_settings.worktrees_folder = Some(invalid_root.to_string_lossy().to_string()); + let parent = WorkspaceEntry { + id: "parent".to_string(), + name: "Parent".to_string(), + path: repo_path.to_string_lossy().to_string(), + kind: WorkspaceKind::Main, + parent_id: None, + worktree: None, + settings: parent_settings, + }; + let worktree = WorkspaceEntry { + id: "wt-3".to_string(), + name: "feature/old".to_string(), + path: worktree_path.to_string_lossy().to_string(), + kind: WorkspaceKind::Worktree, + parent_id: Some(parent.id.clone()), + worktree: Some(WorktreeInfo { + branch: "feature/old".to_string(), + }), + settings: WorkspaceSettings::default(), + }; + let workspaces = Mutex::new(HashMap::from([ + (parent.id.clone(), parent.clone()), + (worktree.id.clone(), worktree.clone()), + ])); + let sessions: Mutex>> = Mutex::new(HashMap::new()); + let app_settings = Mutex::new(AppSettings::default()); + let storage_path = temp_dir.join("workspaces.json"); + + let calls: Arc>>> = Arc::new(StdMutex::new(Vec::new())); + let result = rename_worktree_core( + worktree.id.clone(), + "feature/new".to_string(), + &temp_dir, + &workspaces, + &sessions, + &app_settings, + &storage_path, + |_| Ok(repo_path.clone()), + |_root, branch| { + let branch = branch.to_string(); + async move { Ok(branch) } + }, + |value| sanitize_worktree_name(value), + |_, _, current| Ok(current.to_path_buf()), + |_root, args| { + let calls = calls.clone(); + let args: Vec = args.iter().map(|value| value.to_string()).collect(); + async move { + calls + .lock() + .expect("lock") + .push(args); + Ok(()) + } + }, + |_entry, _default_bin, _codex_args, _codex_home| async move { + Err("spawn not expected".to_string()) + }, + ) + .await; + + let error = result.expect_err("expected invalid worktree root to fail"); + assert!(error.contains("Failed to create worktree directory")); + assert!(calls.lock().expect("lock").is_empty()); + + let stored = workspaces.lock().await; + let entry = stored.get(&worktree.id).expect("stored entry"); + assert_eq!( + entry.worktree.as_ref().map(|worktree| worktree.branch.as_str()), + Some("feature/old") + ); + assert_eq!(entry.path, worktree.path); + }); +} + #[test] fn remove_workspace_succeeds_when_parent_repo_folder_is_missing() { run_async(async { diff --git a/src/features/settings/components/SettingsView.test.tsx b/src/features/settings/components/SettingsView.test.tsx index e8d5a7e038..2147d60a18 100644 --- a/src/features/settings/components/SettingsView.test.tsx +++ b/src/features/settings/components/SettingsView.test.tsx @@ -151,6 +151,7 @@ const baseSettings: AppSettings = { }, ], selectedOpenAppId: "vscode", + globalWorktreesFolder: null, }; const createDoctorResult = () => ({ @@ -689,6 +690,7 @@ describe("SettingsView Environments", () => { await waitFor(() => { expect(onUpdateWorkspaceSettings).toHaveBeenCalledWith("w1", { worktreeSetupScript: "echo updated", + worktreesFolder: null, }); }); }); @@ -704,6 +706,7 @@ describe("SettingsView Environments", () => { await waitFor(() => { expect(onUpdateWorkspaceSettings).toHaveBeenCalledWith("w1", { worktreeSetupScript: null, + worktreesFolder: null, }); }); }); diff --git a/src/features/settings/components/sections/SettingsEnvironmentsSection.tsx b/src/features/settings/components/sections/SettingsEnvironmentsSection.tsx index 8f6b253cbb..f04194cf50 100644 --- a/src/features/settings/components/sections/SettingsEnvironmentsSection.tsx +++ b/src/features/settings/components/sections/SettingsEnvironmentsSection.tsx @@ -29,7 +29,7 @@ export function SettingsEnvironmentsSection({ environmentSavedScript, environmentDirty, worktreesFolderDraft, - worktreesFolderSaved, + worktreesFolderSaved: _worktreesFolderSaved, worktreesFolderDirty, onSetEnvironmentWorkspaceId, onSetEnvironmentDraftScript, diff --git a/src/features/settings/hooks/useAppSettings.ts b/src/features/settings/hooks/useAppSettings.ts index e4dafaccc3..98ebe00da9 100644 --- a/src/features/settings/hooks/useAppSettings.ts +++ b/src/features/settings/hooks/useAppSettings.ts @@ -206,6 +206,7 @@ function buildDefaultSettings(): AppSettings { workspaceGroups: [], openAppTargets: DEFAULT_OPEN_APP_TARGETS, selectedOpenAppId: DEFAULT_OPEN_APP_ID, + globalWorktreesFolder: null, }; }