Skip to content

Comments

fix(openclaw): fix Node.js detection for nvm/mise/fnm-managed installations#12902

Merged
kangfenmao merged 13 commits intomainfrom
DeJeune/node22-nvm-fix
Feb 14, 2026
Merged

fix(openclaw): fix Node.js detection for nvm/mise/fnm-managed installations#12902
kangfenmao merged 13 commits intomainfrom
DeJeune/node22-nvm-fix

Conversation

@DeJeune
Copy link
Collaborator

@DeJeune DeJeune commented Feb 12, 2026

What this PR does

Before this PR:

  • On Windows, getLoginShellEnvironment() runs cmd.exe /c set which just inherits the parent (Electron) process's env — it does NOT re-read the Windows registry. If Node.js is installed via MSI after the app launches, the captured PATH is stale. This causes npm preinstall scripts to fail: npm itself runs (found via commonPaths filesystem fallback), but cmd.exe /d /s /c node ./engine-requirements.js can't find node in the stale PATH. On Unix this isn't an issue because zsh -ilc env sources profile files and picks up nvm/mise/fnm PATH changes.
  • On Unix, OpenClaw fails to start/install when Node.js is installed via nvm, mise, or fnm after Cherry Studio has launched, because the cached shell environment is stale.
  • findExecutableInEnv has a hidden side effect of refreshing the shell env cache, making it unpredictable.
  • startGateway uses stale env because findOpenClawBinary refreshes the cache but the gateway spawn uses a different (stale) env.
  • Install process mutates the shared cached env object, polluting all future callers.
  • Inconsistent spawn strategy: install/uninstall use spawn() + shell: true while gateway operations use spawnWithEnv().

After this PR:

  • Windows: skip the useless cmd.exe /c set entirely. Instead, copy process.env as the base (same result, but faster), then read the current system + user PATH from the Windows registry via reg query, expand %VAR% references, and replace the stale PATH. This ensures newly installed tools (e.g. Node.js MSI) are found immediately.
  • Unix: unchanged — zsh -ilc env still sources profile files correctly.
  • Shell env cache follows CQS (Command-Query Separation): getShellEnv() is a pure query, refreshShellEnv() is an explicit command.
  • findExecutableInEnv no longer refreshes the cache — callers explicitly call refreshShellEnv() when they need fresh env.
  • startGateway refreshes env first, then passes it to both findOpenClawBinary and crossPlatformSpawn.
  • Install process clones the cached env before modifying PATH ({ ...await getShellEnv() }).
  • All spawn calls unified to crossPlatformSpawn (handles Windows .cmd files via cmd.exe /c).
  • OpenClaw UI now checks Node.js version (≥18) and git availability before install, with download URL hints and automatic polling for newly installed tools.
  • Unit tests added for the new Windows registry PATH resolution logic (10 test cases).
image

Why we need it and why it was done in this way

Why registry reads instead of cmd.exe /c set?

On Windows, cmd.exe /c set inherits the parent process env unchanged. Unlike Unix shells that source ~/.bashrc/.zshrc on launch, cmd.exe does not re-read the registry. When a user installs Node.js (via MSI, Scoop, etc.) after Cherry Studio is already running, the new PATH entries only exist in the registry — not in the Electron process's inherited env. Reading HKLM\...\Environment (system PATH) and HKCU\Environment (user PATH) directly gives us the ground-truth PATH at the time of the call.

Why execFileSync instead of crossPlatformSpawn/executeCommand?

  1. Circular dependency: executeCommand internally calls getShellEnv() to obtain env. Since queryRegValue is called by getShellEnvgetLoginShellEnvironmentgetWindowsEnvironment, using executeCommand would create an infinite recursion.
  2. Synchronous is appropriate: reg query completes in milliseconds. Keeping it synchronous allows getWindowsEnvironment() to return directly via Promise.resolve(), simplifying the control flow.
  3. No .cmd shim handling needed: reg.exe is a native executable — it doesn't need the cmd.exe /c wrapping that crossPlatformSpawn provides.
  4. Security: execFileSync executes the binary directly without shell interpolation, avoiding command injection risk.

Why expand %VAR% manually?

Windows registry stores PATH as REG_EXPAND_SZ with embedded references like %SystemRoot%\system32. The reg query output returns the raw string without expansion. We expand these references against the current process.env using case-insensitive lookup to match Windows behavior.

The following tradeoffs were made:

  • CQS over convenience: callers must now explicitly call refreshShellEnv() before findExecutableInEnv() when they need fresh env. This adds a line of code at call sites but makes the caching behavior predictable and eliminates hidden side effects.
  • Clone-on-modify over freeze: we spread-clone the env object in install() rather than Object.freeze() the cache, because freeze would break callers that legitimately need to add env vars (e.g., OPENCLAW_CONFIG_PATH).

The following alternatives were considered:

  • Making getShellEnv() always return a frozen copy — rejected because it would require all callers to spread, even those that only read.
  • Extracting a shared function for install/uninstall — rejected (Rule of Three: only 2 instances, with semantic differences in error handling and sudo retry).
  • Using PowerShell [Environment]::GetEnvironmentVariable instead of reg query — rejected because it has a much higher startup cost (~200ms vs ~5ms) and requires detecting PowerShell availability.

Links to places where the discussion took place: N/A

Breaking changes

None. All changes are internal to the main process. No Redux/IndexedDB schema changes.

Special notes for your reviewer

  • New file: src/main/utils/__tests__/shell-env.test.ts — 10 test cases covering registry PATH resolution (stale replacement, system+user combination, %VAR% expansion, REG_SZ vs REG_EXPAND_SZ, fallback behavior, cherry bin append, no cmd.exe spawn).
  • New helpers in shell-env.ts: queryRegValue(), expandWindowsEnvVars(), readWindowsRegistryPath(), getWindowsEnvironment() — all private, tested through the public refreshShellEnv() API.
  • Function renames: spawnWithEnvcrossPlatformSpawn, executeInEnvexecuteCommand — names now reflect actual responsibility (Windows .cmd adaptation, not "env injection").
  • checkNodeVersion returns a discriminated union { status: 'not_found' } | { status: 'version_low'; version; path } | { status: 'ok'; version; path } instead of the previous checkNpmAvailable boolean.
  • i18n keys renamed from openclaw.node_required.*openclaw.node_missing.* / openclaw.node_version_low.* with translations for all supported locales.

Checklist

Release note

fix(shell-env): on Windows, read PATH from registry instead of inheriting stale Electron process env; fix Node.js detection for nvm/mise/fnm-managed installations; add version check (≥18) and git availability check with download hints in OpenClaw setup UI

🤖 Generated with Claude Code

…ations

When Node.js is installed via version managers (nvm, mise, fnm) after the
app has started, OpenClaw fails to detect it because the cached shell
environment is stale. This commit refactors the shell environment and
process utilities to fix the issue:

- Separate shell env cache into CQS pair: getShellEnv (query) vs
  refreshShellEnv (command) to make cache invalidation explicit
- Remove hidden cache refresh side effect from findExecutableInEnv
- Fix startGateway to use refreshed env consistently
- Add checkNodeVersion with discriminated union return type
- Add node/git version polling and download URL hints in OpenClaw UI
- Unify spawn strategy: replace spawn()+shell:true with crossPlatformSpawn
- Fix cache mutation bug: clone env before PATH augmentation in install()
- Rename spawnWithEnv→crossPlatformSpawn, executeInEnv→executeCommand

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@DeJeune DeJeune requested a review from 0xfullex as a code owner February 12, 2026 18:15
Replace manual parseInt-based version parsing with semver.coerce + semver.lt
for more robust version comparison. semver is already a project dependency
used in AppUpdater.ts.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@DeJeune DeJeune marked this pull request as draft February 12, 2026 19:25
DeJeune and others added 4 commits February 13, 2026 03:38
…ess env

On Windows, `cmd.exe /c set` just inherits the parent Electron process's
env, so PATH changes (e.g. Node.js installed via MSI after app launch)
are never picked up. Replace the shell spawn with direct registry reads
of HKLM and HKCU PATH values, expanding %VAR% references in-place.

This fixes npm preinstall failures where `node` can't be found despite
being installed, because the captured PATH was stale.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover the key scenarios:
- stale PATH replaced by fresh registry value
- system + user PATH combined
- fallback when registry is unavailable
- %VAR% expansion (case-insensitive, unknown vars preserved)
- REG_SZ vs REG_EXPAND_SZ handling
- Cherry Studio bin appended
- no cmd.exe shell spawned on Windows

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Change findExecutable default extensions from ['.exe'] to ['.exe', '.cmd']
  since .cmd is the standard npm shim format on Windows
- Remove commonPaths from FindExecutableOptions — redundant now that
  getShellEnv() reads fresh PATH from the Windows registry
- Remove options parameter from findExecutableInEnv entirely — callers
  no longer need to pass extensions or commonPaths
- Simplify findNpmPath and findOpenClawBinary call sites

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Inline findNpmPath into its two call sites (install, uninstall)
- Remove possiblePaths filesystem fallback from findOpenClawBinary —
  openclaw is installed via npm global, so it's always in PATH
- findOpenClawBinary now delegates entirely to findExecutableInEnv

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@DeJeune DeJeune force-pushed the DeJeune/node22-nvm-fix branch from fe97be7 to 44cf95e Compare February 12, 2026 20:22
DeJeune and others added 5 commits February 13, 2026 04:26
The clone-and-augment pattern was needed when getShellEnv() returned
stale PATH. Now that Windows reads fresh PATH from the registry and
Unix reads from the login shell, the node/git directories are already
in PATH. No need to clone the env or manually prepend directories.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ormSpawn quoting

- Change findExecutableInEnv to return `string | null` instead of
  `{ path, env }` — the returned env was redundant since callers either
  ignored it or obtained it separately via getShellEnv()/refreshShellEnv(),
  and executeCommand already defaults to getShellEnv().
- Fix crossPlatformSpawn to use `shell: true` for .cmd files instead of
  manually constructing `cmd.exe /c` args. The manual approach breaks when
  both the command path and arguments contain spaces (cmd.exe's
  quote-stripping rule 2 mangles the command line).
- Remove env parameter threading from PluginService.cloneRepository and
  resolveDefaultBranch since executeCommand handles it internally.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract NodeCheckResult from OpenClawService.ts to
packages/shared/config/types.ts so it can be shared across the main
process, preload bridge, and renderer without duplication.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kangfenmao

This comment was marked as resolved.

@DeJeune DeJeune marked this pull request as ready for review February 14, 2026 02:41
…version requirements in multiple languages

- Translated "Export to Excel" and related success/error messages in various languages.
- Updated Node.js version requirement messages to provide clearer instructions in the respective languages.
@@ -243,7 +243,7 @@ function normalizeVersion(tag: string): string {
}

function detectChannel(version: string): UpgradeChannel | null {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个改动是否和本次提交有关,另外修改此处会影响应用升级吗

# Conflicts:
#	src/renderer/src/i18n/translate/de-de.json
#	src/renderer/src/i18n/translate/ro-ro.json
@kangfenmao kangfenmao merged commit 26e53c9 into main Feb 14, 2026
5 checks passed
@kangfenmao kangfenmao deleted the DeJeune/node22-nvm-fix branch February 14, 2026 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants