Skip to content

fix(terminal): a revealed paired terminal fits once, at its real size, on Linux - #29668

Merged
nwparker merged 1 commit into
mainfrom
nwparker/fix-paired-reveal-grid-linux
Oct 16, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/fix-paired-reveal-grid-linux

Conversation

@nwparker

@nwparker nwparker commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​112 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​111
Prod 4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​85 $\color{#cf222e}{\Huge{\mathbf{−}}}$​5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​80

ELI5

When you open a parked terminal in a paired (web) client, it should show up once, already at the right size. On Linux it showed up at one size, resized itself, then resized again, all within about a quarter of a second. Two separate measuring mistakes caused this. This PR fixes both, so the terminal is measured correctly the first time.

What Changed

Before: revealing a parked paired terminal on a Linux desktop drew the text at 52x50, reflowed it to 51x49 or 52x49, and sometimes reflowed it again after a second hidden replay (the CI samples show 52x50, then 51x49 for about 125 ms, then 52x49). Each extra resize also sends a resize to the remote program, so a TUI redraws.

After: the pane is created at its real box and lands on one grid (51x49 in the test), whatever grid it started from.

Mechanism, two fixes:

  1. The pane mounted before its host had been placed (RetainedPaneHost). The web client can't use CSS anchor positioning, so the host is placed from a measured tab-group body rect. React runs a child's layout effect before its parent's, so the terminal pane created xterm and fit it while the host still had its placeholder box (top: 32px; height: calc(100% - 32px), 450x804 here, against the real 449x800 at top 36). About 80 ms later the measured rect arrived and the pane refit. The host now renders its children only after the first rect read, which happens in the same commit, before paint. Once the children are mounted, the host never unmounts them again, and a host with no body to measure mounts them right away.
  2. The fit result depended on the grid it started from (StableCellFitAddon). xterm's FitAddon divides the box by terminal.dimensions.css.cell.width. With the DOM renderer (Linux CI, no GPU) that value is round(charWidth * cols) / cols, so it changes with the current column count. For the same 438 px box it proposes 52 cols from a 52-col pane and 51 cols from the 75-col host-grid replay. So a reveal replay landed at 51, and any later fit from 52 kept 52, which caused a second resize and a second replay. The pane now uses a FitAddon subclass that divides by the device cell divided by DPR. That value doesn't depend on the grid, and it matches the cell the renderer actually draws. It reuses the existing terminalGridSizeForBox mirror of FitAddon's math. The new-tab grid predictor (terminal-group-grid-sizes.ts) uses the same cell, so its predictions match real fits.

Why

This is (a): a real product issue, not just a test artifact. The test's width-only resize wait wasn't the cause: in every captured sample the window was already 1080x860 and the pane box was a constant 445x796. The jitter came from the two product bugs above.

  • Fix 1 hits every paired web-client pane on creation, on any OS. Whether it shows depends on whether the 4 px or 1 px placeholder difference crosses a cell boundary, which it does with Linux's 16 px cells.
  • Fix 2 hits any pane whose css cell isn't an integer: the DOM renderer, or WebGL at fractional DPR (for example Windows at 125% or 150%).

Alternatives considered:

  • Delay the reveal until the fit is stable (a settle timer). This hides the symptom, adds latency to every reveal, and still sends the extra PTY resizes.
  • Fix the spec's wait. That wouldn't change anything, because the geometry had already settled.
  • For fix 2, add hysteresis around the current grid. That keeps whichever grid you happened to start from, while the device cell gives one answer per box.

The reveal, fit and claim machinery from #29269, #29331 and #29349 is unchanged. It just gets the right size the first time now.

Linked Issue

N/A: release-gate triage. tests/e2e/headless-paired-remote-terminal-reveal-grid.spec.ts (added by #29331) failed every time on Linux CI.

Visual Proof

N/A: the e2e samples are the proof. Before (Linux, main): visible frames at 52x50, then 51x49, then 52x49. After: visible frames only at the final 51x49.

Testing

  • Reproduced on main, on a real Linux host (openclaw, Ubuntu 24.04, xvfb + openbox like CI, isolated /tmp checkout and HOME, ORCA_BACKGROUND_LAUNCH=1 ORCA_E2E_WEB_CLIENT=1): the spec failed 2/2. A per-sample probe recorded the box, the css cell (8.4231 at 52 cols, 8.4267 at 75, 8.4314 at 51, from one 8.4287 px char) and a stack for every resize. That's how both causes were pinned down.

  • With this PR, same host: the spec passed 3/3. With only fix 1 it also passed 3/3. Fix 2 removes the 51/52 path dependence that caused the second replay in CI run 38865222465.

  • Related e2e on the same host: paired-terminal-switch-desktop-reveal, headless-paired-remote-terminal-viewer-size, terminal-replay-presentation (the WebGL case skips, as on CI), plus the paired remote terminal, split and layout specs. Results are in the report.

  • Unit tests: terminal-stable-cell-fit-addon.test.ts asserts that stock FitAddon is path-dependent and the stable addon isn't. A new RetainedPaneHost test checks the child sees the measured geometry at mount (it fails before the fix with 32px 100%). Folders run single-worker: lib/pane-manager, components/tab-group, runtime/sync-runtime-graph, components/terminal-pane, components/native-chat.

  • Static analysis: the full static analysis and typecheck job steps run locally, including oxlint (root, anti-slop, native and type-aware code quality), changed-code quality, React Doctor, pnpm typecheck, and the max-lines, ts-nocheck, focus-read, layout-writer and Electron-import ratchets.

  • Codex review (gpt-6.1-sol, high): no findings.

  • Platforms: verified live on Linux only. macOS and Windows are covered by unit tests and CI.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • SSH and remote: this is renderer-only. Nothing changes on the wire. A paired client just sends one correct resize instead of two or three.
  • Cross-platform: DOM-renderer users and users at fractional DPR may now get one column fewer in borderline boxes. That is the column count that actually fits: 52 x 8.4287 px is 438.3 px, which overflows 438 px.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…same grid from any grid

The web client positions a retained pane host from a measured tab-group body,
but the pane mounted (and fit xterm) a render before that measurement, at the
32px placeholder box, then refit when the measurement landed. Mount the pane
only after the first read.

FitAddon also divides by xterm's css cell, which is the rounded canvas width
over the current cols, so the same box proposed 51 cols from a 75-col replay
and 52 from a 52-col pane: a revealed pane resized twice. Fit with the
grid-independent device cell instead.
@coderabbitai

coderabbitai Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b811fb84-aa1c-4177-a790-35a1dd7bd094
📥 Commits

Reviewing files that changed from the base of the PR and between 78f2e9b and e7f19d8.

📒 Files selected for processing: 7
  • src/renderer/src/components/tab-group/RetainedPaneHost.test.tsx
  • src/renderer/src/components/tab-group/RetainedPaneHost.tsx
  • src/renderer/src/lib/pane-manager/pane-dom-creation.ts
  • src/renderer/src/lib/pane-manager/terminal-stable-cell-fit-addon.test.ts
  • src/renderer/src/lib/pane-manager/terminal-stable-cell-fit-addon.ts
  • src/renderer/src/runtime/sync-runtime-graph/terminal-group-grid-sizes.test.ts
  • src/renderer/src/runtime/sync-runtime-graph/terminal-group-grid-sizes.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

📝 Walkthrough

RetainedPaneHost now waits for fallback geometry before rendering pane children, unless measurement is unnecessary or unavailable. Pane creation now uses StableCellFitAddon, which calculates fit proposals from stable cell dimensions. Default-pane grid sizing uses the same cell-dimension reader. New tests cover mount timing, fit proposals, and device-cell measurements.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to e7f19

The reviewed changes are mergeable after normal checks; no actionable risk remains established.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main terminal fitting fix and its Linux-specific user impact.
Description check Passed The description is detailed and follows the template. It explains the user impact, implementation, rationale, alternatives, testing, platform coverage, and checklist status. It does not provide an act…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description is detailed and follows the template. It explains the user impact, implementation, rationale, alternatives, testing, platform coverage, and checklist status. It does not provide an actual linked issue and uses N/A for visual proof, but these gaps do not make the otherwise complete description largely incomplete.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant