fix(core): combine .gitignore and .geminiignore logic for correct precedence (#11587)

Co-authored-by: Jacob Richman <jacob314@gmail.com>

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)),
+      );
+    });
+  });
 });

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;
       }

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);
+    });
+  });
 });

packages/core/src/utils/gitIgnoreParser.ts

@@ -16,9 +16,20 @@ export class GitIgnoreParser implements GitIgnoreFilter {
   private projectRoot: string;
   private cache: Map<string, string[]> = 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;