From c1ca3c54eaecb29dd5d6c882d2a5c5d47b2fd8ac Mon Sep 17 00:00:00 2001 From: Chris Bookholt Date: Wed, 1 Jul 2026 19:23:23 -0700 Subject: [PATCH] git-utils: preserve sentinel filter status checks --- codex-rs/git-utils/src/safe_git.rs | 94 +++++++++++++++++++++++- codex-rs/git-utils/src/safe_git_tests.rs | 48 ++++++++++++ 2 files changed, 138 insertions(+), 4 deletions(-) diff --git a/codex-rs/git-utils/src/safe_git.rs b/codex-rs/git-utils/src/safe_git.rs index 6ca208dd3f..11a4a034f5 100644 --- a/codex-rs/git-utils/src/safe_git.rs +++ b/codex-rs/git-utils/src/safe_git.rs @@ -137,12 +137,17 @@ pub(crate) async fn selected_executable_filter_from( ) -> Result)>, GitReadError> { let git_root = resolve_git_root_async(git, cwd).await?; let entries = read_filter_config_async(git, &git_root).await?; - if !entries.values().any(|entry| !entry.value.is_empty()) { + let executable_drivers = + executable_filter_drivers(&entries).map_err(|_| invalid_output("filterConfig"))?; + if executable_drivers.is_empty() { return Ok(None); } let paths = read_paths_async(git, &git_root, PathSelection::Tracked).await?; let attributes = read_filter_attributes_async(git, &git_root, &paths).await?; - selected_executable_filter(&entries, &attributes).map_err(|_| invalid_output("filterSelection")) + let attributes = + resolve_filter_attribute_sentinels_async(git, &git_root, attributes, &executable_drivers) + .await?; + Ok(selected_filter(&executable_drivers, &attributes)) } async fn resolve_git_root_async(git: &GitRunner, cwd: &Path) -> Result { @@ -492,7 +497,7 @@ async fn read_filter_attributes_async( git: &GitRunner, cwd: &Path, paths: &[Vec], -) -> Result, String>, GitReadError> { +) -> Result, FilterAttributeValue>, GitReadError> { if paths.is_empty() { return Ok(BTreeMap::new()); } @@ -526,6 +531,88 @@ async fn read_filter_attributes_async( parse_filter_attributes(&output.stdout, paths).map_err(|_| invalid_output("filterAttributes")) } +async fn resolve_filter_attribute_sentinels_async( + git: &GitRunner, + cwd: &Path, + attributes: BTreeMap, FilterAttributeValue>, + executable_drivers: &BTreeSet, +) -> Result, String>, GitReadError> { + let mut resolved = BTreeMap::new(); + for (path, attribute) in attributes { + match attribute { + FilterAttributeValue::Driver(driver) => { + resolved.insert(path, driver); + } + FilterAttributeValue::AmbiguousSentinel(driver) => { + if executable_drivers.contains(&driver) + && sentinel_spelling_selects_filter_driver_async(git, cwd, &path, &driver) + .await? + { + resolved.insert(path, driver); + } + } + } + } + Ok(resolved) +} + +async fn sentinel_spelling_selects_filter_driver_async( + git: &GitRunner, + cwd: &Path, + path: &[u8], + driver: &str, +) -> Result { + let required = + run_sentinel_selection_probe_async(git, cwd, path, driver, /*required*/ true).await?; + if required.status.success() { + return Ok(false); + } + let optional = + run_sentinel_selection_probe_async(git, cwd, path, driver, /*required*/ false).await?; + if optional.status.success() { + return Ok(true); + } + Err(command_failed( + "filterAttributeSelection", + optional.status.code(), + )) +} + +async fn run_sentinel_selection_probe_async( + git: &GitRunner, + cwd: &Path, + path: &[u8], + driver: &str, + required: bool, +) -> Result { + let path = git_path_argument(path).map_err(|_| invalid_output("filterAttributeSelection"))?; + let mut command = git.tokio_command(); + command + .env("GIT_OPTIONAL_LOCKS", "0") + .args([ + "-c", + &format!("core.hooksPath={DISABLED_HOOKS_PATH}"), + "-c", + "core.fsmonitor=false", + "-c", + &format!("filter.{driver}.required={required}"), + "-c", + &format!("filter.{driver}.clean="), + "-c", + &format!("filter.{driver}.smudge="), + "-c", + &format!("filter.{driver}.process="), + "hash-object", + "--stdin", + ]) + .arg("--path") + .arg(path) + .current_dir(cwd) + .stdin(Stdio::null()) + .kill_on_drop(true); + command_output(git, command, "filterAttributeSelection").await +} + async fn command_output( git: &GitRunner, command: TokioCommand, @@ -731,7 +818,6 @@ fn selected_executable_filter_for( Ok(selected_filter(&executable_drivers, attributes)) } -#[cfg(test)] fn selected_filter( drivers: &BTreeSet, attributes: &BTreeMap, String>, diff --git a/codex-rs/git-utils/src/safe_git_tests.rs b/codex-rs/git-utils/src/safe_git_tests.rs index bb95f8214d..77ceb82738 100644 --- a/codex-rs/git-utils/src/safe_git_tests.rs +++ b/codex-rs/git-utils/src/safe_git_tests.rs @@ -342,6 +342,54 @@ async fn get_has_changes_rejects_configured_clean_filter_without_running_it() { assert!(!configured_filter_ran(&repo_path).await); } +#[tokio::test] +async fn get_has_changes_distinguishes_filter_sentinels_from_literal_driver_names() { + for (driver, sentinel_attribute) in + [("set", "filter"), ("unset", "-filter"), ("unspecified", "")] + { + let temp_dir = tempfile::tempdir().expect("create temp dir"); + let repo_path = create_test_git_repo(&temp_dir).await; + let clean_key = format!("filter.{driver}.clean"); + run_git( + &repo_path, + &[ + "config", + &clean_key, + "git config codex.filterran true && git hash-object --stdin", + ], + ) + .await; + + let sentinel = if sentinel_attribute.is_empty() { + String::new() + } else { + format!("test.txt {sentinel_attribute}\n") + }; + std::fs::write(repo_path.join(".gitattributes"), sentinel) + .expect("write sentinel attribute"); + run_git(&repo_path, &["add", ".gitattributes"]).await; + run_git(&repo_path, &["commit", "-m", "sentinel attribute"]).await; + + assert_eq!(try_get_has_changes(&repo_path).await, Ok(false), "{driver}"); + assert!(!configured_filter_ran(&repo_path).await, "{driver}"); + + std::fs::write( + repo_path.join(".gitattributes"), + format!("test.txt filter={driver}\n"), + ) + .expect("write literal sentinel-named driver"); + assert_eq!( + try_get_has_changes(&repo_path).await, + Err(GitReadError::SelectedExecutableFilter { + driver: driver.to_string(), + path: "test.txt".to_string(), + }), + "{driver}" + ); + assert!(!configured_filter_ran(&repo_path).await, "{driver}"); + } +} + #[tokio::test] async fn checked_has_changes_distinguishes_non_repository() { let temp_dir = tempfile::tempdir().expect("create temp dir");