Skip to content

Commit 26e53c9

Browse files
DeJeuneclaudekangfenmao
authored
fix(openclaw): fix Node.js detection for nvm/mise/fnm-managed installations (#12902)
### 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). <img width="1019" height="765" alt="image" src="https://github.com/user-attachments/assets/5f6a4d27-6d3e-4033-8309-0c98bf8cba4c" /> ### 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* `getShellEnv` → `getLoginShellEnvironment` → `getWindowsEnvironment`, 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: `spawnWithEnv` → `crossPlatformSpawn`, `executeInEnv` → `executeCommand` — 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 - [x] PR: The PR description is expressive enough and will help future contributors - [x] Code: [Write code that humans can understand](https://en.wikiquote.org/wiki/Martin_Fowler#code-for-humans) and [Keep it simple](https://en.wikipedia.org/wiki/KISS_principle) - [x] Refactor: You have [left the code cleaner than you found it (Boy Scout Rule)](https://learning.oreilly.com/library/view/97-things-every/9780096809515/ch08.html) - [x] Upgrade: Impact of this change on upgrade flows was considered and addressed if required - [ ] Documentation: A [user-guide update](https://docs.cherry-ai.com) was considered and is present (link) or not required. You want a user-guide update if it's a user facing feature. ### Release note ```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](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: dev <dev@cherry-ai.com> Co-authored-by: kangfenmao <kangfenmao@qq.com>
1 parent 1035b3f commit 26e53c9

File tree

25 files changed

+837
-348
lines changed

25 files changed

+837
-348
lines changed

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@
231231
"@types/react-infinite-scroll-component": "^5.0.0",
232232
"@types/react-transition-group": "^4.4.12",
233233
"@types/react-window": "^1",
234+
"@types/semver": "^7.7.1",
234235
"@types/swagger-jsdoc": "^6",
235236
"@types/swagger-ui-express": "^4.1.8",
236237
"@types/tinycolor2": "^1",

packages/shared/IpcChannel.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ export enum IpcChannel {
410410

411411
// OpenClaw
412412
OpenClaw_CheckInstalled = 'openclaw:check-installed',
413-
OpenClaw_CheckNpmAvailable = 'openclaw:check-npm-available',
413+
OpenClaw_CheckNodeVersion = 'openclaw:check-node-version',
414414
OpenClaw_CheckGitAvailable = 'openclaw:check-git-available',
415415
OpenClaw_GetNodeDownloadUrl = 'openclaw:get-node-download-url',
416416
OpenClaw_GetGitDownloadUrl = 'openclaw:get-git-download-url',

packages/shared/config/types.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
import type { ProcessingStatus } from '@types'
22

3+
// =============================================================================
4+
// OpenClaw IPC Types
5+
// =============================================================================
6+
7+
export type NodeCheckResult =
8+
| { status: 'not_found' }
9+
| { status: 'version_low'; version: string; path: string }
10+
| { status: 'ok'; version: string; path: string }
11+
312
export type LoaderReturn = {
413
entriesAdded: number
514
uniqueId: string

0 commit comments

Comments
 (0)