Skip to content

refactor(tool): use go-git for gitignore matching#141

Merged
omarluq merged 3 commits into
mainfrom
omarluq/gitignore-reads
Jun 19, 2026
Merged

refactor(tool): use go-git for gitignore matching#141
omarluq merged 3 commits into
mainfrom
omarluq/gitignore-reads

Conversation

@omarluq

@omarluq omarluq commented Jun 19, 2026

Copy link
Copy Markdown
Owner

No description provided.

@coderabbitai

coderabbitai Bot commented Jun 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 15cd09ab-67ab-489e-9bf2-2a4077831318

📥 Commits

Reviewing files that changed from the base of the PR and between 7f9ca0d and 0f8fb0c.

📒 Files selected for processing (2)
  • internal/tool/ignore.go
  • internal/tool/read_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/tool/ignore.go

📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Updated Go module dependencies to support Git .gitignore parsing and filesystem utilities.
  • Refactor
    • Improved read-tool ignore handling to follow standard Git .gitignore matching, including directory patterns, negations, and clearer ignore reasons.
    • Added cached ignore evaluation for the workspace, with automatic invalidation when .gitignore-related inputs change.
  • Tests
    • Expanded and table-driven .gitignore coverage (including nested and negated patterns).
    • Added a test to verify the ignore cache is invalidated after .gitignore updates.

Walkthrough

Replaces the custom gitignore-rule engine in internal/tool/ignore.go with go-git's gitignore matcher and go-billy's osfs filesystem. The data model changes from ignoreRule to ignorePattern (compiled gitignore.Pattern + source string), and all custom glob helpers are replaced by readIgnorePatterns, extractGitignorePatterns, matchingIgnoreReason, and targetIsDirectory. ReadTool and ASTTool are updated to initialize and maintain an ignoreCache for efficient cached evaluation. go.mod gains the required direct and indirect dependencies, and tests are rewritten as table-driven suites covering negation, nested patterns, and cache invalidation.

Changes

go-git gitignore integration

Layer / File(s) Summary
Module dependencies for go-git and go-billy
go.mod
Adds direct requires for go-git/go-billy/v5 and go-git/go-git/v5, and indirect entries for cyphar/filepath-securejoin, go-git/gcfg, jbenet/go-context, golang.org/x/net, and gopkg.in/warnings.v0.
Rewrite ignore evaluation with go-git gitignore matcher
internal/tool/ignore.go
Imports swapped (removes io/fs, path, doublestar; adds go-git/gitignore, go-billy/osfs, slices). ignoreRule replaced by ignorePattern holding a compiled gitignore.Pattern. ignoredReadPath rebuilt to accept a readIgnoreCache parameter, split path parts, detect directory status, build a gitignore.NewMatcher, and derive the reason via matchingIgnoreReason. Custom glob helpers removed and replaced by readIgnorePatterns, readRepositoryIgnorePatterns, extractGitignorePatterns, matchingIgnoreReason, readIgnorePathState, and targetIsDirectory. Cache types readIgnoreCache and repository pattern containers added to manage signature-based freshness.
Tool integration with ignore cache
internal/tool/read.go, internal/tool/ast.go
ReadTool and ASTTool each initialize an ignoreCache during construction and pass it to ignoredReadPath calls during file reading and parsing, enabling cached evaluation of repository gitignore patterns across multiple path checks.
Table-driven tests for negation, nested patterns, and cache invalidation
internal/tool/read_test.go
Adds time import. TestReadToolRespectsGitignoreByDefault rewritten with a richer .gitignore (including !important.log negation and nested patterns), additional test files, and a struct-slice loop asserting refusal messages and exact allowed-read text. TestReadToolInvalidatesGitignoreCache added to verify cache freshness by modifying .gitignore modification times and confirming the tool re-evaluates ignore rules via setReadTestModTime helper and filesystem state snapshots.

Sequence Diagram(s)

No additional sequence diagrams are generated beyond those in the hidden artifact, as the core flow is already visualized there.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Hop hop, the old globs are gone,
go-git now hops the patterns along.
Each .gitignore read with cache so bright,
negations matched and nested in sight.
Cache checks the .git/info/exclude trace—
keeping secrets safe in the warren's place! 🌿

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess relevance to the changeset. Add a description explaining the purpose and impact of switching from custom gitignore matching to go-git's implementation.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: refactoring gitignore matching to use go-git instead of a custom implementation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch omarluq/gitignore-reads

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
internal/tool/ignore.go (1)

95-98: 💤 Low value

Performance consideration: gitignore.ReadPatterns traverses the entire worktree.

The ReadPatterns function walks the directory tree to collect all .gitignore files recursively. For large repositories with deep directory structures, this could introduce latency on every read operation. Consider caching the patterns per workspace root if this becomes a bottleneck.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tool/ignore.go` around lines 95 - 98, The gitignore.ReadPatterns
function traverses the entire directory tree to collect patterns, which causes
performance degradation on large repositories when called repeatedly. Implement
a caching mechanism that stores the parsed patterns indexed by workspaceRoot to
avoid repeated directory traversals. Before calling gitignore.ReadPatterns with
osfs.New(workspaceRoot) and nil, check if patterns have already been cached for
this workspaceRoot and return the cached result if available; only call
ReadPatterns if the cache misses, and store the result in the cache before
returning it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/tool/ignore.go`:
- Around line 95-98: The gitignore.ReadPatterns function traverses the entire
directory tree to collect patterns, which causes performance degradation on
large repositories when called repeatedly. Implement a caching mechanism that
stores the parsed patterns indexed by workspaceRoot to avoid repeated directory
traversals. Before calling gitignore.ReadPatterns with osfs.New(workspaceRoot)
and nil, check if patterns have already been cached for this workspaceRoot and
return the cached result if available; only call ReadPatterns if the cache
misses, and store the result in the cache before returning it.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1f657d61-899f-4742-bcc3-becdf23e9ecf

📥 Commits

Reviewing files that changed from the base of the PR and between bcc50ab and ac85deb.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (3)
  • go.mod
  • internal/tool/ignore.go
  • internal/tool/read_test.go

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 19, 2026
@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.65385% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.39%. Comparing base (bcc50ab) to head (0f8fb0c).

Files with missing lines Patch % Lines
internal/tool/ignore.go 82.82% 10 Missing and 7 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #141      +/-   ##
==========================================
+ Coverage   80.36%   80.39%   +0.02%     
==========================================
  Files         277      277              
  Lines       21824    21838      +14     
==========================================
+ Hits        17539    17556      +17     
+ Misses       3078     3074       -4     
- Partials     1207     1208       +1     
Flag Coverage Δ
unittests 80.39% <83.65%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/tool/read.go`:
- Around line 29-30: The NewReadTool function creates a long-lived
readIgnoreCache that stores gitignore patterns, and these cached patterns are
reused without checking if the .gitignore file has been modified at runtime.
When .gitignore is edited during execution, the stale cached patterns in
readIgnoreCache.repositoryPatterns will not reflect the changes, causing files
to be misclassified. Add staleness detection by tracking the modification time
or hash of the .gitignore file, then validate the cache before using it at lines
76-77 by comparing the current .gitignore state against the previously cached
state and invalidating the cache if changes are detected.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8e6afbbf-9ee2-4087-ba53-f98484f26970

📥 Commits

Reviewing files that changed from the base of the PR and between ac85deb and 7f9ca0d.

📒 Files selected for processing (3)
  • internal/tool/ast.go
  • internal/tool/ignore.go
  • internal/tool/read.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • internal/tool/ignore.go

Comment thread internal/tool/read.go
@omarluq omarluq merged commit 33893c2 into main Jun 19, 2026
15 of 17 checks passed
@omarluq omarluq deleted the omarluq/gitignore-reads branch June 19, 2026 06:56
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant