From e3262f8766d73a281fbc913c7a7f6d876c7cb136 Mon Sep 17 00:00:00 2001 From: Eric Rahm Date: Sat, 1 Nov 2025 10:06:34 -0700 Subject: [PATCH] fix(core): combine .gitignore and .geminiignore logic for correct precedence (#11587) Co-authored-by: Jacob Richman --- .../src/services/fileDiscoveryService.test.ts | 83 +++++++++++++++++++ .../core/src/services/fileDiscoveryService.ts | 18 ++++ .../core/src/utils/gitIgnoreParser.test.ts | 38 +++++++++ packages/core/src/utils/gitIgnoreParser.ts | 34 +++++++- 4 files changed, 169 insertions(+), 4 deletions(-) diff --git a/packages/core/src/services/fileDiscoveryService.test.ts b/packages/core/src/services/fileDiscoveryService.test.ts index c09309b13b..173e114583 100644 --- a/packages/core/src/services/fileDiscoveryService.test.ts +++ b/packages/core/src/services/fileDiscoveryService.test.ts @@ -245,4 +245,87 @@ describe('FileDiscoveryService', () => { ]); }); }); + describe('precedence (.geminiignore over .gitignore)', () => { + beforeEach(async () => { + await fs.mkdir(path.join(projectRoot, '.git')); + }); + + it('should un-ignore a file in .geminiignore that is ignored in .gitignore', async () => { + await createTestFile('.gitignore', '*.txt'); + await createTestFile('.geminiignore', '!important.txt'); + + const service = new FileDiscoveryService(projectRoot); + const files = ['file.txt', 'important.txt'].map((f) => + path.join(projectRoot, f), + ); + + const filtered = service.filterFiles(files); + expect(filtered).toEqual([path.join(projectRoot, 'important.txt')]); + }); + + it('should un-ignore a directory in .geminiignore that is ignored in .gitignore', async () => { + await createTestFile('.gitignore', 'logs/'); + await createTestFile('.geminiignore', '!logs/'); + + const service = new FileDiscoveryService(projectRoot); + const files = ['logs/app.log', 'other/app.log'].map((f) => + path.join(projectRoot, f), + ); + + const filtered = service.filterFiles(files); + expect(filtered).toEqual(files); + }); + + it('should extend ignore rules in .geminiignore', async () => { + await createTestFile('.gitignore', '*.log'); + await createTestFile('.geminiignore', 'temp/'); + + const service = new FileDiscoveryService(projectRoot); + const files = ['app.log', 'temp/file.txt'].map((f) => + path.join(projectRoot, f), + ); + + const filtered = service.filterFiles(files); + expect(filtered).toEqual([]); + }); + + it('should use .gitignore rules if respectGeminiIgnore is false', async () => { + await createTestFile('.gitignore', '*.txt'); + await createTestFile('.geminiignore', '!important.txt'); + + const service = new FileDiscoveryService(projectRoot); + const files = ['file.txt', 'important.txt'].map((f) => + path.join(projectRoot, f), + ); + + const filtered = service.filterFiles(files, { + respectGitIgnore: true, + respectGeminiIgnore: false, + }); + + expect(filtered).toEqual([]); + }); + + it('should use .geminiignore rules if respectGitIgnore is false', async () => { + await createTestFile('.gitignore', '*.txt'); + await createTestFile('.geminiignore', '!important.txt\ntemp/'); + + const service = new FileDiscoveryService(projectRoot); + const files = ['file.txt', 'important.txt', 'temp/file.js'].map((f) => + path.join(projectRoot, f), + ); + + const filtered = service.filterFiles(files, { + respectGitIgnore: false, + respectGeminiIgnore: true, + }); + + // .gitignore is ignored, so *.txt is not applied. + // .geminiignore un-ignores important.txt (which wasn't ignored anyway) + // and ignores temp/ + expect(filtered).toEqual( + ['file.txt', 'important.txt'].map((f) => path.join(projectRoot, f)), + ); + }); + }); }); diff --git a/packages/core/src/services/fileDiscoveryService.ts b/packages/core/src/services/fileDiscoveryService.ts index 7b4d3398bd..4ad2eb7552 100644 --- a/packages/core/src/services/fileDiscoveryService.ts +++ b/packages/core/src/services/fileDiscoveryService.ts @@ -24,6 +24,7 @@ export interface FilterReport { export class FileDiscoveryService { private gitIgnoreFilter: GitIgnoreFilter | null = null; private geminiIgnoreFilter: GeminiIgnoreFilter | null = null; + private combinedIgnoreFilter: GitIgnoreFilter | null = null; private projectRoot: string; constructor(projectRoot: string) { @@ -32,6 +33,15 @@ export class FileDiscoveryService { this.gitIgnoreFilter = new GitIgnoreParser(this.projectRoot); } this.geminiIgnoreFilter = new GeminiIgnoreParser(this.projectRoot); + + if (this.gitIgnoreFilter) { + const geminiPatterns = this.geminiIgnoreFilter.getPatterns(); + // Create combined parser: .gitignore + .geminiignore + this.combinedIgnoreFilter = new GitIgnoreParser( + this.projectRoot, + geminiPatterns, + ); + } } /** @@ -40,6 +50,14 @@ export class FileDiscoveryService { filterFiles(filePaths: string[], options: FilterFilesOptions = {}): string[] { const { respectGitIgnore = true, respectGeminiIgnore = true } = options; return filePaths.filter((filePath) => { + if ( + respectGitIgnore && + respectGeminiIgnore && + this.combinedIgnoreFilter + ) { + return !this.combinedIgnoreFilter.isIgnored(filePath); + } + if (respectGitIgnore && this.gitIgnoreFilter?.isIgnored(filePath)) { return false; } diff --git a/packages/core/src/utils/gitIgnoreParser.test.ts b/packages/core/src/utils/gitIgnoreParser.test.ts index 580da544fe..2afeb823d2 100644 --- a/packages/core/src/utils/gitIgnoreParser.test.ts +++ b/packages/core/src/utils/gitIgnoreParser.test.ts @@ -270,4 +270,42 @@ src/*.tmp expect(parser.isIgnored('bar ')).toBe(false); }); }); + + describe('Extra Patterns', () => { + beforeEach(async () => { + await setupGitRepo(); + }); + + it('should apply extraPatterns with higher precedence than .gitignore', async () => { + await createTestFile('.gitignore', '*.txt'); + + const extraPatterns = ['!important.txt', 'temp/']; + parser = new GitIgnoreParser(projectRoot, extraPatterns); + + expect(parser.isIgnored('file.txt')).toBe(true); + expect(parser.isIgnored('important.txt')).toBe(false); // Un-ignored by extraPatterns + expect(parser.isIgnored('temp/file.js')).toBe(true); // Ignored by extraPatterns + }); + + it('should handle extraPatterns that unignore directories', async () => { + await createTestFile('.gitignore', '/foo/\n/a/*/c/'); + + const extraPatterns = ['!foo/', '!a/*/c/']; + parser = new GitIgnoreParser(projectRoot, extraPatterns); + + expect(parser.isIgnored('foo/bar/file.txt')).toBe(false); + expect(parser.isIgnored('a/b/c/file.txt')).toBe(false); + }); + + it('should handle extraPatterns that unignore directories with nested gitignore', async () => { + await createTestFile('.gitignore', '/foo/'); + await createTestFile('foo/bar/.gitignore', 'file.txt'); + + const extraPatterns = ['!foo/']; + parser = new GitIgnoreParser(projectRoot, extraPatterns); + + expect(parser.isIgnored('foo/bar/file.txt')).toBe(true); + expect(parser.isIgnored('foo/bar/file2.txt')).toBe(false); + }); + }); }); diff --git a/packages/core/src/utils/gitIgnoreParser.ts b/packages/core/src/utils/gitIgnoreParser.ts index 199240e69c..f2dc3d1a28 100644 --- a/packages/core/src/utils/gitIgnoreParser.ts +++ b/packages/core/src/utils/gitIgnoreParser.ts @@ -16,9 +16,20 @@ export class GitIgnoreParser implements GitIgnoreFilter { private projectRoot: string; private cache: Map = new Map(); private globalPatterns: string[] | undefined; + private processedExtraPatterns: string[] = []; - constructor(projectRoot: string) { + constructor( + projectRoot: string, + private readonly extraPatterns?: string[], + ) { this.projectRoot = path.resolve(projectRoot); + if (this.extraPatterns) { + // extraPatterns are assumed to be from project root (like .geminiignore) + this.processedExtraPatterns = this.processPatterns( + this.extraPatterns, + '.', + ); + } } private loadPatternsForFile(patternsFilePath: string): string[] { @@ -40,8 +51,15 @@ export class GitIgnoreParser implements GitIgnoreFilter { .split(path.sep) .join(path.posix.sep); - return content - .split('\n') + const rawPatterns = content.split('\n'); + return this.processPatterns(rawPatterns, relativeBaseDir); + } + + private processPatterns( + rawPatterns: string[], + relativeBaseDir: string, + ): string[] { + return rawPatterns .map((p) => p.trimStart()) .filter((p) => p !== '' && !p.startsWith('#')) .map((p) => { @@ -155,7 +173,10 @@ export class GitIgnoreParser implements GitIgnoreFilter { const relativeDir = path.relative(this.projectRoot, dir); if (relativeDir) { const normalizedRelativeDir = relativeDir.replace(/\\/g, '/'); - if (ig.ignores(normalizedRelativeDir)) { + const igPlusExtras = ignore() + .add(ig) + .add(this.processedExtraPatterns); + if (igPlusExtras.ignores(normalizedRelativeDir)) { // This directory is ignored by an ancestor's .gitignore. // According to git behavior, we don't need to process this // directory's .gitignore, as nothing inside it can be @@ -182,6 +203,11 @@ export class GitIgnoreParser implements GitIgnoreFilter { } } + // Apply extra patterns (e.g. from .geminiignore) last for precedence + if (this.processedExtraPatterns.length > 0) { + ig.add(this.processedExtraPatterns); + } + return ig.ignores(normalizedPath); } catch (_error) { return false;