diff --git a/src/core/ignore/UnifiedIgnoreController.ts b/src/core/ignore/UnifiedIgnoreController.ts new file mode 100644 index 0000000000..720b34bbfd --- /dev/null +++ b/src/core/ignore/UnifiedIgnoreController.ts @@ -0,0 +1,311 @@ +import path from "path" +import { fileExistsAtPath } from "../../utils/fs" +import fs from "fs/promises" +import ignore, { Ignore } from "ignore" +import * as vscode from "vscode" + +export const LOCK_TEXT_SYMBOL = "\u{1F512}" + +/** + * Unified controller that handles both .gitignore and .rooignore patterns + * with proper fallback behavior. When .rooignore is missing or empty, + * falls back to .gitignore patterns for consistent file filtering. + */ +export class UnifiedIgnoreController { + private cwd: string + private ignoreInstance: Ignore + private disposables: vscode.Disposable[] = [] + private rooIgnoreContent: string | undefined + private gitIgnoreContent: string | undefined + private hasRooIgnore: boolean = false + + constructor(cwd: string) { + this.cwd = cwd + this.ignoreInstance = ignore() + this.rooIgnoreContent = undefined + this.gitIgnoreContent = undefined + this.setupFileWatchers() + } + + /** + * Initialize the controller by loading both .gitignore and .rooignore patterns + * Must be called after construction and before using the controller + */ + async initialize(): Promise { + await this.loadIgnorePatterns() + } + + /** + * Set up file watchers for both .gitignore and .rooignore changes + */ + private setupFileWatchers(): void { + // Watch .rooignore + const rooignorePattern = new vscode.RelativePattern(this.cwd, ".rooignore") + const rooIgnoreWatcher = vscode.workspace.createFileSystemWatcher(rooignorePattern) + + // Watch .gitignore + const gitignorePattern = new vscode.RelativePattern(this.cwd, ".gitignore") + const gitIgnoreWatcher = vscode.workspace.createFileSystemWatcher(gitignorePattern) + + // Set up event handlers for .rooignore + this.disposables.push( + rooIgnoreWatcher.onDidChange(() => this.loadIgnorePatterns()), + rooIgnoreWatcher.onDidCreate(() => this.loadIgnorePatterns()), + rooIgnoreWatcher.onDidDelete(() => this.loadIgnorePatterns()), + rooIgnoreWatcher, + ) + + // Set up event handlers for .gitignore + this.disposables.push( + gitIgnoreWatcher.onDidChange(() => this.loadIgnorePatterns()), + gitIgnoreWatcher.onDidCreate(() => this.loadIgnorePatterns()), + gitIgnoreWatcher.onDidDelete(() => this.loadIgnorePatterns()), + gitIgnoreWatcher, + ) + } + + /** + * Load patterns from both .gitignore and .rooignore files with proper fallback logic + */ + private async loadIgnorePatterns(): Promise { + try { + // Reset ignore instance to prevent duplicate patterns + this.ignoreInstance = ignore() + + // Load .rooignore first (higher priority) + const rooIgnorePath = path.join(this.cwd, ".rooignore") + this.hasRooIgnore = await fileExistsAtPath(rooIgnorePath) + + if (this.hasRooIgnore) { + try { + this.rooIgnoreContent = await fs.readFile(rooIgnorePath, "utf8") + // Only use .rooignore if it has actual content (not just whitespace) + const hasContent = this.rooIgnoreContent.trim().length > 0 + if (hasContent) { + this.ignoreInstance.add(this.rooIgnoreContent) + this.ignoreInstance.add(".rooignore") + return // Use .rooignore exclusively when it exists and has content + } + } catch (error) { + console.error("Error reading .rooignore:", error) + this.rooIgnoreContent = undefined + this.hasRooIgnore = false + } + } else { + this.rooIgnoreContent = undefined + } + + // Fallback to .gitignore when .rooignore is missing or empty + await this.loadGitIgnorePatterns() + } catch (error) { + console.error("Unexpected error loading ignore patterns:", error) + } + } + + /** + * Load .gitignore patterns hierarchically (from workspace root up to current directory) + */ + private async loadGitIgnorePatterns(): Promise { + try { + // Find all .gitignore files from the current directory up to the workspace root + const gitignoreFiles = await this.findGitignoreFiles(this.cwd) + + let hasGitIgnoreContent = false + let combinedGitIgnoreContent = "" + + // Add patterns from all .gitignore files (root first, then more specific ones) + for (const gitignoreFile of gitignoreFiles) { + try { + const content = await fs.readFile(gitignoreFile, "utf8") + if (content.trim().length > 0) { + this.ignoreInstance.add(content) + hasGitIgnoreContent = true + combinedGitIgnoreContent += content + "\n" + // Store content from the most specific .gitignore (usually the one in cwd) + if (path.dirname(gitignoreFile) === this.cwd) { + this.gitIgnoreContent = content + } + } + } catch (err) { + console.warn(`Error reading .gitignore at ${gitignoreFile}: ${err}`) + } + } + + // If we found .gitignore content but no specific one in cwd, use combined content + if (hasGitIgnoreContent && !this.gitIgnoreContent) { + this.gitIgnoreContent = combinedGitIgnoreContent.trim() + } + + // Always ignore .gitignore files themselves + if (hasGitIgnoreContent) { + this.ignoreInstance.add(".gitignore") + } + } catch (error) { + console.error("Error loading .gitignore patterns:", error) + } + } + + /** + * Find all .gitignore files from the given directory up to the workspace root + */ + private async findGitignoreFiles(startPath: string): Promise { + const gitignoreFiles: string[] = [] + let currentPath = startPath + + // Walk up the directory tree looking for .gitignore files + while (currentPath && currentPath !== path.dirname(currentPath)) { + const gitignorePath = path.join(currentPath, ".gitignore") + + try { + await fs.access(gitignorePath) + gitignoreFiles.push(gitignorePath) + } catch { + // .gitignore doesn't exist at this level, continue + } + + // Move up one directory + const parentPath = path.dirname(currentPath) + if (parentPath === currentPath) { + break // Reached root + } + currentPath = parentPath + } + + // Return in reverse order (root .gitignore first, then more specific ones) + return gitignoreFiles.reverse() + } + + /** + * Check if a file should be accessible to the LLM + * @param filePath - Path to check (relative to cwd) + * @returns true if file is accessible, false if ignored + */ + validateAccess(filePath: string): boolean { + try { + // Normalize path to be relative to cwd and use forward slashes + const absolutePath = path.resolve(this.cwd, filePath) + const relativePath = path.relative(this.cwd, absolutePath).replace(/\\/g, "/") + + // Use the unified ignore instance which contains either .rooignore or .gitignore patterns + return !this.ignoreInstance.ignores(relativePath) + } catch (error) { + // Ignore is designed to work with relative file paths, so will throw error for paths outside cwd. + // We are allowing access to all files outside cwd. + return true + } + } + + /** + * Check if a terminal command should be allowed to execute based on file access patterns + * @param command - Terminal command to validate + * @returns path of file that is being accessed if it is being accessed, undefined if command is allowed + */ + validateCommand(command: string): string | undefined { + // Always allow if no ignore patterns are loaded + if (!this.rooIgnoreContent && !this.gitIgnoreContent) { + return undefined + } + + // Split command into parts and get the base command + const parts = command.trim().split(/\s+/) + const baseCommand = parts[0].toLowerCase() + + // Commands that read file contents + const fileReadingCommands = [ + // Unix commands + "cat", + "less", + "more", + "head", + "tail", + "grep", + "awk", + "sed", + // PowerShell commands and aliases + "get-content", + "gc", + "type", + "select-string", + "sls", + ] + + if (fileReadingCommands.includes(baseCommand)) { + // Check each argument that could be a file path + for (let i = 1; i < parts.length; i++) { + const arg = parts[i] + // Skip command flags/options (both Unix and PowerShell style) + if (arg.startsWith("-") || arg.startsWith("/")) { + continue + } + // Ignore PowerShell parameter names + if (arg.includes(":")) { + continue + } + // Validate file access + if (!this.validateAccess(arg)) { + return arg + } + } + } + + return undefined + } + + /** + * Filter an array of paths, removing those that should be ignored + * @param paths - Array of paths to filter (relative to cwd) + * @returns Array of allowed paths + */ + filterPaths(paths: string[]): string[] { + try { + return paths + .map((p) => ({ + path: p, + allowed: this.validateAccess(p), + })) + .filter((x) => x.allowed) + .map((x) => x.path) + } catch (error) { + console.error("Error filtering paths:", error) + return [] // Fail closed for security + } + } + + /** + * Get the current ignore instance for external use + * @returns The ignore instance containing the current patterns + */ + getIgnoreInstance(): Ignore { + return this.ignoreInstance + } + + /** + * Check if .rooignore file exists and has content + * @returns true if .rooignore is being used, false if falling back to .gitignore + */ + isUsingRooIgnore(): boolean { + return this.hasRooIgnore && !!this.rooIgnoreContent?.trim() + } + + /** + * Get formatted instructions about the ignore files for the LLM + * @returns Formatted instructions or undefined if no ignore files exist + */ + getInstructions(): string | undefined { + if (this.isUsingRooIgnore()) { + return `# .rooignore\n\n(The following is provided by a root-level .rooignore file where the user has specified files and directories that should not be accessed. When using list_files, you'll notice a ${LOCK_TEXT_SYMBOL} next to files that are blocked. Attempting to access the file's contents e.g. through read_file will result in an error.)\n\n${this.rooIgnoreContent}\n.rooignore` + } else if (this.gitIgnoreContent) { + return `# .gitignore (fallback)\n\n(The following patterns are being used from .gitignore since no .rooignore file was found. Files matching these patterns will be excluded from indexing and file operations. When using list_files, you'll notice a ${LOCK_TEXT_SYMBOL} next to files that are blocked.)\n\n${this.gitIgnoreContent}` + } + + return undefined + } + + /** + * Clean up resources when the controller is no longer needed + */ + dispose(): void { + this.disposables.forEach((d) => d.dispose()) + this.disposables = [] + } +} diff --git a/src/core/ignore/__tests__/UnifiedIgnoreController.spec.ts b/src/core/ignore/__tests__/UnifiedIgnoreController.spec.ts new file mode 100644 index 0000000000..885167f6c7 --- /dev/null +++ b/src/core/ignore/__tests__/UnifiedIgnoreController.spec.ts @@ -0,0 +1,338 @@ +// npx vitest core/ignore/__tests__/UnifiedIgnoreController.spec.ts + +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest" +import { UnifiedIgnoreController } from "../UnifiedIgnoreController" +import * as fs from "fs/promises" +import * as path from "path" +import { fileExistsAtPath } from "../../../utils/fs" + +// Mock dependencies +vi.mock("../../../utils/fs") +vi.mock("fs/promises") +vi.mock("vscode", () => ({ + workspace: { + createFileSystemWatcher: vi.fn(() => ({ + onDidChange: vi.fn(() => ({ dispose: vi.fn() })), + onDidCreate: vi.fn(() => ({ dispose: vi.fn() })), + onDidDelete: vi.fn(() => ({ dispose: vi.fn() })), + dispose: vi.fn(), + })), + }, + RelativePattern: vi.fn(), +})) + +const mockFileExists = vi.mocked(fileExistsAtPath) +const mockReadFile = vi.mocked(fs.readFile) +const mockAccess = vi.mocked(fs.access) + +const TEST_CWD = "/test/workspace" + +describe("UnifiedIgnoreController", () => { + let controller: UnifiedIgnoreController + let consoleSpy: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + }) + + afterEach(() => { + if (controller) { + controller.dispose() + } + consoleSpy.mockRestore() + }) + + describe("Initialization", () => { + it("should initialize with .rooignore when it exists and has content", async () => { + // Setup mocks for .rooignore + mockFileExists.mockResolvedValueOnce(true) // .rooignore exists + mockReadFile.mockResolvedValueOnce("node_modules/\n*.log\n") + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + + expect(controller.isUsingRooIgnore()).toBe(true) + expect(controller.validateAccess("node_modules/package.json")).toBe(false) + expect(controller.validateAccess("src/index.ts")).toBe(true) + }) + + it("should fallback to .gitignore when .rooignore is missing", async () => { + // Setup mocks for missing .rooignore but existing .gitignore + mockFileExists.mockResolvedValueOnce(false) // .rooignore doesn't exist + mockAccess.mockResolvedValueOnce(undefined) // .gitignore exists + mockReadFile.mockResolvedValueOnce("dist/\nbuild/\n") + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + + expect(controller.isUsingRooIgnore()).toBe(false) + expect(controller.validateAccess("dist/main.js")).toBe(false) + expect(controller.validateAccess("src/index.ts")).toBe(true) + }) + + it("should fallback to .gitignore when .rooignore is empty", async () => { + // Setup mocks for empty .rooignore + mockFileExists.mockResolvedValueOnce(true) // .rooignore exists + mockReadFile.mockResolvedValueOnce(" \n\n ") // but is empty/whitespace + mockAccess.mockResolvedValueOnce(undefined) // .gitignore exists + mockReadFile.mockResolvedValueOnce("temp/\n*.tmp\n") + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + + expect(controller.isUsingRooIgnore()).toBe(false) + expect(controller.validateAccess("temp/file.txt")).toBe(false) + expect(controller.validateAccess("test.tmp")).toBe(false) + expect(controller.validateAccess("src/index.ts")).toBe(true) + }) + + it("should handle hierarchical .gitignore files", async () => { + // Setup mocks for missing .rooignore and hierarchical .gitignore + mockFileExists.mockResolvedValueOnce(false) // .rooignore doesn't exist + + // Mock hierarchical .gitignore discovery + mockAccess + .mockResolvedValueOnce(undefined) // /test/workspace/.gitignore exists + .mockRejectedValueOnce(new Error("not found")) // /test/.gitignore doesn't exist + .mockRejectedValueOnce(new Error("not found")) // /.gitignore doesn't exist + + mockReadFile.mockResolvedValueOnce("node_modules/\n*.log\n") // workspace .gitignore + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + + expect(controller.validateAccess("node_modules/package.json")).toBe(false) + expect(controller.validateAccess("debug.log")).toBe(false) + expect(controller.validateAccess("src/index.ts")).toBe(true) + }) + }) + + describe("File Access Validation", () => { + beforeEach(async () => { + // Setup with .rooignore content + mockFileExists.mockResolvedValue(true) + mockReadFile.mockResolvedValue("node_modules/\n*.log\nsecrets/\n") + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + }) + + it("should block access to ignored files", () => { + expect(controller.validateAccess("node_modules/package.json")).toBe(false) + expect(controller.validateAccess("debug.log")).toBe(false) + expect(controller.validateAccess("secrets/api-key.txt")).toBe(false) + }) + + it("should allow access to non-ignored files", () => { + expect(controller.validateAccess("src/index.ts")).toBe(true) + expect(controller.validateAccess("README.md")).toBe(true) + expect(controller.validateAccess("package.json")).toBe(true) + }) + + it("should handle relative paths correctly", () => { + expect(controller.validateAccess("./src/index.ts")).toBe(true) + expect(controller.validateAccess("../outside/file.txt")).toBe(true) // Outside cwd + }) + + it("should handle absolute paths by converting to relative", () => { + const absolutePath = path.join(TEST_CWD, "node_modules", "package.json") + expect(controller.validateAccess(absolutePath)).toBe(false) + }) + }) + + describe("Command Validation", () => { + beforeEach(async () => { + // Setup with .rooignore content + mockFileExists.mockResolvedValue(true) + mockReadFile.mockResolvedValue("secrets/\n*.env\n") + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + }) + + it("should block commands accessing ignored files", () => { + expect(controller.validateCommand("cat secrets/api-key.txt")).toBe("secrets/api-key.txt") + expect(controller.validateCommand("head .env")).toBe(".env") + expect(controller.validateCommand("grep password secrets/config.txt")).toBe("secrets/config.txt") + }) + + it("should allow commands accessing non-ignored files", () => { + expect(controller.validateCommand("cat src/index.ts")).toBeUndefined() + expect(controller.validateCommand("head README.md")).toBeUndefined() + }) + + it("should handle command flags correctly", () => { + expect(controller.validateCommand("cat -n src/index.ts")).toBeUndefined() + expect(controller.validateCommand("grep -r pattern src/")).toBeUndefined() + }) + + it("should allow non-file-reading commands", () => { + expect(controller.validateCommand("ls -la")).toBeUndefined() + expect(controller.validateCommand("mkdir new-dir")).toBeUndefined() + expect(controller.validateCommand("echo hello")).toBeUndefined() + }) + }) + + describe("Path Filtering", () => { + beforeEach(async () => { + // Setup with .rooignore content + mockFileExists.mockResolvedValue(true) + mockReadFile.mockResolvedValue("node_modules/\n*.log\ntemp/\n") + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + }) + + it("should filter out ignored paths", () => { + const paths = [ + "src/index.ts", + "node_modules/package.json", + "README.md", + "debug.log", + "temp/cache.txt", + "package.json", + ] + + const filtered = controller.filterPaths(paths) + + expect(filtered).toEqual(["src/index.ts", "README.md", "package.json"]) + }) + + it("should handle empty path arrays", () => { + expect(controller.filterPaths([])).toEqual([]) + }) + }) + + describe("Instructions Generation", () => { + it("should generate .rooignore instructions when using .rooignore", async () => { + mockFileExists.mockResolvedValue(true) + mockReadFile.mockResolvedValue("node_modules/\n*.log\n") + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + + const instructions = controller.getInstructions() + + expect(instructions).toContain("# .rooignore") + expect(instructions).toContain("node_modules/") + expect(instructions).toContain("*.log") + expect(instructions).toContain("🔒") + }) + + it("should generate .gitignore fallback instructions when using .gitignore", async () => { + mockFileExists.mockResolvedValue(false) // no .rooignore + mockAccess.mockResolvedValue(undefined) // .gitignore exists + mockReadFile.mockResolvedValue("dist/\nbuild/\n") + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + + const instructions = controller.getInstructions() + + expect(instructions).toContain("# .gitignore (fallback)") + expect(instructions).toContain("dist/") + expect(instructions).toContain("build/") + expect(instructions).toContain("🔒") + }) + + it("should return undefined when no ignore files exist", async () => { + mockFileExists.mockResolvedValue(false) // no .rooignore + mockAccess.mockRejectedValue(new Error("not found")) // no .gitignore + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + + expect(controller.getInstructions()).toBeUndefined() + }) + }) + + describe("Error Handling", () => { + it("should handle .rooignore read errors gracefully", async () => { + mockFileExists.mockResolvedValue(true) + mockReadFile.mockRejectedValueOnce(new Error("Permission denied")) + mockAccess.mockResolvedValue(undefined) // fallback to .gitignore + mockReadFile.mockResolvedValueOnce("dist/\n") + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + + // Should fallback to .gitignore + expect(controller.isUsingRooIgnore()).toBe(false) + expect(controller.validateAccess("dist/main.js")).toBe(false) + }) + + it("should handle .gitignore read errors gracefully", async () => { + mockFileExists.mockResolvedValue(false) // no .rooignore + mockAccess.mockResolvedValue(undefined) // .gitignore exists + mockReadFile.mockRejectedValue(new Error("Permission denied")) + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + + // Should allow all access when both files fail + expect(controller.validateAccess("any/file.txt")).toBe(true) + }) + + it("should handle path filtering errors gracefully", async () => { + mockFileExists.mockResolvedValue(true) + mockReadFile.mockResolvedValue("valid/pattern\n") + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + + // Mock an error in the filtering process + const originalValidateAccess = controller.validateAccess + vi.spyOn(controller, "validateAccess").mockImplementation(() => { + throw new Error("Validation error") + }) + + const result = controller.filterPaths(["test.txt"]) + + // Should return empty array on error (fail closed) + expect(result).toEqual([]) + + // Restore original method + controller.validateAccess = originalValidateAccess + }) + }) + + describe("Fallback Behavior", () => { + it("should prioritize .rooignore over .gitignore when both exist", async () => { + // Setup both files existing + mockFileExists.mockResolvedValue(true) // .rooignore exists + mockReadFile.mockResolvedValueOnce("roo-specific/\n") // .rooignore content + // .gitignore should not be read when .rooignore has content + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + + expect(controller.isUsingRooIgnore()).toBe(true) + expect(controller.validateAccess("roo-specific/file.txt")).toBe(false) + }) + + it("should use .gitignore when .rooignore exists but is empty", async () => { + mockFileExists.mockResolvedValue(true) // .rooignore exists + mockReadFile.mockResolvedValueOnce("") // but is empty + mockAccess.mockResolvedValue(undefined) // .gitignore exists + mockReadFile.mockResolvedValueOnce("git-ignored/\n") + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + + expect(controller.isUsingRooIgnore()).toBe(false) + expect(controller.validateAccess("git-ignored/file.txt")).toBe(false) + }) + + it("should allow all access when neither file exists", async () => { + mockFileExists.mockResolvedValue(false) // no .rooignore + mockAccess.mockRejectedValue(new Error("not found")) // no .gitignore + + controller = new UnifiedIgnoreController(TEST_CWD) + await controller.initialize() + + expect(controller.validateAccess("any/file.txt")).toBe(true) + expect(controller.validateCommand("cat any/file.txt")).toBeUndefined() + }) + }) +}) diff --git a/src/services/code-index/__tests__/manager.spec.ts b/src/services/code-index/__tests__/manager.spec.ts index 8c64c2fdc6..08a1f49753 100644 --- a/src/services/code-index/__tests__/manager.spec.ts +++ b/src/services/code-index/__tests__/manager.spec.ts @@ -3,17 +3,30 @@ import { CodeIndexServiceFactory } from "../service-factory" import type { MockedClass } from "vitest" // Mock vscode module -vi.mock("vscode", () => ({ - workspace: { - workspaceFolders: [ - { - uri: { fsPath: "/test/workspace" }, - name: "test", - index: 0, - }, - ], - }, -})) +vi.mock("vscode", () => { + const mockDisposable = { dispose: vi.fn() } + return { + workspace: { + workspaceFolders: [ + { + uri: { fsPath: "/test/workspace" }, + name: "test", + index: 0, + }, + ], + createFileSystemWatcher: vi.fn(() => ({ + onDidCreate: vi.fn(() => mockDisposable), + onDidChange: vi.fn(() => mockDisposable), + onDidDelete: vi.fn(() => mockDisposable), + dispose: vi.fn(), + })), + }, + RelativePattern: vi.fn().mockImplementation((base, pattern) => ({ + base, + pattern, + })), + } +}) // Mock only the essential dependencies vi.mock("../../../utils/path", () => ({ diff --git a/src/services/code-index/manager.ts b/src/services/code-index/manager.ts index 18e0752c34..4223d01a31 100644 --- a/src/services/code-index/manager.ts +++ b/src/services/code-index/manager.ts @@ -10,7 +10,6 @@ import { CodeIndexSearchService } from "./search-service" import { CodeIndexOrchestrator } from "./orchestrator" import { CacheManager } from "./cache-manager" import fs from "fs/promises" -import ignore from "ignore" import path from "path" import { t } from "../../i18n" import { TelemetryService } from "@roo-code/telemetry" @@ -236,7 +235,6 @@ export class CodeIndexManager { this._cacheManager!, ) - const ignoreInstance = ignore() const workspacePath = getWorkspacePath() if (!workspacePath) { @@ -244,20 +242,12 @@ export class CodeIndexManager { return } - const ignorePath = path.join(workspacePath, ".gitignore") - try { - const content = await fs.readFile(ignorePath, "utf8") - ignoreInstance.add(content) - ignoreInstance.add(".gitignore") - } catch (error) { - // Should never happen: reading file failed even though it exists - console.error("Unexpected error loading .gitignore:", error) - TelemetryService.instance.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, { - error: error instanceof Error ? error.message : String(error), - stack: error instanceof Error ? error.stack : undefined, - location: "_recreateServices", - }) - } + // Create unified ignore controller that handles both .gitignore and .rooignore + // with proper fallback behavior + const { UnifiedIgnoreController } = await import("../../core/ignore/UnifiedIgnoreController") + const unifiedIgnoreController = new UnifiedIgnoreController(workspacePath) + await unifiedIgnoreController.initialize() + const ignoreInstance = unifiedIgnoreController.getIgnoreInstance() // (Re)Create shared service instances const { embedder, vectorStore, scanner, fileWatcher } = this._serviceFactory.createServices( diff --git a/src/services/code-index/processors/__tests__/scanner.spec.ts b/src/services/code-index/processors/__tests__/scanner.spec.ts index f90a6c8159..2e441ab319 100644 --- a/src/services/code-index/processors/__tests__/scanner.spec.ts +++ b/src/services/code-index/processors/__tests__/scanner.spec.ts @@ -25,37 +25,50 @@ vi.mock("fs/promises", () => ({ })) // Create a simple mock for vscode since we can't access the real one -vi.mock("vscode", () => ({ - workspace: { - workspaceFolders: [ - { +vi.mock("vscode", () => { + const mockDisposable = { dispose: vi.fn() } + return { + workspace: { + workspaceFolders: [ + { + uri: { + fsPath: "/mock/workspace", + }, + }, + ], + getWorkspaceFolder: vi.fn().mockReturnValue({ uri: { fsPath: "/mock/workspace", }, + }), + fs: { + readFile: vi.fn().mockResolvedValue(Buffer.from("test content")), }, - ], - getWorkspaceFolder: vi.fn().mockReturnValue({ - uri: { - fsPath: "/mock/workspace", - }, - }), - fs: { - readFile: vi.fn().mockResolvedValue(Buffer.from("test content")), + createFileSystemWatcher: vi.fn(() => ({ + onDidCreate: vi.fn(() => mockDisposable), + onDidChange: vi.fn(() => mockDisposable), + onDidDelete: vi.fn(() => mockDisposable), + dispose: vi.fn(), + })), }, - }, - Uri: { - file: vi.fn().mockImplementation((path) => path), - }, - window: { - activeTextEditor: { - document: { - uri: { - fsPath: "/mock/workspace", + Uri: { + file: vi.fn().mockImplementation((path) => path), + }, + window: { + activeTextEditor: { + document: { + uri: { + fsPath: "/mock/workspace", + }, }, }, }, - }, -})) + RelativePattern: vi.fn().mockImplementation((base, pattern) => ({ + base, + pattern, + })), + } +}) vi.mock("../../../../core/ignore/RooIgnoreController") vi.mock("ignore") diff --git a/src/services/code-index/processors/scanner.ts b/src/services/code-index/processors/scanner.ts index 538a1252d7..37c5d3f86b 100644 --- a/src/services/code-index/processors/scanner.ts +++ b/src/services/code-index/processors/scanner.ts @@ -1,6 +1,6 @@ import { listFiles } from "../../glob/list-files" import { Ignore } from "ignore" -import { RooIgnoreController } from "../../../core/ignore/RooIgnoreController" +import { UnifiedIgnoreController } from "../../../core/ignore/UnifiedIgnoreController" import { stat } from "fs/promises" import * as path from "path" import { generateNormalizedAbsolutePath, generateRelativeFilePath } from "../shared/get-relative-path" @@ -62,12 +62,11 @@ export class DirectoryScanner implements IDirectoryScanner { // Filter out directories (marked with trailing '/') const filePaths = allPaths.filter((p) => !p.endsWith("/")) - // Initialize RooIgnoreController if not provided - const ignoreController = new RooIgnoreController(directoryPath) - + // Initialize UnifiedIgnoreController for consistent .gitignore/.rooignore handling + const ignoreController = new UnifiedIgnoreController(directoryPath) await ignoreController.initialize() - // Filter paths using .rooignore + // Filter paths using unified ignore patterns (.rooignore with .gitignore fallback) const allowedPaths = ignoreController.filterPaths(filePaths) // Filter by supported extensions, ignore patterns, and excluded directories diff --git a/src/services/glob/__tests__/gitignore-integration.spec.ts b/src/services/glob/__tests__/gitignore-integration.spec.ts index 1361816b17..f381c59d4c 100644 --- a/src/services/glob/__tests__/gitignore-integration.spec.ts +++ b/src/services/glob/__tests__/gitignore-integration.spec.ts @@ -9,11 +9,26 @@ vi.mock("../../ripgrep", () => ({ })) // Mock vscode -vi.mock("vscode", () => ({ - env: { - appRoot: "/mock/app/root", - }, -})) +vi.mock("vscode", () => { + const mockDisposable = { dispose: vi.fn() } + return { + env: { + appRoot: "/mock/app/root", + }, + workspace: { + createFileSystemWatcher: vi.fn(() => ({ + onDidCreate: vi.fn(() => mockDisposable), + onDidChange: vi.fn(() => mockDisposable), + onDidDelete: vi.fn(() => mockDisposable), + dispose: vi.fn(), + })), + }, + RelativePattern: vi.fn().mockImplementation((base, pattern) => ({ + base, + pattern, + })), + } +}) // Mock child_process to simulate ripgrep behavior vi.mock("child_process", () => ({ diff --git a/src/services/glob/__tests__/gitignore-test.spec.ts b/src/services/glob/__tests__/gitignore-test.spec.ts index cef884b584..af4e10cd36 100644 --- a/src/services/glob/__tests__/gitignore-test.spec.ts +++ b/src/services/glob/__tests__/gitignore-test.spec.ts @@ -9,11 +9,26 @@ vi.mock("../../ripgrep", () => ({ })) // Mock vscode -vi.mock("vscode", () => ({ - env: { - appRoot: "/mock/app/root", - }, -})) +vi.mock("vscode", () => { + const mockDisposable = { dispose: vi.fn() } + return { + env: { + appRoot: "/mock/app/root", + }, + workspace: { + createFileSystemWatcher: vi.fn(() => ({ + onDidCreate: vi.fn(() => mockDisposable), + onDidChange: vi.fn(() => mockDisposable), + onDidDelete: vi.fn(() => mockDisposable), + dispose: vi.fn(), + })), + }, + RelativePattern: vi.fn().mockImplementation((base, pattern) => ({ + base, + pattern, + })), + } +}) vi.mock("child_process", () => ({ spawn: vi.fn(), diff --git a/src/services/glob/list-files.ts b/src/services/glob/list-files.ts index 3164ed1eb0..9297c234a4 100644 --- a/src/services/glob/list-files.ts +++ b/src/services/glob/list-files.ts @@ -7,6 +7,7 @@ import ignore from "ignore" import { arePathsEqual } from "../../utils/path" import { getBinPath } from "../../services/ripgrep" import { DIRS_TO_IGNORE } from "./constants" +import { UnifiedIgnoreController } from "../../core/ignore/UnifiedIgnoreController" /** * List files in a directory, with optional recursive traversal @@ -35,8 +36,10 @@ export async function listFiles(dirPath: string, recursive: boolean, limit: numb // Get files using ripgrep const files = await listFilesWithRipgrep(rgPath, dirPath, recursive, limit) - // Get directories with proper filtering using ignore library - const ignoreInstance = await createIgnoreInstance(dirPath) + // Get directories with proper filtering using unified ignore controller + const unifiedIgnoreController = new UnifiedIgnoreController(dirPath) + await unifiedIgnoreController.initialize() + const ignoreInstance = unifiedIgnoreController.getIgnoreInstance() const directories = await listFilteredDirectories(dirPath, recursive, ignoreInstance) // Combine and format the results @@ -153,63 +156,9 @@ function buildNonRecursiveArgs(): string[] { return args } -/** - * Create an ignore instance that handles .gitignore files properly - * This replaces the custom gitignore parsing with the proper ignore library - */ -async function createIgnoreInstance(dirPath: string): Promise> { - const ignoreInstance = ignore() - const absolutePath = path.resolve(dirPath) - - // Find all .gitignore files from the target directory up to the root - const gitignoreFiles = await findGitignoreFiles(absolutePath) - - // Add patterns from all .gitignore files - for (const gitignoreFile of gitignoreFiles) { - try { - const content = await fs.promises.readFile(gitignoreFile, "utf8") - ignoreInstance.add(content) - } catch (err) { - // Continue if we can't read a .gitignore file - console.warn(`Error reading .gitignore at ${gitignoreFile}: ${err}`) - } - } - - // Always ignore .gitignore files themselves - ignoreInstance.add(".gitignore") - - return ignoreInstance -} - -/** - * Find all .gitignore files from the given directory up to the workspace root - */ -async function findGitignoreFiles(startPath: string): Promise { - const gitignoreFiles: string[] = [] - let currentPath = startPath - - // Walk up the directory tree looking for .gitignore files - while (currentPath && currentPath !== path.dirname(currentPath)) { - const gitignorePath = path.join(currentPath, ".gitignore") - - try { - await fs.promises.access(gitignorePath) - gitignoreFiles.push(gitignorePath) - } catch { - // .gitignore doesn't exist at this level, continue - } - - // Move up one directory - const parentPath = path.dirname(currentPath) - if (parentPath === currentPath) { - break // Reached root - } - currentPath = parentPath - } - - // Return in reverse order (root .gitignore first, then more specific ones) - return gitignoreFiles.reverse() -} +// Note: createIgnoreInstance and findGitignoreFiles functions have been replaced +// by the UnifiedIgnoreController which handles both .gitignore and .rooignore +// with proper fallback behavior /** * List directories with appropriate filtering @@ -312,7 +261,6 @@ function isDirectoryExplicitlyIgnored(dirName: string): boolean { return false } - /** * Combine file and directory results and format them properly */