Skip to content

Fix terminal input during window drags - #4281

Closed
austinywang wants to merge 11 commits into
mainfrom
issue-4125-window-move-cc
Closed

austinywang wants to merge 11 commits into
mainfrom
issue-4125-window-move-cc

Conversation

@austinywang

@austinywang austinywang commented May 18, 2026 •

Copy link
Copy Markdown
Contributor

Closes #4125.

Suppresses terminal mouse reporting while cmux performs explicit titlebar window drags so moving the app window cannot write mouse-report bytes into the shell prompt.


Note

Medium Risk
Touches low-level AppKit mouse/drag event routing and terminal input forwarding; regressions could break window dragging or terminal mouse interactions in edge cases.

Overview
Prevents terminal mouse reporting from firing while the app is performing an explicit window.performDrag(with:), avoiding mouse-report bytes being written into the terminal during window moves.

Adds a per-window, reference-counted terminal-input suppression state in WindowDragHandleView.swift (with withWindowMoveTerminalInputSuppression), wraps titlebar drag initiators in UpdateTitlebarAccessory/WindowDragHandleView, and gates multiple GhosttyTerminalView mouse handlers (down/up/right/other/move/enter/exit/drag/menu) behind shouldForwardTerminalMouseEventToGhostty.

Adds unit tests covering suppression toggling and reference counting.

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


Summary by cubic

Prevents terminal mouse and scroll input from leaking into the shell during explicit titlebar window drags. Input reporting is fully suppressed while dragging so escape bytes aren’t sent.

  • Bug Fixes
    • Added a per-window, reference-counted terminal-input suppression state and withWindowMoveTerminalInputSuppression.
    • Wrapped window.performDrag(with:) in the titlebar accessory and drag handle; gated all GhosttyTerminalView handlers via shouldForwardTerminalMouseEventToGhostty (mouse down/up/right/other, move/enter/exit/drag, context menu, and scrollWheel).
    • Added tests for suppression toggling, reference counting, and the production predicate.

Written for commit af7ddb8. Summary will update on new commits. Review in cubic

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved handling of mouse events and terminal input during window drag operations. Terminal input is now properly suppressed while dragging windows, preventing interference with drag behavior.
    • Fixed mouse position tracking in terminal when exiting drag areas with active mouse buttons.
  • Tests

    • Added test coverage for terminal input suppression during window dragging operations.

Review Change Stack

@vercel

vercel Bot commented May 18, 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 May 19, 2026 7:41am
cmux-staging Building Building Preview, Comment May 19, 2026 7:41am

@coderabbitai

coderabbitai Bot commented May 18, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR implements reference-counted terminal input suppression during window move and drag operations. A new suppression system prevents mouse event forwarding to Ghostty while windows are being dragged, fixing spurious "CC" character insertion into terminal input. The suppression mechanism is integrated into drag handlers and guards mouse event forwarding conditionally.

Changes

Terminal Input Suppression During Window Drag

Layer / File(s) Summary
Terminal input suppression infrastructure
Sources/WindowDragHandleView.swift
New associated-object backing token and reference-counted depth tracking for terminal input suppression. Helper functions manage begin/end suppression state, read suppression depth, and expose predicates (shouldForwardTerminalMouseEventToGhostty, shouldForwardTerminalMousePositionToGhostty) that return false when suppression is active; a generic withWindowMoveTerminalInputSuppression scope wrapper increments/decrements depth around a closure.
Suppression integration in drag handlers
Sources/WindowDragHandleView.swift, Sources/Update/UpdateTitlebarAccessory.swift
Window drag execution in both WindowDragHandleView.mouseDown and titlebar gap drag view now wraps window.performDrag(with:) with withWindowMoveTerminalInputSuppression(window:) in addition to the existing temporary window-movability wrapper, ensuring mouse forwarding is suppressed during the drag operation.
Conditional mouse event forwarding to Ghostty
Sources/GhosttyTerminalView.swift
Mouse event handlers mouseUp, mouseMoved, mouseEntered, and mouseDragged now include early guards that skip Ghostty forwarding when shouldForwardTerminalMouseEventToGhostty(window:) returns false. mouseExited additionally clears Ghostty's cached mouse position only when NSEvent.pressedMouseButtons == 0; if buttons are pressed, it returns early without clearing.
Test coverage for suppression behavior
cmuxTests/WindowAndDragTests.swift
New test testTerminalMouseReportsAreSuppressedDuringExplicitWindowDrag asserts that shouldForwardTerminalMousePositionToGhostty returns false during suppression and true after suppression ends. Test testWindowMoveTerminalInputSuppressionIsReferenceCounted verifies suppression depth is reference-counted: forwarding remains disabled across nested beginWindowMoveTerminalInputSuppression calls and only re-enables after the final matching endWindowMoveTerminalInputSuppression call.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • manaflow-ai/cmux#2725: Both PRs modify mouse event forwarding to Ghostty in GhosttyTerminalView.swift—this PR gates forwarding via a suppression predicate, while that PR adjusts forwarded coordinates.
  • manaflow-ai/cmux#620: Both PRs modify window drag handling and terminal input suppression behavior in WindowDragHandleView.swift.

Poem

🐰 Drags without whispers of "C,"
Suppression counts deep, reference-free,
Mouse guards stand tall 'til the window rests,
Input flows clean through our terminal's tests! ✨


Caution

Pre-merge checks failed

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

  • Ignore

❌ Failed checks (1 error, 1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Cmux Swift Actor Isolation ❌ Error Six new functions in WindowDragHandleView.swift access NSWindow state without explicit @MainActor annotation. Functions modify window associated objects but lack required actor isolation markers. Add @MainActor to six suppression functions that modify window state via objc_setAssociatedObject/objc_getAssociatedObject.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description provides context about the issue and implementation, but lacks key structured sections required by the template. Provide a concise 'Summary' section (what/why), 'Testing' section (how tested/verified), and demo video if applicable. Complete the checklist items to confirm local testing, new tests, and bot review requests.
✅ Passed checks (13 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR directly addresses issue #4125 by suppressing terminal mouse reporting during window drags [#4125], preventing unwanted 'CC' characters from appearing.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the terminal input suppression issue during window drags. No unrelated modifications detected.
Cmux Swift Blocking Runtime ✅ Passed No blocking/timing synchronization. Uses non-blocking objc_setAssociatedObject for suppression state management. Complies with swift-blocking-runtime rules.
Cmux No Hacky Sleeps ✅ Passed Check not applicable: all changes are in Swift files. The rule applies to non-Swift runtime scripts. Swift timing issues are covered separately.
Cmux Swift Concurrency ✅ Passed All changes are synchronous. No DispatchQueue, Task, Combine, or completion handlers introduced. Uses ref-counted state management and defer-based scope wrapper, consistent with AppKit patterns.
Cmux Swift @Concurrent ✅ Passed All new functions are synchronous with no async/await, @concurrent annotations, or heavy operations crossing actor boundaries. Proper patterns for NSView event handlers.
Cmux Swift File And Package Boundaries ✅ Passed Files comply with boundary rules. GhosttyTerminalView-374L, UpdateTitlebarAccessory-7L. WindowDragHandleView+65L (threshold 250L). Focused AppKit glue code with coherent responsibility.
Cmux Swift Logging ✅ Passed No new logging statements (print/debugPrint/dump/NSLog/Logger) in production code. Changes are logic-only: guards, suppression helpers, and wrappers.
Cmux User-Facing Error Privacy ✅ Passed Internal implementation changes only: suppression predicates, wrappers, and tests. No user-facing errors, vendor names, credentials, or sensitive data exposed.
Cmux Swiftui State Layout ✅ Passed Uses AppKit objc_setAssociatedObject for window state, no new @Observable/@Published/@StateObject/@EnvironmentObject, no GeometryReader changes, no render mutations.
Cmux Architecture Rethink ✅ Passed Clean correctness fix for #4125 using per-window reference-counted suppression during drag. No timing patterns, locks, or duplicate owners. Clear invariant with comprehensive tests.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed No new standalone windows added. Changes only modify existing mouse handlers and add helper functions for terminal input suppression during window drags. Test-only fixtures are allowed by the rule.
Title check ✅ Passed The title accurately and concisely describes the main objective: fixing terminal input interference during window drags, which is the core problem addressed across all modified files.
✨ 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-4125-window-move-cc

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 18, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a terminal-input pollution bug where performing an explicit titlebar window drag could write mouse-report escape sequences into the shell prompt. It introduces a per-window, reference-counted suppression counter (via objc_setAssociatedObject on NSWindow) and gates all GhosttyNSView mouse handlers — including button down/up for all three buttons, move, enter, exit, drag, scroll, and context menu — behind shouldForwardTerminalMouseEventToGhostty.

  • WindowDragHandleView.swift adds beginWindowMoveTerminalInputSuppression / endWindowMoveTerminalInputSuppression / withWindowMoveTerminalInputSuppression, all correctly annotated @MainActor, mirroring the existing drag-suppression associated-object pattern.
  • GhosttyTerminalView.swift adds the suppression guard to every affected handler; mouseExited intentionally runs cursor-cleanup code before the guard so the cursor resets even during drags.
  • cmuxTests/WindowAndDragTests.swift adds reference-counting and toggle tests, but places them in FolderWindowMoveSuppressionTests, which is the only XCTestCase class in the file without @MainActor.

Confidence Score: 4/5

Safe to merge after confirming the missing @mainactor on the test class does not cause a compile error in this project's concurrency configuration.

The production suppression logic is well-scoped and correct. FolderWindowMoveSuppressionTests lacks @mainactor while every sibling test class in the file carries it, and the two new test methods call functions that require main-actor isolation. If the project's test target uses Swift 6 strict concurrency, those tests will fail to compile. The fix is a one-line annotation.

cmuxTests/WindowAndDragTests.swift — FolderWindowMoveSuppressionTests needs @mainactor to match all sibling test classes and to legally call the new @mainactor suppression functions.

Important Files Changed

Filename Overview
Sources/WindowDragHandleView.swift Adds per-window, reference-counted terminal-input suppression state via associated objects on NSWindow; all new helpers are correctly @mainactor, mirroring the existing drag-suppression pattern.
Sources/GhosttyTerminalView.swift Gates all terminal mouse handlers (down/up for left/right/other, move/entered/exited/dragged, scrollWheel, context menu) behind shouldForwardTerminalMouseEventToGhostty; mouseExited cursor cleanup correctly runs before the guard.
Sources/Update/UpdateTitlebarAccessory.swift Wraps window.performDrag(with:) in withWindowMoveTerminalInputSuppression in the titlebar accessory drag path, consistent with the same change in WindowDragHandleView.
cmuxTests/WindowAndDragTests.swift Adds two suppression tests to FolderWindowMoveSuppressionTests, which is not annotated @mainactor unlike every other XCTestCase class in this file; the new tests call @mainactor functions from the non-isolated context.

Sequence Diagram

sequenceDiagram
    participant User
    participant DragHandle as "DragHandle / TitlebarAccessory"
    participant Suppressor as withWindowMoveTerminalInputSuppression
    participant NSWindow
    participant GhosttyNSView

    User->>DragHandle: mouseDown on titlebar drag area
    DragHandle->>Suppressor: beginWindowMoveTerminalInputSuppression
    Note over Suppressor: depth becomes 1
    Suppressor->>NSWindow: performDrag nested event loop
    NSWindow-->>GhosttyNSView: mouseMoved / mouseDragged / mouseUp
    GhosttyNSView->>GhosttyNSView: shouldForwardTerminalMouseEventToGhostty returns false
    Note over GhosttyNSView: Events dropped, no bytes written to terminal
    NSWindow-->>DragHandle: performDrag returns
    DragHandle->>Suppressor: endWindowMoveTerminalInputSuppression via defer
    Note over Suppressor: depth back to 0
    Note over GhosttyNSView: Normal mouse reporting resumes
Loading

Comments Outside Diff (1)

  1. cmuxTests/WindowAndDragTests.swift, line 1597 (link)

    P1 FolderWindowMoveSuppressionTests is the only XCTestCase class in this file without @MainActor, yet the two new test methods call beginWindowMoveTerminalInputSuppression, endWindowMoveTerminalInputSuppression, windowMoveTerminalInputSuppressionDepth, and shouldForwardTerminalMouseEventToGhostty — all of which are marked @MainActor. Under Swift 6 strict concurrency these calls are a static isolation error. Every other test class in this file (WindowGlassEffectTests, WindowAccessorTests, DraggableFolderHitTests, etc.) carries @MainActor; FolderWindowMoveSuppressionTests should too.

Reviews (8): Last reviewed commit: "fix: suppress terminal scroll reports du..." | Re-trigger Greptile

Comment thread Sources/WindowDragHandleView.swift Outdated
Comment thread Sources/GhosttyTerminalView.swift Outdated
Comment thread Sources/WindowDragHandleView.swift Outdated

@cubic-dev-ai cubic-dev-ai 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.

No issues found across 4 files

Re-trigger cubic

@cursor cursor 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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit dfe70c8. Configure here.

Comment thread Sources/GhosttyTerminalView.swift
@austinywang

Copy link
Copy Markdown
Contributor Author

Greptile high-priority note checked: this is already satisfied at PR head af7ddb8. cmuxTests/WindowAndDragTests.swift has @MainActor immediately above final class FolderWindowMoveSuppressionTests: XCTestCase, so the new tests call the @MainActor suppression helpers from a main-actor-isolated test class.

@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 — af7ddb8f Deployed May 19, 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.

Window move/drag inserts two "C" characters into terminal input

3 participants