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/cli/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/cli/src')
| -rw-r--r-- | packages/cli/src/config/config.integration.test.ts | 213 | ||||
| -rw-r--r-- | packages/cli/src/config/config.ts | 4 | ||||
| -rw-r--r-- | packages/cli/src/config/settings.ts | 7 | ||||
| -rw-r--r-- | packages/cli/src/gemini.tsx | 3 | ||||
| -rw-r--r-- | packages/cli/src/ui/components/InputPrompt.tsx | 1 | ||||
| -rw-r--r-- | packages/cli/src/ui/hooks/atCommandProcessor.test.ts | 196 | ||||
| -rw-r--r-- | packages/cli/src/ui/hooks/atCommandProcessor.ts | 28 | ||||
| -rw-r--r-- | packages/cli/src/ui/hooks/slashCommandProcessor.ts | 1 | ||||
| -rw-r--r-- | packages/cli/src/ui/hooks/useCompletion.integration.test.ts | 228 | ||||
| -rw-r--r-- | packages/cli/src/ui/hooks/useCompletion.ts | 62 |
10 files changed, 723 insertions, 20 deletions
diff --git a/packages/cli/src/config/config.integration.test.ts b/packages/cli/src/config/config.integration.test.ts new file mode 100644 index 00000000..6a995296 --- /dev/null +++ b/packages/cli/src/config/config.integration.test.ts @@ -0,0 +1,213 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import { tmpdir } from 'os'; +import { Config, ConfigParameters } from '@gemini-code/core'; + +// Mock file discovery service and tool registry +vi.mock('@gemini-code/core', async () => { + const actual = await vi.importActual('@gemini-code/core'); + return { + ...actual, + FileDiscoveryService: vi.fn().mockImplementation(() => ({ + initialize: vi.fn(), + })), + createToolRegistry: vi.fn().mockResolvedValue({}), + }; +}); + +describe('Configuration Integration Tests', () => { + let tempDir: string; + let originalEnv: NodeJS.ProcessEnv; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(tmpdir(), 'gemini-cli-test-')); + originalEnv = { ...process.env }; + process.env.GEMINI_API_KEY = 'test-api-key'; + vi.clearAllMocks(); + }); + + afterEach(() => { + process.env = originalEnv; + if (fs.existsSync(tempDir)) { + fs.rmSync(tempDir, { recursive: true }); + } + }); + + describe('File Filtering Configuration', () => { + it('should load default file filtering settings', async () => { + const configParams: ConfigParameters = { + apiKey: 'test-key', + model: 'test-model', + sandbox: false, + targetDir: tempDir, + debugMode: false, + userAgent: 'test-agent', + fileFilteringRespectGitIgnore: undefined, // Should default to true + fileFilteringAllowBuildArtifacts: undefined, // Should default to false + }; + + const config = new Config(configParams); + + expect(config.getFileFilteringRespectGitIgnore()).toBe(true); + expect(config.getFileFilteringAllowBuildArtifacts()).toBe(false); + }); + + it('should load custom file filtering settings from configuration', async () => { + const configParams: ConfigParameters = { + apiKey: 'test-key', + model: 'test-model', + sandbox: false, + targetDir: tempDir, + debugMode: false, + userAgent: 'test-agent', + fileFilteringRespectGitIgnore: false, + fileFilteringAllowBuildArtifacts: true, + }; + + const config = new Config(configParams); + + expect(config.getFileFilteringRespectGitIgnore()).toBe(false); + expect(config.getFileFilteringAllowBuildArtifacts()).toBe(true); + }); + + it('should merge user and workspace file filtering settings', async () => { + const configParams: ConfigParameters = { + apiKey: 'test-key', + model: 'test-model', + sandbox: false, + targetDir: tempDir, + debugMode: false, + userAgent: 'test-agent', + fileFilteringRespectGitIgnore: true, + fileFilteringAllowBuildArtifacts: true, + }; + + const config = new Config(configParams); + + expect(config.getFileFilteringAllowBuildArtifacts()).toBe(true); + expect(config.getFileFilteringRespectGitIgnore()).toBe(true); + }); + }); + + describe('Configuration Integration', () => { + it('should handle partial configuration objects gracefully', async () => { + const configParams: ConfigParameters = { + apiKey: 'test-key', + model: 'test-model', + sandbox: false, + targetDir: tempDir, + debugMode: false, + userAgent: 'test-agent', + fileFilteringRespectGitIgnore: false, + fileFilteringAllowBuildArtifacts: undefined, // Should default to false + }; + + const config = new Config(configParams); + + // Specified settings should be applied + expect(config.getFileFilteringRespectGitIgnore()).toBe(false); + + // Missing settings should use defaults + expect(config.getFileFilteringAllowBuildArtifacts()).toBe(false); + }); + + it('should handle empty configuration objects gracefully', async () => { + const configParams: ConfigParameters = { + apiKey: 'test-key', + model: 'test-model', + sandbox: false, + targetDir: tempDir, + debugMode: false, + userAgent: 'test-agent', + fileFilteringRespectGitIgnore: undefined, + fileFilteringAllowBuildArtifacts: undefined, + }; + + const config = new Config(configParams); + + // All settings should use defaults + expect(config.getFileFilteringRespectGitIgnore()).toBe(true); + expect(config.getFileFilteringAllowBuildArtifacts()).toBe(false); + }); + + it('should handle missing configuration sections gracefully', async () => { + const configParams: ConfigParameters = { + apiKey: 'test-key', + model: 'test-model', + sandbox: false, + targetDir: tempDir, + debugMode: false, + userAgent: 'test-agent', + // Missing fileFiltering configuration + }; + + const config = new Config(configParams); + + // All git-aware settings should use defaults + expect(config.getFileFilteringRespectGitIgnore()).toBe(true); + expect(config.getFileFilteringAllowBuildArtifacts()).toBe(false); + }); + }); + + describe('Real-world Configuration Scenarios', () => { + it('should handle a security-focused configuration', async () => { + const configParams: ConfigParameters = { + apiKey: 'test-key', + model: 'test-model', + sandbox: false, + targetDir: tempDir, + debugMode: false, + userAgent: 'test-agent', + fileFilteringRespectGitIgnore: true, + fileFilteringAllowBuildArtifacts: false, + }; + + const config = new Config(configParams); + + expect(config.getFileFilteringRespectGitIgnore()).toBe(true); + expect(config.getFileFilteringAllowBuildArtifacts()).toBe(false); + }); + + it('should handle a development-focused configuration', async () => { + const configParams: ConfigParameters = { + apiKey: 'test-key', + model: 'test-model', + sandbox: false, + targetDir: tempDir, + debugMode: false, + userAgent: 'test-agent', + fileFilteringRespectGitIgnore: true, + fileFilteringAllowBuildArtifacts: true, + }; + + const config = new Config(configParams); + + expect(config.getFileFilteringAllowBuildArtifacts()).toBe(true); + }); + + it('should handle a CI/CD environment configuration', async () => { + const configParams: ConfigParameters = { + apiKey: 'test-key', + model: 'test-model', + sandbox: false, + targetDir: tempDir, + debugMode: false, + userAgent: 'test-agent', + fileFilteringRespectGitIgnore: false, // CI might need to see all files + fileFilteringAllowBuildArtifacts: true, + }; + + const config = new Config(configParams); + + expect(config.getFileFilteringRespectGitIgnore()).toBe(false); + expect(config.getFileFilteringAllowBuildArtifacts()).toBe(true); + }); + }); +}); diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 1f85194f..ad5c270e 100644 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -214,6 +214,10 @@ export async function loadCliConfig( vertexai: useVertexAI, showMemoryUsage: argv.show_memory_usage || settings.showMemoryUsage || false, + // Git-aware file filtering settings + fileFilteringRespectGitIgnore: settings.fileFiltering?.respectGitIgnore, + fileFilteringAllowBuildArtifacts: + settings.fileFiltering?.allowBuildArtifacts, }; const config = createServerConfig(configParams); diff --git a/packages/cli/src/config/settings.ts b/packages/cli/src/config/settings.ts index af2e4a2f..8c26ef54 100644 --- a/packages/cli/src/config/settings.ts +++ b/packages/cli/src/config/settings.ts @@ -32,6 +32,13 @@ export interface Settings { showMemoryUsage?: boolean; contextFileName?: string; title?: string; + + // Git-aware file filtering settings + fileFiltering?: { + respectGitIgnore?: boolean; + allowBuildArtifacts?: boolean; + }; + // Add other settings here. } diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 09555ee5..fc42bdec 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -60,6 +60,9 @@ async function main() { const { config, modelWasSwitched, originalModelBeforeSwitch, finalModel } = await loadCliConfig(settings.merged); + // Initialize centralized FileDiscoveryService + await config.getFileService(); + if (modelWasSwitched && originalModelBeforeSwitch) { console.log( `[INFO] Your configured model (${originalModelBeforeSwitch}) was temporarily unavailable. Switched to ${finalModel} for this session.`, diff --git a/packages/cli/src/ui/components/InputPrompt.tsx b/packages/cli/src/ui/components/InputPrompt.tsx index b6c1b8da..c131f5e0 100644 --- a/packages/cli/src/ui/components/InputPrompt.tsx +++ b/packages/cli/src/ui/components/InputPrompt.tsx @@ -68,6 +68,7 @@ export const InputPrompt: React.FC<InputPromptProps> = ({ config.getTargetDir(), isAtCommand(buffer.text) || isSlashCommand(buffer.text), slashCommands, + config, ); const resetCompletionState = completion.resetCompletionState; diff --git a/packages/cli/src/ui/hooks/atCommandProcessor.test.ts b/packages/cli/src/ui/hooks/atCommandProcessor.test.ts index b0a4cc13..a9f43062 100644 --- a/packages/cli/src/ui/hooks/atCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/atCommandProcessor.test.ts @@ -5,8 +5,9 @@ */ import { describe, it, expect, vi, beforeEach, afterEach, Mock } from 'vitest'; +import type { Mocked } from 'vitest'; import { handleAtCommand } from './atCommandProcessor.js'; -import { Config } from '@gemini-code/core'; +import { Config, FileDiscoveryService } from '@gemini-code/core'; import { ToolCallStatus } from '../types.js'; import { UseHistoryManagerReturn } from './useHistoryManager.js'; import * as fsPromises from 'fs/promises'; @@ -18,6 +19,9 @@ const mockConfig = { getToolRegistry: mockGetToolRegistry, getTargetDir: mockGetTargetDir, isSandboxed: vi.fn(() => false), + getFileService: vi.fn(), + getFileFilteringRespectGitIgnore: vi.fn(() => true), + getFileFilteringAllowBuildArtifacts: vi.fn(() => false), } as unknown as Config; const mockReadManyFilesExecute = vi.fn(); @@ -48,8 +52,17 @@ vi.mock('fs/promises', async () => { }; }); +vi.mock('@gemini-code/core', async () => { + const actual = await vi.importActual('@gemini-code/core'); + return { + ...actual, + FileDiscoveryService: vi.fn(), + }; +}); + describe('handleAtCommand', () => { let abortController: AbortController; + let mockFileDiscoveryService: Mocked<FileDiscoveryService>; beforeEach(() => { vi.resetAllMocks(); @@ -73,6 +86,23 @@ describe('handleAtCommand', () => { llmContent: 'No files found', returnDisplay: '', }); + + // Mock FileDiscoveryService + mockFileDiscoveryService = { + initialize: vi.fn(), + shouldIgnoreFile: vi.fn(() => false), + filterFiles: vi.fn((files) => files), + getIgnoreInfo: vi.fn(() => ({ gitIgnored: [], customIgnored: [] })), + isGitRepository: vi.fn(() => true), + }; + vi.mocked(FileDiscoveryService).mockImplementation( + () => mockFileDiscoveryService, + ); + + // Mock getFileService to return the mocked FileDiscoveryService + mockConfig.getFileService = vi + .fn() + .mockResolvedValue(mockFileDiscoveryService); }); afterEach(() => { @@ -143,7 +173,7 @@ ${fileContent}`, 125, ); expect(mockReadManyFilesExecute).toHaveBeenCalledWith( - { paths: [filePath] }, + { paths: [filePath], respectGitIgnore: true }, abortController.signal, ); expect(mockAddItem).toHaveBeenCalledWith( @@ -191,7 +221,7 @@ ${fileContent}`, 126, ); expect(mockReadManyFilesExecute).toHaveBeenCalledWith( - { paths: [resolvedGlob] }, + { paths: [resolvedGlob], respectGitIgnore: true }, abortController.signal, ); expect(mockOnDebugMessage).toHaveBeenCalledWith( @@ -295,7 +325,7 @@ ${fileContent}`, signal: abortController.signal, }); expect(mockReadManyFilesExecute).toHaveBeenCalledWith( - { paths: [unescapedPath] }, + { paths: [unescapedPath], respectGitIgnore: true }, abortController.signal, ); }); @@ -325,7 +355,7 @@ ${content2}`, signal: abortController.signal, }); expect(mockReadManyFilesExecute).toHaveBeenCalledWith( - { paths: [file1, file2] }, + { paths: [file1, file2], respectGitIgnore: true }, abortController.signal, ); expect(result.processedQuery).toEqual([ @@ -368,7 +398,7 @@ ${content2}`, signal: abortController.signal, }); expect(mockReadManyFilesExecute).toHaveBeenCalledWith( - { paths: [file1, file2] }, + { paths: [file1, file2], respectGitIgnore: true }, abortController.signal, ); expect(result.processedQuery).toEqual([ @@ -434,7 +464,7 @@ ${content2}`, }); expect(mockReadManyFilesExecute).toHaveBeenCalledWith( - { paths: [file1, resolvedFile2] }, + { paths: [file1, resolvedFile2], respectGitIgnore: true }, abortController.signal, ); expect(result.processedQuery).toEqual([ @@ -538,7 +568,7 @@ ${fileContent}`, // If the mock is simpler, it might use queryPath if stat(queryPath) succeeds. // The most important part is that *some* version of the path that leads to the content is used. // Let's assume it uses the path from the query if stat confirms it exists (even if different case on disk) - { paths: [queryPath] }, + { paths: [queryPath], respectGitIgnore: true }, abortController.signal, ); expect(mockAddItem).toHaveBeenCalledWith( @@ -557,4 +587,154 @@ ${fileContent}`, ]); expect(result.shouldProceed).toBe(true); }); + + describe('git-aware filtering', () => { + it('should skip git-ignored files in @ commands', async () => { + const gitIgnoredFile = 'node_modules/package.json'; + const query = `@${gitIgnoredFile}`; + + // Mock the file discovery service to report this file as git-ignored + mockFileDiscoveryService.shouldIgnoreFile.mockImplementation( + (path: string) => path === gitIgnoredFile, + ); + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 200, + signal: abortController.signal, + }); + + expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith( + gitIgnoredFile, + ); + expect(mockOnDebugMessage).toHaveBeenCalledWith( + `Path ${gitIgnoredFile} is git-ignored and will be skipped.`, + ); + expect(mockOnDebugMessage).toHaveBeenCalledWith( + 'Ignored 1 git-ignored files: node_modules/package.json', + ); + expect(mockReadManyFilesExecute).not.toHaveBeenCalled(); + expect(result.processedQuery).toEqual([{ text: query }]); + expect(result.shouldProceed).toBe(true); + }); + + it('should process non-git-ignored files normally', async () => { + const validFile = 'src/index.ts'; + const query = `@${validFile}`; + const fileContent = 'console.log("Hello world");'; + + mockFileDiscoveryService.shouldIgnoreFile.mockReturnValue(false); + mockReadManyFilesExecute.mockResolvedValue({ + llmContent: ` +--- ${validFile} --- +${fileContent}`, + returnDisplay: 'Read 1 file.', + }); + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 201, + signal: abortController.signal, + }); + + expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith( + validFile, + ); + expect(mockReadManyFilesExecute).toHaveBeenCalledWith( + { paths: [validFile], respectGitIgnore: true }, + abortController.signal, + ); + expect(result.processedQuery).toEqual([ + { text: `@${validFile}` }, + { text: '\n--- Content from referenced files ---' }, + { text: `\nContent from @${validFile}:\n` }, + { text: fileContent }, + { text: '\n--- End of content ---' }, + ]); + expect(result.shouldProceed).toBe(true); + }); + + it('should handle mixed git-ignored and valid files', async () => { + const validFile = 'README.md'; + const gitIgnoredFile = '.env'; + const query = `@${validFile} @${gitIgnoredFile}`; + const fileContent = '# Project README'; + + mockFileDiscoveryService.shouldIgnoreFile.mockImplementation( + (path: string) => path === gitIgnoredFile, + ); + mockReadManyFilesExecute.mockResolvedValue({ + llmContent: ` +--- ${validFile} --- +${fileContent}`, + returnDisplay: 'Read 1 file.', + }); + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 202, + signal: abortController.signal, + }); + + expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith( + validFile, + ); + expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith( + gitIgnoredFile, + ); + expect(mockOnDebugMessage).toHaveBeenCalledWith( + `Path ${gitIgnoredFile} is git-ignored and will be skipped.`, + ); + expect(mockOnDebugMessage).toHaveBeenCalledWith( + 'Ignored 1 git-ignored files: .env', + ); + expect(mockReadManyFilesExecute).toHaveBeenCalledWith( + { paths: [validFile], respectGitIgnore: true }, + abortController.signal, + ); + expect(result.processedQuery).toEqual([ + { text: `@${validFile} @${gitIgnoredFile}` }, + { text: '\n--- Content from referenced files ---' }, + { text: `\nContent from @${validFile}:\n` }, + { text: fileContent }, + { text: '\n--- End of content ---' }, + ]); + expect(result.shouldProceed).toBe(true); + }); + + it('should always ignore .git directory files', async () => { + const gitFile = '.git/config'; + const query = `@${gitFile}`; + + mockFileDiscoveryService.shouldIgnoreFile.mockReturnValue(true); + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 203, + signal: abortController.signal, + }); + + expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith( + gitFile, + ); + expect(mockOnDebugMessage).toHaveBeenCalledWith( + `Path ${gitFile} is git-ignored and will be skipped.`, + ); + expect(mockReadManyFilesExecute).not.toHaveBeenCalled(); + expect(result.processedQuery).toEqual([{ text: query }]); + expect(result.shouldProceed).toBe(true); + }); + }); }); diff --git a/packages/cli/src/ui/hooks/atCommandProcessor.ts b/packages/cli/src/ui/hooks/atCommandProcessor.ts index ac56ab75..c534207c 100644 --- a/packages/cli/src/ui/hooks/atCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/atCommandProcessor.ts @@ -134,9 +134,14 @@ export async function handleAtCommand({ addItem({ type: 'user', text: query }, userMessageTimestamp); + // Get centralized file discovery service + const fileDiscovery = await config.getFileService(); + const respectGitIgnore = config.getFileFilteringRespectGitIgnore(); + const pathSpecsToRead: string[] = []; const atPathToResolvedSpecMap = new Map<string, string>(); const contentLabelsForDisplay: string[] = []; + const ignoredPaths: string[] = []; const toolRegistry = await config.getToolRegistry(); const readManyFilesTool = toolRegistry.getTool('read_many_files'); @@ -176,6 +181,16 @@ export async function handleAtCommand({ return { processedQuery: null, shouldProceed: false }; } + // Check if path should be ignored by git + if (fileDiscovery.shouldIgnoreFile(pathName)) { + const reason = respectGitIgnore + ? 'git-ignored and will be skipped' + : 'ignored by custom patterns'; + onDebugMessage(`Path ${pathName} is ${reason}.`); + ignoredPaths.push(pathName); + continue; + } + let currentPathSpec = pathName; let resolvedSuccessfully = false; @@ -305,6 +320,14 @@ export async function handleAtCommand({ } initialQueryText = initialQueryText.trim(); + // Inform user about ignored paths + if (ignoredPaths.length > 0) { + const ignoreType = respectGitIgnore ? 'git-ignored' : 'custom-ignored'; + onDebugMessage( + `Ignored ${ignoredPaths.length} ${ignoreType} files: ${ignoredPaths.join(', ')}`, + ); + } + // Fallback for lone "@" or completely invalid @-commands resulting in empty initialQueryText if (pathSpecsToRead.length === 0) { onDebugMessage('No valid file paths found in @ commands to read.'); @@ -324,7 +347,10 @@ export async function handleAtCommand({ const processedQueryParts: PartUnion[] = [{ text: initialQueryText }]; - const toolArgs = { paths: pathSpecsToRead }; + const toolArgs = { + paths: pathSpecsToRead, + respectGitIgnore, // Use configuration setting + }; let toolCallDisplay: IndividualToolCallDisplay; try { diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index 6515ed78..51def9d5 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -161,6 +161,7 @@ export const useSlashCommandProcessor = ( } }, }, + { name: 'corgi', action: (_mainCommand, _subCommand, _args) => { diff --git a/packages/cli/src/ui/hooks/useCompletion.integration.test.ts b/packages/cli/src/ui/hooks/useCompletion.integration.test.ts new file mode 100644 index 00000000..5235dbd5 --- /dev/null +++ b/packages/cli/src/ui/hooks/useCompletion.integration.test.ts @@ -0,0 +1,228 @@ +/** + * @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 { renderHook, act } from '@testing-library/react'; +import { useCompletion } from './useCompletion.js'; +import * as fs from 'fs/promises'; +import { FileDiscoveryService } from '@gemini-code/core'; + +// Mock dependencies +vi.mock('fs/promises'); +vi.mock('@gemini-code/core', async () => { + const actual = await vi.importActual('@gemini-code/core'); + return { + ...actual, + FileDiscoveryService: vi.fn(), + isNodeError: vi.fn((error) => error.code === 'ENOENT'), + escapePath: vi.fn((path) => path), + unescapePath: vi.fn((path) => path), + getErrorMessage: vi.fn((error) => error.message), + }; +}); + +describe('useCompletion git-aware filtering integration', () => { + let mockFileDiscoveryService: Mocked<FileDiscoveryService>; + let mockConfig: { + fileFiltering?: { enabled?: boolean; respectGitignore?: boolean }; + }; + const testCwd = '/test/project'; + const slashCommands = [ + { name: 'help', description: 'Show help', action: vi.fn() }, + { name: 'clear', description: 'Clear screen', action: vi.fn() }, + ]; + + beforeEach(() => { + mockFileDiscoveryService = { + initialize: vi.fn(), + shouldIgnoreFile: vi.fn(), + filterFiles: vi.fn(), + getIgnoreInfo: vi.fn(() => ({ gitIgnored: [], customIgnored: [] })), + }; + + mockConfig = { + getFileFilteringRespectGitIgnore: vi.fn(() => true), + getFileFilteringAllowBuildArtifacts: vi.fn(() => false), + getFileService: vi.fn().mockResolvedValue(mockFileDiscoveryService), + }; + + vi.mocked(FileDiscoveryService).mockImplementation( + () => mockFileDiscoveryService, + ); + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('should filter git-ignored directories from @ completions', async () => { + // Mock fs.readdir to return both regular and git-ignored directories + vi.mocked(fs.readdir).mockResolvedValue([ + { name: 'src', isDirectory: () => true }, + { name: 'node_modules', isDirectory: () => true }, + { name: 'dist', isDirectory: () => true }, + { name: 'README.md', isDirectory: () => false }, + { name: '.env', isDirectory: () => false }, + ] as Array<{ name: string; isDirectory: () => boolean }>); + + // Mock git ignore service to ignore certain files + mockFileDiscoveryService.shouldIgnoreFile.mockImplementation( + (path: string) => + path.includes('node_modules') || + path.includes('dist') || + path.includes('.env'), + ); + + const { result } = renderHook(() => + useCompletion('@', testCwd, true, slashCommands, mockConfig), + ); + + // Wait for async operations to complete + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 150)); // Account for debounce + }); + + expect(result.current.suggestions).toHaveLength(2); + expect(result.current.suggestions).toEqual( + expect.arrayContaining([ + { label: 'src/', value: 'src/' }, + { label: 'README.md', value: 'README.md' }, + ]), + ); + expect(result.current.showSuggestions).toBe(true); + }); + + it('should handle recursive search with git-aware filtering', async () => { + // Mock the recursive file search scenario + vi.mocked(fs.readdir).mockImplementation( + async (dirPath: string | Buffer | URL) => { + if (dirPath === testCwd) { + return [ + { name: 'src', isDirectory: () => true }, + { name: 'node_modules', isDirectory: () => true }, + { name: 'temp', isDirectory: () => true }, + ] as Array<{ name: string; isDirectory: () => boolean }>; + } + if (dirPath.endsWith('/src')) { + return [ + { name: 'index.ts', isDirectory: () => false }, + { name: 'components', isDirectory: () => true }, + ] as Array<{ name: string; isDirectory: () => boolean }>; + } + if (dirPath.endsWith('/temp')) { + return [{ name: 'temp.log', isDirectory: () => false }] as Array<{ + name: string; + isDirectory: () => boolean; + }>; + } + return [] as Array<{ name: string; isDirectory: () => boolean }>; + }, + ); + + // Mock git ignore service + mockFileDiscoveryService.shouldIgnoreFile.mockImplementation( + (path: string) => path.includes('node_modules') || path.includes('temp'), + ); + + const { result } = renderHook(() => + useCompletion('@t', testCwd, true, slashCommands, mockConfig), + ); + + // Wait for async operations to complete + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 150)); + }); + + // Should not include anything from node_modules or dist + const suggestionLabels = result.current.suggestions.map((s) => s.label); + expect(suggestionLabels).not.toContain('temp/'); + expect(suggestionLabels.some((l) => l.includes('node_modules'))).toBe( + false, + ); + }); + + it('should work without config (fallback behavior)', async () => { + vi.mocked(fs.readdir).mockResolvedValue([ + { name: 'src', isDirectory: () => true }, + { name: 'node_modules', isDirectory: () => true }, + { name: 'README.md', isDirectory: () => false }, + ] as Array<{ name: string; isDirectory: () => boolean }>); + + const { result } = renderHook(() => + useCompletion('@', testCwd, true, slashCommands, undefined), + ); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 150)); + }); + + // Without config, should include all files + expect(result.current.suggestions).toHaveLength(3); + expect(result.current.suggestions).toEqual( + expect.arrayContaining([ + { label: 'src/', value: 'src/' }, + { label: 'node_modules/', value: 'node_modules/' }, + { label: 'README.md', value: 'README.md' }, + ]), + ); + }); + + 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 }, + ] as Array<{ name: string; isDirectory: () => boolean }>); + + const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const { result } = renderHook(() => + useCompletion('@', testCwd, true, slashCommands, mockConfig), + ); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 150)); + }); + + // Since we use centralized service, initialization errors are handled at config level + // This test should verify graceful fallback behavior + expect(result.current.suggestions.length).toBeGreaterThanOrEqual(0); + // Should still show completions even if git discovery fails + expect(result.current.suggestions.length).toBeGreaterThan(0); + + consoleSpy.mockRestore(); + }); + + it('should handle directory-specific completions with git filtering', async () => { + vi.mocked(fs.readdir).mockResolvedValue([ + { name: 'component.tsx', isDirectory: () => false }, + { name: 'temp.log', isDirectory: () => false }, + { name: 'index.ts', isDirectory: () => false }, + ] as Array<{ name: string; isDirectory: () => boolean }>); + + mockFileDiscoveryService.shouldIgnoreFile.mockImplementation( + (path: string) => path.includes('.log'), + ); + + const { result } = renderHook(() => + useCompletion('@src/comp', testCwd, true, slashCommands, mockConfig), + ); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 150)); + }); + + // Should filter out .log files but include matching .tsx files + expect(result.current.suggestions).toEqual([ + { label: 'component.tsx', value: 'component.tsx' }, + ]); + }); +}); diff --git a/packages/cli/src/ui/hooks/useCompletion.ts b/packages/cli/src/ui/hooks/useCompletion.ts index f3ad7847..520ba7c6 100644 --- a/packages/cli/src/ui/hooks/useCompletion.ts +++ b/packages/cli/src/ui/hooks/useCompletion.ts @@ -12,6 +12,7 @@ import { escapePath, unescapePath, getErrorMessage, + Config, } from '@gemini-code/core'; import { MAX_SUGGESTIONS_TO_SHOW, @@ -37,6 +38,7 @@ export function useCompletion( cwd: string, isActive: boolean, slashCommands: SlashCommand[], + config?: Config, ): UseCompletionReturn { const [suggestions, setSuggestions] = useState<Suggestion[]>([]); const [activeSuggestionIndex, setActiveSuggestionIndex] = @@ -184,6 +186,7 @@ export function useCompletion( const findFilesRecursively = async ( startDir: string, searchPrefix: string, + fileDiscovery: { shouldIgnoreFile: (path: string) => boolean } | null, currentRelativePath = '', depth = 0, maxDepth = 10, // Limit recursion depth @@ -201,6 +204,19 @@ export function useCompletion( if (foundSuggestions.length >= maxResults) break; const entryPathRelative = path.join(currentRelativePath, entry.name); + const entryPathFromRoot = path.relative( + cwd, + path.join(startDir, entry.name), + ); + + // Check if this entry should be ignored by git-aware filtering + if ( + fileDiscovery && + fileDiscovery.shouldIgnoreFile(entryPathFromRoot) + ) { + continue; + } + if (entry.name.toLowerCase().startsWith(lowerSearchPrefix)) { foundSuggestions.push({ label: entryPathRelative + (entry.isDirectory() ? '/' : ''), @@ -219,6 +235,7 @@ export function useCompletion( await findFilesRecursively( path.join(startDir, entry.name), searchPrefix, // Pass original searchPrefix for recursive calls + fileDiscovery, entryPathRelative, depth + 1, maxDepth, @@ -237,25 +254,48 @@ export function useCompletion( const fetchSuggestions = async () => { setIsLoadingSuggestions(true); let fetchedSuggestions: Suggestion[] = []; + + // Get centralized file discovery service if config is available + const fileDiscovery = config ? await config.getFileService() : null; + try { // If there's no slash, or it's the root, do a recursive search from cwd if (partialPath.indexOf('/') === -1 && prefix) { - fetchedSuggestions = await findFilesRecursively(cwd, prefix); + fetchedSuggestions = await findFilesRecursively( + cwd, + prefix, + fileDiscovery, + ); } else { // Original behavior: list files in the specific directory const lowerPrefix = prefix.toLowerCase(); const entries = await fs.readdir(baseDirAbsolute, { withFileTypes: true, }); - fetchedSuggestions = entries - .filter((entry) => entry.name.toLowerCase().startsWith(lowerPrefix)) - .map((entry) => { - const label = entry.isDirectory() ? entry.name + '/' : entry.name; - return { - label, - value: escapePath(label), // Value for completion should be just the name part - }; - }); + + // Filter entries using git-aware filtering + const filteredEntries = []; + for (const entry of entries) { + if (!entry.name.toLowerCase().startsWith(lowerPrefix)) continue; + + const relativePath = path.relative( + cwd, + path.join(baseDirAbsolute, entry.name), + ); + if (fileDiscovery && fileDiscovery.shouldIgnoreFile(relativePath)) { + continue; + } + + filteredEntries.push(entry); + } + + fetchedSuggestions = filteredEntries.map((entry) => { + const label = entry.isDirectory() ? entry.name + '/' : entry.name; + return { + label, + value: escapePath(label), // Value for completion should be just the name part + }; + }); } // Sort by depth, then directories first, then alphabetically @@ -307,7 +347,7 @@ export function useCompletion( isMounted = false; clearTimeout(debounceTimeout); }; - }, [query, cwd, isActive, resetCompletionState, slashCommands]); + }, [query, cwd, isActive, resetCompletionState, slashCommands, config]); return { suggestions, |
