diff --git a/packages/server/src/server/agent/provider-launch-config.test.ts b/packages/server/src/server/agent/provider-launch-config.test.ts index a2b674558..8cb7aca71 100644 --- a/packages/server/src/server/agent/provider-launch-config.test.ts +++ b/packages/server/src/server/agent/provider-launch-config.test.ts @@ -1,32 +1,11 @@ -import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { describe, expect, test, vi } from "vitest"; import { - findExecutable, - quoteWindowsArgument, - quoteWindowsCommand, resolveProviderCommandPrefix, applyProviderEnv, type ProviderRuntimeSettings, } from "./provider-launch-config.js"; -type FindExecutableDependencies = NonNullable[1]>; - -function createFindExecutableDependencies(): FindExecutableDependencies { - return { - execFileSync: vi.fn(), - execSync: vi.fn(), - existsSync: vi.fn(), - platform: vi.fn(() => "darwin"), - shell: undefined, - }; -} - -let findExecutableDependencies: FindExecutableDependencies; - -beforeEach(() => { - findExecutableDependencies = createFindExecutableDependencies(); -}); - describe("resolveProviderCommandPrefix", () => { test("uses resolved default command in default mode", () => { const resolveDefault = vi.fn(() => "/usr/local/bin/claude"); @@ -134,161 +113,3 @@ describe("applyProviderEnv", () => { expect(env.CLAUDE_CODE_ENABLE_SDK_FILE_CHECKPOINTING).toBeUndefined(); }); }); - -describe("findExecutable", () => { - test("on Windows, resolves executables using current machine and user PATH entries", () => { - findExecutableDependencies.platform = vi.fn(() => "win32"); - process.env.Path = "C:\\Windows\\System32"; - findExecutableDependencies.execFileSync.mockImplementation( - ((command: string, args?: string[]) => { - if (command === "powershell") { - return "C:\\Windows\\System32\r\nC:\\Users\\boudr\\.local\\bin\r\n"; - } - if (command === "where.exe") { - return "C:\\Users\\boudr\\.local\\bin\\claude.exe\r\n"; - } - throw new Error(`unexpected command ${command}`); - }) as any, - ); - - expect(findExecutable("claude", findExecutableDependencies)).toBe( - "C:\\Users\\boudr\\.local\\bin\\claude.exe", - ); - const powershellCall = findExecutableDependencies.execFileSync.mock.calls[0]; - expect(powershellCall?.[0]).toBe("powershell"); - expect(powershellCall?.[1]).toContain("-NoProfile"); - expect(powershellCall?.[1]).toContain("-NonInteractive"); - expect(powershellCall?.[1]).toContain( - '$machine = [Environment]::GetEnvironmentVariable("Path", "Machine"); $user = [Environment]::GetEnvironmentVariable("Path", "User"); if ($machine) { Write-Output $machine }; if ($user) { Write-Output $user }', - ); - const whereCall = findExecutableDependencies.execFileSync.mock.calls[1]; - expect(whereCall?.[0]).toBe("where.exe"); - expect(whereCall?.[1]).toEqual(["claude"]); - expect(whereCall?.[2]?.encoding).toBe("utf8"); - const env = whereCall?.[2]?.env as Record; - expect(env.PATH).toContain("C:\\Users\\boudr\\.local\\bin"); - expect(env.Path).toContain("C:\\Users\\boudr\\.local\\bin"); - }); - - test("on Windows, preserves the first where.exe match", () => { - findExecutableDependencies.platform = vi.fn(() => "win32"); - process.env.Path = "C:\\Windows\\System32"; - findExecutableDependencies.execFileSync.mockImplementation( - ((command: string) => { - if (command === "powershell") { - return "C:\\Windows\\System32\r\nC:\\nvm4w\\nodejs\r\n"; - } - if (command === "where.exe") { - return "C:\\nvm4w\\nodejs\\codex\r\nC:\\nvm4w\\nodejs\\codex.cmd\r\n"; - } - throw new Error(`unexpected command ${command}`); - }) as any, - ); - - expect(findExecutable("codex", findExecutableDependencies)).toBe("C:\\nvm4w\\nodejs\\codex"); - }); - - test("uses the last line from login-shell which output", () => { - findExecutableDependencies.shell = "/bin/zsh"; - findExecutableDependencies.execSync.mockReturnValue( - "echo from profile\n/usr/local/bin/codex\n", - ); - - expect(findExecutable("codex", findExecutableDependencies)).toBe("/usr/local/bin/codex"); - expect(findExecutableDependencies.execSync).toHaveBeenCalledOnce(); - expect(findExecutableDependencies.execFileSync).not.toHaveBeenCalled(); - }); - - test("warns and returns null when the final which line is not an absolute path", () => { - findExecutableDependencies.shell = "/bin/zsh"; - findExecutableDependencies.execSync.mockReturnValue("profile noise\ncodex\n"); - findExecutableDependencies.execFileSync.mockReturnValue("codex\n"); - const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); - - expect(findExecutable("codex", findExecutableDependencies)).toBeNull(); - expect(warnSpy).toHaveBeenCalledTimes(2); - - warnSpy.mockRestore(); - }); - - test("returns direct paths when they exist", () => { - findExecutableDependencies.existsSync.mockReturnValue(true); - - expect(findExecutable("/usr/local/bin/codex", findExecutableDependencies)).toBe( - "/usr/local/bin/codex", - ); - expect(findExecutableDependencies.existsSync).toHaveBeenCalledWith("/usr/local/bin/codex"); - }); -}); - -describe("quoteWindowsCommand", () => { - const originalPlatform = process.platform; - - function setPlatform(value: string) { - Object.defineProperty(process, "platform", { value, writable: true }); - } - - afterEach(() => { - setPlatform(originalPlatform); - }); - - test("quotes a Windows path with spaces", () => { - setPlatform("win32"); - expect(quoteWindowsCommand("C:\\Program Files\\Anthropic\\claude.exe")).toBe( - '"C:\\Program Files\\Anthropic\\claude.exe"', - ); - }); - - test("does not double-quote an already-quoted path", () => { - setPlatform("win32"); - 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", () => { - setPlatform("win32"); - expect(quoteWindowsCommand("C:\\nvm4w\\nodejs\\codex")).toBe("C:\\nvm4w\\nodejs\\codex"); - }); - - test("returns the command unchanged on non-Windows platforms", () => { - setPlatform("darwin"); - expect(quoteWindowsCommand("/usr/local/bin/claude code")).toBe("/usr/local/bin/claude code"); - }); -}); - -describe("quoteWindowsArgument", () => { - const originalPlatform = process.platform; - - function setPlatform(value: string) { - Object.defineProperty(process, "platform", { value, writable: true }); - } - - afterEach(() => { - setPlatform(originalPlatform); - }); - - test("quotes a Windows argument with spaces", () => { - setPlatform("win32"); - expect(quoteWindowsArgument("C:\\Program Files\\Anthropic\\cli.js")).toBe( - '"C:\\Program Files\\Anthropic\\cli.js"', - ); - }); - - test("does not double-quote an already-quoted argument", () => { - setPlatform("win32"); - 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", () => { - setPlatform("win32"); - expect(quoteWindowsArgument("--version")).toBe("--version"); - }); - - test("returns the argument unchanged on non-Windows platforms", () => { - setPlatform("darwin"); - expect(quoteWindowsArgument("/usr/local/bin/claude code")).toBe("/usr/local/bin/claude code"); - }); -}); diff --git a/packages/server/src/server/agent/provider-launch-config.ts b/packages/server/src/server/agent/provider-launch-config.ts index b20ad8a86..facd7671a 100644 --- a/packages/server/src/server/agent/provider-launch-config.ts +++ b/packages/server/src/server/agent/provider-launch-config.ts @@ -1,10 +1,9 @@ -import { execFileSync, execSync } from "node:child_process"; -import { existsSync } from "node:fs"; -import { platform } from "node:os"; -import path from "node:path"; -import { shellEnvSync } from "shell-env"; import { z } from "zod"; +import { + isCommandAvailable, + resolveShellEnv, +} from "../../utils/executable.js"; import type { AgentProvider } from "./agent-sdk-types.js"; import { AgentProviderSchema } from "./provider-manifest.js"; @@ -57,66 +56,6 @@ export type ProviderCommandPrefix = { args: string[]; }; -interface FindExecutableDependencies { - execSync: typeof execSync; - execFileSync: typeof execFileSync; - existsSync: typeof existsSync; - platform: typeof platform; - shell: string | undefined; -} - -function resolveWindowsPathEntries(deps: FindExecutableDependencies): string[] { - try { - const output = deps.execFileSync( - "powershell", - [ - "-NoProfile", - "-NonInteractive", - "-Command", - [ - '$machine = [Environment]::GetEnvironmentVariable("Path", "Machine")', - '$user = [Environment]::GetEnvironmentVariable("Path", "User")', - "if ($machine) { Write-Output $machine }", - "if ($user) { Write-Output $user }", - ].join("; "), - ], - { encoding: "utf8" }, - ); - return output - .split(/\r?\n/) - .flatMap((line) => line.split(";")) - .map((entry) => entry.trim()) - .filter((entry) => entry.length > 0); - } catch { - return []; - } -} - -function resolveExecutableFromWhichOutput( - name: string, - output: string, - source: "login-shell" | "which", -): string | null { - const lines = output - .split(/\r?\n/) - .map((line) => line.trim()) - .filter((line) => line.length > 0); - const candidate = lines.at(-1); - - if (!candidate) { - return null; - } - - if (!path.isAbsolute(candidate)) { - console.warn( - `[findExecutable] Ignoring non-absolute ${source} output for '${name}': ${JSON.stringify(candidate)}`, - ); - return null; - } - - return candidate; -} - export function resolveProviderCommandPrefix( commandConfig: ProviderCommand | undefined, resolveDefaultCommand: () => string, @@ -141,18 +80,6 @@ export function resolveProviderCommandPrefix( }; } -let cachedShellEnv: Record | null = null; - -export function resolveShellEnv(): Record { - if (cachedShellEnv) return cachedShellEnv; - try { - cachedShellEnv = shellEnvSync(); - } catch { - cachedShellEnv = { ...process.env } as Record; - } - return cachedShellEnv; -} - // Env vars that indicate a running Claude Code session. If the daemon itself is // launched from inside Claude Code (e.g. by a Paseo agent), these leak into // child processes and cause "cannot be launched inside another session" errors. @@ -180,128 +107,6 @@ export function applyProviderEnv( return merged; } -/** - * Resolve an executable name to its absolute path the way the user's shell would. - * - * On Unix we first try `$SHELL -lic "which "` so that rc-file PATH - * additions (asdf, nvm, homebrew, nix, etc.) are visible — exactly as if the - * user opened a terminal and typed the command. If that fails (e.g. the login - * shell itself errors) we fall back to a plain `which`. - * - * On Windows we augment the daemon PATH with machine/user registry PATH values - * and return the first `where.exe` match. Launch-time execution decides whether - * the resolved path needs `cmd.exe` semantics (for example npm shims under - * nvm4w such as `C:\nvm4w\nodejs\codex`). - */ -export function findExecutable( - name: string, - dependencies?: FindExecutableDependencies, -): string | null { - const trimmed = name.trim(); - if (!trimmed) { - return null; - } - - const deps: FindExecutableDependencies = { - execSync, - execFileSync, - existsSync, - platform, - shell: process.env["SHELL"], - ...dependencies, - }; - - if (trimmed.includes("/") || trimmed.includes("\\")) { - return deps.existsSync(trimmed) ? trimmed : null; - } - - if (deps.platform() === "win32") { - try { - const inheritedPath = process.env["Path"] ?? process.env["PATH"] ?? ""; - const resolvedPath = [ - ...inheritedPath.split(";"), - ...resolveWindowsPathEntries(deps), - ] - .map((entry) => entry.trim()) - .filter((entry) => entry.length > 0) - .filter((entry, index, entries) => entries.indexOf(entry) === index) - .join(";"); - const env = { - ...process.env, - PATH: resolvedPath, - Path: resolvedPath, - }; - const out = deps.execFileSync("where.exe", [trimmed], { encoding: "utf8", env }).trim(); - return ( - out - .split(/\r?\n/) - .map((line) => line.trim()) - .find((line) => line.length > 0) ?? null - ); - } catch { - return null; - } - } - - // Unix: try the user's login shell so rc-file PATH entries are visible. - const shell = deps.shell; - if (shell) { - try { - const out = deps - .execSync(`${shell} -lic "which ${trimmed}"`, { - encoding: "utf8", - timeout: 5000, - }) - .trim(); - const resolved = resolveExecutableFromWhichOutput(trimmed, out, "login-shell"); - if (resolved) { - return resolved; - } - } catch { - // Login shell failed (broken rc, etc.) — fall through to plain which. - } - } - - try { - return resolveExecutableFromWhichOutput( - trimmed, - deps.execFileSync("which", [trimmed], { encoding: "utf8" }).trim(), - "which", - ); - } catch { - return null; - } -} - -/** - * When spawning with `shell: true` on Windows, the command is passed to - * `cmd.exe /d /s /c "command args"`. The `/s` strips outer quotes, so a - * command path with spaces (e.g. `C:\Program Files\...`) is split at the - * space. Wrapping it in quotes produces the correct `"C:\Program Files\..." args`. - */ -export function quoteWindowsCommand(command: string): string { - if (process.platform !== "win32") return command; - if (!command.includes(" ")) return command; - if (command.startsWith('"') && command.endsWith('"')) return command; - return `"${command}"`; -} - -/** - * `spawn(..., { shell: true })` on Windows also passes argv through `cmd.exe`. - * Any argument containing spaces must be quoted or it will be split before the - * child process sees it. - */ -export function quoteWindowsArgument(argument: string): string { - if (process.platform !== "win32") return argument; - if (!argument.includes(" ")) return argument; - if (argument.startsWith('"') && argument.endsWith('"')) return argument; - return `"${argument}"`; -} - -export function isCommandAvailable(command: string): boolean { - return findExecutable(command) !== null; -} - export function isProviderCommandAvailable( commandConfig: ProviderCommand | undefined, resolveDefaultCommand: () => string, diff --git a/packages/server/src/server/agent/providers/__tests__/claude-agent.event-stream.integration.test.ts b/packages/server/src/server/agent/providers/__tests__/claude-agent.event-stream.integration.test.ts index 4d7f6cded..14c684297 100644 --- a/packages/server/src/server/agent/providers/__tests__/claude-agent.event-stream.integration.test.ts +++ b/packages/server/src/server/agent/providers/__tests__/claude-agent.event-stream.integration.test.ts @@ -17,7 +17,7 @@ import path from "node:path"; import pino from "pino"; import type { AgentSession, AgentStreamEvent } from "../../agent-sdk-types.js"; -import { isCommandAvailable } from "../../provider-launch-config.js"; +import { isCommandAvailable } from "../../../../utils/executable.js"; import { ClaudeAgentClient } from "../claude-agent.js"; // --------------------------------------------------------------------------- diff --git a/packages/server/src/server/agent/providers/acp-agent.ts b/packages/server/src/server/agent/providers/acp-agent.ts index fe40c125d..c288a9315 100644 --- a/packages/server/src/server/agent/providers/acp-agent.ts +++ b/packages/server/src/server/agent/providers/acp-agent.ts @@ -83,10 +83,10 @@ import type { } from "../agent-sdk-types.js"; import { applyProviderEnv, - findExecutable, resolveProviderCommandPrefix, type ProviderRuntimeSettings, } from "../provider-launch-config.js"; +import { findExecutable } from "../../../utils/executable.js"; const DEFAULT_ACP_CAPABILITIES: AgentCapabilityFlags = { supportsStreaming: true, diff --git a/packages/server/src/server/agent/providers/claude-agent-commands.real.e2e.test.ts b/packages/server/src/server/agent/providers/claude-agent-commands.real.e2e.test.ts index 846774483..baf548bfe 100644 --- a/packages/server/src/server/agent/providers/claude-agent-commands.real.e2e.test.ts +++ b/packages/server/src/server/agent/providers/claude-agent-commands.real.e2e.test.ts @@ -1,8 +1,8 @@ import { describe, expect, test } from "vitest"; import pino from "pino"; -import { isCommandAvailable } from "../provider-launch-config.js"; import type { AgentSlashCommand } from "../agent-sdk-types.js"; +import { isCommandAvailable } from "../../../utils/executable.js"; import { ClaudeAgentClient } from "./claude-agent.js"; // Real-Claude contract coverage: validates slash command shape from a live Claude CLI session. diff --git a/packages/server/src/server/agent/providers/claude-agent.integration.test.ts b/packages/server/src/server/agent/providers/claude-agent.integration.test.ts index e109d47e4..90c7d2ca4 100644 --- a/packages/server/src/server/agent/providers/claude-agent.integration.test.ts +++ b/packages/server/src/server/agent/providers/claude-agent.integration.test.ts @@ -6,7 +6,7 @@ import pino from "pino"; import { query, type SDKUserMessage } from "@anthropic-ai/claude-agent-sdk"; import type { AgentSession, AgentStreamEvent, ToolCallTimelineItem } from "../agent-sdk-types.js"; -import { isCommandAvailable } from "../provider-launch-config.js"; +import { isCommandAvailable } from "../../../utils/executable.js"; import { ClaudeAgentClient } from "./claude-agent.js"; import { streamSession } from "./test-utils/session-stream-adapter.js"; diff --git a/packages/server/src/server/agent/providers/claude-agent.max-effort.real.e2e.test.ts b/packages/server/src/server/agent/providers/claude-agent.max-effort.real.e2e.test.ts index 29bcd0eda..34f5fe5b0 100644 --- a/packages/server/src/server/agent/providers/claude-agent.max-effort.real.e2e.test.ts +++ b/packages/server/src/server/agent/providers/claude-agent.max-effort.real.e2e.test.ts @@ -2,7 +2,7 @@ import { describe, expect, test } from "vitest"; import pino from "pino"; import type { AgentStreamEvent, AgentSession } from "../agent-sdk-types.js"; -import { isCommandAvailable } from "../provider-launch-config.js"; +import { isCommandAvailable } from "../../../utils/executable.js"; import { ClaudeAgentClient } from "./claude-agent.js"; import { streamSession } from "./test-utils/session-stream-adapter.js"; diff --git a/packages/server/src/server/agent/providers/claude-agent.ts b/packages/server/src/server/agent/providers/claude-agent.ts index ae72ba30a..e1b4e8b19 100644 --- a/packages/server/src/server/agent/providers/claude-agent.ts +++ b/packages/server/src/server/agent/providers/claude-agent.ts @@ -69,11 +69,13 @@ import type { } from "../agent-sdk-types.js"; import { applyProviderEnv, + type ProviderRuntimeSettings, +} from "../provider-launch-config.js"; +import { findExecutable, quoteWindowsArgument, quoteWindowsCommand, - type ProviderRuntimeSettings, -} from "../provider-launch-config.js"; +} from "../../../utils/executable.js"; import { getOrchestratorModeInstructions } from "../orchestrator-instructions.js"; const fsPromises = promises; diff --git a/packages/server/src/server/agent/providers/codex-app-server-agent.ts b/packages/server/src/server/agent/providers/codex-app-server-agent.ts index c0052a14f..bd9bbdff4 100644 --- a/packages/server/src/server/agent/providers/codex-app-server-agent.ts +++ b/packages/server/src/server/agent/providers/codex-app-server-agent.ts @@ -41,12 +41,14 @@ import { } from "./codex/tool-call-mapper.js"; import { applyProviderEnv, - findExecutable, - quoteWindowsArgument, - quoteWindowsCommand, resolveProviderCommandPrefix, type ProviderRuntimeSettings, } from "../provider-launch-config.js"; +import { + findExecutable, + quoteWindowsArgument, + quoteWindowsCommand, +} from "../../../utils/executable.js"; import { extractCodexTerminalSessionId, nonEmptyString } from "./tool-call-mapper-utils.js"; const DEFAULT_TIMEOUT_MS = 14 * 24 * 60 * 60 * 1000; diff --git a/packages/server/src/server/agent/providers/opencode-agent-commands.real.e2e.test.ts b/packages/server/src/server/agent/providers/opencode-agent-commands.real.e2e.test.ts index dcc2b97ea..ca9a49ae6 100644 --- a/packages/server/src/server/agent/providers/opencode-agent-commands.real.e2e.test.ts +++ b/packages/server/src/server/agent/providers/opencode-agent-commands.real.e2e.test.ts @@ -1,8 +1,8 @@ import { describe, expect, test } from "vitest"; import pino from "pino"; -import { isCommandAvailable } from "../provider-launch-config.js"; import type { AgentSlashCommand } from "../agent-sdk-types.js"; +import { isCommandAvailable } from "../../../utils/executable.js"; import { OpenCodeAgentClient } from "./opencode-agent.js"; describe("opencode agent commands contract (real)", () => { diff --git a/packages/server/src/server/agent/providers/opencode-agent.ts b/packages/server/src/server/agent/providers/opencode-agent.ts index a6ea4ab5a..d0f47b3e5 100644 --- a/packages/server/src/server/agent/providers/opencode-agent.ts +++ b/packages/server/src/server/agent/providers/opencode-agent.ts @@ -34,12 +34,14 @@ import type { } from "../agent-sdk-types.js"; import { applyProviderEnv, - findExecutable, - quoteWindowsArgument, - quoteWindowsCommand, resolveProviderCommandPrefix, type ProviderRuntimeSettings, } from "../provider-launch-config.js"; +import { + findExecutable, + quoteWindowsArgument, + quoteWindowsCommand, +} from "../../../utils/executable.js"; import { mapOpencodeToolCall } from "./opencode/tool-call-mapper.js"; const OPENCODE_CAPABILITIES: AgentCapabilityFlags = { diff --git a/packages/server/src/server/daemon-e2e/agent-configs.ts b/packages/server/src/server/daemon-e2e/agent-configs.ts index 803450f26..82bfd805a 100644 --- a/packages/server/src/server/daemon-e2e/agent-configs.ts +++ b/packages/server/src/server/daemon-e2e/agent-configs.ts @@ -7,7 +7,7 @@ import { join, dirname, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { homedir } from "node:os"; import dotenv from "dotenv"; -import { isCommandAvailable } from "../agent/provider-launch-config.js"; +import { isCommandAvailable } from "../../utils/executable.js"; // Load .env.test eagerly so isProviderAvailable() has credentials at collection time. const serverRoot = resolve(dirname(fileURLToPath(import.meta.url)), "../../.."); diff --git a/packages/server/src/server/exports.ts b/packages/server/src/server/exports.ts index cc07163ba..c617066d3 100644 --- a/packages/server/src/server/exports.ts +++ b/packages/server/src/server/exports.ts @@ -31,11 +31,9 @@ export { // Provider binary resolution export { - findExecutable, applyProviderEnv, - quoteWindowsArgument, - quoteWindowsCommand, } from "./agent/provider-launch-config.js"; +export { findExecutable, quoteWindowsArgument, quoteWindowsCommand } from "../utils/executable.js"; // Provider manifest (source of truth for provider definitions) export { diff --git a/packages/server/src/utils/checkout-git.test.ts b/packages/server/src/utils/checkout-git.test.ts index 26904e40a..77e416669 100644 --- a/packages/server/src/utils/checkout-git.test.ts +++ b/packages/server/src/utils/checkout-git.test.ts @@ -12,7 +12,9 @@ import { import { join } from "path"; import { tmpdir } from "os"; import { + __resetGhPathCacheForTests, __resetPullRequestStatusCacheForTests, + __setGhPathForTests, __setPullRequestStatusCacheTtlForTests, commitAll, getCheckoutDiff, @@ -62,10 +64,12 @@ describe("checkout git utilities", () => { tempDir = setup.tempDir; repoDir = setup.repoDir; paseoHome = join(tempDir, "paseo-home"); + __resetGhPathCacheForTests(); __resetPullRequestStatusCacheForTests(); }); afterEach(() => { + __resetGhPathCacheForTests(); __resetPullRequestStatusCacheForTests(); rmSync(tempDir, { recursive: true, force: true }); }); @@ -582,18 +586,14 @@ const x = 1; const gitPath = execSync("command -v git", { stdio: "pipe" }).toString().trim(); symlinkSync(gitPath, join(fakeBinDir, "git")); - const originalPath = process.env.PATH; - process.env.PATH = fakeBinDir; try { + __setGhPathForTests(null); const status = await getPullRequestStatus(repoDir); expect(status.githubFeaturesEnabled).toBe(false); expect(status.status).toBeNull(); } finally { - if (originalPath === undefined) { - delete process.env.PATH; - } else { - process.env.PATH = originalPath; - } + __resetGhPathCacheForTests(); + __resetPullRequestStatusCacheForTests(); } }); @@ -627,9 +627,8 @@ const x = 1; ); execSync(`chmod +x ${join(fakeBinDir, "gh")}`); - const originalPath = process.env.PATH; - process.env.PATH = `${fakeBinDir}:${originalPath ?? ""}`; try { + __setGhPathForTests(join(fakeBinDir, "gh")); const status = await getPullRequestStatus(repoDir); expect(status.githubFeaturesEnabled).toBe(true); expect(status.status).not.toBeNull(); @@ -639,11 +638,8 @@ const x = 1; expect(status.status?.isMerged).toBe(true); expect(status.status?.state).toBe("merged"); } finally { - if (originalPath === undefined) { - delete process.env.PATH; - } else { - process.env.PATH = originalPath; - } + __resetGhPathCacheForTests(); + __resetPullRequestStatusCacheForTests(); } }); @@ -677,9 +673,8 @@ const x = 1; ); execSync(`chmod +x ${join(fakeBinDir, "gh")}`); - const originalPath = process.env.PATH; - process.env.PATH = `${fakeBinDir}:${originalPath ?? ""}`; try { + __setGhPathForTests(join(fakeBinDir, "gh")); const status = await getPullRequestStatus(repoDir); expect(status.githubFeaturesEnabled).toBe(true); expect(status.status).not.toBeNull(); @@ -689,11 +684,8 @@ const x = 1; expect(status.status?.isMerged).toBe(false); expect(status.status?.state).toBe("closed"); } finally { - if (originalPath === undefined) { - delete process.env.PATH; - } else { - process.env.PATH = originalPath; - } + __resetGhPathCacheForTests(); + __resetPullRequestStatusCacheForTests(); } }); @@ -732,20 +724,16 @@ const x = 1; ); execSync(`chmod +x ${join(fakeBinDir, "gh")}`); - const originalPath = process.env.PATH; - process.env.PATH = `${fakeBinDir}:${originalPath ?? ""}`; try { + __setGhPathForTests(join(fakeBinDir, "gh")); const first = await getPullRequestStatus(repoDir); const second = await getPullRequestStatus(repoDir); expect(first).toEqual(second); expect(first.status?.url).toContain("/pull/123"); expect(readFileSync(callCountPath, "utf8").trim()).toBe("1"); } finally { - if (originalPath === undefined) { - delete process.env.PATH; - } else { - process.env.PATH = originalPath; - } + __resetGhPathCacheForTests(); + __resetPullRequestStatusCacheForTests(); } }); @@ -785,10 +773,9 @@ const x = 1; ); execSync(`chmod +x ${join(fakeBinDir, "gh")}`); - const originalPath = process.env.PATH; - process.env.PATH = `${fakeBinDir}:${originalPath ?? ""}`; __setPullRequestStatusCacheTtlForTests(50); try { + __setGhPathForTests(join(fakeBinDir, "gh")); const first = await getPullRequestStatus(repoDir); await sleep(80); const second = await getPullRequestStatus(repoDir); @@ -796,11 +783,8 @@ const x = 1; expect(second.status?.url).toContain("/pull/2"); expect(readFileSync(callCountPath, "utf8").trim()).toBe("2"); } finally { - if (originalPath === undefined) { - delete process.env.PATH; - } else { - process.env.PATH = originalPath; - } + __resetGhPathCacheForTests(); + __resetPullRequestStatusCacheForTests(); } }); @@ -840,9 +824,8 @@ const x = 1; ); execSync(`chmod +x ${join(fakeBinDir, "gh")}`); - const originalPath = process.env.PATH; - process.env.PATH = `${fakeBinDir}:${originalPath ?? ""}`; try { + __setGhPathForTests(join(fakeBinDir, "gh")); const [first, second] = await Promise.all([ getPullRequestStatus(repoDir), getPullRequestStatus(repoDir), @@ -850,11 +833,8 @@ const x = 1; expect(first).toEqual(second); expect(readFileSync(callCountPath, "utf8").trim()).toBe("1"); } finally { - if (originalPath === undefined) { - delete process.env.PATH; - } else { - process.env.PATH = originalPath; - } + __resetGhPathCacheForTests(); + __resetPullRequestStatusCacheForTests(); } }); diff --git a/packages/server/src/utils/checkout-git.ts b/packages/server/src/utils/checkout-git.ts index 1fbda4393..7324e7607 100644 --- a/packages/server/src/utils/checkout-git.ts +++ b/packages/server/src/utils/checkout-git.ts @@ -6,6 +6,7 @@ import { open as openFile, stat as statFile } from "fs/promises"; import { TTLCache } from "@isaacs/ttlcache"; import type { ParsedDiffFile } from "../server/utils/diff-highlighter.js"; import { parseAndHighlightDiff } from "../server/utils/diff-highlighter.js"; +import { findExecutable, resolveShellEnv } from "./executable.js"; import { isPaseoOwnedWorktreeCwd } from "./worktree.js"; import { requirePaseoWorktreeBaseRefName } from "./worktree-metadata.js"; @@ -23,6 +24,7 @@ const PULL_REQUEST_STATUS_CACHE_MAX = 1_000; let pullRequestStatusCacheTtlMs = DEFAULT_PULL_REQUEST_STATUS_CACHE_TTL_MS; let pullRequestStatusCache = createPullRequestStatusCache(pullRequestStatusCacheTtlMs); const pullRequestStatusInFlight = new Map>(); +let cachedGhPath: string | null | undefined = undefined; function createPullRequestStatusCache(ttlMs: number) { return new TTLCache({ @@ -52,6 +54,14 @@ export function __setPullRequestStatusCacheTtlForTests(ttlMs: number): void { pullRequestStatusInFlight.clear(); } +export function __resetGhPathCacheForTests(): void { + cachedGhPath = undefined; +} + +export function __setGhPathForTests(path: string | null): void { + cachedGhPath = path; +} + async function execGit( command: string, options: { cwd: string; env?: NodeJS.ProcessEnv }, @@ -1731,12 +1741,14 @@ export interface PullRequestStatusResult { githubFeaturesEnabled: boolean; } -async function ensureGhAvailable(cwd: string): Promise { - try { - await execAsync("gh --version", { cwd }); - } catch { - throw new Error("GitHub CLI (gh) is not available or not authenticated"); +function resolveGhPath(): string { + if (cachedGhPath === undefined) { + cachedGhPath = findExecutable("gh"); } + if (cachedGhPath === null) { + throw new Error("GitHub CLI (gh) is not installed or not in PATH"); + } + return cachedGhPath; } function getCommandErrorText(error: unknown): string { @@ -1805,7 +1817,7 @@ export async function createPullRequest( options: CreatePullRequestOptions, ): Promise<{ url: string; number: number }> { await requireGitRepo(cwd); - await ensureGhAvailable(cwd); + const ghPath = resolveGhPath(); const repo = await resolveGitHubRepo(cwd); if (!repo) { throw new Error("Unable to determine GitHub repo from git remote"); @@ -1827,13 +1839,14 @@ export async function createPullRequest( await execAsync(`git push -u origin ${head}`, { cwd }); + const ghEnv = { ...resolveShellEnv(), GIT_TERMINAL_PROMPT: "0" }; const args = ["api", "-X", "POST", `repos/${repo}/pulls`, "-f", `title=${options.title}`]; args.push("-f", `head=${head}`); args.push("-f", `base=${normalizedBase}`); if (options.body) { args.push("-f", `body=${options.body}`); } - const { stdout } = await execFileAsync("gh", args, { cwd }); + const { stdout } = await execFileAsync(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"); @@ -1875,8 +1888,9 @@ async function getPullRequestStatusUncached(cwd: string): Promise[1]>; + +function createFindExecutableDependencies(): FindExecutableDependencies { + return { + execFileSync: vi.fn(), + execSync: vi.fn(), + existsSync: vi.fn(), + platform: vi.fn(() => "darwin"), + shell: undefined, + }; +} + +let findExecutableDependencies: FindExecutableDependencies; + +beforeEach(() => { + findExecutableDependencies = createFindExecutableDependencies(); +}); + +describe("findExecutable", () => { + test("on Windows, resolves executables using current machine and user PATH entries", () => { + findExecutableDependencies.platform = vi.fn(() => "win32"); + process.env.Path = "C:\\Windows\\System32"; + findExecutableDependencies.execFileSync.mockImplementation( + ((command: string, args?: string[]) => { + if (command === "powershell") { + return "C:\\Windows\\System32\r\nC:\\Users\\boudr\\.local\\bin\r\n"; + } + if (command === "where.exe") { + return "C:\\Users\\boudr\\.local\\bin\\claude.exe\r\n"; + } + throw new Error(`unexpected command ${command}`); + }) as any, + ); + + expect(findExecutable("claude", findExecutableDependencies)).toBe( + "C:\\Users\\boudr\\.local\\bin\\claude.exe", + ); + const powershellCall = findExecutableDependencies.execFileSync.mock.calls[0]; + expect(powershellCall?.[0]).toBe("powershell"); + expect(powershellCall?.[1]).toContain("-NoProfile"); + expect(powershellCall?.[1]).toContain("-NonInteractive"); + expect(powershellCall?.[1]).toContain( + '$machine = [Environment]::GetEnvironmentVariable("Path", "Machine"); $user = [Environment]::GetEnvironmentVariable("Path", "User"); if ($machine) { Write-Output $machine }; if ($user) { Write-Output $user }', + ); + const whereCall = findExecutableDependencies.execFileSync.mock.calls[1]; + expect(whereCall?.[0]).toBe("where.exe"); + expect(whereCall?.[1]).toEqual(["claude"]); + expect(whereCall?.[2]?.encoding).toBe("utf8"); + const env = whereCall?.[2]?.env as Record; + expect(env.PATH).toContain("C:\\Users\\boudr\\.local\\bin"); + expect(env.Path).toContain("C:\\Users\\boudr\\.local\\bin"); + }); + + test("on Windows, preserves the first where.exe match", () => { + findExecutableDependencies.platform = vi.fn(() => "win32"); + process.env.Path = "C:\\Windows\\System32"; + findExecutableDependencies.execFileSync.mockImplementation( + ((command: string) => { + if (command === "powershell") { + return "C:\\Windows\\System32\r\nC:\\nvm4w\\nodejs\r\n"; + } + if (command === "where.exe") { + return "C:\\nvm4w\\nodejs\\codex\r\nC:\\nvm4w\\nodejs\\codex.cmd\r\n"; + } + throw new Error(`unexpected command ${command}`); + }) as any, + ); + + expect(findExecutable("codex", findExecutableDependencies)).toBe("C:\\nvm4w\\nodejs\\codex"); + }); + + test("uses the last line from login-shell which output", () => { + findExecutableDependencies.shell = "/bin/zsh"; + findExecutableDependencies.execSync.mockReturnValue( + "echo from profile\n/usr/local/bin/codex\n", + ); + + expect(findExecutable("codex", findExecutableDependencies)).toBe("/usr/local/bin/codex"); + expect(findExecutableDependencies.execSync).toHaveBeenCalledOnce(); + expect(findExecutableDependencies.execFileSync).not.toHaveBeenCalled(); + }); + + test("warns and returns null when the final which line is not an absolute path", () => { + findExecutableDependencies.shell = "/bin/zsh"; + findExecutableDependencies.execSync.mockReturnValue("profile noise\ncodex\n"); + findExecutableDependencies.execFileSync.mockReturnValue("codex\n"); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + + expect(findExecutable("codex", findExecutableDependencies)).toBeNull(); + expect(warnSpy).toHaveBeenCalledTimes(2); + + warnSpy.mockRestore(); + }); + + test("returns direct paths when they exist", () => { + findExecutableDependencies.existsSync.mockReturnValue(true); + + expect(findExecutable("/usr/local/bin/codex", findExecutableDependencies)).toBe( + "/usr/local/bin/codex", + ); + expect(findExecutableDependencies.existsSync).toHaveBeenCalledWith("/usr/local/bin/codex"); + }); +}); + +describe("quoteWindowsCommand", () => { + const originalPlatform = process.platform; + + function setPlatform(value: string) { + Object.defineProperty(process, "platform", { value, writable: true }); + } + + afterEach(() => { + setPlatform(originalPlatform); + }); + + test("quotes a Windows path with spaces", () => { + setPlatform("win32"); + expect(quoteWindowsCommand("C:\\Program Files\\Anthropic\\claude.exe")).toBe( + '"C:\\Program Files\\Anthropic\\claude.exe"', + ); + }); + + test("does not double-quote an already-quoted path", () => { + setPlatform("win32"); + 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", () => { + setPlatform("win32"); + expect(quoteWindowsCommand("C:\\nvm4w\\nodejs\\codex")).toBe("C:\\nvm4w\\nodejs\\codex"); + }); + + test("returns the command unchanged on non-Windows platforms", () => { + setPlatform("darwin"); + expect(quoteWindowsCommand("/usr/local/bin/claude code")).toBe("/usr/local/bin/claude code"); + }); +}); + +describe("quoteWindowsArgument", () => { + const originalPlatform = process.platform; + + function setPlatform(value: string) { + Object.defineProperty(process, "platform", { value, writable: true }); + } + + afterEach(() => { + setPlatform(originalPlatform); + }); + + test("quotes a Windows argument with spaces", () => { + setPlatform("win32"); + expect(quoteWindowsArgument("C:\\Program Files\\Anthropic\\cli.js")).toBe( + '"C:\\Program Files\\Anthropic\\cli.js"', + ); + }); + + test("does not double-quote an already-quoted argument", () => { + setPlatform("win32"); + 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", () => { + setPlatform("win32"); + expect(quoteWindowsArgument("--version")).toBe("--version"); + }); + + test("returns the argument unchanged on non-Windows platforms", () => { + setPlatform("darwin"); + expect(quoteWindowsArgument("/usr/local/bin/claude code")).toBe("/usr/local/bin/claude code"); + }); +}); diff --git a/packages/server/src/utils/executable.ts b/packages/server/src/utils/executable.ts new file mode 100644 index 000000000..4c5fd61a8 --- /dev/null +++ b/packages/server/src/utils/executable.ts @@ -0,0 +1,197 @@ +import { execFileSync, execSync } from "node:child_process"; +import { existsSync } from "node:fs"; +import { platform } from "node:os"; +import path from "node:path"; +import { shellEnvSync } from "shell-env"; + +export interface FindExecutableDependencies { + execSync: typeof execSync; + execFileSync: typeof execFileSync; + existsSync: typeof existsSync; + platform: typeof platform; + shell: string | undefined; +} + +function resolveWindowsPathEntries(deps: FindExecutableDependencies): string[] { + try { + const output = deps.execFileSync( + "powershell", + [ + "-NoProfile", + "-NonInteractive", + "-Command", + [ + '$machine = [Environment]::GetEnvironmentVariable("Path", "Machine")', + '$user = [Environment]::GetEnvironmentVariable("Path", "User")', + "if ($machine) { Write-Output $machine }", + "if ($user) { Write-Output $user }", + ].join("; "), + ], + { encoding: "utf8" }, + ); + return output + .split(/\r?\n/) + .flatMap((line) => line.split(";")) + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0); + } catch { + return []; + } +} + +function resolveExecutableFromWhichOutput( + name: string, + output: string, + source: "login-shell" | "which", +): string | null { + const lines = output + .split(/\r?\n/) + .map((line) => line.trim()) + .filter((line) => line.length > 0); + const candidate = lines.at(-1); + + if (!candidate) { + return null; + } + + if (!path.isAbsolute(candidate)) { + console.warn( + `[findExecutable] Ignoring non-absolute ${source} output for '${name}': ${JSON.stringify(candidate)}`, + ); + return null; + } + + return candidate; +} + +/** + * On Unix we first try `$SHELL -lic "which "` so that rc-file PATH + * additions (asdf, nvm, homebrew, nix, etc.) are visible — exactly as if the + * user opened a terminal and typed the command. If that fails (e.g. the login + * shell itself errors) we fall back to a plain `which`. + * + * On Windows we augment the daemon PATH with machine/user registry PATH values + * and return the first `where.exe` match. Launch-time execution decides whether + * the resolved path needs `cmd.exe` semantics (for example npm shims under + * nvm4w such as `C:\nvm4w\nodejs\codex`). + */ +export function findExecutable( + name: string, + dependencies?: FindExecutableDependencies, +): string | null { + const trimmed = name.trim(); + if (!trimmed) { + return null; + } + + const deps: FindExecutableDependencies = { + execSync, + execFileSync, + existsSync, + platform, + shell: process.env["SHELL"], + ...dependencies, + }; + + if (trimmed.includes("/") || trimmed.includes("\\")) { + return deps.existsSync(trimmed) ? trimmed : null; + } + + if (deps.platform() === "win32") { + try { + const inheritedPath = process.env["Path"] ?? process.env["PATH"] ?? ""; + const resolvedPath = [ + ...inheritedPath.split(";"), + ...resolveWindowsPathEntries(deps), + ] + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0) + .filter((entry, index, entries) => entries.indexOf(entry) === index) + .join(";"); + const env = { + ...process.env, + PATH: resolvedPath, + Path: resolvedPath, + }; + const out = deps.execFileSync("where.exe", [trimmed], { encoding: "utf8", env }).trim(); + return ( + out + .split(/\r?\n/) + .map((line) => line.trim()) + .find((line) => line.length > 0) ?? null + ); + } catch { + return null; + } + } + + // Unix: try the user's login shell so rc-file PATH entries are visible. + const shell = deps.shell; + if (shell) { + try { + const out = deps + .execSync(`${shell} -lic "which ${trimmed}"`, { + encoding: "utf8", + timeout: 5000, + }) + .trim(); + const resolved = resolveExecutableFromWhichOutput(trimmed, out, "login-shell"); + if (resolved) { + return resolved; + } + } catch { + // Login shell failed (broken rc, etc.) — fall through to plain which. + } + } + + try { + return resolveExecutableFromWhichOutput( + trimmed, + deps.execFileSync("which", [trimmed], { encoding: "utf8" }).trim(), + "which", + ); + } catch { + return null; + } +} + +export function isCommandAvailable(command: string): boolean { + return findExecutable(command) !== null; +} + +/** + * When spawning with `shell: true` on Windows, the command is passed to + * `cmd.exe /d /s /c "command args"`. The `/s` strips outer quotes, so a + * command path with spaces (e.g. `C:\Program Files\...`) is split at the + * space. Wrapping it in quotes produces the correct `"C:\Program Files\..." args`. + */ +export function quoteWindowsCommand(command: string): string { + if (process.platform !== "win32") return command; + if (!command.includes(" ")) return command; + if (command.startsWith('"') && command.endsWith('"')) return command; + return `"${command}"`; +} + +/** + * `spawn(..., { shell: true })` on Windows also passes argv through `cmd.exe`. + * Any argument containing spaces must be quoted or it will be split before the + * child process sees it. + */ +export function quoteWindowsArgument(argument: string): string { + if (process.platform !== "win32") return argument; + if (!argument.includes(" ")) return argument; + if (argument.startsWith('"') && argument.endsWith('"')) return argument; + return `"${argument}"`; +} + +let cachedShellEnv: Record | null = null; + +export function resolveShellEnv(): Record { + if (cachedShellEnv) return cachedShellEnv; + try { + cachedShellEnv = shellEnvSync(); + } catch { + cachedShellEnv = { ...process.env } as Record; + } + return cachedShellEnv; +}