Skip to content

Commit a732f18

Browse files
roomotemrubensdaniel-lxs
authored
fix: respect .gitignore patterns for directories in list_files tool (#5393) (#5394)
Co-authored-by: Matt Rubens <[email protected]> Co-authored-by: Daniel <[email protected]>
1 parent 2b7c266 commit a732f18

File tree

3 files changed

+424
-64
lines changed

3 files changed

+424
-64
lines changed

‎src/services/glob/__tests__/gitignore-integration.spec.ts‎

Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
import { vi, describe, it, expect, beforeEach, afterEach } from "vitest"
2+
import * as path from "path"
3+
import * as fs from "fs"
4+
import * as os from "os"
5+
6+
// Mock ripgrep to avoid filesystem dependencies
7+
vi.mock("../../ripgrep", () => ({
8+
getBinPath: vi.fn().mockResolvedValue("/mock/path/to/rg"),
9+
}))
10+
11+
// Mock vscode
12+
vi.mock("vscode", () => ({
13+
env: {
14+
appRoot: "/mock/app/root",
15+
},
16+
}))
17+
18+
// Mock child_process to simulate ripgrep behavior
19+
vi.mock("child_process", () => ({
20+
spawn: vi.fn(),
21+
}))
22+
23+
vi.mock("../../path", () => ({
24+
arePathsEqual: vi.fn().mockReturnValue(false),
25+
}))
26+
27+
import { listFiles } from "../list-files"
28+
import * as childProcess from "child_process"
29+
30+
describe("list-files gitignore integration", () => {
31+
let tempDir: string
32+
let originalCwd: string
33+
34+
beforeEach(async () => {
35+
vi.clearAllMocks()
36+
37+
// Create a temporary directory for testing
38+
tempDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "roo-gitignore-test-"))
39+
originalCwd = process.cwd()
40+
})
41+
42+
afterEach(async () => {
43+
process.chdir(originalCwd)
44+
// Clean up temp directory
45+
await fs.promises.rm(tempDir, { recursive: true, force: true })
46+
})
47+
48+
it("should properly filter directories based on .gitignore patterns", async () => {
49+
// Setup test directory structure
50+
await fs.promises.mkdir(path.join(tempDir, "src"))
51+
await fs.promises.mkdir(path.join(tempDir, "node_modules"))
52+
await fs.promises.mkdir(path.join(tempDir, "build"))
53+
await fs.promises.mkdir(path.join(tempDir, "dist"))
54+
await fs.promises.mkdir(path.join(tempDir, "allowed-dir"))
55+
56+
// Create .gitignore file
57+
await fs.promises.writeFile(path.join(tempDir, ".gitignore"), "node_modules/\nbuild/\ndist/\n*.log\n")
58+
59+
// Create some files
60+
await fs.promises.writeFile(path.join(tempDir, "src", "index.ts"), "console.log('hello')")
61+
await fs.promises.writeFile(path.join(tempDir, "allowed-dir", "file.txt"), "content")
62+
63+
// Mock ripgrep to return files that would not be gitignored
64+
const mockSpawn = vi.mocked(childProcess.spawn)
65+
const mockProcess = {
66+
stdout: {
67+
on: vi.fn((event, callback) => {
68+
if (event === "data") {
69+
// Simulate ripgrep output (files that are not gitignored)
70+
const files =
71+
[path.join(tempDir, "src", "index.ts"), path.join(tempDir, "allowed-dir", "file.txt")].join(
72+
"\n",
73+
) + "\n"
74+
setTimeout(() => callback(files), 10)
75+
}
76+
}),
77+
},
78+
stderr: {
79+
on: vi.fn(),
80+
},
81+
on: vi.fn((event, callback) => {
82+
if (event === "close") {
83+
setTimeout(() => callback(0), 20)
84+
}
85+
}),
86+
kill: vi.fn(),
87+
}
88+
89+
mockSpawn.mockReturnValue(mockProcess as any)
90+
91+
// Call listFiles in recursive mode
92+
const [files, didHitLimit] = await listFiles(tempDir, true, 100)
93+
94+
// Filter out only directories from the results
95+
const directoriesInResult = files.filter((f) => f.endsWith("/"))
96+
97+
// Verify that gitignored directories are NOT included
98+
expect(directoriesInResult).not.toContain(path.join(tempDir, "node_modules") + "/")
99+
expect(directoriesInResult).not.toContain(path.join(tempDir, "build") + "/")
100+
expect(directoriesInResult).not.toContain(path.join(tempDir, "dist") + "/")
101+
102+
// Verify that allowed directories ARE included
103+
expect(directoriesInResult).toContain(path.join(tempDir, "src") + "/")
104+
expect(directoriesInResult).toContain(path.join(tempDir, "allowed-dir") + "/")
105+
})
106+
107+
it("should handle nested .gitignore files correctly", async () => {
108+
// Setup nested directory structure
109+
await fs.promises.mkdir(path.join(tempDir, "src"), { recursive: true })
110+
await fs.promises.mkdir(path.join(tempDir, "src", "components"))
111+
await fs.promises.mkdir(path.join(tempDir, "src", "temp"))
112+
await fs.promises.mkdir(path.join(tempDir, "src", "utils"))
113+
114+
// Create root .gitignore
115+
await fs.promises.writeFile(path.join(tempDir, ".gitignore"), "node_modules/\n")
116+
117+
// Create nested .gitignore in src/
118+
await fs.promises.writeFile(path.join(tempDir, "src", ".gitignore"), "temp/\n")
119+
120+
// Mock ripgrep
121+
const mockSpawn = vi.mocked(childProcess.spawn)
122+
const mockProcess = {
123+
stdout: {
124+
on: vi.fn((event, callback) => {
125+
if (event === "data") {
126+
setTimeout(() => callback(""), 10)
127+
}
128+
}),
129+
},
130+
stderr: {
131+
on: vi.fn(),
132+
},
133+
on: vi.fn((event, callback) => {
134+
if (event === "close") {
135+
setTimeout(() => callback(0), 20)
136+
}
137+
}),
138+
kill: vi.fn(),
139+
}
140+
141+
mockSpawn.mockReturnValue(mockProcess as any)
142+
143+
// Call listFiles in recursive mode
144+
const [files, didHitLimit] = await listFiles(tempDir, true, 100)
145+
146+
// Filter out only directories from the results
147+
const directoriesInResult = files.filter((f) => f.endsWith("/"))
148+
149+
// Verify that nested gitignored directories are NOT included
150+
expect(directoriesInResult).not.toContain(path.join(tempDir, "src", "temp") + "/")
151+
152+
// Verify that allowed directories ARE included
153+
expect(directoriesInResult).toContain(path.join(tempDir, "src") + "/")
154+
expect(directoriesInResult).toContain(path.join(tempDir, "src", "components") + "/")
155+
expect(directoriesInResult).toContain(path.join(tempDir, "src", "utils") + "/")
156+
})
157+
158+
it("should respect .gitignore in non-recursive mode too", async () => {
159+
// Setup test directory structure
160+
await fs.promises.mkdir(path.join(tempDir, "src"))
161+
await fs.promises.mkdir(path.join(tempDir, "node_modules"))
162+
await fs.promises.mkdir(path.join(tempDir, "allowed-dir"))
163+
164+
// Create .gitignore file
165+
await fs.promises.writeFile(path.join(tempDir, ".gitignore"), "node_modules/\n")
166+
167+
// Mock ripgrep for non-recursive mode
168+
const mockSpawn = vi.mocked(childProcess.spawn)
169+
const mockProcess = {
170+
stdout: {
171+
on: vi.fn((event, callback) => {
172+
if (event === "data") {
173+
// In non-recursive mode, ripgrep should now respect .gitignore
174+
const files = [path.join(tempDir, "src"), path.join(tempDir, "allowed-dir")].join("\n") + "\n"
175+
setTimeout(() => callback(files), 10)
176+
}
177+
}),
178+
},
179+
stderr: {
180+
on: vi.fn(),
181+
},
182+
on: vi.fn((event, callback) => {
183+
if (event === "close") {
184+
setTimeout(() => callback(0), 20)
185+
}
186+
}),
187+
kill: vi.fn(),
188+
}
189+
190+
mockSpawn.mockReturnValue(mockProcess as any)
191+
192+
// Call listFiles in NON-recursive mode
193+
const [files, didHitLimit] = await listFiles(tempDir, false, 100)
194+
195+
// Verify ripgrep was called without --no-ignore-vcs (should respect .gitignore)
196+
const [rgPath, args] = mockSpawn.mock.calls[0]
197+
expect(args).not.toContain("--no-ignore-vcs")
198+
199+
// Filter out only directories from the results
200+
const directoriesInResult = files.filter((f) => f.endsWith("/"))
201+
202+
// Verify that gitignored directories are NOT included even in non-recursive mode
203+
expect(directoriesInResult).not.toContain(path.join(tempDir, "node_modules") + "/")
204+
205+
// Verify that allowed directories ARE included
206+
expect(directoriesInResult).toContain(path.join(tempDir, "src") + "/")
207+
expect(directoriesInResult).toContain(path.join(tempDir, "allowed-dir") + "/")
208+
})
209+
})

‎src/services/glob/__tests__/gitignore-test.spec.ts‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
import { vi, describe, it, expect, beforeEach, afterEach } from "vitest"
2+
import * as path from "path"
3+
import * as fs from "fs"
4+
import * as os from "os"
5+
6+
// Mock ripgrep to avoid filesystem dependencies
7+
vi.mock("../../ripgrep", () => ({
8+
getBinPath: vi.fn().mockResolvedValue("/mock/path/to/rg"),
9+
}))
10+
11+
// Mock vscode
12+
vi.mock("vscode", () => ({
13+
env: {
14+
appRoot: "/mock/app/root",
15+
},
16+
}))
17+
18+
vi.mock("child_process", () => ({
19+
spawn: vi.fn(),
20+
}))
21+
22+
vi.mock("../../path", () => ({
23+
arePathsEqual: vi.fn().mockReturnValue(false),
24+
}))
25+
26+
import { listFiles } from "../list-files"
27+
import * as childProcess from "child_process"
28+
29+
describe("list-files gitignore support", () => {
30+
let tempDir: string
31+
let originalCwd: string
32+
33+
beforeEach(async () => {
34+
vi.clearAllMocks()
35+
36+
// Create a temporary directory for testing
37+
tempDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "roo-test-"))
38+
originalCwd = process.cwd()
39+
process.chdir(tempDir)
40+
})
41+
42+
afterEach(async () => {
43+
process.chdir(originalCwd)
44+
// Clean up temp directory
45+
await fs.promises.rm(tempDir, { recursive: true, force: true })
46+
})
47+
48+
it("should respect .gitignore patterns for directories in recursive mode", async () => {
49+
// Setup test directory structure
50+
await fs.promises.mkdir(path.join(tempDir, "src"))
51+
await fs.promises.mkdir(path.join(tempDir, "node_modules"))
52+
await fs.promises.mkdir(path.join(tempDir, "build"))
53+
await fs.promises.mkdir(path.join(tempDir, "ignored-dir"))
54+
55+
// Create .gitignore file
56+
await fs.promises.writeFile(path.join(tempDir, ".gitignore"), "node_modules/\nbuild/\nignored-dir/\n")
57+
58+
// Create some files
59+
await fs.promises.writeFile(path.join(tempDir, "src", "index.ts"), "")
60+
await fs.promises.writeFile(path.join(tempDir, "node_modules", "package.json"), "")
61+
await fs.promises.writeFile(path.join(tempDir, "build", "output.js"), "")
62+
await fs.promises.writeFile(path.join(tempDir, "ignored-dir", "file.txt"), "")
63+
64+
// Mock ripgrep to return only non-ignored files
65+
const mockSpawn = vi.mocked(childProcess.spawn)
66+
const mockProcess = {
67+
stdout: {
68+
on: vi.fn((event, callback) => {
69+
if (event === "data") {
70+
// Ripgrep should respect .gitignore and only return src/index.ts
71+
setTimeout(() => callback(`${path.join(tempDir, "src", "index.ts")}\n`), 10)
72+
}
73+
}),
74+
},
75+
stderr: {
76+
on: vi.fn(),
77+
},
78+
on: vi.fn((event, callback) => {
79+
if (event === "close") {
80+
setTimeout(() => callback(0), 20)
81+
}
82+
}),
83+
kill: vi.fn(),
84+
}
85+
86+
mockSpawn.mockReturnValue(mockProcess as any)
87+
88+
// Call listFiles in recursive mode
89+
const [files, didHitLimit] = await listFiles(tempDir, true, 100)
90+
91+
// Verify that gitignored directories are not included
92+
const directoriesInResult = files.filter((f) => f.endsWith("/"))
93+
94+
expect(directoriesInResult).not.toContain(path.join(tempDir, "node_modules") + "/")
95+
expect(directoriesInResult).not.toContain(path.join(tempDir, "build") + "/")
96+
expect(directoriesInResult).not.toContain(path.join(tempDir, "ignored-dir") + "/")
97+
98+
// But src/ should be included
99+
expect(directoriesInResult).toContain(path.join(tempDir, "src") + "/")
100+
})
101+
102+
it("should handle nested .gitignore files", async () => {
103+
// Setup nested directory structure
104+
await fs.promises.mkdir(path.join(tempDir, "src"), { recursive: true })
105+
await fs.promises.mkdir(path.join(tempDir, "src", "components"))
106+
await fs.promises.mkdir(path.join(tempDir, "src", "temp"))
107+
108+
// Create root .gitignore
109+
await fs.promises.writeFile(path.join(tempDir, ".gitignore"), "node_modules/\n")
110+
111+
// Create nested .gitignore in src/
112+
await fs.promises.writeFile(path.join(tempDir, "src", ".gitignore"), "temp/\n")
113+
114+
// Mock ripgrep
115+
const mockSpawn = vi.mocked(childProcess.spawn)
116+
const mockProcess = {
117+
stdout: {
118+
on: vi.fn((event, callback) => {
119+
if (event === "data") {
120+
setTimeout(() => callback(""), 10)
121+
}
122+
}),
123+
},
124+
stderr: {
125+
on: vi.fn(),
126+
},
127+
on: vi.fn((event, callback) => {
128+
if (event === "close") {
129+
setTimeout(() => callback(0), 20)
130+
}
131+
}),
132+
kill: vi.fn(),
133+
}
134+
135+
mockSpawn.mockReturnValue(mockProcess as any)
136+
137+
// Call listFiles in recursive mode
138+
const [files, didHitLimit] = await listFiles(tempDir, true, 100)
139+
140+
// Verify that nested gitignored directories are not included
141+
const directoriesInResult = files.filter((f) => f.endsWith("/"))
142+
143+
expect(directoriesInResult).not.toContain(path.join(tempDir, "src", "temp") + "/")
144+
expect(directoriesInResult).toContain(path.join(tempDir, "src") + "/")
145+
expect(directoriesInResult).toContain(path.join(tempDir, "src", "components") + "/")
146+
})
147+
})

0 commit comments

Comments
 (0)