Skip to content

Commit 622b1af

Browse files
committed
fix: ensure JSON files respect .rooignore during indexing
- Pass RooIgnoreController from manager through service factory to FileWatcher - Add comprehensive tests to verify JSON and other file types are handled consistently - Fix issue where JSON files were being indexed despite being in .rooignore Fixes #6690
1 parent 1d714c8 commit 622b1af

File tree

6 files changed

+770
-2
lines changed

6 files changed

+770
-2
lines changed

src/services/code-index/__tests__/manager.spec.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,41 @@ vi.mock("vscode", () => ({
1212
index: 0,
1313
},
1414
],
15+
createFileSystemWatcher: vi.fn().mockReturnValue({
16+
onDidCreate: vi.fn().mockReturnValue({ dispose: vi.fn() }),
17+
onDidChange: vi.fn().mockReturnValue({ dispose: vi.fn() }),
18+
onDidDelete: vi.fn().mockReturnValue({ dispose: vi.fn() }),
19+
dispose: vi.fn(),
20+
}),
1521
},
22+
RelativePattern: vi.fn().mockImplementation((base, pattern) => ({ base, pattern })),
1623
}))
1724

1825
// Mock only the essential dependencies
1926
vi.mock("../../../utils/path", () => ({
2027
getWorkspacePath: vi.fn(() => "/test/workspace"),
2128
}))
2229

30+
// Mock fs/promises for RooIgnoreController
31+
vi.mock("fs/promises", () => ({
32+
default: {
33+
readFile: vi.fn().mockRejectedValue(new Error("File not found")), // Simulate no .gitignore/.rooignore
34+
},
35+
}))
36+
37+
// Mock file utils for RooIgnoreController
38+
vi.mock("../../../utils/fs", () => ({
39+
fileExistsAtPath: vi.fn().mockResolvedValue(false), // Simulate no .rooignore file
40+
}))
41+
42+
// Mock ignore module
43+
vi.mock("ignore", () => ({
44+
default: vi.fn().mockReturnValue({
45+
add: vi.fn(),
46+
ignores: vi.fn().mockReturnValue(false),
47+
}),
48+
}))
49+
2350
vi.mock("../state-manager", () => ({
2451
CodeIndexStateManager: vi.fn().mockImplementation(() => ({
2552
onProgressUpdate: vi.fn(),

src/services/code-index/manager.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { CodeIndexServiceFactory } from "./service-factory"
99
import { CodeIndexSearchService } from "./search-service"
1010
import { CodeIndexOrchestrator } from "./orchestrator"
1111
import { CacheManager } from "./cache-manager"
12+
import { RooIgnoreController } from "../../core/ignore/RooIgnoreController"
1213
import fs from "fs/promises"
1314
import ignore from "ignore"
1415
import path from "path"
@@ -244,6 +245,7 @@ export class CodeIndexManager {
244245
return
245246
}
246247

248+
// Create .gitignore instance
247249
const ignorePath = path.join(workspacePath, ".gitignore")
248250
try {
249251
const content = await fs.readFile(ignorePath, "utf8")
@@ -259,11 +261,16 @@ export class CodeIndexManager {
259261
})
260262
}
261263

264+
// Create RooIgnoreController instance
265+
const rooIgnoreController = new RooIgnoreController(workspacePath)
266+
await rooIgnoreController.initialize()
267+
262268
// (Re)Create shared service instances
263269
const { embedder, vectorStore, scanner, fileWatcher } = this._serviceFactory.createServices(
264270
this.context,
265271
this._cacheManager!,
266272
ignoreInstance,
273+
rooIgnoreController,
267274
)
268275

269276
// Validate embedder configuration before proceeding
Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
// Test specifically for the JSON file .rooignore issue
2+
// npx vitest services/code-index/processors/__tests__/file-watcher-json-rooignore.spec.ts
3+
4+
import { FileWatcher } from "../file-watcher"
5+
import { RooIgnoreController } from "../../../../core/ignore/RooIgnoreController"
6+
import * as vscode from "vscode"
7+
import * as path from "path"
8+
9+
// Mock TelemetryService
10+
vi.mock("../../../../../packages/telemetry/src/TelemetryService", () => ({
11+
TelemetryService: {
12+
instance: {
13+
captureEvent: vi.fn(),
14+
},
15+
},
16+
}))
17+
18+
// Mock dependencies
19+
vi.mock("../../cache-manager", () => ({
20+
CacheManager: vi.fn().mockImplementation(() => ({
21+
getHash: vi.fn().mockReturnValue(null),
22+
updateHash: vi.fn(),
23+
deleteHash: vi.fn(),
24+
})),
25+
}))
26+
27+
vi.mock("../parser", () => ({
28+
codeParser: {
29+
parseFile: vi.fn().mockImplementation((filePath) => {
30+
// Return blocks based on the actual file path
31+
return [
32+
{
33+
content: `content from ${path.basename(filePath)}`,
34+
file_path: filePath,
35+
start_line: 1,
36+
end_line: 10,
37+
segmentHash: `hash-${path.basename(filePath)}`,
38+
},
39+
]
40+
}),
41+
},
42+
}))
43+
44+
vi.mock("../../../glob/ignore-utils", () => ({
45+
isPathInIgnoredDirectory: vi.fn().mockReturnValue(false),
46+
}))
47+
48+
vi.mock("../shared/get-relative-path", () => ({
49+
generateRelativeFilePath: vi.fn().mockImplementation((filePath, workspacePath) => {
50+
return path.relative(workspacePath, filePath)
51+
}),
52+
generateNormalizedAbsolutePath: vi.fn().mockImplementation((filePath) => filePath),
53+
}))
54+
55+
// Mock vscode
56+
vi.mock("vscode", () => ({
57+
workspace: {
58+
createFileSystemWatcher: vi.fn(),
59+
fs: {
60+
stat: vi.fn().mockResolvedValue({ size: 1000 }),
61+
readFile: vi.fn().mockResolvedValue(Buffer.from('{"test": "data"}')),
62+
},
63+
},
64+
RelativePattern: vi.fn().mockImplementation((base, pattern) => ({ base, pattern })),
65+
Uri: {
66+
file: vi.fn().mockImplementation((path) => ({ fsPath: path })),
67+
},
68+
EventEmitter: vi.fn().mockImplementation(() => ({
69+
event: vi.fn(),
70+
fire: vi.fn(),
71+
dispose: vi.fn(),
72+
})),
73+
}))
74+
75+
describe("FileWatcher - JSON .rooignore bug", () => {
76+
let fileWatcher: FileWatcher
77+
let mockRooIgnoreController: any
78+
let processFileResult: any
79+
80+
beforeEach(() => {
81+
vi.clearAllMocks()
82+
83+
// Create a simple mock RooIgnoreController
84+
mockRooIgnoreController = {
85+
validateAccess: vi.fn(),
86+
}
87+
})
88+
89+
it("should call RooIgnoreController.validateAccess with correct paths for JSON files", async () => {
90+
const workspacePath = "/workspace"
91+
92+
// Setup RooIgnoreController to block files in 'ignored' folder
93+
mockRooIgnoreController.validateAccess.mockImplementation((filePath: string) => {
94+
// This should receive the full file path
95+
return !filePath.includes("ignored/")
96+
})
97+
98+
// Create FileWatcher with minimal dependencies
99+
fileWatcher = new FileWatcher(
100+
workspacePath,
101+
{} as any, // context
102+
{
103+
getHash: vi.fn().mockReturnValue(null),
104+
updateHash: vi.fn(),
105+
} as any, // cacheManager
106+
undefined, // embedder
107+
undefined, // vectorStore
108+
{ ignores: vi.fn().mockReturnValue(false) } as any, // ignoreInstance
109+
mockRooIgnoreController,
110+
)
111+
112+
// Test processFile directly with different JSON files
113+
const testCases = [
114+
{ path: "/workspace/config.json", shouldProcess: true },
115+
{ path: "/workspace/ignored/secrets.json", shouldProcess: false },
116+
{ path: "/workspace/src/data.json", shouldProcess: true },
117+
{ path: "/workspace/ignored/private.json", shouldProcess: false },
118+
]
119+
120+
for (const { path: filePath, shouldProcess } of testCases) {
121+
const result = await fileWatcher.processFile(filePath)
122+
123+
// Verify validateAccess was called with the correct path
124+
expect(mockRooIgnoreController.validateAccess).toHaveBeenCalledWith(filePath)
125+
126+
if (shouldProcess) {
127+
expect(result.status).not.toBe("skipped")
128+
expect(result.reason).not.toBe("File is ignored by .rooignore or .gitignore")
129+
} else {
130+
expect(result.status).toBe("skipped")
131+
expect(result.reason).toBe("File is ignored by .rooignore or .gitignore")
132+
}
133+
}
134+
135+
// Verify validateAccess was called for all test files
136+
expect(mockRooIgnoreController.validateAccess).toHaveBeenCalledTimes(4)
137+
})
138+
139+
it("should handle both .txt and .json files consistently with .rooignore", async () => {
140+
const workspacePath = "/workspace"
141+
142+
// Setup RooIgnoreController to block all files in 'blocked' folder
143+
mockRooIgnoreController.validateAccess.mockImplementation((filePath: string) => {
144+
return !filePath.includes("/blocked/")
145+
})
146+
147+
fileWatcher = new FileWatcher(
148+
workspacePath,
149+
{} as any,
150+
{
151+
getHash: vi.fn().mockReturnValue(null),
152+
updateHash: vi.fn(),
153+
} as any,
154+
undefined,
155+
undefined,
156+
{ ignores: vi.fn().mockReturnValue(false) } as any,
157+
mockRooIgnoreController,
158+
)
159+
160+
// Test with mixed file types
161+
const mixedFiles = [
162+
{ path: "/workspace/allowed/data.json", type: "json", shouldProcess: true },
163+
{ path: "/workspace/allowed/notes.txt", type: "txt", shouldProcess: true },
164+
{ path: "/workspace/blocked/config.json", type: "json", shouldProcess: false },
165+
{ path: "/workspace/blocked/readme.txt", type: "txt", shouldProcess: false },
166+
]
167+
168+
const results: Record<string, any> = {}
169+
170+
for (const { path: filePath, type, shouldProcess } of mixedFiles) {
171+
const result = await fileWatcher.processFile(filePath)
172+
results[filePath] = result
173+
174+
// Both JSON and TXT files should be handled the same way
175+
if (shouldProcess) {
176+
expect(result.status).not.toBe("skipped")
177+
} else {
178+
expect(result.status).toBe("skipped")
179+
expect(result.reason).toBe("File is ignored by .rooignore or .gitignore")
180+
}
181+
}
182+
183+
// Verify that JSON files are not treated differently than TXT files
184+
expect(results["/workspace/blocked/config.json"].status).toBe(results["/workspace/blocked/readme.txt"].status)
185+
expect(results["/workspace/allowed/data.json"].status).toBe(results["/workspace/allowed/notes.txt"].status)
186+
})
187+
})

0 commit comments

Comments
 (0)