diff --git a/src/main/providers/ssh-pty-errors.ts b/src/main/providers/ssh-pty-errors.ts index 95ef265836a..fa2f3edffad 100644 --- a/src/main/providers/ssh-pty-errors.ts +++ b/src/main/providers/ssh-pty-errors.ts @@ -20,6 +20,15 @@ export function isSshPtyLivenessUnverifiableError(error: unknown): boolean { return message.includes(SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR) } +export function isSshPtyExitedEvidenceError(error: unknown): boolean { + const message = error instanceof Error ? error.message : String(error) + return ( + !isSshPtyIdentityMismatchError(error) && + (message.includes(SSH_SESSION_EXPIRED_ERROR) || + message === 'agent_session_exited_during_start') + ) +} + export function isSshPtyIdentityMismatchError(error: unknown): boolean { const message = error instanceof Error ? error.message : String(error) return message.includes(SSH_PTY_IDENTITY_MISMATCH_ERROR) || /identity mismatch/i.test(message) diff --git a/src/main/providers/ssh-pty-liveness-state.ts b/src/main/providers/ssh-pty-liveness-state.ts new file mode 100644 index 00000000000..c36b8fdd575 --- /dev/null +++ b/src/main/providers/ssh-pty-liveness-state.ts @@ -0,0 +1,58 @@ +export const MAX_SSH_PTY_EXIT_TOMBSTONES = 1000 + +export class SshPtyLivenessState { + readonly livePtyIds = new Set() + private readonly unverifiablePtyIds = new Set() + private readonly exitedPtyIds = new Set() + + constructor( + private readonly toAppPtyId: (id: string) => string, + initialUnverifiablePtyIds: Iterable + ) { + for (const id of initialUnverifiablePtyIds) { + this.acceptUnverifiable(id) + } + } + + clear(): void { + this.livePtyIds.clear() + this.unverifiablePtyIds.clear() + this.exitedPtyIds.clear() + } + + hasRoutingOwnership(id: string): boolean { + return this.livePtyIds.has(id) || this.unverifiablePtyIds.has(id) + } + + probe(id: string): boolean | null { + return this.livePtyIds.has(id) ? true : this.exitedPtyIds.has(id) ? false : null + } + + acceptLive(id: string): void { + const appPtyId = this.toAppPtyId(id) + this.unverifiablePtyIds.delete(appPtyId) + this.exitedPtyIds.delete(appPtyId) + this.livePtyIds.add(appPtyId) + } + + acceptUnverifiable(id: string): void { + const appPtyId = this.toAppPtyId(id) + this.livePtyIds.delete(appPtyId) + this.exitedPtyIds.delete(appPtyId) + this.unverifiablePtyIds.add(appPtyId) + } + + acceptExited(id: string): void { + const appPtyId = this.toAppPtyId(id) + this.livePtyIds.delete(appPtyId) + this.unverifiablePtyIds.delete(appPtyId) + this.exitedPtyIds.delete(appPtyId) + this.exitedPtyIds.add(appPtyId) + if (this.exitedPtyIds.size > MAX_SSH_PTY_EXIT_TOMBSTONES) { + const oldest = this.exitedPtyIds.values().next().value + if (oldest !== undefined) { + this.exitedPtyIds.delete(oldest) + } + } + } +} diff --git a/src/main/providers/ssh-pty-provider.ts b/src/main/providers/ssh-pty-provider.ts index ac0a70117b6..5d90d62e635 100644 --- a/src/main/providers/ssh-pty-provider.ts +++ b/src/main/providers/ssh-pty-provider.ts @@ -23,6 +23,7 @@ import { SshPtySpawnExitRaceTracker } from './ssh-pty-spawn-exit-race' import { SshAgentSessionCapabilities } from './ssh-agent-session-capabilities' import type { PtyProcessInspection } from './pty-process-inspection' import { writeToSshPty, writeToSshPtyWithSettlement } from './ssh-pty-write' +import { SshPtyLivenessState } from './ssh-pty-liveness-state' // Why: sequential relay teardown calls share one absolute budget; convert to the mux-relative timeout only at dispatch. function relayTimeoutOptions(deadlineMs: number | undefined): { timeoutMs: number } | undefined { @@ -33,8 +34,7 @@ function relayTimeoutOptions(deadlineMs: number | undefined): { timeoutMs: numbe export class SshPtyProvider implements IPtyProvider { private mux: SshChannelMultiplexer private connectionId: string - private livePtyIds = new Set() - private unverifiablePtyIds = new Set() + private readonly livenessState: SshPtyLivenessState readonly getAppliedSize: NonNullable private readonly agentSessionCapabilities: SshAgentSessionCapabilities private spawnExitRaces = new SshPtySpawnExitRaceTracker() @@ -47,19 +47,21 @@ export class SshPtyProvider implements IPtyProvider { connectionId: string, mux: SshChannelMultiplexer, private readonly remoteCliBridgeEnv?: RemoteCliBridgeEnv, - readonly providerGeneration = 1 + readonly providerGeneration = 1, + initialUnverifiablePtyIds: Iterable = [] ) { this.connectionId = connectionId this.mux = mux this.agentSessionCapabilities = new SshAgentSessionCapabilities(mux) this.getAppliedSize = createSshPtyAppliedSizeReader(mux, connectionId) + this.livenessState = new SshPtyLivenessState(this.toAppPtyId, initialUnverifiablePtyIds) this.outputState = new SshPtyProviderOutputState(providerGeneration, { mux, toAppPtyId: (id) => this.toAppPtyId(id), - livePtyIds: this.livePtyIds, + livePtyIds: this.livenessState.livePtyIds, recordExit: (relayPtyId, incarnationId) => { - this.unverifiablePtyIds.delete(this.toAppPtyId(relayPtyId)) + this.acceptExitedPty(this.toAppPtyId(relayPtyId)) this.spawnExitRaces.recordExit(relayPtyId, incarnationId) } }) @@ -67,8 +69,7 @@ export class SshPtyProvider implements IPtyProvider { dispose(): void { this.outputState.dispose() - this.livePtyIds.clear() - this.unverifiablePtyIds.clear() + this.livenessState.clear() } getConnectionId = (): string => this.connectionId @@ -104,7 +105,8 @@ export class SshPtyProvider implements IPtyProvider { rememberPtyIncarnation: (relayPtyId, incarnationId) => this.outputState.rememberPtyIncarnation(relayPtyId, incarnationId), acceptLivePty: (relayPtyId) => this.acceptLivePty(relayPtyId), - acceptUnverifiablePty: (relayPtyId) => this.acceptUnverifiablePty(relayPtyId) + acceptUnverifiablePty: (relayPtyId) => this.acceptUnverifiablePty(relayPtyId), + acceptExitedPty: (relayPtyId) => this.acceptExitedPty(relayPtyId) }) } @@ -162,6 +164,7 @@ export class SshPtyProvider implements IPtyProvider { rememberPtyIncarnation: (ptyId, incarnationId) => this.outputState.rememberPtyIncarnation(ptyId, incarnationId) }) + this.acceptLivePty(id) } async attachForReconnect( @@ -181,7 +184,7 @@ export class SshPtyProvider implements IPtyProvider { ...(expected?.tabId ? { expectedTabId: expected.tabId } : {}) } const relayPtyId = this.toRelayPtyId(id) - return await requestSshPtyAttach({ + const result = await requestSshPtyAttach({ mux: this.mux, relayPtyId, params, @@ -191,6 +194,8 @@ export class SshPtyProvider implements IPtyProvider { rememberPtyIncarnation: (ptyId, incarnationId) => this.outputState.rememberPtyIncarnation(ptyId, incarnationId) }) + this.acceptLivePty(id) + return result } write(id: string, data: string): boolean { @@ -218,8 +223,7 @@ export class SshPtyProvider implements IPtyProvider { }, relayTimeoutOptions(opts.deadlineMs) ) - this.livePtyIds.delete(id) - this.unverifiablePtyIds.delete(id) + this.acceptExitedPty(id) } async sendSignal(id: string, signal: string): Promise { @@ -294,19 +298,20 @@ export class SshPtyProvider implements IPtyProvider { } // `hasPty` retains routing ownership; liveness callers must use the tri-state probe. - hasPty = (id: string): boolean => this.livePtyIds.has(id) || this.unverifiablePtyIds.has(id) + hasPty = (id: string): boolean => this.livenessState.hasRoutingOwnership(id) - probePtyLiveness = async (id: string): Promise => - this.unverifiablePtyIds.has(id) ? null : this.livePtyIds.has(id) + probePtyLiveness = async (id: string): Promise => this.livenessState.probe(id) - private acceptLivePty(id: string): void { - this.unverifiablePtyIds.delete(id) - this.livePtyIds.add(id) + acceptLivePty(id: string): void { + this.livenessState.acceptLive(id) } - private acceptUnverifiablePty(id: string): void { - this.livePtyIds.delete(id) - this.unverifiablePtyIds.add(id) + acceptUnverifiablePty(id: string): void { + this.livenessState.acceptUnverifiable(id) + } + + acceptExitedPty(id: string): void { + this.livenessState.acceptExited(id) } async getDefaultShell(): Promise { 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 d824be7957f..7bbc390278e 100644 --- a/src/main/providers/ssh-pty-restore-required-reattach.test.ts +++ b/src/main/providers/ssh-pty-restore-required-reattach.test.ts @@ -4,6 +4,7 @@ import { SSH_PTY_RESTORE_REQUIRED_ERROR, SSH_SESSION_EXPIRED_ERROR } from './ssh-pty-errors' +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` @@ -41,6 +42,33 @@ async function spawnError(provider: SshPtyProvider): Promise { } describe('SSH PTY reattach when the relay requires source restoration', () => { + it('keeps an unprobed PTY unverifiable', async () => { + const provider = new SshPtyProvider('conn-1', createMockMux() as never) + + expect(provider.hasPty('ssh:conn-1@@pty-unprobed')).toBe(false) + await expect(provider.probePtyLiveness('ssh:conn-1@@pty-unprobed')).resolves.toBeNull() + }) + + it('retains routing ownership for a reconnecting PTY without reporting it live', async () => { + const id = 'ssh:conn-1@@pty-reconnecting' + const provider = new SshPtyProvider('conn-1', createMockMux() as never, undefined, 1, [id]) + + expect(provider.hasPty(id)).toBe(true) + await expect(provider.probePtyLiveness(id)).resolves.toBeNull() + }) + + it('bounds exited PTY evidence and evicts to unverifiable', async () => { + const provider = new SshPtyProvider('conn-1', createMockMux() as never) + for (let index = 0; index <= MAX_SSH_PTY_EXIT_TOMBSTONES; index += 1) { + provider.acceptExitedPty(`pty-${index}`) + } + + await expect(provider.probePtyLiveness('ssh:conn-1@@pty-0')).resolves.toBeNull() + await expect( + provider.probePtyLiveness(`ssh:conn-1@@pty-${MAX_SSH_PTY_EXIT_TOMBSTONES}`) + ).resolves.toBe(false) + }) + it('re-attaches over the live PTY instead of reporting exited', async () => { const mux = createMockMux() // Why: the relay retires the stale delivery record as the restoreRequired response settles, diff --git a/src/main/providers/ssh-pty-session-reattach.ts b/src/main/providers/ssh-pty-session-reattach.ts index dda07f09ef3..be1c1baab2f 100644 --- a/src/main/providers/ssh-pty-session-reattach.ts +++ b/src/main/providers/ssh-pty-session-reattach.ts @@ -5,6 +5,7 @@ import { SSH_PTY_LIVENESS_UNVERIFIABLE_ERROR, SSH_PTY_RESTORE_REQUIRED_ERROR, SSH_SESSION_EXPIRED_ERROR, + isSshPtyExitedEvidenceError, isSshPtyIdentityMismatchError, isSshPtyNotFoundError } from './ssh-pty-errors' @@ -262,6 +263,7 @@ export async function reattachSshPtySessionForSpawn( args: Parameters[0] & { acceptLivePty: (relayPtyId: string) => void acceptUnverifiablePty: (relayPtyId: string) => void + acceptExitedPty: (relayPtyId: string) => void } ): Promise { let result: SshPtyReattachResult | undefined @@ -277,11 +279,7 @@ export async function reattachSshPtySessionForSpawn( try { result = await reattachSshPtySessionWithExitFence(args) } catch (error) { - const message = error instanceof Error ? error.message : String(error) - if ( - message.includes(SSH_SESSION_EXPIRED_ERROR) || - message === 'agent_session_exited_during_start' - ) { + if (isSshPtyIdentityMismatchError(error) || isSshPtyExitedEvidenceError(error)) { throw error } // Why: losing contact during verification is unverifiable, not live or exited. @@ -304,6 +302,9 @@ export async function reattachSshPtySessionForSpawn( return spawnResult } catch (error) { await result?.sourceActivationLease?.rollback() + if (isSshPtyExitedEvidenceError(error)) { + args.acceptExitedPty(result?.id ?? toAppSshPtyId(args.connectionId, args.sessionId)) + } throw error } } diff --git a/src/main/ssh/ssh-relay-session.test.ts b/src/main/ssh/ssh-relay-session.test.ts index 2f36c537f08..c7b716c70af 100644 --- a/src/main/ssh/ssh-relay-session.test.ts +++ b/src/main/ssh/ssh-relay-session.test.ts @@ -66,6 +66,9 @@ vi.mock('../providers/ssh-pty-provider', () => ({ isSshPtyIdentityMismatchError: (err: unknown) => (err instanceof Error ? err.message : String(err)).includes('identity mismatch'), SshPtyProvider: class MockSshPtyProvider { + private livePtyIds = new Set() + private unverifiablePtyIds = new Set() + private exitedPtyIds = new Set() onData = vi.fn().mockReturnValue(() => {}) onReplay = vi.fn().mockReturnValue(() => {}) onExit = vi.fn().mockReturnValue(() => {}) @@ -73,6 +76,32 @@ vi.mock('../providers/ssh-pty-provider', () => ({ attachForReconnect = vi.fn().mockResolvedValue({}) setPtyDeliveryPauseAdapter = pauseAdapterMock dispose = vi.fn() + + constructor( + _connectionId: string, + _mux: unknown, + _remoteCliBridgeEnv: unknown, + _providerGeneration: number, + initialUnverifiablePtyIds: Iterable = [] + ) { + this.unverifiablePtyIds = new Set(initialUnverifiablePtyIds) + } + + hasPty = (id: string): boolean => this.livePtyIds.has(id) || this.unverifiablePtyIds.has(id) + probePtyLiveness = vi.fn( + async (id: string): Promise => + this.livePtyIds.has(id) ? true : this.exitedPtyIds.has(id) ? false : null + ) + acceptUnverifiablePty = vi.fn((id: string) => { + this.livePtyIds.delete(id) + this.exitedPtyIds.delete(id) + this.unverifiablePtyIds.add(id) + }) + acceptExitedPty = vi.fn((id: string) => { + this.livePtyIds.delete(id) + this.unverifiablePtyIds.delete(id) + this.exitedPtyIds.add(id) + }) } })) @@ -206,6 +235,41 @@ describe('SshRelaySession', () => { expect(registerSshGitProvider).toHaveBeenCalledWith('target-1', expect.anything()) }) + it('keeps a transiently unreattached PTY unverifiable after the relay becomes ready', async () => { + const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps() + const { getSshPtyProvider } = await import('../ipc/pty') + vi.mocked(getPtyIdsForConnection).mockReturnValue(['pty-unverifiable']) + vi.mocked(getSshPtyProvider).mockImplementationOnce(() => { + const provider = vi.mocked(registerSshPtyProvider).mock.calls.at(-1)?.[1] as { + attachForReconnect: ReturnType + } + provider.attachForReconnect.mockRejectedValue(new Error('transport interrupted')) + return provider as never + }) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) + + try { + await session.establish(mockConn) + const provider = vi.mocked(registerSshPtyProvider).mock.calls.at(-1)?.[1] as { + hasPty: (id: string) => boolean + probePtyLiveness: (id: string) => Promise + } + const appPtyId = 'ssh:target-1@@pty-unverifiable' + + expect(session.getState()).toBe('ready') + expect(provider.hasPty(appPtyId)).toBe(true) + await expect(provider.probePtyLiveness(appPtyId)).resolves.toBeNull() + expect(mockStore.markSshRemotePtyLease).not.toHaveBeenCalledWith( + 'target-1', + 'pty-unverifiable', + 'expired' + ) + } finally { + warn.mockRestore() + } + }) + it('continues provider registration when the relay managed-hook request fails', async () => { process.env.ORCA_FEATURE_REMOTE_AGENT_HOOKS = '1' muxRequestMock.mockImplementation(async (method: string) => { diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index 820eb7e1360..a8b41b49493 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -1009,11 +1009,21 @@ export class SshRelaySession { this.wireUpRemoteOrcaCli(mux, connectionIncarnation) const providerGeneration = allocateSshPtyProviderGeneration() + const initialUnverifiablePtyIds = new Set( + [ + ...getPtyIdsForConnection(this.targetId), + ...this.store + .getSshRemotePtyLeases(this.targetId) + .filter((lease) => lease.state !== 'terminated' && lease.state !== 'expired') + .map((lease) => lease.ptyId) + ].map((ptyId) => toAppSshPtyId(this.targetId, ptyId)) + ) const ptyProvider = new SshPtyProvider( this.targetId, mux, this.remoteCliBridgeEnv ?? undefined, - providerGeneration + providerGeneration, + initialUnverifiablePtyIds ) const consumerOwnerState = this.activePtyConsumerOwner() if (consumerOwnerState) { @@ -2509,7 +2519,7 @@ export class SshRelaySession { if (!shouldContinue()) { return } - this.handlePtyReattachFailure(ptyId, appPtyId, pendingReattach, error) + this.handlePtyReattachFailure(ptyProvider, ptyId, appPtyId, pendingReattach, error) } finally { recoveryActivationLease?.retire() sourceActivationLease?.rollback() @@ -2649,12 +2659,14 @@ export class SshRelaySession { } private handlePtyReattachFailure( + ptyProvider: SshPtyProvider, ptyId: string, appPtyId: string, pending: PendingPtyReattach, error: unknown ): void { if (!isSshPtyNotFoundError(error)) { + ptyProvider.acceptUnverifiablePty?.(appPtyId) pending.restoreRequired = 'reattachAttemptsExhausted' this.wakeRecovery(pending) console.warn( @@ -2672,6 +2684,7 @@ export class SshRelaySession { ) return } + ptyProvider.acceptExitedPty?.(appPtyId) console.warn( `[ssh-relay-session] Dropping stale PTY ${ptyId} for ${this.targetId} after relay reattach failed: ${ error instanceof Error ? error.message : String(error)