Add watchedFiles support to MBT script importers - #8669
Draft
zielinsky wants to merge 7 commits into
Draft
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughMBT watched-file classification now carries explicit watched paths from imported builds into script importers. Successful imports refresh LSP file watchers, and watched-file changes trigger reimports through a digest-bypassing path. ChangesMBT watched-file tracking
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant ProjectMetalsLspService
participant ScriptMbtImporter
participant ConnectionProvider
participant MbtImport
participant LanguageClient
ProjectMetalsLspService->>ScriptMbtImporter: isWatchedFile(changedPath)
ProjectMetalsLspService->>ConnectionProvider: runMbtReimportIgnoringDigest(importers)
ConnectionProvider->>MbtImport: runIgnoringDigest(importers)
MbtImport-->>ConnectionProvider: return import status
ConnectionProvider->>ScriptMbtImporter: updateWatchedFiles(importers)
ConnectionProvider->>LanguageClient: refresh didChangeWatchedFiles registrations
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
zielinsky
force-pushed
the
mbt-watched-files
branch
from
July 10, 2026 13:48
b73e03f to
0944b60
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
metals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scala (1)
1072-1097: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
switchBspServer()still skips the MBT watched-file refresh. This path runsmbtImport.runUnconditionally(...)directly, butupdateMbtWatchedFiles(...)is only called in the other MBT import flows. Script importers switched here will keep stale watched-file caches until another full reimport runs.🤖 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 `@metals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scala` around lines 1072 - 1097, Update the MBT import path in switchBspServer(), specifically the ConnectRequest branch that calls mbtImport.runUnconditionally(...), to invoke updateMbtWatchedFiles(...) after a successful import and before connect(request, progress), matching the other MBT import flows. Preserve the existing failure handling and ensure watched files are refreshed for script importers during server switching.
🤖 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
`@metals/src/main/scala/scala/meta/internal/metals/mbt/importer/ScriptMbtImporter.scala`:
- Around line 78-89: Normalize watched-file patterns when caching them in
updateWatchedFiles, matching the normalization used by isWatchedFile: convert
backslashes to forward slashes and remove any leading "./" segments before
storing them in ScriptMbtImporter.watchedFilesCache. Keep the lookup comparison
consistent so patterns from mbtBuild.getWatchedFiles match normalized relative
paths, including the corresponding logic around the additional referenced
section.
---
Outside diff comments:
In `@metals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scala`:
- Around line 1072-1097: Update the MBT import path in switchBspServer(),
specifically the ConnectRequest branch that calls
mbtImport.runUnconditionally(...), to invoke updateMbtWatchedFiles(...) after a
successful import and before connect(request, progress), matching the other MBT
import flows. Preserve the existing failure handling and ensure watched files
are refreshed for script importers during server switching.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: cbc9f753-6d2c-400c-bda8-3751077971ca
📒 Files selected for processing (11)
metals/src/main/scala/scala/meta/internal/builds/MavenBuildTool.scalametals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scalametals/src/main/scala/scala/meta/internal/metals/ProjectMetalsLspService.scalametals/src/main/scala/scala/meta/internal/metals/mbt/MbtBuild.scalametals/src/main/scala/scala/meta/internal/metals/mbt/importer/BazelMbtBuildSupport.scalametals/src/main/scala/scala/meta/internal/metals/mbt/importer/BazelMbtImporter.scalametals/src/main/scala/scala/meta/internal/metals/mbt/importer/GradleMbtImporter.scalametals/src/main/scala/scala/meta/internal/metals/mbt/importer/MbtImportProvider.scalametals/src/main/scala/scala/meta/internal/metals/mbt/importer/ScriptMbtImporter.scalatests/unit/src/main/scala/tests/MbtJsonBuilder.scalatests/unit/src/test/scala/scala/meta/internal/metals/mbt/importer/ScriptMbtImporterSuite.scala
zielinsky
force-pushed
the
mbt-watched-files
branch
2 times, most recently
from
July 10, 2026 14:22
3db6bb7 to
5fcf8e0
Compare
zielinsky
force-pushed
the
mbt-watched-files
branch
from
July 13, 2026 13:38
5fcf8e0 to
98fd9a4
Compare
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@metals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scala`:
- Around line 287-298: Update refreshMbtWatchedFiles to normalize watcher glob
paths for LSP compatibility: convert folder.toString() backslashes to forward
slashes and remove any leading "./" from each raw pattern before concatenating
it with the root. Keep watcher registration limited to non-empty patterns and
continue passing the normalized paths to FileSystemWatcher.
In
`@metals/src/main/scala/scala/meta/internal/metals/mbt/importer/MbtImport.scala`:
- Around line 98-125: Update runIgnoringDigest to append the same
completion-based digest status update used by runIfApproved: mark the current
digest Installed when the import succeeds and Rejected when it fails or is
declined. Preserve the existing user prompt and result handling, while ensuring
the digest state is restored after every completion.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3bedc82c-14b0-4a61-8114-1fa54c73c23a
📒 Files selected for processing (11)
metals/src/main/scala/scala/meta/internal/builds/MavenBuildTool.scalametals/src/main/scala/scala/meta/internal/metals/ConnectionProvider.scalametals/src/main/scala/scala/meta/internal/metals/ProjectMetalsLspService.scalametals/src/main/scala/scala/meta/internal/metals/mbt/MbtBuild.scalametals/src/main/scala/scala/meta/internal/metals/mbt/MbtGlobMatcher.scalametals/src/main/scala/scala/meta/internal/metals/mbt/importer/BazelMbtImporter.scalametals/src/main/scala/scala/meta/internal/metals/mbt/importer/GradleMbtImporter.scalametals/src/main/scala/scala/meta/internal/metals/mbt/importer/MbtImport.scalametals/src/main/scala/scala/meta/internal/metals/mbt/importer/MbtImportProvider.scalametals/src/main/scala/scala/meta/internal/metals/mbt/importer/ScriptMbtImporter.scalatests/unit/src/test/scala/scala/meta/internal/metals/mbt/importer/ScriptMbtImporterSuite.scala
🚧 Files skipped from review as they are similar to previous changes (9)
- metals/src/main/scala/scala/meta/internal/metals/ProjectMetalsLspService.scala
- metals/src/main/scala/scala/meta/internal/metals/mbt/importer/BazelMbtImporter.scala
- metals/src/main/scala/scala/meta/internal/builds/MavenBuildTool.scala
- metals/src/main/scala/scala/meta/internal/metals/mbt/MbtGlobMatcher.scala
- metals/src/main/scala/scala/meta/internal/metals/mbt/importer/MbtImportProvider.scala
- metals/src/main/scala/scala/meta/internal/metals/mbt/importer/GradleMbtImporter.scala
- metals/src/main/scala/scala/meta/internal/metals/mbt/MbtBuild.scala
- tests/unit/src/test/scala/scala/meta/internal/metals/mbt/importer/ScriptMbtImporterSuite.scala
- metals/src/main/scala/scala/meta/internal/metals/mbt/importer/ScriptMbtImporter.scala
zielinsky
force-pushed
the
mbt-watched-files
branch
3 times, most recently
from
August 6, 2026 20:21
ea633d2 to
816a8bc
Compare
zielinsky
force-pushed
the
mbt-watched-files
branch
from
August 6, 2026 20:22
816a8bc to
0747bba
Compare
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
1 participant
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.
Extracted from #8360
Screen.Recording.2026-07-14.at.11.26.35.mov
Summary by CodeRabbit
New Features
Bug Fixes
Tests