Skip to content

Fix terminal duplication from pixel-only resizes - #4765

Closed
austinywang wants to merge 13 commits into
mainfrom
issue-3052-resizing-the-window-duplicates-terminal
Closed

austinywang wants to merge 13 commits into
mainfrom
issue-3052-resizing-the-window-duplicates-terminal

Conversation

@austinywang

@austinywang austinywang commented May 26, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Add regression coverage for coalescing pixel-only terminal resize churn.
  • Avoid forwarding AppKit pixel-only geometry changes to Ghostty as PTY resizes unless the terminal cell grid changes or live resize has settled.
  • Keep backing-scale changes and first applied sizes flowing through normally.

Issue

Fixes #3052.

Reproduction / testing

  • Issue Resizing the window duplicates terminal contents many times #3052 is a bug. I used the issue body, browser view, and narrowed repro comment to trace the resize path: AppKit layout -> GhosttyNSView.updateSurfaceSize -> TerminalSurface.updateSize -> ghostty_surface_set_size -> Ghostty resize/SIGWINCH.
  • The first commit adds the regression test only; the second commit adds the fix.
  • I did not run local tests, xcodebuild, or reload.sh per task constraints.

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag @codesmith with what you need. Autofix is disabled.


Note

Medium Risk
Touches the hot path from AppKit layout to Ghostty PTY/SIGWINCH sizing; wrong coalescing could stale renderer pixels or miss grid resizes, but behavior is gated on live resize with explicit end-of-resize flush and unit tests.

Overview
Fixes terminal duplication during live window resize (#3052) by not treating every AppKit pixel tweak as a PTY/grid resize.

TerminalSurface.updateSize now takes coalescePixelOnlyResize. When enabled (and scale did not change), it compares target pixels to the current column/row grid via new shouldApplySurfacePixelSizeChange and skips ghostty_surface_set_size if only backing pixels moved within the same grid. Coalescing applies only after a first pixel size was applied; grid changes, non-live layout, and scale updates still forward size changes.

GhosttyNSView turns coalescing on while isWindowLiveResizeActive, and viewDidEndLiveResize calls updateSurfaceSize(forcePixelOnlyResize: true) so renderer pixels flush when live resize ends. TerminalSurfaceResizePolicyTests cover coalesced pixel-only churn, grid changes, non-live updates, and first apply.

Reviewed by Cursor Bugbot for commit fdadf51. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Fixes terminal duplication during live window resize by coalescing pixel-only resize churn and flushing once resizing ends. Uses conservative grid detection (accounts for padding/remainders) so same-grid pixel tweaks don’t send PTY resizes; grid, scale, and first-apply changes still go through. Fixes #3052.

  • Bug Fixes
    • Coalesces AppKit pixel-only updates only during live resize; non-live or scale-change updates still apply.
    • Flushes a final pixel update in viewDidEndLiveResize; live-resize state is tracked via GhosttyNSView.isWindowLiveResizeActive.
    • Exposes TerminalSurface.shouldApplySurfacePixelSizeChange with conservative grid checks and adds Swift Testing coverage in TerminalSurfaceResizePolicyTests.

Written for commit 36efd5c. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Improvements

    • Optimized terminal resizing to eliminate redundant pixel-only updates and prevent unnecessary grid recalculations.
    • Enhanced live-resize responsiveness by ensuring the terminal settles immediately after interactive resize operations.
    • Improved resize efficiency during tab-drag operations by better managing resize deferral logic.
  • Tests

    • Added comprehensive test coverage for terminal surface resize policy scenarios.

@vercel

vercel Bot commented May 26, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Jun 15, 2026 12:15am
cmux-staging Building Building Preview, Comment Jun 15, 2026 12:15am

@coderabbitai

coderabbitai Bot commented May 26, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds pixel-only resize coalescing to prevent redundant PTY resize forwarding to Ghostty during live window dragging. A new static decision function gates forwarding based on whether the target pixel dimensions map to the same terminal grid. updateSize and updateSurfaceSize gain coalescing flags, live-resize deferral skips tab-drag suppression, and viewDidEndLiveResize forces a final reconciliation.

Changes

Terminal Pixel-Resize Coalescing

Layer / File(s) Summary
shouldApplySurfacePixelSizeChange decision function
Packages/CmuxTerminal/.../TerminalSurface+Sizing.swift
Adds a public static function that returns false (skip forwarding) when coalescing is active, a prior pixel size has been applied, and the target pixel dimensions map to the same grid columns/rows as the current state; otherwise returns true. Also adds the coalescePixelOnlyResize parameter to the updateSize signature.
Conditional forwarding path in updateSize
Packages/CmuxTerminal/.../TerminalSurface+Sizing.swift
Replaces the unconditional ghostty_surface_set_size call with a guarded path: reads current surface state via ghostty_surface_size, calls shouldApplySurfacePixelSizeChange, and early-returns with scaleChanged when the pixel-only resize should be coalesced.
GhosttyNSView live-resize wiring
Sources/GhosttyTerminalView.swift
Overrides viewDidEndLiveResize() to force a final updateSurfaceSize(forcePixelOnlyResize: true) and invalidate text input coordinates; introduces isWindowLiveResizeActive to suppress tab-drag deferral during live resize; extends updateSurfaceSize with forcePixelOnlyResize; passes derived coalescePixelOnlyResize to terminalSurface.updateSize.
TerminalSurfaceResizePolicyTests
cmuxTests/TerminalAndGhosttyTests.swift
Adds three unit tests asserting coalescing behavior: same-grid pixel change is skipped, grid-change triggers forwarding, and first pixel-size observation during live resize always applies.

Sequence Diagram(s)

sequenceDiagram
    participant User as User (drag)
    participant NSWindow as NSWindow
    participant GhosttyNSView as GhosttyNSView
    participant TerminalSurface as TerminalSurface+Sizing
    participant Ghostty as Ghostty (ghostty_surface_set_size)

    rect rgba(100, 149, 237, 0.5)
        Note over User,Ghostty: During live resize
        User->>NSWindow: drag window edge
        NSWindow->>GhosttyNSView: layout / bounds change
        GhosttyNSView->>TerminalSurface: updateSize(coalescePixelOnlyResize: true)
        TerminalSurface->>TerminalSurface: ghostty_surface_size → shouldApplySurfacePixelSizeChange
        TerminalSurface-->>GhosttyNSView: return scaleChanged (PTY resize skipped)
    end

    rect rgba(144, 238, 144, 0.5)
        Note over User,Ghostty: After live resize ends
        User->>NSWindow: release mouse
        NSWindow->>GhosttyNSView: viewDidEndLiveResize()
        GhosttyNSView->>TerminalSurface: updateSize(coalescePixelOnlyResize: false)
        TerminalSurface->>Ghostty: ghostty_surface_set_size (final dimensions)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • manaflow-ai/cmux#6057: Modifies the same updateSurfaceSize and resize deferral/retry flow in Sources/GhosttyTerminalView.swift, directly overlapping with the coalescing and pixel-size reconciliation changes here.

Suggested reviewers

  • jesstelford

🐇 Hoppity-hop, the pixels fly,
But now we ask: grid changed? — if no, goodbye!
Live resize drags on, we coalesce with care,
Then viewDidEndLiveResize says "finally, there!"
No more duplicated lines filling the screen,
The terminal stays tidy, composed, and clean. ✨


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 errors, 1 warning)

Check name Status Explanation Resolution
Cmux Swiftui State Layout ❌ Error PR violates .github/review-bot-rules/swiftui-state-layout.md: new GeometryReader measurement driving layout-affecting drop delegates in SidebarWorkspaceGroupHeaderView and TabItemView, with render-... Replace GeometryReader with onGeometryChange modifier and move rowHeight state updates to explicit lifecycle callbacks outside render time, or use immutable snapshots and closure-based parameter passing as documented in the rules.
Cmux Architecture Rethink ❌ Error The shouldApplySurfacePixelSizeChange helper does not account for non-grid pixels (padding), causing incorrect grid change detection when Ghostty surfaces have margins or padding that exceed what... Pass currentWidthPx/currentHeightPx from ghostty_surface_size() to shouldApplySurfacePixelSizeChange to compute non-grid padding, then subtract padding before dividing target pixels by cell size to calculate target grid correctly.
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (18 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preventing terminal duplication that occurs during pixel-only resize operations, which directly addresses the bug in issue #3052.
Linked Issues check ✅ Passed The PR implementation fully addresses issue #3052 by coalescing pixel-only resizes to prevent terminal duplication: adds shouldApplySurfacePixelSizeChange helper, implements coalescePixelOnlyResize parameter in updateSize, integrates with GhosttyNSView live-resize logic, and provides comprehensive unit test coverage in TerminalSurfaceResizePolicyTests.
Out of Scope Changes check ✅ Passed All code changes are directly aligned with fixing issue #3052: new helper method, parameter extensions, live-resize integration, and test coverage are all in scope for preventing pixel-only resize duplication.
Cmux Swift Actor Isolation ✅ Passed No Swift 6 actor isolation violations introduced. New static function uses pure value logic without MainActor; updateSize already has @MainActor for UI-bound access; viewDidEndLiveResize is NSView...
Cmux Swift Blocking Runtime ✅ Passed No blocking or timing-based synchronization (semaphores, sleeps, locks, waits, polling, main-queue sync) introduced in production code. Changes add deterministic boolean logic and test-only unit te...
Cmux Expensive Synchronous Load ✅ Passed PR adds only lightweight state queries (ghostty_surface_size) and arithmetic operations. No expensive synchronous loaders (sysctl, file I/O, network) are introduced on main actor or interactive paths.
Cmux Cache Substitution Correctness ✅ Passed No cache substitution occurs; fresh read via ghostty_surface_size is always called. Not in persistence/history/undo/snapshot path; this is a transient live-resize hot path.
Cmux No Hacky Sleeps ✅ Passed PR modifies only Swift files (.swift); check applies exclusively to TypeScript, JavaScript, shell, or build/runtime scripts. Swift timing is covered by separate rule swift-blocking-runtime.
Cmux Algorithmic Complexity ✅ Passed All changes are O(1) operations on scalars with no collection iterations, nested scans, or algorithmic complexity violations. Resize coalescing actually improves hot-path efficiency.
Cmux Swift Concurrency ✅ Passed PR introduces no legacy async patterns: new sizing helpers and resize logic are pure synchronous code; existing Task in scheduleDeferredSurfaceSizeRetryIfNeeded is pre-existing.
Cmux Swift @Concurrent ✅ Passed All modified Swift functions are synchronous with proper MainActor annotations. No async work, @concurrent abuse, or isolation violations detected.
Cmux Swift File And Package Boundaries ✅ Passed All three files modified comply with swift-file-package-boundaries.md rules: TerminalSurface+Sizing.swift (398 lines in CmuxTerminal package) is a focused sizing extension with coherent single resp...
Cmux Swift Logging ✅ Passed All logging in the PR complies with swift-logging.md: debug sizeLog calls are properly #if DEBUG-guarded, no print/NSLog in production code, and tests use only XCTAssert.
Cmux User-Facing Error Privacy ✅ Passed PR contains no user-facing error messages, alerts, or sensitive information. All strings are API documentation, debug logs (wrapped in #if DEBUG), or test assertions—all developer-only and not show...
Cmux Full Internationalization ✅ Passed PR adds only backend logic and tests with no user-facing text; all new string content is either debug-only (gated with #if DEBUG) or test assertions, which are allowed exceptions per the i18n rules.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR does not add or materially change standalone cmux-owned windows. Production code changes in TerminalSurface+Sizing.swift and GhosttyTerminalView.swift involve sizing logic only; test-only fixtur...
Cmux Source Artifacts ✅ Passed All three changed files (TerminalSurface+Sizing.swift, GhosttyTerminalView.swift, TerminalAndGhosttyTests.swift) are hand-written Swift source/test code in proper directories, not artifacts or proh...
Description check ✅ Passed The PR description is comprehensive and well-structured, covering all required template sections including summary, issue reference, testing details, and checklist items.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-3052-resizing-the-window-duplicates-terminal

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 and usage tips.

@greptile-apps

greptile-apps Bot commented May 26, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

Fixes terminal content duplication during live window resize (#3052) by coalescing pixel-only AppKit layout events that don't change the terminal's column/row grid, preventing redundant ghostty_surface_set_size calls (which carry both renderer pixels and PTY/SIGWINCH geometry) while still forwarding real grid changes and applying the final pixel size when live resize settles.

  • Adds shouldApplySurfacePixelSizeChange to TerminalSurface, a pure static function with conservative grid detection (accounts for sub-cell padding/remainder) that coalesces pixel-only churn only during live resize and always passes through grid changes, scale changes, and first-apply.
  • Wires coalescing in GhosttyNSView.updateSurfaceSize via isWindowLiveResizeActive, and flushes the final pixel size in a new viewDidEndLiveResize override.
  • Adds TerminalSurfaceResizePolicyTests covering coalesced pixel-only churn, ambiguous-remainder edge cases, grid changes, non-live updates, and first apply.

Confidence Score: 5/5

Safe to merge; coalescing is tightly gated on the live-resize window, grid changes and scale changes always pass through, and the final pixel flush in viewDidEndLiveResize ensures Ghostty receives correct dimensions once the user releases the window edge.

The shouldApplySurfacePixelSizeChange logic is correct and overflow-safe (UInt64 widening throughout), all guard paths (zero cell size, zero columns, first apply, scale change) pass through unconditionally, and the test suite covers the cases that would catch regressions in the coalescing decision. No path leaves Ghostty with stale renderer dimensions after live resize settles.

No files require special attention.

Important Files Changed

Filename Overview
Packages/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Sizing.swift Adds shouldApplySurfacePixelSizeChange (pure static, public, testable) and threads coalescePixelOnlyResize through updateSize; logic is correct, arithmetic is overflow-safe (UInt64 widening), and all guard paths (zero cell size, zero columns/rows, scale change) pass through safely.
Sources/GhosttyTerminalView.swift Adds isWindowLiveResizeActive computed property (centralises existing two-site check), passes coalescePixelOnlyResize gated on that property, and adds viewDidEndLiveResize to flush the final pixel size; forcePixelOnlyResize: true is redundant in that flush path (see comment) but harmless.
cmuxTests/TerminalSurfaceResizePolicyTests.swift New test suite for shouldApplySurfacePixelSizeChange; covers coalesced pixel-only churn, ambiguous-remainder conservative detection, grid-change passthrough, coalescing-off passthrough, and first-apply passthrough — all expected values are correct.
.github/swift-file-length-budget.tsv Budget updated to reflect the 14-line growth in GhosttyTerminalView.swift and minor reorderings; no new oversized files introduced.
cmux.xcodeproj/project.pbxproj Adds TerminalSurfaceResizePolicyTests.swift to the test target; file reference and build phase entries are consistent.

Sequence Diagram

sequenceDiagram
    participant AppKit
    participant GhosttyNSView
    participant TerminalSurface
    participant Ghostty

    Note over AppKit,Ghostty: During live resize (isWindowLiveResizeActive = true)

    AppKit->>GhosttyNSView: layout()
    GhosttyNSView->>TerminalSurface: updateSize(coalescePixelOnlyResize: true)
    TerminalSurface->>Ghostty: ghostty_surface_size() [read current grid]
    alt pixel-only change (same columns/rows)
        TerminalSurface-->>GhosttyNSView: skip ghostty_surface_set_size, return scaleChanged
    else grid change OR scale change
        TerminalSurface->>Ghostty: ghostty_surface_set_size(wpx, hpx)
        TerminalSurface-->>GhosttyNSView: return true
    end

    Note over AppKit,Ghostty: Live resize ends (isWindowLiveResizeActive = false)

    AppKit->>GhosttyNSView: viewDidEndLiveResize()
    GhosttyNSView->>TerminalSurface: updateSize(coalescePixelOnlyResize: false)
    TerminalSurface->>Ghostty: ghostty_surface_set_size(final wpx, hpx)
    TerminalSurface-->>GhosttyNSView: return true
Loading

Reviews (10): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

Comment thread Sources/GhosttyTerminalView.swift
Comment thread cmuxTests/TerminalAndGhosttyTests.swift Outdated
@Zhao-Qihao

Copy link
Copy Markdown

Waitting for merge!

@advenimus

Copy link
Copy Markdown

Is this going to be merged? this is breaking cmux ssh function on remote hosts, terminal gets all broken up upon resize of any kind.

@caydenphipps

Copy link
Copy Markdown

Also waiting for merge here

@austinywang

Copy link
Copy Markdown
Contributor Author

Synced this PR with current origin/main and resolved the resize-path conflict. The branch is pushed; CI and review bots are running now. No code-review threads are currently unresolved.

@austinywang

Copy link
Copy Markdown
Contributor Author

Status update: merge conflicts are resolved and the branch is pushed. All GitHub Actions/check-runs are green, and the two Greptile review threads are resolved; latest Greptile summary says safe to merge. The remaining blocker is Vercel: the cmux and cmux-staging deployments for fdadf51 are still queued/pending after 50+ minutes, and recent deployments for both projects are broadly queued, so this appears to be a Vercel queue/integration issue rather than a code failure.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@Packages/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface`+Sizing.swift:
- Around line 55-76: The shouldApplySurfacePixelSizeChange function divides
total pixel dimensions without accounting for padding (non-grid pixels), which
causes it to incorrectly return false when a resize crosses a real Ghostty grid
boundary but only in the padding area, leading updateSize to skip the
ghostty_surface_set_size call. Modify the function signature to accept
currentWidthPx and currentHeightPx parameters, then calculate the non-grid pixel
remainders (the padding) by subtracting the current grid dimensions multiplied
by cell dimensions from these current pixel dimensions. Use these non-grid
remainders when computing target grid dimensions by subtracting them from the
target pixel dimensions before dividing by cell dimensions. Finally, update the
call site at line 145 where this function is invoked to pass
currentSize.width_px and currentSize.height_px as the new parameters.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 646dabaa-dd43-49a2-aa21-5e974ce0ec12

📥 Commits

Reviewing files that changed from the base of the PR and between 9beabf0 and 0358969.

⛔ Files ignored due to path filters (1)
  • .github/swift-file-length-budget.tsv is excluded by !**/*.tsv
📒 Files selected for processing (3)
  • Packages/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Sizing.swift
  • Sources/GhosttyTerminalView.swift
  • cmuxTests/TerminalAndGhosttyTests.swift

Comment thread Packages/CmuxTerminal/Sources/CmuxTerminal/Surface/TerminalSurface+Sizing.swift Outdated
austinywang added a commit that referenced this pull request Jun 19, 2026
…6386)

* test: cover terminal pixel-only resize coalescing

* fix: coalesce terminal pixel-only resizes

* fix: keep pixel coalescing scoped to live resize

* fix: expose resize policy without testing shim

* fix: refresh Swift file length budget

* fix: account for terminal padding during resize coalescing

* fix: keep resize coalescing grid checks conservative

* fix: move resize policy tests to Swift Testing file

* fix: clarify live resize coalescing bypass

* fix: coalesce same-grid terminal resize shrinks

* fix: handle terminal resize padding remainders

* fix: keep terminal resize coalescing conservative

---------

Co-authored-by: austinpower1258 <austinwang115@gmail.com>
Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lawrencecchen lawrencecchen added the stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening. label Sep 23, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in cmux backlog Sep 23, 2026

This branch was successfully deployed

1 active deployment
Preview – cmux — 36efd5cd Deployed Jun 15, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Resizing the window duplicates terminal contents many times

6 participants