Skip to content

Commit 881d925

Browse files
fix(ingestion): exclude venv/node_modules/cache dirs from analyze + all retrieval APIs (#255)
## Problem `codehub analyze` could ingest virtualenv, vendored-dependency, and tool-cache content into the index. The single source of truth for unconditional directory exclusion — `HARDCODED_IGNORES` in `packages/ingestion/src/pipeline/gitignore.ts` — listed `.venv` but **not** the bare `venv`, nor common Python/JS/build/cache directories (`.tox`, `.mypy_cache`, `.pytest_cache`, `.ruff_cache`, `bower_components`, `.pnpm-store`, `.yarn`, `.gradle`, `.parcel-cache`, `.cache`, `.idea`, `.vscode`). Because **every retrieval surface is store-backed** (`query`, `context`, `impact`, `sql`, `pack` all read `store.sqlite`, which is built at scan time), whatever scan ingested leaked into all of them. The fix belongs at scan time, at the one list — no per-tool guards needed. ## What changed - **`HARDCODED_IGNORES` extended.** The scan walker matches each path segment's exact name (`hardcoded.has(name)` in `phases/scan.ts`), so a bare name excludes that directory **at any depth**, not just the repo root. Added: `venv`, `bower_components`, `.pnpm-store`, `.yarn`, `.tox`, `.mypy_cache`, `.pytest_cache`, `.ruff_cache`, `.gradle`, `.parcel-cache`, `.cache`, `.idea`, `.vscode`. The list is now grouped and commented by category. - **Deliberately NOT hardcoded:** `vendor`, `env`, `out`, `bin`, `obj`. These commonly hold first-party source — this repo itself keeps source under `packages/ingestion/src/pipeline/phases/vendor/graphty-leiden.ts`. A hardcoded ignore can't be re-included via `.gitignore` `!`-negation, so hardcoding `vendor` would make OpenCodeHub stop indexing its own code. These are left to the repo's own `.gitignore`, where real vendored deps are already excluded. - **`.gitignore` honoring on analyze** was already correct (`loadGitignoreChain` + layered `shouldIgnore` wired into the scan walk). This PR adds end-to-end regression coverage for it. ## Why retrieval needs no separate change Audited all 28 MCP tools. Every path that returns file paths/content is safe-by-construction once scan exclusion is correct: - Store-backed tools (`query`, `context`, `impact`, `sql`, `detect_changes`, `pack` default engine) only emit nodes the scan kept. - `query` snippet reads touch the filesystem only for nodes already in the store. - Group tools (`group_contracts`, `group_cross_repo_links`) and `list_findings_delta` read persisted `.codehub` registry/SARIF JSON, not arbitrary repo files. - The MCP `scan` tool spawns external scanners that receive `HARDCODED_IGNORES` via the wrappers (same source of truth). - The only FS-walking retrieval path is the **opt-in legacy `repomix` pack engine**, which relies on repomix's own `node_modules`/`.gitignore` defaults (out of scope; slated for removal in M7). ## Tests - `scan.test.ts`: asserts every `HARDCODED_IGNORES` dir is skipped at root **and** nested; that `venv`/`.venv`/`node_modules` never appear in scan output; and that a user-`.gitignore`'d directory is excluded end-to-end through the scan phase. - `gitignore.test.ts`: unit guard pinning the required names, asserting `vendor` is **absent**, and that entries are bare segments (no globs/slashes/dupes). - Full suite green: ingestion 633, scanners 94, CLI 343 — 0 failures. Whole-repo biome lint clean (686 files). `banned-strings` PASS. ## Docs - README "Design choices worth knowing" gains a **First-party source only** row documenting the exclusion contract and the deliberate ambiguous-name exclusions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 7386f7d commit 881d925

4 files changed

Lines changed: 201 additions & 6 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ flowchart LR
7777
| **Apache-2.0, end to end** | Every runtime dep is OSI-approved permissive. No PolyForm, BSL, Commons Clause, Elastic v2, GPL, or AGPL. You can fork, embed, and ship commercial products on top without a license-review detour. |
7878
| **Local-first, offline-capable** | `codehub analyze --offline` opens zero sockets. Your code never leaves your machine. No telemetry. |
7979
| **Deterministic indexing** | Identical inputs produce a byte-identical graph hash. Reproducible. Auditable. Cacheable in CI. |
80+
| **First-party source only** | `analyze` honors the repo's `.gitignore` (nested files included) and always skips dependency installs, virtualenvs, build output, and tool caches — `node_modules`, `.venv`/`venv`, `__pycache__`, `dist`/`build`/`target`, `.next`/`.nuxt`/`.turbo`, `.mypy_cache`/`.pytest_cache`/`.ruff_cache`, `coverage`, and similar. Exclusion is decided once at scan time (`HARDCODED_IGNORES` in `packages/ingestion/src/pipeline/gitignore.ts`), so every retrieval surface — `query`, `context`, `impact`, `sql`, `pack` — inherits it. Ambiguous names that are often real source (`vendor`, `env`, `out`, `bin`) are left to your `.gitignore`, which supports `!`-negation a hardcoded rule can't. |
8081
| **MCP-native** | Works out-of-the-box with Claude Code, Cursor, Codex, Windsurf, OpenCode. The MCP server is the primary interface; CLI exists for scripts and CI. |
8182
| **Single-file embedded storage** | One `store.sqlite` file holds everything — symbols, edges, embeddings, BM25 (FTS5) + HNSW traversal, and the temporal views (cochanges, summaries) — via Node's built-in `node:sqlite`. No daemon, no database to operate, and **zero native storage bindings** (ADR 0019 removed both `@ladybugdb/core` and `@duckdb/node-api`). |
8283
| **15 languages at GA** | TypeScript, JavaScript, Python, Go, Rust, Java, C#, C, C++, Ruby, Kotlin, Swift, PHP, Dart, COBOL — tree-sitter for the first 14 plus a regex provider for fixed-format COBOL. |

packages/ingestion/src/pipeline/gitignore.test.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,53 @@ import { mkdtemp, rm } from "node:fs/promises";
1212
import { tmpdir } from "node:os";
1313
import path from "node:path";
1414
import { test } from "node:test";
15-
import { loadGitignoreChain, parseGitignore, shouldIgnore } from "./gitignore.js";
15+
import {
16+
HARDCODED_IGNORES,
17+
loadGitignoreChain,
18+
parseGitignore,
19+
shouldIgnore,
20+
} from "./gitignore.js";
21+
22+
test("HARDCODED_IGNORES covers the well-known dependency/virtualenv/build/cache dirs", () => {
23+
// Operator contract: venv + node_modules + similar are always excluded,
24+
// even with no .gitignore present. Guards against silent regression of the
25+
// list. Plain `venv` is as load-bearing as `.venv` (both are real
26+
// virtualenv layouts).
27+
const required = [
28+
"node_modules",
29+
"bower_components",
30+
".venv",
31+
"venv",
32+
"__pycache__",
33+
".tox",
34+
".mypy_cache",
35+
".pytest_cache",
36+
".ruff_cache",
37+
"dist",
38+
"build",
39+
"target",
40+
".next",
41+
".nuxt",
42+
".turbo",
43+
"coverage",
44+
".git",
45+
];
46+
const set = new Set(HARDCODED_IGNORES);
47+
for (const name of required) {
48+
assert.ok(set.has(name), `HARDCODED_IGNORES must contain "${name}"`);
49+
}
50+
// `vendor` is intentionally NOT hardcoded — it is ambiguous (real vendored
51+
// first-party source lives under a vendor/ path in this very repo). A
52+
// hardcoded ignore cannot be re-included via .gitignore !negation, so we
53+
// leave vendor exclusion to the repo's own .gitignore.
54+
assert.ok(!set.has("vendor"), "vendor must NOT be hardcoded (left to .gitignore)");
55+
// The list is exact path segments — no globs, no slashes, no duplicates.
56+
assert.equal(set.size, HARDCODED_IGNORES.length, "HARDCODED_IGNORES must not contain duplicates");
57+
for (const name of HARDCODED_IGNORES) {
58+
assert.ok(!name.includes("/"), `"${name}" must be a bare directory segment, not a path`);
59+
assert.ok(!name.includes("*"), `"${name}" must be a literal name, not a glob`);
60+
}
61+
});
1662

1763
test("loadGitignoreChain: root file only — returns a single-entry map", async () => {
1864
const repo = await mkdtemp(path.join(tmpdir(), "och-gi-root-"));

packages/ingestion/src/pipeline/gitignore.ts

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -221,20 +221,59 @@ async function loadDir(
221221
}
222222
}
223223

224-
/** Hardcoded directory names we always skip, even absent a `.gitignore`. */
224+
/**
225+
* Hardcoded directory names we always skip, even absent a `.gitignore`.
226+
*
227+
* The scan walker tests each directory entry's *name* against this set
228+
* (see `phases/scan.ts` — `hardcoded.has(name)`), so a bare name like
229+
* `node_modules` or `venv` excludes that directory at ANY depth, not just
230+
* the repo root. Entries are exact path segments, not globs.
231+
*
232+
* Scope: dependency installs, language/tool virtualenvs, build output, and
233+
* tool caches — directories that never hold first-party source. We
234+
* deliberately exclude ambiguous names that are commonly real source/config
235+
* directories: `env` (often a config module, not only a virtualenv),
236+
* `out`/`bin`/`obj` (frequently first-party), and `vendor` (Go/PHP/Ruby use
237+
* it for third-party deps, but it is also a common name for vendored
238+
* first-party source — this repo keeps source at
239+
* `packages/ingestion/src/pipeline/phases/vendor/`). Those are left to a
240+
* repo's own `.gitignore`, which supports `!negation` for re-inclusion; a
241+
* hardcoded ignore cannot be overridden.
242+
*/
225243
export const HARDCODED_IGNORES: readonly string[] = [
226-
"node_modules",
244+
// Version-control metadata.
227245
".git",
228246
".svn",
229247
".hg",
230-
"dist",
231-
"build",
232-
"target",
248+
// OpenCodeHub's own index / meta directory.
233249
META_DIR_NAME,
250+
// JavaScript / TypeScript dependencies, package-manager stores, and caches.
251+
"node_modules",
252+
"bower_components",
253+
".pnpm-store",
254+
".yarn",
255+
// Python virtualenvs, bytecode caches, and tool caches.
234256
".venv",
257+
"venv",
235258
"__pycache__",
259+
".tox",
260+
".mypy_cache",
261+
".pytest_cache",
262+
".ruff_cache",
263+
// Build / compiler output.
264+
"dist",
265+
"build",
266+
"target",
267+
// Framework build output, bundler + build-tool caches.
236268
".next",
237269
".nuxt",
238270
".turbo",
271+
".gradle",
272+
".parcel-cache",
273+
".cache",
274+
// Test / coverage output.
239275
"coverage",
276+
// Editor / IDE settings (no first-party source).
277+
".idea",
278+
".vscode",
240279
];

packages/ingestion/src/pipeline/phases/scan.test.ts

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import path from "node:path";
77
import { after, before, describe, it } from "node:test";
88
import { promisify } from "node:util";
99
import { KnowledgeGraph } from "@opencodehub/core-types";
10+
import { HARDCODED_IGNORES } from "../gitignore.js";
1011
import type { PipelineContext } from "../types.js";
1112
import { scanPhase } from "./scan.js";
1213

@@ -59,6 +60,114 @@ describe("scanPhase", () => {
5960
assert.ok(!rels.includes("blob.bin"), "binary files must be skipped");
6061
});
6162

63+
it("skips every HARDCODED_IGNORES directory at the repo root and nested", async () => {
64+
// Build a repo where each hardcoded-ignore name appears both at the root
65+
// and one level deep, each holding a source file the scan would otherwise
66+
// pick up. None of those files may appear in the scan output.
67+
const fixture = await mkdtemp(path.join(tmpdir(), "och-scan-hardcoded-"));
68+
try {
69+
await fs.writeFile(path.join(fixture, "real.ts"), "export const R = 1;\n");
70+
for (const name of HARDCODED_IGNORES) {
71+
// Root-level: <name>/leaf.ts
72+
const rootDir = path.join(fixture, name);
73+
await fs.mkdir(rootDir, { recursive: true });
74+
await fs.writeFile(path.join(rootDir, "leaf.ts"), "export const X = 1;\n");
75+
// Nested: src/<name>/leaf.ts — proves per-segment matching at depth.
76+
const nestedDir = path.join(fixture, "src", name);
77+
await fs.mkdir(nestedDir, { recursive: true });
78+
await fs.writeFile(path.join(nestedDir, "leaf.ts"), "export const Y = 2;\n");
79+
}
80+
const ctx: PipelineContext = {
81+
repoPath: fixture,
82+
options: { skipGit: true },
83+
graph: new KnowledgeGraph(),
84+
phaseOutputs: new Map(),
85+
};
86+
const out = await scanPhase.run(ctx, new Map());
87+
const rels = out.files.map((f) => f.relPath);
88+
// The one legitimate source file survives.
89+
assert.ok(rels.includes("real.ts"), "first-party source must be kept");
90+
// No kept path may traverse any hardcoded-ignore directory, at any depth.
91+
for (const name of HARDCODED_IGNORES) {
92+
const offenders = rels.filter((r) => r.split("/").includes(name));
93+
assert.deepEqual(
94+
offenders,
95+
[],
96+
`no scanned path may pass through "${name}/" — found: ${offenders.join(", ")}`,
97+
);
98+
}
99+
} finally {
100+
await rm(fixture, { recursive: true, force: true });
101+
}
102+
});
103+
104+
it("excludes venv/ and node_modules/ specifically, at root and nested", async () => {
105+
// Regression guard for the operator requirement: virtualenvs (.venv AND
106+
// the bare `venv` name) and node_modules must never enter the index.
107+
const fixture = await mkdtemp(path.join(tmpdir(), "och-scan-venv-"));
108+
try {
109+
const layouts = [
110+
"venv/lib/site.py",
111+
".venv/lib/site.py",
112+
"node_modules/pkg/index.js",
113+
"backend/venv/lib/dep.py",
114+
"frontend/node_modules/pkg/index.js",
115+
];
116+
for (const rel of layouts) {
117+
const abs = path.join(fixture, rel);
118+
await fs.mkdir(path.dirname(abs), { recursive: true });
119+
await fs.writeFile(abs, "x\n");
120+
}
121+
await fs.writeFile(path.join(fixture, "app.py"), "print('hi')\n");
122+
const ctx: PipelineContext = {
123+
repoPath: fixture,
124+
options: { skipGit: true },
125+
graph: new KnowledgeGraph(),
126+
phaseOutputs: new Map(),
127+
};
128+
const out = await scanPhase.run(ctx, new Map());
129+
const rels = out.files.map((f) => f.relPath);
130+
assert.ok(rels.includes("app.py"), "first-party source must be kept");
131+
for (const seg of ["venv", ".venv", "node_modules"]) {
132+
assert.ok(
133+
!rels.some((r) => r.split("/").includes(seg)),
134+
`"${seg}/" content must never appear in scan output`,
135+
);
136+
}
137+
} finally {
138+
await rm(fixture, { recursive: true, force: true });
139+
}
140+
});
141+
142+
it("excludes a user-.gitignore'd directory end-to-end through the scan phase", async () => {
143+
// .gitignore honoring on analyze: a directory the repo's own .gitignore
144+
// excludes must not be scanned, even though it is not a hardcoded ignore.
145+
const fixture = await mkdtemp(path.join(tmpdir(), "och-scan-gitignore-"));
146+
try {
147+
await fs.writeFile(path.join(fixture, ".gitignore"), "generated/\nsecret.key\n");
148+
await fs.writeFile(path.join(fixture, "main.ts"), "export const M = 1;\n");
149+
await fs.writeFile(path.join(fixture, "secret.key"), "shh\n");
150+
await fs.mkdir(path.join(fixture, "generated", "deep"), { recursive: true });
151+
await fs.writeFile(path.join(fixture, "generated", "deep", "g.ts"), "export const G = 1;\n");
152+
const ctx: PipelineContext = {
153+
repoPath: fixture,
154+
options: { skipGit: true },
155+
graph: new KnowledgeGraph(),
156+
phaseOutputs: new Map(),
157+
};
158+
const out = await scanPhase.run(ctx, new Map());
159+
const rels = out.files.map((f) => f.relPath);
160+
assert.ok(rels.includes("main.ts"), "tracked source must be kept");
161+
assert.ok(!rels.includes("secret.key"), ".gitignore file pattern must be honored");
162+
assert.ok(
163+
!rels.some((r) => r.startsWith("generated/")),
164+
".gitignore directory pattern must be honored at scan time",
165+
);
166+
} finally {
167+
await rm(fixture, { recursive: true, force: true });
168+
}
169+
});
170+
62171
it("emits deterministic sha256 for each file", async () => {
63172
const one = await scanPhase.run(makeCtx(), new Map());
64173
const two = await scanPhase.run(makeCtx(), new Map());

0 commit comments

Comments
 (0)