Replace abandoned winreg module with PowerShell for Windows PATH management#4032
Open
wojtekn wants to merge 2 commits into
Open
Replace abandoned winreg module with PowerShell for Windows PATH management#4032wojtekn wants to merge 2 commits into
wojtekn wants to merge 2 commits into
Conversation
winreg is abandoned and its 1.2.5 release has a known bug we pinned around. Switch the Windows CLI installation manager to Microsoft's maintained @vscode/windows-registry for reads, and write PATH via PowerShell SetEnvironmentVariable (which also broadcasts WM_SETTINGCHANGE so open shells refresh without a re-login). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Collaborator
📊 Performance Test ResultsComparing 235c247 vs trunk app-size
site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
@vscode/windows-registry is a native module with no prebuilt binaries, so it ran node-gyp on install and failed to compile on Windows CI. The original winreg was pure JS. Drop the native dependency entirely and read the user PATH via PowerShell (raw, unexpanded, matching uninstall.ts), sharing one PowerShell runner with the existing write. No native build step, works everywhere PowerShell does. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
Related issues
How AI was used in this PR
Claude Code investigated all
winregusage, evaluated@vscode/windows-registryas the replacement (adopted first, then dropped once it broke Windows CI — see below), and implemented the final PowerShell-based approach. All changes were reviewed by a human.Proposed Changes
The
winregnpm module is abandoned, and its latest release (1.2.5) ships a known bug — our import was pinned to 1.2.4 with a warning comment to avoid it. This removeswinreg.The Windows CLI installation manager (which reads and writes the user
PATHinHKCU\Environment) now goes through PowerShell for both operations, with no registry npm module at all:DoNotExpandEnvironmentNames) so%VAR%entries survive a read/modify/write round-trip untouched — matching whatapps/cli/commands/uninstall.tsalready does.[Environment]::SetEnvironmentVariable(…, 'User'), which also broadcastsWM_SETTINGCHANGE, so a freshly-opened terminal picks up a newly-installedstudiocommand without a re-login — an improvement over the oldwinreg.set().There is no user-facing behavior change to CLI install/uninstall beyond that PATH-refresh improvement.
Why not
@vscode/windows-registry?The original suggestion (and this PR's first attempt) was
@vscode/windows-registry— well-maintained, from the VS Code team. Two things ruled it out:winregwas pure JavaScript and never compiled.@vscode/windows-registryrunsnode-gyp rebuildon install, which failed on our Windows CI (MSB8020: the ClangCL build tools cannot be found) and would add a native-toolchain requirement to every Windows build/package step.GetStringRegKey/GetDWORDRegKeyonly). It couldn't cover the PATH write anyway — that always had to be PowerShell.Since the write was already PowerShell and the read is a one-liner there too, moving both to PowerShell removes the dependency entirely: no native build, no extra npm package, and it works anywhere PowerShell does (i.e. all Windows). This mirrors the raw-registry pattern already used in
uninstall.ts.apps/cli/commands/uninstall.tsis intentionally unchanged — it never usedwinreg(it runs a detached PowerShell helper after the CLI process exits, which no in-process module can do).Testing Instructions
Automated (any platform):
npm run typecheck— passes across all workspacesnpx eslint --fix apps/studio/src/modules/cli/lib/windows-installation-manager.ts— cleannpm test -- apps/studio/src/tests/index.test.ts— passesManual (Windows required):
autoInstallWindowsCliIfNeededruns (or trigger install from the UI).…\studio\binis added toHKCU\Environment\Path, and that any pre-existing%VAR%entries in PATH remain unexpanded.studio --version— it should resolve without a re-login (verifies theWM_SETTINGCHANGEbroadcast).HKCU\Environment\Path.Pre-merge Checklist