From 460a58639ae5aba84f97c24f8ba957d25b64db94 Mon Sep 17 00:00:00 2001 From: Dudu-0223 Date: Thu, 9 Jul 2026 21:47:39 +0800 Subject: [PATCH] fix: address codex review round 3 glob leaked VCS internals when the model rooted the search AT a VCS directory (path: '.git' or 'sub/.git'): the prune glob !**/.git is matched against root-prefixed candidate paths, which never end in the directory name when the walk starts inside it. Pair each VCS exclude with a contents glob (!**//**), verified empirically to exclude relative, nested, and absolute VCS roots while leaving broad searches untouched. Pinned by the command-construction test and a real-rg integration case rooting at .git. --- packages/fs/tool-fs-search/src/glob.ts | 18 ++++++++++++++---- .../tool-fs-search/tests/integration.spec.ts | 6 ++++++ packages/fs/tool-fs-search/tests/tools.spec.ts | 6 ++++-- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/packages/fs/tool-fs-search/src/glob.ts b/packages/fs/tool-fs-search/src/glob.ts index e565699e2c..09a3e1d9ce 100644 --- a/packages/fs/tool-fs-search/src/glob.ts +++ b/packages/fs/tool-fs-search/src/glob.ts @@ -29,9 +29,12 @@ export const GLOB_MAX_RESULTS = 100 /** * Directory names ripgrep must never descend into for a discovery listing: VCS * metadata stores. `--no-ignore --hidden` would otherwise surface them in every - * broad search. Each is excluded with a negated any-depth `--glob` (see - * {@link buildGlobCommand}), which matches — and prunes — the directory - * wherever it appears. + * broad search. Each name is excluded with TWO negated `--glob`s (see + * {@link buildGlobCommand}): an any-depth directory glob that matches — and + * prunes — the directory during traversal, and a contents glob that still + * excludes the internals when the search root itself is at or inside the + * directory (an explicit `path` of `.git` or `sub/.git`), where the prune glob + * alone never matches. */ export const GLOB_VCS_EXCLUDES: readonly string[] = ['.git', '.svn', '.hg', '.bzr', '.jj', '.sl'] @@ -81,7 +84,14 @@ export function buildGlobCommand(input: GlobInput): string { 'rg --files', `--glob=${singleQuote(input.pattern)}`, '--sort=modified --no-ignore --hidden', - ...GLOB_VCS_EXCLUDES.map(name => `--glob=${singleQuote(`!**/${name}`)}`), + // Two negated globs per VCS name: the bare form prunes the directory + // during traversal; the /** form still excludes the contents when the + // search root is AT or INSIDE the directory (where the bare form, + // matched against root-prefixed paths, never fires). + ...GLOB_VCS_EXCLUDES.flatMap(name => [ + `--glob=${singleQuote(`!**/${name}`)}`, + `--glob=${singleQuote(`!**/${name}/**`)}`, + ]), ] if (input.path !== undefined) parts.push('--', singleQuote(input.path)) return parts.join(' ') diff --git a/packages/fs/tool-fs-search/tests/integration.spec.ts b/packages/fs/tool-fs-search/tests/integration.spec.ts index 481d3af620..36fb2c28e6 100644 --- a/packages/fs/tool-fs-search/tests/integration.spec.ts +++ b/packages/fs/tool-fs-search/tests/integration.spec.ts @@ -87,6 +87,12 @@ describe.skipIf(!hasRg)('search tools over the real bash executor + real rg', () expect(text(await call('glob', { pattern: '*.nomatch' }))).toBe('No files found') }) + it('excludes VCS internals even when the search root IS the VCS directory', async () => { + // The prune glob alone never matches root-prefixed paths when rg is + // rooted at .git; the paired contents glob keeps the exclusion airtight. + expect(text(await call('glob', { pattern: '*', path: '.git' }))).toBe('No files found') + }) + it('classifies an invalid glob as SEARCH_INVALID_PATTERN', async () => { const result = await call('glob', { pattern: '[' }) expect(result.isError).toBe(true) diff --git a/packages/fs/tool-fs-search/tests/tools.spec.ts b/packages/fs/tool-fs-search/tests/tools.spec.ts index 6afca54efd..a3a3e89cce 100644 --- a/packages/fs/tool-fs-search/tests/tools.spec.ts +++ b/packages/fs/tool-fs-search/tests/tools.spec.ts @@ -204,11 +204,13 @@ describe('config validation', () => { }) describe('command construction (shell-safe)', () => { - it('glob: fixed rg --files template with quoted pattern and VCS excludes', () => { + it('glob: fixed rg --files template with quoted pattern and paired VCS excludes', () => { const command = buildGlobCommand({ pattern: '**/*.ts' }) expect(command).toBe( "rg --files --glob='**/*.ts' --sort=modified --no-ignore --hidden " - + "--glob='!**/.git' --glob='!**/.svn' --glob='!**/.hg' --glob='!**/.bzr' --glob='!**/.jj' --glob='!**/.sl'", + + "--glob='!**/.git' --glob='!**/.git/**' --glob='!**/.svn' --glob='!**/.svn/**' " + + "--glob='!**/.hg' --glob='!**/.hg/**' --glob='!**/.bzr' --glob='!**/.bzr/**' " + + "--glob='!**/.jj' --glob='!**/.jj/**' --glob='!**/.sl' --glob='!**/.sl/**'", ) })