fix(core): only honor search-relevant ignore files, not every .*ignore - #430
Open
EricSeastrand wants to merge 1 commit into
Open
fix(core): only honor search-relevant ignore files, not every .*ignore#430EricSeastrand wants to merge 1 commit into
EricSeastrand wants to merge 1 commit into
Conversation
findIgnoreFiles() globbed every `.*ignore` file in the codebase root, so any build-tool filter that happens to live there shapes the search index. A one-line `.dockerignore` containing `*` — a completely ordinary thing for a repository that ships a narrow build context — filters out the entire corpus: "Found 0 code files", and the codebase indexes to nothing. `.dockerignore`, `.npmignore`, `.eslintignore` and friends answer a different question than code search does. "Do not ship this in the image" and "do not lint this" are not "do not let a human read this." Discovery is now an allowlist of ignore files that describe source a reader would not want: `.gitignore`, `.contextignore`, `.cursorignore`, `.codeiumignore`. Any other `.*ignore` file found in the root is logged by name so the omission is visible rather than silent, and can be opted back in with CUSTOM_IGNORE_FILES=".dockerignore,.npmignore" (named to match the existing CUSTOM_IGNORE_PATTERNS / CUSTOM_EXTENSIONS). Selection is pulled into a pure module (`ignore-files.ts`) so it is testable without a filesystem, and exported for callers that want to reuse the list.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Context.findIgnoreFiles()(packages/core/src/context.ts#L1221-L1243) globs every.*ignorefile in the codebase root:So any build-tool filter that happens to sit in the repo root shapes the search index. A
.dockerignorecontaining a single*line — an ordinary thing in a repo that ships a narrow build context — filters out the whole corpus:I hit this on a 2,300-file repository that indexed to nothing. Nothing in the output points at
.dockerignore; it just looks like the indexer found no code..dockerignore,.npmignore,.eslintignoreand friends answer a different question than code search does. "Don't ship this in the image" and "don't lint this" are not "don't let a human read this."Change
Ignore-file discovery becomes an allowlist of ignore files that actually describe source a reader wouldn't want:
.gitignore.contextignore.cursorignore,.codeiumignoreEvery other
.*ignorefile present in the root is logged by name so the omission is visible instead of silent:and can be opted back in per-deployment:
CUSTOM_IGNORE_FILES=".dockerignore,.npmignore"named to match the existing
CUSTOM_IGNORE_PATTERNS/CUSTOM_EXTENSIONS.Selection is a pure function in a new
packages/core/src/ignore-files.ts, so it's testable without a filesystem and the honored list is exported for reuse.Tests
src/ignore-files.test.ts— 7 unit tests on the pure selection/parsing logic.src/context.ignore-files.test.ts— 3 integration tests throughContext:.dockerignore's*no longer reaches the effective pattern set,.contextignorestill does, and theCUSTOM_IGNORE_FILESopt-in restores the old behavior.I confirmed the first integration test fails on
masterbefore the fix (*present in the effective patterns) and passes after. Full core suite: 39/39 green,tsc --noEmitclean in bothpackages/coreandpackages/mcp.Things worth pushing back on
Flagging these myself rather than making you find them:
Allowlist vs. surgical denylist. I could instead have excluded a known set (
.dockerignore,.npmignore, …) and kept globbing the rest. I chose the allowlist because the failure mode is unbounded — any tool can drop a new.*ignorein the root and silently shrink someone's index — and an allowlist fails closed against files nobody has thought of yet. But a denylist is strictly less disruptive to existing users, and if you'd rather have that, I'm happy to flip it; the pure module makes it a one-line change.This changes results in the other direction for some users. Anyone currently relying on
.eslintignore/.prettierignoreto keep files out of their index will get a larger corpus after this. That's the real cost of the change. It's mitigated by the skip log naming the exact files and the env var restoring the old behavior, but it is a behavior change for existing installs, not purely a bug fix.The four names are a judgment call, not an exhaustive list.
.aiignore,.aiexclude,.claudeignoreand others exist in the wild. I deliberately kept the list conservative rather than guessing — adding a name is a one-line PR, andCUSTOM_IGNORE_FILEScovers the gap in the meantime. Happy to seed it with more if you have preferences.Relationship to my other PRs
Independent, no conflicts, mergeable in any order.
#427 fixes the downstream symptom of this bug: when a scan does legitimately yield 0 files,
setCodebaseIndexed()'s Issue-#295 guard returns without clearingindexingCodebases, pinning the codebase at "indexing, 100%" forever. That's how the.dockerignorecase presented to me in the first place — this PR stops the corpus being erased, #427 makes a genuinely empty scan report as failed instead of hanging.Also open: #428 (neutral force-reindex messaging), #429 (force during an active index should abort-and-await rather than wipe).