Add support for an additional exclusion file besides .gitignore and .geminiignore (#16487)

Co-authored-by: Adam Weidman <adamfweidman@google.com>
This commit is contained in:
Alisa
2026-01-27 17:19:13 -08:00
committed by GitHub
parent 18efe82ddc
commit adc8e11bb1
40 changed files with 1394 additions and 612 deletions
+6 -3
View File
@@ -18,7 +18,10 @@ import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.j
import { ToolErrorType } from './tool-error.js';
import * as glob from 'glob';
import { createMockMessageBus } from '../test-utils/mock-message-bus.js';
import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/constants.js';
import {
DEFAULT_FILE_FILTERING_OPTIONS,
GEMINI_IGNORE_FILE_NAME,
} from '../config/constants.js';
vi.mock('glob', { spy: true });
@@ -385,7 +388,7 @@ describe('GlobTool', () => {
it('should respect .geminiignore files by default', async () => {
await fs.writeFile(
path.join(tempRootDir, '.geminiignore'),
path.join(tempRootDir, GEMINI_IGNORE_FILE_NAME),
'gemini-ignored_test.txt',
);
await fs.writeFile(
@@ -423,7 +426,7 @@ describe('GlobTool', () => {
it('should not respect .geminiignore when respect_gemini_ignore is false', async () => {
await fs.writeFile(
path.join(tempRootDir, '.geminiignore'),
path.join(tempRootDir, GEMINI_IGNORE_FILE_NAME),
'gemini-ignored_test.txt',
);
await fs.writeFile(
+5 -1
View File
@@ -15,6 +15,7 @@ import { FileDiscoveryService } from '../services/fileDiscoveryService.js';
import { ToolErrorType } from './tool-error.js';
import { WorkspaceContext } from '../utils/workspaceContext.js';
import { createMockMessageBus } from '../test-utils/mock-message-bus.js';
import { GEMINI_IGNORE_FILE_NAME } from '../config/constants.js';
describe('LSTool', () => {
let lsTool: LSTool;
@@ -182,7 +183,10 @@ describe('LSTool', () => {
it('should respect geminiignore patterns', async () => {
await fs.writeFile(path.join(tempRootDir, 'file1.txt'), 'content1');
await fs.writeFile(path.join(tempRootDir, 'file2.log'), 'content1');
await fs.writeFile(path.join(tempRootDir, '.geminiignore'), '*.log');
await fs.writeFile(
path.join(tempRootDir, GEMINI_IGNORE_FILE_NAME),
'*.log',
);
const invocation = lsTool.build({ dir_path: tempRootDir });
const result = await invocation.execute(abortSignal);
+53 -1
View File
@@ -19,6 +19,7 @@ import { StandardFileSystemService } from '../services/fileSystemService.js';
import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js';
import { WorkspaceContext } from '../utils/workspaceContext.js';
import { createMockMessageBus } from '../test-utils/mock-message-bus.js';
import { GEMINI_IGNORE_FILE_NAME } from '../config/constants.js';
vi.mock('../telemetry/loggers.js', () => ({
logFileOperation: vi.fn(),
@@ -438,7 +439,7 @@ describe('ReadFileTool', () => {
describe('with .geminiignore', () => {
beforeEach(async () => {
await fsp.writeFile(
path.join(tempRootDir, '.geminiignore'),
path.join(tempRootDir, GEMINI_IGNORE_FILE_NAME),
['foo.*', 'ignored/'].join('\n'),
);
const mockConfigInstance = {
@@ -509,6 +510,57 @@ describe('ReadFileTool', () => {
const invocation = tool.build(params);
expect(typeof invocation).not.toBe('string');
});
it('should allow reading ignored files if respectGeminiIgnore is false', async () => {
const ignoredFilePath = path.join(tempRootDir, 'foo.bar');
await fsp.writeFile(ignoredFilePath, 'content', 'utf-8');
const configNoIgnore = {
getFileService: () => new FileDiscoveryService(tempRootDir),
getFileSystemService: () => new StandardFileSystemService(),
getTargetDir: () => tempRootDir,
getWorkspaceContext: () => new WorkspaceContext(tempRootDir),
getFileFilteringOptions: () => ({
respectGitIgnore: true,
respectGeminiIgnore: false,
}),
storage: {
getProjectTempDir: () => path.join(tempRootDir, '.temp'),
},
isInteractive: () => false,
isPathAllowed(this: Config, absolutePath: string): boolean {
const workspaceContext = this.getWorkspaceContext();
if (workspaceContext.isPathWithinWorkspace(absolutePath)) {
return true;
}
const projectTempDir = this.storage.getProjectTempDir();
return isSubpath(path.resolve(projectTempDir), absolutePath);
},
validatePathAccess(
this: Config,
absolutePath: string,
): string | null {
if (this.isPathAllowed(absolutePath)) {
return null;
}
const workspaceDirs = this.getWorkspaceContext().getDirectories();
const projectTempDir = this.storage.getProjectTempDir();
return `Path not in workspace: Attempted path "${absolutePath}" resolves outside the allowed workspace directories: ${workspaceDirs.join(', ')} or the project temp directory: ${projectTempDir}`;
},
} as unknown as Config;
const toolNoIgnore = new ReadFileTool(
configNoIgnore,
createMockMessageBus(),
);
const params: ReadFileToolParams = {
file_path: ignoredFilePath,
};
const invocation = toolNoIgnore.build(params);
expect(typeof invocation).not.toBe('string');
});
});
});
});
+12 -2
View File
@@ -22,6 +22,7 @@ import { getProgrammingLanguage } from '../telemetry/telemetry-utils.js';
import { logFileOperation } from '../telemetry/loggers.js';
import { FileOperationEvent } from '../telemetry/types.js';
import { READ_FILE_TOOL_NAME } from './tool-names.js';
import { FileDiscoveryService } from '../services/fileDiscoveryService.js';
/**
* Parameters for the ReadFile tool
@@ -159,6 +160,7 @@ export class ReadFileTool extends BaseDeclarativeTool<
ToolResult
> {
static readonly Name = READ_FILE_TOOL_NAME;
private readonly fileDiscoveryService: FileDiscoveryService;
constructor(
private config: Config,
@@ -193,6 +195,10 @@ export class ReadFileTool extends BaseDeclarativeTool<
true,
false,
);
this.fileDiscoveryService = new FileDiscoveryService(
config.getTargetDir(),
config.getFileFilteringOptions(),
);
}
protected override validateToolParamValues(
@@ -219,9 +225,13 @@ export class ReadFileTool extends BaseDeclarativeTool<
return 'Limit must be a positive number';
}
const fileService = this.config.getFileService();
const fileFilteringOptions = this.config.getFileFilteringOptions();
if (fileService.shouldIgnoreFile(resolvedPath, fileFilteringOptions)) {
if (
this.fileDiscoveryService.shouldIgnoreFile(
resolvedPath,
fileFilteringOptions,
)
) {
return `File path '${resolvedPath}' is ignored by configured ignore patterns.`;
}
@@ -23,6 +23,7 @@ import {
} from '../utils/ignorePatterns.js';
import * as glob from 'glob';
import { createMockMessageBus } from '../test-utils/mock-message-bus.js';
import { GEMINI_IGNORE_FILE_NAME } from '../config/constants.js';
vi.mock('glob', { spy: true });
@@ -70,7 +71,7 @@ describe('ReadManyFilesTool', () => {
tempDirOutsideRoot = fs.realpathSync(
fs.mkdtempSync(path.join(os.tmpdir(), 'read-many-files-external-')),
);
fs.writeFileSync(path.join(tempRootDir, '.geminiignore'), 'foo.*');
fs.writeFileSync(path.join(tempRootDir, GEMINI_IGNORE_FILE_NAME), 'foo.*');
const fileService = new FileDiscoveryService(tempRootDir);
const mockConfig = {
getFileService: () => fileService,
@@ -79,6 +80,7 @@ describe('ReadManyFilesTool', () => {
getFileFilteringOptions: () => ({
respectGitIgnore: true,
respectGeminiIgnore: true,
customIgnoreFilePaths: [],
}),
getTargetDir: () => tempRootDir,
getWorkspaceDirs: () => [tempRootDir],
@@ -516,6 +518,7 @@ describe('ReadManyFilesTool', () => {
getFileFilteringOptions: () => ({
respectGitIgnore: true,
respectGeminiIgnore: true,
customIgnoreFilePaths: [],
}),
getWorkspaceContext: () => new WorkspaceContext(tempDir1, [tempDir2]),
getTargetDir: () => tempDir1,
+89 -3
View File
@@ -21,6 +21,7 @@ import fs from 'node:fs/promises';
import os from 'node:os';
import type { Config } from '../config/config.js';
import { Storage } from '../config/storage.js';
import { GEMINI_IGNORE_FILE_NAME } from '../config/constants.js';
import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js';
import type { ChildProcess } from 'node:child_process';
import { spawn } from 'node:child_process';
@@ -247,7 +248,17 @@ describe('RipGrepTool', () => {
let ripgrepBinaryPath: string;
let grepTool: RipGrepTool;
const abortSignal = new AbortController().signal;
let mockConfig: Config;
let mockConfig = {
getTargetDir: () => tempRootDir,
getWorkspaceContext: () => createMockWorkspaceContext(tempRootDir),
getDebugMode: () => false,
getFileFilteringRespectGeminiIgnore: () => true,
getFileFilteringOptions: () => ({
respectGitIgnore: true,
respectGeminiIgnore: true,
}),
} as unknown as Config;
beforeEach(async () => {
downloadRipGrepMock.mockReset();
@@ -267,6 +278,10 @@ describe('RipGrepTool', () => {
getWorkspaceContext: () => createMockWorkspaceContext(tempRootDir),
getDebugMode: () => false,
getFileFilteringRespectGeminiIgnore: () => true,
getFileFilteringOptions: () => ({
respectGitIgnore: true,
respectGeminiIgnore: true,
}),
storage: {
getProjectTempDir: vi.fn().mockReturnValue('/tmp/project'),
},
@@ -668,6 +683,57 @@ describe('RipGrepTool', () => {
expect(result.returnDisplay).toContain('(limited)');
}, 10000);
it('should filter out files based on FileDiscoveryService even if ripgrep returns them', async () => {
// Create .geminiignore to ignore 'ignored.txt'
await fs.writeFile(
path.join(tempRootDir, GEMINI_IGNORE_FILE_NAME),
'ignored.txt',
);
// Re-initialize tool so FileDiscoveryService loads the new .geminiignore
const toolWithIgnore = new RipGrepTool(
mockConfig,
createMockMessageBus(),
);
// Mock ripgrep returning both an ignored file and an allowed file
mockSpawn.mockImplementationOnce(
createMockSpawn({
outputData:
JSON.stringify({
type: 'match',
data: {
path: { text: 'ignored.txt' },
line_number: 1,
lines: { text: 'should be ignored\n' },
},
}) +
'\n' +
JSON.stringify({
type: 'match',
data: {
path: { text: 'allowed.txt' },
line_number: 1,
lines: { text: 'should be kept\n' },
},
}) +
'\n',
exitCode: 0,
}),
);
const params: RipGrepToolParams = { pattern: 'should' };
const invocation = toolWithIgnore.build(params);
const result = await invocation.execute(abortSignal);
// Verify ignored file is filtered out
expect(result.llmContent).toContain('allowed.txt');
expect(result.llmContent).toContain('should be kept');
expect(result.llmContent).not.toContain('ignored.txt');
expect(result.llmContent).not.toContain('should be ignored');
expect(result.returnDisplay).toContain('Found 1 match');
});
it('should handle regex special characters correctly', async () => {
// Setup specific mock for this test - regex pattern 'foo.*bar' should match 'const foo = "bar";'
mockSpawn.mockImplementationOnce(
@@ -779,6 +845,10 @@ describe('RipGrepTool', () => {
createMockWorkspaceContext(tempRootDir, [secondDir]),
getDebugMode: () => false,
getFileFilteringRespectGeminiIgnore: () => true,
getFileFilteringOptions: () => ({
respectGitIgnore: true,
respectGeminiIgnore: true,
}),
storage: {
getProjectTempDir: vi.fn().mockReturnValue('/tmp/project'),
},
@@ -887,6 +957,10 @@ describe('RipGrepTool', () => {
createMockWorkspaceContext(tempRootDir, [secondDir]),
getDebugMode: () => false,
getFileFilteringRespectGeminiIgnore: () => true,
getFileFilteringOptions: () => ({
respectGitIgnore: true,
respectGeminiIgnore: true,
}),
storage: {
getProjectTempDir: vi.fn().mockReturnValue('/tmp/project'),
},
@@ -1404,13 +1478,17 @@ describe('RipGrepTool', () => {
});
it('should add .geminiignore when enabled and patterns exist', async () => {
const geminiIgnorePath = path.join(tempRootDir, '.geminiignore');
const geminiIgnorePath = path.join(tempRootDir, GEMINI_IGNORE_FILE_NAME);
await fs.writeFile(geminiIgnorePath, 'ignored.log');
const configWithGeminiIgnore = {
getTargetDir: () => tempRootDir,
getWorkspaceContext: () => createMockWorkspaceContext(tempRootDir),
getDebugMode: () => false,
getFileFilteringRespectGeminiIgnore: () => true,
getFileFilteringOptions: () => ({
respectGitIgnore: true,
respectGeminiIgnore: true,
}),
storage: {
getProjectTempDir: vi.fn().mockReturnValue('/tmp/project'),
},
@@ -1465,13 +1543,17 @@ describe('RipGrepTool', () => {
});
it('should skip .geminiignore when disabled', async () => {
const geminiIgnorePath = path.join(tempRootDir, '.geminiignore');
const geminiIgnorePath = path.join(tempRootDir, GEMINI_IGNORE_FILE_NAME);
await fs.writeFile(geminiIgnorePath, 'ignored.log');
const configWithoutGeminiIgnore = {
getTargetDir: () => tempRootDir,
getWorkspaceContext: () => createMockWorkspaceContext(tempRootDir),
getDebugMode: () => false,
getFileFilteringRespectGeminiIgnore: () => false,
getFileFilteringOptions: () => ({
respectGitIgnore: true,
respectGeminiIgnore: false,
}),
storage: {
getProjectTempDir: vi.fn().mockReturnValue('/tmp/project'),
},
@@ -1618,6 +1700,10 @@ describe('RipGrepTool', () => {
getWorkspaceContext: () =>
createMockWorkspaceContext(tempRootDir, ['/another/dir']),
getDebugMode: () => false,
getFileFilteringOptions: () => ({
respectGitIgnore: true,
respectGeminiIgnore: true,
}),
storage: {
getProjectTempDir: vi.fn().mockReturnValue('/tmp/project'),
},
+28 -11
View File
@@ -23,7 +23,7 @@ import {
FileExclusions,
COMMON_DIRECTORY_EXCLUDES,
} from '../utils/ignorePatterns.js';
import { GeminiIgnoreParser } from '../utils/geminiIgnoreParser.js';
import { FileDiscoveryService } from '../services/fileDiscoveryService.js';
import { execStreaming } from '../utils/shell-utils.js';
import {
DEFAULT_TOTAL_MAX_MATCHES,
@@ -148,7 +148,7 @@ class GrepToolInvocation extends BaseToolInvocation<
> {
constructor(
private readonly config: Config,
private readonly geminiIgnoreParser: GeminiIgnoreParser,
private readonly fileDiscoveryService: FileDiscoveryService,
params: RipGrepToolParams,
messageBus: MessageBus,
_toolName?: string,
@@ -243,6 +243,21 @@ class GrepToolInvocation extends BaseToolInvocation<
signal.removeEventListener('abort', onAbort);
}
if (!this.params.no_ignore) {
const uniqueFiles = Array.from(
new Set(allMatches.map((m) => m.filePath)),
);
const absoluteFilePaths = uniqueFiles.map((f) =>
path.resolve(searchDirAbs, f),
);
const allowedFiles =
this.fileDiscoveryService.filterFiles(absoluteFilePaths);
const allowedSet = new Set(allowedFiles);
allMatches = allMatches.filter((m) =>
allowedSet.has(path.resolve(searchDirAbs, m.filePath)),
);
}
const searchLocationDescription = `in path "${searchDirDisplay}"`;
if (allMatches.length === 0) {
const noMatchMsg = `No matches found for pattern "${this.params.pattern}" ${searchLocationDescription}${this.params.include ? ` (filter: "${this.params.include}")` : ''}.`;
@@ -361,12 +376,11 @@ class GrepToolInvocation extends BaseToolInvocation<
rgArgs.push('--glob', `!${exclude}`);
});
if (this.config.getFileFilteringRespectGeminiIgnore()) {
// Add .geminiignore support (ripgrep natively handles .gitignore)
const geminiIgnorePath = this.geminiIgnoreParser.getIgnoreFilePath();
if (geminiIgnorePath) {
rgArgs.push('--ignore-file', geminiIgnorePath);
}
// Add .geminiignore and custom ignore files support (if provided/mandated)
// (ripgrep natively handles .gitignore)
const geminiIgnorePaths = this.fileDiscoveryService.getIgnoreFilePaths();
for (const ignorePath of geminiIgnorePaths) {
rgArgs.push('--ignore-file', ignorePath);
}
}
@@ -472,7 +486,7 @@ export class RipGrepTool extends BaseDeclarativeTool<
ToolResult
> {
static readonly Name = GREP_TOOL_NAME;
private readonly geminiIgnoreParser: GeminiIgnoreParser;
private readonly fileDiscoveryService: FileDiscoveryService;
constructor(
private readonly config: Config,
@@ -538,7 +552,10 @@ export class RipGrepTool extends BaseDeclarativeTool<
true, // isOutputMarkdown
false, // canUpdateOutput
);
this.geminiIgnoreParser = new GeminiIgnoreParser(config.getTargetDir());
this.fileDiscoveryService = new FileDiscoveryService(
config.getTargetDir(),
config.getFileFilteringOptions(),
);
}
/**
@@ -591,7 +608,7 @@ export class RipGrepTool extends BaseDeclarativeTool<
): ToolInvocation<RipGrepToolParams, ToolResult> {
return new GrepToolInvocation(
this.config,
this.geminiIgnoreParser,
this.fileDiscoveryService,
params,
messageBus ?? this.messageBus,
_toolName,