From fad23944e72bea389d2e058a4ab68eab022eac8f Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 30 Mar 2026 16:44:20 -0400 Subject: [PATCH] fix: add Windows shell guard to remaining spawn sites (#3058) Three spawn call sites were missing `shell: process.platform === "win32"`, causing ENOENT/EINVAL errors on Windows where npm-installed tools are .cmd batch scripts that require shell resolution: - exec.ts: hardcoded `shell: false` -> platform-guarded - lsp/index.ts: missing shell option on project-type command spawn - lsp/lspmux.ts: missing shell option on lspmux binary spawn Adds a structural regression test that scans all spawn sites invoking user-facing binaries and asserts the Windows shell guard is present. Closes #2854 Co-authored-by: Claude Opus 4.6 --- packages/pi-coding-agent/src/core/exec.ts | 4 +- .../pi-coding-agent/src/core/lsp/index.ts | 3 + .../pi-coding-agent/src/core/lsp/lspmux.ts | 3 + .../core/tools/spawn-shell-windows.test.ts | 92 +++++++++++++++++++ 4 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 packages/pi-coding-agent/src/core/tools/spawn-shell-windows.test.ts diff --git a/packages/pi-coding-agent/src/core/exec.ts b/packages/pi-coding-agent/src/core/exec.ts index b7dd046c4..9d12e8c23 100644 --- a/packages/pi-coding-agent/src/core/exec.ts +++ b/packages/pi-coding-agent/src/core/exec.ts @@ -39,7 +39,9 @@ export async function execCommand( return new Promise((resolve) => { const proc = spawn(command, args, { cwd, - shell: false, + // On Windows, npm/npx/tsc etc. are .cmd scripts that require shell + // resolution. Without this, spawn fails with ENOENT or EINVAL (#2854). + shell: process.platform === "win32", stdio: ["ignore", "pipe", "pipe"], }); diff --git a/packages/pi-coding-agent/src/core/lsp/index.ts b/packages/pi-coding-agent/src/core/lsp/index.ts index 61237e7eb..bd2718634 100644 --- a/packages/pi-coding-agent/src/core/lsp/index.ts +++ b/packages/pi-coding-agent/src/core/lsp/index.ts @@ -340,6 +340,9 @@ async function runWorkspaceDiagnostics( const proc = spawn(cmd, cmdArgs, { cwd, stdio: ["ignore", "pipe", "pipe"], + // On Windows, project-type commands (tsc, cargo, etc.) may be .cmd + // wrappers that need shell resolution to avoid ENOENT/EINVAL (#2854). + shell: process.platform === "win32", }); const abortHandler = () => { proc.kill(); diff --git a/packages/pi-coding-agent/src/core/lsp/lspmux.ts b/packages/pi-coding-agent/src/core/lsp/lspmux.ts index 05ef13b38..6e01d7807 100644 --- a/packages/pi-coding-agent/src/core/lsp/lspmux.ts +++ b/packages/pi-coding-agent/src/core/lsp/lspmux.ts @@ -90,6 +90,9 @@ async function checkServerRunning(binaryPath: string): Promise { try { const proc = spawn(binaryPath, ["status"], { stdio: ["ignore", "pipe", "pipe"], + // On Windows, the binary may be a .cmd wrapper requiring shell + // resolution to avoid ENOENT/EINVAL (#2854). + shell: process.platform === "win32", }); const exited = await Promise.race([ diff --git a/packages/pi-coding-agent/src/core/tools/spawn-shell-windows.test.ts b/packages/pi-coding-agent/src/core/tools/spawn-shell-windows.test.ts new file mode 100644 index 000000000..a7929a1dd --- /dev/null +++ b/packages/pi-coding-agent/src/core/tools/spawn-shell-windows.test.ts @@ -0,0 +1,92 @@ +/** + * spawn-shell-windows.test.ts — Regression test for Windows spawn ENOENT/EINVAL. + * + * On Windows, npm/npx/tsc and other tools are installed as .cmd batch scripts. + * Node's `spawn()` without `shell: true` cannot execute .cmd files, resulting + * in ENOENT or EINVAL errors. Every spawn site that may invoke a user-installed + * binary (not `node` or a shell like `sh`/`bash`/`cmd`) must include + * `shell: process.platform === "win32"` so the call is resolved through cmd.exe + * on Windows while remaining a direct exec on POSIX. + * + * This test structurally scans all spawn sites and verifies the guard is present. + * + * Fixes: gsd-build/gsd-2#2854 + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join, dirname, relative } from "node:path"; +import { fileURLToPath } from "node:url"; + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const coreDir = join(__dirname, ".."); + +/** + * Files that call `spawn()` with a user-facing binary (not `node`, `sh`, `bash`, + * or `cmd`) and therefore need the Windows shell guard. + * + * If a file spawns only hardcoded system binaries (like `node` in rpc-client.ts), + * it does not need the guard and should NOT appear here. + */ +const SPAWN_FILES_NEEDING_SHELL_GUARD = [ + // Extension's GSD client — spawns the `gsd` binary which is a .cmd on Windows + join(coreDir, "..", "..", "..", "vscode-extension", "src", "gsd-client.ts"), + // exec.ts — used by extensions to run arbitrary commands + join(coreDir, "exec.ts"), + // LSP index — spawns project-type commands (tsc, cargo, etc.) + join(coreDir, "lsp", "index.ts"), + // LSP client — spawns LSP server binaries (npx, etc.) + join(coreDir, "lsp", "client.ts"), + // LSP mux — spawns lspmux binary + join(coreDir, "lsp", "lspmux.ts"), + // Package manager — spawns npm/yarn/pnpm + join(coreDir, "package-manager.ts"), +]; + +test("all spawn sites that invoke user-facing binaries include shell: process.platform === 'win32'", () => { + const failures: string[] = []; + + for (const file of SPAWN_FILES_NEEDING_SHELL_GUARD) { + let content: string; + try { + content = readFileSync(file, "utf-8"); + } catch { + // File may not exist in this checkout — skip + continue; + } + + const lines = content.split("\n"); + + // Find all spawn(..., { ... }) call sites and check each one + // for the presence of `shell: process.platform === "win32"` within + // 5 lines after the spawn call. + for (let i = 0; i < lines.length; i++) { + const line = lines[i]!; + // Skip comments + if (line.trim().startsWith("//") || line.trim().startsWith("*")) continue; + + // Detect a spawn() call + if (/\bspawn\(/.test(line)) { + // Look ahead up to 8 lines for the shell guard + const lookahead = lines.slice(i, i + 8).join("\n"); + const hasShellGuard = + /shell:\s*process\.platform\s*===\s*["']win32["']/.test(lookahead); + + if (!hasShellGuard) { + const relPath = relative(join(coreDir, "..", ".."), file); + failures.push(`${relPath}:${i + 1}`); + } + } + } + } + + assert.deepEqual( + failures, + [], + `The following spawn sites are missing 'shell: process.platform === "win32"':\n` + + failures.map(f => ` - ${f}`).join("\n") + + `\nOn Windows, .cmd wrapper scripts (npm, npx, tsc, gsd) require shell ` + + `resolution. Without this guard, spawn fails with ENOENT or EINVAL.`, + ); +});