Keep new terminal tabs open during startup (#6796)

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Brennan Benson 2026-06-29 16:04:31 -07:00 committed by GitHub
parent 08e824dda9
commit 53e2582cbe
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 157 additions and 33 deletions

View File

@ -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')

View File

@ -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.

View File

@ -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

View File

@ -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<ReconcilableBinding>
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<TerminalPaneVisibilitySnapshot | null>(null)
const linkProviderDisposablesRef = useRef(new Map<number, IDisposable>())
const terminalHandleLinkDisposablesRef = useRef(new Map<number, IDisposable>())
const fileLinkClickFallbackDisposablesRef = useRef(new Map<number, IDisposable>())
@ -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<ReconcilableBinding>,
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