Repository navigation
fix(rust): match managed rg globs under ./-prefixed search paths - #229
Open
JayOfTheKeyboard wants to merge 1 commit into
Open
JayOfTheKeyboard wants to merge 1 commit into
JayOfTheKeyboard wants to merge 1 commit into
Conversation
Managed rg resolved relative search paths with root.join(path), so `.`, `./` and `./src` walked `<root>/./...`. The ignore crate strips the root from candidates as a byte prefix and matched `./src/a.rs`, so rules with a slash never matched: `-g 'src/**' needle .` returned nothing, and `-g '!docs/**'` or an --ignore-file pattern such as `docs/*.rs` excluded nothing. Drop `.` segments from relative walk roots, matching what ripgrep sees from the workspace root.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In the Rust
zg, managed--rg(and the MCPzvec_grep_rgtool, which calls the same engine method) ignores every glob or--ignore-filerule that contains a/when the search path starts with.. Agents append.torgcommands all the time. Include globs then return nothing, and exclusions are silently dropped.Measured on a 207-file Go repository, with
zgbuilt frommain(28ef200) and compared against ripgrep 14.1.1. Each row counts matchingpath:linepairs.zgonmainrgzgwith this PR--rg -g 'internal/**' budget(no path)--rg -g 'internal/**' budget .--rg -g 'internal/**' budget ./--rg -g 'internal/**' budget ./internal--rg -g 'internal/policy/*.go' budget .--rg -g '!internal/**' budget .--rg -g '!/internal/**' budget .--rg --ignore-file x.ignore budget .(file holdsinternal/policy/**)--rg --ignore-file x.ignore budget(no path)Over MCP (
zg --server --stdio --mcp-toolset full):With this PR, the second call returns the same results as the first.
Globs without a slash (
*.go,!*.md) are unaffected. So are paths written without./(internal,internal/) and the workspace's own.gitignorefiles.Cause
build_walkerresolves each search path withresolve_path, that isroot.join(path), so.becomes<root>/.and walk entries look like<root>/./internal/a.go. The override matcher is built withOverrideBuilder::new(root), andignore'sGitignore::stripremoves the root as a byte prefix and then one leading/. What reaches the glob is therefore./internal/a.go, whichinternal/**does not match. Rules from--ignore-filefail the same way. ripgrep never hits this, because it walks./internalfrom the current directory andstripdrops a leading./. The TypeScript implementation passes the paths torgunchanged, withcwd: root, so it matches ripgrep too.Change
Search paths given to the walker go through a new
resolve_match_path, which drops.segments from relative paths (root.join(path).components().collect()).resolve_pathitself is unchanged, socheck_paths,is_single_file_search, and the loading of pattern and ignore files behave as before, including how missing paths and trailing slashes are reported.Not changed
rg -g 'internal/**' budget "$PWD/."also matches nothing in ripgrep 14.1.1 (and!internal/**excludes nothing), so normalising absolute paths would makezgdiffer fromrg. Happy to normalise those too if you would rather fix it than match it.resolve_path. A./prefix on the ignore file (--ignore-file ./x.ignore) was never the problem, and the unit test passes with or without it. Only the search path matters.zg <query> -g ...) do not take search paths and are unaffected. I checked-g 'internal/**'and-g '!internal/**'against an index of the same repository.Test
path_rules_match_search_paths_with_a_current_directory_prefixinzg-engine/src/lexical/mod.rsruns eight combinations of search path and glob or--ignore-fileoversrc/keep.rsanddocs/drop.rs. It collects every mismatch and expects onlysrc/keep.rseach time. The two cases with no path are controls that pass onmain.On the parent commit, the other six cases fail:
Mutation check: removing
.components().collect()fails the same six cases, and so does pointingbuild_walkerback atresolve_path.Validation
cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings, andRUSTDOCFLAGS="-D warnings" cargo doc -p zg-engine --no-depspass.cargo test -p zg-engine --lib lexical::passes (16 tests, including the new one), and so doescargo test -p zg-engine --lib(461 passed, 8 ignored).cargo test --workspace --no-fail-fast(final state): 801 passed, 20 failed, 8 ignored. Every failure is in a test that runs a local mock HTTP server or the daemon. The same group fails onmainon this machine (795 passed, 26 failed), and which of those tests fail changes from run to run. Another process here probes newly opened loopback ports with a Go HTTP client, and the mocks reject its request (user-agent: go-http-client/1.1, noauthorizationheader). The daemon test that exercises managed rg,full_toolset_exposes_lifecycle_tools_and_runs_managed_rg, passed 3 of 3 runs on its own on this branch, and 2 of 3 onmain. I'm relying on CI for this group.path:linesets, not only the counts.