From 035ee3ca63c52847fa314e2277179099e90a7d33 Mon Sep 17 00:00:00 2001
From: A <258483684+la14-1@users.noreply.github.com>
Date: Tue, 17 Mar 2026 22:54:38 -0700
Subject: [PATCH] fix(ssh): always escalate to SIGKILL in killWithTimeout
 (#2752)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

proc.killed is true as soon as kill() is called, not when the process
exits. This meant SIGKILL escalation was always skipped, leaving stuck
processes hanging indefinitely. Remove the faulty guard and always
attempt SIGKILL after the grace period — try/catch handles already-dead
processes.

Co-authored-by: Claude <claude@anthropic.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 .../src/__tests__/kill-with-timeout.test.ts   | 54 +++++++++++++++++++
 packages/cli/src/shared/ssh.ts                |  7 +--
 2 files changed, 55 insertions(+), 6 deletions(-)
 create mode 100644 packages/cli/src/__tests__/kill-with-timeout.test.ts

diff --git a/packages/cli/src/__tests__/kill-with-timeout.test.ts b/packages/cli/src/__tests__/kill-with-timeout.test.ts
new file mode 100644
index 00000000..39824df9
--- /dev/null
+++ b/packages/cli/src/__tests__/kill-with-timeout.test.ts
@@ -0,0 +1,54 @@
+import { describe, expect, it } from "bun:test";
+import { killWithTimeout } from "../shared/ssh";
+
+describe("killWithTimeout", () => {
+  it("sends SIGKILL after grace period if process ignores SIGTERM", async () => {
+    const signals: (number | undefined)[] = [];
+    const proc = {
+      kill(signal?: number) {
+        signals.push(signal);
+      },
+    };
+
+    killWithTimeout(proc, 100);
+    await new Promise((r) => setTimeout(r, 200));
+
+    expect(signals).toEqual([
+      undefined,
+      9,
+    ]);
+  });
+
+  it("does not throw if process is already dead when SIGKILL fires", async () => {
+    let callCount = 0;
+    const proc = {
+      kill(signal?: number) {
+        callCount++;
+        if (callCount > 1) {
+          throw new Error("No such process");
+        }
+      },
+    };
+
+    killWithTimeout(proc, 50);
+    await new Promise((r) => setTimeout(r, 150));
+
+    // Should not throw — the error is caught internally
+    expect(callCount).toBe(2);
+  });
+
+  it("does not send SIGKILL if initial SIGTERM throws", async () => {
+    const signals: (number | undefined)[] = [];
+    const proc = {
+      kill(_signal?: number) {
+        throw new Error("No such process");
+      },
+    };
+
+    killWithTimeout(proc, 50);
+    await new Promise((r) => setTimeout(r, 150));
+
+    // No signals recorded because the first kill() threw
+    expect(signals).toEqual([]);
+  });
+});
diff --git a/packages/cli/src/shared/ssh.ts b/packages/cli/src/shared/ssh.ts
index bf1538fa..26e639e1 100644
--- a/packages/cli/src/shared/ssh.ts
+++ b/packages/cli/src/shared/ssh.ts
@@ -112,7 +112,6 @@ export function sleep(ms: number): Promise<void> {
 export function killWithTimeout(
   proc: {
     kill(signal?: number): void;
-    readonly killed: boolean;
   },
   gracePeriodMs = 5000,
 ): void {
@@ -121,11 +120,7 @@ export function killWithTimeout(
     return;
   }
   const sigkillTimer = setTimeout(() => {
-    tryCatch(() => {
-      if (!proc.killed) {
-        proc.kill(9);
-      }
-    });
+    tryCatch(() => proc.kill(9));
   }, gracePeriodMs);
   // Don't let this timer keep the event loop alive — the process may already
   // be dead from SIGTERM, so there's no reason to block exit for 5 seconds.