mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-06 08:18:39 +00:00
feat: implement unified .gitignore/.rooignore handling for consistent file indexing
- Create UnifiedIgnoreController that combines .gitignore and .rooignore processing - Implement fallback behavior: .gitignore used when .rooignore is missing or empty - Update CodeIndexManager to use unified ignore patterns instead of separate systems - Update DirectoryScanner to use UnifiedIgnoreController for consistent filtering - Update list-files service to support .rooignore patterns via unified controller - Add comprehensive test suite with 23 test cases covering all functionality - Fix VSCode mocks in test files to include missing RelativePattern and file watcher APIs Fixes #5655: Resolves inconsistent .gitignore/.rooignore handling in codebase indexing
This commit is contained in:
parent
e84dd0a2cf
commit
70f59011aa
9 changed files with 767 additions and 125 deletions
311
src/core/ignore/UnifiedIgnoreController.ts
Normal file
311
src/core/ignore/UnifiedIgnoreController.ts
Normal file
|
|
@ -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<void> {
|
||||
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<void> {
|
||||
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<void> {
|
||||
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<string[]> {
|
||||
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 = []
|
||||
}
|
||||
}
|
||||
338
src/core/ignore/__tests__/UnifiedIgnoreController.spec.ts
Normal file
338
src/core/ignore/__tests__/UnifiedIgnoreController.spec.ts
Normal file
|
|
@ -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<typeof vi.spyOn>
|
||||
|
||||
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()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
@ -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", () => ({
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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", () => ({
|
||||
|
|
|
|||
|
|
@ -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(),
|
||||
|
|
|
|||
|
|
@ -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<ReturnType<typeof ignore>> {
|
||||
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<string[]> {
|
||||
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
|
||||
*/
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue