From ccc1415f6d450d24a4fa5d2e3efe48fe13681566 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 20 Jun 2026 12:08:02 +0200 Subject: [PATCH] fix(ui): clean up wrapper signal descendants --- scripts/ui.js | 106 ++++++++++++++++++++++++++++++++++++---- test/scripts/ui.test.ts | 68 ++++++++++++++++++++++++++ 2 files changed, 165 insertions(+), 9 deletions(-) diff --git a/scripts/ui.js b/scripts/ui.js index 6385c4025966..7e2bca0c20b5 100644 --- a/scripts/ui.js +++ b/scripts/ui.js @@ -102,17 +102,47 @@ function runSpawnCall(spawnCall, label) { } let forwardedSignal = null; + let forwardedSignalPids = []; let forceKillTimer = null; + let forwardedSignalDrainTimer = null; + const clearForwardedSignalTimers = () => { + if (forceKillTimer) { + clearTimeout(forceKillTimer); + forceKillTimer = null; + } + if (forwardedSignalDrainTimer) { + clearInterval(forwardedSignalDrainTimer); + forwardedSignalDrainTimer = null; + } + }; + const finishForwardedSignal = () => { + cleanupSignalHandlers(); + process.kill(process.pid, forwardedSignal); + }; + const waitForForwardedSignalChildren = () => { + if (!forwardedSignal || processTreeIsAlive(forwardedSignalPids)) { + return; + } + finishForwardedSignal(); + }; // Keep UI dev children in the foreground process group for native TTY - // resize/job-control behavior. Forward direct wrapper shutdown signals. + // resize/job-control behavior. Forward wrapper shutdown signals to the + // captured child tree instead of using a detached process group. const forwardedSignals = ["SIGTERM", "SIGHUP"]; const signalHandlers = new Map( forwardedSignals.map((signal) => [ signal, () => { - forwardedSignal ??= signal; - child.kill(signal); - forceKillTimer ??= setTimeout(() => child.kill("SIGKILL"), 5_000); + if (!forwardedSignal) { + forwardedSignal = signal; + forwardedSignalPids = collectChildProcessTreePids(child); + signalProcessTree(child, signal, forwardedSignalPids); + forwardedSignalDrainTimer = setInterval(waitForForwardedSignalChildren, 25); + forceKillTimer = setTimeout(() => { + signalProcessTree(child, "SIGKILL", forwardedSignalPids); + }, 5_000); + forceKillTimer.unref?.(); + } }, ]), ); @@ -120,9 +150,7 @@ function runSpawnCall(spawnCall, label) { for (const [signal, handler] of signalHandlers) { process.off(signal, handler); } - if (forceKillTimer) { - clearTimeout(forceKillTimer); - } + clearForwardedSignalTimers(); }; for (const [signal, handler] of signalHandlers) { process.on(signal, handler); @@ -134,11 +162,11 @@ function runSpawnCall(spawnCall, label) { process.exit(1); }); child.on("exit", (code, signal) => { - cleanupSignalHandlers(); if (forwardedSignal) { - process.kill(process.pid, forwardedSignal); + waitForForwardedSignalChildren(); return; } + cleanupSignalHandlers(); if (signal) { process.kill(process.pid, signal); return; @@ -149,6 +177,66 @@ function runSpawnCall(spawnCall, label) { }); } +function collectChildProcessTreePids(child) { + if (process.platform === "win32" || typeof child.pid !== "number") { + return typeof child.pid === "number" ? [child.pid] : []; + } + const ps = spawnSync("ps", ["-axo", "pid=,ppid="], { encoding: "utf8" }); + if (ps.status !== 0) { + return [child.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 = [child.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 signalProcessTree(child, signal, pids) { + if (process.platform === "win32") { + child.kill(signal); + return; + } + if (pids.length === 0) { + child.kill(signal); + return; + } + for (const pid of pids.toReversed()) { + try { + process.kill(pid, signal); + } catch (error) { + if (error?.code !== "ESRCH") { + throw error; + } + } + } +} + function run(cmd, args) { runSpawnCall(resolveSpawnCall(cmd, args), cmd); } diff --git a/test/scripts/ui.test.ts b/test/scripts/ui.test.ts index 50a2f385ba5f..31c4356ce9e5 100644 --- a/test/scripts/ui.test.ts +++ b/test/scripts/ui.test.ts @@ -243,4 +243,72 @@ describe("scripts/ui windows spawn behavior", () => { } }, ); + + it.runIf(process.platform !== "win32")( + "cleans pnpm descendants before forwarding wrapper SIGTERM", + async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-ui-wrapper-tree-")); + const runnerPath = path.join(tempDir, "pnpm.mjs"); + const readyFile = path.join(tempDir, "ready"); + const descendantPidFile = path.join(tempDir, "descendant.pid"); + let descendantPid: number | undefined; + + fs.writeFileSync( + runnerPath, + [ + "import { spawn } from 'node:child_process';", + "import fs from 'node:fs';", + "fs.writeFileSync(process.env.READY_FILE, 'ready');", + "const child = spawn(process.execPath, ['-e', \"process.on('SIGTERM', () => {}); setInterval(() => {}, 1000);\"], { stdio: 'ignore' });", + "child.unref();", + "fs.writeFileSync(process.env.DESCENDANT_PID_FILE, String(child.pid));", + "process.on('SIGTERM', () => process.exit(0));", + "setInterval(() => {}, 1000);", + ].join("\n"), + ); + + const wrapper = spawn(process.execPath, ["scripts/ui.js", "install"], { + cwd: path.resolve("."), + env: { + ...process.env, + DESCENDANT_PID_FILE: descendantPidFile, + npm_execpath: runnerPath, + READY_FILE: readyFile, + }, + stdio: "ignore", + }); + + try { + await waitFor(() => fs.existsSync(descendantPidFile), "UI runner descendant readiness"); + descendantPid = Number(fs.readFileSync(descendantPidFile, "utf8")); + await new Promise((resolve) => { + setTimeout(resolve, 300); + }); + + wrapper.kill("SIGTERM"); + const exit = await waitForExit(wrapper, 8_000); + + expect(exit).toEqual({ code: null, signal: "SIGTERM" }); + await waitFor( + () => !descendantPid || !pidAlive(descendantPid), + "UI runner descendant exit", + ); + } finally { + wrapper.kill("SIGKILL"); + if (descendantPid && pidAlive(descendantPid)) { + process.kill(descendantPid, "SIGKILL"); + } + fs.rmSync(tempDir, { force: true, recursive: true }); + } + }, + ); }); + +function pidAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +}