Fix run_command cleanup after shell closes

.memory/run-command-process-cleanup.md

@@ -0,0 +1,25 @@
+# run_command process-group cleanup, issue #424
+
+Unix `run_command` owns its detached process group until requested termination finishes. Shell `close` settles the command only after the SIGTERM-to-SIGKILL cleanup promise completes or the group is confirmed absent. A closing shell alone does not cancel escalation: a redirected background descendant can remain in the group. Successful commands take no cleanup delay, and abort, timeout, output-limit, and spawn-error result formatting stays the same.
+
+The existing coding-tools suite now runs a Node worker that ignores SIGTERM and writes a heartbeat. It covers early shell closure, a resistant foreground process, timeout, and output overflow. The cancellation case fails against `11b58367` because the heartbeat continues after the command settles. The test has fallback PID cleanup and skips Windows, whose process-tree behavior is outside this issue. No Remote, UI, native-client, or onboarding contract changed; no plan status changed.
+
+Local verification on macOS Node 26.10.0: focused process regression, root type-check, focused ESLint, and `npm run test:cli` (99 tests) pass. The full coding-tools suite is 58/59 because an unrelated native-glob comparison fails identically on unchanged production source; its one targeted rerun also fails. Linux verification was unavailable because the configured Docker daemon is stopped.
+
+## Latest-main integration, 2026-10-16
+
+Merged `origin/main` at `ee218d1f` into PR #494 with merge commit `555fdfd26`. The sole conflict was `CodingToolOptions`: retain this PR's test deadline seam and main's native `fileMutatorBinary` together. `package.json` matches main exactly, so its test scripts are preserved. The merged coding-tools file runs both the native edit/write helper and the process-group cleanup.
+
+After building the native file mutator, eight focused `run_command`/`edit_file` cases passed. Full coding-tools passed 66/67; only the pre-existing native-glob symlink comparison failed. Root and CLI type checks, focused ESLint, and the full bundled CLI suite (99/99) passed. Hosted exact-head checks are pending.
+
+Main advanced through the 0.57.0 release while these checks ran. Merged `origin/main` at `515de673` as `7452b181` without conflicts; it changed release metadata and the plan index only. With refreshed root dependencies, the native helper build, eight focused process/edit cases, root and CLI type checks, focused ESLint, and bundled CLI suite (99/99) all passed again. The root `package.json` still matches main exactly. Hosted validation is pending on the pushed integration head.
+
+Merged `origin/main` at `716636cf9` as `a1cdee738` without conflicts. This brings in the Bot Files owner-authority fix (#509) and simulator coordinate assertion fix (#508). `run_command` source and tests, `package.json`, the Remote contract revision (32), native clients, and protocol fixtures are unchanged by this merge. The CLI Bot Files constructor now receives its owner audience while `run_command` remains in the pinned coding-tools path.
+
+Post-merge local checks passed: eight focused process/edit tests; 162 Remote Bot Files/router/service tests (one skip); root and CLI type checks; scoped ESLint; native helper and desktop builds; Android `AidenBotContractTest`; and iOS `AidenRemoteClientTests` on an iPhone 18 Pro simulator. The CLI bundled suite passed 97/99 on its first resource-contended run, with two long-running Bot/codemode cases failing; both passed on one isolated rerun. The focused simulator stream E2E case timed out before its simulator assertion while a model response was still generating, then passed on one rerun. Treat those first-run failures as flakes, not clean first passes. Hosted exact-head validation remains pending.
+
+Merged `origin/main` at `264b537a4` as `f8f420cac` without conflicts. This includes #510 (Bot Web Search), #511 (keyboard focus/input modality), and #512 (Bot message attachments, Remote revision 33). The root `package.json` matches main exactly; revision 33 matches the shared fixture. Relative to current main, this PR still changes only the process cleanup and its test, plus its memory/papercut notes.
+
+On that merge, focused process/edit tests passed 8/8; Remote Bot Files/session/router/protocol tests passed 192/192; Bot authority/application and input-modality tests passed 84/84. Root, CLI, and E2E TypeScript checks, scoped ESLint, CLI bundle build, desktop build, Android `AidenBotContractTest`, and iOS `AidenBotSessionTests` plus `AidenRemoteClientTests` on an iPhone 18 Pro simulator passed. Hosted exact-head CI and reviews are still required before merge.
+
+Merged `origin/main` at `5661bae0e` as `9e694dfba` without conflicts. Main's #496 changes only `device-service.ts`, `local-device-host.ts`, and their lifecycle tests. The agent-device shim that can be placed on `run_command`'s PATH still uses its existing service path; this PR's process cleanup and tests remain the sole production/test delta against main. `package.json` and the Remote revision 33 fixture match main exactly. Local Device Hub service/host tests passed 80/80, focused process/edit tests 8/8, root and CLI type checks, scoped ESLint, desktop build, and `git diff --check` passed. New exact-head hosted CI and reviews remain required.

.papercuts/run-command-process-cleanup.md

@@ -0,0 +1,13 @@
+# run_command cleanup workflow
+
+- The managed worktree started without `node_modules`, so the focused regression could not run until `npm ci --ignore-scripts` populated root dependencies. The install completed in 7 seconds.
+- Node 26.10.0's native `glob` follows the fixture symlink for `**/*`, while the existing `foreground glob agrees with native glob` test expects it not to. The full coding-tools run failed 58/59; one targeted rerun and an unchanged-production baseline both failed the same way. This is separate from process cleanup.
+- Linux verification was unavailable locally: `docker` is installed, but its configured OrbStack daemon socket is not running.
+- The 2026-10-16 main merge conflicted only where #494's test deadline option and main's native file-mutator option were added to the same interface. Keeping both resolved it; the root test script matched main without manual edits.
+- `.memory/` and `.papercuts/` are ignored even for previously committed files; a path-specific `git add` rejected their updates. Stage tracked updates with `git add -u` instead.
+- Main advanced to the 0.57.0 release while the first merged head was being validated. A second ordinary merge and focused rerun were needed before publishing the final exact head.
+- The Android Gradle wrapper lives in `android/`, so an initial repository-root `./gradlew` invocation failed before any build work. The focused test was rerun from `android/`.
+- Running bundled CLI and native iOS/Android builds together saturated the host: the CLI Bot parity and codemode tests took roughly 16 minutes and failed with authority revocation/SIGTERM (exit 143). Both passed on one isolated rerun; the Bot case still took 13 minutes. This appears timing-sensitive and is not evidence that the tests are healthy under load.
+- The simulator stream E2E case first timed out waiting for a saved assistant reply while generation remained active, before reaching the #508 coordinate assertion. It passed on one rerun (14.6 seconds); keep the first failure visible as a flake.
+- Main advanced again after the `716636cf9` integration was published; #510, #511, and #512 required another clean merge and revision-33 contract verification before the PR could receive final exact-head checks.
+- Main advanced once more with #496 after the revision-33 head was pushed, requiring another merge and local Device Hub lifecycle rerun before exact-head validation.

main/services/coding-tools.test.ts

@@ -1601,6 +1601,73 @@ test("run_command cancellation kills the shell process group", async () => {
   }
 });
 
+test("run_command waits for resistant process-group cleanup after cancellation, timeout, and output overflow", async () => {
+  if (process.platform === "win32") return;
+  for (const trigger of ["cancel", "timeout", "output", "foreground"] as const) {
+    const root = await fs.mkdtemp(path.join(os.tmpdir(), `aiden-command-${trigger}-`));
+    const controller = new AbortController();
+    let running: Promise<unknown> | undefined;
+    try {
+      await fs.writeFile(path.join(root, "worker.cjs"), `
+const fs = require("node:fs");
+process.on("SIGTERM", () => {});
+fs.writeFileSync("ready", String(process.pid));
+setInterval(() => fs.appendFileSync("heartbeat", "x"), 25);
+`);
+      if (trigger === "output") {
+        await fs.writeFile(path.join(root, "output.cjs"), 'process.stdout.write("x".repeat(11 * 1024 * 1024));');
+      }
+      const command = trigger === "foreground"
+        ? "node worker.cjs"
+        : trigger === "output"
+          ? "node worker.cjs >/dev/null 2>&1 & while [ ! -f ready ]; do sleep 0.01; done; node output.cjs; wait"
+          : "node worker.cjs >/dev/null 2>&1 & wait";
+      const runCommand = buildCodingTools(root, undefined, {
+        commandTimeoutMs: trigger === "timeout" ? 2_000 : undefined,
+      }).find((tool) => tool.name === "run_command")!;
+      running = runCommand.execute(trigger, { command }, controller.signal);
+
+      let pid = 0;
+      const readyDeadline = Date.now() + 5_000;
+      while (Date.now() < readyDeadline) {
+        pid = Number(await fs.readFile(path.join(root, "ready"), "utf8").catch(() => "0"));
+        const heartbeat = await fs.readFile(path.join(root, "heartbeat")).catch(() => Buffer.alloc(0));
+        if (pid > 0 && heartbeat.length >= 2) break;
+        await new Promise((resolve) => setTimeout(resolve, 25));
+      }
+      assert.ok(pid > 0, `${trigger}: worker did not become ready`);
+      if (trigger === "cancel" || trigger === "foreground") {
+        controller.abort(new Error(`${trigger} cancellation`));
+        await assert.rejects(running, new RegExp(`${trigger} cancellation`, "u"));
+      } else {
+        const result = await running as Awaited<ReturnType<typeof runCommand.execute>>;
+        assert.equal(result.isError, true);
+        assert.match(
+          result.content[0].type === "text" ? result.content[0].text : "",
+          trigger === "timeout" ? /^Command timed out\./u : /^Command exceeded the output limit\./u,
+        );
+      }
+
+      const before = (await fs.readFile(path.join(root, "heartbeat"))).length;
+      await new Promise((resolve) => setTimeout(resolve, 250));
+      const after = (await fs.readFile(path.join(root, "heartbeat"))).length;
+      assert.equal(after, before, `${trigger}: descendant kept writing after the command settled`);
+      const status = await execFileAsync("ps", ["-o", "stat=", "-p", String(pid)])
+        .then(({ stdout }) => stdout.trim())
+        .catch(() => "");
+      assert.ok(!status || status.startsWith("Z"), `${trigger}: descendant is still executing (${status})`);
+    } finally {
+      controller.abort(new Error("fixture cleanup"));
+      await running?.catch(() => {});
+      const pid = Number(await fs.readFile(path.join(root, "ready"), "utf8").catch(() => "0"));
+      if (pid > 0) {
+        try { process.kill(pid, "SIGKILL"); } catch { /* Already exited. */ }
+      }
+      await fs.rm(root, { recursive: true, force: true });
+    }
+  }
+});
+
 test("agent commands find user CLIs from a macOS GUI launch PATH", async () => {
   const home = await fs.mkdtemp(path.join(os.tmpdir(), "aiden-agent-path-"));
   try {

main/services/coding-tools.ts

@@ -1943,6 +1943,8 @@ function makeParentGrep(workspace: WorkspaceRootGuard): AgentTool {
  */
 export interface CodingToolOptions {
   pathPrefix?: string | (() => string | null | undefined);
+  /** A shorter deadline for process-lifecycle tests; production uses the fixed limit. */
+  commandTimeoutMs?: number;
   /** Trusted native helper from the application installation, if packaged outside Electron. */
   fileMutatorBinary?: string;
 }
@@ -2005,6 +2007,18 @@ function makeRunCommand(workspace: WorkspaceRootGuard, options: CodingToolOption
         let settled = false;
         let terminationRequested = false;
         let forceKill: ReturnType<typeof setTimeout> | undefined;
+        let finishTermination: (() => void) | undefined;
+        let terminationCleanup: Promise<void> | undefined;
+
+        const processGroupExists = () => {
+          if (process.platform === "win32" || !child.pid) return false;
+          try {
+            process.kill(-child.pid, 0);
+            return true;
+          } catch (error) {
+            return (error as NodeJS.ErrnoException).code !== "ESRCH";
+          }
+        };
 
         const signalProcess = (processSignal: NodeJS.Signals) => {
           if (process.platform !== "win32" && child.pid) {
@@ -2021,10 +2035,17 @@ function makeRunCommand(workspace: WorkspaceRootGuard, options: CodingToolOption
           if (terminationRequested) return;
           terminationRequested = true;
           signalProcess("SIGTERM");
-          forceKill = setTimeout(() => {
-            if (!settled) signalProcess("SIGKILL");
-          }, 1_000);
-          forceKill.unref?.();
+          if (!child.pid) return;
+          terminationCleanup = new Promise<void>((resolve) => {
+            finishTermination = resolve;
+            forceKill = setTimeout(() => {
+              // The shell can close before its descendants leave the process group.
+              // Keep this timer owned by the group, independent of shell settlement.
+              signalProcess("SIGKILL");
+              finishTermination?.();
+              finishTermination = undefined;
+            }, 1_000);
+          });
         };
         const abort = () => {
           aborted = true;
@@ -2046,30 +2067,39 @@ function makeRunCommand(workspace: WorkspaceRootGuard, options: CodingToolOption
           if (settled) return;
           settled = true;
           clearTimeout(timeout);
-          clearTimeout(forceKill);
           signal?.removeEventListener("abort", abort);
-          reject(error);
+          void (async () => {
+            await terminationCleanup;
+            reject(error);
+          })();
         });
         child.once("close", (exitCode) => {
           if (settled) return;
           settled = true;
           clearTimeout(timeout);
-          clearTimeout(forceKill);
           signal?.removeEventListener("abort", abort);
-          resolve({
-            stdout: Buffer.concat(stdout).toString("utf-8"),
-            stderr: Buffer.concat(stderr).toString("utf-8"),
-            exitCode,
-            timedOut,
-            outputLimitExceeded,
-            aborted,
-          });
+          if (finishTermination && !processGroupExists()) {
+            clearTimeout(forceKill);
+            finishTermination();
+            finishTermination = undefined;
+          }
+          void (async () => {
+            await terminationCleanup;
+            resolve({
+              stdout: Buffer.concat(stdout).toString("utf-8"),
+              stderr: Buffer.concat(stderr).toString("utf-8"),
+              exitCode,
+              timedOut,
+              outputLimitExceeded,
+              aborted,
+            });
+          })();
         });
         signal?.addEventListener("abort", abort, { once: true });
         const timeout = setTimeout(() => {
           timedOut = true;
           terminate();
-        }, COMMAND_TIMEOUT_MS);
+        }, options.commandTimeoutMs ?? COMMAND_TIMEOUT_MS);
         timeout.unref?.();
       });
       if (result.aborted) {