authorgravatar for julius0216@outlook.comjulius <julius0216@outlook.com> 2026-03-04 04:54:58-08:00
committergravatar for jmarminge@gmail.comJulius Marminge <jmarminge@gmail.com> 2026-03-03 21:03:16-08:00
log2ae63f4aa2a2af7b0ed605582aef7c9025297e13
tree11c96812181337393372866ad02bdf5b82d2b363
parent3d0233e3b0071ba3cab0d0a8a3585d955979e234

Fix shell: true side effects on Windows (orphaned processes, swallowed ENOENT)

Add tree-kill via taskkill /T /F in processRunner and codexAppServerManager so timeout enforcement and session stop properly terminate the entire process tree instead of just the cmd.exe wrapper. Detect Windows command not found (exit 9009) in normalizeExitError. Pre-validate editor commands with isCommandAvailable in open.ts before spawning. Fix ProviderHealth test import and dev-runner test path assertion for Windows. Made-with: Cursor

5 files changed, 82 insertions(+), 28 deletions(-)

apps/server/src/codexAppServerManager.ts+19-2
...@@ -1,4 +1,4 @@...@@ -1,4 +1,4 @@
1import { type ChildProcessWithoutNullStreams, spawn } from "node:child_process";1import { type ChildProcessWithoutNullStreams, spawn, spawnSync } from "node:child_process";
2import { randomUUID } from "node:crypto";2import { randomUUID } from "node:crypto";
3import { EventEmitter } from "node:events";3import { EventEmitter } from "node:events";
4import readline from "node:readline";4import readline from "node:readline";
...@@ -104,6 +104,23 @@ const RECOVERABLE_THREAD_RESUME_ERROR_SNIPPETS = [...@@ -104,6 +104,23 @@ const RECOVERABLE_THREAD_RESUME_ERROR_SNIPPETS = [
104 "does not exist",104 "does not exist",
105];105];
106106
107/**
108 * On Windows with `shell: true`, `child.kill()` only terminates the `cmd.exe`
109 * wrapper, leaving the actual command running. Use `taskkill /T` to kill the
110 * entire process tree instead.
111 */
112function killChildTree(child: ChildProcessWithoutNullStreams): void {
113 if (process.platform === "win32" && child.pid !== undefined) {
114 try {
115 spawnSync("taskkill", ["/pid", String(child.pid), "/T", "/F"], { stdio: "ignore" });
116 return;
117 } catch {
118 // fallback to direct kill
119 }
120 }
121 child.kill();
122}
123
107export function normalizeCodexModelSlug(124export function normalizeCodexModelSlug(
108 model: string | undefined | null,125 model: string | undefined | null,
109 preferredId?: string,126 preferredId?: string,
...@@ -476,7 +493,7 @@ export class CodexAppServerManager extends EventEmitter<CodexAppServerManagerEve...@@ -476,7 +493,7 @@ export class CodexAppServerManager extends EventEmitter<CodexAppServerManagerEve
476 context.output.close();493 context.output.close();
477494
478 if (!context.child.killed) {495 if (!context.child.killed) {
479 context.child.kill();496 killChildTree(context.child);
480 }497 }
481498
482 this.updateSession(context, {499 this.updateSession(context, {
apps/server/src/open.ts+28-20
...@@ -224,29 +224,37 @@ export const resolveEditorLaunch = Effect.fnUntraced(function* (...@@ -224,29 +224,37 @@ export const resolveEditorLaunch = Effect.fnUntraced(function* (
224});224});
225225
226export const launchDetached = (launch: EditorLaunch) =>226export const launchDetached = (launch: EditorLaunch) =>
227 Effect.callback<void, OpenError>((resume) => {227 Effect.gen(function* () {
228 let child;228 if (!isCommandAvailable(launch.command)) {
229 try {229 return yield* new OpenError({ message: `Editor command not found: ${launch.command}` });
230 child = spawn(launch.command, [...launch.args], {
231 detached: true,
232 stdio: "ignore",
233 shell: process.platform === "win32",
234 });
235 } catch (error) {
236 return resume(
237 Effect.fail(new OpenError({ message: "failed to spawn detached process", cause: error })),
238 );
239 }230 }
240231
241 const handleSpawn = () => {232 yield* Effect.callback<void, OpenError>((resume) => {
242 child.unref();233 let child;
243 resume(Effect.void);234 try {
244 };235 child = spawn(launch.command, [...launch.args], {
236 detached: true,
237 stdio: "ignore",
238 shell: process.platform === "win32",
239 });
240 } catch (error) {
241 return resume(
242 Effect.fail(
243 new OpenError({ message: "failed to spawn detached process", cause: error }),
244 ),
245 );
246 }
245247
246 child.once("spawn", handleSpawn);248 const handleSpawn = () => {
247 child.once("error", (cause) =>249 child.unref();
248 resume(Effect.fail(new OpenError({ message: "failed to spawn detached process", cause }))),250 resume(Effect.void);
249 );251 };
252
253 child.once("spawn", handleSpawn);
254 child.once("error", (cause) =>
255 resume(Effect.fail(new OpenError({ message: "failed to spawn detached process", cause }))),
256 );
257 });
250 });258 });
251259
252const make = Effect.gen(function* () {260const make = Effect.gen(function* () {
apps/server/src/processRunner.ts+31-4
...@@ -1,4 +1,4 @@...@@ -1,4 +1,4 @@
1import { spawn } from "node:child_process";1import { type ChildProcess as ChildProcessHandle, spawn, spawnSync } from "node:child_process";
22
3export interface ProcessRunOptions {3export interface ProcessRunOptions {
4 cwd?: string | undefined;4 cwd?: string | undefined;
...@@ -37,11 +37,21 @@ function normalizeSpawnError(command: string, args: readonly string[], error: un...@@ -37,11 +37,21 @@ function normalizeSpawnError(command: string, args: readonly string[], error: un
37 return new Error(`Failed to run ${commandLabel(command, args)}: ${error.message}`);37 return new Error(`Failed to run ${commandLabel(command, args)}: ${error.message}`);
38}38}
3939
40function isWindowsCommandNotFound(code: number | null, stderr: string): boolean {
41 if (process.platform !== "win32") return false;
42 if (code === 9009) return true;
43 return /is not recognized as an internal or external command/i.test(stderr);
44}
45
40function normalizeExitError(46function normalizeExitError(
41 command: string,47 command: string,
42 args: readonly string[],48 args: readonly string[],
43 result: ProcessRunResult,49 result: ProcessRunResult,
44): Error {50): Error {
51 if (isWindowsCommandNotFound(result.code, result.stderr)) {
52 return new Error(`Command not found: ${command}`);
53 }
54
45 const reason = result.timedOut55 const reason = result.timedOut
46 ? "timed out"56 ? "timed out"
47 : `failed (code=${result.code ?? "null"}, signal=${result.signal ?? "null"})`;57 : `failed (code=${result.code ?? "null"}, signal=${result.signal ?? "null"})`;
...@@ -70,6 +80,23 @@ function normalizeBufferError(...@@ -70,6 +80,23 @@ function normalizeBufferError(
7080
71const DEFAULT_MAX_BUFFER_BYTES = 8 * 1024 * 1024;81const DEFAULT_MAX_BUFFER_BYTES = 8 * 1024 * 1024;
7282
83/**
84 * On Windows with `shell: true`, `child.kill()` only terminates the `cmd.exe`
85 * wrapper, leaving the actual command running. Use `taskkill /T` to kill the
86 * entire process tree instead.
87 */
88function killChild(child: ChildProcessHandle, signal: NodeJS.Signals = "SIGTERM"): void {
89 if (process.platform === "win32" && child.pid !== undefined) {
90 try {
91 spawnSync("taskkill", ["/pid", String(child.pid), "/T", "/F"], { stdio: "ignore" });
92 return;
93 } catch {
94 // fallback to direct kill
95 }
96 }
97 child.kill(signal);
98}
99
73function appendChunkWithinLimit(100function appendChunkWithinLimit(
74 target: string,101 target: string,
75 currentBytes: number,102 currentBytes: number,
...@@ -127,9 +154,9 @@ export async function runProcess(...@@ -127,9 +154,9 @@ export async function runProcess(
127154
128 const timeoutTimer = setTimeout(() => {155 const timeoutTimer = setTimeout(() => {
129 timedOut = true;156 timedOut = true;
130 child.kill("SIGTERM");157 killChild(child, "SIGTERM");
131 forceKillTimer = setTimeout(() => {158 forceKillTimer = setTimeout(() => {
132 child.kill("SIGKILL");159 killChild(child, "SIGKILL");
133 }, 1_000);160 }, 1_000);
134 }, timeoutMs);161 }, timeoutMs);
135162
...@@ -144,7 +171,7 @@ export async function runProcess(...@@ -144,7 +171,7 @@ export async function runProcess(
144 };171 };
145172
146 const fail = (error: Error): void => {173 const fail = (error: Error): void => {
147 child.kill("SIGTERM");174 killChild(child, "SIGTERM");
148 finalize(() => {175 finalize(() => {
149 reject(error);176 reject(error);
150 });177 });
apps/server/src/provider/Layers/ProviderHealth.test.ts+2-1
...@@ -1,4 +1,5 @@...@@ -1,4 +1,5 @@
1import { assert, it } from "@effect/vitest";1import assert from "node:assert/strict";
2import { it } from "@effect/vitest";
2import { Effect, Layer, Sink, Stream } from "effect";3import { Effect, Layer, Sink, Stream } from "effect";
3import * as PlatformError from "effect/PlatformError";4import * as PlatformError from "effect/PlatformError";
4import { ChildProcessSpawner } from "effect/unstable/process";5import { ChildProcessSpawner } from "effect/unstable/process";
scripts/dev-runner.test.ts+2-1
...@@ -1,4 +1,5 @@...@@ -1,4 +1,5 @@
1import * as NodeServices from "@effect/platform-node/NodeServices";1import * as NodeServices from "@effect/platform-node/NodeServices";
2import { resolve } from "node:path";
2import { assert, describe, it } from "@effect/vitest";3import { assert, describe, it } from "@effect/vitest";
3import { Effect } from "effect";4import { Effect } from "effect";
45
...@@ -86,7 +87,7 @@ it.layer(NodeServices.layer)("dev-runner", (it) => {...@@ -86,7 +87,7 @@ it.layer(NodeServices.layer)("dev-runner", (it) => {
86 devUrl: new URL("http://localhost:7331"),87 devUrl: new URL("http://localhost:7331"),
87 });88 });
8889
89 assert.equal(env.T3CODE_STATE_DIR, "/tmp/override-state");90 assert.equal(env.T3CODE_STATE_DIR, resolve("/tmp/override-state"));
90 assert.equal(env.T3CODE_PORT, "4222");91 assert.equal(env.T3CODE_PORT, "4222");
91 assert.equal(env.VITE_WS_URL, "ws://localhost:4222");92 assert.equal(env.VITE_WS_URL, "ws://localhost:4222");
92 assert.equal(env.T3CODE_NO_BROWSER, "1");93 assert.equal(env.T3CODE_NO_BROWSER, "1");