Repository navigation
Forward mouse input to TUI applications - #7759
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Ghostty mouse encoding, asynchronous PTY event dispatch, and PTY-pane forwarding for clicks, drags, motion, and wheel events while preserving Shift-based selection behavior. Updates mutation barriers, tests, usage text, mouse documentation, and plugin input specifications. ChangesPTY mouse passthrough
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant MouseEncoder
participant PtyInputDispatcher
participant PTY
User->>App: Mouse input
App->>MouseEncoder: Encode MouseInput
MouseEncoder->>App: Ghostty mouse bytes
App->>PtyInputDispatcher: Enqueue PTY event
PtyInputDispatcher->>PTY: Write encoded bytes
Possibly related PRs
🚥 Pre-merge checks | ✅ 24 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (24 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Greptile SummaryThis PR forwards mouse input through the TUI PTY path and updates the related protocol surfaces. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (97): Last reviewed commit: "Always deliver the paired mouse release" | Re-trigger Greptile |
| sys::ghostty_mouse_encoder_setopt( | ||
| self.encoder, | ||
| sys::GHOSTTY_MOUSE_ENCODER_OPT_ANY_BUTTON_PRESSED, | ||
| &input.any_button_pressed as *const _ as *const c_void, | ||
| ); |
There was a problem hiding this comment.
Wheel forwarding passes any_button_pressed through this encoder option, and the app sends true for plain ScrollUp and ScrollDown events. If Ghostty applies this flag to wheel buttons the same way it applies it to motion, a normal wheel tick can be encoded as a button-held wheel event, so mouse-aware TUI apps can misread ordinary scrolling as a drag-wheel input.
There was a problem hiding this comment.
Fixed in 8b3de53. Wheel events now pass any_button_pressed=false; that flag is reserved for active-button motion outside the viewport.
— Claude Code
| (x as f32 - content.x as f32 + 0.5) * cell_width as f32, | ||
| (y as f32 - content.y as f32 + 0.5) * cell_height as f32, |
There was a problem hiding this comment.
Drag::PtyMouse stores the content rect from the press and every later drag/release reuses it for coordinate conversion. If the pane is resized or the sidebar layout changes before release, the pointer is encoded against the old rect, so the TUI app can receive drag or release coordinates for the wrong cell.
There was a problem hiding this comment.
Fixed in 8b3de53. Drag and release coordinate conversion now resolves the current pane content rect by surface, with a resize-during-drag regression assertion.
— Claude Code
| } | ||
| } | ||
| MouseEventKind::Up(MouseButton::Left) => { | ||
| if self.finish_pty_mouse_drag(mouse.column, mouse.row, mouse.modifiers) { |
There was a problem hiding this comment.
Match Active Button This release path can end the PTY drag for the wrong physical button. If right down starts
Drag::PtyMouse, then left down is ignored by the new guard, but left up still calls finish_pty_mouse_drag; that sends a right-button release and clears self.drag while the right button is still held. The inner TUI sees the drag end early, and later motion or the actual right-up is no longer forwarded as part of that drag. Only the release for the stored active button should finish the PTY mouse drag.
There was a problem hiding this comment.
Fixed in 71fc0ae. The drag tracks secondary presses separately, so a secondary release no longer ends the active button; the actual active-button release is required.
— Claude Code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b3de53bc9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// Queue an event without blocking the UI. A wedged remote endpoint may | ||
| /// fill the bounded queue, in which case new input is dropped. | ||
| pub fn enqueue(&self, event: PtyMouseInputEvent) { | ||
| let _ = self.tx.try_send(event); |
There was a problem hiding this comment.
Preserve release events when the mouse queue fills
For remote sessions, a slow or blocked control socket can fill this 512-slot channel with motion events while the worker is stuck in RemoteSession::send_bytes; because try_send drops every new event, the subsequent button release or wheel event can be discarded too. That leaves mouse-aware TUIs seeing a press/drag with no release even after the socket catches up, so only coalescible motion should be dropped or releases should be prioritized.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 71fc0ae. The bounded queue now evicts stale motion for ordered events and prioritizes releases under saturation. A regression test fills the queue and verifies the release displaces motion.
— Claude Code
| } | ||
| } | ||
| MouseEventKind::Up(MouseButton::Left) => { | ||
| if self.finish_pty_mouse_drag(mouse.column, mouse.row, mouse.modifiers) { |
There was a problem hiding this comment.
Ignore releases from non-active mouse buttons
When a PTY drag is active, extra Down events are ignored, but any Up branch still calls finish_pty_mouse_drag, which always releases the stored active button. If a user presses right in a mouse-aware TUI, then taps/releases left before releasing right, this left release sends a right-button release and clears self.drag; the real right release is then no longer forwarded, leaving the inner app's button state out of sync.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 71fc0ae. Secondary down/up pairs are tracked and consumed independently from the active PTY drag, while ambiguous legacy releases remain supported when there was no observed secondary press.
— Claude Code
| if reported_button != active_button && ignored_buttons & reported_bit != 0 { | ||
| if let Some(Drag::PtyMouse { ignored_buttons, .. }) = &mut self.drag { | ||
| *ignored_buttons &= !reported_bit; | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
A release for a non-active button can still end the active PTY drag when its matching press was not recorded in ignored_buttons. For example, if a right-button PTY drag is active and a left-button Up arrives after its Down was missed, this branch falls through, clears self.drag, and sends a right-button release to the inner app. That leaves the real right-button drag stopped early. Only the stored active button should finish the PTY drag; other button releases should be consumed after clearing any recorded ignored bit.
| if reported_button != active_button && ignored_buttons & reported_bit != 0 { | |
| if let Some(Drag::PtyMouse { ignored_buttons, .. }) = &mut self.drag { | |
| *ignored_buttons &= !reported_bit; | |
| } | |
| return true; | |
| } | |
| if reported_button != active_button { | |
| if let Some(Drag::PtyMouse { ignored_buttons, .. }) = &mut self.drag { | |
| *ignored_buttons &= !reported_bit; | |
| } | |
| return true; | |
| } |
There was a problem hiding this comment.
Fixed in 982b45a. Only the stored active button can finish the PTY drag; every non-active release is consumed without relabeling or clearing it. The host requests SGR mouse reporting, which preserves release identity.
— Claude Code
| if let Some(index) = events.iter().position(|queued| queued.kind == PtyInputKind::Motion) { | ||
| events.remove(index); | ||
| } else { | ||
| return false; |
There was a problem hiding this comment.
When the remote input worker is blocked and the queue reaches QUEUE_CAPACITY with only ordered input and prior releases, this branch returns false because there is no motion event to evict. finish_pty_mouse_drag and cancel_pty_mouse_drag ignore that enqueue failure, so a remote PTY app can receive the mouse press but never receive the release. That leaves the app in a stuck button state even though releases are meant to be protected by the reserve.
There was a problem hiding this comment.
Fixed in ef5473d. Every accepted mouse press now increments an exact release reservation; its matching release consumes that reservation, so capacity is guaranteed without evicting ordered events.
— Claude Code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5939e784f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if reported_button != active_button { | ||
| return true; |
There was a problem hiding this comment.
Handle ambiguous mouse releases during PTY drags
On terminals where crossterm cannot identify the button for MouseEventKind::Up (its docs say it returns MouseButton::Left in that case), releasing a right- or middle-button PTY drag reaches this branch and is consumed without sending the stored button's release or clearing self.drag. That leaves the inner mouse-aware app with a stuck button and cmux keeps ignoring new mouse downs until focus loss or another cancellation path; treat an unmatched Left release as ambiguous when no secondary button was observed.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
| }; | ||
| let (surface, handle, reservation_id, fallback, content, button) = | ||
| (*surface, handle.clone(), *reservation_id, release_bytes.clone(), *content, *button); | ||
| if reported_button != button && self.ignored_pty_mouse_buttons.remove(&reported_button) { |
There was a problem hiding this comment.
This guard still lets an untracked non-active release finish the active PTY drag. If a secondary button was pressed before the drag started, or its Down was missed, remove(&reported_button) returns false and the code falls through to send the stored active button's release. The inner TUI can see the active drag end while the physical active button is still held. Reject every reported_button != button before encoding the active release.
| PtyMouseReleaseCapture::NotReported => { | ||
| self.pty_input.cancel_release_reservation(reservation_id); |
There was a problem hiding this comment.
This branch still drops the stored release after an accepted press. Drag::PtyMouse stores press-time release bytes, but mouse-up re-encodes against the terminal's current mouse mode. If the TUI disables or changes mouse reporting while the button is held, capture_pty_mouse_release can return NotReported; this branch then cancels the reservation without sending the stored bytes. The PTY app can receive button-down without button-up and remain stuck in a drag or selection state. The cancellation path needs the same fallback behavior.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30fa9c9808
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if reported_button != button && self.ignored_pty_mouse_buttons.remove(&reported_button) { | ||
| return true; |
There was a problem hiding this comment.
Handle ambiguous secondary mouse releases
Fresh evidence: this version records secondary Down events, but the release gate still keys only off reported_button. On terminals where crossterm reports an unknown Up button as Left, releasing a secondary right/middle button during an active PTY drag arrives as Up(Left); when Left is the active button or is not the recorded secondary, this falls through, sends the active button's release, and clears self.drag. The real active release is then no longer forwarded, so mouse-aware apps can end up with mismatched button state; ambiguous Left releases should be consumed while any secondary button is outstanding.
Useful? React with 👍 / 👎.
| if reported_button != button && self.ignored_pty_mouse_buttons.remove(&reported_button) { | ||
| return true; |
There was a problem hiding this comment.
Reject Mismatched Releases This still lets an untracked non-active button-up finish the active PTY drag. The guard only consumes a mismatched release when that button was previously recorded in
ignored_pty_mouse_buttons; if the secondary button was already down before the PTY drag started, its Down was missed, or the host only reports the Up, remove(&reported_button) returns false and execution falls through to encode the stored active button's release. That clears self.drag while the physical active button can still be held, so the inner TUI sees its drag or selection end early and the real active-button release is no longer forwarded. Every reported_button != button release should be consumed without sending the active release; the ignored set should only be bookkeeping.
| PtyMouseReleaseCapture::NotReported => { | ||
| self.pty_input.cancel_release_reservation(reservation_id); | ||
| } |
There was a problem hiding this comment.
Send Stored Release This
NotReported branch still drops the stored fallback release after an accepted press. A Drag::PtyMouse is only created after press-time release_bytes were captured and the press was queued with a reservation, but mouse-up re-encodes against the terminal's current mouse mode. If the TUI disables or changes mouse reporting while the button is held, capture_pty_mouse_release can return NotReported here; cancelling the reservation without enqueueing fallback leaves the PTY app with a button-down and no matching button-up. The cancel path below has the same NotReported behavior, so both paths need to send the stored fallback release before cancelling the reservation.
| if reported_button != button && self.ignored_pty_mouse_buttons.remove(&reported_button) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This still only consumes a different button's release when that button was recorded in ignored_pty_mouse_buttons. If a left-up arrives while a right-button PTY drag is active, but the left-down was missed or happened outside this path, remove(&reported_button) returns false and the code falls through. That sends a right-button release and clears self.drag while the physical right button is still held, so the TUI can end its drag early and the real right-up is no longer forwarded.
| if reported_button != button && self.ignored_pty_mouse_buttons.remove(&reported_button) { | |
| return true; | |
| } | |
| if reported_button != button { | |
| self.ignored_pty_mouse_buttons.remove(&reported_button); | |
| return true; | |
| } |
| PtyMouseReleaseCapture::NotReported => { | ||
| self.pty_input.cancel_release_reservation(reservation_id); |
There was a problem hiding this comment.
This NotReported branch still drops the stored fallback release after a press has already been accepted. A Drag::PtyMouse is only created after press-time release_bytes were captured and the press was queued with a release reservation, but mouse-up re-encodes against the terminal's current mouse mode. If the TUI disables or changes mouse reporting while the button is held, capture_pty_mouse_release() can return NotReported; cancelling the reservation without enqueueing the stored fallback leaves the PTY app with a button-down and no matching button-up. The cancellation path has the same branch, so focus loss or layout changes can hit the same missing release.
| if reported_button != button && self.ignored_pty_mouse_buttons.remove(&reported_button) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Reject Mismatched Releases
This still lets a non-active Up finish the active PTY drag when that button was not recorded in ignored_pty_mouse_buttons. The guard only returns early for mismatched releases that had a prior in-drag Down; if a secondary button was already down before the PTY drag started, its Down was missed, or the host reports only the Up, remove(&reported_button) returns false and this falls through to encode the stored active button's release and clear self.drag. That can end the inner TUI drag while the physical active button is still held, and the real active-button release will no longer be forwarded. Every reported_button != button release should be consumed without sending the active release.
| if reported_button != button && self.ignored_pty_mouse_buttons.remove(&reported_button) { | |
| return true; | |
| } | |
| if reported_button != button { | |
| self.ignored_pty_mouse_buttons.remove(&reported_button); | |
| return true; | |
| } |
| PtyMouseReleaseCapture::NotReported => { | ||
| self.pty_input.cancel_release_reservation(reservation_id); | ||
| } |
There was a problem hiding this comment.
Send Stored Release
This NotReported branch still drops the stored fallback release after an accepted press. A Drag::PtyMouse is only created after press-time release_bytes were captured, but mouse-up re-encodes against the terminal's current mouse mode. If the TUI disables or changes mouse reporting while the button is held, capture_pty_mouse_release() can return NotReported here. Canceling the reservation without enqueueing the stored fallback leaves the PTY app with a button-down and no matching button-up, so selections or drags can remain stuck after cmux clears local drag state.
|
|
||
| fn enqueue_pty_release( | ||
|
|
There was a problem hiding this comment.
Cancel With Fallback
The cancel path can drop the same stored release. Focus loss, prompts, menus, or mutation barriers can call cancel_pty_mouse_drag after the PTY already received the press; if the app has since disabled or changed mouse reporting, the fresh release capture returns NotReported and this branch cancels the reservation without enqueueing the stored press-time fallback bytes. The inner TUI can remain stuck in a pressed or dragging state even though cmux clears its local drag.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c109013b6b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return this.attachOnlyEvent(event.event) | ||
| || event.event === "scroll-changed" | ||
| || this.isSurfaceOverflow(event, surface); |
There was a problem hiding this comment.
Route browser attach events on shared transports
When a TypeScript client uses the default shared transport, attachSurface() only delivers events accepted by this predicate. Protocol v7 now has the server send browser-state as the initial attach event and frame updates for browser surfaces, but neither name is accepted here, so those events are discarded before the stream sees them; the attach request then resolves after the ok response and next() just times out. Add the browser attach event names (and frame decoding/types) to this path so the advertised browser attach stream works without requiring a dedicated transport.
Useful? React with 👍 / 👎.
| if reported_button != button && self.ignored_pty_mouse_buttons.remove(&reported_button) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
This still lets an untracked non-active button-up finish the active PTY drag. The guard only consumes a mismatched release when that button was recorded in ignored_pty_mouse_buttons; if the secondary button was already down before the PTY drag started, its Down was missed, or the host reports only the Up, remove(&reported_button) returns false and execution falls through to encode the stored active button's release. That clears self.drag while the active button can still be held, so the inner TUI sees its drag or selection end early and the real active-button release is no longer forwarded.
| PtyMouseReleaseCapture::NotReported => { | ||
| self.pty_input.cancel_release_reservation(reservation_id); | ||
| } |
There was a problem hiding this comment.
This branch still drops the stored release after an accepted press. A Drag::PtyMouse is only created after press-time release bytes were captured, but mouse-up re-encodes against the terminal's current mouse mode. If the TUI disables or changes mouse reporting while the button is held, capture_pty_mouse_release() can return NotReported; canceling the reservation without enqueueing the stored fallback leaves the PTY app with a button-down and no matching button-up, so selections or drags can remain stuck after cmux clears local drag state.
| } | ||
|
|
||
| fn enque |
There was a problem hiding this comment.
The cancellation path can drop the same stored release. Focus loss, prompts, menus, or mutation barriers can call cancel_pty_mouse_drag after the PTY already received the press; if the app has since disabled or changed mouse reporting, the fresh release capture returns NotReported and this branch cancels the reservation without enqueueing the stored press-time fallback bytes. The inner TUI can remain stuck in a pressed or dragging state even though cmux clears its local drag.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 594215867d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cols: number; | ||
| rows: number; | ||
| } | ||
| export interface ResizeSurfaceResult { accepted: boolean } |
There was a problem hiding this comment.
Make accepted optional for legacy resize responses
When this SDK is used against a protocol 5/6 server, resize-surface still returns an empty {} data object; client.resizeSurface() compensates with result.accepted ?? true, but the generic request("resize-surface", ...) API infers this interface and tells callers accepted is always a boolean. Code using the typed generic request can therefore branch on undefined at runtime without any TypeScript warning; model accepted as optional or normalize the generic response too.
Useful? React with 👍 / 👎.
| PtyMouseReleaseCapture::NotReported => { | ||
| self.pty_input.cancel_release_reservation(reservation_id); |
There was a problem hiding this comment.
This branch still drops the paired release when the PTY stops reporting mouse before button-up. A Drag::PtyMouse only exists after press-time release_bytes were captured and the press was accepted with a reservation, but capture_pty_mouse_release() re-encodes against the terminal's current mouse mode. If the TUI disables or changes mouse reporting while the button is held, this path cancels the reservation without enqueueing the stored fallback, so the PTY app can receive a button-down without the matching button-up and keep a selection or drag stuck. The cancel path below has the same NotReported handling, so both paths need to enqueue the stored fallback before canceling the reservation.
| PtyMouseReleaseCapture::NotReported => { | ||
| self.pty_input.cancel_release_reservation(reservation_id); | ||
| } |
There was a problem hiding this comment.
When a PTY app accepts a mouse press and then disables or changes mouse reporting before mouse-up, capture_pty_mouse_release can return NotReported. At this point the drag already has the press-time release_bytes, but this branch only cancels the reservation. The inner app can receive the button-down without the matching button-up and stay stuck in a drag or selection state after cmux clears its local drag. The cancel path has the same branch, so focus loss, prompts, menus, or layout changes can hit the same missing release.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3869725102
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| protocol = &info.Protocol | ||
| } | ||
| if *protocol > 6 || (*protocol > 5 && !c.allowProtocolV6Attach) { | ||
| if *protocol > 7 || (*protocol > 5 && !c.allowProtocolV6Attach) { |
There was a problem hiding this comment.
Allow zero-value Go clients to attach to v7 servers
With the server now advertising protocol 7, callers using the normal zero-value Options path leave allowProtocolV6Attach false, so AttachSurface rejects every current server here before opening even PTY attach streams. The Go binding now parses the newer attach events, and the e2e only passes because it explicitly opts in; default Go clients should accept v7 or default this option to true.
Useful? React with 👍 / 👎.
| protocol = self._protocol if self._protocol is not None else self.identify().protocol | ||
| if protocol > 7: | ||
| raise ProtocolError(f"unsupported protocol {protocol}; maximum supported is 7") | ||
| if protocol > 5 and not self.allow_protocol_v6_attach: |
There was a problem hiding this comment.
Allow default Python clients to attach to v7 servers
With protocol 7 now reported by current servers, CmuxClient() still defaults allow_protocol_v6_attach to false, so attach_surface() raises here for every default Python client unless callers know to pass a legacy-named opt-in. The parser now handles the newer resized replay/overflow fields and the Python e2e only succeeds because it opts in explicitly; default clients should accept v7 attach streams out of the box.
Useful? React with 👍 / 👎.
| | Attach stream v6 | `attach-surface` command | `vt-state`, `resized`, `output`, `colors-changed`, `scroll-changed`, `detached` | | ||
| | Subscribe stream | `subscribe` command | `tree-changed`, `layout-changed`, `surface-output`, `scroll-changed`, `surface-resized`, `surface-resize-failed`, `surface-exited`, `title-changed`, `bell`, `notification`, `config-reload-requested`, `window-title-requested`, `client-attached`, `client-changed`, `client-detached`, `empty`, `overflow` | | ||
| | Attach stream v5 | `attach-surface` command | `vt-state`, `output`, `detached`, `overflow` | | ||
| | Attach stream v6 | `attach-surface` command | `vt-state`, `resized`, `output`, `colors-changed`, `scroll-changed`, `detached`, `overflow` | |
There was a problem hiding this comment.
Document browser attach events in the v7 spec
This table is now the protocol-v7 event contract, but the attach stream row is still PTY-only. The server sends browser-state as the initial attach event and frame updates for browser surfaces, so bindings or conformance checks generated from this spec will continue to omit or drop the browser attach stream; add a v7 attach entry or include those event names here.
Useful? React with 👍 / 👎.
| PtyMouseReleaseCapture::NotReported => { | ||
| self.pty_input.cancel_release_reservation(reservation_id); | ||
| } |
There was a problem hiding this comment.
When a PTY app accepts a mouse press and then disables or changes mouse reporting before mouse-up, capture_pty_mouse_release can return NotReported. This branch then cancels the release reservation without sending the stored fallback bytes captured when the drag started. The inner app has already received the button-down, so it can stay in a pressed or dragging state after cmux clears local drag state. The cancellation path has the same NotReported behavior for focus loss, menus, and layout changes, so both paths need to enqueue the stored fallback release before cancelling the reservation.
|
@codex review |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
Summary
Testing
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --locked -- -D warningscargo test --workspace --lockedpython3 scripts/smoke-tui.pyNotes
python3 scripts/smoke-attach.pyreaches the second attach, then fails its output-quiescence assertion; the same failure reproduces on unchangedorigin/mainNeed help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Touches core mux event delivery and a breaking-ish protocol bump (v7 wire shapes), which affects every attach/subscribe client; coalescing and overflow disconnect change observable event ordering for slow consumers.
Overview
Bumps the control socket to protocol v7 and replaces the mux’s simple broadcast channel with a per-subscriber event bus that coalesces high-churn
title-changedandsurface-outputper surface, caps pending work (~4k slots), and disconnects slow readers with anoverflowevent (subscribe vs attach-scoped variants).Wire/API changes:
title-changednow includes the authoritativetitlestring; attachresizedevents use areplayfield (bindings still accept legacy v6data). PTY/browser paths emitMuxEvent::TitleChanged { surface, title }instead of surface-only IDs.Runtime hardening: browser resize is serialized (
resize_transaction), state is committed after CDP round-trips without clearing frames delivered mid-resize, andSurface::resizepropagates errors viaanyhow::Result. Conformance runner gains attach streams, title/replay fixtures, and overflow parsing; all language bindings and e2e checks are updated for protocol ≤7.Reviewed by Cursor Bugbot for commit eeb7e82. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Forwards mouse input to PTY apps and bumps the control socket to protocol v7 with a bounded, per-subscriber event bus. Improves attach, resize, and recovery for local, remote, and web clients with authoritative titles, scoped overflow, and safer, retried browser resizes.
New Features
ghostty-vt::MouseEncoder/MouseEncoders(X10/UTF‑8/SGR/URxvt; SGR‑pixel) on an ordered, off-loop input lane with motion coalescing, backpressure, reserved release bytes, and remote release retries; preserves ordering across local/remote and keeps cmux selection/menus behind Shift.title-changed/surface-output; slow consumers receiveoverflow { error, scope, surface? }and only the affected stream closes; shared transports can multiplex attach plus one subscription (extra subs require astreamTransportFactory).title-changed { surface, title }; attachresized { replay }(v6datastill accepted);resize-surface -> { accepted, reservation_id? }(legacy empty result defaults toaccepted: true);surface-resized { ..., reservation_id? }; newsurface-resize-failed { surface, cols, rows, error, retry_after_ms?, reservation_id? }.TRANSPORT_SAFE_CAPTURE_MEGAPIXELS(2.0 MP) withbrowser.capture_scaleas a max; web requires v7 and recovers attach overflow with bounded retries; SDKs reconcile authoritative titles and treat overflow as terminal per stream.Bug Fixes
Written for commit f19ead0. Summary will update on new commits.
Summary by CodeRabbit