fix(windows): handle spaces in Node.js path for plugin install on Windows by manusjs · Pull Request #37592 · openclaw/openclaw

manusjs · 2026-03-06T09:59:12Z

What

Fix two bugs that break openclaw plugins install on Windows when Node.js is installed in a path containing spaces (e.g. the default C:\Program Files\nodejs\).

Fixes #37563

Bug 1 (primary) — space-containing resolved paths not wrapped in cmd.exe

resolveNpmArgvForWindows resolves npm pack to:

["C:\Program Files\nodejs\node.exe",
 "C:\Program Files\nodejs\node_modules\npm\bin\npm-cli.js",
 "pack", ...]

These were passed directly to spawn() without the cmd.exe wrapper, causing Node's CreateProcessW escaping to garble paths with spaces:

npm pack failed: ': \Progran' <garbled binary output>

Fix: After resolveNpmArgvForWindows succeeds, check if any resolved argv entry contains a space. If so, route through cmd.exe + buildCmdExeCommandLine (which uses escapeForCmdExe), same as the existing .cmd/.bat wrapper path. Applied in both runCommandWithTimeout and runExec.

Bug 2 (secondary) — EINVAL when npm-cli.js not found at standard path

When Node is installed via nvm-windows, volta, or fnm, npm-cli.js is not at nodeDir/node_modules/npm/bin/. resolveNpmArgvForWindows returns null, and resolveCommand('npm') returned bare 'npm' (npm was removed from cmdCommands in a prior fix), causing spawn EINVAL on Node 18.20.2+.

Fix: Add 'npm' and 'npx' back to the cmdCommands list in resolveCommand so they resolve to npm.cmd/npx.cmd as a fallback — identical to how pnpm/yarn are handled. Updated the comment to reflect this.

Testing

All 55 existing process tests pass. Platform-specific behavior (Windows spawn semantics) can only be fully verified on a Windows machine, but the logic paths are well-covered by the existing exec test suite.

…dows Two bugs when Node.js is installed in a path with spaces (e.g. the default C:\Program Files\nodejs\): Bug 1 (primary): resolveNpmArgvForWindows resolves npm/npx to [node.exe, npm-cli.js, ...] where both paths may contain spaces. These were passed directly to spawn() without cmd.exe wrapping, causing Node.js argument escaping to garble the command line. Fix: detect when any resolved argv entry contains a space and route through cmd.exe + buildCmdExeCommandLine (which uses escapeForCmdExe) in both runCommandWithTimeout and runExec. Bug 2 (secondary): when resolveNpmArgvForWindows returns null (Node installed via nvm-windows, volta, fnm — npm-cli.js not at the standard nodeDir path), resolveCommand('npm') returned bare 'npm' causing spawn EINVAL on Node 18.20.2+ (CVE-2024-27980 mitigation rejects .cmd without shell=true). Fix: add 'npm' and 'npx' to the cmdCommands list in resolveCommand so they resolve to npm.cmd/npx.cmd and get the cmd.exe wrapper path as a fallback, identical to how pnpm/yarn are handled. Fixes openclaw#37563

greptile-apps · 2026-03-06T10:02:19Z

Greptile Summary

This PR fixes two related Windows bugs that caused plugins install to fail when Node.js is installed under a path containing spaces (e.g. the default C:\Program Files\nodejs\). It correctly reuses the existing cmd.exe/buildCmdExeCommandLine infrastructure and the logic for both fixes is sound.

Bug 1 (space in resolved path): After resolveNpmArgvForWindows succeeds, the PR now checks whether any resolved argv entry contains a space and, if so, routes through the cmd.exe wrapper in both runExec and runCommandWithTimeout. The escaping via buildCmdExeCommandLine/escapeForCmdExe is consistent with how .cmd shims were already handled.
Bug 2 (cli script not found): npm and npx are re-added to cmdCommands in resolveCommand, so they fall back to npm.cmd/npx.cmd when resolveNpmArgvForWindows returns null (nvm-windows, volta, fnm). Since .cmd extensions are already gated through the cmd.exe wrapper via isWindowsBatchCommand, this does not re-introduce the Node 18.20.2+ EINVAL regression.
Minor inconsistency: wasNpmResolved is tracked with an explicit boolean in runExec but via reference-inequality (finalArgv !== argv) in runCommandWithTimeout; the two approaches are functionally equivalent but inconsistent.
No new tests: The new code paths (space-containing resolved paths and the .cmd fallback) are not covered by the updated test suite, even though they are fully mockable without a Windows machine.

Confidence Score: 4/5

Safe to merge — the fix is logically correct and the new code paths reuse well-tested infrastructure, with only minor style inconsistencies and missing test coverage.
Both fixes are targeted and use the existing cmd.exe/buildCmdExeCommandLine pattern correctly. The wasNpmResolved reference-inequality check in runCommandWithTimeout is functionally equivalent to the explicit boolean used in runExec. The one meaningful gap is the absence of unit tests for the new code paths, which could mask regressions in future refactors — but this doesn't affect correctness of the current change.
src/process/exec.ts — new space-detection and .cmd fallback branches lack unit-test coverage

_{Last reviewed commit: 0c2289f}

greptile-apps · 2026-03-06T10:02:23Z

src/process/exec.ts

  const finalArgv = process.platform === "win32" ? (resolveNpmArgvForWindows(argv) ?? argv) : argv;
-  const resolvedCommand = finalArgv !== argv ? (finalArgv[0] ?? "") : resolveCommand(argv[0] ?? "");
-  const useCmdWrapper = isWindowsBatchCommand(resolvedCommand);
+  const wasNpmResolved = process.platform === "win32" && finalArgv !== argv;


Inconsistent wasNpmResolved tracking across functions

runCommandWithTimeout detects whether npm was resolved via reference-inequality (finalArgv !== argv), relying on the fact that resolveNpmArgvForWindows returns a fresh array or null. runExec (line 121-127) uses an explicit wasNpmResolved = true boolean instead, which is clearer and more resilient to future changes in resolveNpmArgvForWindows.

Consider storing the result of resolveNpmArgvForWindows explicitly to keep the two functions consistent:

const resolvedArgv = process.platform === "win32" ? resolveNpmArgvForWindows(argv) : null; const wasNpmResolved = resolvedArgv !== null; const finalArgv = resolvedArgv ?? argv;

Prompt To Fix With AI

This is a comment left during a code review. Path: src/process/exec.ts Line: 232 Comment: **Inconsistent `wasNpmResolved` tracking across functions** `runCommandWithTimeout` detects whether npm was resolved via reference-inequality (`finalArgv !== argv`), relying on the fact that `resolveNpmArgvForWindows` returns a fresh array or `null`. `runExec` (line 121-127) uses an explicit `wasNpmResolved = true` boolean instead, which is clearer and more resilient to future changes in `resolveNpmArgvForWindows`. Consider storing the result of `resolveNpmArgvForWindows` explicitly to keep the two functions consistent: ``` const resolvedArgv = process.platform === "win32" ? resolveNpmArgvForWindows(argv) : null; const wasNpmResolved = resolvedArgv !== null; const finalArgv = resolvedArgv ?? argv; ``` How can I resolve this? If you propose a fix, please make it concise.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

greptile-apps · 2026-03-06T10:02:24Z

src/process/exec.ts

+    const useCmdWrapper =
+      isWindowsBatchCommand(execCommand) ||
+      (wasNpmResolved && [execCommand, ...execArgs].some((a) => a.includes(" ")));


No unit tests for the new space-in-path and .cmd fallback code paths

exec.windows.test.ts covers the pre-existing .cmd wrapping for pnpm/yarn, but neither of the two new branches introduced here are exercised:

wasNpmResolved && [execCommand, ...execArgs].some((a) => a.includes(" ")) in runExec

The equivalent branch in runCommandWithTimeout (line 236-238)

The npm/npx → .cmd fallback path via resolveCommand

Both paths are mockable without a real Windows machine: process.platform can be spied on, fs.existsSync can be mocked to return false (triggering the fallback) or true with a path that includes a space (triggering the wrapper). Given that the bugs were subtle enough to slip through previously, covering these paths would give future maintainers a safety net.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/process/exec.ts Line: 136-138 Comment: **No unit tests for the new space-in-path and `.cmd` fallback code paths** `exec.windows.test.ts` covers the pre-existing `.cmd` wrapping for `pnpm`/`yarn`, but neither of the two new branches introduced here are exercised: - `wasNpmResolved && [execCommand, ...execArgs].some((a) => a.includes(" "))` in `runExec` - The equivalent branch in `runCommandWithTimeout` (line 236-238) - The `npm`/`npx` → `.cmd` fallback path via `resolveCommand` Both paths are mockable without a real Windows machine: `process.platform` can be spied on, `fs.existsSync` can be mocked to return `false` (triggering the fallback) or `true` with a path that includes a space (triggering the wrapper). Given that the bugs were subtle enough to slip through previously, covering these paths would give future maintainers a safety net. How can I resolve this? If you propose a fix, please make it concise.

openclaw-barnacle bot added the size: XS label Mar 6, 2026

greptile-apps bot reviewed Mar 6, 2026

View reviewed changes

BradGroux mentioned this pull request Mar 17, 2026

WORKING: All Microsoft Issues and PRs #49126

Draft

Copilot AI mentioned this pull request Mar 27, 2026

List Azure and Windows open PRs available for review johnsonshi/openclaw#1

Closed

17 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(windows): handle spaces in Node.js path for plugin install on Windows#37592

fix(windows): handle spaces in Node.js path for plugin install on Windows#37592
manusjs wants to merge 1 commit intoopenclaw:mainfrom
manusjs:fix/plugin-install-windows-path-spaces

manusjs commented Mar 6, 2026

Uh oh!

greptile-apps bot commented Mar 6, 2026

Uh oh!

greptile-apps bot Mar 6, 2026

Uh oh!

greptile-apps bot Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

manusjs commented Mar 6, 2026

What

Bug 1 (primary) — space-containing resolved paths not wrapped in cmd.exe

Bug 2 (secondary) — EINVAL when npm-cli.js not found at standard path

Testing

Uh oh!

greptile-apps bot commented Mar 6, 2026

Greptile Summary

Confidence Score: 4/5

Uh oh!

greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant