Add .geminiignore support to the glob tool. (#8086)

This commit is contained in:
Tommaso Sciortino
2025-09-10 09:54:50 -07:00
committed by GitHub
parent ef70c17936
commit 0e3a0d7dc5
6 changed files with 388 additions and 470 deletions
@@ -16,6 +16,12 @@ export interface FilterFilesOptions {
respectGeminiIgnore?: boolean; respectGeminiIgnore?: boolean;
} }
export interface FilterReport {
filteredPaths: string[];
gitIgnoredCount: number;
geminiIgnoredCount: number;
}
export class FileDiscoveryService { export class FileDiscoveryService {
private gitIgnoreFilter: GitIgnoreFilter | null = null; private gitIgnoreFilter: GitIgnoreFilter | null = null;
private geminiIgnoreFilter: GitIgnoreFilter | null = null; private geminiIgnoreFilter: GitIgnoreFilter | null = null;
@@ -65,6 +71,42 @@ export class FileDiscoveryService {
}); });
} }
/**
* Filters a list of file paths based on git ignore rules and returns a report
* with counts of ignored files.
*/
filterFilesWithReport(
filePaths: string[],
opts: FilterFilesOptions = {
respectGitIgnore: true,
respectGeminiIgnore: true,
},
): FilterReport {
const filteredPaths: string[] = [];
let gitIgnoredCount = 0;
let geminiIgnoredCount = 0;
for (const filePath of filePaths) {
if (opts.respectGitIgnore && this.shouldGitIgnoreFile(filePath)) {
gitIgnoredCount++;
continue;
}
if (opts.respectGeminiIgnore && this.shouldGeminiIgnoreFile(filePath)) {
geminiIgnoredCount++;
continue;
}
filteredPaths.push(filePath);
}
return {
filteredPaths,
gitIgnoredCount,
geminiIgnoredCount,
};
}
/** /**
* Checks if a single file should be git-ignored * Checks if a single file should be git-ignored
*/ */
+87
View File
@@ -28,6 +28,10 @@ describe('GlobTool', () => {
const mockConfig = { const mockConfig = {
getFileService: () => new FileDiscoveryService(tempRootDir), getFileService: () => new FileDiscoveryService(tempRootDir),
getFileFilteringRespectGitIgnore: () => true, getFileFilteringRespectGitIgnore: () => true,
getFileFilteringOptions: () => ({
respectGitIgnore: true,
respectGeminiIgnore: true,
}),
getTargetDir: () => tempRootDir, getTargetDir: () => tempRootDir,
getWorkspaceContext: () => createMockWorkspaceContext(tempRootDir), getWorkspaceContext: () => createMockWorkspaceContext(tempRootDir),
getFileExclusions: () => ({ getFileExclusions: () => ({
@@ -38,6 +42,7 @@ describe('GlobTool', () => {
beforeEach(async () => { beforeEach(async () => {
// Create a unique root directory for each test run // Create a unique root directory for each test run
tempRootDir = await fs.mkdtemp(path.join(os.tmpdir(), 'glob-tool-root-')); tempRootDir = await fs.mkdtemp(path.join(os.tmpdir(), 'glob-tool-root-'));
await fs.writeFile(path.join(tempRootDir, '.git'), ''); // Fake git repo
globTool = new GlobTool(mockConfig); globTool = new GlobTool(mockConfig);
// Create some test files and directories within this root // Create some test files and directories within this root
@@ -366,6 +371,88 @@ describe('GlobTool', () => {
expect(result.llmContent).toContain('FileD.MD'); expect(result.llmContent).toContain('FileD.MD');
}); });
}); });
describe('ignore file handling', () => {
it('should respect .gitignore files by default', async () => {
await fs.writeFile(path.join(tempRootDir, '.gitignore'), '*.ignored.txt');
await fs.writeFile(
path.join(tempRootDir, 'a.ignored.txt'),
'ignored content',
);
await fs.writeFile(
path.join(tempRootDir, 'b.notignored.txt'),
'not ignored content',
);
const params: GlobToolParams = { pattern: '*.txt' };
const invocation = globTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 3 file(s)'); // fileA.txt, FileB.TXT, b.notignored.txt
expect(result.llmContent).not.toContain('a.ignored.txt');
});
it('should respect .geminiignore files by default', async () => {
await fs.writeFile(
path.join(tempRootDir, '.geminiignore'),
'*.geminiignored.txt',
);
await fs.writeFile(
path.join(tempRootDir, 'a.geminiignored.txt'),
'ignored content',
);
await fs.writeFile(
path.join(tempRootDir, 'b.notignored.txt'),
'not ignored content',
);
const params: GlobToolParams = { pattern: '*.txt' };
const invocation = globTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 3 file(s)'); // fileA.txt, FileB.TXT, b.notignored.txt
expect(result.llmContent).not.toContain('a.geminiignored.txt');
});
it('should not respect .gitignore when respect_git_ignore is false', async () => {
await fs.writeFile(path.join(tempRootDir, '.gitignore'), '*.ignored.txt');
await fs.writeFile(
path.join(tempRootDir, 'a.ignored.txt'),
'ignored content',
);
const params: GlobToolParams = {
pattern: '*.txt',
respect_git_ignore: false,
};
const invocation = globTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 3 file(s)'); // fileA.txt, FileB.TXT, a.ignored.txt
expect(result.llmContent).toContain('a.ignored.txt');
});
it('should not respect .geminiignore when respect_gemini_ignore is false', async () => {
await fs.writeFile(
path.join(tempRootDir, '.geminiignore'),
'*.geminiignored.txt',
);
await fs.writeFile(
path.join(tempRootDir, 'a.geminiignored.txt'),
'ignored content',
);
const params: GlobToolParams = {
pattern: '*.txt',
respect_gemini_ignore: false,
};
const invocation = globTool.build(params);
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Found 3 file(s)'); // fileA.txt, FileB.TXT, a.geminiignored.txt
expect(result.llmContent).toContain('a.geminiignored.txt');
});
});
}); });
describe('sortFileEntries', () => { describe('sortFileEntries', () => {
+42 -28
View File
@@ -10,7 +10,10 @@ import { glob, escape } from 'glob';
import type { ToolInvocation, ToolResult } from './tools.js'; import type { ToolInvocation, ToolResult } from './tools.js';
import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js'; import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js';
import { shortenPath, makeRelative } from '../utils/paths.js'; import { shortenPath, makeRelative } from '../utils/paths.js';
import type { Config } from '../config/config.js'; import {
type Config,
DEFAULT_FILE_FILTERING_OPTIONS,
} from '../config/config.js';
import { ToolErrorType } from './tool-error.js'; import { ToolErrorType } from './tool-error.js';
// Subset of 'Path' interface provided by 'glob' that we can implement for testing // Subset of 'Path' interface provided by 'glob' that we can implement for testing
@@ -72,6 +75,11 @@ export interface GlobToolParams {
* Whether to respect .gitignore patterns (optional, defaults to true) * Whether to respect .gitignore patterns (optional, defaults to true)
*/ */
respect_git_ignore?: boolean; respect_git_ignore?: boolean;
/**
* Whether to respect .geminiignore patterns (optional, defaults to true)
*/
respect_gemini_ignore?: boolean;
} }
class GlobToolInvocation extends BaseToolInvocation< class GlobToolInvocation extends BaseToolInvocation<
@@ -128,14 +136,10 @@ class GlobToolInvocation extends BaseToolInvocation<
} }
// Get centralized file discovery service // Get centralized file discovery service
const respectGitIgnore =
this.params.respect_git_ignore ??
this.config.getFileFilteringRespectGitIgnore();
const fileDiscovery = this.config.getFileService(); const fileDiscovery = this.config.getFileService();
// Collect entries from all search directories // Collect entries from all search directories
let allEntries: GlobPath[] = []; const allEntries: GlobPath[] = [];
for (const searchDir of searchDirectories) { for (const searchDir of searchDirectories) {
let pattern = this.params.pattern; let pattern = this.params.pattern;
const fullPath = path.join(searchDir, pattern); const fullPath = path.join(searchDir, pattern);
@@ -155,33 +159,32 @@ class GlobToolInvocation extends BaseToolInvocation<
signal, signal,
})) as GlobPath[]; })) as GlobPath[];
allEntries = allEntries.concat(entries); allEntries.push(...entries);
} }
const entries = allEntries; const relativePaths = allEntries.map((p) =>
path.relative(this.config.getTargetDir(), p.fullpath()),
);
// Apply git-aware filtering if enabled and in git repository const { filteredPaths, gitIgnoredCount, geminiIgnoredCount } =
let filteredEntries = entries; fileDiscovery.filterFilesWithReport(relativePaths, {
let gitIgnoredCount = 0; respectGitIgnore:
this.params?.respect_git_ignore ??
if (respectGitIgnore) { this.config.getFileFilteringOptions().respectGitIgnore ??
const relativePaths = entries.map((p) => DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore,
path.relative(this.config.getTargetDir(), p.fullpath()), respectGeminiIgnore:
); this.params?.respect_gemini_ignore ??
const filteredRelativePaths = fileDiscovery.filterFiles(relativePaths, { this.config.getFileFilteringOptions().respectGeminiIgnore ??
respectGitIgnore, DEFAULT_FILE_FILTERING_OPTIONS.respectGeminiIgnore,
}); });
const filteredAbsolutePaths = new Set(
filteredRelativePaths.map((p) =>
path.resolve(this.config.getTargetDir(), p),
),
);
filteredEntries = entries.filter((entry) => const filteredAbsolutePaths = new Set(
filteredAbsolutePaths.has(entry.fullpath()), filteredPaths.map((p) => path.resolve(this.config.getTargetDir(), p)),
); );
gitIgnoredCount = entries.length - filteredEntries.length;
} const filteredEntries = allEntries.filter((entry) =>
filteredAbsolutePaths.has(entry.fullpath()),
);
if (!filteredEntries || filteredEntries.length === 0) { if (!filteredEntries || filteredEntries.length === 0) {
let message = `No files found matching pattern "${this.params.pattern}"`; let message = `No files found matching pattern "${this.params.pattern}"`;
@@ -193,6 +196,9 @@ class GlobToolInvocation extends BaseToolInvocation<
if (gitIgnoredCount > 0) { if (gitIgnoredCount > 0) {
message += ` (${gitIgnoredCount} files were git-ignored)`; message += ` (${gitIgnoredCount} files were git-ignored)`;
} }
if (geminiIgnoredCount > 0) {
message += ` (${geminiIgnoredCount} files were gemini-ignored)`;
}
return { return {
llmContent: message, llmContent: message,
returnDisplay: `No files found`, returnDisplay: `No files found`,
@@ -225,6 +231,9 @@ class GlobToolInvocation extends BaseToolInvocation<
if (gitIgnoredCount > 0) { if (gitIgnoredCount > 0) {
resultMessage += ` (${gitIgnoredCount} additional files were git-ignored)`; resultMessage += ` (${gitIgnoredCount} additional files were git-ignored)`;
} }
if (geminiIgnoredCount > 0) {
resultMessage += ` (${geminiIgnoredCount} additional files were gemini-ignored)`;
}
resultMessage += `, sorted by modification time (newest first):\n${fileListDescription}`; resultMessage += `, sorted by modification time (newest first):\n${fileListDescription}`;
return { return {
@@ -282,6 +291,11 @@ export class GlobTool extends BaseDeclarativeTool<GlobToolParams, ToolResult> {
'Optional: Whether to respect .gitignore patterns when finding files. Only available in git repositories. Defaults to true.', 'Optional: Whether to respect .gitignore patterns when finding files. Only available in git repositories. Defaults to true.',
type: 'boolean', type: 'boolean',
}, },
respect_gemini_ignore: {
description:
'Optional: Whether to respect .geminiignore patterns when finding files. Defaults to true.',
type: 'boolean',
},
}, },
required: ['pattern'], required: ['pattern'],
type: 'object', type: 'object',
+163 -321
View File
@@ -4,65 +4,38 @@
* SPDX-License-Identifier: Apache-2.0 * SPDX-License-Identifier: Apache-2.0
*/ */
/* eslint-disable @typescript-eslint/no-explicit-any */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import fs from 'node:fs/promises';
import { describe, it, expect, beforeEach, vi } from 'vitest';
import fs from 'node:fs';
import path from 'node:path'; import path from 'node:path';
import os from 'node:os';
vi.mock('fs', () => ({
default: {
statSync: vi.fn(),
readdirSync: vi.fn(),
},
statSync: vi.fn(),
readdirSync: vi.fn(),
mkdirSync: vi.fn(),
}));
import { LSTool } from './ls.js'; import { LSTool } from './ls.js';
import type { Config } from '../config/config.js'; import type { Config } from '../config/config.js';
import type { WorkspaceContext } from '../utils/workspaceContext.js'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js';
import type { FileDiscoveryService } from '../services/fileDiscoveryService.js';
import { ToolErrorType } from './tool-error.js'; import { ToolErrorType } from './tool-error.js';
import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js';
describe('LSTool', () => { describe('LSTool', () => {
let lsTool: LSTool; let lsTool: LSTool;
let tempRootDir: string;
let tempSecondaryDir: string;
let mockConfig: Config; let mockConfig: Config;
let mockWorkspaceContext: WorkspaceContext; const abortSignal = new AbortController().signal;
let mockFileService: FileDiscoveryService;
const mockPrimaryDir = '/home/user/project';
const mockSecondaryDir = '/home/user/other-project';
beforeEach(() => { beforeEach(async () => {
vi.resetAllMocks(); tempRootDir = await fs.mkdtemp(path.join(os.tmpdir(), 'ls-tool-root-'));
tempSecondaryDir = await fs.mkdtemp(
path.join(os.tmpdir(), 'ls-tool-secondary-'),
);
// Mock WorkspaceContext const mockWorkspaceContext = createMockWorkspaceContext(tempRootDir, [
mockWorkspaceContext = { tempSecondaryDir,
getDirectories: vi ]);
.fn()
.mockReturnValue([mockPrimaryDir, mockSecondaryDir]),
isPathWithinWorkspace: vi
.fn()
.mockImplementation(
(path) =>
path.startsWith(mockPrimaryDir) ||
path.startsWith(mockSecondaryDir),
),
addDirectory: vi.fn(),
} as unknown as WorkspaceContext;
// Mock FileService
mockFileService = {
shouldGitIgnoreFile: vi.fn().mockReturnValue(false),
shouldGeminiIgnoreFile: vi.fn().mockReturnValue(false),
} as unknown as FileDiscoveryService;
// Mock Config
mockConfig = { mockConfig = {
getTargetDir: vi.fn().mockReturnValue(mockPrimaryDir), getTargetDir: () => tempRootDir,
getWorkspaceContext: vi.fn().mockReturnValue(mockWorkspaceContext), getWorkspaceContext: () => mockWorkspaceContext,
getFileService: vi.fn().mockReturnValue(mockFileService), getFileService: () => new FileDiscoveryService(tempRootDir),
getFileFilteringOptions: vi.fn().mockReturnValue({ getFileFilteringOptions: () => ({
respectGitIgnore: true, respectGitIgnore: true,
respectGeminiIgnore: true, respectGeminiIgnore: true,
}), }),
@@ -71,221 +44,132 @@ describe('LSTool', () => {
lsTool = new LSTool(mockConfig); lsTool = new LSTool(mockConfig);
}); });
afterEach(async () => {
await fs.rm(tempRootDir, { recursive: true, force: true });
await fs.rm(tempSecondaryDir, { recursive: true, force: true });
});
describe('parameter validation', () => { describe('parameter validation', () => {
it('should accept valid absolute paths within workspace', () => { it('should accept valid absolute paths within workspace', async () => {
const params = { const testPath = path.join(tempRootDir, 'src');
path: '/home/user/project/src', await fs.mkdir(testPath);
};
vi.mocked(fs.statSync).mockReturnValue({ const invocation = lsTool.build({ path: testPath });
isDirectory: () => true,
} as fs.Stats);
const invocation = lsTool.build(params);
expect(invocation).toBeDefined(); expect(invocation).toBeDefined();
}); });
it('should reject relative paths', () => { it('should reject relative paths', () => {
const params = { expect(() => lsTool.build({ path: './src' })).toThrow(
path: './src',
};
expect(() => lsTool.build(params)).toThrow(
'Path must be absolute: ./src', 'Path must be absolute: ./src',
); );
}); });
it('should reject paths outside workspace with clear error message', () => { it('should reject paths outside workspace with clear error message', () => {
const params = { expect(() => lsTool.build({ path: '/etc/passwd' })).toThrow(
path: '/etc/passwd', `Path must be within one of the workspace directories: ${tempRootDir}, ${tempSecondaryDir}`,
};
expect(() => lsTool.build(params)).toThrow(
'Path must be within one of the workspace directories: /home/user/project, /home/user/other-project',
); );
}); });
it('should accept paths in secondary workspace directory', () => { it('should accept paths in secondary workspace directory', async () => {
const params = { const testPath = path.join(tempSecondaryDir, 'lib');
path: '/home/user/other-project/lib', await fs.mkdir(testPath);
};
vi.mocked(fs.statSync).mockReturnValue({ const invocation = lsTool.build({ path: testPath });
isDirectory: () => true,
} as fs.Stats);
const invocation = lsTool.build(params);
expect(invocation).toBeDefined(); expect(invocation).toBeDefined();
}); });
}); });
describe('execute', () => { describe('execute', () => {
it('should list files in a directory', async () => { it('should list files in a directory', async () => {
const testPath = '/home/user/project/src'; await fs.writeFile(path.join(tempRootDir, 'file1.txt'), 'content1');
const mockFiles = ['file1.ts', 'file2.ts', 'subdir']; await fs.mkdir(path.join(tempRootDir, 'subdir'));
const mockStats = { await fs.writeFile(
isDirectory: vi.fn(), path.join(tempSecondaryDir, 'secondary-file.txt'),
mtime: new Date(), 'secondary',
size: 1024, );
};
vi.mocked(fs.statSync).mockImplementation((path: any) => { const invocation = lsTool.build({ path: tempRootDir });
const pathStr = path.toString(); const result = await invocation.execute(abortSignal);
if (pathStr === testPath) {
return { isDirectory: () => true } as fs.Stats;
}
// For individual files
if (pathStr.toString().endsWith('subdir')) {
return { ...mockStats, isDirectory: () => true, size: 0 } as fs.Stats;
}
return { ...mockStats, isDirectory: () => false } as fs.Stats;
});
vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any);
const invocation = lsTool.build({ path: testPath });
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toContain('[DIR] subdir'); expect(result.llmContent).toContain('[DIR] subdir');
expect(result.llmContent).toContain('file1.ts'); expect(result.llmContent).toContain('file1.txt');
expect(result.llmContent).toContain('file2.ts');
expect(result.returnDisplay).toBe('Listed 3 item(s).');
});
it('should list files from secondary workspace directory', async () => {
const testPath = '/home/user/other-project/lib';
const mockFiles = ['module1.js', 'module2.js'];
vi.mocked(fs.statSync).mockImplementation((path: any) => {
if (path.toString() === testPath) {
return { isDirectory: () => true } as fs.Stats;
}
return {
isDirectory: () => false,
mtime: new Date(),
size: 2048,
} as fs.Stats;
});
vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any);
const invocation = lsTool.build({ path: testPath });
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toContain('module1.js');
expect(result.llmContent).toContain('module2.js');
expect(result.returnDisplay).toBe('Listed 2 item(s).'); expect(result.returnDisplay).toBe('Listed 2 item(s).');
}); });
it('should handle empty directories', async () => { it('should list files from secondary workspace directory', async () => {
const testPath = '/home/user/project/empty'; await fs.writeFile(path.join(tempRootDir, 'file1.txt'), 'content1');
await fs.mkdir(path.join(tempRootDir, 'subdir'));
vi.mocked(fs.statSync).mockReturnValue({ await fs.writeFile(
isDirectory: () => true, path.join(tempSecondaryDir, 'secondary-file.txt'),
} as fs.Stats); 'secondary',
vi.mocked(fs.readdirSync).mockReturnValue([]);
const invocation = lsTool.build({ path: testPath });
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toBe(
'Directory /home/user/project/empty is empty.',
); );
const invocation = lsTool.build({ path: tempSecondaryDir });
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('secondary-file.txt');
expect(result.returnDisplay).toBe('Listed 1 item(s).');
});
it('should handle empty directories', async () => {
const emptyDir = path.join(tempRootDir, 'empty');
await fs.mkdir(emptyDir);
const invocation = lsTool.build({ path: emptyDir });
const result = await invocation.execute(abortSignal);
expect(result.llmContent).toBe(`Directory ${emptyDir} is empty.`);
expect(result.returnDisplay).toBe('Directory is empty.'); expect(result.returnDisplay).toBe('Directory is empty.');
}); });
it('should respect ignore patterns', async () => { it('should respect ignore patterns', async () => {
const testPath = '/home/user/project/src'; await fs.writeFile(path.join(tempRootDir, 'file1.txt'), 'content1');
const mockFiles = ['test.js', 'test.spec.js', 'index.js']; await fs.writeFile(path.join(tempRootDir, 'file2.log'), 'content1');
vi.mocked(fs.statSync).mockImplementation((path: any) => {
const pathStr = path.toString();
if (pathStr === testPath) {
return { isDirectory: () => true } as fs.Stats;
}
return {
isDirectory: () => false,
mtime: new Date(),
size: 1024,
} as fs.Stats;
});
vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any);
const invocation = lsTool.build({ const invocation = lsTool.build({
path: testPath, path: tempRootDir,
ignore: ['*.spec.js'], ignore: ['*.log'],
}); });
const result = await invocation.execute(new AbortController().signal); const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('test.js'); expect(result.llmContent).toContain('file1.txt');
expect(result.llmContent).toContain('index.js'); expect(result.llmContent).not.toContain('file2.log');
expect(result.llmContent).not.toContain('test.spec.js'); expect(result.returnDisplay).toBe('Listed 1 item(s).');
expect(result.returnDisplay).toBe('Listed 2 item(s).');
}); });
it('should respect gitignore patterns', async () => { it('should respect gitignore patterns', async () => {
const testPath = '/home/user/project/src'; await fs.writeFile(path.join(tempRootDir, 'file1.txt'), 'content1');
const mockFiles = ['file1.js', 'file2.js', 'ignored.js']; await fs.writeFile(path.join(tempRootDir, 'file2.log'), 'content1');
await fs.writeFile(path.join(tempRootDir, '.git'), '');
await fs.writeFile(path.join(tempRootDir, '.gitignore'), '*.log');
const invocation = lsTool.build({ path: tempRootDir });
const result = await invocation.execute(abortSignal);
vi.mocked(fs.statSync).mockImplementation((path: any) => { expect(result.llmContent).toContain('file1.txt');
const pathStr = path.toString(); expect(result.llmContent).not.toContain('file2.log');
if (pathStr === testPath) { // .git is always ignored by default.
return { isDirectory: () => true } as fs.Stats; expect(result.returnDisplay).toBe('Listed 2 item(s). (2 git-ignored)');
}
return {
isDirectory: () => false,
mtime: new Date(),
size: 1024,
} as fs.Stats;
});
vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any);
(mockFileService.shouldGitIgnoreFile as any).mockImplementation(
(path: string) => path.includes('ignored.js'),
);
const invocation = lsTool.build({ path: testPath });
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toContain('file1.js');
expect(result.llmContent).toContain('file2.js');
expect(result.llmContent).not.toContain('ignored.js');
expect(result.returnDisplay).toBe('Listed 2 item(s). (1 git-ignored)');
}); });
it('should respect geminiignore patterns', async () => { it('should respect geminiignore patterns', async () => {
const testPath = '/home/user/project/src'; await fs.writeFile(path.join(tempRootDir, 'file1.txt'), 'content1');
const mockFiles = ['file1.js', 'file2.js', 'private.js']; await fs.writeFile(path.join(tempRootDir, 'file2.log'), 'content1');
await fs.writeFile(path.join(tempRootDir, '.geminiignore'), '*.log');
const invocation = lsTool.build({ path: tempRootDir });
const result = await invocation.execute(abortSignal);
vi.mocked(fs.statSync).mockImplementation((path: any) => { expect(result.llmContent).toContain('file1.txt');
const pathStr = path.toString(); expect(result.llmContent).not.toContain('file2.log');
if (pathStr === testPath) {
return { isDirectory: () => true } as fs.Stats;
}
return {
isDirectory: () => false,
mtime: new Date(),
size: 1024,
} as fs.Stats;
});
vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any);
(mockFileService.shouldGeminiIgnoreFile as any).mockImplementation(
(path: string) => path.includes('private.js'),
);
const invocation = lsTool.build({ path: testPath });
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toContain('file1.js');
expect(result.llmContent).toContain('file2.js');
expect(result.llmContent).not.toContain('private.js');
expect(result.returnDisplay).toBe('Listed 2 item(s). (1 gemini-ignored)'); expect(result.returnDisplay).toBe('Listed 2 item(s). (1 gemini-ignored)');
}); });
it('should handle non-directory paths', async () => { it('should handle non-directory paths', async () => {
const testPath = '/home/user/project/file.txt'; const testPath = path.join(tempRootDir, 'file1.txt');
await fs.writeFile(testPath, 'content1');
vi.mocked(fs.statSync).mockReturnValue({
isDirectory: () => false,
} as fs.Stats);
const invocation = lsTool.build({ path: testPath }); const invocation = lsTool.build({ path: testPath });
const result = await invocation.execute(new AbortController().signal); const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Path is not a directory'); expect(result.llmContent).toContain('Path is not a directory');
expect(result.returnDisplay).toBe('Error: Path is not a directory.'); expect(result.returnDisplay).toBe('Error: Path is not a directory.');
@@ -293,14 +177,9 @@ describe('LSTool', () => {
}); });
it('should handle non-existent paths', async () => { it('should handle non-existent paths', async () => {
const testPath = '/home/user/project/does-not-exist'; const testPath = path.join(tempRootDir, 'does-not-exist');
vi.mocked(fs.statSync).mockImplementation(() => {
throw new Error('ENOENT: no such file or directory');
});
const invocation = lsTool.build({ path: testPath }); const invocation = lsTool.build({ path: testPath });
const result = await invocation.execute(new AbortController().signal); const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Error listing directory'); expect(result.llmContent).toContain('Error listing directory');
expect(result.returnDisplay).toBe('Error: Failed to list directory.'); expect(result.returnDisplay).toBe('Error: Failed to list directory.');
@@ -308,54 +187,38 @@ describe('LSTool', () => {
}); });
it('should sort directories first, then files alphabetically', async () => { it('should sort directories first, then files alphabetically', async () => {
const testPath = '/home/user/project/src'; await fs.writeFile(path.join(tempRootDir, 'a-file.txt'), 'content1');
const mockFiles = ['z-file.ts', 'a-dir', 'b-file.ts', 'c-dir']; await fs.writeFile(path.join(tempRootDir, 'b-file.txt'), 'content1');
await fs.mkdir(path.join(tempRootDir, 'x-dir'));
await fs.mkdir(path.join(tempRootDir, 'y-dir'));
vi.mocked(fs.statSync).mockImplementation((path: any) => { const invocation = lsTool.build({ path: tempRootDir });
if (path.toString() === testPath) { const result = await invocation.execute(abortSignal);
return { isDirectory: () => true } as fs.Stats;
}
if (path.toString().endsWith('-dir')) {
return {
isDirectory: () => true,
mtime: new Date(),
size: 0,
} as fs.Stats;
}
return {
isDirectory: () => false,
mtime: new Date(),
size: 1024,
} as fs.Stats;
});
vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any);
const invocation = lsTool.build({ path: testPath });
const result = await invocation.execute(new AbortController().signal);
const lines = ( const lines = (
typeof result.llmContent === 'string' ? result.llmContent : '' typeof result.llmContent === 'string' ? result.llmContent : ''
).split('\n'); )
const entries = lines.slice(1).filter((line: string) => line.trim()); // Skip header .split('\n')
expect(entries[0]).toBe('[DIR] a-dir'); .filter(Boolean);
expect(entries[1]).toBe('[DIR] c-dir'); const entries = lines.slice(1); // Skip header
expect(entries[2]).toBe('b-file.ts');
expect(entries[3]).toBe('z-file.ts'); expect(entries[0]).toBe('[DIR] x-dir');
expect(entries[1]).toBe('[DIR] y-dir');
expect(entries[2]).toBe('a-file.txt');
expect(entries[3]).toBe('b-file.txt');
}); });
it('should handle permission errors gracefully', async () => { it('should handle permission errors gracefully', async () => {
const testPath = '/home/user/project/restricted'; const restrictedDir = path.join(tempRootDir, 'restricted');
await fs.mkdir(restrictedDir);
vi.mocked(fs.statSync).mockReturnValue({ // To simulate a permission error in a cross-platform way,
isDirectory: () => true, // we mock fs.readdir to throw an error.
} as fs.Stats); const error = new Error('EACCES: permission denied');
vi.mocked(fs.readdirSync).mockImplementation(() => { vi.spyOn(fs, 'readdir').mockRejectedValueOnce(error);
throw new Error('EACCES: permission denied');
});
const invocation = lsTool.build({ path: testPath }); const invocation = lsTool.build({ path: restrictedDir });
const result = await invocation.execute(new AbortController().signal); const result = await invocation.execute(abortSignal);
expect(result.llmContent).toContain('Error listing directory'); expect(result.llmContent).toContain('Error listing directory');
expect(result.llmContent).toContain('permission denied'); expect(result.llmContent).toContain('permission denied');
@@ -363,62 +226,57 @@ describe('LSTool', () => {
expect(result.error?.type).toBe(ToolErrorType.LS_EXECUTION_ERROR); expect(result.error?.type).toBe(ToolErrorType.LS_EXECUTION_ERROR);
}); });
it('should throw for invalid params at build time', async () => { it('should throw for invalid params at build time', () => {
expect(() => lsTool.build({ path: '../outside' })).toThrow( expect(() => lsTool.build({ path: '../outside' })).toThrow(
'Path must be absolute: ../outside', 'Path must be absolute: ../outside',
); );
}); });
it('should handle errors accessing individual files during listing', async () => { it('should handle errors accessing individual files during listing', async () => {
const testPath = '/home/user/project/src'; await fs.writeFile(path.join(tempRootDir, 'file1.txt'), 'content1');
const mockFiles = ['accessible.ts', 'inaccessible.ts']; const problematicFile = path.join(tempRootDir, 'problematic.txt');
await fs.writeFile(problematicFile, 'content2');
vi.mocked(fs.statSync).mockImplementation((path: any) => { // To simulate an error on a single file in a cross-platform way,
if (path.toString() === testPath) { // we mock fs.stat to throw for a specific file. This avoids
return { isDirectory: () => true } as fs.Stats; // platform-specific behavior with things like dangling symlinks.
const originalStat = fs.stat;
const statSpy = vi.spyOn(fs, 'stat').mockImplementation(async (p) => {
if (p.toString() === problematicFile) {
throw new Error('Simulated stat error');
} }
if (path.toString().endsWith('inaccessible.ts')) { return originalStat(p);
throw new Error('EACCES: permission denied');
}
return {
isDirectory: () => false,
mtime: new Date(),
size: 1024,
} as fs.Stats;
}); });
vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any);
// Spy on console.error to verify it's called // Spy on console.error to verify it's called
const consoleErrorSpy = vi const consoleErrorSpy = vi
.spyOn(console, 'error') .spyOn(console, 'error')
.mockImplementation(() => {}); .mockImplementation(() => {});
const invocation = lsTool.build({ path: testPath }); const invocation = lsTool.build({ path: tempRootDir });
const result = await invocation.execute(new AbortController().signal); const result = await invocation.execute(abortSignal);
// Should still list the accessible file // Should still list the other files
expect(result.llmContent).toContain('accessible.ts'); expect(result.llmContent).toContain('file1.txt');
expect(result.llmContent).not.toContain('inaccessible.ts'); expect(result.llmContent).not.toContain('problematic.txt');
expect(result.returnDisplay).toBe('Listed 1 item(s).'); expect(result.returnDisplay).toBe('Listed 1 item(s).');
// Verify error was logged // Verify error was logged
expect(consoleErrorSpy).toHaveBeenCalledWith( expect(consoleErrorSpy).toHaveBeenCalledWith(
expect.stringContaining('Error accessing'), expect.stringMatching(/Error accessing.*problematic\.txt/s),
); );
statSpy.mockRestore();
consoleErrorSpy.mockRestore(); consoleErrorSpy.mockRestore();
}); });
}); });
describe('getDescription', () => { describe('getDescription', () => {
it('should return shortened relative path', () => { it('should return shortened relative path', () => {
const deeplyNestedDir = path.join(tempRootDir, 'deeply', 'nested');
const params = { const params = {
path: `${mockPrimaryDir}/deeply/nested/directory`, path: path.join(deeplyNestedDir, 'directory'),
}; };
vi.mocked(fs.statSync).mockReturnValue({
isDirectory: () => true,
} as fs.Stats);
const invocation = lsTool.build(params); const invocation = lsTool.build(params);
const description = invocation.getDescription(); const description = invocation.getDescription();
expect(description).toBe(path.join('deeply', 'nested', 'directory')); expect(description).toBe(path.join('deeply', 'nested', 'directory'));
@@ -426,31 +284,27 @@ describe('LSTool', () => {
it('should handle paths in secondary workspace', () => { it('should handle paths in secondary workspace', () => {
const params = { const params = {
path: `${mockSecondaryDir}/lib`, path: path.join(tempSecondaryDir, 'lib'),
}; };
vi.mocked(fs.statSync).mockReturnValue({
isDirectory: () => true,
} as fs.Stats);
const invocation = lsTool.build(params); const invocation = lsTool.build(params);
const description = invocation.getDescription(); const description = invocation.getDescription();
expect(description).toBe(path.join('..', 'other-project', 'lib')); const expected = path.relative(tempRootDir, params.path);
expect(description).toBe(expected);
}); });
}); });
describe('workspace boundary validation', () => { describe('workspace boundary validation', () => {
it('should accept paths in primary workspace directory', () => { it('should accept paths in primary workspace directory', async () => {
const params = { path: `${mockPrimaryDir}/src` }; const testPath = path.join(tempRootDir, 'src');
vi.mocked(fs.statSync).mockReturnValue({ await fs.mkdir(testPath);
isDirectory: () => true, const params = { path: testPath };
} as fs.Stats);
expect(lsTool.build(params)).toBeDefined(); expect(lsTool.build(params)).toBeDefined();
}); });
it('should accept paths in secondary workspace directory', () => { it('should accept paths in secondary workspace directory', async () => {
const params = { path: `${mockSecondaryDir}/lib` }; const testPath = path.join(tempSecondaryDir, 'lib');
vi.mocked(fs.statSync).mockReturnValue({ await fs.mkdir(testPath);
isDirectory: () => true, const params = { path: testPath };
} as fs.Stats);
expect(lsTool.build(params)).toBeDefined(); expect(lsTool.build(params)).toBeDefined();
}); });
@@ -462,28 +316,16 @@ describe('LSTool', () => {
}); });
it('should list files from secondary workspace directory', async () => { it('should list files from secondary workspace directory', async () => {
const testPath = `${mockSecondaryDir}/tests`; await fs.writeFile(
const mockFiles = ['test1.spec.ts', 'test2.spec.ts']; path.join(tempSecondaryDir, 'secondary-file.txt'),
'secondary',
);
vi.mocked(fs.statSync).mockImplementation((path: any) => { const invocation = lsTool.build({ path: tempSecondaryDir });
if (path.toString() === testPath) { const result = await invocation.execute(abortSignal);
return { isDirectory: () => true } as fs.Stats;
}
return {
isDirectory: () => false,
mtime: new Date(),
size: 512,
} as fs.Stats;
});
vi.mocked(fs.readdirSync).mockReturnValue(mockFiles as any); expect(result.llmContent).toContain('secondary-file.txt');
expect(result.returnDisplay).toBe('Listed 1 item(s).');
const invocation = lsTool.build({ path: testPath });
const result = await invocation.execute(new AbortController().signal);
expect(result.llmContent).toContain('test1.spec.ts');
expect(result.llmContent).toContain('test2.spec.ts');
expect(result.returnDisplay).toBe('Listed 2 item(s).');
}); });
}); });
}); });
+28 -49
View File
@@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0 * SPDX-License-Identifier: Apache-2.0
*/ */
import fs from 'node:fs'; import fs from 'node:fs/promises';
import path from 'node:path'; import path from 'node:path';
import type { ToolInvocation, ToolResult } from './tools.js'; import type { ToolInvocation, ToolResult } from './tools.js';
import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js'; import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js';
@@ -133,7 +133,7 @@ class LSToolInvocation extends BaseToolInvocation<LSToolParams, ToolResult> {
*/ */
async execute(_signal: AbortSignal): Promise<ToolResult> { async execute(_signal: AbortSignal): Promise<ToolResult> {
try { try {
const stats = fs.statSync(this.params.path); const stats = await fs.stat(this.params.path);
if (!stats) { if (!stats) {
// fs.statSync throws on non-existence, so this check might be redundant // fs.statSync throws on non-existence, so this check might be redundant
// but keeping for clarity. Error message adjusted. // but keeping for clarity. Error message adjusted.
@@ -151,28 +151,7 @@ class LSToolInvocation extends BaseToolInvocation<LSToolParams, ToolResult> {
); );
} }
const files = fs.readdirSync(this.params.path); const files = await fs.readdir(this.params.path);
const defaultFileIgnores =
this.config.getFileFilteringOptions() ?? DEFAULT_FILE_FILTERING_OPTIONS;
const fileFilteringOptions = {
respectGitIgnore:
this.params.file_filtering_options?.respect_git_ignore ??
defaultFileIgnores.respectGitIgnore,
respectGeminiIgnore:
this.params.file_filtering_options?.respect_gemini_ignore ??
defaultFileIgnores.respectGeminiIgnore,
};
// Get centralized file discovery service
const fileDiscovery = this.config.getFileService();
const entries: FileEntry[] = [];
let gitIgnoredCount = 0;
let geminiIgnoredCount = 0;
if (files.length === 0) { if (files.length === 0) {
// Changed error message to be more neutral for LLM // Changed error message to be more neutral for LLM
return { return {
@@ -181,38 +160,39 @@ class LSToolInvocation extends BaseToolInvocation<LSToolParams, ToolResult> {
}; };
} }
for (const file of files) { const relativePaths = files.map((file) =>
if (this.shouldIgnore(file, this.params.ignore)) { path.relative(
continue;
}
const fullPath = path.join(this.params.path, file);
const relativePath = path.relative(
this.config.getTargetDir(), this.config.getTargetDir(),
fullPath, path.join(this.params.path, file),
); ),
);
// Check if this file should be ignored based on git or gemini ignore rules const fileDiscovery = this.config.getFileService();
if ( const { filteredPaths, gitIgnoredCount, geminiIgnoredCount } =
fileFilteringOptions.respectGitIgnore && fileDiscovery.filterFilesWithReport(relativePaths, {
fileDiscovery.shouldGitIgnoreFile(relativePath) respectGitIgnore:
) { this.params.file_filtering_options?.respect_git_ignore ??
gitIgnoredCount++; this.config.getFileFilteringOptions().respectGitIgnore ??
continue; DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore,
} respectGeminiIgnore:
if ( this.params.file_filtering_options?.respect_gemini_ignore ??
fileFilteringOptions.respectGeminiIgnore && this.config.getFileFilteringOptions().respectGeminiIgnore ??
fileDiscovery.shouldGeminiIgnoreFile(relativePath) DEFAULT_FILE_FILTERING_OPTIONS.respectGeminiIgnore,
) { });
geminiIgnoredCount++;
const entries = [];
for (const relativePath of filteredPaths) {
const fullPath = path.resolve(this.config.getTargetDir(), relativePath);
if (this.shouldIgnore(path.basename(fullPath), this.params.ignore)) {
continue; continue;
} }
try { try {
const stats = fs.statSync(fullPath); const stats = await fs.stat(fullPath);
const isDir = stats.isDirectory(); const isDir = stats.isDirectory();
entries.push({ entries.push({
name: file, name: path.basename(fullPath),
path: fullPath, path: fullPath,
isDirectory: isDir, isDirectory: isDir,
size: isDir ? 0 : stats.size, size: isDir ? 0 : stats.size,
@@ -244,7 +224,6 @@ class LSToolInvocation extends BaseToolInvocation<LSToolParams, ToolResult> {
if (geminiIgnoredCount > 0) { if (geminiIgnoredCount > 0) {
ignoredMessages.push(`${geminiIgnoredCount} gemini-ignored`); ignoredMessages.push(`${geminiIgnoredCount} gemini-ignored`);
} }
if (ignoredMessages.length > 0) { if (ignoredMessages.length > 0) {
resultMessage += `\n\n(${ignoredMessages.join(', ')})`; resultMessage += `\n\n(${ignoredMessages.join(', ')})`;
} }
+26 -72
View File
@@ -18,8 +18,10 @@ import {
getSpecificMimeType, getSpecificMimeType,
} from '../utils/fileUtils.js'; } from '../utils/fileUtils.js';
import type { PartListUnion } from '@google/genai'; import type { PartListUnion } from '@google/genai';
import type { Config } from '../config/config.js'; import {
import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/config.js'; type Config,
DEFAULT_FILE_FILTERING_OPTIONS,
} from '../config/config.js';
import { FileOperation } from '../telemetry/metrics.js'; import { FileOperation } from '../telemetry/metrics.js';
import { getProgrammingLanguage } from '../telemetry/telemetry-utils.js'; import { getProgrammingLanguage } from '../telemetry/telemetry-utils.js';
import { logFileOperation } from '../telemetry/loggers.js'; import { logFileOperation } from '../telemetry/loggers.js';
@@ -173,20 +175,6 @@ ${finalExclusionPatternsForDescription
useDefaultExcludes = true, useDefaultExcludes = true,
} = this.params; } = this.params;
const defaultFileIgnores =
this.config.getFileFilteringOptions() ?? DEFAULT_FILE_FILTERING_OPTIONS;
const fileFilteringOptions = {
respectGitIgnore:
this.params.file_filtering_options?.respect_git_ignore ??
defaultFileIgnores.respectGitIgnore, // Use the property from the returned object
respectGeminiIgnore:
this.params.file_filtering_options?.respect_gemini_ignore ??
defaultFileIgnores.respectGeminiIgnore, // Use the property from the returned object
};
// Get centralized file discovery service
const fileDiscovery = this.config.getFileService();
const filesToConsider = new Set<string>(); const filesToConsider = new Set<string>();
const skippedFiles: Array<{ path: string; reason: string }> = []; const skippedFiles: Array<{ path: string; reason: string }> = [];
const processedFilesRelativePaths: string[] = []; const processedFilesRelativePaths: string[] = [];
@@ -227,71 +215,37 @@ ${finalExclusionPatternsForDescription
allEntries.add(entry); allEntries.add(entry);
} }
} }
const entries = Array.from(allEntries); const relativeEntries = Array.from(allEntries).map((p) =>
path.relative(this.config.getTargetDir(), p),
);
const gitFilteredEntries = fileFilteringOptions.respectGitIgnore const fileDiscovery = this.config.getFileService();
? fileDiscovery const { filteredPaths, gitIgnoredCount, geminiIgnoredCount } =
.filterFiles( fileDiscovery.filterFilesWithReport(relativeEntries, {
entries.map((p) => path.relative(this.config.getTargetDir(), p)), respectGitIgnore:
{ this.params.file_filtering_options?.respect_git_ignore ??
respectGitIgnore: true, this.config.getFileFilteringOptions().respectGitIgnore ??
respectGeminiIgnore: false, DEFAULT_FILE_FILTERING_OPTIONS.respectGitIgnore,
}, respectGeminiIgnore:
) this.params.file_filtering_options?.respect_gemini_ignore ??
.map((p) => path.resolve(this.config.getTargetDir(), p)) this.config.getFileFilteringOptions().respectGeminiIgnore ??
: entries; DEFAULT_FILE_FILTERING_OPTIONS.respectGeminiIgnore,
});
// Apply gemini ignore filtering if enabled for (const relativePath of filteredPaths) {
const finalFilteredEntries = fileFilteringOptions.respectGeminiIgnore
? fileDiscovery
.filterFiles(
gitFilteredEntries.map((p) =>
path.relative(this.config.getTargetDir(), p),
),
{
respectGitIgnore: false,
respectGeminiIgnore: true,
},
)
.map((p) => path.resolve(this.config.getTargetDir(), p))
: gitFilteredEntries;
let gitIgnoredCount = 0;
let geminiIgnoredCount = 0;
for (const absoluteFilePath of entries) {
// Security check: ensure the glob library didn't return something outside the workspace. // Security check: ensure the glob library didn't return something outside the workspace.
const fullPath = path.resolve(this.config.getTargetDir(), relativePath);
if ( if (
!this.config !this.config.getWorkspaceContext().isPathWithinWorkspace(fullPath)
.getWorkspaceContext()
.isPathWithinWorkspace(absoluteFilePath)
) { ) {
skippedFiles.push({ skippedFiles.push({
path: absoluteFilePath, path: fullPath,
reason: `Security: Glob library returned path outside workspace. Path: ${absoluteFilePath}`, reason: `Security: Glob library returned path outside workspace. Path: ${fullPath}`,
}); });
continue; continue;
} }
filesToConsider.add(fullPath);
// Check if this file was filtered out by git ignore
if (
fileFilteringOptions.respectGitIgnore &&
!gitFilteredEntries.includes(absoluteFilePath)
) {
gitIgnoredCount++;
continue;
}
// Check if this file was filtered out by gemini ignore
if (
fileFilteringOptions.respectGeminiIgnore &&
!finalFilteredEntries.includes(absoluteFilePath)
) {
geminiIgnoredCount++;
continue;
}
filesToConsider.add(absoluteFilePath);
} }
// Add info about git-ignored files if any were filtered // Add info about git-ignored files if any were filtered