diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 321632f8568..338e2f6e3d1 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -1,4 +1,5 @@ /* eslint-disable max-lines -- Why: PTY IPC is centralized in one main-process module so spawn env scoping, lifecycle cleanup, process inspection, and renderer IPC stay behind one audited boundary. */ +import { parsePtyExitedError } from '../../shared/ssh-pty-failure-tokens' import { join, delimiter } from 'node:path' import { randomUUID } from 'node:crypto' import { statSync } from 'node:fs' @@ -1121,12 +1122,19 @@ function isPtyProvenGoneForReplacement(err: unknown, ptyId: string): boolean { if (!parseAppSshPtyId(ptyId)) { return isPtyAlreadyGoneError(err) } - // The ONLY proving answer is the expiry the reattach mints after verifying the relay's proof - // names this shell and this incarnation. Accepting a bare SSH_PTY_EXITED too was over-acceptance: - // the reattach deliberately rethrows one it could NOT tie to us, and honouring that here retires - // the owner and respawns onto a shell somebody else is still using. const message = err instanceof Error ? err.message : String(err) - return message.includes(SSH_SESSION_EXPIRED_ERROR) + if (message.includes(SSH_SESSION_EXPIRED_ERROR)) { + // The reattach mints this only after verifying the relay's proof names this shell AND a + // relay-attested incarnation. Strongest available evidence. + return true + } + // A raw SSH_PTY_EXITED is one the reattach could not tie to an attested incarnation — either + // because the host is too old to attest one, or the lease's was synthesized. Accepting it + // blindly retired owners on a proof about somebody else's shell; refusing it outright stranded + // panes whose exit really was proven and for which no incarnation exists to check. Require the + // proof to name THIS pty: without an incarnation that is the only evidence there is, and an id + // mismatch is exactly the case that must not authorize a respawn. + return parsePtyExitedError(message)?.id === ptyId } function isPtyAlreadyGoneError(err: unknown): boolean { diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 945bc69e026..89c6e78a322 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -2868,9 +2868,11 @@ export function hoistSshPartitionsIntoLocalSession( /** Passed explicitly: pane-identity normalization prunes partitions before this runs. */ sourcePartitions?: PersistedState['workspaceSessionsByHostId'] ): boolean { - if (state.settings?.sshPartitionHoistSeed === 'done') { - return false - } + // Per-tab, because `ssh:` is a LIVE plane, not a legacy one — the headless and CLI + // surfaces still write it. A one-shot flag would snapshot it once and strand everything added + // afterwards. Recording tabs instead keeps the hoist repeatable while never re-adding a tab the + // user has closed, which is the resurrection this guard exists to prevent. + const alreadyHoisted = new Set(state.settings?.sshHoistedTabIds ?? []) const partitions = sourcePartitions ?? state.workspaceSessionsByHostId if (!partitions) { return false @@ -2887,11 +2889,14 @@ export function hoistSshPartitionsIntoLocalSession( for (const [worktreeId, tabs] of Object.entries(partition.tabsByWorktree ?? {})) { const existing = local.tabsByWorktree?.[worktreeId] ?? [] const known = new Set(existing.map((tab) => tab.id)) - const missing = (tabs ?? []).filter((tab) => !known.has(tab.id)) + const missing = (tabs ?? []).filter((tab) => !known.has(tab.id) && !alreadyHoisted.has(tab.id)) if (missing.length === 0) { continue } local.tabsByWorktree = { ...local.tabsByWorktree, [worktreeId]: [...existing, ...missing] } + for (const tab of missing) { + alreadyHoisted.add(tab.id) + } changed = true } for (const [tabId, layout] of Object.entries(partition.terminalLayoutsByTabId ?? {})) { @@ -2921,11 +2926,9 @@ export function hoistSshPartitionsIntoLocalSession( changed = true } } - // Mark it done even when nothing moved. The partition itself must SURVIVE — headless and CLI - // own panes in that plane — so the only way to stop re-hoisting is to record that we have run. - // Without this, "local has no such tab" is both the hoist condition and the state produced by - // the user closing a tab, so closed panes came back on the next launch. - state.settings = { ...state.settings, sshPartitionHoistSeed: 'done' } + if (changed) { + state.settings = { ...state.settings, sshHoistedTabIds: [...alreadyHoisted] } + } return changed } diff --git a/src/main/ssh-partition-upgrade-tab-loss.test.ts b/src/main/ssh-partition-upgrade-tab-loss.test.ts index e4c11fcc2a0..661ba5cc0fc 100644 --- a/src/main/ssh-partition-upgrade-tab-loss.test.ts +++ b/src/main/ssh-partition-upgrade-tab-loss.test.ts @@ -49,9 +49,9 @@ async function createStore(state: Record = {}) { function stripHoistMarker(): void { const file = join(testState.dir, 'orca-data.json') const state = JSON.parse(readFileSync(file, 'utf-8')) as { - settings?: { sshPartitionHoistSeed?: string } + settings?: { sshHoistedTabIds?: string[] } } - delete state.settings?.sshPartitionHoistSeed + delete state.settings?.sshHoistedTabIds writeFileSync(file, JSON.stringify(state), 'utf-8') } @@ -195,4 +195,32 @@ describe('an SSH pane whose membership was persisted into the ssh partition', () 'the migration re-added a tab the user had closed' ).toHaveLength(0) }) + + // The ssh: plane is still written by the headless and CLI surfaces, so the hoist has to + // stay repeatable. A one-shot flag would snapshot the plane once and strand every pane added + // after it — the same tab loss, made permanent instead of lasting one launch. + it('still hoists a tab the partition gains after an earlier hoist', async () => { + const before = await createStore() + before.setWorkspaceSession(paneSession() as never, SSH_PARTITION) + before.flushOrThrow() + stripHoistMarker() + + const upgraded = await reopenStore() + expect(upgraded.getWorkspaceSession().tabsByWorktree?.[WORKTREE] ?? []).toHaveLength(1) + + // The live plane gains a SECOND pane after the first hoist has already run. + const second = paneSession() + second.tabsByWorktree[WORKTREE] = [ + ...second.tabsByWorktree[WORKTREE], + { ...second.tabsByWorktree[WORKTREE][0], id: 'tab-2' } + ] + upgraded.setWorkspaceSession(second as never, SSH_PARTITION) + upgraded.flushOrThrow() + + const relaunched = await reopenStore() + expect( + (relaunched.getWorkspaceSession().tabsByWorktree?.[WORKTREE] ?? []).map((tab) => tab.id).sort(), + 'a pane the partition gained after the first hoist was stranded' + ).toEqual(['tab-1', 'tab-2']) + }) }) diff --git a/src/shared/types.ts b/src/shared/types.ts index 710da60e962..dfb37513cd3 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -3210,8 +3210,10 @@ export type GlobalSettings = { /** One-shot cohort marker for the tab-switch keybinding swap. 'pending' = * pre-existing install (seed pins old chords, then flips to 'done'); 'done' = fresh install. */ tabSwitchKeybindingSeed?: 'pending' | 'done' - /** One-shot: SSH host session partitions have been folded into the local session. */ - sshPartitionHoistSeed?: 'done' + /** Tabs already folded from an SSH host partition into the local session. Per-tab, not a + * one-shot flag: that plane is still written by the headless and CLI surfaces, so a single + * stamp would strand every pane they add after it. */ + sshHoistedTabIds?: string[] /** Local voice/dictation config. Optional for pre-voice profiles; getDefaultSettings() hydrates defaults via the persistence merge. */ voice?: VoiceSettings }