From 9facdccb374660effd99e26f6edbe1ca42bc656f Mon Sep 17 00:00:00 2001 From: Chris Bookholt Date: Fri, 15 May 2026 10:07:54 -0700 Subject: [PATCH] Ignore configured hooks in git helpers (#22843) ## What - Internal Git helper commands now ignore configured hook directories during repository bookkeeping. ## Why - These helper flows should stay consistent even when a repository has hook-directory configuration of its own. ## How - Pass a command-local `core.hooksPath` override in the shared helper path and the Git-info helper path. - Add regressions for the baseline index rewrite flow and the metadata status flow. ## Validation - `cargo fmt --manifest-path /Users/bookholt/code/codex/codex-rs/Cargo.toml --all --check` - `cargo test --manifest-path /Users/bookholt/code/codex/codex-rs/Cargo.toml -p codex-git-utils` - `cargo test --manifest-path /Users/bookholt/code/codex/codex-rs/Cargo.toml -p codex-core test_get_has_changes_` --- codex-rs/core/src/git_info_tests.rs | 44 +++++++++++++++++++++++++++ codex-rs/git-utils/src/baseline.rs | 45 ++++++++++++++++++++++++++++ codex-rs/git-utils/src/info.rs | 3 ++ codex-rs/git-utils/src/operations.rs | 9 +++++- 4 files changed, 100 insertions(+), 1 deletion(-) diff --git a/codex-rs/core/src/git_info_tests.rs b/codex-rs/core/src/git_info_tests.rs index b13db36d1e..af62816f72 100644 --- a/codex-rs/core/src/git_info_tests.rs +++ b/codex-rs/core/src/git_info_tests.rs @@ -379,6 +379,50 @@ async fn test_get_has_changes_ignores_repo_fsmonitor_config() { ); } +#[cfg(unix)] +#[tokio::test] +async fn test_get_has_changes_ignores_configured_hooks_path() { + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let repo_path = create_test_git_repo(&temp_dir).await; + let hooks_dir = repo_path.join(".git/hooks-path-test"); + let hook_path = hooks_dir.join("post-index-change"); + let marker_path = repo_path.join("hook-ran"); + + fs::create_dir_all(&hooks_dir).expect("create hook dir"); + fs::write( + &hook_path, + format!( + "#!/bin/sh\nprintf ran > \"{}\"\n", + marker_path.to_string_lossy() + ), + ) + .expect("write post-index-change hook"); + let mut permissions = fs::metadata(&hook_path) + .expect("read hook metadata") + .permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&hook_path, permissions).expect("mark hook executable"); + + Command::new("git") + .args([ + "config", + "core.hooksPath", + hooks_dir.to_string_lossy().as_ref(), + ]) + .current_dir(&repo_path) + .output() + .await + .expect("configure hooks path"); + + fs::write(repo_path.join("test.txt"), "test content").expect("refresh tracked file"); + + assert_eq!(get_has_changes(&repo_path).await, Some(false)); + assert!( + !marker_path.exists(), + "metadata collection should not invoke configured hook directories" + ); +} + #[tokio::test] async fn test_get_git_working_tree_state_clean_repo() { let temp_dir = TempDir::new().expect("Failed to create temp dir"); diff --git a/codex-rs/git-utils/src/baseline.rs b/codex-rs/git-utils/src/baseline.rs index c63b894049..083dd6010f 100644 --- a/codex-rs/git-utils/src/baseline.rs +++ b/codex-rs/git-utils/src/baseline.rs @@ -584,6 +584,51 @@ mod tests { assert_eq!(git_stdout(&root, &["ls-files"]), "MEMORY.md\n"); } + #[cfg(unix)] + #[tokio::test] + async fn write_index_ignores_configured_hooks_path() { + use std::os::unix::fs::PermissionsExt; + + let home = TempDir::new().expect("tempdir"); + let root = home.path().join("repo"); + let hooks_dir = root.join(".git/hooks-path-test"); + let marker_path = root.join("hook-ran"); + let hook_path = hooks_dir.join("post-index-change"); + + fs::create_dir_all(&root).expect("create root"); + fs::write(root.join("MEMORY.md"), "baseline").expect("write memory"); + reset_git_repository(&root).await.expect("reset repo"); + fs::create_dir_all(&hooks_dir).expect("create hook dir"); + fs::write( + &hook_path, + format!( + "#!/bin/sh\nprintf ran > \"{}\"\n", + marker_path.to_string_lossy() + ), + ) + .expect("write post-index-change hook"); + let mut permissions = fs::metadata(&hook_path) + .expect("read hook metadata") + .permissions(); + permissions.set_mode(0o755); + fs::set_permissions(&hook_path, permissions).expect("mark hook executable"); + git_stdout( + &root, + &[ + "config", + "core.hooksPath", + hooks_dir.to_string_lossy().as_ref(), + ], + ); + + write_index_from_head(&root).expect("rewrite baseline index"); + + assert!( + !marker_path.exists(), + "baseline index writes should not invoke configured hook directories" + ); + } + #[tokio::test] async fn diff_reports_added_modified_and_deleted_files() { let home = TempDir::new().expect("tempdir"); diff --git a/codex-rs/git-utils/src/info.rs b/codex-rs/git-utils/src/info.rs index 8c7c4046d4..722d556b37 100644 --- a/codex-rs/git-utils/src/info.rs +++ b/codex-rs/git-utils/src/info.rs @@ -40,6 +40,7 @@ pub fn get_git_repo_root(base_dir: &Path) -> Option { /// Timeout for git commands to prevent freezing on large repositories const GIT_COMMAND_TIMEOUT: TokioDuration = TokioDuration::from_secs(5); +const DISABLED_HOOKS_PATH: &str = if cfg!(windows) { "NUL" } else { "/dev/null" }; #[derive(Serialize, Deserialize, Clone, Debug, JsonSchema, TS)] pub struct GitInfo { @@ -375,6 +376,8 @@ async fn run_git_command_with_timeout(args: &[&str], cwd: &Path) -> Option Result<(), GitToolingError> { match run_git_for_stdout( path, @@ -98,7 +100,12 @@ where { let iterator = args.into_iter(); let (lower, upper) = iterator.size_hint(); - let mut args_vec = Vec::with_capacity(upper.unwrap_or(lower)); + let mut args_vec = Vec::with_capacity(upper.unwrap_or(lower) + 2); + // Keep internal Git helper commands independent of configured hook directories. + args_vec.push(OsString::from("-c")); + args_vec.push(OsString::from(format!( + "core.hooksPath={DISABLED_HOOKS_PATH}" + ))); for arg in iterator { args_vec.push(OsString::from(arg.as_ref())); }