diff --git a/docs/reference/commands.md b/docs/reference/commands.md index e95be7826c7..1e610a1dc21 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -532,8 +532,9 @@ your prompt to Gemini. These commands include git-aware filtering. - `What is this file about? @README.md` - **Details:** - If a path to a single file is provided, the content of that file is read. - - If a path to a directory is provided, the command attempts to read the - content of files within that directory and any subdirectories. + - If a path to a directory is provided, the directory path is resolved and + passed to the model so it can inspect or list the directory using its + tools without eagerly reading every nested file into the prompt. - Spaces in paths should be escaped with a backslash (for example, `@My\ Documents/file.txt`). - The command uses the `read_many_files` tool internally. The content is diff --git a/packages/cli/src/ui/hooks/atCommandProcessor.test.ts b/packages/cli/src/ui/hooks/atCommandProcessor.test.ts index b41abdb5688..8e9ef08c01b 100644 --- a/packages/cli/src/ui/hooks/atCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/atCommandProcessor.test.ts @@ -242,7 +242,7 @@ describe('handleAtCommand', () => { ); }); - it('should process a valid directory path and convert to glob', async () => { + it('should resolve a valid directory path without eagerly reading all files', async () => { const fileContent = 'This is the file content.'; const filePath = await createTestFile( path.join(testRootDir, 'path', 'to', 'file.txt'), @@ -250,9 +250,7 @@ describe('handleAtCommand', () => { ); const dirPath = path.dirname(filePath); const relativeDirPath = getRelativePath(dirPath); - const relativeFilePath = getRelativePath(filePath); const query = `@${dirPath}`; - const resolvedGlob = path.join(relativeDirPath, '**'); const result = await handleAtCommand({ query, @@ -263,17 +261,115 @@ describe('handleAtCommand', () => { signal: abortController.signal, }); + expect(result).toEqual({ + processedQuery: [{ text: `@${relativeDirPath}` }], + }); + expect(mockAddItem).not.toHaveBeenCalled(); + expect(mockOnDebugMessage).toHaveBeenCalledWith( + `Path ${dirPath} resolved to directory: ${dirPath}, using relative path: ${relativeDirPath}`, + ); + }); + + it('should normalize an absolute path to the workspace root directory to @.', async () => { + const query = `List files in @${testRootDir}`; + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 1261, + signal: abortController.signal, + }); + + expect(result).toEqual({ + processedQuery: [{ text: 'List files in @.' }], + }); + expect(mockAddItem).not.toHaveBeenCalled(); + expect(mockOnDebugMessage).toHaveBeenCalledWith( + `Path ${testRootDir} resolved to directory: ${testRootDir}, using relative path: .`, + ); + }); + + it('should not eagerly read file contents when asking to list files in @', async () => { + await createTestFile( + path.join(testRootDir, 'Documents', 'internal', 'dev-link.sh'), + '#!/bin/bash\necho "dev-link"', + ); + await createTestFile( + path.join( + testRootDir, + 'Documents', + 'internal', + 'poc', + 'gemini_poc', + 'lib', + 'large_module.py', + ), + 'x = 1\n'.repeat(1000), + ); + + const query = 'List all files stored in @Documents/internal/'; + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 127, + signal: abortController.signal, + }); + expect(result).toEqual({ processedQuery: [ - { text: `@${resolvedGlob}` }, + { text: 'List all files stored in @Documents/internal/' }, + ], + }); + expect(mockAddItem).not.toHaveBeenCalled(); + }); + + it('should read @ content while skipping eager file reads for @ in mixed queries', async () => { + const fileContent = 'File A content'; + const filePath = await createTestFile( + path.join(testRootDir, 'fileA.txt'), + fileContent, + ); + await createTestFile( + path.join(testRootDir, 'Documents', 'internal', 'nested.txt'), + 'Nested directory file content that should not be eagerly read', + ); + + const relativeFilePath = getRelativePath(filePath); + const query = `Compare @${relativeFilePath} with files in @Documents/internal/`; + + const result = await handleAtCommand({ + query, + config: mockConfig, + addItem: mockAddItem, + onDebugMessage: mockOnDebugMessage, + messageId: 1271, + signal: abortController.signal, + }); + + expect(result).toEqual({ + processedQuery: [ + { + text: `Compare @${relativeFilePath} with files in @Documents/internal/`, + }, { text: '\n--- Content from referenced files ---' }, { text: `\nContent from @${relativeFilePath}:\n` }, { text: fileContent }, { text: '\n--- End of content ---' }, ], }); - expect(mockOnDebugMessage).toHaveBeenCalledWith( - `Path ${dirPath} resolved to directory, using glob: ${resolvedGlob}`, + expect(mockAddItem).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'tool_group', + tools: [ + expect.objectContaining({ status: CoreToolCallStatus.Success }), + ], + }), + 1271, ); }); @@ -1271,17 +1367,13 @@ describe('handleAtCommand', () => { expect(result.processedQuery).not.toBeNull(); expect(result.error).toBeUndefined(); - expect(result.processedQuery).toEqual( - expect.arrayContaining([ - { text: `Check @${path.join(subDirPath, '**')} please.` }, - expect.objectContaining({ - text: '\n--- Content from referenced files ---', - }), - ]), - ); + expect(result.processedQuery).toEqual([ + { text: `Check @${subDirPath} please.` }, + ]); + expect(mockAddItem).not.toHaveBeenCalled(); expect(mockOnDebugMessage).toHaveBeenCalledWith( - expect.stringContaining(`using glob: ${path.join(subDirPath, '**')}`), + expect.stringContaining(`using relative path: ${subDirPath}`), ); }); }); diff --git a/packages/cli/src/ui/hooks/atCommandProcessor.ts b/packages/cli/src/ui/hooks/atCommandProcessor.ts index 45a7e104824..687476eb815 100644 --- a/packages/cli/src/ui/hooks/atCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/atCommandProcessor.ts @@ -225,6 +225,7 @@ interface ResolvedFile { pathSpec: string; displayLabel: string; absolutePath?: string; + isDirectory?: boolean; } interface IgnoredFile { @@ -299,28 +300,20 @@ async function resolveFilePaths( if (result.status === 'resolved') { const { absolutePath, relativePath, stats } = result.resolved; - if (stats.isDirectory()) { - const pathSpec = path.join(relativePath, '**'); - resolvedFiles.push({ - part, - pathSpec, - displayLabel: path.isAbsolute(pathName) ? relativePath : pathName, - absolutePath, - }); - onDebugMessage( - `Path ${pathName} resolved to directory, using glob: ${pathSpec}`, - ); - } else { - resolvedFiles.push({ - part, - pathSpec: relativePath, - displayLabel: path.isAbsolute(pathName) ? relativePath : pathName, - absolutePath, - }); - onDebugMessage( - `Path ${pathName} resolved to file: ${absolutePath}, using relative path: ${relativePath}`, - ); - } + const isDirectory = stats.isDirectory(); + const normalizedRelativePath = relativePath || '.'; + resolvedFiles.push({ + part, + pathSpec: normalizedRelativePath, + displayLabel: path.isAbsolute(pathName) + ? normalizedRelativePath + : pathName, + absolutePath, + isDirectory, + }); + onDebugMessage( + `Path ${pathName} resolved to ${isDirectory ? 'directory' : 'file'}: ${absolutePath}, using relative path: ${normalizedRelativePath}`, + ); } else if ( result.status === 'not_found' || result.status === 'unauthorized' @@ -549,7 +542,8 @@ async function readLocalFiles( display?: IndividualToolCallDisplay; error?: string; }> { - if (resolvedFiles.length === 0) { + const filesToRead = resolvedFiles.filter((rf) => !(rf.isDirectory ?? false)); + if (filesToRead.length === 0) { return { parts: [] }; } @@ -558,15 +552,10 @@ async function readLocalFiles( config.getMessageBus(), ); - const pathSpecsToRead = resolvedFiles.map((rf) => { - if (rf.absolutePath) { - return rf.pathSpec.endsWith('**') - ? path.join(rf.absolutePath, '**') - : rf.absolutePath; - } - return rf.pathSpec; - }); - const fileLabelsForDisplay = resolvedFiles.map((rf) => rf.displayLabel); + const pathSpecsToRead = filesToRead.map( + (rf) => rf.absolutePath ?? rf.pathSpec, + ); + const fileLabelsForDisplay = filesToRead.map((rf) => rf.displayLabel); const respectFileIgnore = config.getFileFilteringOptions(); const toolArgs = { @@ -604,7 +593,7 @@ async function readLocalFiles( const fileActualContent = match[2].trim(); // Find the display label for this path - const resolvedFile = resolvedFiles.find( + const resolvedFile = filesToRead.find( (rf) => rf.absolutePath === filePathSpecInContent || rf.pathSpec === filePathSpecInContent, @@ -698,7 +687,8 @@ function reportIgnoredFiles( /** * Processes user input containing one or more '@' commands. - * - Workspace paths are read via the 'read_many_files' tool. + * - Workspace file paths are read via the 'read_many_files' tool. + * - Workspace directory paths are resolved to relative paths in the query without eagerly reading their contents. * - MCP resource URIs are read via each server's `resources/read`. * The user query is updated with inline content blocks so the LLM receives the * referenced context directly.