fix(coding-agent): spawn Windows npm shims directly

closes #4623

packages/coding-agent/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- Fixed npm-family package commands on Windows to avoid shell argument splitting when install prefixes contain spaces ([#4623](https://github.com/earendil-works/pi/issues/4623)).
+
 ## [0.75.0] - 2026-05-17
 
 ### Breaking Changes

packages/coding-agent/src/config.ts

@@ -3,7 +3,7 @@ import { accessSync, constants, existsSync, readFileSync, realpathSync } from "f
 import { homedir } from "os";
 import { basename, dirname, join, resolve, sep, win32 } from "path";
 import { fileURLToPath } from "url";
-import { shouldUseWindowsShell } from "./utils/child-process.js";
+import { resolveSpawnCommand } from "./utils/child-process.js";
 
 // =============================================================================
 // Package Detection
@@ -151,10 +151,18 @@ function readCommandOutput(
 	args: string[],
 	options: { requireSuccess?: boolean } = {},
 ): string | undefined {
-	const result = spawnSync(command, args, {
+	let resolved: ReturnType<typeof resolveSpawnCommand>;
+	try {
+		resolved = resolveSpawnCommand(command, args);
+	} catch (error) {
+		if (options.requireSuccess) {
+			throw error;
+		}
+		return undefined;
+	}
+	const result = spawnSync(resolved.command, resolved.args, {
 		encoding: "utf-8",
 		stdio: ["ignore", "pipe", "pipe"],
-		shell: shouldUseWindowsShell(command),
 	});
 	if (result.status === 0) return result.stdout.trim() || undefined;
 	if (options.requireSuccess) {

packages/coding-agent/src/core/package-manager.ts

@@ -28,7 +28,7 @@ import { globSync } from "glob";
 import ignore from "ignore";
 import { minimatch } from "minimatch";
 import { CONFIG_DIR_NAME } from "../config.js";
-import { shouldUseWindowsShell } from "../utils/child-process.js";
+import { resolveSpawnCommand } from "../utils/child-process.js";
 import { type GitSource, parseGitUrl } from "../utils/git.js";
 import { canonicalizePath, isLocalPath } from "../utils/paths.js";
 import { isStdoutTakenOver } from "./output-guard.js";
@@ -2408,11 +2408,12 @@ export class DefaultPackageManager implements PackageManager {
 	}
 
 	private spawnCommand(command: string, args: string[], options?: { cwd?: string }): ChildProcess {
-		return spawn(command, args, {
+		const env = getEnv();
+		const resolved = resolveSpawnCommand(command, args, { env });
+		return spawn(resolved.command, resolved.args, {
 			cwd: options?.cwd,
 			stdio: isStdoutTakenOver() ? ["ignore", 2, 2] : "inherit",
-			shell: shouldUseWindowsShell(command),
-			env: getEnv(),
+			env,
 		});
 	}
 
@@ -2422,11 +2423,12 @@ export class DefaultPackageManager implements PackageManager {
 		options?: { cwd?: string; env?: Record<string, string> },
 	): ChildProcessByStdio<null, Readable, Readable> {
 		const baseEnv = getEnv();
-		return spawn(command, args, {
+		const env = options?.env ? { ...baseEnv, ...options.env } : baseEnv;
+		const resolved = resolveSpawnCommand(command, args, { env });
+		return spawn(resolved.command, resolved.args, {
 			cwd: options?.cwd,
 			stdio: ["ignore", "pipe", "pipe"],
-			shell: shouldUseWindowsShell(command),
-			env: options?.env ? { ...baseEnv, ...options.env } : baseEnv,
+			env,
 		});
 	}
 
@@ -2489,11 +2491,12 @@ export class DefaultPackageManager implements PackageManager {
 	}
 
 	private runCommandSync(command: string, args: string[]): string {
-		const result = spawnSync(command, args, {
+		const env = getEnv();
+		const resolved = resolveSpawnCommand(command, args, { env });
+		const result = spawnSync(resolved.command, resolved.args, {
 			stdio: ["ignore", "pipe", "pipe"],
 			encoding: "utf-8",
-			shell: shouldUseWindowsShell(command),
-			env: getEnv(),
+			env,
 		});
 		if (result.error || result.status !== 0) {
 			throw new Error(

packages/coding-agent/src/package-manager-cli.ts

@@ -12,7 +12,7 @@ import {
 } from "./config.js";
 import { DefaultPackageManager } from "./core/package-manager.js";
 import { SettingsManager } from "./core/settings-manager.js";
-import { shouldUseWindowsShell } from "./utils/child-process.js";
+import { resolveSpawnCommand } from "./utils/child-process.js";
 import { getLatestPiRelease, isNewerPackageVersion } from "./utils/version-check.js";
 
 export type PackageCommand = "install" | "remove" | "update" | "list";
@@ -316,10 +316,9 @@ async function runSelfUpdate(command: SelfUpdateCommand): Promise<void> {
 	console.log(chalk.dim(`Updating ${APP_NAME} with ${command.display}...`));
 	for (const step of command.steps ?? [command]) {
 		await new Promise<void>((resolve, reject) => {
-			// Windows package managers are commonly .cmd shims. Use the shell so Node can execute them.
-			const child = spawn(step.command, step.args, {
+			const resolved = resolveSpawnCommand(step.command, step.args);
+			const child = spawn(resolved.command, resolved.args, {
 				stdio: "inherit",
-				shell: shouldUseWindowsShell(step.command),
 			});
 			child.on("error", (error) => {
 				reject(error);

packages/coding-agent/src/utils/child-process.ts

@@ -1,14 +1,82 @@
 import type { ChildProcess } from "node:child_process";
-import { basename } from "node:path";
+import { existsSync, readFileSync } from "node:fs";
+import { dirname, extname, join, resolve, sep } from "node:path";
 
 const EXIT_STDIO_GRACE_MS = 100;
+const WINDOWS_COMMAND_EXTENSIONS = ["", ".exe", ".cmd", ".bat"];
+const WINDOWS_COMMAND_SHIM_RE = /\.(?:cmd|bat)$/i;
+const NODE_SHIM_SCRIPT_RE = /(?:%~dp0|%dp0%|%basedir%)[^"'\r\n<>|&]*?\.(?:cjs|mjs|js)/i;
 
-const WINDOWS_SHELL_COMMANDS = new Set(["npm", "npx", "pnpm", "yarn", "yarnpkg", "corepack"]);
+export interface ResolvedSpawnCommand {
+	command: string;
+	args: string[];
+}
 
-export function shouldUseWindowsShell(command: string): boolean {
-	if (process.platform !== "win32") return false;
-	const commandName = basename(command).toLowerCase();
-	return commandName.endsWith(".cmd") || commandName.endsWith(".bat") || WINDOWS_SHELL_COMMANDS.has(commandName);
+function findWindowsCommand(command: string, env: NodeJS.ProcessEnv): string | undefined {
+	const candidates = extname(command)
+		? [command]
+		: WINDOWS_COMMAND_EXTENSIONS.map((extension) => `${command}${extension}`);
+	const hasPath = command.includes("/") || command.includes("\\") || /^[a-zA-Z]:/.test(command);
+	if (hasPath) {
+		const match = candidates.find((candidate) => existsSync(candidate));
+		return match ? resolve(match) : undefined;
+	}
+
+	const pathValue = env.PATH ?? env.Path ?? env.path;
+	if (!pathValue) return undefined;
+	for (const dir of pathValue.split(";")) {
+		for (const candidate of candidates) {
+			const path = join(dir, candidate);
+			if (existsSync(path)) return resolve(path);
+		}
+	}
+	return undefined;
+}
+
+function expandShimPath(path: string, shimPath: string): string {
+	const shimDir = dirname(shimPath);
+	return resolve(
+		path
+			.replace(/%~dp0[\\/]?/gi, `${shimDir}${sep}`)
+			.replace(/%dp0%[\\/]?/gi, `${shimDir}${sep}`)
+			.replace(/%basedir%[\\/]?/gi, `${shimDir}${sep}`)
+			.replace(/\\/g, sep),
+	);
+}
+
+function findNodeShimScript(shimPath: string): string | undefined {
+	const match = readFileSync(shimPath, "utf-8").match(NODE_SHIM_SCRIPT_RE);
+	if (!match) return undefined;
+	const scriptPath = expandShimPath(match[0], shimPath);
+	return existsSync(scriptPath) ? scriptPath : undefined;
+}
+
+export function resolveSpawnCommand(
+	command: string,
+	args: string[],
+	options: { env?: NodeJS.ProcessEnv } = {},
+): ResolvedSpawnCommand {
+	if (process.platform !== "win32") {
+		return { command, args };
+	}
+
+	const env = options.env ?? process.env;
+	const resolvedCommand = findWindowsCommand(command, env);
+	if (!resolvedCommand) {
+		return { command, args };
+	}
+
+	if (!WINDOWS_COMMAND_SHIM_RE.test(resolvedCommand)) {
+		return { command: resolvedCommand, args };
+	}
+
+	const script = findNodeShimScript(resolvedCommand);
+	if (!script) {
+		throw new Error(`Refusing to run Windows command shim without a shell: ${resolvedCommand}`);
+	}
+	const localNode = join(dirname(resolvedCommand), "node.exe");
+	const nodeCommand = existsSync(localNode) ? localNode : (findWindowsCommand("node.exe", env) ?? "node");
+	return { command: nodeCommand, args: [script, ...args] };
 }
 
 /**

packages/coding-agent/test/package-manager.test.ts

@@ -6,7 +6,7 @@ import { PassThrough } from "node:stream";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { DefaultPackageManager, type ProgressEvent, type ResolvedResource } from "../src/core/package-manager.js";
 import { SettingsManager } from "../src/core/settings-manager.js";
-import { shouldUseWindowsShell } from "../src/utils/child-process.js";
+import { resolveSpawnCommand } from "../src/utils/child-process.js";
 
 function normalizeForMatch(value: string): string {
 	return value.replace(/\\/g, "/");
@@ -618,13 +618,58 @@ Content`,
 	});
 
 	describe("windows command spawning", () => {
-		it("should avoid the shell for git so Windows paths with spaces stay single arguments", () => {
+		it("should keep unresolved executables as argv commands", () => {
 			vi.spyOn(process, "platform", "get").mockReturnValue("win32");
 
-			expect(shouldUseWindowsShell("git")).toBe(false);
-			expect(shouldUseWindowsShell("npm")).toBe(true);
-			expect(shouldUseWindowsShell("pnpm")).toBe(true);
-			expect(shouldUseWindowsShell("C:/Program Files/nodejs/npm.cmd")).toBe(true);
+			const resolved = resolveSpawnCommand("git", ["clone", "repo", "C:\\Users\\A B\\repo"], {
+				env: { PATH: tempDir },
+			});
+
+			expect(resolved).toEqual({ command: "git", args: ["clone", "repo", "C:\\Users\\A B\\repo"] });
+		});
+
+		it("should prefer Windows executables over command scripts", () => {
+			vi.spyOn(process, "platform", "get").mockReturnValue("win32");
+			const binDir = join(tempDir, "bin");
+			mkdirSync(binDir, { recursive: true });
+			writeFileSync(join(binDir, "npm.exe"), "");
+			writeFileSync(join(binDir, "npm.cmd"), "@echo off\r\n");
+
+			const args = ["install", "pkg", "--prefix", "C:\\Users\\A B\\.pi\\npm"];
+			const resolved = resolveSpawnCommand("npm", args, { env: { PATH: binDir } });
+
+			expect(resolved).toEqual({ command: join(binDir, "npm.exe"), args });
+		});
+
+		it("should resolve npm command shims to their Node entrypoint without a shell", () => {
+			vi.spyOn(process, "platform", "get").mockReturnValue("win32");
+			const binDir = join(tempDir, "node-bin");
+			const npmCli = join(binDir, "node_modules", "npm", "bin", "npm-cli.js");
+			mkdirSync(join(binDir, "node_modules", "npm", "bin"), { recursive: true });
+			writeFileSync(join(binDir, "node.exe"), "");
+			writeFileSync(npmCli, "");
+			writeFileSync(
+				join(binDir, "npm.cmd"),
+				'@ECHO off\r\nSET "dp0=%~dp0"\r\nSET "_prog=%dp0%\\node.exe"\r\nendLocal & goto #_undefined_# 2>NUL || title %COMSPEC% & "%_prog%" "%dp0%\\node_modules\\npm\\bin\\npm-cli.js" %*\r\n',
+			);
+
+			const args = ["install", "pkg", "--prefix", "C:\\Users\\A B\\.pi\\npm"];
+			const resolved = resolveSpawnCommand("npm", args, { env: { PATH: binDir } });
+
+			expect(resolved).toEqual({ command: join(binDir, "node.exe"), args: [npmCli, ...args] });
+		});
+
+		it("should reject unrecognized Windows command scripts instead of using cmd.exe", () => {
+			vi.spyOn(process, "platform", "get").mockReturnValue("win32");
+			const binDir = join(tempDir, "bin");
+			mkdirSync(binDir, { recursive: true });
+			writeFileSync(join(binDir, "npm.cmd"), "@echo off\r\necho %*\r\n");
+
+			expect(() =>
+				resolveSpawnCommand("npm", ["install", "pkg"], {
+					env: { PATH: binDir },
+				}),
+			).toThrow("Refusing to run Windows command shim without a shell");
 		});
 	});