fix(cron): kill command process trees on timeout

vincentkoc · vincentkoc · commit 1fa471349f8a · 2026-06-08T04:39:34.000+02:00
diff --git a/src/cron/command-runner.test.ts b/src/cron/command-runner.test.ts
@@ -1,3 +1,7 @@
+import fs from "node:fs/promises";
+import os from "node:os";
+import path from "node:path";
+import { setTimeout as delay } from "node:timers/promises";
 import { describe, expect, it } from "vitest";
 import { runCronCommandJob } from "./command-runner.js";
 import type { CronJob } from "./types.js";
@@ -90,6 +94,31 @@ describe("runCronCommandJob", () => {
     });
   });
 
+  it.skipIf(process.platform === "win32")("kills shell process groups on timeout", async () => {
+    const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-cron-command-"));
+    const markerPath = path.join(tempDir, "survived");
+    const childScript = [
+      `setTimeout(() => require("node:fs").writeFileSync(${JSON.stringify(markerPath)}, "alive"), 350)`,
+      "setInterval(() => {}, 1000)",
+    ].join(";");
+    const shellCommand = `${JSON.stringify(process.execPath)} -e ${JSON.stringify(childScript)}`;
+
+    const result = await runCronCommandJob({
+      job: makeCommandJob({
+        kind: "command",
+        argv: ["sh", "-lc", shellCommand],
+        timeoutSeconds: 0.05,
+      }),
+    });
+
+    expect(result.status).toBe("error");
+    expect(result.error).toBe("command timed out");
+
+    await delay(700);
+    await expect(fs.access(markerPath)).rejects.toThrow();
+    await fs.rm(tempDir, { recursive: true, force: true });
+  });
+
   it("marks no-output timeouts as cron errors", async () => {
     const result = await runCronCommandJob({
       job: makeCommandJob({
diff --git a/src/cron/command-runner.ts b/src/cron/command-runner.ts
@@ -116,6 +116,7 @@ export async function runCronCommandJob(params: {
       ...(noOutputTimeoutMs !== undefined ? { noOutputTimeoutMs } : {}),
       ...(payload.outputMaxBytes !== undefined ? { maxOutputBytes: payload.outputMaxBytes } : {}),
       ...(params.abortSignal ? { signal: params.abortSignal } : {}),
+      killProcessTree: true,
     });
     const ok =
       result.code === 0 &&
diff --git a/src/process/exec.ts b/src/process/exec.ts
@@ -256,6 +256,7 @@ export type CommandOptions = {
   noOutputTimeoutMs?: number;
   signal?: AbortSignal;
   maxOutputBytes?: number;
+  killProcessTree?: boolean;
 };
 
 const WINDOWS_CLOSE_STATE_SETTLE_TIMEOUT_MS = 250;
@@ -368,7 +369,8 @@ export async function runCommandWithTimeout(
 ): Promise<SpawnResult> {
   const options: CommandOptions =
     typeof optionsOrTimeout === "number" ? { timeoutMs: optionsOrTimeout } : optionsOrTimeout;
-  const { timeoutMs, cwd, input, baseEnv, env, noOutputTimeoutMs, signal } = options;
+  const { timeoutMs, cwd, input, baseEnv, env, noOutputTimeoutMs, signal, killProcessTree } =
+    options;
   const hasInput = input !== undefined;
   const resolvedEnv = resolveCommandEnv({ argv, baseEnv, env });
   const stdio = resolveCommandStdio({ hasInput, preferInherit: true });
@@ -393,6 +395,9 @@ export async function runCommandWithTimeout(
     stdio,
     cwd,
     env: resolvedEnv,
+    // Cron shell wrappers need their own process group so timeout/abort kills
+    // reach foreground children instead of leaving duplicate scheduled work.
+    ...(killProcessTree && process.platform !== "win32" ? { detached: true } : {}),
     windowsHide: invocation.windowsHide,
     windowsVerbatimArguments: invocation.windowsVerbatimArguments,
     ...(shouldSpawnWithShell({ resolvedCommand: invocation.command, platform: process.platform })
@@ -455,6 +460,19 @@ export async function runCommandWithTimeout(
           // Fall through to Node's direct child kill as a last resort.
         }
       }
+      if (
+        killProcessTree &&
+        process.platform !== "win32" &&
+        typeof child.pid === "number" &&
+        child.pid > 0
+      ) {
+        try {
+          process.kill(-child.pid, "SIGKILL");
+          return;
+        } catch {
+          // Fall through to Node's direct child kill as a last resort.
+        }
+      }
       child.kill("SIGKILL");
     };
 
