From e9e07818e41ff36467c9e7a230877366d03cb6ea Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 6 Aug 2026 16:43:52 -0700 Subject: [PATCH] Revert "Revert "fix(terminal): report the replay destination grid from measured hidden panes"" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restores 390ae5ce52 as the settled state; both files are now byte-identical to it. The visibility gate stays off the replay destination-grid report. What settles it: the measurability precheck means only a pane whose fit measured a real box ever reaches the report, so display:none panes never get there. The remaining hidden case is the background measure lease, which lays a pane out full size on purpose so it can be measured — an authoritative grid. With the presence lock still applying and this path never signalling SIGWINCH, the residual risk is smaller than the client/host grid disagreement the gate would cause, which is the failure class this PR exists to fix. Correcting an overstatement in 390ae5ce52's message: withholding the report does not pin the host "permanently". Reveal itself will not recover it — fitRevealedPane skips a pane whose pixels never changed, and remote PTYs have no size-reassertion drift repair — but once the pane is visible any later layout change fires onResize -> forwardPtyResize, which reports. The accurate scope is "until the next layout change after reveal". --- .../terminal-pane/pty-connection.test.ts | 10 +++++----- .../terminal-pane/pty-connection.ts | 19 +++++++------------ 2 files changed, 12 insertions(+), 17 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 784f329a102..bfde8f99a5e 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -9094,7 +9094,10 @@ describe('connectPanePty', () => { expect(transport.resize.mock.calls).toEqual([]) }) - it('refits but does not report the destination grid from a background measure-lease pane', async () => { + it('reports the destination grid from a background measure-lease pane', async () => { + // Why this test: the report is deliberately NOT gated on visibility here. A hidden + // pane that still measures is under the background measure lease, so its grid is + // authoritative — and reveal never re-sends it, because the pixels never changed. const { pane, transport, replayCallback } = await connectRemoteSnapshotGridPane( 'remote:web-env-1@@pty-replay-grid-hidden', { visible: false } @@ -9103,12 +9106,9 @@ describe('connectPanePty', () => { replayCallback.current?.('remote snapshot bytes', { snapshotCols: 80, snapshotRows: 24 }) await flushAsyncTicks(20) - // A measure-lease pane is fully sized, so the fit lands — but reporting from a - // hidden pane would persist as a claim on the next subscribe. Its grid stays - // unreported until the next layout change after reveal; see the send condition. expect(pane.fitAddon.fit).toHaveBeenCalled() expect(pane.terminal.cols).toBe(120) - expect(transport.resize).not.toHaveBeenCalled() + expect(transport.resize).toHaveBeenCalledWith(120, 40) }) it('arms no fit retry after replaying into an unmeasurable pane', async () => { diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index 0f0b0052184..5d0ddb7c8fd 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -5813,21 +5813,16 @@ export function connectPanePty( } const destinationCols = pane.terminal.cols const destinationRows = pane.terminal.rows - // Why the visibility gate, matching the sibling at the hidden-snapshot - // restore: the transport remembers every viewport handed to resize() - // before the claim branch and replays it with claim intent on the next - // subscribe, so a send from a hidden pane reaches other clients as a - // durable claim. Accepted tradeoff: a background measure-lease pane is - // fully sized, so its fit succeeds and its grid then goes unreported - // until the next layout change after reveal — fitRevealedPane will not - // re-fit a pane whose pixels never changed (opacity does not affect - // getBoundingClientRect) and remote PTYs have no ptySizeReassertion - // drift repair. A stale grid until the next resize is the more - // conservative failure than an unrevocable wrong-grid claim. + // Why NOT gated on visibility, unlike the sibling fits: reaching here + // means the fit measured a real box, which for a hidden pane means the + // background measure lease deliberately laid it out full size — an + // authoritative grid, not hidden-layout churn. Reveal will not re-send + // it either: the pixels never changed, so fitRevealedPane skips, and + // remote PTYs are excluded from ptySizeReassertion. This is the only + // chance to report. (This path never signals SIGWINCH.) if ( destinationCols > 0 && destinationRows > 0 && - isRendererPtyResizeAuthoritative() && !shouldSuppressDesktopPtyResize() ) { transport.resize(destinationCols, destinationRows)