fix(lsp): make update_workspace_folders perform incremental updates per LSP spec #7789
Conversation
🦋 Changeset detectedLatest commit: 86bf00d The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe PR changes workspace-folder handling in the LSP server from full replacement to incremental updates. The Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)crates/biome_*/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.rs📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
🔇 Additional comments (1)
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 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/biome_lsp/src/session.rs (1)
618-619: Optional: Remove trailing blank lines.Two consecutive blank lines here—one would suffice.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/fair-crabs-play.md(1 hunks)crates/biome_lsp/src/server.rs(1 hunks)crates/biome_lsp/src/session.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
crates/biome_*/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place core crates under /crates/biome_*/
Files:
crates/biome_lsp/src/session.rscrates/biome_lsp/src/server.rs
**/*.rs
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*.rs: Format Rust files before committing (e.g., viajust fwhich formats Rust)
Document rules, assists, and options with inline rustdoc in source
Files:
crates/biome_lsp/src/session.rscrates/biome_lsp/src/server.rs
.changeset/*.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
.changeset/*.md: In changesets, only use #### or ##### headers; other header levels are not allowed
Changesets should cover user-facing changes only; internal changes do not need changesets
Use past tense for what you did and present tense for current Biome behavior in changesets
When fixing a bug in a changeset, start with an issue link (e.g., “Fixed #1234: …”)
When referencing a rule or assist in a changeset, include a link to its page on the website
Include code blocks in changesets when applicable to illustrate changes
End every sentence in a changeset with a period
Files:
.changeset/fair-crabs-play.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Documentation
- GitHub Check: End-to-end tests
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test Node.js API
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: autofix
- GitHub Check: lint
🔇 Additional comments (3)
.changeset/fair-crabs-play.md (1)
1-5: LGTM!The changeset clearly describes the fix in past tense and follows the guidelines.
crates/biome_lsp/src/session.rs (1)
598-616: Incremental update logic looks correct.The implementation properly handles both cases: updating existing folders (retain + extend) and initialising when none exist. The empty checks are sensible optimisations.
crates/biome_lsp/src/server.rs (1)
410-410: LGTM!The call now correctly passes both
addedandremovedto align with the updated API signature. The removed folders are properly used earlier (lines 390-408) to close projects before updating the list.
Summary
The current implementation of
update_workspace_foldersincorrectly replaces the entire workspace folders list with only theaddedfolders from thedidChangeWorkspaceFoldersevent.According to the LSP specification, the
addedandremovedfields contain only the delta changes, not the complete list. This can be verified by loggingevent.addedandevent.removedin VSCode Extension'sworkspace.onDidChangeWorkspaceFoldershandler.This behavior may cause issues depending on how the plugin is implemented. When a new workspace folder is added to an already-open workspace, the existing folders are removed from the session's
workspace_folderslist, causing Biome diagnostics to stop working in those folders.The official Biome VSCode extension does not experience this issue due to two implementation details:
workspace.onDidChangeWorkspaceFolders(code)LanguageClient, it provides theworkspaceFolderfield as an option. This prevents the LSP from receiving theDidChangeWorkspaceFoldersevent (code) (ref)Therefore, this change does not affect the behavior of the official plugin.
Test Plan
I personally verified that there are no issues with the operation by adding logs in both the extension plugin and the
update_workspace_foldersfunction to observe behavior in VSCode. I believe it would be difficult to verify this behavior without adding such code.The verification steps were as follows:
Run
cargo build --bin biomeon the current branch to build the CLI.In the Biome Monorepo extension (which does not use the workspace field in LanguageClient), set the Biome binary path to the CLI built in step 1, build the extension, and install it in VSCode.
Add a new workspace in VSCode to trigger the
DidChangeWorkspaceFoldersevent.Check whether Biome diagnostics are visible for the focused file.
I also confirmed that the official plugin is unaffected by setting the
biome.lsp.binconfiguration.If reproduction is needed, I can share the branch used for these tests.
Docs
Currently, this change has no impact on end users.