git: Add SSH support for removing and renaming git worktrees (#50759)

Anthony Eid created

This should be the last step in implementing full git worktree support
in the `GitStore`. We still need to add UI for that allows a user to
rename a git worktree and, by extension git branches if we use the git
picker to do so.

Also, I added a helper function called `disallow_guest_request::<T>` to
the `Collab::rpc` that is used to specify a proto request isn't allowed
to be sent by a guest. This enabled me to add a regression test that
checks that a guest isn't allowed to delete a git worktree, without the
test hanging forever because it's waiting for the proto server to
respond.

Since SSH connections send the proto message directly from client to
remote host, this won't affect those requests.

Before you mark this PR as ready for review, make sure that you have:
- [x] Added solid test coverage and/or screenshots from doing manual
testing
- [x] Done a self-review taking into account security and performance
aspects
- [ ] Aligned any UI changes with the [UI
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)

Release Notes:

- git: Add SSH support for removing git worktrees

Change summary

crates/agent_ui/src/agent_panel.rs                                    |   6 
crates/collab/src/rpc.rs                                              |  20 
crates/collab/tests/integration/git_tests.rs                          |  61 
crates/collab/tests/integration/remote_editing_collaboration_tests.rs | 116 
crates/project/src/git_store.rs                                       |  86 
crates/proto/proto/git.proto                                          |  14 
crates/proto/proto/zed.proto                                          |   4 
crates/proto/src/proto.rs                                             |   6 
8 files changed, 308 insertions(+), 5 deletions(-)

Detailed changes

crates/agent_ui/src/agent_panel.rs 🔗

@@ -374,12 +374,15 @@ pub fn init(cx: &mut App) {
 
                         panel.update(cx, |panel, cx| {
                             panel.external_thread(
+                                None,
+                                None,
                                 None,
                                 None,
                                 Some(AgentInitialContent::ContentBlock {
                                     blocks: content_blocks,
                                     auto_submit: true,
                                 }),
+                                true,
                                 window,
                                 cx,
                             );
@@ -399,12 +402,15 @@ pub fn init(cx: &mut App) {
 
                         panel.update(cx, |panel, cx| {
                             panel.external_thread(
+                                None,
+                                None,
                                 None,
                                 None,
                                 Some(AgentInitialContent::ContentBlock {
                                     blocks: content_blocks,
                                     auto_submit: true,
                                 }),
+                                true,
                                 window,
                                 cx,
                             );

crates/collab/src/rpc.rs 🔗

@@ -439,6 +439,8 @@ impl Server {
             .add_request_handler(forward_mutating_project_request::<proto::GitRemoveRemote>)
             .add_request_handler(forward_read_only_project_request::<proto::GitGetWorktrees>)
             .add_request_handler(forward_mutating_project_request::<proto::GitCreateWorktree>)
+            .add_request_handler(disallow_guest_request::<proto::GitRemoveWorktree>)
+            .add_request_handler(disallow_guest_request::<proto::GitRenameWorktree>)
             .add_request_handler(forward_mutating_project_request::<proto::CheckForPushedCommits>)
             .add_message_handler(broadcast_project_message_from_host::<proto::AdvertiseContexts>)
             .add_message_handler(update_context)
@@ -2250,6 +2252,24 @@ where
     Ok(())
 }
 
+async fn disallow_guest_request<T>(
+    _request: T,
+    response: Response<T>,
+    _session: MessageContext,
+) -> Result<()>
+where
+    T: RequestMessage,
+{
+    response.peer.respond_with_error(
+        response.receipt,
+        ErrorCode::Forbidden
+            .message("request is not allowed for guests".to_string())
+            .to_proto(),
+    )?;
+    response.responded.store(true, SeqCst);
+    Ok(())
+}
+
 async fn lsp_query(
     request: proto::LspQuery,
     response: Response<proto::LspQuery>,

crates/collab/tests/integration/git_tests.rs 🔗

@@ -302,6 +302,67 @@ async fn test_remote_git_worktrees(
         worktree_directory.join("bugfix-branch")
     );
     assert_eq!(bugfix_worktree.sha.as_ref(), "fake-sha");
+
+    // Client B (guest) attempts to rename a worktree. This should fail
+    // because worktree renaming is not forwarded through collab
+    let rename_result = cx_b
+        .update(|cx| {
+            repo_b.update(cx, |repository, _| {
+                repository.rename_worktree(
+                    worktree_directory.join("feature-branch"),
+                    worktree_directory.join("renamed-branch"),
+                )
+            })
+        })
+        .await
+        .unwrap();
+    assert!(
+        rename_result.is_err(),
+        "Guest should not be able to rename worktrees via collab"
+    );
+
+    executor.run_until_parked();
+
+    // Verify worktrees are unchanged — still 3
+    let worktrees = cx_b
+        .update(|cx| repo_b.update(cx, |repository, _| repository.worktrees()))
+        .await
+        .unwrap()
+        .unwrap();
+    assert_eq!(
+        worktrees.len(),
+        3,
+        "Worktree count should be unchanged after failed rename"
+    );
+
+    // Client B (guest) attempts to remove a worktree. This should fail
+    // because worktree removal is not forwarded through collab
+    let remove_result = cx_b
+        .update(|cx| {
+            repo_b.update(cx, |repository, _| {
+                repository.remove_worktree(worktree_directory.join("feature-branch"), false)
+            })
+        })
+        .await
+        .unwrap();
+    assert!(
+        remove_result.is_err(),
+        "Guest should not be able to remove worktrees via collab"
+    );
+
+    executor.run_until_parked();
+
+    // Verify worktrees are unchanged — still 3
+    let worktrees = cx_b
+        .update(|cx| repo_b.update(cx, |repository, _| repository.worktrees()))
+        .await
+        .unwrap()
+        .unwrap();
+    assert_eq!(
+        worktrees.len(),
+        3,
+        "Worktree count should be unchanged after failed removal"
+    );
 }
 
 #[gpui::test]

crates/collab/tests/integration/remote_editing_collaboration_tests.rs 🔗

@@ -518,6 +518,122 @@ async fn test_ssh_collaboration_git_worktrees(
         server_worktrees[1].path,
         worktree_directory.join("feature-branch")
     );
+
+    // Host (client A) renames the worktree via SSH
+    let repo_a = cx_a.update(|cx| {
+        project_a
+            .read(cx)
+            .repositories(cx)
+            .values()
+            .next()
+            .unwrap()
+            .clone()
+    });
+    cx_a.update(|cx| {
+        repo_a.update(cx, |repository, _| {
+            repository.rename_worktree(
+                PathBuf::from("/project/feature-branch"),
+                PathBuf::from("/project/renamed-branch"),
+            )
+        })
+    })
+    .await
+    .unwrap()
+    .unwrap();
+
+    executor.run_until_parked();
+
+    let host_worktrees = cx_a
+        .update(|cx| repo_a.update(cx, |repository, _| repository.worktrees()))
+        .await
+        .unwrap()
+        .unwrap();
+    assert_eq!(
+        host_worktrees.len(),
+        2,
+        "Host should still have 2 worktrees after rename"
+    );
+    assert_eq!(
+        host_worktrees[1].path,
+        PathBuf::from("/project/renamed-branch")
+    );
+
+    let server_worktrees = {
+        let server_repo = server_cx.update(|cx| {
+            headless_project.update(cx, |headless_project, cx| {
+                headless_project
+                    .git_store
+                    .read(cx)
+                    .repositories()
+                    .values()
+                    .next()
+                    .unwrap()
+                    .clone()
+            })
+        });
+        server_cx
+            .update(|cx| server_repo.update(cx, |repo, _| repo.worktrees()))
+            .await
+            .unwrap()
+            .unwrap()
+    };
+    assert_eq!(
+        server_worktrees.len(),
+        2,
+        "Server should still have 2 worktrees after rename"
+    );
+    assert_eq!(
+        server_worktrees[1].path,
+        PathBuf::from("/project/renamed-branch")
+    );
+
+    // Host (client A) removes the renamed worktree via SSH
+    cx_a.update(|cx| {
+        repo_a.update(cx, |repository, _| {
+            repository.remove_worktree(PathBuf::from("/project/renamed-branch"), false)
+        })
+    })
+    .await
+    .unwrap()
+    .unwrap();
+
+    executor.run_until_parked();
+
+    let host_worktrees = cx_a
+        .update(|cx| repo_a.update(cx, |repository, _| repository.worktrees()))
+        .await
+        .unwrap()
+        .unwrap();
+    assert_eq!(
+        host_worktrees.len(),
+        1,
+        "Host should only have the main worktree after removal"
+    );
+
+    let server_worktrees = {
+        let server_repo = server_cx.update(|cx| {
+            headless_project.update(cx, |headless_project, cx| {
+                headless_project
+                    .git_store
+                    .read(cx)
+                    .repositories()
+                    .values()
+                    .next()
+                    .unwrap()
+                    .clone()
+            })
+        });
+        server_cx
+            .update(|cx| server_repo.update(cx, |repo, _| repo.worktrees()))
+            .await
+            .unwrap()
+            .unwrap()
+    };
+    assert_eq!(
+        server_worktrees.len(),
+        1,
+        "Server should only have the main worktree after removal"
+    );
 }
 
 #[gpui::test]

crates/project/src/git_store.rs 🔗

@@ -578,6 +578,8 @@ impl GitStore {
         client.add_entity_request_handler(Self::handle_git_clone);
         client.add_entity_request_handler(Self::handle_get_worktrees);
         client.add_entity_request_handler(Self::handle_create_worktree);
+        client.add_entity_request_handler(Self::handle_remove_worktree);
+        client.add_entity_request_handler(Self::handle_rename_worktree);
     }
 
     pub fn is_local(&self) -> bool {
@@ -2384,6 +2386,44 @@ impl GitStore {
         Ok(proto::Ack {})
     }
 
+    async fn handle_remove_worktree(
+        this: Entity<Self>,
+        envelope: TypedEnvelope<proto::GitRemoveWorktree>,
+        mut cx: AsyncApp,
+    ) -> Result<proto::Ack> {
+        let repository_id = RepositoryId::from_proto(envelope.payload.repository_id);
+        let repository_handle = Self::repository_for_request(&this, repository_id, &mut cx)?;
+        let path = PathBuf::from(envelope.payload.path);
+        let force = envelope.payload.force;
+
+        repository_handle
+            .update(&mut cx, |repository_handle, _| {
+                repository_handle.remove_worktree(path, force)
+            })
+            .await??;
+
+        Ok(proto::Ack {})
+    }
+
+    async fn handle_rename_worktree(
+        this: Entity<Self>,
+        envelope: TypedEnvelope<proto::GitRenameWorktree>,
+        mut cx: AsyncApp,
+    ) -> Result<proto::Ack> {
+        let repository_id = RepositoryId::from_proto(envelope.payload.repository_id);
+        let repository_handle = Self::repository_for_request(&this, repository_id, &mut cx)?;
+        let old_path = PathBuf::from(envelope.payload.old_path);
+        let new_path = PathBuf::from(envelope.payload.new_path);
+
+        repository_handle
+            .update(&mut cx, |repository_handle, _| {
+                repository_handle.rename_worktree(old_path, new_path)
+            })
+            .await??;
+
+        Ok(proto::Ack {})
+    }
+
     async fn handle_get_branches(
         this: Entity<Self>,
         envelope: TypedEnvelope<proto::GitGetBranches>,
@@ -5731,6 +5771,7 @@ impl Repository {
     }
 
     pub fn remove_worktree(&mut self, path: PathBuf, force: bool) -> oneshot::Receiver<Result<()>> {
+        let id = self.id;
         self.send_job(
             Some(format!("git worktree remove: {}", path.display()).into()),
             move |repo, _cx| async move {
@@ -5738,10 +5779,47 @@ impl Repository {
                     RepositoryState::Local(LocalRepositoryState { backend, .. }) => {
                         backend.remove_worktree(path, force).await
                     }
-                    RepositoryState::Remote(_) => {
-                        anyhow::bail!(
-                            "Removing worktrees on remote repositories is not yet supported"
-                        )
+                    RepositoryState::Remote(RemoteRepositoryState { project_id, client }) => {
+                        client
+                            .request(proto::GitRemoveWorktree {
+                                project_id: project_id.0,
+                                repository_id: id.to_proto(),
+                                path: path.to_string_lossy().to_string(),
+                                force,
+                            })
+                            .await?;
+
+                        Ok(())
+                    }
+                }
+            },
+        )
+    }
+
+    pub fn rename_worktree(
+        &mut self,
+        old_path: PathBuf,
+        new_path: PathBuf,
+    ) -> oneshot::Receiver<Result<()>> {
+        let id = self.id;
+        self.send_job(
+            Some(format!("git worktree move: {}", old_path.display()).into()),
+            move |repo, _cx| async move {
+                match repo {
+                    RepositoryState::Local(LocalRepositoryState { backend, .. }) => {
+                        backend.rename_worktree(old_path, new_path).await
+                    }
+                    RepositoryState::Remote(RemoteRepositoryState { project_id, client }) => {
+                        client
+                            .request(proto::GitRenameWorktree {
+                                project_id: project_id.0,
+                                repository_id: id.to_proto(),
+                                old_path: old_path.to_string_lossy().to_string(),
+                                new_path: new_path.to_string_lossy().to_string(),
+                            })
+                            .await?;
+
+                        Ok(())
                     }
                 }
             },

crates/proto/proto/git.proto 🔗

@@ -583,6 +583,20 @@ message GitCreateWorktree {
   optional string commit = 5;
 }
 
+message GitRemoveWorktree {
+  uint64 project_id = 1;
+  uint64 repository_id = 2;
+  string path = 3;
+  bool force = 4;
+}
+
+message GitRenameWorktree {
+  uint64 project_id = 1;
+  uint64 repository_id = 2;
+  string old_path = 3;
+  string new_path = 4;
+}
+
 message RunGitHook {
   enum GitHook {
     PRE_COMMIT = 0;

crates/proto/proto/zed.proto 🔗

@@ -474,7 +474,9 @@ message Envelope {
 
     SpawnKernel spawn_kernel = 426;
     SpawnKernelResponse spawn_kernel_response = 427;
-    KillKernel kill_kernel = 428; // current max
+    KillKernel kill_kernel = 428;
+    GitRemoveWorktree git_remove_worktree = 431;
+    GitRenameWorktree git_rename_worktree = 432; // current max
   }
 
   reserved 87 to 88;

crates/proto/src/proto.rs 🔗

@@ -354,6 +354,8 @@ messages!(
     (GitGetWorktrees, Background),
     (GitWorktreesResponse, Background),
     (GitCreateWorktree, Background),
+    (GitRemoveWorktree, Background),
+    (GitRenameWorktree, Background),
     (ShareAgentThread, Foreground),
     (GetSharedAgentThread, Foreground),
     (GetSharedAgentThreadResponse, Foreground),
@@ -557,6 +559,8 @@ request_messages!(
     (RemoteStarted, Ack),
     (GitGetWorktrees, GitWorktreesResponse),
     (GitCreateWorktree, Ack),
+    (GitRemoveWorktree, Ack),
+    (GitRenameWorktree, Ack),
     (TrustWorktrees, Ack),
     (RestrictWorktrees, Ack),
     (FindSearchCandidatesChunk, Ack),
@@ -747,6 +751,8 @@ entity_messages!(
     NewExternalAgentVersionAvailable,
     GitGetWorktrees,
     GitCreateWorktree,
+    GitRemoveWorktree,
+    GitRenameWorktree,
     TrustWorktrees,
     RestrictWorktrees,
     FindSearchCandidatesChunk,