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 (!**/<name>/**), 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.
This commit is contained in:
@@ -29,9 +29,12 @@ export const GLOB_MAX_RESULTS = 100
|
|||||||
/**
|
/**
|
||||||
* Directory names ripgrep must never descend into for a discovery listing: VCS
|
* Directory names ripgrep must never descend into for a discovery listing: VCS
|
||||||
* metadata stores. `--no-ignore --hidden` would otherwise surface them in every
|
* metadata stores. `--no-ignore --hidden` would otherwise surface them in every
|
||||||
* broad search. Each is excluded with a negated any-depth `--glob` (see
|
* broad search. Each name is excluded with TWO negated `--glob`s (see
|
||||||
* {@link buildGlobCommand}), which matches — and prunes — the directory
|
* {@link buildGlobCommand}): an any-depth directory glob that matches — and
|
||||||
* wherever it appears.
|
* 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']
|
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',
|
'rg --files',
|
||||||
`--glob=${singleQuote(input.pattern)}`,
|
`--glob=${singleQuote(input.pattern)}`,
|
||||||
'--sort=modified --no-ignore --hidden',
|
'--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))
|
if (input.path !== undefined) parts.push('--', singleQuote(input.path))
|
||||||
return parts.join(' ')
|
return parts.join(' ')
|
||||||
|
|||||||
@@ -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')
|
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 () => {
|
it('classifies an invalid glob as SEARCH_INVALID_PATTERN', async () => {
|
||||||
const result = await call('glob', { pattern: '[' })
|
const result = await call('glob', { pattern: '[' })
|
||||||
expect(result.isError).toBe(true)
|
expect(result.isError).toBe(true)
|
||||||
|
|||||||
@@ -204,11 +204,13 @@ describe('config validation', () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe('command construction (shell-safe)', () => {
|
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' })
|
const command = buildGlobCommand({ pattern: '**/*.ts' })
|
||||||
expect(command).toBe(
|
expect(command).toBe(
|
||||||
"rg --files --glob='**/*.ts' --sort=modified --no-ignore --hidden "
|
"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/**'",
|
||||||
)
|
)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user