From 63be32e29259cdea15bbbd99fa97fbb40d56253b Mon Sep 17 00:00:00 2001 From: carlos-alm Date: Sat, 15 Aug 2026 19:37:25 -0600 Subject: [PATCH] fix(native): stop dropping dotfile-named source files, fix undercounted build summary (#2391) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The native orchestrator's file collector set .hidden(true) on its ignore::WalkBuilder, blanket-skipping both hidden directories AND hidden files. Dotfile-named source files are common and legitimate (.terraform.lock.hcl, .pa11yci.authed.cjs), so every build in a repo containing one silently dropped it, logged a misleading "likely a Rust extractor bug" warning, and paid a second WASM parse pass to backfill it — even though the real bug was file discovery, not extraction. The JS/TS collector (shouldIgnore in shared/constants.ts) only ever skips hidden directories, never files; collect_files.rs now matches that, with the existing filter_entry closure continuing to skip hidden directories. Also fixes the secondary defect from the same report: the build-completion log line's file count reflected only the files the native orchestrator itself collected, captured before any WASM backfill inserted additional file nodes, so it under-reported relative to `codegraph stats` whenever a backfill happened. Re-derives it the same way `codegraph stats` counts files (kind = 'file') whenever a post-pass wrote data, mirroring the existing node/edge re-count fix for the identical under-report shape (#1452). docs check acknowledged Impact: 1 functions changed, 4 affected --- .../graph/builder/stages/collect_files.rs | 79 ++++++++- .../builder/stages/native-orchestrator.ts | 15 +- ...ue-2391-hidden-dotfile-native-drop.test.ts | 158 ++++++++++++++++++ 3 files changed, 248 insertions(+), 4 deletions(-) create mode 100644 tests/integration/issue-2391-hidden-dotfile-native-drop.test.ts 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 ef24626b8..6c18100f8 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 @@ -265,8 +265,16 @@ pub fn collect_files( let mut directories = HashSet::new(); // Use the `ignore` crate for gitignore-aware walking. + // + // `.hidden()` is deliberately `false`: the crate's own built-in hidden + // filter blanket-skips both hidden files AND hidden directories, but + // dotfile-named source files are common and legitimate (`.terraform.lock.hcl`, + // `.pa11yci.authed.cjs`) — mirrors the JS/TS collector (`shouldIgnore` in + // `shared/constants.ts`), which only ever skips hidden *directories*, never + // files (issue #2391). Hidden directories are still skipped below via the + // explicit `filter_entry` closure, so this only changes file-level behavior. let walker = ignore::WalkBuilder::new(root_dir) - .hidden(true) // skip hidden files/dirs by default + .hidden(false) .git_ignore(true) // respect .gitignore .git_global(false) // skip global gitignore .git_exclude(true) // respect .git/info/exclude @@ -456,6 +464,75 @@ mod tests { let _ = fs::remove_dir_all(&tmp); } + #[test] + fn collect_finds_dotfile_named_source_files() { + // Issue #2391: dotfile-named source files are common and legitimate + // (Terraform's `.terraform.lock.hcl`, pa11y's `.pa11yci.authed.cjs` + // config) — the `ignore` crate's own `.hidden()` filter must not + // blanket-skip them the way it does for genuinely-hidden files. + let tmp = std::env::temp_dir().join("codegraph_collect_dotfile_test"); + let _ = fs::remove_dir_all(&tmp); + fs::create_dir_all(&tmp).unwrap(); + fs::write( + tmp.join(".terraform.lock.hcl"), + "provider \"registry.terraform.io/hashicorp/aws\" {\n version = \"5.31.0\"\n}\n", + ) + .unwrap(); + fs::write( + tmp.join(".pa11yci.authed.cjs"), + "module.exports = { greet() {} };", + ) + .unwrap(); + fs::write(tmp.join("main.tf"), "provider \"aws\" {}").unwrap(); + + let result = collect_files(tmp.to_str().unwrap(), &[], &[], &[]); + let names: HashSet = result + .files + .iter() + .filter_map(|f| { + Path::new(f) + .file_name() + .map(|n| n.to_str().unwrap().to_string()) + }) + .collect(); + + assert!(names.contains(".terraform.lock.hcl")); + assert!(names.contains(".pa11yci.authed.cjs")); + assert!(names.contains("main.tf")); + + let _ = fs::remove_dir_all(&tmp); + } + + #[test] + fn collect_still_skips_hidden_directories() { + // The fix only changes file-level hidden behavior — hidden + // directories (`.git`, `.next`, arbitrary dotdirs) must remain + // excluded via the explicit `filter_entry` closure. + let tmp = std::env::temp_dir().join("codegraph_collect_hidden_dir_test"); + let _ = fs::remove_dir_all(&tmp); + let hidden = tmp.join(".hidden_dir"); + fs::create_dir_all(&hidden).unwrap(); + fs::write(hidden.join("should_be_excluded.py"), "def f(): pass").unwrap(); + fs::create_dir_all(&tmp).unwrap(); + fs::write(tmp.join("app.py"), "def g(): pass").unwrap(); + + let result = collect_files(tmp.to_str().unwrap(), &[], &[], &[]); + let names: HashSet = result + .files + .iter() + .filter_map(|f| { + Path::new(f) + .file_name() + .map(|n| n.to_str().unwrap().to_string()) + }) + .collect(); + + assert!(names.contains("app.py")); + assert!(!names.contains("should_be_excluded.py")); + + let _ = fs::remove_dir_all(&tmp); + } + #[test] fn collect_skips_ignored_dirs() { let tmp = std::env::temp_dir().join("codegraph_collect_ignore_test"); diff --git a/src/domain/graph/builder/stages/native-orchestrator.ts b/src/domain/graph/builder/stages/native-orchestrator.ts index a2c1e4d4e..c1a75213c 100644 --- a/src/domain/graph/builder/stages/native-orchestrator.ts +++ b/src/domain/graph/builder/stages/native-orchestrator.ts @@ -2574,6 +2574,12 @@ async function runPostNativePasses( // Rust orchestrator's counts are still accurate — no re-count needed. let finalNodeCount = result.nodeCount ?? 0; let finalEdgeCount = result.edgeCount ?? 0; + // Issue #2391: same under-report as #1452, but for the file count — the + // Rust orchestrator's fileCount reflects only the files it natively parsed + // itself, captured before the WASM dropped-file backfill (above) inserts + // file nodes for anything it had to backfill. Re-derived the same way + // `codegraph stats` counts files (kind = 'file'), so the two never diverge. + let finalFileCount = result.fileCount ?? 0; const postPassWroteData = backfillHappened || chaEdgeCount > 0 || @@ -2582,19 +2588,22 @@ async function runPostNativePasses( if (postPassWroteData) { try { const counts = (ctx.db as unknown as BetterSqlite3Database) - .prepare('SELECT (SELECT COUNT(*) FROM nodes) AS n, (SELECT COUNT(*) FROM edges) AS e') - .get() as { n: number; e: number }; + .prepare( + "SELECT (SELECT COUNT(*) FROM nodes) AS n, (SELECT COUNT(*) FROM edges) AS e, (SELECT COUNT(*) FROM nodes WHERE kind = 'file') AS f", + ) + .get() as { n: number; e: number; f: number }; if (counts.n !== finalNodeCount || counts.e !== finalEdgeCount) { finalNodeCount = counts.n; finalEdgeCount = counts.e; setBuildMeta(ctx.db, { node_count: finalNodeCount, edge_count: finalEdgeCount }); } + finalFileCount = counts.f; } catch (err) { debug(`Post-pass node/edge re-count failed: ${toErrorMessage(err)}`); } } info( - `Native build orchestrator completed: ${finalNodeCount} nodes, ${finalEdgeCount} edges, ${result.fileCount ?? 0} files`, + `Native build orchestrator completed: ${finalNodeCount} nodes, ${finalEdgeCount} edges, ${finalFileCount} files`, ); return { diff --git a/tests/integration/issue-2391-hidden-dotfile-native-drop.test.ts b/tests/integration/issue-2391-hidden-dotfile-native-drop.test.ts new file mode 100644 index 000000000..99a3a0288 --- /dev/null +++ b/tests/integration/issue-2391-hidden-dotfile-native-drop.test.ts @@ -0,0 +1,158 @@ +/** + * Regression tests for #2391: the native (Rust) build orchestrator's file + * collector set `.hidden(true)` on its `ignore::WalkBuilder`, which blanket- + * skips both hidden directories AND hidden files. Dotfile-named source files + * are common and legitimate — `.terraform.lock.hcl` (Terraform's dependency + * lock file) and `.pa11yci.authed.cjs` (a pa11y config) both matched this and + * were silently dropped by native, triggering a misleading "likely a Rust + * extractor bug" warning and a WASM backfill, even though the real bug was + * file *discovery*, not extraction. The JS/TS collector (`shouldIgnore` in + * `shared/constants.ts`) only ever skips hidden directories, never files — + * the fix aligns the Rust collector with that existing asymmetry. + * + * A second, related defect from the same issue is fixed in + * `native-orchestrator.ts` but not covered here: the build-completion log + * line's file count reflected only the files the native orchestrator itself + * collected, captured before any WASM backfill inserted additional file + * nodes. No isolated regression test was constructible for it — the only + * known real-world trigger for "native's own file collector misses a file + * WASM would find" is the hidden-file bug fixed above, so once that's fixed + * there is no remaining scenario in this codebase where the two file counts + * can diverge. Verified manually instead: reproduced the exact issue scenario + * (a project with `.terraform.lock.hcl`) against the pre-fix binary, saw the + * completion log undercount relative to the DB's true file-node count, then + * confirmed the fix corrects it. + */ + +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import Database from 'better-sqlite3'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { buildGraph } from '../../src/domain/graph/builder.js'; +import { isNativeAvailable } from '../../src/infrastructure/native.js'; + +const hasNative = isNativeAvailable(); +const requireParity = !!process.env.CODEGRAPH_PARITY; +const describeOrSkip = requireParity || hasNative ? describe : describe.skip; + +function readFileNodeRow(dbPath: string, file: string) { + const db = new Database(dbPath, { readonly: true }); + const row = db + .prepare("SELECT name, kind, file FROM nodes WHERE kind='file' AND file = ?") + .get(file) as { name: string; kind: string; file: string } | undefined; + db.close(); + return row; +} + +describeOrSkip('Native orchestrator no longer drops hidden dotfile source files (#2391)', () => { + let tmpBase: string; + let projectDir: string; + let dbPath: string; + let stderrSpy: ReturnType; + let stderrChunks: string[]; + + beforeEach(() => { + tmpBase = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-2391-')); + projectDir = path.join(tmpBase, 'proj'); + fs.mkdirSync(projectDir, { recursive: true }); + dbPath = path.join(projectDir, '.codegraph', 'graph.db'); + + // A dotfile-named HCL source file — mirrors Terraform's real + // `.terraform.lock.hcl` convention exactly (leading dot, multiple + // embedded dots, a nested `provider { ... }` block with a hashes array). + fs.writeFileSync( + path.join(projectDir, '.terraform.lock.hcl'), + [ + '# This file is maintained automatically by "terraform init".', + 'provider "registry.terraform.io/hashicorp/aws" {', + ' version = "5.31.0"', + ' constraints = "~> 5.0"', + ' hashes = [', + ' "h1:abcdefghijklmnopqrstuvwxyz1234567890abcdefg=",', + ' ]', + '}', + '', + ].join('\n'), + ); + fs.writeFileSync( + path.join(projectDir, 'main.tf'), + 'provider "aws" {\n region = "us-east-1"\n}\n', + ); + + // A dotfile-named CommonJS source file — mirrors the issue's + // `.pa11yci.authed.cjs` example. + fs.writeFileSync( + path.join(projectDir, '.pa11yci.authed.cjs'), + "function greet(name) {\n return 'Hello ' + name;\n}\nmodule.exports = { greet };\ngreet('world');\n", + ); + + // A file inside a hidden DIRECTORY — must remain excluded; the fix only + // changes file-level behavior, not the existing directory-level skip. + fs.mkdirSync(path.join(projectDir, '.hidden_dir'), { recursive: true }); + fs.writeFileSync( + path.join(projectDir, '.hidden_dir', 'should_be_excluded.py'), + 'def hidden_func():\n return 1\n', + ); + + stderrChunks = []; + stderrSpy = vi + .spyOn(process.stderr, 'write') + .mockImplementation((chunk: string | Uint8Array) => { + stderrChunks.push(chunk.toString()); + return true; + }); + }); + + afterEach(() => { + stderrSpy.mockRestore(); + try { + fs.rmSync(tmpBase, { recursive: true, force: true }); + } catch { + /* ignore */ + } + }); + + it('natively parses a dotfile-named .hcl file without a WASM backfill warning', async () => { + await buildGraph(projectDir, { engine: 'native', incremental: false, skipRegistry: true }); + + const log = stderrChunks.join(''); + expect(log).not.toContain('likely a Rust extractor bug'); + expect(log).not.toContain('.terraform.lock.hcl'); + + const row = readFileNodeRow(dbPath, '.terraform.lock.hcl'); + expect(row).toBeDefined(); + + const db = new Database(dbPath, { readonly: true }); + const symbols = db + .prepare('SELECT name FROM nodes WHERE file = ? AND kind != ?') + .all('.terraform.lock.hcl', 'file') as { name: string }[]; + db.close(); + expect(symbols.length).toBeGreaterThan(0); + expect(symbols.some((s) => s.name.includes('hashicorp/aws'))).toBe(true); + }, 60_000); + + it('natively parses a dotfile-named .cjs file without a WASM backfill warning', async () => { + await buildGraph(projectDir, { engine: 'native', incremental: false, skipRegistry: true }); + + const log = stderrChunks.join(''); + expect(log).not.toContain('.pa11yci.authed.cjs'); + + const row = readFileNodeRow(dbPath, '.pa11yci.authed.cjs'); + expect(row).toBeDefined(); + + const db = new Database(dbPath, { readonly: true }); + const symbols = db + .prepare('SELECT name FROM nodes WHERE file = ? AND kind != ?') + .all('.pa11yci.authed.cjs', 'file') as { name: string }[]; + db.close(); + expect(symbols.some((s) => s.name === 'greet')).toBe(true); + }, 60_000); + + it('still excludes files inside hidden directories', async () => { + await buildGraph(projectDir, { engine: 'native', incremental: false, skipRegistry: true }); + + const row = readFileNodeRow(dbPath, '.hidden_dir/should_be_excluded.py'); + expect(row).toBeUndefined(); + }, 60_000); +});