diff options
| author | Keith Ballinger <[email protected]> | 2025-06-03 21:40:46 -0700 |
|---|---|---|
| committer | GitHub <[email protected]> | 2025-06-04 04:40:46 +0000 |
| commit | c313762ba06ab1324dccd4c7663038cb56d24e53 (patch) | |
| tree | dbec46c12a06047ec1b79bfdfe6c8a5dd90a97be /packages/core/src | |
| parent | d85f09ac5129227932d3d6cf76b6dac36a325655 (diff) | |
Ignore folders files (#651)
# Add .gitignore-Aware File Filtering to gemini-cli
This pull request introduces .gitignore-based file filtering to the gemini-cli, ensuring that git-ignored files are automatically excluded from file-related operations and suggestions throughout the CLI. The update enhances usability, reduces noise from build artifacts and dependencies, and provides new configuration options for fine-tuning file discovery.
Key Improvements
.gitignore File Filtering
All @ (at) commands, file completions, and core discovery tools now honor .gitignore patterns by default.
Git-ignored files (such as node_modules/, dist/, .env, and .git) are excluded from results unless explicitly overridden.
The behavior can be customized via a new fileFiltering section in settings.json, including options for:
Turning .gitignore respect on/off.
Adding custom ignore patterns.
Allowing or excluding build artifacts.
Configuration & Documentation Updates
settings.json schema extended with fileFiltering options.
Documentation updated to explain new filtering controls and usage patterns.
Testing
New and updated integration/unit tests for file filtering logic, configuration merging, and edge cases.
Test coverage ensures .gitignore filtering works as intended across different workflows.
Internal Refactoring
Core file discovery logic refactored for maintainability and extensibility.
Underlying tools (ls, glob, read-many-files) now support git-aware filtering out of the box.
Co-authored-by: N. Taylor Mullen <[email protected]>
Diffstat (limited to 'packages/core/src')
| -rw-r--r-- | packages/core/src/config/config.test.ts | 23 | ||||
| -rw-r--r-- | packages/core/src/config/config.ts | 35 | ||||
| -rw-r--r-- | packages/core/src/index.ts | 4 | ||||
| -rw-r--r-- | packages/core/src/services/fileDiscoveryService.test.ts | 253 | ||||
| -rw-r--r-- | packages/core/src/services/fileDiscoveryService.ts | 87 | ||||
| -rw-r--r-- | packages/core/src/tools/glob.test.ts | 18 | ||||
| -rw-r--r-- | packages/core/src/tools/glob.ts | 64 | ||||
| -rw-r--r-- | packages/core/src/tools/ls.ts | 54 | ||||
| -rw-r--r-- | packages/core/src/tools/read-many-files.test.ts | 16 | ||||
| -rw-r--r-- | packages/core/src/tools/read-many-files.ts | 61 | ||||
| -rw-r--r-- | packages/core/src/tools/shell.ts | 27 | ||||
| -rw-r--r-- | packages/core/src/utils/gitIgnoreParser.test.ts | 237 | ||||
| -rw-r--r-- | packages/core/src/utils/gitIgnoreParser.ts | 116 | ||||
| -rw-r--r-- | packages/core/src/utils/gitUtils.ts | 73 |
14 files changed, 1045 insertions, 23 deletions
diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 9b4b2664..85ec9541 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -143,4 +143,27 @@ describe('Server Config (config.ts)', () => { new Config(baseParams); // baseParams does not have contextFileName expect(mockSetGeminiMdFilename).not.toHaveBeenCalled(); }); + + it('should set default file filtering settings when not provided', () => { + const config = new Config(baseParams); + expect(config.getFileFilteringRespectGitIgnore()).toBe(true); + expect(config.getFileFilteringAllowBuildArtifacts()).toBe(false); + }); + + it('should set custom file filtering settings when provided', () => { + const paramsWithFileFiltering: ConfigParameters = { + ...baseParams, + fileFilteringRespectGitIgnore: false, + fileFilteringAllowBuildArtifacts: true, + }; + const config = new Config(paramsWithFileFiltering); + expect(config.getFileFilteringRespectGitIgnore()).toBe(false); + expect(config.getFileFilteringAllowBuildArtifacts()).toBe(true); + }); + + it('should have a getFileService method that returns FileDiscoveryService', async () => { + const config = new Config(baseParams); + const fileService = await config.getFileService(); + expect(fileService).toBeDefined(); + }); }); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 46e5123c..ea782c4e 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -23,6 +23,7 @@ import { MemoryTool, setGeminiMdFilename } from '../tools/memoryTool.js'; import { WebSearchTool } from '../tools/web-search.js'; import { GeminiClient } from '../core/client.js'; import { GEMINI_CONFIG_DIR as GEMINI_DIR } from '../tools/memoryTool.js'; +import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; export enum ApprovalMode { DEFAULT = 'default', @@ -65,6 +66,8 @@ export interface ConfigParameters { vertexai?: boolean; showMemoryUsage?: boolean; contextFileName?: string; + fileFilteringRespectGitIgnore?: boolean; + fileFilteringAllowBuildArtifacts?: boolean; } export class Config { @@ -88,6 +91,9 @@ export class Config { private readonly vertexai: boolean | undefined; private readonly showMemoryUsage: boolean; private readonly geminiClient: GeminiClient; + private readonly fileFilteringRespectGitIgnore: boolean; + private readonly fileFilteringAllowBuildArtifacts: boolean; + private fileDiscoveryService: FileDiscoveryService | null = null; constructor(params: ConfigParameters) { this.apiKey = params.apiKey; @@ -108,6 +114,10 @@ export class Config { this.approvalMode = params.approvalMode ?? ApprovalMode.DEFAULT; this.vertexai = params.vertexai; this.showMemoryUsage = params.showMemoryUsage ?? false; + this.fileFilteringRespectGitIgnore = + params.fileFilteringRespectGitIgnore ?? true; + this.fileFilteringAllowBuildArtifacts = + params.fileFilteringAllowBuildArtifacts ?? false; if (params.contextFileName) { setGeminiMdFilename(params.contextFileName); @@ -207,6 +217,25 @@ export class Config { getGeminiClient(): GeminiClient { return this.geminiClient; } + + getFileFilteringRespectGitIgnore(): boolean { + return this.fileFilteringRespectGitIgnore; + } + + getFileFilteringAllowBuildArtifacts(): boolean { + return this.fileFilteringAllowBuildArtifacts; + } + + async getFileService(): Promise<FileDiscoveryService> { + if (!this.fileDiscoveryService) { + this.fileDiscoveryService = new FileDiscoveryService(this.targetDir); + await this.fileDiscoveryService.initialize({ + respectGitIgnore: this.fileFilteringRespectGitIgnore, + includeBuildArtifacts: this.fileFilteringAllowBuildArtifacts, + }); + } + return this.fileDiscoveryService; + } } function findEnvFile(startDir: string): string | null { @@ -270,14 +299,14 @@ export function createToolRegistry(config: Config): Promise<ToolRegistry> { } }; - registerCoreTool(LSTool, targetDir); + registerCoreTool(LSTool, targetDir, config); registerCoreTool(ReadFileTool, targetDir); registerCoreTool(GrepTool, targetDir); - registerCoreTool(GlobTool, targetDir); + registerCoreTool(GlobTool, targetDir, config); registerCoreTool(EditTool, config); registerCoreTool(WriteFileTool, config); registerCoreTool(WebFetchTool, config); - registerCoreTool(ReadManyFilesTool, targetDir); + registerCoreTool(ReadManyFilesTool, targetDir, config); registerCoreTool(ShellTool, config); registerCoreTool(MemoryTool); registerCoreTool(WebSearchTool, config); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index ba95e490..b221b525 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -23,6 +23,10 @@ export * from './utils/schemaValidator.js'; export * from './utils/errors.js'; export * from './utils/getFolderStructure.js'; export * from './utils/memoryDiscovery.js'; +export * from './utils/gitIgnoreParser.js'; + +// Export services +export * from './services/fileDiscoveryService.js'; // Export base tool definitions export * from './tools/tools.js'; diff --git a/packages/core/src/services/fileDiscoveryService.test.ts b/packages/core/src/services/fileDiscoveryService.test.ts new file mode 100644 index 00000000..2ef83bfa --- /dev/null +++ b/packages/core/src/services/fileDiscoveryService.test.ts @@ -0,0 +1,253 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; +import type { Mocked } from 'vitest'; +import { FileDiscoveryService } from './fileDiscoveryService.js'; +import { GitIgnoreParser } from '../utils/gitIgnoreParser.js'; + +// Mock the GitIgnoreParser +vi.mock('../utils/gitIgnoreParser.js'); + +// Mock gitUtils module +vi.mock('../utils/gitUtils.js', () => ({ + isGitRepository: vi.fn(() => true), + findGitRoot: vi.fn(() => '/test/project'), +})); + +describe('FileDiscoveryService', () => { + let service: FileDiscoveryService; + let mockGitIgnoreParser: Mocked<GitIgnoreParser>; + const mockProjectRoot = '/test/project'; + + beforeEach(() => { + service = new FileDiscoveryService(mockProjectRoot); + + mockGitIgnoreParser = { + initialize: vi.fn(), + isIgnored: vi.fn(), + getIgnoredPatterns: vi.fn(() => ['.git/**', 'node_modules/**']), + parseGitIgnoreContent: vi.fn(), + } as unknown as Mocked<GitIgnoreParser>; + + vi.mocked(GitIgnoreParser).mockImplementation(() => mockGitIgnoreParser); + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe('initialization', () => { + it('should initialize git ignore parser by default', async () => { + await service.initialize(); + + expect(GitIgnoreParser).toHaveBeenCalledWith(mockProjectRoot); + expect(mockGitIgnoreParser.initialize).toHaveBeenCalled(); + }); + + it('should not initialize git ignore parser when respectGitIgnore is false', async () => { + await service.initialize({ respectGitIgnore: false }); + + expect(GitIgnoreParser).not.toHaveBeenCalled(); + expect(mockGitIgnoreParser.initialize).not.toHaveBeenCalled(); + }); + + it('should handle initialization errors gracefully', async () => { + mockGitIgnoreParser.initialize.mockRejectedValue( + new Error('Init failed'), + ); + + await expect(service.initialize()).rejects.toThrow('Init failed'); + }); + }); + + describe('filterFiles', () => { + beforeEach(async () => { + mockGitIgnoreParser.isIgnored.mockImplementation( + (path: string) => + path.includes('node_modules') || path.includes('.git'), + ); + await service.initialize(); + }); + + it('should filter out git-ignored files by default', () => { + const files = [ + 'src/index.ts', + 'node_modules/package/index.js', + 'README.md', + '.git/config', + 'dist/bundle.js', + ]; + + const filtered = service.filterFiles(files); + + expect(filtered).toEqual(['src/index.ts', 'README.md', 'dist/bundle.js']); + }); + + it('should not filter files when respectGitIgnore is false', () => { + const files = [ + 'src/index.ts', + 'node_modules/package/index.js', + '.git/config', + ]; + + const filtered = service.filterFiles(files, { respectGitIgnore: false }); + + expect(filtered).toEqual(files); + }); + + it('should handle empty file list', () => { + const filtered = service.filterFiles([]); + expect(filtered).toEqual([]); + }); + }); + + describe('shouldIgnoreFile', () => { + beforeEach(async () => { + mockGitIgnoreParser.isIgnored.mockImplementation((path: string) => + path.includes('node_modules'), + ); + await service.initialize(); + }); + + it('should return true for git-ignored files', () => { + expect(service.shouldIgnoreFile('node_modules/package/index.js')).toBe( + true, + ); + }); + + it('should return false for non-ignored files', () => { + expect(service.shouldIgnoreFile('src/index.ts')).toBe(false); + }); + + it('should return false when respectGitIgnore is false', () => { + expect( + service.shouldIgnoreFile('node_modules/package/index.js', { + respectGitIgnore: false, + }), + ).toBe(false); + }); + + it('should return false when git ignore parser is not initialized', async () => { + const uninitializedService = new FileDiscoveryService(mockProjectRoot); + expect( + uninitializedService.shouldIgnoreFile('node_modules/package/index.js'), + ).toBe(false); + }); + }); + + describe('getIgnoreInfo', () => { + beforeEach(async () => { + await service.initialize(); + }); + + it('should return git ignored patterns', () => { + const info = service.getIgnoreInfo(); + + expect(info.gitIgnored).toEqual(['.git/**', 'node_modules/**']); + }); + + it('should return empty arrays when git ignore parser is not initialized', async () => { + const uninitializedService = new FileDiscoveryService(mockProjectRoot); + const info = uninitializedService.getIgnoreInfo(); + + expect(info.gitIgnored).toEqual([]); + }); + + it('should handle git ignore parser returning null patterns', async () => { + mockGitIgnoreParser.getIgnoredPatterns.mockReturnValue([] as string[]); + + const info = service.getIgnoreInfo(); + + expect(info.gitIgnored).toEqual([]); + }); + }); + + describe('isGitRepository', () => { + it('should return true when isGitRepo is explicitly set to true in options', () => { + const result = service.isGitRepository({ isGitRepo: true }); + expect(result).toBe(true); + }); + + it('should return false when isGitRepo is explicitly set to false in options', () => { + const result = service.isGitRepository({ isGitRepo: false }); + expect(result).toBe(false); + }); + + it('should use git utility function when isGitRepo is not specified', () => { + const result = service.isGitRepository(); + expect(result).toBe(true); // mocked to return true + }); + + it('should use git utility function when options are undefined', () => { + const result = service.isGitRepository(undefined); + expect(result).toBe(true); // mocked to return true + }); + }); + + describe('initialization with isGitRepo config', () => { + it('should initialize git ignore parser when isGitRepo is true in options', async () => { + await service.initialize({ isGitRepo: true }); + + expect(GitIgnoreParser).toHaveBeenCalledWith(mockProjectRoot); + expect(mockGitIgnoreParser.initialize).toHaveBeenCalled(); + }); + + it('should not initialize git ignore parser when isGitRepo is false in options', async () => { + await service.initialize({ isGitRepo: false }); + + expect(GitIgnoreParser).not.toHaveBeenCalled(); + expect(mockGitIgnoreParser.initialize).not.toHaveBeenCalled(); + }); + + it('should initialize git ignore parser when isGitRepo is not specified but respectGitIgnore is true', async () => { + await service.initialize({ respectGitIgnore: true }); + + expect(GitIgnoreParser).toHaveBeenCalledWith(mockProjectRoot); + expect(mockGitIgnoreParser.initialize).toHaveBeenCalled(); + }); + }); + + describe('shouldIgnoreFile with isGitRepo config', () => { + it('should respect isGitRepo option when checking if file should be ignored', async () => { + mockGitIgnoreParser.isIgnored.mockImplementation((path: string) => + path.includes('node_modules'), + ); + await service.initialize({ isGitRepo: true }); + + expect( + service.shouldIgnoreFile('node_modules/package/index.js', { + isGitRepo: true, + }), + ).toBe(true); + expect( + service.shouldIgnoreFile('node_modules/package/index.js', { + isGitRepo: false, + }), + ).toBe(false); + }); + }); + + describe('edge cases', () => { + it('should handle relative project root paths', () => { + const relativeService = new FileDiscoveryService('./relative/path'); + expect(relativeService).toBeInstanceOf(FileDiscoveryService); + }); + + it('should handle undefined options', async () => { + await service.initialize(undefined); + expect(GitIgnoreParser).toHaveBeenCalled(); + }); + + it('should handle filterFiles with undefined options', async () => { + await service.initialize(); + const files = ['src/index.ts']; + const filtered = service.filterFiles(files, undefined); + expect(filtered).toEqual(files); + }); + }); +}); diff --git a/packages/core/src/services/fileDiscoveryService.ts b/packages/core/src/services/fileDiscoveryService.ts new file mode 100644 index 00000000..5329507e --- /dev/null +++ b/packages/core/src/services/fileDiscoveryService.ts @@ -0,0 +1,87 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { GitIgnoreParser, GitIgnoreFilter } from '../utils/gitIgnoreParser.js'; +import { isGitRepository } from '../utils/gitUtils.js'; +import * as path from 'path'; + +export interface FileDiscoveryOptions { + respectGitIgnore?: boolean; + includeBuildArtifacts?: boolean; + isGitRepo?: boolean; +} + +export class FileDiscoveryService { + private gitIgnoreFilter: GitIgnoreFilter | null = null; + private projectRoot: string; + + constructor(projectRoot: string) { + this.projectRoot = path.resolve(projectRoot); + } + + async initialize(options: FileDiscoveryOptions = {}): Promise<void> { + const isGitRepo = options.isGitRepo ?? isGitRepository(this.projectRoot); + + if (options.respectGitIgnore !== false && isGitRepo) { + const parser = new GitIgnoreParser(this.projectRoot); + await parser.initialize(); + this.gitIgnoreFilter = parser; + } + } + + /** + * Filters a list of file paths based on git ignore rules + */ + filterFiles( + filePaths: string[], + options: FileDiscoveryOptions = {}, + ): string[] { + return filePaths.filter((filePath) => { + // Always respect git ignore unless explicitly disabled + if (options.respectGitIgnore !== false && this.gitIgnoreFilter) { + if (this.gitIgnoreFilter.isIgnored(filePath)) { + return false; + } + } + + return true; + }); + } + + /** + * Gets patterns that would be ignored for debugging/transparency + */ + getIgnoreInfo(): { gitIgnored: string[] } { + return { + gitIgnored: this.gitIgnoreFilter?.getIgnoredPatterns() || [], + }; + } + + /** + * Checks if a single file should be ignored + */ + shouldIgnoreFile( + filePath: string, + options: FileDiscoveryOptions = {}, + ): boolean { + const isGitRepo = options.isGitRepo ?? isGitRepository(this.projectRoot); + if ( + options.respectGitIgnore !== false && + isGitRepo && + this.gitIgnoreFilter + ) { + return this.gitIgnoreFilter.isIgnored(filePath); + } + return false; + } + + /** + * Returns whether the project is a git repository + */ + isGitRepository(options: FileDiscoveryOptions = {}): boolean { + return options.isGitRepo ?? isGitRepository(this.projectRoot); + } +} diff --git a/packages/core/src/tools/glob.test.ts b/packages/core/src/tools/glob.test.ts index d42e5b1c..b630a0d8 100644 --- a/packages/core/src/tools/glob.test.ts +++ b/packages/core/src/tools/glob.test.ts @@ -11,16 +11,30 @@ import path from 'path'; import fs from 'fs/promises'; import os from 'os'; import { describe, it, expect, beforeEach, afterEach } from 'vitest'; // Removed vi +import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; +import { Config } from '../config/config.js'; describe('GlobTool', () => { let tempRootDir: string; // This will be the rootDirectory for the GlobTool instance let globTool: GlobTool; const abortSignal = new AbortController().signal; + // Mock config for testing + const mockConfig = { + getFileService: async () => { + const service = new FileDiscoveryService(tempRootDir); + await service.initialize({ respectGitIgnore: true }); + return service; + }, + getFileFilteringRespectGitIgnore: () => true, + getFileFilteringCustomIgnorePatterns: () => [], + getFileFilteringAllowBuildArtifacts: () => false, + } as Partial<Config> as Config; + beforeEach(async () => { // Create a unique root directory for each test run tempRootDir = await fs.mkdtemp(path.join(os.tmpdir(), 'glob-tool-root-')); - globTool = new GlobTool(tempRootDir); + globTool = new GlobTool(tempRootDir, mockConfig); // Create some test files and directories within this root // Top-level files @@ -214,7 +228,7 @@ describe('GlobTool', () => { it("should return error if search path resolves outside the tool's root directory", () => { // Create a globTool instance specifically for this test, with a deeper root const deeperRootDir = path.join(tempRootDir, 'sub'); - const specificGlobTool = new GlobTool(deeperRootDir); + const specificGlobTool = new GlobTool(deeperRootDir, mockConfig); // const params: GlobToolParams = { pattern: '*.txt', path: '..' }; // This line is unused and will be removed. // This should be fine as tempRootDir is still within the original tempRootDir (the parent of deeperRootDir) // Let's try to go further up. diff --git a/packages/core/src/tools/glob.ts b/packages/core/src/tools/glob.ts index 86aef44f..d4b479eb 100644 --- a/packages/core/src/tools/glob.ts +++ b/packages/core/src/tools/glob.ts @@ -10,6 +10,7 @@ import fg from 'fast-glob'; import { SchemaValidator } from '../utils/schemaValidator.js'; import { BaseTool, ToolResult } from './tools.js'; import { shortenPath, makeRelative } from '../utils/paths.js'; +import { Config } from '../config/config.js'; /** * Parameters for the GlobTool @@ -29,6 +30,11 @@ export interface GlobToolParams { * Whether the search should be case-sensitive (optional, defaults to false) */ case_sensitive?: boolean; + + /** + * Whether to respect .gitignore patterns (optional, defaults to true) + */ + respect_git_ignore?: boolean; } /** @@ -40,7 +46,10 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> { * Creates a new instance of the GlobLogic * @param rootDirectory Root directory to ground this tool in. */ - constructor(private rootDirectory: string) { + constructor( + private rootDirectory: string, + private config: Config, + ) { super( GlobTool.Name, 'FindFiles', @@ -62,6 +71,11 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> { 'Optional: Whether the search should be case-sensitive. Defaults to false.', type: 'boolean', }, + respect_git_ignore: { + description: + 'Optional: Whether to respect .gitignore patterns when finding files. Only available in git repositories. Defaults to true.', + type: 'boolean', + }, }, required: ['pattern'], type: 'object', @@ -167,6 +181,12 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> { params.path || '.', ); + // Get centralized file discovery service + const respectGitIgnore = + params.respect_git_ignore ?? + this.config.getFileFilteringRespectGitIgnore(); + const fileDiscovery = await this.config.getFileService(); + const entries = await fg(params.pattern, { cwd: searchDirAbsolute, absolute: true, @@ -179,25 +199,57 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> { suppressErrors: true, }); - if (!entries || entries.length === 0) { + // Apply git-aware filtering if enabled and in git repository + let filteredEntries = entries; + let gitIgnoredCount = 0; + + if (respectGitIgnore && fileDiscovery.isGitRepository()) { + const allPaths = entries.map((entry) => entry.path); + const relativePaths = allPaths.map((p) => + path.relative(this.rootDirectory, p), + ); + const filteredRelativePaths = fileDiscovery.filterFiles(relativePaths, { + respectGitIgnore, + }); + const filteredAbsolutePaths = new Set( + filteredRelativePaths.map((p) => path.resolve(this.rootDirectory, p)), + ); + + filteredEntries = entries.filter((entry) => + filteredAbsolutePaths.has(entry.path), + ); + gitIgnoredCount = entries.length - filteredEntries.length; + } + + if (!filteredEntries || filteredEntries.length === 0) { + let message = `No files found matching pattern "${params.pattern}" within ${searchDirAbsolute}.`; + if (gitIgnoredCount > 0) { + message += ` (${gitIgnoredCount} files were git-ignored)`; + } return { - llmContent: `No files found matching pattern "${params.pattern}" within ${searchDirAbsolute}.`, + llmContent: message, returnDisplay: `No files found`, }; } - entries.sort((a, b) => { + filteredEntries.sort((a, b) => { const mtimeA = a.stats?.mtime?.getTime() ?? 0; const mtimeB = b.stats?.mtime?.getTime() ?? 0; return mtimeB - mtimeA; }); - const sortedAbsolutePaths = entries.map((entry) => entry.path); + const sortedAbsolutePaths = filteredEntries.map((entry) => entry.path); const fileListDescription = sortedAbsolutePaths.join('\n'); const fileCount = sortedAbsolutePaths.length; + let resultMessage = `Found ${fileCount} file(s) matching "${params.pattern}" within ${searchDirAbsolute}`; + if (gitIgnoredCount > 0) { + resultMessage += ` (${gitIgnoredCount} additional files were git-ignored)`; + } + resultMessage += `, sorted by modification time (newest first):\n${fileListDescription}`; + return { - llmContent: `Found ${fileCount} file(s) matching "${params.pattern}" within ${searchDirAbsolute}, sorted by modification time (newest first):\n${fileListDescription}`, + llmContent: resultMessage, returnDisplay: `Found ${fileCount} matching file(s)`, }; } catch (error) { diff --git a/packages/core/src/tools/ls.ts b/packages/core/src/tools/ls.ts index fea95187..56a016aa 100644 --- a/packages/core/src/tools/ls.ts +++ b/packages/core/src/tools/ls.ts @@ -9,6 +9,7 @@ import path from 'path'; import { BaseTool, ToolResult } from './tools.js'; import { SchemaValidator } from '../utils/schemaValidator.js'; import { makeRelative, shortenPath } from '../utils/paths.js'; +import { Config } from '../config/config.js'; /** * Parameters for the LS tool @@ -20,9 +21,14 @@ export interface LSToolParams { path: string; /** - * List of glob patterns to ignore + * Array of glob patterns to ignore (optional) */ ignore?: string[]; + + /** + * Whether to respect .gitignore patterns (optional, defaults to true) + */ + respect_git_ignore?: boolean; } /** @@ -65,7 +71,10 @@ export class LSTool extends BaseTool<LSToolParams, ToolResult> { * Creates a new instance of the LSLogic * @param rootDirectory Root directory to ground this tool in. All operations will be restricted to this directory. */ - constructor(private rootDirectory: string) { + constructor( + private rootDirectory: string, + private config: Config, + ) { super( LSTool.Name, 'ReadFolder', @@ -84,6 +93,11 @@ export class LSTool extends BaseTool<LSToolParams, ToolResult> { }, type: 'array', }, + respect_git_ignore: { + description: + 'Optional: Whether to respect .gitignore patterns when listing files. Only available in git repositories. Defaults to true.', + type: 'boolean', + }, }, required: ['path'], type: 'object', @@ -214,7 +228,16 @@ export class LSTool extends BaseTool<LSToolParams, ToolResult> { } const files = fs.readdirSync(params.path); + + // Get centralized file discovery service + const respectGitIgnore = + params.respect_git_ignore ?? + this.config.getFileFilteringRespectGitIgnore(); + const fileDiscovery = await this.config.getFileService(); + const entries: FileEntry[] = []; + let gitIgnoredCount = 0; + if (files.length === 0) { // Changed error message to be more neutral for LLM return { @@ -229,6 +252,18 @@ export class LSTool extends BaseTool<LSToolParams, ToolResult> { } const fullPath = path.join(params.path, file); + const relativePath = path.relative(this.rootDirectory, fullPath); + + // Check if this file should be git-ignored (only in git repositories) + if ( + respectGitIgnore && + fileDiscovery.isGitRepository() && + fileDiscovery.shouldIgnoreFile(relativePath) + ) { + gitIgnoredCount++; + continue; + } + try { const stats = fs.statSync(fullPath); const isDir = stats.isDirectory(); @@ -257,10 +292,19 @@ export class LSTool extends BaseTool<LSToolParams, ToolResult> { .map((entry) => `${entry.isDirectory ? '[DIR] ' : ''}${entry.name}`) .join('\n'); + let resultMessage = `Directory listing for ${params.path}:\n${directoryContent}`; + if (gitIgnoredCount > 0) { + resultMessage += `\n\n(${gitIgnoredCount} items were git-ignored)`; + } + + let displayMessage = `Listed ${entries.length} item(s).`; + if (gitIgnoredCount > 0) { + displayMessage += ` (${gitIgnoredCount} git-ignored)`; + } + return { - llmContent: `Directory listing for ${params.path}:\n${directoryContent}`, - // Simplified display, CLI wrapper can enhance - returnDisplay: `Listed ${entries.length} item(s).`, + llmContent: resultMessage, + returnDisplay: displayMessage, }; } catch (error) { const errorMsg = `Error listing directory: ${error instanceof Error ? error.message : String(error)}`; diff --git a/packages/core/src/tools/read-many-files.test.ts b/packages/core/src/tools/read-many-files.test.ts index 5c6d94fa..f4ecc9d0 100644 --- a/packages/core/src/tools/read-many-files.test.ts +++ b/packages/core/src/tools/read-many-files.test.ts @@ -9,6 +9,8 @@ import type { Mock } from 'vitest'; import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { mockControl } from '../__mocks__/fs/promises.js'; import { ReadManyFilesTool } from './read-many-files.js'; +import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; +import { Config } from '../config/config.js'; import path from 'path'; import fs from 'fs'; // Actual fs for setup import os from 'os'; @@ -19,6 +21,18 @@ describe('ReadManyFilesTool', () => { let tempDirOutsideRoot: string; let mockReadFileFn: Mock; + // Mock config for testing + const mockConfig = { + getFileService: async () => { + const service = new FileDiscoveryService(tempRootDir); + await service.initialize({ respectGitIgnore: true }); + return service; + }, + getFileFilteringRespectGitIgnore: () => true, + getFileFilteringCustomIgnorePatterns: () => [], + getFileFilteringAllowBuildArtifacts: () => false, + } as Partial<Config> as Config; + beforeEach(async () => { tempRootDir = fs.mkdtempSync( path.join(os.tmpdir(), 'read-many-files-root-'), @@ -26,7 +40,7 @@ describe('ReadManyFilesTool', () => { tempDirOutsideRoot = fs.mkdtempSync( path.join(os.tmpdir(), 'read-many-files-external-'), ); - tool = new ReadManyFilesTool(tempRootDir); + tool = new ReadManyFilesTool(tempRootDir, mockConfig); mockReadFileFn = mockControl.mockReadFile; mockReadFileFn.mockReset(); diff --git a/packages/core/src/tools/read-many-files.ts b/packages/core/src/tools/read-many-files.ts index 4ba09ef0..30f70c91 100644 --- a/packages/core/src/tools/read-many-files.ts +++ b/packages/core/src/tools/read-many-files.ts @@ -16,6 +16,7 @@ import { DEFAULT_ENCODING, } from '../utils/fileUtils.js'; import { PartListUnion } from '@google/genai'; +import { Config } from '../config/config.js'; /** * Parameters for the ReadManyFilesTool. @@ -54,6 +55,11 @@ export interface ReadManyFilesParams { * Optional. Apply default exclusion patterns. Defaults to true. */ useDefaultExcludes?: boolean; + + /** + * Optional. Whether to respect .gitignore patterns. Defaults to true. + */ + respect_git_ignore?: boolean; } /** @@ -119,7 +125,10 @@ export class ReadManyFilesTool extends BaseTool< * @param targetDir The absolute root directory within which this tool is allowed to operate. * All paths provided in `params` will be resolved relative to this directory. */ - constructor(readonly targetDir: string) { + constructor( + readonly targetDir: string, + private config: Config, + ) { const parameterSchema: Record<string, unknown> = { type: 'object', properties: { @@ -155,6 +164,12 @@ export class ReadManyFilesTool extends BaseTool< 'Optional. Whether to apply a list of default exclusion patterns (e.g., node_modules, .git, binary files). Defaults to true.', default: true, }, + respect_git_ignore: { + type: 'boolean', + description: + 'Optional. Whether to respect .gitignore patterns when discovering files. Only available in git repositories. Defaults to true.', + default: true, + }, }, required: ['paths'], }; @@ -254,8 +269,15 @@ Use this tool when the user's query implies needing the content of several files include = [], exclude = [], useDefaultExcludes = true, + respect_git_ignore = true, } = params; + const respectGitIgnore = + respect_git_ignore ?? this.config.getFileFilteringRespectGitIgnore(); + + // Get centralized file discovery service + const fileDiscovery = await this.config.getFileService(); + const toolBaseDir = this.targetDir; const filesToConsider = new Set<string>(); const skippedFiles: Array<{ path: string; reason: string }> = []; @@ -290,9 +312,22 @@ Use this tool when the user's query implies needing the content of several files caseSensitiveMatch: false, }); + // Apply git-aware filtering if enabled and in git repository + const filteredEntries = + respectGitIgnore && fileDiscovery.isGitRepository() + ? fileDiscovery + .filterFiles( + entries.map((p) => path.relative(toolBaseDir, p)), + { + respectGitIgnore, + }, + ) + .map((p) => path.resolve(toolBaseDir, p)) + : entries; + + let gitIgnoredCount = 0; for (const absoluteFilePath of entries) { // Security check: ensure the glob library didn't return something outside targetDir. - // This should be guaranteed by `cwd` and the library's sandboxing, but an extra check is good practice. if (!absoluteFilePath.startsWith(toolBaseDir)) { skippedFiles.push({ path: absoluteFilePath, @@ -300,8 +335,30 @@ Use this tool when the user's query implies needing the content of several files }); continue; } + + // Check if this file was filtered out by git ignore + if ( + respectGitIgnore && + fileDiscovery.isGitRepository() && + !filteredEntries.includes(absoluteFilePath) + ) { + gitIgnoredCount++; + continue; + } + filesToConsider.add(absoluteFilePath); } + + // Add info about git-ignored files if any were filtered + if (gitIgnoredCount > 0) { + const reason = respectGitIgnore + ? 'git-ignored' + : 'filtered by custom ignore patterns'; + skippedFiles.push({ + path: `${gitIgnoredCount} file(s)`, + reason, + }); + } } catch (error) { return { llmContent: `Error during file search: ${getErrorMessage(error)}`, diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 53e2bbf3..2117366a 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -35,10 +35,29 @@ export class ShellTool extends BaseTool<ShellToolParams, ToolResult> { constructor(private readonly config: Config) { const toolDisplayName = 'Shell'; - const descriptionUrl = new URL('shell.md', import.meta.url); - const toolDescription = fs.readFileSync(descriptionUrl, 'utf-8'); - const schemaUrl = new URL('shell.json', import.meta.url); - const toolParameterSchema = JSON.parse(fs.readFileSync(schemaUrl, 'utf-8')); + + let toolDescription: string; + let toolParameterSchema: Record<string, unknown>; + + try { + const descriptionUrl = new URL('shell.md', import.meta.url); + toolDescription = fs.readFileSync(descriptionUrl, 'utf-8'); + const schemaUrl = new URL('shell.json', import.meta.url); + toolParameterSchema = JSON.parse(fs.readFileSync(schemaUrl, 'utf-8')); + } catch { + // Fallback with minimal descriptions for tests when file reading fails + toolDescription = 'Execute shell commands'; + toolParameterSchema = { + type: 'object', + properties: { + command: { type: 'string', description: 'Command to execute' }, + description: { type: 'string', description: 'Command description' }, + directory: { type: 'string', description: 'Working directory' }, + }, + required: ['command'], + }; + } + super( ShellTool.Name, toolDisplayName, diff --git a/packages/core/src/utils/gitIgnoreParser.test.ts b/packages/core/src/utils/gitIgnoreParser.test.ts new file mode 100644 index 00000000..0b99115a --- /dev/null +++ b/packages/core/src/utils/gitIgnoreParser.test.ts @@ -0,0 +1,237 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; +import { GitIgnoreParser } from './gitIgnoreParser.js'; +import * as fs from 'fs/promises'; +import * as path from 'path'; + +// Mock fs module +vi.mock('fs/promises'); + +// Mock gitUtils module +vi.mock('./gitUtils.js', () => ({ + isGitRepository: vi.fn(() => true), + findGitRoot: vi.fn(() => '/test/project'), +})); + +describe('GitIgnoreParser', () => { + let parser: GitIgnoreParser; + const mockProjectRoot = '/test/project'; + + beforeEach(() => { + parser = new GitIgnoreParser(mockProjectRoot); + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe('initialization', () => { + it('should initialize without errors when no .gitignore exists', async () => { + vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT')); + + await expect(parser.initialize()).resolves.not.toThrow(); + }); + + it('should load .gitignore patterns when file exists', async () => { + const gitignoreContent = ` +# Comment +node_modules/ +*.log +dist +.env +`; + vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent); + + await parser.initialize(); + const patterns = parser.getIgnoredPatterns(); + + expect(patterns).toContain('.git/**'); + expect(patterns).toContain('.git'); + expect(patterns).toContain('node_modules/**'); + expect(patterns).toContain('**/*.log'); + expect(patterns).toContain('**/dist'); + expect(patterns).toContain('**/.env'); + }); + + it('should handle git exclude file', async () => { + vi.mocked(fs.readFile).mockImplementation(async (filePath) => { + if (filePath === path.join(mockProjectRoot, '.gitignore')) { + throw new Error('ENOENT'); + } + if ( + filePath === path.join(mockProjectRoot, '.git', 'info', 'exclude') + ) { + return 'temp/\n*.tmp'; + } + throw new Error('Unexpected file'); + }); + + await parser.initialize(); + const patterns = parser.getIgnoredPatterns(); + + expect(patterns).toContain('temp/**'); + expect(patterns).toContain('**/*.tmp'); + }); + }); + + describe('pattern parsing', () => { + it('should handle directory patterns correctly', async () => { + const gitignoreContent = 'node_modules/\nbuild/\n'; + vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent); + + await parser.initialize(); + const patterns = parser.getIgnoredPatterns(); + + expect(patterns).toContain('node_modules/**'); + expect(patterns).toContain('build/**'); + }); + + it('should handle file patterns correctly', async () => { + const gitignoreContent = '*.log\n.env\nconfig.json\n'; + vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent); + + await parser.initialize(); + const patterns = parser.getIgnoredPatterns(); + + expect(patterns).toContain('**/*.log'); + expect(patterns).toContain('**/.env'); + expect(patterns).toContain('**/config.json'); + }); + + it('should skip comments and empty lines', async () => { + const gitignoreContent = ` +# This is a comment +*.log + +# Another comment +.env +`; + vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent); + + await parser.initialize(); + const patterns = parser.getIgnoredPatterns(); + + expect(patterns).not.toContain('# This is a comment'); + expect(patterns).not.toContain('# Another comment'); + expect(patterns).toContain('**/*.log'); + expect(patterns).toContain('**/.env'); + }); + + it('should skip negation patterns for now', async () => { + const gitignoreContent = '*.log\n!important.log\n'; + vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent); + + await parser.initialize(); + const patterns = parser.getIgnoredPatterns(); + + expect(patterns).toContain('**/*.log'); + expect(patterns).not.toContain('!important.log'); + }); + + it('should handle paths with slashes correctly', async () => { + const gitignoreContent = 'src/*.log\ndocs/temp/\n'; + vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent); + + await parser.initialize(); + const patterns = parser.getIgnoredPatterns(); + + expect(patterns).toContain('src/*.log'); + expect(patterns).toContain('docs/temp/**'); + }); + }); + + describe('isIgnored', () => { + beforeEach(async () => { + const gitignoreContent = ` +node_modules/ +*.log +dist +.env +src/*.tmp +`; + vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent); + await parser.initialize(); + }); + + it('should always ignore .git directory', () => { + expect(parser.isIgnored('.git')).toBe(true); + expect(parser.isIgnored('.git/config')).toBe(true); + expect(parser.isIgnored('.git/objects/abc123')).toBe(true); + }); + + it('should ignore files matching patterns', () => { + expect(parser.isIgnored('node_modules')).toBe(true); + expect(parser.isIgnored('node_modules/package')).toBe(true); + expect(parser.isIgnored('app.log')).toBe(true); + expect(parser.isIgnored('logs/app.log')).toBe(true); + expect(parser.isIgnored('dist')).toBe(true); + expect(parser.isIgnored('.env')).toBe(true); + expect(parser.isIgnored('config/.env')).toBe(true); + }); + + it('should ignore files with path-specific patterns', () => { + expect(parser.isIgnored('src/temp.tmp')).toBe(true); + expect(parser.isIgnored('other/temp.tmp')).toBe(false); + }); + + it('should not ignore files that do not match patterns', () => { + expect(parser.isIgnored('src/index.ts')).toBe(false); + expect(parser.isIgnored('README.md')).toBe(false); + expect(parser.isIgnored('package.json')).toBe(false); + }); + + it('should handle absolute paths correctly', () => { + const absolutePath = path.join( + mockProjectRoot, + 'node_modules', + 'package', + ); + expect(parser.isIgnored(absolutePath)).toBe(true); + }); + + it('should handle paths outside project root', () => { + const outsidePath = '/other/project/file.txt'; + expect(parser.isIgnored(outsidePath)).toBe(false); + }); + + it('should handle relative paths correctly', () => { + expect(parser.isIgnored('./node_modules')).toBe(true); + expect(parser.isIgnored('../file.txt')).toBe(false); + }); + + it('should normalize path separators on Windows', () => { + expect(parser.isIgnored('node_modules\\package')).toBe(true); + expect(parser.isIgnored('src\\temp.tmp')).toBe(true); + }); + }); + + describe('getIgnoredPatterns', () => { + it('should return a copy of patterns array', async () => { + const gitignoreContent = '*.log\n'; + vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent); + + await parser.initialize(); + const patterns1 = parser.getIgnoredPatterns(); + const patterns2 = parser.getIgnoredPatterns(); + + expect(patterns1).not.toBe(patterns2); // Different array instances + expect(patterns1).toEqual(patterns2); // Same content + }); + + it('should always include .git patterns', async () => { + vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT')); + + await parser.initialize(); + const patterns = parser.getIgnoredPatterns(); + + expect(patterns).toContain('.git/**'); + expect(patterns).toContain('.git'); + }); + }); +}); diff --git a/packages/core/src/utils/gitIgnoreParser.ts b/packages/core/src/utils/gitIgnoreParser.ts new file mode 100644 index 00000000..098281ca --- /dev/null +++ b/packages/core/src/utils/gitIgnoreParser.ts @@ -0,0 +1,116 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'fs/promises'; +import * as path from 'path'; +import { minimatch } from 'minimatch'; +import { isGitRepository } from './gitUtils.js'; + +export interface GitIgnoreFilter { + isIgnored(filePath: string): boolean; + getIgnoredPatterns(): string[]; +} + +export class GitIgnoreParser implements GitIgnoreFilter { + private ignorePatterns: string[] = []; + private projectRoot: string; + private isGitRepo: boolean = false; + + constructor(projectRoot: string) { + this.projectRoot = path.resolve(projectRoot); + } + + async initialize(): Promise<void> { + this.isGitRepo = isGitRepository(this.projectRoot); + if (this.isGitRepo) { + const gitIgnoreFiles = [ + path.join(this.projectRoot, '.gitignore'), + path.join(this.projectRoot, '.git', 'info', 'exclude'), + ]; + + // Always ignore .git directory regardless of .gitignore content + this.ignorePatterns = ['.git/**', '.git']; + + for (const gitIgnoreFile of gitIgnoreFiles) { + try { + const content = await fs.readFile(gitIgnoreFile, 'utf-8'); + const patterns = this.parseGitIgnoreContent(content); + this.ignorePatterns.push(...patterns); + } catch (_error) { + // File doesn't exist or can't be read, continue silently + } + } + } + } + + private parseGitIgnoreContent(content: string): string[] { + return content + .split('\n') + .map((line) => line.trim()) + .filter((line) => line && !line.startsWith('#')) + .map((pattern) => { + // Handle negation patterns (!) - for now we'll skip them + if (pattern.startsWith('!')) { + return null; + } + + // Convert gitignore patterns to minimatch-compatible patterns + if (pattern.endsWith('/')) { + // Directory pattern - match directory and all contents + const dirPattern = pattern.slice(0, -1); // Remove trailing slash + return [dirPattern, dirPattern + '/**']; + } + + // If pattern doesn't contain /, it should match at any level + if (!pattern.includes('/') && !pattern.startsWith('**/')) { + return '**/' + pattern; + } + + return pattern; + }) + .filter((pattern) => pattern !== null) + .flat() as string[]; + } + + isIgnored(filePath: string): boolean { + // If not a git repository, nothing is ignored + if (!this.isGitRepo) { + return false; + } + + // Normalize the input path (handle ./ prefixes) + let cleanPath = filePath; + if (cleanPath.startsWith('./')) { + cleanPath = cleanPath.slice(2); + } + + // Convert to relative path from project root + const relativePath = path.relative( + this.projectRoot, + path.resolve(this.projectRoot, cleanPath), + ); + + // Handle paths that go outside project root + if (relativePath.startsWith('..')) { + return false; + } + + // Normalize path separators for cross-platform compatibility + const normalizedPath = relativePath.replace(/\\/g, '/'); + + return this.ignorePatterns.some((pattern) => + minimatch(normalizedPath, pattern, { + dot: true, + matchBase: false, + flipNegate: false, + }), + ); + } + + getIgnoredPatterns(): string[] { + return [...this.ignorePatterns]; + } +} diff --git a/packages/core/src/utils/gitUtils.ts b/packages/core/src/utils/gitUtils.ts new file mode 100644 index 00000000..90ec27aa --- /dev/null +++ b/packages/core/src/utils/gitUtils.ts @@ -0,0 +1,73 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'fs'; +import * as path from 'path'; + +/** + * Checks if a directory is within a git repository + * @param directory The directory to check + * @returns true if the directory is in a git repository, false otherwise + */ +export function isGitRepository(directory: string): boolean { + try { + let currentDir = path.resolve(directory); + + while (true) { + const gitDir = path.join(currentDir, '.git'); + + // Check if .git exists (either as directory or file for worktrees) + if (fs.existsSync(gitDir)) { + return true; + } + + const parentDir = path.dirname(currentDir); + + // If we've reached the root directory, stop searching + if (parentDir === currentDir) { + break; + } + + currentDir = parentDir; + } + + return false; + } catch (_error) { + // If any filesystem error occurs, assume not a git repo + return false; + } +} + +/** + * Finds the root directory of a git repository + * @param directory Starting directory to search from + * @returns The git repository root path, or null if not in a git repository + */ +export function findGitRoot(directory: string): string | null { + try { + let currentDir = path.resolve(directory); + + while (true) { + const gitDir = path.join(currentDir, '.git'); + + if (fs.existsSync(gitDir)) { + return currentDir; + } + + const parentDir = path.dirname(currentDir); + + if (parentDir === currentDir) { + break; + } + + currentDir = parentDir; + } + + return null; + } catch (_error) { + return null; + } +} |
