Skip to content

Commit a59ca7b

Browse files
roomoteroomote
committed
fix: always exclude .git directories from codebase indexing
- Added explicit .git exclusion in ripgrep arguments for both recursive and non-recursive modes - This prevents indexing of .git folder contents which improves performance - Added comprehensive tests to verify .git exclusion in various scenarios Fixes #9048
1 parent 6965e5c commit a59ca7b

File tree

2 files changed

+232
-1
lines changed

2 files changed

+232
-1
lines changed

‎src/services/glob/__tests__/list-files-git-exclusion.spec.ts‎

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
// npx vitest src/services/glob/__tests__/list-files-git-exclusion.spec.ts
2+
3+
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
4+
import * as childProcess from "child_process"
5+
import * as path from "path"
6+
import * as os from "os"
7+
import * as fs from "fs"
8+
import { listFiles } from "../list-files"
9+
10+
// Mock child_process module
11+
vi.mock("child_process", () => ({
12+
spawn: vi.fn(),
13+
}))
14+
15+
// Mock dependencies
16+
vi.mock("../../ripgrep", () => ({
17+
getBinPath: vi.fn(async () => "/usr/bin/rg"),
18+
}))
19+
20+
vi.mock("vscode", () => ({
21+
env: {
22+
appRoot: "/test/app/root",
23+
},
24+
workspace: {
25+
getConfiguration: vi.fn(() => ({
26+
get: vi.fn(() => undefined),
27+
})),
28+
},
29+
}))
30+
31+
describe("list-files .git exclusion", () => {
32+
let tempDir: string
33+
let originalCwd: string
34+
35+
beforeEach(async () => {
36+
// Create a temporary directory for testing
37+
tempDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "roo-git-exclusion-test-"))
38+
originalCwd = process.cwd()
39+
40+
// Mock fs.promises.access to simulate files exist
41+
vi.spyOn(fs.promises, "access").mockResolvedValue(undefined)
42+
vi.spyOn(fs.promises, "readdir").mockImplementation(async () => [])
43+
})
44+
45+
afterEach(async () => {
46+
// Clean up temporary directory
47+
await fs.promises.rm(tempDir, { recursive: true, force: true })
48+
vi.restoreAllMocks()
49+
})
50+
51+
it("should always exclude .git directories in recursive mode", async () => {
52+
// Mock ripgrep spawn
53+
const mockSpawn = vi.mocked(childProcess.spawn)
54+
mockSpawn.mockImplementation((command: string, args: readonly string[]) => {
55+
const mockProcess = {
56+
stdout: {
57+
on: (event: string, callback: (data: any) => void) => {
58+
if (event === "data") {
59+
// Simulate ripgrep output
60+
setTimeout(() => callback(`${path.join(tempDir, "src", "index.ts")}\n`), 10)
61+
}
62+
},
63+
},
64+
stderr: {
65+
on: vi.fn(),
66+
},
67+
on: (event: string, callback: (code: number | null) => void) => {
68+
if (event === "close") {
69+
setTimeout(() => callback(0), 20)
70+
}
71+
},
72+
kill: vi.fn(),
73+
} as any
74+
return mockProcess
75+
})
76+
77+
// Call listFiles in recursive mode
78+
const [files, limitReached] = await listFiles(tempDir, true, 100)
79+
80+
// Verify ripgrep was called with the .git exclusion pattern
81+
expect(mockSpawn).toHaveBeenCalled()
82+
const [rgPath, args] = mockSpawn.mock.calls[0]
83+
84+
// Check that the arguments include the .git exclusion pattern
85+
expect(args).toContain("-g")
86+
expect(args).toContain("!**/.git/**")
87+
})
88+
89+
it("should exclude .git directories even when explicitly targeting a hidden directory", async () => {
90+
// Create a hidden directory path
91+
const hiddenDirPath = path.join(tempDir, ".hidden-dir")
92+
93+
// Mock ripgrep spawn
94+
const mockSpawn = vi.mocked(childProcess.spawn)
95+
mockSpawn.mockImplementation((command: string, args: readonly string[]) => {
96+
const mockProcess = {
97+
stdout: {
98+
on: (event: string, callback: (data: any) => void) => {
99+
if (event === "data") {
100+
// Simulate ripgrep output
101+
setTimeout(() => callback(`${path.join(hiddenDirPath, "file.ts")}\n`), 10)
102+
}
103+
},
104+
},
105+
stderr: {
106+
on: vi.fn(),
107+
},
108+
on: (event: string, callback: (code: number | null) => void) => {
109+
if (event === "close") {
110+
setTimeout(() => callback(0), 20)
111+
}
112+
},
113+
kill: vi.fn(),
114+
} as any
115+
return mockProcess
116+
})
117+
118+
// Call listFiles targeting a hidden directory
119+
const [files, limitReached] = await listFiles(hiddenDirPath, true, 100)
120+
121+
// Verify ripgrep was called with the .git exclusion pattern
122+
expect(mockSpawn).toHaveBeenCalled()
123+
const [rgPath, args] = mockSpawn.mock.calls[0]
124+
125+
// Even when targeting a hidden directory, .git should still be excluded
126+
expect(args).toContain("-g")
127+
expect(args).toContain("!**/.git/**")
128+
129+
// But the command should have the special flags for hidden directories
130+
expect(args).toContain("--no-ignore-vcs")
131+
expect(args).toContain("--no-ignore")
132+
})
133+
134+
it("should exclude .git directories in non-recursive mode", async () => {
135+
// Mock ripgrep spawn
136+
const mockSpawn = vi.mocked(childProcess.spawn)
137+
mockSpawn.mockImplementation((command: string, args: readonly string[]) => {
138+
const mockProcess = {
139+
stdout: {
140+
on: (event: string, callback: (data: any) => void) => {
141+
if (event === "data") {
142+
// Simulate ripgrep output for non-recursive
143+
setTimeout(() => callback(`${path.join(tempDir, "file.ts")}\n`), 10)
144+
}
145+
},
146+
},
147+
stderr: {
148+
on: vi.fn(),
149+
},
150+
on: (event: string, callback: (code: number | null) => void) => {
151+
if (event === "close") {
152+
setTimeout(() => callback(0), 20)
153+
}
154+
},
155+
kill: vi.fn(),
156+
} as any
157+
return mockProcess
158+
})
159+
160+
// Call listFiles in non-recursive mode
161+
const [files, limitReached] = await listFiles(tempDir, false, 100)
162+
163+
// Verify ripgrep was called with the .git exclusion patterns
164+
expect(mockSpawn).toHaveBeenCalled()
165+
const [rgPath, args] = mockSpawn.mock.calls[0]
166+
167+
// Check that the arguments include the .git exclusion patterns for non-recursive mode
168+
expect(args).toContain("-g")
169+
expect(args).toContain("!.git")
170+
expect(args).toContain("!.git/**")
171+
})
172+
173+
it("should exclude .git even when it's the target directory", async () => {
174+
// Create a .git directory path
175+
const gitDirPath = path.join(tempDir, ".git")
176+
177+
// Mock ripgrep spawn
178+
const mockSpawn = vi.mocked(childProcess.spawn)
179+
mockSpawn.mockImplementation((command: string, args: readonly string[]) => {
180+
const mockProcess = {
181+
stdout: {
182+
on: (event: string, callback: (data: any) => void) => {
183+
if (event === "data") {
184+
// Simulate empty output (no files found)
185+
setTimeout(() => callback(""), 10)
186+
}
187+
},
188+
},
189+
stderr: {
190+
on: vi.fn(),
191+
},
192+
on: (event: string, callback: (code: number | null) => void) => {
193+
if (event === "close") {
194+
setTimeout(() => callback(0), 20)
195+
}
196+
},
197+
kill: vi.fn(),
198+
} as any
199+
return mockProcess
200+
})
201+
202+
// Call listFiles targeting .git directory
203+
const [files, limitReached] = await listFiles(gitDirPath, true, 100)
204+
205+
// Verify ripgrep was called with the .git exclusion pattern
206+
expect(mockSpawn).toHaveBeenCalled()
207+
const [rgPath, args] = mockSpawn.mock.calls[0]
208+
209+
// .git should still be excluded even when it's the target
210+
expect(args).toContain("-g")
211+
expect(args).toContain("!**/.git/**")
212+
})
213+
})

‎src/services/glob/list-files.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,14 +260,23 @@ function buildRecursiveArgs(dirPath: string): string[] {
260260
args.push("-g", "**/*")
261261
}
262262

263+
// CRITICAL: Always exclude .git directories for performance reasons
264+
// This must come before other exclusions to ensure it's always applied
265+
args.push("-g", "!**/.git/**")
266+
263267
// Apply directory exclusions for recursive searches
264268
for (const dir of DIRS_TO_IGNORE) {
269+
// Skip .git since we already handled it above
270+
if (dir === ".git") {
271+
continue
272+
}
273+
265274
// Special handling for hidden directories pattern
266275
if (dir === ".*") {
267276
// If we're explicitly targeting a hidden directory, don't exclude hidden files/dirs
268277
// This allows the target hidden directory and all its contents to be listed
269278
if (!isTargetingHiddenDir) {
270-
// Not targeting hidden dir: exclude all hidden directories
279+
// Not targeting hidden dir: exclude all hidden directories (except .git which is already excluded)
271280
args.push("-g", `!**/.*/**`)
272281
}
273282
// If targeting hidden dir: don't add any exclusion for hidden directories
@@ -305,8 +314,17 @@ function buildNonRecursiveArgs(): string[] {
305314
// Respect .gitignore in non-recursive mode too
306315
// (ripgrep respects .gitignore by default)
307316

317+
// CRITICAL: Always exclude .git directories for performance reasons
318+
args.push("-g", "!.git")
319+
args.push("-g", "!.git/**")
320+
308321
// Apply directory exclusions for non-recursive searches
309322
for (const dir of DIRS_TO_IGNORE) {
323+
// Skip .git since we already handled it above
324+
if (dir === ".git") {
325+
continue
326+
}
327+
310328
if (dir === ".*") {
311329
// For hidden directories in non-recursive mode, we want to show the directories
312330
// themselves but not their contents. Since we're using --maxdepth 1, this

0 commit comments

Comments
 (0)