From f88065b57c4367c7d5eecf62139f873d8cb3f386 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 20 Aug 2026 03:35:04 -0700 Subject: [PATCH] refactor(ssh): drop the ambiguous-exit set no liveness verdict reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The set was written, deleted, cleared and capped, but nothing ever read it, so its bound guarded nothing and its guarding test passed with the whole set gone. acceptUnverifiableExit keeps what does matter — refusing the live claims an open evidence window could still settle after the exit — and the test now asserts the verdict itself stays unverifiable rather than counting entries. --- src/main/providers/ssh-pty-liveness-state.ts | 14 ++------------ .../ssh-pty-restore-required-reattach.test.ts | 14 ++++++-------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/src/main/providers/ssh-pty-liveness-state.ts b/src/main/providers/ssh-pty-liveness-state.ts index 23eec1876ce..83f625ebe38 100644 --- a/src/main/providers/ssh-pty-liveness-state.ts +++ b/src/main/providers/ssh-pty-liveness-state.ts @@ -1,5 +1,4 @@ export const MAX_SSH_PTY_EXIT_TOMBSTONES = 1000 -export const MAX_SSH_PTY_AMBIGUOUS_EXIT_STATES = 1000 export type SshPtyLiveEvidence = { valid: boolean } export type SshPtyLiveEvidenceWindow = { @@ -10,7 +9,6 @@ export type SshPtyLiveEvidenceWindow = { export class SshPtyLivenessState { readonly livePtyIds = new Set() private readonly exitedPtyIds = new Set() - private readonly ambiguousExitPtyIds = new Set() private readonly pendingLiveEvidenceByPtyId = new Map>() private readonly liveEvidenceWindows = new Set() @@ -30,7 +28,6 @@ export class SshPtyLivenessState { this.liveEvidenceWindows.clear() this.livePtyIds.clear() this.exitedPtyIds.clear() - this.ambiguousExitPtyIds.clear() } probe(id: string): boolean | null { @@ -39,7 +36,6 @@ export class SshPtyLivenessState { acceptLive(id: string): void { const appPtyId = this.toAppPtyId(id) - this.ambiguousExitPtyIds.delete(appPtyId) this.exitedPtyIds.delete(appPtyId) this.livePtyIds.add(appPtyId) } @@ -96,7 +92,6 @@ export class SshPtyLivenessState { acceptExited(id: string): void { const appPtyId = this.toAppPtyId(id) - this.ambiguousExitPtyIds.delete(appPtyId) this.refuseLiveEvidenceAfterExit(appPtyId) this.livePtyIds.delete(appPtyId) this.exitedPtyIds.delete(appPtyId) @@ -109,17 +104,12 @@ export class SshPtyLivenessState { } } + // Why not a tombstone: an exit nobody could attribute leaves the verdict unverifiable. It differs + // from acceptUnverifiable only in also refusing the live claims an open window could still settle. acceptUnverifiableExit(id: string): void { const appPtyId = this.toAppPtyId(id) this.acceptUnverifiable(appPtyId) this.refuseLiveEvidenceAfterExit(appPtyId) - this.ambiguousExitPtyIds.add(appPtyId) - if (this.ambiguousExitPtyIds.size > MAX_SSH_PTY_AMBIGUOUS_EXIT_STATES) { - const oldest = this.ambiguousExitPtyIds.values().next().value - if (oldest !== undefined) { - this.ambiguousExitPtyIds.delete(oldest) - } - } } // Why: an exit the owning host reported outranks every live claim an open window can still diff --git a/src/main/providers/ssh-pty-restore-required-reattach.test.ts b/src/main/providers/ssh-pty-restore-required-reattach.test.ts index a3eb8c9d85f..eefa8f0eae9 100644 --- a/src/main/providers/ssh-pty-restore-required-reattach.test.ts +++ b/src/main/providers/ssh-pty-restore-required-reattach.test.ts @@ -4,10 +4,7 @@ import { SSH_PTY_RESTORE_REQUIRED_ERROR, SSH_SESSION_EXPIRED_ERROR } from './ssh-pty-errors' -import { - MAX_SSH_PTY_AMBIGUOUS_EXIT_STATES, - MAX_SSH_PTY_EXIT_TOMBSTONES -} from './ssh-pty-liveness-state' +import { MAX_SSH_PTY_EXIT_TOMBSTONES } from './ssh-pty-liveness-state' import { SshPtyProvider } from './ssh-pty-provider' // The relay answers `restoreRequired` from its DELIVERY layer only: `requireRestore` @@ -174,15 +171,16 @@ describe('SSH PTY reattach when the relay requires source restoration', () => { ).resolves.toBe(false) }) - it('bounds ambiguous exit retention without disturbing the other panes on the host', async () => { + it('leaves an unattributable exit unverifiable without disturbing the other panes', async () => { const mux = createMockMux() const provider = new SshPtyProvider('conn-1', mux as never) provider.acceptLivePty('ssh:conn-1@@pty-live') + provider.acceptLivePty('ssh:conn-1@@pty-ambiguous') - for (let index = 0; index <= MAX_SSH_PTY_AMBIGUOUS_EXIT_STATES; index += 1) { - provider.acceptAmbiguousExitPty(`pty-${index}`) - } + provider.acceptAmbiguousExitPty('pty-ambiguous') + // Why not `false`: no host attributed this exit to the pane, so it is not evidence of death. + await expect(provider.probePtyLiveness('ssh:conn-1@@pty-ambiguous')).resolves.toBeNull() expect(mux.dispose).not.toHaveBeenCalled() await expect(provider.probePtyLiveness('ssh:conn-1@@pty-live')).resolves.toBe(true) })