From 51a9db1d10eea212018645aa21dffa35e6e262cd Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 6 Aug 2026 16:11:57 -0700 Subject: [PATCH] fix(terminal): guard the remote snapshot destination-grid report MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The destination fit that follows a source-grid replay called transport.resize directly, bypassing the mobile presence lock every other renderer-originated PTY resize honors — so a phone-parked PTY could be dragged back to desktop dims by a reconnect. Guard the report with shouldSuppressDesktopPtyResize() and positive dimensions; the fit itself still always runs, since xterm would otherwise stay parked at the host grid. Also skip the source-grid round trip for a body-less snapshot frame: a frame carrying only a mid-escape tail has no rows to rewrap, so resizing to the host grid (a fallback 80x24 when the host has no serializable buffer) and back was two reflows at a grid nothing renders at. Adds regression coverage for the remote onReplayData meta path, which had none: the pre-existing snapshot-grid tests drive the connect-result path. --- .../terminal-pane/pty-connection.test.ts | 105 +++++++++++++++++- .../terminal-pane/pty-connection.ts | 16 ++- 2 files changed, 119 insertions(+), 2 deletions(-) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 351b3d244e7..4a716685232 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -308,7 +308,12 @@ type ConnectCallbacks = { ) => void onReplayData?: ( data: string, - meta?: { clearBeforeReplay?: boolean; snapshotCols?: number; snapshotRows?: number } + meta?: { + clearBeforeReplay?: boolean + pendingEscapeTailAnsi?: string + snapshotCols?: number + snapshotRows?: number + } ) => void onError?: (msg: string) => void onWriteUnavailable?: () => void @@ -741,6 +746,46 @@ function enableActiveRuntimeEnvironment(environmentId = 'env-1'): void { } as StoreState } +// Connects a remote pane whose viewport (120x40) differs from the host snapshot grid, +// exposing the onReplayData callback plus the grid each replay write parsed at. +async function connectRemoteSnapshotGridPane(ptyId: string) { + const { connectPanePty } = await import('./pty-connection') + enableActiveRuntimeEnvironment() + const pane = createPane(1) + const colsAtWrite = new Map() + pane.terminal.write = vi.fn((data: string, callback?: () => void) => { + colsAtWrite.set(data, pane.terminal.cols) + callback?.() + }) as typeof pane.terminal.write + pane.terminal.resize.mockImplementation((cols: number, rows: number) => { + pane.terminal.cols = cols + pane.terminal.rows = rows + }) + pane.fitAddon = { + ...pane.fitAddon, + fit: vi.fn(() => { + pane.terminal.cols = 120 + pane.terminal.rows = 40 + }), + proposeDimensions: vi.fn(() => ({ cols: 120, rows: 40 })) + } as never + const transport = createMockTransport(ptyId) + const replayCallback: { current: ConnectCallbacks['onReplayData'] | null } = { current: null } + transport.connect.mockImplementation(async ({ callbacks }: { callbacks: ConnectCallbacks }) => { + replayCallback.current = callbacks.onReplayData ?? null + return { id: ptyId, replay: '' } + }) + transportFactoryQueue.push(transport) + + connectPanePty(pane as never, createManager(1, 1) as never, createDeps() as never) + await flushAsyncTicks(20) + transport.resize.mockClear() + pane.terminal.resize.mockClear() + vi.mocked(pane.fitAddon.fit).mockClear() + expect(replayCallback.current).toBeTypeOf('function') + return { pane, transport, replayCallback, colsAtWrite } +} + function createKeyboardEventTarget() { const handlers = new Set<(event: KeyboardEvent) => void>() return { @@ -8999,6 +9044,64 @@ describe('connectPanePty', () => { expect(writes.slice(tailIndex + 1)).toEqual([]) }) + it('parses onReplayData at the host snapshot grid, then reports the destination fit', async () => { + // Why this test: the connect-result snapshot path is covered, but the remote onReplayData + // meta path had none — severing its source-grid resize kept the suite green. + const { pane, transport, replayCallback, colsAtWrite } = await connectRemoteSnapshotGridPane( + 'remote:web-env-1@@pty-replay-grid' + ) + + replayCallback.current?.('remote snapshot bytes', { snapshotCols: 80, snapshotRows: 24 }) + await flushAsyncTicks(20) + + // Host grid while the bytes parse, destination grid once the transaction restores. + expect(colsAtWrite.get('remote snapshot bytes')).toBe(80) + expect(pane.terminal.resize).toHaveBeenCalledWith(80, 24) + expect(pane.fitAddon.fit).toHaveBeenCalled() + expect(pane.terminal.cols).toBe(120) + expect(transport.resize).toHaveBeenCalledWith(120, 40) + }) + + it('refits but does not report the destination grid while mobile owns the replayed PTY', async () => { + const { setDriverForPty } = await import('@/lib/pane-manager/mobile-driver-state') + const ptyId = 'remote:web-env-1@@pty-replay-grid-locked' + const { pane, transport, replayCallback, colsAtWrite } = + await connectRemoteSnapshotGridPane(ptyId) + + try { + setDriverForPty(ptyId, { kind: 'mobile', clientId: 'phone-1' }) + replayCallback.current?.('remote snapshot bytes', { snapshotCols: 80, snapshotRows: 24 }) + await flushAsyncTicks(20) + } finally { + setDriverForPty(ptyId, { kind: 'idle' }) + } + + // xterm still leaves the host grid; only the SIGWINCH-bearing report is withheld. + expect(colsAtWrite.get('remote snapshot bytes')).toBe(80) + expect(pane.fitAddon.fit).toHaveBeenCalled() + expect(pane.terminal.cols).toBe(120) + expect(transport.resize).not.toHaveBeenCalled() + }) + + it('skips the source-grid trip for a body-less remote snapshot frame', async () => { + const { pane, transport, replayCallback } = await connectRemoteSnapshotGridPane( + 'remote:web-env-1@@pty-replay-grid-tail' + ) + + replayCallback.current?.('', { + pendingEscapeTailAnsi: '\x1b[3', + snapshotCols: 80, + snapshotRows: 24 + }) + await flushAsyncTicks(20) + + // Nothing to rewrap, so the host grid (a fallback 80x24 when the host has no + // serializable buffer) must not drag xterm through a two-reflow round trip. + expect(pane.terminal.resize).not.toHaveBeenCalled() + expect(pane.fitAddon.fit).not.toHaveBeenCalled() + expect(transport.resize).not.toHaveBeenCalled() + }) + it('preserves live modes and injects focus-in after focused agent reattach', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport('tab-pty') diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 35a0f144710..e4fbd3366a5 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -5687,6 +5687,9 @@ export function connectPanePty( ) if ( snapshotDimensions && + // Why: a body-less frame (mid-escape tail only) has no rows to rewrap, so the + // source-grid trip would just reflow twice at a grid nothing renders at. + data.length > 0 && (pane.terminal.cols !== snapshotDimensions.cols || pane.terminal.rows !== snapshotDimensions.rows) ) { @@ -5794,7 +5797,18 @@ export function connectPanePty( if (!isCurrentReplay()) { return } - transport.resize(pane.terminal.cols, pane.terminal.rows) + const destinationCols = pane.terminal.cols + const destinationRows = pane.terminal.rows + // Why: the fit itself must always run — xterm is parked at the host + // grid — but reporting it while mobile owns the PTY would drag the + // parked phone grid back to desktop dims (see forwardPtyResize). + if ( + destinationCols > 0 && + destinationRows > 0 && + !shouldSuppressDesktopPtyResize() + ) { + transport.resize(destinationCols, destinationRows) + } }, { shouldContinue: isCurrentReplay, retryIfUnmeasurable: true } )