@@ -2795,6 +2795,7 @@ impl AgentPanel {
PathBuf,
futures::channel::oneshot::Receiver<Result<()>>,
)>,
+ fs: Arc<dyn Fs>,
cx: &mut AsyncWindowContext,
) -> Result<Vec<PathBuf>> {
let mut created_paths: Vec<PathBuf> = Vec::new();
@@ -2803,10 +2804,10 @@ impl AgentPanel {
let mut first_error: Option<anyhow::Error> = None;
for (repo, new_path, receiver) in creation_infos {
+ repos_and_paths.push((repo.clone(), new_path.clone()));
match receiver.await {
Ok(Ok(())) => {
- created_paths.push(new_path.clone());
- repos_and_paths.push((repo, new_path));
+ created_paths.push(new_path);
}
Ok(Err(err)) => {
if first_error.is_none() {
@@ -2825,34 +2826,66 @@ impl AgentPanel {
return Ok(created_paths);
};
- // Rollback all successfully created worktrees
- let mut rollback_receivers = Vec::new();
+ // Rollback all attempted worktrees (both successful and failed)
+ let mut rollback_futures = Vec::new();
for (rollback_repo, rollback_path) in &repos_and_paths {
- if let Ok(receiver) = cx.update(|_, cx| {
- rollback_repo.update(cx, |repo, _cx| {
- repo.remove_worktree(rollback_path.clone(), true)
+ let receiver = cx
+ .update(|_, cx| {
+ rollback_repo.update(cx, |repo, _cx| {
+ repo.remove_worktree(rollback_path.clone(), true)
+ })
})
- }) {
- rollback_receivers.push((rollback_path.clone(), receiver));
- }
+ .ok();
+
+ rollback_futures.push((rollback_path.clone(), receiver));
}
+
let mut rollback_failures: Vec<String> = Vec::new();
- for (path, receiver) in rollback_receivers {
- match receiver.await {
- Ok(Ok(())) => {}
- Ok(Err(rollback_err)) => {
- log::error!(
- "failed to rollback worktree at {}: {rollback_err}",
- path.display()
- );
- rollback_failures.push(format!("{}: {rollback_err}", path.display()));
+ for (path, receiver_opt) in rollback_futures {
+ let mut git_remove_failed = false;
+
+ if let Some(receiver) = receiver_opt {
+ match receiver.await {
+ Ok(Ok(())) => {}
+ Ok(Err(rollback_err)) => {
+ log::error!(
+ "git worktree remove failed for {}: {rollback_err}",
+ path.display()
+ );
+ git_remove_failed = true;
+ }
+ Err(canceled) => {
+ log::error!(
+ "git worktree remove failed for {}: {canceled}",
+ path.display()
+ );
+ git_remove_failed = true;
+ }
}
- Err(rollback_err) => {
- log::error!(
- "failed to rollback worktree at {}: {rollback_err}",
- path.display()
- );
- rollback_failures.push(format!("{}: {rollback_err}", path.display()));
+ } else {
+ log::error!(
+ "failed to dispatch git worktree remove for {}",
+ path.display()
+ );
+ git_remove_failed = true;
+ }
+
+ // `git worktree remove` normally removes this directory, but since
+ // `git worktree remove` failed (or wasn't dispatched), manually rm the directory.
+ if git_remove_failed {
+ if let Err(fs_err) = fs
+ .remove_dir(
+ &path,
+ fs::RemoveOptions {
+ recursive: true,
+ ignore_if_not_exists: true,
+ },
+ )
+ .await
+ {
+ let msg = format!("{}: failed to remove directory: {fs_err}", path.display());
+ log::error!("{}", msg);
+ rollback_failures.push(msg);
}
}
}
@@ -3058,8 +3091,10 @@ impl AgentPanel {
}
};
+ let fs = cx.update(|_, cx| <dyn Fs>::global(cx))?;
+
let created_paths =
- match Self::await_and_rollback_on_failure(creation_infos, cx).await {
+ match Self::await_and_rollback_on_failure(creation_infos, fs, cx).await {
Ok(paths) => paths,
Err(err) => {
this.update_in(cx, |this, window, cx| {
@@ -4769,6 +4804,7 @@ mod tests {
};
use acp_thread::{StubAgentConnection, ThreadStatus};
use agent_servers::CODEX_ID;
+ use feature_flags::FeatureFlagAppExt;
use fs::FakeFs;
use gpui::{TestAppContext, VisualTestContext};
use project::Project;
@@ -6671,6 +6707,287 @@ mod tests {
});
}
+ #[gpui::test]
+ async fn test_rollback_all_succeed_returns_ok(cx: &mut TestAppContext) {
+ init_test(cx);
+ let fs = FakeFs::new(cx.executor());
+ cx.update(|cx| {
+ cx.update_flags(true, vec!["agent-v2".to_string()]);
+ agent::ThreadStore::init_global(cx);
+ language_model::LanguageModelRegistry::test(cx);
+ <dyn fs::Fs>::set_global(fs.clone(), cx);
+ });
+
+ fs.insert_tree(
+ "/project",
+ json!({
+ ".git": {},
+ "src": { "main.rs": "fn main() {}" }
+ }),
+ )
+ .await;
+
+ let project = Project::test(fs.clone(), [Path::new("/project")], cx).await;
+ cx.executor().run_until_parked();
+
+ let repository = project.read_with(cx, |project, cx| {
+ project.repositories(cx).values().next().unwrap().clone()
+ });
+
+ let multi_workspace =
+ cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx));
+
+ let path_a = PathBuf::from("/worktrees/branch/project_a");
+ let path_b = PathBuf::from("/worktrees/branch/project_b");
+
+ let (sender_a, receiver_a) = futures::channel::oneshot::channel::<Result<()>>();
+ let (sender_b, receiver_b) = futures::channel::oneshot::channel::<Result<()>>();
+ sender_a.send(Ok(())).unwrap();
+ sender_b.send(Ok(())).unwrap();
+
+ let creation_infos = vec![
+ (repository.clone(), path_a.clone(), receiver_a),
+ (repository.clone(), path_b.clone(), receiver_b),
+ ];
+
+ let fs_clone = fs.clone();
+ let result = multi_workspace
+ .update(cx, |_, window, cx| {
+ window.spawn(cx, async move |cx| {
+ AgentPanel::await_and_rollback_on_failure(creation_infos, fs_clone, cx).await
+ })
+ })
+ .unwrap()
+ .await;
+
+ let paths = result.expect("all succeed should return Ok");
+ assert_eq!(paths, vec![path_a, path_b]);
+ }
+
+ #[gpui::test]
+ async fn test_rollback_on_failure_attempts_all_worktrees(cx: &mut TestAppContext) {
+ init_test(cx);
+ let fs = FakeFs::new(cx.executor());
+ cx.update(|cx| {
+ cx.update_flags(true, vec!["agent-v2".to_string()]);
+ agent::ThreadStore::init_global(cx);
+ language_model::LanguageModelRegistry::test(cx);
+ <dyn fs::Fs>::set_global(fs.clone(), cx);
+ });
+
+ fs.insert_tree(
+ "/project",
+ json!({
+ ".git": {},
+ "src": { "main.rs": "fn main() {}" }
+ }),
+ )
+ .await;
+
+ let project = Project::test(fs.clone(), [Path::new("/project")], cx).await;
+ cx.executor().run_until_parked();
+
+ let repository = project.read_with(cx, |project, cx| {
+ project.repositories(cx).values().next().unwrap().clone()
+ });
+
+ // Actually create a worktree so it exists in FakeFs for rollback to find.
+ let success_path = PathBuf::from("/worktrees/branch/project");
+ cx.update(|cx| {
+ repository.update(cx, |repo, _| {
+ repo.create_worktree(
+ git::repository::CreateWorktreeTarget::NewBranch {
+ branch_name: "branch".to_string(),
+ base_sha: None,
+ },
+ success_path.clone(),
+ )
+ })
+ })
+ .await
+ .unwrap()
+ .unwrap();
+ cx.executor().run_until_parked();
+
+ // Verify the worktree directory exists before rollback.
+ assert!(
+ fs.is_dir(&success_path).await,
+ "worktree directory should exist before rollback"
+ );
+
+ let multi_workspace =
+ cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx));
+
+ // Build creation_infos: one success, one failure.
+ let failed_path = PathBuf::from("/worktrees/branch/failed_project");
+
+ let (sender_ok, receiver_ok) = futures::channel::oneshot::channel::<Result<()>>();
+ let (sender_err, receiver_err) = futures::channel::oneshot::channel::<Result<()>>();
+ sender_ok.send(Ok(())).unwrap();
+ sender_err
+ .send(Err(anyhow!("branch already exists")))
+ .unwrap();
+
+ let creation_infos = vec![
+ (repository.clone(), success_path.clone(), receiver_ok),
+ (repository.clone(), failed_path.clone(), receiver_err),
+ ];
+
+ let fs_clone = fs.clone();
+ let result = multi_workspace
+ .update(cx, |_, window, cx| {
+ window.spawn(cx, async move |cx| {
+ AgentPanel::await_and_rollback_on_failure(creation_infos, fs_clone, cx).await
+ })
+ })
+ .unwrap()
+ .await;
+
+ assert!(
+ result.is_err(),
+ "should return error when any creation fails"
+ );
+ let err_msg = result.unwrap_err().to_string();
+ assert!(
+ err_msg.contains("branch already exists"),
+ "error should mention the original failure: {err_msg}"
+ );
+
+ // The successful worktree should have been rolled back by git.
+ cx.executor().run_until_parked();
+ assert!(
+ !fs.is_dir(&success_path).await,
+ "successful worktree directory should be removed by rollback"
+ );
+ }
+
+ #[gpui::test]
+ async fn test_rollback_on_canceled_receiver(cx: &mut TestAppContext) {
+ init_test(cx);
+ let fs = FakeFs::new(cx.executor());
+ cx.update(|cx| {
+ cx.update_flags(true, vec!["agent-v2".to_string()]);
+ agent::ThreadStore::init_global(cx);
+ language_model::LanguageModelRegistry::test(cx);
+ <dyn fs::Fs>::set_global(fs.clone(), cx);
+ });
+
+ fs.insert_tree(
+ "/project",
+ json!({
+ ".git": {},
+ "src": { "main.rs": "fn main() {}" }
+ }),
+ )
+ .await;
+
+ let project = Project::test(fs.clone(), [Path::new("/project")], cx).await;
+ cx.executor().run_until_parked();
+
+ let repository = project.read_with(cx, |project, cx| {
+ project.repositories(cx).values().next().unwrap().clone()
+ });
+
+ let multi_workspace =
+ cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx));
+
+ let path = PathBuf::from("/worktrees/branch/project");
+
+ // Drop the sender to simulate a canceled receiver.
+ let (_sender, receiver) = futures::channel::oneshot::channel::<Result<()>>();
+ drop(_sender);
+
+ let creation_infos = vec![(repository.clone(), path.clone(), receiver)];
+
+ let fs_clone = fs.clone();
+ let result = multi_workspace
+ .update(cx, |_, window, cx| {
+ window.spawn(cx, async move |cx| {
+ AgentPanel::await_and_rollback_on_failure(creation_infos, fs_clone, cx).await
+ })
+ })
+ .unwrap()
+ .await;
+
+ assert!(
+ result.is_err(),
+ "should return error when receiver is canceled"
+ );
+ let err_msg = result.unwrap_err().to_string();
+ assert!(
+ err_msg.contains("canceled"),
+ "error should mention cancellation: {err_msg}"
+ );
+ }
+
+ #[gpui::test]
+ async fn test_rollback_cleans_up_orphan_directories(cx: &mut TestAppContext) {
+ init_test(cx);
+ let fs = FakeFs::new(cx.executor());
+ cx.update(|cx| {
+ cx.update_flags(true, vec!["agent-v2".to_string()]);
+ agent::ThreadStore::init_global(cx);
+ language_model::LanguageModelRegistry::test(cx);
+ <dyn fs::Fs>::set_global(fs.clone(), cx);
+ });
+
+ fs.insert_tree(
+ "/project",
+ json!({
+ ".git": {},
+ "src": { "main.rs": "fn main() {}" }
+ }),
+ )
+ .await;
+
+ let project = Project::test(fs.clone(), [Path::new("/project")], cx).await;
+ cx.executor().run_until_parked();
+
+ let repository = project.read_with(cx, |project, cx| {
+ project.repositories(cx).values().next().unwrap().clone()
+ });
+
+ let multi_workspace =
+ cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx));
+
+ // Simulate the orphan state: create_dir_all was called but git
+ // worktree add failed, leaving a directory with leftover files.
+ let orphan_path = PathBuf::from("/worktrees/branch/orphan_project");
+ fs.insert_tree(
+ "/worktrees/branch/orphan_project",
+ json!({ "leftover.txt": "junk" }),
+ )
+ .await;
+
+ assert!(
+ fs.is_dir(&orphan_path).await,
+ "orphan dir should exist before rollback"
+ );
+
+ let (sender, receiver) = futures::channel::oneshot::channel::<Result<()>>();
+ sender.send(Err(anyhow!("hook failed"))).unwrap();
+
+ let creation_infos = vec![(repository.clone(), orphan_path.clone(), receiver)];
+
+ let fs_clone = fs.clone();
+ let result = multi_workspace
+ .update(cx, |_, window, cx| {
+ window.spawn(cx, async move |cx| {
+ AgentPanel::await_and_rollback_on_failure(creation_infos, fs_clone, cx).await
+ })
+ })
+ .unwrap()
+ .await;
+
+ cx.executor().run_until_parked();
+
+ assert!(result.is_err());
+ assert!(
+ !fs.is_dir(&orphan_path).await,
+ "orphan worktree directory should be removed by filesystem cleanup"
+ );
+ }
+
#[gpui::test]
async fn test_worktree_creation_for_remote_project(
cx: &mut TestAppContext,