diff --git a/src/main/ssh/ssh-relay-orphan-abandon-paths.test.ts b/src/main/ssh/ssh-relay-orphan-abandon-paths.test.ts index 9d08ac7da0f..cab697afd0d 100644 --- a/src/main/ssh/ssh-relay-orphan-abandon-paths.test.ts +++ b/src/main/ssh/ssh-relay-orphan-abandon-paths.test.ts @@ -186,24 +186,24 @@ describe('SshRelaySession abandoned remote PTYs', () => { expect(clearProviderPtyState).not.toHaveBeenCalledWith(APP_PTY_ID) }) - it('retires the lease without a kill when the relay proves the PTY is gone', async () => { - // pty.attach verifies process liveness before answering not-found, so this is the one branch - // with positive proof of death — and a dead process needs no shutdown request. + // INVERTED for STA-3077 step E-0. This case previously asserted that a bare not-found retires the + // lease and reports `pty:exit { code: -1 }`, on the premise that attach proves liveness before + // answering not-found. It does not: the relay answers not-found for a pane-identity mismatch and + // for any id it merely cannot hand back, so the premise licensed a fabricated death certificate. + // The clause is kept — inverted — so the new intent stays covered rather than silently dropped. + it('leaves the shell running when the relay only reports the PTY as not found', async () => { const { deps, shutdown } = await establishWithFailingReattach( new Error('PTY "pty-live" not found') ) expect(shutdown).not.toHaveBeenCalled() - expect(deps.mockStore.markSshRemotePtyLease).toHaveBeenCalledWith( + expect(deps.mockStore.markSshRemotePtyLease).not.toHaveBeenCalledWith( 'target-1', 'pty-live', 'expired' ) - expect(clearProviderPtyState).toHaveBeenCalledWith(APP_PTY_ID) - expect(deps.mockWindow.webContents.send).toHaveBeenCalledWith('pty:exit', { - id: APP_PTY_ID, - code: -1 - }) + expect(clearProviderPtyState).not.toHaveBeenCalledWith(APP_PTY_ID) + expect(deps.mockWindow.webContents.send).not.toHaveBeenCalledWith('pty:exit', expect.anything()) }) it('keeps a recovered session attached when reattach succeeds after an earlier drop', async () => { diff --git a/src/main/ssh/ssh-relay-session-rejected-delivery.test.ts b/src/main/ssh/ssh-relay-session-rejected-delivery.test.ts index e4fbb44142f..90ec6857a46 100644 --- a/src/main/ssh/ssh-relay-session-rejected-delivery.test.ts +++ b/src/main/ssh/ssh-relay-session-rejected-delivery.test.ts @@ -246,7 +246,8 @@ describe('SshRelaySession rejected PTY delivery recovery', () => { const recovery = reattachKnownPty.mock.calls[0]?.[0] expect(recovery?.ptyId).toBe('pty-bad') expect(Array.from(recovery?.activeLeaseByPtyId.keys() ?? [])).toEqual(['pty-bad']) - expect(Array.from(recovery?.expectedIdentityByPtyId.keys() ?? [])).toEqual([]) + // The sibling `expectedIdentityByPtyId` clause is gone with the map itself (STA-3077 step A): + // reattach no longer carries pane identity at all, which subsumes "carries none for this pty". expect(recovery?.targetedDeliveryRecovery).toBe('fresh-activation') expect(deps.mockStore.markSshRemotePtyLeasesAttachedAsync).toHaveBeenCalledWith('target-1', [ 'pty-bad' @@ -356,7 +357,8 @@ describe('SshRelaySession rejected PTY delivery recovery', () => { internals.reattachRejectedPty('pty-bad', mux, 23, 'confirm-existing') ).resolves.toBe(true) - expect(attachForReconnect).toHaveBeenCalledWith('pty-bad', undefined, checkpoint) + // Two args since step A dropped the expected-identity parameter from the attach call. + expect(attachForReconnect).toHaveBeenCalledWith('pty-bad', checkpoint) expect(commit).toHaveBeenCalledOnce() expect(mux.dispose).not.toHaveBeenCalled() }) diff --git a/src/main/ssh/ssh-relay-session.test.ts b/src/main/ssh/ssh-relay-session.test.ts index 7478caa8c48..80ef9857f5e 100644 --- a/src/main/ssh/ssh-relay-session.test.ts +++ b/src/main/ssh/ssh-relay-session.test.ts @@ -648,7 +648,11 @@ describe('SshRelaySession', () => { expect(mockStore.markSshRemotePtyLeasesAttachedAsync).not.toHaveBeenCalled() }) - it('invalidates and broadcasts remote PTYs that cannot reattach after relay reconnect', async () => { + // INVERTED for STA-3077 step E-0. This case previously asserted that a PTY the relay reports as + // not-found is invalidated and broadcast as an exit. A not-found is not proof of death, so the + // broadcast was a fabricated one; what must still hold is that the healthy sibling reattaches and + // the unproven one is left alone rather than written off. + it('leaves an unreattachable remote PTY alone while its sibling reattaches', async () => { const { mockConn, mockStore, mockPortForward, getMainWindow, mockWindow } = createMockDeps() const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) await session.establish(mockConn) @@ -670,12 +674,9 @@ describe('SshRelaySession', () => { expect(mockAttach).toHaveBeenCalledWith('pty-stale') expect(mockAttach).toHaveBeenCalledWith('pty-live') - expect(clearProviderPtyState).toHaveBeenCalledWith('ssh:target-1@@pty-stale') - expect(deletePtyOwnership).toHaveBeenCalledWith('ssh:target-1@@pty-stale') - expect(mockWindow.webContents.send).toHaveBeenCalledWith('pty:exit', { - id: 'ssh:target-1@@pty-stale', - code: -1 - }) + expect(clearProviderPtyState).not.toHaveBeenCalledWith('ssh:target-1@@pty-stale') + expect(deletePtyOwnership).not.toHaveBeenCalledWith('ssh:target-1@@pty-stale') + expect(mockWindow.webContents.send).not.toHaveBeenCalledWith('pty:exit', expect.anything()) }) it('retries transient reattach failure without tearing down provider registration', async () => { diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index ed072b9189b..fcebb46fc9f 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -11,7 +11,7 @@ import { SshPtyProvider } from '../providers/ssh-pty-provider' import type { SshPtyAttachResult } from '../providers/ssh-pty-session-reattach' import type { SshPtyDataCallback, SshPtyExitCallback } from '../providers/ssh-pty-provider-contract' import type { SshPtyRecoveryActivationLease } from '../providers/ssh-pty-notification-routing' -import { isSshPtyIdentityMismatchError, isSshPtyNotFoundError } from '../providers/ssh-pty-errors' +import { isSshPtyNotFoundError } from '../providers/ssh-pty-errors' import { toAppSshPtyId, toRelaySshPtyId } from '../providers/ssh-pty-id' import { SshFilesystemProvider } from '../providers/ssh-filesystem-provider' import { isMethodNotFoundError } from './ssh-filesystem-stream-reader' @@ -2418,7 +2418,7 @@ export class SshRelaySession { if (!shouldContinue()) { return } - this.handlePtyReattachFailure(ptyId, appPtyId, pendingReattach, error) + this.handlePtyReattachFailure(ptyId, pendingReattach, error) } finally { recoveryActivationLease?.retire() sourceActivationLease?.rollback() @@ -2561,40 +2561,19 @@ export class SshRelaySession { private handlePtyReattachFailure( ptyId: string, - appPtyId: string, pending: PendingPtyReattach, error: unknown ): void { - if (!isSshPtyNotFoundError(error)) { - pending.restoreRequired = 'reattachAttemptsExhausted' - this.wakeRecovery(pending) - console.warn( - `[ssh-relay-session] Leaving PTY ${ptyId} detached for ${this.targetId} after bounded reattach attempts failed: ${ - error instanceof Error ? error.message : String(error) - }` - ) - return - } - if (isSshPtyIdentityMismatchError(error)) { - console.warn( - `[ssh-relay-session] Ignoring stale PTY ${ptyId} for ${this.targetId} after relay identity mismatch: ${ - error instanceof Error ? error.message : String(error) - }` - ) - return - } + // No attach failure proves the shell exited — not even a not-found, which the relay also + // returns when it merely cannot hand this id back. Only an exit the relay observed on a live + // stream may report one, so every failure leaves the pane detached and recoverable. + pending.restoreRequired = 'reattachAttemptsExhausted' + this.wakeRecovery(pending) console.warn( - `[ssh-relay-session] Dropping stale PTY ${ptyId} for ${this.targetId} after relay reattach failed: ${ + `[ssh-relay-session] Leaving PTY ${ptyId} detached for ${this.targetId} after reattach failed: ${ error instanceof Error ? error.message : String(error) }` ) - clearProviderPtyState(appPtyId) - deletePtyOwnership(appPtyId) - this.store.markSshRemotePtyLease(this.targetId, ptyId, 'expired') - const win = this.getMainWindow() - if (win && !win.isDestroyed()) { - win.webContents.send('pty:exit', { id: appPtyId, code: -1 }) - } } private async sourceRecoveryRequest(