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 8dd2bc348..9c6bc5452 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -10699,7 +10699,7 @@ describe('connectPanePty', () => { } } - it('fires listSessions at most once across many keystrokes in one resume window', async () => { + it('does not fire listSessions for first input on a fresh mount', async () => { const listSessions = vi.mocked(window.api.pty.listSessions) listSessions.mockClear() const { typeKeystroke } = await connectActivePaneWithInput() @@ -10708,19 +10708,31 @@ describe('connectPanePty', () => { typeKeystroke('x') } - expect(listSessions).toHaveBeenCalledTimes(1) + expect(listSessions).not.toHaveBeenCalled() }) - it('re-arms one re-check after a visibility resume', async () => { + it('fires listSessions once for the first input after a visibility resume', async () => { const listSessions = vi.mocked(window.api.pty.listSessions) listSessions.mockClear() const { binding, typeKeystroke } = await connectActivePaneWithInput() + binding.noteVisibilityResume() + typeKeystroke('a') + typeKeystroke('b') + + expect(listSessions).toHaveBeenCalledTimes(1) + }) + + it('re-arms one re-check after a second visibility resume', async () => { + const listSessions = vi.mocked(window.api.pty.listSessions) + listSessions.mockClear() + const { binding, typeKeystroke } = await connectActivePaneWithInput() + + binding.noteVisibilityResume() typeKeystroke('a') typeKeystroke('b') expect(listSessions).toHaveBeenCalledTimes(1) - // Resume re-arms exactly one more re-check. binding.noteVisibilityResume() typeKeystroke('c') typeKeystroke('d') @@ -10743,8 +10755,35 @@ describe('connectPanePty', () => { paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) } }) const pane = createPane(2) - connectPanePty(pane as never, manager as never, deps as never) + const binding = connectPanePty(pane as never, manager as never, deps as never) as unknown as { + noteVisibilityResume: () => void + } + binding.noteVisibilityResume() + sendTerminalInputThroughPane(pane, 'x') + sendTerminalInputThroughPane(pane, 'y') + + expect(listSessions).not.toHaveBeenCalled() + }) + + it('never fires listSessions for an SSH pane after resume', async () => { + const listSessions = vi.mocked(window.api.pty.listSessions) + listSessions.mockClear() + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('ssh-pty-2') + transport.getConnectionId.mockReturnValue('ssh-connection-1') + transportFactoryQueue.push(transport) + const manager = createManager(2) + const deps = createDeps({ + restoredLeafId: LEAF_2, + paneTransportsRef: { current: new Map([[1, createMockTransport('pty-pane-1')]]) } + }) + const pane = createPane(2) + const binding = connectPanePty(pane as never, manager as never, deps as never) as unknown as { + noteVisibilityResume: () => void + } + + binding.noteVisibilityResume() sendTerminalInputThroughPane(pane, 'x') sendTerminalInputThroughPane(pane, 'y') diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index a8e2118c2..153a12e36 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -2282,7 +2282,11 @@ export function connectPanePty( const rows = pane.terminal.rows return cols > 0 && rows > 0 ? { cols, rows } : null }, - resize: (cols, rows) => transport.resize(cols, rows), + resize: (cols, rows) => { + if (!shouldSuppressDesktopPtyResize()) { + transport.resize(cols, rows) + } + }, // Why: confirm the PTY actually applied the size we forwarded before the // reconcile hands off. transport.resize is fire-and-forget for daemon/SSH // PTYs, so the loop can otherwise settle on a size the PTY dropped, leaving @@ -4550,19 +4554,11 @@ export function connectPanePty( onExit(currentPtyId) } - // Why (perf): the only moment a daemon session can be reaped behind the - // renderer's back is while the pane was surface-hidden. So the input-driven - // re-check is only useful in the window right after a resume — once it (or - // the resume pass) has confirmed liveness for this resume, re-polling on - // every subsequent keystroke is pure waste: listSessions() is a - // renderer→main→daemon round-trip (DaemonPtyAdapter.listProcesses requests - // `listSessions` from the daemon subprocess), so an ungated per-keystroke - // re-check would put a process-enumeration round-trip on the typing hot path - // for every healthy local pane. Fire at most ONCE per resume window; reset - // on the next hide→show. This preserves the "reduces not eliminates the - // first-keystroke drop" intent — the first keystroke after a resume still - // triggers exactly one re-check. - let livenessRecheckFiredSinceResume = false + // Why (perf + startup correctness): listSessions() is authoritative only + // after a real visibility resume. Fresh PTY startup can briefly lag the daemon + // listing, so newborn terminals start disarmed and noteVisibilityResume grants + // exactly one first-input liveness probe for the next resume window. + let livenessRecheckArmedForResume = false // Why (Defect #2 defense-in-depth): in the broken state sendInput returns // true (connected/ptyId still set) so the dropped keystroke is invisible to @@ -4571,9 +4567,13 @@ export function connectPanePty( // pass alone. It REDUCES but cannot eliminate the first-keystroke drop (that // byte is already gone daemon-side). const recheckLivenessAfterInput = (): void => { - if (disposed || livenessRecheckFiredSinceResume) { + if (disposed || !livenessRecheckArmedForResume) { return } + // Why: consume the resume token before inspecting provider details so SSH, + // remote-runtime, and concurrent keystrokes cannot retry this hot-path check + // until the lifecycle reports another true hidden-to-visible resume. + livenessRecheckArmedForResume = false const currentPtyId = transport.getPtyId() const currentConnectionId = transport.getConnectionId?.() if ( @@ -4588,9 +4588,6 @@ export function connectPanePty( ) { return } - // Why: set BEFORE the IPC so concurrent keystrokes coalesce to one in-flight - // request rather than fanning out a round-trip per byte typed. - livenessRecheckFiredSinceResume = true void window.api.pty .listSessions() .then((sessions) => { @@ -4608,7 +4605,7 @@ export function connectPanePty( // visible again. Called from the lifecycle visibility effect; the gate // keeps the typing hot path off the listSessions IPC between resumes. noteVisibilityResume() { - livenessRecheckFiredSinceResume = false + livenessRecheckArmedForResume = true // Why: re-assert the PTY size on resume so a resize that was dropped while // this pane was hidden self-heals on show, instead of waiting for a manual // resize that may never change xterm's column count. diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts index 97136f027..3cadbc92c 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts @@ -2,6 +2,8 @@ import { describe, expect, it, vi } from 'vitest' import { applyTerminalScrollbackRowsToMountedPanes, clearQueuedInitialCwdAfterFirstPane, + getPreviousVisibleForTerminalPane, + isTerminalPaneVisibilityResume, mapRestoredPaneTitlesByPaneId, resolvePaneLinkCwd, resolvePaneSeedCwd, @@ -290,6 +292,39 @@ describe('suppressIntentionalPaneCloseExit', () => { }) describe('scheduleVisibilityReconcilePass', () => { + it('ignores previous visibility from a different terminal identity', () => { + expect( + getPreviousVisibleForTerminalPane({ + previous: { tabId: 'tab-old', cwd: '/repo', isVisible: false }, + tabId: 'tab-new', + cwd: '/repo' + }) + ).toBeNull() + expect( + getPreviousVisibleForTerminalPane({ + previous: { tabId: 'tab-1', cwd: '/repo-old', isVisible: false }, + tabId: 'tab-1', + cwd: '/repo-new' + }) + ).toBeNull() + expect( + getPreviousVisibleForTerminalPane({ + previous: { tabId: 'tab-1', cwd: '/repo', isVisible: false }, + tabId: 'tab-1', + cwd: '/repo' + }) + ).toBe(false) + }) + + it('identifies only hidden-to-visible changes as visibility resumes', () => { + expect(isTerminalPaneVisibilityResume({ previousIsVisible: null, isVisible: true })).toBe(false) + expect(isTerminalPaneVisibilityResume({ previousIsVisible: true, isVisible: true })).toBe(false) + expect(isTerminalPaneVisibilityResume({ previousIsVisible: true, isVisible: false })).toBe( + false + ) + expect(isTerminalPaneVisibilityResume({ previousIsVisible: false, isVisible: true })).toBe(true) + }) + it('schedules a reconcile pass over the bindings when becoming visible', async () => { const reconcileIfSessionDead = vi.fn() const listSessions = vi @@ -297,6 +332,7 @@ describe('scheduleVisibilityReconcilePass', () => { .mockResolvedValue([{ id: 'live-1', cwd: '/a', title: 'a' }]) const scheduled = scheduleVisibilityReconcilePass({ + previousIsVisible: false, isVisible: true, bindings: [{ reconcileIfSessionDead }], listSessions @@ -310,12 +346,29 @@ describe('scheduleVisibilityReconcilePass', () => { expect(reconcileIfSessionDead).toHaveBeenCalledWith(new Set(['live-1'])) }) + it('does not schedule on an initially visible mount', () => { + const listSessions = vi + .fn<() => Promise<{ id: string; cwd: string; title: string }[]>>() + .mockResolvedValue([]) + + const scheduled = scheduleVisibilityReconcilePass({ + previousIsVisible: null, + isVisible: true, + bindings: [{ reconcileIfSessionDead: vi.fn() }], + listSessions + }) + + expect(scheduled).toBe(false) + expect(listSessions).not.toHaveBeenCalled() + }) + it('self-gates: does not schedule when hiding (isVisible false)', () => { const listSessions = vi .fn<() => Promise<{ id: string; cwd: string; title: string }[]>>() .mockResolvedValue([]) const scheduled = scheduleVisibilityReconcilePass({ + previousIsVisible: true, isVisible: false, bindings: [{ reconcileIfSessionDead: vi.fn() }], listSessions diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index d2f2ab271..9fce06be2 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -435,15 +435,41 @@ export function shouldDetachPaneTransportOnUnmount(args: { /** * Self-gating dead-session reconcile pass scheduled from the isVisible effect. * Why self-gate: the effect fires on BOTH isVisible true and false, but we only - * reconcile on resume (becoming visible), never on hide. Returns true when the - * pass was scheduled so the resume-unit test can assert the gate. + * reconcile on resume (hidden to visible), never on hide or initial mount. + * Returns true when the pass was scheduled so the resume-unit test can assert + * the gate. */ +export function isTerminalPaneVisibilityResume(args: { + previousIsVisible: boolean | null + isVisible: boolean +}): boolean { + return args.previousIsVisible === false && args.isVisible +} + +type TerminalPaneVisibilitySnapshot = { + tabId: string + cwd: string | null | undefined + isVisible: boolean +} + +export function getPreviousVisibleForTerminalPane(args: { + previous: TerminalPaneVisibilitySnapshot | null + tabId: string + cwd: string | null | undefined +}): boolean | null { + if (args.previous?.tabId !== args.tabId || args.previous.cwd !== args.cwd) { + return null + } + return args.previous.isVisible +} + export function scheduleVisibilityReconcilePass(args: { + previousIsVisible: boolean | null isVisible: boolean bindings: Iterable listSessions: () => Promise<{ id: string; cwd: string; title: string }[]> }): boolean { - if (!args.isVisible) { + if (!isTerminalPaneVisibilityResume(args)) { return false } // Why: fire-and-forget so the async listSessions IPC never blocks the @@ -515,6 +541,7 @@ export function useTerminalPaneLifecycle({ ) const systemPrefersDarkRef = useRef(systemPrefersDark) systemPrefersDarkRef.current = systemPrefersDark + const previousVisibleForReconcileRef = useRef(null) const linkProviderDisposablesRef = useRef(new Map()) const terminalHandleLinkDisposablesRef = useRef(new Map()) const fileLinkClickFallbackDisposablesRef = useRef(new Map()) @@ -1639,7 +1666,14 @@ export function useTerminalPaneLifecycle({ }, [tabId, cwd]) useEffect(() => { + const previousIsVisible = getPreviousVisibleForTerminalPane({ + previous: previousVisibleForReconcileRef.current, + tabId, + cwd + }) + previousVisibleForReconcileRef.current = { tabId, cwd, isVisible } isVisibleRef.current = isVisible + const resumedFromHidden = isTerminalPaneVisibilityResume({ previousIsVisible, isVisible }) for (const panePtyBinding of panePtyBindingsRef.current.values()) { const bindingWithVisibility = panePtyBinding as IDisposable & { syncProcessTracking?: () => void @@ -1649,21 +1683,22 @@ export function useTerminalPaneLifecycle({ // Why: re-arm the once-per-resume input liveness re-check so the typing // hot path stays off the listSessions IPC between resumes (the re-check // is only useful right after a hidden→visible flip). - if (isVisible) { + if (resumedFromHidden) { bindingWithVisibility.noteVisibilityResume?.() } } // Why: the reconcile pass self-gates on becoming visible (resume) — the - // effect also fires on hide — and runs fire-and-forget alongside - // syncProcessTracking. reconcileDeadSessions re-validates identity at apply - // time so a racing reattach is not clobbered. + // effect also fires on hide and initial mount. Initial visible mounts are + // fresh PTY startup, so an early listSessions snapshot must not close the + // newborn tab before the daemon lists it. scheduleVisibilityReconcilePass({ + previousIsVisible, isVisible, bindings: panePtyBindingsRef.current.values() as Iterable, listSessions: () => window.api.pty.listSessions() }) - // eslint-disable-next-line react-hooks/exhaustive-deps -- Why: visibility flips must refresh existing PTY process tracking even though the ref object identity is stable. - }, [isVisible, isVisibleRef, panePtyBindingsRef]) + // eslint-disable-next-line react-hooks/exhaustive-deps -- Why: visibility and terminal identity changes must refresh existing PTY process tracking even though the ref object identity is stable. + }, [cwd, isVisible, isVisibleRef, panePtyBindingsRef, tabId]) useEffect(() => { const manager = managerRef.current