From 8509ca9e32fcfc1ef4394bc6702bd5fe3bb4fb88 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 21 Jul 2026 13:03:54 -0700 Subject: [PATCH] fix(dashboard-popout): harden terminal clipboard routing --- .../AgentTerminalPreview.test.tsx | 139 ++++++++++++++++-- .../dashboard-popout/AgentTerminalPreview.tsx | 79 ++++++++-- 2 files changed, 198 insertions(+), 20 deletions(-) diff --git a/src/renderer/src/components/dashboard-popout/AgentTerminalPreview.test.tsx b/src/renderer/src/components/dashboard-popout/AgentTerminalPreview.test.tsx index e1a26257374..673bae2446b 100644 --- a/src/renderer/src/components/dashboard-popout/AgentTerminalPreview.test.tsx +++ b/src/renderer/src/components/dashboard-popout/AgentTerminalPreview.test.tsx @@ -3,6 +3,14 @@ import '@testing-library/jest-dom/vitest' import { act, cleanup, render, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + TERMINAL_PASTE_CHUNK_MAX_BYTES, + TERMINAL_PASTE_DIRECT_MAX_BYTES +} from '@/components/terminal-pane/terminal-paste-limits' +import { + BRACKETED_PASTE_END, + BRACKETED_PASTE_START +} from '@/components/terminal-pane/terminal-bracketed-paste' const terminalHarness = vi.hoisted(() => ({ instances: [] as { @@ -13,6 +21,7 @@ const terminalHarness = vi.hoisted(() => ({ resize: ReturnType reset: ReturnType paste: ReturnType + modes: { bracketedPasteMode: boolean } selectionText: string customKeyHandler: ((event: KeyboardEvent) => boolean) | null }[], @@ -38,7 +47,11 @@ vi.mock('@xterm/xterm', () => ({ dispose = vi.fn() resize = vi.fn() reset = vi.fn() - paste = vi.fn() + modes = { bracketedPasteMode: false } + paste = vi.fn((data: string) => { + terminalHarness.userInputListener?.() + this.onDataListener?.(data) + }) getSelection = vi.fn(() => this.selectionText) attachCustomKeyEventHandler = vi.fn((handler: (event: KeyboardEvent) => boolean) => { this.customKeyHandler = handler @@ -75,7 +88,7 @@ vi.mock('@/store', () => { import { AgentTerminalPreview } from './AgentTerminalPreview' describe('AgentTerminalPreview', () => { - const input = vi.fn(async () => true) + const input = vi.fn(async (_ptyId: string, _data: string) => true) const ack = vi.fn(async () => {}) const unsubscribe = vi.fn(async () => {}) const connect = vi.fn() @@ -153,14 +166,24 @@ describe('AgentTerminalPreview', () => { await waitFor(() => expect(terminal.customKeyHandler).not.toBeNull()) terminal.selectionText = 'selected text' - const handled = terminal.customKeyHandler!( - new KeyboardEvent('keydown', { key: 'C', code: 'KeyC', ctrlKey: true, shiftKey: true }) + const keydown = new KeyboardEvent('keydown', { + key: 'C', + code: 'KeyC', + ctrlKey: true, + shiftKey: true, + cancelable: true + }) + const handled = terminal.customKeyHandler!(keydown) + const keyupHandled = terminal.customKeyHandler!( + new KeyboardEvent('keyup', { key: 'C', code: 'KeyC', ctrlKey: true, shiftKey: true }) ) expect(handled).toBe(false) + expect(keyupHandled).toBe(false) + expect(keydown.defaultPrevented).toBe(true) expect(writeClipboardText).toHaveBeenCalledWith('selected text') }) - it('lets the copy chord fall through to xterm when nothing is selected', async () => { + it('keeps an empty copy chord from leaking terminal input', async () => { render() await waitFor(() => expect(terminalHarness.instances).toHaveLength(1)) const terminal = terminalHarness.instances[0]! @@ -169,7 +192,7 @@ describe('AgentTerminalPreview', () => { const handled = terminal.customKeyHandler!( new KeyboardEvent('keydown', { key: 'C', code: 'KeyC', ctrlKey: true, shiftKey: true }) ) - expect(handled).toBe(true) + expect(handled).toBe(false) expect(writeClipboardText).not.toHaveBeenCalled() }) @@ -186,6 +209,7 @@ describe('AgentTerminalPreview', () => { act(() => emitAppMenuPaste?.()) await waitFor(() => expect(terminal.paste).toHaveBeenCalledWith('clip-text')) + expect(input).toHaveBeenCalledWith('pty-1', 'clip-text') }) it('ignores the app-menu paste signal when focus is outside the preview', async () => { @@ -199,10 +223,14 @@ describe('AgentTerminalPreview', () => { }) it('leaves plain Ctrl+V to the Edit-menu accelerator but handles the shifted paste chord', async () => { - render() + const view = render() await waitFor(() => expect(terminalHarness.instances).toHaveLength(1)) const terminal = terminalHarness.instances[0]! await waitFor(() => expect(terminal.customKeyHandler).not.toBeNull()) + const host = view.container.querySelector('.origin-bottom-left')! + const terminalInput = document.createElement('input') + host.appendChild(terminalInput) + terminalInput.focus() const plain = terminal.customKeyHandler!( new KeyboardEvent('keydown', { key: 'v', code: 'KeyV', ctrlKey: true }) @@ -210,12 +238,105 @@ describe('AgentTerminalPreview', () => { expect(plain).toBe(true) expect(readClipboardText).not.toHaveBeenCalled() - const shifted = terminal.customKeyHandler!( - new KeyboardEvent('keydown', { key: 'V', code: 'KeyV', ctrlKey: true, shiftKey: true }) + const shiftedEvent = new KeyboardEvent('keydown', { + key: 'V', + code: 'KeyV', + ctrlKey: true, + shiftKey: true, + cancelable: true + }) + const shifted = terminal.customKeyHandler!(shiftedEvent) + const repeated = terminal.customKeyHandler!( + new KeyboardEvent('keydown', { + key: 'V', + code: 'KeyV', + ctrlKey: true, + shiftKey: true, + repeat: true + }) ) expect(shifted).toBe(false) + expect(repeated).toBe(false) + expect(shiftedEvent.defaultPrevented).toBe(true) await waitFor(() => expect(terminal.paste).toHaveBeenCalledWith('clip-text')) expect(readClipboardText).toHaveBeenCalledTimes(1) + expect( + terminal.customKeyHandler!( + new KeyboardEvent('keyup', { key: 'V', code: 'KeyV', ctrlKey: true, shiftKey: true }) + ) + ).toBe(false) + }) + + it('cancels an async paste when the preview loses focus', async () => { + let resolveClipboard!: (text: string) => void + readClipboardText.mockReturnValueOnce( + new Promise((resolve) => { + resolveClipboard = resolve + }) + ) + const view = render() + await waitFor(() => expect(terminalHarness.instances).toHaveLength(1)) + const terminal = terminalHarness.instances[0]! + const host = view.container.querySelector('.origin-bottom-left')! + const terminalInput = document.createElement('input') + const outsideInput = document.createElement('input') + host.appendChild(terminalInput) + view.container.appendChild(outsideInput) + terminalInput.focus() + + act(() => emitAppMenuPaste?.()) + outsideInput.focus() + await act(async () => resolveClipboard('stale text')) + + expect(terminal.paste).not.toHaveBeenCalled() + expect(input).not.toHaveBeenCalled() + }) + + it('streams large pastes as bounded IPC payloads instead of one renderer-blocking write', async () => { + const largePaste = 'x'.repeat(TERMINAL_PASTE_DIRECT_MAX_BYTES + 1) + readClipboardText.mockResolvedValueOnce(largePaste) + const view = render() + await waitFor(() => expect(terminalHarness.instances).toHaveLength(1)) + const terminal = terminalHarness.instances[0]! + const host = view.container.querySelector('.origin-bottom-left')! + const terminalInput = document.createElement('input') + host.appendChild(terminalInput) + terminalInput.focus() + + act(() => emitAppMenuPaste?.()) + const expectedChunks = Math.ceil(largePaste.length / TERMINAL_PASTE_CHUNK_MAX_BYTES) + await waitFor(() => expect(input).toHaveBeenCalledTimes(expectedChunks)) + + const payloads = input.mock.calls.map(([, data]) => data as string) + expect(terminal.paste).not.toHaveBeenCalled() + expect(payloads.join('')).toBe(largePaste) + expect(payloads.every((payload) => payload.length <= TERMINAL_PASTE_CHUNK_MAX_BYTES)).toBe(true) + }) + + it('closes a bracketed large paste when focus changes between chunks', async () => { + readClipboardText.mockResolvedValueOnce('x'.repeat(TERMINAL_PASTE_DIRECT_MAX_BYTES + 1)) + const view = render() + await waitFor(() => expect(terminalHarness.instances).toHaveLength(1)) + const terminal = terminalHarness.instances[0]! + terminal.modes.bracketedPasteMode = true + const host = view.container.querySelector('.origin-bottom-left')! + const terminalInput = document.createElement('input') + const outsideInput = document.createElement('input') + host.appendChild(terminalInput) + view.container.appendChild(outsideInput) + terminalInput.focus() + input.mockImplementationOnce(async () => { + outsideInput.focus() + return true + }) + + act(() => emitAppMenuPaste?.()) + await waitFor(() => expect(input).toHaveBeenCalledTimes(2)) + + expect(input.mock.calls.map(([, data]) => data)).toEqual([ + BRACKETED_PASTE_START, + BRACKETED_PASTE_END + ]) }) it('keeps the existing terminal visible while a resync snapshot is captured', async () => { diff --git a/src/renderer/src/components/dashboard-popout/AgentTerminalPreview.tsx b/src/renderer/src/components/dashboard-popout/AgentTerminalPreview.tsx index 89efae286f9..0caf399bda9 100644 --- a/src/renderer/src/components/dashboard-popout/AgentTerminalPreview.tsx +++ b/src/renderer/src/components/dashboard-popout/AgentTerminalPreview.tsx @@ -4,6 +4,11 @@ import '@xterm/xterm/css/xterm.css' import { buildDefaultTerminalOptions } from '@/lib/pane-manager/pane-terminal-options' import { getShortcutPlatform } from '@/lib/shortcut-platform' import { subscribeToTerminalUserInput } from '@/components/terminal-pane/terminal-user-input-signal' +import { + executeTerminalPastePlan, + planTerminalPasteWithYield +} from '@/components/terminal-pane/terminal-paste-coordinator' +import { resolveTerminalPasteRuntime } from '@/components/terminal-pane/terminal-paste-runtime' import { TERMINAL_PASTE_MAX_BYTES } from '@/components/terminal-pane/terminal-paste-limits' import { composeActiveTerminalTheme } from '@/components/terminal-pane/terminal-appearance' import { useSystemPrefersDark } from '@/components/terminal-pane/use-system-prefers-dark' @@ -118,17 +123,50 @@ export function AgentTerminalPreview({ ptyId }: { ptyId: string }): React.JSX.El }) } - const pasteClipboardText = async (): Promise => { + const pasteClipboardText = async ( + activeElementAtDispatch: Element | null, + source: 'keyboard' | 'app-menu' + ): Promise => { let text: string try { text = await window.api.ui.readClipboardText({ maxBytes: TERMINAL_PASTE_MAX_BYTES }) } catch { return } - if (!disposed && terminal && text) { - // Why: paste() flows through onData as user input, so the existing PTY routing applies. - terminal.paste(text) + const pasteTerminal = terminal + if (!pasteTerminal || !text) { + return } + const targetIsCurrent = (): boolean => + !disposed && + terminal === pasteTerminal && + activeElementAtDispatch !== null && + document.activeElement === activeElementAtDispatch && + container.contains(activeElementAtDispatch) + if (!targetIsCurrent()) { + return + } + const platform = getShortcutPlatform() + const plan = await planTerminalPasteWithYield({ + text, + source, + target: { + kind: 'terminal', + paneId: 0, + leafId: ptyId, + ptyId, + runtime: resolveTerminalPasteRuntime({ platform, ptyId }) + }, + terminalBracketedPasteMode: pasteTerminal.modes.bracketedPasteMode + }) + await executeTerminalPastePlan(plan, { + // Why: stream large pastes so the renderer never emits one huge IPC payload. + pasteText: (pasteText) => pasteTerminal.paste(pasteText), + writePty: (data) => window.api.terminalPreview.input(ptyId, data), + isTargetCurrent: targetIsCurrent, + // Why: if focus changes mid-bracketed paste, the closing marker must still reach the live PTY. + canContinue: () => true + }) } const installClipboardShortcuts = (): void => { @@ -136,18 +174,33 @@ export function AgentTerminalPreview({ ptyId }: { ptyId: string }): React.JSX.El return } const platform = getShortcutPlatform() + const consumedClipboardKeys = new Set() + const consumeEvent = (event: KeyboardEvent): false => { + event.preventDefault() + event.stopPropagation() + return false + } terminal.attachCustomKeyEventHandler((event) => { if (event.type !== 'keydown') { + const keyIdentity = event.code || event.key + if (consumedClipboardKeys.has(keyIdentity)) { + if (event.type === 'keyup') { + consumedClipboardKeys.delete(keyIdentity) + } + return consumeEvent(event) + } return true } const keybindings = useAppStore.getState().keybindings if (keybindingMatchesAction('terminal.copySelection', event, platform, keybindings)) { + const keyIdentity = event.code || event.key + const firstKeydown = !consumedClipboardKeys.has(keyIdentity) + consumedClipboardKeys.add(keyIdentity) const selection = terminal?.getSelection() - if (!selection) { - return true + if (firstKeydown && selection) { + void window.api.ui.writeClipboardText(selection).catch(() => undefined) } - void window.api.ui.writeClipboardText(selection).catch(() => undefined) - return false + return consumeEvent(event) } // Why: plain Mod+V is the Edit-menu accelerator, which reaches this window as ui:appMenuPaste — matching it here too would paste twice. const isMenuPasteChord = @@ -161,8 +214,12 @@ export function AgentTerminalPreview({ ptyId }: { ptyId: string }): React.JSX.El !isMenuPasteChord && keybindingMatchesAction('terminal.paste', event, platform, keybindings) ) { - void pasteClipboardText() - return false + const keyIdentity = event.code || event.key + if (!consumedClipboardKeys.has(keyIdentity)) { + consumedClipboardKeys.add(keyIdentity) + void pasteClipboardText(document.activeElement, 'keyboard') + } + return consumeEvent(event) } return true }) @@ -295,7 +352,7 @@ export function AgentTerminalPreview({ ptyId }: { ptyId: string }): React.JSX.El const offAppMenuPaste = window.api.ui.onAppMenuPaste(() => { const active = document.activeElement if (active && container.contains(active)) { - void pasteClipboardText() + void pasteClipboardText(active, 'app-menu') } })