From a0cc9f35ba1ddf85751dc1c2e0c037729f09dc4e Mon Sep 17 00:00:00 2001 From: Vincent Date: Fri, 21 Aug 2026 12:08:20 +0800 Subject: [PATCH] fix(terminal): mark captured shortcut input interactive (#13800) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(terminal): mark captured shortcut input interactive Refresh the interactive-redraw timestamp after a captured shortcut send succeeds, so the composer redraw that follows takes the low-latency foreground path instead of waiting out the 1s coalesce fallback. Orca's captured shortcut path sends directly through the captured pane transport to preserve pane, PTY and transport identity, and so bypasses xterm's onData -- the only place that previously stamped the timestamp. An idle pane therefore scheduled the post-Shift+Enter redraw as ordinary throughput. Measured in a real Pi pane on Windows: ~1029ms before, ~16-20ms after, at an identical ~2.3KB redraw. Stale pane bindings and rejected transport sends cannot refresh it. Fixes #10203 Refs #13598 * fix(terminal): keep captured shortcuts out of the pane-teardown signal lastTerminalInputAt has two readers, and the previous commit only meant to change one of them. onExit reads it as "the user never typed into this pane" to keep a newborn pane mounted when its shell dies on startup (the failing-.envrc direnv case, pty-connection.ts onExit) so the error stays visible and the worktree stays active. Stamping it from a captured shortcut therefore made a single Shift+Enter before that exit close the tab and bounce the user to Landing -- on every platform, since captured shortcuts are not Windows-only. Split the redraw window onto its own timestamp so captured shortcuts open the fast path without arming the teardown, and leave onExit's behaviour byte-identical to main. * test(terminal): pin the captured-shortcut wiring and its staleness guard Deleting the onAccepted block, or replacing its binding-identity check with true, both left the whole suite green — so nothing pinned the part of this PR that actually ships. Cover both against the existing IME keyboard harness. --------- Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- .../keyboard-handlers-ime.test.tsx | 56 ++++++++++++++++++- .../terminal-pane/keyboard-handlers.ts | 12 +++- ...y-connection-foreground-write-path.test.ts | 26 +++++++++ .../pty-connection-pty-exit-teardown.test.ts | 28 ++++++++++ .../terminal-pane/pty-connection.ts | 23 ++++++-- .../terminal-captured-input-dispatch.test.ts | 45 ++++++++++++++- .../terminal-captured-input-dispatch.ts | 15 +++-- 7 files changed, 188 insertions(+), 17 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime.test.tsx b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime.test.tsx index 536d3e9c9cb..1155e661701 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers-ime.test.tsx +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers-ime.test.tsx @@ -29,7 +29,9 @@ function keyboardEvent( return event } -function createHarness(): { +type ShortcutBinding = { markShortcutTerminalInputSent: ReturnType } + +function createHarness(bindings?: Map): { deps: KeyboardHandlersDeps editable: HTMLInputElement sendInput: ReturnType @@ -77,7 +79,7 @@ function createHarness(): { keyboardScopeRef: { current: scope }, managerRef: { current: manager }, paneTransportsRef: { current: new Map([[pane.id, transport]]) }, - panePtyBindingsRef: { current: new Map() }, + panePtyBindingsRef: { current: (bindings ?? new Map()) as never }, paneCwdRef: { current: new Map() }, fallbackCwd: '', expandedPaneIdRef: { current: null }, @@ -168,6 +170,56 @@ describe('Windows IME keyboard ownership', () => { harness.dispose() }) + it('marks a captured shortcut send as interactive input', () => { + const binding = { markShortcutTerminalInputSent: vi.fn() } + const harness = createHarness(new Map([[1, binding]])) + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { + key: 'Enter', + code: 'Enter', + keyCode: 13, + timeStamp: 10, + shiftKey: true + }) + ) + + expect(harness.sendInput).toHaveBeenCalledTimes(1) + expect(binding.markShortcutTerminalInputSent).toHaveBeenCalledTimes(1) + hook.unmount() + harness.dispose() + }) + + it('does not mark input for a pane binding replaced between capture and send', () => { + // Why: the sender captures the binding, then re-reads it at send time — a rehomed + // or reconnected pane must not have its redraw scheduling refreshed by the old one. + const captured = { markShortcutTerminalInputSent: vi.fn() } + const replacement = { markShortcutTerminalInputSent: vi.fn() } + const bindings = new Map([[1, captured]]) + let reads = 0 + bindings.get = ((paneId: number) => + paneId === 1 ? (reads++ === 0 ? captured : replacement) : undefined) as never + const harness = createHarness(bindings) + const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) + + harness.terminalInput.dispatchEvent( + keyboardEvent('keydown', { + key: 'Enter', + code: 'Enter', + keyCode: 13, + timeStamp: 10, + shiftKey: true + }) + ) + + expect(harness.sendInput).toHaveBeenCalledTimes(1) + expect(captured.markShortcutTerminalInputSent).not.toHaveBeenCalled() + expect(replacement.markShortcutTerminalInputSent).not.toHaveBeenCalled() + hook.unmount() + harness.dispose() + }) + it('does not route an editable-target Enter keyup into the terminal', () => { const harness = createHarness() const hook = renderHook(() => useTerminalKeyboardShortcuts(harness.deps)) diff --git a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts index 3614af3b888..bd1cd5ced2c 100644 --- a/src/renderer/src/components/terminal-pane/keyboard-handlers.ts +++ b/src/renderer/src/components/terminal-pane/keyboard-handlers.ts @@ -21,7 +21,8 @@ import { createTerminalImeDeferredChordSender } from './terminal-ime-deferred-ch import { hasPendingTerminalImeComposition } from './terminal-ime-composition-route' import { requestCapturedTerminalReconfirmation, - sendCapturedTerminalInput + sendCapturedTerminalInput, + type TerminalCapturedInputBinding } from './terminal-captured-input-dispatch' import { keybindingMatchesAction, @@ -457,7 +458,7 @@ export function useTerminalKeyboardShortcuts({ const capturedTransport = paneTransportsRef.current.get(pane.id) const capturedPtyId = capturedTransport?.getPtyId() ?? null const capturedBinding = panePtyBindingsRef.current.get(pane.id) as - | (IDisposable & { requestWindowsShiftEnterReconfirmation?: () => void }) + | (IDisposable & TerminalCapturedInputBinding) | undefined const getCurrentManager = () => managerRef.current const getCurrentTransport = () => paneTransportsRef.current.get(pane.id) @@ -473,7 +474,12 @@ export function useTerminalKeyboardShortcuts({ currentTransport: getCurrentTransport(), capturedTransport, capturedPtyId, - data: overrideData + data: overrideData, + onAccepted: () => { + if (getCurrentBinding() === capturedBinding) { + capturedBinding?.markShortcutTerminalInputSent?.() + } + } }) if (sent) { recordTerminalUserInputForLeaf(tabId, pane.leafId) diff --git a/src/renderer/src/components/terminal-pane/pty-connection-foreground-write-path.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-foreground-write-path.test.ts index 983775b8cd8..743f06f6f42 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-foreground-write-path.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-foreground-write-path.test.ts @@ -256,6 +256,32 @@ describe('connectPanePty', () => { expect(transport.sendInput).toHaveBeenCalledWith('a') }) + it('keeps large ANSI redraws after captured shortcut input on the immediate path', async () => { + const { connectPanePty } = await import('./pty-connection') + const pane = createPane(1) + const transport = createMockTransport('pty-1') + const capturedDataCallback: { current: ((data: string) => void) | null } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + capturedDataCallback.current = callbacks.onData ?? null + return 'pty-1' + }) + transportFactoryQueue.push(transport) + + const binding = connectPanePty( + pane as never, + createManager(1) as never, + createDeps() as never + ) as unknown as { markShortcutTerminalInputSent: () => void } + await flushAsyncTicks() + binding.markShortcutTerminalInputSent() + + const redraw = `\x1b[2J\x1b[H${'pi composer redraw '.repeat(200)}` + expect(redraw.length).toBeGreaterThan(2_048) + capturedDataCallback.current?.(redraw) + + expect(pane.terminal.write).toHaveBeenCalledWith(redraw, expect.any(Function)) + }) + it('does not let OpenTUI-style small ANSI redraw bursts monopolize foreground writes', async () => { const { connectPanePty } = await import('./pty-connection') const pane = createPane(1) diff --git a/src/renderer/src/components/terminal-pane/pty-connection-pty-exit-teardown.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-pty-exit-teardown.test.ts index c31fbf5fd3f..ae55be07860 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-pty-exit-teardown.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-pty-exit-teardown.test.ts @@ -335,6 +335,34 @@ describe('connectPanePty', () => { expect(manager.closePane).not.toHaveBeenCalled() }) + it('keeps a worktree sole terminal mounted when only a captured shortcut preceded the exit', async () => { + // Why (regression): captured shortcuts refresh the redraw window, but that must not + // count as "the user typed into this pane" — otherwise Shift+Enter before a direnv + // failure closes the tab and bounces the user to Landing. + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('tab-pty') + transportFactoryQueue.push(transport) + const manager = createManager(1) + const deps = createDeps() + + const binding = connectPanePty( + createPane(1) as never, + manager as never, + deps as never + ) as unknown as { markShortcutTerminalInputSent: () => void } + const onPtySpawn = createdTransportOptions[0]?.onPtySpawn as + | ((ptyId: string) => void) + | undefined + const onPtyExit = createdTransportOptions[0]?.onPtyExit as ((ptyId: string) => void) | undefined + + onPtySpawn?.('tab-pty') + binding.markShortcutTerminalInputSent() + onPtyExit?.('tab-pty') + + expect(deps.onPtyExitRef.current).not.toHaveBeenCalled() + expect(manager.closePane).not.toHaveBeenCalled() + }) + it('tears down the sole terminal when a freshly-spawned PTY exits after the user typed input', async () => { // Why: an explicit `exit` (or any typed input) is a deliberate close, not a failed-startup shell, so the worktree should deactivate as before. const { connectPanePty } = await import('./pty-connection') diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 18641a18793..c701e383084 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -708,6 +708,8 @@ type PanePtyBinding = IDisposable & { sampleForegroundAgentOnFocus: () => void /** Reconfirm after direct shortcut input, which bypasses PTY onData. */ requestWindowsShiftEnterReconfirmation: () => void + /** Refresh interactive redraw scheduling after captured shortcut input. */ + markShortcutTerminalInputSent: () => void reconcileIfSessionDead: (liveSessionIds: Set, snapshotRequestedAt?: number) => void reconcileIfSessionMissing: (hasPty: HasPty, livenessRequestedAt?: number) => void } @@ -3828,16 +3830,24 @@ export function connectPanePty( Boolean(connectionId) && !shouldDeliverStartupViaTerminalPaste const hadExistingPaneTransportAtConnect = deps.paneTransportsRef.current.size > 0 let lastTerminalInputAt = Number.NEGATIVE_INFINITY + // Why: separate from lastTerminalInputAt because onExit reads that one as + // "the user never typed into this pane" to keep a dead newborn pane mounted. + // Captured shortcuts must open the redraw window without arming that teardown. + let lastInteractiveRedrawInputAt = Number.NEGATIVE_INFINITY let hasReceivedPtyOutput = false let deferredReattachLiveData: DeferredReattachLiveDataQueue | null = null let reattachLiveDataDeferralDepth = 0 let deferredReattachLiveDataOwners = new Map() let transportStreamGeneration = 0 - const markTerminalInputSent = (): void => { - lastTerminalInputAt = performance.now() + const markInteractiveRedrawInput = (): void => { + lastInteractiveRedrawInputAt = performance.now() // Why: input must probe a wedged xterm even when the PTY produces no renderer output. requestTerminalWritePipelineProbe(pane.terminal) } + const markTerminalInputSent = (): void => { + lastTerminalInputAt = performance.now() + markInteractiveRedrawInput() + } const recordTerminalInputForHibernation = (): void => { useAppStore.getState().recordTerminalInput(cacheKey) } @@ -6347,7 +6357,7 @@ export function connectPanePty( return consumeForegroundImmediateBudget(data.length) } const recentInput = - performance.now() - lastTerminalInputAt <= FOREGROUND_INTERACTIVE_REDRAW_WINDOW_MS + performance.now() - lastInteractiveRedrawInputAt <= FOREGROUND_INTERACTIVE_REDRAW_WINDOW_MS if ( recentInput && data.length <= FOREGROUND_INTERACTIVE_REDRAW_CHARS && @@ -6387,7 +6397,7 @@ export function connectPanePty( } { const rewriteOutputPrefersRenderRefresh = foregroundRewriteOutputPrefersRenderRefresh(data) const recentInput = - performance.now() - lastTerminalInputAt <= FOREGROUND_INTERACTIVE_REDRAW_WINDOW_MS + performance.now() - lastInteractiveRedrawInputAt <= FOREGROUND_INTERACTIVE_REDRAW_WINDOW_MS if (foregroundRendererRiskOutputPrefersRenderRefresh(data)) { return { refresh: true, @@ -6517,7 +6527,7 @@ export function connectPanePty( // Why: recompute the latch on every synchronized START so each frame's interactivity is judged by its own open time and can't leak across a same-chunk close+open; clear only on leaving synchronized output. if (synchronizedForegroundOutput && synchronizedOutputStarted) { synchronizedForegroundFrameInteractive = - performance.now() - lastTerminalInputAt <= + performance.now() - lastInteractiveRedrawInputAt <= FOREGROUND_SYNCHRONIZED_FRAME_INTERACTIVE_WINDOW_MS } else if (!nextSynchronizedForegroundOutputActive && !synchronizedOutputEnded) { synchronizedForegroundFrameInteractive = false @@ -9484,6 +9494,9 @@ export function connectPanePty( sampleVisiblePaneForegroundAgent() }, SHIFT_ENTER_RECONFIRM_IDLE_MS) }, + markShortcutTerminalInputSent() { + markInteractiveRedrawInput() + }, reconcileIfSessionDead, reconcileIfSessionMissing, dispose() { diff --git a/src/renderer/src/components/terminal-pane/terminal-captured-input-dispatch.test.ts b/src/renderer/src/components/terminal-pane/terminal-captured-input-dispatch.test.ts index 521100fbd65..df997a87afc 100644 --- a/src/renderer/src/components/terminal-pane/terminal-captured-input-dispatch.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-captured-input-dispatch.test.ts @@ -5,10 +5,10 @@ import { sendCapturedTerminalInput } from './terminal-captured-input-dispatch' -function createTransport(ptyId: string | null): PtyTransport { +function createTransport(ptyId: string | null, sendResult = true): PtyTransport { return { getPtyId: vi.fn(() => ptyId), - sendInput: vi.fn(() => true) + sendInput: vi.fn(() => sendResult) } as unknown as PtyTransport } @@ -31,11 +31,48 @@ describe('sendCapturedTerminalInput', () => { } ) + it('runs the accepted callback only after a successful captured send', () => { + const transport = createTransport('pty-original') + const onAccepted = vi.fn() + + expect( + sendCapturedTerminalInput({ + targetPaneMounted: true, + currentTransport: transport, + capturedTransport: transport, + capturedPtyId: 'pty-original', + data: '\x1b[13;2u', + onAccepted + }) + ).toBe(true) + + expect(onAccepted).toHaveBeenCalledOnce() + }) + + it('does not run the accepted callback for a rejected captured send', () => { + const transport = createTransport('pty-original', false) + const onAccepted = vi.fn() + + expect( + sendCapturedTerminalInput({ + targetPaneMounted: true, + currentTransport: transport, + capturedTransport: transport, + capturedPtyId: 'pty-original', + data: '\x1b[13;2u', + onAccepted + }) + ).toBe(false) + + expect(onAccepted).not.toHaveBeenCalled() + }) + it.each(['local IPC', 'SSH remote runtime'])( 'does not deliver to a replacement %s transport for a reused pane', () => { const original = createTransport('pty-original') const replacement = createTransport('pty-replacement') + const onAccepted = vi.fn() expect( sendCapturedTerminalInput({ @@ -43,11 +80,13 @@ describe('sendCapturedTerminalInput', () => { currentTransport: replacement, capturedTransport: original, capturedPtyId: 'pty-original', - data: '\r' + data: '\r', + onAccepted }) ).toBe(false) expect(original.sendInput).not.toHaveBeenCalled() expect(replacement.sendInput).not.toHaveBeenCalled() + expect(onAccepted).not.toHaveBeenCalled() } ) diff --git a/src/renderer/src/components/terminal-pane/terminal-captured-input-dispatch.ts b/src/renderer/src/components/terminal-pane/terminal-captured-input-dispatch.ts index e275f15bd99..8f96ba80efb 100644 --- a/src/renderer/src/components/terminal-pane/terminal-captured-input-dispatch.ts +++ b/src/renderer/src/components/terminal-pane/terminal-captured-input-dispatch.ts @@ -6,10 +6,12 @@ type CapturedTerminalInputDispatch = { capturedTransport: PtyTransport | undefined capturedPtyId: string | null data: string + onAccepted?: () => void } -export type TerminalReconfirmationBinding = { +export type TerminalCapturedInputBinding = { requestWindowsShiftEnterReconfirmation?: () => void + markShortcutTerminalInputSent?: () => void } export function sendCapturedTerminalInput({ @@ -17,7 +19,8 @@ export function sendCapturedTerminalInput({ currentTransport, capturedTransport, capturedPtyId, - data + data, + onAccepted }: CapturedTerminalInputDispatch): boolean { if ( !targetPaneMounted || @@ -28,12 +31,16 @@ export function sendCapturedTerminalInput({ ) { return false } - return capturedTransport.sendInput(data) + const sent = capturedTransport.sendInput(data) + if (sent) { + onAccepted?.() + } + return sent } export function requestCapturedTerminalReconfirmation( currentBinding: object | undefined, - capturedBinding: TerminalReconfirmationBinding | undefined + capturedBinding: TerminalCapturedInputBinding | undefined ): void { if (currentBinding === capturedBinding) { capturedBinding?.requestWindowsShiftEnterReconfirmation?.()