Handle missing blobs in diff snapshots

crates/prek/src/git.rs

@@ -390,9 +390,22 @@ pub(crate) async fn get_diff(path: &Path) -> Result<Vec<u8>, Error> {
         .arg("--ignore-submodules")
         .arg("--")
         .arg(path)
-        .check(true)
+        // This diff is only used as a best-effort before/after snapshot of
+        // hook changes. Some CI environments keep enough of `.git` for
+        // `git ls-files` but omit blob objects needed by `git diff`; Git then
+        // exits 128 on stderr with empty stdout. Keep comparing stdout in that
+        // case so `run --all-files` can still run against the files Git can
+        // enumerate.
+        .check(false)
         .output()
         .await?;
+    if !output.status.success() {
+        debug!(
+            status = %output.status,
+            stderr = %String::from_utf8_lossy(&output.stderr),
+            "Continuing with git diff stdout despite non-zero exit status"
+        );
+    }
     Ok(output.stdout)
 }
 

crates/prek/tests/run.rs

@@ -2824,15 +2824,16 @@ fn git_commit_a_currently_fails_when_hook_writes_to_temp_git_index() -> Result<(
     let context = TestContext::new();
     context.init_project();
 
-    // Repro for #1786 documenting the current behavior.
-    // `git commit -a` exports `GIT_INDEX_FILE=.git/index.lock` to the pre-commit hook
-    // process. If the hook inherits that env var and then runs a git command that writes
-    // to an index in a different repository, Git will write those entries into the parent
-    // repo's temporary index instead.
+    // Repro for #1786 documenting the current behavior. `git commit -a`
+    // exports `GIT_INDEX_FILE=.git/index.lock` to the hook process. If the
+    // hook inherits that env var and then runs a git command that writes to an
+    // index in a different repository, Git writes those entries into the
+    // parent repo's temporary index instead.
     //
     // The important detail is that the temp repo stages `file.txt`, matching a tracked
-    // path in the parent repo. That makes prek's post-hook `git diff` read the corrupted
-    // parent index entry and fail with `fatal: unable to read <hash>`.
+    // path in the parent repo. `prek` treats the post-hook diff as a best-effort
+    // snapshot, so the commit continues until Git tries to build trees from the
+    // corrupted temporary index and fails with `invalid object ... for 'file.txt'`.
     context
         .work_dir()
         .child("hook.sh")
@@ -2893,8 +2894,8 @@ fn git_commit_a_currently_fails_when_hook_writes_to_temp_git_index() -> Result<(
         .chain([
             (r"\[master \w{7}\]", r"[master COMMIT]"),
             (
-                r"fatal: unable to read [0-9a-f]{40}",
-                "fatal: unable to read [HASH]",
+                r"invalid object 100644 [0-9a-f]{40}",
+                "invalid object 100644 [HASH]",
             ),
         ])
         .collect::<Vec<_>>();
@@ -2905,13 +2906,11 @@ fn git_commit_a_currently_fails_when_hook_writes_to_temp_git_index() -> Result<(
     ----- stdout -----
 
     ----- stderr -----
-    error: Command `git diff` exited with an error:
-
-    [status]
-    exit status: 128
-
-    [stderr]
-    fatal: unable to read [HASH]
+    write-temp-index.........................................................Passed
+    - hook id: write-temp-index
+    - duration: [TIME]
+    error: invalid object 100644 [HASH] for 'file.txt'
+    error: Error building trees
     "
     );
 

crates/prek/tests/skipped_hooks.rs

@@ -7,10 +7,12 @@
 //! Includes regression tests for #1335: when all hooks in a group are skipped,
 //! prek should not call `git diff` to check for file modifications.
 
+use std::time::{Duration, SystemTime};
+
 use anyhow::Result;
 use assert_fs::prelude::*;
 
-use crate::common::{TestContext, cmd_snapshot};
+use crate::common::{TestContext, cmd_snapshot, git_cmd};
 
 mod common;
 
@@ -22,6 +24,20 @@ fn hook_env_count(context: &TestContext) -> Result<usize> {
     Ok(hooks_dir.read_dir()?.count())
 }
 
+fn remove_loose_blob(cwd: &assert_fs::fixture::ChildPath, filename: &str) -> Result<()> {
+    let output = git_cmd(cwd).arg("hash-object").arg(filename).output()?;
+    assert!(output.status.success(), "git hash-object should succeed");
+    let blob = String::from_utf8(output.stdout)?;
+    let blob = blob.trim_ascii();
+    let object_path = cwd
+        .child(".git")
+        .child("objects")
+        .child(&blob[..2])
+        .child(&blob[2..]);
+    fs_err::remove_file(object_path.path())?;
+    Ok(())
+}
+
 /// All hooks skip when no staged files match their file patterns.
 #[test]
 fn all_hooks_skipped_no_matching_files() -> Result<()> {
@@ -660,6 +676,70 @@ fn all_files_with_existing_unstaged_changes_uses_snapshot_baseline() -> Result<(
     Ok(())
 }
 
+#[test]
+fn all_files_clean_missing_blob_ignores_diff_snapshot_errors() -> Result<()> {
+    let context = TestContext::new();
+    context.init_project();
+
+    let cwd = context.work_dir();
+    context.write_pre_commit_config(indoc::indoc! {r#"
+        repos:
+          - repo: local
+            hooks:
+              - id: noop
+                name: noop
+                language: system
+                entry: python3 -c "pass"
+                pass_filenames: false
+    "#});
+
+    cwd.child("file.txt").write_str("original\n")?;
+    context.git_add(".");
+    context.git_commit("init");
+
+    remove_loose_blob(cwd, "file.txt")?;
+
+    // Make the index stat data stale while keeping file content unchanged. A
+    // full `git diff` now exits non-zero because the blob is missing, but its
+    // stdout is still a usable best-effort before/after snapshot.
+    fs_err::OpenOptions::new()
+        .write(true)
+        .open(cwd.child("file.txt").path())?
+        .set_modified(SystemTime::now() + Duration::from_secs(10))?;
+
+    let output = context
+        .run()
+        .arg("--all-files")
+        .env("RUST_LOG", "prek::git=trace")
+        .output()?;
+
+    assert!(
+        output.status.success(),
+        "`--all-files` should not require blob objects when hooks leave a clean tree.\n\
+         stdout:\n{}\n\
+         stderr:\n{}",
+        String::from_utf8_lossy(&output.stdout),
+        String::from_utf8_lossy(&output.stderr)
+    );
+
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    let ignored_diff_errors = stderr
+        .matches("Continuing with git diff stdout despite non-zero exit status")
+        .count();
+    assert_eq!(
+        ignored_diff_errors, 2,
+        "Expected before/after git diff errors to be logged and ignored, found {ignored_diff_errors}.\n\
+         Trace output:\n{stderr}"
+    );
+    assert!(
+        !stderr.contains("Command `git diff` exited with an error"),
+        "missing blobs should not turn hook modification detection into a fatal git diff error.\n\
+         stderr:\n{stderr}"
+    );
+
+    Ok(())
+}
+
 #[test]
 fn later_project_snapshots_diff_left_by_previous_project() -> Result<()> {
     let context = TestContext::new();