fix(e2e): reap signaled PTY command trees

scripts/e2e/lib/run-with-pty.mjs

@@ -1,8 +1,8 @@
 #!/usr/bin/env node
-// Runs an E2E command under a pseudo-terminal.
+import { spawnSync } from "node:child_process";
 import fs from "node:fs";
 import process from "node:process";
-import { spawn } from "@lydell/node-pty";
+import { spawn as spawnPty } from "@lydell/node-pty";
 import { readPositiveIntEnv } from "./env-limits.mjs";
 
 const [logPath, command, ...args] = process.argv.slice(2);
@@ -17,6 +17,9 @@ if (!logPath || !command) {
 let exiting = false;
 let forwardedSignal = null;
 let forceKillTimer = null;
+let terminationDrainTimer = null;
+let terminationPids = [];
+let pendingExitCode = null;
 let logFailed = false;
 const outputLimitMarker = `\n[run-with-pty output truncated after ${OUTPUT_MAX_BYTES} bytes]\n`;
 const outputState = {
@@ -25,7 +28,7 @@ const outputState = {
 };
 
 const log = fs.createWriteStream(logPath, { flags: "w" });
-const pty = spawn(command, args, {
+const pty = spawnPty(command, args, {
   name: process.env.TERM || "xterm-256color",
   cols: readPositiveIntEnv("COLUMNS", 120),
   rows: readPositiveIntEnv("LINES", 40),
@@ -43,11 +46,7 @@ log.on("error", (error) => {
     process.exit(1);
   }
   if (!exiting) {
-    pty.kill("SIGTERM");
-    forceKillTimer ??= setTimeout(() => {
-      pty.kill("SIGKILL");
-    }, FORCE_KILL_MS);
-    forceKillTimer.unref?.();
+    terminatePtyTree("SIGTERM");
   }
 });
 
@@ -86,18 +85,23 @@ pty.onData((data) => {
 
 pty.onExit(({ exitCode, signal }) => {
   exiting = true;
-  clearTimeout(forceKillTimer);
+  if (terminationPids.length === 0) {
+    clearTerminationTimers();
+  }
   if (logFailed) {
-    process.exit(1);
+    exitWhenTerminationDrains(1);
+    return;
   }
   log.end(() => {
     if (forwardedSignal) {
-      process.exit(signalExitCode(forwardedSignal));
+      exitWhenTerminationDrains(signalExitCode(forwardedSignal));
+      return;
     }
     if (typeof exitCode === "number") {
-      process.exit(exitCode);
+      exitWhenTerminationDrains(exitCode);
+      return;
     }
-    process.exit(signal ? 128 + signal : 1);
+    exitWhenTerminationDrains(signal ? 128 + signal : 1);
   });
 });
 
@@ -109,15 +113,108 @@ for (const signal of ["SIGHUP", "SIGINT", "SIGTERM"]) {
   process.on(signal, () => {
     if (!exiting) {
       forwardedSignal ??= signal;
-      pty.kill(signal);
-      forceKillTimer ??= setTimeout(() => {
-        pty.kill("SIGKILL");
-      }, FORCE_KILL_MS);
-      forceKillTimer.unref?.();
+      terminatePtyTree(signal);
     }
   });
 }
 
+function terminatePtyTree(signal) {
+  // node-pty kill() targets only pty.pid on Unix; wrapper-owned shutdowns
+  // keep the captured child tree alive until ignored descendants drain.
+  if (terminationPids.length === 0) {
+    terminationPids = collectPtyProcessTreePids();
+  }
+  signalPtyProcessTree(signal);
+  forceKillTimer ??= setTimeout(() => {
+    signalPtyProcessTree("SIGKILL");
+  }, FORCE_KILL_MS);
+  forceKillTimer.unref?.();
+}
+
+function exitWhenTerminationDrains(exitCode) {
+  pendingExitCode = exitCode;
+  if (processTreeIsAlive(terminationPids)) {
+    terminationDrainTimer ??= setInterval(finishIfTerminationDrained, 25);
+    return;
+  }
+  finishIfTerminationDrained();
+}
+
+function finishIfTerminationDrained() {
+  if (processTreeIsAlive(terminationPids)) {
+    return;
+  }
+  clearTerminationTimers();
+  process.exit(pendingExitCode ?? 1);
+}
+
+function clearTerminationTimers() {
+  if (forceKillTimer) {
+    clearTimeout(forceKillTimer);
+    forceKillTimer = null;
+  }
+  if (terminationDrainTimer) {
+    clearInterval(terminationDrainTimer);
+    terminationDrainTimer = null;
+  }
+}
+
+function collectPtyProcessTreePids() {
+  if (process.platform === "win32" || typeof pty.pid !== "number") {
+    return typeof pty.pid === "number" ? [pty.pid] : [];
+  }
+  const ps = spawnSync("ps", ["-axo", "pid=,ppid="], { encoding: "utf8" });
+  if (ps.status !== 0) {
+    return [pty.pid];
+  }
+  const childrenByParent = new Map();
+  for (const line of ps.stdout.split("\n")) {
+    const match = line.trim().match(/^(\d+)\s+(\d+)$/u);
+    if (!match) {
+      continue;
+    }
+    const pid = Number(match[1]);
+    const ppid = Number(match[2]);
+    const siblings = childrenByParent.get(ppid) ?? [];
+    siblings.push(pid);
+    childrenByParent.set(ppid, siblings);
+  }
+  const pids = [pty.pid];
+  for (const parentPid of pids) {
+    for (const pid of childrenByParent.get(parentPid) ?? []) {
+      pids.push(pid);
+    }
+  }
+  return [...new Set(pids)];
+}
+
+function processTreeIsAlive(pids) {
+  return pids.some((pid) => {
+    try {
+      process.kill(pid, 0);
+      return true;
+    } catch (error) {
+      return error?.code === "EPERM";
+    }
+  });
+}
+
+function signalPtyProcessTree(signal) {
+  if (process.platform === "win32" || terminationPids.length === 0) {
+    pty.kill(signal);
+    return;
+  }
+  for (const pid of terminationPids.toReversed()) {
+    try {
+      process.kill(pid, signal);
+    } catch (error) {
+      if (error?.code !== "ESRCH") {
+        throw error;
+      }
+    }
+  }
+}
+
 function signalExitCode(signal) {
   switch (signal) {
     case "SIGHUP":

test/scripts/e2e-run-with-pty.test.ts

@@ -1,5 +1,6 @@
 // E2E Run With Pty tests cover e2e run with pty script behavior.
 import { spawn } from "node:child_process";
+import { existsSync } from "node:fs";
 import { mkdtemp, readFile, rm } from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
@@ -127,15 +128,23 @@ describe("run-with-pty", () => {
   posixIt("escalates forwarded termination signals for PTY commands that ignore them", async () => {
     const tempRoot = await mkdtemp(path.join(os.tmpdir(), "openclaw-run-with-pty-"));
     const logPath = path.join(tempRoot, "pty.log");
+    const descendantPidPath = path.join(tempRoot, "descendant.pid");
+    let descendantPid: number | null = null;
+    const probeCode = `
+const { spawn } = require("node:child_process");
+const fs = require("node:fs");
+process.on("SIGTERM", () => process.exit(0));
+const descendant = spawn(process.execPath, [
+  "-e",
+  "process.on('SIGTERM',()=>{});process.on('SIGHUP',()=>{});setInterval(()=>{},1000);",
+], { stdio: "ignore" });
+fs.writeFileSync(${JSON.stringify(descendantPidPath)}, String(descendant.pid));
+console.log("ready");
+setInterval(() => {}, 1000);
+`;
     const child = spawn(
       process.execPath,
-      [
-        scriptPath,
-        logPath,
-        process.execPath,
-        "-e",
-        "process.on('SIGTERM', () => {}); console.log('ready'); setInterval(() => {}, 1000);",
-      ],
+      [scriptPath, logPath, process.execPath, "-e", probeCode],
       {
         env: {
           ...process.env,
@@ -156,7 +165,8 @@ describe("run-with-pty", () => {
     });
 
     try {
-      await waitFor(() => stdout.text().includes("ready"));
+      await waitFor(() => stdout.text().includes("ready") && existsSync(descendantPidPath));
+      descendantPid = Number(await readFile(descendantPidPath, "utf8"));
       child.kill("SIGTERM");
       const result = await waitForClose(child, 5_000);
       const log = await readFile(logPath, "utf8");
@@ -164,7 +174,11 @@ describe("run-with-pty", () => {
       expect(result).toEqual({ code: 143, signal: null });
       expect(stderr.text()).toBe("");
       expect(log).toContain("ready");
+      expect(isProcessAlive(descendantPid)).toBe(false);
     } finally {
+      if (descendantPid && isProcessAlive(descendantPid)) {
+        process.kill(descendantPid, "SIGKILL");
+      }
       if (child.pid && isProcessAlive(child.pid)) {
         child.kill("SIGKILL");
       }