diff --git a/apps/server/src/codexAppServerManager.ts b/apps/server/src/codexAppServerManager.ts index b197a07fd447a54fd553ad089a4715c1114cd840..7b19c7be5ce8c19daeef4063c7672dbb038e3a37 100644 --- a/apps/server/src/codexAppServerManager.ts +++ b/apps/server/src/codexAppServerManager.ts @@ -1,4 +1,4 @@ -import { type ChildProcessWithoutNullStreams, spawn } from "node:child_process"; +import { type ChildProcessWithoutNullStreams, spawn, spawnSync } from "node:child_process"; import { randomUUID } from "node:crypto"; import { EventEmitter } from "node:events"; import readline from "node:readline"; @@ -104,6 +104,23 @@ 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, @@ -476,7 +493,7 @@ export class CodexAppServerManager extends EventEmitter - 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.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 }), + ), + ); + } + + 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 }))), ); - } - - 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 }))), - ); + }); }); const make = Effect.gen(function* () { diff --git a/apps/server/src/processRunner.ts b/apps/server/src/processRunner.ts index 75c939617f160135759ca10f6a1752eda00b441a..cbf4b76063fdab636a160f58d611a50c3b138a3c 100644 --- a/apps/server/src/processRunner.ts +++ b/apps/server/src/processRunner.ts @@ -1,4 +1,4 @@ -import { spawn } from "node:child_process"; +import { type ChildProcess as ChildProcessHandle, spawn, spawnSync } from "node:child_process"; export interface ProcessRunOptions { cwd?: string | undefined; @@ -37,11 +37,21 @@ 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; + if (code === 9009) return true; + return /is not recognized as an internal or external command/i.test(stderr); +} + function normalizeExitError( command: string, args: readonly string[], result: ProcessRunResult, ): Error { + if (isWindowsCommandNotFound(result.code, result.stderr)) { + return new Error(`Command not found: ${command}`); + } + const reason = result.timedOut ? "timed out" : `failed (code=${result.code ?? "null"}, signal=${result.signal ?? "null"})`; @@ -70,6 +80,23 @@ 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. + */ +function killChild(child: ChildProcessHandle, signal: NodeJS.Signals = "SIGTERM"): 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(signal); +} + function appendChunkWithinLimit( target: string, currentBytes: number, @@ -127,9 +154,9 @@ export async function runProcess( const timeoutTimer = setTimeout(() => { timedOut = true; - child.kill("SIGTERM"); + killChild(child, "SIGTERM"); forceKillTimer = setTimeout(() => { - child.kill("SIGKILL"); + killChild(child, "SIGKILL"); }, 1_000); }, timeoutMs); @@ -144,7 +171,7 @@ export async function runProcess( }; const fail = (error: Error): void => { - child.kill("SIGTERM"); + killChild(child, "SIGTERM"); finalize(() => { reject(error); }); diff --git a/apps/server/src/provider/Layers/ProviderHealth.test.ts b/apps/server/src/provider/Layers/ProviderHealth.test.ts index 2c190179b03121e88b340f25309213df71a59ef4..58eac64da0a1b8a1f8d4d8c8595b44a965383394 100644 --- a/apps/server/src/provider/Layers/ProviderHealth.test.ts +++ b/apps/server/src/provider/Layers/ProviderHealth.test.ts @@ -1,4 +1,5 @@ -import { assert, it } from "@effect/vitest"; +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"; diff --git a/scripts/dev-runner.test.ts b/scripts/dev-runner.test.ts index ee8094970d1b7ffb8ed8f63890a49fa91d631484..61809a40f3260038c2d6820d2722b7e2af73e5d6 100644 --- a/scripts/dev-runner.test.ts +++ b/scripts/dev-runner.test.ts @@ -1,4 +1,5 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; +import { resolve } from "node:path"; import { assert, describe, it } from "@effect/vitest"; import { Effect } from "effect"; @@ -86,7 +87,7 @@ it.layer(NodeServices.layer)("dev-runner", (it) => { devUrl: new URL("http://localhost:7331"), }); - assert.equal(env.T3CODE_STATE_DIR, "/tmp/override-state"); + assert.equal(env.T3CODE_STATE_DIR, resolve("/tmp/override-state")); assert.equal(env.T3CODE_PORT, "4222"); assert.equal(env.VITE_WS_URL, "ws://localhost:4222"); assert.equal(env.T3CODE_NO_BROWSER, "1");