From f352072dace4d4b7bb23b9d6e280e0624db91a4f Mon Sep 17 00:00:00 2001 From: itMrBoy <58385353+itMrBoy@users.noreply.github.com> Date: Sun, 14 Jun 2026 19:48:42 +0800 Subject: [PATCH] Fix coding-agent terminal shortcuts not working on Windows (#1509) * Fix coding-agent terminal shortcuts not working on Windows On Windows, opening a Terminal profile (Claude Code / Codex / OpenCode) did nothing, and afterwards even a plain New Terminal stopped working until restart. Three Windows-specific problems combined: - A failed node-pty conpty spawn completes asynchronously on its conout worker thread; the uncaught exception escaped the per-request try/catch and crashed the whole terminal worker, severing every terminal. Add uncaughtException / unhandledRejection guards so a single bad spawn keeps the worker alive. - Profile commands were passed to conpty unresolved. conpty's CreateProcess ignores PATHEXT, so bare codex (npm codex.cmd) wasn't found and .cmd/.bat shims can't run directly. Add resolveTerminalSpawnCommand() to resolve the real path and route .cmd/.bat through cmd.exe /c on Windows. - winget-installed CLIs (e.g. Claude Code) live under the winget Packages dir and aren't on PATH. findExecutable() now falls back to those known install locations, so all providers and terminal profiles benefit. Also surface a failed terminal create to the user via a toast instead of silently dropping the error. * Address review: simplify winget scan, narrow worker exception guard - Drop the single-element WINGET_PACKAGE_BIN_SUBDIRS loop in executable.ts and map package dirs directly to the root .exe candidate. - Remove the unhandledRejection handler in the terminal worker (only the uncaughtException path is exercised by conpty's async spawn failure) and expand the comment to explain the keep-alive trade-off and the absence of a worker restart path. * Refactor executable resolution out of utils * Serialize terminal create requests in worker --------- Co-authored-by: danniel Co-authored-by: Mohamed Boudra --- .../terminals/use-workspace-terminals.ts | 21 +++- .../screens/workspace/workspace-screen.tsx | 10 ++ .../executable-resolution.probe.test.ts} | 2 +- .../executable-resolution.test.ts} | 34 +++++- .../executable-resolution.ts} | 27 +++-- .../src/executable-resolution/windows.ts | 92 ++++++++++++++++ .../server/agent/provider-launch-config.ts | 5 +- .../server/agent/provider-registry.test.ts | 11 +- .../providers/claude/agent.real.e2e.test.ts | 2 +- .../agent/providers/claude/agent.test.ts | 2 +- .../claude/sdk-behavior.real.e2e.test.ts | 2 +- .../agent/providers/codex-app-server-agent.ts | 5 +- .../providers/opencode/server-manager.ts | 2 +- .../providers/provider-windows-launch.test.ts | 2 +- .../daemon-e2e/real-provider-test-config.ts | 2 +- packages/server/src/server/exports.ts | 2 +- .../server/src/services/github-service.ts | 2 +- .../src/terminal/terminal-worker-process.ts | 102 ++++++++++++++---- packages/server/src/terminal/terminal.test.ts | 45 ++++++++ packages/server/src/terminal/terminal.ts | 66 +++++++++++- packages/server/src/utils/github-remote.ts | 2 +- .../src/utils/spawn.launch-regression.test.ts | 2 +- 22 files changed, 385 insertions(+), 55 deletions(-) rename packages/server/src/{utils/executable.probe.test.ts => executable-resolution/executable-resolution.probe.test.ts} (98%) rename packages/server/src/{utils/executable.test.ts => executable-resolution/executable-resolution.test.ts} (88%) rename packages/server/src/{utils/executable.ts => executable-resolution/executable-resolution.ts} (82%) create mode 100644 packages/server/src/executable-resolution/windows.ts diff --git a/packages/app/src/screens/workspace/terminals/use-workspace-terminals.ts b/packages/app/src/screens/workspace/terminals/use-workspace-terminals.ts index c3ad8b075..3d350fb66 100644 --- a/packages/app/src/screens/workspace/terminals/use-workspace-terminals.ts +++ b/packages/app/src/screens/workspace/terminals/use-workspace-terminals.ts @@ -43,6 +43,7 @@ interface UseWorkspaceTerminalsInput { onScriptTerminalSelected: (terminalId: string) => void; onWorkspacePathUnavailable: () => void; onTerminalCreateQueued: () => void; + onTerminalCreateFailed: (reason: string) => void; } export function useWorkspaceTerminals(input: UseWorkspaceTerminalsInput) { @@ -60,6 +61,7 @@ export function useWorkspaceTerminals(input: UseWorkspaceTerminalsInput) { onScriptTerminalSelected, onWorkspacePathUnavailable, onTerminalCreateQueued, + onTerminalCreateFailed, } = input; const { t } = useTranslation(); const queryClient = useQueryClient(); @@ -119,11 +121,19 @@ export function useWorkspaceTerminals(input: UseWorkspaceTerminalsInput) { if (!client || !workspaceDirectory) { throw new Error(t("workspace.terminal.hostDisconnected")); } - if (_input?.profile) { - const { name, command, args } = _input.profile; - return await client.createTerminal(workspaceDirectory, name, undefined, { command, args }); + const payload = _input?.profile + ? await client.createTerminal(workspaceDirectory, _input.profile.name, undefined, { + command: _input.profile.command, + args: _input.profile.args, + }) + : await client.createTerminal(workspaceDirectory); + // The daemon reports a failed spawn (e.g. a profile command that isn't + // installed) via payload.error with a null terminal. Surface it instead + // of silently treating the create as a no-op success. + if (!payload.terminal && payload.error) { + throw new Error(payload.error); } - return await client.createTerminal(workspaceDirectory); + return payload; }, onSuccess: (payload, createInput) => { const createdTerminal = payload.terminal; @@ -145,6 +155,9 @@ export function useWorkspaceTerminals(input: UseWorkspaceTerminalsInput) { }); } }, + onError: (error: unknown) => { + onTerminalCreateFailed(error instanceof Error ? error.message : String(error)); + }, }); const killMutation = useMutation({ mutationFn: async (terminalId: string) => { diff --git a/packages/app/src/screens/workspace/workspace-screen.tsx b/packages/app/src/screens/workspace/workspace-screen.tsx index 6b3a7b8f3..8d7986ba7 100644 --- a/packages/app/src/screens/workspace/workspace-screen.tsx +++ b/packages/app/src/screens/workspace/workspace-screen.tsx @@ -1642,6 +1642,7 @@ interface WorkspaceTerminalTabActions { handleScriptTerminalSelected: (terminalId: string) => void; handleWorkspacePathUnavailable: () => void; handleTerminalCreateQueued: () => void; + handleTerminalCreateFailed: (reason: string) => void; } function useWorkspaceTerminalTabActions({ @@ -1678,12 +1679,19 @@ function useWorkspaceTerminalTabActions({ const handleTerminalCreateQueued = useCallback(() => { toast.show(labels.terminalQueued); }, [labels.terminalQueued, toast]); + const handleTerminalCreateFailed = useCallback( + (reason: string) => { + toast.error(reason); + }, + [toast], + ); return { handleTerminalCreated, handleScriptTerminalSelected, handleWorkspacePathUnavailable, handleTerminalCreateQueued, + handleTerminalCreateFailed, }; } @@ -1826,6 +1834,7 @@ function WorkspaceScreenContent({ handleScriptTerminalSelected, handleWorkspacePathUnavailable, handleTerminalCreateQueued, + handleTerminalCreateFailed, } = useWorkspaceTerminalTabActions({ persistenceKey, focusWorkspacePane, @@ -1866,6 +1875,7 @@ function WorkspaceScreenContent({ onScriptTerminalSelected: handleScriptTerminalSelected, onWorkspacePathUnavailable: handleWorkspacePathUnavailable, onTerminalCreateQueued: handleTerminalCreateQueued, + onTerminalCreateFailed: handleTerminalCreateFailed, }); const { archiveAgent } = useArchiveAgent(); diff --git a/packages/server/src/utils/executable.probe.test.ts b/packages/server/src/executable-resolution/executable-resolution.probe.test.ts similarity index 98% rename from packages/server/src/utils/executable.probe.test.ts rename to packages/server/src/executable-resolution/executable-resolution.probe.test.ts index 1ce56c5cd..0715084d3 100644 --- a/packages/server/src/utils/executable.probe.test.ts +++ b/packages/server/src/executable-resolution/executable-resolution.probe.test.ts @@ -13,7 +13,7 @@ import { performance } from "node:perf_hooks"; import { afterEach, describe, expect, test } from "vitest"; import { isPlatform } from "../test-utils/platform.js"; -import { probeExecutable } from "./executable.js"; +import { probeExecutable } from "./executable-resolution.js"; const timeoutMs = 1000; const timeoutSlackMs = 500; diff --git a/packages/server/src/utils/executable.test.ts b/packages/server/src/executable-resolution/executable-resolution.test.ts similarity index 88% rename from packages/server/src/utils/executable.test.ts rename to packages/server/src/executable-resolution/executable-resolution.test.ts index 88e7bc9a3..fbb62a5a8 100644 --- a/packages/server/src/utils/executable.test.ts +++ b/packages/server/src/executable-resolution/executable-resolution.test.ts @@ -1,4 +1,4 @@ -import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, copyFileSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, test } from "vitest"; @@ -8,7 +8,7 @@ import { findExecutable, quoteWindowsArgument, quoteWindowsCommand, -} from "./executable.js"; +} from "./executable-resolution.js"; import { isPlatform } from "../test-utils/platform.js"; const originalEnv = { @@ -100,6 +100,36 @@ describe("findExecutable", () => { expectWindowsPathsEqual(await findExecutable("foo"), cmd); }); + + test("finds a .cmd for an extensionless absolute path", async () => { + const dir = makeTempDir(); + const command = path.join(dir, "codex"); + const cmd = writeExecutable(`${command}.cmd`, "@echo off\r\necho 0.1\r\n"); + + expectWindowsPathsEqual(await findExecutable(command), cmd); + }); + + test("finds a winget portable executable outside PATH", async () => { + const originalLocalAppData = process.env.LOCALAPPDATA; + const localAppData = makeTempDir(); + const packageDir = path.join( + localAppData, + "Microsoft", + "WinGet", + "Packages", + "Anthropic.ClaudeCode_abc", + ); + mkdirSync(packageDir, { recursive: true }); + const claude = path.join(packageDir, "claude.exe"); + copyFileSync(process.execPath, claude); + process.env.LOCALAPPDATA = localAppData; + process.env.PATH = makeTempDir(); + try { + expectWindowsPathsEqual(await findExecutable("claude"), claude); + } finally { + process.env.LOCALAPPDATA = originalLocalAppData; + } + }); }); test("returns an invokable absolute path", async () => { diff --git a/packages/server/src/utils/executable.ts b/packages/server/src/executable-resolution/executable-resolution.ts similarity index 82% rename from packages/server/src/utils/executable.ts rename to packages/server/src/executable-resolution/executable-resolution.ts index c7da25ba8..a6edcc6e7 100644 --- a/packages/server/src/utils/executable.ts +++ b/packages/server/src/executable-resolution/executable-resolution.ts @@ -1,10 +1,10 @@ import { createRequire } from "node:module"; import { existsSync } from "node:fs"; -import { extname } from "node:path"; -import { execCommand } from "./spawn.js"; -import { isWindowsCommandScript } from "./windows-command.js"; +import { execCommand } from "../utils/spawn.js"; +import { isWindowsCommandScript } from "../utils/windows-command.js"; +import { windowsExecutableResolution } from "./windows.js"; -export { quoteWindowsArgument, quoteWindowsCommand } from "./windows-command.js"; +export { quoteWindowsArgument, quoteWindowsCommand } from "../utils/windows-command.js"; type Which = (command: string, options: { all: true }) => Promise; @@ -102,14 +102,10 @@ export function executableExists( executablePath: string, exists: typeof existsSync = existsSync, ): string | null { - if (exists(executablePath)) return executablePath; - if (process.platform === "win32" && !extname(executablePath)) { - for (const ext of [".exe", ".cmd"]) { - const candidate = executablePath + ext; - if (exists(candidate)) return candidate; - } + if (process.platform === "win32") { + return windowsExecutableResolution.exists(executablePath, { exists }); } - return null; + return exists(executablePath) ? executablePath : null; } export async function findExecutable( @@ -121,6 +117,15 @@ export async function findExecutable( return null; } + if (process.platform === "win32") { + return windowsExecutableResolution.find(trimmed, { + enumeratePathCandidates: enumerateCandidates, + probeExecutable, + exists: existsSync, + probeTimeoutMs, + }); + } + if (hasPathSeparator(trimmed)) { return (await probeExecutable(trimmed, probeTimeoutMs)) ? trimmed : null; } diff --git a/packages/server/src/executable-resolution/windows.ts b/packages/server/src/executable-resolution/windows.ts new file mode 100644 index 000000000..5070a86d5 --- /dev/null +++ b/packages/server/src/executable-resolution/windows.ts @@ -0,0 +1,92 @@ +import { readdirSync } from "node:fs"; +import { extname, join } from "node:path"; + +interface WindowsFindExecutableOptions { + enumeratePathCandidates: (name: string) => Promise; + probeExecutable: (executablePath: string, timeoutMs: number) => Promise; + exists: (path: string) => boolean; + localAppData?: string; + probeTimeoutMs: number; +} + +interface WindowsExecutableExistsOptions { + exists: (path: string) => boolean; +} + +function hasPathSeparator(value: string): boolean { + return value.includes("/") || value.includes("\\"); +} + +function enumerateLiteralPathCandidates(executablePath: string): string[] { + if (extname(executablePath)) { + return [executablePath]; + } + return [executablePath, `${executablePath}.exe`, `${executablePath}.cmd`]; +} + +function enumerateWingetPackageCandidates( + name: string, + localAppData: string | undefined, +): string[] { + if (!localAppData) { + return []; + } + + const wingetPackages = join(localAppData, "Microsoft", "WinGet", "Packages"); + let packageDirs: string[]; + try { + packageDirs = readdirSync(wingetPackages, { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name); + } catch { + return []; + } + + const exeName = `${name}.exe`; + return packageDirs.map((packageDir) => join(wingetPackages, packageDir, exeName)); +} + +async function find(input: string, options: WindowsFindExecutableOptions): Promise { + if (hasPathSeparator(input)) { + return findFirstProbeable(enumerateLiteralPathCandidates(input), options); + } + + const pathCandidates = await options.enumeratePathCandidates(input); + const wingetCandidates = enumerateWingetPackageCandidates( + input, + options.localAppData ?? process.env.LOCALAPPDATA, + ).filter(options.exists); + + return findFirstProbeable([...pathCandidates, ...wingetCandidates], options); +} + +async function findFirstProbeable( + candidates: string[], + options: WindowsFindExecutableOptions, +): Promise { + const seen = new Set(); + for (const candidate of candidates) { + if (seen.has(candidate)) { + continue; + } + seen.add(candidate); + if (await options.probeExecutable(candidate, options.probeTimeoutMs)) { + return candidate; + } + } + return null; +} + +function exists(executablePath: string, options: WindowsExecutableExistsOptions): string | null { + for (const candidate of enumerateLiteralPathCandidates(executablePath)) { + if (options.exists(candidate)) { + return candidate; + } + } + return null; +} + +export const windowsExecutableResolution = { + exists, + find, +}; diff --git a/packages/server/src/server/agent/provider-launch-config.ts b/packages/server/src/server/agent/provider-launch-config.ts index 52d5398de..b75a361fa 100644 --- a/packages/server/src/server/agent/provider-launch-config.ts +++ b/packages/server/src/server/agent/provider-launch-config.ts @@ -1,5 +1,8 @@ import { isAbsolute } from "node:path"; -import { executableExists, findExecutable } from "../../utils/executable.js"; +import { + executableExists, + findExecutable, +} from "../../executable-resolution/executable-resolution.js"; import { createExternalProcessEnv, type ProcessEnvRecord } from "../paseo-env.js"; export { AgentProviderRuntimeSettingsMapSchema, diff --git a/packages/server/src/server/agent/provider-registry.test.ts b/packages/server/src/server/agent/provider-registry.test.ts index 6f3846e2c..b31ccd73f 100644 --- a/packages/server/src/server/agent/provider-registry.test.ts +++ b/packages/server/src/server/agent/provider-registry.test.ts @@ -44,7 +44,7 @@ const mockState = vi.hoisted(() => { }; }); -vi.mock("../../utils/executable.js", () => ({ +vi.mock("../../executable-resolution/executable-resolution.js", () => ({ isCommandAvailable: mockState.isCommandAvailable, })); @@ -90,7 +90,8 @@ vi.mock("./providers/claude/agent.js", () => ({ ? Reflect.get(this.runtimeSettings, "command") : undefined; if (command?.mode === "replace") { - const { isCommandAvailable } = await import("../../utils/executable.js"); + const { isCommandAvailable } = + await import("../../executable-resolution/executable-resolution.js"); return await isCommandAvailable(command.argv?.[0] ?? ""); } return true; @@ -138,7 +139,8 @@ vi.mock("./providers/codex-app-server-agent.js", () => ({ ? Reflect.get(this.runtimeSettings, "command") : undefined; if (command?.mode === "replace") { - const { isCommandAvailable } = await import("../../utils/executable.js"); + const { isCommandAvailable } = + await import("../../executable-resolution/executable-resolution.js"); return await isCommandAvailable(command.argv?.[0] ?? ""); } return true; @@ -188,7 +190,8 @@ vi.mock("./providers/copilot-acp-agent.js", () => ({ ? Reflect.get(this.runtimeSettings, "command") : undefined; if (command?.mode === "replace") { - const { isCommandAvailable } = await import("../../utils/executable.js"); + const { isCommandAvailable } = + await import("../../executable-resolution/executable-resolution.js"); return await isCommandAvailable(command.argv?.[0] ?? ""); } return true; diff --git a/packages/server/src/server/agent/providers/claude/agent.real.e2e.test.ts b/packages/server/src/server/agent/providers/claude/agent.real.e2e.test.ts index b0178f83c..74ff780aa 100644 --- a/packages/server/src/server/agent/providers/claude/agent.real.e2e.test.ts +++ b/packages/server/src/server/agent/providers/claude/agent.real.e2e.test.ts @@ -16,7 +16,7 @@ import { getRealProviderConfig, getRealProviderRuntimeSettings, } from "../../../daemon-e2e/real-provider-test-config.js"; -import { findExecutable } from "../../../../utils/executable.js"; +import { findExecutable } from "../../../../executable-resolution/executable-resolution.js"; import { withTimeout } from "../../../../utils/promise-timeout.js"; import { claudeQuery } from "./query.js"; import { streamSession } from "../test-utils/session-stream-adapter.js"; diff --git a/packages/server/src/server/agent/providers/claude/agent.test.ts b/packages/server/src/server/agent/providers/claude/agent.test.ts index 6005a1d7c..b35ad47ea 100644 --- a/packages/server/src/server/agent/providers/claude/agent.test.ts +++ b/packages/server/src/server/agent/providers/claude/agent.test.ts @@ -5,7 +5,7 @@ import { afterEach, describe, expect, test, vi } from "vitest"; import type { SDKMessage } from "@anthropic-ai/claude-agent-sdk"; import { createTestLogger } from "../../../../test-utils/test-logger.js"; -import * as executableUtils from "../../../../utils/executable.js"; +import * as executableUtils from "../../../../executable-resolution/executable-resolution.js"; import { ClaudeAgentClient, convertClaudeHistoryEntry, diff --git a/packages/server/src/server/agent/providers/claude/sdk-behavior.real.e2e.test.ts b/packages/server/src/server/agent/providers/claude/sdk-behavior.real.e2e.test.ts index a38bbe1e6..95c4379c7 100644 --- a/packages/server/src/server/agent/providers/claude/sdk-behavior.real.e2e.test.ts +++ b/packages/server/src/server/agent/providers/claude/sdk-behavior.real.e2e.test.ts @@ -10,7 +10,7 @@ import { canRunRealProvider, getRealProviderRuntimeSettings, } from "../../../daemon-e2e/real-provider-test-config.js"; -import { findExecutable } from "../../../../utils/executable.js"; +import { findExecutable } from "../../../../executable-resolution/executable-resolution.js"; import { claudeQuery } from "./query.js"; class Pushable implements AsyncIterable { 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 4f5e270ed..82880c15a 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 @@ -58,7 +58,10 @@ import { type ProviderRuntimeSettings, type ResolvedProviderLaunch, } from "../provider-launch-config.js"; -import { findExecutable, probeExecutable } from "../../../utils/executable.js"; +import { + findExecutable, + probeExecutable, +} from "../../../executable-resolution/executable-resolution.js"; import { createPathEquivalenceMatcher } from "../../../utils/path.js"; import { spawnProcess } from "../../../utils/spawn.js"; import { extractCodexTerminalSessionId, nonEmptyString } from "./tool-call-mapper-utils.js"; diff --git a/packages/server/src/server/agent/providers/opencode/server-manager.ts b/packages/server/src/server/agent/providers/opencode/server-manager.ts index 1a4b4db94..78502b303 100644 --- a/packages/server/src/server/agent/providers/opencode/server-manager.ts +++ b/packages/server/src/server/agent/providers/opencode/server-manager.ts @@ -2,7 +2,7 @@ import type { ChildProcess } from "node:child_process"; import net from "node:net"; import type { Logger } from "pino"; -import { findExecutable } from "../../../../utils/executable.js"; +import { findExecutable } from "../../../../executable-resolution/executable-resolution.js"; import { spawnProcess } from "../../../../utils/spawn.js"; import { terminateWithTreeKill } from "../../../../utils/tree-kill.js"; import { diff --git a/packages/server/src/server/agent/providers/provider-windows-launch.test.ts b/packages/server/src/server/agent/providers/provider-windows-launch.test.ts index 2d64b36d3..93ea5e526 100644 --- a/packages/server/src/server/agent/providers/provider-windows-launch.test.ts +++ b/packages/server/src/server/agent/providers/provider-windows-launch.test.ts @@ -5,7 +5,7 @@ import path from "node:path"; import pino from "pino"; import { afterEach, describe, expect, test } from "vitest"; -import { findExecutable } from "../../../utils/executable.js"; +import { findExecutable } from "../../../executable-resolution/executable-resolution.js"; import { spawnProcess } from "../../../utils/spawn.js"; import { PiCliRuntime } from "./pi/cli-runtime.js"; diff --git a/packages/server/src/server/daemon-e2e/real-provider-test-config.ts b/packages/server/src/server/daemon-e2e/real-provider-test-config.ts index b7edca291..cc7b71d49 100644 --- a/packages/server/src/server/daemon-e2e/real-provider-test-config.ts +++ b/packages/server/src/server/daemon-e2e/real-provider-test-config.ts @@ -10,7 +10,7 @@ import { ClaudeAgentClient } from "../agent/providers/claude/agent.js"; import { CodexAppServerAgentClient } from "../agent/providers/codex-app-server-agent.js"; import { OpenCodeAgentClient } from "../agent/providers/opencode-agent.js"; import { PiRpcAgentClient } from "../agent/providers/pi/agent.js"; -import { isCommandAvailable } from "../../utils/executable.js"; +import { isCommandAvailable } from "../../executable-resolution/executable-resolution.js"; export const realProviders = ["claude", "codex", "opencode", "pi"] as const; export type RealProvider = (typeof realProviders)[number]; diff --git a/packages/server/src/server/exports.ts b/packages/server/src/server/exports.ts index 1673770c1..0961e08b1 100644 --- a/packages/server/src/server/exports.ts +++ b/packages/server/src/server/exports.ts @@ -53,7 +53,7 @@ export { type ProviderOverride, type ProviderProfileModel, } from "./agent/provider-launch-config.js"; -export { findExecutable } from "../utils/executable.js"; +export { findExecutable } from "../executable-resolution/executable-resolution.js"; export { execCommand, spawnProcess } from "../utils/spawn.js"; // Provider manifest (source of truth for provider definitions) diff --git a/packages/server/src/services/github-service.ts b/packages/server/src/services/github-service.ts index 25a6d6955..53e089522 100644 --- a/packages/server/src/services/github-service.ts +++ b/packages/server/src/services/github-service.ts @@ -1,6 +1,6 @@ import { z } from "zod"; import type { GitHubSearchKind } from "@getpaseo/protocol/messages"; -import { findExecutable } from "../utils/executable.js"; +import { findExecutable } from "../executable-resolution/executable-resolution.js"; import { resolveGitHubRemote } from "../utils/github-remote.js"; import { runGitCommand } from "../utils/run-git-command.js"; import { execCommand } from "../utils/spawn.js"; diff --git a/packages/server/src/terminal/terminal-worker-process.ts b/packages/server/src/terminal/terminal-worker-process.ts index 09c6ab881..403ea0cb9 100644 --- a/packages/server/src/terminal/terminal-worker-process.ts +++ b/packages/server/src/terminal/terminal-worker-process.ts @@ -9,11 +9,33 @@ import type { WorkerTerminalInfo, } from "./terminal-worker-protocol.js"; +type TerminalCreateRequest = Extract; + const manager = createTerminalManager(); const unsubscribeByTerminalId = new Map void>>(); const outputCoalescerByTerminalId = new Map(); let ipcClosing = false; +interface InFlightTerminalCreateRequest { + requestId: string; + errorReported: boolean; +} + +let inFlightTerminalCreateRequest: InFlightTerminalCreateRequest | null = null; + +// The conpty failure signal is process-scoped, not request-scoped. Serializing +// creates keeps an async spawn failure attributable to exactly one request. +let createTerminalQueue: Promise = Promise.resolve(); + +// node-pty completes its Windows conpty spawn asynchronously on a separate +// conout worker thread. When that spawn fails (bad cwd, missing command, etc.) +// it throws an exception there that cannot be caught at the call site and would +// otherwise crash this worker process and sever every existing terminal. +process.on("uncaughtException", (error) => { + console.error("Terminal worker uncaught exception (kept alive):", error); + reportInFlightTerminalCreateFailure(error); +}); + function sendToParent(message: TerminalWorkerToParentMessage): void { if (ipcClosing || !process.connected || !process.send) { return; @@ -48,6 +70,23 @@ function toTerminalInfo(session: TerminalSession): WorkerTerminalInfo { }; } +function terminalWorkerErrorMessage(error: unknown): string { + return error instanceof Error ? error.message : "Terminal worker request failed"; +} + +function reportInFlightTerminalCreateFailure(error: unknown): void { + if (!inFlightTerminalCreateRequest || inFlightTerminalCreateRequest.errorReported) { + return; + } + inFlightTerminalCreateRequest.errorReported = true; + sendToParent({ + type: "response", + requestId: inFlightTerminalCreateRequest.requestId, + ok: false, + error: terminalWorkerErrorMessage(error), + }); +} + function clearTerminalSubscriptions(terminalId: string): void { const subscriptions = unsubscribeByTerminalId.get(terminalId); if (subscriptions) { @@ -146,6 +185,49 @@ manager.subscribeTerminalsChanged((event) => { }); }); +function enqueueCreateTerminalRequest(message: TerminalCreateRequest): Promise { + const nextRequest = createTerminalQueue.then(() => handleCreateTerminalRequest(message)); + createTerminalQueue = nextRequest.catch(() => {}); + return nextRequest; +} + +async function handleCreateTerminalRequest(message: TerminalCreateRequest): Promise { + const request: InFlightTerminalCreateRequest = { + requestId: message.requestId, + errorReported: false, + }; + inFlightTerminalCreateRequest = request; + try { + const session = await manager.createTerminal(message.options); + if (request.errorReported) { + session.kill(); + return; + } + watchTerminal(session); + const initialSnapshot = session.getStateSnapshot(); + sendToParent({ + type: "terminalCreated", + terminal: toTerminalInfo(session), + state: initialSnapshot.state, + }); + sendToParent({ + type: "response", + requestId: message.requestId, + ok: true, + result: { + terminal: toTerminalInfo(session), + state: initialSnapshot.state, + }, + }); + } catch (error) { + reportInFlightTerminalCreateFailure(error); + } finally { + if (inFlightTerminalCreateRequest === request) { + inFlightTerminalCreateRequest = null; + } + } +} + async function handleRequest(message: TerminalWorkerRequest): Promise { switch (message.type) { case "getTerminals": { @@ -160,23 +242,7 @@ async function handleRequest(message: TerminalWorkerRequest): Promise { } case "createTerminal": { - const session = await manager.createTerminal(message.options); - watchTerminal(session); - const initialSnapshot = session.getStateSnapshot(); - sendToParent({ - type: "terminalCreated", - terminal: toTerminalInfo(session), - state: initialSnapshot.state, - }); - sendToParent({ - type: "response", - requestId: message.requestId, - ok: true, - result: { - terminal: toTerminalInfo(session), - state: initialSnapshot.state, - }, - }); + await enqueueCreateTerminalRequest(message); return; } @@ -285,7 +351,7 @@ process.on("message", (message: TerminalWorkerRequest) => { type: "response", requestId: message.requestId, ok: false, - error: error instanceof Error ? error.message : "Terminal worker request failed", + error: terminalWorkerErrorMessage(error), }); }); }); diff --git a/packages/server/src/terminal/terminal.test.ts b/packages/server/src/terminal/terminal.test.ts index 0ac68b4f7..c23b0cd95 100644 --- a/packages/server/src/terminal/terminal.test.ts +++ b/packages/server/src/terminal/terminal.test.ts @@ -5,6 +5,7 @@ import { createTerminal, ensureNodePtySpawnHelperExecutableForCurrentPlatform, resolveDefaultTerminalShell, + resolveTerminalSpawnCommand, humanizeProcessTitle, normalizeProcessTitle, resolveZshShellIntegrationDir, @@ -203,6 +204,50 @@ describe("createTerminal", () => { ).toBe("C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe"); }); + it("passes profile commands through untouched on non-Windows", async () => { + const resolveExecutable = vi.fn(async () => "/usr/local/bin/claude"); + const resolved = await resolveTerminalSpawnCommand("claude", ["--foo"], { + platform: "linux", + resolveExecutable, + }); + + expect(resolved).toEqual({ command: "claude", args: ["--foo"] }); + // Non-Windows must not pay the executable-resolution cost. + expect(resolveExecutable).not.toHaveBeenCalled(); + }); + + it("keeps the original command when it cannot be resolved on Windows", async () => { + const resolved = await resolveTerminalSpawnCommand("claude", [], { + platform: "win32", + resolveExecutable: async () => null, + }); + + // Falls back to the bare command so the terminal surfaces the error itself. + expect(resolved).toEqual({ command: "claude", args: [] }); + }); + + it("routes resolved .cmd shims through cmd.exe on Windows", async () => { + const resolved = await resolveTerminalSpawnCommand("claude", ["--foo"], { + platform: "win32", + env: { ComSpec: "C:\\Windows\\System32\\cmd.exe" }, + resolveExecutable: async () => "C:\\npm\\claude.cmd", + }); + + expect(resolved).toEqual({ + command: "C:\\Windows\\System32\\cmd.exe", + args: ["/c", "C:\\npm\\claude.cmd", "--foo"], + }); + }); + + it("uses the resolved .exe path directly on Windows", async () => { + const resolved = await resolveTerminalSpawnCommand("claude", ["--foo"], { + platform: "win32", + resolveExecutable: async () => "C:\\tools\\claude.exe", + }); + + expect(resolved).toEqual({ command: "C:\\tools\\claude.exe", args: ["--foo"] }); + }); + it("creates a terminal session with an id, name, and cwd", async () => { const session = trackSession( await createTerminal({ diff --git a/packages/server/src/terminal/terminal.ts b/packages/server/src/terminal/terminal.ts index 41fef38ab..692fa258f 100644 --- a/packages/server/src/terminal/terminal.ts +++ b/packages/server/src/terminal/terminal.ts @@ -3,11 +3,12 @@ import xterm, { type Terminal as TerminalType } from "@xterm/headless"; import { randomUUID } from "crypto"; import { chmodSync, existsSync, mkdirSync, readFileSync, statSync } from "node:fs"; import { tmpdir, userInfo } from "node:os"; -import { basename, dirname, join } from "node:path"; +import { basename, dirname, extname, join } from "node:path"; import { createRequire } from "node:module"; import { fileURLToPath } from "node:url"; import { createExternalProcessEnv } from "../server/paseo-env.js"; import { writePrivateFileAtomicSync } from "../server/private-files.js"; +import { findExecutable } from "../executable-resolution/executable-resolution.js"; import type { TerminalCell, TerminalState } from "@getpaseo/protocol/messages"; import { TerminalInputModeTracker } from "@getpaseo/protocol/terminal-input-mode"; @@ -204,6 +205,64 @@ export function resolveDefaultTerminalShell( return env.SHELL || "/bin/sh"; } +export interface ResolvedTerminalCommand { + command: string; + args: string[]; +} + +export interface ResolveTerminalSpawnCommandOptions { + platform?: NodeJS.Platform; + env?: Record; + resolveExecutable?: (name: string) => Promise; +} + +/** + * Resolve a terminal profile command (e.g. `claude`) into something node-pty's + * conpty backend can actually launch on Windows. + * + * On Windows, conpty's underlying `CreateProcess` does not apply `PATHEXT`, so a + * bare `claude` (installed by npm as `claude.cmd`) fails with `error code: 2` + * (`ERROR_FILE_NOT_FOUND`). Worse, conpty completes the spawn asynchronously on + * its own conout worker thread, so that failure surfaces as an uncaught + * exception that takes down the whole terminal worker process. Resolving the + * real path up front — and routing `.cmd`/`.bat` shims through `cmd.exe /c` + * (node-pty has no `shell` option) — keeps the profile launchable. + * + * Non-Windows and the default-shell path (no explicit command) are unchanged. + */ +export async function resolveTerminalSpawnCommand( + command: string, + args: string[], + options: ResolveTerminalSpawnCommandOptions = {}, +): Promise { + const platform = options.platform ?? process.platform; + if (platform !== "win32") { + return { command, args }; + } + + const resolveExecutable = options.resolveExecutable ?? findExecutable; + const resolved = await resolveExecutable(command); + if (!resolved) { + // Leave the command as-is so the terminal itself surfaces the "not found" + // error to the user instead of silently doing nothing. + return { command, args }; + } + + // `.cmd`/`.bat` shims are batch scripts that conpty's CreateProcess cannot + // launch directly; they must run through cmd.exe (node-pty has no `shell` + // option, so build the `cmd /c` invocation ourselves). Checked by extension + // rather than isWindowsCommandScript() because that helper gates on the live + // process.platform, which is wrong once we're already on the win32 branch. + const extension = extname(resolved).toLowerCase(); + if (extension === ".cmd" || extension === ".bat") { + const env = options.env ?? process.env; + const comSpec = env.ComSpec || env.COMSPEC || "C:\\Windows\\System32\\cmd.exe"; + return { command: comSpec, args: ["/c", resolved, ...args] }; + } + + return { command: resolved, args }; +} + export function resolveZshShellIntegrationDir(): string { return fileURLToPath(new URL("./shell-integration/zsh", import.meta.url)); } @@ -636,8 +695,9 @@ export async function createTerminal(options: CreateTerminalOptions): Promise | null = null; diff --git a/packages/server/src/utils/spawn.launch-regression.test.ts b/packages/server/src/utils/spawn.launch-regression.test.ts index b1eedad40..847214c62 100644 --- a/packages/server/src/utils/spawn.launch-regression.test.ts +++ b/packages/server/src/utils/spawn.launch-regression.test.ts @@ -4,7 +4,7 @@ import path from "node:path"; import type { ChildProcess } from "node:child_process"; import { afterEach, describe, expect, test } from "vitest"; -import { findExecutable } from "./executable.js"; +import { findExecutable } from "../executable-resolution/executable-resolution.js"; import { spawnProcess } from "./spawn.js"; import { isPlatform } from "../test-utils/platform.js";