From b657bbdb475078d58773eba239e223efa4fb0fb2 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sat, 25 Apr 2026 18:57:08 -0400 Subject: [PATCH] Invalidate stale backing PTYs --- src/main/daemon/daemon-server.test.ts | 18 ++++ src/main/daemon/daemon-server.ts | 47 +++++++- src/main/ipc/ssh-relay-helpers.test.ts | 144 +++++++++++++++++++++++++ src/main/ipc/ssh-relay-helpers.ts | 17 ++- 4 files changed, 221 insertions(+), 5 deletions(-) create mode 100644 src/main/ipc/ssh-relay-helpers.test.ts diff --git a/src/main/daemon/daemon-server.test.ts b/src/main/daemon/daemon-server.test.ts index b8167948d54..5537782cc36 100644 --- a/src/main/daemon/daemon-server.test.ts +++ b/src/main/daemon/daemon-server.test.ts @@ -183,6 +183,24 @@ describe('DaemonServer', () => { 'Session not found' ) }) + + it('emits exit when a fire-and-forget write targets a missing session', async () => { + await startServer() + const c = await connectClient() + + const exitEvent = new Promise((resolve) => { + c.onEvent((event) => resolve(event)) + }) + + c.notify('write', { sessionId: 'missing-session', data: 'hi' }) + + await expect(exitEvent).resolves.toMatchObject({ + type: 'event', + event: 'exit', + sessionId: 'missing-session', + payload: { code: -1 } + }) + }) }) describe('authentication', () => { diff --git a/src/main/daemon/daemon-server.ts b/src/main/daemon/daemon-server.ts index 781b5386af7..108f78df2b9 100644 --- a/src/main/daemon/daemon-server.ts +++ b/src/main/daemon/daemon-server.ts @@ -4,7 +4,13 @@ import { writeFileSync, chmodSync, unlinkSync } from 'fs' import { encodeNdjson, createNdjsonParser } from './ndjson' import { TerminalHost } from './terminal-host' import type { SubprocessHandle } from './session' -import { PROTOCOL_VERSION, NOTIFY_PREFIX, type HelloMessage, type DaemonRequest } from './types' +import { + PROTOCOL_VERSION, + NOTIFY_PREFIX, + SessionNotFoundError, + type HelloMessage, + type DaemonRequest +} from './types' export type DaemonServerOptions = { socketPath: string @@ -230,11 +236,25 @@ export class DaemonServer { } case 'write': - this.host.write(request.payload.sessionId, request.payload.data) + try { + this.host.write(request.payload.sessionId, request.payload.data) + } catch (err) { + if (err instanceof SessionNotFoundError) { + this.sendExitEvent(client, request.payload.sessionId, -1) + } + throw err + } return {} case 'resize': - this.host.resize(request.payload.sessionId, request.payload.cols, request.payload.rows) + try { + this.host.resize(request.payload.sessionId, request.payload.cols, request.payload.rows) + } catch (err) { + if (err instanceof SessionNotFoundError) { + this.sendExitEvent(client, request.payload.sessionId, -1) + } + throw err + } return {} case 'kill': @@ -274,4 +294,25 @@ export class DaemonServer { throw new Error(`Unknown request type: ${(request as { type: string }).type}`) } } + + private sendExitEvent( + client: ConnectedClient | undefined, + sessionId: string, + code: number + ): void { + if (!client?.streamSocket) { + return + } + // Why: write/resize are notification-heavy and intentionally do not wait + // for replies. If their target session is gone, this synthetic exit is the + // only signal the renderer gets to clear stale terminal pane bindings. + client.streamSocket.write( + encodeNdjson({ + type: 'event', + event: 'exit', + sessionId, + payload: { code } + }) + ) + } } diff --git a/src/main/ipc/ssh-relay-helpers.test.ts b/src/main/ipc/ssh-relay-helpers.test.ts new file mode 100644 index 00000000000..baf855a6630 --- /dev/null +++ b/src/main/ipc/ssh-relay-helpers.test.ts @@ -0,0 +1,144 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { + mockDeployAndLaunchRelay, + mockMux, + mockPtyProvider, + mockRegisterSshPtyProvider, + mockUnregisterSshPtyProvider, + mockGetSshPtyProvider, + mockGetPtyIdsForConnection, + mockClearProviderPtyState, + mockDeletePtyOwnership, + mockRegisterSshFilesystemProvider, + mockUnregisterSshFilesystemProvider, + mockGetSshFilesystemProvider, + mockRegisterSshGitProvider, + mockUnregisterSshGitProvider +} = vi.hoisted(() => ({ + mockDeployAndLaunchRelay: vi.fn(), + mockMux: { + dispose: vi.fn(), + isDisposed: vi.fn().mockReturnValue(false), + notify: vi.fn(), + request: vi.fn(), + onNotification: vi.fn().mockReturnValue(() => {}) + }, + mockPtyProvider: { + attach: vi.fn(), + onData: vi.fn(), + onExit: vi.fn() + }, + mockRegisterSshPtyProvider: vi.fn(), + mockUnregisterSshPtyProvider: vi.fn(), + mockGetSshPtyProvider: vi.fn(), + mockGetPtyIdsForConnection: vi.fn(), + mockClearProviderPtyState: vi.fn(), + mockDeletePtyOwnership: vi.fn(), + mockRegisterSshFilesystemProvider: vi.fn(), + mockUnregisterSshFilesystemProvider: vi.fn(), + mockGetSshFilesystemProvider: vi.fn(), + mockRegisterSshGitProvider: vi.fn(), + mockUnregisterSshGitProvider: vi.fn() +})) + +vi.mock('../ssh/ssh-relay-deploy', () => ({ + deployAndLaunchRelay: mockDeployAndLaunchRelay +})) + +vi.mock('../ssh/ssh-channel-multiplexer', () => ({ + SshChannelMultiplexer: class MockSshChannelMultiplexer { + constructor() { + return mockMux + } + } +})) + +vi.mock('../providers/ssh-pty-provider', () => ({ + SshPtyProvider: class MockSshPtyProvider { + constructor() { + return mockPtyProvider + } + } +})) + +vi.mock('../providers/ssh-filesystem-provider', () => ({ + SshFilesystemProvider: class MockSshFilesystemProvider {} +})) + +vi.mock('../providers/ssh-git-provider', () => ({ + SshGitProvider: class MockSshGitProvider {} +})) + +vi.mock('./pty', () => ({ + registerSshPtyProvider: mockRegisterSshPtyProvider, + unregisterSshPtyProvider: mockUnregisterSshPtyProvider, + getSshPtyProvider: mockGetSshPtyProvider, + getPtyIdsForConnection: mockGetPtyIdsForConnection, + clearPtyOwnershipForConnection: vi.fn(), + clearProviderPtyState: mockClearProviderPtyState, + deletePtyOwnership: mockDeletePtyOwnership +})) + +vi.mock('../providers/ssh-filesystem-dispatch', () => ({ + registerSshFilesystemProvider: mockRegisterSshFilesystemProvider, + unregisterSshFilesystemProvider: mockUnregisterSshFilesystemProvider, + getSshFilesystemProvider: mockGetSshFilesystemProvider +})) + +vi.mock('../providers/ssh-git-dispatch', () => ({ + registerSshGitProvider: mockRegisterSshGitProvider, + unregisterSshGitProvider: mockUnregisterSshGitProvider +})) + +import { reestablishRelayStack } from './ssh-relay-helpers' + +describe('reestablishRelayStack', () => { + const mockWindow = { + isDestroyed: () => false, + webContents: { + send: vi.fn() + } + } + const mockConnection = {} + const mockConnectionManager = { + getConnection: vi.fn() + } + const activeMultiplexers = new Map() + const store = { + getRepos: vi.fn().mockReturnValue([]) + } + + beforeEach(() => { + vi.clearAllMocks() + mockConnectionManager.getConnection.mockReturnValue(mockConnection) + mockDeployAndLaunchRelay.mockResolvedValue({ + transport: { write: vi.fn(), onData: vi.fn(), onClose: vi.fn() } + }) + mockGetPtyIdsForConnection.mockReturnValue([]) + mockPtyProvider.attach.mockResolvedValue(undefined) + activeMultiplexers.clear() + }) + + it('invalidates and broadcasts remote PTYs that cannot reattach after relay reconnect', async () => { + mockGetPtyIdsForConnection.mockReturnValue(['pty-stale']) + mockPtyProvider.attach.mockRejectedValue(new Error('PTY "pty-stale" not found')) + + await reestablishRelayStack( + 'ssh-1', + () => mockWindow as never, + mockConnectionManager as never, + activeMultiplexers, + null, + store as never + ) + + expect(mockPtyProvider.attach).toHaveBeenCalledWith('pty-stale') + expect(mockClearProviderPtyState).toHaveBeenCalledWith('pty-stale') + expect(mockDeletePtyOwnership).toHaveBeenCalledWith('pty-stale') + expect(mockWindow.webContents.send).toHaveBeenCalledWith('pty:exit', { + id: 'pty-stale', + code: -1 + }) + }) +}) diff --git a/src/main/ipc/ssh-relay-helpers.ts b/src/main/ipc/ssh-relay-helpers.ts index 95364c93d52..d66787a944e 100644 --- a/src/main/ipc/ssh-relay-helpers.ts +++ b/src/main/ipc/ssh-relay-helpers.ts @@ -219,8 +219,21 @@ export async function reestablishRelayStack( for (const ptyId of ptyIds) { try { await ptyProvider.attach(ptyId) - } catch { - // PTY may have exited during the disconnect — ignore + } catch (err) { + console.warn( + `[ssh] Dropping stale PTY ${ptyId} for ${targetId} after relay reattach failed: ${ + err instanceof Error ? err.message : String(err) + }` + ) + clearProviderPtyState(ptyId) + deletePtyOwnership(ptyId) + // Why: if the new relay cannot reattach this id, the remote backing + // process is gone. Tell the renderer so it clears stale pane bindings + // instead of keeping a cursor-only terminal that routes input nowhere. + const win = getMainWindow() + if (win && !win.isDestroyed()) { + win.webContents.send('pty:exit', { id: ptyId, code: -1 }) + } } } } catch (err) {