fix: gitignore handling (#8177)

packages/core/src/services/fileDiscoveryService.ts

@@ -5,12 +5,12 @@
  */
 
 import type { GitIgnoreFilter } from '../utils/gitIgnoreParser.js';
+import type { GeminiIgnoreFilter } from '../utils/geminiIgnoreParser.js';
 import { GitIgnoreParser } from '../utils/gitIgnoreParser.js';
+import { GeminiIgnoreParser } from '../utils/geminiIgnoreParser.js';
 import { isGitRepository } from '../utils/gitUtils.js';
 import * as path from 'node:path';
 
-const GEMINI_IGNORE_FILE_NAME = '.geminiignore';
-
 export interface FilterFilesOptions {
   respectGitIgnore?: boolean;
   respectGeminiIgnore?: boolean;
@@ -24,27 +24,15 @@ export interface FilterReport {
 
 export class FileDiscoveryService {
   private gitIgnoreFilter: GitIgnoreFilter | null = null;
-  private geminiIgnoreFilter: GitIgnoreFilter | null = null;
+  private geminiIgnoreFilter: GeminiIgnoreFilter | null = null;
   private projectRoot: string;
 
   constructor(projectRoot: string) {
     this.projectRoot = path.resolve(projectRoot);
     if (isGitRepository(this.projectRoot)) {
-      const parser = new GitIgnoreParser(this.projectRoot);
-      try {
-        parser.loadGitRepoPatterns();
-      } catch (_error) {
-        // ignore file not found
-      }
-      this.gitIgnoreFilter = parser;
+      this.gitIgnoreFilter = new GitIgnoreParser(this.projectRoot);
     }
-    const gParser = new GitIgnoreParser(this.projectRoot);
-    try {
-      gParser.loadPatterns(GEMINI_IGNORE_FILE_NAME);
-    } catch (_error) {
-      // ignore file not found
-    }
-    this.geminiIgnoreFilter = gParser;
+    this.geminiIgnoreFilter = new GeminiIgnoreParser(this.projectRoot);
   }
 
   /**

packages/core/src/utils/geminiIgnoreParser.test.ts

@@ -0,0 +1,70 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { GeminiIgnoreParser } from './geminiIgnoreParser.js';
+import * as fs from 'node:fs/promises';
+import * as path from 'node:path';
+import * as os from 'node:os';
+
+describe('GeminiIgnoreParser', () => {
+  let projectRoot: string;
+
+  async function createTestFile(filePath: string, content = '') {
+    const fullPath = path.join(projectRoot, filePath);
+    await fs.mkdir(path.dirname(fullPath), { recursive: true });
+    await fs.writeFile(fullPath, content);
+  }
+
+  beforeEach(async () => {
+    projectRoot = await fs.mkdtemp(
+      path.join(os.tmpdir(), 'geminiignore-test-'),
+    );
+  });
+
+  afterEach(async () => {
+    await fs.rm(projectRoot, { recursive: true, force: true });
+    vi.restoreAllMocks();
+  });
+
+  describe('when .geminiignore exists', () => {
+    beforeEach(async () => {
+      await createTestFile(
+        '.geminiignore',
+        'ignored.txt\n# A comment\n/ignored_dir/\n',
+      );
+      await createTestFile('ignored.txt', 'ignored');
+      await createTestFile('not_ignored.txt', 'not ignored');
+      await createTestFile(
+        path.join('ignored_dir', 'file.txt'),
+        'in ignored dir',
+      );
+      await createTestFile(
+        path.join('subdir', 'not_ignored.txt'),
+        'not ignored',
+      );
+    });
+
+    it('should ignore files specified in .geminiignore', () => {
+      const parser = new GeminiIgnoreParser(projectRoot);
+      expect(parser.getPatterns()).toEqual(['ignored.txt', '/ignored_dir/']);
+      expect(parser.isIgnored('ignored.txt')).toBe(true);
+      expect(parser.isIgnored('not_ignored.txt')).toBe(false);
+      expect(parser.isIgnored(path.join('ignored_dir', 'file.txt'))).toBe(true);
+      expect(parser.isIgnored(path.join('subdir', 'not_ignored.txt'))).toBe(
+        false,
+      );
+    });
+  });
+
+  describe('when .geminiignore does not exist', () => {
+    it('should not load any patterns and not ignore any files', () => {
+      const parser = new GeminiIgnoreParser(projectRoot);
+      expect(parser.getPatterns()).toEqual([]);
+      expect(parser.isIgnored('any_file.txt')).toBe(false);
+    });
+  });
+});

packages/core/src/utils/geminiIgnoreParser.ts

@@ -0,0 +1,81 @@
+/**
+ * @license
+ * Copyright 2025 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import * as fs from 'node:fs';
+import * as path from 'node:path';
+import ignore from 'ignore';
+
+export interface GeminiIgnoreFilter {
+  isIgnored(filePath: string): boolean;
+  getPatterns(): string[];
+}
+
+export class GeminiIgnoreParser implements GeminiIgnoreFilter {
+  private projectRoot: string;
+  private patterns: string[] = [];
+  private ig = ignore();
+
+  constructor(projectRoot: string) {
+    this.projectRoot = path.resolve(projectRoot);
+    this.loadPatterns();
+  }
+
+  private loadPatterns(): void {
+    const patternsFilePath = path.join(this.projectRoot, '.geminiignore');
+    let content: string;
+    try {
+      content = fs.readFileSync(patternsFilePath, 'utf-8');
+    } catch (_error) {
+      // ignore file not found
+      return;
+    }
+
+    this.patterns = (content ?? '')
+      .split('\n')
+      .map((p) => p.trim())
+      .filter((p) => p !== '' && !p.startsWith('#'));
+
+    this.ig.add(this.patterns);
+  }
+
+  isIgnored(filePath: string): boolean {
+    if (this.patterns.length === 0) {
+      return false;
+    }
+
+    if (!filePath || typeof filePath !== 'string') {
+      return false;
+    }
+
+    if (
+      filePath.startsWith('\\') ||
+      filePath === '/' ||
+      filePath.includes('\0')
+    ) {
+      return false;
+    }
+
+    const resolved = path.resolve(this.projectRoot, filePath);
+    const relativePath = path.relative(this.projectRoot, resolved);
+
+    if (relativePath === '' || relativePath.startsWith('..')) {
+      return false;
+    }
+
+    // Even in windows, Ignore expects forward slashes.
+    const normalizedPath = relativePath.replace(/\\/g, '/');
+
+    if (normalizedPath.startsWith('/') || normalizedPath === '') {
+      return false;
+    }
+
+    return this.ig.ignores(normalizedPath);
+  }
+
+  getPatterns(): string[] {
+    return this.patterns;
+  }
+}

packages/core/src/utils/gitIgnoreParser.test.ts

@@ -33,14 +33,16 @@ describe('GitIgnoreParser', () => {
     await fs.rm(projectRoot, { recursive: true, force: true });
   });
 
-  describe('initialization', () => {
-    it('should initialize without errors when no .gitignore exists', async () => {
+  describe('Basic ignore behaviors', () => {
+    beforeEach(async () => {
       await setupGitRepo();
-      expect(() => parser.loadGitRepoPatterns()).not.toThrow();
     });
 
-    it('should load .gitignore patterns when file exists', async () => {
-      await setupGitRepo();
+    it('should not ignore files when no .gitignore exists', async () => {
+      expect(parser.isIgnored('file.txt')).toBe(false);
+    });
+
+    it('should ignore files based on a root .gitignore', async () => {
       const gitignoreContent = `
 # Comment
 node_modules/
@@ -50,52 +52,28 @@ node_modules/
 `;
       await createTestFile('.gitignore', gitignoreContent);
 
-      parser.loadGitRepoPatterns();
-
-      expect(parser.getPatterns()).toEqual([
-        '.git',
-        'node_modules/',
-        '*.log',
-        '/dist',
-        '.env',
-      ]);
       expect(parser.isIgnored(path.join('node_modules', 'some-lib'))).toBe(
         true,
       );
       expect(parser.isIgnored(path.join('src', 'app.log'))).toBe(true);
       expect(parser.isIgnored(path.join('dist', 'index.js'))).toBe(true);
       expect(parser.isIgnored('.env')).toBe(true);
+      expect(parser.isIgnored('src/index.js')).toBe(false);
     });
 
     it('should handle git exclude file', async () => {
-      await setupGitRepo();
       await createTestFile(
         path.join('.git', 'info', 'exclude'),
         'temp/\n*.tmp',
       );
 
-      parser.loadGitRepoPatterns();
-      expect(parser.getPatterns()).toEqual(['.git', 'temp/', '*.tmp']);
       expect(parser.isIgnored(path.join('temp', 'file.txt'))).toBe(true);
       expect(parser.isIgnored(path.join('src', 'file.tmp'))).toBe(true);
-    });
-
-    it('should handle custom patterns file name', async () => {
-      // No .git directory for this test
-      await createTestFile('.geminiignore', 'temp/\n*.tmp');
-
-      parser.loadPatterns('.geminiignore');
-      expect(parser.getPatterns()).toEqual(['temp/', '*.tmp']);
-      expect(parser.isIgnored(path.join('temp', 'file.txt'))).toBe(true);
-      expect(parser.isIgnored(path.join('src', 'file.tmp'))).toBe(true);
-    });
-
-    it('should initialize without errors when no .geminiignore exists', () => {
-      expect(() => parser.loadPatterns('.geminiignore')).not.toThrow();
+      expect(parser.isIgnored('src/file.js')).toBe(false);
     });
   });
 
-  describe('isIgnored', () => {
+  describe('isIgnored path handling', () => {
     beforeEach(async () => {
       await setupGitRepo();
       const gitignoreContent = `
@@ -107,7 +85,6 @@ src/*.tmp
 !src/important.tmp
 `;
       await createTestFile('.gitignore', gitignoreContent);
-      parser.loadGitRepoPatterns();
     });
 
     it('should always ignore .git directory', () => {
@@ -205,8 +182,6 @@ src/*.tmp
     });
 
     it('should handle nested .gitignore files correctly', async () => {
-      parser.loadGitRepoPatterns();
-
       // From root .gitignore
       expect(parser.isIgnored('root-ignored.txt')).toBe(true);
       expect(parser.isIgnored('a/root-ignored.txt')).toBe(true);
@@ -230,34 +205,27 @@ src/*.tmp
       expect(parser.isIgnored('a/d/f/g')).toBe(true);
       expect(parser.isIgnored('a/f/g')).toBe(false);
     });
-
-    it('should correctly transform patterns from nested gitignore files', () => {
-      parser.loadGitRepoPatterns();
-      const patterns = parser.getPatterns();
-
-      // From root .gitignore
-      expect(patterns).toContain('root-ignored.txt');
-
-      // From a/.gitignore
-      expect(patterns).toContain('/a/b'); // /b becomes /a/b
-      expect(patterns).toContain('/a/**/c'); // c becomes /a/**/c
-
-      // From a/d/.gitignore
-      expect(patterns).toContain('/a/d/**/e.txt'); // e.txt becomes /a/d/**/e.txt
-      expect(patterns).toContain('/a/d/f/g'); // f/g becomes /a/d/f/g
-    });
   });
 
   describe('precedence rules', () => {
-    it('should prioritize root .gitignore over .git/info/exclude', async () => {
+    beforeEach(async () => {
       await setupGitRepo();
+    });
+
+    it('should prioritize nested .gitignore over root .gitignore', async () => {
+      await createTestFile('.gitignore', '*.log');
+      await createTestFile('a/b/.gitignore', '!special.log');
+
+      expect(parser.isIgnored('a/b/any.log')).toBe(true);
+      expect(parser.isIgnored('a/b/special.log')).toBe(false);
+    });
+
+    it('should prioritize .gitignore over .git/info/exclude', async () => {
       // Exclude all .log files
       await createTestFile(path.join('.git', 'info', 'exclude'), '*.log');
       // But make an exception in the root .gitignore
       await createTestFile('.gitignore', '!important.log');
 
-      parser.loadGitRepoPatterns();
-
       expect(parser.isIgnored('some.log')).toBe(true);
       expect(parser.isIgnored('important.log')).toBe(false);
       expect(parser.isIgnored(path.join('subdir', 'some.log'))).toBe(true);
@@ -266,15 +234,4 @@ src/*.tmp
       );
     });
   });
-
-  describe('getIgnoredPatterns', () => {
-    it('should return the raw patterns added', async () => {
-      await setupGitRepo();
-      const gitignoreContent = '*.log\n!important.log';
-      await createTestFile('.gitignore', gitignoreContent);
-
-      parser.loadGitRepoPatterns();
-      expect(parser.getPatterns()).toEqual(['.git', '*.log', '!important.log']);
-    });
-  });
 });

packages/core/src/utils/gitIgnoreParser.ts

@@ -6,93 +6,38 @@
 
 import * as fs from 'node:fs';
 import * as path from 'node:path';
-import ignore, { type Ignore } from 'ignore';
-import { isGitRepository } from './gitUtils.js';
+import ignore from 'ignore';
 
 export interface GitIgnoreFilter {
   isIgnored(filePath: string): boolean;
-  getPatterns(): string[];
 }
 
 export class GitIgnoreParser implements GitIgnoreFilter {
   private projectRoot: string;
-  private ig: Ignore = ignore();
-  private patterns: string[] = [];
-  private readonly maxScannedDirs = 200;
+  private cache: Map<string, string[]> = new Map();
+  private globalPatterns: string[] | undefined;
 
   constructor(projectRoot: string) {
     this.projectRoot = path.resolve(projectRoot);
   }
 
-  loadGitRepoPatterns(): void {
-    if (!isGitRepository(this.projectRoot)) return;
-
-    // Always ignore .git directory regardless of .gitignore content
-    this.addPatterns(['.git']);
-
-    this.loadPatterns(path.join('.git', 'info', 'exclude'));
-    this.findAndLoadGitignoreFiles(this.projectRoot);
-  }
-
-  private findAndLoadGitignoreFiles(startDir: string): void {
-    const queue: string[] = [startDir];
-    let scannedDirs = 0;
-    let queueHead = 0;
-
-    while (queueHead < queue.length && scannedDirs < this.maxScannedDirs) {
-      const dir = queue[queueHead];
-      queueHead++;
-      scannedDirs++;
-
-      const relativeDir = path.relative(this.projectRoot, dir);
-
-      // For sub-directories, check if they are ignored before proceeding.
-      // The root directory (relativeDir === '') should not be checked.
-      if (relativeDir && this.isIgnored(relativeDir)) {
-        continue;
-      }
-
-      // Load patterns from .gitignore in the current directory
-      const gitignorePath = path.join(dir, '.gitignore');
-      if (fs.existsSync(gitignorePath)) {
-        this.loadPatterns(path.relative(this.projectRoot, gitignorePath));
-      }
-
-      // Recurse into subdirectories
-      try {
-        const entries = fs.readdirSync(dir, { withFileTypes: true });
-        for (const entry of entries) {
-          if (entry.name === '.git') {
-            continue;
-          }
-          if (entry.isDirectory()) {
-            queue.push(path.join(dir, entry.name));
-          }
-        }
-      } catch (_error) {
-        // ignore readdir errors
-      }
-    }
-  }
-
-  loadPatterns(patternsFileName: string): void {
-    const patternsFilePath = path.join(this.projectRoot, patternsFileName);
+  private loadPatternsForFile(patternsFilePath: string): string[] {
     let content: string;
     try {
       content = fs.readFileSync(patternsFilePath, 'utf-8');
     } catch (_error) {
-      // ignore file not found
-      return;
+      return [];
     }
 
-    // .git/info/exclude file patterns are relative to project root and not file directory
-    const isExcludeFile =
-      patternsFileName.replace(/\\/g, '/') === '.git/info/exclude';
+    const isExcludeFile = patternsFilePath.endsWith(
+      path.join('.git', 'info', 'exclude'),
+    );
+
     const relativeBaseDir = isExcludeFile
       ? '.'
-      : path.dirname(patternsFileName);
+      : path.dirname(path.relative(this.projectRoot, patternsFilePath));
 
-    const patterns = (content ?? '')
+    return content
       .split('\n')
       .map((p) => p.trim())
       .filter((p) => p !== '' && !p.startsWith('#'))
@@ -150,12 +95,6 @@ export class GitIgnoreParser implements GitIgnoreFilter {
         return newPattern;
       })
       .filter((p) => p !== '');
-    this.addPatterns(patterns);
-  }
-
-  private addPatterns(patterns: string[]) {
-    this.ig.add(patterns);
-    this.patterns.push(...patterns);
   }
 
   isIgnored(filePath: string): boolean {
@@ -163,11 +102,8 @@ export class GitIgnoreParser implements GitIgnoreFilter {
       return false;
     }
 
-    if (
-      filePath.startsWith('\\') ||
-      filePath === '/' ||
-      filePath.includes('\0')
-    ) {
+    const absoluteFilePath = path.resolve(this.projectRoot, filePath);
+    if (!absoluteFilePath.startsWith(this.projectRoot)) {
       return false;
     }
 
@@ -186,13 +122,68 @@ export class GitIgnoreParser implements GitIgnoreFilter {
         return false;
       }
 
-      return this.ig.ignores(normalizedPath);
+      const ig = ignore();
+
+      // Always ignore .git directory
+      ig.add('.git');
+
+      // Load global patterns from .git/info/exclude on first call
+      if (this.globalPatterns === undefined) {
+        const excludeFile = path.join(
+          this.projectRoot,
+          '.git',
+          'info',
+          'exclude',
+        );
+        this.globalPatterns = fs.existsSync(excludeFile)
+          ? this.loadPatternsForFile(excludeFile)
+          : [];
+      }
+      ig.add(this.globalPatterns);
+
+      const pathParts = relativePath.split(path.sep);
+
+      const dirsToVisit = [this.projectRoot];
+      let currentAbsDir = this.projectRoot;
+      // Collect all directories in the path
+      for (let i = 0; i < pathParts.length - 1; i++) {
+        currentAbsDir = path.join(currentAbsDir, pathParts[i]);
+        dirsToVisit.push(currentAbsDir);
+      }
+
+      for (const dir of dirsToVisit) {
+        const relativeDir = path.relative(this.projectRoot, dir);
+        if (relativeDir) {
+          const normalizedRelativeDir = relativeDir.replace(/\\/g, '/');
+          if (ig.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
+            // un-ignored.
+            break;
+          }
+        }
+
+        if (this.cache.has(dir)) {
+          const patterns = this.cache.get(dir);
+          if (patterns) {
+            ig.add(patterns);
+          }
+        } else {
+          const gitignorePath = path.join(dir, '.gitignore');
+          if (fs.existsSync(gitignorePath)) {
+            const patterns = this.loadPatternsForFile(gitignorePath);
+            this.cache.set(dir, patterns);
+            ig.add(patterns);
+          } else {
+            this.cache.set(dir, []); // Cache miss
+          }
+        }
+      }
+
+      return ig.ignores(normalizedPath);
     } catch (_error) {
       return false;
     }
   }
-
-  getPatterns(): string[] {
-    return this.patterns;
-  }
 }