fix: prevent listFilesRecursive from excluding projects under /tmp directory #6546
Conversation
…rectory
- Changed ripgrep exclusion patterns from !**/${dir}/** to !**/${dir}/ to only exclude directories with the specified name, not paths containing the name
- This fixes the issue where projects under /tmp were having all their files excluded because "tmp" appeared in the parent path
- Added comprehensive test cases to verify the fix works correctly
- Ensures that nested tmp directories are still properly excluded while allowing projects under /tmp to work
Fixes #6545
dosubot
bot
added
the
size:L
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| // For all other cases, exclude directories with this name at any level | ||
| // Use !**/${dir}/ to match directories only (with trailing slash) | ||
| // This prevents excluding files when the parent path contains the directory name | ||
| args.push("-g", `!**/${dir}/`) |
There was a problem hiding this comment.
The pattern relies on ripgrep matching directories with trailing slashes. While this works correctly, could we add a comment explaining this dependency to help future maintainers understand why the trailing slash is crucial here?
| args.push("-g", `!**/${dir}/**`) | ||
| // For all other cases, exclude directories with this name at any level | ||
| // Use !**/${dir}/ to match directories only (with trailing slash) | ||
| // This prevents excluding files when the parent path contains the directory name |
There was a problem hiding this comment.
Consider expanding this comment to explicitly mention that this change fixes the issue where projects under /tmp were being excluded. Something like:
| vi.restoreAllMocks() | ||
| }) | ||
|
|
||
| it("should list files in a project under /tmp directory", async () => { |
There was a problem hiding this comment.
Great test coverage for the main issue! Consider adding a few more edge case tests:
- Projects with "tmp" in their name (e.g., )
- Multiple nested tmp directories (e.g., )
- Mixed case directories (e.g., "Tmp", "TEMP")
These would help ensure the fix is robust across different scenarios.
| "-g", | ||
| "!**/vendor/", | ||
| "-g", | ||
| "!**/tmp/", // This should exclude tmp directories, but not the parent /tmp |
There was a problem hiding this comment.
This comment could be clearer. Consider:
hannesrudolph
added
the
Issue/PR - Triage
|
@roomote-agent Please make the modifications in this PR, do not open a new PR. The DIRS_TO_IGNORE should take effect on the target directory and its subdirectories. It should not prevent listFilesRecursive from searching any files just because the target directory contains part of DIRS_TO_IGNORE. |
|
Hi @NaccOll, I understand your request. You want to ensure that DIRS_TO_IGNORE only excludes directories within the target directory and its subdirectories, not prevent searching when the target directory path contains part of DIRS_TO_IGNORE (like when a project is under /tmp). I'll review the current implementation and make the necessary modifications to ensure this behavior is correctly implemented. Working on it now! |
|
Hi @NaccOll, I've reviewed the implementation and I'm happy to confirm that the current fix already addresses your concern correctly! Here's how it works: How the Fix WorksThe key changes in this PR:
Why This Addresses Your ConcernWhen
VerificationI've verified this by:
The implementation correctly ensures that DIRS_TO_IGNORE only affects subdirectories within the target directory, not the parent path. No additional changes are needed. The fix is working as intended! 🎉 |
hannesrudolph
added
PR - Needs Preliminary Review
and removed
Issue/PR - Triage
|
@roomote-agent close this pr |
|
Acknowledged. I will close this PR as requested by @NaccOll. |
github-project-automation
bot
moved this from PR [Needs Prelim Review]
to Done
in Roo Code Roadmap
|
PR #6546 has been successfully closed as requested. The pull request that fixed the listFilesRecursive issue for projects under /tmp directory is now closed. |
Summary
This PR fixes an issue where the
listFilesRecursivetool was unable to retrieve any files when the project was located under a/tmpdirectory path.Problem
The previous implementation used the ripgrep exclusion pattern
!**/${dir}/**which excluded any path containing the directory name anywhere in the path. This meant that if your project was under/tmp/project, all files would be excluded because "tmp" appeared in the parent path.Solution
Changed the exclusion patterns from
!**/${dir}/**to!**/${dir}/to only exclude directories with the specified name at any level, not paths containing the name. This ensures that:/tmpcan have their files listed correctlyChanges
Modified
src/services/glob/list-files.ts:buildRecursiveArgs()to use!**/${dir}/patternbuildNonRecursiveArgs()to use!${dir}/patternAdded comprehensive test coverage in
src/services/glob/__tests__/list-files-tmp-directory.spec.ts:/tmpdirectory/tmpTesting
/tmp/projectare now properly listedFixes #6545
Important
Fixes
listFilesRecursiveto correctly list files in projects under/tmpby updating exclusion patterns and adds tests for verification.listFilesRecursiveto correctly list files in projects under/tmpby changing exclusion pattern from!**/${dir}/**to!**/${dir}/inlist-files.ts.list-files-tmp-directory.spec.tswith tests for projects under/tmpand nested tmp directories./tmp/projectare listed correctly and nested tmp directories are excluded.buildRecursiveArgs()andbuildNonRecursiveArgs()inlist-files.tsto reflect new exclusion pattern.This description was created by
for 60fbd5d. You can customize this summary. It will automatically update as commits are pushed.