fix(repair): discover newest NVM Node release first - #2645
rudycelekli wants to merge 1 commit into
Conversation
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
|
[Medium risk] Changes how the desktop app discovers Node versions. The PR appears safe to merge based on the reviewed changes. SummaryThe PR changes Electron’s NVM directory search to order recognized Node releases numerically, while preserving inherited PATH precedence.
Reviews (1) · Last reviewed commit: "fix(repair): discover newest NVM Node re..." |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughNVM release directories are now sorted by descending numeric Node version instead of lexicographic order. A regression test checks that ordering and confirms inherited PATH entries remain first. The documentation and changelog describe the discovery behavior. ChangesNVM Node Discovery
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to Desktop discovery should now prefer the numerically newest installed NVM Node when falling back from inherited PATH. No concrete merge-blocking risk is established by the supplied review; full platform qualification remains outstanding. 🚥 Pre-merge checks | ✅ 7 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (7 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (2 skipped: 2 unsupported.) Full details: Cross-Platform Default ParityExplanation The default discovery behavior now prefers the newest NVM release on macOS and Linux, but Windows never searches NVM release directories. Resolution Add equivalent Windows NVM release discovery with numeric ordering and verify the default behavior on macOS, Windows, and Linux; or move NVM release discovery behind an explicit opt-in such as a Settings toggle, environment variable, or CLI flag.
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 |
|
All contributors on this pull request have signed the VoiceStudio CLA. Thank you! |
|
recheck |
Summary
With installed NVM releases v9, v22 and v24 and no usable inherited Node path, desktop discovery sorts version folder names lexicographically. This puts v9 ahead of v24. Similarly v22.9 precedes v22.10. Discovery should prefer the newest numeric release while retaining inherited PATH precedence.
Closes #2644.
Changes
Type
Testing
Hosted CI audit at 2026-10-06T12:35:20.584679+00:00: no failing latest checks; required CLA passes. All latest reported checks completed successfully. No upstream merge performed.
Real temporary NVM directory regression: 1 failed and 3 passed before; all 4 pass after. macOS filesystem/subprocess coverage only.
git diff --checkand the repository changelog style gate pass.Focused JavaScript tests used Node 24.19.0 and a reused Vitest 5.0.1 runtime; this repository pins Vitest 4.1.11. No full build qualification is claimed.
Full backend suites and
bun run check:electronwere not run locally. The human CLA signature is registered and the required CLA status passes; hosted results are reported separately below.Checklist
package.json,pyproject.toml,backend/core/version.py, and lockfilestests/fixtures/omnivoice_data/still loads green on thesmoke-matrixCI job (macOS + Windows + Linux)Release cadence
VoiceStudio ships continuous-to-main — no release candidates, no soak windows.
Every merged PR is immediately part of rolling source (
main) and Docker:latest. Electron artifact rehearsals validate desktop packages without publishing.Version bumps require owner approval; validated releases are tagged from
mainand published explicitly under the release checklist.
Users who want stability install an Electron release or pin Docker
:stable.NVM release directories are now searched in descending numeric version order, while inherited
PATHcandidates retain precedence. This prevents older releases such as v9 or v22.9 from outranking v24 or v22.10. Full backend checks and cross-platform smoke tests remain pending, so cross-platform behavior needs verification.