diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c37346fef..20f20ef36 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -120,6 +120,7 @@ jobs: working-directory: packages/server run: > npx vitest run + src/utils/executable.probe.test.ts src/utils/executable.test.ts src/utils/spawn.launch-regression.test.ts src/utils/spawn.percent-escape.test.ts diff --git a/packages/server/src/server/agent/providers/claude-agent.ts b/packages/server/src/server/agent/providers/claude-agent.ts index 2b15643a6..42749ebe4 100644 --- a/packages/server/src/server/agent/providers/claude-agent.ts +++ b/packages/server/src/server/agent/providers/claude-agent.ts @@ -79,6 +79,7 @@ import { createProviderEnvSpec, type ProviderRuntimeSettings, } from "../provider-launch-config.js"; +import { buildSelfNodeCommand } from "../../paseo-env.js"; import { findExecutable, isCommandAvailable } from "../../../utils/executable.js"; import { withTimeout } from "../../../utils/promise-timeout.js"; import { execCommand, spawnProcess } from "../../../utils/spawn.js"; @@ -342,14 +343,26 @@ function applyRuntimeSettingsToClaudeOptions( // When the SDK passes a native binary path (from pathToClaudeCodeExecutable) // or the user overrides the command via runtime settings, use that directly. const isDefaultRuntime = resolved.command === "node" || resolved.command === "bun"; - const command = isDefaultRuntime ? process.execPath : resolved.command; - const child = spawnProcess(command, resolved.args, { + const providerEnvSpec = createProviderEnvSpec({ + baseEnv: spawnOptions.env, + runtimeSettings, + overlays: [launchEnv], + }); + const providerEnv = createProviderEnv({ + baseEnv: spawnOptions.env, + runtimeSettings, + overlays: [launchEnv], + }); + const selfNodeCommand = isDefaultRuntime + ? buildSelfNodeCommand(resolved.args, providerEnv) + : null; + const command = selfNodeCommand?.command ?? resolved.command; + const args = selfNodeCommand?.args ?? resolved.args; + const child = spawnProcess(command, args, { cwd: spawnOptions.cwd, - ...createProviderEnvSpec({ - baseEnv: spawnOptions.env, - runtimeSettings, - overlays: [launchEnv], - }), + ...(selfNodeCommand + ? { env: selfNodeCommand.env, envMode: "internal" as const } + : providerEnvSpec), signal: spawnOptions.signal, stdio: ["pipe", "pipe", "pipe"], // Bypass cmd.exe on Windows: the SDK passes --mcp-config with inline JSON diff --git a/packages/server/src/server/paseo-env.test.ts b/packages/server/src/server/paseo-env.test.ts index c0f13e82d..9155e8d5a 100644 --- a/packages/server/src/server/paseo-env.test.ts +++ b/packages/server/src/server/paseo-env.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "vitest"; import { + buildSelfNodeCommand, createExternalCommandProcessEnv, createExternalProcessEnv, createPaseoInternalEnv, @@ -69,13 +70,13 @@ describe("paseo env contract", () => { expect(env.PATH).toBe("/custom/bin"); }); - test("builds process.execPath external command env with Electron node mode", () => { + test("builds external command env without process.execPath special-casing", () => { const env = createExternalCommandProcessEnv(process.execPath, baseEnv, { ELECTRON_RUN_AS_NODE: "0", PASEO_NODE_ENV: "test", }); - expect(env[ELECTRON_RUN_AS_NODE]).toBe("1"); + expect(env[ELECTRON_RUN_AS_NODE]).toBeUndefined(); expect(env.NODE_ENV).toBe("development"); expect(env.PASEO_AGENT_ID).toBe("agent-123"); expect(env.PATH).toBe("/usr/bin"); @@ -85,16 +86,19 @@ describe("paseo env contract", () => { expect(env.PASEO_SUPERVISED).toBeUndefined(); }); - test("always re-adds Electron node mode after scrubbing process.execPath overlays", () => { - const env = createExternalCommandProcessEnv(process.execPath, baseEnv, { - ELECTRON_RUN_AS_NODE: undefined, + test("builds self node command with Electron node mode", () => { + const command = buildSelfNodeCommand(["script.js"], { + CUSTOM: "value", }); - for (const key of runtimeControlEnvKeys) { - if (key === ELECTRON_RUN_AS_NODE) continue; - expect(env[key]).toBeUndefined(); - } - expect(env[ELECTRON_RUN_AS_NODE]).toBe("1"); + expect(command.command).toBe(process.execPath); + expect(command.args).toEqual(["script.js"]); + expect(command.env[ELECTRON_RUN_AS_NODE]).toBe("1"); + expect(command.env.CUSTOM).toBe("value"); + expect(command.env.ELECTRON_NO_ATTACH_CONSOLE).toBeUndefined(); + expect(command.env.PASEO_DESKTOP_MANAGED).toBeUndefined(); + expect(command.env[PASEO_NODE_ENV]).toBeUndefined(); + expect(command.env.PASEO_SUPERVISED).toBeUndefined(); }); test("does not add Electron node mode for non-execPath commands", () => { diff --git a/packages/server/src/server/paseo-env.ts b/packages/server/src/server/paseo-env.ts index a73a59d87..b2268a3d6 100644 --- a/packages/server/src/server/paseo-env.ts +++ b/packages/server/src/server/paseo-env.ts @@ -1,6 +1,3 @@ -import { realpathSync } from "node:fs"; -import path from "node:path"; - const PASEO_NODE_ENV = "PASEO_NODE_ENV"; const ELECTRON_RUN_AS_NODE = "ELECTRON_RUN_AS_NODE"; @@ -14,9 +11,7 @@ const RUNTIME_CONTROL_ENV_KEYS = [ export type PaseoNodeEnv = "development" | "production" | "test"; export type ProcessEnvRecord = Record; -type ExternalProcessEnv = NodeJS.ProcessEnv & Record; - -let resolvedProcessExecPath: string | undefined; +export type ExternalProcessEnv = NodeJS.ProcessEnv & Record; function buildInternalProcessEnv(baseEnv: T): T { return { ...baseEnv }; @@ -38,37 +33,6 @@ function buildExternalProcessEnv( return sanitized as ExternalProcessEnv; } -function normalizeExecutablePath(executablePath: string): string { - return process.platform === "win32" ? executablePath.toLowerCase() : executablePath; -} - -function resolveExecutablePath(executablePath: string): string | undefined { - try { - return realpathSync.native(executablePath); - } catch { - return undefined; - } -} - -function isProcessExecPathCommand(command: string): boolean { - if (command === process.execPath) { - return true; - } - if (!path.isAbsolute(command)) { - return false; - } - - resolvedProcessExecPath ??= resolveExecutablePath(process.execPath); - const resolvedCommand = resolveExecutablePath(command); - if (!resolvedCommand || !resolvedProcessExecPath) { - return false; - } - - return ( - normalizeExecutablePath(resolvedCommand) === normalizeExecutablePath(resolvedProcessExecPath) - ); -} - export function createPaseoInternalEnv(baseEnv: NodeJS.ProcessEnv): NodeJS.ProcessEnv { return buildInternalProcessEnv(baseEnv); } @@ -81,15 +45,34 @@ export function createExternalProcessEnv( } export function createExternalCommandProcessEnv( - command: string, + _command: string, baseEnv: ProcessEnvRecord, ...overlays: ProcessEnvRecord[] ): ExternalProcessEnv { - const env = buildExternalProcessEnv(baseEnv, overlays); - if (isProcessExecPathCommand(command)) { - env[ELECTRON_RUN_AS_NODE] = "1"; + // Deprecated command parameter: retained while callers migrate to createExternalProcessEnv. + return buildExternalProcessEnv(baseEnv, overlays); +} + +export function buildSelfNodeCommand( + args: string[], + envOverlay?: ProcessEnvRecord, +): { + command: string; + args: string[]; + env: ExternalProcessEnv; +} { + const env = buildExternalProcessEnv(process.env, []); + Object.assign(env, { [ELECTRON_RUN_AS_NODE]: "1" }, envOverlay); + for (const [key, value] of Object.entries(env)) { + if (value === undefined) { + delete env[key]; + } } - return env; + return { + command: process.execPath, + args, + env, + }; } export function resolvePaseoNodeEnv(env: NodeJS.ProcessEnv): PaseoNodeEnv | undefined { diff --git a/packages/server/src/utils/executable.probe.test.ts b/packages/server/src/utils/executable.probe.test.ts new file mode 100644 index 000000000..18ced11db --- /dev/null +++ b/packages/server/src/utils/executable.probe.test.ts @@ -0,0 +1,159 @@ +import { + chmodSync, + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { performance } from "node:perf_hooks"; +import { afterEach, describe, expect, test } from "vitest"; + +import { probeExecutable } from "./executable.js"; + +const timeoutMs = 1000; +const timeoutSlackMs = 500; +const tempDirs: string[] = []; + +interface ProbeFixture { + name: string; + expected: boolean; + create: (dir: string) => { executablePath: string; pidFile?: string }; +} + +function makeTempDir(): string { + const dir = mkdtempSync(path.join(os.tmpdir(), "paseo-probe-test-")); + tempDirs.push(dir); + return dir; +} + +function writeExecutable(filePath: string, content: string | Buffer): string { + writeFileSync(filePath, content); + if (process.platform !== "win32") { + chmodSync(filePath, 0o755); + } + return filePath; +} + +function scriptPath(dir: string, name: string): string { + return process.platform === "win32" ? path.join(dir, `${name}.cmd`) : path.join(dir, name); +} + +function createHangingFixture(dir: string): string { + if (process.platform === "win32") { + return writeExecutable( + scriptPath(dir, "hangs"), + "@echo off\r\n:loop\r\ntimeout /T 5 /NOBREAK > NUL\r\ngoto loop\r\n", + ); + } + return writeExecutable( + scriptPath(dir, "hangs"), + `#!/bin/sh\ntrap '' TERM\necho $$ > "${path.join(dir, "hangs.pid")}"\nwhile :; do :; done\n`, + ); +} + +function createNoVersionFixture(dir: string): string { + if (process.platform === "win32") { + return writeExecutable(scriptPath(dir, "no-version"), "@echo off\r\nexit /b 0\r\n"); + } + return writeExecutable(scriptPath(dir, "no-version"), "#!/bin/sh\nexit 0\n"); +} + +function createNonZeroFixture(dir: string): string { + if (process.platform === "win32") { + return writeExecutable( + scriptPath(dir, "non-zero"), + "@echo off\r\necho oops 1>&2\r\nexit /b 1\r\n", + ); + } + return writeExecutable(scriptPath(dir, "non-zero"), "#!/bin/sh\necho oops 1>&2\nexit 1\n"); +} + +function createSlowSuccessFixture(dir: string): string { + if (process.platform === "win32") { + return writeExecutable( + scriptPath(dir, "slow-success"), + "@echo off\r\nping -n 1 127.0.0.1 > NUL\r\nexit /b 0\r\n", + ); + } + return writeExecutable(scriptPath(dir, "slow-success"), "#!/bin/sh\nsleep 0.05\nexit 0\n"); +} + +function createDirectoryFixture(dir: string): string { + const directoryPath = path.join(dir, "candidate-directory"); + mkdirSync(directoryPath); + return directoryPath; +} + +function missingAbsolutePath(): string { + return process.platform === "win32" ? "C:\\no\\such\\path.exe" : "/no/such/path"; +} + +async function waitForFile(filePath: string): Promise { + const deadline = performance.now() + timeoutSlackMs; + while (!existsSync(filePath) && performance.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 10)); + } +} + +const fixtures: ProbeFixture[] = [ + { + name: "hangs forever after starting", + expected: true, + create: (dir) => ({ + executablePath: createHangingFixture(dir), + pidFile: process.platform === "win32" ? undefined : path.join(dir, "hangs.pid"), + }), + }, + { + name: "does not know --version and exits zero", + expected: true, + create: (dir) => ({ executablePath: createNoVersionFixture(dir) }), + }, + { + name: "exits non-zero immediately", + expected: true, + create: (dir) => ({ executablePath: createNonZeroFixture(dir) }), + }, + { + name: "starts slowly and exits zero", + expected: true, + create: (dir) => ({ executablePath: createSlowSuccessFixture(dir) }), + }, + { + name: "points at a directory", + expected: false, + create: (dir) => ({ executablePath: createDirectoryFixture(dir) }), + }, + { + name: "does not exist at an absolute path", + expected: false, + create: () => ({ executablePath: missingAbsolutePath() }), + }, +]; + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } +}); + +describe("probeExecutable", () => { + test.each(fixtures)("$name", async ({ create, expected }) => { + const { executablePath, pidFile } = create(makeTempDir()); + const startedAt = performance.now(); + + const result = await probeExecutable(executablePath, timeoutMs); + + expect(result).toBe(expected); + expect(performance.now() - startedAt).toBeLessThanOrEqual(timeoutMs + timeoutSlackMs); + if (pidFile) { + await waitForFile(pidFile); + const pid = Number(readFileSync(pidFile, "utf8")); + expect(() => process.kill(pid, 0)).toThrow(expect.objectContaining({ code: "ESRCH" })); + } + }); +}); diff --git a/packages/server/src/utils/executable.ts b/packages/server/src/utils/executable.ts index 001f4e63f..8f4225f35 100644 --- a/packages/server/src/utils/executable.ts +++ b/packages/server/src/utils/executable.ts @@ -1,8 +1,7 @@ -import type { ChildProcess } from "node:child_process"; import { createRequire } from "node:module"; import { existsSync } from "node:fs"; import { extname } from "node:path"; -import { spawnProcess } from "./spawn.js"; +import { execCommand } from "./spawn.js"; import { isWindowsCommandScript } from "./windows-command.js"; export { quoteWindowsArgument, quoteWindowsCommand } from "./windows-command.js"; @@ -18,6 +17,25 @@ function hasPathSeparator(value: string): boolean { } async function enumerateCandidates(name: string): Promise { + if (process.platform !== "win32" && existsSync("/usr/bin/which")) { + return enumerateCandidatesViaSystemWhich(name); + } + return enumerateCandidatesViaLibrary(name); +} + +async function enumerateCandidatesViaSystemWhich(name: string): Promise { + try { + const { stdout } = await execCommand("/usr/bin/which", ["-a", name], { + timeout: 3000, + killSignal: "SIGKILL", + }); + return Array.from(new Set(stdout.trim().split("\n").filter(Boolean))); + } catch { + return []; + } +} + +async function enumerateCandidatesViaLibrary(name: string): Promise { let candidates: string[]; try { candidates = await which(name, { all: true }); @@ -39,57 +57,42 @@ async function enumerateCandidates(name: string): Promise { }); } -async function probeExecutable(executablePath: string): Promise { - return await new Promise((resolve) => { - let pendingResolve: ((result: boolean) => void) | null = resolve; - let started = false; - let timer: NodeJS.Timeout | undefined; - - const settle = (result: boolean) => { - if (!pendingResolve) { - return; - } - const fn = pendingResolve; - pendingResolve = null; - if (timer) { - clearTimeout(timer); - } - fn(result); - }; - - let child: ChildProcess; - try { - child = spawnProcess(executablePath, ["--version"], { - stdio: "ignore", - // Windows batch shims (.cmd/.bat) require cmd.exe; native binaries do not. - shell: isWindowsCommandScript(executablePath), - }); - } catch { - settle(false); - return; - } - - timer = setTimeout(() => { - if (started) { - child.kill(); - settle(true); - return; - } - settle(false); - }, PROBE_TIMEOUT_MS) as unknown as NodeJS.Timeout; - timer.unref(); - - child.once("spawn", () => { - started = true; +export async function probeExecutable( + executablePath: string, + timeoutMs = PROBE_TIMEOUT_MS, +): Promise { + try { + await execCommand(executablePath, ["--version"], { + timeout: timeoutMs, + killSignal: "SIGKILL", + maxBuffer: 64 * 1024, + shell: isWindowsCommandScript(executablePath), }); - child.once("error", () => { - // ENOENT/EACCES/EPERM/UNKNOWN here means the OS could not start the candidate. - settle(started); - }); - child.once("exit", () => { - settle(started); - }); - }); + return true; + } catch (error) { + return classifyProbeError(error); + } +} + +function classifyProbeError(error: unknown): boolean { + const err = error as NodeJS.ErrnoException & { + killed?: boolean; + }; + if (err.killed) { + return true; + } + if (typeof err.code === "number") { + return true; + } + if ( + err.code === "ENOENT" || + err.code === "EACCES" || + err.code === "ENOEXEC" || + err.code === "UNKNOWN" + ) { + return false; + } + return false; } /** @@ -109,18 +112,23 @@ export function executableExists( return null; } -export async function findExecutable(name: string): Promise { +export async function findExecutable( + name: string, + probeTimeoutMs = PROBE_TIMEOUT_MS, +): Promise { const trimmed = name.trim(); if (!trimmed) { return null; } if (hasPathSeparator(trimmed)) { - return (await probeExecutable(trimmed)) ? trimmed : null; + return (await probeExecutable(trimmed, probeTimeoutMs)) ? trimmed : null; } const candidates = await enumerateCandidates(trimmed); - const probeResults = await Promise.all(candidates.map((candidate) => probeExecutable(candidate))); + const probeResults = await Promise.all( + candidates.map((candidate) => probeExecutable(candidate, probeTimeoutMs)), + ); const firstMatch = probeResults.findIndex((result) => result); return firstMatch === -1 ? null : candidates[firstMatch]; } diff --git a/packages/server/src/utils/spawn.test.ts b/packages/server/src/utils/spawn.test.ts index 65cf2ea74..43635d0e9 100644 --- a/packages/server/src/utils/spawn.test.ts +++ b/packages/server/src/utils/spawn.test.ts @@ -1,8 +1,10 @@ import { mkdtempSync, realpathSync, rmSync } from "node:fs"; +import * as fs from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; -import { afterEach, describe, expect, test } from "vitest"; +import { afterEach, describe, expect, test, vi } from "vitest"; +import { buildSelfNodeCommand } from "../server/paseo-env.js"; import { execCommand, spawnProcess } from "./spawn.js"; const printEnvScript = ` @@ -93,7 +95,7 @@ describe("execCommand", () => { expect(parsePrintedEnv(result.stdout)).toEqual({ CUSTOM: "from-overlay", ELECTRON_NO_ATTACH_CONSOLE: null, - ELECTRON_RUN_AS_NODE: "1", + ELECTRON_RUN_AS_NODE: null, PASEO_DESKTOP_MANAGED: null, PASEO_NODE_ENV: null, PASEO_SUPERVISED: null, @@ -150,7 +152,7 @@ describe("execCommand", () => { expect(parsePrintedEnv(Buffer.concat(stdoutChunks).toString())).toEqual({ CUSTOM: "spawn-overlay", ELECTRON_NO_ATTACH_CONSOLE: null, - ELECTRON_RUN_AS_NODE: "1", + ELECTRON_RUN_AS_NODE: null, PASEO_DESKTOP_MANAGED: null, PASEO_NODE_ENV: null, PASEO_SUPERVISED: null, @@ -180,4 +182,37 @@ describe("execCommand", () => { PASEO_SUPERVISED: "1", }); }); + + test("does not realpath commands while finalizing external command env", async () => { + const realpathSpy = vi.spyOn(fs.realpathSync, "native"); + + await execCommand("/some/random/binary", ["--version"], { + env: { + PATH: process.env.PATH, + }, + timeout: 100, + }).catch(() => {}); + + expect(realpathSpy).not.toHaveBeenCalled(); + }); + + test("self node command explicitly enables Electron node mode", async () => { + const command = buildSelfNodeCommand(["-e", printEnvScript], { + CUSTOM: "from-helper", + }); + + const result = await execCommand(command.command, command.args, { + env: command.env, + envMode: "internal", + }); + + expect(parsePrintedEnv(result.stdout)).toEqual({ + CUSTOM: "from-helper", + ELECTRON_NO_ATTACH_CONSOLE: null, + ELECTRON_RUN_AS_NODE: "1", + PASEO_DESKTOP_MANAGED: null, + PASEO_NODE_ENV: null, + PASEO_SUPERVISED: null, + }); + }); }); diff --git a/packages/server/src/utils/spawn.ts b/packages/server/src/utils/spawn.ts index c8073c4e1..52e10653d 100644 --- a/packages/server/src/utils/spawn.ts +++ b/packages/server/src/utils/spawn.ts @@ -23,8 +23,10 @@ export type SpawnProcessOptions = Omit & ExternalEnvOptions interface ExecCommandOptions extends ExternalEnvOptions { cwd?: string; encoding?: BufferEncoding; + killSignal?: NodeJS.Signals; timeout?: number; maxBuffer?: number; + shell?: boolean | string; } interface ExecCommandResult { @@ -87,7 +89,7 @@ export async function execCommand( const { baseEnv, env, envOverlay } = options ?? {}; const resolvedBaseEnv = env ?? baseEnv ?? process.env; const isWindows = process.platform === "win32"; - const shell = shouldUseWindowsShell(command); + const shell = shouldUseWindowsShell(command, options?.shell); const shouldQuoteForShell = isWindows && shell !== false; const resolvedCommand = shouldQuoteForShell ? quoteWindowsCommand(command) : command; const resolvedArgs = shouldQuoteForShell ? args.map(quoteWindowsArgument) : args; @@ -104,6 +106,7 @@ export async function execCommand( cwd: options?.cwd, env: childEnv, encoding: options?.encoding ?? "utf8", + killSignal: options?.killSignal, timeout: options?.timeout, maxBuffer: options?.maxBuffer, shell,