Fix Windows npm shim app-server launch (#16)

knip.json

@@ -2,5 +2,5 @@
   "entry": ["src/main.ts", "scripts/**/*.mjs", "*.config.{ts,mjs}", "tests/**/*.test.{ts,tsx}"],
   "project": ["src/**/*.{ts,tsx}", "tests/**/*.{ts,tsx}", "scripts/**/*.mjs", "*.config.{ts,mjs}"],
   "ignore": ["src/generated/**"],
-  "ignoreBinaries": ["codex"]
+  "ignoreBinaries": ["codex", "taskkill"]
 }

src/app-server/connection/transport.ts

@@ -3,6 +3,12 @@ import * as readline from "node:readline";
 
 import type { RpcOutboundMessage } from "./rpc-messages";
 
+interface AppServerSpawnSpec {
+  command: string;
+  args: string[];
+  killProcessTreeOnStop: boolean;
+}
+
 export interface AppServerTransport {
   start(): void;
   send(message: RpcOutboundMessage): void;
@@ -17,10 +23,36 @@ export interface AppServerTransportHandlers {
   onError: (error: Error) => void;
 }
 
+export function createAppServerSpawnSpec(
+  codexPath: string,
+  options: { platform?: NodeJS.Platform; comSpec?: string } = {},
+): AppServerSpawnSpec {
+  const platform = options.platform ?? process.platform;
+  if (platform !== "win32" || !isWindowsCommandScript(codexPath)) {
+    return { command: codexPath, args: ["app-server"], killProcessTreeOnStop: false };
+  }
+
+  const comSpec = options.comSpec?.trim() || process.env["ComSpec"]?.trim() || process.env["COMSPEC"]?.trim() || "cmd.exe";
+  return {
+    command: comSpec,
+    args: ["/d", "/c", `${quoteWindowsCmdArgument(codexPath)} app-server`],
+    killProcessTreeOnStop: true,
+  };
+}
+
+function isWindowsCommandScript(path: string): boolean {
+  return /\.(?:bat|cmd)$/i.test(path);
+}
+
+function quoteWindowsCmdArgument(value: string): string {
+  return `"${value.replace(/"/g, '""')}"`;
+}
+
 export class StdioAppServerTransport implements AppServerTransport {
   private process: ChildProcessWithoutNullStreams | null = null;
   private reader: readline.Interface | null = null;
   private stderrBuffer = "";
+  private killProcessTreeOnStop = false;
 
   constructor(
     private readonly codexPath: string,
@@ -33,7 +65,9 @@ export class StdioAppServerTransport implements AppServerTransport {
       throw new Error("Codex app-server is already running.");
     }
 
-    this.process = spawn(this.codexPath, ["app-server"], {
+    const launch = createAppServerSpawnSpec(this.codexPath);
+    this.killProcessTreeOnStop = launch.killProcessTreeOnStop;
+    this.process = spawn(launch.command, launch.args, {
       cwd: this.cwd,
       stdio: ["pipe", "pipe", "pipe"],
     });
@@ -43,6 +77,7 @@ export class StdioAppServerTransport implements AppServerTransport {
       this.reader?.close();
       this.reader = null;
       this.process = null;
+      this.killProcessTreeOnStop = false;
       this.handlers.onError(error instanceof Error ? error : new Error(String(error)));
     });
 
@@ -51,6 +86,7 @@ export class StdioAppServerTransport implements AppServerTransport {
       this.reader?.close();
       this.reader = null;
       this.process = null;
+      this.killProcessTreeOnStop = false;
       this.handlers.onExit(code, signal);
     });
 
@@ -80,10 +116,16 @@ export class StdioAppServerTransport implements AppServerTransport {
     this.flushStderr();
     this.reader?.close();
     this.reader = null;
-    if (this.process && !this.process.killed) {
-      this.process.kill();
+    const child = this.process;
+    if (child && !child.killed) {
+      if (this.killProcessTreeOnStop && typeof child.pid === "number") {
+        killWindowsProcessTree(child.pid);
+      } else {
+        child.kill();
+      }
     }
     this.process = null;
+    this.killProcessTreeOnStop = false;
   }
 
   isRunning(): boolean {
@@ -106,3 +148,11 @@ export class StdioAppServerTransport implements AppServerTransport {
     if (trimmed.length > 0) this.handlers.onLog(trimmed);
   }
 }
+
+function killWindowsProcessTree(pid: number): void {
+  const killer = spawn("taskkill", ["/pid", String(pid), "/t", "/f"], {
+    stdio: "ignore",
+    windowsHide: true,
+  });
+  killer.on("error", () => undefined);
+}

tests/app-server/transport-windows-launch.test.ts

@@ -0,0 +1,83 @@
+import { EventEmitter } from "node:events";
+import { PassThrough } from "node:stream";
+import { afterEach, describe, expect, it, vi } from "vitest";
+
+const spawnMock = vi.fn();
+
+vi.mock("node:child_process", () => ({
+  spawn: spawnMock,
+}));
+
+describe("StdioAppServerTransport Windows launch", () => {
+  const originalPlatform = process.platform;
+
+  afterEach(() => {
+    Object.defineProperty(process, "platform", { value: originalPlatform });
+    vi.resetModules();
+    spawnMock.mockReset();
+    vi.unstubAllEnvs();
+  });
+
+  it("starts Windows command shims through cmd.exe", async () => {
+    Object.defineProperty(process, "platform", { value: "win32" });
+    vi.stubEnv("ComSpec", String.raw`C:\Windows\System32\cmd.exe`);
+    const { StdioAppServerTransport } = await import("../../src/app-server/connection/transport");
+    const codexCmd = String.raw`C:\Users\me\AppData\Roaming\npm\codex.cmd`;
+    const cwd = String.raw`C:\vault`;
+
+    spawnMock.mockReturnValue(fakeChildProcess());
+
+    new StdioAppServerTransport(codexCmd, cwd, {
+      onLine: vi.fn(),
+      onLog: vi.fn(),
+      onExit: vi.fn(),
+      onError: vi.fn(),
+    }).start();
+
+    expect(spawnMock).toHaveBeenCalledWith(String.raw`C:\Windows\System32\cmd.exe`, ["/d", "/c", `"${codexCmd}" app-server`], {
+      cwd,
+      stdio: ["pipe", "pipe", "pipe"],
+    });
+  });
+
+  it("kills the cmd.exe process tree when stopping Windows command shim launches", async () => {
+    Object.defineProperty(process, "platform", { value: "win32" });
+    vi.stubEnv("ComSpec", String.raw`C:\Windows\System32\cmd.exe`);
+    const { StdioAppServerTransport } = await import("../../src/app-server/connection/transport");
+    const codexCmd = String.raw`C:\Users\me\AppData\Roaming\npm\codex.cmd`;
+    const cmdProcess = fakeChildProcess({ pid: 4242 });
+
+    spawnMock.mockReturnValue(cmdProcess);
+
+    const transport = new StdioAppServerTransport(codexCmd, String.raw`C:\vault`, {
+      onLine: vi.fn(),
+      onLog: vi.fn(),
+      onExit: vi.fn(),
+      onError: vi.fn(),
+    });
+    transport.start();
+    transport.stop();
+
+    expect(cmdProcess.kill).not.toHaveBeenCalled();
+    expect(spawnMock).toHaveBeenLastCalledWith("taskkill", ["/pid", "4242", "/t", "/f"], {
+      stdio: "ignore",
+      windowsHide: true,
+    });
+  });
+});
+
+function fakeChildProcess(options: { pid?: number } = {}) {
+  const stdin = Object.assign(new EventEmitter(), {
+    destroyed: false,
+    writable: true,
+    write: vi.fn(),
+  });
+  return Object.assign(new EventEmitter(), {
+    stdout: new PassThrough(),
+    stderr: new PassThrough(),
+    stdin,
+    killed: false,
+    kill: vi.fn(),
+    pid: options.pid,
+  });
+}

tests/app-server/transport.test.ts

@@ -1,12 +1,46 @@
 import { describe, expect, it, vi } from "vitest";
 
-import { type AppServerTransportHandlers, StdioAppServerTransport } from "../../src/app-server/connection/transport";
+import {
+  type AppServerTransportHandlers,
+  createAppServerSpawnSpec,
+  StdioAppServerTransport,
+} from "../../src/app-server/connection/transport";
 
 interface TestableTransport {
   handleStderr(text: string): void;
   flushStderr(): void;
 }
 
+describe("createAppServerSpawnSpec", () => {
+  it("starts regular commands directly", () => {
+    expect(createAppServerSpawnSpec("codex", { platform: "darwin" })).toEqual({
+      command: "codex",
+      args: ["app-server"],
+      killProcessTreeOnStop: false,
+    });
+  });
+
+  it("starts Windows executables directly", () => {
+    const codexExe = String.raw`C:\Program Files\Codex\codex.exe`;
+
+    expect(createAppServerSpawnSpec(codexExe, { platform: "win32" })).toEqual({
+      command: codexExe,
+      args: ["app-server"],
+      killProcessTreeOnStop: false,
+    });
+  });
+
+  it("quotes Windows command shim paths that contain spaces", () => {
+    const codexBat = String.raw`C:\Program Files\nodejs\codex.bat`;
+
+    expect(createAppServerSpawnSpec(codexBat, { platform: "win32", comSpec: " " })).toEqual({
+      command: "cmd.exe",
+      args: ["/d", "/c", `"${codexBat}" app-server`],
+      killProcessTreeOnStop: true,
+    });
+  });
+});
+
 describe("StdioAppServerTransport", () => {
   it("emits complete stderr lines and flushes the final partial line", () => {
     const { onLog, transport } = transportFixture();