fix(vcs): refresh packed refs after file replacement (#1115)

Git Log could keep old commits and report 'unknown revision HEAD' after Git replaced packed-refs without a newer modification time: gitoxide's packed-ref snapshot only reloads for a newer mtime. The cached repository handle now also watches the common packed-refs file and compares modification time plus length before reuse; covered for the main repository and a linked worktree.

Co-authored-by: marianomd <[email protected]>

crates/ide-vcs/src/repo.rs

@@ -13,16 +13,27 @@ pub(crate) fn gx(e: impl std::fmt::Display) -> IdeError {
 
 pub(crate) struct Cache {
     repo: gix::ThreadSafeRepository,
-    config_files: [PathBuf; 3],
-    config_stamp: Vec<Option<SystemTime>>,
+    files: [PathBuf; 4],
+    stamp: Vec<Option<(SystemTime, u64)>>,
 }
 
-fn config_files(git_dir: &Path, common_dir: &Path) -> [PathBuf; 3] {
-    [common_dir.join("config"), git_dir.join("config.worktree"), common_dir.join("config.worktree")]
+fn cache_files(git_dir: &Path, common_dir: &Path) -> [PathBuf; 4] {
+    [
+        common_dir.join("config"),
+        git_dir.join("config.worktree"),
+        common_dir.join("config.worktree"),
+        common_dir.join("packed-refs"),
+    ]
 }
 
-fn stamp(files: &[PathBuf]) -> Vec<Option<SystemTime>> {
-    files.iter().map(|f| std::fs::metadata(f).and_then(|m| m.modified()).ok()).collect()
+fn stamp(files: &[PathBuf]) -> Vec<Option<(SystemTime, u64)>> {
+    files
+        .iter()
+        .map(|f| {
+            let metadata = std::fs::metadata(f).ok()?;
+            Some((metadata.modified().ok()?, metadata.len()))
+        })
+        .collect()
 }
 
 pub(crate) fn discover(path: &Path) -> IdeResult<Option<Repo>> {
@@ -45,8 +56,8 @@ pub(crate) fn discover(path: &Path) -> IdeResult<Option<Repo>> {
     let root = absolute(workdir)?;
     let git_dir = absolute(repo.git_dir())?;
     let common_dir = absolute(repo.common_dir())?;
-    let files = config_files(&git_dir, &common_dir);
-    let cache = Cache { config_stamp: stamp(&files), config_files: files, repo: ts };
+    let files = cache_files(&git_dir, &common_dir);
+    let cache = Cache { stamp: stamp(&files), files, repo: ts };
     Ok(Some(Repo {
         execution_allowed: std::sync::atomic::AtomicBool::new(true),
         root,
@@ -73,8 +84,8 @@ pub(crate) fn absolute(p: &Path) -> IdeResult<PathBuf> {
 }
 
 impl Repo {
-    /// A thread-local gitoxide handle; reopened when the git configuration changed
-    /// on disk (e.g. after `push --set-upstream` or `git config` in a terminal).
+    /// A thread-local gitoxide handle; reopened when configuration or packed refs change.
+    /// gitoxide's packed-ref snapshot only reloads for a newer mtime, which a replacement may preserve.
     pub(crate) fn gix(&self) -> IdeResult<gix::Repository> {
         let _scope = crate::ExecutionScope::enter(self.execution_allowed.load(std::sync::atomic::Ordering::Acquire));
         if !self.execution_allowed.load(std::sync::atomic::Ordering::Acquire) {
@@ -82,12 +93,12 @@ impl Repo {
                 .map_err(gx);
         }
         let mut cache = self.inner.lock().unwrap_or_else(|p| p.into_inner());
-        let now = stamp(&cache.config_files);
-        if now != cache.config_stamp {
+        let now = stamp(&cache.files);
+        if now != cache.stamp {
             match gix::ThreadSafeRepository::open(&self.git_dir) {
                 Ok(fresh) => {
                     cache.repo = fresh;
-                    cache.config_stamp = now;
+                    cache.stamp = now;
                 }
                 Err(e) => tracing::warn!("reopening repository failed: {e}"),
             }

crates/ide-vcs/tests/repo.rs

@@ -682,6 +682,8 @@ fn file_at_in_a_blobless_clone() {
     fx.write("f.txt", "v2\n");
     fx.commit_all("v2");
     fx.git(&["config", "uploadpack.allowFilter", "true"]);
+    // Git refuses a clone source with a local hooks-path override.
+    fx.git(&["config", "--unset", "core.hooksPath"]);
     let clone = fx.dir.parent().unwrap().join("clone");
     let url = format!("file://{}", fx.dir.display());
     git_in(fx.dir.parent().unwrap(), &["clone", "-q", "--filter=blob:none", &url, "clone"]);
@@ -1257,6 +1259,62 @@ fn errors_carry_git_messages() {
     assert!(repo.stash_apply(5, false, false).is_err());
 }
 
+#[test]
+fn packed_refs_reload_without_a_newer_modification_time() {
+    for keep_time in [false, true] {
+        for linked in [false, true] {
+            let fx = Fixture::new();
+            fx.write("file.txt", "one\n");
+            let first = fx.commit_all("one");
+            fx.git(&["branch", "-m", "master"]);
+            let root = if linked {
+                let path = fx._tmp.path().join("linked");
+                fx.git(&["worktree", "add", "-qb", "agent/check", path.to_str().unwrap()]);
+                path
+            } else {
+                fx.dir.clone()
+            };
+            let packed = fx.path(".git/packed-refs");
+            std::fs::write(
+                &packed,
+                format!("# pack-refs with: peeled fully-peeled sorted \n{first} refs/remotes/origin/master\n"),
+            )
+            .unwrap();
+            let set_modified = |time| {
+                std::fs::OpenOptions::new().write(true).open(&packed).unwrap().set_modified(time).unwrap();
+            };
+            set_modified(std::time::SystemTime::now() + std::time::Duration::from_secs(3600));
+            let original_time = std::fs::metadata(&packed).unwrap().modified().unwrap();
+            let original_len = std::fs::metadata(&packed).unwrap().len();
+            let repo = Repo::discover(&root).unwrap().unwrap();
+            let query = LogQuery { branches: vec!["HEAD".into()], limit: 1, ..Default::default() };
+            assert_eq!(repo.log(&query).unwrap()[0].id, first);
+
+            std::fs::write(root.join("file.txt"), "two\n").unwrap();
+            git_in(&root, &["add", "file.txt"]);
+            git_in(&root, &["commit", "-qm", "two"]);
+            let second = git_in(&root, &["rev-parse", "HEAD"]);
+            fx.git(&["pack-refs", "--all", "--prune"]);
+            set_modified(if keep_time { original_time } else { original_time - std::time::Duration::from_secs(60) });
+            assert_ne!(std::fs::metadata(&packed).unwrap().len(), original_len);
+            let branch = if linked { "agent/check" } else { "master" };
+            assert!(!fx.path(&format!(".git/refs/heads/{branch}")).exists());
+            for revision in ["HEAD", "@", branch] {
+                let q = LogQuery { branches: vec![revision.into()], ..query.clone() };
+                let rows = repo.log(&q).unwrap_or_else(|error| {
+                    panic!("linked={linked}, keep_time={keep_time}, revision={revision}: {error}")
+                });
+                assert_eq!(rows[0].id, second);
+            }
+            assert_eq!(repo.head().unwrap().1.as_deref(), Some(second.as_str()));
+            assert_eq!(
+                repo.file_at(Path::new("file.txt"), Some("HEAD"), encoding_rs::UTF_8).unwrap().as_deref(),
+                Some("two\n")
+            );
+        }
+    }
+}
+
 #[test]
 fn status_reflects_external_changes_without_reopen() {
     let fx = Fixture::new();