diff --git a/src/renderer/src/lib/workspace-session-patch.ts b/src/renderer/src/lib/workspace-session-patch.ts index 17fd11a7787..4e56654a8f5 100644 --- a/src/renderer/src/lib/workspace-session-patch.ts +++ b/src/renderer/src/lib/workspace-session-patch.ts @@ -77,6 +77,8 @@ export function buildWorkspaceSessionPatch( 'tabsByWorktree', 'ptyIdsByTabId', 'lastKnownRelayPtyIdByTabId', + 'pendingReconnectPtyIdByTabId', + 'deferredSshSessionIdsByTabId', 'repos', 'worktreesByRepo' ] as const) diff --git a/src/renderer/src/lib/workspace-session-relevant-fields.test.ts b/src/renderer/src/lib/workspace-session-relevant-fields.test.ts index 25c96b3bd2e..2dc1346f534 100644 --- a/src/renderer/src/lib/workspace-session-relevant-fields.test.ts +++ b/src/renderer/src/lib/workspace-session-relevant-fields.test.ts @@ -37,7 +37,9 @@ describe('SESSION_RELEVANT_FIELDS', () => { defaultTerminalTabsAppliedByWorktreeId: true, closedTerminalTabTombstonesByTabId: true, sleepingAgentSessionsByPaneKey: true, - clientHostedBrowserCloseIntentsByEnvironment: true + clientHostedBrowserCloseIntentsByEnvironment: true, + pendingReconnectPtyIdByTabId: true, + deferredSshSessionIdsByTabId: true } it('contains every key of WorkspaceSessionSnapshot', () => { diff --git a/src/renderer/src/lib/workspace-session.ts b/src/renderer/src/lib/workspace-session.ts index 31ddc63704a..330a150b636 100644 --- a/src/renderer/src/lib/workspace-session.ts +++ b/src/renderer/src/lib/workspace-session.ts @@ -61,6 +61,9 @@ export type WorkspaceSessionSnapshot = Pick< activeWorkspaceExecutionHostId?: AppState['activeWorkspaceExecutionHostId'] sleepingAgentSessionsByPaneKey?: AppState['sleepingAgentSessionsByPaneKey'] clientHostedBrowserCloseIntentsByEnvironment?: AppState['clientHostedBrowserCloseIntentsByEnvironment'] + /** Optional so the many partial snapshot fixtures keep type-checking; see buildTerminalSessionData. */ + pendingReconnectPtyIdByTabId?: AppState['pendingReconnectPtyIdByTabId'] + deferredSshSessionIdsByTabId?: AppState['deferredSshSessionIdsByTabId'] } // Why: shallow-equality gate for the debounced session writer; _exhaustive below keeps it in sync with the snapshot type. @@ -97,7 +100,9 @@ export const SESSION_RELEVANT_FIELDS = [ 'defaultTerminalTabsAppliedByWorktreeId', 'closedTerminalTabTombstonesByTabId', 'sleepingAgentSessionsByPaneKey', - 'clientHostedBrowserCloseIntentsByEnvironment' + 'clientHostedBrowserCloseIntentsByEnvironment', + 'pendingReconnectPtyIdByTabId', + 'deferredSshSessionIdsByTabId' ] as const satisfies readonly (keyof WorkspaceSessionSnapshot)[] type _MissingSessionField = Exclude< @@ -222,8 +227,18 @@ export function buildTerminalSessionData( // Why: relay reconnect keeps lastKnown but clears tab.ptyId; the !tab.ptyId guard excludes slept tabs (which keep ptyId as a wake hint). const lastKnown = snapshot.lastKnownRelayPtyIdByTabId + // Why the two reconnect maps (#17743): hydration nulls tab.ptyId, empties ptyIdsByTabId, and + // never restores lastKnown, so on a fresh process they are the ONLY surviving handle for a + // relay-backed tab between restore and rebind. Persisting without them republishes the nulled + // row over the id the file (and the relay snapshot) still held, which is the client's own + // bookkeeping being read as evidence the remote PTY is gone. Both already count as live + // ownership for the orphan sweep (terminal-orphan-helpers) and for retirement planning. + const pendingReconnect = snapshot.pendingReconnectPtyIdByTabId ?? {} + const deferredSshSessions = snapshot.deferredSshSessionIdsByTabId ?? {} + const restoredSessionId = (tabId: string): string | undefined => + lastKnown[tabId] || pendingReconnect[tabId] || deferredSshSessions[tabId] const hasReconnectableSession = (tab: { id: string; ptyId: string | null }): boolean => - hasLivePty(tab.id) || (!tab.ptyId && Boolean(lastKnown[tab.id])) + hasLivePty(tab.id) || (!tab.ptyId && Boolean(restoredSessionId(tab.id))) const activeWorktreeIdsOnShutdown = Object.entries(tabsByWorktree) .filter(([, tabs]) => tabs.some(hasReconnectableSession)) @@ -249,7 +264,7 @@ export function buildTerminalSessionData( if (!hasReconnectableSession(tab)) { continue } - const sessionId = tab.ptyId || lastKnown[tab.id] + const sessionId = tab.ptyId || restoredSessionId(tab.id) if (sessionId) { remoteSessionIdsByTabId[tab.id] = sessionId } diff --git a/src/renderer/src/store/terminals/restored-relay-session-identity.test.ts b/src/renderer/src/store/terminals/restored-relay-session-identity.test.ts new file mode 100644 index 00000000000..e7e2ed15296 --- /dev/null +++ b/src/renderer/src/store/terminals/restored-relay-session-identity.test.ts @@ -0,0 +1,143 @@ +import { describe, expect, it } from 'vitest' +import type { WorkspaceSessionState } from '../../../../shared/workspace-session-state-types' +import { buildWorkspaceSessionPayload } from '@/lib/workspace-session' +import { getOrphanTerminalIds } from '../slices/terminal-orphan-helpers' +import { createTestStore, makeTab, makeWorktree } from '../slices/store-test-helpers' + +const TARGET_ID = 'target' +const REPO_ID = 'repo-ssh' +const WORKTREE_ID = `${REPO_ID}::/work/demo` +const TAB_ID = 'tab-ssh' +const RELAY_PTY_ID = 'ssh:target@@pty-42' + +function connectedSshState(status: 'connected' | 'disconnected') { + return new Map([ + [TARGET_ID, { targetId: TARGET_ID, status, error: null, reconnectAttempt: 0 }] + ]) as never +} + +function seedStore(status: 'connected' | 'disconnected' = 'connected') { + const store = createTestStore() + store.setState({ + repos: [ + { + id: REPO_ID, + path: '/work/demo', + displayName: 'demo', + badgeColor: '#000', + addedAt: 1, + connectionId: TARGET_ID, + executionHostId: 'ssh:target' + } + ], + worktreesByRepo: { + [REPO_ID]: [ + makeWorktree({ + id: WORKTREE_ID, + repoId: REPO_ID, + path: '/work/demo', + hostId: 'ssh:target' + }) + ] + }, + sshConnectionStates: connectedSshState(status), + sshTargetsHydrated: true, + sshTargetLabels: new Map([[TARGET_ID, 'demo host']]), + hydrationSucceeded: true + }) + return store +} + +/** What the previous run wrote: a live relay session recorded on the row AND in the id map. */ +function persistedSession(): WorkspaceSessionState { + return { + activeRepoId: REPO_ID, + activeWorktreeId: WORKTREE_ID, + activeTabId: TAB_ID, + tabsByWorktree: { + [WORKTREE_ID]: [makeTab({ id: TAB_ID, worktreeId: WORKTREE_ID, ptyId: RELAY_PTY_ID })] + }, + terminalLayoutsByTabId: {}, + activeWorktreeIdsOnShutdown: [WORKTREE_ID], + activeTabIdByWorktree: { [WORKTREE_ID]: TAB_ID }, + remoteSessionIdsByTabId: { [TAB_ID]: RELAY_PTY_ID }, + activeConnectionIdsAtShutdown: [TARGET_ID] + } +} + +describe('restored relay session identity (#17743)', () => { + it('does not republish a hydration-nulled ptyId over the persisted relay session id', () => { + const store = seedStore() + store.getState().hydrateWorkspaceSession(persistedSession()) + + // Hydration deliberately nulls the row; that is the contract, not the bug. + expect(store.getState().tabsByWorktree[WORKTREE_ID][0].ptyId).toBeNull() + expect(store.getState().ptyIdsByTabId[TAB_ID]).toEqual([]) + expect(store.getState().lastKnownRelayPtyIdByTabId[TAB_ID]).toBeUndefined() + + const payload = buildWorkspaceSessionPayload(store.getState()) + + expect(payload.remoteSessionIdsByTabId).toEqual({ [TAB_ID]: RELAY_PTY_ID }) + expect(payload.activeWorktreeIdsOnShutdown).toContain(WORKTREE_ID) + expect(payload.activeConnectionIdsAtShutdown).toEqual([TARGET_ID]) + }) + + it('rebinds the restored tab to its live relay PTY and keeps publishing that id', async () => { + const store = seedStore() + store.getState().hydrateWorkspaceSession(persistedSession()) + + await store.getState().reconnectPersistedTerminals() + + expect(store.getState().tabsByWorktree[WORKTREE_ID][0].ptyId).toBe(RELAY_PTY_ID) + expect(store.getState().ptyIdsByTabId[TAB_ID]).toEqual([RELAY_PTY_ID]) + expect(buildWorkspaceSessionPayload(store.getState()).remoteSessionIdsByTabId).toEqual({ + [TAB_ID]: RELAY_PTY_ID + }) + }) + + it('keeps a deferred relay session id after a disconnect clears the row binding', async () => { + const store = seedStore('disconnected') + store.getState().hydrateWorkspaceSession(persistedSession()) + await store.getState().reconnectPersistedTerminals() + + expect(store.getState().deferredSshSessionIdsByTabId[TAB_ID]).toBe(RELAY_PTY_ID) + + // A relay drop clears the row binding. Loss of contact is not evidence the remote PTY exited, + // so the handle must survive into the next write. + store.getState().clearDirectSshTargetPtyBindings(TARGET_ID) + + expect(store.getState().tabsByWorktree[WORKTREE_ID][0].ptyId).toBeNull() + expect(store.getState().ptyIdsByTabId[TAB_ID]).toEqual([]) + expect(buildWorkspaceSessionPayload(store.getState()).remoteSessionIdsByTabId).toEqual({ + [TAB_ID]: RELAY_PTY_ID + }) + }) + + it('leaves a tab whose relay handle is unverifiable alone instead of retiring it', async () => { + const store = seedStore('disconnected') + store.getState().hydrateWorkspaceSession(persistedSession()) + await store.getState().reconnectPersistedTerminals() + store.getState().clearDirectSshTargetPtyBindings(TARGET_ID) + + // No live PTY, no row binding, host unreachable — `unverifiable`, never `exited`. + expect([...getOrphanTerminalIds(store.getState(), WORKTREE_ID)]).toEqual([]) + expect(store.getState().tabsByWorktree[WORKTREE_ID].map((tab) => tab.id)).toEqual([TAB_ID]) + }) + + it('does not invent a session id for a tab that never had one', () => { + const store = seedStore() + const session = persistedSession() + session.tabsByWorktree[WORKTREE_ID] = [ + makeTab({ id: TAB_ID, worktreeId: WORKTREE_ID, ptyId: null }) + ] + delete session.remoteSessionIdsByTabId + delete session.activeConnectionIdsAtShutdown + session.activeWorktreeIdsOnShutdown = [] + store.getState().hydrateWorkspaceSession(session) + + const payload = buildWorkspaceSessionPayload(store.getState()) + + expect(payload.remoteSessionIdsByTabId).toBeUndefined() + expect(payload.activeWorktreeIdsOnShutdown).toEqual([]) + }) +})