From 10ef68f01e9ea3cf4a37e80e0e5a2cc321866e5e Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 24 Oct 2026 18:31:31 -0700 Subject: [PATCH] fix(terminal): keep keys typed before a pane's terminal exists (#33281) * fix(terminal): keep keys typed before a pane's PTY exists * fix(terminal): hold pre-spawn keys only in panes with no startup work * fix(terminal): no pre-spawn hold for live-entry resumes or main-started panes * fix(terminal): keep the deferred-split hold exactly as before * fix(terminal): drop held keys if a resume or startup work appears before the bind --- .../pty-connection-preconnect-input.test.ts | 262 ++++++++++++++++++ .../cold-restore-resume-startup.ts | 39 ++- .../pty-connection/pane-transport-options.ts | 35 ++- .../pty-connection/run-deferred-connect.ts | 5 + .../pty-transport-input-write.test.ts | 54 ++++ .../terminal-pane/pty-transport-types.ts | 4 +- .../components/terminal-pane/pty-transport.ts | 4 + 7 files changed, 386 insertions(+), 17 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/pty-connection-preconnect-input.test.ts diff --git a/src/renderer/src/components/terminal-pane/pty-connection-preconnect-input.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-preconnect-input.test.ts new file mode 100644 index 00000000000..7b0a7467b21 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection-preconnect-input.test.ts @@ -0,0 +1,262 @@ +import type * as React from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { connectPanePty } from './pty-connection' +import { makePaneKey } from '../../../../shared/stable-pane-id' +import { flushAsyncTicks } from './pty-connection-test-async' +import { + createManager, + createMockTransport, + createPane, + leafIdForPane, + type MockTransport +} from './pty-connection-test-pane-fixtures' +import { buildPaneConnectionDeps } from './pty-connection-test-deps' +import { + installTerminalTestGlobals, + restoreTerminalTestGlobals +} from './pty-connection-test-environment' +import { createInitialStoreState } from './pty-connection-test-store-fixtures' +import type { StoreState } from './pty-connection-test-store-state' +import { clearWorktreeSleepIntent, markWorktreeSleepIntent } from '@/lib/worktree-sleep-intent' + +const { + notifyCodexPaneBoundForStaleSweep, + scheduleRuntimeGraphSync, + shouldSeedCacheTimerOnInitialTitle, + toastInfo +} = vi.hoisted(() => ({ + notifyCodexPaneBoundForStaleSweep: vi.fn(), + scheduleRuntimeGraphSync: vi.fn(), + shouldSeedCacheTimerOnInitialTitle: vi.fn(() => false), + toastInfo: vi.fn() +})) + +let mockStoreState: StoreState +let transportFactoryQueue: MockTransport[] = [] +let createdTransportOptions: Record[] = [] +let storeSubscribers: ((state: StoreState) => void)[] = [] + +const layoutOwner = vi.hoisted(() => ({ isMain: false })) + +vi.mock('../../../../shared/layout-owner-switch', () => ({ + get ORCA_LAYOUT_OWNER_IS_MAIN() { + return layoutOwner.isMain + } +})) + +vi.mock('@/runtime/sync-runtime-graph', () => ({ scheduleRuntimeGraphSync })) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => mockStoreState, + subscribe: (listener: (state: StoreState) => void) => { + storeSubscribers.push(listener) + return () => { + storeSubscribers = storeSubscribers.filter((candidate) => candidate !== listener) + } + } + } +})) + +vi.mock('@/lib/agent-status', async (importOriginal) => { + const { buildAgentStatusModuleMock } = await import('./pty-connection-test-environment') + return buildAgentStatusModuleMock(await importOriginal>()) +}) + +vi.mock('./cache-timer-seeding', () => ({ shouldSeedCacheTimerOnInitialTitle })) + +vi.mock('sonner', () => ({ toast: { info: toastInfo } })) + +vi.mock('@/lib/codex-stale-pane-sweep', () => ({ notifyCodexPaneBoundForStaleSweep })) + +vi.mock('react', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useCallback: unknown>(fn: T): T => fn + } +}) + +vi.mock('./pty-transport', () => ({ + createIpcPtyTransport: vi.fn((options: Record) => { + createdTransportOptions.push(options) + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + }) +})) + +function connectPlainPane( + overrides: Record = {} +): ReturnType { + const connectArgs = [ + createPane(1), + createManager(1), + buildPaneConnectionDeps(() => mockStoreState, overrides) + ] + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the shared pane fixtures implement the members connectPanePty touches. + return connectPanePty(...(connectArgs as unknown as Parameters)) +} + +describe('connectPanePty pre-connect input', () => { + beforeEach(async () => { + vi.clearAllMocks() + transportFactoryQueue = [] + createdTransportOptions = [] + storeSubscribers = [] + mockStoreState = createInitialStoreState(() => mockStoreState) + await installTerminalTestGlobals() + }) + + afterEach(async () => { + layoutOwner.isMain = false + clearWorktreeSleepIntent('wt-1') + await restoreTerminalTestGlobals() + }) + + it('holds keys typed before an ordinary pane has a PTY', async () => { + const transport = createMockTransport('pty-1') + transportFactoryQueue.push(transport) + + const binding = connectPlainPane() + await flushAsyncTicks(20) + + expect(createdTransportOptions[0]?.bufferInputUntilConnect).toBe(true) + binding.dispose() + }) + + // Why: held keys would flush ahead of the startup command, so a typed prompt would run in the shell. + it('keeps today’s drop for a pane with a startup command', async () => { + transportFactoryQueue.push(createMockTransport('pty-1')) + + const binding = connectPlainPane({ startup: { command: 'claude' } }) + await flushAsyncTicks(20) + + expect(createdTransportOptions[0]?.bufferInputUntilConnect).toBeUndefined() + binding.dispose() + }) + + it('keeps holding a deferred split’s keys even when it has startup work, as before', async () => { + transportFactoryQueue.push(createMockTransport('pty-1')) + + const binding = connectPlainPane({ + startup: { command: 'claude' }, + cwdPromise: new Promise(() => {}) + }) + + expect(createdTransportOptions[0]?.bufferInputUntilConnect).toBe(true) + binding.dispose() + }) + + it('keeps today’s drop for a pane that resumes a sleeping agent', async () => { + const paneKey = makePaneKey('tab-1', leafIdForPane(1)) + mockStoreState.sleepingAgentSessionsByPaneKey[paneKey] = { + paneKey, + tabId: 'tab-1', + worktreeId: 'wt-1' + } + transportFactoryQueue.push(createMockTransport()) + + const binding = connectPlainPane() + + expect(createdTransportOptions[0]?.bufferInputUntilConnect).toBeUndefined() + binding.dispose() + }) + + it('keeps today’s drop for a pane that resumes its live agent status entry', async () => { + const paneKey = makePaneKey('tab-1', leafIdForPane(1)) + mockStoreState.agentStatusByPaneKey[paneKey] = { + state: 'working', + prompt: '', + agentType: 'claude', + paneKey, + updatedAt: 1_000, + stateStartedAt: 1_000, + stateHistory: [], + providerSession: { key: 'session_id', id: 'session-1' } + } + transportFactoryQueue.push(createMockTransport()) + + const binding = connectPlainPane() + + expect(createdTransportOptions[0]?.bufferInputUntilConnect).toBeUndefined() + binding.dispose() + }) + + it('keeps today’s drop for a pane main starts, whose launch this window cannot see', async () => { + layoutOwner.isMain = true + transportFactoryQueue.push(createMockTransport()) + + const binding = connectPlainPane() + + expect(createdTransportOptions[0]?.bufferInputUntilConnect).toBeUndefined() + binding.dispose() + }) + + it('stops releasing held keys once a resume source lands before the first bind', async () => { + transportFactoryQueue.push(createMockTransport()) + + const binding = connectPlainPane() + const release = createdTransportOptions[0]?.releasePreconnectInput + expect(typeof release).toBe('function') + expect(typeof release === 'function' && release()).toBe(true) + + const paneKey = makePaneKey('tab-1', leafIdForPane(1)) + mockStoreState.agentStatusByPaneKey[paneKey] = { + state: 'working', + prompt: '', + agentType: 'claude', + paneKey, + updatedAt: 1_000, + stateStartedAt: 1_000, + stateHistory: [], + providerSession: { key: 'session_id', id: 'session-1' } + } + + expect(typeof release === 'function' && release()).toBe(false) + binding.dispose() + }) + + it('keeps releasing a deferred split’s held keys as before when startup work appears', async () => { + transportFactoryQueue.push(createMockTransport()) + + const binding = connectPlainPane({ cwdPromise: new Promise(() => {}) }) + const release = createdTransportOptions[0]?.releasePreconnectInput + const paneKey = makePaneKey('tab-1', leafIdForPane(1)) + mockStoreState.sleepingAgentSessionsByPaneKey[paneKey] = { + paneKey, + tabId: 'tab-1', + worktreeId: 'wt-1' + } + + expect(typeof release === 'function' && release()).toBe(true) + binding.dispose() + }) + + it('drops keys typed into a deliberately slept pane instead of replaying them on wake', async () => { + markWorktreeSleepIntent('wt-1') + const transport = Object.assign(createMockTransport(), { abandonPreconnectInput: vi.fn() }) + transportFactoryQueue.push(transport) + + const binding = connectPlainPane() + await flushAsyncTicks(20) + + expect(transport.connect).not.toHaveBeenCalled() + expect(transport.abandonPreconnectInput).toHaveBeenCalledOnce() + binding.dispose() + }) + + it('keeps a deferred split’s held keys through a deliberate sleep, as before', async () => { + markWorktreeSleepIntent('wt-1') + const transport = Object.assign(createMockTransport(), { abandonPreconnectInput: vi.fn() }) + transportFactoryQueue.push(transport) + + const binding = connectPlainPane({ cwdPromise: Promise.resolve('/split') }) + await flushAsyncTicks(20) + + expect(transport.abandonPreconnectInput).not.toHaveBeenCalled() + binding.dispose() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/pty-connection/cold-restore-resume-startup.ts b/src/renderer/src/components/terminal-pane/pty-connection/cold-restore-resume-startup.ts index 45eb6063920..b2fa7557c66 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/cold-restore-resume-startup.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/cold-restore-resume-startup.ts @@ -20,27 +20,38 @@ import type { ColdRestoreAgentResumeStartup } from './fresh-spawn-types' import type { ConnectPanePtySession } from './connect-pane-pty-session' +type AppState = ReturnType + +/** The agent session a cold restore of this pane would resume; read-only, so pre-spawn checks can share it. */ +export function findColdRestoreResumeSource(session: ConnectPanePtySession, state: AppState) { + const entry = state.agentStatusByPaneKey[session.cacheKey] + const sleepingRecordEntry = session.getSleepingRecordForPane(state) + const sleepingRecord = sleepingRecordEntry?.record + const liveEntry = entry && entry.state !== 'done' ? entry : null + const agent = liveEntry ? liveEntry.agentType : sleepingRecord?.agent + if (!agent || !isResumableTuiAgent(agent)) { + return null + } + const providerSession = normalizeAgentProviderSession( + liveEntry ? liveEntry.providerSession : sleepingRecord?.providerSession + ) + return providerSession ? { agent, providerSession, liveEntry, sleepingRecordEntry } : null +} + export function bindBuildColdRestoreAgentResumeStartup(session: ConnectPanePtySession): void { session.buildColdRestoreAgentResumeStartup = (): ColdRestoreAgentResumeStartup | null => { if (session.pendingStartupCommand) { return null } const state = useAppStore.getState() - const entry = state.agentStatusByPaneKey[session.cacheKey] - const sleepingRecordEntry = session.getSleepingRecordForPane(state) + const source = findColdRestoreResumeSource(session, state) + if (!source) { + return null + } + const { agent, providerSession, sleepingRecordEntry } = source + const entry = source.liveEntry const sleepingRecord = sleepingRecordEntry?.record - - const useLiveEntry = entry && entry.state !== 'done' - const agent = useLiveEntry ? entry.agentType : sleepingRecord?.agent - if (!agent || !isResumableTuiAgent(agent)) { - return null - } - const providerSession = normalizeAgentProviderSession( - useLiveEntry ? entry.providerSession : sleepingRecord?.providerSession - ) - if (!providerSession) { - return null - } + const useLiveEntry = entry !== null // Why: this is the second issuer of `--resume`, and the one that handles a quit/live record // whose pane still exists — the sweep hands those here rather than launching them. A session id // names a transcript on the machine that captured it, so replaying one over a pane now attached diff --git a/src/renderer/src/components/terminal-pane/pty-connection/pane-transport-options.ts b/src/renderer/src/components/terminal-pane/pty-connection/pane-transport-options.ts index 3f1d192aa71..d4b3a58f922 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/pane-transport-options.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/pane-transport-options.ts @@ -1,17 +1,48 @@ import { useAppStore } from '@/store' import { shouldRetainDisposedPaneSpawn } from './disposed-spawn-retention' +import { findColdRestoreResumeSource } from './cold-restore-resume-startup' +import { mainStartsPane } from './main-starts-pane' import { requestTerminalPaneRecovery } from '../terminal-pane-recovery' import type { IpcPtyTransportOptions } from '../pty-transport-types' import type { ConnectPanePtySession } from './connect-pane-pty-session' +/** A split waiting on its inherited cwd, or remounted with handed-off keys; it holds keys as before. */ +export function isDeferredSplitInputHold(session: ConnectPanePtySession): boolean { + return Boolean(session.deps.cwdPromise || session.deps.preconnectInput?.length) +} + +/** + * Deferred splits and plain shells hold keys typed before their PTY binds (a first spawn can wait + * seconds on the host). + * Why not panes with startup work: held keys would flush ahead of the startup command or an agent + * resume (sleeping record or live status entry), so a typed prompt would run as a shell command or + * corrupt the launch line. A pane main starts is excluded too: main picks its launch, unseen here. + */ +function holdsInputUntilConnect(session: ConnectPanePtySession): boolean { + if (isDeferredSplitInputHold(session)) { + return true + } + const state = useAppStore.getState() + return ( + !session.paneStartup && + !session.getSleepingRecordForPane(state) && + !findColdRestoreResumeSource(session, state) && + !mainStartsPane(session) + ) +} + /** The spawn/attach options a pane hands its PTY transport. */ export function buildPaneTransportOptions(session: ConnectPanePtySession): IpcPtyTransportOptions { return { terminalKittyKeyboardProtocol: session.pane.terminal.options.vtExtensions?.kittyKeyboard === true, cwd: session.deps.cwd, - ...(session.deps.cwdPromise || session.deps.preconnectInput?.length - ? { bufferInputUntilConnect: true } + ...(holdsInputUntilConnect(session) + ? { + bufferInputUntilConnect: true, + // Why: a resume source can land between mount and bind; keys must not flush ahead of it. + releasePreconnectInput: () => holdsInputUntilConnect(session) + } : {}), ...(session.deps.preconnectInput?.length ? { preconnectInput: session.deps.preconnectInput } diff --git a/src/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.ts b/src/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.ts index 22b86db8974..b79bbf88a09 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/run-deferred-connect.ts @@ -10,6 +10,7 @@ import { hasWorktreeSleepIntent, onWorktreeSleepIntentCleared } from '@/lib/work import { recordPtyConnectDiagnostic } from './pty-connect-limits' import { defersToMainPaneStart } from './main-pane-start-request' import { findTerminalTabForPane } from './terminal-tab-id' +import { isDeferredSplitInputHold } from './pane-transport-options' import type { ConnectPanePtySession } from './connect-pane-pty-session' import { bindBuildColdRestoreAgentResumeStartup } from './cold-restore-resume-startup' @@ -52,6 +53,10 @@ export function installRunDeferredConnect(session: ConnectPanePtySession): void } if (!wakeWaitStarted) { wakeWaitStarted = true + // Why: keys typed into a slept shell must not run in the shell a later wake reattaches. + if (!isDeferredSplitInputHold(session)) { + session.transport.abandonPreconnectInput?.() + } recordPtyConnectDiagnostic( `pane=${session.pane.id} tab=${session.deps.tabId} -> WAIT FOR WAKE (deliberate sleep)` ) diff --git a/src/renderer/src/components/terminal-pane/pty-transport-input-write.test.ts b/src/renderer/src/components/terminal-pane/pty-transport-input-write.test.ts index df2c11e08d4..90883d4bfdb 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-input-write.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-input-write.test.ts @@ -117,6 +117,60 @@ describe('createIpcPtyTransport', () => { expect(delivered).toEqual(['first-', 'second-', 'third']) }) + it('delivers keys typed during a slow SSH spawn in order once the PTY binds', async () => { + const spawn = createDeferred<{ id: string }>() + vi.mocked(window.api.pty.spawn).mockReturnValue(spawn.promise as never) + const { createIpcPtyTransport } = await import('./pty-transport') + const transport = createIpcPtyTransport({ + connectionId: 'ssh-1', + bufferInputUntilConnect: true + }) + + expect(transport.sendInput('ec', 'driving')).toBe(true) + const connecting = transport.connect({ url: '', callbacks: {} }) + expect(transport.sendInput('ho hi\r', 'driving')).toBe(true) + expect(window.api.pty.write).not.toHaveBeenCalled() + + spawn.resolve({ id: 'pty-1' }) + await connecting + await flushAsyncTicks() + expect(vi.mocked(window.api.pty.write).mock.calls).toEqual([ + ['pty-1', 'ec', 'driving'], + ['pty-1', 'ho hi\r', 'driving'] + ]) + + // Live input still writes straight through. + expect(transport.sendInput('next', 'driving')).toBe(true) + await flushAsyncTicks() + expect(window.api.pty.write).toHaveBeenLastCalledWith('pty-1', 'next', 'driving') + }) + + it('drops held keys instead of flushing them when startup work appeared before the bind', async () => { + const spawn = createDeferred<{ id: string }>() + vi.mocked(window.api.pty.spawn).mockReturnValue(spawn.promise as never) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined) + let release = true + const { createIpcPtyTransport } = await import('./pty-transport') + const transport = createIpcPtyTransport({ + bufferInputUntilConnect: true, + releasePreconnectInput: () => release + }) + + const connecting = transport.connect({ url: '', callbacks: {} }) + expect(transport.sendInput('make the tests pass\r', 'driving')).toBe(true) + release = false + spawn.resolve({ id: 'pty-1' }) + await connecting + await flushAsyncTicks() + + expect(window.api.pty.write).not.toHaveBeenCalled() + // Live input after the bind is unchanged. + expect(transport.sendInput('next', 'driving')).toBe(true) + await flushAsyncTicks() + expect(window.api.pty.write).toHaveBeenCalledWith('pty-1', 'next', 'driving') + warn.mockRestore() + }) + it('flushes remount-handoff input before newly typed input without recapturing the seed', async () => { const spawn = createDeferred<{ id: string }>() vi.mocked(window.api.pty.spawn).mockReturnValue(spawn.promise as never) diff --git a/src/renderer/src/components/terminal-pane/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport-types.ts index a40cb95936c..af5e4b8d94d 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -288,8 +288,10 @@ export type PtyTransport = { export type IpcPtyTransportOptions = { cwd?: string - /** Retain bounded user input while a visible split waits to start its PTY. */ + /** Retain bounded user input until the pane's first PTY connection binds. */ bufferInputUntilConnect?: boolean + /** Re-checked when held input would flush; false drops it (startup work appeared after the hold began). */ + releasePreconnectInput?: () => boolean /** Seed a fresh transport with input handed off from a remounted deferred split. */ preconnectInput?: readonly PtyPreconnectInputEntry[] /** Records newly retained input against a remount-safe deferred split handoff. */ diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index b6b446abd8a..7756a7bf701 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -105,6 +105,10 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra } return } + if (opts.releasePreconnectInput && !opts.releasePreconnectInput()) { + dropPreconnectInput('startup work appeared before it attached') + return + } await preconnectInputBuffer.flush({ isCurrent: () => !destroyed && connected && ptyId === id, sendInput: (data, inputKind) => inputWriteQueue.enqueue(id, data, inputKind),