fix(process): prevent orphaned Windows child process trees (#115535)

* test(process): reproduce Windows taskkill process-tree leak

* fix(process): stop leaked Windows child process trees

Escalate only when Windows taskkill reports that graceful process-tree termination failed. Preserve awaited taskkill completion, grace-period fallback, one-shot signaling, and PID-reuse protection.

Closes #110789

Supersedes #112202

Co-authored-by: Mohammed Alkindi <alkndymhmd692@gmail.com>

---------

Co-authored-by: Mohammed Alkindi <alkndymhmd692@gmail.com>

packages/agent-core/src/harness/env/kill-tree.ts

@@ -15,7 +15,8 @@ export type KillProcessTreeOptions = {
 /**
  * Best-effort process-tree termination with graceful shutdown.
  * - Windows: use taskkill /T to include descendants. Sends SIGTERM-equivalent
- *   first (without /F), then force-kills if process survives.
+ *   first (without /F), then force-kills if taskkill refuses or the process
+ *   survives the grace period.
  * - Unix: send SIGTERM to process group first, wait grace period, then SIGKILL.
  *
  * Group kill (`process.kill(-pid, ...)`) is only used when the PID is verified
@@ -169,18 +170,19 @@ function signalProcessTreeUnix(
   }
 }
 
-function runTaskkill(args: string[]): Promise<void> {
+function runTaskkill(args: string[], onExit?: (code: number | null) => void): Promise<void> {
   return new Promise((resolve) => {
     let settled = false;
-    const finish = () => {
+    const finish = (code: number | null) => {
       if (settled) {
         return;
       }
       settled = true;
       clearTimeout(completionTimer);
+      onExit?.(code);
       resolve();
     };
-    const completionTimer = setTimeout(finish, TASKKILL_COMPLETION_TIMEOUT_MS);
+    const completionTimer = setTimeout(() => finish(null), TASKKILL_COMPLETION_TIMEOUT_MS);
     completionTimer.unref?.();
     try {
       const child = spawn("taskkill", args, {
@@ -188,35 +190,61 @@ function runTaskkill(args: string[]): Promise<void> {
         detached: true,
         windowsHide: true,
       });
-      child.once("error", finish);
-      child.once("close", finish);
+      // A failed spawn emits error before a close with a negative errno. Only
+      // taskkill's first actual outcome may authorize immediate escalation.
+      child.once("error", () => finish(null));
+      child.once("close", (code) => finish(code));
     } catch {
       // Ignore taskkill spawn failures.
-      finish();
+      finish(null);
     }
   });
 }
 
 function killProcessTreeWindows(pid: number, graceMs: number): void {
-  signalProcessTreeWindows(pid, "SIGTERM");
-
-  setTimeout(() => {
+  let forced = false;
+  let graceTimer: ReturnType<typeof setTimeout> | undefined;
+  const forceKill = () => {
+    if (forced) {
+      return;
+    }
+    // Latch before probing: a later live PID could belong to a reused,
+    // unrelated Windows process tree.
+    forced = true;
+    if (graceTimer !== undefined) {
+      clearTimeout(graceTimer);
+      graceTimer = undefined;
+    }
     if (!isProcessAlive(pid)) {
       return;
     }
     signalProcessTreeWindows(pid, "SIGKILL");
-  }, graceMs).unref();
+  };
+
+  signalProcessTreeWindows(pid, "SIGTERM", (code) => {
+    if (code !== null && code !== 0) {
+      forceKill();
+    }
+  });
+
+  graceTimer = setTimeout(forceKill, graceMs);
+  graceTimer.unref();
 }
 
-function signalProcessTreeWindows(pid: number, signal: "SIGTERM" | "SIGKILL"): void {
-  void signalProcessTreeWindowsAndWait(pid, signal);
+function signalProcessTreeWindows(
+  pid: number,
+  signal: "SIGTERM" | "SIGKILL",
+  onExit?: (code: number | null) => void,
+): void {
+  void signalProcessTreeWindowsAndWait(pid, signal, onExit);
 }
 
 function signalProcessTreeWindowsAndWait(
   pid: number,
   signal: "SIGTERM" | "SIGKILL",
+  onExit?: (code: number | null) => void,
 ): Promise<void> {
   const args =
     signal === "SIGKILL" ? ["/F", "/T", "/PID", String(pid)] : ["/T", "/PID", String(pid)];
-  return runTaskkill(args);
+  return runTaskkill(args, onExit);
 }

src/process/exec.windows.integration.test.ts

@@ -1,6 +1,9 @@
+import { spawn, spawnSync } from "node:child_process";
+import { once } from "node:events";
 import process from "node:process";
-import { describe, expect, it } from "vitest";
+import { describe, expect, it, vi } from "vitest";
 import { runUtf8CommandWithTimeout } from "./exec.js";
+import { killProcessTree } from "./kill-tree.js";
 
 describe("runUtf8CommandWithTimeout Windows integration", () => {
   it.runIf(process.platform === "win32")(
@@ -21,4 +24,56 @@ describe("runUtf8CommandWithTimeout Windows integration", () => {
       expect(result.stderrTruncatedBytes).toBe(5);
     },
   );
+
+  it.runIf(process.platform === "win32")(
+    "force-kills a real Windows process tree when graceful taskkill refuses it",
+    async () => {
+      const program = [
+        'const { spawn } = require("node:child_process");',
+        'const child = spawn(process.execPath, ["-e", "setInterval(() => {}, 1000)"], { stdio: "ignore", windowsHide: true });',
+        'child.once("spawn", () => process.stdout.write(String(child.pid) + "\\n"));',
+        'child.once("error", () => process.exit(1));',
+        "setInterval(() => {}, 1000);",
+      ].join("\n");
+      const parent = spawn(process.execPath, ["-e", program], {
+        stdio: ["ignore", "pipe", "ignore"],
+        windowsHide: true,
+      });
+      const parentPid = parent.pid;
+      const parentStdout = parent.stdout;
+
+      if (parentPid === undefined || parentStdout === null) {
+        parent.kill();
+        throw new Error("Could not start the Windows process tree");
+      }
+
+      try {
+        const [output] = await once(parentStdout, "data");
+        const childPid = Number.parseInt(String(output).trim(), 10);
+        expect(Number.isSafeInteger(childPid)).toBe(true);
+        expect(() => process.kill(parentPid, 0)).not.toThrow();
+        expect(() => process.kill(childPid, 0)).not.toThrow();
+
+        // An unforced taskkill refuses Node console processes. Cleanup must not
+        // depend on this unref'd timer surviving an application shutdown.
+        killProcessTree(parentPid, { graceMs: 30_000 });
+
+        await vi.waitFor(
+          () => {
+            expect(() => process.kill(parentPid, 0)).toThrow();
+            expect(() => process.kill(childPid, 0)).toThrow();
+          },
+          { timeout: 5_000, interval: 50 },
+        );
+      } finally {
+        spawnSync("taskkill", ["/F", "/T", "/PID", String(parentPid)], {
+          stdio: "ignore",
+          timeout: 5_000,
+          windowsHide: true,
+        });
+        parentStdout.destroy();
+      }
+    },
+    15_000,
+  );
 });

src/process/kill-tree.test.ts

@@ -63,7 +63,7 @@ describe("killProcessTree", () => {
     readFileSyncMock.mockImplementation(() => {
       throw new Error("proc unavailable");
     });
-    spawnMock.mockClear();
+    spawnMock.mockReset();
     spawnSyncMock.mockClear();
     killSpy = vi.spyOn(process, "kill");
     vi.useFakeTimers();
@@ -113,6 +113,92 @@ describe("killProcessTree", () => {
     });
   });
 
+  it("on Windows force-kills immediately when graceful taskkill refuses a live process tree", async () => {
+    const gracefulTaskkill = new EventEmitter();
+    spawnMock.mockReturnValueOnce(gracefulTaskkill);
+    killSpy.mockImplementation(() => true);
+
+    await withMockedPlatform("win32", async () => {
+      killProcessTree(4711, { graceMs: 30_000 });
+
+      expectTaskkillCall(0, ["/T", "/PID", "4711"]);
+      gracefulTaskkill.emit("close", 128);
+
+      expect(spawnMock).toHaveBeenCalledTimes(2);
+      expectTaskkillCall(1, ["/F", "/T", "/PID", "4711"]);
+    });
+  });
+
+  it("on Windows does not force-kill a disappeared or reused PID after taskkill fails", async () => {
+    const gracefulTaskkill = new EventEmitter();
+    spawnMock.mockReturnValueOnce(gracefulTaskkill);
+    let processWasReused = false;
+    killSpy.mockImplementation(((pid: number, signal?: NodeJS.Signals | number) => {
+      if (pid === 4712 && signal === 0 && !processWasReused) {
+        throw new Error("ESRCH");
+      }
+      return true;
+    }) as typeof process.kill);
+
+    await withMockedPlatform("win32", async () => {
+      killProcessTree(4712, { graceMs: 25 });
+      gracefulTaskkill.emit("close", 128);
+      expect(spawnMock).toHaveBeenCalledTimes(1);
+
+      processWasReused = true;
+      await vi.advanceTimersByTimeAsync(25);
+
+      expect(spawnMock).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  it("on Windows force-kills only once when taskkill failure races the grace timer", async () => {
+    const gracefulTaskkill = new EventEmitter();
+    spawnMock.mockReturnValueOnce(gracefulTaskkill);
+    killSpy.mockImplementation(() => true);
+
+    await withMockedPlatform("win32", async () => {
+      killProcessTree(4713, { graceMs: 20 });
+      gracefulTaskkill.emit("close", 128);
+      await vi.advanceTimersByTimeAsync(20);
+
+      expect(spawnMock).toHaveBeenCalledTimes(2);
+      expectTaskkillCall(1, ["/F", "/T", "/PID", "4713"]);
+    });
+  });
+
+  it("on Windows waits for the grace timer when graceful taskkill cannot start", async () => {
+    const gracefulTaskkill = new EventEmitter();
+    spawnMock.mockReturnValueOnce(gracefulTaskkill);
+    killSpy.mockImplementation(() => true);
+
+    await withMockedPlatform("win32", async () => {
+      killProcessTree(4714, { graceMs: 15 });
+      expect(() => gracefulTaskkill.emit("error", new Error("spawn ENOENT"))).not.toThrow();
+      gracefulTaskkill.emit("close", -4058);
+      expect(spawnMock).toHaveBeenCalledTimes(1);
+
+      await vi.advanceTimersByTimeAsync(15);
+
+      expect(spawnMock).toHaveBeenCalledTimes(2);
+      expectTaskkillCall(1, ["/F", "/T", "/PID", "4714"]);
+    });
+  });
+
+  it("on Windows keeps an explicitly requested failed tree signal single-shot", async () => {
+    const gracefulTaskkill = new EventEmitter();
+    spawnMock.mockReturnValueOnce(gracefulTaskkill);
+
+    await withMockedPlatform("win32", async () => {
+      signalProcessTree(4715, "SIGTERM");
+      gracefulTaskkill.emit("close", 128);
+      await vi.advanceTimersByTimeAsync(60_000);
+
+      expect(spawnMock).toHaveBeenCalledTimes(1);
+      expectTaskkillCall(0, ["/T", "/PID", "4715"]);
+    });
+  });
+
   it("on Unix sends SIGTERM first and skips SIGKILL when process exits", async () => {
     killSpy.mockImplementation(((pid: number, signal?: NodeJS.Signals | number) => {
       if (pid === -3333 && signal === 0) {