Skip to content

Conversation

@roo-code-preview
Copy link

@roo-code-preview roo-code-preview bot commented Sep 9, 2025

This PR attempts to address Issue #5656 by fixing the inconsistent behavior of ignore patterns in the codebase indexing system.

Changes Made

🔧 Core Fixes

  • Added .gitignore fallback support: When .rooignore does not exist, the system now falls back to using .gitignore patterns
  • Fixed .rooignore restart requirement: Enhanced file watchers to properly reload ignore patterns without requiring VSCode restart
  • Fixed clearIndexData() bug: Services are now recreated after clearing index data to ensure ignore patterns are properly reloaded

🏗️ Implementation Details

  • Enhanced RooIgnoreController to handle both .gitignore and .rooignore patterns with proper fallback logic
  • Updated CodeIndexManager to properly manage service lifecycle and cleanup
  • Simplified DirectoryScanner to avoid duplicate ignore filtering
  • Added comprehensive file watching for both .gitignore and .rooignore changes

✅ Issues Resolved

  1. .gitignore completely ignored → Now used as fallback when .rooignore doesn't exist
  2. .rooignore requires VSCode restart → File watchers now properly reload patterns automatically
  3. Clear Index Data ignores .rooignore → Services are recreated to ensure patterns are reloaded

Testing

  • All existing tests pass
  • Updated test error messages to reflect new functionality
  • TypeScript compilation successful
  • Linting passes

Backward Compatibility

  • Maintains full backward compatibility
  • Existing .rooignore behavior unchanged when file exists
  • No breaking changes to public APIs

Feedback and guidance are welcome!


Important

Improves ignore pattern handling by adding .gitignore fallback, enhancing file watchers, and fixing service recreation in clearIndexData().

  • Behavior:
    • Adds fallback to .gitignore in RooIgnoreController when .rooignore is absent.
    • Enhances file watchers in RooIgnoreController to reload ignore patterns without VSCode restart.
    • Fixes clearIndexData() in manager.ts to recreate services and reload ignore patterns.
  • Implementation:
    • Updates RooIgnoreController to handle both .gitignore and .rooignore with fallback logic.
    • Modifies DirectoryScanner to avoid duplicate ignore filtering.
    • Updates CodeIndexManager to manage service lifecycle and cleanup.
  • Testing:
    • Updates tests in RooIgnoreController.spec.ts to reflect new ignore pattern handling.
    • Ensures all existing tests pass and updates test error messages.
  • Misc:
    • Maintains backward compatibility with existing .rooignore behavior.

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

- Add .gitignore fallback support when .rooignore does not exist
- Fix .rooignore changes requiring VSCode restart by enhancing file watchers
- Fix clearIndexData() ignoring .rooignore by recreating services
- Remove duplicate ignore filtering in DirectoryScanner
- Update RooIgnoreController to handle both .gitignore and .rooignore patterns
- Add proper cleanup and service lifecycle management

Fixes #5656
} else {
this.rooIgnoreContent = undefined

// Fallback to .gitignore if .rooignore doesn't exist
Copy link
Contributor

Choose a reason for hiding this comment

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

The fallback logic to load .gitignore if .rooignore is missing looks solid. Consider adding dedicated tests to verify behavior when only .gitignore exists, ensuring the fallback (usingGitIgnoreFallback flag) works as expected.

Copy link
Contributor

@roomote roomote bot left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! I've reviewed the changes and found several issues that need attention:

Critical Issues:

  1. Memory leak in file watchers (line 77 in RooIgnoreController.ts) - The file watchers are being added to disposables twice, which could cause disposal issues.
  2. Missing test coverage - The new .gitignore fallback functionality lacks test coverage.

Important Suggestions:
3. Redundant code (lines 315-337 in manager.ts) - The .gitignore loading logic is now redundant since RooIgnoreController handles both files.
4. Race condition potential - The loadIgnorePatterns() method could be called rapidly if both files change simultaneously.

Minor Improvements:
5. Documentation clarity - The getInstructions() method documentation could be clearer about when it returns .gitignore vs .rooignore instructions.
6. Good work on removing duplicate ignore filtering in scanner.ts with clear explanatory comments!

Overall, the implementation addresses the core issues from #5656, but these items should be addressed before merging.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Sep 9, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Sep 9, 2025
@daniel-lxs daniel-lxs closed this Sep 9, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Sep 9, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Sep 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 Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. 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.

4 participants