diff --git a/src/main/persistence.ts b/src/main/persistence.ts index e9e07cf3771..6aff60e9c13 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -7356,14 +7356,31 @@ export class Store { */ /** The PTY a pane is durably bound to. The desktop plane's home is `local` — the renderer is its * only publisher of pane membership. Hedging into `ssh:` let the headless plane's copy - * outvote the live binding and silently no-op supersession (STA-3077). */ + * outvote the live binding and silently no-op supersession (STA-3077). + * + * Keyed on the leaf, falling back across tabs: a lease freezes its tabId at write time, and + * `detachTerminalPaneToTab` moves a live pane, so the named tab can be the one the pane left. + * Missing the binding there makes it lose to recency and retires the pane's own shell. */ private durablyBoundPtyIdForPane( targetId: string, tabId: string, leafId: string ): string | undefined { + const findLeafBinding = (session: WorkspaceSessionState | undefined): string | undefined => { + const layouts = session?.terminalLayoutsByTabId + return ( + layouts?.[tabId]?.ptyIdsByLeafId?.[leafId] ?? + Object.values(layouts ?? {}).find((layout) => layout?.ptyIdsByLeafId?.[leafId]) + ?.ptyIdsByLeafId?.[leafId] + ) + } + // Local FIRST, and only then the headless plane. Local-first is what STA-3077 needed: the old + // code preferred `ssh:`, so a copy no live writer maintained outvoted the real binding + // and supersession no-opped. The fallback is not that hedge — it only speaks for a pane local + // says nothing about, so a headless-owned pane still gets a vote before its lease is retired. const boundPtyId = - this.state.workspaceSession?.terminalLayoutsByTabId?.[tabId]?.ptyIdsByLeafId?.[leafId] + findLeafBinding(this.state.workspaceSession) ?? + findLeafBinding(this.state.workspaceSessionsByHostId?.[toSshExecutionHostId(targetId)]) return boundPtyId ? this.getRelayPtyIdForSshLeaseComparison(targetId, boundPtyId) : undefined } diff --git a/src/main/ssh-pane-binding-partition.test.ts b/src/main/ssh-pane-binding-partition.test.ts index 95db734ae39..977609a9db7 100644 --- a/src/main/ssh-pane-binding-partition.test.ts +++ b/src/main/ssh-pane-binding-partition.test.ts @@ -225,6 +225,40 @@ describe('STA-3077 step P: one pane, one live claim across partitions', () => { }) }) +describe('STA-3077: arbitration follows the pane, not the tab it was written in', () => { + // A lease freezes its tabId when written; `detachTerminalPaneToTab` then moves the live pane and + // its PTY into a new tab. Looking the binding up under the frozen tab finds nothing, so the bound + // shell loses to recency and supersession retires the pane's OWN shell in favour of a stale one. + it('finds the binding after the pane has been moved to another tab', async () => { + const MOVED_TAB = 'tab-moved-to' + const store = await createStore() + // The renderer publishes the pane in its NEW tab; the lease still names the old one. + store.setWorkspaceSession( + { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { [WORKTREE]: [{ id: MOVED_TAB, worktreeId: WORKTREE }] }, + terminalLayoutsByTabId: { + [MOVED_TAB]: { + root: { type: 'leaf' as const, leafId: LEAF }, + activeLeafId: LEAF, + expandedLeafId: null, + ptyIdsByLeafId: { [LEAF]: appPtyId('pty-bound') } + } + } + } as never, + LOCAL_EXECUTION_HOST_ID + ) + sshSpawnUpsertsLease(store, 'pty-bound') + // A newer, unbound lease arrives for the same pane under the lease's frozen tab. + sshSpawnUpsertsLease(store, 'pty-newer') + + store.supersedeDuplicatePaneLeases(TARGET) + + // The bound shell must win. Keyed on the frozen tab, it is invisible and recency retires it. + expect(liveLeaseIdsForPane(store)).toEqual(['pty-bound']) + }) +}) + describe('STA-3077 step P: the desktop plane resolves from its own home', () => { // INVERTED. This clause used to require the two partitions to AGREE after load, which assumed // `ssh:` was a stale spill of this plane's state. It is not — it is the headless/CLI diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 734f303bec2..2e6726a64b4 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -9719,6 +9719,52 @@ describe('connectPanePty', () => { // shells still running under a relay process it replaced. Respawning there starts a second // `--resume` against the same agent session and both processes append to one transcript. This is // the last route to that defect, and it is the reason this pane surfaces as disconnected instead. + // The reattach can also FULFIL with a result that says the session expired, or with no usable + // pty id at all. Both trace back to the same relay not-found, so neither proves the shell exited — + // and both used to clear the binding and cold-restore resume the agent, which is the duplicate + // transcript this pane exists to prevent. SSH only: a daemon/local provider is authoritative + // about its own ptys and keeps replacing in place. + it.each([ + ['a result flagged sessionExpired', { id: 'tab-pty', sessionExpired: true }], + ['a result carrying no pty id', {}] + ])('surfaces the pane instead of resuming when an SSH reattach returns %s', async (_l, res) => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('tab-pty') + transport.getPtyId.mockImplementation(() => null) + transport.connect.mockImplementation(async ({ sessionId }: { sessionId?: string }) => + sessionId ? res : { id: 'respawned-pty' } + ) + transportFactoryQueue.push(transport) + + // connectionId resolves from the store's repo list, not from deps — an SSH-owned repo is what + // makes this pane SSH, and the branch under test is SSH-only by design. + mockStoreState = { + ...mockStoreState, + repos: [{ id: 'repo1', connectionId: 'target-a', displayName: 'orca' }] + } as StoreState + const pane = createPane(1) + const manager = createManager(1) + const onPtyRecoveryState = vi.fn<(paneId: number, state: RecoveryStateProbe) => void>() + const deps = createDeps({ + restoredLeafId: LEAF_1, + restoredPtyIdByLeafId: { [LEAF_1]: 'tab-pty' }, + onPtyRecoveryStateRef: { current: onPtyRecoveryState } + }) + + connectPanePty(pane as never, manager as never, deps as never) + await flushAsyncTicks(30) + + // Producer pin: the restore ran and returned this shape, or the clauses below are vacuous. + expect(transport.connect.mock.calls[0]?.[0]?.sessionId).toBe('tab-pty') + expect( + onPtyRecoveryState.mock.calls.find( + ([paneId, state]) => paneId === pane.id && state?.phase === 'disconnected' + )?.[1]?.unreachablePane + ).toBeDefined() + expect(transport.connect).toHaveBeenCalledTimes(1) + expect(deps.clearTabPtyId).not.toHaveBeenCalled() + }) + it('does not respawn a pane whose restore was answered with a not-found', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport('tab-pty') diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 5c790bf9579..dfea76a1cea 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -8126,14 +8126,19 @@ export function connectPanePty( paneId: pane.id, ptyId: staleSessionId ?? null }) - // Why: a stale restored session can fail reattach after mount; don't leave xterm alive without a backing PTY. - if (staleSessionId) { - deps.clearExitedPanePtyLayoutBinding(pane.id, staleSessionId) - } else { - deps.syncPanePtyLayoutBinding(pane.id, null) + // For an SSH pane a missing id says the reattach could not answer, not that the shell + // exited — a replaced relay reports that for shells still running under its predecessor — + // so surface it and let the user choose. A daemon/local pane is not ambiguous that way: its + // provider is authoritative about its own ptys, and it keeps replacing in place. + if (connectionId && staleSessionId) { + publishUnreachablePane(staleSessionId) + return false } if (staleSessionId) { + deps.clearExitedPanePtyLayoutBinding(pane.id, staleSessionId) deps.clearTabPtyId(deps.tabId, staleSessionId) + } else { + deps.syncPanePtyLayoutBinding(pane.id, null) } startFreshColdRestoreAgentResume(coldRestoreStartup, { forceBlankRestoredViewport: true @@ -8149,15 +8154,18 @@ export function connectPanePty( : {}) }) if (connectResult?.sessionExpired) { + // For SSH, `sessionExpired` traces back to the relay's not-found, which it also answers for + // shells still running under a relay process it replaced — not proof the shell exited. + if (connectionId && staleSessionId) { + publishUnreachablePane(staleSessionId) + return false + } if (staleSessionId) { deps.clearExitedPanePtyLayoutBinding(pane.id, staleSessionId) + deps.clearTabPtyId(deps.tabId, staleSessionId) } else { deps.syncPanePtyLayoutBinding(pane.id, null) } - if (staleSessionId) { - deps.clearTabPtyId(deps.tabId, staleSessionId) - } - // Why: SSH sleep/reconnect can invalidate the relay PTY while the tab stays mounted; replace the dead lease in-place, not a stale overlay. startFreshColdRestoreAgentResume(coldRestoreStartup, { forceBlankRestoredViewport: true })