fix(process): keep signal forwarding through child errors (#126493)

src/fleet/containers.runtime.test.ts

@@ -1,6 +1,14 @@
+import type { ChildProcess } from "node:child_process";
+import { EventEmitter } from "node:events";
 import fs from "node:fs/promises";
 import { beforeEach, describe, expect, it, vi } from "vitest";
 
+const spawnMock = vi.hoisted(() => vi.fn());
+
+vi.mock("node:child_process", () => ({
+  spawn: spawnMock,
+}));
+
 const profileMocks = vi.hoisted(() => ({
   buildCellRunArgs: vi.fn((_profile: unknown, options: { environmentFile: string }) => [
     "run",
@@ -34,6 +42,23 @@ function successfulExecutor() {
   }));
 }
 
+function createStreamChild(pid?: number): ChildProcess {
+  const stdout = Object.assign(new EventEmitter(), {
+    pause: vi.fn(),
+    resume: vi.fn(),
+  });
+  const stderr = Object.assign(new EventEmitter(), {
+    pause: vi.fn(),
+    resume: vi.fn(),
+  });
+  return Object.assign(new EventEmitter(), {
+    pid,
+    stdout,
+    stderr,
+    kill: vi.fn(() => true),
+  }) as unknown as ChildProcess;
+}
+
 describe("fleet container runtime", () => {
   beforeEach(() => {
     vi.clearAllMocks();
@@ -464,6 +489,39 @@ describe("fleet container runtime", () => {
     ).rejects.toThrow(/podman logs failed with signal SIGSEGV/iu);
   });
 
+  it("rejects a log stream spawn error when the child has no pid", async () => {
+    const child = createStreamChild();
+    spawnMock.mockReturnValue(child);
+    const runtime = createFleetContainerRuntime(successfulExecutor());
+
+    const logs = runtime.logs("podman", "cell-acme", { follow: true, redactValues: [] });
+    child.emit("error", new Error("spawn failed"));
+
+    await expect(logs).rejects.toThrow("spawn failed");
+    child.emit("close", -2, null);
+  });
+
+  it("waits for log stream close across repeated operational errors", async () => {
+    const child = createStreamChild(4242);
+    spawnMock.mockReturnValue(child);
+    const runtime = createFleetContainerRuntime(successfulExecutor());
+    let settled = false;
+
+    const logs = runtime
+      .logs("podman", "cell-acme", { follow: true, redactValues: [] })
+      .finally(() => {
+        settled = true;
+      });
+    child.emit("error", new Error("first signal delivery failed"));
+    child.emit("error", new Error("second signal delivery failed"));
+    await Promise.resolve();
+
+    expect(settled).toBe(false);
+
+    child.emit("close", 0, null);
+    await expect(logs).resolves.toBeUndefined();
+  });
+
   it("creates and removes a labeled per-cell network", async () => {
     const executor = successfulExecutor();
     const runtime = createFleetContainerRuntime(executor);

src/fleet/containers.runtime.ts

@@ -521,7 +521,11 @@ const defaultFleetContainerStreamExecutor: FleetContainerStreamExecutor = (
     };
     pipeWithBackpressure(child.stdout, process.stdout, stdout);
     pipeWithBackpressure(child.stderr, process.stderr, stderr);
-    child.once("error", reject);
+    child.on("error", (error) => {
+      if (child.pid === undefined) {
+        reject(error);
+      }
+    });
     child.once("close", (code, signal) => {
       stdout.flush();
       stderr.flush();

src/process/child-process-bridge.test.ts

@@ -0,0 +1,39 @@
+import type { ChildProcess } from "node:child_process";
+import { EventEmitter } from "node:events";
+import { describe, expect, it, vi } from "vitest";
+import { attachChildProcessBridge } from "./child-process-bridge.js";
+
+describe("attachChildProcessBridge", () => {
+  it.each(["exit", "close"] as const)(
+    "keeps forwarding after operational errors until child %s",
+    (terminalEvent) => {
+      const signal: NodeJS.Signals = "SIGTERM";
+      const existingListeners = new Set(process.listeners(signal));
+      const kill = vi.fn(() => true);
+      const child = Object.assign(new EventEmitter(), {
+        pid: 4242,
+        kill,
+      }) as unknown as ChildProcess;
+      child.on("error", () => {});
+
+      const { detach } = attachChildProcessBridge(child, { signals: [signal] });
+      const signalListener = process
+        .listeners(signal)
+        .find((listener) => !existingListeners.has(listener));
+
+      try {
+        expect(signalListener).toBeDefined();
+        child.emit("error", new Error("signal delivery failed"));
+        expect(process.listeners(signal)).toContain(signalListener);
+
+        signalListener?.(signal);
+        expect(kill).toHaveBeenCalledWith(signal);
+
+        child.emit(terminalEvent, 0, null);
+        expect(process.listeners(signal)).not.toContain(signalListener);
+      } finally {
+        detach();
+      }
+    },
+  );
+});

src/process/child-process-bridge.ts

@@ -13,7 +13,7 @@ const defaultSignals: NodeJS.Signals[] =
     ? ["SIGTERM", "SIGINT", "SIGBREAK"]
     : ["SIGTERM", "SIGINT", "SIGHUP", "SIGQUIT"];
 
-/** Forwards process termination signals to a child and detaches on child exit/error. */
+/** Forwards process termination signals to a child and detaches on terminal lifecycle events. */
 export function attachChildProcessBridge(
   child: ChildProcess,
   { signals = defaultSignals, onSignal }: ChildProcessBridgeOptions = {},
@@ -43,8 +43,10 @@ export function attachChildProcessBridge(
     listeners.clear();
   };
 
+  // Child errors can report failed signal/IPC operations while the PID stays live.
+  // Keep forwarding until exit, with close covering failed spawn and final handle cleanup.
   child.once("exit", detach);
-  child.once("error", detach);
+  child.once("close", detach);
 
   return { detach };
 }

src/process/respawn-child-runner.test.ts

@@ -205,4 +205,68 @@ describe("runRespawnChildWithSignalBridge", () => {
       },
     );
   });
+
+  it("settles a spawn error when the child has no pid", () => {
+    const { child } = createChild();
+    const onError = vi.fn();
+    const exit = vi.fn();
+
+    runRespawnChildWithSignalBridge({
+      command: "missing-command",
+      args: [],
+      env: {},
+      runtime: {
+        spawn: vi.fn(() => child) as unknown as typeof spawn,
+        attachChildProcessBridge: vi.fn(),
+        exit: exit as unknown as (code?: number) => never,
+      },
+      onError,
+    });
+
+    const error = new Error("spawn failed");
+    child.emit("error", error);
+
+    expect(onError).toHaveBeenCalledWith(error);
+    expect(exit).toHaveBeenCalledWith(1);
+  });
+
+  it("keeps escalation active across repeated operational errors", () => {
+    vi.useFakeTimers();
+    const { child, kill } = createChild(5678);
+    const onError = vi.fn();
+    const exit = vi.fn();
+    let onSignal: ((signal: NodeJS.Signals) => void) | undefined;
+
+    try {
+      runRespawnChildWithSignalBridge({
+        command: "/usr/bin/node",
+        args: ["/repo/openclaw/dist/entry.js"],
+        env: {},
+        runtime: {
+          spawn: vi.fn(() => child) as unknown as typeof spawn,
+          attachChildProcessBridge: vi.fn((_child, options) => {
+            onSignal = options?.onSignal;
+            return { detach: vi.fn() };
+          }),
+          exit: exit as unknown as (code?: number) => never,
+        },
+        onError,
+      });
+
+      onSignal?.("SIGTERM");
+      child.emit("error", new Error("first signal delivery failed"));
+      child.emit("error", new Error("second signal delivery failed"));
+      vi.advanceTimersByTime(2_000);
+
+      expect(onError).not.toHaveBeenCalled();
+      expect(exit).not.toHaveBeenCalled();
+      expect(kill).toHaveBeenNthCalledWith(1, "SIGTERM");
+      expect(kill).toHaveBeenNthCalledWith(2, process.platform === "win32" ? "SIGTERM" : "SIGKILL");
+
+      child.emit("exit", null, "SIGKILL");
+      expect(exit).toHaveBeenCalledWith(1);
+    } finally {
+      vi.useRealTimers();
+    }
+  });
 });

src/process/respawn-child-runner.ts

@@ -120,7 +120,10 @@ export function runRespawnChildWithSignalBridge(params: {
     runtime.exit(code ?? 1);
   });
 
-  child.once("error", (error) => {
+  child.on("error", (error) => {
+    if (child.pid !== undefined) {
+      return;
+    }
     clearSignalTimers();
     onError(error);
     runtime.exit(1);

src/tui/tui-launch.test.ts

@@ -21,8 +21,8 @@ import { launchTuiCli } from "./tui-launch.js";
 const originalArgv = [...process.argv];
 const originalExecArgv = [...process.execArgv];
 
-function createChildProcess(): ChildProcess {
-  return new EventEmitter() as ChildProcess;
+function createChildProcess(pid?: number): ChildProcess {
+  return Object.assign(new EventEmitter(), { pid }) as ChildProcess;
 }
 
 function expectSpawned(expectedArgs: string[]): SpawnOptions {
@@ -187,4 +187,37 @@ describe("launchTuiCli", () => {
     ]);
     expect(options.env).toBe(process.env);
   });
+
+  it("rejects a spawn error when the child has no pid", async () => {
+    const child = createChildProcess();
+    spawnMock.mockImplementation(() => {
+      queueMicrotask(() => child.emit("error", new Error("spawn failed")));
+      return child;
+    });
+
+    await expect(launchTuiCli({ deliver: false })).rejects.toThrow(
+      "failed to launch TUI: spawn failed",
+    );
+    expect(detachMock).toHaveBeenCalledOnce();
+  });
+
+  it("waits for terminal exit across repeated operational errors", async () => {
+    const child = createChildProcess(4242);
+    spawnMock.mockReturnValue(child);
+    let settled = false;
+
+    const launched = launchTuiCli({ deliver: false }).finally(() => {
+      settled = true;
+    });
+    child.emit("error", new Error("first signal delivery failed"));
+    child.emit("error", new Error("second signal delivery failed"));
+    await Promise.resolve();
+
+    expect(settled).toBe(false);
+    expect(detachMock).not.toHaveBeenCalled();
+
+    child.emit("exit", 0, null);
+    await expect(launched).resolves.toBeUndefined();
+    expect(detachMock).toHaveBeenCalledOnce();
+  });
 });

src/tui/tui-launch.ts

@@ -59,7 +59,10 @@ export async function launchTuiCli(opts: TuiOptions): Promise<void> {
     });
     const { detach } = attachChildProcessBridge(child);
 
-    child.once("error", (error) => {
+    child.on("error", (error) => {
+      if (child.pid !== undefined) {
+        return;
+      }
       detach();
       reject(new Error(`failed to launch TUI: ${formatErrorMessage(error)}`));
     });