From 31dc093fbb271f1e317283ac1015f14f46e8eae3 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 14 Jul 2026 00:32:27 -0700 Subject: [PATCH] avoid duplicate exit persistence retries --- src/main/ipc/pty.test.ts | 18 +++++++++-------- src/main/ipc/pty.ts | 9 +++++++++ .../ssh/ssh-relay-generation-reattach.test.ts | 1 + .../ssh-relay-session-terminal-error.test.ts | 20 +++++++++++++++++++ src/main/ssh/ssh-relay-session.test.ts | 1 + src/main/ssh/ssh-relay-session.ts | 15 ++++++++------ 6 files changed, 50 insertions(+), 14 deletions(-) diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 950dafd2da8..c877e7927c1 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -3615,7 +3615,11 @@ describe('registerPtyHandlers', () => { it('keeps exit fanout live while durable removal retries', async () => { vi.useFakeTimers() const exitListeners = new Set<(payload: { id: string; code: number }) => void>() - const shutdown = vi.fn().mockResolvedValue(undefined) + const shutdown = vi.fn(async () => { + for (const listener of exitListeners) { + listener({ id: 'local-exit', code: 0 }) + } + }) const remove = vi .fn() .mockImplementationOnce(() => { @@ -3641,16 +3645,14 @@ describe('registerPtyHandlers', () => { removePendingLocalPtyShutdown: remove } as never) - await expect(handlers.get('pty:kill')!(null, { id: 'local-exit' })).resolves.toBeUndefined() - expect(() => { - for (const listener of exitListeners) { - listener({ id: 'local-exit', code: 0 }) - } - }).not.toThrow() + await expect(handlers.get('pty:kill')!(null, { id: 'local-exit' })).rejects.toThrow( + 'exit intent removal disk full' + ) expect(runtime.onPtyExit).toHaveBeenCalledWith('local-exit', 0) + // The synchronous exit callback owns settlement while shutdown is in flight. expect(remove).toHaveBeenCalledOnce() - await vi.advanceTimersByTimeAsync(500) + await vi.advanceTimersByTimeAsync(250) expect(shutdown).toHaveBeenCalledOnce() expect(remove).toHaveBeenCalledTimes(2) }) diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 5f57139f153..244bf5533e8 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -754,6 +754,11 @@ function attemptRetainedPtyShutdown(retained: RetainedPtyShutdown): Promise ({ clearPtyOwnershipForConnection: vi.fn(), clearProviderPtyState: vi.fn(), deletePtyOwnership: vi.fn(), + hasPendingSshShutdown: vi.fn().mockReturnValue(false), releasePendingSshShutdown: vi.fn(), setPtyOwnership: vi.fn(), answerStartupTerminalColorQueriesForPty: vi.fn((_id: string, data: string) => data) diff --git a/src/main/ssh/ssh-relay-session-terminal-error.test.ts b/src/main/ssh/ssh-relay-session-terminal-error.test.ts index 8f573a8fcf3..7f0d0dee79c 100644 --- a/src/main/ssh/ssh-relay-session-terminal-error.test.ts +++ b/src/main/ssh/ssh-relay-session-terminal-error.test.ts @@ -58,6 +58,7 @@ vi.mock('../ipc/pty', () => ({ clearPtyOwnershipForConnection: vi.fn(), clearProviderPtyState: vi.fn(), deletePtyOwnership: vi.fn(), + hasPendingSshShutdown: vi.fn().mockReturnValue(false), releasePendingSshShutdown: vi.fn(), setPtyOwnership: vi.fn(), answerStartupTerminalColorQueriesForPty: vi.fn((_id: string, data: string) => data) @@ -203,4 +204,23 @@ describe('SshRelaySession terminal relay error (RelayVersionMismatchError)', () code: 0 }) }) + + it('leaves SSH exit persistence to an existing retry owner', async () => { + const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps() + const { hasPendingSshShutdown, registerSshPtyProvider } = await import('../ipc/pty') + vi.mocked(hasPendingSshShutdown).mockReturnValue(true) + const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) + await session.establish(mockConn) + const ptyProvider = vi.mocked(registerSshPtyProvider).mock.calls[0]?.[1] as unknown as { + onExit: ReturnType + } + const onExit = ptyProvider.onExit.mock.calls[0]?.[0] as (payload: { + id: string + code: number + }) => void + + onExit({ id: 'ssh-pty-1', code: 0 }) + + expect(mockStore.markSshRemotePtyLease).not.toHaveBeenCalled() + }) }) diff --git a/src/main/ssh/ssh-relay-session.test.ts b/src/main/ssh/ssh-relay-session.test.ts index b124ba6861f..b74df517b87 100644 --- a/src/main/ssh/ssh-relay-session.test.ts +++ b/src/main/ssh/ssh-relay-session.test.ts @@ -73,6 +73,7 @@ vi.mock('../ipc/pty', () => ({ clearPtyOwnershipForConnection: vi.fn(), clearProviderPtyState: vi.fn(), deletePtyOwnership: vi.fn(), + hasPendingSshShutdown: vi.fn().mockReturnValue(false), releasePendingSshShutdown: vi.fn(), setPtyOwnership: vi.fn(), answerStartupTerminalColorQueriesForPty: vi.fn((_id: string, data: string) => data) diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index 0d8e1b9075e..65fff208095 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -41,6 +41,7 @@ import { clearPtyOwnershipForConnection, clearProviderPtyState, deletePtyOwnership, + hasPendingSshShutdown, releasePendingSshShutdown, setPtyOwnership, answerStartupTerminalColorQueriesForPty @@ -1168,12 +1169,14 @@ export class SshRelaySession { clearProviderPtyState(payload.id) deletePtyOwnership(payload.id) this.forwardedReattachReplayByPty.delete(payload.id) - try { - this.store.markSshRemotePtyLease(this.targetId, payload.id, 'terminated') - } catch (error) { - // Why: disk failure must not abort PTY exit delivery; a retained kill - // owner, when present, retries the durable lease transition separately. - console.warn('[ssh-relay-session] Failed to persist PTY exit:', error) + if (!hasPendingSshShutdown(payload.id)) { + try { + this.store.markSshRemotePtyLease(this.targetId, payload.id, 'terminated') + } catch (error) { + // Why: disk failure must not abort PTY exit delivery; a retained kill + // owner, when present, retries the durable lease transition separately. + console.warn('[ssh-relay-session] Failed to persist PTY exit:', error) + } } this.runtime?.onPtyExit(payload.id, payload.code) const win = this.getMainWindow()