fix: respect nested .gitignore files during indexing (#178)#183
Open
dLo999 wants to merge 1 commit intoDeusData:mainfrom
Open
fix: respect nested .gitignore files during indexing (#178)#183dLo999 wants to merge 1 commit intoDeusData:mainfrom
dLo999 wants to merge 1 commit intoDeusData:mainfrom
Conversation
When walking subdirectories, load any .gitignore found in the current directory and apply its patterns against paths relative to that directory. The nearest ancestor gitignore is propagated to all descendant directories so that patterns like `.output` in `webapp/.gitignore` exclude `webapp/.output/` and everything inside it. The root .gitignore continues to be handled separately (via the existing `gitignore` parameter with repo-root-relative paths). walk_dir skips re-loading it by only checking for a nested gitignore when `rel_prefix` is non-empty (i.e. inside a subdirectory). Adds three integration tests covering: basic nested exclusion, deep descendant exclusion (gitignore two levels above), and stacking of root and nested gitignores.
dLo999
commented
Mar 29, 2026
Author
dLo999
left a comment
There was a problem hiding this comment.
Review Summary
Implements nested .gitignore support in file discovery. walk_dir() now loads .gitignore files from subdirectories, computes relative paths for matching, and passes them through the recursion tree. Three new tests cover basic nesting, deep subdirectory propagation, and root+nested stacking.
Findings
All nits — no code changes needed:
- [nit] src/discover/discover.c:234 —
local_rel_path()skips bounds check on prefix strip (safe in practice, paths always well-formed from walk_dir) - [nit] src/discover/discover.c:267 — 4096-byte gi_path buffer could overflow with >4085-byte dir_path (filesystem limits prevent this)
- [nit] tests/test_discover.c — No three-level nested gitignore test (root + sub + sub/sub). Design handles it via parameter passing but untested.
- [nit] tests/test_discover.c:671 — Test creates
main.gobut doesn't assert it IS indexed (only validates exclusions)
CI Status
No CI on fork branch. Local: 2589 passed, 0 failed, 3 new tests. Behavioral verification with exact issue scenario (webapp/.output/) confirmed fix.
Verdict
APPROVE — Clean implementation, resource-safe, correctly solves the issue. All findings are minor nits consistent with existing code style.
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.
Closes #178
Summary
Fixes nested
.gitignorefiles not being respected during file discovery. Previously only the root.gitignorewas loaded — subdirectory.gitignorefiles were silently ignored, causing build artifacts (e.g.,webapp/.output/) to be fully indexed.Changes
src/discover/discover.c:walk_dirnow checks for.gitignorein each subdirectory. When found, loads it and matches paths relative to that directory. The loaded gitignore is passed to recursive calls and freed on return. Root.gitignorestill uses repo-relative paths (unchanged).tests/test_discover.c: 3 new tests:discover_nested_gitignore— basic case matching issue scenariodiscover_nested_gitignore_deep— pattern applies to nested subdirectoriesdiscover_nested_gitignore_stacks_with_root— root and nested gitignores both applyTest results
Build: Compiles cleanly on macOS (Apple Clang, arm64)
Test suite: 2589 passed, 0 failed (incremental tests skipped — network offline). 3 new tests pass.
Behavioral verification (exact issue scenario):
Created test repo:
Indexed with
cli index_repository, queried all nodes:webapp/src/app.jswebapp/.output/bundle.jswebapp/.output/vendor.jsdist/out.jsEdge cases:
generated/excludeswebapp/src/generated/).gitignorenot re-loaded (guard:rel_prefix[0] \!= '\0')README: Line 331 already says ".gitignore hierarchy" — now accurate, no update needed.
Generated with agent-team via /issue