diff --git a/packages/cli/src/utils/orphanCleanup.test.ts b/packages/cli/src/utils/orphanCleanup.test.ts new file mode 100644 index 000000000..b3dc2afdf --- /dev/null +++ b/packages/cli/src/utils/orphanCleanup.test.ts @@ -0,0 +1,55 @@ +import { describe, it, expect } from "vitest"; +import { spawn } from "node:child_process"; +import { killProcessTree, killOrphanedProcesses } from "./orphanCleanup.js"; + +const IS_UNIX = process.platform !== "win32"; + +describe.skipIf(!IS_UNIX)("killProcessTree", () => { + it("kills a process and all its children", async () => { + // Spawn a parent that spawns two sleeping children + const parent = spawn("bash", ["-c", "sleep 60 & sleep 60 & wait"], { stdio: "ignore" }); + // Let children spawn + await new Promise((r) => setTimeout(r, 200)); + + const exitPromise = new Promise((resolve) => parent.on("close", resolve)); + killProcessTree(parent.pid!); + + await exitPromise; + + // Verify parent is dead + expect(() => process.kill(parent.pid!, 0)).toThrow(); + }, 5000); + + it("handles non-existent PID gracefully", () => { + // Should not throw for a PID that doesn't exist + killProcessTree(999999999); + }); + + it("escalates to SIGKILL after grace period", async () => { + // Spawn a process that traps SIGTERM + const proc = spawn("bash", ["-c", "trap '' TERM; sleep 60"], { stdio: "ignore" }); + await new Promise((r) => setTimeout(r, 100)); + + const exitPromise = new Promise((resolve) => proc.on("close", resolve)); + killProcessTree(proc.pid!); + + // Should die within 1s (500ms SIGKILL grace + buffer) + await exitPromise; + expect(() => process.kill(proc.pid!, 0)).toThrow(); + }, 5000); +}); + +describe.skipIf(!IS_UNIX)("killOrphanedProcesses", () => { + it("returns 0 when no orphans exist", () => { + const killed = killOrphanedProcesses(); + expect(killed).toBe(0); + }); + + it("does not kill non-orphaned Chrome processes", () => { + // Our current process is not an orphan (PPID !== 1), so any + // chrome-headless-shell processes we'd find with our PID as + // ancestor wouldn't be killed. + const killed = killOrphanedProcesses(); + expect(killed).toBe(0); + }); +}); diff --git a/packages/cli/src/utils/orphanCleanup.ts b/packages/cli/src/utils/orphanCleanup.ts index 3b7157853..a34f01775 100644 --- a/packages/cli/src/utils/orphanCleanup.ts +++ b/packages/cli/src/utils/orphanCleanup.ts @@ -41,17 +41,27 @@ export function killProcessTree(pid: number, signal: NodeJS.Signals = "SIGTERM") if (process.platform === "win32") return; const descendants = getDescendants(pid); - for (const child of descendants.reverse()) { + const allPids = [...descendants.reverse(), pid]; + + for (const p of allPids) { try { - process.kill(child, signal); + process.kill(p, signal); } catch { // Already exited. } } - try { - process.kill(pid, signal); - } catch { - // Already exited. + + // Escalate to SIGKILL after a short grace period for any survivors. + if (signal !== "SIGKILL") { + setTimeout(() => { + for (const p of allPids) { + try { + process.kill(p, "SIGKILL"); + } catch { + // Already exited. + } + } + }, 500).unref(); } } diff --git a/packages/engine/src/utils/processTracker.test.ts b/packages/engine/src/utils/processTracker.test.ts new file mode 100644 index 000000000..0e8d72232 --- /dev/null +++ b/packages/engine/src/utils/processTracker.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { spawn } from "node:child_process"; +import { trackChildProcess, killTrackedProcesses } from "./processTracker.js"; + +// Reset tracked set between tests by killing everything +beforeEach(() => { + killTrackedProcesses(); +}); + +describe("trackChildProcess", () => { + it("tracks a spawned process and removes it after exit", async () => { + const proc = spawn("echo", ["hello"], { stdio: "ignore" }); + trackChildProcess(proc); + + await new Promise((resolve) => proc.on("close", resolve)); + + // After exit, killTrackedProcesses should be a no-op (nothing to kill) + killTrackedProcesses(); + }); + + it("removes the process on spawn error", async () => { + const proc = spawn("/nonexistent-binary-that-does-not-exist", { stdio: "ignore" }); + trackChildProcess(proc); + + await new Promise((resolve) => proc.on("error", () => resolve())); + + killTrackedProcesses(); + }); +}); + +describe("killTrackedProcesses", () => { + it("kills a running process", async () => { + const proc = spawn("sleep", ["60"], { stdio: "ignore" }); + trackChildProcess(proc); + + const exitPromise = new Promise((resolve) => proc.on("close", resolve)); + killTrackedProcesses(); + + const code = await exitPromise; + // SIGTERM exit: code is null (killed by signal) + expect(code).toBeNull(); + }); + + it("handles already-exited processes gracefully", async () => { + const proc = spawn("true", { stdio: "ignore" }); + trackChildProcess(proc); + + await new Promise((resolve) => proc.on("close", resolve)); + + // Should not throw even though process already exited + killTrackedProcesses(); + }); + + it("escalates to SIGKILL for processes that ignore SIGTERM", async () => { + // Spawn a process that traps SIGTERM (bash ignoring it) + const proc = spawn("bash", ["-c", "trap '' TERM; sleep 60"], { stdio: "ignore" }); + trackChildProcess(proc); + + const exitPromise = new Promise((resolve) => proc.on("close", resolve)); + killTrackedProcesses(); + + // The 500ms SIGKILL escalation should kill it + await exitPromise; + expect(proc.killed).toBe(true); + }, 5000); + + it("is idempotent — second call is a no-op", () => { + const proc = spawn("sleep", ["60"], { stdio: "ignore" }); + trackChildProcess(proc); + + killTrackedProcesses(); + killTrackedProcesses(); + }); +});