From 959616039443e160d389887231dc9fab0ea26ec4 Mon Sep 17 00:00:00 2001 From: Neil Date: Fri, 11 Sep 2026 04:49:30 -0700 Subject: [PATCH] fix(relay): keep a revived pane whose pid only refuses the liveness probe `revive` gated each serialized pane on a hand-rolled `process.kill(pid, 0)` in a bare try/catch, so any refusal retired the pane. EPERM means the process exists under another uid; only ESRCH is evidence of absence. The file already imports `isProcessAlive`, whose ESRCH-only contract `reapPtyProvenExited` documents 450 lines earlier -- this call site just did not use it. Reuse it rather than keeping a second implementation of the same concept. Malformed pids still skip, as before. See docs/reference/ssh-execution-boundary.md. --- src/relay/pty-handler-revive.test.ts | 31 ++++++++++++++++++++++++++++ src/relay/pty-handler.ts | 8 +++---- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/src/relay/pty-handler-revive.test.ts b/src/relay/pty-handler-revive.test.ts index e184cd56817..6dc2a8d4b30 100644 --- a/src/relay/pty-handler-revive.test.ts +++ b/src/relay/pty-handler-revive.test.ts @@ -518,6 +518,37 @@ describe('PtyHandler', () => { expect(JSON.parse(live).map((entry: { id: string }) => entry.id)).toEqual(['pty-21']) }) + // Why: the pid gate is the one place revive turns an observation into "this pane is + // finished". `kill(pid, 0)` answers EPERM when the process exists under another uid, and + // the same ESRCH-only rule `reapPtyProvenExited` applies has to hold here + // (docs/reference/ssh-execution-boundary.md). + it('keeps a pane whose pid refuses the probe and drops only a proven-gone one', async () => { + const state = JSON.stringify([ + { id: 'pty-30', pid: 424242, cols: 80, rows: 24, cwd: LIVE_CWD }, + { id: 'pty-31', pid: 434343, cols: 80, rows: 24, cwd: LIVE_CWD } + ]) + const killSpy = vi.spyOn(process, 'kill').mockImplementation((pid) => { + if (pid === 424242) { + throw Object.assign(new Error('kill EPERM'), { code: 'EPERM' }) + } + if (pid === 434343) { + throw Object.assign(new Error('kill ESRCH'), { code: 'ESRCH' }) + } + return true + }) + try { + await dispatcher.callRequest('pty.revive', { state }) + } finally { + killSpy.mockRestore() + } + + expect(mockPtySpawn).toHaveBeenCalledTimes(1) + const live = (await dispatcher.callRequest('pty.serialize', { + ids: ['pty-30', 'pty-31'] + })) as string + expect(JSON.parse(live).map((entry: { id: string }) => entry.id)).toEqual(['pty-30']) + }) + describe('a Windows relay reviving a WSL pane', () => { const worktreeId = 'r::/remote/wsl-worktree' const historyFile = join( diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index cdf436bca2a..e69e3c1da56 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -2907,10 +2907,10 @@ export class PtyHandler { if (this.ptys.has(entry.id) || this.pendingReviveIds.has(entry.id)) { continue } - // Only re-attach if the original process is still alive - try { - process.kill(entry.pid, 0) - } catch { + // Only re-attach if the host proves the original process is still there. `isProcessAlive` + // is ESRCH-only for the same reason `reapPtyProvenExited` is: a refusal this host cannot + // resolve is unverifiable, not absence (docs/reference/ssh-execution-boundary.md). + if (!Number.isInteger(entry.pid) || entry.pid <= 0 || !isProcessAlive(entry.pid)) { continue } const ownedPath = entry.worktreeId