fix: centralize Windows exec handling and async migrations (#318)

* Add Windows exec foundation helpers

* server: centralize windows exec handling phase 2

* migrate cli daemon status to execCommand

* refactor: complete phase 4 async exec migrations

* test: cover windows exec helpers

* fix: use beforeEach for context.skip in test setup
This commit is contained in:
Mohamed Boudra
2026-04-13 16:20:59 +08:00
committed by GitHub
parent f0c59ec534
commit bf76915884
44 changed files with 3048 additions and 2781 deletions
+3 -6
View File
@@ -1,5 +1,3 @@
import { execFile } from "child_process";
import { promisify } from "util";
import { resolve, dirname, basename } from "path";
import { existsSync, realpathSync } from "fs";
import { open as openFile, stat as statFile } from "fs/promises";
@@ -8,10 +6,9 @@ import type { ParsedDiffFile } from "../server/utils/diff-highlighter.js";
import { parseAndHighlightDiff } from "../server/utils/diff-highlighter.js";
import { findExecutable } from "./executable.js";
import { runGitCommand } from "./run-git-command.js";
import { execCommand } from "./spawn.js";
import { isPaseoOwnedWorktreeCwd } from "./worktree.js";
import { requirePaseoWorktreeBaseRefName } from "./worktree-metadata.js";
const execFileAsync = promisify(execFile);
const READ_ONLY_GIT_ENV: NodeJS.ProcessEnv = {
...process.env,
GIT_OPTIONAL_LOCKS: "0",
@@ -1899,7 +1896,7 @@ export async function createPullRequest(
if (options.body) {
args.push("-f", `body=${options.body}`);
}
const { stdout } = await execFileAsync(ghPath, args, { cwd, env: ghEnv });
const { stdout } = await execCommand(ghPath, args, { cwd, env: ghEnv });
const parsed = JSON.parse(stdout.trim());
if (!parsed?.url || !parsed?.number) {
throw new Error("GitHub CLI did not return PR url/number");
@@ -1951,7 +1948,7 @@ async function getPullRequestStatusUncached(cwd: string): Promise<PullRequestSta
};
}
try {
const { stdout } = await execFileAsync(
const { stdout } = await execCommand(
ghPath,
["pr", "view", "--json", "url,title,state,baseRefName,headRefName,mergedAt"],
{ cwd, env: { ...process.env, GIT_TERMINAL_PROMPT: "0" } },
+197 -58
View File
@@ -1,78 +1,209 @@
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
import { promisify } from "node:util";
import { afterEach, describe, expect, test, vi } from "vitest";
import { findExecutableSync, quoteWindowsArgument, quoteWindowsCommand } from "./executable.js";
type ExecFileCallback = (error: Error | null, stdout: string, stderr: string) => void;
type FindExecutableDependencies = NonNullable<Parameters<typeof findExecutableSync>[1]>;
async function loadExecutableModule(params?: {
execFileImpl?: (
command: string,
args: string[],
options: unknown,
callback: ExecFileCallback,
) => void;
}) {
vi.resetModules();
function createFindExecutableDependencies(): FindExecutableDependencies {
const execFileMock = vi.fn(
params?.execFileImpl ??
((_command: string, _args: string[], _options: unknown, callback: ExecFileCallback) => {
callback(new Error("execFile not mocked"), "", "");
}),
);
Object.assign(execFileMock, {
[promisify.custom]: (command: string, args: string[], options: unknown) =>
new Promise<{ stdout: string; stderr: string }>((resolve, reject) => {
execFileMock(
command,
args,
options,
(error: Error | null, stdout: string, stderr: string) => {
if (error) {
reject(error);
return;
}
resolve({ stdout, stderr });
},
);
}),
});
vi.doMock("node:child_process", () => ({
execFile: execFileMock,
}));
const module = await import("./executable.js");
return {
execFileSync: vi.fn(),
existsSync: vi.fn(),
platform: vi.fn(() => "darwin"),
...module,
execFileMock,
};
}
let findExecutableDependencies: FindExecutableDependencies;
describe("findExecutable", () => {
const originalPlatform = process.platform;
beforeEach(() => {
findExecutableDependencies = createFindExecutableDependencies();
});
function setPlatform(value: string) {
Object.defineProperty(process, "platform", { value, writable: true });
}
describe("findExecutableSync", () => {
test("on Windows, resolves executables using where.exe with inherited PATH", () => {
findExecutableDependencies.platform = vi.fn(() => "win32");
findExecutableDependencies.execFileSync.mockReturnValue(
"C:\\Users\\boudr\\.local\\bin\\claude.exe\r\n",
);
afterEach(() => {
setPlatform(originalPlatform);
});
expect(findExecutableSync("claude", findExecutableDependencies)).toBe(
test("on Windows, resolves executables using where.exe with inherited PATH", async () => {
setPlatform("win32");
const { execFileMock, findExecutable } = await loadExecutableModule({
execFileImpl: (_command, _args, _options, callback) => {
callback(null, "C:\\Users\\boudr\\.local\\bin\\claude.exe\r\n", "");
},
});
await expect(findExecutable("claude")).resolves.toBe(
"C:\\Users\\boudr\\.local\\bin\\claude.exe",
);
expect(findExecutableDependencies.execFileSync).toHaveBeenCalledOnce();
const call = findExecutableDependencies.execFileSync.mock.calls[0];
expect(execFileMock).toHaveBeenCalledOnce();
const call = execFileMock.mock.calls[0];
expect(call?.[0]).toBe("where.exe");
expect(call?.[1]).toEqual(["claude"]);
expect(call?.[2]?.encoding).toBe("utf8");
expect(call?.[2]?.windowsHide).toBe(true);
});
test("on Windows, preserves the first where.exe match", () => {
findExecutableDependencies.platform = vi.fn(() => "win32");
findExecutableDependencies.execFileSync.mockReturnValue(
"C:\\nvm4w\\nodejs\\codex\r\nC:\\nvm4w\\nodejs\\codex.cmd\r\n",
);
expect(findExecutableSync("codex", findExecutableDependencies)).toBe(
"C:\\nvm4w\\nodejs\\codex",
);
});
test("on Unix, uses the last line from which output", () => {
findExecutableDependencies.execFileSync.mockReturnValue("/usr/local/bin/codex\n");
expect(findExecutableSync("codex", findExecutableDependencies)).toBe("/usr/local/bin/codex");
expect(findExecutableDependencies.execFileSync).toHaveBeenCalledWith("which", ["codex"], {
expect(call?.[2]).toMatchObject({
encoding: "utf8",
windowsHide: true,
});
});
test("warns and returns null when the final which line is not an absolute path", () => {
findExecutableDependencies.execFileSync.mockReturnValue("codex\n");
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
test("on Windows, prefers an executable match from where.exe output", async () => {
setPlatform("win32");
const { findExecutable } = await loadExecutableModule({
execFileImpl: (_command, _args, _options, callback) => {
callback(null, "C:\\nvm4w\\nodejs\\codex\r\nC:\\nvm4w\\nodejs\\codex.cmd\r\n", "");
},
});
expect(findExecutableSync("codex", findExecutableDependencies)).toBeNull();
await expect(findExecutable("codex")).resolves.toBe("C:\\nvm4w\\nodejs\\codex.cmd");
});
test("on Windows, prefers .exe over .cmd, .ps1, and extensionless candidates", async () => {
setPlatform("win32");
const { findExecutable } = await loadExecutableModule({
execFileImpl: (_command, _args, _options, callback) => {
callback(
null,
[
"C:\\nvm4w\\nodejs\\codex",
"C:\\nvm4w\\nodejs\\codex.ps1",
"C:\\nvm4w\\nodejs\\codex.cmd",
"C:\\nvm4w\\nodejs\\codex.exe",
].join("\r\n"),
"",
);
},
});
await expect(findExecutable("codex")).resolves.toBe("C:\\nvm4w\\nodejs\\codex.exe");
});
test("on Windows, returns null when where.exe output is empty", async () => {
setPlatform("win32");
const { findExecutable } = await loadExecutableModule({
execFileImpl: (_command, _args, _options, callback) => {
callback(null, "\r\n", "");
},
});
await expect(findExecutable("codex")).resolves.toBeNull();
});
test("on Windows, falls back to the first extensionless candidate when needed", async () => {
setPlatform("win32");
const { findExecutable } = await loadExecutableModule({
execFileImpl: (_command, _args, _options, callback) => {
callback(null, "C:\\nvm4w\\nodejs\\codex\r\n", "");
},
});
await expect(findExecutable("codex")).resolves.toBe("C:\\nvm4w\\nodejs\\codex");
});
test("on Unix, uses the last line from which output", async () => {
const { execFileMock, findExecutable } = await loadExecutableModule({
execFileImpl: (_command, _args, _options, callback) => {
callback(null, "/usr/local/bin/codex\n", "");
},
});
await expect(findExecutable("codex")).resolves.toBe("/usr/local/bin/codex");
expect(execFileMock).toHaveBeenCalledWith(
"which",
["codex"],
{ encoding: "utf8" },
expect.any(Function),
);
});
test("warns and returns null when the final which line is not an absolute path", async () => {
const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
const { findExecutable } = await loadExecutableModule({
execFileImpl: (_command, _args, _options, callback) => {
callback(null, "codex\n", "");
},
});
await expect(findExecutable("codex")).resolves.toBeNull();
expect(warnSpy).toHaveBeenCalledOnce();
warnSpy.mockRestore();
});
test("returns direct paths when they exist", () => {
findExecutableDependencies.existsSync.mockReturnValue(true);
test("returns null when which lookup fails", async () => {
const { findExecutable } = await loadExecutableModule({
execFileImpl: (_command, _args, _options, callback) => {
callback(new Error("which failed"), "", "");
},
});
expect(findExecutableSync("/usr/local/bin/codex", findExecutableDependencies)).toBe(
"/usr/local/bin/codex",
);
expect(findExecutableDependencies.existsSync).toHaveBeenCalledWith("/usr/local/bin/codex");
await expect(findExecutable("codex")).resolves.toBeNull();
});
});
describe("executableExists", () => {
const originalPlatform = process.platform;
function setPlatform(value: string) {
Object.defineProperty(process, "platform", { value, writable: true });
}
afterEach(() => {
setPlatform(originalPlatform);
});
test("returns the path when it already exists", async () => {
const { executableExists } = await loadExecutableModule();
const exists = vi.fn((candidate: string) => candidate === "/usr/local/bin/codex");
expect(executableExists("/usr/local/bin/codex", exists)).toBe("/usr/local/bin/codex");
});
test("on Windows, falls back to .exe, .cmd, then .ps1 for extensionless paths", async () => {
setPlatform("win32");
const { executableExists } = await loadExecutableModule();
const exists = vi.fn((candidate: string) => candidate === "C:\\tools\\codex.cmd");
expect(executableExists("C:\\tools\\codex", exists)).toBe("C:\\tools\\codex.cmd");
});
test("returns null when no matching path exists", async () => {
const { executableExists } = await loadExecutableModule();
const exists = vi.fn(() => false);
expect(executableExists("/missing/codex", exists)).toBeNull();
});
});
@@ -87,27 +218,31 @@ describe("quoteWindowsCommand", () => {
setPlatform(originalPlatform);
});
test("quotes a Windows path with spaces", () => {
test("quotes a Windows path with spaces", async () => {
setPlatform("win32");
const { quoteWindowsCommand } = await loadExecutableModule();
expect(quoteWindowsCommand("C:\\Program Files\\Anthropic\\claude.exe")).toBe(
'"C:\\Program Files\\Anthropic\\claude.exe"',
);
});
test("does not double-quote an already-quoted path", () => {
test("does not double-quote an already-quoted path", async () => {
setPlatform("win32");
const { quoteWindowsCommand } = await loadExecutableModule();
expect(quoteWindowsCommand('"C:\\Program Files\\Anthropic\\claude.exe"')).toBe(
'"C:\\Program Files\\Anthropic\\claude.exe"',
);
});
test("returns the command unchanged when there are no spaces", () => {
test("returns the command unchanged when there are no spaces", async () => {
setPlatform("win32");
const { quoteWindowsCommand } = await loadExecutableModule();
expect(quoteWindowsCommand("C:\\nvm4w\\nodejs\\codex")).toBe("C:\\nvm4w\\nodejs\\codex");
});
test("returns the command unchanged on non-Windows platforms", () => {
test("returns the command unchanged on non-Windows platforms", async () => {
setPlatform("darwin");
const { quoteWindowsCommand } = await loadExecutableModule();
expect(quoteWindowsCommand("/usr/local/bin/claude code")).toBe("/usr/local/bin/claude code");
});
});
@@ -123,27 +258,31 @@ describe("quoteWindowsArgument", () => {
setPlatform(originalPlatform);
});
test("quotes a Windows argument with spaces", () => {
test("quotes a Windows argument with spaces", async () => {
setPlatform("win32");
const { quoteWindowsArgument } = await loadExecutableModule();
expect(quoteWindowsArgument("C:\\Program Files\\Anthropic\\cli.js")).toBe(
'"C:\\Program Files\\Anthropic\\cli.js"',
);
});
test("does not double-quote an already-quoted argument", () => {
test("does not double-quote an already-quoted argument", async () => {
setPlatform("win32");
const { quoteWindowsArgument } = await loadExecutableModule();
expect(quoteWindowsArgument('"C:\\Program Files\\Anthropic\\cli.js"')).toBe(
'"C:\\Program Files\\Anthropic\\cli.js"',
);
});
test("returns the argument unchanged when there are no spaces", () => {
test("returns the argument unchanged when there are no spaces", async () => {
setPlatform("win32");
const { quoteWindowsArgument } = await loadExecutableModule();
expect(quoteWindowsArgument("--version")).toBe("--version");
});
test("returns the argument unchanged on non-Windows platforms", () => {
test("returns the argument unchanged on non-Windows platforms", async () => {
setPlatform("darwin");
const { quoteWindowsArgument } = await loadExecutableModule();
expect(quoteWindowsArgument("/usr/local/bin/claude code")).toBe("/usr/local/bin/claude code");
});
});
+30 -63
View File
@@ -1,15 +1,21 @@
import { execFile, execFileSync } from "node:child_process";
import { execFile } from "node:child_process";
import { existsSync } from "node:fs";
import { platform } from "node:os";
import path from "node:path";
import path, { extname } from "node:path";
import { promisify } from "node:util";
const execFileAsync = promisify(execFile);
export interface FindExecutableDependencies {
execFileSync: typeof execFileSync;
existsSync: typeof existsSync;
platform: typeof platform;
function pickBestWindowsCandidate(lines: string[]): string | null {
const candidates = lines.filter((line) => line.length > 0);
if (candidates.length === 0) return null;
const extPriority = [".exe", ".cmd", ".ps1"];
for (const ext of extPriority) {
const match = candidates.find((candidate) => candidate.toLowerCase().endsWith(ext));
if (match) return match;
}
return candidates[0] ?? null;
}
function resolveExecutableFromWhichOutput(
@@ -44,58 +50,18 @@ function resolveExecutableFromWhichOutput(
* enriches it at startup via inheritLoginShellEnv(); on Windows, Electron
* inherits the full user environment from Explorer.
*/
export function findExecutableSync(
name: string,
dependencies?: FindExecutableDependencies,
export function executableExists(
executablePath: string,
exists: typeof existsSync = existsSync,
): string | null {
const trimmed = name.trim();
if (!trimmed) {
return null;
}
const deps: FindExecutableDependencies = {
execFileSync,
existsSync,
platform,
...dependencies,
};
if (trimmed.includes("/") || trimmed.includes("\\")) {
return deps.existsSync(trimmed) ? trimmed : null;
}
if (deps.platform() === "win32") {
try {
const out = deps
.execFileSync("where.exe", [trimmed], {
encoding: "utf8",
windowsHide: true,
})
.trim();
return (
out
.split(/\r?\n/)
.map((line) => line.trim())
.find((line) => line.length > 0) ?? null
);
} catch {
return null;
if (exists(executablePath)) return executablePath;
if (process.platform === "win32" && !extname(executablePath)) {
for (const ext of [".exe", ".cmd", ".ps1"]) {
const candidate = executablePath + ext;
if (exists(candidate)) return candidate;
}
}
try {
return resolveExecutableFromWhichOutput(
trimmed,
deps.execFileSync("which", [trimmed], { encoding: "utf8" }).trim(),
"which",
);
} catch {
return null;
}
}
export function isCommandAvailableSync(command: string): boolean {
return findExecutableSync(command) !== null;
return null;
}
export async function findExecutable(name: string): Promise<string | null> {
@@ -105,21 +71,22 @@ export async function findExecutable(name: string): Promise<string | null> {
}
if (trimmed.includes("/") || trimmed.includes("\\")) {
return existsSync(trimmed) ? trimmed : null;
return executableExists(trimmed);
}
if (platform() === "win32") {
if (process.platform === "win32") {
try {
const { stdout } = await execFileAsync("where.exe", [trimmed], {
encoding: "utf8",
windowsHide: true,
});
return (
stdout
.trim()
.split(/\r?\n/)
.map((line) => line.trim())
.find((line) => line.length > 0) ?? null
pickBestWindowsCandidate(
stdout
.trim()
.split(/\r?\n/)
.map((line) => line.trim()),
) ?? null
);
} catch {
return null;
+54
View File
@@ -0,0 +1,54 @@
import { mkdtempSync, realpathSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import path from "node:path";
import { afterEach, describe, expect, test } from "vitest";
import { execCommand } from "./spawn.js";
describe("execCommand", () => {
const tempDirs: string[] = [];
afterEach(() => {
for (const tempDir of tempDirs) {
rmSync(tempDir, { recursive: true, force: true });
}
tempDirs.length = 0;
});
test("returns stdout and stderr for a successful command", async () => {
await expect(execCommand("echo", ["hello"])).resolves.toEqual({
stdout: "hello\n",
stderr: "",
});
});
test("rejects when the command times out", async () => {
const command =
process.platform === "win32"
? {
command: process.execPath,
args: ["-e", "setTimeout(() => {}, 10_000)"],
}
: { command: "sleep", args: ["10"] };
await expect(execCommand(command.command, command.args, { timeout: 100 })).rejects.toThrow();
});
test("runs the command in the provided cwd", async () => {
const cwd = realpathSync(mkdtempSync(path.join(tmpdir(), "spawn-test-")));
tempDirs.push(cwd);
const command =
process.platform === "win32"
? {
command: process.execPath,
args: ["-e", "console.log(process.cwd())"],
}
: { command: "pwd", args: [] };
await expect(execCommand(command.command, command.args, { cwd })).resolves.toEqual({
stdout: `${cwd}\n`,
stderr: "",
});
});
});
+57 -19
View File
@@ -1,15 +1,27 @@
import { spawn, type ChildProcess, type SpawnOptions } from "node:child_process";
import { execFile, spawn, type ChildProcess, type SpawnOptions } from "node:child_process";
import { promisify } from "node:util";
const execFileAsync = promisify(execFile);
interface ExecCommandOptions {
cwd?: string;
env?: NodeJS.ProcessEnv;
encoding?: BufferEncoding;
timeout?: number;
maxBuffer?: number;
}
interface ExecCommandResult {
stdout: string;
stderr: string;
}
function quoteForCmd(value: string): string {
if (!value.includes(" ")) return value;
if (value.startsWith('"') && value.endsWith('"')) return value;
return `"${value}"`;
}
/**
* Platform-aware spawn that centralizes Windows shell and quoting concerns.
*
* On Windows:
* - Enables `shell: true` (routes through cmd.exe) unless the caller explicitly sets `shell`
* - Quotes the command and arguments so paths with spaces survive cmd.exe parsing
* - Always sets `windowsHide: true` to prevent console window flashes
*
* On other platforms the call is passed through to `spawn` unchanged (with `windowsHide: true`).
*/
export function spawnProcess(
command: string,
args: string[],
@@ -27,12 +39,38 @@ export function spawnProcess(
});
}
/**
* Quote a string for cmd.exe if it contains spaces and isn't already quoted.
* No-op for strings without spaces or strings that are already double-quoted.
*/
function quoteForCmd(value: string): string {
if (!value.includes(" ")) return value;
if (value.startsWith('"') && value.endsWith('"')) return value;
return `"${value}"`;
export async function execCommand(
command: string,
args: string[],
options?: ExecCommandOptions,
): Promise<ExecCommandResult> {
const isWindows = process.platform === "win32";
const resolvedCommand = isWindows ? quoteForCmd(command) : command;
const resolvedArgs = isWindows ? args.map(quoteForCmd) : args;
return execFileAsync(resolvedCommand, resolvedArgs, {
cwd: options?.cwd,
env: options?.env,
encoding: options?.encoding ?? "utf8",
timeout: options?.timeout,
maxBuffer: options?.maxBuffer,
shell: isWindows,
windowsHide: true,
}) as Promise<ExecCommandResult>;
}
export function platformShell(): { command: string; flag: string[] } {
if (process.platform === "win32") {
return { command: "cmd.exe", flag: ["/c"] };
}
return { command: "/bin/sh", flag: ["-lc"] };
}
export function platformBash(): { command: string; flag: string[] } {
if (process.platform === "win32") {
return { command: "cmd.exe", flag: ["/c"] };
}
return { command: "/bin/bash", flag: ["-lc"] };
}
+5 -3
View File
@@ -1,4 +1,4 @@
import { exec, spawn } from "child_process";
import { exec } from "child_process";
import { promisify } from "util";
import { existsSync, mkdirSync, readFileSync, realpathSync, rmSync, statSync } from "fs";
import { join, basename, dirname, resolve, sep } from "path";
@@ -13,6 +13,7 @@ import {
writePaseoWorktreeRuntimeMetadata,
} from "./worktree-metadata.js";
import { runGitCommand } from "./run-git-command.js";
import { platformBash, spawnProcess } from "./spawn.js";
import { resolvePaseoHome } from "../server/paseo-home.js";
interface PaseoConfig {
@@ -204,7 +205,7 @@ async function execSetupCommand(
const { stdout, stderr } = await execAsync(command, {
cwd: options.cwd,
env: options.env,
shell: "/bin/bash",
...(process.platform === "win32" ? {} : { shell: "/bin/bash" }),
});
return {
command,
@@ -275,7 +276,8 @@ async function execSetupCommandStreamed(options: {
cwd: options.cwd,
});
const child = spawn("/bin/bash", ["-lc", options.command], {
const shell = platformBash();
const child = spawnProcess(shell.command, [...shell.flag, options.command], {
cwd: options.cwd,
env: options.env,
stdio: ["ignore", "pipe", "pipe"],