fix(deps): bump biome to 2.5.6 to stop the LSP daemon memory leak - #317
Merged
Conversation
## Why The Biome LSP daemon for this repo was sitting at **9.7 GB RSS-equivalent and 95% CPU** after ~2.5 days of uptime. `footprint` showed 9,944 MB spread across **112,109 un-freed mmap regions** — a leak signature, not heap growth. (`ps` reported only 1.8 MB RSS because macOS had compressed essentially all of it.) This is a known upstream regression, [biomejs/biome#10658](biomejs/biome#10658), fixed in **2.5.1** by [#10689](biomejs/biome#10689): > The issue was caused by the "Go-to definition" editor feature, which was enabled by default … the feature **triggers the scanner to build the module graph**. This caused memory leak issues. That matches the observed daemon exactly — its logs showed **8,082 `update_module_graph_internal`** spans in a single hour, plus 8,901 `open_file_internal` with `reason=Index`. 2.5.1 also fixed the daemon failing to shut down when the editor closes mid-scan, which is why the leaking process was orphaned at PPID 1, outliving its editor session by over a day. devkit was the only project on the affected machine still pinned to 2.5.0 — every other repo was on 2.5.1+ and idle at ~2 MB. The `^2.5.0` range already permitted 2.5.6; it was the **lockfile** holding it back. ## What changed **Config (3 files)** - `package.json` + `bun.lock` — `@biomejs/biome` 2.5.0 → **2.5.6** - `biome.jsonc` — `$schema` bumped to match, and `!!dist` force-ignore added On the force-ignore: `dist/` is already unlinted (it is not in the `files.includes` allowlist), but a single `!` still lets the LSP scanner index it for the module graph, and it is rewritten wholesale on every build. `!!` keeps the scanner out entirely, per Biome's own guidance for output folders. Verified zero lint-coverage change — `biome check dist/...` already reported "paths were provided but ignored". **Formatting (17 files)** Biome 2.5.6 changed how `it.each([...])(…)` calls are broken. These are pure formatter output from `biome check --write`, scoped to exactly the affected files. **Correctness (1 file)** `cli/__tests__/reconcile-detect-merged.test.mts` — 2 × `lint/correctness/noUnsafeOptionalChaining`, caught by a rule 2.5.0 missed. `mock.calls[0]?.[2]` short-circuits to `undefined` and then `.stdio` is read off it; the `as { stdio?: unknown }` cast hid it from TypeScript. Completed the optional chain rather than asserting non-null, so a missing mock call now fails the assertion cleanly instead of throwing a TypeError. ## Verification - `biome check` — clean on all 18 touched files - `bun run typecheck` — exit 0 - `bunx vitest run cli/__tests__/reconcile-detect-merged.test.mts` — 9/9 passing ## Notes for review - `cli/lib/install/package-json.mts:48` still writes `'@biomejs/biome': '^2.5.0'` into consumer projects. The caret means fresh installs already resolve to 2.5.6, so no consumer is getting the leak — but anyone with a pre-existing lockfile is pinned to it exactly as this repo was. Left out deliberately: it needs a `dist` rebuild and belongs with a release rather than as a drive-by. - The formatting churn is a direct consequence of the version bump, not unrelated cleanup. It was scoped to the 18 affected files; a repo-wide `--write` would have touched 119 files of unrelated in-flight work.
Contributor
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesConfiguration and test maintenance
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@cli/__tests__/checklist-scripts.test.mts`:
- Around line 212-221: Update the checklist test around the `spawnSync`
invocation to configure the fixture’s existing `backendRoots` with a nonmatching
root, while retaining the injected `DEVKIT_*_ROOTS` value of `' . '`. After
generation, assert that the generated state includes `src/auth$(touch
INJECTED).ts`, proving the injected root normalizes to and controls the
repository-root scan rather than falling back to the fixture configuration.
🪄 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 Plus
Run ID: 7a75a26b-fd8f-46d4-916a-e76127f6c3d6
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
biome.jsonccli/__tests__/checklist-scripts.test.mtscli/__tests__/decision-edit-guard.test.mtscli/__tests__/knip-check-hook.test.mtscli/__tests__/reconcile-detect-merged.test.mtscli/__tests__/review-asset-runtime.test.mtscli/__tests__/review-gate-supervisor.test.mtscli/__tests__/review-private-dependencies.test.mtscli/lib/install/install-hooks.test.mtsgate-engine/coverage/__tests__/produce.test.mtsgate-engine/coverage/__tests__/run.test.mtsgate-engine/critique/__tests__/capture-normalizer.test.mtsgate-engine/critique/__tests__/evidence-bindings.test.mtsgate-engine/critique/__tests__/evidence-lineage.test.mtsgate-engine/critique/__tests__/immutable-file.test.mtsgate-engine/critique/__tests__/persistence-lock.test.mtsgate-engine/critique/lifecycle/work-quarantine.test.mtsgate-engine/deterministic/__tests__/run.test.mtsgate-engine/judge/__tests__/verdict-store.test.mtspackage.json
…ust that it is accepted Addresses the CodeRabbit review comment on PR #317. `expect(dot.status).toBe(0)` proved only that `' . '` was *accepted* — it passed equally if the script ignored `DEVKIT_REVIEW_*_ROOTS` and fell back to the fixture's configured `backendRoots: ['src']`, which already covers the crafted file. Now the fixture's configured roots are pointed at `no-such-root` (both backend and frontend), so nothing but the injected root can reach `src/auth\$(touch INJECTED).ts`. The test then asserts the generated state contains that path, using the `stateName` element already present in `REVIEW_ROOT_CASES` but previously unused by this case. Verified load-bearing by mutation: swapping `' . '` for a valid-but-non-matching root fails all 6 cases on the new assertion. 39/39 pass as written; biome and tsc clean.
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.
Why
The Biome LSP daemon for this repo was sitting at 9.7 GB RSS-equivalent and 95% CPU after ~2.5 days of uptime.
footprintshowed 9,944 MB spread across 112,109 un-freed mmap regions — a leak signature, not heap growth. (psreported only 1.8 MB RSS because macOS had compressed essentially all of it.)This is a known upstream regression, biomejs/biome#10658, fixed in 2.5.1 by #10689:
That matches the observed daemon exactly — its logs showed 8,082
update_module_graph_internalspans in a single hour, plus 8,901open_file_internalwithreason=Index.2.5.1 also fixed the daemon failing to shut down when the editor closes mid-scan, which is why the leaking process was orphaned at PPID 1, outliving its editor session by over a day.
devkit was the only project on the affected machine still pinned to 2.5.0 — every other repo was on 2.5.1+ and idle at ~2 MB. The
^2.5.0range already permitted 2.5.6; it was the lockfile holding it back.What changed
Config (3 files)
package.json+bun.lock—@biomejs/biome2.5.0 → 2.5.6biome.jsonc—$schemabumped to match, and!!distforce-ignore addedOn the force-ignore:
dist/is already unlinted (it is not in thefiles.includesallowlist), but a single!still lets the LSP scanner index it for the module graph, and it is rewritten wholesale on every build.!!keeps the scanner out entirely, per Biome's own guidance for output folders. Verified zero lint-coverage change —biome check dist/...already reported "paths were provided but ignored".Formatting (17 files)
Biome 2.5.6 changed how
it.each([...])(…)calls are broken. These are pure formatter output frombiome check --write, scoped to exactly the affected files.Correctness (1 file)
cli/__tests__/reconcile-detect-merged.test.mts— 2 ×lint/correctness/noUnsafeOptionalChaining, caught by a rule 2.5.0 missed.mock.calls[0]?.[2]short-circuits toundefinedand then.stdiois read off it; theas { stdio?: unknown }cast hid it from TypeScript. Completed the optional chain rather than asserting non-null, so a missing mock call now fails the assertion cleanly instead of throwing a TypeError.Verification
biome check— clean on all 18 touched filesbun run typecheck— exit 0bunx vitest run cli/__tests__/reconcile-detect-merged.test.mts— 9/9 passingNotes for review
cli/lib/install/package-json.mts:48still writes'@biomejs/biome': '^2.5.0'into consumer projects. The caret means fresh installs already resolve to 2.5.6, so no consumer is getting the leak — but anyone with a pre-existing lockfile is pinned to it exactly as this repo was. Left out deliberately: it needs adistrebuild and belongs with a release rather than as a drive-by.--writewould have touched 119 files of unrelated in-flight work.Summary by CodeRabbit
Chores
Tests