From 5bdd5fd176376b3035491c0e54f056fed5c0dd25 Mon Sep 17 00:00:00 2001 From: ZETA <126369952+zeta987@users.noreply.github.com> Date: Tue, 31 Mar 2026 16:03:08 +0800 Subject: [PATCH 1/2] fix: add shell and windowsHide options for Windows spawn in app-server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows, spawn("codex", ["app-server"]) fails with ENOENT because Node.js cannot resolve .cmd shims without shell: true. This adds platform-gated shell and windowsHide options to the app-server spawn call, and uses terminateProcessTree for proper process tree cleanup since shell: true wraps the child in cmd.exe. Without terminateProcessTree, plain SIGTERM only kills cmd.exe and leaves the actual codex node process orphaned — verified with 274+ zombie node.exe processes accumulating on Windows. Fixes #32 Fixes #46 Co-Authored-By: Claude Opus 4.6 (1M context) --- plugins/codex/scripts/lib/app-server.mjs | 19 +++++++++++++++++-- plugins/codex/scripts/lib/process.mjs | 3 ++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/plugins/codex/scripts/lib/app-server.mjs b/plugins/codex/scripts/lib/app-server.mjs index abf3d0c..97c5342 100644 --- a/plugins/codex/scripts/lib/app-server.mjs +++ b/plugins/codex/scripts/lib/app-server.mjs @@ -14,6 +14,7 @@ import { spawn } from "node:child_process"; import readline from "node:readline"; import { parseBrokerEndpoint } from "./broker-endpoint.mjs"; import { ensureBrokerSession } from "./broker-lifecycle.mjs"; +import { terminateProcessTree } from "./process.mjs"; const PLUGIN_MANIFEST_URL = new URL("../../.claude-plugin/plugin.json", import.meta.url); const PLUGIN_MANIFEST = JSON.parse(fs.readFileSync(PLUGIN_MANIFEST_URL, "utf8")); @@ -188,7 +189,9 @@ class SpawnedCodexAppServerClient extends AppServerClientBase { this.proc = spawn("codex", ["app-server"], { cwd: this.cwd, env: this.options.env, - stdio: ["pipe", "pipe", "pipe"] + stdio: ["pipe", "pipe", "pipe"], + shell: process.platform === "win32", + windowsHide: true }); this.proc.stdout.setEncoding("utf8"); @@ -238,7 +241,19 @@ class SpawnedCodexAppServerClient extends AppServerClientBase { this.proc.stdin.end(); setTimeout(() => { if (this.proc && !this.proc.killed) { - this.proc.kill("SIGTERM"); + // On Windows with shell: true, the direct child is cmd.exe. + // Use terminateProcessTree to kill the entire tree including + // the grandchild node process. + if (process.platform === "win32") { + try { + terminateProcessTree(this.proc.pid); + } catch { + // Best-effort cleanup inside an unref'd timer — swallow errors + // to avoid crashing the host process during shutdown. + } + } else { + this.proc.kill("SIGTERM"); + } } }, 50).unref?.(); } diff --git a/plugins/codex/scripts/lib/process.mjs b/plugins/codex/scripts/lib/process.mjs index b145ad8..0948dbd 100644 --- a/plugins/codex/scripts/lib/process.mjs +++ b/plugins/codex/scripts/lib/process.mjs @@ -8,7 +8,8 @@ export function runCommand(command, args = [], options = {}) { encoding: "utf8", input: options.input, stdio: options.stdio ?? "pipe", - shell: process.platform === "win32" + shell: process.platform === "win32", + windowsHide: true }); return { From 94d3db8f4f249db786e9f535abf1466e5b469ddc Mon Sep 17 00:00:00 2001 From: ZETA <126369952+zeta987@users.noreply.github.com> Date: Tue, 31 Mar 2026 18:58:35 +0800 Subject: [PATCH 2/2] fix: guard terminateProcessTree against PID reuse after process exit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ChildProcess.killed only reflects whether .kill() was called by this process — it stays false when the child exits on its own. On Windows, where PIDs are recycled quickly, the 50 ms timer could fire after cmd.exe has exited and its PID has been reassigned, causing taskkill to terminate an unrelated process. Add an exitCode === null check so the tree-kill path is skipped once the child has already exited. Co-Authored-By: Claude Opus 4.6 (1M context) --- plugins/codex/scripts/lib/app-server.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/codex/scripts/lib/app-server.mjs b/plugins/codex/scripts/lib/app-server.mjs index 97c5342..fec105c 100644 --- a/plugins/codex/scripts/lib/app-server.mjs +++ b/plugins/codex/scripts/lib/app-server.mjs @@ -240,7 +240,7 @@ class SpawnedCodexAppServerClient extends AppServerClientBase { if (this.proc && !this.proc.killed) { this.proc.stdin.end(); setTimeout(() => { - if (this.proc && !this.proc.killed) { + if (this.proc && !this.proc.killed && this.proc.exitCode === null) { // On Windows with shell: true, the direct child is cmd.exe. // Use terminateProcessTree to kill the entire tree including // the grandchild node process.