From de8bffe24045b396212f4f63de8960ec8380ea07 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 2 Oct 2026 07:29:07 -0700 Subject: [PATCH] Fix terminal width cutoff on wide panes (#24687) * fix(terminal): let wide panes use up to 1024 columns Adapt the wider viewport limit proposed in #16578 to the current runtime, shared RPC schemas, and preview sizing. Co-authored-by: innocarpe * test(terminal): wait for probe output after command echo * test(terminal): align RPC boundary with wider viewport limit --------- Co-authored-by: innocarpe --- .../runtime/remote-desktop-driver.test.ts | 10 ++ ...terminal-manifest-characterization.test.ts | 9 +- src/main/runtime/terminal-viewport.ts | 8 +- .../preview-grid-claim.test.ts | 63 ++++----- .../dashboard-popout/preview-grid-claim.ts | 19 +-- .../terminal-viewport-schemas-params.ts | 3 +- src/shared/terminal-viewport.test.ts | 29 ++++ src/shared/terminal-viewport.ts | 17 +++ ...ired-remote-terminal-wide-viewport.spec.ts | 124 ++++++++++++++++++ 9 files changed, 228 insertions(+), 54 deletions(-) create mode 100644 src/shared/terminal-viewport.test.ts create mode 100644 src/shared/terminal-viewport.ts create mode 100644 tests/e2e/paired-remote-terminal-wide-viewport.spec.ts diff --git a/src/main/runtime/remote-desktop-driver.test.ts b/src/main/runtime/remote-desktop-driver.test.ts index f8f3fd12037..afe120dbf6a 100644 --- a/src/main/runtime/remote-desktop-driver.test.ts +++ b/src/main/runtime/remote-desktop-driver.test.ts @@ -154,6 +154,16 @@ describe('remote desktop viewer width driver', () => { expect(driverEvents).toHaveLength(0) }) + it('keeps a wide desktop viewport through resize', async () => { + const { runtime } = createRuntime() + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 500, 40) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 500, rows: 40 }) + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 800, 40) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 800, rows: 40 }) + await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 2000, 40) + expect(runtime.getTerminalSize('pty-1')).toEqual({ cols: 1024, rows: 40 }) + }) + it('sizes the PTY to the latest active desktop viewer', async () => { const { runtime } = createRuntime() await runtime.updateRemoteDesktopViewer('pty-1', 'sub-A', 'viewer-A', 100, 40) diff --git a/src/main/runtime/rpc/methods/terminal-manifest-characterization.test.ts b/src/main/runtime/rpc/methods/terminal-manifest-characterization.test.ts index b5e7df475bb..d063edb397a 100644 --- a/src/main/runtime/rpc/methods/terminal-manifest-characterization.test.ts +++ b/src/main/runtime/rpc/methods/terminal-manifest-characterization.test.ts @@ -93,9 +93,16 @@ describe('terminal RPC manifest characterization', () => { schemaFor('terminal.updateViewport').parse({ terminal: 'term', client: { id: 'client' }, - viewport: { cols: 241, rows: 120 } + viewport: { cols: 1025, rows: 120 } }) ).toThrow() + expect(() => + schemaFor('terminal.updateViewport').parse({ + terminal: 'term', + client: { id: 'client' }, + viewport: { cols: 1024, rows: 120 } + }) + ).not.toThrow() expect(() => schemaFor('terminal.subscribe').parse({ terminal: 'term', diff --git a/src/main/runtime/terminal-viewport.ts b/src/main/runtime/terminal-viewport.ts index 3130f9a1a06..efffe04a9d8 100644 --- a/src/main/runtime/terminal-viewport.ts +++ b/src/main/runtime/terminal-viewport.ts @@ -1,7 +1 @@ -// Clamp terminal dimensions to the PTY's supported range (cols 20–240, rows 8–120). -export function clampTerminalViewport(cols: number, rows: number): { cols: number; rows: number } { - return { - cols: Math.max(20, Math.min(240, Math.round(cols))), - rows: Math.max(8, Math.min(120, Math.round(rows))) - } -} +export { clampTerminalViewport } from '../../shared/terminal-viewport' diff --git a/src/renderer/src/components/dashboard-popout/preview-grid-claim.test.ts b/src/renderer/src/components/dashboard-popout/preview-grid-claim.test.ts index e933174e87d..0d2022152eb 100644 --- a/src/renderer/src/components/dashboard-popout/preview-grid-claim.test.ts +++ b/src/renderer/src/components/dashboard-popout/preview-grid-claim.test.ts @@ -13,38 +13,41 @@ describe('createPreviewGridClaim', () => { vi.useRealTimers() }) - it('waits for a resize signal instead of polling while layout is unmeasurable', async () => { - vi.useFakeTimers() - const fit = vi.fn(async () => ({ cols: 90, rows: 30 })) - Object.assign(window, { api: { terminalPreview: { fit } } }) - const box = document.createElement('div') - const container = document.createElement('div') - const screen = document.createElement('div') - screen.className = 'xterm-screen' - box.appendChild(container) - container.appendChild(screen) - dimension(box, 'clientWidth', 900) - dimension(box, 'clientHeight', 480) - dimension(screen, 'offsetWidth', 0) - dimension(screen, 'offsetHeight', 0) - const claim = createPreviewGridClaim({ - ptyId: 'pty-1', - container, - getTerminal: () => ({ cols: 80, rows: 24 }) as never - }) + it.each([90, 500, 1024, 2000])( + 'waits for a measurable layout and fits %i columns', + async (cols) => { + vi.useFakeTimers() + const fit = vi.fn(async () => ({ cols: 90, rows: 30 })) + Object.assign(window, { api: { terminalPreview: { fit } } }) + const box = document.createElement('div') + const container = document.createElement('div') + const screen = document.createElement('div') + screen.className = 'xterm-screen' + box.appendChild(container) + container.appendChild(screen) + dimension(box, 'clientWidth', cols * 10) + dimension(box, 'clientHeight', 480) + dimension(screen, 'offsetWidth', 0) + dimension(screen, 'offsetHeight', 0) + const claim = createPreviewGridClaim({ + ptyId: 'pty-1', + container, + getTerminal: () => ({ cols: 80, rows: 24 }) as never + }) - claim.schedule() - await vi.advanceTimersByTimeAsync(1_000) - expect(fit).not.toHaveBeenCalled() - expect(vi.getTimerCount()).toBe(0) + claim.schedule() + await vi.advanceTimersByTimeAsync(1_000) + expect(fit).not.toHaveBeenCalled() + expect(vi.getTimerCount()).toBe(0) - dimension(screen, 'offsetWidth', 800) - dimension(screen, 'offsetHeight', 384) - claim.schedule() - await vi.advanceTimersByTimeAsync(200) - expect(fit).toHaveBeenCalledWith('pty-1', 90, 30) - claim.dispose() - }) + dimension(screen, 'offsetWidth', 800) + dimension(screen, 'offsetHeight', 384) + claim.schedule() + await vi.advanceTimersByTimeAsync(200) + expect(fit).toHaveBeenCalledWith('pty-1', Math.min(cols, 1024), 30) + claim.dispose() + } + ) it('coalesces a continuous resize burst into one settled fit request', async () => { vi.useFakeTimers() diff --git a/src/renderer/src/components/dashboard-popout/preview-grid-claim.ts b/src/renderer/src/components/dashboard-popout/preview-grid-claim.ts index 6f145e13ea1..a9d94f84f28 100644 --- a/src/renderer/src/components/dashboard-popout/preview-grid-claim.ts +++ b/src/renderer/src/components/dashboard-popout/preview-grid-claim.ts @@ -1,16 +1,7 @@ import type { Terminal } from '@xterm/xterm' +import { clampTerminalViewport } from '../../../../shared/terminal-viewport' const FIT_REQUEST_DEBOUNCE_MS = 200 -// Mirror the runtime's clampTerminalViewport so a request always matches what lands. -const FIT_MIN_COLS = 20 -const FIT_MAX_COLS = 240 -const FIT_MIN_ROWS = 8 -const FIT_MAX_ROWS = 120 - -function clampGridAxis(value: number, min: number, max: number): number { - return Math.min(max, Math.max(min, value)) -} - /** * Negotiates the PTY grid for the popout terminal dialog: measures the live * terminal's cell size, computes the grid the dialog box can hold, and asks @@ -52,11 +43,9 @@ export function createPreviewGridClaim(args: { ) { return } - const cols = clampGridAxis(Math.floor(box.clientWidth / cellWidth), FIT_MIN_COLS, FIT_MAX_COLS) - const rows = clampGridAxis( - Math.floor(box.clientHeight / cellHeight), - FIT_MIN_ROWS, - FIT_MAX_ROWS + const { cols, rows } = clampTerminalViewport( + Math.floor(box.clientWidth / cellWidth), + Math.floor(box.clientHeight / cellHeight) ) const fitKey = `${cols}x${rows}` if (fitKey === lastRequestedFit) { diff --git a/src/shared/rpc-contract/terminal-viewport-schemas-params.ts b/src/shared/rpc-contract/terminal-viewport-schemas-params.ts index 416ce7859e0..e355864b11d 100644 --- a/src/shared/rpc-contract/terminal-viewport-schemas-params.ts +++ b/src/shared/rpc-contract/terminal-viewport-schemas-params.ts @@ -1,4 +1,5 @@ import { z } from 'zod' +import { TERMINAL_VIEWPORT_MAX_COLS } from '../terminal-viewport' import { requiredString } from './rpc-param-primitives' export const TerminalHandle = z.object({ terminal: requiredString('Missing terminal handle') }) @@ -41,7 +42,7 @@ export const TerminalUpdateViewport = TerminalHandle.extend({ type: z.enum(['mobile', 'desktop']).default('mobile').optional() }), viewport: z.object({ - cols: z.number().int().min(20).max(240), + cols: z.number().int().min(20).max(TERMINAL_VIEWPORT_MAX_COLS), rows: z.number().int().min(8).max(120) }), claim: z.boolean().optional() diff --git a/src/shared/terminal-viewport.test.ts b/src/shared/terminal-viewport.test.ts new file mode 100644 index 00000000000..5721ac2959e --- /dev/null +++ b/src/shared/terminal-viewport.test.ts @@ -0,0 +1,29 @@ +import { describe, expect, it } from 'vitest' +import { clampTerminalViewport } from './terminal-viewport' +import { TerminalUpdateViewport } from './rpc-contract/terminal-viewport-schemas-params' + +describe('terminal viewport', () => { + it.each([240, 500, 1024])('preserves %i columns in the clamp and RPC', (cols) => { + expect(clampTerminalViewport(cols, 40)).toEqual({ cols, rows: 40 }) + expect( + TerminalUpdateViewport.parse({ + terminal: 'pty-1', + client: { id: 'desktop-1' }, + viewport: { cols, rows: 40 } + }).viewport + ).toEqual({ cols, rows: 40 }) + }) + + it('bounds oversized grids and preserves minimum dimensions and rounding', () => { + expect(clampTerminalViewport(2000, 200)).toEqual({ cols: 1024, rows: 120 }) + expect(clampTerminalViewport(10, 4)).toEqual({ cols: 20, rows: 8 }) + expect(clampTerminalViewport(500.4, 40.6)).toEqual({ cols: 500, rows: 41 }) + expect( + TerminalUpdateViewport.safeParse({ + terminal: 'pty-1', + client: { id: 'desktop-1' }, + viewport: { cols: 1025, rows: 40 } + }).success + ).toBe(false) + }) +}) diff --git a/src/shared/terminal-viewport.ts b/src/shared/terminal-viewport.ts new file mode 100644 index 00000000000..c8ae9252c8b --- /dev/null +++ b/src/shared/terminal-viewport.ts @@ -0,0 +1,17 @@ +export const TERMINAL_VIEWPORT_MIN_COLS = 20 +export const TERMINAL_VIEWPORT_MAX_COLS = 1024 +export const TERMINAL_VIEWPORT_MIN_ROWS = 8 +export const TERMINAL_VIEWPORT_MAX_ROWS = 120 + +export function clampTerminalViewport(cols: number, rows: number): { cols: number; rows: number } { + return { + cols: Math.max( + TERMINAL_VIEWPORT_MIN_COLS, + Math.min(TERMINAL_VIEWPORT_MAX_COLS, Math.round(cols)) + ), + rows: Math.max( + TERMINAL_VIEWPORT_MIN_ROWS, + Math.min(TERMINAL_VIEWPORT_MAX_ROWS, Math.round(rows)) + ) + } +} diff --git a/tests/e2e/paired-remote-terminal-wide-viewport.spec.ts b/tests/e2e/paired-remote-terminal-wide-viewport.spec.ts new file mode 100644 index 00000000000..54a08a9a0a9 --- /dev/null +++ b/tests/e2e/paired-remote-terminal-wide-viewport.spec.ts @@ -0,0 +1,124 @@ +import { expect, test } from './helpers/orca-app' +import { + createRuntimeDesktopPairingOffer, + launchPairedElectronClient +} from './helpers/paired-electron-client' +import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { + execInTerminal, + focusActiveTerminalInput, + waitForActivePanePtyId, + waitForActiveTerminalManager, + waitForTerminalOutput +} from './helpers/terminal' + +test('paired terminal fills a pane wider than 240 columns', async ({ + orcaPage, + testRepoPath +}, testInfo) => { + test.skip(process.platform === 'win32', 'The width probe uses POSIX stty') + test.setTimeout(180_000) + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + await ensureTerminalVisible(orcaPage) + await waitForActiveTerminalManager(orcaPage) + const hostPtyId = await waitForActivePanePtyId(orcaPage) + await execInTerminal(orcaPage, hostPtyId, "printf 'WIDE_PROBE_READY\\n'") + await waitForTerminalOutput(orcaPage, 'WIDE_PROBE_READY') + const client = await launchPairedElectronClient( + await createRuntimeDesktopPairingOffer(orcaPage), + testInfo, + 'Wide terminal proof' + ) + try { + const page = client.page + await page.setViewportSize({ width: 3200, height: 1000 }) + await expect + .poll( + () => + page.evaluate( + (repoPath) => + window.__store + ?.getState() + .allWorktrees() + .find((worktree) => worktree.path === repoPath)?.id, + testRepoPath + ), + { timeout: 60_000 } + ) + .toBeTruthy() + await page.evaluate( + ({ repoPath, environmentId }) => { + const state = window.__store?.getState() + const worktree = state?.allWorktrees().find((entry) => entry.path === repoPath) + if (!state || !worktree) { + throw new Error('Paired worktree unavailable') + } + state.setActiveWorktree(worktree.id, `runtime:${environmentId}`) + }, + { repoPath: testRepoPath, environmentId: client.environmentId } + ) + await ensureTerminalVisible(page, 30_000) + await waitForActiveTerminalManager(page, 30_000) + await waitForTerminalOutput(page, 'WIDE_PROBE_READY', 30_000) + const ptyId = await waitForActivePanePtyId(page, 30_000) + expect(ptyId.startsWith(`remote:${client.environmentId}@@`)).toBe(true) + await expect + .poll( + () => + page.evaluate(() => { + const state = window.__store?.getState() + const tabId = state?.activeWorktreeId + ? state.activeTabIdByWorktree[state.activeWorktreeId] + : null + return tabId + ? (window.__paneManagers?.get(tabId)?.getActivePane()?.terminal.cols ?? 0) + : 0 + }), + { timeout: 30_000 } + ) + .toBeGreaterThan(240) + await focusActiveTerminalInput(page) + await page.keyboard.type( + "cols=$(stty size | awk '{print $2}'); printf '\\033[2J\\033[HPTY_WIDTH=%s\\r\\n' \"$cols\"; printf '%*s' \"$((cols - 10))\" '' | tr ' ' '='; printf 'RIGHT_EDGE\\r\\n'" + ) + await page.keyboard.press('Enter') + await waitForTerminalOutput(page, 'RIGHT_EDGE', 30_000) + const readRendered = () => + page.evaluate(() => { + const state = window.__store?.getState() + const tabId = state?.activeWorktreeId + ? state.activeTabIdByWorktree[state.activeWorktreeId] + : null + const pane = tabId ? window.__paneManagers?.get(tabId)?.getActivePane() : null + if (!pane) { + throw new Error('Terminal unavailable') + } + const buffer = pane.terminal.buffer.active + return { + cols: pane.terminal.cols, + lines: Array.from( + { length: pane.terminal.rows }, + (_, row) => buffer.getLine(buffer.viewportY + row)?.translateToString(true) ?? '' + ) + } + }) + await expect + .poll( + async () => { + const rendered = await readRendered() + return { + widthMatches: rendered.lines.includes(`PTY_WIDTH=${rendered.cols}`), + rightEdge: rendered.lines.some( + (line) => line.length === rendered.cols && line.endsWith('RIGHT_EDGE') + ) + } + }, + { timeout: 30_000 } + ) + .toEqual({ widthMatches: true, rightEdge: true }) + await page.screenshot({ path: testInfo.outputPath('wide-terminal.png') }) + } finally { + await client.dispose() + } +})