git-utils: refine staging filter policy

codex-rs/git-utils/src/patch_paths.rs

@@ -9,7 +9,7 @@ use crate::apply::safe_git_config_parts;
 use crate::apply::write_temp_patch;
 use crate::git_command::GitRunner;
 use crate::git_config::path_is_within;
-use crate::safe_git::ensure_no_selected_executable_git_filters;
+use crate::safe_git::ensure_no_selected_git_add_filters;
 
 pub(crate) fn extract_effective_paths_from_patch(
     git: &GitRunner,
@@ -239,7 +239,6 @@ pub(crate) fn stage_effective_paths(
     git_root: &Path,
     paths: &[String],
 ) -> io::Result<()> {
-    ensure_no_selected_executable_git_filters(git, git_root, paths, &[])?;
     let confined = confine_patch_paths(git, git_root, paths)?;
     let mut existing = Vec::new();
     for path in confined.into_exact_leaves()? {
@@ -256,6 +255,7 @@ pub(crate) fn stage_effective_paths(
     if existing.is_empty() {
         return Ok(());
     }
+    ensure_no_selected_git_add_filters(git, git_root, &existing, &[])?;
     let mut args = vec![
         "--literal-pathspecs".to_string(),
         "add".to_string(),

codex-rs/git-utils/src/patch_paths_tests.rs

@@ -427,6 +427,31 @@ fn commit_seed(root: &Path) {
     assert_eq!(run(root, &["git", "commit", "-m", "seed"]).0, 0);
 }
 
+fn init_repo_with_selected_filter(config_key: &str) -> tempfile::TempDir {
+    let repo = init_repo();
+    let root = repo.path();
+    std::fs::write(root.join(".gitattributes"), "file.txt filter=selected\n")
+        .expect("write attributes");
+    std::fs::write(root.join("file.txt"), "old\n").expect("write tracked file");
+    assert_eq!(run(root, &["git", "add", "."]).0, 0);
+    assert_eq!(run(root, &["git", "commit", "-m", "base"]).0, 0);
+    assert_eq!(
+        run(
+            root,
+            &[
+                "git",
+                "config",
+                config_key,
+                "codex-definitely-missing-filter-command",
+            ],
+        )
+        .0,
+        0
+    );
+    std::fs::write(root.join("file.txt"), "new\n").expect("modify tracked file");
+    repo
+}
+
 #[cfg(any(unix, windows))]
 fn create_dir_alias(target: &Path, alias: &Path) -> DirectoryAlias {
     #[cfg(unix)]
@@ -787,3 +812,109 @@ fn staging_rejects_global_lfs_filter_without_running_it() {
     assert_eq!(diff_code, 0, "read staged paths: {diff_err}");
     assert!(staged.is_empty(), "staging changed the index: {staged}");
 }
+
+#[test]
+fn staging_rejects_selected_process_but_allows_smudge_only_filter() {
+    let process_repo = init_repo_with_selected_filter("filter.selected.process");
+    let process_root = process_repo.path();
+    let process_git = GitRunner::for_cwd_io(process_root).expect("trusted Git");
+    let error = stage_effective_paths(&process_git, process_root, &["file.txt".to_string()])
+        .expect_err("reject selected process filter");
+    assert_eq!(error.kind(), io::ErrorKind::Unsupported);
+
+    let smudge_repo = init_repo_with_selected_filter("filter.selected.smudge");
+    let smudge_root = smudge_repo.path();
+    let smudge_git = GitRunner::for_cwd_io(smudge_root).expect("trusted Git");
+    stage_effective_paths(&smudge_git, smudge_root, &["file.txt".to_string()])
+        .expect("smudge-only filters do not run during staging");
+    let (code, staged, stderr) = run(smudge_root, &["git", "diff", "--cached", "--name-only"]);
+    assert_eq!(code, 0, "read staged paths: {stderr}");
+    assert_eq!(staged, "file.txt\n");
+}
+
+#[test]
+fn staging_filter_probe_uses_only_existing_non_directory_leaves() {
+    let repo = init_repo();
+    let root = repo.path();
+    std::fs::write(
+        root.join(".gitattributes"),
+        "missing.txt filter=selected\ndeleted.txt filter=selected\n",
+    )
+    .expect("write attributes");
+    std::fs::write(root.join("deleted.txt"), "old\n").expect("write deleted fixture");
+    std::fs::write(root.join("kept.txt"), "old\n").expect("write kept fixture");
+    assert_eq!(run(root, &["git", "add", "."]).0, 0);
+    assert_eq!(run(root, &["git", "commit", "-m", "base"]).0, 0);
+    assert_eq!(
+        run(
+            root,
+            &[
+                "git",
+                "config",
+                "filter.selected.clean",
+                "codex-definitely-missing-filter-command",
+            ],
+        )
+        .0,
+        0
+    );
+    std::fs::remove_file(root.join("deleted.txt")).expect("delete tracked file");
+    std::fs::write(root.join("kept.txt"), "new\n").expect("modify kept fixture");
+
+    let git = GitRunner::for_cwd_io(root).expect("trusted Git");
+    stage_effective_paths(
+        &git,
+        root,
+        &[
+            "missing.txt".to_string(),
+            "deleted.txt".to_string(),
+            "kept.txt".to_string(),
+        ],
+    )
+    .expect("probe only the surviving staging leaf");
+
+    let (code, staged, stderr) = run(root, &["git", "diff", "--cached", "--name-only"]);
+    assert_eq!(code, 0, "read staged paths: {stderr}");
+    assert_eq!(staged, "kept.txt\n");
+    assert!(!root.join("missing.txt").exists());
+    assert!(!root.join("deleted.txt").exists());
+}
+
+#[test]
+fn staging_skips_filter_probe_when_no_leaf_will_be_staged() {
+    let repo = init_repo();
+    let root = repo.path();
+    std::fs::write(
+        root.join(".gitattributes"),
+        "missing.txt filter=selected\ndeleted.txt filter=selected\n",
+    )
+    .expect("write attributes");
+    std::fs::write(root.join("deleted.txt"), "old\n").expect("write deleted fixture");
+    assert_eq!(run(root, &["git", "add", "."]).0, 0);
+    assert_eq!(run(root, &["git", "commit", "-m", "base"]).0, 0);
+    assert_eq!(
+        run(
+            root,
+            &[
+                "git",
+                "config",
+                "filter.selected.clean",
+                "codex-definitely-missing-filter-command",
+            ],
+        )
+        .0,
+        0
+    );
+    std::fs::remove_file(root.join("deleted.txt")).expect("delete tracked file");
+    let before = git_index_bytes(root);
+
+    let git = GitRunner::for_cwd_io(root).expect("trusted Git");
+    stage_effective_paths(
+        &git,
+        root,
+        &["missing.txt".to_string(), "deleted.txt".to_string()],
+    )
+    .expect("skip staging when every effective leaf is absent");
+
+    assert_eq!(git_index_bytes(root), before);
+}

codex-rs/git-utils/src/safe_git.rs

@@ -47,6 +47,37 @@ pub(crate) fn ensure_no_selected_executable_git_filters(
     cwd: &Path,
     paths: &[String],
     git_config_args: &[String],
+) -> io::Result<()> {
+    ensure_no_selected_executable_git_filters_for(
+        git,
+        cwd,
+        paths,
+        git_config_args,
+        FilterExecution::AnyWorktreeOperation,
+    )
+}
+
+pub(crate) fn ensure_no_selected_git_add_filters(
+    git: &GitRunner,
+    cwd: &Path,
+    paths: &[String],
+    git_config_args: &[String],
+) -> io::Result<()> {
+    ensure_no_selected_executable_git_filters_for(
+        git,
+        cwd,
+        paths,
+        git_config_args,
+        FilterExecution::GitAdd,
+    )
+}
+
+fn ensure_no_selected_executable_git_filters_for(
+    git: &GitRunner,
+    cwd: &Path,
+    paths: &[String],
+    git_config_args: &[String],
+    execution: FilterExecution,
 ) -> io::Result<()> {
     let entries = read_filter_config(git, cwd, git_config_args)?;
     if !entries.values().any(|entry| !entry.value.is_empty()) {
@@ -57,7 +88,8 @@ pub(crate) fn ensure_no_selected_executable_git_filters(
         .map(|path| path.as_bytes().to_vec())
         .collect::<Vec<_>>();
     let attributes = read_filter_attributes(git, cwd, &paths, git_config_args)?;
-    if let Some((driver, path)) = selected_executable_filter(&entries, &attributes)? {
+    if let Some((driver, path)) = selected_executable_filter_for(&entries, &attributes, execution)?
+    {
         return Err(io::Error::new(
             io::ErrorKind::Unsupported,
             format!(
@@ -69,6 +101,19 @@ pub(crate) fn ensure_no_selected_executable_git_filters(
     Ok(())
 }
 
+#[derive(Clone, Copy)]
+enum FilterExecution {
+    AnyWorktreeOperation,
+    GitAdd,
+}
+
+#[derive(Clone, Copy, PartialEq, Eq)]
+enum FilterCommand {
+    Clean,
+    Smudge,
+    Process,
+}
+
 fn read_filter_config(
     git: &GitRunner,
     cwd: &Path,
@@ -182,14 +227,19 @@ fn read_filter_attributes(
     parse_filter_attributes(&output.stdout, paths)
 }
 
-fn selected_executable_filter(
+fn selected_executable_filter_for(
     entries: &BTreeMap<String, GitConfigEntry>,
     attributes: &BTreeMap<Vec<u8>, String>,
+    execution: FilterExecution,
 ) -> io::Result<Option<(String, Vec<u8>)>> {
     let mut executable_drivers = BTreeSet::new();
     for entry in entries.values() {
-        let driver = filter_driver_name(&entry.key)?;
-        if !entry.value.is_empty() {
+        let (driver, command) = filter_driver_and_command(&entry.key)?;
+        let relevant = match execution {
+            FilterExecution::AnyWorktreeOperation => true,
+            FilterExecution::GitAdd => command != FilterCommand::Smudge,
+        };
+        if relevant && !entry.value.is_empty() {
             executable_drivers.insert(driver);
         }
     }
@@ -201,16 +251,37 @@ fn selected_executable_filter(
     Ok(None)
 }
 
+#[cfg(test)]
+fn selected_executable_filter(
+    entries: &BTreeMap<String, GitConfigEntry>,
+    attributes: &BTreeMap<Vec<u8>, String>,
+) -> io::Result<Option<(String, Vec<u8>)>> {
+    selected_executable_filter_for(entries, attributes, FilterExecution::AnyWorktreeOperation)
+}
+
+#[cfg(test)]
 fn filter_driver_name(key: &str) -> io::Result<String> {
+    filter_driver_and_command(key).map(|(driver, _command)| driver)
+}
+
+fn filter_driver_and_command(key: &str) -> io::Result<(String, FilterCommand)> {
     let Some(remainder) = key.strip_prefix("filter.") else {
         return Err(invalid_filter_output("malformed filter config key"));
     };
-    let driver = [".clean", ".smudge", ".process"]
-        .into_iter()
-        .find_map(|suffix| remainder.strip_suffix(suffix))
-        .filter(|driver| !driver.is_empty())
-        .ok_or_else(|| invalid_filter_output("malformed filter config key"))?;
-    Ok(driver.to_string())
+    let (driver, command) = [
+        (".clean", FilterCommand::Clean),
+        (".smudge", FilterCommand::Smudge),
+        (".process", FilterCommand::Process),
+    ]
+    .into_iter()
+    .find_map(|(suffix, command)| {
+        remainder
+            .strip_suffix(suffix)
+            .map(|driver| (driver, command))
+    })
+    .filter(|(driver, _command)| !driver.is_empty())
+    .ok_or_else(|| invalid_filter_output("malformed filter config key"))?;
+    Ok((driver.to_string(), command))
 }
 
 fn write_nul_paths(input: &mut std::fs::File, paths: &[Vec<u8>]) -> io::Result<()> {

codex-rs/git-utils/src/safe_git_tests.rs

@@ -66,6 +66,30 @@ fn selected_filter_policy_allows_effective_empty_value() {
     );
 }
 
+#[test]
+fn git_add_filter_policy_rejects_clean_and_process_but_allows_smudge_only() {
+    let selected = BTreeMap::from([(b"file.txt".to_vec(), "demo".to_string())]);
+    for (key, rejected) in [
+        ("filter.demo.clean", true),
+        ("filter.demo.smudge", false),
+        ("filter.demo.process", true),
+    ] {
+        let entries = filter_entries(
+            GitConfigScope::Local,
+            Path::new(".git/config"),
+            key,
+            "codex-definitely-missing-filter-command",
+        );
+        assert_eq!(
+            selected_executable_filter_for(&entries, &selected, FilterExecution::GitAdd)
+                .expect("Git add filter policy")
+                .is_some(),
+            rejected,
+            "{key}"
+        );
+    }
+}
+
 #[test]
 fn filter_attribute_parser_rejects_malformed_or_unexpected_records() {
     let paths = vec![b"a.txt".to_vec(), b"b.txt".to_vec()];