fix(server): clean up OpenCode process trees

Spawn the shared OpenCode server in its own POSIX process group and terminate provider child process trees with graceful timeout escalation before forcing SIGKILL. Reuse the same helper for Codex app-server cleanup so process-tree behavior is covered consistently across providers.

Add cross-platform unit coverage for macOS/Linux process groups, Windows taskkill cleanup, and timeout escalation. Add the process-tree test to the Windows-critical CI allowlist; Linux full server tests already include it.

Refs #227

.github/workflows/ci.yml

@@ -124,6 +124,7 @@ jobs:
           src/utils/spawn.launch-regression.test.ts
           src/utils/spawn.percent-escape.test.ts
           src/utils/spawn.test.ts
+          src/utils/process-tree.test.ts
           src/utils/run-git-command.test.ts
           src/utils/checkout-git-rev-parse.test.ts
           src/terminal/worker-terminal-manager.test.ts

packages/server/src/server/agent/providers/codex-app-server-agent.ts

@@ -50,6 +50,7 @@ import {
   type ProviderRuntimeSettings,
 } from "../provider-launch-config.js";
 import { findExecutable, isCommandAvailable } from "../../../utils/executable.js";
+import { terminateProcessTree } from "../../../utils/process-tree.js";
 import { spawnProcess } from "../../../utils/spawn.js";
 import { extractCodexTerminalSessionId, nonEmptyString } from "./tool-call-mapper-utils.js";
 import { buildCodexFeatures, codexModelSupportsFastMode } from "./codex-feature-definitions.js";
@@ -602,17 +603,12 @@ class CodexAppServerClient {
   private nextId = 1;
   private disposed = false;
   private stderrBuffer = "";
-  private readonly exitPromise: Promise<void>;
-  private resolveExitPromise: (() => void) | null = null;
 
   constructor(
     private readonly child: ChildProcessWithoutNullStreams,
     private readonly logger: Logger,
   ) {
     this.rl = readline.createInterface({ input: child.stdout });
-    this.exitPromise = new Promise<void>((resolve) => {
-      this.resolveExitPromise = resolve;
-    });
     this.rl.on("line", (line) => this.handleLine(line));
 
     child.stderr.on("data", (chunk) => {
@@ -630,8 +626,6 @@ class CodexAppServerClient {
       }
       this.pending.clear();
       this.disposed = true;
-      this.resolveExitPromise?.();
-      this.resolveExitPromise = null;
     });
 
     child.on("exit", (code, signal) => {
@@ -646,8 +640,6 @@ class CodexAppServerClient {
       }
       this.pending.clear();
       this.disposed = true;
-      this.resolveExitPromise?.();
-      this.resolveExitPromise = null;
     });
   }
 
@@ -704,39 +696,21 @@ class CodexAppServerClient {
     } catch {
       // ignore
     }
-    signalChildProcessTree(this.child, "SIGTERM");
-    if (await this.waitForExit(APP_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT_MS)) {
-      return;
-    }
-
-    this.logger.warn(
-      { timeoutMs: APP_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT_MS },
-      "Codex app-server did not exit after SIGTERM; sending SIGKILL",
-    );
-    signalChildProcessTree(this.child, "SIGKILL");
-    if (await this.waitForExit(APP_SERVER_FORCE_SHUTDOWN_TIMEOUT_MS)) {
-      return;
-    }
-
-    this.logger.warn(
-      { timeoutMs: APP_SERVER_FORCE_SHUTDOWN_TIMEOUT_MS },
-      "Codex app-server did not report exit after SIGKILL",
-    );
-  }
-
-  private async waitForExit(timeoutMs: number): Promise<boolean> {
-    let timer: NodeJS.Timeout | null = null;
-    try {
-      return await Promise.race([
-        this.exitPromise.then(() => true),
-        new Promise<boolean>((resolve) => {
-          timer = setTimeout(() => resolve(false), timeoutMs);
-        }),
-      ]);
-    } finally {
-      if (timer) {
-        clearTimeout(timer);
-      }
+    const result = await terminateProcessTree(this.child, {
+      gracefulTimeoutMs: APP_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT_MS,
+      forceTimeoutMs: APP_SERVER_FORCE_SHUTDOWN_TIMEOUT_MS,
+      onForceSignal: () => {
+        this.logger.warn(
+          { timeoutMs: APP_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT_MS },
+          "Codex app-server did not exit after SIGTERM; sending SIGKILL",
+        );
+      },
+    });
+    if (result === "kill-timeout") {
+      this.logger.warn(
+        { timeoutMs: APP_SERVER_FORCE_SHUTDOWN_TIMEOUT_MS },
+        "Codex app-server did not report exit after SIGKILL",
+      );
     }
   }
 
@@ -789,30 +763,6 @@ class CodexAppServerClient {
   }
 }
 
-function signalChildProcessTree(
-  child: ChildProcessWithoutNullStreams,
-  signal: NodeJS.Signals,
-): void {
-  if (child.exitCode !== null || child.signalCode !== null) {
-    return;
-  }
-
-  if (process.platform !== "win32" && typeof child.pid === "number" && child.pid > 0) {
-    try {
-      process.kill(-child.pid, signal);
-      return;
-    } catch {
-      // Fall back to the direct child when no separate process group exists.
-    }
-  }
-
-  try {
-    child.kill(signal);
-  } catch {
-    // ignore
-  }
-}
-
 function toAgentUsage(tokenUsage: unknown): AgentUsage | undefined {
   if (!tokenUsage || typeof tokenUsage !== "object") return undefined;
   const usage = tokenUsage as {

packages/server/src/server/agent/providers/opencode-agent.ts

@@ -46,6 +46,7 @@ import {
   type ProviderRuntimeSettings,
 } from "../provider-launch-config.js";
 import { findExecutable, isCommandAvailable } from "../../../utils/executable.js";
+import { terminateProcessTree } from "../../../utils/process-tree.js";
 import { withTimeout } from "../../../utils/promise-timeout.js";
 import { spawnProcess } from "../../../utils/spawn.js";
 import { buildToolCallDisplayModel } from "../../../shared/tool-call-display.js";
@@ -109,6 +110,8 @@ type OpenCodeMcpConfig =
 
 const MCP_ALREADY_PRESENT_ERROR_TOKENS = ["already", "exists", "connected"] as const;
 const OPENCODE_PROVIDER_LIST_TIMEOUT_MS = 30_000;
+const OPENCODE_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT_MS = 5_000;
+const OPENCODE_SERVER_FORCE_SHUTDOWN_TIMEOUT_MS = 1_000;
 const OPENCODE_HANDLED_BUILTIN_SLASH_COMMANDS: AgentSlashCommand[] = [
   { name: "compact", description: "Compact the current session", argumentHint: "" },
   { name: "summarize", description: "Compact the current session", argumentHint: "" },
@@ -888,6 +891,7 @@ export class OpenCodeServerManager {
         launchPrefix.command,
         [...launchPrefix.args, "serve", "--port", String(port)],
         {
+          detached: process.platform !== "win32",
           stdio: ["ignore", "pipe", "pipe"],
           ...createProviderEnvSpec({ runtimeSettings: this.runtimeSettings }),
         },
@@ -990,24 +994,22 @@ export class OpenCodeServerManager {
     if (server.process.killed) {
       return;
     }
-    await new Promise<void>((resolve) => {
-      let pendingResolve: (() => void) | null = resolve;
-      const settle = () => {
-        if (!pendingResolve) return;
-        const fn = pendingResolve;
-        pendingResolve = null;
-        fn();
-      };
-      const timeout = setTimeout(() => {
-        server.process.kill("SIGKILL");
-        settle();
-      }, 5000);
-      server.process.on("exit", () => {
-        clearTimeout(timeout);
-        settle();
-      });
-      server.process.kill("SIGTERM");
+    const result = await terminateProcessTree(server.process, {
+      gracefulTimeoutMs: OPENCODE_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT_MS,
+      forceTimeoutMs: OPENCODE_SERVER_FORCE_SHUTDOWN_TIMEOUT_MS,
+      onForceSignal: () => {
+        this.logger.warn(
+          { timeoutMs: OPENCODE_SERVER_GRACEFUL_SHUTDOWN_TIMEOUT_MS },
+          "OpenCode server did not exit after SIGTERM; sending SIGKILL",
+        );
+      },
     });
+    if (result === "kill-timeout") {
+      this.logger.warn(
+        { timeoutMs: OPENCODE_SERVER_FORCE_SHUTDOWN_TIMEOUT_MS },
+        "OpenCode server did not report exit after SIGKILL",
+      );
+    }
   }
 }
 

packages/server/src/utils/process-tree.test.ts

@@ -0,0 +1,259 @@
+import { execFile, spawn } from "node:child_process";
+import { EventEmitter } from "node:events";
+import { promisify } from "node:util";
+import { describe, expect, test, vi } from "vitest";
+
+import { signalProcessTree, terminateProcessTree, type ProcessTreeTarget } from "./process-tree.js";
+
+const execFileAsync = promisify(execFile);
+
+describe("signalProcessTree", () => {
+  test.each(["darwin", "linux"] as const)("signals the %s process group", (platform) => {
+    const child = createChild({ pid: 1234 });
+    const kill = vi.fn(() => true);
+
+    signalProcessTree(child, "SIGTERM", { platform, kill });
+
+    expect(kill).toHaveBeenCalledWith(-1234, "SIGTERM");
+    expect(child.kill).not.toHaveBeenCalled();
+  });
+
+  test("falls back to direct child signaling when POSIX process group signaling fails", () => {
+    const child = createChild({ pid: 1234 });
+
+    signalProcessTree(child, "SIGTERM", {
+      platform: "darwin",
+      kill: vi.fn(() => {
+        throw new Error("no process group");
+      }),
+    });
+
+    expect(child.kill).toHaveBeenCalledWith("SIGTERM");
+  });
+
+  test("uses taskkill for Windows process trees", () => {
+    const child = createChild({ pid: 1234 });
+    const execFileMock = vi.fn((_file, _args, callback) => {
+      callback(null);
+    });
+
+    signalProcessTree(child, "SIGTERM", { platform: "win32", execFile: execFileMock });
+
+    expect(execFileMock).toHaveBeenCalledWith(
+      "taskkill",
+      ["/pid", "1234", "/T"],
+      expect.any(Function),
+    );
+    expect(child.kill).not.toHaveBeenCalled();
+  });
+
+  test("forces Windows process tree cleanup for SIGKILL", () => {
+    const child = createChild({ pid: 1234 });
+    const execFileMock = vi.fn((_file, _args, callback) => {
+      callback(null);
+    });
+
+    signalProcessTree(child, "SIGKILL", { platform: "win32", execFile: execFileMock });
+
+    expect(execFileMock).toHaveBeenCalledWith(
+      "taskkill",
+      ["/pid", "1234", "/T", "/F"],
+      expect.any(Function),
+    );
+  });
+
+  test("falls back to direct child signaling when taskkill cannot run", () => {
+    const child = createChild({ pid: 1234 });
+    const execFileMock = vi.fn((_file, _args, callback) => {
+      callback(Object.assign(new Error("taskkill failed"), { code: "ENOENT" }));
+    });
+
+    signalProcessTree(child, "SIGTERM", { platform: "win32", execFile: execFileMock });
+
+    expect(child.kill).toHaveBeenCalledWith("SIGTERM");
+  });
+
+  test("does not signal an exited child", () => {
+    const child = createChild({ pid: 1234, exitCode: 0 });
+    const kill = vi.fn(() => true);
+
+    signalProcessTree(child, "SIGTERM", { platform: "darwin", kill });
+
+    expect(kill).not.toHaveBeenCalled();
+    expect(child.kill).not.toHaveBeenCalled();
+  });
+
+  test("terminateProcessTree returns after graceful exit", async () => {
+    const child = createEventedChild({ pid: 1234 });
+    const kill = vi.fn(() => true);
+
+    const resultPromise = terminateProcessTree(child, {
+      platform: "darwin",
+      gracefulTimeoutMs: 100,
+      forceTimeoutMs: 100,
+      kill,
+    });
+    child.emit("exit");
+
+    await expect(resultPromise).resolves.toBe("terminated");
+    expect(kill).toHaveBeenCalledWith(-1234, "SIGTERM");
+    expect(kill).not.toHaveBeenCalledWith(-1234, "SIGKILL");
+  });
+
+  test("terminateProcessTree sends SIGKILL after graceful timeout", async () => {
+    vi.useFakeTimers();
+    try {
+      const child = createEventedChild({ pid: 1234 });
+      const kill = vi.fn(() => true);
+      const onForceSignal = vi.fn();
+
+      const resultPromise = terminateProcessTree(child, {
+        platform: "darwin",
+        gracefulTimeoutMs: 100,
+        forceTimeoutMs: 100,
+        kill,
+        onForceSignal,
+      });
+
+      await vi.advanceTimersByTimeAsync(100);
+      expect(onForceSignal).toHaveBeenCalledTimes(1);
+      expect(kill).toHaveBeenNthCalledWith(1, -1234, "SIGTERM");
+      expect(kill).toHaveBeenNthCalledWith(2, -1234, "SIGKILL");
+
+      child.emit("exit");
+      await expect(resultPromise).resolves.toBe("killed");
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  test("terminateProcessTree reports when forced cleanup does not exit", async () => {
+    vi.useFakeTimers();
+    try {
+      const child = createEventedChild({ pid: 1234 });
+      const kill = vi.fn(() => true);
+
+      const resultPromise = terminateProcessTree(child, {
+        platform: "darwin",
+        gracefulTimeoutMs: 100,
+        forceTimeoutMs: 100,
+        kill,
+      });
+
+      await vi.advanceTimersByTimeAsync(200);
+
+      await expect(resultPromise).resolves.toBe("kill-timeout");
+      expect(kill).toHaveBeenNthCalledWith(1, -1234, "SIGTERM");
+      expect(kill).toHaveBeenNthCalledWith(2, -1234, "SIGKILL");
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
+  test.skipIf(process.platform === "win32")("kills a detached POSIX process group", async () => {
+    const child = spawn("/bin/sh", ["-c", "sleep 60 & wait"], {
+      detached: true,
+      stdio: "ignore",
+    });
+    const pid = child.pid;
+    if (typeof pid !== "number") {
+      throw new Error("spawned shell did not expose a pid");
+    }
+
+    let childPids: number[] = [];
+    try {
+      childPids = await waitForChildren(pid);
+      await expect(
+        terminateProcessTree(child, {
+          gracefulTimeoutMs: 1000,
+          forceTimeoutMs: 1000,
+        }),
+      ).resolves.toMatch(/^(terminated|killed)$/);
+      await waitForNoLivePids(childPids);
+    } finally {
+      cleanupPidGroup(pid);
+      for (const childPid of childPids) {
+        cleanupPid(childPid);
+      }
+    }
+  });
+});
+
+function createChild(options: { pid?: number; exitCode?: number | null } = {}): ProcessTreeTarget {
+  return {
+    pid: options.pid,
+    exitCode: options.exitCode ?? null,
+    signalCode: null,
+    kill: vi.fn(() => true),
+  };
+}
+
+function createEventedChild(options: { pid?: number }): ProcessTreeTarget & EventEmitter {
+  const child = new EventEmitter() as ProcessTreeTarget & EventEmitter;
+  child.pid = options.pid;
+  child.exitCode = null;
+  child.signalCode = null;
+  child.kill = vi.fn(() => true);
+  return child;
+}
+
+async function waitForChildren(parentPid: number): Promise<number[]> {
+  return waitFor(async () => {
+    const { stdout } = await execFileAsync("pgrep", ["-P", String(parentPid)]).catch(() => ({
+      stdout: "",
+    }));
+    const pids = stdout
+      .trim()
+      .split(/\s+/)
+      .filter(Boolean)
+      .map((value) => Number.parseInt(value, 10))
+      .filter((value) => Number.isInteger(value) && value > 0);
+    return pids.length > 0 ? pids : null;
+  });
+}
+
+async function waitForNoLivePids(pids: number[]): Promise<void> {
+  await waitFor(() => (pids.every((pid) => !isPidAlive(pid)) ? true : null));
+}
+
+async function waitFor<T>(probe: () => T | null | Promise<T | null>): Promise<T> {
+  const deadline = Date.now() + 3000;
+  let lastError: unknown;
+  while (Date.now() < deadline) {
+    try {
+      const value = await probe();
+      if (value !== null) {
+        return value;
+      }
+    } catch (error) {
+      lastError = error;
+    }
+    await new Promise((resolve) => setTimeout(resolve, 50));
+  }
+  throw lastError instanceof Error ? lastError : new Error("timed out waiting for condition");
+}
+
+function isPidAlive(pid: number): boolean {
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+function cleanupPidGroup(pid: number): void {
+  try {
+    process.kill(-pid, "SIGKILL");
+  } catch {
+    // ignore cleanup races
+  }
+}
+
+function cleanupPid(pid: number): void {
+  try {
+    process.kill(pid, "SIGKILL");
+  } catch {
+    // ignore cleanup races
+  }
+}

packages/server/src/utils/process-tree.ts

@@ -0,0 +1,155 @@
+import { execFile } from "node:child_process";
+
+export interface ProcessTreeTarget {
+  pid?: number;
+  killed?: boolean;
+  exitCode?: number | null;
+  signalCode?: NodeJS.Signals | null;
+  kill(signal?: NodeJS.Signals | number): boolean;
+  once?(event: "exit", listener: () => void): unknown;
+}
+
+interface SignalProcessTreeOptions {
+  platform?: NodeJS.Platform;
+  kill?: (pid: number, signal?: NodeJS.Signals | number) => boolean;
+  execFile?: (
+    file: string,
+    args: string[],
+    callback: (error: NodeJS.ErrnoException | null) => void,
+  ) => unknown;
+}
+
+interface TerminateProcessTreeOptions extends SignalProcessTreeOptions {
+  gracefulSignal?: NodeJS.Signals;
+  forceSignal?: NodeJS.Signals;
+  gracefulTimeoutMs: number;
+  forceTimeoutMs?: number;
+  onForceSignal?: () => void;
+}
+
+export type TerminateProcessTreeResult =
+  | "already-exited"
+  | "terminated"
+  | "killed"
+  | "kill-timeout";
+
+export async function terminateProcessTree(
+  child: ProcessTreeTarget,
+  options: TerminateProcessTreeOptions,
+): Promise<TerminateProcessTreeResult> {
+  if (isProcessExited(child)) {
+    return "already-exited";
+  }
+
+  const exitPromise = waitForProcessExit(child);
+  signalProcessTree(child, options.gracefulSignal ?? "SIGTERM", options);
+  if (await waitForExitOrTimeout(exitPromise, options.gracefulTimeoutMs)) {
+    return "terminated";
+  }
+
+  options.onForceSignal?.();
+  signalProcessTree(child, options.forceSignal ?? "SIGKILL", options);
+  if (options.forceTimeoutMs === undefined) {
+    return "killed";
+  }
+  return (await waitForExitOrTimeout(exitPromise, options.forceTimeoutMs))
+    ? "killed"
+    : "kill-timeout";
+}
+
+export function signalProcessTree(
+  child: ProcessTreeTarget,
+  signal: NodeJS.Signals,
+  options: SignalProcessTreeOptions = {},
+): void {
+  if (isProcessExited(child)) {
+    return;
+  }
+
+  const pid = child.pid;
+  if (typeof pid === "number" && pid > 0) {
+    const platform = options.platform ?? process.platform;
+    if (platform === "win32") {
+      signalWindowsProcessTree(child, pid, signal, options);
+      return;
+    }
+
+    try {
+      (options.kill ?? process.kill)(-pid, signal);
+      return;
+    } catch {
+      // Fall back to the direct child when no separate process group exists.
+    }
+  }
+
+  signalDirectChild(child, signal);
+}
+
+function signalWindowsProcessTree(
+  child: ProcessTreeTarget,
+  pid: number,
+  signal: NodeJS.Signals,
+  options: SignalProcessTreeOptions,
+): void {
+  const args = ["/pid", String(pid), "/T"];
+  if (signal === "SIGKILL") {
+    args.push("/F");
+  }
+
+  try {
+    (options.execFile ?? execFile)("taskkill", args, (error) => {
+      if (error) {
+        signalDirectChild(child, signal);
+      }
+    });
+  } catch {
+    signalDirectChild(child, signal);
+  }
+}
+
+function signalDirectChild(child: ProcessTreeTarget, signal: NodeJS.Signals): void {
+  try {
+    child.kill(signal);
+  } catch {
+    // Ignore cleanup races.
+  }
+}
+
+function isProcessExited(child: ProcessTreeTarget): boolean {
+  return (
+    (child.exitCode !== null && child.exitCode !== undefined) ||
+    (child.signalCode !== null && child.signalCode !== undefined)
+  );
+}
+
+function waitForProcessExit(child: ProcessTreeTarget): Promise<void> {
+  if (isProcessExited(child)) {
+    return Promise.resolve();
+  }
+  if (!child.once) {
+    return new Promise(() => undefined);
+  }
+
+  return new Promise((resolve) => {
+    child.once?.("exit", resolve);
+  });
+}
+
+async function waitForExitOrTimeout(
+  exitPromise: Promise<void>,
+  timeoutMs: number,
+): Promise<boolean> {
+  let timer: NodeJS.Timeout | null = null;
+  try {
+    return await Promise.race([
+      exitPromise.then(() => true),
+      new Promise<boolean>((resolve) => {
+        timer = setTimeout(() => resolve(false), timeoutMs);
+      }),
+    ]);
+  } finally {
+    if (timer) {
+      clearTimeout(timer);
+    }
+  }
+}