From e6dc91b8cac74e1048aed7221b1b23861ffa68fa Mon Sep 17 00:00:00 2001 From: Quentin Date: Mon, 9 Mar 2026 23:59:09 -0400 Subject: [PATCH 1/2] refactor(server): share codex cli probes across startup and health checks --- apps/server/src/codexAppServerManager.ts | 61 +----- apps/server/src/codexCliProbe.test.ts | 57 +++++ apps/server/src/codexCliProbe.ts | 202 ++++++++++++++++++ .../provider/Layers/ProviderHealth.test.ts | 157 +++++--------- .../src/provider/Layers/ProviderHealth.ts | 117 +++++----- 5 files changed, 370 insertions(+), 224 deletions(-) create mode 100644 apps/server/src/codexCliProbe.test.ts create mode 100644 apps/server/src/codexCliProbe.ts diff --git a/apps/server/src/codexAppServerManager.ts b/apps/server/src/codexAppServerManager.ts index a8a8ce4607b3..8581b1ee83a1 100644 --- a/apps/server/src/codexAppServerManager.ts +++ b/apps/server/src/codexAppServerManager.ts @@ -22,11 +22,7 @@ import { import { normalizeModelSlug } from "@t3tools/shared/model"; import { Effect, ServiceMap } from "effect"; -import { - formatCodexCliUpgradeMessage, - isCodexCliVersionSupported, - parseCodexCliVersion, -} from "./provider/codexCliVersion"; +import { assertSupportedCodexCliVersion } from "./codexCliProbe"; type PendingRequestKey = string; @@ -144,8 +140,6 @@ export interface CodexThreadSnapshot { turns: CodexThreadTurnSnapshot[]; } -const CODEX_VERSION_CHECK_TIMEOUT_MS = 4_000; - const ANSI_ESCAPE_CHAR = String.fromCharCode(27); const ANSI_ESCAPE_REGEX = new RegExp(`${ANSI_ESCAPE_CHAR}\\[[0-9;]*m`, "g"); const CODEX_STDERR_LOG_REGEX = @@ -543,7 +537,7 @@ export class CodexAppServerManager extends EventEmitter { + await assertSupportedCodexCliVersion(input); } private updateSession(context: CodexSessionContext, updates: Partial): void { @@ -1521,51 +1515,6 @@ function readCodexProviderOptions(input: CodexAppServerStartSessionInput): { }; } -function assertSupportedCodexCliVersion(input: { - readonly binaryPath: string; - readonly cwd: string; - readonly homePath?: string; -}): void { - const result = spawnSync(input.binaryPath, ["--version"], { - cwd: input.cwd, - env: { - ...process.env, - ...(input.homePath ? { CODEX_HOME: input.homePath } : {}), - }, - encoding: "utf8", - shell: process.platform === "win32", - stdio: ["ignore", "pipe", "pipe"], - timeout: CODEX_VERSION_CHECK_TIMEOUT_MS, - maxBuffer: 1024 * 1024, - }); - - if (result.error) { - const lower = result.error.message.toLowerCase(); - if ( - lower.includes("enoent") || - lower.includes("command not found") || - lower.includes("not found") - ) { - throw new Error(`Codex CLI (${input.binaryPath}) is not installed or not executable.`); - } - throw new Error( - `Failed to execute Codex CLI version check: ${result.error.message || String(result.error)}`, - ); - } - - const stdout = result.stdout ?? ""; - const stderr = result.stderr ?? ""; - if (result.status !== 0) { - const detail = stderr.trim() || stdout.trim() || `Command exited with code ${result.status}.`; - throw new Error(`Codex CLI version check failed. ${detail}`); - } - - const parsedVersion = parseCodexCliVersion(`${stdout}\n${stderr}`); - if (parsedVersion && !isCodexCliVersionSupported(parsedVersion)) { - throw new Error(formatCodexCliUpgradeMessage(parsedVersion)); - } -} - function readResumeCursorThreadId(resumeCursor: unknown): string | undefined { if (!resumeCursor || typeof resumeCursor !== "object" || Array.isArray(resumeCursor)) { return undefined; diff --git a/apps/server/src/codexCliProbe.test.ts b/apps/server/src/codexCliProbe.test.ts new file mode 100644 index 000000000000..2ce09d497e9f --- /dev/null +++ b/apps/server/src/codexCliProbe.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it, vi } from "vitest"; + +import { + assertSupportedCodexCliVersion, + type CodexCliCommandRunner, +} from "./codexCliProbe"; + +describe("assertSupportedCodexCliVersion", () => { + it("retries a timed out version check once before succeeding", async () => { + const runCommand = vi + .fn() + .mockResolvedValueOnce({ + stdout: "", + stderr: "", + code: 124, + timedOut: true, + }) + .mockResolvedValueOnce({ + stdout: "codex-cli 0.112.0\n", + stderr: "", + code: 0, + }); + + await expect( + assertSupportedCodexCliVersion( + { + binaryPath: "codex", + cwd: "/tmp", + }, + runCommand, + ), + ).resolves.toBeUndefined(); + + expect(runCommand).toHaveBeenCalledTimes(2); + }); + + it("surfaces a timeout after exhausting retries", async () => { + const runCommand = vi.fn().mockResolvedValue({ + stdout: "", + stderr: "", + code: 124, + timedOut: true, + }); + + await expect( + assertSupportedCodexCliVersion( + { + binaryPath: "codex", + cwd: "/tmp", + }, + runCommand, + ), + ).rejects.toThrow("Failed to execute Codex CLI version check: Timed out while running command."); + + expect(runCommand).toHaveBeenCalledTimes(2); + }); +}); diff --git a/apps/server/src/codexCliProbe.ts b/apps/server/src/codexCliProbe.ts new file mode 100644 index 000000000000..6ec349f952a1 --- /dev/null +++ b/apps/server/src/codexCliProbe.ts @@ -0,0 +1,202 @@ +import { spawn } from "node:child_process"; + +import { + formatCodexCliUpgradeMessage, + isCodexCliVersionSupported, + parseCodexCliVersion, +} from "./provider/codexCliVersion"; + +export const DEFAULT_CODEX_CLI_TIMEOUT_MS = 15_000; +export const DEFAULT_CODEX_CLI_TIMEOUT_RETRY_COUNT = 1; + +export interface CodexCliCommandResult { + readonly stdout: string; + readonly stderr: string; + readonly code: number; + readonly timedOut?: boolean; +} + +export interface CodexCliCommandInput { + readonly binaryPath: string; + readonly args: ReadonlyArray; + readonly cwd?: string; + readonly homePath?: string; + readonly timeoutMs?: number; + readonly retryCount?: number; +} + +export type CodexCliCommandRunner = ( + input: CodexCliCommandInput, +) => Promise; + +function isCommandMissingError(error: unknown): boolean { + if (!(error instanceof Error)) return false; + const lower = error.message.toLowerCase(); + return ( + lower.includes("command not found") || + (lower.includes("spawn") && lower.includes("enoent")) || + lower.includes("not found") + ); +} + +function isTimeoutError(error: unknown): boolean { + if (!(error instanceof Error)) return false; + return error.message.toLowerCase().includes("etimedout"); +} + +export async function runCodexCliCommand( + input: CodexCliCommandInput, +): Promise { + const timeoutMs = input.timeoutMs ?? DEFAULT_CODEX_CLI_TIMEOUT_MS; + + return await new Promise((resolve, reject) => { + let stdout = ""; + let stderr = ""; + let settled = false; + + const child = spawn(input.binaryPath, [...input.args], { + ...(input.cwd ? { cwd: input.cwd } : {}), + env: { + ...process.env, + ...(input.homePath ? { CODEX_HOME: input.homePath } : {}), + }, + shell: process.platform === "win32", + stdio: ["ignore", "pipe", "pipe"], + }); + + const finish = (callback: () => void) => { + if (settled) return; + settled = true; + clearTimeout(timeout); + child.stdout?.removeAllListeners("data"); + child.stderr?.removeAllListeners("data"); + child.removeAllListeners("error"); + child.removeAllListeners("close"); + callback(); + }; + + child.stdout?.on("data", (chunk: string | Buffer) => { + stdout += chunk.toString(); + }); + child.stderr?.on("data", (chunk: string | Buffer) => { + stderr += chunk.toString(); + }); + + child.once("error", (error) => { + finish(() => reject(error)); + }); + + child.once("close", (code) => { + finish(() => + resolve({ + stdout, + stderr, + code: typeof code === "number" ? code : 0, + }), + ); + }); + + const timeout = setTimeout(() => { + try { + child.kill("SIGKILL"); + } catch { + // Best-effort cleanup only. + } + + finish(() => + resolve({ + stdout, + stderr, + code: 124, + timedOut: true, + }), + ); + }, timeoutMs); + }); +} + +export async function runCodexCliCommandWithRetry( + input: CodexCliCommandInput, + runCommand: CodexCliCommandRunner = runCodexCliCommand, +): Promise { + const retryCount = input.retryCount ?? DEFAULT_CODEX_CLI_TIMEOUT_RETRY_COUNT; + + for (let attempt = 0; attempt <= retryCount; attempt += 1) { + try { + const result = await runCommand(input); + if (!result.timedOut || attempt === retryCount) { + return result; + } + } catch (error) { + if (!isTimeoutError(error) || attempt === retryCount) { + throw error; + } + } + } + + return { + stdout: "", + stderr: "", + code: 124, + timedOut: true, + }; +} + +function detailFromResult(result: CodexCliCommandResult): string { + if (result.timedOut) return "Timed out while running command."; + + const stderr = result.stderr.trim(); + if (stderr.length > 0) return stderr; + + const stdout = result.stdout.trim(); + if (stdout.length > 0) return stdout; + + return `Command exited with code ${result.code}.`; +} + +export async function assertSupportedCodexCliVersion( + input: { + readonly binaryPath: string; + readonly cwd: string; + readonly homePath?: string; + }, + runCommand: CodexCliCommandRunner = runCodexCliCommand, +): Promise { + let result: CodexCliCommandResult; + try { + result = await runCodexCliCommandWithRetry( + { + binaryPath: input.binaryPath, + args: ["--version"], + cwd: input.cwd, + ...(input.homePath ? { homePath: input.homePath } : {}), + }, + runCommand, + ); + } catch (error) { + if (isCommandMissingError(error)) { + throw new Error(`Codex CLI (${input.binaryPath}) is not installed or not executable.`, { + cause: error, + }); + } + throw new Error( + `Failed to execute Codex CLI version check: ${error instanceof Error ? error.message : String(error)}`, + { + cause: error, + }, + ); + } + + if (result.timedOut) { + throw new Error(`Failed to execute Codex CLI version check: ${detailFromResult(result)}`); + } + + if (result.code !== 0) { + throw new Error(`Codex CLI version check failed. ${detailFromResult(result)}`); + } + + const parsedVersion = parseCodexCliVersion(`${result.stdout}\n${result.stderr}`); + if (parsedVersion && !isCodexCliVersionSupported(parsedVersion)) { + throw new Error(formatCodexCliUpgradeMessage(parsedVersion)); + } +} diff --git a/apps/server/src/provider/Layers/ProviderHealth.test.ts b/apps/server/src/provider/Layers/ProviderHealth.test.ts index 90df9b691fe9..9933412c25e9 100644 --- a/apps/server/src/provider/Layers/ProviderHealth.test.ts +++ b/apps/server/src/provider/Layers/ProviderHealth.test.ts @@ -1,93 +1,53 @@ 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"; +import { type CodexCliCommandResult } from "../../codexCliProbe"; +import { makeCheckCodexProviderStatus, 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( - handler: (args: ReadonlyArray) => { stdout: string; stderr: string; code: number }, +function runCheck( + handler: (args: ReadonlyArray) => Promise | CodexCliCommandResult, ) { - return Layer.succeed( - ChildProcessSpawner.ChildProcessSpawner, - ChildProcessSpawner.make((command) => { - const cmd = command as unknown as { args: ReadonlyArray }; - return Effect.succeed(mockHandle(handler(cmd.args))); - }), - ); -} - -function failingSpawnerLayer(description: string) { - return Layer.succeed( - ChildProcessSpawner.ChildProcessSpawner, - ChildProcessSpawner.make(() => - Effect.fail( - PlatformError.systemError({ - _tag: "NotFound", - module: "ChildProcess", - method: "spawn", - description, - }), - ), - ), - ); + return makeCheckCodexProviderStatus((input) => Promise.resolve(handler(input.args))); } // ── Tests ─────────────────────────────────────────────────────────── it.effect("returns ready when codex is installed and authenticated", () => Effect.gen(function* () { - const status = yield* checkCodexProviderStatus; + const status = yield* runCheck((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"); - }).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: "Logged in\n", stderr: "", code: 0 }; - throw new Error(`Unexpected args: ${joined}`); - }), - ), - ), + }), ); it.effect("returns unavailable when codex is missing", () => Effect.gen(function* () { - const status = yield* checkCodexProviderStatus; + const status = yield* makeCheckCodexProviderStatus(() => + Promise.reject(new Error("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 unavailable when codex is below the minimum supported version", () => Effect.gen(function* () { - const status = yield* checkCodexProviderStatus; + const status = yield* runCheck((args) => { + const joined = args.join(" "); + if (joined === "--version") return { stdout: "codex 0.36.0\n", stderr: "", code: 0 }; + throw new Error(`Unexpected args: ${joined}`); + }); assert.strictEqual(status.provider, "codex"); assert.strictEqual(status.status, "error"); assert.strictEqual(status.available, false); @@ -96,20 +56,19 @@ it.effect("returns unavailable when codex is below the minimum supported version status.message, "Codex CLI v0.36.0 is too old for T3 Code. Upgrade to v0.37.0 or newer and restart T3 Code.", ); - }).pipe( - Effect.provide( - mockSpawnerLayer((args) => { - const joined = args.join(" "); - if (joined === "--version") return { stdout: "codex 0.36.0\n", stderr: "", code: 0 }; - throw new Error(`Unexpected args: ${joined}`); - }), - ), - ), + }), ); it.effect("returns unauthenticated when auth probe reports login required", () => Effect.gen(function* () { - const status = yield* checkCodexProviderStatus; + const status = yield* runCheck((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); @@ -118,25 +77,19 @@ 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'", () => Effect.gen(function* () { - const status = yield* checkCodexProviderStatus; + const status = yield* runCheck((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); @@ -145,22 +98,19 @@ 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* runCheck((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); @@ -169,18 +119,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 4eb6a288cb53..1666829175e7 100644 --- a/apps/server/src/provider/Layers/ProviderHealth.ts +++ b/apps/server/src/provider/Layers/ProviderHealth.ts @@ -13,8 +13,13 @@ import type { ServerProviderStatus, ServerProviderStatusState, } from "@t3tools/contracts"; -import { Array, Effect, Fiber, Layer, Option, Result, Stream } from "effect"; -import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; +import { Data, Effect, Fiber, Layer, Result } from "effect"; + +import { + runCodexCliCommandWithRetry, + type CodexCliCommandRunner, + type CodexCliCommandResult, +} from "../../codexCliProbe"; import { formatCodexCliUpgradeMessage, @@ -23,16 +28,13 @@ import { } from "../codexCliVersion"; import { ProviderHealth, type ProviderHealthShape } from "../Services/ProviderHealth"; -const DEFAULT_TIMEOUT_MS = 4_000; const CODEX_PROVIDER = "codex" as const; // ── Pure helpers ──────────────────────────────────────────────────── -export interface CommandResult { - readonly stdout: string; - readonly stderr: string; - readonly code: number; -} +class ProviderHealthCommandError extends Data.TaggedError("ProviderHealthCommandError")<{ + readonly cause: Error; +}> {} function nonEmptyTrimmed(value: string | undefined): string | undefined { if (!value) return undefined; @@ -40,9 +42,17 @@ function nonEmptyTrimmed(value: string | undefined): string | undefined { return trimmed.length > 0 ? trimmed : undefined; } +function unwrapCommandError(error: unknown): Error | undefined { + if (error instanceof ProviderHealthCommandError) { + return error.cause; + } + return error instanceof Error ? error : undefined; +} + function isCommandMissingCause(error: unknown): boolean { - if (!(error instanceof Error)) return false; - const lower = error.message.toLowerCase(); + const cause = unwrapCommandError(error); + if (!cause) return false; + const lower = cause.message.toLowerCase(); return ( lower.includes("command not found: codex") || lower.includes("spawn codex enoent") || @@ -52,7 +62,7 @@ function isCommandMissingCause(error: unknown): boolean { } function detailFromResult( - result: CommandResult & { readonly timedOut?: boolean }, + result: CodexCliCommandResult, ): string | undefined { if (result.timedOut) return "Timed out while running command."; const stderr = nonEmptyTrimmed(result.stderr); @@ -65,6 +75,12 @@ function detailFromResult( return undefined; } +function normalizePromiseError(error: unknown): ProviderHealthCommandError { + return new ProviderHealthCommandError({ + cause: error instanceof Error ? error : new Error(String(error)), + }); +} + function extractAuthBoolean(value: unknown): boolean | undefined { if (Array.isArray(value)) { for (const entry of value) { @@ -87,7 +103,7 @@ function extractAuthBoolean(value: unknown): boolean | undefined { return undefined; } -export function parseAuthStatusFromOutput(result: CommandResult): { +export function parseAuthStatusFromOutput(result: CodexCliCommandResult): { readonly status: ServerProviderStatusState; readonly authStatus: ServerProviderAuthStatus; readonly message?: string; @@ -167,53 +183,27 @@ 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), - ); - -const runCodexCommand = (args: ReadonlyArray) => +export const makeCheckCodexProviderStatus = ( + runCommand: CodexCliCommandRunner = runCodexCliCommandWithRetry, +): Effect.Effect => Effect.gen(function* () { - const spawner = yield* ChildProcessSpawner.ChildProcessSpawner; - const command = ChildProcess.make("codex", [...args], { - shell: process.platform === "win32", - }); - - 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); - -// ── 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), + const versionProbe = yield* Effect.tryPromise({ + try: () => + runCommand({ + binaryPath: "codex", + args: ["--version"], + }), + catch: normalizePromiseError, + }).pipe( Effect.result, ); if (Result.isFailure(versionProbe)) { const error = versionProbe.failure; + const cause = unwrapCommandError(error); return { provider: CODEX_PROVIDER, status: "error" as const, @@ -222,11 +212,12 @@ export const checkCodexProviderStatus: Effect.Effect< 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)}.`, + : `Failed to execute Codex CLI health check: ${cause?.message ?? String(error)}.`, }; } - if (Option.isNone(versionProbe.success)) { + const version = versionProbe.success; + if (version.timedOut) { return { provider: CODEX_PROVIDER, status: "error" as const, @@ -237,7 +228,6 @@ export const checkCodexProviderStatus: Effect.Effect< }; } - const version = versionProbe.success.value; if (version.code !== 0) { const detail = detailFromResult(version); return { @@ -265,13 +255,20 @@ export const checkCodexProviderStatus: Effect.Effect< } // Probe 2: `codex login status` — is the user authenticated? - const authProbe = yield* runCodexCommand(["login", "status"]).pipe( - Effect.timeoutOption(DEFAULT_TIMEOUT_MS), + const authProbe = yield* Effect.tryPromise({ + try: () => + runCommand({ + binaryPath: "codex", + args: ["login", "status"], + }), + catch: normalizePromiseError, + }).pipe( Effect.result, ); if (Result.isFailure(authProbe)) { const error = authProbe.failure; + const cause = unwrapCommandError(error); return { provider: CODEX_PROVIDER, status: "warning" as const, @@ -279,13 +276,13 @@ export const checkCodexProviderStatus: Effect.Effect< authStatus: "unknown" as const, checkedAt, message: - error instanceof Error - ? `Could not verify Codex authentication status: ${error.message}.` + cause instanceof Error + ? `Could not verify Codex authentication status: ${cause.message}.` : "Could not verify Codex authentication status.", }; } - if (Option.isNone(authProbe.success)) { + if (authProbe.success.timedOut) { return { provider: CODEX_PROVIDER, status: "warning" as const, @@ -296,7 +293,7 @@ export const checkCodexProviderStatus: Effect.Effect< }; } - const parsed = parseAuthStatusFromOutput(authProbe.success.value); + const parsed = parseAuthStatusFromOutput(authProbe.success); return { provider: CODEX_PROVIDER, status: parsed.status, @@ -307,6 +304,8 @@ export const checkCodexProviderStatus: Effect.Effect< } satisfies ServerProviderStatus; }); +export const checkCodexProviderStatus = makeCheckCodexProviderStatus(); + // ── Layer ─────────────────────────────────────────────────────────── export const ProviderHealthLive = Layer.effect( From c6b8f2452ef926d70bc204852aca0200586daf0d Mon Sep 17 00:00:00 2001 From: Quentin Date: Tue, 10 Mar 2026 13:11:53 -0400 Subject: [PATCH 2/2] fix(server): harden codex cli probe validation --- apps/server/src/codexCliProbe.test.ts | 51 +++++++++++++++++++++++++++ apps/server/src/codexCliProbe.ts | 23 ++++++++++-- 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/apps/server/src/codexCliProbe.test.ts b/apps/server/src/codexCliProbe.test.ts index 2ce09d497e9f..9ccea8fb8c34 100644 --- a/apps/server/src/codexCliProbe.test.ts +++ b/apps/server/src/codexCliProbe.test.ts @@ -1,10 +1,43 @@ +import { EventEmitter } from "node:events"; import { describe, expect, it, vi } from "vitest"; +const { spawnMock } = vi.hoisted(() => ({ + spawnMock: vi.fn(), +})); + +vi.mock("node:child_process", () => ({ + spawn: spawnMock, +})); + import { assertSupportedCodexCliVersion, type CodexCliCommandRunner, + runCodexCliCommand, } from "./codexCliProbe"; +class MockChildProcess extends EventEmitter { + readonly stdout = new EventEmitter(); + readonly stderr = new EventEmitter(); + readonly kill = vi.fn(); +} + +describe("runCodexCliCommand", () => { + it("rejects when the child process closes without an exit code", async () => { + const child = new MockChildProcess(); + spawnMock.mockReturnValueOnce(child); + + const commandPromise = runCodexCliCommand({ + binaryPath: "codex", + args: ["--version"], + timeoutMs: 1_000, + }); + + child.emit("close", null, null); + + await expect(commandPromise).rejects.toThrow("Codex CLI process exited without an exit code."); + }); +}); + describe("assertSupportedCodexCliVersion", () => { it("retries a timed out version check once before succeeding", async () => { const runCommand = vi @@ -54,4 +87,22 @@ describe("assertSupportedCodexCliVersion", () => { expect(runCommand).toHaveBeenCalledTimes(2); }); + + it("rejects when the version output cannot be parsed", async () => { + const runCommand = vi.fn().mockResolvedValue({ + stdout: "codex-cli version unknown\n", + stderr: "", + code: 0, + }); + + await expect( + assertSupportedCodexCliVersion( + { + binaryPath: "codex", + cwd: "/tmp", + }, + runCommand, + ), + ).rejects.toThrow("Codex CLI version check failed. Could not parse Codex CLI version from output."); + }); }); diff --git a/apps/server/src/codexCliProbe.ts b/apps/server/src/codexCliProbe.ts index 6ec349f952a1..3feb5a50e429 100644 --- a/apps/server/src/codexCliProbe.ts +++ b/apps/server/src/codexCliProbe.ts @@ -86,12 +86,25 @@ export async function runCodexCliCommand( finish(() => reject(error)); }); - child.once("close", (code) => { + child.once("close", (code, signal) => { + if (typeof code !== "number") { + finish(() => + reject( + new Error( + signal + ? `Codex CLI process exited from signal ${signal} without an exit code.` + : "Codex CLI process exited without an exit code.", + ), + ), + ); + return; + } + finish(() => resolve({ stdout, stderr, - code: typeof code === "number" ? code : 0, + code, }), ); }); @@ -196,7 +209,11 @@ export async function assertSupportedCodexCliVersion( } const parsedVersion = parseCodexCliVersion(`${result.stdout}\n${result.stderr}`); - if (parsedVersion && !isCodexCliVersionSupported(parsedVersion)) { + if (!parsedVersion) { + throw new Error("Codex CLI version check failed. Could not parse Codex CLI version from output."); + } + + if (!isCodexCliVersionSupported(parsedVersion)) { throw new Error(formatCodexCliUpgradeMessage(parsedVersion)); } }