fix(agent-core): prevent glob searches from surfacing ignored files#1280
fix(agent-core): prevent glob searches from surfacing ignored files#1280morluto wants to merge 4 commits into
Conversation
🦋 Changeset detectedLatest commit: 47b17e8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
morluto
force-pushed
the
fix/glob-ignore-files-clean
branch
from
abbbc65 to
da812a8
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abbbc6532a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (normalizedPattern.startsWith('./')) { | ||
| normalizedPattern = normalizedPattern.slice(2); |
There was a problem hiding this comment.
When callers pass a pattern such as ./src/*.ts, this strips the ./ and returns src/a.ts; however rg --help says --glob uses .gitignore glob rules, and verified rg --files --glob './src/*.ts' . returns no matches for src/a.ts while src/*.ts does. Because this tool is otherwise trying to preserve ripgrep-style glob semantics while moving filtering in-process, normalizing away ./ broadens these patterns and can surface files the user did not actually match.
Useful? React with 👍 / 👎.
| const fn = picomatch(escapedPattern, opts); | ||
| matcher = (relPath: string) => fn(relPath.split('/').pop()!); | ||
| } else { | ||
| const fn = picomatch(escapedPattern, opts); | ||
| matcher = (relPath: string) => fn(relPath); |
There was a problem hiding this comment.
When a directory contains a file literally named like a brace glob, e.g. {a,b}.ts, this matcher includes it for pattern: "{a,b}.ts" because picomatch treats an exact input equal to the glob string as a match in addition to the expanded alternatives. I checked the CLI behavior (rg --help says --glob uses .gitignore globs, and rg --files --glob '{a,b}.ts' . returns only a.ts/b.ts), so this in-process filtering can now surface false positives for brace-expansion searches.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47b17e8769
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Related Issue
Resolves #1244
Problem
Globdocuments that it respects.gitignore,.ignore, and.rgignoreby default, but two ripgrep behaviors made that false in common cases.First, passing the user's pattern as a positive
--globfilter can re-include files that ignore rules would otherwise exclude. That means a search likeGlob({ pattern: "*.ts" })could return ignored files even when.gitignoreexcludes*.ts.Second, ripgrep only applies
.gitignorefiles outside git repositories when--no-require-gitis set. SinceGlobdid not pass that flag, plain project directories without.gitdid not honor.gitignore.What changed
rg --filesenumerate non-ignored files first instead of passing user patterns as positive--globfilters.--no-require-gitfor non-git search roots..gitignorebehavior in both git and non-git directories.Reference Patterns
This follows two useful reference patterns:
.gitand uses--no-require-gitoutside repos, which makes.gitignorework in normal non-git folders.Kimi Code's implementation combines those patterns while preserving existing sensitive-file exclusions and specific glob matching.
Validation
pnpm --filter @moonshot-ai/agent-core exec vitest run test/tools/glob.test.tspnpm exec oxlint packages/agent-core/src/tools/builtin/file/glob.ts packages/agent-core/src/tools/support/run-rg.ts packages/agent-core/test/tools/glob.test.tsChecklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update. Existing docs already describe the intended ignore-file behavior; this PR restores that behavior.