diff --git a/crates/codegraph-core/src/domain/graph/builder/pipeline.rs b/crates/codegraph-core/src/domain/graph/builder/pipeline.rs index b1f12748f..8be7497a8 100644 --- a/crates/codegraph-core/src/domain/graph/builder/pipeline.rs +++ b/crates/codegraph-core/src/domain/graph/builder/pipeline.rs @@ -985,6 +985,7 @@ fn collect_source_files( &db_files, &journal.changed, &journal.removed, + &config.ignore_dirs, &config.include, &config.exclude, ) diff --git a/crates/codegraph-core/src/domain/graph/builder/stages/collect_files.rs b/crates/codegraph-core/src/domain/graph/builder/stages/collect_files.rs index 6c18100f8..dd9ef117a 100644 --- a/crates/codegraph-core/src/domain/graph/builder/stages/collect_files.rs +++ b/crates/codegraph-core/src/domain/graph/builder/stages/collect_files.rs @@ -339,11 +339,44 @@ pub fn collect_files( CollectResult { files, directories } } +/// Whether any directory segment of `rel_path` is ignored — mirrors +/// `collect_files`' per-entry `filter_entry` check (an `ignore_set` name +/// match, or a hidden directory other than `.` itself), but applied to an +/// already-flat relative path from the DB/journal instead of a live +/// directory walk (issue #2512: the incremental fast path never re-applied +/// ignore-dir filtering, so a file indexed before a directory was added to +/// the ignore list — e.g. `target` in #2374 — survived every subsequent +/// incremental rebuild indefinitely). +fn path_has_ignored_segment(rel_path: &str, ignore_set: &HashSet) -> bool { + let normalized = rel_path.replace('\\', "/"); + let mut parts = normalized.split('/').peekable(); + while let Some(seg) = parts.next() { + let is_dir_segment = parts.peek().is_some(); + // Mirrors collect_files' filter_entry closure exactly: both the + // ignore_set match and the hidden-directory rule apply only to + // directory segments there, never to the final filename (Greptile + // review on PR #2576 for #2512: an earlier version of this check + // matched every segment including the filename, which could drop a + // real source file the full walk would keep). + if !is_dir_segment { + continue; + } + if ignore_set.contains(seg) { + return true; + } + if seg.starts_with('.') && seg != "." { + return true; + } + } + false +} + /// Reconstruct file list from DB file_hashes + journal deltas (fast path). /// /// Applies `include_patterns` / `exclude_patterns` so incremental builds honor /// config changes — the paths in the DB were collected under an earlier config -/// that may have had different glob filters. +/// that may have had different glob filters. Also re-applies `extra_ignore_dirs` +/// merged with `DEFAULT_IGNORE_DIRS` (issue #2512), matching `collect_files`. /// /// Returns `None` when the fast path isn't applicable. pub fn try_fast_collect( @@ -351,6 +384,7 @@ pub fn try_fast_collect( db_files: &[String], journal_changed: &[String], journal_removed: &[String], + extra_ignore_dirs: &[String], include_patterns: &[String], exclude_patterns: &[String], ) -> CollectResult { @@ -364,6 +398,12 @@ pub fn try_fast_collect( file_set.insert(changed.clone()); } + let ignore_set: HashSet = DEFAULT_IGNORE_DIRS + .iter() + .map(|s| s.to_string()) + .chain(extra_ignore_dirs.iter().cloned()) + .collect(); + let include_set = build_glob_set(include_patterns); let exclude_set = build_glob_set(exclude_patterns); let has_filters = include_set.is_some() || exclude_set.is_some(); @@ -374,6 +414,9 @@ pub fn try_fast_collect( let mut directories = HashSet::new(); for rel_path in &file_set { + if path_has_ignored_segment(rel_path, &ignore_set) { + continue; + } if has_filters { let norm = rel_path.replace('\\', "/"); if !passes_include_exclude(&norm, include_set.as_deref(), exclude_set.as_deref()) { @@ -642,7 +685,7 @@ mod tests { let changed = vec!["src/d.ts".to_string()]; let removed = vec!["src/b.ts".to_string()]; - let result = try_fast_collect(root, &db_files, &changed, &removed, &[], &[]); + let result = try_fast_collect(root, &db_files, &changed, &removed, &[], &[], &[]); assert_eq!(result.files.len(), 3); // a, c, d let names: HashSet<&str> = result .files @@ -655,6 +698,78 @@ mod tests { assert!(names.contains("d.ts")); } + // Regression test for issue #2512: the fast path reconstructs its file + // set purely from file_hashes + journal deltas, so a file indexed + // before a directory joined the ignore list (e.g. DEFAULT_IGNORE_DIRS' + // own `target`, or a config-level ignore_dirs override) survived every + // subsequent incremental rebuild indefinitely, since only the full walk + // applied ignore-dir filtering. + #[test] + fn fast_collect_re_applies_default_ignore_dirs() { + let root = "/project"; + let db_files = vec![ + "src/a.ts".to_string(), + "target/debug/build.rs".to_string(), + "node_modules/pkg/index.js".to_string(), + ]; + + let result = try_fast_collect(root, &db_files, &[], &[], &[], &[], &[]); + let names: HashSet<&str> = result + .files + .iter() + .map(|f| f.rsplit('/').next().unwrap_or(f)) + .collect(); + assert!(names.contains("a.ts")); + assert!( + !names.contains("build.rs"), + "target/ is in DEFAULT_IGNORE_DIRS and must be filtered by the fast path too" + ); + assert!( + !names.contains("index.js"), + "node_modules/ is in DEFAULT_IGNORE_DIRS and must be filtered by the fast path too" + ); + } + + #[test] + fn fast_collect_re_applies_config_ignore_dirs() { + let root = "/project"; + let db_files = vec![ + "src/a.ts".to_string(), + "vendor_extra/pkg/thing.go".to_string(), + ]; + let extra_ignore = vec!["vendor_extra".to_string()]; + + let result = try_fast_collect(root, &db_files, &[], &[], &extra_ignore, &[], &[]); + let names: HashSet<&str> = result + .files + .iter() + .map(|f| f.rsplit('/').next().unwrap_or(f)) + .collect(); + assert!(names.contains("a.ts")); + assert!( + !names.contains("thing.go"), + "a config-level ignore_dirs entry must be re-applied by the fast path too" + ); + } + + // Greptile review on PR #2576 for #2512: the ignore-set match (and the + // hidden-directory rule) must apply only to DIRECTORY segments, exactly + // like collect_files' own filter_entry closure — never to the final + // filename. A file whose bare name happens to equal an ignore-dir entry + // must still be collected. + #[test] + fn fast_collect_does_not_drop_a_file_whose_name_matches_an_ignore_dir_entry() { + let root = "/project"; + let db_files = vec!["src/vendor".to_string()]; + + let result = try_fast_collect(root, &db_files, &[], &[], &[], &[], &[]); + assert_eq!( + result.files.len(), + 1, + "a file literally named 'vendor' is not a directory and must survive" + ); + } + #[test] fn build_glob_set_memoizes_identical_pattern_lists() { // Guards the performance optimization: long-running hosts (watch mode, @@ -742,7 +857,7 @@ mod tests { ]; let exclude = vec!["**/*.test.ts".to_string()]; - let result = try_fast_collect(root, &db_files, &[], &[], &[], &exclude); + let result = try_fast_collect(root, &db_files, &[], &[], &[], &[], &exclude); let names: HashSet<&str> = result .files .iter() diff --git a/src/domain/graph/builder/stages/collect-files.ts b/src/domain/graph/builder/stages/collect-files.ts index 5adace8be..fc8b958fd 100644 --- a/src/domain/graph/builder/stages/collect-files.ts +++ b/src/domain/graph/builder/stages/collect-files.ts @@ -9,7 +9,7 @@ import fs from 'node:fs'; import path from 'node:path'; import { performance } from 'node:perf_hooks'; import { debug, info } from '../../../../infrastructure/logger.js'; -import { normalizePath } from '../../../../shared/constants.js'; +import { buildIgnoreSet, normalizePath } from '../../../../shared/constants.js'; import { compileGlobs, matchesAny } from '../../../../shared/globs.js'; import { readJournal } from '../../journal.js'; import type { PipelineContext } from '../context.js'; @@ -19,6 +19,27 @@ import { readGitignorePatterns, } from '../helpers.js'; +/** + * Whether any directory segment of `relPath` is ignored — mirrors the full + * walk's per-entry `shouldIgnore`-style check (IGNORE_DIRS name match, or a + * hidden directory other than `.` itself), but applied to an already-flat + * relative path from the DB/journal instead of a live directory walk + * (issue #2512: the incremental fast path never re-applied ignore-dir + * filtering, so a file indexed before a directory was added to the ignore + * list — e.g. `target` in #2374 — survived every subsequent incremental + * rebuild indefinitely). + */ +function pathHasIgnoredSegment(relPath: string, ignoreSet: ReadonlySet): boolean { + const segments = relPath.split('/'); + for (let i = 0; i < segments.length; i++) { + const seg = segments[i] ?? ''; + if (ignoreSet.has(seg)) return true; + const isDirSegment = i < segments.length - 1; + if (isDirSegment && seg.startsWith('.') && seg !== '.') return true; + } + return false; +} + /** * Reconstruct allFiles from DB file_hashes + journal deltas. * Returns null when the fast path isn't applicable (first build, no journal, etc). @@ -83,15 +104,21 @@ function tryFastCollect( // config changes (paths from the DB were collected under older config). // Also apply gitignore patterns so the incremental fast path is consistent // with the full filesystem walk (which calls readGitignorePatterns too). + // Also re-apply IGNORE_DIRS (+ config.ignoreDirs/ignoreAdditionalDirs) so a + // file indexed before a directory joined the ignore list self-heals on the + // next incremental build rather than surviving indefinitely (issue #2512). const includeRegexes = compileGlobs(config?.include); const excludeRegexes = compileGlobs(config?.exclude); const hasGlobFilters = includeRegexes.length > 0 || excludeRegexes.length > 0; const gitignoreRegexes = readGitignorePatterns(rootDir); + const extraIgnoreDirs = [...(config?.ignoreDirs ?? []), ...(config?.ignoreAdditionalDirs ?? [])]; + const ignoreSet = buildIgnoreSet(extraIgnoreDirs.length ? extraIgnoreDirs : undefined); const files: string[] = []; const directories = new Set(); for (const relPath of fileSet) { const normRel = normalizePath(relPath); + if (pathHasIgnoredSegment(normRel, ignoreSet)) continue; if (gitignoreRegexes.length > 0 && matchesAny(gitignoreRegexes, normRel)) continue; if (hasGlobFilters && !passesIncludeExclude(normRel, includeRegexes, excludeRegexes)) continue; const absPath = path.join(rootDir, relPath); diff --git a/tests/builder/collect-files.test.ts b/tests/builder/collect-files.test.ts index 638a6edf1..75bcf9298 100644 --- a/tests/builder/collect-files.test.ts +++ b/tests/builder/collect-files.test.ts @@ -5,9 +5,11 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest'; +import { closeDb, initSchema, openDb } from '../../src/db/index.js'; import { PipelineContext } from '../../src/domain/graph/builder/context.js'; import { readGitignorePatterns } from '../../src/domain/graph/builder/helpers.js'; import { collectFiles } from '../../src/domain/graph/builder/stages/collect-files.js'; +import { appendJournalEntries, writeJournalHeader } from '../../src/domain/graph/journal.js'; let tmpDir: string; @@ -74,6 +76,63 @@ describe('collectFiles stage', () => { expect(ctx.parseChanges).toHaveLength(0); expect(ctx.removed).toContain('nonexistent.js'); }); + + // Regression test for issue #2512: the incremental fast path + // (tryFastCollect) reconstructs allFiles purely from file_hashes + journal + // deltas, never re-applying IGNORE_DIRS — so a file that was indexed + // before its directory joined the ignore list (e.g. `vendor`) survives + // every subsequent incremental rebuild, unlike the full filesystem walk. + it('fast path re-applies IGNORE_DIRS to file_hashes rows from before a dir was ignored', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-stage-collect-fastpath-')); + const dbDir = path.join(dir, '.codegraph'); + fs.mkdirSync(dbDir, { recursive: true }); + fs.mkdirSync(path.join(dir, 'src'), { recursive: true }); + fs.mkdirSync(path.join(dir, 'vendor'), { recursive: true }); + fs.writeFileSync(path.join(dir, 'src', 'a.ts'), 'export const a = 1;'); + fs.writeFileSync(path.join(dir, 'vendor', 'stale.go'), 'package vendor'); + + const db = openDb(path.join(dbDir, 'graph.db')); + initSchema(db); + // Simulate a file_hashes row that predates `vendor` joining IGNORE_DIRS + // (or that slipped in through any other means) — the exact scenario + // #2512 describes. + db.prepare('INSERT INTO file_hashes (file, hash, mtime, size) VALUES (?, ?, ?, ?)').run( + 'vendor/stale.go', + 'deadbeef', + 1, + 1, + ); + db.prepare('INSERT INTO file_hashes (file, hash, mtime, size) VALUES (?, ?, ?, ?)').run( + 'src/a.ts', + 'cafebabe', + 1, + 1, + ); + + // A journal with entries proves the watcher was active — required for + // tryFastCollect to apply at all (an empty-but-valid journal falls + // through to the full walk, which would mask this bug). + writeJournalHeader(dbDir, Date.now()); + appendJournalEntries(dbDir, [{ file: 'src/a.ts' }]); + + const ctx = new PipelineContext(); + ctx.rootDir = dir; + ctx.dbPath = path.join(dbDir, 'graph.db'); + ctx.db = db; + ctx.opts = {}; + ctx.incremental = true; + ctx.forceFullRebuild = false; + ctx.config = {}; + + await collectFiles(ctx); + + const relFiles = ctx.allFiles.map((f) => path.relative(dir, f).replace(/\\/g, '/')); + expect(relFiles).toContain('src/a.ts'); + expect(relFiles).not.toContain('vendor/stale.go'); + + closeDb(db); + fs.rmSync(dir, { recursive: true, force: true }); + }); }); describe('readGitignorePatterns', () => {