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>
This commit is contained in:
Neil
2026-10-15 19:11:34 -07:00
committed by GitHub
co-authored by Yimin
parent 9d1cd6a7cf
commit a06225e92b
11 changed files with 152 additions and 19 deletions
@@ -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,
@@ -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) => {
+1 -11
View File
@@ -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<typeof spawnCommitBindingOrigin>[0] & {
id: string
incarnationId?: string
@@ -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'
@@ -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)
})
})
}
@@ -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')
@@ -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)
@@ -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()
@@ -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)
}
@@ -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
}
+14
View File
@@ -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)
}