fix(index): match target include/exclude patterns as globs (#1072) (#1118)

`ResolvedTarget::globs_claim` decides whether a file is indexed, and the
matcher behind it recognized three shapes — a `**/*.ext` suffix, a `dir/**`
subtree, and a literal — then fell through to substring containment:

    path == pattern || path.contains(pattern.trim_matches('*'))

Containment is not glob matching. `*.rs` asked only whether `.rs` appeared
anywhere in the path, claiming `notes.rs.bak` and `src/lib.rs.orig` as Rust
sources; a bare `*` trims to the empty string, which every path contains, so
it claimed the whole tree; and a literal such as `README.md` claimed
`docs/README.md`. These are wrong answers about which files get indexed and
they fail silently.

Replace the cascade with `globset`, already compiled on every build as
`ignore`'s own matching layer, so the package graph and the audit/deny gates
are unchanged. Three options are pinned rather than defaulted, each because
the default is wrong for a repo-relative config pattern: `literal_separator`
(off by default, which would let `*` cross a `/`), `backslash_escape` (whose
default is platform-dependent, so one config would claim different files on
Unix and Windows), and building the candidate from the rendered bytes instead
of re-deriving it through a `Path`. Patterns compile once per process; a
pattern that is not a legal glob claims no files and is logged at `warn`.

A trailing `/` is normalized to `/**`, keeping `include = ["src/"]` meaning
the subtree — anchored now, where containment also matched `a/src/lib.rs`.

Behaviour change, documented in docs/config/targets.md: every shipped default
is a `**/*.ext` suffix and is unaffected, but a user pattern that relied on
the containment fallback now means what a glob says it means.

The `dir/**` separator boundary and the "one definition of target matching"
invariant recorded on `globs_claim` are restated for the new matcher: the
boundary is now correct by construction rather than a hand-written prefix
check.

Also pins the wildcard case of the git-status pathspec. A file NAME may
contain `*`, which gix reads as a wildcard, so a clean file was reported dirty
because a sibling matching the same pattern had edits — and blame then refused
the committed attribution. `:(literal)` already covered this alongside `\` and
a leading `:`; only the wildcard lacked a regression test.

Co-authored-by: Claude Opus 5 <[email protected]>

Author: Nitjsefnie

Cargo.lock

@@ -94,6 +94,10 @@ futures-util = { version = "0.3", default-features = false, features = ["alloc"]
 # `rand`/`rand_core` pulled in for this), keeping `from_seed` the sole key constructor.
 getrandom = "0.4"
 gix = { version = "0.86.0", default-features = false, features = ["status", "parallel", "sha1", "sha256", "revision", "blob-diff", "blame"] }
+# The glob matcher behind a target's include/exclude patterns. Already compiled on every build as
+# `ignore`'s own matching layer (the same one ripgrep uses), so depending on it directly adds
+# nothing to the lock file, the audit gate or the deny gate.
+globset = "0.4.19"
 httpdate = "1.0"
 libc = "0.2"
 # Win32 bindings for the Lens discovery credential's owner-only DACL. Unix keeps the token private

Cargo.toml

@@ -17,6 +17,7 @@ unicode-normalization.workspace = true
 anyhow.workspace = true
 dunce.workspace = true
 gix.workspace = true
+globset.workspace = true
 libc.workspace = true
 path-slash.workspace = true
 serde.workspace = true

crates/rag-rat-base/Cargo.toml

@@ -0,0 +1,148 @@
+//! The glob matcher behind a target's `include` / `exclude` patterns.
+//!
+//! **Real globs, via [`globset`].** This used to be a hand-written cascade over three shapes — a
+//! `**/*.ext` suffix, a `dir/**` subtree, and a literal — whose last line fell through to
+//! `path.contains(pattern.trim_matches('*'))`. Substring containment is not glob matching: `*.rs`
+//! became `contains(".rs")` and claimed `notes.rs.bak` and `src/lib.rs.orig`, and `*` (which trims
+//! to the empty string, contained by every path) claimed the whole tree. Those are wrong answers
+//! about which files are indexed, and they fail silently. [`globset`] is `ignore`'s own matching
+//! layer — already compiled on every build, and the matcher ripgrep uses for this exact job — so
+//! `**`, `*`, `?`, `[…]` and `{…}` now mean what a glob says they mean.
+//!
+//! Three options are pinned explicitly rather than taken from the defaults, because each default is
+//! wrong for a repo-relative config pattern:
+//!
+//! 1. **`literal_separator(true)`.** `globset` defaults it to *false*, where `*` and `?` match `/`
+//!    as well — so `src/*.rs` would claim `src/a/b/deep.rs`. A config pattern names path
+//!    components, so a wildcard must stop at a separator; only `**` crosses one.
+//! 2. **`backslash_escape(true)`.** `globset` defaults this to `!is_separator('\\')` — *true* on
+//!    Unix, *false* on Windows. Left unpinned, one `rag-rat.toml` would claim different files on
+//!    different platforms. Pinned on, `\` escapes the next character everywhere, so a pattern
+//!    reaches a Unix file whose NAME contains a backslash by spelling it `\\`.
+//! 3. **The candidate is built from the rendered BYTES**, never re-derived through a [`Path`]. The
+//!    caller already holds the `/`-separated rendering `files.path` is stored with
+//!    ([`crate::paths::path_string`]); round-tripping it through `Path` would re-run
+//!    platform-specific separator handling over a string that is already canonical.
+//!
+//! One shape is normalized before compiling rather than pinned as an option: a trailing `/` names a
+//! subtree, so `src/` compiles as `src/**` (see [`compile`]).
+//!
+//! [`Path`]: std::path::Path
+
+use std::collections::HashMap;
+use std::sync::{LazyLock, RwLock};
+
+use globset::{Candidate, GlobBuilder, GlobMatcher};
+
+/// Compiled matchers keyed by the pattern text, so a pattern is turned into a regex once per
+/// process instead of once per file.
+///
+/// [`super::ResolvedTarget::globs_claim`] is the per-file gate of the indexing walk and is also
+/// asked once per path (across every target) by the incremental resolver, so pattern compilation
+/// must not sit on that path. The pattern set is bounded by the config file, so the map needs no
+/// eviction. A pattern that fails to compile is memoized as [`None`] — the compile error is
+/// reported once, not once per file.
+static COMPILED: LazyLock<RwLock<HashMap<String, Option<GlobMatcher>>>> =
+    LazyLock::new(|| RwLock::new(HashMap::new()));
+
+/// Whether one config glob claims `path`, a `/`-separated repo-relative rendering.
+///
+/// A pattern that is not a legal glob (an unclosed `[`, a dangling `\`) claims NOTHING and is
+/// reported once. Config load does not reject such a pattern today, so the matcher has to answer
+/// something; claiming nothing keeps a typo from silently widening an `include`.
+pub(crate) fn pattern_claims(path: &str, pattern: &str) -> bool {
+    let candidate = Candidate::from_bytes(path.as_bytes());
+    let cached = COMPILED.read().ok().and_then(|compiled| {
+        compiled
+            .get(pattern)
+            .map(|matcher| matcher.as_ref().is_some_and(|m| m.is_match_candidate(&candidate)))
+    });
+    if let Some(claims) = cached {
+        return claims;
+    }
+    let matcher = compile(pattern);
+    let claims = matcher.as_ref().is_some_and(|m| m.is_match_candidate(&candidate));
+    if let Ok(mut compiled) = COMPILED.write() {
+        compiled.entry(pattern.to_string()).or_insert(matcher);
+    }
+    claims
+}
+
+/// Compile one pattern, or [`None`] (plus a warning) when it is not a legal glob.
+///
+/// A trailing `/` names a ROOT-ANCHORED SUBTREE, so `src/` compiles as `src/**`. As a bare glob it
+/// would name a single path ending in a separator and claim nothing at all — and a target whose
+/// `include` silently claims nothing indexes zero files, a worse and quieter failure than the
+/// over-claiming this replacement removes. This is rag-rat's own target dialect: close to but not
+/// identical to gitignore's trailing slash, which matches unanchored (a `src/` there would also
+/// match `a/src/`). Here `src/` is the ROOT `src/`, matching what the old substring fallback got
+/// *approximately* right — it matched `src/` by containment, but also claimed `a/src/lib.rs`.
+/// Normalizing to `src/**` keeps the intent and adds the anchor the fallback lacked.
+fn compile(pattern: &str) -> Option<GlobMatcher> {
+    let subtree = pattern.strip_suffix('/').map(|dir| format!("{dir}/**"));
+    let pattern = subtree.as_deref().unwrap_or(pattern);
+    match GlobBuilder::new(pattern).literal_separator(true).backslash_escape(true).build() {
+        Ok(glob) => Some(glob.compile_matcher()),
+        Err(error) => {
+            tracing::warn!(
+                pattern,
+                %error,
+                "target include/exclude pattern is not a valid glob; it claims no files",
+            );
+            None
+        },
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn a_wildcard_does_not_cross_a_separator() {
+        // `literal_separator` is OFF by default in globset, which would make `*` match `/`.
+        assert!(pattern_claims("src/lib.rs", "src/*.rs"));
+        assert!(!pattern_claims("src/a/deep.rs", "src/*.rs"));
+        assert!(pattern_claims("src/a/deep.rs", "src/**/*.rs"));
+    }
+
+    #[test]
+    fn a_backslash_escape_is_pinned_on_every_platform() {
+        // globset's default here is platform-dependent (`!is_separator('\\')`), so one config would
+        // otherwise claim different files on Unix and Windows. Pinned ON: `\\` is one literal `\`.
+        assert!(pattern_claims("drafts\\secret.md", "drafts\\\\secret.md"));
+        assert!(!pattern_claims("drafts/secret.md", "drafts\\\\secret.md"));
+    }
+
+    #[test]
+    fn a_trailing_slash_names_the_subtree_and_anchors_it() {
+        assert!(pattern_claims("src/lib.rs", "src/"));
+        assert!(pattern_claims("src/a/b/deep.rs", "src/"));
+        // The anchor the substring fallback lacked: `src/` is the ROOT `src/`, not any `src/`.
+        assert!(!pattern_claims("a/src/lib.rs", "src/"));
+        assert!(!pattern_claims("srcx/lib.rs", "src/"));
+        // …and the directory itself is not one of its own children.
+        assert!(!pattern_claims("src", "src/"));
+    }
+
+    #[test]
+    fn an_illegal_glob_claims_nothing_instead_of_everything() {
+        // A dangling escape is a compile error; the matcher must not fall back to "matches all".
+        assert!(!pattern_claims("src/lib.rs", "src/lib.rs\\"));
+        // The memoized failure answers the same way on the second call.
+        assert!(!pattern_claims("src/lib.rs", "src/lib.rs\\"));
+    }
+
+    #[test]
+    fn a_compiled_pattern_is_reused_across_calls() {
+        // Second call must come off the memo and agree with the first — the property the per-file
+        // hot path depends on.
+        let pattern = "**/*.reused-in-a-test";
+        assert!(pattern_claims("a/b.reused-in-a-test", pattern));
+        assert!(pattern_claims("a/b.reused-in-a-test", pattern));
+        assert!(
+            COMPILED.read().expect("cache lock").contains_key(pattern),
+            "the pattern must be compiled once and memoized",
+        );
+    }
+}

crates/rag-rat-base/src/config/globs.rs

@@ -5,6 +5,7 @@ use thiserror::Error;
 use crate::language::LanguageError;
 
 mod discovery;
+mod globs;
 mod load;
 mod raw;
 mod types;

crates/rag-rat-base/src/config/mod.rs

@@ -3359,3 +3359,113 @@ fn a_directory_include_glob_does_not_claim_a_backslash_named_sibling() {
     assert!(!target.globs_claim("foo\\bar.rs"), "a file NAMED `foo\\bar.rs` is not under foo/");
     assert!(!target.globs_claim("foobar.rs"), "a prefix of the NAME is not a directory boundary");
 }
+
+/// A target carrying exactly the given patterns; the language/kind are irrelevant to
+/// [`ResolvedTarget::globs_claim`], which reads only `include` and `exclude`.
+fn glob_target(include: &[&str], exclude: &[&str]) -> ResolvedTarget {
+    ResolvedTarget {
+        name: "globs".to_string(),
+        language: Language::Rust,
+        directories: vec![PathBuf::from(".")],
+        include: include.iter().map(|pattern| (*pattern).to_string()).collect(),
+        exclude: exclude.iter().map(|pattern| (*pattern).to_string()).collect(),
+        kind: TargetKind::Source,
+    }
+}
+
+/// A `*` is a WILDCARD over one path component, not a substring probe. The matcher used to fall
+/// through to `path.contains(pattern.trim_matches('*'))`, so `*.rs` asked only whether `.rs`
+/// appeared anywhere in the path — claiming a backup (`notes.rs.bak`) and a conflict leftover
+/// (`src/lib.rs.orig`) as Rust sources, and a wrong answer about what gets indexed.
+#[test]
+fn a_star_glob_matches_a_name_instead_of_containing_a_substring() {
+    let target = glob_target(&["*.rs"], &[]);
+
+    assert!(target.globs_claim("lib.rs"), "a root-level `.rs` file is claimed");
+    assert!(!target.globs_claim("notes.rs.bak"), "`.rs` inside the NAME is not a `.rs` file");
+    assert!(!target.globs_claim("src/lib.rs.orig"), "nor is a conflict leftover");
+    // A single `*` stops at a separator, so an unanchored `*.rs` is root-level only. `**/*.rs` (the
+    // shipped default) is the pattern that reaches every depth.
+    assert!(!target.globs_claim("src/lib.rs"), "one `*` does not cross a separator");
+    assert!(glob_target(&["**/*.rs"], &[]).globs_claim("src/lib.rs"), "`**/` does");
+}
+
+/// The same fallthrough made a bare `*` claim the entire tree: `"*".trim_matches('*')` is the empty
+/// string, and every path contains it. `*` is one component's worth of wildcard.
+#[test]
+fn a_bare_star_claims_one_component_not_the_whole_tree() {
+    let target = glob_target(&["*"], &[]);
+
+    assert!(target.globs_claim("README.md"), "a root-level file is claimed");
+    assert!(!target.globs_claim("src/lib.rs"), "a nested file is not");
+    assert!(!target.globs_claim("docs/deep/guide.md"), "nor a deeper one");
+    assert!(glob_target(&["**"], &[]).globs_claim("docs/deep/guide.md"), "`**` is the tree");
+}
+
+/// A pattern with no wildcard names a PATH, not a substring of one — on both sides. The old
+/// fallthrough let a literal claim any path it appeared inside, so an `exclude` of `vendor` also
+/// excluded `x/vendor/dep.rs` while an `include` of `src/lib.rs` also claimed `src/lib.rs.orig`.
+#[test]
+fn a_literal_pattern_names_a_path_not_a_substring_of_one() {
+    let include = glob_target(&["src/lib.rs", "README.md"], &[]);
+    assert!(include.globs_claim("src/lib.rs"), "the literal path itself is claimed");
+    assert!(!include.globs_claim("src/lib.rs.orig"), "a longer name is not that path");
+    assert!(!include.globs_claim("a/src/lib.rs"), "nor is the same tail deeper in the tree");
+    assert!(include.globs_claim("README.md"), "a root-level literal is claimed");
+    assert!(!include.globs_claim("docs/README.md"), "a same-named file elsewhere is not");
+
+    let exclude = glob_target(&["**/*.rs"], &["vendor"]);
+    assert!(
+        exclude.globs_claim("vendor/dep.rs"),
+        "`vendor` excludes the FILE `vendor`, not a tree"
+    );
+    assert!(exclude.globs_claim("x/vendor/dep.rs"), "and certainly not one nested elsewhere");
+    assert!(
+        !glob_target(&["**/*.rs"], &["vendor/**"]).globs_claim("vendor/dep.rs"),
+        "`vendor/**` is how a subtree is excluded",
+    );
+}
+
+/// The vocabulary is now real glob syntax, not the three hand-recognized shapes. Character classes,
+/// alternates, `?`, and a `**` in the MIDDLE of a pattern all used to fall through to substring
+/// containment over the pattern with its outer `*`s trimmed.
+#[test]
+fn the_full_glob_vocabulary_is_available() {
+    assert!(glob_target(&["**/*.[ch]"], &[]).globs_claim("include/lib.h"), "character class");
+    assert!(!glob_target(&["**/*.[ch]"], &[]).globs_claim("include/lib.hpp"), "and it is bounded");
+    assert!(glob_target(&["{lib,main}.rs"], &[]).globs_claim("main.rs"), "alternates");
+    assert!(!glob_target(&["{lib,main}.rs"], &[]).globs_claim("other.rs"), "and they are bounded");
+    assert!(glob_target(&["a?c.rs"], &[]).globs_claim("abc.rs"), "single-character wildcard");
+    assert!(!glob_target(&["a?c.rs"], &[]).globs_claim("ac.rs"), "which matches exactly one");
+    assert!(glob_target(&["src/**/*.rs"], &[]).globs_claim("src/a/b/deep.rs"), "interior `**`");
+    assert!(glob_target(&["src/**/*.rs"], &[]).globs_claim("src/lib.rs"), "which spans zero dirs");
+    assert!(
+        !glob_target(&["**/*.rs"], &["**/generated/**"]).globs_claim("src/generated/api.rs"),
+        "an interior `**` on the exclude side excludes a generated subtree at any depth",
+    );
+}
+
+/// Every shipped default is a `**/*.ext` suffix ([`Language::default_include_globs`]), the one
+/// shape the old cascade got right. Replacing the matcher must not move a single one of them.
+#[test]
+fn the_shipped_default_globs_claim_exactly_what_they_did() {
+    let target = glob_target(&["**/*.rs"], &[]);
+
+    for claimed in ["lib.rs", "src/lib.rs", "src/a/b/deep.rs", "foo\\bar.rs", ".rs"] {
+        assert!(target.globs_claim(claimed), "`**/*.rs` claims {claimed}");
+    }
+    for unclaimed in ["lib.h", "notes.rs.bak", "src/lib.rs.orig", "my.rs.template"] {
+        assert!(!target.globs_claim(unclaimed), "`**/*.rs` does not claim {unclaimed}");
+    }
+}
+
+/// Config load does not reject a malformed pattern, so the matcher has to answer something for one.
+/// It claims NOTHING: a typo that silently widened an `include` to the whole tree, or narrowed an
+/// `exclude` away, is the failure this whole function exists to stop.
+#[test]
+fn a_pattern_that_is_not_a_legal_glob_claims_nothing() {
+    // A dangling escape — `\` with nothing after it — is a globset compile error.
+    assert!(!glob_target(&["src/lib.rs\\"], &[]).globs_claim("src/lib.rs"));
+    // On the exclude side an uncompilable pattern excludes nothing, so the include still stands.
+    assert!(glob_target(&["**/*.rs"], &["oops\\"]).globs_claim("src/lib.rs"));
+}

crates/rag-rat-base/src/config/tests.rs

@@ -4,7 +4,7 @@ use std::time::Duration;
 
 use serde::{Deserialize, Serialize};
 
-use super::ConfigError;
+use super::{ConfigError, globs};
 use crate::embedding_models::{
     Backend, EmbeddingModelSpec, FASTEMBED_MODEL_ID, MODEL2VEC_MODEL_ID, spec,
 };
@@ -1098,30 +1098,21 @@ impl ResolvedTarget {
     ///
     /// The single definition of that matching, deliberately: the walk decides what to index and
     /// the per-path resolver decides what an already-known path belongs to, and a second copy of
-    /// the rules lets those two answers drift.
+    /// the rules lets those two answers drift. The patterns are real globs, matched by
+    /// [`globs::pattern_claims`] — the only place a target pattern is interpreted, and the only
+    /// place the glob dialect (see that module for the three pinned `globset` options) is decided.
+    ///
+    /// The boundary a `dir/**` prefix needs — it claims what is INSIDE `dir/`, so it must end at a
+    /// separator, never at a prefix of a NAME (`drafts/**` is not `draftsman.md`, and not the Unix
+    /// file named `drafts\secret.md`) — is now correct by construction: `globset` compiles a
+    /// trailing `/**` to a regex that requires the separator, and `*` cannot cross one at all.
+    /// Pinned by `a_directory_glob_needs_a_separator_after_its_prefix` and
+    /// `a_directory_include_glob_does_not_claim_a_backslash_named_sibling`, not by a hand-written
+    /// prefix check.
     pub fn globs_claim(&self, relative_path: &str) -> bool {
-        !self.exclude.iter().any(|pattern| glob_claims(relative_path, pattern))
-            && self.include.iter().any(|pattern| glob_claims(relative_path, pattern))
-    }
-}
-
-/// Whether one config glob claims `path` (a `/`-separated repo-relative rendering). Three shapes:
-/// a `**/*.ext` suffix, a `dir/**` subtree, and a literal (exact or substring).
-///
-/// A `dir/**` subtree requires the prefix to END AT A SEPARATOR. A bare `starts_with` reads a
-/// PREFIX OF A NAME as a directory boundary, so `drafts/**` also claimed the Unix file
-/// `drafts\secret.md` (and `draftsman.md`), excluding a file that is not in `drafts/` at all —
-/// which is exactly what preserving a literal backslash in the rendering exists to prevent.
-fn glob_claims(path: &str, pattern: &str) -> bool {
-    if let Some(extension) = pattern.strip_prefix("**/*.") {
-        return path.ends_with(&format!(".{extension}"));
-    }
-    if let Some(prefix) = pattern.strip_suffix("/**") {
-        return path
-            .strip_prefix(prefix)
-            .is_some_and(|rest| rest.is_empty() || rest.starts_with('/'));
+        !self.exclude.iter().any(|pattern| globs::pattern_claims(relative_path, pattern))
+            && self.include.iter().any(|pattern| globs::pattern_claims(relative_path, pattern))
     }
-    path == pattern || path.contains(pattern.trim_matches('*'))
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]