Commit Graph

2 Commits

Author SHA1 Message Date
SuYao
26e53c9a4b 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>
2026-02-14 13:13:20 +08:00
beyondkmp
038d30831c ♻️ refactor: implement config-based update system with version compatibility control (#11147)
* ♻️ refactor: implement config-based update system with version compatibility control

Replace GitHub API-based update discovery with JSON config file system. Support
version gating (users below v1.7 must upgrade to v1.7.0 before v2.0). Auto-select
GitHub/GitCode config source based on IP location. Simplify fallback logic.

Changes:
- Add update-config.json with version compatibility rules
- Implement _fetchUpdateConfig() and _findCompatibleChannel()
- Remove legacy _getReleaseVersionFromGithub() and GitHub API dependency
- Refactor _setFeedUrl() with simplified fallback to default feed URLs
- Add design documentation in docs/UPDATE_CONFIG_DESIGN.md

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* fix(i18n): Auto update translations for PR #11147

* format code

* 🔧 chore: update config for v1.7.5 → v2.0.0 → v2.1.6 upgrade path

Update version configuration to support multi-step upgrade path:
- v1.6.x users → v1.7.5 (last v1.x release)
- v1.7.x users → v2.0.0 (v2.x intermediate version)
- v2.0.0+ users → v2.1.6 (current latest)

Changes:
- Update 1.7.0 → 1.7.5 with fixed feedUrl
- Set 2.0.0 as intermediate version with fixed feedUrl
- Add 2.1.6 as current latest pointing to releases/latest

This ensures users upgrade through required intermediate versions
before jumping to major releases.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* 🔧 chore: refactor update config with constants and adjust versions

Refactor update configuration system and adjust to actual versions:

- Add UpdateConfigUrl enum in constant.ts for centralized config URLs
- Point to test server (birdcat.top) for development testing
- Update AppUpdater.ts to use UpdateConfigUrl constants
- Adjust update-config.json to actual v1.6.7 with rc/beta channels
- Remove v2.1.6 entry (not yet released)
- Set package version to 1.6.5 for testing upgrade path
- Add update-config.example.json for reference

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* update version

*  test: add comprehensive unit tests for AppUpdater config system

Add extensive test coverage for new config-based update system including:
- Config fetching with IP-based source selection (GitHub/GitCode)
- Channel compatibility matching with version constraints
- Smart fallback from rc/beta to latest when appropriate
- Multi-step upgrade path validation (1.6.3 → 1.6.7 → 2.0.0)
- Error handling for network and HTTP failures

Test Coverage:
- _fetchUpdateConfig: 4 tests (GitHub/GitCode selection, error handling)
- _findCompatibleChannel: 9 tests (channel matching, version comparison)
- Upgrade Path: 3 tests (version gating scenarios)
- Total: 30 tests, 100% passing

Also optimize _findCompatibleChannel logic with better variable naming
and log messages.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

*  test: add complete multi-step upgrade path tests (1.6.3 → 1.7.5 → 2.0.0 → 2.1.6)

Add comprehensive test suite for complete upgrade journey including:
- Individual step validation (1.6.3→1.7.5, 1.7.5→2.0.0, 2.0.0→2.1.6)
- Full multi-step upgrade simulation with version progression
- Version gating enforcement (block skipping intermediate versions)
- Verification that 1.6.3 cannot directly upgrade to 2.0.0 or 2.1.6
- Verification that 1.7.5 cannot skip 2.0.0 to reach 2.1.6

Test Coverage:
- 6 new tests for complete upgrade path scenarios
- Total: 36 tests, 100% passing

This ensures the version compatibility system correctly enforces
intermediate version upgrades for major releases.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* 📝 docs: reorganize update config documentation with English translation

Move update configuration design document to docs/technical/ directory
and add English translation for international contributors.

Changes:
- Move docs/UPDATE_CONFIG_DESIGN.md → docs/technical/app-update-config-zh.md
- Add docs/technical/app-update-config-en.md (English translation)
- Organize technical documentation in dedicated directory

Documentation covers:
- Config-based update system design and rationale
- JSON schema with version compatibility control
- Multi-step upgrade path examples (1.6.3 → 1.7.5 → 2.0.0 → 2.1.6)
- TypeScript type definitions and matching algorithms
- GitHub/GitCode source selection for different regions

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* format code

*  test: add tests for latest channel self-comparison prevention

Add tests to verify the optimization that prevents comparing latest
channel with itself when latest is requested, and ensures rc/beta
channels are returned when they are newer than latest.

New tests:
- should not compare latest with itself when requesting latest channel
- should return rc when rc version > latest version
- should return beta when beta version > latest version

These tests ensure the requestedChannel !== UpgradeChannel.LATEST
check works correctly and users get the right channel based on
version comparisons.

Test Coverage: 39 tests, 100% passing

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* update github/gitcode

* format code

* update rc version

* ♻️ refactor: merge update configs into single multi-mirror file

- Merge app-upgrade-config-github.json and app-upgrade-config-gitcode.json into single app-upgrade-config.json
- Add UpdateMirror enum for type-safe mirror selection
- Optimize _fetchUpdateConfig to receive mirror parameter, eliminating duplicate IP country checks
- Update ChannelConfig interface to use Record<UpdateMirror, string> for feedUrls
- Rename documentation files from app-update-config-* to app-upgrade-config-*
- Update docs with new multi-mirror configuration structure

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

*  test: update AppUpdater tests for multi-mirror configuration

- Add UpdateMirror enum import
- Update _fetchUpdateConfig tests to accept mirror parameter
- Convert all feedUrl to feedUrls structure in test mocks
- Update test expectations to match new ChannelConfig interface
- All 39 tests passing

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* format code

* delete files

* 📝 docs: add UpdateMirror enum to type definitions

- Add UpdateMirror enum definition in both EN and ZH docs
- Update ChannelConfig to use Record<UpdateMirror, string>
- Add comments showing equivalent structure for clarity

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* 🐛 fix: return actual channel from _findCompatibleChannel

Fix channel mismatch issue where requesting rc/beta but getting latest:
- Change _findCompatibleChannel return type to include actual channel
- Return { config, channel } instead of just config
- Update _setFeedUrl to use actualChannel instead of requestedChannel
- Update all test expectations to match new return structure
- Add channel assertions to key tests

This ensures autoUpdater.channel matches the actual feed URL being used.

Fixes issue where:
- User requests 'rc' channel
- latest >= rc, so latest config is returned
- But channel was set to 'rc' with latest URL 
- Now channel is correctly set to 'latest' 

All 39 tests passing 

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

* update version

* udpate version

* update config

* add no cache header

* update files

* 🤖 chore: automate app upgrade config updates

* format code

* update workflow

* update get method

* docs: document upgrade workflow automation

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: GitHub Action <action@github.com>
2025-11-14 17:49:40 +08:00