From 1cfa892db1022a94708804320fefa81cc6feceea Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:17:15 -0700 Subject: [PATCH] fix(desktop): make the zsh probe test legs visible and run them on CI The zsh login-shell legs in remote-lifecycle.test.ts and ssh-connection.test.ts silently returned when zsh was missing, and the js-tests runner image ships no zsh, so the #111949 coverage never ran on CI and a wrapper regression stayed green. - js-tests.yml: install zsh on the Linux runner before the checks. - Both legs now report vitest skips ('zsh not installed') instead of passing; the ssh-connection leg is its own test so the skip is visible. - Docs: note the zsh degraded mode (no process-group kill for a hung probe's grandchildren) in the SSH connection guide. --- .github/workflows/js-tests.yml | 7 +++++ .../desktop/electron/remote-lifecycle.test.ts | 10 +++---- apps/desktop/electron/ssh-connection.test.ts | 30 ++++++++++++------- .../user-guide/multi-connection-desktop.md | 5 +++- 4 files changed, 35 insertions(+), 17 deletions(-) diff --git a/.github/workflows/js-tests.yml b/.github/workflows/js-tests.yml index 956908293c..62e3a21e69 100644 --- a/.github/workflows/js-tests.yml +++ b/.github/workflows/js-tests.yml @@ -23,6 +23,13 @@ jobs: node-version: 26 cache: npm + # The desktop SSH watchdog tests run their probe through a real zsh + # login shell (#111949). The ubuntu image ships no zsh and the tests + # skip without it, so install it here or those legs never run on CI. + - name: Install zsh + if: runner.os == 'Linux' + run: sudo apt-get install -y zsh + - name: grab npm 12 run: | # No-op once the bundled npm is already 12.x — saves ~5-15s/job and diff --git a/apps/desktop/electron/remote-lifecycle.test.ts b/apps/desktop/electron/remote-lifecycle.test.ts index 052c743b27..9e5b0f196a 100644 --- a/apps/desktop/electron/remote-lifecycle.test.ts +++ b/apps/desktop/electron/remote-lifecycle.test.ts @@ -1813,17 +1813,17 @@ test('remote SSH ownership capability requires both secure bootstrap flags', asy assert.equal(await remoteSupportsSshOwnership(unsupported, '/x/hermes'), false) }) -test('capability probe survives a zsh login shell on the remote (#111949)', async () => { - if (process.platform === 'win32') { - return - } - +test.skipIf(process.platform === 'win32')('capability probe survives a zsh login shell on the remote (#111949)', async t => { // sshd runs the remote command under the account's LOGIN shell. A bare // `set -m` is fatal in a non-interactive zsh, so the watchdog-wrapped probe // used to return nothing and a current remote was reported as unsupported. const zsh = await exec('command -v zsh || true').then(r => r.stdout.trim()) + // CI installs zsh (js-tests.yml); locally a missing zsh must show as a + // skip, not a pass, or a wrapper regression stays green unnoticed. if (!zsh) { + t.skip('zsh not installed') + return } diff --git a/apps/desktop/electron/ssh-connection.test.ts b/apps/desktop/electron/ssh-connection.test.ts index c8b9e92f97..6da72f6148 100644 --- a/apps/desktop/electron/ssh-connection.test.ts +++ b/apps/desktop/electron/ssh-connection.test.ts @@ -1074,6 +1074,25 @@ test('stopTunnelChild waits for process exit', async () => { assert.equal(stopped, true) }) +test.skipIf(process.platform === 'win32')('withRemoteTimeout runs a healthy probe under a zsh login shell (#111949)', async t => { + // SSH runs the remote command through the account's login shell. In + // non-interactive zsh, a bare `set -m` is fatal, so the wrapper must still + // run a healthy probe rather than reporting the remote as unsupported. + const zsh = await execFileAsync('sh', ['-c', 'command -v zsh || true']).then(r => r.stdout.trim()) + + // CI installs zsh (js-tests.yml); locally a missing zsh must show as a + // skip, not a pass, or a wrapper regression stays green unnoticed. + if (!zsh) { + t.skip('zsh not installed') + + return + } + + const { stdout: zshStdout } = await execFileAsync(zsh, ['-fc', withRemoteTimeout('echo zsh-ok', 5)]) + + assert.equal(zshStdout, 'zsh-ok\n') +}) + test('withRemoteTimeout kills a hung probe remotely instead of orphaning it (#110478)', async () => { if (process.platform === 'win32') { return @@ -1092,17 +1111,6 @@ test('withRemoteTimeout kills a hung probe remotely instead of orphaning it (#11 ) assert.ok(REMOTE_PROBE_TIMEOUT_SECS * 1000 < 20_000, 'remote watchdog fires before the local exec timeout') - // SSH runs the remote command through the account's login shell. In - // non-interactive zsh, a bare `set -m` is fatal, so the wrapper must still - // run a healthy probe rather than reporting the remote as unsupported. - const zsh = await execFileAsync('sh', ['-c', 'command -v zsh || true']).then(r => r.stdout.trim()) - - if (zsh) { - const { stdout: zshStdout } = await execFileAsync(zsh, ['-fc', withRemoteTimeout('echo zsh-ok', 5)]) - - assert.equal(zshStdout, 'zsh-ok\n') - } - // Behavior through a real POSIX shell: healthy output passes through … const healthyStart = Date.now() const { stdout } = await execFileAsync('sh', ['-c', withRemoteTimeout('echo hello', 5)]) diff --git a/website/docs/user-guide/multi-connection-desktop.md b/website/docs/user-guide/multi-connection-desktop.md index 81e45bf801..b0d3aeb8ed 100644 --- a/website/docs/user-guide/multi-connection-desktop.md +++ b/website/docs/user-guide/multi-connection-desktop.md @@ -130,7 +130,10 @@ authentication; manage sign-in from the registered connection controls. - *SSH only:* - **SSH host** — one composite field in `user@host:22` form (user and port optional). Your SSH key is used; the app adopts a dashboard - token over the tunnel. + token over the tunnel. Remote probes run under the account's login + shell; on a `zsh` login shell the probe watchdog cannot kill the whole + process group, so a hung probe's grandchildren may linger on the remote + (bash/sh remotes reap them). 5. Click **Save connection** (or **Cancel**). 6. Click **Test** on the new row and wait for *"Reachable"*.