diff --git a/src/main/providers/ssh-pty-identity-mismatch-is-not-death.test.ts b/src/main/providers/ssh-pty-identity-mismatch-is-not-death.test.ts index 83e40cdb742..6b9f64ba3e3 100644 --- a/src/main/providers/ssh-pty-identity-mismatch-is-not-death.test.ts +++ b/src/main/providers/ssh-pty-identity-mismatch-is-not-death.test.ts @@ -99,3 +99,43 @@ describe('a genuine absence is still a death', () => { expect(error.message).not.toContain(SSH_PTY_IDENTITY_MISMATCH_ERROR) }) }) + +// The relay only compares identity fields present on BOTH sides ("absent +// identity stays permissive", relay/pty-handler.ts). So not sending them stops +// every relay version from rejecting a moved pane — including relays already +// deployed on people's hosts, with no wire change and no redeploy. +// +// Nothing is lost: the check existed to catch a relay restart recycling pty-N +// for a new shell, and in exactly that case the pane and tab both still match, +// so it accepted the wrong shell anyway. +describe('reattach does not ask the relay to police pane identity', () => { + async function attachParams(): Promise> { + const request = vi.fn().mockRejectedValue(new Error('boom')) + try { + await reattachSshPtySession({ + mux: { request, notify: vi.fn() } as never, + connectionId: CONNECTION_ID, + sessionId: RELAY_PTY_ID, + options: { + cols: 80, + rows: 24, + paneKey: 'tab-new:leaf-1', + tabId: 'tab-new', + env: { ORCA_PANE_KEY: 'tab-old:leaf-1', ORCA_TAB_ID: 'tab-old' } + } as never + }) + } catch { + // the attach failure is not what this clause is about + } + const call = request.mock.calls.at(0) + return (call?.[1] ?? call?.[0] ?? {}) as Record + } + + it('sends no expected pane key', async () => { + expect(await attachParams()).not.toHaveProperty('expectedPaneKey') + }) + + it('sends no expected tab id', async () => { + expect(await attachParams()).not.toHaveProperty('expectedTabId') + }) +}) diff --git a/src/main/providers/ssh-pty-provider.test.ts b/src/main/providers/ssh-pty-provider.test.ts index d299e65b716..46dc3ff2e6f 100644 --- a/src/main/providers/ssh-pty-provider.test.ts +++ b/src/main/providers/ssh-pty-provider.test.ts @@ -639,7 +639,9 @@ describe('SshPtyProvider', () => { }) }) - it('reattaches with explicit pane identity when hook env was stripped', async () => { + // Pane identity is deliberately no longer sent: the relay's copy is frozen + // at spawn, so moving a pane to another tab made it refuse a live shell. + it('reattaches without asking the relay to police pane identity', async () => { mux.request.mockResolvedValue({ replay: 'buffered-output' }) await provider.spawn({ @@ -654,9 +656,7 @@ describe('SshPtyProvider', () => { id: 'pty-old', cols: 80, rows: 24, - suppressReplayNotification: true, - expectedPaneKey: 'tab-a:leaf-a', - expectedTabId: 'tab-a' + suppressReplayNotification: true }) }) @@ -739,28 +739,6 @@ describe('SshPtyProvider', () => { ) }) - it('attachForReconnect forwards expected identity when provided', async () => { - await provider.attachForReconnect(scopedPty1, { - paneKey: 'tab-a:leaf-a', - tabId: 'tab-a' - }) - - expectRequest( - mux.request, - 'pty.attach', - { - id: 'pty-1', - suppressReplayNotification: true, - expectedPaneKey: 'tab-a:leaf-a', - expectedTabId: 'tab-a' - }, - expect.objectContaining({ - timeoutMs: 10_000, - beforeResolve: expect.any(Function) - }) - ) - }) - it('write sends pty.data notification', () => { provider.write(scopedPty1, 'hello') expect(mux.notify).toHaveBeenCalledWith('pty.data', { id: 'pty-1', data: 'hello' }) diff --git a/src/main/providers/ssh-pty-provider.ts b/src/main/providers/ssh-pty-provider.ts index f516a3b3f24..a3d8a7109d1 100644 --- a/src/main/providers/ssh-pty-provider.ts +++ b/src/main/providers/ssh-pty-provider.ts @@ -182,19 +182,16 @@ export class SshPtyProvider implements IPtyProvider { async attachForReconnect( id: string, - expected?: { paneKey?: string; tabId?: string }, sourceRecovery?: PtySourceRecoveryRequest ): Promise { // Why: reconnect owns replay delivery so stale/duplicate attach results can - // be filtered before they reach the renderer. The expected identity lets the - // relay reject a cross-generation id collision instead of reattaching this - // lease to a different pane's freshly spawned PTY. + // be filtered before they reach the renderer. Pane identity is deliberately + // not sent — see reattachSshPtySession; the relay's copy is frozen at spawn, + // so it rejected panes that had merely moved tabs. const params = { id: this.toRelayPtyId(id), suppressReplayNotification: true, - ...(sourceRecovery ? { sourceRecovery } : {}), - ...(expected?.paneKey ? { expectedPaneKey: expected.paneKey } : {}), - ...(expected?.tabId ? { expectedTabId: expected.tabId } : {}) + ...(sourceRecovery ? { sourceRecovery } : {}) } const relayPtyId = this.toRelayPtyId(id) return await requestSshPtyAttach({ diff --git a/src/main/providers/ssh-pty-session-reattach.ts b/src/main/providers/ssh-pty-session-reattach.ts index 25ff2bc4933..acd7c2db7b8 100644 --- a/src/main/providers/ssh-pty-session-reattach.ts +++ b/src/main/providers/ssh-pty-session-reattach.ts @@ -185,9 +185,11 @@ export async function reattachSshPtySession(args: { const relaySessionId = toRelaySshPtyId(args.connectionId, args.sessionId) console.warn(`[ssh-pty] spawn() called with sessionId=${args.sessionId}, attempting pty.attach`) try { - // Why: expected pane identity prevents a reused relay id from attaching the wrong shell. - const expectedPaneKey = args.options.paneKey ?? args.options.env?.ORCA_PANE_KEY - const expectedTabId = args.options.tabId ?? args.options.env?.ORCA_TAB_ID + // Why no expected pane identity: the relay froze it at spawn, so moving a + // pane to another tab made it refuse a live shell — and refuse by saying + // "not found", which read as death. It never caught what it was for either: + // a relay restart recycling this id for a new shell leaves pane and tab + // matching. Recycling is caught by the incarnation the attach returns. const attachResult = await requestSshPtyAttach({ mux: args.mux, relayPtyId: relaySessionId, @@ -195,9 +197,7 @@ export async function reattachSshPtySession(args: { id: relaySessionId, cols: args.options.cols, rows: args.options.rows, - suppressReplayNotification: true, - ...(expectedPaneKey ? { expectedPaneKey } : {}), - ...(expectedTabId ? { expectedTabId } : {}) + suppressReplayNotification: true }, installSourceActivation: args.installSourceActivation, rememberPtyIncarnation: args.rememberPtyIncarnation diff --git a/src/main/ssh/ssh-relay-session-data-delivery.test.ts b/src/main/ssh/ssh-relay-session-data-delivery.test.ts index 2eca6802f28..2a6edf1840e 100644 --- a/src/main/ssh/ssh-relay-session-data-delivery.test.ts +++ b/src/main/ssh/ssh-relay-session-data-delivery.test.ts @@ -403,7 +403,6 @@ describe('SshRelaySession data delivery', () => { expect(retryCalls[0]).toHaveProperty('resume') expect(attachForReconnectMock).toHaveBeenCalledWith( 'pty-1', - undefined, Object.freeze({ status: 'checkpointUnavailable' }) ) second.dispose() @@ -762,7 +761,6 @@ describe('SshRelaySession data delivery', () => { expect(attachForReconnectMock).toHaveBeenCalledWith( 'pty-1', - undefined, expect.objectContaining({ status: 'checkpoint', deliveryToken: 'old-token', diff --git a/src/main/ssh/ssh-relay-session-model-migration.test.ts b/src/main/ssh/ssh-relay-session-model-migration.test.ts index ae5db55e0cc..ab1512d82dc 100644 --- a/src/main/ssh/ssh-relay-session-model-migration.test.ts +++ b/src/main/ssh/ssh-relay-session-model-migration.test.ts @@ -199,7 +199,6 @@ describe('SshRelaySession model migration', () => { expect(attachForReconnectMock).toHaveBeenCalledWith( 'pty-1', - undefined, Object.freeze({ status: 'checkpoint', clientGeneration: 1, @@ -259,7 +258,6 @@ describe('SshRelaySession model migration', () => { expect(attachForReconnectMock).toHaveBeenCalledWith( 'pty-1', - undefined, Object.freeze({ status: 'checkpointUnavailable' }) ) }) diff --git a/src/main/ssh/ssh-relay-session-recovery-races.test.ts b/src/main/ssh/ssh-relay-session-recovery-races.test.ts index 26dd798ac5a..f8853832ed1 100644 --- a/src/main/ssh/ssh-relay-session-recovery-races.test.ts +++ b/src/main/ssh/ssh-relay-session-recovery-races.test.ts @@ -705,7 +705,7 @@ describe('SshRelaySession recovery race fencing', () => { await session.reconnect(deps.mockConn) expect(attachForReconnectMock).toHaveBeenCalledTimes(2) - expect(attachForReconnectMock.mock.calls.at(-1)?.[2]).toMatchObject({ + expect(attachForReconnectMock.mock.calls.at(-1)?.[1]).toMatchObject({ status: 'checkpoint', deliveryToken: 'new-token', acceptedSourceEndSu: 8 @@ -763,7 +763,7 @@ describe('SshRelaySession recovery race fencing', () => { const replacementReconnect = session.reconnect(deps.mockConn) await Promise.all([staleReconnect, replacementReconnect]) - const recoveryRequests = attachForReconnectMock.mock.calls.map((call) => call[2]) + const recoveryRequests = attachForReconnectMock.mock.calls.map((call) => call[1]) expect(recoveryRequests).toHaveLength(2) expect(recoveryRequests[1]).toMatchObject({ status: 'checkpoint', diff --git a/src/main/ssh/ssh-relay-session.test.ts b/src/main/ssh/ssh-relay-session.test.ts index 570fca21fd4..7478caa8c48 100644 --- a/src/main/ssh/ssh-relay-session.test.ts +++ b/src/main/ssh/ssh-relay-session.test.ts @@ -531,26 +531,10 @@ describe('SshRelaySession', () => { ) }) - it('forwards a lease tab identity to reattach so a reset relay cannot cross-wire it', async () => { - const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps() - const { getSshPtyProvider } = await import('../ipc/pty') - const mockAttach = vi.fn().mockResolvedValue(undefined) - vi.mocked(getSshPtyProvider).mockReturnValue({ - attachForReconnect: mockAttach, - dispose: vi.fn() - } as unknown as ReturnType) - vi.mocked(getPtyIdsForConnection).mockReturnValue([]) - vi.mocked(mockStore.getSshRemotePtyLeases).mockReturnValue([ - { targetId: 'target-1', ptyId: 'pty-1', state: 'detached', tabId: 'tab-a' } - ] as ReturnType) - - const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) - await session.establish(mockConn) - - expect(mockAttach).toHaveBeenCalledWith('pty-1', { tabId: 'tab-a' }) - }) - - it('forwards a lease pane identity when leaf identity is available', async () => { + // The relay froze pane identity at spawn, so sending it made a moved pane + // unreachable — and it never caught the reused-id case it was written for, + // because after a relay restart the pane and tab both still match. + it('does not forward pane identity to reattach', async () => { const { mockConn, mockStore, mockPortForward, getMainWindow } = createMockDeps() const { getSshPtyProvider } = await import('../ipc/pty') const mockAttach = vi.fn().mockResolvedValue(undefined) @@ -567,10 +551,7 @@ describe('SshRelaySession', () => { const session = new SshRelaySession('target-1', getMainWindow, mockStore, mockPortForward) await session.establish(mockConn) - expect(mockAttach).toHaveBeenCalledWith('pty-1', { - paneKey: `tab-a:${leafId}`, - tabId: 'tab-a' - }) + expect(mockAttach).toHaveBeenCalledWith('pty-1') }) it('does not expire a live reused relay id when attach rejects identity mismatch', async () => { @@ -602,10 +583,6 @@ describe('SshRelaySession', () => { await session.reconnect(mockConn) - expect(mockAttach).toHaveBeenCalledWith('pty-1', { - paneKey: `tab-old:${staleLeafId}`, - tabId: 'tab-old' - }) expect(clearProviderPtyState).not.toHaveBeenCalledWith('ssh:target-1@@pty-1') expect(deletePtyOwnership).not.toHaveBeenCalledWith('ssh:target-1@@pty-1') expect(mockStore.markSshRemotePtyLease).not.toHaveBeenCalledWith('target-1', 'pty-1', 'expired') diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index 7f26fe5f4d6..ed072b9189b 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -100,8 +100,6 @@ import { SSH_AI_VAULT_LIST_SESSIONS_TIMEOUT_MS, type SshAiVaultRelayListParams } from '../../shared/ssh-ai-vault-relay' -import { isTerminalLeafId, makePaneKey } from '../../shared/stable-pane-id' -import { isValidTerminalTabId } from '../../shared/terminal-tab-id' import { openSshPtyConsumerSession, type OpenSshPtyConsumerSessionOptions, @@ -187,28 +185,8 @@ type RemoteCliBridgeEnv = { pathDelimiter?: ':' | ';' } -type ExpectedPtyIdentity = { paneKey?: string; tabId?: string } type TargetedDeliveryRecovery = 'confirm-existing' | 'fresh-activation' -function expectedIdentityForLease(lease: { - tabId?: string - leafId?: string -}): ExpectedPtyIdentity | null { - if (typeof lease.tabId !== 'string' || lease.tabId.length === 0) { - return null - } - const paneKey = - isValidTerminalTabId(lease.tabId) && - typeof lease.leafId === 'string' && - isTerminalLeafId(lease.leafId) - ? makePaneKey(lease.tabId, lease.leafId) - : undefined - return { - ...(paneKey ? { paneKey } : {}), - tabId: lease.tabId - } -} - function parseRecoveryComplete(params: Record): PtySourceRecoveryComplete | null { if ( typeof params.id !== 'string' || @@ -1919,15 +1897,11 @@ export class SshRelaySession { const activeLeaseByPtyId = activeLease ? new Map([[relayPtyId, activeLease]]) : new Map() - const expectedIdentity = activeLease ? expectedIdentityForLease(activeLease) : undefined const attachedLeaseIds = new Set() await this.reattachKnownPty({ ptyProvider, ptyId: relayPtyId, activeLeaseByPtyId, - expectedIdentityByPtyId: expectedIdentity - ? new Map([[relayPtyId, expectedIdentity]]) - : new Map(), attachedLeaseIds, mux, providerGeneration, @@ -2198,15 +2172,6 @@ export class SshRelaySession { .filter((lease) => lease.state !== 'terminated' && lease.state !== 'expired') const activeLeaseByPtyId = new Map(activeLeases.map((lease) => [lease.ptyId, lease])) const leasedPtyIds = activeLeases.map((lease) => lease.ptyId) - // Why: pass pane identity so the relay can reject cross-generation id collisions; tabId falls back for pre-leafId leases. - const expectedIdentityByPtyId = new Map( - activeLeases - .map((lease): [string, ExpectedPtyIdentity] | null => { - const expected = expectedIdentityForLease(lease) - return expected ? [lease.ptyId, expected] : null - }) - .filter((entry): entry is [string, ExpectedPtyIdentity] => entry !== null) - ) const attachedLeaseIds = new Set() // Why: after app restart ptyOwnership is empty, but durable SSH leases still describe grace-window survivors. const ptyIds = Array.from( @@ -2234,7 +2199,6 @@ export class SshRelaySession { ptyProvider, ptyId, activeLeaseByPtyId, - expectedIdentityByPtyId, attachedLeaseIds, mux, providerGeneration, @@ -2267,7 +2231,6 @@ export class SshRelaySession { ptyProvider: SshPtyProvider ptyId: string activeLeaseByPtyId: Map - expectedIdentityByPtyId: Map attachedLeaseIds: Set mux: SshChannelMultiplexer providerGeneration: number @@ -2278,7 +2241,6 @@ export class SshRelaySession { ptyProvider, ptyId, activeLeaseByPtyId, - expectedIdentityByPtyId, attachedLeaseIds, mux, providerGeneration, @@ -2309,7 +2271,6 @@ export class SshRelaySession { const attachResult = await this.attachPtyWithRetry( ptyProvider, ptyId, - expectedIdentityByPtyId.get(ptyId), recoveryRequest, shouldContinue ) @@ -2533,7 +2494,6 @@ export class SshRelaySession { private async attachPtyWithRetry( ptyProvider: SshPtyProvider, ptyId: string, - expectedIdentity: ExpectedPtyIdentity | undefined, recoveryRequest: PtySourceRecoveryRequest | undefined, shouldContinue: () => boolean ): Promise { @@ -2543,12 +2503,7 @@ export class SshRelaySession { throw lastError ?? new Error('PTY reattach attempt is no longer current') } try { - return await this.attachPtyWithDeadline( - ptyProvider, - ptyId, - expectedIdentity, - recoveryRequest - ) + return await this.attachPtyWithDeadline(ptyProvider, ptyId, recoveryRequest) } catch (error) { lastError = error if (!shouldContinue() || isSshPtyNotFoundError(error) || attempt === 1) { @@ -2563,7 +2518,6 @@ export class SshRelaySession { private async attachPtyWithDeadline( ptyProvider: SshPtyProvider, ptyId: string, - expectedIdentity: ExpectedPtyIdentity | undefined, recoveryRequest: PtySourceRecoveryRequest | undefined ): Promise { let timer: ReturnType | undefined @@ -2578,13 +2532,9 @@ export class SshRelaySession { timer.unref?.() }) try { - const attach = expectedIdentity - ? recoveryRequest - ? ptyProvider.attachForReconnect(ptyId, expectedIdentity, recoveryRequest) - : ptyProvider.attachForReconnect(ptyId, expectedIdentity) - : recoveryRequest - ? ptyProvider.attachForReconnect(ptyId, undefined, recoveryRequest) - : ptyProvider.attachForReconnect(ptyId) + const attach = recoveryRequest + ? ptyProvider.attachForReconnect(ptyId, recoveryRequest) + : ptyProvider.attachForReconnect(ptyId) const guardedAttach = attach.then((result) => { if (timedOut) { result.sourceActivationLease?.rollback()