fix(server): process-group kills never reach every process the user owns (#14461)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

Author: juliusmarminge

apps/server/src/orchestration-v2/Adapters/PiRpc.ts

@@ -30,6 +30,8 @@ import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";
 import { HostProcessPlatform } from "@t3tools/shared/hostProcess";
 import { resolveSpawnCommand } from "@t3tools/shared/shell";
 
+import { signalProcessGroup } from "../../process/processGroup.ts";
+
 export class PiRpcError extends Schema.TaggedError<PiRpcError>()("PiRpcError", {
   operation: Schema.String,
   detail: Schema.optional(Schema.String),
@@ -238,7 +240,7 @@ export const makePiRpcConnection = Effect.fnUntraced(function* (options: PiRpcSp
       if (platform === "win32") {
         process.kill(Number(child.pid), signal);
       } else {
-        process.kill(-Number(child.pid), signal);
+        signalProcessGroup(Number(child.pid), signal);
       }
       return true;
     } catch {
@@ -250,7 +252,8 @@ export const makePiRpcConnection = Effect.fnUntraced(function* (options: PiRpcSp
   const hasExited = (): boolean => {
     if (childExited) return true;
     try {
-      process.kill(platform === "win32" ? Number(child.pid) : -Number(child.pid), 0);
+      if (platform === "win32") process.kill(Number(child.pid), 0);
+      else signalProcessGroup(Number(child.pid), 0);
       return false;
     } catch {
       return true;

apps/server/src/process/processGroup.test.ts

@@ -0,0 +1,45 @@
+// @effect-diagnostics nodeBuiltinImport:off
+import * as NodeChildProcess from "node:child_process";
+
+import { describe, expect, it } from "@effect/vitest";
+import { HostProcessPlatform } from "@t3tools/shared/hostProcess";
+
+import { signalProcessGroup } from "./processGroup.ts";
+
+const errorCode = (run: () => void) => {
+  try {
+    run();
+    return undefined;
+  } catch (cause) {
+    return (cause as NodeJS.ErrnoException).code;
+  }
+};
+
+describe.skipIf(HostProcessPlatform.defaultValue() === "win32")("signalProcessGroup", () => {
+  // Signal 0 only probes, so this is safe to run unguarded: `kill(-1, 0)` and
+  // `kill(0, 0)` succeed, which is how a fake spawner's pid 1 became
+  // `kill(-1, SIGKILL)` and took down every process the user owned.
+  it("never reaches this server's group or every process the user owns", () => {
+    for (const pid of [1, 0, -1, Number.NaN, 1.5]) {
+      expect(
+        errorCode(() => signalProcessGroup(pid, 0)),
+        `pid ${pid}`,
+      ).toBe("ESRCH");
+    }
+  });
+
+  it("signals a spawned process group", async () => {
+    const child = NodeChildProcess.spawn("/bin/sh", ["-c", "sleep 600 & wait"], {
+      detached: true,
+      stdio: "ignore",
+    });
+    const exited = new Promise<NodeJS.Signals | null>((resolve) =>
+      child.once("exit", (_code, signal) => resolve(signal)),
+    );
+    const pid = child.pid!;
+
+    expect(errorCode(() => signalProcessGroup(pid, 0))).toBeUndefined();
+    signalProcessGroup(pid, "SIGKILL");
+    expect(await exited).toBe("SIGKILL");
+  });
+});

apps/server/src/process/processGroup.ts

@@ -0,0 +1,16 @@
+/**
+ * Signals a POSIX process group this server spawned, as `process.kill(-pid)`.
+ *
+ * `kill(0)` signals this server's own group and `kill(-1)` every process the
+ * user owns, and a fake spawner in tests reports pid 1. So a pid of 0 or 1, or
+ * one that is not an integer, fails with ESRCH like a group that has already
+ * exited, and callers keep their existing handling.
+ */
+export function signalProcessGroup(pid: number, signal: NodeJS.Signals | 0): void {
+  if (!Number.isSafeInteger(pid) || pid <= 1) {
+    throw Object.assign(new Error(`kill ESRCH: not a spawned process group (${pid})`), {
+      code: "ESRCH",
+    });
+  }
+  process.kill(-pid, signal);
+}

apps/server/src/provider/OpenCodeServerLedger.ts

@@ -9,6 +9,7 @@ import * as Schema from "effect/Schema";
 import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";
 
 import * as ServerConfig from "../config.ts";
+import { signalProcessGroup } from "../process/processGroup.ts";
 
 const ProcessIdentity = Schema.Struct({ pid: Schema.Int, startTime: Schema.String });
 type ProcessIdentity = typeof ProcessIdentity.Type;
@@ -91,15 +92,15 @@ const parseDarwinPs = (output: string): ReadonlyArray<ObservedProcess> =>
 
 const signalGroup = (pgid: number, signal: NodeJS.Signals) => {
   try {
-    process.kill(-pgid, signal);
+    signalProcessGroup(pgid, signal);
   } catch {
     // The group may already be gone.
   }
 };
 
 const groupExists = (pgid: number) => {
   try {
-    process.kill(-pgid, 0);
+    signalProcessGroup(pgid, 0);
     return true;
   } catch (cause) {
     return (cause as NodeJS.ErrnoException | undefined)?.code !== "ESRCH";

apps/server/src/provider/acp/AcpSessionRuntime.ts

@@ -29,6 +29,7 @@ import type * as EffectAcpProtocol from "effect-acp/protocol";
 import { resolveSpawnCommand } from "@t3tools/shared/shell";
 import { HostProcessPlatform } from "@t3tools/shared/hostProcess";
 
+import { signalProcessGroup } from "../../process/processGroup.ts";
 import { appendAcpStderrTail, sanitizeAcpStderrExcerpt } from "./AcpStderr.ts";
 import {
   collectSessionConfigOptionValues,
@@ -1647,7 +1648,7 @@ export const make = (
     const signalOwnedProcessGroup = (signal: NodeJS.Signals) =>
       Effect.try({
         try: () => {
-          process.kill(-Number(child.pid), signal);
+          signalProcessGroup(Number(child.pid), signal);
           return true;
         },
         catch: (cause) =>

apps/server/src/provider/opencodeRuntime.ts

@@ -30,6 +30,7 @@ import * as Schema from "effect/Schema";
 import * as Stream from "effect/Stream";
 import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process";
 
+import { signalProcessGroup } from "../process/processGroup.ts";
 import { isWindowsCommandNotFound } from "../processRunner.ts";
 import * as OpenCodeServerLedger from "./OpenCodeServerLedger.ts";
 import { collectStreamAsString } from "./providerSnapshot.ts";
@@ -609,7 +610,7 @@ const makeOpenCodeRuntime = Effect.gen(function* () {
           ? child.kill({ killSignal: "SIGKILL" }).pipe(Effect.asVoid)
           : Effect.sync(() => {
               try {
-                process.kill(-Number(child.pid), "SIGKILL");
+                signalProcessGroup(Number(child.pid), "SIGKILL");
               } catch {
                 // The command and its process group may already have exited.
               }
@@ -731,7 +732,7 @@ const makeOpenCodeRuntime = Effect.gen(function* () {
           ? child.kill({ killSignal: signal, forceKillAfter: "1 second" }).pipe(Effect.asVoid)
           : Effect.sync(() => {
               try {
-                process.kill(-Number(child.pid), signal);
+                signalProcessGroup(Number(child.pid), signal);
               } catch {
                 // The direct child may already have exited after starting the
                 // server; the process group kill is best-effort cleanup for