fix(ssh): keep recovering a remote pane whose reattach could not reach the host

A rejected delivery frame drives a targeted reattach. When that reattach failed
for any reason other than a host-confirmed absence, the relay recorded the PTY
as unverifiable and then gated the retry on hasPty, which reads false for
`unverifiable` and `exited` alike. The pane's output delivery was abandoned
outright: no retry, no channel-drop escalation, no user-facing signal, while the
remote process kept running.

Gate the retry on the tri-state verdict instead, so only a positive `exited`
verdict stops recovery and an unverifiable reattach still escalates to the
recoverable relay reconnect that restores the pane.

The suite could not see this: the provider doubles answered hasPty only, so they
read every verdict as two-state. They now use a real SshPtyProvider.

src/main/ssh/ssh-relay-session-rejected-delivery.test.ts

@@ -1,6 +1,6 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 import type { SshPtyDataCallback } from '../providers/ssh-pty-provider-contract'
-import type { SshPtyProvider } from '../providers/ssh-pty-provider'
+import { SshPtyProvider } from '../providers/ssh-pty-provider'
 import type { SshChannelMultiplexer } from './ssh-channel-multiplexer'
 import type { SshPtyConsumerSessionState } from './ssh-pty-consumer-session'
 import { SshRelaySession } from './ssh-relay-session'
@@ -113,6 +113,20 @@ function prepareSession() {
   return { deps, internals, mux, session }
 }
 
+// Why a real provider instead of `{ hasPty }`: recovery turns on the tri-state liveness verdict, and
+// a double that only answers hasPty reads `unverifiable` and `exited` alike — the one distinction
+// this path must not collapse.
+function livenessProvider(): SshPtyProvider {
+  const mux = {
+    request: vi.fn().mockResolvedValue(undefined),
+    notify: vi.fn(),
+    onNotification: vi.fn().mockReturnValue(vi.fn()),
+    dispose: vi.fn(),
+    isDisposed: vi.fn().mockReturnValue(false)
+  }
+  return new SshPtyProvider('target-1', mux as never, undefined, 23)
+}
+
 describe('SshRelaySession rejected PTY delivery recovery', () => {
   beforeEach(() => {
     vi.clearAllMocks()
@@ -364,7 +378,9 @@ describe('SshRelaySession rejected PTY delivery recovery', () => {
     const { internals, mux, session } = prepareSession()
     const reattach = vi.fn().mockResolvedValue(false)
     internals.reattachRejectedPty = reattach
-    getSshPtyProviderMock.mockReturnValue({ hasPty: () => true } as unknown as SshPtyProvider)
+    const provider = livenessProvider()
+    provider.acceptLivePty('ssh:target-1@@pty-bad')
+    getSshPtyProviderMock.mockReturnValue(provider)
     const onTerminalError = vi.fn()
     session.setOnTerminalRelayError(onTerminalError)
     const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
@@ -385,13 +401,47 @@ describe('SshRelaySession rejected PTY delivery recovery', () => {
     expect(acceptOutputDataMock).not.toHaveBeenCalled()
   })
 
+  // Why unverifiable must keep recovering: a reattach that cannot reach the host proves nothing
+  // about the remote process, only that this client cannot resume its output. Abandoning recovery
+  // there drops the pane's output silently while the agent keeps running on the host.
+  it('keeps recovering when a failed reattach leaves the rejected PTY unverifiable', async () => {
+    const { internals, mux, session } = prepareSession()
+    const provider = livenessProvider()
+    provider.acceptLivePty('ssh:target-1@@pty-bad')
+    getSshPtyProviderMock.mockReturnValue(provider)
+    // Why: this is what handlePtyReattachFailure records for every non-notFound attach failure.
+    const reattach = vi.fn(async () => {
+      provider.acceptUnverifiablePty('ssh:target-1@@pty-bad')
+      return false
+    })
+    internals.reattachRejectedPty = reattach
+    const onTerminalError = vi.fn()
+    session.setOnTerminalRelayError(onTerminalError)
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
+
+    await internals.acceptPtyData(rejectedPayload())
+    await vi.waitFor(() => expect(mux.dispose).toHaveBeenCalledOnce(), { timeout: 2000 })
+
+    expect(reattach).toHaveBeenCalledTimes(2)
+    expect(mux.dispose).toHaveBeenCalledWith('connection_lost')
+    expect(warn).toHaveBeenCalledWith(
+      expect.stringContaining('PTY pty-bad delivery recovery exhausted')
+    )
+    expect(onTerminalError).not.toHaveBeenCalled()
+    // Why: nothing here is evidence the remote process died, so it must stay unverifiable.
+    await expect(provider.probePtyLiveness('ssh:target-1@@pty-bad')).resolves.toBeNull()
+  })
+
   it('stops without an error when the rejected PTY exited during recovery', async () => {
     const { internals, mux, session } = prepareSession()
     const reattach = vi.fn().mockResolvedValue(false)
     internals.reattachRejectedPty = reattach
     // Why: reattachKnownPty resolves without claiming the lease when the PTY exits mid-attach, so a
-    // plain "not recovered" is indistinguishable from a failure until liveness is checked.
-    getSshPtyProviderMock.mockReturnValue({ hasPty: () => false } as unknown as SshPtyProvider)
+    // plain "not recovered" is indistinguishable from a failure until liveness is checked. Only a
+    // host-confirmed exit stops recovery here, so the provider must carry that exact verdict.
+    const provider = livenessProvider()
+    provider.acceptExitedPty('ssh:target-1@@pty-bad')
+    getSshPtyProviderMock.mockReturnValue(provider)
     const onTerminalError = vi.fn()
     session.setOnTerminalRelayError(onTerminalError)
 
@@ -408,7 +458,11 @@ describe('SshRelaySession rejected PTY delivery recovery', () => {
   // starts rejecting across every PTY at once would otherwise open one attach round trip per PTY.
   it('caps concurrent targeted reattaches and coalesces repeats for one PTY', async () => {
     const { internals } = prepareSession()
-    getSshPtyProviderMock.mockReturnValue({ hasPty: () => true } as unknown as SshPtyProvider)
+    const provider = livenessProvider()
+    for (let index = 0; index < 20; index++) {
+      provider.acceptLivePty(`ssh:target-1@@pty-${index}`)
+    }
+    getSshPtyProviderMock.mockReturnValue(provider)
     vi.spyOn(console, 'warn').mockImplementation(() => {})
     const release: (() => void)[] = []
     let active = 0
@@ -455,7 +509,9 @@ describe('SshRelaySession rejected PTY delivery recovery', () => {
     const { internals, mux, session } = prepareSession()
     const reattach = vi.fn().mockResolvedValue(true)
     internals.reattachRejectedPty = reattach
-    getSshPtyProviderMock.mockReturnValue({ hasPty: () => true } as unknown as SshPtyProvider)
+    const provider = livenessProvider()
+    provider.acceptLivePty('ssh:target-1@@pty-bad')
+    getSshPtyProviderMock.mockReturnValue(provider)
     const onTerminalError = vi.fn()
     session.setOnTerminalRelayError(onTerminalError)
 

src/main/ssh/ssh-relay-session.ts

@@ -1958,14 +1958,14 @@ export class SshRelaySession {
             attempt.attempts = 0
             return
           }
-          this.retryRejectedPtyDelivery(payload, source, appPtyId)
+          void this.retryRejectedPtyDelivery(payload, source, appPtyId)
         },
         (error: unknown) => {
           console.warn(`[ssh-relay-session] PTY ${relayPtyId} targeted delivery recovery failed`, {
             providerGeneration,
             error: error instanceof Error ? error.message : String(error)
           })
-          this.retryRejectedPtyDelivery(payload, source, appPtyId)
+          void this.retryRejectedPtyDelivery(payload, source, appPtyId)
         }
       )
   }
@@ -1973,16 +1973,37 @@ export class SshRelaySession {
   // Why liveness is checked before retrying: reattachKnownPty resolves without claiming the lease
   // when the PTY exited mid-attach, which is indistinguishable from a failed reattach at the call
   // site. Retrying that race twice would drop the relay channel over an ordinary PTY exit.
-  private retryRejectedPtyDelivery(
+  // Why the tri-state verdict and not hasPty: hasPty reads false for `unverifiable` too, and an
+  // unverifiable PTY is precisely the one this retry exists for — a reattach that could not reach
+  // the host is not evidence the pane died. Abandoning it there would drop the pane's output
+  // silently while the remote process keeps running.
+  private async retryRejectedPtyDelivery(
     payload: SshPtyDataPayload,
     source: SshPtyDataPayload['source'],
     appPtyId: string
-  ): void {
+  ): Promise<void> {
+    const mux = this.mux
+    const providerGeneration = this.activePtyProviderGeneration
     const ptyProvider = getSshPtyProvider(this.targetId) as SshPtyProvider | undefined
-    if (!ptyProvider || typeof ptyProvider.hasPty !== 'function' || !ptyProvider.hasPty(appPtyId)) {
+    if (!ptyProvider || typeof ptyProvider.probePtyLiveness !== 'function') {
       this.rejectedPtyRecoveryAttempts.delete(appPtyId)
       return
     }
+    const verdict = await ptyProvider.probePtyLiveness(appPtyId).catch(() => null)
+    if (verdict === false) {
+      this.rejectedPtyRecoveryAttempts.delete(appPtyId)
+      return
+    }
+    // Why re-checked after the probe: teardown clears this retry set, and a timer armed afterwards
+    // would outlive the channel it was recovering.
+    if (
+      !mux ||
+      this.mux !== mux ||
+      mux.isDisposed() ||
+      this.activePtyProviderGeneration !== providerGeneration
+    ) {
+      return
+    }
     const timer = setTimeout(() => {
       this.rejectedPtyRecoveryRetries.delete(timer)
       this.recoverRejectedPtyDelivery(payload, source)