diff options
27 files changed, 221 insertions, 567 deletions
diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 9f00718f..ca7cfa48 100644 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -136,7 +136,6 @@ export async function loadHierarchicalGeminiMemory( export async function loadCliConfig( settings: Settings, extensions: Extension[], - geminiIgnorePatterns: string[], sessionId: string, ): Promise<Config> { loadEnvironment(); @@ -158,9 +157,6 @@ export async function loadCliConfig( const extensionContextFilePaths = extensions.flatMap((e) => e.contextFiles); const fileService = new FileDiscoveryService(process.cwd()); - await fileService.initialize({ - respectGitIgnore: settings.fileFiltering?.respectGitIgnore, - }); // Call the (now wrapper) loadHierarchicalGeminiMemory which calls the server's version const { memoryContent, fileCount } = await loadHierarchicalGeminiMemory( process.cwd(), @@ -193,7 +189,6 @@ export async function loadCliConfig( approvalMode: argv.yolo || false ? ApprovalMode.YOLO : ApprovalMode.DEFAULT, showMemoryUsage: argv.show_memory_usage || settings.showMemoryUsage || false, - geminiIgnorePatterns, accessibility: settings.accessibility, telemetry: argv.telemetry !== undefined diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 6cd246db..ae8d38ef 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -15,7 +15,6 @@ import { LoadedSettings, loadSettings } from './config/settings.js'; import { themeManager } from './ui/themes/theme-manager.js'; import { getStartupWarnings } from './utils/startupWarnings.js'; import { runNonInteractive } from './nonInteractiveCli.js'; -import { loadGeminiIgnorePatterns } from './utils/loadIgnorePatterns.js'; import { loadExtensions, Extension } from './config/extension.js'; import { cleanupCheckpoints } from './utils/cleanup.js'; import { @@ -41,7 +40,6 @@ export async function main() { const settings = loadSettings(workspaceRoot); setWindowTitle(basename(workspaceRoot), settings); - const geminiIgnorePatterns = await loadGeminiIgnorePatterns(workspaceRoot); await cleanupCheckpoints(); if (settings.errors.length > 0) { for (const error of settings.errors) { @@ -56,15 +54,10 @@ export async function main() { } const extensions = loadExtensions(workspaceRoot); - const config = await loadCliConfig( - settings.merged, - extensions, - geminiIgnorePatterns, - sessionId, - ); + const config = await loadCliConfig(settings.merged, extensions, sessionId); // Initialize centralized FileDiscoveryService - await config.getFileService(); + config.getFileService(); if (config.getCheckpointEnabled()) { try { await config.getGitService(); @@ -199,7 +192,6 @@ async function loadNonInteractiveConfig( return await loadCliConfig( nonInteractiveSettings, extensions, - config.getGeminiIgnorePatterns(), config.getSessionId(), ); } diff --git a/packages/cli/src/ui/App.tsx b/packages/cli/src/ui/App.tsx index 40935f93..a49c5874 100644 --- a/packages/cli/src/ui/App.tsx +++ b/packages/cli/src/ui/App.tsx @@ -139,7 +139,7 @@ const App = ({ config, settings, startupWarnings = [] }: AppProps) => { const { memoryContent, fileCount } = await loadHierarchicalGeminiMemory( process.cwd(), config.getDebugMode(), - await config.getFileService(), + config.getFileService(), ); config.setUserMemory(memoryContent); config.setGeminiMdFileCount(fileCount); diff --git a/packages/cli/src/ui/hooks/atCommandProcessor.test.ts b/packages/cli/src/ui/hooks/atCommandProcessor.test.ts index 9a80c95c..6380a187 100644 --- a/packages/cli/src/ui/hooks/atCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/atCommandProcessor.test.ts @@ -89,7 +89,7 @@ describe('handleAtCommand', () => { // Mock FileDiscoveryService mockFileDiscoveryService = { initialize: vi.fn(), - shouldIgnoreFile: vi.fn(() => false), + shouldGitIgnoreFile: vi.fn(() => false), filterFiles: vi.fn((files) => files), getIgnoreInfo: vi.fn(() => ({ gitIgnored: [] })), isGitRepository: vi.fn(() => true), @@ -101,7 +101,7 @@ describe('handleAtCommand', () => { // Mock getFileService to return the mocked FileDiscoveryService mockConfig.getFileService = vi .fn() - .mockResolvedValue(mockFileDiscoveryService); + .mockReturnValue(mockFileDiscoveryService); }); afterEach(() => { @@ -581,7 +581,7 @@ describe('handleAtCommand', () => { const query = `@${gitIgnoredFile}`; // Mock the file discovery service to report this file as git-ignored - mockFileDiscoveryService.shouldIgnoreFile.mockImplementation( + mockFileDiscoveryService.shouldGitIgnoreFile.mockImplementation( (path: string) => path === gitIgnoredFile, ); @@ -594,7 +594,7 @@ describe('handleAtCommand', () => { signal: abortController.signal, }); - expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith( + expect(mockFileDiscoveryService.shouldGitIgnoreFile).toHaveBeenCalledWith( gitIgnoredFile, ); expect(mockOnDebugMessage).toHaveBeenCalledWith( @@ -613,7 +613,7 @@ describe('handleAtCommand', () => { const query = `@${validFile}`; const fileContent = 'console.log("Hello world");'; - mockFileDiscoveryService.shouldIgnoreFile.mockReturnValue(false); + mockFileDiscoveryService.shouldGitIgnoreFile.mockReturnValue(false); mockReadManyFilesExecute.mockResolvedValue({ llmContent: [`--- ${validFile} ---\n\n${fileContent}\n\n`], returnDisplay: 'Read 1 file.', @@ -628,7 +628,7 @@ describe('handleAtCommand', () => { signal: abortController.signal, }); - expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith( + expect(mockFileDiscoveryService.shouldGitIgnoreFile).toHaveBeenCalledWith( validFile, ); expect(mockReadManyFilesExecute).toHaveBeenCalledWith( @@ -651,7 +651,7 @@ describe('handleAtCommand', () => { const query = `@${validFile} @${gitIgnoredFile}`; const fileContent = '# Project README'; - mockFileDiscoveryService.shouldIgnoreFile.mockImplementation( + mockFileDiscoveryService.shouldGitIgnoreFile.mockImplementation( (path: string) => path === gitIgnoredFile, ); mockReadManyFilesExecute.mockResolvedValue({ @@ -668,10 +668,10 @@ describe('handleAtCommand', () => { signal: abortController.signal, }); - expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith( + expect(mockFileDiscoveryService.shouldGitIgnoreFile).toHaveBeenCalledWith( validFile, ); - expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith( + expect(mockFileDiscoveryService.shouldGitIgnoreFile).toHaveBeenCalledWith( gitIgnoredFile, ); expect(mockOnDebugMessage).toHaveBeenCalledWith( @@ -698,7 +698,7 @@ describe('handleAtCommand', () => { const gitFile = '.git/config'; const query = `@${gitFile}`; - mockFileDiscoveryService.shouldIgnoreFile.mockReturnValue(true); + mockFileDiscoveryService.shouldGitIgnoreFile.mockReturnValue(true); const result = await handleAtCommand({ query, @@ -709,7 +709,7 @@ describe('handleAtCommand', () => { signal: abortController.signal, }); - expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith( + expect(mockFileDiscoveryService.shouldGitIgnoreFile).toHaveBeenCalledWith( gitFile, ); expect(mockOnDebugMessage).toHaveBeenCalledWith( diff --git a/packages/cli/src/ui/hooks/atCommandProcessor.ts b/packages/cli/src/ui/hooks/atCommandProcessor.ts index edbbdc21..d6c5eded 100644 --- a/packages/cli/src/ui/hooks/atCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/atCommandProcessor.ts @@ -135,7 +135,7 @@ export async function handleAtCommand({ addItem({ type: 'user', text: query }, userMessageTimestamp); // Get centralized file discovery service - const fileDiscovery = await config.getFileService(); + const fileDiscovery = config.getFileService(); const respectGitIgnore = config.getFileFilteringRespectGitIgnore(); const pathSpecsToRead: string[] = []; @@ -182,7 +182,7 @@ export async function handleAtCommand({ } // Check if path should be ignored by git - if (fileDiscovery.shouldIgnoreFile(pathName)) { + if (fileDiscovery.shouldGitIgnoreFile(pathName)) { const reason = respectGitIgnore ? 'git-ignored and will be skipped' : 'ignored by custom patterns'; diff --git a/packages/cli/src/ui/hooks/useCompletion.integration.test.ts b/packages/cli/src/ui/hooks/useCompletion.integration.test.ts index 3ee24a8a..43ded9d0 100644 --- a/packages/cli/src/ui/hooks/useCompletion.integration.test.ts +++ b/packages/cli/src/ui/hooks/useCompletion.integration.test.ts @@ -10,6 +10,7 @@ import { renderHook, act } from '@testing-library/react'; import { useCompletion } from './useCompletion.js'; import * as fs from 'fs/promises'; import { FileDiscoveryService } from '@gemini-cli/core'; +import { glob } from 'glob'; // Mock dependencies vi.mock('fs/promises'); @@ -24,6 +25,7 @@ vi.mock('@gemini-cli/core', async () => { getErrorMessage: vi.fn((error) => error.message), }; }); +vi.mock('glob'); describe('useCompletion git-aware filtering integration', () => { let mockFileDiscoveryService: Mocked<FileDiscoveryService>; @@ -38,16 +40,13 @@ describe('useCompletion git-aware filtering integration', () => { beforeEach(() => { mockFileDiscoveryService = { - initialize: vi.fn(), - shouldIgnoreFile: vi.fn(), + shouldGitIgnoreFile: vi.fn(), filterFiles: vi.fn(), - getIgnoreInfo: vi.fn(() => ({ gitIgnored: [] })), - glob: vi.fn().mockResolvedValue([]), }; mockConfig = { getFileFilteringRespectGitIgnore: vi.fn(() => true), - getFileService: vi.fn().mockResolvedValue(mockFileDiscoveryService), + getFileService: vi.fn().mockReturnValue(mockFileDiscoveryService), }; vi.mocked(FileDiscoveryService).mockImplementation( @@ -71,7 +70,7 @@ describe('useCompletion git-aware filtering integration', () => { ] as Array<{ name: string; isDirectory: () => boolean }>); // Mock git ignore service to ignore certain files - mockFileDiscoveryService.shouldIgnoreFile.mockImplementation( + mockFileDiscoveryService.shouldGitIgnoreFile.mockImplementation( (path: string) => path.includes('node_modules') || path.includes('dist') || @@ -125,7 +124,7 @@ describe('useCompletion git-aware filtering integration', () => { ); // Mock git ignore service - mockFileDiscoveryService.shouldIgnoreFile.mockImplementation( + mockFileDiscoveryService.shouldGitIgnoreFile.mockImplementation( (path: string) => path.includes('node_modules') || path.includes('temp'), ); @@ -173,10 +172,6 @@ describe('useCompletion git-aware filtering integration', () => { }); it('should handle git discovery service initialization failure gracefully', async () => { - mockFileDiscoveryService.initialize.mockRejectedValue( - new Error('Git not found'), - ); - vi.mocked(fs.readdir).mockResolvedValue([ { name: 'src', isDirectory: () => true }, { name: 'README.md', isDirectory: () => false }, @@ -208,7 +203,7 @@ describe('useCompletion git-aware filtering integration', () => { { name: 'index.ts', isDirectory: () => false }, ] as Array<{ name: string; isDirectory: () => boolean }>); - mockFileDiscoveryService.shouldIgnoreFile.mockImplementation( + mockFileDiscoveryService.shouldGitIgnoreFile.mockImplementation( (path: string) => path.includes('.log'), ); @@ -228,7 +223,7 @@ describe('useCompletion git-aware filtering integration', () => { it('should use glob for top-level @ completions when available', async () => { const globResults = [`${testCwd}/src/index.ts`, `${testCwd}/README.md`]; - mockFileDiscoveryService.glob.mockResolvedValue(globResults); + vi.mocked(glob).mockResolvedValue(globResults); const { result } = renderHook(() => useCompletion('@s', testCwd, true, slashCommands, mockConfig), @@ -238,9 +233,10 @@ describe('useCompletion git-aware filtering integration', () => { await new Promise((resolve) => setTimeout(resolve, 150)); }); - expect(mockFileDiscoveryService.glob).toHaveBeenCalledWith('**/s*', { + expect(glob).toHaveBeenCalledWith('**/s*', { cwd: testCwd, dot: false, + nocase: true, }); expect(fs.readdir).not.toHaveBeenCalled(); // Ensure glob is used instead of readdir expect(result.current.suggestions).toEqual([ @@ -255,7 +251,7 @@ describe('useCompletion git-aware filtering integration', () => { `${testCwd}/.gitignore`, `${testCwd}/src/index.ts`, ]; - mockFileDiscoveryService.glob.mockResolvedValue(globResults); + vi.mocked(glob).mockResolvedValue(globResults); const { result } = renderHook(() => useCompletion('@.', testCwd, true, slashCommands, mockConfig), @@ -265,9 +261,10 @@ describe('useCompletion git-aware filtering integration', () => { await new Promise((resolve) => setTimeout(resolve, 150)); }); - expect(mockFileDiscoveryService.glob).toHaveBeenCalledWith('**/.*', { + expect(glob).toHaveBeenCalledWith('**/.*', { cwd: testCwd, dot: true, + nocase: true, }); expect(fs.readdir).not.toHaveBeenCalled(); expect(result.current.suggestions).toEqual([ diff --git a/packages/cli/src/ui/hooks/useCompletion.ts b/packages/cli/src/ui/hooks/useCompletion.ts index 0aa04263..b7603720 100644 --- a/packages/cli/src/ui/hooks/useCompletion.ts +++ b/packages/cli/src/ui/hooks/useCompletion.ts @@ -7,6 +7,7 @@ import { useState, useEffect, useCallback } from 'react'; import * as fs from 'fs/promises'; import * as path from 'path'; +import { glob } from 'glob'; import { isNodeError, escapePath, @@ -187,7 +188,7 @@ export function useCompletion( const findFilesRecursively = async ( startDir: string, searchPrefix: string, - fileDiscovery: { shouldIgnoreFile: (path: string) => boolean } | null, + fileDiscovery: { shouldGitIgnoreFile: (path: string) => boolean } | null, currentRelativePath = '', depth = 0, maxDepth = 10, // Limit recursion depth @@ -218,7 +219,7 @@ export function useCompletion( // Check if this entry should be ignored by git-aware filtering if ( fileDiscovery && - fileDiscovery.shouldIgnoreFile(entryPathFromRoot) + fileDiscovery.shouldGitIgnoreFile(entryPathFromRoot) ) { continue; } @@ -263,9 +264,10 @@ export function useCompletion( maxResults = 50, ): Promise<Suggestion[]> => { const globPattern = `**/${searchPrefix}*`; - const files = await fileDiscoveryService.glob(globPattern, { + const files = await glob(globPattern, { cwd, dot: searchPrefix.startsWith('.'), + nocase: true, }); const suggestions: Suggestion[] = files @@ -285,9 +287,7 @@ export function useCompletion( setIsLoadingSuggestions(true); let fetchedSuggestions: Suggestion[] = []; - const fileDiscoveryService = config - ? await config.getFileService() - : null; + const fileDiscoveryService = config ? config.getFileService() : null; try { // If there's no slash, or it's the root, do a recursive search from cwd @@ -326,7 +326,7 @@ export function useCompletion( ); if ( fileDiscoveryService && - fileDiscoveryService.shouldIgnoreFile(relativePath) + fileDiscoveryService.shouldGitIgnoreFile(relativePath) ) { continue; } diff --git a/packages/cli/src/utils/loadIgnorePatterns.test.ts b/packages/cli/src/utils/loadIgnorePatterns.test.ts deleted file mode 100644 index 5ff89c4d..00000000 --- a/packages/cli/src/utils/loadIgnorePatterns.test.ts +++ /dev/null @@ -1,150 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import { - vi, - describe, - it, - expect, - beforeEach, - afterEach, - Mock, - beforeAll, -} from 'vitest'; -import * as path from 'node:path'; -import { loadGeminiIgnorePatterns } from './loadIgnorePatterns.js'; -import os from 'node:os'; - -// Define the type for our mock function explicitly. -type ReadFileSyncMockType = Mock< - (path: string, encoding: string) => string | Buffer ->; - -// Declare a variable to hold our mock function instance. -let mockedFsReadFileSync: ReadFileSyncMockType; - -vi.mock('node:fs', async () => { - const actualFsModule = - await vi.importActual<typeof import('node:fs')>('node:fs'); - return { - ...actualFsModule, - readFileSync: vi.fn(), // The factory creates and returns the vi.fn() instance. - }; -}); - -let actualFs: typeof import('node:fs'); - -describe('loadGeminiIgnorePatterns', () => { - let tempDir: string; - let consoleLogSpy: Mock< - (message?: unknown, ...optionalParams: unknown[]) => void - >; - - beforeAll(async () => { - actualFs = await vi.importActual<typeof import('node:fs')>('node:fs'); - const mockedFsModule = await import('node:fs'); - mockedFsReadFileSync = - mockedFsModule.readFileSync as unknown as ReadFileSyncMockType; - }); - - beforeEach(() => { - tempDir = actualFs.mkdtempSync( - path.join(os.tmpdir(), 'gemini-ignore-test-'), - ); - consoleLogSpy = vi - .spyOn(console, 'log') - .mockImplementation(() => {}) as Mock< - (message?: unknown, ...optionalParams: unknown[]) => void - >; - mockedFsReadFileSync.mockReset(); - }); - - afterEach(() => { - if (actualFs.existsSync(tempDir)) { - actualFs.rmSync(tempDir, { recursive: true, force: true }); - } - vi.restoreAllMocks(); - }); - - it('should load and parse patterns from .geminiignore, ignoring comments and empty lines', async () => { - const ignoreContent = [ - '# This is a comment', - 'pattern1', - ' pattern2 ', // Should be trimmed - '', // Empty line - 'pattern3 # Inline comment', // Handled by trim - '*.log', - '!important.file', - ].join('\n'); - const ignoreFilePath = path.join(tempDir, '.geminiignore'); - actualFs.writeFileSync(ignoreFilePath, ignoreContent); - - const patterns = await loadGeminiIgnorePatterns(tempDir); - - expect(patterns).toEqual([ - 'pattern1', - 'pattern2', - 'pattern3 # Inline comment', - '*.log', - '!important.file', - ]); - expect(consoleLogSpy).toHaveBeenCalledWith( - expect.stringContaining('Loaded 5 patterns from .geminiignore'), - ); - }); - - it('should return an empty array and log info if .geminiignore is not found', async () => { - const patterns = await loadGeminiIgnorePatterns(tempDir); - expect(patterns).toEqual([]); - expect(consoleLogSpy).not.toHaveBeenCalled(); - }); - - it('should return an empty array if .geminiignore is empty', async () => { - const ignoreFilePath = path.join(tempDir, '.geminiignore'); - actualFs.writeFileSync(ignoreFilePath, ''); - - const patterns = await loadGeminiIgnorePatterns(tempDir); - expect(patterns).toEqual([]); - expect(consoleLogSpy).not.toHaveBeenCalledWith( - expect.stringContaining('Loaded 0 patterns from .geminiignore'), - ); - expect(consoleLogSpy).not.toHaveBeenCalledWith( - expect.stringContaining('No .geminiignore file found'), - ); - }); - - it('should return an empty array if .geminiignore contains only comments and empty lines', async () => { - const ignoreContent = [ - '# Comment 1', - ' # Comment 2 with leading spaces', - '', - ' ', // Whitespace only line - ].join('\n'); - const ignoreFilePath = path.join(tempDir, '.geminiignore'); - actualFs.writeFileSync(ignoreFilePath, ignoreContent); - - const patterns = await loadGeminiIgnorePatterns(tempDir); - expect(patterns).toEqual([]); - expect(consoleLogSpy).not.toHaveBeenCalledWith( - expect.stringContaining('Loaded 0 patterns from .geminiignore'), - ); - expect(consoleLogSpy).not.toHaveBeenCalledWith( - expect.stringContaining('No .geminiignore file found'), - ); - }); - - it('should correctly handle patterns with inline comments if not starting with #', async () => { - const ignoreContent = 'src/important # but not this part'; - const ignoreFilePath = path.join(tempDir, '.geminiignore'); - actualFs.writeFileSync(ignoreFilePath, ignoreContent); - - const patterns = await loadGeminiIgnorePatterns(tempDir); - expect(patterns).toEqual(['src/important # but not this part']); - expect(consoleLogSpy).toHaveBeenCalledWith( - expect.stringContaining('Loaded 1 patterns from .geminiignore'), - ); - }); -}); diff --git a/packages/cli/src/utils/loadIgnorePatterns.ts b/packages/cli/src/utils/loadIgnorePatterns.ts deleted file mode 100644 index 34efc8c8..00000000 --- a/packages/cli/src/utils/loadIgnorePatterns.ts +++ /dev/null @@ -1,56 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -import * as path from 'node:path'; -import { GitIgnoreParser } from '@gemini-cli/core'; - -const GEMINI_IGNORE_FILE_NAME = '.geminiignore'; - -/** - * Loads and parses a .geminiignore file from the given workspace root. - * The .geminiignore file follows a format similar to .gitignore. - * - * @param workspaceRoot The absolute path to the workspace root where the .geminiignore file is expected. - * @returns An array of glob patterns extracted from the .geminiignore file. Returns an empty array - * if the file does not exist or contains no valid patterns. - */ -export async function loadGeminiIgnorePatterns( - workspaceRoot: string, -): Promise<string[]> { - const parser = new GitIgnoreParser(workspaceRoot); - - try { - await parser.loadPatterns(GEMINI_IGNORE_FILE_NAME); - } catch (error: unknown) { - const ignoreFilePath = path.join(workspaceRoot, GEMINI_IGNORE_FILE_NAME); - if ( - error instanceof Error && - 'code' in error && - typeof error.code === 'string' - ) { - if (error.code === 'ENOENT') { - // .geminiignore not found, which is fine. - } else { - // Other error reading the file (e.g., permissions) - console.warn( - `[WARN] Could not read .geminiignore file at ${ignoreFilePath}: ${error.message}`, - ); - } - } else { - // For other types of errors, or if code is not available - console.warn( - `[WARN] An unexpected error occurred while trying to read ${ignoreFilePath}: ${String(error)}`, - ); - } - } - const loadedPatterns = parser.getPatterns(); - if (loadedPatterns.length > 0) { - console.log( - `[INFO] Loaded ${loadedPatterns.length} patterns from .geminiignore`, - ); - } - return loadedPatterns; -} diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 4a5cae1b..ea555bd4 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -142,9 +142,9 @@ describe('Server Config (config.ts)', () => { expect(config.getTelemetryEnabled()).toBe(TELEMETRY); }); - it('should have a getFileService method that returns FileDiscoveryService', async () => { + it('should have a getFileService method that returns FileDiscoveryService', () => { const config = new Config(baseParams); - const fileService = await config.getFileService(); + const fileService = config.getFileService(); expect(fileService).toBeDefined(); }); }); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 32c3129d..d841f4b3 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -81,7 +81,6 @@ export interface ConfigParameters { approvalMode?: ApprovalMode; showMemoryUsage?: boolean; contextFileName?: string | string[]; - geminiIgnorePatterns?: string[]; accessibility?: AccessibilitySettings; telemetry?: boolean; telemetryLogUserPromptsEnabled?: boolean; @@ -119,7 +118,6 @@ export class Config { private readonly telemetryLogUserPromptsEnabled: boolean; private readonly telemetryOtlpEndpoint: string; private readonly geminiClient: GeminiClient; - private readonly geminiIgnorePatterns: string[] = []; private readonly fileFilteringRespectGitIgnore: boolean; private fileDiscoveryService: FileDiscoveryService | null = null; private gitService: GitService | undefined = undefined; @@ -165,9 +163,6 @@ export class Config { if (params.contextFileName) { setGeminiMdFilename(params.contextFileName); } - if (params.geminiIgnorePatterns) { - this.geminiIgnorePatterns = params.geminiIgnorePatterns; - } this.toolRegistry = createToolRegistry(this); this.geminiClient = new GeminiClient(this); @@ -296,10 +291,6 @@ export class Config { return path.join(this.targetDir, GEMINI_DIR); } - getGeminiIgnorePatterns(): string[] { - return this.geminiIgnorePatterns; - } - getFileFilteringRespectGitIgnore(): boolean { return this.fileFilteringRespectGitIgnore; } @@ -320,12 +311,9 @@ export class Config { return this.bugCommand; } - async getFileService(): Promise<FileDiscoveryService> { + getFileService(): FileDiscoveryService { if (!this.fileDiscoveryService) { this.fileDiscoveryService = new FileDiscoveryService(this.targetDir); - await this.fileDiscoveryService.initialize({ - respectGitIgnore: this.fileFilteringRespectGitIgnore, - }); } return this.fileDiscoveryService; } diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index fd44bf03..682d9461 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -91,7 +91,7 @@ export class GeminiClient { }); const platform = process.platform; const folderStructure = await getFolderStructure(cwd, { - fileService: await this.config.getFileService(), + fileService: this.config.getFileService(), }); const context = ` Okay, just setting up the context for our chat. diff --git a/packages/core/src/services/fileDiscoveryService.test.ts b/packages/core/src/services/fileDiscoveryService.test.ts index 368f4d23..d7530cd6 100644 --- a/packages/core/src/services/fileDiscoveryService.test.ts +++ b/packages/core/src/services/fileDiscoveryService.test.ts @@ -8,15 +8,13 @@ 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'; +import * as gitUtils from '../utils/gitUtils.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'), -})); +vi.mock('../utils/gitUtils.js'); describe('FileDiscoveryService', () => { let service: FileDiscoveryService; @@ -24,16 +22,16 @@ describe('FileDiscoveryService', () => { 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(), + loadPatterns: vi.fn(), + loadGitRepoPatterns: vi.fn(), } as unknown as Mocked<GitIgnoreParser>; vi.mocked(GitIgnoreParser).mockImplementation(() => mockGitIgnoreParser); + vi.mocked(gitUtils.isGitRepository).mockReturnValue(true); + vi.mocked(gitUtils.findGitRoot).mockReturnValue('/test/project'); vi.clearAllMocks(); }); @@ -42,36 +40,30 @@ describe('FileDiscoveryService', () => { }); describe('initialization', () => { - it('should initialize git ignore parser by default', async () => { - await service.initialize(); - + it('should initialize git ignore parser by default', () => { + service = new FileDiscoveryService(mockProjectRoot); 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(); + expect(GitIgnoreParser).toHaveBeenCalledTimes(2); + expect(mockGitIgnoreParser.loadGitRepoPatterns).toHaveBeenCalled(); + expect(mockGitIgnoreParser.loadPatterns).toHaveBeenCalled(); }); - it('should handle initialization errors gracefully', async () => { - mockGitIgnoreParser.initialize.mockRejectedValue( - new Error('Init failed'), - ); + it('should not initialize git ignore parser when not a git repo', () => { + vi.mocked(gitUtils.isGitRepository).mockReturnValue(false); + service = new FileDiscoveryService(mockProjectRoot); - await expect(service.initialize()).rejects.toThrow('Init failed'); + expect(GitIgnoreParser).toHaveBeenCalledOnce(); + expect(mockGitIgnoreParser.loadGitRepoPatterns).not.toHaveBeenCalled(); }); }); describe('filterFiles', () => { - beforeEach(async () => { + beforeEach(() => { mockGitIgnoreParser.isIgnored.mockImplementation( (path: string) => path.includes('node_modules') || path.includes('.git'), ); - await service.initialize(); + service = new FileDiscoveryService(mockProjectRoot); }); it('should filter out git-ignored files by default', () => { @@ -106,102 +98,22 @@ describe('FileDiscoveryService', () => { }); }); - describe('shouldIgnoreFile', () => { - beforeEach(async () => { + describe('shouldGitIgnoreFile', () => { + beforeEach(() => { mockGitIgnoreParser.isIgnored.mockImplementation((path: string) => path.includes('node_modules'), ); - await service.initialize(); + service = new FileDiscoveryService(mockProjectRoot); }); it('should return true for git-ignored files', () => { - expect(service.shouldIgnoreFile('node_modules/package/index.js')).toBe( + expect(service.shouldGitIgnoreFile('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('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); + expect(service.shouldGitIgnoreFile('src/index.ts')).toBe(false); }); }); @@ -211,13 +123,7 @@ describe('FileDiscoveryService', () => { 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(); + it('should handle filterFiles with undefined options', () => { 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 index 7531f90a..984f3f53 100644 --- a/packages/core/src/services/fileDiscoveryService.ts +++ b/packages/core/src/services/fileDiscoveryService.ts @@ -7,41 +7,37 @@ import { GitIgnoreParser, GitIgnoreFilter } from '../utils/gitIgnoreParser.js'; import { isGitRepository } from '../utils/gitUtils.js'; import * as path from 'path'; -import { glob, type GlobOptions } from 'glob'; -export interface FileDiscoveryOptions { +const GEMINI_IGNORE_FILE_NAME = '.geminiignore'; + +export interface FilterFilesOptions { respectGitIgnore?: boolean; - includeBuildArtifacts?: boolean; - isGitRepo?: boolean; + respectGeminiIgnore?: boolean; } export class FileDiscoveryService { private gitIgnoreFilter: GitIgnoreFilter | null = null; + private geminiIgnoreFilter: 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) { + if (isGitRepository(this.projectRoot)) { const parser = new GitIgnoreParser(this.projectRoot); - await parser.initialize(); + try { + parser.loadGitRepoPatterns(); + } catch (_error) { + // ignore file not found + } this.gitIgnoreFilter = parser; } - } - - async glob( - pattern: string | string[], - options: GlobOptions = {}, - ): Promise<string[]> { - const files = (await glob(pattern, { - ...options, - nocase: true, - })) as string[]; - return this.filterFiles(files); + const gParser = new GitIgnoreParser(this.projectRoot); + try { + gParser.loadPatterns(GEMINI_IGNORE_FILE_NAME); + } catch (_error) { + // ignore file not found + } + this.geminiIgnoreFilter = gParser; } /** @@ -49,42 +45,49 @@ export class FileDiscoveryService { */ filterFiles( filePaths: string[], - options: FileDiscoveryOptions = {}, + options: FilterFilesOptions = { + respectGitIgnore: true, + respectGeminiIgnore: true, + }, ): 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; - } + if (options.respectGitIgnore && this.shouldGitIgnoreFile(filePath)) { + return false; + } + if ( + options.respectGeminiIgnore && + this.shouldGeminiIgnoreFile(filePath) + ) { + return false; } - return true; }); } /** - * Checks if a single file should be ignored + * Checks if a single file should be git-ignored */ - shouldIgnoreFile( - filePath: string, - options: FileDiscoveryOptions = {}, - ): boolean { - const isGitRepo = options.isGitRepo ?? isGitRepository(this.projectRoot); - if ( - options.respectGitIgnore !== false && - isGitRepo && - this.gitIgnoreFilter - ) { + shouldGitIgnoreFile(filePath: string): boolean { + if (this.gitIgnoreFilter) { return this.gitIgnoreFilter.isIgnored(filePath); } return false; } /** - * Returns whether the project is a git repository + * Checks if a single file should be gemini-ignored + */ + shouldGeminiIgnoreFile(filePath: string): boolean { + if (this.geminiIgnoreFilter) { + return this.geminiIgnoreFilter.isIgnored(filePath); + } + return false; + } + + /** + * Returns loaded patterns from .geminiignore */ - isGitRepository(options: FileDiscoveryOptions = {}): boolean { - return options.isGitRepo ?? isGitRepository(this.projectRoot); + getGeminiIgnorePatterns(): string[] { + return this.geminiIgnoreFilter?.getPatterns() ?? []; } } diff --git a/packages/core/src/tools/glob.test.ts b/packages/core/src/tools/glob.test.ts index 21f7f0fe..a4bc99a9 100644 --- a/packages/core/src/tools/glob.test.ts +++ b/packages/core/src/tools/glob.test.ts @@ -20,11 +20,7 @@ describe('GlobTool', () => { // Mock config for testing const mockConfig = { - getFileService: async () => { - const service = new FileDiscoveryService(tempRootDir); - await service.initialize({ respectGitIgnore: true }); - return service; - }, + getFileService: () => new FileDiscoveryService(tempRootDir), getFileFilteringRespectGitIgnore: () => true, } as Partial<Config> as Config; diff --git a/packages/core/src/tools/glob.ts b/packages/core/src/tools/glob.ts index d94a380a..0dd2f411 100644 --- a/packages/core/src/tools/glob.ts +++ b/packages/core/src/tools/glob.ts @@ -221,7 +221,7 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> { const respectGitIgnore = params.respect_git_ignore ?? this.config.getFileFilteringRespectGitIgnore(); - const fileDiscovery = await this.config.getFileService(); + const fileDiscovery = this.config.getFileService(); const entries = (await glob(params.pattern, { cwd: searchDirAbsolute, @@ -239,7 +239,7 @@ export class GlobTool extends BaseTool<GlobToolParams, ToolResult> { let filteredEntries = entries; let gitIgnoredCount = 0; - if (respectGitIgnore && fileDiscovery.isGitRepository()) { + if (respectGitIgnore) { const relativePaths = entries.map((p) => path.relative(this.rootDirectory, p.fullpath()), ); diff --git a/packages/core/src/tools/ls.ts b/packages/core/src/tools/ls.ts index 56a016aa..6e3869be 100644 --- a/packages/core/src/tools/ls.ts +++ b/packages/core/src/tools/ls.ts @@ -233,7 +233,7 @@ export class LSTool extends BaseTool<LSToolParams, ToolResult> { const respectGitIgnore = params.respect_git_ignore ?? this.config.getFileFilteringRespectGitIgnore(); - const fileDiscovery = await this.config.getFileService(); + const fileDiscovery = this.config.getFileService(); const entries: FileEntry[] = []; let gitIgnoredCount = 0; @@ -257,8 +257,7 @@ export class LSTool extends BaseTool<LSToolParams, ToolResult> { // Check if this file should be git-ignored (only in git repositories) if ( respectGitIgnore && - fileDiscovery.isGitRepository() && - fileDiscovery.shouldIgnoreFile(relativePath) + fileDiscovery.shouldGitIgnoreFile(relativePath) ) { gitIgnoredCount++; continue; diff --git a/packages/core/src/tools/read-file.test.ts b/packages/core/src/tools/read-file.test.ts index 39c22d06..987b451c 100644 --- a/packages/core/src/tools/read-file.test.ts +++ b/packages/core/src/tools/read-file.test.ts @@ -11,6 +11,7 @@ import path from 'path'; import os from 'os'; import fs from 'fs'; // For actual fs operations in setup import { Config } from '../config/config.js'; +import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; // Mock fileUtils.processSingleFileContent vi.mock('../utils/fileUtils', async () => { @@ -34,9 +35,14 @@ describe('ReadFileTool', () => { tempRootDir = fs.mkdtempSync( path.join(os.tmpdir(), 'read-file-tool-root-'), ); + fs.writeFileSync( + path.join(tempRootDir, '.geminiignore'), + ['foo.*'].join('\n'), + ); + const fileService = new FileDiscoveryService(tempRootDir); const mockConfigInstance = { - getGeminiIgnorePatterns: () => ['**/foo.bar', 'foo.baz', 'foo.*'], - } as Config; + getFileService: () => fileService, + } as unknown as Config; tool = new ReadFileTool(tempRootDir, mockConfigInstance); mockProcessSingleFileContent.mockReset(); }); @@ -235,7 +241,6 @@ describe('ReadFileTool', () => { }; const result = await tool.execute(params, abortSignal); expect(result.returnDisplay).toContain('foo.bar'); - expect(result.returnDisplay).toContain('foo.*'); expect(result.returnDisplay).not.toContain('foo.baz'); }); }); diff --git a/packages/core/src/tools/read-file.ts b/packages/core/src/tools/read-file.ts index 5c5994d7..bed95746 100644 --- a/packages/core/src/tools/read-file.ts +++ b/packages/core/src/tools/read-file.ts @@ -5,7 +5,6 @@ */ import path from 'path'; -import micromatch from 'micromatch'; import { SchemaValidator } from '../utils/schemaValidator.js'; import { makeRelative, shortenPath } from '../utils/paths.js'; import { BaseTool, ToolResult } from './tools.js'; @@ -37,11 +36,10 @@ export interface ReadFileToolParams { */ export class ReadFileTool extends BaseTool<ReadFileToolParams, ToolResult> { static readonly Name: string = 'read_file'; - private readonly geminiIgnorePatterns: string[]; constructor( private rootDirectory: string, - config: Config, + private config: Config, ) { super( ReadFileTool.Name, @@ -70,7 +68,6 @@ export class ReadFileTool extends BaseTool<ReadFileToolParams, ToolResult> { }, ); this.rootDirectory = path.resolve(rootDirectory); - this.geminiIgnorePatterns = config.getGeminiIgnorePatterns() || []; } validateToolParams(params: ReadFileToolParams): string | null { @@ -97,16 +94,10 @@ export class ReadFileTool extends BaseTool<ReadFileToolParams, ToolResult> { return 'Limit must be a positive number'; } - // Check against .geminiignore patterns - if (this.geminiIgnorePatterns.length > 0) { + const fileService = this.config.getFileService(); + if (fileService.shouldGeminiIgnoreFile(params.path)) { const relativePath = makeRelative(params.path, this.rootDirectory); - if (micromatch.isMatch(relativePath, this.geminiIgnorePatterns)) { - // Get patterns that matched to show in the error message - const matchingPatterns = this.geminiIgnorePatterns.filter((p) => - micromatch.isMatch(relativePath, p), - ); - return `File path '${shortenPath(relativePath)}' is ignored by the following .geminiignore pattern(s):\n\n${matchingPatterns.join('\n')}`; - } + return `File path '${shortenPath(relativePath)}' is ignored by .geminiignore pattern(s).`; } return null; diff --git a/packages/core/src/tools/read-many-files.test.ts b/packages/core/src/tools/read-many-files.test.ts index 90a5a312..63cc7bb2 100644 --- a/packages/core/src/tools/read-many-files.test.ts +++ b/packages/core/src/tools/read-many-files.test.ts @@ -21,17 +21,6 @@ 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, - getGeminiIgnorePatterns: () => ['**/foo.bar', 'foo.baz', 'foo.*'], - } as Partial<Config> as Config; - beforeEach(async () => { tempRootDir = fs.mkdtempSync( path.join(os.tmpdir(), 'read-many-files-root-'), @@ -39,6 +28,13 @@ describe('ReadManyFilesTool', () => { tempDirOutsideRoot = fs.mkdtempSync( path.join(os.tmpdir(), 'read-many-files-external-'), ); + fs.writeFileSync(path.join(tempRootDir, '.geminiignore'), 'foo.*'); + const fileService = new FileDiscoveryService(tempRootDir); + const mockConfig = { + getFileService: () => fileService, + getFileFilteringRespectGitIgnore: () => true, + } as Partial<Config> as Config; + tool = new ReadManyFilesTool(tempRootDir, mockConfig); mockReadFileFn = mockControl.mockReadFile; @@ -369,13 +365,13 @@ describe('ReadManyFilesTool', () => { it('should return error if path is ignored by a .geminiignore pattern', async () => { createFile('foo.bar', ''); - createFile('qux/foo.baz', ''); + createFile('bar.ts', ''); createFile('foo.quux', ''); - const params = { paths: ['foo.bar', 'qux/foo.baz', 'foo.quux'] }; + const params = { paths: ['foo.bar', 'bar.ts', 'foo.quux'] }; const result = await tool.execute(params, new AbortController().signal); expect(result.returnDisplay).not.toContain('foo.bar'); - expect(result.returnDisplay).toContain('qux/foo.baz'); expect(result.returnDisplay).not.toContain('foo.quux'); + expect(result.returnDisplay).toContain('bar.ts'); }); }); }); diff --git a/packages/core/src/tools/read-many-files.ts b/packages/core/src/tools/read-many-files.ts index daf9bc04..107e16b3 100644 --- a/packages/core/src/tools/read-many-files.ts +++ b/packages/core/src/tools/read-many-files.ts @@ -119,7 +119,7 @@ export class ReadManyFilesTool extends BaseTool< ToolResult > { static readonly Name: string = 'read_many_files'; - private readonly geminiIgnorePatterns: string[]; + private readonly geminiIgnorePatterns: string[] = []; /** * Creates an instance of ReadManyFilesTool. @@ -191,7 +191,9 @@ Use this tool when the user's query implies needing the content of several files parameterSchema, ); this.targetDir = path.resolve(targetDir); - this.geminiIgnorePatterns = config.getGeminiIgnorePatterns() || []; + this.geminiIgnorePatterns = config + .getFileService() + .getGeminiIgnorePatterns(); } validateParams(params: ReadManyFilesParams): string | null { @@ -292,7 +294,7 @@ Use this tool when the user's query implies needing the content of several files respect_git_ignore ?? this.config.getFileFilteringRespectGitIgnore(); // Get centralized file discovery service - const fileDiscovery = await this.config.getFileService(); + const fileDiscovery = this.config.getFileService(); const toolBaseDir = this.targetDir; const filesToConsider = new Set<string>(); @@ -323,18 +325,16 @@ Use this tool when the user's query implies needing the content of several files signal, }); - // 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; + const filteredEntries = respectGitIgnore + ? 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) { @@ -348,11 +348,7 @@ Use this tool when the user's query implies needing the content of several files } // Check if this file was filtered out by git ignore - if ( - respectGitIgnore && - fileDiscovery.isGitRepository() && - !filteredEntries.includes(absoluteFilePath) - ) { + if (respectGitIgnore && !filteredEntries.includes(absoluteFilePath)) { gitIgnoredCount++; continue; } @@ -362,12 +358,9 @@ Use this tool when the user's query implies needing the content of several files // 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, + reason: 'ignored', }); } } catch (error) { diff --git a/packages/core/src/utils/bfsFileSearch.test.ts b/packages/core/src/utils/bfsFileSearch.test.ts index f313c427..83e9b0b9 100644 --- a/packages/core/src/utils/bfsFileSearch.test.ts +++ b/packages/core/src/utils/bfsFileSearch.test.ts @@ -4,18 +4,19 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { Dirent, PathLike } from 'fs'; +import * as fs from 'fs'; import { vi, describe, it, expect, beforeEach } from 'vitest'; -import * as fs from 'fs/promises'; +import * as fsPromises from 'fs/promises'; import * as gitUtils from './gitUtils.js'; import { bfsFileSearch } from './bfsFileSearch.js'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; +vi.mock('fs'); vi.mock('fs/promises'); vi.mock('./gitUtils.js'); -const createMockDirent = (name: string, isFile: boolean): Dirent => { - const dirent = new Dirent(); +const createMockDirent = (name: string, isFile: boolean): fs.Dirent => { + const dirent = new fs.Dirent(); dirent.name = name; dirent.isFile = () => isFile; dirent.isDirectory = () => !isFile; @@ -24,9 +25,9 @@ const createMockDirent = (name: string, isFile: boolean): Dirent => { // Type for the specific overload we're using type ReaddirWithFileTypes = ( - path: PathLike, + path: fs.PathLike, options: { withFileTypes: true }, -) => Promise<Dirent[]>; +) => Promise<fs.Dirent[]>; describe('bfsFileSearch', () => { beforeEach(() => { @@ -34,7 +35,7 @@ describe('bfsFileSearch', () => { }); it('should find a file in the root directory', async () => { - const mockFs = vi.mocked(fs); + const mockFs = vi.mocked(fsPromises); const mockReaddir = mockFs.readdir as unknown as ReaddirWithFileTypes; vi.mocked(mockReaddir).mockResolvedValue([ createMockDirent('file1.txt', true), @@ -46,7 +47,7 @@ describe('bfsFileSearch', () => { }); it('should find a file in a subdirectory', async () => { - const mockFs = vi.mocked(fs); + const mockFs = vi.mocked(fsPromises); const mockReaddir = mockFs.readdir as unknown as ReaddirWithFileTypes; vi.mocked(mockReaddir).mockImplementation(async (dir) => { if (dir === '/test') { @@ -63,7 +64,7 @@ describe('bfsFileSearch', () => { }); it('should ignore specified directories', async () => { - const mockFs = vi.mocked(fs); + const mockFs = vi.mocked(fsPromises); const mockReaddir = mockFs.readdir as unknown as ReaddirWithFileTypes; vi.mocked(mockReaddir).mockImplementation(async (dir) => { if (dir === '/test') { @@ -89,7 +90,7 @@ describe('bfsFileSearch', () => { }); it('should respect maxDirs limit', async () => { - const mockFs = vi.mocked(fs); + const mockFs = vi.mocked(fsPromises); const mockReaddir = mockFs.readdir as unknown as ReaddirWithFileTypes; vi.mocked(mockReaddir).mockImplementation(async (dir) => { if (dir === '/test') { @@ -115,7 +116,7 @@ describe('bfsFileSearch', () => { }); it('should respect .gitignore files', async () => { - const mockFs = vi.mocked(fs); + const mockFs = vi.mocked(fsPromises); const mockGitUtils = vi.mocked(gitUtils); mockGitUtils.isGitRepository.mockReturnValue(true); const mockReaddir = mockFs.readdir as unknown as ReaddirWithFileTypes; @@ -135,10 +136,9 @@ describe('bfsFileSearch', () => { } return []; }); - mockFs.readFile.mockResolvedValue('subdir2'); + vi.mocked(fs).readFileSync.mockReturnValue('subdir2'); const fileService = new FileDiscoveryService('/test'); - await fileService.initialize(); const result = await bfsFileSearch('/test', { fileName: 'file1.txt', fileService, diff --git a/packages/core/src/utils/bfsFileSearch.ts b/packages/core/src/utils/bfsFileSearch.ts index 7cd33c03..e552f520 100644 --- a/packages/core/src/utils/bfsFileSearch.ts +++ b/packages/core/src/utils/bfsFileSearch.ts @@ -69,7 +69,7 @@ export async function bfsFileSearch( for (const entry of entries) { const fullPath = path.join(currentDir, entry.name); - if (fileService?.shouldIgnoreFile(fullPath)) { + if (fileService?.shouldGitIgnoreFile(fullPath)) { continue; } diff --git a/packages/core/src/utils/getFolderStructure.test.ts b/packages/core/src/utils/getFolderStructure.test.ts index 63724ba8..83d83624 100644 --- a/packages/core/src/utils/getFolderStructure.test.ts +++ b/packages/core/src/utils/getFolderStructure.test.ts @@ -7,6 +7,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { describe, it, expect, vi, beforeEach, afterEach, Mock } from 'vitest'; import fsPromises from 'fs/promises'; +import * as fs from 'fs'; import { Dirent as FSDirent } from 'fs'; import * as nodePath from 'path'; import { getFolderStructure } from './getFolderStructure.js'; @@ -23,6 +24,7 @@ vi.mock('path', async (importOriginal) => { }); vi.mock('fs/promises'); +vi.mock('fs'); vi.mock('./gitUtils.js'); // Import 'path' again here, it will be the mocked version @@ -308,7 +310,7 @@ describe('getFolderStructure gitignore', () => { return []; }); - (fsPromises.readFile as Mock).mockImplementation(async (p) => { + (fs.readFileSync as Mock).mockImplementation((p) => { const path = p.toString(); if (path === '/test/project/.gitignore') { return 'ignored.txt\nnode_modules/\n.gemini/\n!/.gemini/config.yaml'; @@ -321,7 +323,6 @@ describe('getFolderStructure gitignore', () => { it('should ignore files and folders specified in .gitignore', async () => { const fileService = new FileDiscoveryService('/test/project'); - await fileService.initialize(); const structure = await getFolderStructure('/test/project', { fileService, }); @@ -332,9 +333,9 @@ describe('getFolderStructure gitignore', () => { it('should not ignore files if respectGitIgnore is false', async () => { const fileService = new FileDiscoveryService('/test/project'); - await fileService.initialize({ respectGitIgnore: false }); const structure = await getFolderStructure('/test/project', { fileService, + respectGitIgnore: false, }); expect(structure).toContain('ignored.txt'); // node_modules is still ignored by default diff --git a/packages/core/src/utils/getFolderStructure.ts b/packages/core/src/utils/getFolderStructure.ts index 7aee377b..6798a147 100644 --- a/packages/core/src/utils/getFolderStructure.ts +++ b/packages/core/src/utils/getFolderStructure.ts @@ -26,6 +26,8 @@ interface FolderStructureOptions { fileIncludePattern?: RegExp; /** For filtering files. */ fileService?: FileDiscoveryService; + /** Whether to use .gitignore patterns. */ + respectGitIgnore?: boolean; } // Define a type for the merged options where fileIncludePattern remains optional @@ -124,8 +126,8 @@ async function readFullStructure( } const fileName = entry.name; const filePath = path.join(currentPath, fileName); - if (options.fileService) { - if (options.fileService.shouldIgnoreFile(filePath)) { + if (options.respectGitIgnore && options.fileService) { + if (options.fileService.shouldGitIgnoreFile(filePath)) { continue; } } @@ -159,8 +161,8 @@ async function readFullStructure( const subFolderPath = path.join(currentPath, subFolderName); let isIgnoredByGit = false; - if (options?.fileService) { - if (options.fileService.shouldIgnoreFile(subFolderPath)) { + if (options.respectGitIgnore && options.fileService) { + if (options.fileService.shouldGitIgnoreFile(subFolderPath)) { isIgnoredByGit = true; } } @@ -293,6 +295,7 @@ export async function getFolderStructure( ignoredFolders: options?.ignoredFolders ?? DEFAULT_IGNORED_FOLDERS, fileIncludePattern: options?.fileIncludePattern, fileService: options?.fileService, + respectGitIgnore: options?.respectGitIgnore ?? true, }; try { diff --git a/packages/core/src/utils/gitIgnoreParser.test.ts b/packages/core/src/utils/gitIgnoreParser.test.ts index 12084266..f58d50be 100644 --- a/packages/core/src/utils/gitIgnoreParser.test.ts +++ b/packages/core/src/utils/gitIgnoreParser.test.ts @@ -6,12 +6,12 @@ import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; import { GitIgnoreParser } from './gitIgnoreParser.js'; -import * as fs from 'fs/promises'; +import * as fs from 'fs'; import * as path from 'path'; import { isGitRepository } from './gitUtils.js'; // Mock fs module -vi.mock('fs/promises'); +vi.mock('fs'); // Mock gitUtils module vi.mock('./gitUtils.js'); @@ -23,8 +23,7 @@ describe('GitIgnoreParser', () => { beforeEach(() => { parser = new GitIgnoreParser(mockProjectRoot); // Reset mocks before each test - vi.mocked(fs.readFile).mockClear(); - vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT')); // Default to no file + vi.mocked(fs.readFileSync).mockClear(); vi.mocked(isGitRepository).mockReturnValue(true); }); @@ -33,11 +32,11 @@ describe('GitIgnoreParser', () => { }); describe('initialization', () => { - it('should initialize without errors when no .gitignore exists', async () => { - await expect(parser.initialize()).resolves.not.toThrow(); + it('should initialize without errors when no .gitignore exists', () => { + expect(() => parser.loadGitRepoPatterns()).not.toThrow(); }); - it('should load .gitignore patterns when file exists', async () => { + it('should load .gitignore patterns when file exists', () => { const gitignoreContent = ` # Comment node_modules/ @@ -45,11 +44,9 @@ node_modules/ /dist .env `; - vi.mocked(fs.readFile) - .mockResolvedValueOnce(gitignoreContent) - .mockRejectedValue(new Error('ENOENT')); + vi.mocked(fs.readFileSync).mockReturnValueOnce(gitignoreContent); - await parser.initialize(); + parser.loadGitRepoPatterns(); expect(parser.getPatterns()).toEqual([ '.git', @@ -64,8 +61,8 @@ node_modules/ expect(parser.isIgnored('.env')).toBe(true); }); - it('should handle git exclude file', async () => { - vi.mocked(fs.readFile).mockImplementation(async (filePath) => { + it('should handle git exclude file', () => { + vi.mocked(fs.readFileSync).mockImplementation((filePath) => { if ( filePath === path.join(mockProjectRoot, '.git', 'info', 'exclude') ) { @@ -74,30 +71,34 @@ node_modules/ throw new Error('ENOENT'); }); - await parser.initialize(); + parser.loadGitRepoPatterns(); expect(parser.getPatterns()).toEqual(['.git', 'temp/', '*.tmp']); expect(parser.isIgnored('temp/file.txt')).toBe(true); expect(parser.isIgnored('src/file.tmp')).toBe(true); }); - it('should handle custom patterns file name', async () => { + it('should handle custom patterns file name', () => { vi.mocked(isGitRepository).mockReturnValue(false); - vi.mocked(fs.readFile).mockImplementation(async (filePath) => { + vi.mocked(fs.readFileSync).mockImplementation((filePath) => { if (filePath === path.join(mockProjectRoot, '.geminiignore')) { return 'temp/\n*.tmp'; } throw new Error('ENOENT'); }); - await parser.initialize('.geminiignore'); + parser.loadPatterns('.geminiignore'); expect(parser.getPatterns()).toEqual(['temp/', '*.tmp']); expect(parser.isIgnored('temp/file.txt')).toBe(true); expect(parser.isIgnored('src/file.tmp')).toBe(true); }); + + it('should initialize without errors when no .geminiignore exists', () => { + expect(() => parser.loadPatterns('.geminiignore')).not.toThrow(); + }); }); describe('isIgnored', () => { - beforeEach(async () => { + beforeEach(() => { const gitignoreContent = ` node_modules/ *.log @@ -106,10 +107,8 @@ node_modules/ src/*.tmp !src/important.tmp `; - vi.mocked(fs.readFile) - .mockResolvedValueOnce(gitignoreContent) - .mockRejectedValue(new Error('ENOENT')); - await parser.initialize(); + vi.mocked(fs.readFileSync).mockReturnValueOnce(gitignoreContent); + parser.loadGitRepoPatterns(); }); it('should always ignore .git directory', () => { @@ -165,11 +164,12 @@ src/*.tmp }); describe('getIgnoredPatterns', () => { - it('should return the raw patterns added', async () => { + it('should return the raw patterns added', () => { const gitignoreContent = '*.log\n!important.log'; - vi.mocked(fs.readFile).mockResolvedValueOnce(gitignoreContent); + vi.mocked(fs.readFileSync).mockReturnValueOnce(gitignoreContent); - await parser.initialize(); + parser.loadGitRepoPatterns(); + expect(parser.getPatterns()).toEqual(['.git', '*.log', '!important.log']); }); }); }); diff --git a/packages/core/src/utils/gitIgnoreParser.ts b/packages/core/src/utils/gitIgnoreParser.ts index 16367a4a..69797282 100644 --- a/packages/core/src/utils/gitIgnoreParser.ts +++ b/packages/core/src/utils/gitIgnoreParser.ts @@ -4,18 +4,18 @@ * SPDX-License-Identifier: Apache-2.0 */ -import * as fs from 'fs/promises'; +import * as fs from 'fs'; import * as path from 'path'; import ignore, { type Ignore } from 'ignore'; import { isGitRepository } from './gitUtils.js'; export interface GitIgnoreFilter { isIgnored(filePath: string): boolean; + getPatterns(): string[]; } export class GitIgnoreParser implements GitIgnoreFilter { private projectRoot: string; - private isGitRepo: boolean = false; private ig: Ignore = ignore(); private patterns: string[] = []; @@ -23,33 +23,28 @@ export class GitIgnoreParser implements GitIgnoreFilter { this.projectRoot = path.resolve(projectRoot); } - async initialize(patternsFileName?: string): Promise<void> { - const patternFiles = []; - if (patternsFileName && patternsFileName !== '') { - patternFiles.push(patternsFileName); - } + loadGitRepoPatterns(): void { + if (!isGitRepository(this.projectRoot)) return; - this.isGitRepo = isGitRepository(this.projectRoot); - if (this.isGitRepo) { - patternFiles.push('.gitignore'); - patternFiles.push(path.join('.git', 'info', 'exclude')); + // Always ignore .git directory regardless of .gitignore content + this.addPatterns(['.git']); - // Always ignore .git directory regardless of .gitignore content - this.addPatterns(['.git']); - } + const patternFiles = ['.gitignore', path.join('.git', 'info', 'exclude')]; for (const pf of patternFiles) { - try { - await this.loadPatterns(pf); - } catch (_error) { - // File doesn't exist or can't be read, continue silently - } + this.loadPatterns(pf); } } - async loadPatterns(patternsFileName: string): Promise<void> { + loadPatterns(patternsFileName: string): void { const patternsFilePath = path.join(this.projectRoot, patternsFileName); - const content = await fs.readFile(patternsFilePath, 'utf-8'); - const patterns = content + let content: string; + try { + content = fs.readFileSync(patternsFilePath, 'utf-8'); + } catch (_error) { + // ignore file not found + return; + } + const patterns = (content ?? '') .split('\n') .map((p) => p.trim()) .filter((p) => p !== '' && !p.startsWith('#')); |