Skip to content

Conversation

@roomote
Copy link
Collaborator

@roomote roomote commented Jul 4, 2025

Description

Fixes #5393

This PR resolves an issue where the list_files tool incorrectly returned directories that should be ignored according to .gitignore patterns. The problem was caused by inconsistent gitignore handling between file and directory listing logic.

Root Cause

The list_files tool had three main issues:

  1. Ripgrep handled files correctly - respected .gitignore in recursive mode
  2. Directory listing was broken - custom directory scanning logic didn't properly handle .gitignore patterns
  3. Non-recursive mode ignored .gitignore entirely - used --no-ignore-vcs flag

Changes Made

🔧 Replaced custom gitignore parsing with proper ignore library

  • Removed the limited custom parseGitignoreFile and isIgnoredByGitignore functions
  • Added proper createIgnoreInstance using the same ignore library used elsewhere in the codebase
  • Now handles complex gitignore patterns correctly (wildcards, negation, etc.)

📁 Fixed parent .gitignore file handling

  • Added findGitignoreFiles function that walks up the directory tree
  • Now respects .gitignore files from parent directories, not just the target directory

🔄 Made non-recursive mode respect .gitignore

  • Removed the --no-ignore-vcs flag from non-recursive ripgrep calls
  • Both recursive and non-recursive modes now consistently respect .gitignore

🎯 Improved directory filtering logic

  • Updated shouldIncludeDirectory to use the ignore library for proper pattern matching
  • Uses relative paths for accurate gitignore pattern matching

Testing

  • All existing tests pass (98/98 tests in glob and tools suites)
  • New gitignore-specific tests (5/5 tests covering various scenarios)
  • Verified both recursive and non-recursive modes respect .gitignore
  • Tested nested .gitignore file handling
  • Confirmed complex gitignore patterns work correctly

Verification of Acceptance Criteria

  • Directories matching .gitignore patterns are now properly excluded from list_files output
  • Both recursive and non-recursive modes consistently respect .gitignore patterns
  • Parent .gitignore files are properly considered when filtering directories
  • Complex gitignore patterns (wildcards, negation, etc.) work correctly
  • No regression in existing file filtering functionality

Files Changed

  • src/services/glob/list-files.ts - Main implementation fix
  • src/services/glob/__tests__/gitignore-test.spec.ts - Added comprehensive tests
  • src/services/glob/__tests__/gitignore-integration.spec.ts - Added integration tests

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Comments added for complex logic
  • No breaking changes
  • All tests passing
  • Type checking passes
  • Linting passes

Impact

This fix ensures that the list_files tool now consistently respects .gitignore patterns for both files and directories in both recursive and non-recursive modes, resolving the reported issue and improving the overall reliability of directory listing functionality.


Important

Fixes list_files tool to respect .gitignore patterns for directories using the ignore library.

  • Behavior:
    • list_files tool now respects .gitignore patterns for directories in both recursive and non-recursive modes.
    • Removed --no-ignore-vcs flag from non-recursive ripgrep calls.
  • Implementation:
    • Replaced custom gitignore parsing with ignore library in list-files.ts.
    • Added createIgnoreInstance and findGitignoreFiles functions for handling .gitignore files.
    • Updated shouldIncludeDirectory to use ignore library for pattern matching.
  • Testing:
    • Added gitignore-test.spec.ts and gitignore-integration.spec.ts for comprehensive testing of .gitignore handling.
    • Verified handling of nested .gitignore files and complex patterns.

This description was created by Ellipsis for 3b11b58. You can customize this summary. It will automatically update as commits are pushed.

…5393)

- Replace custom gitignore parsing with proper 'ignore' library
- Fix directory filtering to respect .gitignore patterns in both recursive and non-recursive modes
- Add support for parent .gitignore files
- Remove --no-ignore-vcs flag from non-recursive mode for consistent behavior
- Add comprehensive tests for gitignore functionality

Fixes #5393
@roomote roomote requested review from cte, jr and mrubens as code owners July 4, 2025 14:42
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jul 4, 2025
@dosubot dosubot bot added the bug Something isn't working label Jul 4, 2025
@delve-auditor
Copy link

delve-auditor bot commented Jul 4, 2025

No security or compliance issues detected. Reviewed everything up to f1250a4.

Security Overview
  • 🔎 Scanned files: 3 changed file(s)
Detected Code Changes

The diff is too large to display a summary of code changes.

Reply to this PR with @delve-auditor followed by a description of what change you want and we'll auto-submit a change to this PR to implement it.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 4, 2025
@hannesrudolph hannesrudolph moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jul 7, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jul 7, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Jul 9, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 9, 2025
@mrubens mrubens merged commit a732f18 into main Jul 9, 2025
7 of 9 checks passed
@mrubens mrubens deleted the fix/issue-5393-gitignore-directories branch July 9, 2025 20:02
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jul 9, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

list_files incorrectly returns .gitignore'd directories

5 participants