feat(git): Refactor gitignore handling to use `ignore` library instead of `minimatch` (#864)

4 files changed, 57 insertions, 216 deletions

diff --git a/packages/core/src/services/fileDiscoveryService.test.ts b/packages/core/src/services/fileDiscoveryService.test.ts
index 2ef83bfa..368f4d23 100644
--- a/packages/core/src/services/fileDiscoveryService.test.ts
+++ b/packages/core/src/services/fileDiscoveryService.test.ts
@@ -140,33 +140,6 @@ describe('FileDiscoveryService', () => {
     });
   });
 
-  describe('getIgnoreInfo', () => {
-    beforeEach(async () => {
-      await service.initialize();
-    });
-
-    it('should return git ignored patterns', () => {
-      const info = service.getIgnoreInfo();
-
-      expect(info.gitIgnored).toEqual(['.git/**', 'node_modules/**']);
-    });
-
-    it('should return empty arrays when git ignore parser is not initialized', async () => {
-      const uninitializedService = new FileDiscoveryService(mockProjectRoot);
-      const info = uninitializedService.getIgnoreInfo();
-
-      expect(info.gitIgnored).toEqual([]);
-    });
-
-    it('should handle git ignore parser returning null patterns', async () => {
-      mockGitIgnoreParser.getIgnoredPatterns.mockReturnValue([] as string[]);
-
-      const info = service.getIgnoreInfo();
-
-      expect(info.gitIgnored).toEqual([]);
-    });
-  });
-
   describe('isGitRepository', () => {
     it('should return true when isGitRepo is explicitly set to true in options', () => {
       const result = service.isGitRepository({ isGitRepo: true });
diff --git a/packages/core/src/services/fileDiscoveryService.ts b/packages/core/src/services/fileDiscoveryService.ts
index 5329507e..3874e752 100644
--- a/packages/core/src/services/fileDiscoveryService.ts
+++ b/packages/core/src/services/fileDiscoveryService.ts
@@ -52,15 +52,6 @@ export class FileDiscoveryService {
   }
 
   /**
-   * Gets patterns that would be ignored for debugging/transparency
-   */
-  getIgnoreInfo(): { gitIgnored: string[] } {
-    return {
-      gitIgnored: this.gitIgnoreFilter?.getIgnoredPatterns() || [],
-    };
-  }
-
-  /**
    * Checks if a single file should be ignored
    */
   shouldIgnoreFile(
diff --git a/packages/core/src/utils/gitIgnoreParser.test.ts b/packages/core/src/utils/gitIgnoreParser.test.ts
index 0b99115a..1646a5b9 100644
--- a/packages/core/src/utils/gitIgnoreParser.test.ts
+++ b/packages/core/src/utils/gitIgnoreParser.test.ts
@@ -15,7 +15,6 @@ vi.mock('fs/promises');
 // Mock gitUtils module
 vi.mock('./gitUtils.js', () => ({
   isGitRepository: vi.fn(() => true),
-  findGitRoot: vi.fn(() => '/test/project'),
 }));
 
 describe('GitIgnoreParser', () => {
@@ -24,7 +23,9 @@ describe('GitIgnoreParser', () => {
 
   beforeEach(() => {
     parser = new GitIgnoreParser(mockProjectRoot);
-    vi.clearAllMocks();
+    // Reset mocks before each test
+    vi.mocked(fs.readFile).mockClear();
+    vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT')); // Default to no file
   });
 
   afterEach(() => {
@@ -33,8 +34,6 @@ describe('GitIgnoreParser', () => {
 
   describe('initialization', () => {
     it('should initialize without errors when no .gitignore exists', async () => {
-      vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT'));
-
       await expect(parser.initialize()).resolves.not.toThrow();
     });
 
@@ -43,106 +42,35 @@ describe('GitIgnoreParser', () => {
 # Comment
 node_modules/
 *.log
-dist
+/dist
 .env
 `;
-      vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent);
+      vi.mocked(fs.readFile)
+        .mockResolvedValueOnce(gitignoreContent)
+        .mockRejectedValue(new Error('ENOENT'));
 
       await parser.initialize();
-      const patterns = parser.getIgnoredPatterns();
 
-      expect(patterns).toContain('.git/**');
-      expect(patterns).toContain('.git');
-      expect(patterns).toContain('node_modules/**');
-      expect(patterns).toContain('**/*.log');
-      expect(patterns).toContain('**/dist');
-      expect(patterns).toContain('**/.env');
+      expect(parser.isIgnored('node_modules/some-lib')).toBe(true);
+      expect(parser.isIgnored('src/app.log')).toBe(true);
+      expect(parser.isIgnored('dist/index.js')).toBe(true);
+      expect(parser.isIgnored('.env')).toBe(true);
     });
 
     it('should handle git exclude file', async () => {
       vi.mocked(fs.readFile).mockImplementation(async (filePath) => {
-        if (filePath === path.join(mockProjectRoot, '.gitignore')) {
-          throw new Error('ENOENT');
-        }
         if (
           filePath === path.join(mockProjectRoot, '.git', 'info', 'exclude')
         ) {
           return 'temp/\n*.tmp';
         }
-        throw new Error('Unexpected file');
+        throw new Error('ENOENT');
       });
 
       await parser.initialize();
-      const patterns = parser.getIgnoredPatterns();
 
-      expect(patterns).toContain('temp/**');
-      expect(patterns).toContain('**/*.tmp');
-    });
-  });
-
-  describe('pattern parsing', () => {
-    it('should handle directory patterns correctly', async () => {
-      const gitignoreContent = 'node_modules/\nbuild/\n';
-      vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent);
-
-      await parser.initialize();
-      const patterns = parser.getIgnoredPatterns();
-
-      expect(patterns).toContain('node_modules/**');
-      expect(patterns).toContain('build/**');
-    });
-
-    it('should handle file patterns correctly', async () => {
-      const gitignoreContent = '*.log\n.env\nconfig.json\n';
-      vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent);
-
-      await parser.initialize();
-      const patterns = parser.getIgnoredPatterns();
-
-      expect(patterns).toContain('**/*.log');
-      expect(patterns).toContain('**/.env');
-      expect(patterns).toContain('**/config.json');
-    });
-
-    it('should skip comments and empty lines', async () => {
-      const gitignoreContent = `
-# This is a comment
-*.log
-
-# Another comment
-.env
-`;
-      vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent);
-
-      await parser.initialize();
-      const patterns = parser.getIgnoredPatterns();
-
-      expect(patterns).not.toContain('# This is a comment');
-      expect(patterns).not.toContain('# Another comment');
-      expect(patterns).toContain('**/*.log');
-      expect(patterns).toContain('**/.env');
-    });
-
-    it('should skip negation patterns for now', async () => {
-      const gitignoreContent = '*.log\n!important.log\n';
-      vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent);
-
-      await parser.initialize();
-      const patterns = parser.getIgnoredPatterns();
-
-      expect(patterns).toContain('**/*.log');
-      expect(patterns).not.toContain('!important.log');
-    });
-
-    it('should handle paths with slashes correctly', async () => {
-      const gitignoreContent = 'src/*.log\ndocs/temp/\n';
-      vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent);
-
-      await parser.initialize();
-      const patterns = parser.getIgnoredPatterns();
-
-      expect(patterns).toContain('src/*.log');
-      expect(patterns).toContain('docs/temp/**');
+      expect(parser.isIgnored('temp/file.txt')).toBe(true);
+      expect(parser.isIgnored('src/file.tmp')).toBe(true);
     });
   });
 
@@ -151,28 +79,32 @@ dist
       const gitignoreContent = `
 node_modules/
 *.log
-dist
-.env
+/dist
+/.env
 src/*.tmp
+!src/important.tmp
 `;
-      vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent);
+      vi.mocked(fs.readFile)
+        .mockResolvedValueOnce(gitignoreContent)
+        .mockRejectedValue(new Error('ENOENT'));
       await parser.initialize();
     });
 
     it('should always ignore .git directory', () => {
       expect(parser.isIgnored('.git')).toBe(true);
       expect(parser.isIgnored('.git/config')).toBe(true);
-      expect(parser.isIgnored('.git/objects/abc123')).toBe(true);
+      expect(parser.isIgnored(path.join(mockProjectRoot, '.git', 'HEAD'))).toBe(
+        true,
+      );
     });
 
     it('should ignore files matching patterns', () => {
-      expect(parser.isIgnored('node_modules')).toBe(true);
-      expect(parser.isIgnored('node_modules/package')).toBe(true);
+      expect(parser.isIgnored('node_modules/package/index.js')).toBe(true);
       expect(parser.isIgnored('app.log')).toBe(true);
       expect(parser.isIgnored('logs/app.log')).toBe(true);
-      expect(parser.isIgnored('dist')).toBe(true);
+      expect(parser.isIgnored('dist/bundle.js')).toBe(true);
       expect(parser.isIgnored('.env')).toBe(true);
-      expect(parser.isIgnored('config/.env')).toBe(true);
+      expect(parser.isIgnored('config/.env')).toBe(false); // .env is anchored to root
     });
 
     it('should ignore files with path-specific patterns', () => {
@@ -180,29 +112,28 @@ src/*.tmp
       expect(parser.isIgnored('other/temp.tmp')).toBe(false);
     });
 
+    it('should handle negation patterns', () => {
+      expect(parser.isIgnored('src/important.tmp')).toBe(false);
+    });
+
     it('should not ignore files that do not match patterns', () => {
       expect(parser.isIgnored('src/index.ts')).toBe(false);
       expect(parser.isIgnored('README.md')).toBe(false);
-      expect(parser.isIgnored('package.json')).toBe(false);
     });
 
     it('should handle absolute paths correctly', () => {
-      const absolutePath = path.join(
-        mockProjectRoot,
-        'node_modules',
-        'package',
-      );
+      const absolutePath = path.join(mockProjectRoot, 'node_modules', 'lib');
       expect(parser.isIgnored(absolutePath)).toBe(true);
     });
 
-    it('should handle paths outside project root', () => {
-      const outsidePath = '/other/project/file.txt';
+    it('should handle paths outside project root by not ignoring them', () => {
+      const outsidePath = path.resolve(mockProjectRoot, '../other/file.txt');
       expect(parser.isIgnored(outsidePath)).toBe(false);
     });
 
     it('should handle relative paths correctly', () => {
-      expect(parser.isIgnored('./node_modules')).toBe(true);
-      expect(parser.isIgnored('../file.txt')).toBe(false);
+      expect(parser.isIgnored('node_modules/some-package')).toBe(true);
+      expect(parser.isIgnored('../some/other/file.txt')).toBe(false);
     });
 
     it('should normalize path separators on Windows', () => {
@@ -212,26 +143,11 @@ src/*.tmp
   });
 
   describe('getIgnoredPatterns', () => {
-    it('should return a copy of patterns array', async () => {
-      const gitignoreContent = '*.log\n';
-      vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent);
+    it('should return the raw patterns added', async () => {
+      const gitignoreContent = '*.log\n!important.log';
+      vi.mocked(fs.readFile).mockResolvedValueOnce(gitignoreContent);
 
       await parser.initialize();
-      const patterns1 = parser.getIgnoredPatterns();
-      const patterns2 = parser.getIgnoredPatterns();
-
-      expect(patterns1).not.toBe(patterns2); // Different array instances
-      expect(patterns1).toEqual(patterns2); // Same content
-    });
-
-    it('should always include .git patterns', async () => {
-      vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT'));
-
-      await parser.initialize();
-      const patterns = parser.getIgnoredPatterns();
-
-      expect(patterns).toContain('.git/**');
-      expect(patterns).toContain('.git');
     });
   });
 });
diff --git a/packages/core/src/utils/gitIgnoreParser.ts b/packages/core/src/utils/gitIgnoreParser.ts
index 098281ca..ae1a7a01 100644
--- a/packages/core/src/utils/gitIgnoreParser.ts
+++ b/packages/core/src/utils/gitIgnoreParser.ts
@@ -6,18 +6,17 @@
 
 import * as fs from 'fs/promises';
 import * as path from 'path';
-import { minimatch } from 'minimatch';
+import ignore, { Ignore } from 'ignore';
 import { isGitRepository } from './gitUtils.js';
 
 export interface GitIgnoreFilter {
   isIgnored(filePath: string): boolean;
-  getIgnoredPatterns(): string[];
 }
 
 export class GitIgnoreParser implements GitIgnoreFilter {
-  private ignorePatterns: string[] = [];
   private projectRoot: string;
   private isGitRepo: boolean = false;
+  private ig: Ignore = ignore();
 
   constructor(projectRoot: string) {
     this.projectRoot = path.resolve(projectRoot);
@@ -32,13 +31,13 @@ export class GitIgnoreParser implements GitIgnoreFilter {
       ];
 
       // Always ignore .git directory regardless of .gitignore content
-      this.ignorePatterns = ['.git/**', '.git'];
+      this.addPatterns(['.git']);
 
       for (const gitIgnoreFile of gitIgnoreFiles) {
         try {
           const content = await fs.readFile(gitIgnoreFile, 'utf-8');
-          const patterns = this.parseGitIgnoreContent(content);
-          this.ignorePatterns.push(...patterns);
+          const patterns = content.split('\n').map((p) => p.trim());
+          this.addPatterns(patterns);
         } catch (_error) {
           // File doesn't exist or can't be read, continue silently
         }
@@ -46,71 +45,33 @@ export class GitIgnoreParser implements GitIgnoreFilter {
     }
   }
 
-  private parseGitIgnoreContent(content: string): string[] {
-    return content
-      .split('\n')
-      .map((line) => line.trim())
-      .filter((line) => line && !line.startsWith('#'))
-      .map((pattern) => {
-        // Handle negation patterns (!) - for now we'll skip them
-        if (pattern.startsWith('!')) {
-          return null;
-        }
-
-        // Convert gitignore patterns to minimatch-compatible patterns
-        if (pattern.endsWith('/')) {
-          // Directory pattern - match directory and all contents
-          const dirPattern = pattern.slice(0, -1); // Remove trailing slash
-          return [dirPattern, dirPattern + '/**'];
-        }
-
-        // If pattern doesn't contain /, it should match at any level
-        if (!pattern.includes('/') && !pattern.startsWith('**/')) {
-          return '**/' + pattern;
-        }
-
-        return pattern;
-      })
-      .filter((pattern) => pattern !== null)
-      .flat() as string[];
+  private addPatterns(patterns: string[]) {
+    this.ig.add(patterns);
   }
 
   isIgnored(filePath: string): boolean {
-    // If not a git repository, nothing is ignored
     if (!this.isGitRepo) {
       return false;
     }
 
-    // Normalize the input path (handle ./ prefixes)
-    let cleanPath = filePath;
-    if (cleanPath.startsWith('./')) {
-      cleanPath = cleanPath.slice(2);
-    }
-
-    // Convert to relative path from project root
-    const relativePath = path.relative(
-      this.projectRoot,
-      path.resolve(this.projectRoot, cleanPath),
-    );
+    const relativePath = path.isAbsolute(filePath)
+      ? path.relative(this.projectRoot, filePath)
+      : filePath;
 
-    // Handle paths that go outside project root
-    if (relativePath.startsWith('..')) {
+    if (relativePath === '' || relativePath.startsWith('..')) {
       return false;
     }
 
-    // Normalize path separators for cross-platform compatibility
-    const normalizedPath = relativePath.replace(/\\/g, '/');
+    let normalizedPath = relativePath.replace(/\\/g, '/');
+    if (normalizedPath.startsWith('./')) {
+      normalizedPath = normalizedPath.substring(2);
+    }
 
-    return this.ignorePatterns.some((pattern) =>
-      minimatch(normalizedPath, pattern, {
-        dot: true,
-        matchBase: false,
-        flipNegate: false,
-      }),
-    );
+    const ignored = this.ig.ignores(normalizedPath);
+    return ignored;
   }
 
-  getIgnoredPatterns(): string[] {
-    return [...this.ignorePatterns];
+  getGitRepoRoot(): string {
+    return this.projectRoot;
   }
 }