diff --git a/CHANGELOG.md b/CHANGELOG.md index 05826d4b6..22c05035f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ - WhatsApp: add `channels.whatsapp.sendReadReceipts` to disable auto read receipts. (#882) — thanks @chrisrodz. ### Fixes +- Gateway: forward termination signals to respawned CLI child processes to avoid orphaned systemd runs. (#933) — thanks @roshanasingh4. - Browser: add tests for snapshot labels/efficient query params and labeled image responses. - macOS: ensure launchd log directory exists with a test-only override. (#909) — thanks @roshanasingh4. - Packaging: run `pnpm build` on `prepack` so npm publishes include fresh `dist/` output. diff --git a/src/entry.ts b/src/entry.ts index 4cf1e4eb4..cac3ff4a8 100644 --- a/src/entry.ts +++ b/src/entry.ts @@ -1,8 +1,9 @@ #!/usr/bin/env node +import { spawn } from "node:child_process"; import process from "node:process"; import { applyCliProfileEnv, parseCliProfileArgs } from "./cli/profile.js"; -import { spawnWithSignalForwarding } from "./process/spawn-with-signal-forwarding.js"; +import { attachChildProcessBridge } from "./process/child-process-bridge.js"; if (process.argv.includes("--no-color")) { process.env.NO_COLOR = "1"; @@ -24,7 +25,7 @@ function ensureExperimentalWarningSuppressed(): boolean { process.env.CLAWDBOT_NODE_OPTIONS_READY = "1"; process.env.NODE_OPTIONS = `${nodeOptions} ${EXPERIMENTAL_WARNING_FLAG}`.trim(); - const { child } = spawnWithSignalForwarding( + const child = spawn( process.execPath, [...process.execArgv, ...process.argv.slice(1)], { @@ -33,7 +34,9 @@ function ensureExperimentalWarningSuppressed(): boolean { }, ); - child.on("exit", (code, signal) => { + attachChildProcessBridge(child); + + child.once("exit", (code, signal) => { if (signal) { process.exitCode = 1; return; @@ -41,6 +44,14 @@ function ensureExperimentalWarningSuppressed(): boolean { process.exit(code ?? 1); }); + child.once("error", (error) => { + console.error( + "[clawdbot] Failed to respawn CLI:", + error instanceof Error ? (error.stack ?? error.message) : error, + ); + process.exit(1); + }); + // Parent must not continue running the CLI. return true; } diff --git a/src/process/spawn-with-signal-forwarding.test.ts b/src/process/child-process-bridge.test.ts similarity index 81% rename from src/process/spawn-with-signal-forwarding.test.ts rename to src/process/child-process-bridge.test.ts index 29e187da3..00d376c0f 100644 --- a/src/process/spawn-with-signal-forwarding.test.ts +++ b/src/process/child-process-bridge.test.ts @@ -1,10 +1,11 @@ +import { spawn } from "node:child_process"; import net from "node:net"; import path from "node:path"; import process from "node:process"; import { afterEach, describe, expect, it } from "vitest"; -import { spawnWithSignalForwarding } from "./spawn-with-signal-forwarding.js"; +import { attachChildProcessBridge } from "./child-process-bridge.js"; function waitForLine(stream: NodeJS.ReadableStream, timeoutMs = 10_000): Promise { return new Promise((resolve, reject) => { @@ -52,10 +53,19 @@ function canConnect(port: number): Promise { }); } -describe("spawnWithSignalForwarding", () => { +describe("attachChildProcessBridge", () => { const children: Array<{ kill: (signal?: NodeJS.Signals) => boolean }> = []; + const detachments: Array<() => void> = []; afterEach(() => { + for (const detach of detachments) { + try { + detach(); + } catch { + // ignore + } + } + detachments.length = 0; for (const child of children) { try { child.kill("SIGKILL"); @@ -67,15 +77,16 @@ describe("spawnWithSignalForwarding", () => { }); it( - "forwards SIGTERM to spawned child", + "forwards SIGTERM to the wrapped child", async () => { - const tsxPath = path.resolve(process.cwd(), "node_modules/.bin/tsx"); - const childPath = path.resolve(process.cwd(), "test/fixtures/signal-forwarding/child.ts"); + const childPath = path.resolve(process.cwd(), "test/fixtures/child-process-bridge/child.js"); - const { child } = spawnWithSignalForwarding(tsxPath, [childPath], { + const child = spawn(process.execPath, [childPath], { stdio: ["ignore", "pipe", "inherit"], env: process.env, }); + const { detach } = attachChildProcessBridge(child); + detachments.push(detach); children.push(child); if (!child.stdout) throw new Error("expected stdout"); diff --git a/src/process/child-process-bridge.ts b/src/process/child-process-bridge.ts new file mode 100644 index 000000000..5bad6c2b4 --- /dev/null +++ b/src/process/child-process-bridge.ts @@ -0,0 +1,47 @@ +import type { ChildProcess } from "node:child_process"; +import process from "node:process"; + +export type ChildProcessBridgeOptions = { + signals?: NodeJS.Signals[]; + onSignal?: (signal: NodeJS.Signals) => void; +}; + +const defaultSignals: NodeJS.Signals[] = + process.platform === "win32" + ? ["SIGTERM", "SIGINT", "SIGBREAK"] + : ["SIGTERM", "SIGINT", "SIGHUP", "SIGQUIT"]; + +export function attachChildProcessBridge( + child: ChildProcess, + { signals = defaultSignals, onSignal }: ChildProcessBridgeOptions = {}, +): { detach: () => void } { + const listeners = new Map void>(); + for (const signal of signals) { + const listener = (): void => { + onSignal?.(signal); + try { + child.kill(signal); + } catch { + // ignore + } + }; + try { + process.on(signal, listener); + listeners.set(signal, listener); + } catch { + // Unsupported signal on this platform. + } + } + + const detach = (): void => { + for (const [signal, listener] of listeners) { + process.off(signal, listener); + } + listeners.clear(); + }; + + child.once("exit", detach); + child.once("error", detach); + + return { detach }; +} diff --git a/src/process/spawn-with-signal-forwarding.ts b/src/process/spawn-with-signal-forwarding.ts deleted file mode 100644 index af57a9ce0..000000000 --- a/src/process/spawn-with-signal-forwarding.ts +++ /dev/null @@ -1,40 +0,0 @@ -import type { ChildProcess, SpawnOptions } from "node:child_process"; -import { spawn } from "node:child_process"; -import process from "node:process"; - -export type SpawnWithSignalForwardingOptions = { - signals?: NodeJS.Signals[]; -}; - -export function spawnWithSignalForwarding( - command: string, - args: string[], - options: SpawnOptions, - { signals = ["SIGTERM", "SIGINT", "SIGHUP", "SIGQUIT"] }: SpawnWithSignalForwardingOptions = {}, -): { child: ChildProcess; detach: () => void } { - const child = spawn(command, args, options); - - const listeners = new Map void>(); - for (const signal of signals) { - const listener = (): void => { - try { - child.kill(signal); - } catch { - // ignore - } - }; - listeners.set(signal, listener); - process.on(signal, listener); - } - - const detach = (): void => { - for (const [signal, listener] of listeners) { - process.off(signal, listener); - } - listeners.clear(); - }; - - child.once("exit", detach); - - return { child, detach }; -} diff --git a/test/fixtures/signal-forwarding/child.ts b/test/fixtures/child-process-bridge/child.js similarity index 93% rename from test/fixtures/signal-forwarding/child.ts rename to test/fixtures/child-process-bridge/child.js index a9b55c3fc..3c24f2172 100644 --- a/test/fixtures/signal-forwarding/child.ts +++ b/test/fixtures/child-process-bridge/child.js @@ -11,7 +11,7 @@ server.listen(0, "127.0.0.1", () => { process.stdout.write(`${addr.port}\n`); }); -const shutdown = (): void => { +const shutdown = () => { server.close(() => process.exit(0)); };