From 3bc6a022c90cdabe447b470ff67930e14533ec78 Mon Sep 17 00:00:00 2001 From: chelsealong Date: Sun, 27 Sep 2026 05:38:47 +0000 Subject: [PATCH] fix(desktop): report the real POSIX drain refusal instead of a generic error The managed-update termination probe (buildOwnedTerminationCommand) signals REFUSED, DARWIN_UNAVAILABLE, UNAVAILABLE, and TIMEOUT via a non-zero process exit code, but SshConnection.exec() throws on any non-zero remote exit before the caller can inspect stdout. terminateOwnedDashboardForUpdate's status classification (result === 'REFUSED' / 'DARWIN_UNAVAILABLE' / ...) therefore never ran for any of those outcomes; every one collapsed into the generic "Could not terminate the Desktop-owned remote serve for update." On a Darwin remote this fires on every managed SSH update, since the script unconditionally refuses to signal there. Align the probe with this file's own established convention (used by buildOwnedStaleTerminationCommand, pidIsOurDashboard, remoteProcessCreationTime): always exit 0 and let stdout carry the status. --- .../desktop/electron/remote-lifecycle.test.ts | 48 +++++++++++++++++++ apps/desktop/electron/remote-lifecycle.ts | 14 +++--- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/apps/desktop/electron/remote-lifecycle.test.ts b/apps/desktop/electron/remote-lifecycle.test.ts index 7ce4dcf388..b8da6875e4 100644 --- a/apps/desktop/electron/remote-lifecycle.test.ts +++ b/apps/desktop/electron/remote-lifecycle.test.ts @@ -1414,6 +1414,54 @@ test('managed update drain never falls back to a bare kill when the identity-bou ) }) +test.skipIf(process.platform === 'win32')( + 'managed update drain reports the real POSIX refusal, not a generic transport error', + async () => { + // Regression for #124617: the termination probe prints REFUSED (and, on + // Darwin, DARWIN_UNAVAILABLE) but used to exit non-zero for it. ssh.exec() + // throws on any non-zero remote exit, so the specific message built below + // from the probe's stdout was dead code in production — every refusal + // collapsed into the generic "Could not terminate the Desktop-owned + // remote serve for update." Run the real probe (not a canned string) + // against a live-but-foreign local process so its actual exit code + // decides the outcome, the same way the real SSH transport does. + const dummy = spawn('sleep', ['5'], { stdio: 'ignore' }) + + try { + const lock = ownedLock({ pid: dummy.pid }) + const rawLock = JSON.stringify(lock) + + const ssh: Pick = { + async exec(cmd: string): Promise { + if (/cat .*lock\.json/.test(cmd)) { + return rawLock + } + + if (/kill -0 /.test(cmd)) { + return 'ALIVE' + } + + if (/fields\[19\]/.test(cmd)) { + return lock.creationTime + } + + if (/print\("OWNED" if ok else "FOREIGN"\)/.test(cmd)) { + return 'OWNED' + } + + // The real termination probe: execute it for real so its actual + // exit code — not a canned string — is what drives the caller. + return (await exec(cmd)).stdout + } + } + + await assert.rejects(terminateOwnedDashboardForUpdate(ssh, lock), /identity changed at the signal boundary/) + } finally { + dummy.kill('SIGKILL') + } + } +) + test('connect() respawns when the dashboard is wedged (alive pid, probe fails)', async () => { const reuseToken = 'stored' const lock = ownedLock({ tokenFingerprint: fingerprintToken(reuseToken) }) diff --git a/apps/desktop/electron/remote-lifecycle.ts b/apps/desktop/electron/remote-lifecycle.ts index 4b734dc805..e331ebcc4f 100644 --- a/apps/desktop/electron/remote-lifecycle.ts +++ b/apps/desktop/electron/remote-lifecycle.ts @@ -940,35 +940,35 @@ def owned(args): pidfd=None if sys.platform.startswith("linux"): if not hasattr(os,"pidfd_open") or not hasattr(signal,"pidfd_send_signal"): - print("UNAVAILABLE");sys.exit(2) + print("UNAVAILABLE");sys.exit(0) try:pidfd=os.pidfd_open(pid,0) except ProcessLookupError:print("ALREADY_STOPPED");sys.exit(0) - except (OSError,PermissionError):print("UNAVAILABLE");sys.exit(2) + except (OSError,PermissionError):print("UNAVAILABLE");sys.exit(0) try: live_creation,live_args=identity_before_signal() if live_creation!=expected_creation or not owned(live_args): - print("REFUSED");sys.exit(3) + print("REFUSED");sys.exit(0) if (sys.platform=="darwin"): # Darwin has no pidfd-style signal binding. Refuse instead of accepting the # residual PID-reuse window between ps and os.kill; reconnect will surface # the still-running remote owner for an explicit retry. - print("DARWIN_UNAVAILABLE");sys.exit(2) + print("DARWIN_UNAVAILABLE");sys.exit(0) try: if pidfd is not None:signal.pidfd_send_signal(pidfd,signal.SIGTERM) else:os.kill(pid,signal.SIGTERM) except ProcessLookupError:print("ALREADY_STOPPED");sys.exit(0) if pidfd is not None: poller=select.poll();poller.register(pidfd,select.POLLIN) - if not poller.poll(10000):print("TIMEOUT");sys.exit(4) + if not poller.poll(10000):print("TIMEOUT");sys.exit(0) else: deadline=time.monotonic()+10 while time.monotonic()