avoid duplicate exit persistence retries

This commit is contained in:
Brennan Benson
2026-07-14 00:32:27 -07:00
parent 44dc2d4838
commit 31dc093fbb
6 changed files with 50 additions and 14 deletions
+10 -8
View File
@@ -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)
})
+9
View File
@@ -754,6 +754,11 @@ function attemptRetainedPtyShutdown(retained: RetainedPtyShutdown): Promise<void
if (retained.provider !== currentProvider) {
return
}
// Why: an exit callback can settle this owner before shutdown() resolves.
// Its durable retry is already scheduled, so do not settle it twice.
if (retained.providerShutdownComplete) {
return
}
if (providerAlreadyGone) {
retained.providerShutdownComplete = true
retained.providerExitObserved = false
@@ -1615,6 +1620,10 @@ export function releasePendingSshShutdown(id: string): void {
scheduleRetainedShutdownRetries()
}
export function hasPendingSshShutdown(id: string): boolean {
return pendingSshShutdownRetries.has(id)
}
export function _resetRetainedPtyShutdownsForTests(): void {
pendingLocalShutdownRetries.clear()
pendingSshShutdownRetries.clear()
@@ -60,6 +60,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)
@@ -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<typeof vi.fn>
}
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()
})
})
+1
View File
@@ -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)
+9 -6
View File
@@ -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()