From fc644450c9be65bb413d2f0e14ca2b86a1a5e1ab Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 30 May 2026 09:34:22 -0700 Subject: [PATCH] fix: clean up ssh browse channel listeners (#3769) --- src/main/ipc/ssh-browse.test.ts | 27 ++++++++++++++ src/main/ipc/ssh-browse.ts | 62 ++++++++++++++++++++++++++------- 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/src/main/ipc/ssh-browse.test.ts b/src/main/ipc/ssh-browse.test.ts index 4d288be2394..76de8d30f5a 100644 --- a/src/main/ipc/ssh-browse.test.ts +++ b/src/main/ipc/ssh-browse.test.ts @@ -59,6 +59,12 @@ describe('registerSshBrowseHandler', () => { ] }) expect(exec).toHaveBeenCalledWith('cd "$HOME" && pwd && command ls -1Ap') + expect(channel.listenerCount('data')).toBe(0) + expect(channel.listenerCount('exit')).toBe(0) + expect(channel.listenerCount('close')).toBe(0) + expect(channel.listenerCount('error')).toBe(0) + expect(channel.stderr.listenerCount('data')).toBe(0) + expect(channel.stderr.listenerCount('error')).toBe(0) }) it('escapes remote browse paths before invoking command ls', async () => { @@ -81,4 +87,25 @@ describe('registerSshBrowseHandler', () => { }) expect(exec).toHaveBeenCalledWith("cd '/tmp/it'\\''s here' && pwd && command ls -1Ap") }) + + it('rejects and detaches listeners when the browse channel errors', async () => { + const channel = createMockChannel() + const exec = vi.fn().mockResolvedValue(channel) + const getConnectionManager = () => ({ + getConnection: () => ({ exec }) + }) + registerSshBrowseHandler(getConnectionManager as never) + + const resultPromise = handler(null, { targetId: 'ssh-1', dirPath: '/tmp' }) + await Promise.resolve() + channel.emit('error', new Error('remote disconnected')) + + await expect(resultPromise).rejects.toThrow('remote disconnected') + expect(channel.listenerCount('data')).toBe(0) + expect(channel.listenerCount('exit')).toBe(0) + expect(channel.listenerCount('close')).toBe(0) + expect(channel.listenerCount('error')).toBe(0) + expect(channel.stderr.listenerCount('data')).toBe(0) + expect(channel.stderr.listenerCount('error')).toBe(0) + }) }) diff --git a/src/main/ipc/ssh-browse.ts b/src/main/ipc/ssh-browse.ts index 78a7eb1ea94..d75732f76e0 100644 --- a/src/main/ipc/ssh-browse.ts +++ b/src/main/ipc/ssh-browse.ts @@ -44,19 +44,48 @@ export function registerSshBrowseHandler( let stdout = '' let stderr = '' let exitCode: number | null = null + let settled = false - channel.on('data', (data: Buffer) => { + const cleanup = (): void => { + channel.off('data', onStdoutData) + channel.stderr.off('data', onStderrData) + channel.off('exit', onExit) + channel.off('close', onClose) + channel.off('error', onError) + channel.stderr.off('error', onError) + } + const rejectOnce = (error: Error): void => { + if (settled) { + return + } + settled = true + cleanup() + reject(error) + } + const resolveOnce = (result: { entries: RemoteDirEntry[]; resolvedPath: string }): void => { + if (settled) { + return + } + settled = true + cleanup() + resolve(result) + } + + const onStdoutData = (data: Buffer): void => { stdout += data.toString() - }) - channel.stderr.on('data', (data: Buffer) => { + } + const onStderrData = (data: Buffer): void => { stderr += data.toString() - }) + } // `exit` fires before `close`; capture the code so we can distinguish // a failed `ls` that still produced `pwd` output from an empty listing. - channel.on('exit', (code: number | null) => { + const onExit = (code: number | null): void => { exitCode = code - }) - channel.on('close', () => { + } + const onError = (error: Error): void => { + rejectOnce(error) + } + const onClose = (): void => { // A null exitCode means the server closed the channel without // sending an exit-status message (or signalled termination). We // can't assume success — falling back to "empty stdout = empty @@ -70,17 +99,17 @@ export function registerSshBrowseHandler( (exitCode === null ? 'Remote listing failed (channel closed without exit status)' : `Remote listing failed (exit ${exitCode})`) - reject(new Error(msg)) + rejectOnce(new Error(msg)) return } if (stderr.trim() && !stdout.trim()) { - reject(new Error(stderr.trim())) + rejectOnce(new Error(stderr.trim())) return } const lines = stdout.trim().split('\n') if (lines.length === 0) { - reject(new Error('Empty response from remote')) + rejectOnce(new Error('Empty response from remote')) return } @@ -107,8 +136,17 @@ export function registerSshBrowseHandler( return a.name.localeCompare(b.name) }) - resolve({ entries, resolvedPath }) - }) + resolveOnce({ entries, resolvedPath }) + } + + channel.on('data', onStdoutData) + channel.stderr.on('data', onStderrData) + channel.on('exit', onExit) + channel.on('close', onClose) + // Why: SSH exec streams emit `error` on transport loss; without a + // scoped listener, a disappearing remote can become process-fatal. + channel.on('error', onError) + channel.stderr.on('error', onError) }) } )