-
Notifications
You must be signed in to change notification settings - Fork 102
fix(dev-config): activate Node via nvm instead of conflicting MSI install (#55)#68
fix(dev-config): activate Node via nvm instead of conflicting MSI install (#55) #68primetimetank21 wants to merge 4 commits into
Conversation
primetimetank21
commented
Jun 8, 2026
@microsoft-github-policy-service agree
...crosoft#55) dev-config.winget installed both OpenJS.NodeJS.LTS (MSI) and CoreyButler.NVMforWindows, but never ran nvm install / nvm use. nvm4w owns the front of PATH via the C:\nvm4w\nodejs junction, which pointed at nothing, so node failed with "command not found" in new terminals. - Remove the OpenJS.NodeJS.LTS MSI resource (ends the PATH race) - Keep CoreyButler.NVMforWindows as the canonical Node manager - Add an NvmActivateNode DSC script that runs nvm install 24 + nvm use 24, refreshing PATH before and after so Node 24 is active for later steps - Update both READMEs (13 -> 12 apps) Node is pinned to major 24 to match the cli-checks.ps1 ^v24. assertion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
d038267 to
22e0bbc
Compare
primetimetank21
commented
Jun 8, 2026
Why this PR pins Node 24 rather than nvm install lts
Three reasons drove the decision to pin 24 explicitly:
-
Repo upgrade pattern is deliberate manual bumps, not auto-tracking.
The most recent example: PR Update Python pinned version from 3.13 to 3.14 #58 ("Update Python pinned version from 3.13 to 3.14", merged 2026年06月04日) advanced Python by swapping the explicit winget id (Python.Python.3.13→Python.Python.3.14) and synchronizing the test assertion regex in the same commit — a deliberate pin bump, not a floating alias. Python (3.13 → 3.14, issue Move to latest Python version (3.13 -> 3.14) #22 ) and Java (21 → 25, issue Move to latest OpenJDK version (21 -> 25) #23 ) were each advanced via an explicit issue → PR → merge cycle. There is no auto-advancing version automation anywhere in the repo, so trackingltswould be out of step with how every other runtime version is managed here. -
The acceptance test hardcodes
^v24..
src/tests/calm-os/cli-checks.ps1assertsnode --versionagainst^v24.. Pinningnvm install 24keeps install, activation, and test internally consistent and deterministic — no hidden coupling to shift over time. -
ltshas a latent failure date.
Node 26 enters LTS on 2026年10月28日. After that date,nvm install ltswould resolve to Node 26 and the existing^v24.test would fail with zero code change. Pinning avoids that silent breakage.
The original OpenJS.NodeJS.LTS package was a quality signal ("stable LTS line, not bleeding-edge") rather than a policy commitment to auto-advance; the README's "(currently Node 24 LTS)" note reflects that v24 is the intended version, not a promise to follow whatever LTS resolves to.
Happy to be redirected: There's no written policy that explicitly rules out lts-tracking — this is an inference from observed practice. If the maintainers prefer nvm install lts, the right companion change would be updating the ^v24. test assertion to ^v[0-9]+\. (or similar) so the two stay in sync. Just let me know and I'll revise accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR resolves a broken Node.js setup caused by installing both the Node.js MSI and NVM for Windows in the same dev-config run, by standardizing on NVM and explicitly activating a Node 24 toolchain during provisioning.
Changes:
- Removed the
OpenJS.NodeJS.LTSpackage fromdev-config.wingetto avoid PATH/junction conflicts with NVM for Windows. - Added an idempotent DSC
PowerShellScriptresource (NvmActivateNode) that installs/uses Node major 24 vianvm. - Updated README docs to reflect the Node/NVM approach and package list changes.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
windows-dev-config/dev-config.winget |
Drops Node MSI install and adds a post-NVM activation script for Node 24. |
src/windows-dev-config/dev-config.winget |
Mirror of the dev-config update (NVM-only + Node 24 activation). |
windows-dev-config/README.md |
Updates documentation to reflect NVM-managed Node 24 and adjusts the apps list. |
src/windows-dev-config/README.md |
Mirror of the README documentation update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- NvmActivateNode: add PowerShell to dependsOn (matches file convention so pwsh 7 is present before the script runs) - Drop misleading "LTS" wording from the Node 24 activation description (script pins by major, not the lts alias) - README: add Windows Terminal to the app list and reframe the count as 11 core apps plus 2 optional (Oh My Posh, PowerToys) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- NvmActivateNode: add PowerShell to dependsOn (matches file convention so pwsh 7 is present before the script runs) - Drop misleading "LTS" wording from the Node 24 activation description (script pins by major, not the lts alias) - README: add Windows Terminal to the app list and reframe the count as 11 core apps plus 2 optional (Oh My Posh, PowerToys) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
3ab8222 to
a334cdb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
...ivateNode Addresses Copilot review on microsoft#68: - Guard node --version with Get-Command in getScript/testScript so the DSC run doesn't throw before Node is activated. - Switch the activation log line from Write-Host to Write-Information. Applied to both mirror config files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Need to go back and do the following:
- Only edit the
src/as per dev guide (so revert top-level changes I made 😅)
...signed-copy edits (src/ only)
Conflict in src/windows-dev-config/dev-config.winget resolved: kept nvm-activation changes
(NvmActivateNode resource, NodeJS MSI removal, Node-24 pin, PowerShell dependsOn, Get-Command
guards, Write-Information) while incorporating main's silent-install additions. Top-level
windows-dev-config/{README.md,dev-config.winget} reverted to match main per src/docs/development.md
(signed release copies regenerated by sign pipeline; only src/ is editable source).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Summary
Fixes #55
Root cause:
dev-config.wingetinstalled bothOpenJS.NodeJS.LTS(MSI) andCoreyButler.NVMforWindowsin the same provisioning run. nvm4w prependsC:\nvm4w\nodejs(a junction) to the Machine PATH, but becausenvm installwas never called, the junction target was empty. In any new terminal,noderesolved to the dead symlink (PATH priority) instead of the MSI install, producing "command not found". The conflict was internal todev-config.wingetitself, before any workload was applied.Changes
src/windows-dev-config/dev-config.wingetNodeJS(OpenJS.NodeJS.LTS) MSI resource; keepnvmForNode; addNvmActivateNodePowerShellScript resourcesrc/windows-dev-config/README.mdNodeJSrow; updatenvmForNoderow; clean up pinned-id referencesOnly the editable source under
src/windows-dev-config/is changed; the top-level signed release copies are regenerated by the sign pipeline (seesrc/docs/development.md) and are intentionally left untouched.Fix detail
Remove the
NodeJS(OpenJS.NodeJS.LTS) MSI resource -- eliminates the PATH race.Keep
nvmForNode(CoreyButler.NVMforWindows) as the canonical Node manager.Add
NvmActivateNode-- aMicrosoft.DSC.Transitional/PowerShellScriptresource withdependsOn: [nvmForNode]. Its three-phasesetScript:$env:Pathfrom Machine + User registry PATH sonvmis visible in the DSC process.nvm install 24thennvm use 24(throws on non-zero exit code).$env:Pathagain sonode/npmare visible to downstream resources.The
testScriptalso refreshes PATH and assertsnvmis on PATH andnode --versionmatches^v24\..Node is pinned to major 24 (not the
ltsalias) to stay in sync with thecli-checks.ps1assertion^v24.and avoid LTS-drift failures when Node 26 eventually becomes LTS.Acceptance criteria (cli-checks.ps1)
The three existing assertions remain valid and unchanged:
nvm versionsucceedsnode --versionmatches^v24.npm --versionsucceedsManual verification needed
nvm use 24creates a filesystem junction atC:\nvm4w\nodejs, which requires elevation and Developer Mode.securityContext: elevatedis set onNvmActivateNode. Please confirm junction creation succeeds on a real provisioning run.