fix(coding-agent): use cross-spawn for Windows shims

closes #4665
This commit is contained in:
Mario Zechner
2026-05-18 11:30:41 +02:00
parent b8f51957a0
commit 64150668b8
8 changed files with 124 additions and 153 deletions

View File

@@ -9,6 +9,7 @@
- Fixed Windows npm self-updates to move loaded native dependency packages out of the active install before reinstalling pi ([#4157](https://github.com/earendil-works/pi/issues/4157)).
- Fixed `pi update --self` detection for pnpm v11 global installs whose package path resolves through the pnpm store ([#4647](https://github.com/earendil-works/pi/issues/4647)).
- Fixed Windows pnpm self-updates to resolve pnpm command shims and run through pnpm instead of requiring manual updates ([#4157](https://github.com/earendil-works/pi/issues/4157)).
- Fixed Windows npm-family command execution to use cross-spawn instead of parsing `.cmd` shim internals ([#4665](https://github.com/earendil-works/pi/issues/4665)).
## [0.75.1] - 2026-05-18

View File

@@ -43,6 +43,7 @@
"@earendil-works/pi-tui": "^0.75.1",
"@silvia-odwyer/photon-node": "^0.3.4",
"chalk": "^5.5.0",
"cross-spawn": "^7.0.6",
"diff": "^8.0.2",
"glob": "^13.0.1",
"highlight.js": "^10.7.3",
@@ -65,6 +66,7 @@
"@mariozechner/clipboard": "^0.3.6"
},
"devDependencies": {
"@types/cross-spawn": "^6.0.6",
"@types/diff": "^7.0.2",
"@types/hosted-git-info": "^3.0.5",
"@types/ms": "^2.1.0",

View File

@@ -1,9 +1,8 @@
import { spawnSync } from "child_process";
import { accessSync, constants, existsSync, readFileSync, realpathSync } from "fs";
import { homedir } from "os";
import { basename, dirname, join, resolve, sep, win32 } from "path";
import { fileURLToPath } from "url";
import { resolveSpawnCommand } from "./utils/child-process.js";
import { spawnProcessSync } from "./utils/child-process.js";
// =============================================================================
// Package Detection
@@ -151,16 +150,7 @@ function readCommandOutput(
args: string[],
options: { requireSuccess?: boolean } = {},
): string | undefined {
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, {
const result = spawnProcessSync(command, args, {
encoding: "utf-8",
stdio: ["ignore", "pipe", "pipe"],
});

View File

@@ -1,4 +1,4 @@
import { type ChildProcess, type ChildProcessByStdio, spawn, spawnSync } from "node:child_process";
import type { ChildProcess, ChildProcessByStdio } from "node:child_process";
import { createHash } from "node:crypto";
import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs";
import { homedir, tmpdir } from "node:os";
@@ -28,7 +28,7 @@ import { globSync } from "glob";
import ignore from "ignore";
import { minimatch } from "minimatch";
import { CONFIG_DIR_NAME } from "../config.js";
import { resolveSpawnCommand } from "../utils/child-process.js";
import { spawnProcess, spawnProcessSync } 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";
@@ -2409,8 +2409,7 @@ export class DefaultPackageManager implements PackageManager {
private spawnCommand(command: string, args: string[], options?: { cwd?: string }): ChildProcess {
const env = getEnv();
const resolved = resolveSpawnCommand(command, args, { env });
return spawn(resolved.command, resolved.args, {
return spawnProcess(command, args, {
cwd: options?.cwd,
stdio: isStdoutTakenOver() ? ["ignore", 2, 2] : "inherit",
env,
@@ -2424,8 +2423,7 @@ export class DefaultPackageManager implements PackageManager {
): ChildProcessByStdio<null, Readable, Readable> {
const baseEnv = getEnv();
const env = options?.env ? { ...baseEnv, ...options.env } : baseEnv;
const resolved = resolveSpawnCommand(command, args, { env });
return spawn(resolved.command, resolved.args, {
return spawnProcess(command, args, {
cwd: options?.cwd,
stdio: ["ignore", "pipe", "pipe"],
env,
@@ -2492,8 +2490,7 @@ export class DefaultPackageManager implements PackageManager {
private runCommandSync(command: string, args: string[]): string {
const env = getEnv();
const resolved = resolveSpawnCommand(command, args, { env });
const result = spawnSync(resolved.command, resolved.args, {
const result = spawnProcessSync(command, args, {
stdio: ["ignore", "pipe", "pipe"],
encoding: "utf-8",
env,

View File

@@ -1,5 +1,4 @@
import chalk from "chalk";
import { spawn } from "child_process";
import { selectConfig } from "./cli/config-selector.js";
import {
APP_NAME,
@@ -14,7 +13,7 @@ import {
} from "./config.js";
import { DefaultPackageManager } from "./core/package-manager.js";
import { SettingsManager } from "./core/settings-manager.js";
import { resolveSpawnCommand } from "./utils/child-process.js";
import { spawnProcess } from "./utils/child-process.js";
import { getLatestPiRelease, isNewerPackageVersion } from "./utils/version-check.js";
import {
cleanupWindowsSelfUpdateQuarantine,
@@ -322,8 +321,7 @@ 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) => {
const resolved = resolveSpawnCommand(step.command, step.args);
const child = spawn(resolved.command, resolved.args, {
const child = spawnProcess(step.command, step.args, {
stdio: "inherit",
});
child.on("error", (error) => {

View File

@@ -1,84 +1,38 @@
import type { ChildProcess } from "node:child_process";
import { existsSync, readFileSync } from "node:fs";
import { dirname, extname, join, resolve, sep } from "node:path";
import {
type ChildProcess,
type ChildProcessByStdio,
spawn as nodeSpawn,
spawnSync as nodeSpawnSync,
type SpawnOptions,
type SpawnOptionsWithStdioTuple,
type SpawnSyncOptionsWithStringEncoding,
type SpawnSyncReturns,
type StdioNull,
type StdioPipe,
} from "node:child_process";
import type { Readable } from "node:stream";
import crossSpawn from "cross-spawn";
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)/gi;
export interface ResolvedSpawnCommand {
command: string;
args: string[];
}
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 matches = [...readFileSync(shimPath, "utf-8").matchAll(NODE_SHIM_SCRIPT_RE)];
for (const match of matches.reverse()) {
const scriptPath = expandShimPath(match[0], shimPath);
if (existsSync(scriptPath)) return scriptPath;
}
return undefined;
}
export function resolveSpawnCommand(
export function spawnProcess(
command: string,
args: string[],
options: { env?: NodeJS.ProcessEnv } = {},
): ResolvedSpawnCommand {
if (process.platform !== "win32") {
return { command, args };
}
options: SpawnOptionsWithStdioTuple<StdioNull, StdioPipe, StdioPipe>,
): ChildProcessByStdio<null, Readable, Readable>;
export function spawnProcess(command: string, args: string[], options: SpawnOptions): ChildProcess;
export function spawnProcess(command: string, args: string[], options: SpawnOptions): ChildProcess {
return process.platform === "win32" ? crossSpawn(command, args, options) : nodeSpawn(command, args, options);
}
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] };
export function spawnProcessSync(
command: string,
args: string[],
options: SpawnSyncOptionsWithStringEncoding,
): SpawnSyncReturns<string> {
return process.platform === "win32"
? crossSpawn.sync(command, args, options)
: nodeSpawnSync(command, args, options);
}
/**

View File

@@ -6,7 +6,6 @@ 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 { resolveSpawnCommand } from "../src/utils/child-process.js";
function normalizeForMatch(value: string): string {
return value.replace(/\\/g, "/");
@@ -617,59 +616,19 @@ Content`,
});
});
describe("windows command spawning", () => {
it("should keep unresolved executables as argv commands", () => {
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
describe("command spawning", () => {
it("should preserve argv entries containing spaces", () => {
const managerWithInternals = packageManager as unknown as {
runCommandSync(command: string, args: string[]): string;
};
const valueWithSpace = "C:\\Users\\A B\\.pi\\npm";
const output = managerWithInternals.runCommandSync(process.execPath, [
"-e",
"console.log(process.argv[1])",
valueWithSpace,
]);
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");
expect(output).toBe(valueWithSpace);
});
});