fix(ssh): stop sending pane identity on reattach

The relay froze pane identity at spawn, so moving a pane to another tab made it
refuse a live shell — and refuse by saying 'not found'. The comparison is
presence-guarded, so not sending the fields disarms it on every relay version
including ones already installed on hosts: no wire change, no redeploy.

Nothing is lost. It existed to catch a relay restart recycling pty-N for a new
shell, and in exactly that case pane and tab both still match, so it accepted
the wrong shell anyway. The incarnation the attach returns is what distinguishes
those, and it already crosses the wire.

Removes the whole client-side apparatus: the expected-identity type, its
per-lease derivation, its map, and the parameter threaded through four layers.

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Neil
2026-08-09 00:43:00 -07:00
co-authored by Orca
parent e2524b0472
commit c51be8072b
9 changed files with 65 additions and 127 deletions
@@ -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<Record<string, unknown>> {
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<string, unknown>
}
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')
})
})
+4 -26
View File
@@ -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' })
+4 -7
View File
@@ -182,19 +182,16 @@ export class SshPtyProvider implements IPtyProvider {
async attachForReconnect(
id: string,
expected?: { paneKey?: string; tabId?: string },
sourceRecovery?: PtySourceRecoveryRequest
): Promise<SshPtyAttachResult> {
// 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({
@@ -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
@@ -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',
@@ -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' })
)
})
@@ -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',
+5 -28
View File
@@ -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<typeof getSshPtyProvider>)
vi.mocked(getPtyIdsForConnection).mockReturnValue([])
vi.mocked(mockStore.getSshRemotePtyLeases).mockReturnValue([
{ targetId: 'target-1', ptyId: 'pty-1', state: 'detached', tabId: 'tab-a' }
] as ReturnType<typeof mockStore.getSshRemotePtyLeases>)
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')
+4 -54
View File
@@ -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<string, unknown>): PtySourceRecoveryComplete | null {
if (
typeof params.id !== 'string' ||
@@ -1919,15 +1897,11 @@ export class SshRelaySession {
const activeLeaseByPtyId = activeLease
? new Map<string, SshPtyLease>([[relayPtyId, activeLease]])
: new Map<string, SshPtyLease>()
const expectedIdentity = activeLease ? expectedIdentityForLease(activeLease) : undefined
const attachedLeaseIds = new Set<string>()
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<string>()
// 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<string, SshPtyLease>
expectedIdentityByPtyId: Map<string, ExpectedPtyIdentity>
attachedLeaseIds: Set<string>
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<SshPtyAttachResult> {
@@ -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<SshPtyAttachResult> {
let timer: ReturnType<typeof setTimeout> | 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()