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) +}