Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions server/drivers/acp/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import type {
} from "../../contracts.ts";
import { newEventId, newId } from "../../contracts.ts";
import { augmentedPath } from "../../env-path.ts";
import { killTree } from "../../kill-tree.ts";
import { appendNative } from "../native.ts";

export interface AcpConfig {
Expand Down Expand Up @@ -188,15 +189,7 @@ export function createAcpDriver(support: AcpSupport): ProviderDriver<AcpConfig>
send({ jsonrpc: "2.0", id, method, params });
});

const stop = () => {
try {
process.kill(-child.pid!, "SIGTERM");
} catch {
try {
child.kill("SIGTERM");
} catch {}
}
};
const stop = () => killTree(child); // process groups are POSIX-only

const settle = (ok: boolean, stopReason: string | null) => {
if (state.settled) return;
Expand Down
13 changes: 3 additions & 10 deletions server/drivers/claude.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { fileURLToPath } from "node:url";

import { DATA_DIR } from "../config.ts";
import { augmentedPath } from "../env-path.ts";
import { killTree } from "../kill-tree.ts";

import type {
DriverCreateInput,
Expand Down Expand Up @@ -326,7 +327,7 @@ export const ClaudeDriver: ProviderDriver<ClaudeConfig> = {
cwd: turn.cwd ?? homedir(),
env,
stdio: ["pipe", "pipe", "pipe"],
detached: true, // own process group: killing -pid reaps child MCP servers
detached: true, // own process group, so killTree reaps child MCP servers (-pid on POSIX, taskkill /T on win32)
});

let settled = false;
Expand Down Expand Up @@ -446,15 +447,7 @@ export const ClaudeDriver: ProviderDriver<ClaudeConfig> = {
}
});

const stop = () => {
try {
process.kill(-child.pid!, "SIGTERM");
} catch {
try {
child.kill("SIGTERM");
} catch {}
}
};
const stop = () => killTree(child); // process groups are POSIX-only
active.set(threadId, { stop, turnId, broker });
emit({ ...base(threadId, turnId), type: "turn.started" });

Expand Down
11 changes: 2 additions & 9 deletions server/drivers/codex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import type {
} from "../contracts.ts";
import { newEventId, newId } from "../contracts.ts";
import { augmentedPath } from "../env-path.ts";
import { killTree } from "../kill-tree.ts";
import { appendNative } from "./native.ts";

const DRIVER_KIND = "codex";
Expand Down Expand Up @@ -117,15 +118,7 @@ export const CodexDriver: ProviderDriver<CodexConfig> = {
send({ jsonrpc: "2.0", id, method, params });
});

const stop = () => {
try {
process.kill(-child.pid!, "SIGTERM");
} catch {
try {
child.kill("SIGTERM");
} catch {}
}
};
const stop = () => killTree(child); // process groups are POSIX-only

const settle = (ok: boolean, stopReason: string | null) => {
if (state.settled) return;
Expand Down
50 changes: 50 additions & 0 deletions server/kill-tree.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// The drivers spawn their CLI detached so that stopping a turn also stops
// whatever the CLI started (its MCP servers). That guarantee is the whole
// contract of killTree, so it is what gets tested: a grandchild must not
// survive the kill on either platform.
import { spawn } from "node:child_process";
import { describe, expect, it } from "vitest";

import { killTree } from "./kill-tree.ts";

const IDLE = "setInterval(() => {}, 1000)";

function alive(pid: number): boolean {
try {
process.kill(pid, 0);
return true;
} catch {
return false;
}
}

describe("killTree", () => {
it("reaps a grandchild, not just the process it was handed", async () => {
// a stand-in CLI: spawns one helper, reports its pid, then idles
const parent = spawn(
process.execPath,
[
"-e",
`const c = require("node:child_process").spawn(process.execPath, ["-e", ${JSON.stringify(IDLE)}], { stdio: "ignore" });` +
`console.log(c.pid); ${IDLE}`,
],
{ stdio: ["ignore", "pipe", "ignore"], detached: true },
);
const grandchild = await new Promise<number>((resolve) =>
parent.stdout!.once("data", (c) => resolve(Number(String(c).trim()))),
);
expect(grandchild).toBeGreaterThan(0);
expect(alive(grandchild)).toBe(true);

killTree(parent);

// wait for both, and read the parent's death off the child object: a
// POSIX parent stays a live pid as a zombie until node reaps it
const exited = () => parent.exitCode !== null || parent.signalCode !== null;
for (let i = 0; i < 100 && (alive(grandchild) || !exited()); i++) {
await new Promise((r) => setTimeout(r, 100));
}
expect(alive(grandchild)).toBe(false);
expect(exited()).toBe(true);
});
});
33 changes: 33 additions & 0 deletions server/kill-tree.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Killing a CLI *and* everything it started, on every platform.
//
// The drivers spawn their CLI detached so it leads its own process group,
// then kill -pid to reap the whole group — the CLI's MCP servers and
// helper processes included. Process groups are a POSIX concept:
// process.kill(-pid) throws EINVAL on Windows, and the fallback that
// catches it kills the CLI alone, orphaning every server it started.
// Windows tracks the parent/child tree instead, which is what
// `taskkill /T` walks.
import { execFile, type ChildProcess } from "node:child_process";

/** Terminate a spawned CLI together with its descendants. Best-effort and
* synchronous to call: nothing here throws. */
export function killTree(child: ChildProcess): void {
const pid = child.pid;
if (!pid || child.exitCode !== null || child.signalCode !== null) return;
if (process.platform === "win32") {
execFile("taskkill", ["/T", "/F", "/PID", String(pid)], (err) => {
if (!err) return;
try {
child.kill(); // taskkill unavailable or already gone — at least the CLI
} catch {}
});
return;
}
try {
process.kill(-pid, "SIGTERM");
} catch {
try {
child.kill("SIGTERM");
} catch {}
}
}