Polish agent checkpoints (#29265)

Release Notes:

- Improved performance of agent checkpoint creation.
- Fixed a bug that sometimes caused accidental deletions when restoring
to a previous agent checkpoint.
- Fixed a bug that caused checkpoints to be visible in the Git history.

crates/agent/src/thread.rs

@@ -619,24 +619,12 @@ impl Thread {
                     .await
                     .unwrap_or(false);
 
-                if equal {
+                if !equal {
-                    git_store
-                        .update(cx, |store, cx| {
-                            store.delete_checkpoint(pending_checkpoint.git_checkpoint, cx)
-                        })?
-                        .detach();
-                } else {
                     this.update(cx, |this, cx| {
                         this.insert_checkpoint(pending_checkpoint, cx)
                     })?;
                 }
 
-                git_store
-                    .update(cx, |store, cx| {
-                        store.delete_checkpoint(final_checkpoint, cx)
-                    })?
-                    .detach();
-
                 Ok(())
             }
             Err(_) => this.update(cx, |this, cx| {

crates/fs/src/fake_git_repo.rs

@@ -431,10 +431,6 @@ impl GitRepository for FakeGitRepository {
         unimplemented!()
     }
 
-    fn delete_checkpoint(&self, _checkpoint: GitRepositoryCheckpoint) -> BoxFuture<Result<()>> {
-        unimplemented!()
-    }
-
     fn diff_checkpoints(
         &self,
         _base_checkpoint: GitRepositoryCheckpoint,

crates/git/src/repository.rs

@@ -314,9 +314,6 @@ pub trait GitRepository: Send + Sync {
         right: GitRepositoryCheckpoint,
     ) -> BoxFuture<Result<bool>>;
 
-    /// Deletes a previously-created checkpoint.
-    fn delete_checkpoint(&self, checkpoint: GitRepositoryCheckpoint) -> BoxFuture<Result<()>>;
-
     /// Computes a diff between two checkpoints.
     fn diff_checkpoints(
         &self,
@@ -374,7 +371,6 @@ impl RealGitRepository {
 
 #[derive(Clone, Debug)]
 pub struct GitRepositoryCheckpoint {
-    pub ref_name: String,
     pub commit_sha: Oid,
 }
 
@@ -1225,11 +1221,8 @@ impl GitRepository for RealGitRepository {
                     } else {
                         git.run(&["commit-tree", &tree, "-m", "Checkpoint"]).await?
                     };
-                    let ref_name = format!("refs/zed/{}", Uuid::new_v4());
-                    git.run(&["update-ref", &ref_name, &checkpoint_sha]).await?;
 
                     Ok(GitRepositoryCheckpoint {
-                        ref_name,
                         commit_sha: checkpoint_sha.parse()?,
                     })
                 })
@@ -1308,21 +1301,6 @@ impl GitRepository for RealGitRepository {
             .boxed()
     }
 
-    fn delete_checkpoint(&self, checkpoint: GitRepositoryCheckpoint) -> BoxFuture<Result<()>> {
-        let working_directory = self.working_directory();
-        let git_binary_path = self.git_binary_path.clone();
-
-        let executor = self.executor.clone();
-        self.executor
-            .spawn(async move {
-                let working_directory = working_directory?;
-                let git = GitBinary::new(git_binary_path, working_directory, executor);
-                git.run(&["update-ref", "-d", &checkpoint.ref_name]).await?;
-                Ok(())
-            })
-            .boxed()
-    }
-
     fn diff_checkpoints(
         &self,
         base_checkpoint: GitRepositoryCheckpoint,
@@ -1340,8 +1318,8 @@ impl GitRepository for RealGitRepository {
                     "diff",
                     "--find-renames",
                     "--patch",
-                    &base_checkpoint.ref_name,
+                    &base_checkpoint.commit_sha.to_string(),
-                    &target_checkpoint.ref_name,
+                    &target_checkpoint.commit_sha.to_string(),
                 ])
                 .await
             })
@@ -1414,6 +1392,15 @@ impl GitBinary {
             }
         });
 
+        // Copy the default index file so that Git doesn't have to rebuild the
+        // whole index from scratch. This might fail if this is an empty repository.
+        smol::fs::copy(
+            self.working_directory.join(".git").join("index"),
+            &index_file_path,
+        )
+        .await
+        .ok();
+
         self.index_file_path = Some(index_file_path.clone());
         let result = f(self).await;
         self.index_file_path = None;
@@ -1856,13 +1843,6 @@ mod tests {
                 .ok(),
             None
         );
-
-        // Garbage collecting after deleting a checkpoint makes it unreachable.
-        repo.delete_checkpoint(checkpoint.clone()).await.unwrap();
-        repo.gc().await.unwrap();
-        repo.restore_checkpoint(checkpoint.clone())
-            .await
-            .unwrap_err();
     }
 
     #[gpui::test]
@@ -1975,7 +1955,7 @@ mod tests {
                     let git_binary_path = git_binary_path.clone();
                     let working_directory = working_directory?;
                     let git = GitBinary::new(git_binary_path, working_directory, executor);
-                    git.run(&["gc", "--prune=now"]).await?;
+                    git.run(&["gc", "--prune"]).await?;
                     Ok(())
                 })
                 .boxed()

crates/project/src/git_store.rs

@@ -832,32 +832,6 @@ impl GitStore {
         })
     }
 
-    pub fn delete_checkpoint(
-        &self,
-        checkpoint: GitStoreCheckpoint,
-        cx: &mut App,
-    ) -> Task<Result<()>> {
-        let repositories_by_work_directory_abs_path = self
-            .repositories
-            .values()
-            .map(|repo| (repo.read(cx).snapshot.work_directory_abs_path.clone(), repo))
-            .collect::<HashMap<_, _>>();
-
-        let mut tasks = Vec::new();
-        for (work_dir_abs_path, checkpoint) in checkpoint.checkpoints_by_work_dir_abs_path {
-            if let Some(repository) =
-                repositories_by_work_directory_abs_path.get(&work_dir_abs_path)
-            {
-                let delete = repository.update(cx, |this, _| this.delete_checkpoint(checkpoint));
-                tasks.push(async move { delete.await? });
-            }
-        }
-        cx.background_spawn(async move {
-            future::try_join_all(tasks).await?;
-            Ok(())
-        })
-    }
-
     /// Blames a buffer.
     pub fn blame_buffer(
         &self,
@@ -3795,20 +3769,6 @@ impl Repository {
         })
     }
 
-    pub fn delete_checkpoint(
-        &mut self,
-        checkpoint: GitRepositoryCheckpoint,
-    ) -> oneshot::Receiver<Result<()>> {
-        self.send_job(None, move |repo, _cx| async move {
-            match repo {
-                RepositoryState::Local { backend, .. } => {
-                    backend.delete_checkpoint(checkpoint).await
-                }
-                RepositoryState::Remote { .. } => Err(anyhow!("not implemented yet")),
-            }
-        })
-    }
-
     pub fn diff_checkpoints(
         &mut self,
         base_checkpoint: GitRepositoryCheckpoint,