diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index aacfd9f04ce..94252a72e84 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -1,5 +1,6 @@ /* 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 type { PtyBindingRefusalReason } from '../persistence' import { join, delimiter } from 'node:path' import { randomUUID } from 'node:crypto' import { statSync } from 'node:fs' @@ -581,9 +582,11 @@ export function bindPaneShell(args: { mayCreate?: boolean expectedBinding?: { ptyId: string; incarnationId?: string } expectedSourceBinding?: PtyBindingSourceExpectation -}): { bound: boolean; tabId: string } { +}): { bound: boolean; tabId: string; refusalReason?: PtyBindingRefusalReason } { const tabId = args.tabId + const refusal: { reason?: PtyBindingRefusalReason } = {} const bound = args.store?.persistPtyBinding({ + refusal, worktreeId: args.worktreeId, tabId, leafId: args.leafId, @@ -595,7 +598,13 @@ export function bindPaneShell(args: { ...(args.expectedSourceBinding ? { expectedSourceBinding: args.expectedSourceBinding } : {}) }) if (bound === false) { - return { bound: false, tabId } + // Why route the pane key even on refusal, unless the tab is gone: a refusal for stale + // membership leaves a LIVE tab whose pane is already listed. Dropping its routing on top of + // its binding orphans the shell from the surfaces that can still show it. + if (refusal.reason !== 'noTab') { + rememberPaneKeyForPty(args.ptyId, makePaneKey(tabId, args.leafId)) + } + return { bound: false, tabId, ...(refusal.reason ? { refusalReason: refusal.reason } : {}) } } rememberPaneKeyForPty(args.ptyId, makePaneKey(tabId, args.leafId)) return { bound: true, tabId } diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 851c09988af..9a1fdfd899f 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -2853,6 +2853,10 @@ export type PtyBindingSourceExpectation = { /** Caps the hoist ledger. Far above any realistic open-tab count, so a live tab is never evicted. */ const MAX_HOISTED_TAB_IDS = 512 +/** Why a `mayCreate: false` binding write was refused. `noTab` means the pane has no durable tab, + * so publishing it anywhere would be publishing something that does not exist. */ +export type PtyBindingRefusalReason = 'noTab' | 'noMembership' + /** Expired leases are unreachable from reattach; kept only long enough to outlive any in-flight * recovery window by orders of magnitude. */ const EXPIRED_SSH_LEASE_RETENTION_MS = 7 * 24 * 60 * 60 * 1000 @@ -7055,11 +7059,15 @@ export class Store { /** Reattach passes false: an absent durable pane never authorizes creating UI. * Defaults true so the spawn path keeps its force-quit-race branches. */ mayCreate?: boolean + /** Filled in when the write is refused, so a caller can tell "no such tab" — where anything + * it publishes would be a ghost — from "the tab is live but its membership is stale". */ + refusal?: { reason?: PtyBindingRefusalReason } expectedSourceBinding?: PtyBindingSourceExpectation }, hostId?: string | null ): boolean { const mayCreate = args.mayCreate ?? true + let refusalReason: PtyBindingRefusalReason | undefined const resolvedHostId = this.resolveHostId(hostId) const session = this.getWorkspaceSession(resolvedHostId) const paneKey = `${args.tabId}:${args.leafId}` @@ -7152,6 +7160,7 @@ export class Store { tab.ptyId = args.ptyId } else { terminalMembershipChanged = true + refusalReason = 'noTab' // Why: pty:spawn can beat the debounced writer; persist a minimal tab so hydration won't prune the binding as orphaned. const nextTabs = [ ...(tabs ?? []), @@ -7174,6 +7183,9 @@ export class Store { } if (!mayCreate && terminalMembershipChanged) { restoreSession() + if (args.refusal) { + args.refusal.reason = refusalReason ?? 'noMembership' + } return false } if (!isTerminalLeafId(args.leafId)) { @@ -7191,12 +7203,14 @@ export class Store { if (layout) { if (!layout.root) { terminalMembershipChanged = true + refusalReason ??= 'noMembership' // Why: createTab can persist an empty layout before TerminalPane mounts; the sync binding still needs a durable root. layout.root = { type: 'leaf', leafId: args.leafId } layout.activeLeafId = args.leafId layout.expandedLeafId = null } else if (!layoutContainsLeafId(layout.root, args.leafId)) { terminalMembershipChanged = true + refusalReason ??= 'noMembership' // Why: splitPane spawns before its snapshot reaches main; add a minimal leaf so a crash can't strand the pane's binding. layout.root = { type: 'split', @@ -7215,6 +7229,7 @@ export class Store { } } else { terminalMembershipChanged = true + refusalReason ??= 'noMembership' // Why: first tab spawn — persist a minimal layout so a SIGKILL before the renderer snapshot can't lose ptyIdsByLeafId. session.terminalLayoutsByTabId = { ...session.terminalLayoutsByTabId, @@ -7228,6 +7243,9 @@ export class Store { } if (!mayCreate && terminalMembershipChanged) { restoreSession() + if (args.refusal) { + args.refusal.reason = refusalReason ?? 'noMembership' + } return false } advanceTopologyFence() diff --git a/src/main/ssh-partition-upgrade-tab-loss.test.ts b/src/main/ssh-partition-upgrade-tab-loss.test.ts index 02b70e1673b..8976780344a 100644 --- a/src/main/ssh-partition-upgrade-tab-loss.test.ts +++ b/src/main/ssh-partition-upgrade-tab-loss.test.ts @@ -275,4 +275,49 @@ describe('an SSH pane whose membership was persisted into the ssh partition', () 'a tab whose pane had not mounted yet was stranded in the partition' ).toHaveLength(1) }) + + // The refusal reason decides whether a still-live pane keeps its routing and its mobile/CLI + // surface. Getting it wrong in either direction is a real failure: 'noTab' on a live tab + // orphans a running shell, and 'noMembership' on a missing tab publishes something that does + // not exist. + describe('why a mayCreate:false write was refused', () => { + it('reports noTab when the pane has no durable tab at all', async () => { + const store = await createStore() + const refusal: { reason?: string } = {} + + const bound = store.persistPtyBinding({ + worktreeId: WORKTREE, + tabId: TAB, + leafId: LEAF, + ptyId: PTY, + mayCreate: false, + refusal + } as never) + + expect(bound).toBe(false) + expect(refusal.reason).toBe('noTab') + }) + + it('reports noMembership when the tab is live but the leaf is not in its layout', async () => { + const store = await createStore() + // A live tab with a layout that does not contain the leaf being bound. + store.setWorkspaceSession(paneSession() as never) + const refusal: { reason?: string } = {} + + const bound = store.persistPtyBinding({ + worktreeId: WORKTREE, + tabId: TAB, + leafId: LEAF_TWO, + ptyId: PTY, + mayCreate: false, + refusal + } as never) + + expect(bound).toBe(false) + expect( + refusal.reason, + 'a live tab was reported as missing, which orphans its running shell' + ).toBe('noMembership') + }) + }) }) diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index ea56dd88786..a292184352a 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -1,6 +1,7 @@ /* oxlint-disable max-lines */ // Why: single authority for all relay lifecycle state per SSH target (previously scattered across module Maps/Sets with duplicated paths). +import type { PtyBindingRefusalReason } from '../persistence' import { randomUUID } from 'node:crypto' import type { BrowserWindow } from 'electron' import { deployAndLaunchRelay } from './ssh-relay-deploy' @@ -2602,7 +2603,8 @@ export class SshRelaySession { // live layout outranks it. Resolved before the write, so a throw cannot lose it and leave the // graph registered under the tab the pane left. const tabId = resolvePaneShellTabId(this.store, lease.leafId) ?? lease.tabId - let bind: { bound: boolean; tabId: string } | null = null + let bind: { bound: boolean; tabId: string; refusalReason?: PtyBindingRefusalReason } | null = + null try { bind = bindPaneShell({ store: this.store, @@ -2616,14 +2618,23 @@ export class SshRelaySession { } catch (error) { console.error('[ssh-relay-session] Failed to persist reconnect incarnation:', error) } - if (bind?.bound === false) { - // Unresolved, not dead: the remote shell keeps running and stays - // reattachable once a durable pane names it again. + if (bind?.bound === false && bind.refusalReason === 'noTab') { + // No durable tab owns this leaf, so there is nothing to publish it against — registering + // would announce a pane that does not exist. Unresolved, not dead: the shell keeps running + // and stays reattachable once a durable pane names it again. console.info( `[ssh-relay-session] No durable pane owns ${appPtyId} on ${this.targetId}; left running unbound.` ) return } + if (bind?.bound === false) { + // The tab IS live — only its membership record is stale, and the mobile/CLI listing already + // publishes that tab. Returning here left a visible pane wired to a stale pty while its + // shell ran on unreachable, recoverable only by editing state by hand. + console.info( + `[ssh-relay-session] Stale membership for ${appPtyId} on ${this.targetId}; registering against its live tab.` + ) + } // The tab the bind resolved, not the lease's frozen one — a moved pane must not be recorded // in the graph under the tab it left while its record and fence name the tab it is in. this.runtime?.registerPty(appPtyId, lease.worktreeId, this.targetId, {