From a06225e92bfb9bf6dffe5b9df13d35425c380c3a Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 15 Oct 2026 19:11:34 -0700 Subject: [PATCH] fix(terminal): focus and hover reports no longer take a mirrored terminal's size (#29239) * fix(terminal): focus and hover reports no longer take a mirrored terminal's size A TUI re-arms focus reporting (DECSET 1004) after every resize and xterm answers with CSI I / CSI O. Each mirror of the terminal treated that reply as user activity and claimed the PTY at its own grid, so two panes of different sizes resized the PTY back and forth several times a second. - Renderer: focus reports and button-less hover reports still reach the PTY but no longer claim the viewport or consume a keydown's pending interrupt intent. - Host: terminal.send skips the viewport claim for focus reports and query replies, covering older clients on the stream-less send path. - The untyped-input predicate moves to shared so both sides use one definition. Fixes #26469 Refs #21731 (section 1: desktop-to-desktop flicker) Co-authored-by: Yimin <47510889+Yiminnn@users.noreply.github.com> * fix(terminal): a focus-report send with Enter or interrupt still claims the viewport --------- Co-authored-by: Yimin <47510889+Yiminnn@users.noreply.github.com> --- .../methods/terminal/terminal-send-method.ts | 11 +++- src/main/runtime/rpc/terminal-send.test.ts | 39 ++++++++++++++ src/main/runtime/terminal-run-facts.ts | 12 +---- .../terminal/layout/terminal-input-floor.ts | 2 +- .../pty-connection-setup-split-spawn.spec.ts | 54 ++++++++++++++++++- .../pty-connection/interrupt-input-intent.ts | 4 ++ .../pty-connection/pty-input-forward.ts | 8 +-- .../pty-connection/pty-input-recovery.ts | 8 ++- .../terminal-passive-input-report.ts | 7 +++ .../terminal-pointer-input-sequences.ts | 12 +++++ src/shared/terminal-untyped-input.ts | 14 +++++ 11 files changed, 152 insertions(+), 19 deletions(-) create mode 100644 src/renderer/src/components/terminal-pane/terminal-passive-input-report.ts create mode 100644 src/shared/terminal-untyped-input.ts diff --git a/src/main/runtime/rpc/methods/terminal/terminal-send-method.ts b/src/main/runtime/rpc/methods/terminal/terminal-send-method.ts index f7ae8b29e5a..f162192474d 100644 --- a/src/main/runtime/rpc/methods/terminal/terminal-send-method.ts +++ b/src/main/runtime/rpc/methods/terminal/terminal-send-method.ts @@ -1,6 +1,7 @@ import { assertLegacyAiVaultResumeCommandAllowed } from '../../../../ai-vault/structured-session-ownership' import { InvalidArgumentError, defineMethod } from '../../core' import { isTerminalQueryReply } from '../../../../../shared/terminal-query-reply' +import { isUntypedTerminalInput } from '../../../../../shared/terminal-untyped-input' import { assertTerminalAgentSendable } from '../../terminal-agent-send-guard' import { TerminalSend } from '../../../../../shared/rpc-contract/terminal-unary-params' import { @@ -102,7 +103,15 @@ export const TERMINAL_SEND_METHODS = [ leaf?.ptyId && params.client?.type === 'desktop' && params.claimViewport === true && - params.viewport + params.viewport && + // Why: clients attach their viewport to every send, so a focus report or query reply + // would otherwise claim the PTY and start a resize war between two mirrors (#26469). + !( + typeof params.text === 'string' && + isUntypedTerminalInput(params.text) && + params.enter !== true && + params.interrupt !== true + ) ) { const claim = await updateViewportForClient( runtime, diff --git a/src/main/runtime/rpc/terminal-send.test.ts b/src/main/runtime/rpc/terminal-send.test.ts index a6b451b1488..15351c5cabc 100644 --- a/src/main/runtime/rpc/terminal-send.test.ts +++ b/src/main/runtime/rpc/terminal-send.test.ts @@ -182,6 +182,45 @@ describe('terminal send RPC', () => { expect(sendTerminal).toHaveBeenCalled() }) + it('does not claim the viewport for a focus report or query reply (#26469)', async () => { + const refreshRemoteDesktopViewer = vi.fn().mockResolvedValue(true) + const sendTerminal = vi.fn().mockResolvedValue({ + handle: 'terminal-1', + accepted: true, + bytesWritten: 3 + }) + const runtime = stubRuntime({ + resolveLiveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), + getDriver: vi.fn().mockReturnValue({ kind: 'idle' }), + refreshRemoteDesktopViewer, + sendTerminal + }) + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + const send = (text: string, suffix: { enter?: boolean } = {}) => + dispatcher.dispatch( + makeRequest('terminal.send', { + terminal: 'terminal-1', + text, + ...suffix, + client: { id: 'desktop-1', type: 'desktop' }, + viewport: { cols: 96, rows: 32 }, + claimViewport: true + }) + ) + + await expect(send('\x1b[I')).resolves.toMatchObject({ ok: true }) + await expect(send('\x1b[O\x1b[I')).resolves.toMatchObject({ ok: true }) + await expect(send('\x1b[?62;22c')).resolves.toMatchObject({ ok: true }) + expect(refreshRemoteDesktopViewer).not.toHaveBeenCalled() + expect(sendTerminal).toHaveBeenCalledTimes(3) + + await expect(send('\x1b[I', { enter: true })).resolves.toMatchObject({ ok: true }) + expect(refreshRemoteDesktopViewer).toHaveBeenCalledTimes(1) + + await expect(send('a')).resolves.toMatchObject({ ok: true }) + expect(refreshRemoteDesktopViewer).toHaveBeenCalledWith('pty-1', 'desktop-1', 96, 32, true) + }) + it('accepts legacy clientless mobile input when the current driver is mobile', async () => { const write = vi.fn() const sendTerminal = vi.fn().mockImplementation(async (_handle, _action, options) => { diff --git a/src/main/runtime/terminal-run-facts.ts b/src/main/runtime/terminal-run-facts.ts index f14f1a3c7f7..3f16a384ab0 100644 --- a/src/main/runtime/terminal-run-facts.ts +++ b/src/main/runtime/terminal-run-facts.ts @@ -2,7 +2,7 @@ import { spawnCommitBindingOrigin, type PtySpawnCommitOrigin } from '../persistence/loading-store/pty-binding-span' -import { isTerminalQueryReply } from '../../shared/terminal-query-reply' +import { isUntypedTerminalInput } from '../../shared/terminal-untyped-input' import type { TerminalInputKind } from '../../shared/terminal-input-kind' export type TerminalRunFacts = { @@ -12,16 +12,6 @@ export type TerminalRunFacts = { firstUserInputAt: number | null } -// Why: a paired client's xterm answers focus changes (CSI I / CSI O) through input that carries no -// provenance; the desktop renderer already excludes them via xterm's user-input signal. -// oxlint-disable-next-line no-control-regex -- focus reports are ESC-framed sequences by definition. -const TERMINAL_FOCUS_REPORTS_ONLY_RE = new RegExp('^(?:\\u001b\\[[IO])+$') - -/** Input with no provenance that no person typed: a whole terminal reply or only focus reports. */ -export function isUntypedTerminalInput(payload: string): boolean { - return isTerminalQueryReply(payload) || TERMINAL_FOCUS_REPORTS_ONLY_RE.test(payload) -} - export type TerminalSpawnCommit = Parameters[0] & { id: string incarnationId?: string diff --git a/src/main/runtime/terminal/layout/terminal-input-floor.ts b/src/main/runtime/terminal/layout/terminal-input-floor.ts index 503b9ed510f..43435491b13 100644 --- a/src/main/runtime/terminal/layout/terminal-input-floor.ts +++ b/src/main/runtime/terminal/layout/terminal-input-floor.ts @@ -2,7 +2,7 @@ import { OrcaRuntimeWithOnPtyExit } from '../lifecycle/pty-exit-lifecycle' import type { PtyLivenessVerdict } from '../../../../shared/pty-liveness-verdict' import type { DriverState } from '../../orca-runtime-core' import type { TerminalInputKind } from '../../../../shared/terminal-input-kind' -import { isUntypedTerminalInput } from '../../terminal-run-facts' +import { isUntypedTerminalInput } from '../../../../shared/terminal-untyped-input' import { clampTerminalViewport } from '../../terminal-viewport' import { getPtyTerminalState, getTerminalState } from '../../terminal-wait-results' diff --git a/src/renderer/src/components/terminal-pane/pty-connection-setup-split-spawn.spec.ts b/src/renderer/src/components/terminal-pane/pty-connection-setup-split-spawn.spec.ts index 34b9936a8d4..fec0d47a6d0 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-setup-split-spawn.spec.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-setup-split-spawn.spec.ts @@ -1,7 +1,12 @@ import { bindPtyConnectionTestMocks, ptyConnectionTestMocks } from './pty-connection-mocks.spec' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { flushAsyncTicks, createDeferred } from './pty-connection-test-async' -import { createRect, createMeasuredElement } from './pty-connection-test-dom' +import { + createKeyboardEventTarget, + createRect, + createMeasuredElement, + keyEvent +} from './pty-connection-test-dom' import { createMockTransport, createPane, @@ -619,5 +624,52 @@ export function registerPtyConnectionSetupSplitSpawnTests(): void { expect(transport.claimViewport).toHaveBeenCalledTimes(2) setFitOverride('pty-live', 'desktop-fit', 0, 0) }) + + it('forwards focus and hover reports without claiming a parked mirror (#26469)', async () => { + // Regression: a TUI re-arms DECSET 1004 after every SIGWINCH and xterm answers with a focus + // report. Claiming on it made two mirrors of different grids take the PTY from each other. + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport('pty-live') + transportFactoryQueue.push(transport) + mockStoreState = { + ...mockStoreState, + tabsByWorktree: { 'wt-1': [{ id: 'tab-1', ptyId: null }] } + } + const keyTarget = createKeyboardEventTarget() + const pane = createPane(1) + Object.assign(pane.terminal, { element: keyTarget.target }) + const registered: { onData?: (data: string) => void } = {} + pane.terminal.onData.mockImplementation((handler: (data: string) => void) => { + registered.onData = handler + return { dispose: vi.fn() } + }) + const { setFitOverride } = await import('@/lib/pane-manager/mobile-fit-overrides') + connectPanePty(pane as never, createManager(1) as never, createDeps() as never) + const send = registered.onData + if (!send) { + throw new Error('expected onData handler to be registered') + } + setFitOverride('pty-live', 'remote-desktop-fit', 80, 24) + transport.claimViewport.mockClear() + + for (const passive of ['\x1b[I', '\x1b[O', '\x1b[<35;10;5M', '\x1b[MC*%']) { + send(passive) + expect(transport.sendInput).toHaveBeenCalledWith(passive, 'query-reply') + } + expect(transport.claimViewport).not.toHaveBeenCalled() + + // A focus report landing between Ctrl+C's keydown and its byte must not consume the intent. + keyTarget.dispatch(keyEvent({ key: 'c', ctrlKey: true })) + send('\x1b[I') + expect(transport.claimViewport).not.toHaveBeenCalled() + send('\x03') + expect(transport.sendInputAccepted).toHaveBeenCalledWith('\x03', 'query-reply') + expect(transport.sendInputAccepted).not.toHaveBeenCalledWith('\x1b[I', 'query-reply') + expect(transport.claimViewport).toHaveBeenCalledTimes(1) + + send('\x1b[<0;10;5M') + expect(transport.claimViewport).toHaveBeenCalledTimes(2) + setFitOverride('pty-live', 'desktop-fit', 0, 0) + }) }) } diff --git a/src/renderer/src/components/terminal-pane/pty-connection/interrupt-input-intent.ts b/src/renderer/src/components/terminal-pane/pty-connection/interrupt-input-intent.ts index 9a6147ab649..d04b130d4e9 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/interrupt-input-intent.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/interrupt-input-intent.ts @@ -5,6 +5,7 @@ import { createAgentInterruptInference } from '../agent-interrupt-inference' import { createAgentQuestionAnsweredInference } from '../agent-question-answered-inference' import type { AgentInterruptInputIntent } from '../../../../../shared/agent-interrupt-intent' import { markTerminalBracketedPasteInterrupted } from '../terminal-bracketed-paste' +import { isPassiveTerminalInputReport } from '../terminal-passive-input-report' import { REATTACH_IDLE_AGENT_CURSOR_RESET_DELAY_MS } from './foreground-output-scan' @@ -119,6 +120,9 @@ export function installInterruptInputIntent(session: ConnectPanePtySession): voi session.clearPendingTerminalInputIntent() }, 0) } + // Why: a focus or hover report landing between a keydown and its byte must not consume the intent. + session.pendingTerminalInputIntentFor = (data: string): AgentInterruptInputIntent | null => + isPassiveTerminalInputReport(data) ? null : session.pendingTerminalInputIntent session.inputMatchesIntent = (intent: AgentInterruptInputIntent, data: string): boolean => { return ( (intent === 'plain-escape' && data === '\x1b') || (intent === 'ctrl-c' && data === '\x03') diff --git a/src/renderer/src/components/terminal-pane/pty-connection/pty-input-forward.ts b/src/renderer/src/components/terminal-pane/pty-connection/pty-input-forward.ts index e58bd64bde8..1e643839553 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/pty-input-forward.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/pty-input-forward.ts @@ -94,7 +94,7 @@ export function installPtyInputForward(session: ConnectPanePtySession): void { } // Why xterm's provenance: its own focus reports reach onData too, and no person typed them. const inputKind = wasUserInput ? 'driving' : 'query-reply' - const intent = session.pendingTerminalInputIntent + const intent = session.pendingTerminalInputIntentFor(data) // Why: real xterm can deliver the terminal byte even when our DOM keydown // listener missed the press. Exact Ctrl+C/Escape bytes are still safe to // infer for local/remote acknowledged writes; SSH fire-and-forget remains @@ -109,7 +109,7 @@ export function installPtyInputForward(session: ConnectPanePtySession): void { session.interruptStatusBaselineSequence += 1 } const capturedBaselineSequence = session.interruptStatusBaselineSequence - session.claimViewportForUserActivity() + session.claimViewportForUserActivity(data) if (acknowledgedIntent === 'ctrl-c') { // Why: the accepted-write callback is async; let the next command be // inferred if the user cancelled an oversized line and immediately typed. @@ -145,7 +145,7 @@ export function installPtyInputForward(session: ConnectPanePtySession): void { return } if (intent) { - session.claimViewportForUserActivity() + session.claimViewportForUserActivity(data) if (session.transport.sendInput(data, inputKind)) { session.markAcceptedTerminalInputSent() session.observeAcceptedShellCommandInput(data) @@ -156,7 +156,7 @@ export function installPtyInputForward(session: ConnectPanePtySession): void { session.clearPendingTerminalInputIntent() return } - session.claimViewportForUserActivity() + session.claimViewportForUserActivity(data) if (session.transport.sendInput(data, inputKind)) { session.markAcceptedTerminalInputSent() session.observeAcceptedShellCommandInput(data) diff --git a/src/renderer/src/components/terminal-pane/pty-connection/pty-input-recovery.ts b/src/renderer/src/components/terminal-pane/pty-connection/pty-input-recovery.ts index 12b4f742413..b3877a9e356 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection/pty-input-recovery.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection/pty-input-recovery.ts @@ -28,6 +28,7 @@ import { legacyRelayBridgePtyIdForPane } from '../legacy-relay-bridge-panes' import type { ConnectPanePtySession } from './connect-pane-pty-session' import type { PtyTransport } from '../pty-transport-types' +import { isPassiveTerminalInputReport } from '../terminal-passive-input-report' import { resolveTerminalInlineImagesEnabled } from '../../../../../shared/terminal-inline-images-settings' /** Transport creation, terminal capability replies, viewport claims, and undeliverable-input recovery. */ @@ -131,11 +132,16 @@ export function installPtyInputRecovery(session: ConnectPanePtySession): void { session.sendDesktopQueryReplyImmediate ) - session.claimViewportForUserActivity = (): void => { + session.claimViewportForUserActivity = (input?: string): void => { const currentPtyId = session.transport.getPtyId() if (!currentPtyId || getFitOverrideForPty(currentPtyId)?.mode !== 'remote-desktop-fit') { return } + // Why: TUIs re-arm DECSET 1004 on every SIGWINCH and xterm answers with a focus report; claiming + // on it made two mirrors of different grids take the PTY from each other forever (#26469). + if (input !== undefined && isPassiveTerminalInputReport(input)) { + return + } let proposed: { cols: number; rows: number } | undefined try { proposed = session.pane.fitAddon.proposeDimensions() diff --git a/src/renderer/src/components/terminal-pane/terminal-passive-input-report.ts b/src/renderer/src/components/terminal-pane/terminal-passive-input-report.ts new file mode 100644 index 00000000000..e013fd0ea76 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-passive-input-report.ts @@ -0,0 +1,7 @@ +import { isTerminalFocusReportOnly } from '../../../../shared/terminal-untyped-input' +import { isXtermHoverMotionReport } from './terminal-pointer-input-sequences' + +/** Bytes xterm emits without anyone driving the terminal: focus reports and button-less hover. */ +export function isPassiveTerminalInputReport(data: string): boolean { + return isTerminalFocusReportOnly(data) || isXtermHoverMotionReport(data) +} diff --git a/src/renderer/src/components/terminal-pane/terminal-pointer-input-sequences.ts b/src/renderer/src/components/terminal-pane/terminal-pointer-input-sequences.ts index 61ebbf9b022..7234c699b3e 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pointer-input-sequences.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pointer-input-sequences.ts @@ -13,3 +13,15 @@ export function isXtermMouseReport(data: string): boolean { export function isXtermWheelCursorKey(data: string): boolean { return data === '\x1b[A' || data === '\x1b[B' || data === '\x1bOA' || data === '\x1bOB' } + +/** True for a motion report with no button held (any-motion `?1003` hover), in X10 or SGR form. */ +export function isXtermHoverMotionReport(data: string): boolean { + if (!isXtermMouseReport(data)) { + return false + } + const button = data.startsWith('\x1b[<') + ? Number(data.slice(3, data.indexOf(';'))) + : data.charCodeAt(3) - 32 + // Why the mask: bit 32 is motion, low bits 3 mean no button, 4/8/16 are modifiers, 64+ is wheel. + return (button & 0xe3) === 0x23 +} diff --git a/src/shared/terminal-untyped-input.ts b/src/shared/terminal-untyped-input.ts new file mode 100644 index 00000000000..023e39c6399 --- /dev/null +++ b/src/shared/terminal-untyped-input.ts @@ -0,0 +1,14 @@ +import { isTerminalQueryReply } from './terminal-query-reply' + +// Why: xterm answers focus changes (CSI I / CSI O) through input that carries no provenance. +// oxlint-disable-next-line no-control-regex -- focus reports are ESC-framed sequences by definition. +const TERMINAL_FOCUS_REPORTS_ONLY_RE = new RegExp('^(?:\\u001b\\[[IO])+$') + +export function isTerminalFocusReportOnly(payload: string): boolean { + return TERMINAL_FOCUS_REPORTS_ONLY_RE.test(payload) +} + +/** Input with no provenance that no person typed: a whole terminal reply or only focus reports. */ +export function isUntypedTerminalInput(payload: string): boolean { + return isTerminalQueryReply(payload) || isTerminalFocusReportOnly(payload) +}