From 33cd423d56d31f20aa792d7fcab43c98eb8790c7 Mon Sep 17 00:00:00 2001
From: teknium1 <127238744+teknium1@users.noreply.github.com>
Date: Mon, 14 Sep 2026 21:39:10 -0700
Subject: [PATCH] fix: remote probe watchdog kills the probe's process group,
 not just its child
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The watchdog SIGKILLed only the direct child of the wrapper. A remote
`hermes` launcher that runs the CLI without exec leaves the hung grandchild
alive after its parent dies — exactly the broken-launcher class from
#110478 — so the probe still orphaned a process on the remote. Enabling
job control (`set -m`) around the probe start puts it in its own process
group; the watchdog now kills `-$__htp` first and the direct pid as the
fallback for shells that cannot turn job control on without a tty (dash),
where behaviour is unchanged.

Review finding: watchdog SIGKILLs only its direct child; a non-exec launcher leaves the hung grandchild orphaned.
---
 .../desktop/electron/remote-lifecycle.test.ts |  2 +-
 apps/desktop/electron/ssh-connection.test.ts  | 21 +++++++++++++++++++
 apps/desktop/electron/ssh-connection.ts       |  8 +++++--
 3 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/apps/desktop/electron/remote-lifecycle.test.ts b/apps/desktop/electron/remote-lifecycle.test.ts
index b04ddf5128..44b1af36d6 100644
--- a/apps/desktop/electron/remote-lifecycle.test.ts
+++ b/apps/desktop/electron/remote-lifecycle.test.ts
@@ -1845,7 +1845,7 @@ test('probes run under the remote watchdog so a hung CLI cannot orphan (#110478)
 
   assert.equal(await remoteSupportsSshOwnership(helpSsh, '/x/hermes'), true)
   assert.ok(helpProbe.includes('kill -9'), 'ownership probe wrapped in the remote watchdog')
-  assert.ok(helpProbe.includes('$( ('), 'watchdog nested around the inner serve --help')
+  assert.ok(/\$\(.*\(.*serve --help.*\) <\/dev\/null &/.test(helpProbe), 'watchdog nested around the inner serve --help')
 })
 
 test('cleanupStale escalates to SIGKILL when the backend survives the graceful wait (#91668 quit-during-active-turn)', async () => {
diff --git a/apps/desktop/electron/ssh-connection.test.ts b/apps/desktop/electron/ssh-connection.test.ts
index ae0f6f25c9..5d9b223c87 100644
--- a/apps/desktop/electron/ssh-connection.test.ts
+++ b/apps/desktop/electron/ssh-connection.test.ts
@@ -1122,4 +1122,25 @@ test('withRemoteTimeout kills a hung probe remotely instead of orphaning it (#11
   const { stdout: strays } = await execFileAsync('sh', ['-c', `ps -eo args | grep "[s]leep ${hungSecs}$" || true`])
 
   assert.equal(strays.trim(), '', 'killed probe left no orphan process')
+
+  // … including the grandchild of a launcher that runs the CLI without exec
+  // (the broken-launcher class of #110478). Needs a shell with job control
+  // off a tty; bash has it, dash does not.
+  const bash = await execFileAsync('sh', ['-c', 'command -v bash || true']).then(r => r.stdout.trim())
+
+  if (bash) {
+    const grandSecs = hungSecs + 1
+    const launcher = `sh -c 'sleep ${grandSecs}; echo done'`
+
+    const err2: any = await execFileAsync(bash, ['-c', withRemoteTimeout(launcher, 1)]).then(
+      () => null,
+      e => e
+    )
+
+    assert.ok(err2 && err2.code !== 0, 'hung launcher must exit non-zero')
+
+    const { stdout: grandStrays } = await execFileAsync('sh', ['-c', `ps -eo args | grep "[s]leep ${grandSecs}$" || true`])
+
+    assert.equal(grandStrays.trim(), '', 'watchdog killed the launcher’s grandchild too')
+  }
 })
diff --git a/apps/desktop/electron/ssh-connection.ts b/apps/desktop/electron/ssh-connection.ts
index c4d3469020..0af6437f82 100644
--- a/apps/desktop/electron/ssh-connection.ts
+++ b/apps/desktop/electron/ssh-connection.ts
@@ -348,9 +348,13 @@ function buildInteractiveSshArgs(conn, remoteCwd, connectTimeoutMs?, remoteComma
 function withRemoteTimeout(remoteCommand, timeoutSecs = REMOTE_PROBE_TIMEOUT_SECS) {
   const secs = Number.isFinite(timeoutSecs) && timeoutSecs > 0 ? Math.floor(timeoutSecs) : REMOTE_PROBE_TIMEOUT_SECS
 
+  // Job control (`set -m`) puts the probe in its own process group so the
+  // watchdog can also reach a grandchild left behind by a launcher that runs
+  // the CLI without exec. Shells that cannot enable it without a tty fall
+  // back to killing the direct child.
   return (
-    `(${remoteCommand}) </dev/null & __htp=$!; ` +
-    `(sleep ${secs} </dev/null >/dev/null 2>&1; kill -9 $__htp 2>/dev/null) & __htw=$!; ` +
+    `set -m 2>/dev/null; (${remoteCommand}) </dev/null & __htp=$!; set +m 2>/dev/null; ` +
+    `(sleep ${secs} </dev/null >/dev/null 2>&1; kill -9 -- -$__htp 2>/dev/null; kill -9 $__htp 2>/dev/null) & __htw=$!; ` +
     `wait $__htp; __htrc=$?; ` +
     `kill $__htw 2>/dev/null; wait $__htw 2>/dev/null; ` +
     `exit $__htrc`