From d899f994c65171a280c0dc4de219fdd51089fff7 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 5 Mar 2026 01:11:29 -0800 Subject: [PATCH 01/10] Improve Windows WSL process and editor/browser interop - Centralize process spawning/termination in `processRunner` for consistent Windows behavior - Add runtime environment detection to handle WSL-hosted command resolution - Update open/editor/browser launch logic to prefer Linux shims and fall back to Windows executables - Expand server tests for WSL launch paths and shared process helpers --- apps/server/src/codexAppServerManager.ts | 26 +- .../src/git/Layers/CodexTextGeneration.ts | 146 ++++----- apps/server/src/git/Layers/GitService.ts | 128 +++----- apps/server/src/open.test.ts | 214 +++++++++++++- apps/server/src/open.ts | 278 +++++++++++++++--- apps/server/src/processRunner.test.ts | 18 +- apps/server/src/processRunner.ts | 178 +++++++++-- .../provider/Layers/ProviderHealth.test.ts | 141 +++------ .../src/provider/Layers/ProviderHealth.ts | 239 ++++++++------- apps/server/src/runtimeEnvironment.test.ts | 72 +++++ apps/server/src/runtimeEnvironment.ts | 54 ++++ .../src/terminal/Layers/Manager.test.ts | 98 +++++- apps/server/src/terminal/Layers/Manager.ts | 118 ++++++-- apps/server/src/wsServer.test.ts | 10 + apps/server/src/wsServer.ts | 3 + packages/contracts/src/server.ts | 19 ++ 16 files changed, 1222 insertions(+), 520 deletions(-) create mode 100644 apps/server/src/runtimeEnvironment.test.ts create mode 100644 apps/server/src/runtimeEnvironment.ts diff --git a/apps/server/src/codexAppServerManager.ts b/apps/server/src/codexAppServerManager.ts index 7b19c7be5c..fac4f2de8b 100644 --- a/apps/server/src/codexAppServerManager.ts +++ b/apps/server/src/codexAppServerManager.ts @@ -1,4 +1,4 @@ -import { type ChildProcessWithoutNullStreams, spawn, spawnSync } from "node:child_process"; +import { type ChildProcessWithoutNullStreams } from "node:child_process"; import { randomUUID } from "node:crypto"; import { EventEmitter } from "node:events"; import readline from "node:readline"; @@ -18,6 +18,7 @@ import { type ProviderSessionStartInput, type ProviderTurnStartResult, } from "@t3tools/contracts"; +import { killProcessTree, spawnPipedProcess } from "./processRunner"; type PendingRequestKey = string; @@ -104,23 +105,6 @@ const RECOVERABLE_THREAD_RESUME_ERROR_SNIPPETS = [ "does not exist", ]; -/** - * On Windows with `shell: true`, `child.kill()` only terminates the `cmd.exe` - * wrapper, leaving the actual command running. Use `taskkill /T` to kill the - * entire process tree instead. - */ -function killChildTree(child: ChildProcessWithoutNullStreams): void { - if (process.platform === "win32" && child.pid !== undefined) { - try { - spawnSync("taskkill", ["/pid", String(child.pid), "/T", "/F"], { stdio: "ignore" }); - return; - } catch { - // fallback to direct kill - } - } - child.kill(); -} - export function normalizeCodexModelSlug( model: string | undefined | null, preferredId?: string, @@ -195,14 +179,12 @@ export class CodexAppServerManager extends EventEmitter; }; - const readStreamAsString = ( - operation: string, - stream: Stream.Stream, - ): Effect.Effect => - Effect.gen(function* () { - let text = ""; - yield* Stream.runForEach(stream, (chunk) => - Effect.sync(() => { - text += Buffer.from(chunk).toString("utf8"); - }), - ).pipe( - Effect.mapError((cause) => - normalizeCodexError(operation, cause, "Failed to collect process output"), - ), - ); - return text; - }); - const tempDir = process.env.TMPDIR ?? process.env.TEMP ?? process.env.TMP ?? "/tmp"; const writeTempFile = ( @@ -203,68 +184,59 @@ const makeCodexTextGeneration = Effect.gen(function* () { ); const outputPath = yield* writeTempFile(operation, "codex-output", ""); - const runCodexCommand = Effect.gen(function* () { - const command = ChildProcess.make( - "codex", - [ - "exec", - "--ephemeral", - "-s", - "read-only", - "--model", - CODEX_MODEL, - "--config", - `model_reasoning_effort="${CODEX_REASONING_EFFORT}"`, - "--output-schema", - schemaPath, - "--output-last-message", - outputPath, - ...imagePaths.flatMap((imagePath) => ["--image", imagePath]), - "-", - ], - { - cwd, - shell: process.platform === "win32", - stdin: { - stream: Stream.make(new TextEncoder().encode(prompt)), + const runCodexCommand = Effect.tryPromise({ + try: async () => { + const result = await runProcess( + "codex", + [ + "exec", + "--ephemeral", + "-s", + "read-only", + "--model", + CODEX_MODEL, + "--config", + `model_reasoning_effort="${CODEX_REASONING_EFFORT}"`, + "--output-schema", + schemaPath, + "--output-last-message", + outputPath, + ...imagePaths.flatMap((imagePath) => ["--image", imagePath]), + "-", + ], + { + cwd, + stdin: prompt, + allowNonZeroExit: true, + timeoutMs: CODEX_TIMEOUT_MS, + maxBufferBytes: 1_000_000, + outputMode: "truncate", }, - }, - ); - - const child = yield* commandSpawner - .spawn(command) - .pipe( - Effect.mapError((cause) => - normalizeCodexError(operation, cause, "Failed to spawn Codex CLI process"), - ), ); - const [stdout, stderr, exitCode] = yield* Effect.all( - [ - readStreamAsString(operation, child.stdout), - readStreamAsString(operation, child.stderr), - child.exitCode.pipe( - Effect.map((value) => Number(value)), - Effect.mapError((cause) => - normalizeCodexError(operation, cause, "Failed to read Codex CLI exit code"), - ), - ), - ], - { concurrency: "unbounded" }, - ); - - if (exitCode !== 0) { - const stderrDetail = stderr.trim(); - const stdoutDetail = stdout.trim(); - const detail = stderrDetail.length > 0 ? stderrDetail : stdoutDetail; - return yield* new TextGenerationError({ - operation, - detail: - detail.length > 0 - ? `Codex CLI command failed: ${detail}` - : `Codex CLI command failed with code ${exitCode}.`, - }); - } + if (result.timedOut) { + throw new TextGenerationError({ operation, detail: "Codex CLI request timed out." }); + } + + const stdout = result.stdout; + const stderr = result.stderr; + const exitCode = result.code ?? 0; + + if (exitCode !== 0) { + const stderrDetail = stderr.trim(); + const stdoutDetail = stdout.trim(); + const detail = stderrDetail.length > 0 ? stderrDetail : stdoutDetail; + throw new TextGenerationError({ + operation, + detail: + detail.length > 0 + ? `Codex CLI command failed: ${detail}` + : `Codex CLI command failed with code ${exitCode}.`, + }); + } + }, + catch: (error) => + normalizeCodexError(operation, error, "Failed to execute Codex CLI process"), }); const cleanup = Effect.all( @@ -275,19 +247,7 @@ const makeCodexTextGeneration = Effect.gen(function* () { ).pipe(Effect.asVoid); return yield* Effect.gen(function* () { - yield* runCodexCommand.pipe( - Effect.scoped, - Effect.timeoutOption(CODEX_TIMEOUT_MS), - Effect.flatMap( - Option.match({ - onNone: () => - Effect.fail( - new TextGenerationError({ operation, detail: "Codex CLI request timed out." }), - ), - onSome: () => Effect.void, - }), - ), - ); + yield* runCodexCommand; return yield* fileSystem.readFileString(outputPath).pipe( Effect.mapError( diff --git a/apps/server/src/git/Layers/GitService.ts b/apps/server/src/git/Layers/GitService.ts index d3f07e3151..8cf00c969f 100644 --- a/apps/server/src/git/Layers/GitService.ts +++ b/apps/server/src/git/Layers/GitService.ts @@ -1,13 +1,13 @@ /** - * Git process helpers - Effect-native git execution with typed errors. + * Git process helpers - runtime-aware git execution with typed errors. * * Centralizes child-process git invocation for server modules. This module * only executes git commands and reports structured failures. * * @module GitServiceLive */ -import { Effect, Layer, Option, Schema, Stream } from "effect"; -import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; +import { Effect, Layer, Schema } from "effect"; +import { runProcess } from "../../processRunner.ts"; import { GitCommandError } from "../Errors.ts"; import { ExecuteGitInput, @@ -39,37 +39,7 @@ function toGitCommandError( }); } -const collectOutput = Effect.fn(function* ( - input: Pick, - stream: Stream.Stream, - maxOutputBytes: number, -): Effect.fn.Return { - const decoder = new TextDecoder(); - let bytes = 0; - let text = ""; - - yield* Stream.runForEach(stream, (chunk) => - Effect.gen(function* () { - bytes += chunk.byteLength; - if (bytes > maxOutputBytes) { - return yield* new GitCommandError({ - operation: input.operation, - command: quoteGitCommand(input.args), - cwd: input.cwd, - detail: `${quoteGitCommand(input.args)} output exceeded ${maxOutputBytes} bytes and was truncated.`, - }); - } - text += decoder.decode(chunk, { stream: true }); - }), - ).pipe(Effect.mapError(toGitCommandError(input, "output stream failed."))); - - text += decoder.decode(); - return text; -}); - -const makeGitService = Effect.gen(function* () { - const commandSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; - +const makeGitService = Effect.sync(() => { const execute: GitServiceShape["execute"] = Effect.fnUntraced(function* (input) { const commandInput = { ...input, @@ -78,62 +48,48 @@ const makeGitService = Effect.gen(function* () { const timeoutMs = input.timeoutMs ?? DEFAULT_TIMEOUT_MS; const maxOutputBytes = input.maxOutputBytes ?? DEFAULT_MAX_OUTPUT_BYTES; - const commandEffect = Effect.gen(function* () { - const child = yield* commandSpawner - .spawn( - ChildProcess.make("git", commandInput.args, { - cwd: commandInput.cwd, - ...(input.env ? { env: input.env } : {}), - }), - ) - .pipe(Effect.mapError(toGitCommandError(commandInput, "failed to spawn."))); - - const [stdout, stderr, exitCode] = yield* Effect.all( - [ - collectOutput(commandInput, child.stdout, maxOutputBytes), - collectOutput(commandInput, child.stderr, maxOutputBytes), - child.exitCode.pipe( - Effect.map((value) => Number(value)), - Effect.mapError(toGitCommandError(commandInput, "failed to report exit code.")), - ), - ], - { concurrency: "unbounded" }, - ); - - if (!input.allowNonZeroExit && exitCode !== 0) { - const trimmedStderr = stderr.trim(); - return yield* new GitCommandError({ - operation: commandInput.operation, - command: quoteGitCommand(commandInput.args), + return yield* Effect.tryPromise({ + try: async () => { + const result = await runProcess("git", commandInput.args, { cwd: commandInput.cwd, - detail: - trimmedStderr.length > 0 - ? `${quoteGitCommand(commandInput.args)} failed: ${trimmedStderr}` - : `${quoteGitCommand(commandInput.args)} failed with code ${exitCode}.`, + ...(input.env ? { env: input.env } : {}), + timeoutMs, + allowNonZeroExit: true, + maxBufferBytes: maxOutputBytes, + outputMode: "error", }); - } - return { code: exitCode, stdout, stderr } satisfies ExecuteGitResult; - }); + if (result.timedOut) { + throw new GitCommandError({ + operation: commandInput.operation, + command: quoteGitCommand(commandInput.args), + cwd: commandInput.cwd, + detail: `${quoteGitCommand(commandInput.args)} timed out.`, + }); + } + + const exitCode = result.code ?? 0; + if (!input.allowNonZeroExit && exitCode !== 0) { + const trimmedStderr = result.stderr.trim(); + throw new GitCommandError({ + operation: commandInput.operation, + command: quoteGitCommand(commandInput.args), + cwd: commandInput.cwd, + detail: + trimmedStderr.length > 0 + ? `${quoteGitCommand(commandInput.args)} failed: ${trimmedStderr}` + : `${quoteGitCommand(commandInput.args)} failed with code ${exitCode}.`, + }); + } - return yield* commandEffect.pipe( - Effect.scoped, - Effect.timeoutOption(timeoutMs), - Effect.flatMap((result) => - Option.match(result, { - onNone: () => - Effect.fail( - new GitCommandError({ - operation: commandInput.operation, - command: quoteGitCommand(commandInput.args), - cwd: commandInput.cwd, - detail: `${quoteGitCommand(commandInput.args)} timed out.`, - }), - ), - onSome: Effect.succeed, - }), - ), - ); + return { + code: exitCode, + stdout: result.stdout, + stderr: result.stderr, + } satisfies ExecuteGitResult; + }, + catch: toGitCommandError(commandInput, "failed to run."), + }); }); return { diff --git a/apps/server/src/open.test.ts b/apps/server/src/open.test.ts index 0f864554e9..b324102909 100644 --- a/apps/server/src/open.test.ts +++ b/apps/server/src/open.test.ts @@ -7,6 +7,7 @@ import { assert, describe, it } from "@effect/vitest"; import { isCommandAvailable, launchDetached, + resolveBrowserLaunch, resolveAvailableEditors, resolveEditorLaunch, } from "./open"; @@ -18,7 +19,7 @@ describe("resolveEditorLaunch", () => { Effect.gen(function* () { const cursorLaunch = yield* resolveEditorLaunch( { cwd: "/tmp/workspace", editor: "cursor" }, - "darwin", + { platform: "darwin" }, ); assert.deepEqual(cursorLaunch, { command: "cursor", @@ -27,7 +28,7 @@ describe("resolveEditorLaunch", () => { const vscodeLaunch = yield* resolveEditorLaunch( { cwd: "/tmp/workspace", editor: "vscode" }, - "darwin", + { platform: "darwin" }, ); assert.deepEqual(vscodeLaunch, { command: "code", @@ -36,7 +37,7 @@ describe("resolveEditorLaunch", () => { const zedLaunch = yield* resolveEditorLaunch( { cwd: "/tmp/workspace", editor: "zed" }, - "darwin", + { platform: "darwin" }, ); assert.deepEqual(zedLaunch, { command: "zed", @@ -49,7 +50,7 @@ describe("resolveEditorLaunch", () => { Effect.gen(function* () { const lineOnly = yield* resolveEditorLaunch( { cwd: "/tmp/workspace/AGENTS.md:48", editor: "cursor" }, - "darwin", + { platform: "darwin" }, ); assert.deepEqual(lineOnly, { command: "cursor", @@ -58,7 +59,7 @@ describe("resolveEditorLaunch", () => { const lineAndColumn = yield* resolveEditorLaunch( { cwd: "/tmp/workspace/src/open.ts:71:5", editor: "cursor" }, - "darwin", + { platform: "darwin" }, ); assert.deepEqual(lineAndColumn, { command: "cursor", @@ -67,7 +68,7 @@ describe("resolveEditorLaunch", () => { const vscodeLineAndColumn = yield* resolveEditorLaunch( { cwd: "/tmp/workspace/src/open.ts:71:5", editor: "vscode" }, - "darwin", + { platform: "darwin" }, ); assert.deepEqual(vscodeLineAndColumn, { command: "code", @@ -76,7 +77,7 @@ describe("resolveEditorLaunch", () => { const zedLineAndColumn = yield* resolveEditorLaunch( { cwd: "/tmp/workspace/src/open.ts:71:5", editor: "zed" }, - "darwin", + { platform: "darwin" }, ); assert.deepEqual(zedLineAndColumn, { command: "zed", @@ -89,7 +90,7 @@ describe("resolveEditorLaunch", () => { Effect.gen(function* () { const launch1 = yield* resolveEditorLaunch( { cwd: "/tmp/workspace", editor: "file-manager" }, - "darwin", + { platform: "darwin" }, ); assert.deepEqual(launch1, { command: "open", @@ -98,7 +99,7 @@ describe("resolveEditorLaunch", () => { const launch2 = yield* resolveEditorLaunch( { cwd: "C:\\workspace", editor: "file-manager" }, - "win32", + { platform: "win32" }, ); assert.deepEqual(launch2, { command: "explorer", @@ -107,7 +108,7 @@ describe("resolveEditorLaunch", () => { const launch3 = yield* resolveEditorLaunch( { cwd: "/tmp/workspace", editor: "file-manager" }, - "linux", + { platform: "linux" }, ); assert.deepEqual(launch3, { command: "xdg-open", @@ -115,6 +116,112 @@ describe("resolveEditorLaunch", () => { }); }), ); + + it.effect("prefers linux editor shims in wsl-hosted mode when available", () => + Effect.gen(function* () { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-open-wsl-linux-editor-")); + try { + fs.writeFileSync(path.join(dir, "code"), "#!/bin/sh\n", { mode: 0o755 }); + const launch = yield* resolveEditorLaunch( + { cwd: "/home/julius/project/src/open.ts:71:5", editor: "vscode" }, + { + platform: "linux", + runtimeEnvironment: { + platform: "linux", + pathStyle: "posix", + isWsl: true, + windowsInteropMode: "wsl-hosted", + wslDistroName: "Ubuntu", + }, + env: { + PATH: dir, + }, + }, + ); + assert.deepEqual(launch, { + command: "code", + args: ["--goto", "/home/julius/project/src/open.ts:71:5"], + }); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }), + ); + + it.effect("falls back to windows editor executables in wsl-hosted mode", () => + Effect.gen(function* () { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-open-wsl-win-editor-")); + try { + fs.writeFileSync(path.join(dir, "code.exe"), "MZ", { mode: 0o755 }); + const launch = yield* resolveEditorLaunch( + { cwd: "/home/julius/project/src/open.ts:71:5", editor: "vscode" }, + { + platform: "linux", + runtimeEnvironment: { + platform: "linux", + pathStyle: "posix", + isWsl: true, + windowsInteropMode: "wsl-hosted", + wslDistroName: "Ubuntu", + }, + env: { + PATH: dir, + }, + translateWslPathToWindows: (target) => + target.replace( + "/home/julius/project", + "\\\\wsl.localhost\\Ubuntu\\home\\julius\\project", + ), + }, + ); + assert.deepEqual(launch, { + command: "code.exe", + args: [ + "--goto", + "\\\\wsl.localhost\\Ubuntu\\home\\julius\\project/src/open.ts:71:5", + ], + }); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }), + ); + + it.effect("translates file-manager targets for wsl-hosted mode", () => + Effect.gen(function* () { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-open-wsl-file-manager-")); + try { + fs.writeFileSync(path.join(dir, "explorer.exe"), "MZ", { mode: 0o755 }); + const launch = yield* resolveEditorLaunch( + { cwd: "/home/julius/project/src/open.ts:71:5", editor: "file-manager" }, + { + platform: "linux", + runtimeEnvironment: { + platform: "linux", + pathStyle: "posix", + isWsl: true, + windowsInteropMode: "wsl-hosted", + wslDistroName: "Ubuntu", + }, + env: { + PATH: dir, + }, + translateWslPathToWindows: (target) => + target.replace( + "/home/julius/project", + "\\\\wsl.localhost\\Ubuntu\\home\\julius\\project", + ), + }, + ); + assert.deepEqual(launch, { + command: "explorer.exe", + args: ["\\\\wsl.localhost\\Ubuntu\\home\\julius\\project/src/open.ts"], + }); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }), + ); }); describe("launchDetached", () => { @@ -210,13 +317,94 @@ describe("resolveAvailableEditors", () => { try { fs.writeFileSync(path.join(dir, "cursor.CMD"), "@echo off\r\n", "utf8"); fs.writeFileSync(path.join(dir, "explorer.EXE"), "MZ", "utf8"); - const editors = resolveAvailableEditors("win32", { - PATH: dir, - PATHEXT: ".COM;.EXE;.BAT;.CMD", + const editors = resolveAvailableEditors({ + platform: "win32", + env: { + PATH: dir, + PATHEXT: ".COM;.EXE;.BAT;.CMD", + }, }); assert.deepEqual(editors, ["cursor", "file-manager"]); } finally { fs.rmSync(dir, { recursive: true, force: true }); } }); + + it("accepts windows editor executables in wsl-hosted mode", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-editors-wsl-")); + try { + fs.writeFileSync(path.join(dir, "code.exe"), "MZ", { mode: 0o755 }); + fs.writeFileSync(path.join(dir, "explorer.exe"), "MZ", { mode: 0o755 }); + const editors = resolveAvailableEditors({ + platform: "linux", + runtimeEnvironment: { + platform: "linux", + pathStyle: "posix", + isWsl: true, + windowsInteropMode: "wsl-hosted", + wslDistroName: "Ubuntu", + }, + env: { + PATH: dir, + }, + }); + assert.deepEqual(editors, ["vscode", "file-manager"]); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); +}); + +describe("resolveBrowserLaunch", () => { + it("prefers wslview in wsl-hosted mode", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-browser-wsl-")); + try { + fs.writeFileSync(path.join(dir, "wslview"), "#!/bin/sh\n", { mode: 0o755 }); + const launch = resolveBrowserLaunch("http://localhost:3773", { + platform: "linux", + runtimeEnvironment: { + platform: "linux", + pathStyle: "posix", + isWsl: true, + windowsInteropMode: "wsl-hosted", + wslDistroName: "Ubuntu", + }, + env: { + PATH: dir, + }, + }); + assert.deepEqual(launch, { + command: "wslview", + args: ["http://localhost:3773"], + }); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); + + it("falls back to explorer.exe in wsl-hosted mode", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-browser-wsl-explorer-")); + try { + fs.writeFileSync(path.join(dir, "explorer.exe"), "MZ", { mode: 0o755 }); + const launch = resolveBrowserLaunch("http://localhost:3773", { + platform: "linux", + runtimeEnvironment: { + platform: "linux", + pathStyle: "posix", + isWsl: true, + windowsInteropMode: "wsl-hosted", + wslDistroName: "Ubuntu", + }, + env: { + PATH: dir, + }, + }); + assert.deepEqual(launch, { + command: "explorer.exe", + args: ["http://localhost:3773"], + }); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); }); diff --git a/apps/server/src/open.ts b/apps/server/src/open.ts index 5c742fba9d..b933ad89a0 100644 --- a/apps/server/src/open.ts +++ b/apps/server/src/open.ts @@ -6,12 +6,13 @@ * * @module Open */ -import { spawn } from "node:child_process"; import { accessSync, constants, statSync } from "node:fs"; import { extname, join } from "node:path"; -import { EDITORS, type EditorId } from "@t3tools/contracts"; +import { EDITORS, type EditorId, type ServerRuntimeEnvironment } from "@t3tools/contracts"; import { ServiceMap, Schema, Effect, Layer } from "effect"; +import { spawnDetachedProcess, spawnProcessSync } from "./processRunner"; +import { detectServerRuntimeEnvironment } from "./runtimeEnvironment"; // ============================== // Definitions @@ -37,7 +38,24 @@ interface CommandAvailabilityOptions { readonly env?: NodeJS.ProcessEnv; } +interface ResolveOpenOptions extends CommandAvailabilityOptions { + readonly runtimeEnvironment?: ServerRuntimeEnvironment; + readonly translateWslPathToWindows?: (target: string) => string; +} + const LINE_COLUMN_SUFFIX_PATTERN = /:\d+(?::\d+)?$/; +const WINDOWS_ABSOLUTE_PATH_PATTERN = /^[A-Za-z]:[\\/]/; +const WINDOWS_UNC_PATH_PATTERN = /^\\\\/; + +function detectRuntimeEnvironment(options: ResolveOpenOptions): ServerRuntimeEnvironment { + return ( + options.runtimeEnvironment ?? + detectServerRuntimeEnvironment({ + ...(options.platform !== undefined ? { platform: options.platform } : {}), + ...(options.env !== undefined ? { env: options.env } : {}), + }) + ); +} function shouldUseGotoFlag(editorId: EditorId, target: string): boolean { return (editorId === "cursor" || editorId === "vscode") && LINE_COLUMN_SUFFIX_PATTERN.test(target); @@ -160,14 +178,20 @@ export function isCommandAvailable( } export function resolveAvailableEditors( - platform: NodeJS.Platform = process.platform, - env: NodeJS.ProcessEnv = process.env, + options: ResolveOpenOptions = {}, ): ReadonlyArray { + const platform = options.platform ?? process.platform; + const env = options.env ?? process.env; + const runtimeEnvironment = detectRuntimeEnvironment({ ...options, platform, env }); const available: EditorId[] = []; for (const editor of EDITORS) { - const command = editor.command ?? fileManagerCommandForPlatform(platform); - if (isCommandAvailable(command, { platform, env })) { + const launch = resolveOpenCommand(editor.id, { + platform, + env, + runtimeEnvironment, + }); + if (launch.some((entry) => isCommandAvailable(entry, { platform, env }))) { available.push(editor.id); } } @@ -201,60 +225,223 @@ export class Open extends ServiceMap.Service()("t3/open") {} // Implementations // ============================== +function splitPathAndPosition(value: string): { + path: string; + line: string | undefined; + column: string | undefined; +} { + let path = value; + let column: string | undefined; + let line: string | undefined; + + const columnMatch = path.match(/:(\d+)$/); + if (!columnMatch?.[1]) { + return { path, line: undefined, column: undefined }; + } + + column = columnMatch[1]; + path = path.slice(0, -columnMatch[0].length); + + const lineMatch = path.match(/:(\d+)$/); + if (lineMatch?.[1]) { + line = lineMatch[1]; + path = path.slice(0, -lineMatch[0].length); + } else { + line = column; + column = undefined; + } + + return { path, line, column }; +} + +function stripLineColumnSuffix(value: string): string { + const { path } = splitPathAndPosition(value); + return path; +} + +function formatPathWithPosition(input: { + readonly path: string; + readonly line?: string; + readonly column?: string; +}): string { + if (!input.line) { + return input.path; + } + return `${input.path}:${input.line}${input.column ? `:${input.column}` : ""}`; +} + +function isWindowsPath(value: string): boolean { + return WINDOWS_ABSOLUTE_PATH_PATTERN.test(value) || WINDOWS_UNC_PATH_PATTERN.test(value); +} + +function defaultTranslateWslPathToWindows(target: string): string { + const parsed = splitPathAndPosition(target); + if (parsed.path.length === 0 || !parsed.path.startsWith("/") || isWindowsPath(parsed.path)) { + return target; + } + + const result = spawnProcessSync("wslpath", ["-w", parsed.path], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }); + if (result.error) { + throw result.error; + } + if (result.status !== 0) { + const detail = result.stderr.trim() || result.stdout.trim() || "wslpath failed"; + throw new Error(detail); + } + + const translatedPath = result.stdout.trim(); + if (translatedPath.length === 0) { + throw new Error("wslpath returned an empty path"); + } + + return formatPathWithPosition({ + path: translatedPath, + ...(parsed.line ? { line: parsed.line } : {}), + ...(parsed.column ? { column: parsed.column } : {}), + }); +} + +function resolveOpenCommand( + editorId: EditorId, + options: ResolveOpenOptions, +): ReadonlyArray { + const platform = options.platform ?? process.platform; + const env = options.env ?? process.env; + const runtimeEnvironment = detectRuntimeEnvironment({ ...options, platform, env }); + const editorDef = EDITORS.find((editor) => editor.id === editorId); + if (!editorDef) { + return []; + } + + if (editorId === "file-manager") { + if (runtimeEnvironment.windowsInteropMode === "wsl-hosted") { + return ["explorer.exe", "xdg-open"]; + } + return [fileManagerCommandForPlatform(platform)]; + } + + if (!editorDef.command) { + return []; + } + + if (runtimeEnvironment.windowsInteropMode === "wsl-hosted") { + return [editorDef.command, `${editorDef.command}.exe`]; + } + + return [editorDef.command]; +} + +export function resolveBrowserLaunch( + target: string, + options: ResolveOpenOptions = {}, +): EditorLaunch | null { + const platform = options.platform ?? process.platform; + const env = options.env ?? process.env; + const runtimeEnvironment = detectRuntimeEnvironment({ ...options, platform, env }); + + if (runtimeEnvironment.windowsInteropMode === "wsl-hosted") { + if (isCommandAvailable("wslview", { platform, env })) { + return { command: "wslview", args: [target] }; + } + if (isCommandAvailable("explorer.exe", { platform, env })) { + return { command: "explorer.exe", args: [target] }; + } + } + + return null; +} + export const resolveEditorLaunch = Effect.fnUntraced(function* ( input: OpenInEditorInput, - platform: NodeJS.Platform = process.platform, + options: ResolveOpenOptions = {}, ): Effect.fn.Return { + const platform = options.platform ?? process.platform; + const env = options.env ?? process.env; + const runtimeEnvironment = detectRuntimeEnvironment({ ...options, platform, env }); const editorDef = EDITORS.find((editor) => editor.id === input.editor); if (!editorDef) { return yield* new OpenError({ message: `Unknown editor: ${input.editor}` }); } + const translateWslPathToWindows = options.translateWslPathToWindows ?? defaultTranslateWslPathToWindows; + if (editorDef.command) { - return shouldUseGotoFlag(editorDef.id, input.cwd) - ? { command: editorDef.command, args: ["--goto", input.cwd] } - : { command: editorDef.command, args: [input.cwd] }; + const candidateCommands = resolveOpenCommand(editorDef.id, { + platform, + env, + runtimeEnvironment, + }); + const command = + candidateCommands.find((entry) => isCommandAvailable(entry, { platform, env })) ?? + candidateCommands[0]; + if (!command) { + return yield* new OpenError({ message: `Editor command not found: ${editorDef.command}` }); + } + + const usesWindowsPath = runtimeEnvironment.windowsInteropMode === "wsl-hosted" && command.endsWith(".exe"); + const target = usesWindowsPath + ? yield* Effect.try({ + try: () => translateWslPathToWindows(input.cwd), + catch: (cause) => + new OpenError({ + message: "Failed to translate WSL path for Windows editor launch", + cause, + }), + }) + : input.cwd; + + return shouldUseGotoFlag(editorDef.id, target) + ? { command, args: ["--goto", target] } + : { command, args: [target] }; } if (editorDef.id !== "file-manager") { return yield* new OpenError({ message: `Unsupported editor: ${input.editor}` }); } - return { command: fileManagerCommandForPlatform(platform), args: [input.cwd] }; + const candidateCommands = resolveOpenCommand(editorDef.id, { + platform, + env, + runtimeEnvironment, + }); + const command = + candidateCommands.find((entry) => isCommandAvailable(entry, { platform, env })) ?? + candidateCommands[0]; + if (!command) { + return yield* new OpenError({ message: "Editor command not found: file-manager" }); + } + + let target = stripLineColumnSuffix(input.cwd); + if (runtimeEnvironment.windowsInteropMode === "wsl-hosted" && command === "explorer.exe") { + target = yield* Effect.try({ + try: () => stripLineColumnSuffix(translateWslPathToWindows(target)), + catch: (cause) => + new OpenError({ + message: "Failed to translate WSL path for Windows file manager launch", + cause, + }), + }); + } + + return { command, args: [target] }; }); export const launchDetached = (launch: EditorLaunch) => - Effect.gen(function* () { - if (!isCommandAvailable(launch.command)) { - return yield* new OpenError({ message: `Editor command not found: ${launch.command}` }); - } - - yield* Effect.callback((resume) => { - let child; - try { - child = spawn(launch.command, [...launch.args], { - detached: true, - stdio: "ignore", - shell: process.platform === "win32", - }); - } catch (error) { - return resume( - Effect.fail( - new OpenError({ message: "failed to spawn detached process", cause: error }), - ), - ); + Effect.tryPromise({ + try: async () => { + if (!isCommandAvailable(launch.command)) { + throw new OpenError({ message: `Editor command not found: ${launch.command}` }); } - const handleSpawn = () => { - child.unref(); - resume(Effect.void); - }; - - child.once("spawn", handleSpawn); - child.once("error", (cause) => - resume(Effect.fail(new OpenError({ message: "failed to spawn detached process", cause }))), - ); - }); + await spawnDetachedProcess(launch.command, [...launch.args]); + }, + catch: (cause) => + Schema.is(OpenError)(cause) + ? cause + : new OpenError({ message: "failed to spawn detached process", cause }), }); const make = Effect.gen(function* () { @@ -264,11 +451,16 @@ const make = Effect.gen(function* () { }); return { - openBrowser: (target) => - Effect.tryPromise({ + openBrowser: (target) => { + const launch = resolveBrowserLaunch(target); + if (launch) { + return launchDetached(launch); + } + return Effect.tryPromise({ try: () => open.default(target), catch: (cause) => new OpenError({ message: "Browser auto-open failed", cause }), - }), + }); + }, openInEditor: (input) => Effect.flatMap(resolveEditorLaunch(input), launchDetached), } satisfies OpenShape; }); diff --git a/apps/server/src/processRunner.test.ts b/apps/server/src/processRunner.test.ts index dd909116d4..0df630a8e3 100644 --- a/apps/server/src/processRunner.test.ts +++ b/apps/server/src/processRunner.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { runProcess } from "./processRunner"; +import { runProcess, spawnDetachedProcess, spawnProcessSync } from "./processRunner"; describe("runProcess", () => { it("fails when output exceeds max buffer in default mode", async () => { @@ -20,4 +20,20 @@ describe("runProcess", () => { expect(result.stdoutTruncated).toBe(true); expect(result.stderrTruncated).toBe(false); }); + + it("runs sync commands through the shared spawn strategy", () => { + const result = spawnProcessSync("node", ["-e", "process.stdout.write('ok')"], { + stdio: ["ignore", "pipe", "pipe"], + }); + + expect(result.error).toBeUndefined(); + expect(result.status).toBe(0); + expect(result.stdout).toBe("ok"); + }); + + it("spawns detached commands through the shared spawn strategy", async () => { + await expect( + spawnDetachedProcess(process.execPath, ["-e", "process.exit(0)"]), + ).resolves.toBeUndefined(); + }); }); diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts index cbf4b76063..b522e8c6e6 100644 --- a/apps/server/src/processRunner.ts +++ b/apps/server/src/processRunner.ts @@ -1,4 +1,33 @@ -import { type ChildProcess as ChildProcessHandle, spawn, spawnSync } from "node:child_process"; +import { + type ChildProcess as ChildProcessHandle, + type ChildProcessWithoutNullStreams, + spawn, + spawnSync, + type StdioOptions, +} from "node:child_process"; + +import type { ServerRuntimeEnvironment } from "@t3tools/contracts"; + +import { detectServerRuntimeEnvironment } from "./runtimeEnvironment"; + +interface ProcessSpawnBaseOptions { + cwd?: string | undefined; + env?: NodeJS.ProcessEnv | undefined; + runtimeEnvironment?: ServerRuntimeEnvironment | undefined; + shell?: boolean | undefined; +} + +export interface ProcessSpawnOptions extends ProcessSpawnBaseOptions { + stdio?: StdioOptions | undefined; + detached?: boolean | undefined; +} + +export interface ProcessSpawnSyncOptions extends ProcessSpawnBaseOptions { + stdio?: StdioOptions | undefined; + detached?: boolean | undefined; + encoding?: BufferEncoding | undefined; + input?: string | undefined; +} export interface ProcessRunOptions { cwd?: string | undefined; @@ -8,6 +37,8 @@ export interface ProcessRunOptions { allowNonZeroExit?: boolean | undefined; maxBufferBytes?: number | undefined; outputMode?: "error" | "truncate" | undefined; + runtimeEnvironment?: ServerRuntimeEnvironment | undefined; + shell?: boolean | undefined; } export interface ProcessRunResult { @@ -24,6 +55,89 @@ function commandLabel(command: string, args: readonly string[]): string { return [command, ...args].join(" "); } +function resolveRuntimeEnvironment( + runtimeEnvironment: ServerRuntimeEnvironment | undefined, +): ServerRuntimeEnvironment { + return runtimeEnvironment ?? detectServerRuntimeEnvironment(); +} + +function shouldUseShell(options: ProcessSpawnBaseOptions): boolean { + if (options.shell !== undefined) { + return options.shell; + } + + return resolveRuntimeEnvironment(options.runtimeEnvironment).platform === "windows"; +} + +function toSpawnOptions(options: ProcessSpawnOptions) { + return { + cwd: options.cwd, + env: options.env, + shell: shouldUseShell(options), + ...(options.stdio !== undefined ? { stdio: options.stdio } : {}), + ...(options.detached !== undefined ? { detached: options.detached } : {}), + }; +} + +export function spawnProcess( + command: string, + args: readonly string[], + options: ProcessSpawnOptions = {}, +): ChildProcessHandle { + return spawn(command, args, toSpawnOptions(options)); +} + +export function spawnPipedProcess( + command: string, + args: readonly string[], + options: Omit = {}, +): ChildProcessWithoutNullStreams { + return spawnProcess(command, args, { + ...options, + stdio: "pipe", + }) as ChildProcessWithoutNullStreams; +} + +export function spawnProcessSync( + command: string, + args: readonly string[], + options: ProcessSpawnSyncOptions = {}, +) { + return spawnSync(command, args, { + cwd: options.cwd, + env: options.env, + shell: shouldUseShell(options), + encoding: options.encoding ?? "utf8", + ...(options.stdio !== undefined ? { stdio: options.stdio } : {}), + ...(options.detached !== undefined ? { detached: options.detached } : {}), + ...(options.input !== undefined ? { input: options.input } : {}), + }); +} + +export function spawnDetachedProcess( + command: string, + args: readonly string[], + options: Omit = {}, +): Promise { + return new Promise((resolve, reject) => { + const child = spawnProcess(command, args, { + ...options, + detached: true, + stdio: "ignore", + }); + + const handleSpawn = () => { + child.unref(); + resolve(); + }; + + child.once("spawn", handleSpawn); + child.once("error", (error) => { + reject(normalizeSpawnError(command, args, error)); + }); + }); +} + function normalizeSpawnError(command: string, args: readonly string[], error: unknown): Error { if (!(error instanceof Error)) { return new Error(`Failed to run ${commandLabel(command, args)}.`); @@ -37,8 +151,12 @@ function normalizeSpawnError(command: string, args: readonly string[], error: un return new Error(`Failed to run ${commandLabel(command, args)}: ${error.message}`); } -function isWindowsCommandNotFound(code: number | null, stderr: string): boolean { - if (process.platform !== "win32") return false; +function isWindowsCommandNotFound( + code: number | null, + stderr: string, + runtimeEnvironment?: ServerRuntimeEnvironment, +): boolean { + if (resolveRuntimeEnvironment(runtimeEnvironment).platform !== "windows") return false; if (code === 9009) return true; return /is not recognized as an internal or external command/i.test(stderr); } @@ -47,8 +165,9 @@ function normalizeExitError( command: string, args: readonly string[], result: ProcessRunResult, + runtimeEnvironment?: ServerRuntimeEnvironment, ): Error { - if (isWindowsCommandNotFound(result.code, result.stderr)) { + if (isWindowsCommandNotFound(result.code, result.stderr, runtimeEnvironment)) { return new Error(`Command not found: ${command}`); } @@ -85,11 +204,30 @@ const DEFAULT_MAX_BUFFER_BYTES = 8 * 1024 * 1024; * wrapper, leaving the actual command running. Use `taskkill /T` to kill the * entire process tree instead. */ -function killChild(child: ChildProcessHandle, signal: NodeJS.Signals = "SIGTERM"): void { - if (process.platform === "win32" && child.pid !== undefined) { +export function killProcessTree( + child: ChildProcessHandle, + options: { + runtimeEnvironment?: ServerRuntimeEnvironment | undefined; + signal?: NodeJS.Signals | undefined; + } = {}, +): void { + const signal = options.signal ?? "SIGTERM"; + if ( + resolveRuntimeEnvironment(options.runtimeEnvironment).platform === "windows" && + child.pid !== undefined + ) { try { - spawnSync("taskkill", ["/pid", String(child.pid), "/T", "/F"], { stdio: "ignore" }); - return; + const result = spawnProcessSync("taskkill", ["/pid", String(child.pid), "/T", "/F"], { + stdio: "ignore", + shell: false, + runtimeEnvironment: options.runtimeEnvironment, + }); + if (!result.error && result.status === 0) { + return; + } + if (result.error) { + throw result.error; + } } catch { // fallback to direct kill } @@ -135,12 +273,7 @@ export async function runProcess( const outputMode = options.outputMode ?? "error"; return new Promise((resolve, reject) => { - const child = spawn(command, args, { - cwd: options.cwd, - env: options.env, - stdio: "pipe", - shell: process.platform === "win32", - }); + const child = spawnPipedProcess(command, args, options); let stdout = ""; let stderr = ""; @@ -154,9 +287,15 @@ export async function runProcess( const timeoutTimer = setTimeout(() => { timedOut = true; - killChild(child, "SIGTERM"); + killProcessTree(child, { + runtimeEnvironment: options.runtimeEnvironment, + signal: "SIGTERM", + }); forceKillTimer = setTimeout(() => { - killChild(child, "SIGKILL"); + killProcessTree(child, { + runtimeEnvironment: options.runtimeEnvironment, + signal: "SIGKILL", + }); }, 1_000); }, timeoutMs); @@ -171,7 +310,10 @@ export async function runProcess( }; const fail = (error: Error): void => { - killChild(child, "SIGTERM"); + killProcessTree(child, { + runtimeEnvironment: options.runtimeEnvironment, + signal: "SIGTERM", + }); finalize(() => { reject(error); }); @@ -244,7 +386,7 @@ export async function runProcess( finalize(() => { if (!options.allowNonZeroExit && (timedOut || (code !== null && code !== 0))) { - reject(normalizeExitError(command, args, result)); + reject(normalizeExitError(command, args, result, options.runtimeEnvironment)); return; } resolve(result); diff --git a/apps/server/src/provider/Layers/ProviderHealth.test.ts b/apps/server/src/provider/Layers/ProviderHealth.test.ts index 58eac64da0..e663b07055 100644 --- a/apps/server/src/provider/Layers/ProviderHealth.test.ts +++ b/apps/server/src/provider/Layers/ProviderHealth.test.ts @@ -1,93 +1,63 @@ import assert from "node:assert/strict"; import { it } from "@effect/vitest"; -import { Effect, Layer, Sink, Stream } from "effect"; -import * as PlatformError from "effect/PlatformError"; -import { ChildProcessSpawner } from "effect/unstable/process"; +import { Effect } from "effect"; import { checkCodexProviderStatus, parseAuthStatusFromOutput } from "./ProviderHealth"; // ── Test helpers ──────────────────────────────────────────────────── -const encoder = new TextEncoder(); - -function mockHandle(result: { stdout: string; stderr: string; code: number }) { - return ChildProcessSpawner.makeHandle({ - pid: ChildProcessSpawner.ProcessId(1), - exitCode: Effect.succeed(ChildProcessSpawner.ExitCode(result.code)), - isRunning: Effect.succeed(false), - kill: () => Effect.void, - stdin: Sink.drain, - stdout: Stream.make(encoder.encode(result.stdout)), - stderr: Stream.make(encoder.encode(result.stderr)), - all: Stream.empty, - getInputFd: () => Sink.drain, - getOutputFd: () => Stream.empty, - }); -} - -function mockSpawnerLayer( +function mockRunner( handler: (args: ReadonlyArray) => { stdout: string; stderr: string; code: number }, ) { - return Layer.succeed( - ChildProcessSpawner.ChildProcessSpawner, - ChildProcessSpawner.make((command) => { - const cmd = command as unknown as { args: ReadonlyArray }; - return Effect.succeed(mockHandle(handler(cmd.args))); - }), - ); + return (args: ReadonlyArray) => Promise.resolve(handler(args)); } -function failingSpawnerLayer(description: string) { - return Layer.succeed( - ChildProcessSpawner.ChildProcessSpawner, - ChildProcessSpawner.make(() => - Effect.fail( - PlatformError.systemError({ - _tag: "NotFound", - module: "ChildProcess", - method: "spawn", - description, - }), - ), - ), - ); +function failingRunner(description: string) { + return () => Promise.reject(new Error(description)); } // ── Tests ─────────────────────────────────────────────────────────── it.effect("returns ready when codex is installed and authenticated", () => Effect.gen(function* () { - const status = yield* checkCodexProviderStatus; - assert.strictEqual(status.provider, "codex"); - assert.strictEqual(status.status, "ready"); - assert.strictEqual(status.available, true); - assert.strictEqual(status.authStatus, "authenticated"); - }).pipe( - Effect.provide( - mockSpawnerLayer((args) => { + const status = yield* checkCodexProviderStatus( + mockRunner((args) => { const joined = args.join(" "); if (joined === "--version") return { stdout: "codex 1.0.0\n", stderr: "", code: 0 }; if (joined === "login status") return { stdout: "Logged in\n", stderr: "", code: 0 }; throw new Error(`Unexpected args: ${joined}`); }), - ), - ), + ); + assert.strictEqual(status.provider, "codex"); + assert.strictEqual(status.status, "ready"); + assert.strictEqual(status.available, true); + assert.strictEqual(status.authStatus, "authenticated"); + }), ); it.effect("returns unavailable when codex is missing", () => Effect.gen(function* () { - const status = yield* checkCodexProviderStatus; + const status = yield* checkCodexProviderStatus(failingRunner("spawn codex ENOENT")); assert.strictEqual(status.provider, "codex"); assert.strictEqual(status.status, "error"); assert.strictEqual(status.available, false); assert.strictEqual(status.authStatus, "unknown"); assert.strictEqual(status.message, "Codex CLI (`codex`) is not installed or not on PATH."); - }).pipe(Effect.provide(failingSpawnerLayer("spawn codex ENOENT"))), + }), ); it.effect("returns unauthenticated when auth probe reports login required", () => Effect.gen(function* () { - const status = yield* checkCodexProviderStatus; + const status = yield* checkCodexProviderStatus( + mockRunner((args) => { + const joined = args.join(" "); + if (joined === "--version") return { stdout: "codex 1.0.0\n", stderr: "", code: 0 }; + if (joined === "login status") { + return { stdout: "", stderr: "Not logged in. Run codex login.", code: 1 }; + } + throw new Error(`Unexpected args: ${joined}`); + }), + ); assert.strictEqual(status.provider, "codex"); assert.strictEqual(status.status, "error"); assert.strictEqual(status.available, true); @@ -96,25 +66,22 @@ it.effect("returns unauthenticated when auth probe reports login required", () = status.message, "Codex CLI is not authenticated. Run `codex login` and try again.", ); - }).pipe( - Effect.provide( - mockSpawnerLayer((args) => { - const joined = args.join(" "); - if (joined === "--version") return { stdout: "codex 1.0.0\n", stderr: "", code: 0 }; - if (joined === "login status") { - return { stdout: "", stderr: "Not logged in. Run codex login.", code: 1 }; - } - throw new Error(`Unexpected args: ${joined}`); - }), - ), - ), + }), ); it.effect( - "returns unauthenticated when login status output includes 'not logged in'", + "returns unauthenticated when login status output includes 'not logged in'", () => Effect.gen(function* () { - const status = yield* checkCodexProviderStatus; + const status = yield* checkCodexProviderStatus( + mockRunner((args) => { + const joined = args.join(" "); + if (joined === "--version") return { stdout: "codex 1.0.0\n", stderr: "", code: 0 }; + if (joined === "login status") + return { stdout: "Not logged in\n", stderr: "", code: 1 }; + throw new Error(`Unexpected args: ${joined}`); + }), + ); assert.strictEqual(status.provider, "codex"); assert.strictEqual(status.status, "error"); assert.strictEqual(status.available, true); @@ -123,22 +90,21 @@ it.effect( status.message, "Codex CLI is not authenticated. Run `codex login` and try again.", ); - }).pipe( - Effect.provide( - mockSpawnerLayer((args) => { - const joined = args.join(" "); - if (joined === "--version") return { stdout: "codex 1.0.0\n", stderr: "", code: 0 }; - if (joined === "login status") - return { stdout: "Not logged in\n", stderr: "", code: 1 }; - throw new Error(`Unexpected args: ${joined}`); - }), - ), - ), + }), ); it.effect("returns warning when login status command is unsupported", () => Effect.gen(function* () { - const status = yield* checkCodexProviderStatus; + const status = yield* checkCodexProviderStatus( + mockRunner((args) => { + const joined = args.join(" "); + if (joined === "--version") return { stdout: "codex 1.0.0\n", stderr: "", code: 0 }; + if (joined === "login status") { + return { stdout: "", stderr: "error: unknown command 'login'", code: 2 }; + } + throw new Error(`Unexpected args: ${joined}`); + }), + ); assert.strictEqual(status.provider, "codex"); assert.strictEqual(status.status, "warning"); assert.strictEqual(status.available, true); @@ -147,18 +113,7 @@ it.effect("returns warning when login status command is unsupported", () => status.message, "Codex CLI authentication status command is unavailable in this Codex version.", ); - }).pipe( - Effect.provide( - mockSpawnerLayer((args) => { - const joined = args.join(" "); - if (joined === "--version") return { stdout: "codex 1.0.0\n", stderr: "", code: 0 }; - if (joined === "login status") { - return { stdout: "", stderr: "error: unknown command 'login'", code: 2 }; - } - throw new Error(`Unexpected args: ${joined}`); - }), - ), - ), + }), ); // ── Pure function tests ───────────────────────────────────────────── diff --git a/apps/server/src/provider/Layers/ProviderHealth.ts b/apps/server/src/provider/Layers/ProviderHealth.ts index e936275866..75e1ab40ae 100644 --- a/apps/server/src/provider/Layers/ProviderHealth.ts +++ b/apps/server/src/provider/Layers/ProviderHealth.ts @@ -4,7 +4,7 @@ * Performs one-time provider readiness probes when the server starts and * keeps the resulting snapshot in memory for `server.getConfig`. * - * Uses effect's ChildProcessSpawner to run CLI probes natively. + * Uses the shared process runner to run CLI probes natively. * * @module ProviderHealthLive */ @@ -13,20 +13,43 @@ import type { ServerProviderStatus, ServerProviderStatusState, } from "@t3tools/contracts"; -import { Effect, Layer, Option, Result, Stream } from "effect"; -import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; +import { Data, Effect, Layer, Result } from "effect"; +import { runProcess } from "../../processRunner"; import { ProviderHealth, type ProviderHealthShape } from "../Services/ProviderHealth"; const DEFAULT_TIMEOUT_MS = 4_000; const CODEX_PROVIDER = "codex" as const; +class ProviderHealthCommandError extends Data.TaggedError("ProviderHealthCommandError")<{ + readonly message: string; + readonly cause?: unknown; +}> {} + // ── Pure helpers ──────────────────────────────────────────────────── export interface CommandResult { readonly stdout: string; readonly stderr: string; readonly code: number; + readonly timedOut?: boolean; +} + +function errorMessage(error: unknown): string { + if (error instanceof Error) { + return error.message; + } + + if ( + error && + typeof error === "object" && + "message" in error && + typeof (error as { message?: unknown }).message === "string" + ) { + return (error as { message: string }).message; + } + + return String(error); } function nonEmptyTrimmed(value: string | undefined): string | undefined { @@ -36,8 +59,7 @@ function nonEmptyTrimmed(value: string | undefined): string | undefined { } function isCommandMissingCause(error: unknown): boolean { - if (!(error instanceof Error)) return false; - const lower = error.message.toLowerCase(); + const lower = errorMessage(error).toLowerCase(); return ( lower.includes("command not found: codex") || lower.includes("spawn codex enoent") || @@ -162,140 +184,115 @@ export function parseAuthStatusFromOutput(result: CommandResult): { }; } -// ── Effect-native command execution ───────────────────────────────── - -const collectStreamAsString = (stream: Stream.Stream): Effect.Effect => - Stream.runFold( - stream, - () => "", - (acc, chunk) => acc + new TextDecoder().decode(chunk), - ); +export type RunProviderHealthCommand = ( + args: ReadonlyArray, +) => Promise; -const runCodexCommand = (args: ReadonlyArray) => - Effect.gen(function* () { - const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; - const command = ChildProcess.make("codex", [...args], { - shell: process.platform === "win32", - }); +const defaultRunCodexCommand: RunProviderHealthCommand = async (args) => { + const result = await runProcess("codex", [...args], { + allowNonZeroExit: true, + timeoutMs: DEFAULT_TIMEOUT_MS, + maxBufferBytes: 262_144, + outputMode: "truncate", + }); - const child = yield* spawner.spawn(command); - - const [stdout, stderr, exitCode] = yield* Effect.all( - [ - collectStreamAsString(child.stdout), - collectStreamAsString(child.stderr), - child.exitCode.pipe(Effect.map(Number)), - ], - { concurrency: "unbounded" }, - ); - - return { stdout, stderr, code: exitCode } satisfies CommandResult; - }).pipe(Effect.scoped); + return { + stdout: result.stdout, + stderr: result.stderr, + code: result.code ?? 0, + timedOut: result.timedOut, + } satisfies CommandResult; +}; // ── Health check ──────────────────────────────────────────────────── -export const checkCodexProviderStatus: Effect.Effect< - ServerProviderStatus, - never, - ChildProcessSpawner.ChildProcessSpawner -> = Effect.gen(function* () { - const checkedAt = new Date().toISOString(); - - // Probe 1: `codex --version` — is the CLI reachable? - const versionProbe = yield* runCodexCommand(["--version"]).pipe( - Effect.timeoutOption(DEFAULT_TIMEOUT_MS), - Effect.result, - ); - - if (Result.isFailure(versionProbe)) { - const error = versionProbe.failure; - return { - provider: CODEX_PROVIDER, - status: "error" as const, - available: false, - authStatus: "unknown" as const, - checkedAt, - message: isCommandMissingCause(error) - ? "Codex CLI (`codex`) is not installed or not on PATH." - : `Failed to execute Codex CLI health check: ${error instanceof Error ? error.message : String(error)}.`, - }; - } - - if (Option.isNone(versionProbe.success)) { - return { - provider: CODEX_PROVIDER, - status: "error" as const, - available: false, - authStatus: "unknown" as const, - checkedAt, - message: "Codex CLI is installed but failed to run. Timed out while running command.", - }; - } - - const version = versionProbe.success.value; - if (version.code !== 0) { - const detail = detailFromResult(version); - return { - provider: CODEX_PROVIDER, - status: "error" as const, - available: false, - authStatus: "unknown" as const, - checkedAt, - message: detail - ? `Codex CLI is installed but failed to run. ${detail}` - : "Codex CLI is installed but failed to run.", - }; - } +export const checkCodexProviderStatus = ( + runCodexCommand: RunProviderHealthCommand = defaultRunCodexCommand, +): Effect.Effect => + Effect.gen(function* () { + const checkedAt = new Date().toISOString(); + + // Probe 1: `codex --version` — is the CLI reachable? + const versionProbe = yield* Effect.tryPromise({ + try: () => runCodexCommand(["--version"]), + catch: (error) => + new ProviderHealthCommandError({ + message: errorMessage(error), + ...(error !== undefined ? { cause: error } : {}), + }), + }).pipe(Effect.result); + + if (Result.isFailure(versionProbe)) { + const error = versionProbe.failure; + return { + provider: CODEX_PROVIDER, + status: "error" as const, + available: false, + authStatus: "unknown" as const, + checkedAt, + message: isCommandMissingCause(error) + ? "Codex CLI (`codex`) is not installed or not on PATH." + : `Failed to execute Codex CLI health check: ${errorMessage(error)}.`, + }; + } - // Probe 2: `codex login status` — is the user authenticated? - const authProbe = yield* runCodexCommand(["login", "status"]).pipe( - Effect.timeoutOption(DEFAULT_TIMEOUT_MS), - Effect.result, - ); + const version = versionProbe.success; + if (version.code !== 0) { + const detail = detailFromResult(version); + return { + provider: CODEX_PROVIDER, + status: "error" as const, + available: false, + authStatus: "unknown" as const, + checkedAt, + message: detail + ? `Codex CLI is installed but failed to run. ${detail}` + : "Codex CLI is installed but failed to run.", + }; + } - if (Result.isFailure(authProbe)) { - const error = authProbe.failure; - return { - provider: CODEX_PROVIDER, - status: "warning" as const, - available: true, - authStatus: "unknown" as const, - checkedAt, - message: - error instanceof Error - ? `Could not verify Codex authentication status: ${error.message}.` - : "Could not verify Codex authentication status.", - }; - } + // Probe 2: `codex login status` — is the user authenticated? + const authProbe = yield* Effect.tryPromise({ + try: () => runCodexCommand(["login", "status"]), + catch: (error) => + new ProviderHealthCommandError({ + message: errorMessage(error), + ...(error !== undefined ? { cause: error } : {}), + }), + }).pipe(Effect.result); + + if (Result.isFailure(authProbe)) { + const error = authProbe.failure; + return { + provider: CODEX_PROVIDER, + status: "warning" as const, + available: true, + authStatus: "unknown" as const, + checkedAt, + message: + errorMessage(error).length > 0 + ? `Could not verify Codex authentication status: ${errorMessage(error)}.` + : "Could not verify Codex authentication status.", + }; + } - if (Option.isNone(authProbe.success)) { + const parsed = parseAuthStatusFromOutput(authProbe.success); return { provider: CODEX_PROVIDER, - status: "warning" as const, + status: parsed.status, available: true, - authStatus: "unknown" as const, + authStatus: parsed.authStatus, checkedAt, - message: "Could not verify Codex authentication status. Timed out while running command.", - }; - } - - const parsed = parseAuthStatusFromOutput(authProbe.success.value); - return { - provider: CODEX_PROVIDER, - status: parsed.status, - available: true, - authStatus: parsed.authStatus, - checkedAt, - ...(parsed.message ? { message: parsed.message } : {}), - } satisfies ServerProviderStatus; -}); + ...(parsed.message ? { message: parsed.message } : {}), + } satisfies ServerProviderStatus; + }); // ── Layer ─────────────────────────────────────────────────────────── export const ProviderHealthLive = Layer.effect( ProviderHealth, Effect.gen(function* () { - const codexStatus = yield* checkCodexProviderStatus; + const codexStatus = yield* checkCodexProviderStatus(); return { getStatuses: Effect.succeed([codexStatus]), } satisfies ProviderHealthShape; diff --git a/apps/server/src/runtimeEnvironment.test.ts b/apps/server/src/runtimeEnvironment.test.ts new file mode 100644 index 0000000000..670cda23bb --- /dev/null +++ b/apps/server/src/runtimeEnvironment.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "vitest"; + +import { detectServerRuntimeEnvironment } from "./runtimeEnvironment"; + +describe("detectServerRuntimeEnvironment", () => { + it("detects native windows mode", () => { + expect( + detectServerRuntimeEnvironment({ + platform: "win32", + env: {}, + osRelease: "10.0.26100", + }), + ).toEqual({ + platform: "windows", + pathStyle: "windows", + isWsl: false, + windowsInteropMode: "windows-native", + wslDistroName: null, + }); + }); + + it("detects wsl-hosted mode from environment variables", () => { + expect( + detectServerRuntimeEnvironment({ + platform: "linux", + env: { + WSL_DISTRO_NAME: "Ubuntu-24.04", + WSL_INTEROP: "/run/WSL/123_interop", + }, + osRelease: "6.6.87.2-microsoft-standard-WSL2", + }), + ).toEqual({ + platform: "linux", + pathStyle: "posix", + isWsl: true, + windowsInteropMode: "wsl-hosted", + wslDistroName: "Ubuntu-24.04", + }); + }); + + it("detects wsl-hosted mode from os release when env is unavailable", () => { + expect( + detectServerRuntimeEnvironment({ + platform: "linux", + env: {}, + osRelease: "5.15.167.4-microsoft-standard-WSL2", + }), + ).toEqual({ + platform: "linux", + pathStyle: "posix", + isWsl: true, + windowsInteropMode: "wsl-hosted", + wslDistroName: null, + }); + }); + + it("leaves non-wsl posix hosts outside the windows interop modes", () => { + expect( + detectServerRuntimeEnvironment({ + platform: "darwin", + env: {}, + osRelease: "24.3.0", + }), + ).toEqual({ + platform: "macos", + pathStyle: "posix", + isWsl: false, + windowsInteropMode: null, + wslDistroName: null, + }); + }); +}); diff --git a/apps/server/src/runtimeEnvironment.ts b/apps/server/src/runtimeEnvironment.ts new file mode 100644 index 0000000000..5119f6f2a8 --- /dev/null +++ b/apps/server/src/runtimeEnvironment.ts @@ -0,0 +1,54 @@ +import os from "node:os"; + +import type { ServerRuntimeEnvironment } from "@t3tools/contracts"; + +interface DetectServerRuntimeEnvironmentOptions { + readonly platform?: NodeJS.Platform; + readonly env?: NodeJS.ProcessEnv; + readonly osRelease?: string; +} + +function normalizePlatform(platform: NodeJS.Platform): ServerRuntimeEnvironment["platform"] { + switch (platform) { + case "win32": + return "windows"; + case "darwin": + return "macos"; + default: + return "linux"; + } +} + +function detectWsl(options: { + readonly platform: NodeJS.Platform; + readonly env: NodeJS.ProcessEnv; + readonly osRelease: string; +}): boolean { + if (options.platform !== "linux") { + return false; + } + + if (options.env.WSL_DISTRO_NAME || options.env.WSL_INTEROP) { + return true; + } + + return options.osRelease.toLowerCase().includes("microsoft"); +} + +export function detectServerRuntimeEnvironment( + options: DetectServerRuntimeEnvironmentOptions = {}, +): ServerRuntimeEnvironment { + const platform = options.platform ?? process.platform; + const env = options.env ?? process.env; + const osRelease = options.osRelease ?? os.release(); + const normalizedPlatform = normalizePlatform(platform); + const isWsl = detectWsl({ platform, env, osRelease }); + + return { + platform: normalizedPlatform, + pathStyle: normalizedPlatform === "windows" ? "windows" : "posix", + isWsl, + windowsInteropMode: normalizedPlatform === "windows" ? "windows-native" : isWsl ? "wsl-hosted" : null, + wslDistroName: isWsl ? env.WSL_DISTRO_NAME?.trim() || null : null, + }; +} diff --git a/apps/server/src/terminal/Layers/Manager.test.ts b/apps/server/src/terminal/Layers/Manager.test.ts index 0d45bfcb29..79c09f1727 100644 --- a/apps/server/src/terminal/Layers/Manager.test.ts +++ b/apps/server/src/terminal/Layers/Manager.test.ts @@ -4,6 +4,7 @@ import path from "node:path"; import { DEFAULT_TERMINAL_ID, + type ServerRuntimeEnvironment, type TerminalEvent, type TerminalOpenInput, } from "@t3tools/contracts"; @@ -16,7 +17,11 @@ import { type PtyProcess, type PtySpawnInput, } from "../Services/PTY"; -import { TerminalManagerRuntime } from "./Manager"; +import { + resolveShellCandidates, + TerminalManagerRuntime, + validateTerminalCwdForRuntime, +} from "./Manager"; import { Effect, Encoding } from "effect"; class FakePtyProcess implements PtyProcess { @@ -172,6 +177,7 @@ describe("TerminalManager", () => { options: { shellResolver?: () => string; subprocessChecker?: (terminalPid: number) => Promise; + runtimeEnvironment?: ServerRuntimeEnvironment; subprocessPollIntervalMs?: number; processKillGraceMs?: number; maxRetainedInactiveSessions?: number; @@ -186,6 +192,7 @@ describe("TerminalManager", () => { ptyAdapter, historyLineLimit, shellResolver: options.shellResolver ?? (() => "/bin/bash"), + ...(options.runtimeEnvironment ? { runtimeEnvironment: options.runtimeEnvironment } : {}), ...(options.subprocessChecker ? { subprocessChecker: options.subprocessChecker } : {}), ...(options.subprocessPollIntervalMs ? { subprocessPollIntervalMs: options.subprocessPollIntervalMs } @@ -669,4 +676,93 @@ describe("TerminalManager", () => { manager.dispose(); }); + + it("resolves Windows shell candidates from runtime environment instead of host assumptions", () => { + const runtimeEnvironment: ServerRuntimeEnvironment = { + platform: "windows", + pathStyle: "windows", + isWsl: false, + windowsInteropMode: "windows-native", + wslDistroName: null, + }; + + const candidates = resolveShellCandidates(() => "/bin/bash -l", runtimeEnvironment); + + expect(candidates.map((candidate) => candidate.shell)).toContain("cmd.exe"); + expect(candidates.map((candidate) => candidate.shell)).toContain("powershell.exe"); + expect(candidates.map((candidate) => candidate.shell)).not.toContain("/bin/sh"); + }); + + it("resolves POSIX shell candidates for wsl-hosted runtime even when the requested shell is Windows-style", () => { + const runtimeEnvironment: ServerRuntimeEnvironment = { + platform: "linux", + pathStyle: "posix", + isWsl: true, + windowsInteropMode: "wsl-hosted", + wslDistroName: "Ubuntu", + }; + + const candidates = resolveShellCandidates(() => "powershell.exe", runtimeEnvironment); + + expect(candidates.map((candidate) => candidate.shell)).toContain("powershell.exe"); + expect(candidates.map((candidate) => candidate.shell)).toContain("/bin/bash"); + expect(candidates.map((candidate) => candidate.shell)).toContain("sh"); + }); + + it("rejects POSIX cwd values for windows-native terminal runtimes", async () => { + const runtimeEnvironment: ServerRuntimeEnvironment = { + platform: "windows", + pathStyle: "windows", + isWsl: false, + windowsInteropMode: "windows-native", + wslDistroName: null, + }; + const { manager, ptyAdapter } = makeManager(5, { runtimeEnvironment }); + + await expect(manager.open(openInput({ cwd: "/home/julius/project" }))).rejects.toThrow( + "Terminal cwd does not match Windows runtime path style", + ); + expect(ptyAdapter.spawnInputs).toHaveLength(0); + + manager.dispose(); + }); + + it("rejects Windows cwd values for wsl-hosted terminal runtimes", async () => { + const runtimeEnvironment: ServerRuntimeEnvironment = { + platform: "linux", + pathStyle: "posix", + isWsl: true, + windowsInteropMode: "wsl-hosted", + wslDistroName: "Ubuntu", + }; + const { manager, ptyAdapter } = makeManager(5, { runtimeEnvironment }); + + await expect(manager.open(openInput({ cwd: "C:\\Users\\julius\\project" }))).rejects.toThrow( + "Terminal cwd does not match POSIX runtime path style", + ); + expect(ptyAdapter.spawnInputs).toHaveLength(0); + + manager.dispose(); + }); + + it("validates cwd compatibility as a pure runtime helper", () => { + expect( + validateTerminalCwdForRuntime("/repo", { + platform: "linux", + pathStyle: "posix", + isWsl: true, + windowsInteropMode: "wsl-hosted", + wslDistroName: "Ubuntu", + }), + ).toBeNull(); + expect( + validateTerminalCwdForRuntime("\\\\wsl.localhost\\Ubuntu\\repo", { + platform: "linux", + pathStyle: "posix", + isWsl: true, + windowsInteropMode: "wsl-hosted", + wslDistroName: "Ubuntu", + }), + ).toContain("POSIX runtime path style"); + }); }); diff --git a/apps/server/src/terminal/Layers/Manager.ts b/apps/server/src/terminal/Layers/Manager.ts index a0e4ad705c..ac91340a6b 100644 --- a/apps/server/src/terminal/Layers/Manager.ts +++ b/apps/server/src/terminal/Layers/Manager.ts @@ -9,12 +9,14 @@ import { TerminalOpenInput, TerminalResizeInput, TerminalWriteInput, + type ServerRuntimeEnvironment, type TerminalEvent, type TerminalSessionSnapshot, } from "@t3tools/contracts"; import { Effect, Encoding, Layer, Path, Schema } from "effect"; import { createLogger } from "../../logger"; +import { detectServerRuntimeEnvironment } from "../../runtimeEnvironment"; import { PtyAdapter, PtyAdapterShape, type PtyExitEvent, type PtyProcess } from "../Services/PTY"; import { runProcess } from "../../processRunner"; import { ServerConfig } from "../../config"; @@ -35,6 +37,8 @@ const DEFAULT_MAX_RETAINED_INACTIVE_SESSIONS = 128; const DEFAULT_OPEN_COLS = 120; const DEFAULT_OPEN_ROWS = 30; const TERMINAL_ENV_BLOCKLIST = new Set(["PORT", "ELECTRON_RENDERER_PORT", "ELECTRON_RUN_AS_NODE"]); +const WINDOWS_ABSOLUTE_PATH_PATTERN = /^[A-Za-z]:[\\/]/; +const WINDOWS_UNC_PATH_PATTERN = /^\\\\/; const decodeTerminalOpenInput = Schema.decodeUnknownSync(TerminalOpenInput); const decodeTerminalWriteInput = Schema.decodeUnknownSync(TerminalWriteInput); @@ -44,19 +48,22 @@ const decodeTerminalCloseInput = Schema.decodeUnknownSync(TerminalCloseInput); type TerminalSubprocessChecker = (terminalPid: number) => Promise; -function defaultShellResolver(): string { - if (process.platform === "win32") { +function defaultShellResolver(runtimeEnvironment: ServerRuntimeEnvironment): string { + if (runtimeEnvironment.platform === "windows") { return process.env.ComSpec ?? "cmd.exe"; } return process.env.SHELL ?? "bash"; } -function normalizeShellCommand(value: string | undefined): string | null { +function normalizeShellCommand( + value: string | undefined, + runtimeEnvironment: ServerRuntimeEnvironment, +): string | null { if (!value) return null; const trimmed = value.trim(); if (trimmed.length === 0) return null; - if (process.platform === "win32") { + if (runtimeEnvironment.platform === "windows") { return trimmed; } @@ -65,10 +72,13 @@ function normalizeShellCommand(value: string | undefined): string | null { return firstToken.replace(/^['"]|['"]$/g, ""); } -function shellCandidateFromCommand(command: string | null): ShellCandidate | null { +function shellCandidateFromCommand( + command: string | null, + runtimeEnvironment: ServerRuntimeEnvironment, +): ShellCandidate | null { if (!command || command.length === 0) return null; const shellName = path.basename(command).toLowerCase(); - if (process.platform !== "win32" && shellName === "zsh") { + if (runtimeEnvironment.platform !== "windows" && shellName === "zsh") { return { shell: command, args: ["-o", "nopromptsp"] }; } return { shell: command }; @@ -92,27 +102,36 @@ function uniqueShellCandidates(candidates: Array): ShellC return ordered; } -function resolveShellCandidates(shellResolver: () => string): ShellCandidate[] { - const requested = shellCandidateFromCommand(normalizeShellCommand(shellResolver())); +export function resolveShellCandidates( + shellResolver: () => string, + runtimeEnvironment: ServerRuntimeEnvironment = detectServerRuntimeEnvironment(), +): ShellCandidate[] { + const requested = shellCandidateFromCommand( + normalizeShellCommand(shellResolver(), runtimeEnvironment), + runtimeEnvironment, + ); - if (process.platform === "win32") { + if (runtimeEnvironment.platform === "windows") { return uniqueShellCandidates([ requested, - shellCandidateFromCommand(process.env.ComSpec ?? null), - shellCandidateFromCommand("powershell.exe"), - shellCandidateFromCommand("cmd.exe"), + shellCandidateFromCommand(process.env.ComSpec ?? null, runtimeEnvironment), + shellCandidateFromCommand("powershell.exe", runtimeEnvironment), + shellCandidateFromCommand("cmd.exe", runtimeEnvironment), ]); } return uniqueShellCandidates([ requested, - shellCandidateFromCommand(normalizeShellCommand(process.env.SHELL)), - shellCandidateFromCommand("/bin/zsh"), - shellCandidateFromCommand("/bin/bash"), - shellCandidateFromCommand("/bin/sh"), - shellCandidateFromCommand("zsh"), - shellCandidateFromCommand("bash"), - shellCandidateFromCommand("sh"), + shellCandidateFromCommand( + normalizeShellCommand(process.env.SHELL, runtimeEnvironment), + runtimeEnvironment, + ), + shellCandidateFromCommand("/bin/zsh", runtimeEnvironment), + shellCandidateFromCommand("/bin/bash", runtimeEnvironment), + shellCandidateFromCommand("/bin/sh", runtimeEnvironment), + shellCandidateFromCommand("zsh", runtimeEnvironment), + shellCandidateFromCommand("bash", runtimeEnvironment), + shellCandidateFromCommand("sh", runtimeEnvironment), ]); } @@ -178,6 +197,13 @@ async function checkWindowsSubprocessActivity(terminalPid: number): Promise { - if (!Number.isInteger(terminalPid) || terminalPid <= 0) { - return false; +function createDefaultSubprocessChecker(runtimeEnvironment: ServerRuntimeEnvironment) { + return async (terminalPid: number): Promise => { + if (!Number.isInteger(terminalPid) || terminalPid <= 0) { + return false; + } + if (runtimeEnvironment.platform === "windows") { + return checkWindowsSubprocessActivity(terminalPid); + } + return checkPosixSubprocessActivity(terminalPid); + }; +} + +function isWindowsPath(value: string): boolean { + return WINDOWS_ABSOLUTE_PATH_PATTERN.test(value) || WINDOWS_UNC_PATH_PATTERN.test(value); +} + +export function validateTerminalCwdForRuntime( + cwd: string, + runtimeEnvironment: ServerRuntimeEnvironment = detectServerRuntimeEnvironment(), +): string | null { + if (runtimeEnvironment.pathStyle === "windows") { + if (cwd.startsWith("/")) { + return `Terminal cwd does not match Windows runtime path style: ${cwd}`; + } + return null; } - if (process.platform === "win32") { - return checkWindowsSubprocessActivity(terminalPid); + + if (isWindowsPath(cwd)) { + return `Terminal cwd does not match POSIX runtime path style: ${cwd}`; } - return checkPosixSubprocessActivity(terminalPid); + return null; } function capHistory(history: string, maxLines: number): string { @@ -316,6 +365,7 @@ interface TerminalManagerOptions { ptyAdapter: PtyAdapterShape; shellResolver?: () => string; subprocessChecker?: TerminalSubprocessChecker; + runtimeEnvironment?: ServerRuntimeEnvironment; subprocessPollIntervalMs?: number; processKillGraceMs?: number; maxRetainedInactiveSessions?: number; @@ -333,6 +383,7 @@ export class TerminalManagerRuntime extends EventEmitter private readonly threadLocks = new Map>(); private readonly persistDebounceMs: number; private readonly subprocessChecker: TerminalSubprocessChecker; + private readonly runtimeEnvironment: ServerRuntimeEnvironment; private readonly subprocessPollIntervalMs: number; private readonly processKillGraceMs: number; private readonly maxRetainedInactiveSessions: number; @@ -343,12 +394,15 @@ export class TerminalManagerRuntime extends EventEmitter constructor(options: TerminalManagerOptions) { super(); + this.runtimeEnvironment = options.runtimeEnvironment ?? detectServerRuntimeEnvironment(); this.logsDir = options.logsDir ?? path.resolve(process.cwd(), ".logs", "terminals"); this.historyLineLimit = options.historyLineLimit ?? DEFAULT_HISTORY_LINE_LIMIT; this.ptyAdapter = options.ptyAdapter; - this.shellResolver = options.shellResolver ?? defaultShellResolver; + this.shellResolver = + options.shellResolver ?? (() => defaultShellResolver(this.runtimeEnvironment)); this.persistDebounceMs = DEFAULT_PERSIST_DEBOUNCE_MS; - this.subprocessChecker = options.subprocessChecker ?? defaultSubprocessChecker; + this.subprocessChecker = + options.subprocessChecker ?? createDefaultSubprocessChecker(this.runtimeEnvironment); this.subprocessPollIntervalMs = options.subprocessPollIntervalMs ?? DEFAULT_SUBPROCESS_POLL_INTERVAL_MS; this.processKillGraceMs = options.processKillGraceMs ?? DEFAULT_PROCESS_KILL_GRACE_MS; @@ -589,7 +643,7 @@ export class TerminalManagerRuntime extends EventEmitter let ptyProcess: PtyProcess | null = null; let startedShell: string | null = null; try { - const shellCandidates = resolveShellCandidates(this.shellResolver); + const shellCandidates = resolveShellCandidates(this.shellResolver, this.runtimeEnvironment); const terminalEnv = createTerminalSpawnEnv(process.env, session.runtimeEnv); let lastSpawnError: unknown = null; @@ -1051,6 +1105,11 @@ export class TerminalManagerRuntime extends EventEmitter } private async assertValidCwd(cwd: string): Promise { + const runtimePathError = validateTerminalCwdForRuntime(cwd, this.runtimeEnvironment); + if (runtimePathError) { + throw new Error(runtimePathError); + } + let stats: fs.Stats; try { stats = await fs.promises.stat(cwd); @@ -1173,10 +1232,11 @@ export const TerminalManagerLive = Layer.effect( const { stateDir } = yield* ServerConfig; const { join } = yield* Path.Path; const logsDir = join(stateDir, "logs", "terminals"); + const runtimeEnvironment = detectServerRuntimeEnvironment(); const ptyAdapter = yield* PtyAdapter; const runtime = yield* Effect.acquireRelease( - Effect.sync(() => new TerminalManagerRuntime({ logsDir, ptyAdapter })), + Effect.sync(() => new TerminalManagerRuntime({ logsDir, ptyAdapter, runtimeEnvironment })), (r) => Effect.sync(() => r.dispose()), ); diff --git a/apps/server/src/wsServer.test.ts b/apps/server/src/wsServer.test.ts index e0dc4a079d..b71f389228 100644 --- a/apps/server/src/wsServer.test.ts +++ b/apps/server/src/wsServer.test.ts @@ -51,6 +51,7 @@ import type { GitCoreShape } from "./git/Services/GitCore.ts"; import { GitCore } from "./git/Services/GitCore.ts"; import { GitCommandError, GitManagerError } from "./git/Errors.ts"; import { MigrationError } from "@effect/sql-sqlite-bun/SqliteMigrator"; +import { detectServerRuntimeEnvironment } from "./runtimeEnvironment"; interface PendingMessages { queue: unknown[]; @@ -85,6 +86,8 @@ const defaultProviderHealthService: ProviderHealthShape = { getStatuses: Effect.succeed(defaultProviderStatuses), }; +const defaultRuntimeEnvironment = detectServerRuntimeEnvironment(); + class MockTerminalManager implements TerminalManagerShape { private readonly sessions = new Map(); private readonly listeners = new Set<(event: TerminalEvent) => void>(); @@ -760,6 +763,7 @@ describe("WebSocket Server", () => { issues: [], providers: defaultProviderStatuses, availableEditors: expect.any(Array), + runtimeEnvironment: defaultRuntimeEnvironment, }); expectAvailableEditors((response.result as { availableEditors: unknown }).availableEditors); }); @@ -786,6 +790,7 @@ describe("WebSocket Server", () => { issues: [], providers: defaultProviderStatuses, availableEditors: expect.any(Array), + runtimeEnvironment: defaultRuntimeEnvironment, }); expectAvailableEditors((response.result as { availableEditors: unknown }).availableEditors); @@ -822,6 +827,7 @@ describe("WebSocket Server", () => { ], providers: defaultProviderStatuses, availableEditors: expect.any(Array), + runtimeEnvironment: defaultRuntimeEnvironment, }); expectAvailableEditors((response.result as { availableEditors: unknown }).availableEditors); expect(fs.readFileSync(keybindingsPath, "utf8")).toBe("{ not-json"); @@ -857,6 +863,7 @@ describe("WebSocket Server", () => { issues: Array<{ kind: string; index?: number; message: string }>; providers: ReadonlyArray; availableEditors: unknown; + runtimeEnvironment: unknown; }; expect(result.cwd).toBe("/my/workspace"); expect(result.keybindingsConfigPath).toBe(keybindingsPath); @@ -877,6 +884,7 @@ describe("WebSocket Server", () => { expect(result.keybindings.some((entry) => entry.command === "terminal.new")).toBe(true); expect(result.providers).toEqual(defaultProviderStatuses); expectAvailableEditors(result.availableEditors); + expect(result.runtimeEnvironment).toEqual(defaultRuntimeEnvironment); }); it("pushes server.configUpdated issues when keybindings file changes", async () => { @@ -977,6 +985,7 @@ describe("WebSocket Server", () => { issues: [], providers: defaultProviderStatuses, availableEditors: expect.any(Array), + runtimeEnvironment: defaultRuntimeEnvironment, }); expectAvailableEditors((response.result as { availableEditors: unknown }).availableEditors); }); @@ -1025,6 +1034,7 @@ describe("WebSocket Server", () => { issues: [], providers: defaultProviderStatuses, availableEditors: expect.any(Array), + runtimeEnvironment: defaultRuntimeEnvironment, }); expectAvailableEditors( (configResponse.result as { availableEditors: unknown }).availableEditors, diff --git a/apps/server/src/wsServer.ts b/apps/server/src/wsServer.ts index 50af3b8b10..9c098ce82c 100644 --- a/apps/server/src/wsServer.ts +++ b/apps/server/src/wsServer.ts @@ -59,6 +59,7 @@ import { Open, resolveAvailableEditors } from "./open"; import { ServerConfig } from "./config"; import { GitCore } from "./git/Services/GitCore.ts"; import { tryHandleProjectFaviconRequest } from "./projectFaviconRoute"; +import { detectServerRuntimeEnvironment } from "./runtimeEnvironment"; import { ATTACHMENTS_ROUTE_PREFIX, normalizeAttachmentRelativePath, @@ -196,6 +197,7 @@ export const createServer = Effect.fn(function* (): Effect.fn.Return< autoBootstrapProjectFromCwd, } = serverConfig; const availableEditors = resolveAvailableEditors(); + const runtimeEnvironment = detectServerRuntimeEnvironment(); const gitManager = yield* GitManager; const terminalManager = yield* TerminalManager; @@ -745,6 +747,7 @@ export const createServer = Effect.fn(function* (): Effect.fn.Return< issues: keybindingsConfig.issues, providers: providerStatuses, availableEditors, + runtimeEnvironment, }; case WS_METHODS.serverUpsertKeybinding: { diff --git a/packages/contracts/src/server.ts b/packages/contracts/src/server.ts index 5607be35c4..182b97fa61 100644 --- a/packages/contracts/src/server.ts +++ b/packages/contracts/src/server.ts @@ -49,6 +49,24 @@ export type ServerProviderStatus = typeof ServerProviderStatus.Type; export const ServerProviderStatuses = Schema.Array(ServerProviderStatus); export type ServerProviderStatuses = typeof ServerProviderStatuses.Type; +export const ServerRuntimePlatform = Schema.Literals(["windows", "linux", "macos"]); +export type ServerRuntimePlatform = typeof ServerRuntimePlatform.Type; + +export const ServerPathStyle = Schema.Literals(["windows", "posix"]); +export type ServerPathStyle = typeof ServerPathStyle.Type; + +export const ServerWindowsInteropMode = Schema.Literals(["windows-native", "wsl-hosted"]); +export type ServerWindowsInteropMode = typeof ServerWindowsInteropMode.Type; + +export const ServerRuntimeEnvironment = Schema.Struct({ + platform: ServerRuntimePlatform, + pathStyle: ServerPathStyle, + isWsl: Schema.Boolean, + windowsInteropMode: Schema.NullOr(ServerWindowsInteropMode), + wslDistroName: Schema.NullOr(TrimmedNonEmptyString), +}); +export type ServerRuntimeEnvironment = typeof ServerRuntimeEnvironment.Type; + export const ServerConfig = Schema.Struct({ cwd: TrimmedNonEmptyString, keybindingsConfigPath: TrimmedNonEmptyString, @@ -56,6 +74,7 @@ export const ServerConfig = Schema.Struct({ issues: ServerConfigIssues, providers: ServerProviderStatuses, availableEditors: Schema.Array(EditorId), + runtimeEnvironment: ServerRuntimeEnvironment, }); export type ServerConfig = typeof ServerConfig.Type; From 07c392fabffe7f5aa05e7e00e90801877e419d2e Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 5 Mar 2026 01:36:14 -0800 Subject: [PATCH 02/10] Use Effect-managed Codex app-server process lifecycle - Replace Node child process/readline handling with Effect streams and scoped handles - Wire CodexAppServerManager to adapter services for runtime-managed execution - Harden write/exit/stderr handling and close process scope on shutdown Co-authored-by: codex --- apps/server/src/codexAppServerManager.ts | 199 ++++++++++++------ apps/server/src/processRunner.ts | 50 ++++- .../src/provider/Layers/CodexAdapter.ts | 6 +- 3 files changed, 181 insertions(+), 74 deletions(-) diff --git a/apps/server/src/codexAppServerManager.ts b/apps/server/src/codexAppServerManager.ts index fac4f2de8b..17db3ff181 100644 --- a/apps/server/src/codexAppServerManager.ts +++ b/apps/server/src/codexAppServerManager.ts @@ -1,7 +1,5 @@ -import { type ChildProcessWithoutNullStreams } from "node:child_process"; import { randomUUID } from "node:crypto"; import { EventEmitter } from "node:events"; -import readline from "node:readline"; import { ApprovalRequestId, @@ -18,7 +16,9 @@ import { type ProviderSessionStartInput, type ProviderTurnStartResult, } from "@t3tools/contracts"; -import { killProcessTree, spawnPipedProcess } from "./processRunner"; +import { Effect, Exit, ServiceMap, Scope, Stream } from "effect"; +import type { ChildProcessSpawner } from "effect/unstable/process"; +import { makeRuntimeCommand, spawnManagedCommand } from "./processRunner"; type PendingRequestKey = string; @@ -41,8 +41,8 @@ interface PendingApprovalRequest { interface CodexSessionContext { session: ProviderSession; - child: ChildProcessWithoutNullStreams; - output: readline.Interface; + child: ChildProcessSpawner.ChildProcessHandle; + scope: Scope.Closeable; pending: Map; pendingApprovals: Map; nextRequestId: number; @@ -158,6 +158,18 @@ export interface CodexAppServerManagerEvents { export class CodexAppServerManager extends EventEmitter { private readonly sessions = new Map(); + private readonly services: ServiceMap.ServiceMap | undefined; + + constructor(services?: ServiceMap.ServiceMap) { + super(); + this.services = services; + } + + private runPromise(effect: Effect.Effect): Promise { + return this.services + ? Effect.runPromiseWith(this.services as ServiceMap.ServiceMap)(effect) + : Effect.runPromise(effect as Effect.Effect); + } async startSession(input: ProviderSessionStartInput): Promise { const sessionId = ProviderSessionId.makeUnsafe(randomUUID()); @@ -179,19 +191,23 @@ export class CodexAppServerManager extends EventEmitter undefined); this.updateSession(context, { status: "closed", @@ -515,47 +526,76 @@ export class CodexAppServerManager extends EventEmitter { - this.handleStdoutLine(context, line); - }); - - context.child.stderr.on("data", (chunk: Buffer) => { - const raw = chunk.toString(); - const lines = raw.split(/\r?\n/g); - for (const rawLine of lines) { - const classified = classifyCodexStderrLine(rawLine); - if (!classified) { - continue; - } - - this.emitErrorEvent(context, "process/stderr", classified.message); - } - }); - - context.child.on("error", (error) => { - const message = error.message || "codex app-server process errored."; - this.updateSession(context, { - status: "error", - lastError: message, - }); - this.emitErrorEvent(context, "process/error", message); - }); + private async attachProcessListeners(context: CodexSessionContext): Promise { + await this.runPromise( + Effect.gen( + function* (this: CodexAppServerManager) { + yield* Effect.forkScoped( + Stream.runForEach( + Stream.splitLines(Stream.decodeText(context.child.stdout)), + (line) => + Effect.sync(() => { + this.handleStdoutLine(context, line); + }), + ), + ); - context.child.on("exit", (code, signal) => { - if (context.stopping) { - return; - } + yield* Effect.forkScoped( + Stream.runForEach(Stream.decodeText(context.child.stderr), (raw) => + Effect.sync(() => { + const lines = raw.split(/\r?\n/g); + for (const rawLine of lines) { + const classified = classifyCodexStderrLine(rawLine); + if (!classified) { + continue; + } + + this.emitErrorEvent(context, "process/stderr", classified.message); + } + }), + ), + ); - const message = `codex app-server exited (code=${code ?? "null"}, signal=${signal ?? "null"}).`; - this.updateSession(context, { - status: "closed", - activeTurnId: undefined, - lastError: code === 0 ? context.session.lastError : message, - }); - this.emitLifecycleEvent(context, "session/exited", message); - this.sessions.delete(context.session.sessionId); - }); + yield* Effect.forkScoped( + Effect.matchCauseEffect(context.child.exitCode, { + onFailure: (cause) => + Effect.sync(() => { + if (context.stopping) { + return; + } + + const message = + cause instanceof Error + ? cause.message + : "codex app-server process errored."; + this.updateSession(context, { + status: "error", + activeTurnId: undefined, + lastError: message, + }); + this.emitErrorEvent(context, "process/error", message); + this.sessions.delete(context.session.sessionId); + }), + onSuccess: (code) => + Effect.sync(() => { + if (context.stopping) { + return; + } + + const message = `codex app-server exited (code=${code}).`; + this.updateSession(context, { + status: "closed", + activeTurnId: undefined, + lastError: code === 0 ? context.session.lastError : message, + }); + this.emitLifecycleEvent(context, "session/exited", message); + this.sessions.delete(context.session.sessionId); + }), + }), + ); + }.bind(this), + ).pipe(Scope.provide(context.scope)), + ); } private handleStdoutLine(context: CodexSessionContext, line: string): void { @@ -711,19 +751,31 @@ export class CodexAppServerManager extends EventEmitter { + this.emitErrorEvent( + context, + "protocol/writeError", + error instanceof Error ? error.message : String(error), + ); }); return; } - this.writeMessage(context, { + void this.writeMessage(context, { id: request.id, error: { code: -32601, message: `Unsupported server request: ${request.method}`, }, + }).catch((error) => { + this.emitErrorEvent( + context, + "protocol/writeError", + error instanceof Error ? error.message : String(error), + ); }); } @@ -766,23 +818,30 @@ export class CodexAppServerManager extends EventEmitter { + clearTimeout(timeout); + context.pending.delete(String(id)); + reject(error instanceof Error ? error : new Error(String(error))); }); }); return result as TResponse; } - private writeMessage(context: CodexSessionContext, message: unknown): void { + private async writeMessage(context: CodexSessionContext, message: unknown): Promise { const encoded = JSON.stringify(message); - if (!context.child.stdin.writable) { + if (context.stopping) { throw new Error("Cannot write to codex app-server stdin."); } - - context.child.stdin.write(`${encoded}\n`); + await this.runPromise( + Stream.run(context.child.stdin)(Stream.make(new TextEncoder().encode(`${encoded}\n`))).pipe( + Scope.provide(context.scope), + ), + ); } private emitLifecycleEvent(context: CodexSessionContext, method: string, message: string): void { diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts index b522e8c6e6..63eac85805 100644 --- a/apps/server/src/processRunner.ts +++ b/apps/server/src/processRunner.ts @@ -7,6 +7,8 @@ import { } from "node:child_process"; import type { ServerRuntimeEnvironment } from "@t3tools/contracts"; +import { Effect, Exit, Scope } from "effect"; +import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; import { detectServerRuntimeEnvironment } from "./runtimeEnvironment"; @@ -17,11 +19,20 @@ interface ProcessSpawnBaseOptions { shell?: boolean | undefined; } +interface RuntimeShellOptions { + runtimeEnvironment?: ServerRuntimeEnvironment | undefined; + shell?: boolean | string | undefined; +} + export interface ProcessSpawnOptions extends ProcessSpawnBaseOptions { stdio?: StdioOptions | undefined; detached?: boolean | undefined; } +export interface RuntimeCommandOptions extends ChildProcess.CommandOptions { + runtimeEnvironment?: ServerRuntimeEnvironment | undefined; +} + export interface ProcessSpawnSyncOptions extends ProcessSpawnBaseOptions { stdio?: StdioOptions | undefined; detached?: boolean | undefined; @@ -61,7 +72,7 @@ function resolveRuntimeEnvironment( return runtimeEnvironment ?? detectServerRuntimeEnvironment(); } -function shouldUseShell(options: ProcessSpawnBaseOptions): boolean { +function shouldUseShell(options: RuntimeShellOptions): boolean | string { if (options.shell !== undefined) { return options.shell; } @@ -79,6 +90,43 @@ function toSpawnOptions(options: ProcessSpawnOptions) { }; } +export function toRuntimeCommandOptions( + options: RuntimeCommandOptions = {}, +): ChildProcess.CommandOptions { + return { + ...options, + shell: options.shell ?? shouldUseShell(options), + }; +} + +export function makeRuntimeCommand( + command: string, + args: ReadonlyArray, + options: RuntimeCommandOptions = {}, +): ChildProcess.StandardCommand { + return ChildProcess.make(command, [...args], toRuntimeCommandOptions(options)); +} + +export interface ManagedChildProcess { + readonly scope: Scope.Closeable; + readonly handle: ChildProcessSpawner.ChildProcessHandle; +} + +export const spawnManagedCommand = (command: ChildProcess.Command) => + Effect.gen(function* () { + const scope = yield* Scope.make("sequential"); + const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const handle = yield* spawner.spawn(command).pipe( + Scope.provide(scope), + Effect.tapError(() => Scope.close(scope, Exit.void)), + ); + + return { + scope, + handle, + } satisfies ManagedChildProcess; + }); + export function spawnProcess( command: string, args: readonly string[], diff --git a/apps/server/src/provider/Layers/CodexAdapter.ts b/apps/server/src/provider/Layers/CodexAdapter.ts index 8bec738893..23f46fa515 100644 --- a/apps/server/src/provider/Layers/CodexAdapter.ts +++ b/apps/server/src/provider/Layers/CodexAdapter.ts @@ -433,6 +433,7 @@ const makeCodexAdapter = (options?: CodexAdapterLiveOptions) => const fileSystem = yield* FileSystem.FileSystem; const serverConfig = yield* Effect.service(ServerConfig); const directory = yield* ProviderSessionDirectory; + const services = yield* Effect.services(); const nativeEventLogger = options?.nativeEventLogger ?? (options?.nativeEventLogPath !== undefined @@ -449,7 +450,7 @@ const makeCodexAdapter = (options?: CodexAdapterLiveOptions) => if (options?.makeManager) { return options.makeManager(); } - return new CodexAppServerManager(); + return new CodexAppServerManager(services); }), (manager) => Effect.sync(() => { @@ -587,7 +588,7 @@ const makeCodexAdapter = (options?: CodexAdapterLiveOptions) => const runtimeEventQueue = yield* Queue.unbounded(); yield* Effect.acquireRelease( - Effect.gen(function* () { + Effect.sync(() => { const writeNativeEvent = (event: ProviderEvent) => Effect.gen(function* () { if (!nativeEventLogger) { @@ -604,7 +605,6 @@ const makeCodexAdapter = (options?: CodexAdapterLiveOptions) => yield* nativeEventLogger.write(event, orchestrationThreadId); }); - const services = yield* Effect.services(); const listener = (event: ProviderEvent) => Effect.gen(function* () { yield* writeNativeEvent(event); From 4c833458777ee730c46b757b531df5677c84a313 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 5 Mar 2026 11:17:54 -0800 Subject: [PATCH 03/10] Wire NodeServices into Codex app server manager runtime - pass `NodeServices` service map through `CodexAdapter` and provider layer - make `CodexAppServerManager` run effects with injected services - update manager tests to initialize/dispose a NodeServices runtime per test --- apps/server/src/codexAppServerManager.test.ts | 33 ++++++++++++++--- apps/server/src/codexAppServerManager.ts | 36 +++++++++---------- .../src/provider/Layers/CodexAdapter.ts | 13 ++++--- apps/server/src/serverLayers.ts | 2 +- 4 files changed, 56 insertions(+), 28 deletions(-) diff --git a/apps/server/src/codexAppServerManager.test.ts b/apps/server/src/codexAppServerManager.test.ts index 7ec669d4c6..a476d4e49c 100644 --- a/apps/server/src/codexAppServerManager.test.ts +++ b/apps/server/src/codexAppServerManager.test.ts @@ -1,4 +1,6 @@ -import { describe, expect, it, vi } from "vitest"; +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { ManagedRuntime, ServiceMap } from "effect"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ProviderSessionId } from "@t3tools/contracts"; import { @@ -10,8 +12,31 @@ import { const asSessionId = (value: string): ProviderSessionId => ProviderSessionId.makeUnsafe(value); +let runtime: ManagedRuntime.ManagedRuntime | null = null; +let services: ServiceMap.ServiceMap | null = null; + +beforeEach(async () => { + runtime = ManagedRuntime.make(NodeServices.layer); + services = await runtime.services(); +}); + +afterEach(async () => { + services = null; + if (runtime) { + await runtime.dispose(); + } + runtime = null; +}); + +function createManager() { + if (!services) { + throw new Error("Test runtime services not initialized."); + } + return new CodexAppServerManager(services); +} + function createSendTurnHarness() { - const manager = new CodexAppServerManager(); + const manager = createManager(); const context = { session: { sessionId: "sess_1", @@ -47,7 +72,7 @@ function createSendTurnHarness() { } function createThreadControlHarness() { - const manager = new CodexAppServerManager(); + const manager = createManager(); const context = { session: { sessionId: "sess_1", @@ -148,7 +173,7 @@ describe("isRecoverableThreadResumeError", () => { describe("startSession", () => { it("emits session/startFailed when resolving cwd throws before process launch", async () => { - const manager = new CodexAppServerManager(); + const manager = createManager(); const events: Array<{ method: string; kind: string; message?: string }> = []; manager.on("event", (event) => { events.push({ diff --git a/apps/server/src/codexAppServerManager.ts b/apps/server/src/codexAppServerManager.ts index 17db3ff181..b57b44b7a5 100644 --- a/apps/server/src/codexAppServerManager.ts +++ b/apps/server/src/codexAppServerManager.ts @@ -16,6 +16,7 @@ import { type ProviderSessionStartInput, type ProviderTurnStartResult, } from "@t3tools/contracts"; +import type * as NodeServices from "@effect/platform-node/NodeServices"; import { Effect, Exit, ServiceMap, Scope, Stream } from "effect"; import type { ChildProcessSpawner } from "effect/unstable/process"; import { makeRuntimeCommand, spawnManagedCommand } from "./processRunner"; @@ -158,17 +159,20 @@ export interface CodexAppServerManagerEvents { export class CodexAppServerManager extends EventEmitter { private readonly sessions = new Map(); - private readonly services: ServiceMap.ServiceMap | undefined; + private readonly runPromise: ( + effect: Effect.Effect, + options?: Effect.RunOptions | undefined, + ) => Promise; - constructor(services?: ServiceMap.ServiceMap) { + constructor(services?: ServiceMap.ServiceMap) { super(); - this.services = services; - } - - private runPromise(effect: Effect.Effect): Promise { - return this.services - ? Effect.runPromiseWith(this.services as ServiceMap.ServiceMap)(effect) - : Effect.runPromise(effect as Effect.Effect); + this.runPromise = services + ? Effect.runPromiseWith(services) + : ((effect, options) => + Effect.runPromise( + effect as unknown as Effect.Effect, + options, + )) as typeof this.runPromise; } async startSession(input: ProviderSessionStartInput): Promise { @@ -531,12 +535,10 @@ export class CodexAppServerManager extends EventEmitter - Effect.sync(() => { - this.handleStdoutLine(context, line); - }), + Stream.runForEach(Stream.splitLines(Stream.decodeText(context.child.stdout)), (line) => + Effect.sync(() => { + this.handleStdoutLine(context, line); + }), ), ); @@ -565,9 +567,7 @@ export class CodexAppServerManager extends EventEmitter const fileSystem = yield* FileSystem.FileSystem; const serverConfig = yield* Effect.service(ServerConfig); const directory = yield* ProviderSessionDirectory; - const services = yield* Effect.services(); + const services = yield* Effect.services(); const nativeEventLogger = options?.nativeEventLogger ?? (options?.nativeEventLogPath !== undefined @@ -595,11 +596,13 @@ const makeCodexAdapter = (options?: CodexAdapterLiveOptions) => return; } const orchestrationThreadId = yield* Effect.catch( - directory.getThreadId(event.sessionId).pipe( - Effect.map((threadIdOption) => - Option.isSome(threadIdOption) ? threadIdOption.value : null, + directory + .getThreadId(event.sessionId) + .pipe( + Effect.map((threadIdOption) => + Option.isSome(threadIdOption) ? threadIdOption.value : null, + ), ), - ), () => Effect.succeed(null), ); yield* nativeEventLogger.write(event, orchestrationThreadId); diff --git a/apps/server/src/serverLayers.ts b/apps/server/src/serverLayers.ts index cc4f139577..87cab2db56 100644 --- a/apps/server/src/serverLayers.ts +++ b/apps/server/src/serverLayers.ts @@ -38,7 +38,7 @@ import { NodePtyAdapterLive } from "./terminal/Layers/NodePTY"; export function makeServerProviderLayer(): Layer.Layer< ProviderService, ProviderUnsupportedError, - SqlClient.SqlClient | ServerConfig | FileSystem.FileSystem + SqlClient.SqlClient | ServerConfig | FileSystem.FileSystem | NodeServices.NodeServices > { return Effect.gen(function* () { const { stateDir } = yield* ServerConfig; From f185b72a6629d6b55bdfcbeadf803575bbaafefa Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 5 Mar 2026 11:44:07 -0800 Subject: [PATCH 04/10] Handle null-exit and timeout failures in Codex and Git flows - squash Effect causes to surface real Codex process error messages - normalize Git process results to error on signal/null exit and keep details - treat timed-out Codex health probes/auth checks as unavailable or warning with clear messages - add tests for cause extraction, Git signal termination, and provider timeout paths --- apps/server/src/codexAppServerManager.test.ts | 8 ++ apps/server/src/codexAppServerManager.ts | 10 ++- apps/server/src/git/Layers/GitService.test.ts | 29 ++++++- apps/server/src/git/Layers/GitService.ts | 82 ++++++++++++------- apps/server/src/processRunner.ts | 2 +- .../provider/Layers/ProviderHealth.test.ts | 33 +++++++- .../src/provider/Layers/ProviderHealth.ts | 14 +++- 7 files changed, 141 insertions(+), 37 deletions(-) diff --git a/apps/server/src/codexAppServerManager.test.ts b/apps/server/src/codexAppServerManager.test.ts index a476d4e49c..e88ded4a13 100644 --- a/apps/server/src/codexAppServerManager.test.ts +++ b/apps/server/src/codexAppServerManager.test.ts @@ -1,4 +1,5 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; +import { Cause } from "effect"; import { ManagedRuntime, ServiceMap } from "effect"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ProviderSessionId } from "@t3tools/contracts"; @@ -7,6 +8,7 @@ import { CodexAppServerManager, classifyCodexStderrLine, isRecoverableThreadResumeError, + messageFromCodexProcessCause, normalizeCodexModelSlug, } from "./codexAppServerManager"; @@ -171,6 +173,12 @@ describe("isRecoverableThreadResumeError", () => { }); }); +describe("messageFromCodexProcessCause", () => { + it("extracts the underlying error message from an effect cause", () => { + expect(messageFromCodexProcessCause(Cause.fail(new Error("boom")))).toBe("boom"); + }); +}); + describe("startSession", () => { it("emits session/startFailed when resolving cwd throws before process launch", async () => { const manager = createManager(); diff --git a/apps/server/src/codexAppServerManager.ts b/apps/server/src/codexAppServerManager.ts index b57b44b7a5..5a1ed26dee 100644 --- a/apps/server/src/codexAppServerManager.ts +++ b/apps/server/src/codexAppServerManager.ts @@ -17,7 +17,7 @@ import { type ProviderTurnStartResult, } from "@t3tools/contracts"; import type * as NodeServices from "@effect/platform-node/NodeServices"; -import { Effect, Exit, ServiceMap, Scope, Stream } from "effect"; +import { Cause, Effect, Exit, ServiceMap, Scope, Stream } from "effect"; import type { ChildProcessSpawner } from "effect/unstable/process"; import { makeRuntimeCommand, spawnManagedCommand } from "./processRunner"; @@ -153,6 +153,11 @@ export function isRecoverableThreadResumeError(error: unknown): boolean { return RECOVERABLE_THREAD_RESUME_ERROR_SNIPPETS.some((snippet) => message.includes(snippet)); } +export function messageFromCodexProcessCause(cause: Cause.Cause): string { + const squashed = Cause.squash(cause); + return squashed instanceof Error ? squashed.message : "codex app-server process errored."; +} + export interface CodexAppServerManagerEvents { event: [event: ProviderEvent]; } @@ -566,8 +571,7 @@ export class CodexAppServerManager extends EventEmitter { } }), ); + + it("normalizeGitProcessResult fails when git exits without a code", () => { + try { + void normalizeGitProcessResult( + { + operation: "GitProcess.test.signal", + cwd: process.cwd(), + args: ["status"], + }, + {}, + { + code: null, + signal: "SIGTERM", + stdout: "", + stderr: "", + timedOut: false, + }, + ); + assert.fail("Expected normalizeGitProcessResult to throw."); + } catch (error) { + assert.equal(Schema.is(GitCommandError)(error), true); + if (!Schema.is(GitCommandError)(error)) { + return; + } + assert.equal(error.detail, "git status terminated by signal SIGTERM."); + } + }); }); diff --git a/apps/server/src/git/Layers/GitService.ts b/apps/server/src/git/Layers/GitService.ts index 8cf00c969f..8df2bee3c9 100644 --- a/apps/server/src/git/Layers/GitService.ts +++ b/apps/server/src/git/Layers/GitService.ts @@ -39,6 +39,59 @@ function toGitCommandError( }); } +export function normalizeGitProcessResult( + commandInput: Pick, + input: Pick, + result: { + readonly stdout: string; + readonly stderr: string; + readonly code: number | null; + readonly signal: NodeJS.Signals | null; + readonly timedOut: boolean; + }, +): ExecuteGitResult { + if (result.timedOut) { + throw new GitCommandError({ + operation: commandInput.operation, + command: quoteGitCommand(commandInput.args), + cwd: commandInput.cwd, + detail: `${quoteGitCommand(commandInput.args)} timed out.`, + }); + } + + if (result.code === null) { + throw new GitCommandError({ + operation: commandInput.operation, + command: quoteGitCommand(commandInput.args), + cwd: commandInput.cwd, + detail: + result.signal !== null + ? `${quoteGitCommand(commandInput.args)} terminated by signal ${result.signal}.` + : `${quoteGitCommand(commandInput.args)} terminated before reporting an exit code.`, + }); + } + + const exitCode = result.code; + if (!input.allowNonZeroExit && exitCode !== 0) { + const trimmedStderr = result.stderr.trim(); + throw new GitCommandError({ + operation: commandInput.operation, + command: quoteGitCommand(commandInput.args), + cwd: commandInput.cwd, + detail: + trimmedStderr.length > 0 + ? `${quoteGitCommand(commandInput.args)} failed: ${trimmedStderr}` + : `${quoteGitCommand(commandInput.args)} failed with code ${exitCode}.`, + }); + } + + return { + code: exitCode, + stdout: result.stdout, + stderr: result.stderr, + } satisfies ExecuteGitResult; +} + const makeGitService = Effect.sync(() => { const execute: GitServiceShape["execute"] = Effect.fnUntraced(function* (input) { const commandInput = { @@ -59,34 +112,7 @@ const makeGitService = Effect.sync(() => { outputMode: "error", }); - if (result.timedOut) { - throw new GitCommandError({ - operation: commandInput.operation, - command: quoteGitCommand(commandInput.args), - cwd: commandInput.cwd, - detail: `${quoteGitCommand(commandInput.args)} timed out.`, - }); - } - - const exitCode = result.code ?? 0; - if (!input.allowNonZeroExit && exitCode !== 0) { - const trimmedStderr = result.stderr.trim(); - throw new GitCommandError({ - operation: commandInput.operation, - command: quoteGitCommand(commandInput.args), - cwd: commandInput.cwd, - detail: - trimmedStderr.length > 0 - ? `${quoteGitCommand(commandInput.args)} failed: ${trimmedStderr}` - : `${quoteGitCommand(commandInput.args)} failed with code ${exitCode}.`, - }); - } - - return { - code: exitCode, - stdout: result.stdout, - stderr: result.stderr, - } satisfies ExecuteGitResult; + return normalizeGitProcessResult(commandInput, input, result); }, catch: toGitCommandError(commandInput, "failed to run."), }); diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts index 63eac85805..6bd3162faf 100644 --- a/apps/server/src/processRunner.ts +++ b/apps/server/src/processRunner.ts @@ -127,7 +127,7 @@ export const spawnManagedCommand = (command: ChildProcess.Command) => } satisfies ManagedChildProcess; }); -export function spawnProcess( +function spawnProcess( command: string, args: readonly string[], options: ProcessSpawnOptions = {}, diff --git a/apps/server/src/provider/Layers/ProviderHealth.test.ts b/apps/server/src/provider/Layers/ProviderHealth.test.ts index e663b07055..7ad8eea5f7 100644 --- a/apps/server/src/provider/Layers/ProviderHealth.test.ts +++ b/apps/server/src/provider/Layers/ProviderHealth.test.ts @@ -7,7 +7,12 @@ import { checkCodexProviderStatus, parseAuthStatusFromOutput } from "./ProviderH // ── Test helpers ──────────────────────────────────────────────────── function mockRunner( - handler: (args: ReadonlyArray) => { stdout: string; stderr: string; code: number }, + handler: (args: ReadonlyArray) => { + stdout: string; + stderr: string; + code: number | null; + timedOut?: boolean; + }, ) { return (args: ReadonlyArray) => Promise.resolve(handler(args)); } @@ -116,6 +121,32 @@ it.effect("returns warning when login status command is unsupported", () => }), ); +it.effect("returns unavailable when codex version probe times out", () => + Effect.gen(function* () { + const status = yield* checkCodexProviderStatus( + mockRunner((args) => { + const joined = args.join(" "); + if (joined === "--version") { + return { stdout: "", stderr: "", code: null, timedOut: true }; + } + throw new Error(`Unexpected args: ${joined}`); + }), + ); + assert.strictEqual(status.provider, "codex"); + assert.strictEqual(status.status, "error"); + assert.strictEqual(status.available, false); + assert.strictEqual(status.authStatus, "unknown"); + assert.strictEqual(status.message, "Codex CLI is installed but failed to run. Timed out while running command."); + }), +); + +it("parseAuthStatusFromOutput: timed out auth check is warning", () => { + const parsed = parseAuthStatusFromOutput({ stdout: "", stderr: "", code: null, timedOut: true }); + assert.strictEqual(parsed.status, "warning"); + assert.strictEqual(parsed.authStatus, "unknown"); + assert.strictEqual(parsed.message, "Timed out while checking Codex authentication status."); +}); + // ── Pure function tests ───────────────────────────────────────────── it("parseAuthStatusFromOutput: exit code 0 with no auth markers is ready", () => { diff --git a/apps/server/src/provider/Layers/ProviderHealth.ts b/apps/server/src/provider/Layers/ProviderHealth.ts index 75e1ab40ae..183d63661e 100644 --- a/apps/server/src/provider/Layers/ProviderHealth.ts +++ b/apps/server/src/provider/Layers/ProviderHealth.ts @@ -31,7 +31,7 @@ class ProviderHealthCommandError extends Data.TaggedError("ProviderHealthCommand export interface CommandResult { readonly stdout: string; readonly stderr: string; - readonly code: number; + readonly code: number | null; readonly timedOut?: boolean; } @@ -109,6 +109,14 @@ export function parseAuthStatusFromOutput(result: CommandResult): { readonly authStatus: ServerProviderAuthStatus; readonly message?: string; } { + if (result.timedOut) { + return { + status: "warning", + authStatus: "unknown", + message: "Timed out while checking Codex authentication status.", + }; + } + const lowerOutput = `${result.stdout}\n${result.stderr}`.toLowerCase(); if ( @@ -199,7 +207,7 @@ const defaultRunCodexCommand: RunProviderHealthCommand = async (args) => { return { stdout: result.stdout, stderr: result.stderr, - code: result.code ?? 0, + code: result.code, timedOut: result.timedOut, } satisfies CommandResult; }; @@ -237,7 +245,7 @@ export const checkCodexProviderStatus = ( } const version = versionProbe.success; - if (version.code !== 0) { + if (version.timedOut || version.code !== 0) { const detail = detailFromResult(version); return { provider: CODEX_PROVIDER, From c11666f8bc908324e87a9b9ebc5d9468b45f1ace Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 5 Mar 2026 12:39:09 -0800 Subject: [PATCH 05/10] Fix Windows process launching for native and batch commands - add a shared launch-plan resolver to avoid default shell mode on Windows - resolve executables via PATH/PATHEXT and route `.cmd`/`.bat` through `cmd.exe /d /s /c` - update runtime/spawn helpers and add focused tests for Windows + WSL behavior --- apps/server/src/processRunner.test.ts | 136 ++++++++++++- apps/server/src/processRunner.ts | 273 ++++++++++++++++++++++++-- 2 files changed, 395 insertions(+), 14 deletions(-) diff --git a/apps/server/src/processRunner.test.ts b/apps/server/src/processRunner.test.ts index 0df630a8e3..2c9a9af46a 100644 --- a/apps/server/src/processRunner.test.ts +++ b/apps/server/src/processRunner.test.ts @@ -1,6 +1,25 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + import { describe, expect, it } from "vitest"; -import { runProcess, spawnDetachedProcess, spawnProcessSync } from "./processRunner"; +import { + makeRuntimeCommand, + resolveProcessLaunchPlan, + runProcess, + spawnDetachedProcess, + spawnProcessSync, +} from "./processRunner"; + +function withTempDir(run: (dir: string) => void): void { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-process-runner-")); + try { + run(dir); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +} describe("runProcess", () => { it("fails when output exceeds max buffer in default mode", async () => { @@ -37,3 +56,118 @@ describe("runProcess", () => { ).resolves.toBeUndefined(); }); }); + +describe("resolveProcessLaunchPlan", () => { + it("resolves native windows executables without using a shell", () => { + withTempDir((dir) => { + fs.writeFileSync(path.join(dir, "git.EXE"), "MZ"); + const plan = resolveProcessLaunchPlan("git", ["status"], { + env: { + PATH: dir, + PATHEXT: ".COM;.EXE;.BAT;.CMD", + }, + inheritParentEnv: false, + runtimeEnvironment: { + platform: "windows", + pathStyle: "windows", + isWsl: false, + windowsInteropMode: "windows-native", + wslDistroName: null, + }, + }); + + expect(plan.command).toBe(path.join(dir, "git.EXE")); + expect(plan.args).toEqual(["status"]); + expect(plan.shell).toBe(false); + }); + }); + + it("wraps windows batch launchers through cmd.exe without default shell mode", () => { + withTempDir((dir) => { + const wrapperPath = path.join(dir, "code.CMD"); + fs.writeFileSync(wrapperPath, "@echo off\r\n"); + const plan = resolveProcessLaunchPlan("code", ["C:\\repo\\a&b.ts"], { + env: { + PATH: dir, + PATHEXT: ".COM;.EXE;.BAT;.CMD", + COMSPEC: "C:\\Windows\\System32\\cmd.exe", + }, + inheritParentEnv: false, + runtimeEnvironment: { + platform: "windows", + pathStyle: "windows", + isWsl: false, + windowsInteropMode: "windows-native", + wslDistroName: null, + }, + }); + + expect(plan.command).toBe("C:\\Windows\\System32\\cmd.exe"); + expect(plan.args.slice(0, 3)).toEqual(["/d", "/s", "/c"]); + expect(plan.args[3]).toBe(`"${wrapperPath}" "C:\\repo\\a^&b.ts"`); + expect(plan.shell).toBe(false); + }); + }); + + it("keeps wsl-hosted commands on the linux direct exec path", () => { + const plan = resolveProcessLaunchPlan("code", ["/home/julius/repo"], { + runtimeEnvironment: { + platform: "linux", + pathStyle: "posix", + isWsl: true, + windowsInteropMode: "wsl-hosted", + wslDistroName: "Ubuntu", + }, + }); + + expect(plan.command).toBe("code"); + expect(plan.args).toEqual(["/home/julius/repo"]); + expect(plan.shell).toBe(false); + }); + + it("preserves explicit shell configuration", () => { + const plan = resolveProcessLaunchPlan("git", ["status"], { + shell: true, + runtimeEnvironment: { + platform: "windows", + pathStyle: "windows", + isWsl: false, + windowsInteropMode: "windows-native", + wslDistroName: null, + }, + }); + + expect(plan.command).toBe("git"); + expect(plan.args).toEqual(["status"]); + expect(plan.shell).toBe(true); + }); +}); + +describe("makeRuntimeCommand", () => { + it("uses the shared launch plan for batch commands on windows", () => { + withTempDir((dir) => { + const wrapperPath = path.join(dir, "code.CMD"); + fs.writeFileSync(wrapperPath, "@echo off\r\n"); + + const command = makeRuntimeCommand("code", ["C:\\repo\\a&b.ts"], { + env: { + PATH: dir, + PATHEXT: ".COM;.EXE;.BAT;.CMD", + COMSPEC: "C:\\Windows\\System32\\cmd.exe", + }, + extendEnv: false, + runtimeEnvironment: { + platform: "windows", + pathStyle: "windows", + isWsl: false, + windowsInteropMode: "windows-native", + wslDistroName: null, + }, + }); + + expect(command.command).toBe("C:\\Windows\\System32\\cmd.exe"); + expect(command.args).toEqual(["/d", "/s", "/c", `"${wrapperPath}" "C:\\repo\\a^&b.ts"`]); + expect(command.options.shell).toBe(false); + }); + }); +}); diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts index 6bd3162faf..29cd1d624a 100644 --- a/apps/server/src/processRunner.ts +++ b/apps/server/src/processRunner.ts @@ -5,6 +5,8 @@ import { spawnSync, type StdioOptions, } from "node:child_process"; +import { statSync } from "node:fs"; +import { extname, join } from "node:path"; import type { ServerRuntimeEnvironment } from "@t3tools/contracts"; import { Effect, Exit, Scope } from "effect"; @@ -24,6 +26,11 @@ interface RuntimeShellOptions { shell?: boolean | string | undefined; } +interface ProcessLaunchPlanOptions extends RuntimeShellOptions { + env?: NodeJS.ProcessEnv | Record | undefined; + inheritParentEnv?: boolean | undefined; +} + export interface ProcessSpawnOptions extends ProcessSpawnBaseOptions { stdio?: StdioOptions | undefined; detached?: boolean | undefined; @@ -62,6 +69,20 @@ export interface ProcessRunResult { stderrTruncated?: boolean | undefined; } +export interface ProcessLaunchPlan { + readonly command: string; + readonly args: ReadonlyArray; + readonly shell: boolean | string; + readonly runtimeEnvironment: ServerRuntimeEnvironment; +} + +interface ResolvedWindowsCommand { + readonly path: string; + readonly kind: "native" | "batch"; +} + +const WINDOWS_BATCH_EXECUTABLE_EXTENSIONS = new Set([".CMD", ".BAT"]); + function commandLabel(command: string, args: readonly string[]): string { return [command, ...args].join(" "); } @@ -72,19 +93,214 @@ function resolveRuntimeEnvironment( return runtimeEnvironment ?? detectServerRuntimeEnvironment(); } -function shouldUseShell(options: RuntimeShellOptions): boolean | string { +function resolvePathEnvironmentVariable(env: NodeJS.ProcessEnv): string { + return env.PATH ?? env.Path ?? env.path ?? ""; +} + +function resolveWindowsPathExtensions(env: NodeJS.ProcessEnv): ReadonlyArray { + const rawValue = env.PATHEXT; + const fallback = [".COM", ".EXE", ".BAT", ".CMD"]; + if (!rawValue) return fallback; + + const parsed = rawValue + .split(";") + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0) + .map((entry) => (entry.startsWith(".") ? entry.toUpperCase() : `.${entry.toUpperCase()}`)); + return parsed.length > 0 ? Array.from(new Set(parsed)) : fallback; +} + +function resolveCommandCandidates( + command: string, + windowsPathExtensions: ReadonlyArray, +): ReadonlyArray { + const extension = extname(command); + const normalizedExtension = extension.toUpperCase(); + + if (extension.length > 0 && windowsPathExtensions.includes(normalizedExtension)) { + const commandWithoutExtension = command.slice(0, -extension.length); + return Array.from( + new Set([ + command, + `${commandWithoutExtension}${normalizedExtension}`, + `${commandWithoutExtension}${normalizedExtension.toLowerCase()}`, + ]), + ); + } + + const candidates: string[] = [command]; + for (const candidateExtension of windowsPathExtensions) { + candidates.push(`${command}${candidateExtension}`); + candidates.push(`${command}${candidateExtension.toLowerCase()}`); + } + return Array.from(new Set(candidates)); +} + +function stripWrappingQuotes(value: string): string { + return value.replace(/^"+|"+$/g, ""); +} + +function isExecutableFile( + filePath: string, + windowsPathExtensions: ReadonlyArray, +): boolean { + try { + const stat = statSync(filePath); + if (!stat.isFile()) return false; + const extension = extname(filePath); + if (extension.length === 0) return false; + if (windowsPathExtensions.length === 0) { + return true; + } + return windowsPathExtensions.includes(extension.toUpperCase()); + } catch { + return false; + } +} + +function resolveEffectiveEnvironment(options: ProcessLaunchPlanOptions): NodeJS.ProcessEnv { + const env = (options.env ?? {}) as NodeJS.ProcessEnv; + if (options.inheritParentEnv === false) { + return { ...env }; + } + + return { + ...process.env, + ...env, + }; +} + +function resolveWindowsCommand( + command: string, + env: NodeJS.ProcessEnv, +): ResolvedWindowsCommand | null { + const windowsPathExtensions = resolveWindowsPathExtensions(env); + const candidates = resolveCommandCandidates(command, windowsPathExtensions); + + const classify = (filePath: string): ResolvedWindowsCommand => { + const extension = extname(filePath).toUpperCase(); + return { + path: filePath, + kind: WINDOWS_BATCH_EXECUTABLE_EXTENSIONS.has(extension) ? "batch" : "native", + }; + }; + + if (command.includes("/") || command.includes("\\")) { + for (const candidate of candidates) { + if (isExecutableFile(candidate, windowsPathExtensions)) { + return classify(candidate); + } + } + return null; + } + + const pathEntries = resolvePathEnvironmentVariable(env) + .split(";") + .map((entry) => stripWrappingQuotes(entry.trim())) + .filter((entry) => entry.length > 0); + + for (const pathEntry of pathEntries) { + for (const candidate of candidates) { + const candidatePath = join(pathEntry, candidate); + if (isExecutableFile(candidatePath, windowsPathExtensions)) { + return classify(candidatePath); + } + } + } + + return null; +} + +function quoteWindowsBatchArgument(argument: string): string { + if (argument.length === 0) { + return '""'; + } + + const escaped = argument + .replaceAll("^", "^^") + .replaceAll("%", "%%") + .replaceAll("!", "^!") + .replaceAll("&", "^&") + .replaceAll("|", "^|") + .replaceAll("<", "^<") + .replaceAll(">", "^>") + .replaceAll("(", "^(") + .replaceAll(")", "^)") + .replaceAll('"', '""'); + return `"${escaped}"`; +} + +function buildWindowsBatchCommandLine( + command: string, + args: ReadonlyArray, +): string { + return [quoteWindowsBatchArgument(command), ...args.map(quoteWindowsBatchArgument)].join(" "); +} + +function resolveWindowsCommandShell(env: NodeJS.ProcessEnv): string { + return env.ComSpec ?? env.COMSPEC ?? process.env.ComSpec ?? process.env.COMSPEC ?? "cmd.exe"; +} + +export function resolveProcessLaunchPlan( + command: string, + args: ReadonlyArray, + options: ProcessLaunchPlanOptions = {}, +): ProcessLaunchPlan { + const runtimeEnvironment = resolveRuntimeEnvironment(options.runtimeEnvironment); if (options.shell !== undefined) { - return options.shell; + return { + command, + args: [...args], + shell: options.shell, + runtimeEnvironment, + }; + } + + if (runtimeEnvironment.platform !== "windows") { + return { + command, + args: [...args], + shell: false, + runtimeEnvironment, + }; + } + + const env = resolveEffectiveEnvironment(options); + const resolved = resolveWindowsCommand(command, env); + if (!resolved) { + return { + command, + args: [...args], + shell: false, + runtimeEnvironment, + }; } - return resolveRuntimeEnvironment(options.runtimeEnvironment).platform === "windows"; + if (resolved.kind === "batch") { + return { + command: resolveWindowsCommandShell(env), + args: ["/d", "/s", "/c", buildWindowsBatchCommandLine(resolved.path, args)], + shell: false, + runtimeEnvironment, + }; + } + + return { + command: resolved.path, + args: [...args], + shell: false, + runtimeEnvironment, + }; } -function toSpawnOptions(options: ProcessSpawnOptions) { +function toSpawnOptions( + options: ProcessSpawnOptions, + launchPlan: ProcessLaunchPlan, +) { return { cwd: options.cwd, env: options.env, - shell: shouldUseShell(options), + shell: launchPlan.shell, ...(options.stdio !== undefined ? { stdio: options.stdio } : {}), ...(options.detached !== undefined ? { detached: options.detached } : {}), }; @@ -93,9 +309,16 @@ function toSpawnOptions(options: ProcessSpawnOptions) { export function toRuntimeCommandOptions( options: RuntimeCommandOptions = {}, ): ChildProcess.CommandOptions { + const launchPlan = resolveProcessLaunchPlan("", [], { + env: options.env, + runtimeEnvironment: options.runtimeEnvironment, + shell: options.shell, + inheritParentEnv: options.extendEnv !== false, + }); + return { ...options, - shell: options.shell ?? shouldUseShell(options), + shell: launchPlan.shell, }; } @@ -104,7 +327,16 @@ export function makeRuntimeCommand( args: ReadonlyArray, options: RuntimeCommandOptions = {}, ): ChildProcess.StandardCommand { - return ChildProcess.make(command, [...args], toRuntimeCommandOptions(options)); + const launchPlan = resolveProcessLaunchPlan(command, args, { + env: options.env, + runtimeEnvironment: options.runtimeEnvironment, + shell: options.shell, + inheritParentEnv: options.extendEnv !== false, + }); + return ChildProcess.make(launchPlan.command, launchPlan.args, { + ...toRuntimeCommandOptions(options), + shell: launchPlan.shell, + }); } export interface ManagedChildProcess { @@ -132,7 +364,14 @@ function spawnProcess( args: readonly string[], options: ProcessSpawnOptions = {}, ): ChildProcessHandle { - return spawn(command, args, toSpawnOptions(options)); + const launchPlan = resolveProcessLaunchPlan(command, args, { + env: options.env, + runtimeEnvironment: options.runtimeEnvironment, + shell: options.shell, + inheritParentEnv: options.env === undefined, + }); + + return spawn(launchPlan.command, launchPlan.args, toSpawnOptions(options, launchPlan)); } export function spawnPipedProcess( @@ -151,10 +390,17 @@ export function spawnProcessSync( args: readonly string[], options: ProcessSpawnSyncOptions = {}, ) { - return spawnSync(command, args, { + const launchPlan = resolveProcessLaunchPlan(command, args, { + env: options.env, + runtimeEnvironment: options.runtimeEnvironment, + shell: options.shell, + inheritParentEnv: options.env === undefined, + }); + + return spawnSync(launchPlan.command, launchPlan.args, { cwd: options.cwd, env: options.env, - shell: shouldUseShell(options), + shell: launchPlan.shell, encoding: options.encoding ?? "utf8", ...(options.stdio !== undefined ? { stdio: options.stdio } : {}), ...(options.detached !== undefined ? { detached: options.detached } : {}), @@ -248,9 +494,10 @@ function normalizeBufferError( const DEFAULT_MAX_BUFFER_BYTES = 8 * 1024 * 1024; /** - * On Windows with `shell: true`, `child.kill()` only terminates the `cmd.exe` - * wrapper, leaving the actual command running. Use `taskkill /T` to kill the - * entire process tree instead. + * On Windows, commands may still execute through a `cmd.exe` wrapper for + * explicit shell usage or `.cmd` / `.bat` launchers. `child.kill()` only + * terminates the wrapper, leaving the actual command running. Use + * `taskkill /T` to kill the entire process tree instead. */ export function killProcessTree( child: ChildProcessHandle, From aff26b834e7df0bfe2cec7c5f17b3ab27c4b6908 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 5 Mar 2026 14:27:35 -0800 Subject: [PATCH 06/10] Improve Windows/WSL command resolution and process handling - Extract shared command/path executable resolution into `commandResolution` - Resolve Windows relative launchers from `cwd` and fix batch argument quoting - Use managed Effect runtime lifecycle in `CodexAppServerManager` - Surface signal-based provider health failures clearly and expand tests - Exclude `*.browser.ts(x)` from web TypeScript project --- apps/server/src/codexAppServerManager.test.ts | 7 + apps/server/src/codexAppServerManager.ts | 27 +- apps/server/src/commandResolution.ts | 92 ++++++ apps/server/src/open.test.ts | 297 +++++++++--------- apps/server/src/open.ts | 83 +---- apps/server/src/processRunner.test.ts | 30 +- apps/server/src/processRunner.ts | 137 +++----- .../provider/Layers/ProviderHealth.test.ts | 22 ++ .../src/provider/Layers/ProviderHealth.ts | 7 + apps/web/tsconfig.json | 3 +- 10 files changed, 382 insertions(+), 323 deletions(-) create mode 100644 apps/server/src/commandResolution.ts diff --git a/apps/server/src/codexAppServerManager.test.ts b/apps/server/src/codexAppServerManager.test.ts index e88ded4a13..2526c033c0 100644 --- a/apps/server/src/codexAppServerManager.test.ts +++ b/apps/server/src/codexAppServerManager.test.ts @@ -179,6 +179,13 @@ describe("messageFromCodexProcessCause", () => { }); }); +describe("constructor", () => { + it("supports the optional no-services path without unsafe casting", () => { + const manager = new CodexAppServerManager(); + expect(() => manager.stopAll()).not.toThrow(); + }); +}); + describe("startSession", () => { it("emits session/startFailed when resolving cwd throws before process launch", async () => { const manager = createManager(); diff --git a/apps/server/src/codexAppServerManager.ts b/apps/server/src/codexAppServerManager.ts index 5a1ed26dee..96058f8dc3 100644 --- a/apps/server/src/codexAppServerManager.ts +++ b/apps/server/src/codexAppServerManager.ts @@ -16,8 +16,8 @@ import { type ProviderSessionStartInput, type ProviderTurnStartResult, } from "@t3tools/contracts"; -import type * as NodeServices from "@effect/platform-node/NodeServices"; -import { Cause, Effect, Exit, ServiceMap, Scope, Stream } from "effect"; +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { Cause, Effect, Exit, ManagedRuntime, ServiceMap, Scope, Stream } from "effect"; import type { ChildProcessSpawner } from "effect/unstable/process"; import { makeRuntimeCommand, spawnManagedCommand } from "./processRunner"; @@ -164,6 +164,9 @@ export interface CodexAppServerManagerEvents { export class CodexAppServerManager extends EventEmitter { private readonly sessions = new Map(); + private readonly ownedRuntime: + | ManagedRuntime.ManagedRuntime + | null; private readonly runPromise: ( effect: Effect.Effect, options?: Effect.RunOptions | undefined, @@ -171,13 +174,15 @@ export class CodexAppServerManager extends EventEmitter) { super(); - this.runPromise = services - ? Effect.runPromiseWith(services) - : ((effect, options) => - Effect.runPromise( - effect as unknown as Effect.Effect, - options, - )) as typeof this.runPromise; + if (services) { + this.ownedRuntime = null; + this.runPromise = Effect.runPromiseWith(services); + return; + } + + const runtime = ManagedRuntime.make(NodeServices.layer); + this.ownedRuntime = runtime; + this.runPromise = runtime.runPromise.bind(runtime); } async startSession(input: ProviderSessionStartInput): Promise { @@ -520,6 +525,10 @@ export class CodexAppServerManager extends EventEmitter { + const rawValue = env.PATHEXT; + const fallback = [".COM", ".EXE", ".BAT", ".CMD"]; + if (!rawValue) return fallback; + + const parsed = rawValue + .split(";") + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0) + .map((entry) => (entry.startsWith(".") ? entry.toUpperCase() : `.${entry.toUpperCase()}`)); + + return parsed.length > 0 ? Array.from(new Set(parsed)) : fallback; +} + +export function resolveCommandCandidates( + command: string, + platform: NodeJS.Platform, + windowsPathExtensions: ReadonlyArray, +): ReadonlyArray { + if (platform !== "win32") { + return [command]; + } + + const extension = extname(command); + const normalizedExtension = extension.toUpperCase(); + + if (extension.length > 0 && windowsPathExtensions.includes(normalizedExtension)) { + const commandWithoutExtension = command.slice(0, -extension.length); + return Array.from( + new Set([ + command, + `${commandWithoutExtension}${normalizedExtension}`, + `${commandWithoutExtension}${normalizedExtension.toLowerCase()}`, + ]), + ); + } + + const candidates: string[] = [command]; + for (const candidateExtension of windowsPathExtensions) { + candidates.push(`${command}${candidateExtension}`); + candidates.push(`${command}${candidateExtension.toLowerCase()}`); + } + return Array.from(new Set(candidates)); +} + +interface ResolveExecutableFileOptions { + readonly platform: NodeJS.Platform; + readonly windowsPathExtensions: ReadonlyArray; + readonly cwd?: string | undefined; +} + +export function resolveExecutableFile( + filePath: string, + options: ResolveExecutableFileOptions, +): string | null { + const candidatePath = + options.cwd !== undefined && !isAbsolute(filePath) ? resolve(options.cwd, filePath) : filePath; + + try { + const stat = statSync(candidatePath); + if (!stat.isFile()) return null; + + if (options.platform === "win32") { + const extension = extname(candidatePath); + if (extension.length === 0) return null; + return options.windowsPathExtensions.includes(extension.toUpperCase()) ? candidatePath : null; + } + + accessSync(candidatePath, constants.X_OK); + return candidatePath; + } catch { + return null; + } +} + +export function isExecutableFile( + filePath: string, + options: ResolveExecutableFileOptions, +): boolean { + return resolveExecutableFile(filePath, options) !== null; +} diff --git a/apps/server/src/open.test.ts b/apps/server/src/open.test.ts index b324102909..bc8ad03394 100644 --- a/apps/server/src/open.test.ts +++ b/apps/server/src/open.test.ts @@ -1,8 +1,9 @@ +import assert from "node:assert/strict"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { assert, describe, it } from "@effect/vitest"; +import { describe, expect, it } from "vitest"; import { isCommandAvailable, @@ -12,117 +13,130 @@ import { resolveEditorLaunch, } from "./open"; import { Effect } from "effect"; -import { assertSuccess } from "@effect/vitest/utils"; describe("resolveEditorLaunch", () => { - it.effect("returns commands for command-based editors", () => - Effect.gen(function* () { - const cursorLaunch = yield* resolveEditorLaunch( + it("returns commands for command-based editors", async () => { + const cursorLaunch = await Effect.runPromise( + resolveEditorLaunch( { cwd: "/tmp/workspace", editor: "cursor" }, { platform: "darwin" }, - ); - assert.deepEqual(cursorLaunch, { - command: "cursor", - args: ["/tmp/workspace"], - }); + ), + ); + assert.deepEqual(cursorLaunch, { + command: "cursor", + args: ["/tmp/workspace"], + }); - const vscodeLaunch = yield* resolveEditorLaunch( + const vscodeLaunch = await Effect.runPromise( + resolveEditorLaunch( { cwd: "/tmp/workspace", editor: "vscode" }, { platform: "darwin" }, - ); - assert.deepEqual(vscodeLaunch, { - command: "code", - args: ["/tmp/workspace"], - }); + ), + ); + assert.deepEqual(vscodeLaunch, { + command: "code", + args: ["/tmp/workspace"], + }); - const zedLaunch = yield* resolveEditorLaunch( + const zedLaunch = await Effect.runPromise( + resolveEditorLaunch( { cwd: "/tmp/workspace", editor: "zed" }, { platform: "darwin" }, - ); - assert.deepEqual(zedLaunch, { - command: "zed", - args: ["/tmp/workspace"], - }); - }), - ); + ), + ); + assert.deepEqual(zedLaunch, { + command: "zed", + args: ["/tmp/workspace"], + }); + }); - it.effect("uses --goto when editor supports line/column suffixes", () => - Effect.gen(function* () { - const lineOnly = yield* resolveEditorLaunch( + it("uses --goto when editor supports line/column suffixes", async () => { + const lineOnly = await Effect.runPromise( + resolveEditorLaunch( { cwd: "/tmp/workspace/AGENTS.md:48", editor: "cursor" }, { platform: "darwin" }, - ); - assert.deepEqual(lineOnly, { - command: "cursor", - args: ["--goto", "/tmp/workspace/AGENTS.md:48"], - }); + ), + ); + assert.deepEqual(lineOnly, { + command: "cursor", + args: ["--goto", "/tmp/workspace/AGENTS.md:48"], + }); - const lineAndColumn = yield* resolveEditorLaunch( + const lineAndColumn = await Effect.runPromise( + resolveEditorLaunch( { cwd: "/tmp/workspace/src/open.ts:71:5", editor: "cursor" }, { platform: "darwin" }, - ); - assert.deepEqual(lineAndColumn, { - command: "cursor", - args: ["--goto", "/tmp/workspace/src/open.ts:71:5"], - }); + ), + ); + assert.deepEqual(lineAndColumn, { + command: "cursor", + args: ["--goto", "/tmp/workspace/src/open.ts:71:5"], + }); - const vscodeLineAndColumn = yield* resolveEditorLaunch( + const vscodeLineAndColumn = await Effect.runPromise( + resolveEditorLaunch( { cwd: "/tmp/workspace/src/open.ts:71:5", editor: "vscode" }, { platform: "darwin" }, - ); - assert.deepEqual(vscodeLineAndColumn, { - command: "code", - args: ["--goto", "/tmp/workspace/src/open.ts:71:5"], - }); + ), + ); + assert.deepEqual(vscodeLineAndColumn, { + command: "code", + args: ["--goto", "/tmp/workspace/src/open.ts:71:5"], + }); - const zedLineAndColumn = yield* resolveEditorLaunch( + const zedLineAndColumn = await Effect.runPromise( + resolveEditorLaunch( { cwd: "/tmp/workspace/src/open.ts:71:5", editor: "zed" }, { platform: "darwin" }, - ); - assert.deepEqual(zedLineAndColumn, { - command: "zed", - args: ["/tmp/workspace/src/open.ts:71:5"], - }); - }), - ); + ), + ); + assert.deepEqual(zedLineAndColumn, { + command: "zed", + args: ["/tmp/workspace/src/open.ts:71:5"], + }); + }); - it.effect("maps file-manager editor to OS open commands", () => - Effect.gen(function* () { - const launch1 = yield* resolveEditorLaunch( + it("maps file-manager editor to OS open commands", async () => { + const launch1 = await Effect.runPromise( + resolveEditorLaunch( { cwd: "/tmp/workspace", editor: "file-manager" }, { platform: "darwin" }, - ); - assert.deepEqual(launch1, { - command: "open", - args: ["/tmp/workspace"], - }); + ), + ); + assert.deepEqual(launch1, { + command: "open", + args: ["/tmp/workspace"], + }); - const launch2 = yield* resolveEditorLaunch( + const launch2 = await Effect.runPromise( + resolveEditorLaunch( { cwd: "C:\\workspace", editor: "file-manager" }, { platform: "win32" }, - ); - assert.deepEqual(launch2, { - command: "explorer", - args: ["C:\\workspace"], - }); + ), + ); + assert.deepEqual(launch2, { + command: "explorer", + args: ["C:\\workspace"], + }); - const launch3 = yield* resolveEditorLaunch( + const launch3 = await Effect.runPromise( + resolveEditorLaunch( { cwd: "/tmp/workspace", editor: "file-manager" }, { platform: "linux" }, - ); - assert.deepEqual(launch3, { - command: "xdg-open", - args: ["/tmp/workspace"], - }); - }), - ); + ), + ); + assert.deepEqual(launch3, { + command: "xdg-open", + args: ["/tmp/workspace"], + }); + }); - it.effect("prefers linux editor shims in wsl-hosted mode when available", () => - Effect.gen(function* () { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-open-wsl-linux-editor-")); - try { - fs.writeFileSync(path.join(dir, "code"), "#!/bin/sh\n", { mode: 0o755 }); - const launch = yield* resolveEditorLaunch( + it("prefers linux editor shims in wsl-hosted mode when available", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-open-wsl-linux-editor-")); + try { + fs.writeFileSync(path.join(dir, "code"), "#!/bin/sh\n", { mode: 0o755 }); + const launch = await Effect.runPromise( + resolveEditorLaunch( { cwd: "/home/julius/project/src/open.ts:71:5", editor: "vscode" }, { platform: "linux", @@ -137,23 +151,23 @@ describe("resolveEditorLaunch", () => { PATH: dir, }, }, - ); - assert.deepEqual(launch, { - command: "code", - args: ["--goto", "/home/julius/project/src/open.ts:71:5"], - }); - } finally { - fs.rmSync(dir, { recursive: true, force: true }); - } - }), - ); + ), + ); + assert.deepEqual(launch, { + command: "code", + args: ["--goto", "/home/julius/project/src/open.ts:71:5"], + }); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); - it.effect("falls back to windows editor executables in wsl-hosted mode", () => - Effect.gen(function* () { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-open-wsl-win-editor-")); - try { - fs.writeFileSync(path.join(dir, "code.exe"), "MZ", { mode: 0o755 }); - const launch = yield* resolveEditorLaunch( + it("falls back to windows editor executables in wsl-hosted mode", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-open-wsl-win-editor-")); + try { + fs.writeFileSync(path.join(dir, "code.exe"), "MZ", { mode: 0o755 }); + const launch = await Effect.runPromise( + resolveEditorLaunch( { cwd: "/home/julius/project/src/open.ts:71:5", editor: "vscode" }, { platform: "linux", @@ -173,26 +187,26 @@ describe("resolveEditorLaunch", () => { "\\\\wsl.localhost\\Ubuntu\\home\\julius\\project", ), }, - ); - assert.deepEqual(launch, { - command: "code.exe", - args: [ - "--goto", - "\\\\wsl.localhost\\Ubuntu\\home\\julius\\project/src/open.ts:71:5", - ], - }); - } finally { - fs.rmSync(dir, { recursive: true, force: true }); - } - }), - ); + ), + ); + assert.deepEqual(launch, { + command: "code.exe", + args: [ + "--goto", + "\\\\wsl.localhost\\Ubuntu\\home\\julius\\project/src/open.ts:71:5", + ], + }); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); - it.effect("translates file-manager targets for wsl-hosted mode", () => - Effect.gen(function* () { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-open-wsl-file-manager-")); - try { - fs.writeFileSync(path.join(dir, "explorer.exe"), "MZ", { mode: 0o755 }); - const launch = yield* resolveEditorLaunch( + it("translates file-manager targets for wsl-hosted mode", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "t3-open-wsl-file-manager-")); + try { + fs.writeFileSync(path.join(dir, "explorer.exe"), "MZ", { mode: 0o755 }); + const launch = await Effect.runPromise( + resolveEditorLaunch( { cwd: "/home/julius/project/src/open.ts:71:5", editor: "file-manager" }, { platform: "linux", @@ -212,38 +226,39 @@ describe("resolveEditorLaunch", () => { "\\\\wsl.localhost\\Ubuntu\\home\\julius\\project", ), }, - ); - assert.deepEqual(launch, { - command: "explorer.exe", - args: ["\\\\wsl.localhost\\Ubuntu\\home\\julius\\project/src/open.ts"], - }); - } finally { - fs.rmSync(dir, { recursive: true, force: true }); - } - }), - ); + ), + ); + assert.deepEqual(launch, { + command: "explorer.exe", + args: ["\\\\wsl.localhost\\Ubuntu\\home\\julius\\project/src/open.ts"], + }); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }); }); describe("launchDetached", () => { - it.effect("resolves when command can be spawned", () => - Effect.gen(function* () { - const result = yield* launchDetached({ - command: process.execPath, - args: ["-e", "process.exit(0)"], - }).pipe(Effect.result); - assertSuccess(result, undefined); - }), - ); + it("resolves when command can be spawned", async () => { + await expect( + Effect.runPromise( + launchDetached({ + command: process.execPath, + args: ["-e", "process.exit(0)"], + }), + ), + ).resolves.toBeUndefined(); + }); - it.effect("rejects when command does not exist", () => - Effect.gen(function* () { - const result = yield* launchDetached({ + it("rejects when command does not exist", async () => { + const result = await Effect.runPromise( + launchDetached({ command: `t3code-no-such-command-${Date.now()}`, args: [], - }).pipe(Effect.result); - assert.equal(result._tag, "Failure"); - }), - ); + }).pipe(Effect.result), + ); + assert.equal(result._tag, "Failure"); + }); }); describe("isCommandAvailable", () => { diff --git a/apps/server/src/open.ts b/apps/server/src/open.ts index b933ad89a0..78c4e60a46 100644 --- a/apps/server/src/open.ts +++ b/apps/server/src/open.ts @@ -6,11 +6,17 @@ * * @module Open */ -import { accessSync, constants, statSync } from "node:fs"; -import { extname, join } from "node:path"; +import { join } from "node:path"; import { EDITORS, type EditorId, type ServerRuntimeEnvironment } from "@t3tools/contracts"; import { ServiceMap, Schema, Effect, Layer } from "effect"; +import { + isExecutableFile, + resolveCommandCandidates, + resolvePathEnvironmentVariable, + resolveWindowsPathExtensions, + stripWrappingQuotes, +} from "./commandResolution"; import { spawnDetachedProcess, spawnProcessSync } from "./processRunner"; import { detectServerRuntimeEnvironment } from "./runtimeEnvironment"; @@ -72,75 +78,6 @@ function fileManagerCommandForPlatform(platform: NodeJS.Platform): string { } } -function stripWrappingQuotes(value: string): string { - return value.replace(/^"+|"+$/g, ""); -} - -function resolvePathEnvironmentVariable(env: NodeJS.ProcessEnv): string { - return env.PATH ?? env.Path ?? env.path ?? ""; -} - -function resolveWindowsPathExtensions(env: NodeJS.ProcessEnv): ReadonlyArray { - const rawValue = env.PATHEXT; - const fallback = [".COM", ".EXE", ".BAT", ".CMD"]; - if (!rawValue) return fallback; - - const parsed = rawValue - .split(";") - .map((entry) => entry.trim()) - .filter((entry) => entry.length > 0) - .map((entry) => (entry.startsWith(".") ? entry.toUpperCase() : `.${entry.toUpperCase()}`)); - return parsed.length > 0 ? Array.from(new Set(parsed)) : fallback; -} - -function resolveCommandCandidates( - command: string, - platform: NodeJS.Platform, - windowsPathExtensions: ReadonlyArray, -): ReadonlyArray { - if (platform !== "win32") return [command]; - const extension = extname(command); - const normalizedExtension = extension.toUpperCase(); - - if (extension.length > 0 && windowsPathExtensions.includes(normalizedExtension)) { - const commandWithoutExtension = command.slice(0, -extension.length); - return Array.from( - new Set([ - command, - `${commandWithoutExtension}${normalizedExtension}`, - `${commandWithoutExtension}${normalizedExtension.toLowerCase()}`, - ]), - ); - } - - const candidates: string[] = []; - for (const extension of windowsPathExtensions) { - candidates.push(`${command}${extension}`); - candidates.push(`${command}${extension.toLowerCase()}`); - } - return Array.from(new Set(candidates)); -} - -function isExecutableFile( - filePath: string, - platform: NodeJS.Platform, - windowsPathExtensions: ReadonlyArray, -): boolean { - try { - const stat = statSync(filePath); - if (!stat.isFile()) return false; - if (platform === "win32") { - const extension = extname(filePath); - if (extension.length === 0) return false; - return windowsPathExtensions.includes(extension.toUpperCase()); - } - accessSync(filePath, constants.X_OK); - return true; - } catch { - return false; - } -} - function resolvePathDelimiter(platform: NodeJS.Platform): string { return platform === "win32" ? ";" : ":"; } @@ -156,7 +93,7 @@ export function isCommandAvailable( if (command.includes("/") || command.includes("\\")) { return commandCandidates.some((candidate) => - isExecutableFile(candidate, platform, windowsPathExtensions), + isExecutableFile(candidate, { platform, windowsPathExtensions }), ); } @@ -169,7 +106,7 @@ export function isCommandAvailable( for (const pathEntry of pathEntries) { for (const candidate of commandCandidates) { - if (isExecutableFile(join(pathEntry, candidate), platform, windowsPathExtensions)) { + if (isExecutableFile(join(pathEntry, candidate), { platform, windowsPathExtensions })) { return true; } } diff --git a/apps/server/src/processRunner.test.ts b/apps/server/src/processRunner.test.ts index 2c9a9af46a..c8b13be211 100644 --- a/apps/server/src/processRunner.test.ts +++ b/apps/server/src/processRunner.test.ts @@ -104,11 +104,37 @@ describe("resolveProcessLaunchPlan", () => { expect(plan.command).toBe("C:\\Windows\\System32\\cmd.exe"); expect(plan.args.slice(0, 3)).toEqual(["/d", "/s", "/c"]); - expect(plan.args[3]).toBe(`"${wrapperPath}" "C:\\repo\\a^&b.ts"`); + expect(plan.args[3]).toBe(`"${wrapperPath}" "C:\\repo\\a&b.ts"`); expect(plan.shell).toBe(false); }); }); + it("resolves relative batch launchers against the configured cwd", () => { + withTempDir((dir) => { + const wrapperPath = path.join(dir, "code.CMD"); + fs.writeFileSync(wrapperPath, "@echo off\r\n"); + + const plan = resolveProcessLaunchPlan("./code", ["pkg@^1.0"], { + cwd: dir, + env: { + PATHEXT: ".COM;.EXE;.BAT;.CMD", + COMSPEC: "C:\\Windows\\System32\\cmd.exe", + }, + inheritParentEnv: false, + runtimeEnvironment: { + platform: "windows", + pathStyle: "windows", + isWsl: false, + windowsInteropMode: "windows-native", + wslDistroName: null, + }, + }); + + expect(plan.command).toBe("C:\\Windows\\System32\\cmd.exe"); + expect(plan.args[3]).toBe(`"${wrapperPath}" "pkg@^1.0"`); + }); + }); + it("keeps wsl-hosted commands on the linux direct exec path", () => { const plan = resolveProcessLaunchPlan("code", ["/home/julius/repo"], { runtimeEnvironment: { @@ -166,7 +192,7 @@ describe("makeRuntimeCommand", () => { }); expect(command.command).toBe("C:\\Windows\\System32\\cmd.exe"); - expect(command.args).toEqual(["/d", "/s", "/c", `"${wrapperPath}" "C:\\repo\\a^&b.ts"`]); + expect(command.args).toEqual(["/d", "/s", "/c", `"${wrapperPath}" "C:\\repo\\a&b.ts"`]); expect(command.options.shell).toBe(false); }); }); diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts index 29cd1d624a..30018717ae 100644 --- a/apps/server/src/processRunner.ts +++ b/apps/server/src/processRunner.ts @@ -5,13 +5,19 @@ import { spawnSync, type StdioOptions, } from "node:child_process"; -import { statSync } from "node:fs"; import { extname, join } from "node:path"; import type { ServerRuntimeEnvironment } from "@t3tools/contracts"; import { Effect, Exit, Scope } from "effect"; import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; +import { + resolveCommandCandidates, + resolveExecutableFile, + resolvePathEnvironmentVariable, + resolveWindowsPathExtensions, + stripWrappingQuotes, +} from "./commandResolution"; import { detectServerRuntimeEnvironment } from "./runtimeEnvironment"; interface ProcessSpawnBaseOptions { @@ -27,11 +33,12 @@ interface RuntimeShellOptions { } interface ProcessLaunchPlanOptions extends RuntimeShellOptions { + cwd?: string | undefined; env?: NodeJS.ProcessEnv | Record | undefined; inheritParentEnv?: boolean | undefined; } -export interface ProcessSpawnOptions extends ProcessSpawnBaseOptions { +interface ProcessSpawnOptions extends ProcessSpawnBaseOptions { stdio?: StdioOptions | undefined; detached?: boolean | undefined; } @@ -40,7 +47,7 @@ export interface RuntimeCommandOptions extends ChildProcess.CommandOptions { runtimeEnvironment?: ServerRuntimeEnvironment | undefined; } -export interface ProcessSpawnSyncOptions extends ProcessSpawnBaseOptions { +interface ProcessSpawnSyncOptions extends ProcessSpawnBaseOptions { stdio?: StdioOptions | undefined; detached?: boolean | undefined; encoding?: BufferEncoding | undefined; @@ -93,71 +100,6 @@ function resolveRuntimeEnvironment( return runtimeEnvironment ?? detectServerRuntimeEnvironment(); } -function resolvePathEnvironmentVariable(env: NodeJS.ProcessEnv): string { - return env.PATH ?? env.Path ?? env.path ?? ""; -} - -function resolveWindowsPathExtensions(env: NodeJS.ProcessEnv): ReadonlyArray { - const rawValue = env.PATHEXT; - const fallback = [".COM", ".EXE", ".BAT", ".CMD"]; - if (!rawValue) return fallback; - - const parsed = rawValue - .split(";") - .map((entry) => entry.trim()) - .filter((entry) => entry.length > 0) - .map((entry) => (entry.startsWith(".") ? entry.toUpperCase() : `.${entry.toUpperCase()}`)); - return parsed.length > 0 ? Array.from(new Set(parsed)) : fallback; -} - -function resolveCommandCandidates( - command: string, - windowsPathExtensions: ReadonlyArray, -): ReadonlyArray { - const extension = extname(command); - const normalizedExtension = extension.toUpperCase(); - - if (extension.length > 0 && windowsPathExtensions.includes(normalizedExtension)) { - const commandWithoutExtension = command.slice(0, -extension.length); - return Array.from( - new Set([ - command, - `${commandWithoutExtension}${normalizedExtension}`, - `${commandWithoutExtension}${normalizedExtension.toLowerCase()}`, - ]), - ); - } - - const candidates: string[] = [command]; - for (const candidateExtension of windowsPathExtensions) { - candidates.push(`${command}${candidateExtension}`); - candidates.push(`${command}${candidateExtension.toLowerCase()}`); - } - return Array.from(new Set(candidates)); -} - -function stripWrappingQuotes(value: string): string { - return value.replace(/^"+|"+$/g, ""); -} - -function isExecutableFile( - filePath: string, - windowsPathExtensions: ReadonlyArray, -): boolean { - try { - const stat = statSync(filePath); - if (!stat.isFile()) return false; - const extension = extname(filePath); - if (extension.length === 0) return false; - if (windowsPathExtensions.length === 0) { - return true; - } - return windowsPathExtensions.includes(extension.toUpperCase()); - } catch { - return false; - } -} - function resolveEffectiveEnvironment(options: ProcessLaunchPlanOptions): NodeJS.ProcessEnv { const env = (options.env ?? {}) as NodeJS.ProcessEnv; if (options.inheritParentEnv === false) { @@ -173,9 +115,10 @@ function resolveEffectiveEnvironment(options: ProcessLaunchPlanOptions): NodeJS. function resolveWindowsCommand( command: string, env: NodeJS.ProcessEnv, + cwd?: string, ): ResolvedWindowsCommand | null { const windowsPathExtensions = resolveWindowsPathExtensions(env); - const candidates = resolveCommandCandidates(command, windowsPathExtensions); + const candidates = resolveCommandCandidates(command, "win32", windowsPathExtensions); const classify = (filePath: string): ResolvedWindowsCommand => { const extension = extname(filePath).toUpperCase(); @@ -187,8 +130,13 @@ function resolveWindowsCommand( if (command.includes("/") || command.includes("\\")) { for (const candidate of candidates) { - if (isExecutableFile(candidate, windowsPathExtensions)) { - return classify(candidate); + const resolvedCandidate = resolveExecutableFile(candidate, { + platform: "win32", + windowsPathExtensions, + cwd, + }); + if (resolvedCandidate) { + return classify(resolvedCandidate); } } return null; @@ -202,7 +150,12 @@ function resolveWindowsCommand( for (const pathEntry of pathEntries) { for (const candidate of candidates) { const candidatePath = join(pathEntry, candidate); - if (isExecutableFile(candidatePath, windowsPathExtensions)) { + if ( + resolveExecutableFile(candidatePath, { + platform: "win32", + windowsPathExtensions, + }) + ) { return classify(candidatePath); } } @@ -217,16 +170,9 @@ function quoteWindowsBatchArgument(argument: string): string { } const escaped = argument - .replaceAll("^", "^^") .replaceAll("%", "%%") - .replaceAll("!", "^!") - .replaceAll("&", "^&") - .replaceAll("|", "^|") - .replaceAll("<", "^<") - .replaceAll(">", "^>") - .replaceAll("(", "^(") - .replaceAll(")", "^)") - .replaceAll('"', '""'); + .replace(/(\\*)"/g, (_match, slashes: string) => `${slashes}${slashes}\\"`) + .replace(/(\\+)$/g, "$1$1"); return `"${escaped}"`; } @@ -266,7 +212,7 @@ export function resolveProcessLaunchPlan( } const env = resolveEffectiveEnvironment(options); - const resolved = resolveWindowsCommand(command, env); + const resolved = resolveWindowsCommand(command, env, options.cwd); if (!resolved) { return { command, @@ -306,19 +252,14 @@ function toSpawnOptions( }; } -export function toRuntimeCommandOptions( +function toRuntimeCommandOptions( options: RuntimeCommandOptions = {}, + shell: boolean | string, ): ChildProcess.CommandOptions { - const launchPlan = resolveProcessLaunchPlan("", [], { - env: options.env, - runtimeEnvironment: options.runtimeEnvironment, - shell: options.shell, - inheritParentEnv: options.extendEnv !== false, - }); - + const { runtimeEnvironment: _runtimeEnvironment, shell: _shell, ...commandOptions } = options; return { - ...options, - shell: launchPlan.shell, + ...commandOptions, + shell, }; } @@ -328,18 +269,18 @@ export function makeRuntimeCommand( options: RuntimeCommandOptions = {}, ): ChildProcess.StandardCommand { const launchPlan = resolveProcessLaunchPlan(command, args, { + cwd: options.cwd, env: options.env, runtimeEnvironment: options.runtimeEnvironment, shell: options.shell, inheritParentEnv: options.extendEnv !== false, }); return ChildProcess.make(launchPlan.command, launchPlan.args, { - ...toRuntimeCommandOptions(options), - shell: launchPlan.shell, + ...toRuntimeCommandOptions(options, launchPlan.shell), }); } -export interface ManagedChildProcess { +interface ManagedChildProcess { readonly scope: Scope.Closeable; readonly handle: ChildProcessSpawner.ChildProcessHandle; } @@ -365,6 +306,7 @@ function spawnProcess( options: ProcessSpawnOptions = {}, ): ChildProcessHandle { const launchPlan = resolveProcessLaunchPlan(command, args, { + cwd: options.cwd, env: options.env, runtimeEnvironment: options.runtimeEnvironment, shell: options.shell, @@ -374,7 +316,7 @@ function spawnProcess( return spawn(launchPlan.command, launchPlan.args, toSpawnOptions(options, launchPlan)); } -export function spawnPipedProcess( +function spawnPipedProcess( command: string, args: readonly string[], options: Omit = {}, @@ -391,6 +333,7 @@ export function spawnProcessSync( options: ProcessSpawnSyncOptions = {}, ) { const launchPlan = resolveProcessLaunchPlan(command, args, { + cwd: options.cwd, env: options.env, runtimeEnvironment: options.runtimeEnvironment, shell: options.shell, @@ -499,7 +442,7 @@ const DEFAULT_MAX_BUFFER_BYTES = 8 * 1024 * 1024; * terminates the wrapper, leaving the actual command running. Use * `taskkill /T` to kill the entire process tree instead. */ -export function killProcessTree( +function killProcessTree( child: ChildProcessHandle, options: { runtimeEnvironment?: ServerRuntimeEnvironment | undefined; diff --git a/apps/server/src/provider/Layers/ProviderHealth.test.ts b/apps/server/src/provider/Layers/ProviderHealth.test.ts index 7ad8eea5f7..fcce114d22 100644 --- a/apps/server/src/provider/Layers/ProviderHealth.test.ts +++ b/apps/server/src/provider/Layers/ProviderHealth.test.ts @@ -11,6 +11,7 @@ function mockRunner( stdout: string; stderr: string; code: number | null; + signal?: NodeJS.Signals | null; timedOut?: boolean; }, ) { @@ -147,6 +148,27 @@ it("parseAuthStatusFromOutput: timed out auth check is warning", () => { assert.strictEqual(parsed.message, "Timed out while checking Codex authentication status."); }); +it.effect("reports signal-based auth probe failures without 'code null'", () => + Effect.gen(function* () { + const status = yield* checkCodexProviderStatus( + mockRunner((args) => { + const joined = args.join(" "); + if (joined === "--version") return { stdout: "codex 1.0.0\n", stderr: "", code: 0 }; + if (joined === "login status") { + return { stdout: "", stderr: "", code: null, signal: "SIGTERM" }; + } + throw new Error(`Unexpected args: ${joined}`); + }), + ); + assert.strictEqual(status.status, "warning"); + assert.strictEqual(status.authStatus, "unknown"); + assert.strictEqual( + status.message, + "Could not verify Codex authentication status. Command exited from signal SIGTERM.", + ); + }), +); + // ── Pure function tests ───────────────────────────────────────────── it("parseAuthStatusFromOutput: exit code 0 with no auth markers is ready", () => { diff --git a/apps/server/src/provider/Layers/ProviderHealth.ts b/apps/server/src/provider/Layers/ProviderHealth.ts index 183d63661e..af7abcd7b6 100644 --- a/apps/server/src/provider/Layers/ProviderHealth.ts +++ b/apps/server/src/provider/Layers/ProviderHealth.ts @@ -32,6 +32,7 @@ export interface CommandResult { readonly stdout: string; readonly stderr: string; readonly code: number | null; + readonly signal?: NodeJS.Signals | null; readonly timedOut?: boolean; } @@ -76,6 +77,11 @@ function detailFromResult( if (stderr) return stderr; const stdout = nonEmptyTrimmed(result.stdout); if (stdout) return stdout; + if (result.code === null) { + return result.signal + ? `Command exited from signal ${result.signal}.` + : "Command exited without an exit code."; + } if (result.code !== 0) { return `Command exited with code ${result.code}.`; } @@ -208,6 +214,7 @@ const defaultRunCodexCommand: RunProviderHealthCommand = async (args) => { stdout: result.stdout, stderr: result.stderr, code: result.code, + signal: result.signal, timedOut: result.timedOut, } satisfies CommandResult; }; diff --git a/apps/web/tsconfig.json b/apps/web/tsconfig.json index a500ba56c1..fa6026fa14 100644 --- a/apps/web/tsconfig.json +++ b/apps/web/tsconfig.json @@ -21,5 +21,6 @@ } ] }, - "include": ["src", "vite.config.ts"] + "include": ["src", "vite.config.ts"], + "exclude": ["src/**/*.browser.ts", "src/**/*.browser.tsx"] } From 4119c721fcadbe8a43c4683a0f51b6c86f539d2a Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 5 Mar 2026 14:30:38 -0800 Subject: [PATCH 07/10] Split CI tests into cross-platform matrix job - Remove tests from the quality job to keep lint/typecheck/build focused - Add a dedicated test job that runs on both Linux and Windows runners --- .github/workflows/ci.yml | 47 +++++++++++++++++++++++++++++++++++----- 1 file changed, 42 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 787e07bce7..233e5a10d1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,7 +8,7 @@ on: jobs: quality: - name: Lint, Typecheck, Test, Browser Test, Build + name: Lint, Typecheck, Browser Test, Build runs-on: blacksmith-4vcpu-ubuntu-2404 steps: - name: Checkout @@ -51,9 +51,6 @@ jobs: - name: Typecheck run: bun run typecheck - - name: Test - run: bun run test - - name: Install browser test runtime run: | cd apps/web @@ -61,7 +58,6 @@ jobs: - name: Browser test run: bun run --cwd apps/web test:browser - - name: Build desktop pipeline run: bun run build:desktop @@ -69,3 +65,44 @@ jobs: run: | test -f apps/desktop/dist-electron/preload.js grep -nE "desktopBridge|getWsUrl|PICK_FOLDER_CHANNEL|wsUrl" apps/desktop/dist-electron/preload.js + + test: + name: Test (${{ matrix.label }}) + runs-on: ${{ matrix.runner }} + strategy: + fail-fast: false + matrix: + include: + - label: Linux + runner: blacksmith-4vcpu-ubuntu-2404 + - label: Windows + runner: blacksmith-4vcpu-windows-2025 + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Bun + uses: oven-sh/setup-bun@v2 + with: + bun-version-file: package.json + + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version-file: package.json + + - name: Cache Bun and Turbo + uses: actions/cache@v4 + with: + path: | + ~/.bun/install/cache + .turbo + key: ${{ runner.os }}-bun-${{ hashFiles('bun.lock') }}-${{ hashFiles('turbo.json') }} + restore-keys: | + ${{ runner.os }}-bun-${{ hashFiles('bun.lock') }}- + + - name: Install dependencies + run: bun install --frozen-lockfile + + - name: Test + run: bun run test From e5cea81fea229722125b3390e29bfdcce28840e0 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 5 Mar 2026 14:50:56 -0800 Subject: [PATCH 08/10] Harden server tests for Windows and WSL interop - make fake codex test launcher cross-platform (`.cmd` + Node script, PATH delimiter aware) - stabilize git/integration temp repo behavior (`core.autocrlf=false`, retrying `rmSync` cleanup) - remove OS-fragile assertions in CLI, keybindings, and terminal manager tests --- .../OrchestrationEngineHarness.integration.ts | 3 +- .../git/Layers/CodexTextGeneration.test.ts | 110 +++++++++++------- apps/server/src/git/Layers/GitCore.test.ts | 63 ++++------ apps/server/src/keybindings.test.ts | 65 +++++++---- apps/server/src/main.test.ts | 6 +- .../Layers/CheckpointReactor.test.ts | 3 +- .../src/terminal/Layers/Manager.test.ts | 4 +- 7 files changed, 147 insertions(+), 107 deletions(-) diff --git a/apps/server/integration/OrchestrationEngineHarness.integration.ts b/apps/server/integration/OrchestrationEngineHarness.integration.ts index 45eb883c6e..9c7fae9d51 100644 --- a/apps/server/integration/OrchestrationEngineHarness.integration.ts +++ b/apps/server/integration/OrchestrationEngineHarness.integration.ts @@ -70,6 +70,7 @@ function initializeGitWorkspace(cwd: string) { runGit(cwd, ["init", "--initial-branch=main"]); runGit(cwd, ["config", "user.email", "test@example.com"]); runGit(cwd, ["config", "user.name", "Test User"]); + runGit(cwd, ["config", "core.autocrlf", "false"]); fs.writeFileSync(path.join(cwd, "README.md"), "v1\n", "utf8"); runGit(cwd, ["add", "."]); runGit(cwd, ["commit", "-m", "Initial"]); @@ -388,7 +389,7 @@ export const makeOrchestrationIntegrationHarness = Effect.gen(function* () { yield* shutdown.pipe( Effect.ensuring( Effect.sync(() => { - fs.rmSync(rootDir, { recursive: true, force: true }); + fs.rmSync(rootDir, { recursive: true, force: true, maxRetries: 10, retryDelay: 50 }); }), ), ); diff --git a/apps/server/src/git/Layers/CodexTextGeneration.test.ts b/apps/server/src/git/Layers/CodexTextGeneration.test.ts index 9642f0b06a..494fbe26de 100644 --- a/apps/server/src/git/Layers/CodexTextGeneration.test.ts +++ b/apps/server/src/git/Layers/CodexTextGeneration.test.ts @@ -19,56 +19,78 @@ function makeFakeCodexBinary(dir: string) { const fs = yield* FileSystem.FileSystem; const path = yield* Path.Path; const binDir = path.join(dir, "bin"); - const codexPath = path.join(binDir, "codex"); + const launcherPath = path.join(binDir, process.platform === "win32" ? "codex.cmd" : "codex"); + const scriptPath = path.join(binDir, "codex.cjs"); yield* fs.makeDirectory(binDir, { recursive: true }); yield* fs.writeFileString( - codexPath, + scriptPath, [ - "#!/bin/sh", - 'output_path=""', - "while [ $# -gt 0 ]; do", - ' if [ "$1" = "--image" ]; then', - " shift", - ' if [ -n "$1" ]; then', - ' seen_image="1"', - " fi", - " continue", - " fi", - ' if [ "$1" = "--output-last-message" ]; then', - " shift", - ' output_path="$1"', - " fi", - " shift", - "done", - 'stdin_content="$(cat)"', - 'if [ "$T3_FAKE_CODEX_REQUIRE_IMAGE" = "1" ] && [ "$seen_image" != "1" ]; then', - ' printf "%s\\n" "missing --image input" >&2', - " exit 2", - "fi", - 'if [ -n "$T3_FAKE_CODEX_STDIN_MUST_CONTAIN" ]; then', - ' printf "%s" "$stdin_content" | grep -F -- "$T3_FAKE_CODEX_STDIN_MUST_CONTAIN" >/dev/null || {', - ' printf "%s\\n" "stdin missing expected content" >&2', - " exit 3", + 'const fs = require("node:fs");', + "", + "async function readStdin() {", + " const chunks = [];", + " for await (const chunk of process.stdin) {", + " chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk));", " }", - "fi", - 'if [ -n "$T3_FAKE_CODEX_STDIN_MUST_NOT_CONTAIN" ]; then', - ' if printf "%s" "$stdin_content" | grep -F -- "$T3_FAKE_CODEX_STDIN_MUST_NOT_CONTAIN" >/dev/null; then', - ' printf "%s\\n" "stdin contained forbidden content" >&2', - " exit 4", - " fi", - "fi", - 'if [ -n "$T3_FAKE_CODEX_STDERR" ]; then', - ' printf "%s\\n" "$T3_FAKE_CODEX_STDERR" >&2', - "fi", - 'if [ -n "$output_path" ]; then', - ' node -e \'const fs=require("node:fs"); const value=process.argv[2] ?? ""; fs.writeFileSync(process.argv[1], Buffer.from(value, "base64"));\' "$output_path" "${T3_FAKE_CODEX_OUTPUT_B64:-e30=}"', - "fi", - 'exit "${T3_FAKE_CODEX_EXIT_CODE:-0}"', + ' return Buffer.concat(chunks).toString("utf8");', + "}", + "", + "(async () => {", + ' let outputPath = "";', + " let seenImage = false;", + " for (let index = 2; index < process.argv.length; index += 1) {", + " const arg = process.argv[index];", + ' if (arg === "--image") {', + " const imagePath = process.argv[index + 1];", + " if (imagePath) {", + " seenImage = true;", + " index += 1;", + " }", + " continue;", + " }", + ' if (arg === "--output-last-message") {', + ' outputPath = process.argv[index + 1] ?? "";', + " index += 1;", + " }", + " }", + " const stdinContent = await readStdin();", + ' if (process.env.T3_FAKE_CODEX_REQUIRE_IMAGE === "1" && !seenImage) {', + ' process.stderr.write("missing --image input\\n");', + " process.exit(2);", + " }", + " const mustContain = process.env.T3_FAKE_CODEX_STDIN_MUST_CONTAIN;", + ' if (mustContain && !stdinContent.includes(mustContain)) {', + ' process.stderr.write("stdin missing expected content\\n");', + " process.exit(3);", + " }", + " const mustNotContain = process.env.T3_FAKE_CODEX_STDIN_MUST_NOT_CONTAIN;", + ' if (mustNotContain && stdinContent.includes(mustNotContain)) {', + ' process.stderr.write("stdin contained forbidden content\\n");', + " process.exit(4);", + " }", + " if (process.env.T3_FAKE_CODEX_STDERR) {", + ' process.stderr.write(`${process.env.T3_FAKE_CODEX_STDERR}\\n`);', + " }", + " if (outputPath) {", + ' fs.writeFileSync(outputPath, Buffer.from(process.env.T3_FAKE_CODEX_OUTPUT_B64 ?? "e30=", "base64"));', + " }", + ' process.exit(Number(process.env.T3_FAKE_CODEX_EXIT_CODE ?? "0"));', + "})().catch((error) => {", + ' process.stderr.write(`${error instanceof Error ? error.message : String(error)}\\n`);', + " process.exit(1);", + "});", "", ].join("\n"), ); - yield* fs.chmod(codexPath, 0o755); + + yield* fs.writeFileString( + launcherPath, + process.platform === "win32" + ? `@echo off\r\n"${process.execPath}" "${scriptPath}" %*\r\n` + : `#!/bin/sh\nexec "${process.execPath}" "${scriptPath}" "$@"\n`, + ); + yield* fs.chmod(launcherPath, 0o755); return binDir; }); } @@ -96,9 +118,11 @@ function withFakeCodexEnv( const previousRequireImage = process.env.T3_FAKE_CODEX_REQUIRE_IMAGE; const previousStdinMustContain = process.env.T3_FAKE_CODEX_STDIN_MUST_CONTAIN; const previousStdinMustNotContain = process.env.T3_FAKE_CODEX_STDIN_MUST_NOT_CONTAIN; + const pathDelimiter = process.platform === "win32" ? ";" : ":"; yield* Effect.sync(() => { - process.env.PATH = `${binDir}:${previousPath ?? ""}`; + process.env.PATH = + previousPath && previousPath.length > 0 ? `${binDir}${pathDelimiter}${previousPath}` : binDir; process.env.T3_FAKE_CODEX_OUTPUT_B64 = Buffer.from(input.output, "utf8").toString("base64"); if (input.exitCode !== undefined) { diff --git a/apps/server/src/git/Layers/GitCore.test.ts b/apps/server/src/git/Layers/GitCore.test.ts index 0c0e914032..900d3218ff 100644 --- a/apps/server/src/git/Layers/GitCore.test.ts +++ b/apps/server/src/git/Layers/GitCore.test.ts @@ -1,17 +1,18 @@ -import { existsSync } from "node:fs"; +import { existsSync, mkdtempSync, rmSync } from "node:fs"; +import os from "node:os"; import path from "node:path"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { it } from "@effect/vitest"; -import { Effect, FileSystem, Layer, PlatformError, Scope } from "effect"; -import { describe, expect, vi } from "vitest"; +import { Effect, FileSystem, Layer, PlatformError } from "effect"; +import { afterEach, describe, expect, vi } from "vitest"; import { GitServiceLive } from "./GitService.ts"; import { GitService, type GitServiceShape } from "../Services/GitService.ts"; import { GitCoreLive } from "./GitCore.ts"; import { GitCore, type GitCoreShape } from "../Services/GitCore.ts"; import { GitCommandError } from "../Errors.ts"; -import { type ProcessRunResult, runProcess } from "../../processRunner.ts"; +import { runProcess } from "../../processRunner.ts"; // ── Helpers ── @@ -21,13 +22,20 @@ const GitCoreTestLayer = GitCoreLive.pipe( Layer.provide(NodeServices.layer), ); const TestLayer = Layer.mergeAll(NodeServices.layer, GitServiceTestLayer, GitCoreTestLayer); +const tempDirs = new Set(); -function makeTmpDir( - prefix = "git-test-", -): Effect.Effect { - return Effect.gen(function* () { - const fileSystem = yield* FileSystem.FileSystem; - return yield* fileSystem.makeTempDirectoryScoped({ prefix }); +afterEach(() => { + for (const dir of tempDirs) { + rmSync(dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 50 }); + } + tempDirs.clear(); +}); + +function makeTmpDir(prefix = "git-test-"): Effect.Effect { + return Effect.sync(() => { + const dir = mkdtempSync(path.join(os.tmpdir(), prefix)); + tempDirs.add(dir); + return dir; }); } @@ -60,31 +68,6 @@ function git( }); } -function runShellCommand(input: { - command: string; - cwd: string; - timeoutMs?: number; - maxOutputBytes?: number; -}): Effect.Effect { - return Effect.promise(() => { - const shellPath = - process.platform === "win32" - ? (process.env.ComSpec ?? "cmd.exe") - : (process.env.SHELL ?? "/bin/sh"); - - const args = - process.platform === "win32" ? ["/d", "/s", "/c", input.command] : ["-lc", input.command]; - - return runProcess(shellPath, args, { - cwd: input.cwd, - timeoutMs: input.timeoutMs ?? 30_000, - allowNonZeroExit: true, - maxBufferBytes: input.maxOutputBytes ?? 1_000_000, - outputMode: "truncate", - }); - }); -} - const makeIsolatedGitCore = (gitService: GitServiceShape) => Effect.promise(async () => { const gitServiceLayer = Layer.succeed(GitService, gitService); @@ -182,6 +165,7 @@ function initRepoWithCommit( yield* initGitRepo({ cwd }); yield* git(cwd, ["config", "user.email", "test@test.com"]); yield* git(cwd, ["config", "user.name", "Test"]); + yield* git(cwd, ["config", "core.autocrlf", "false"]); yield* writeTextFile(path.join(cwd, "README.md"), "# test\n"); yield* git(cwd, ["add", "."]); yield* git(cwd, ["commit", "-m", "initial commit"]); @@ -215,12 +199,13 @@ function commitWithDate( it.layer(TestLayer)("git integration", (it) => { describe("shell process execution", () => { it.effect("caps captured output when maxOutputBytes is exceeded", () => - Effect.gen(function* () { - const result = yield* runShellCommand({ - command: `node -e "process.stdout.write('x'.repeat(2000))"`, + Effect.promise(async () => { + const result = await runProcess(process.execPath, ["-e", "process.stdout.write('x'.repeat(2000))"], { cwd: process.cwd(), timeoutMs: 10_000, - maxOutputBytes: 128, + allowNonZeroExit: true, + maxBufferBytes: 128, + outputMode: "truncate", }); expect(result.code).toBe(0); diff --git a/apps/server/src/keybindings.test.ts b/apps/server/src/keybindings.test.ts index 4a10d5816b..53f04fa844 100644 --- a/apps/server/src/keybindings.test.ts +++ b/apps/server/src/keybindings.test.ts @@ -17,22 +17,46 @@ import { } from "./keybindings"; const KeybindingsConfigJson = Schema.fromJsonString(KeybindingsConfig); -const makeKeybindingsLayer = () => - KeybindingsLive.pipe( - Layer.provideMerge( - Layer.effect( - ServerConfig, - Effect.gen(function* () { - const fs = yield* FileSystem.FileSystem; - const { join } = yield* Path.Path; - const dir = yield* fs.makeTempDirectoryScoped({ prefix: "t3code-server-config-test-" }); - const configPath = join(dir, "keybindings.json"); - return { keybindingsConfigPath: configPath } as ServerConfigShape; - }), - ), - ), +const makeKeybindingsLayer = (options?: { + readonly fileSystemLayer?: Layer.Layer; +}) => { + const serverConfigLayer = Layer.effect( + ServerConfig, + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const { join } = yield* Path.Path; + const dir = yield* fs.makeTempDirectoryScoped({ prefix: "t3code-server-config-test-" }); + const configPath = join(dir, "keybindings.json"); + return { keybindingsConfigPath: configPath } as ServerConfigShape; + }), ); + return options?.fileSystemLayer + ? KeybindingsLive.pipe( + Layer.provideMerge(options.fileSystemLayer), + Layer.provideMerge(serverConfigLayer), + ) + : KeybindingsLive.pipe(Layer.provideMerge(serverConfigLayer)); +}; + +const makeTempWriteFailingFileSystemLayer = () => + Layer.effect( + FileSystem.FileSystem, + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + return { + ...fs, + writeFileString: ((filePath: string, ...args: [string, ...unknown[]]) => + filePath.endsWith(".tmp") + ? Effect.fail(new Error("simulated temp write failure")) + : (fs.writeFileString as (...allArgs: [string, ...unknown[]]) => ReturnType)( + filePath, + ...args, + )) as typeof fs.writeFileString, + } satisfies typeof fs; + }), + ).pipe(Layer.provide(NodeServices.layer)); + const toDetailResult = (effect: Effect.Effect) => effect.pipe( Effect.mapError((error) => error.detail), @@ -392,13 +416,10 @@ it.layer(NodeServices.layer)("keybindings", (it) => { it.effect("fails when config directory is not writable", () => Effect.gen(function* () { - const fs = yield* FileSystem.FileSystem; const { keybindingsConfigPath } = yield* ServerConfig; - const { dirname } = yield* Path.Path; yield* writeKeybindingsConfig(keybindingsConfigPath, [ { key: "mod+j", command: "terminal.toggle" }, ]); - yield* fs.chmod(dirname(keybindingsConfigPath), 0o500); const result = yield* Effect.gen(function* () { const keybindings = yield* Keybindings; @@ -409,12 +430,16 @@ it.layer(NodeServices.layer)("keybindings", (it) => { }).pipe(toDetailResult); assertFailure(result, "failed to write keybindings config"); - yield* fs.chmod(dirname(keybindingsConfigPath), 0o700); - const persisted = yield* readKeybindingsConfig(keybindingsConfigPath); const persistedView = persisted.map(({ key, command }) => ({ key, command })); assert.deepEqual(persistedView, [{ key: "mod+j", command: "terminal.toggle" }]); - }).pipe(Effect.provide(makeKeybindingsLayer())), + }).pipe( + Effect.provide( + makeKeybindingsLayer({ + fileSystemLayer: makeTempWriteFailingFileSystemLayer(), + }), + ), + ), ); it.effect("caches loaded resolved config across repeated reads", () => diff --git a/apps/server/src/main.test.ts b/apps/server/src/main.test.ts index e0d794aee6..716acf13f0 100644 --- a/apps/server/src/main.test.ts +++ b/apps/server/src/main.test.ts @@ -1,4 +1,5 @@ import * as Http from "node:http"; +import path from "node:path"; import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, it, vi } from "@effect/vitest"; import * as ConfigProvider from "effect/ConfigProvider"; @@ -25,6 +26,7 @@ const serverStart = Effect.acquireRelease( () => Effect.sync(() => stop()), ); const findAvailablePort = vi.fn((preferred: number) => Effect.succeed(preferred)); +const resolveExpectedStateDir = (value: string) => path.resolve(value); // Shared service layer used by this CLI test suite. const testLayer = Layer.mergeAll( @@ -100,7 +102,7 @@ it.layer(testLayer)("server cli", (it) => { assert.equal(resolvedConfig?.mode, "desktop"); assert.equal(resolvedConfig?.port, 4010); assert.equal(resolvedConfig?.host, "0.0.0.0"); - assert.equal(resolvedConfig?.stateDir, "/tmp/t3-cli-state"); + assert.equal(resolvedConfig?.stateDir, resolveExpectedStateDir("/tmp/t3-cli-state")); assert.equal(resolvedConfig?.devUrl?.toString(), "http://127.0.0.1:5173/"); assert.equal(resolvedConfig?.noBrowser, true); assert.equal(resolvedConfig?.authToken, "auth-secret"); @@ -135,7 +137,7 @@ it.layer(testLayer)("server cli", (it) => { assert.equal(resolvedConfig?.mode, "desktop"); assert.equal(resolvedConfig?.port, 4999); assert.equal(resolvedConfig?.host, "100.88.10.4"); - assert.equal(resolvedConfig?.stateDir, "/tmp/t3-env-state"); + assert.equal(resolvedConfig?.stateDir, resolveExpectedStateDir("/tmp/t3-env-state")); assert.equal(resolvedConfig?.devUrl?.toString(), "http://localhost:5173/"); assert.equal(resolvedConfig?.noBrowser, true); assert.equal(resolvedConfig?.authToken, "env-token"); diff --git a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts index 25ca97b804..dbee690da6 100644 --- a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts +++ b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts @@ -155,6 +155,7 @@ function createGitRepository() { runGit(cwd, ["init", "--initial-branch=main"]); runGit(cwd, ["config", "user.email", "test@example.com"]); runGit(cwd, ["config", "user.name", "Test User"]); + runGit(cwd, ["config", "core.autocrlf", "false"]); fs.writeFileSync(path.join(cwd, "README.md"), "v1\n", "utf8"); runGit(cwd, ["add", "."]); runGit(cwd, ["commit", "-m", "Initial"]); @@ -209,7 +210,7 @@ describe("CheckpointReactor", () => { while (tempDirs.length > 0) { const dir = tempDirs.pop(); if (dir) { - fs.rmSync(dir, { recursive: true, force: true }); + fs.rmSync(dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 50 }); } } }); diff --git a/apps/server/src/terminal/Layers/Manager.test.ts b/apps/server/src/terminal/Layers/Manager.test.ts index 79c09f1727..92bc452339 100644 --- a/apps/server/src/terminal/Layers/Manager.test.ts +++ b/apps/server/src/terminal/Layers/Manager.test.ts @@ -575,7 +575,9 @@ describe("TerminalManager", () => { expect(snapshot.status).toBe("running"); expect(ptyAdapter.spawnInputs.length).toBeGreaterThanOrEqual(2); - expect(ptyAdapter.spawnInputs[0]?.shell).toBe("/definitely/missing-shell"); + expect(ptyAdapter.spawnInputs[0]?.shell).toBe( + process.platform === "win32" ? "/definitely/missing-shell -l" : "/definitely/missing-shell", + ); if (process.platform === "win32") { expect( From 870d6943ed0d8da9292ab4ff5d456d60174ea218 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 5 Mar 2026 16:14:36 -0800 Subject: [PATCH 09/10] Run Windows batch launchers via shell and harden test cleanup - Launch Windows .bat/.cmd wrappers directly with `shell` set to COMSPEC to preserve argument/path handling (including spaced paths) - Update process runner tests to cover the new Windows launch behavior - Add `removeDirectoryBestEffort` and use it in integration/tests to reduce flaky temp-dir cleanup failures --- .../OrchestrationEngineHarness.integration.ts | 5 +- apps/server/src/git/Layers/GitCore.test.ts | 7 +-- .../Layers/CheckpointReactor.test.ts | 3 +- apps/server/src/processRunner.test.ts | 47 +++++++++++++++---- apps/server/src/processRunner.ts | 25 ++-------- .../testUtils/removeDirectoryBestEffort.ts | 40 ++++++++++++++++ 6 files changed, 89 insertions(+), 38 deletions(-) create mode 100644 apps/server/src/testUtils/removeDirectoryBestEffort.ts diff --git a/apps/server/integration/OrchestrationEngineHarness.integration.ts b/apps/server/integration/OrchestrationEngineHarness.integration.ts index 9c7fae9d51..ace8744856 100644 --- a/apps/server/integration/OrchestrationEngineHarness.integration.ts +++ b/apps/server/integration/OrchestrationEngineHarness.integration.ts @@ -57,6 +57,7 @@ import { type TestProviderAdapterHarness, } from "./TestProviderAdapter.integration.ts"; import { ServerConfig } from "../src/config.ts"; +import { removeDirectoryBestEffort } from "../src/testUtils/removeDirectoryBestEffort.ts"; function runGit(cwd: string, args: ReadonlyArray) { return execFileSync("git", args, { @@ -388,9 +389,7 @@ export const makeOrchestrationIntegrationHarness = Effect.gen(function* () { yield* shutdown.pipe( Effect.ensuring( - Effect.sync(() => { - fs.rmSync(rootDir, { recursive: true, force: true, maxRetries: 10, retryDelay: 50 }); - }), + Effect.promise(() => removeDirectoryBestEffort(rootDir)), ), ); }); diff --git a/apps/server/src/git/Layers/GitCore.test.ts b/apps/server/src/git/Layers/GitCore.test.ts index 900d3218ff..8d494931ad 100644 --- a/apps/server/src/git/Layers/GitCore.test.ts +++ b/apps/server/src/git/Layers/GitCore.test.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdtempSync, rmSync } from "node:fs"; +import { existsSync, mkdtempSync } from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -13,6 +13,7 @@ import { GitCoreLive } from "./GitCore.ts"; import { GitCore, type GitCoreShape } from "../Services/GitCore.ts"; import { GitCommandError } from "../Errors.ts"; import { runProcess } from "../../processRunner.ts"; +import { removeDirectoryBestEffort } from "../../testUtils/removeDirectoryBestEffort.ts"; // ── Helpers ── @@ -24,9 +25,9 @@ const GitCoreTestLayer = GitCoreLive.pipe( const TestLayer = Layer.mergeAll(NodeServices.layer, GitServiceTestLayer, GitCoreTestLayer); const tempDirs = new Set(); -afterEach(() => { +afterEach(async () => { for (const dir of tempDirs) { - rmSync(dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 50 }); + await removeDirectoryBestEffort(dir); } tempDirs.clear(); }); diff --git a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts index dbee690da6..521b243676 100644 --- a/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts +++ b/apps/server/src/orchestration/Layers/CheckpointReactor.test.ts @@ -38,6 +38,7 @@ import { } from "../../provider/Services/ProviderService.ts"; import { checkpointRefForThreadTurn } from "../../checkpointing/Utils.ts"; import { ServerConfig } from "../../config.ts"; +import { removeDirectoryBestEffort } from "../../testUtils/removeDirectoryBestEffort.ts"; const asProjectId = (value: string): ProjectId => ProjectId.makeUnsafe(value); const asSessionId = (value: string): ProviderSessionId => ProviderSessionId.makeUnsafe(value); @@ -210,7 +211,7 @@ describe("CheckpointReactor", () => { while (tempDirs.length > 0) { const dir = tempDirs.pop(); if (dir) { - fs.rmSync(dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 50 }); + await removeDirectoryBestEffort(dir); } } }); diff --git a/apps/server/src/processRunner.test.ts b/apps/server/src/processRunner.test.ts index c8b13be211..a50ca708f5 100644 --- a/apps/server/src/processRunner.test.ts +++ b/apps/server/src/processRunner.test.ts @@ -102,10 +102,9 @@ describe("resolveProcessLaunchPlan", () => { }, }); - expect(plan.command).toBe("C:\\Windows\\System32\\cmd.exe"); - expect(plan.args.slice(0, 3)).toEqual(["/d", "/s", "/c"]); - expect(plan.args[3]).toBe(`"${wrapperPath}" "C:\\repo\\a&b.ts"`); - expect(plan.shell).toBe(false); + expect(plan.command).toBe(wrapperPath); + expect(plan.args).toEqual(["C:\\repo\\a&b.ts"]); + expect(plan.shell).toBe("C:\\Windows\\System32\\cmd.exe"); }); }); @@ -130,8 +129,38 @@ describe("resolveProcessLaunchPlan", () => { }, }); - expect(plan.command).toBe("C:\\Windows\\System32\\cmd.exe"); - expect(plan.args[3]).toBe(`"${wrapperPath}" "pkg@^1.0"`); + expect(plan.command).toBe(wrapperPath); + expect(plan.args).toEqual(["pkg@^1.0"]); + expect(plan.shell).toBe("C:\\Windows\\System32\\cmd.exe"); + }); + }); + + it("preserves spaced batch launcher paths on windows", () => { + withTempDir((rootDir) => { + const dir = path.join(rootDir, "runner admin"); + fs.mkdirSync(dir); + const wrapperPath = path.join(dir, "code.CMD"); + fs.writeFileSync(wrapperPath, "@echo off\r\n"); + + const plan = resolveProcessLaunchPlan("code", ["--status"], { + env: { + PATH: dir, + PATHEXT: ".COM;.EXE;.BAT;.CMD", + COMSPEC: "C:\\Windows\\System32\\cmd.exe", + }, + inheritParentEnv: false, + runtimeEnvironment: { + platform: "windows", + pathStyle: "windows", + isWsl: false, + windowsInteropMode: "windows-native", + wslDistroName: null, + }, + }); + + expect(plan.command).toBe(wrapperPath); + expect(plan.args).toEqual(["--status"]); + expect(plan.shell).toBe("C:\\Windows\\System32\\cmd.exe"); }); }); @@ -191,9 +220,9 @@ describe("makeRuntimeCommand", () => { }, }); - expect(command.command).toBe("C:\\Windows\\System32\\cmd.exe"); - expect(command.args).toEqual(["/d", "/s", "/c", `"${wrapperPath}" "C:\\repo\\a&b.ts"`]); - expect(command.options.shell).toBe(false); + expect(command.command).toBe(wrapperPath); + expect(command.args).toEqual(["C:\\repo\\a&b.ts"]); + expect(command.options.shell).toBe("C:\\Windows\\System32\\cmd.exe"); }); }); }); diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts index 30018717ae..67c51e375e 100644 --- a/apps/server/src/processRunner.ts +++ b/apps/server/src/processRunner.ts @@ -164,25 +164,6 @@ function resolveWindowsCommand( return null; } -function quoteWindowsBatchArgument(argument: string): string { - if (argument.length === 0) { - return '""'; - } - - const escaped = argument - .replaceAll("%", "%%") - .replace(/(\\*)"/g, (_match, slashes: string) => `${slashes}${slashes}\\"`) - .replace(/(\\+)$/g, "$1$1"); - return `"${escaped}"`; -} - -function buildWindowsBatchCommandLine( - command: string, - args: ReadonlyArray, -): string { - return [quoteWindowsBatchArgument(command), ...args.map(quoteWindowsBatchArgument)].join(" "); -} - function resolveWindowsCommandShell(env: NodeJS.ProcessEnv): string { return env.ComSpec ?? env.COMSPEC ?? process.env.ComSpec ?? process.env.COMSPEC ?? "cmd.exe"; } @@ -224,9 +205,9 @@ export function resolveProcessLaunchPlan( if (resolved.kind === "batch") { return { - command: resolveWindowsCommandShell(env), - args: ["/d", "/s", "/c", buildWindowsBatchCommandLine(resolved.path, args)], - shell: false, + command: resolved.path, + args: [...args], + shell: resolveWindowsCommandShell(env), runtimeEnvironment, }; } diff --git a/apps/server/src/testUtils/removeDirectoryBestEffort.ts b/apps/server/src/testUtils/removeDirectoryBestEffort.ts new file mode 100644 index 0000000000..671d1c5eb8 --- /dev/null +++ b/apps/server/src/testUtils/removeDirectoryBestEffort.ts @@ -0,0 +1,40 @@ +import fs from "node:fs"; + +const RETRYABLE_REMOVE_ERROR_CODES = new Set(["EBUSY", "ENOTEMPTY", "EPERM"]); + +function isRetryableRemoveError(error: unknown): boolean { + if (!error || typeof error !== "object") { + return false; + } + + const code = "code" in error ? error.code : undefined; + return typeof code === "string" && RETRYABLE_REMOVE_ERROR_CODES.has(code); +} + +export async function removeDirectoryBestEffort( + dir: string, + options?: { + readonly maxRetries?: number; + readonly retryDelayMs?: number; + }, +): Promise { + const maxRetries = options?.maxRetries ?? 20; + const retryDelayMs = options?.retryDelayMs ?? 100; + + for (let attempt = 0; attempt <= maxRetries; attempt += 1) { + try { + fs.rmSync(dir, { recursive: true, force: true }); + return; + } catch (error) { + if (!isRetryableRemoveError(error)) { + throw error; + } + + if (attempt === maxRetries) { + return; + } + + await new Promise((resolve) => setTimeout(resolve, retryDelayMs)); + } + } +} From aae0832ccb2912cc5fe3f30876edd4138a47024c Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Thu, 5 Mar 2026 17:41:55 -0800 Subject: [PATCH 10/10] Harden Codex session shutdown and WSL interop handling - Guard writes when app-server stdin is unavailable - Close session scopes reliably on unexpected exits and during stopAll - Stream stderr by line and make managed command spawn uninterruptible - Fix explorer.exe WSL path translation to preserve valid Windows targets --- apps/server/src/codexAppServerManager.test.ts | 156 +++++++++++++++++- apps/server/src/codexAppServerManager.ts | 141 +++++++++++----- apps/server/src/open.ts | 2 +- apps/server/src/processRunner.ts | 43 +++-- 4 files changed, 278 insertions(+), 64 deletions(-) diff --git a/apps/server/src/codexAppServerManager.test.ts b/apps/server/src/codexAppServerManager.test.ts index 2526c033c0..ede59b55c5 100644 --- a/apps/server/src/codexAppServerManager.test.ts +++ b/apps/server/src/codexAppServerManager.test.ts @@ -1,6 +1,5 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; -import { Cause } from "effect"; -import { ManagedRuntime, ServiceMap } from "effect"; +import { Cause, Effect, ManagedRuntime, ServiceMap } from "effect"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ProviderSessionId } from "@t3tools/contracts"; @@ -103,6 +102,58 @@ function createThreadControlHarness() { return { manager, context, requireSession, sendRequest, updateSession }; } +function createMinimalSessionContext(sessionId = asSessionId("sess_1")) { + return { + session: { + sessionId, + provider: "codex", + status: "ready", + threadId: "thread_1", + createdAt: "2026-02-10T00:00:00.000Z", + updatedAt: "2026-02-10T00:00:00.000Z", + }, + child: { + stdin: {}, + isRunning: Effect.succeed(true), + }, + scope: {}, + scopeClosePromise: null, + pending: new Map(), + pendingApprovals: new Map(), + nextRequestId: 1, + stopping: false, + } as unknown as { + session: { + sessionId: ProviderSessionId; + provider: "codex"; + status: "ready" | "closed"; + threadId: string; + createdAt: string; + updatedAt: string; + activeTurnId?: string; + lastError?: string; + }; + child: { + stdin: unknown; + isRunning: Effect.Effect; + }; + scope: unknown; + scopeClosePromise: Promise | null; + pending: Map< + string, + { + method: string; + timeout: ReturnType; + resolve: (value: unknown) => void; + reject: (error: Error) => void; + } + >; + pendingApprovals: Map; + nextRequestId: number; + stopping: boolean; + }; +} + describe("classifyCodexStderrLine", () => { it("ignores empty lines", () => { expect(classifyCodexStderrLine(" ")).toBeNull(); @@ -186,6 +237,107 @@ describe("constructor", () => { }); }); +describe("session lifecycle guards", () => { + it("rejects writes when the codex process is not running", async () => { + const manager = createManager(); + const context = createMinimalSessionContext(); + context.child.isRunning = Effect.succeed(false); + const runPromise = vi + .spyOn(manager as unknown as { runPromise: (...args: unknown[]) => Promise }, "runPromise") + .mockResolvedValue(false); + + await expect( + ( + manager as unknown as { writeMessage: (ctx: unknown, message: unknown) => Promise } + ).writeMessage(context, { + method: "initialized", + }), + ).rejects.toThrow("Cannot write to codex app-server stdin."); + expect(runPromise).toHaveBeenCalledTimes(1); + }); + + it("closes session scope on unexpected process exit", async () => { + const manager = createManager(); + const context = createMinimalSessionContext(); + const pendingReject = vi.fn(); + context.pending.set("1", { + method: "thread/start", + timeout: setTimeout(() => undefined, 1000), + resolve: () => undefined, + reject: pendingReject, + }); + ( + manager as unknown as { + sessions: Map; + } + ).sessions.set(context.session.sessionId, context); + const runPromise = vi + .spyOn(manager as unknown as { runPromise: (...args: unknown[]) => Promise }, "runPromise") + .mockResolvedValue(undefined); + + await ( + manager as unknown as { + handleUnexpectedProcessExit: ( + ctx: unknown, + outcome: { kind: "failure"; message: string } | { kind: "exit"; code: number }, + ) => Promise; + } + ).handleUnexpectedProcessExit(context, { kind: "exit", code: 1 }); + + expect(pendingReject).toHaveBeenCalledWith( + expect.objectContaining({ + message: "Session terminated before request completed.", + }), + ); + expect(context.session.status).toBe("closed"); + expect(context.session.lastError).toBe("codex app-server exited (code=1)."); + expect(runPromise).toHaveBeenCalledTimes(1); + expect( + ( + manager as unknown as { + sessions: Map; + } + ).sessions.has(context.session.sessionId), + ).toBe(false); + }); + + it("waits for active scope closes before disposing owned runtime", async () => { + const manager = new CodexAppServerManager(); + const context = createMinimalSessionContext(); + ( + manager as unknown as { + sessions: Map; + } + ).sessions.set(context.session.sessionId, context); + let resolveClose: (() => void) | undefined; + const closePromise = new Promise((resolve) => { + resolveClose = resolve; + }); + vi.spyOn( + manager as unknown as { runPromise: (...args: unknown[]) => Promise }, + "runPromise", + ).mockReturnValue(closePromise); + const ownedRuntime = ( + manager as unknown as { + ownedRuntime: { dispose: () => Promise } | null; + } + ).ownedRuntime; + if (!ownedRuntime) { + throw new Error("Expected manager to own a runtime."); + } + const disposeSpy = vi.spyOn(ownedRuntime, "dispose").mockResolvedValue(undefined); + + manager.stopAll(); + + expect(disposeSpy).not.toHaveBeenCalled(); + resolveClose?.(); + await closePromise; + await new Promise((resolve) => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(disposeSpy).toHaveBeenCalledTimes(1); + }); +}); + describe("startSession", () => { it("emits session/startFailed when resolving cwd throws before process launch", async () => { const manager = createManager(); diff --git a/apps/server/src/codexAppServerManager.ts b/apps/server/src/codexAppServerManager.ts index 96058f8dc3..807e8a547a 100644 --- a/apps/server/src/codexAppServerManager.ts +++ b/apps/server/src/codexAppServerManager.ts @@ -44,6 +44,7 @@ interface CodexSessionContext { session: ProviderSession; child: ChildProcessSpawner.ChildProcessHandle; scope: Scope.Closeable; + scopeClosePromise: Promise | null; pending: Map; pendingApprovals: Map; nextRequestId: number; @@ -164,6 +165,7 @@ export interface CodexAppServerManagerEvents { export class CodexAppServerManager extends EventEmitter { private readonly sessions = new Map(); + private readonly pendingScopeCloses = new Set>(); private readonly ownedRuntime: | ManagedRuntime.ManagedRuntime | null; @@ -222,6 +224,7 @@ export class CodexAppServerManager extends EventEmitter undefined); + void this.closeSessionScope(context); this.updateSession(context, { status: "closed", @@ -527,7 +525,14 @@ export class CodexAppServerManager extends EventEmitter 0 + ? Promise.allSettled(pendingScopeCloses).then(() => undefined) + : Promise.resolve(); + void closeComplete.finally(() => { + void this.ownedRuntime?.dispose(); + }); } } @@ -557,17 +562,14 @@ export class CodexAppServerManager extends EventEmitter + Stream.runForEach(Stream.splitLines(Stream.decodeText(context.child.stderr)), (rawLine) => Effect.sync(() => { - const lines = raw.split(/\r?\n/g); - for (const rawLine of lines) { - const classified = classifyCodexStderrLine(rawLine); - if (!classified) { - continue; - } - - this.emitErrorEvent(context, "process/stderr", classified.message); + const classified = classifyCodexStderrLine(rawLine); + if (!classified) { + return; } + + this.emitErrorEvent(context, "process/stderr", classified.message); }), ), ); @@ -575,35 +577,19 @@ export class CodexAppServerManager extends EventEmitter - Effect.sync(() => { - if (context.stopping) { - return; - } - - const message = messageFromCodexProcessCause(cause); - this.updateSession(context, { - status: "error", - activeTurnId: undefined, - lastError: message, - }); - this.emitErrorEvent(context, "process/error", message); - this.sessions.delete(context.session.sessionId); - }), + Effect.promise(() => + this.handleUnexpectedProcessExit(context, { + kind: "failure", + message: messageFromCodexProcessCause(cause), + }), + ), onSuccess: (code) => - Effect.sync(() => { - if (context.stopping) { - return; - } - - const message = `codex app-server exited (code=${code}).`; - this.updateSession(context, { - status: "closed", - activeTurnId: undefined, - lastError: code === 0 ? context.session.lastError : message, - }); - this.emitLifecycleEvent(context, "session/exited", message); - this.sessions.delete(context.session.sessionId); - }), + Effect.promise(() => + this.handleUnexpectedProcessExit(context, { + kind: "exit", + code, + }), + ), }), ); }.bind(this), @@ -850,6 +836,12 @@ export class CodexAppServerManager extends EventEmitter Effect.succeed(false))), + ); + if (!isRunning) { + throw new Error("Cannot write to codex app-server stdin."); + } await this.runPromise( Stream.run(context.child.stdin)(Stream.make(new TextEncoder().encode(`${encoded}\n`))).pipe( Scope.provide(context.scope), @@ -857,6 +849,65 @@ export class CodexAppServerManager extends EventEmitter { + if (context.scopeClosePromise) { + return context.scopeClosePromise; + } + + const closePromise = this.runPromise(Scope.close(context.scope, Exit.void)) + .catch(() => undefined) + .finally(() => { + if (context.scopeClosePromise === closePromise) { + context.scopeClosePromise = null; + } + this.pendingScopeCloses.delete(closePromise); + }); + context.scopeClosePromise = closePromise; + this.pendingScopeCloses.add(closePromise); + return closePromise; + } + + private async handleUnexpectedProcessExit( + context: CodexSessionContext, + outcome: { kind: "failure"; message: string } | { kind: "exit"; code: number }, + ): Promise { + if (context.stopping) { + return; + } + + context.stopping = true; + this.rejectPendingRequests(context, "Session terminated before request completed."); + context.pendingApprovals.clear(); + + if (outcome.kind === "failure") { + this.updateSession(context, { + status: "error", + activeTurnId: undefined, + lastError: outcome.message, + }); + this.emitErrorEvent(context, "process/error", outcome.message); + } else { + const message = `codex app-server exited (code=${outcome.code}).`; + this.updateSession(context, { + status: "closed", + activeTurnId: undefined, + lastError: outcome.code === 0 ? context.session.lastError : message, + }); + this.emitLifecycleEvent(context, "session/exited", message); + } + + this.sessions.delete(context.session.sessionId); + await this.closeSessionScope(context); + } + private emitLifecycleEvent(context: CodexSessionContext, method: string, message: string): void { this.emitEvent({ id: EventId.makeUnsafe(randomUUID()), diff --git a/apps/server/src/open.ts b/apps/server/src/open.ts index 78c4e60a46..a77308dd3e 100644 --- a/apps/server/src/open.ts +++ b/apps/server/src/open.ts @@ -354,7 +354,7 @@ export const resolveEditorLaunch = Effect.fnUntraced(function* ( let target = stripLineColumnSuffix(input.cwd); if (runtimeEnvironment.windowsInteropMode === "wsl-hosted" && command === "explorer.exe") { target = yield* Effect.try({ - try: () => stripLineColumnSuffix(translateWslPathToWindows(target)), + try: () => translateWslPathToWindows(target), catch: (cause) => new OpenError({ message: "Failed to translate WSL path for Windows file manager launch", diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts index 67c51e375e..e1d36f52ec 100644 --- a/apps/server/src/processRunner.ts +++ b/apps/server/src/processRunner.ts @@ -267,19 +267,21 @@ interface ManagedChildProcess { } export const spawnManagedCommand = (command: ChildProcess.Command) => - Effect.gen(function* () { - const scope = yield* Scope.make("sequential"); - const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; - const handle = yield* spawner.spawn(command).pipe( - Scope.provide(scope), - Effect.tapError(() => Scope.close(scope, Exit.void)), - ); - - return { - scope, - handle, - } satisfies ManagedChildProcess; - }); + Effect.uninterruptible( + Effect.gen(function* () { + const scope = yield* Scope.make("sequential"); + const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const handle = yield* spawner.spawn(command).pipe( + Scope.provide(scope), + Effect.tapError(() => Scope.close(scope, Exit.void)), + ); + + return { + scope, + handle, + } satisfies ManagedChildProcess; + }), + ); function spawnProcess( command: string, @@ -344,15 +346,24 @@ export function spawnDetachedProcess( stdio: "ignore", }); + const cleanup = () => { + child.off("spawn", handleSpawn); + child.off("error", handleError); + }; + const handleSpawn = () => { + cleanup(); child.unref(); resolve(); }; - child.once("spawn", handleSpawn); - child.once("error", (error) => { + const handleError = (error: Error) => { + cleanup(); reject(normalizeSpawnError(command, args, error)); - }); + }; + + child.once("spawn", handleSpawn); + child.once("error", handleError); }); }