morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[GH-ISSUE #526] Windows: \ ea doctor\ and \ ea setup\ crash with \spawn EINVAL\ (probe of npm .CMD shims) #271

New issue

Closed

opened 2026-10-15 18:23:16 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-15 18:23:16 +00:00

Owner

Originally created by @whft on GitHub (Oct 5, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/526

Summary

rea doctor and rea setup terminate immediately on Windows x64 with an opaque error, before any prompt or diagnostic output:

$ npx --yes rea-agents@latest setup --dry-run --json
{"code":"UNKNOWN","message":"spawn EINVAL"}

$ npx --yes rea-agents@latest doctor --json
{"code":"UNKNOWN","message":"spawn EINVAL"}

setup is completely unusable on Windows, so no MCP registration or bundled skill can be installed. doctor is documented as strictly read-only and is the recommended diagnostic, so it is unusable too.

Environment


REA	3.2.1 (latest)
OS	Windows 11 x64
Node	v24.19.0

Not specific to Node 24 — the same restriction exists on Node 22.19+.

Root cause

src/application/RuntimeExecutableDiagnostics.ts → executeVersion() spawns every discovered runtime candidate directly:

const child = spawn(path, ["--version"], {
  windowsHide: true,
  env: environmentWithPath(effectivePath),
  stdio: ["ignore", "pipe", "pipe"],
});

inspectRuntimeExecutables() probes node, npm, and npx (TOOL_NAMES). On Windows those resolve to the npm shims npm.CMD / npx.CMD, and since the CVE-2024-27980 hardening Node refuses to execute .cmd/.bat without a shell:

$ node -e "const {spawn}=require('child_process'); try { spawn(process.argv[1],['--version'],{windowsHide:true}) } catch(e) { console.log('SYNC THROW', e.code) }" "C:\Program Files\nodejs\npx.CMD"
SYNC THROW EINVAL

Two things make this fatal rather than cosmetic:

The throw is synchronous, raised inside the new Promise executor. It rejects the promise instead of reaching the child.once("error") handler further down, so the existing runtime_spawn_failed classification is unreachable for this case.
It propagates out of the diagnostic. executeVersion → probeCandidate → inspectRuntimeExecutables → runDoctor, so a single unprobeable candidate aborts the entire command rather than degrading to one failed entry.

Confirmed stack:

Error: spawn EINVAL
    at ChildProcess.spawn (node:internal/child_process:458:11)
    at spawn (node:child_process:813:9)
    at RuntimeExecutableDiagnostics.js:79:19
    at executeVersion (RuntimeExecutableDiagnostics.js:78:60)
    at probeCandidate (RuntimeExecutableDiagnostics.js:61:26)
    at Promise.all (index 1)
    at mapConcurrentBounded (RuntimeExecutableDiagnostics.js:196:5)
    at inspectRuntimeExecutables (RuntimeExecutableDiagnostics.js:37:25)
  errno: -4071, code: 'EINVAL', syscall: 'spawn'

Note for whoever fixes it: `shell: true` alone is not sufficient

The obvious fix gets past the crash but still produces a wrong answer, because Node concatenates the command line without quoting:

$ node -e "... spawn('C:\\Program Files\\nodejs\\npx.CMD',['--version'],{shell:true}) ..."
exit=1  stderr="'C:\Program' is not recognized as an internal or external command"

Any npm install location with a space in the path (C:\Program Files\...) triggers this. It needs either an explicit cmd.exe invocation with the double-quoted command line, or a pre-quoted file with shell: true.

Measured results:

invocation	result
`spawn(npx.CMD, …)`	EINVAL, synchronous throw
`spawn(npx.CMD, …, {shell:true})`	exit 1 — `'C:\Program' is not recognized`
`spawn(npx.CMD, …, {shell:true})`, file pre-quoted	exit 0, `11.17.0`
`spawn(comspec, ['/d','/s','/c','""<path>" --version'], {windowsVerbatimArguments:true})`	exit 0, `11.17.0`

Proposed fix

/** Node refuses to execute a `.cmd`/`.bat` shim without a shell (EINVAL). */
const isWindowsBatchShim = (path: string): boolean =>
  process.platform === "win32" && /\.(?:cmd|bat)$/i.test(path);

const executeVersion = (
  path: string,
  timeoutMs: number | undefined,
  effectivePath: string,
): Promise<VersionResult> =>
  new Promise((resolve) => {
    const windowsBatch = isWindowsBatchShim(path);
    let child: ChildProcess;
    try {
      child = spawn(
        windowsBatch ? (process.env.ComSpec ?? "cmd.exe") : path,
        windowsBatch
          ? ["/d", "/s", "/c", `""${path}" --version"`]
          : ["--version"],
        {
          windowsHide: true,
          env: environmentWithPath(effectivePath),
          stdio: ["ignore", "pipe", "pipe"],
          // cmd.exe must receive the double-quoted command line verbatim so a
          // shim path containing spaces is not split into a bogus short path.
          ...(windowsBatch ? { windowsVerbatimArguments: true } : {}),
        },
      );
    } catch (cause) {
      // spawn() reports some rejections synchronously; one unprobeable
      // candidate must never abort the whole diagnostic.
      resolve({ ok: false, failure: synchronousProbeFailure(cause) });
      return;
    }
    // …unchanged

The try/catch is the more important half: it restores the intended fail-closed behavior so that no future spawn() rejection can take down the whole report.

Verification

Same harness, upstream 3.2.1 vs. patched:

case	upstream	patched
`npx.CMD` in a directory containing a space	process dies, EINVAL	`npx=OK:1.2.3`
failing `npm.CMD`	process dies, EINVAL	`npm=FAIL:runtime_nonzero_exit`
nonexistent executable	process dies, EINVAL	`node=FAIL:runtime_spawn_failed`

Live doctor --json after the fix:

"runtime_executables": {
  "launcher_node": "C:\\Program Files\\nodejs\\node.exe",
  "candidates": [
    { "tool": "npm", "lexical_path": "C:\\Program Files\\nodejs\\npm.CMD",
      "version": "11.17.0", "healthy": true, "failure": null },
    { "tool": "npx", "lexical_path": "C:\\Program Files\\nodejs\\npx.CMD",
      "version": "11.17.0", "healthy": true, "failure": null }
  ]
}

tsc --noEmit --strict is clean. A regression test covering the three cases above is included in the PR.

Why this shipped

inspectRuntimeExecutables has no test coverage in the repo — no test file references it. Adding the three cases should be part of the fix.

Impact

Windows users cannot run rea setup at all, so the documented MCP + skill integration path is unavailable.
Windows users cannot run rea doctor, including the documented --json mode that distinguishes unsupported hosts, missing dependencies, and config drift.
The failure is reported as UNKNOWN / spawn EINVAL with no hint that a Windows shim is involved, so it is hard to self-diagnose.

Originally created by @whft on GitHub (Oct 5, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/526 ## Summary `rea doctor` and `rea setup` terminate immediately on Windows x64 with an opaque error, before any prompt or diagnostic output: ``` $ npx --yes rea-agents@latest setup --dry-run --json {"code":"UNKNOWN","message":"spawn EINVAL"} $ npx --yes rea-agents@latest doctor --json {"code":"UNKNOWN","message":"spawn EINVAL"} ``` `setup` is completely unusable on Windows, so no MCP registration or bundled skill can be installed. `doctor` is documented as strictly read-only and is the recommended diagnostic, so it is unusable too. ## Environment | | | |---|---| | REA | 3.2.1 (latest) | | OS | Windows 11 x64 | | Node | v24.19.0 | Not specific to Node 24 — the same restriction exists on Node 22.19+. ## Root cause `src/application/RuntimeExecutableDiagnostics.ts` → `executeVersion()` spawns every discovered runtime candidate directly: ```ts const child = spawn(path, ["--version"], { windowsHide: true, env: environmentWithPath(effectivePath), stdio: ["ignore", "pipe", "pipe"], }); ``` `inspectRuntimeExecutables()` probes `node`, `npm`, and `npx` (`TOOL_NAMES`). On Windows those resolve to the npm shims `npm.CMD` / `npx.CMD`, and since the CVE-2024-27980 hardening Node refuses to execute `.cmd`/`.bat` without a shell: ``` $ node -e "const {spawn}=require('child_process'); try { spawn(process.argv[1],['--version'],{windowsHide:true}) } catch(e) { console.log('SYNC THROW', e.code) }" "C:\Program Files\nodejs\npx.CMD" SYNC THROW EINVAL ``` Two things make this fatal rather than cosmetic: 1. **The throw is synchronous**, raised inside the `new Promise` executor. It rejects the promise instead of reaching the `child.once("error")` handler further down, so the existing `runtime_spawn_failed` classification is unreachable for this case. 2. **It propagates out of the diagnostic.** `executeVersion` → `probeCandidate` → `inspectRuntimeExecutables` → `runDoctor`, so a single unprobeable candidate aborts the entire command rather than degrading to one failed entry. Confirmed stack: ``` Error: spawn EINVAL at ChildProcess.spawn (node:internal/child_process:458:11) at spawn (node:child_process:813:9) at RuntimeExecutableDiagnostics.js:79:19 at executeVersion (RuntimeExecutableDiagnostics.js:78:60) at probeCandidate (RuntimeExecutableDiagnostics.js:61:26) at Promise.all (index 1) at mapConcurrentBounded (RuntimeExecutableDiagnostics.js:196:5) at inspectRuntimeExecutables (RuntimeExecutableDiagnostics.js:37:25) errno: -4071, code: 'EINVAL', syscall: 'spawn' ``` ## Note for whoever fixes it: `shell: true` alone is not sufficient The obvious fix gets past the crash but still produces a wrong answer, because Node concatenates the command line without quoting: ``` $ node -e "... spawn('C:\\Program Files\\nodejs\\npx.CMD',['--version'],{shell:true}) ..." exit=1 stderr="'C:\Program' is not recognized as an internal or external command" ``` Any npm install location with a space in the path (`C:\Program Files\...`) triggers this. It needs either an explicit `cmd.exe` invocation with the double-quoted command line, or a pre-quoted `file` with `shell: true`. Measured results: | invocation | result | |---|---| | `spawn(npx.CMD, …)` | **EINVAL, synchronous throw** | | `spawn(npx.CMD, …, {shell:true})` | exit 1 — `'C:\Program' is not recognized` | | `spawn(npx.CMD, …, {shell:true})`, file pre-quoted | exit 0, `11.17.0` | | `spawn(comspec, ['/d','/s','/c','""<path>" --version'], {windowsVerbatimArguments:true})` | exit 0, `11.17.0` | ## Proposed fix ```ts /** Node refuses to execute a `.cmd`/`.bat` shim without a shell (EINVAL). */ const isWindowsBatchShim = (path: string): boolean => process.platform === "win32" && /\.(?:cmd|bat)$/i.test(path); const executeVersion = ( path: string, timeoutMs: number | undefined, effectivePath: string, ): Promise<VersionResult> => new Promise((resolve) => { const windowsBatch = isWindowsBatchShim(path); let child: ChildProcess; try { child = spawn( windowsBatch ? (process.env.ComSpec ?? "cmd.exe") : path, windowsBatch ? ["/d", "/s", "/c", `""${path}" --version"`] : ["--version"], { windowsHide: true, env: environmentWithPath(effectivePath), stdio: ["ignore", "pipe", "pipe"], // cmd.exe must receive the double-quoted command line verbatim so a // shim path containing spaces is not split into a bogus short path. ...(windowsBatch ? { windowsVerbatimArguments: true } : {}), }, ); } catch (cause) { // spawn() reports some rejections synchronously; one unprobeable // candidate must never abort the whole diagnostic. resolve({ ok: false, failure: synchronousProbeFailure(cause) }); return; } // …unchanged ``` The `try`/`catch` is the more important half: it restores the intended fail-closed behavior so that no future `spawn()` rejection can take down the whole report. ## Verification Same harness, upstream 3.2.1 vs. patched: | case | upstream | patched | |---|---|---| | `npx.CMD` in a directory containing a space | **process dies, EINVAL** | `npx=OK:1.2.3` | | failing `npm.CMD` | **process dies, EINVAL** | `npm=FAIL:runtime_nonzero_exit` | | nonexistent executable | **process dies, EINVAL** | `node=FAIL:runtime_spawn_failed` | Live `doctor --json` after the fix: ```json "runtime_executables": { "launcher_node": "C:\\Program Files\\nodejs\\node.exe", "candidates": [ { "tool": "npm", "lexical_path": "C:\\Program Files\\nodejs\\npm.CMD", "version": "11.17.0", "healthy": true, "failure": null }, { "tool": "npx", "lexical_path": "C:\\Program Files\\nodejs\\npx.CMD", "version": "11.17.0", "healthy": true, "failure": null } ] } ``` `tsc --noEmit --strict` is clean. A regression test covering the three cases above is included in the PR. ## Why this shipped `inspectRuntimeExecutables` has no test coverage in the repo — no test file references it. Adding the three cases should be part of the fix. ## Impact - Windows users cannot run `rea setup` at all, so the documented MCP + skill integration path is unavailable. - Windows users cannot run `rea doctor`, including the documented `--json` mode that distinguishes unsupported hosts, missing dependencies, and config drift. - The failure is reported as `UNKNOWN` / `spawn EINVAL` with no hint that a Windows shim is involved, so it is hard to self-diagnose.

bsakaguchi

2026-10-15 18:23:16 +00:00

closed this issue
added the
area: setup

bug
labels

bsakaguchi commented

2026-10-15 18:23:18 +00:00

Author

Owner

@morluto commented on GitHub (Oct 5, 2026):

The spawn EINVAL half of this report is resolved by #505 (same fix as #504): batch-file probes now go through cmd.exe, so rea doctor and rea setup no longer abort on the npm.cmd/npx.cmd shims.

The other blocker you correctly identified separately — npm silently skipping the @lydell/node-pty-win32-x64 optional dependency, which makes the PTY backend look unsupported — is not fixed, because it is a packaging/diagnostics problem rather than the spawn failure. It is now tracked as #568, which covers classifying the actual cause (missing native module vs ABI mismatch vs genuine probe failure) and surfacing it through rea doctor.

Closing this one as the crash is fixed; #568 carries the remainder.

@morluto commented on GitHub (Oct 5, 2026): The `spawn EINVAL` half of this report is resolved by #505 (same fix as #504): batch-file probes now go through `cmd.exe`, so `rea doctor` and `rea setup` no longer abort on the `npm.cmd`/`npx.cmd` shims. The other blocker you correctly identified separately — npm silently skipping the `@lydell/node-pty-win32-x64` optional dependency, which makes the PTY backend look unsupported — is **not** fixed, because it is a packaging/diagnostics problem rather than the spawn failure. It is now tracked as #568, which covers classifying the actual cause (missing native module vs ABI mismatch vs genuine probe failure) and surfacing it through `rea doctor`. Closing this one as the crash is fixed; #568 carries the remainder.

bsakaguchi referenced this issue

2026-10-15 18:28:29 +00:00

[GH-ISSUE #2383] [Tracker] Audit analysis and debugging coverage for the TIOBE top 100 languages (October 2026) #794

bsakaguchi referenced this issue

2026-10-16 21:12:26 +00:00

[PR #271] [MERGED] test(dotnet): add managed conformance verifier #1912