Add opt-in viewer input forwarding#17
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an opt-in “manual input forwarding” mode to the GPUI viewer, enabling the viewer UI to forward mouse/scroll/keyboard/paste into the isolated workspace (with explicit in-viewer confirmation), and plumbs the new capability through CLI, MCP tool params, registry, and documentation.
Changes:
- Introduces
--input-forwarding/input_forwardingflags and propagates them through viewer launch, registry identity, and server tool schema. - Implements coordinate mapping + event forwarding (click/drag/scroll/key/paste) with confirmation/arming UX and refresh bursts.
- Updates help/docs and expands tests for args parsing, coordinate mapping, layer-shell keyboard interactivity, and registry path behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/viewer.rs | Implements input-forwarding UI state machine, event forwarding, bounds tracking, registry identity changes, and tests. |
| src/server.rs | Adds input_forwarding to workspace_open_viewer tool params, wiring through to viewer::open_viewer, plus catalog/docs tests. |
| src/main.rs | Adds --input-forwarding CLI parsing, help text, and tests. |
| docs/gpui-viewer-direction.md | Documents the new input-forwarding behavior and reuse semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if !self.input_forwarding_allowed { | ||
| return; | ||
| } | ||
| self.focus_handle.focus(window, cx); | ||
| let Some(point) = self.input_forwarding_workspace_point(event.position) else { | ||
| return; | ||
| }; |
| fn schedule_input_refresh_burst(&mut self, cx: &mut Context<Self>) { | ||
| if !self.screen_stream { | ||
| return; | ||
| } | ||
|
|
||
| self.input_forwarding_burst_generation = | ||
| self.input_forwarding_burst_generation.wrapping_add(1); | ||
| let generation = self.input_forwarding_burst_generation; | ||
| self.request_refresh(cx, None); | ||
|
|
||
| let executor = cx.background_executor().clone(); | ||
| self._input_refresh_burst_task = Some(cx.spawn(async move |this, cx| { | ||
| for delay_ms in INPUT_FORWARD_REFRESH_BURST_DELAYS_MS { | ||
| executor.timer(Duration::from_millis(delay_ms)).await; | ||
| if this | ||
| .update(cx, move |viewer: &mut AgentWorkspaceViewer, cx| { | ||
| if viewer.input_forwarding_burst_generation != generation { | ||
| return; | ||
| } | ||
| if viewer.action_in_flight.is_none() && !viewer.interaction_active() { | ||
| viewer.request_refresh(cx, None); | ||
| } | ||
| }) | ||
| .is_err() | ||
| { | ||
| break; | ||
| } | ||
| } | ||
| })); | ||
| } |
| &mut self, | ||
| id: Option<&GlobalElementId>, | ||
| inspector_id: Option<&InspectorElementId>, | ||
| bounds: Bounds<Pixels>, | ||
| request_layout: &mut Self::RequestLayoutState, | ||
| window: &mut Window, | ||
| cx: &mut App, | ||
| ) -> Self::PrepaintState { | ||
| if let Ok(mut stored_bounds) = self.bounds.lock() { | ||
| *stored_bounds = Some(bounds); | ||
| } | ||
| self.div | ||
| .prepaint(id, inspector_id, bounds, request_layout, window, cx) | ||
| } |
| fn scroll_wheel_to_workspace_scroll( | ||
| delta: ScrollDelta, | ||
| ) -> Option<(workspace::ScrollDirection, u8)> { | ||
| let (x, y, unit) = match delta { | ||
| ScrollDelta::Pixels(point) => (point.x.as_f32(), point.y.as_f32(), 80.0), | ||
| ScrollDelta::Lines(point) => (point.x, point.y, 1.0), | ||
| }; |
| let amount = (magnitude / unit).ceil().clamp(1.0, 12.0) as u8; | ||
| Some((direction, amount)) | ||
| } |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces manual input forwarding to the GPUI viewer, allowing users to forward mouse clicks, drags, scrolls, keyboard inputs, and paste actions into the isolated workspace. It adds a new --input-forwarding CLI flag, updates the workspace_open_viewer tool, implements coordinate mapping, and adds a double-click confirmation toggle in the viewer UI. Feedback on these changes highlights several improvement opportunities: first, synchronous IPC calls in the input forwarding event handlers should be run asynchronously on a background thread to avoid blocking the main UI thread; second, the shift modifier should be excluded from the paste keystroke check to prevent intercepting standard terminal paste shortcuts like Ctrl+Shift+V; and third, cx.stop_propagation() should be called when a paste is attempted with an empty clipboard to prevent the keystroke from propagating to other GPUI elements.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| match workspace::move_pointer(&self.target_id, point.x, point.y) { | ||
| Ok(response) if response.ok => { | ||
| self.input_forwarding_drag = Some(InputForwardingDrag { | ||
| start: point, | ||
| start_position: event.position, | ||
| button, | ||
| }); | ||
| } | ||
| Ok(response) => self.record_input_forwarding_response(response), | ||
| Err(error) => self.record_input_forwarding_error(error), | ||
| } |
There was a problem hiding this comment.
Synchronous Blocking I/O on the UI Thread
The input forwarding event handlers (such as begin_input_forward, end_input_forward, forward_scroll, and forward_key_down) perform synchronous blocking Unix socket IPC calls (e.g., workspace::move_pointer, workspace::drag, workspace::click, workspace::scroll, workspace::paste_text, workspace::type_text, workspace::key) directly on the main/UI thread.
If the workspace daemon is slow, under load, or unresponsive, this will block the GPUI event loop, causing the viewer UI to freeze or stutter.
Recommendation
Consider running these IPC calls asynchronously on a background thread using cx.background_executor().spawn(...) or a sequential channel/queue to preserve input order without blocking the main UI thread.
| fn is_paste_keystroke(keystroke: &gpui::Keystroke) -> bool { | ||
| let key = keystroke.key.trim(); | ||
| (keystroke.modifiers.control || keystroke.modifiers.platform) | ||
| && !keystroke.modifiers.alt | ||
| && key.eq_ignore_ascii_case("v") | ||
| } |
There was a problem hiding this comment.
Shift Key Interception in Paste Keystroke
In is_paste_keystroke, the check does not exclude the shift modifier. This means Ctrl+Shift+V (or Cmd+Shift+V) will be intercepted as a host paste gesture.
However, in terminal emulators (which are common in these workspaces), Ctrl+Shift+V is the standard shortcut to paste from the workspace's internal clipboard. Intercepting it prevents the user from using the workspace's internal clipboard paste in terminals.
Recommendation
Exclude the shift modifier from the paste keystroke check to allow Ctrl+Shift+V to be forwarded to the workspace.
fn is_paste_keystroke(keystroke: &gpui::Keystroke) -> bool {
let key = keystroke.key.trim();
(keystroke.modifiers.control || keystroke.modifiers.platform)
&& !keystroke.modifiers.alt
&& !keystroke.modifiers.shift
&& key.eq_ignore_ascii_case("v")
}| _ => { | ||
| self.message = "Host clipboard has no text to paste".to_string(); | ||
| self.error = None; | ||
| cx.notify(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Missing cx.stop_propagation() on Empty Clipboard Paste
In forward_key_down, if is_paste_keystroke is true but the host clipboard is empty, the match arm returns early without calling cx.stop_propagation().
This allows the paste keystroke (e.g., Ctrl+V or Cmd+V) to propagate to other elements in the GPUI window, which could trigger unintended behaviors or text input in the viewer chrome.
Recommendation
Call cx.stop_propagation() before returning in the empty clipboard match arm.
_ => {
self.message = "Host clipboard has no text to paste".to_string();
self.error = None;
cx.stop_propagation();
cx.notify();
return;
}
|
@avifenesh I added this feature locally to help my agents handle human verification steps manually. Since it changes the viewer/interaction model, I'm not sure if this is something you want upstreamed. Let me know if you want it and I can address reviews and get it ready for merge. |
@lsaether Hi :) Thanks! |
|
hey @lsaether - checking in, is this still moving? happy to review once you take it out of draft |
Summary
Adds an explicit, opt-in path for manual human input forwarding in the GPUI viewer.
--input-forwardingviewer launch option and MCPworkspace_open_viewerparameter.Inputconfirmation before clicks, scroll, keyboard, and paste events are forwarded.click,drag,scroll,key,type_text,paste_text) so input still targets the isolated workspace rather than the host desktop.Checklist
cargo fmt --checkpassescargo clippy --locked -- -D warningspassescargo test --lockedpassesgit diff --checkis clean (no trailing whitespace / conflict markers)scripts/integration_smoke.shrun for changes touching runtime behavior (MCP tools, permissions, workspace control, viewer, browser) — attempted locally; see notesNotes for reviewers
Evidence
Local quality gates:
Additional checks:
Integration smoke was attempted locally after satisfying the missing
xmessagedependency. It passed the MCP/viewer/terminal/local-only stages, then stopped at the disabled-network DNS assertion. Direct network access was blocked, but DNS resolution still returned in this local environment, so I am treating that as an environment-specific smoke failure rather than a regression from this input-forwarding change.Isolation / permission boundary
The new viewer input path is intentionally explicit:
active/read_only/paused) remains part of the forwarding readiness check;Known behavior intentionally preserved
Forwarded input schedules short screenshot refresh bursts so the viewer catches up after clicks/typing. That can make the existing viewer refresh status (for example,
Refreshing workspace snapshot...) briefly visible/flickery. This PR intentionally preserves the original refresh/status behavior instead of adding an opinionated chrome-smoothing change. If a quieter viewer chrome is desired, that can be addressed as a separate follow-up contribution.