From 860846db6d13dd52a4f8faf8c3bb20fe758f0805 Mon Sep 17 00:00:00 2001 From: Edwin Date: Wed, 19 Aug 2026 09:32:58 -0700 Subject: [PATCH 1/2] fix tui modal input precedence over playbook --- crates/cli/src/app.rs | 315 ++++++++++++++++++--- crates/cli/src/ui.rs | 17 +- crates/e2e/tests/tui_smoke.rs | 122 ++++++++ specs/0205-tui-topmost-modal-owns-input.md | 30 ++ 4 files changed, 438 insertions(+), 46 deletions(-) create mode 100644 specs/0205-tui-topmost-modal-owns-input.md diff --git a/crates/cli/src/app.rs b/crates/cli/src/app.rs index 979ea42e..8955c5d0 100644 --- a/crates/cli/src/app.rs +++ b/crates/cli/src/app.rs @@ -4231,6 +4231,16 @@ impl OperatorChannelRowHit { } /// Last-frame geometry for hit-testing mouse clicks. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ModalOwner { + Playbook, + Tasks, + RemoteControl, + Help, + SessionPicker, + Configure, +} + #[derive(Debug, Clone, Default)] pub struct LayoutSnapshot { pub list_area: Option, @@ -4329,6 +4339,10 @@ pub struct LayoutSnapshot { /// Mouse clicks outside this rect dismiss the modal instead of /// falling through to panes underneath it. pub modal_area: Option, + /// Identity of the surface that wrote `modal_area` last. Input routing + /// must follow the same topmost order as rendering; checking whether an + /// underlying modal merely exists is insufficient when modals overlap. + pub modal_owner: Option, /// Clickable controls inside the `/remote-control` dialog (tunnel-type /// buttons, `[ back ]`/`[ stop ]`, and the `Enter`/`Esc`/`o` key hints). /// Populated by `render_remote_control_popup`; empty whenever the dialog @@ -4776,6 +4790,7 @@ impl LayoutSnapshot { prompt_turn_hits: _, remote_control_hits: _, configure_tab_hits: _, + modal_owner: _, // Consumed inside the main block's own render pass (or carries no // geometry at all), so it stays in main-block coordinates. frame_area: _, @@ -7154,6 +7169,33 @@ impl App { return; } + // True topmost modals get the same precedence for terminal paste + // events as for individual keys. The session picker owns paste as its + // typeahead; configure's auto-open semantics close and reroute the + // unclaimed text; Help closes and consumes; remote control consumes + // the paste (and its name form accepts the same valid characters as + // ordinary key events). + if self.session_picker_insert_text(&text) { + return; + } + if self.configure_popup.is_some() { + self.configure_popup = None; + } + if self.help_visible { + self.help_visible = false; + return; + } + if self.remote_control_popup.is_some() { + for ch in text.chars() { + self.handle_remote_control_key(KeyEvent::new( + KeyCode::Char(ch), + KeyModifiers::NONE, + )) + .await; + } + return; + } + // An inline title edit has the same modal precedence for terminal // paste events as it does for ordinary keys. Consume the whole paste // at the edit cursor instead of leaking it into the pane's PTY. @@ -7170,13 +7212,6 @@ impl App { return; } - // The session picker owns paste events at the same precedence as - // ordinary key events. Without this, `C-x b` accepted typed text in - // its search line but sent pasted text to the previously focused PTY. - if self.session_picker_insert_text(&text) { - return; - } - // Mirror the keystroke routing precedence (see `on_key`): pasted // text lands in the playbook only when no minibuffer/palette overlay is // capturing input *and* the view pane holds focus. With an overlay open, @@ -11501,10 +11536,38 @@ impl App { if self.configure_click_tab(ev.column, ev.row) { return; } + // The popup closed and this same click must continue + // through ordinary routing (spec 0069). Clear last-frame + // modal ownership so the stale rectangle cannot consume + // the re-dispatched click below. + self.layout.modal_area = None; + self.layout.modal_owner = None; } _ => return, } } + // A rendered modal above the Playbook owns pointer input before any + // pane-level hit test or drag can start. Mouse-up is where ordinary + // clicks dispatch; consume the matching down event so it cannot focus, + // rename, resize, or select text on the covered surface. The session + // picker remains keyboard-only by design (spec 0063), so it consumes + // every mouse event without acting on it. + match self.layout.modal_owner { + Some(ModalOwner::RemoteControl) if self.remote_control_popup.is_some() => { + if matches!(ev.kind, MouseEventKind::Up(MouseButton::Left)) { + self.handle_left_click(ev.column, ev.row).await; + } + return; + } + Some(ModalOwner::Tasks) if self.tasks_popup.is_some() => { + if matches!(ev.kind, MouseEventKind::Up(MouseButton::Left)) { + self.handle_left_click(ev.column, ev.row).await; + } + return; + } + Some(ModalOwner::SessionPicker) if self.session_picker_active() => return, + _ => {} + } // The harness completion menu is painted above the underlying panes, // so vertical wheel events within its bounds navigate its highlight // instead of scrolling the obscured surface below. @@ -11589,7 +11652,11 @@ impl App { && self.dragging_lineage_scrollbar.is_none() && self.text_selection.is_none() && self.mouse_over_tutorial_card(ev.column, ev.row); - if !card_owns_event && self.handle_playbook_mouse(&ev).await { + let playbook_is_top_modal = self.layout.modal_owner == Some(ModalOwner::Playbook) + || (self.layout.modal_owner.is_none() + && self.layout.modal_area.is_some() + && self.playbook_popup.is_some()); + if !card_owns_event && playbook_is_top_modal && self.handle_playbook_mouse(&ev).await { return; } // URL clicks must be intercepted before the child-mouse-forward path so @@ -12186,7 +12253,14 @@ impl App { return; } if let Some(modal) = self.layout.modal_area { - if self.playbook_popup.is_some() { + let owner = self.layout.modal_owner.or_else(|| { + // Compatibility for tests and the brief pre-first-frame state + // that fabricate only the historical `modal_area` field. + self.playbook_popup + .is_some() + .then_some(ModalOwner::Playbook) + }); + if owner == Some(ModalOwner::Playbook) { if contains(modal, col, row) { self.place_playbook_cursor(modal, col, row); return; @@ -12197,24 +12271,20 @@ impl App { // a session / focuses a pane; the playbook then follows the new // selection (the prior playbook is stashed, not destroyed) via // sync_playbook_popup_with_selection. + } else if owner == Some(ModalOwner::SessionPicker) { + // Keyboard-only in v1 (spec 0063): the topmost dialog still + // blocks clicks from reaching the Playbook or panes below. + return; } else if !contains(modal, col, row) { self.dismiss_modal(); return; } else { - if let Some(dialog) = self.operator_dialog.as_mut() { - // The definition rows start immediately inside the - // one-cell border. Clicking a row selects it; keyboard - // typing/cycling then edits that field. - let field = row.saturating_sub(modal.y.saturating_add(1)) as usize; - if field < OPERATOR_FIELD_COUNT { - dialog.focus = OperatorDialogFocus::Field(field); - } - return; - } // The /remote-control dialog is the one informational modal // with live controls: dispatch a click that lands on one of // its registered buttons / key hints before the swallow below. - if self.remote_control_popup.is_some() { + if owner == Some(ModalOwner::RemoteControl) + && self.remote_control_popup.is_some() + { if let Some(action) = self .layout .remote_control_hits @@ -12505,24 +12575,27 @@ impl App { // The operator editor is deliberately absent here: it is the operator // view itself, not a modal over it, and Esc inside it reverts edits // rather than closing anything (spec 0175). - if self.configure_popup.take().is_some() { - return; - } - if self.playbook_popup.is_some() { - // Remember caret + scroll so reopening restores them, matching the - // toggle-close path. - self.remember_playbook_view_state(); - self.playbook_popup = None; - return; - } - if self.tasks_popup.take().is_some() { - return; - } - if self.remote_control_popup.is_some() { - self.close_remote_control_popup(); - return; + match self.layout.modal_owner { + Some(ModalOwner::Configure) => self.configure_popup = None, + Some(ModalOwner::Playbook) => { + // Remember caret + scroll so reopening restores them, matching + // the toggle-close path. + self.remember_playbook_view_state(); + self.playbook_popup = None; + } + Some(ModalOwner::Tasks) => self.tasks_popup = None, + Some(ModalOwner::RemoteControl) => self.close_remote_control_popup(), + Some(ModalOwner::Help) => self.help_visible = false, + Some(ModalOwner::SessionPicker) => {} + None if self.configure_popup.take().is_some() => {} + None if self.remote_control_popup.is_some() => self.close_remote_control_popup(), + None if self.tasks_popup.take().is_some() => {} + None if self.playbook_popup.is_some() => { + self.remember_playbook_view_state(); + self.playbook_popup = None; + } + None => self.help_visible = false, } - self.help_visible = false; } pub fn hovered_url(&self) -> Option { @@ -13103,6 +13176,14 @@ impl App { if self.configure_popup.is_some() && self.handle_configure_key(key).await { return; } + // The remote-control dialog is rendered above pane surfaces and owns + // every key, including keys it does not use. Keep it in the same + // precedence tier as the other explicit topmost dialogs, before + // lineage, tasks fallthrough, pinned terminals, and the Playbook. + if self.remote_control_popup.is_some() { + self.handle_remote_control_key(key).await; + return; + } // The sidebar's lineage section, keyboard-focused (bare `Tab` from // the list pane — spec 0081): owns // navigation/merge/discard/jump keys while focused; anything else @@ -13161,13 +13242,6 @@ impl App { self.handle_playbook_key(key).await; return; } - // /remote-control modal: arrow keys pick a tunnel provider, - // Enter starts it, Esc closes. Every key is consumed while it's - // open — see `handle_remote_control_key`. - if self.remote_control_popup.is_some() { - self.handle_remote_control_key(key).await; - return; - } // Prompt captures all input when open — with one exception: // the minibuffer intent is just a focus marker for a // PTY-backed panel, so keys go to the minibuffer session's @@ -17467,6 +17541,7 @@ mod tests { prompt_turn_hits: Vec::new(), prompt_choice_hits: Vec::new(), modal_area: None, + modal_owner: None, remote_control_hits: Vec::new(), session_title_name_hits: Vec::new(), session_harness_hits: Vec::new(), @@ -39852,6 +39927,106 @@ mod tests { server.abort(); } + /// Regression: the rolled-down Playbook used to take keyboard and mouse + /// input before the remote-control dialog painted above it. Exercise the + /// real frame snapshot plus top-level event dispatch with and without the + /// underlying Playbook so both states keep identical dialog behavior. + #[tokio::test] + async fn remote_control_events_have_same_precedence_over_rolled_down_playbook() { + use construct_protocol::{RemoteProviderInfo, TunnelProvider}; + use crossterm::event::{MouseButton, MouseEvent, MouseEventKind}; + + for playbook_open in [false, true] { + let (mut app, _dir, server) = captured_app().await; + if playbook_open { + app.playbook_popup = Some(playbook_popup_for_test("s1", "draft", 2)); + app.focus = PaneFocus::View; + } + app.remote_control_popup = Some(RemoteControlPopup::Choose(RemoteControlChoose { + base: remote_ok_fixture(false), + options: vec![ + RemoteProviderInfo { + provider: TunnelProvider::Cloudflare, + available: true, + detail: None, + }, + RemoteProviderInfo { + provider: TunnelProvider::Construct, + available: true, + detail: None, + }, + ], + selected: 0, + active: None, + })); + + let backend = ratatui::backend::TestBackend::new(120, 40); + let mut terminal = ratatui::Terminal::new(backend).expect("terminal"); + terminal + .draw(|f| crate::ui::render(f, &mut app)) + .expect("draw remote over base surface"); + assert_eq!( + app.layout.modal_owner, + Some(ModalOwner::RemoteControl), + "rendered remote dialog must identify itself as topmost" + ); + + app.on_key(KeyEvent::new(KeyCode::Right, KeyModifiers::NONE)) + .await; + let selected = app + .remote_control_popup + .as_ref() + .and_then(|popup| match popup { + RemoteControlPopup::Choose(choose) => Some(choose.selected), + _ => None, + }); + assert_eq!(selected, Some(1)); + if let Some(playbook) = app.playbook_popup.as_ref() { + assert_eq!(playbook.buffer, "draft"); + } + app.on_paste("pasted".to_string()).await; + if let Some(playbook) = app.playbook_popup.as_ref() { + assert_eq!(playbook.buffer, "draft"); + } + + if let Some(RemoteControlPopup::Choose(choose)) = + app.remote_control_popup.as_mut() + { + choose.selected = 0; + } + let hit = app + .layout + .remote_control_hits + .iter() + .find(|hit| matches!(hit.action, RemoteControlHitAction::SelectProvider(1))) + .cloned() + .expect("second provider hit from rendered frame"); + let event = |kind| MouseEvent { + kind, + column: hit.x_start, + row: hit.y, + modifiers: KeyModifiers::NONE, + }; + app.on_mouse(event(MouseEventKind::Down(MouseButton::Left))) + .await; + app.on_mouse(event(MouseEventKind::Up(MouseButton::Left))) + .await; + + let selected = app + .remote_control_popup + .as_ref() + .and_then(|popup| match popup { + RemoteControlPopup::Choose(choose) => Some(choose.selected), + _ => None, + }); + assert_eq!(selected, Some(1)); + if let Some(playbook) = app.playbook_popup.as_ref() { + assert_eq!(playbook.cursor, 2); + } + server.abort(); + } + } + #[tokio::test] async fn remote_ready_view_back_stop_and_hints_are_clickable() { use crossterm::event::KeyCode; @@ -41543,6 +41718,13 @@ mod tests { app.focus = PaneFocus::View; app.help_visible = true; + let backend = ratatui::backend::TestBackend::new(120, 40); + let mut terminal = ratatui::Terminal::new(backend).expect("terminal"); + terminal + .draw(|f| crate::ui::render(f, &mut app)) + .expect("draw help over Playbook"); + assert_eq!(app.layout.modal_owner, Some(ModalOwner::Help)); + app.on_key(KeyEvent::new(KeyCode::Char('x'), KeyModifiers::NONE)) .await; @@ -41557,6 +41739,44 @@ mod tests { server.abort(); } + #[tokio::test] + async fn keyboard_only_session_picker_blocks_playbook_mouse_after_render() { + use crossterm::event::{MouseButton, MouseEvent, MouseEventKind}; + + let (mut app, _dir, server) = captured_app().await; + app.playbook_popup = Some(playbook_popup_for_test("s1", "draft", 2)); + app.focus = PaneFocus::View; + app.open_session_picker(SessionPickerPurpose::Switch); + + let backend = ratatui::backend::TestBackend::new(120, 40); + let mut terminal = ratatui::Terminal::new(backend).expect("terminal"); + terminal + .draw(|f| crate::ui::render(f, &mut app)) + .expect("draw picker over Playbook"); + assert_eq!( + app.layout.modal_owner, + Some(ModalOwner::SessionPicker) + ); + let modal = app.layout.modal_area.expect("session-picker area"); + let click = |kind| MouseEvent { + kind, + column: modal.x + 2, + row: modal.y + 2, + modifiers: KeyModifiers::NONE, + }; + app.on_mouse(click(MouseEventKind::Down(MouseButton::Left))) + .await; + app.on_mouse(click(MouseEventKind::Up(MouseButton::Left))) + .await; + + assert!(app.session_picker_active()); + assert_eq!( + app.playbook_popup.as_ref().map(|popup| popup.cursor), + Some(2) + ); + server.abort(); + } + #[tokio::test] async fn help_modal_arrow_and_page_keys_scroll_instead_of_closing() { let (mut app, _dir, server) = captured_app().await; @@ -41636,6 +41856,8 @@ mod tests { use crossterm::event::MouseButton; let (mut app, _dir, server) = captured_app().await; + app.playbook_popup = Some(playbook_popup_for_test("s1", "draft", 2)); + app.focus = PaneFocus::View; app.help_visible = true; let backend = ratatui::backend::TestBackend::new(120, 15); let mut terminal = ratatui::Terminal::new(backend).expect("terminal"); @@ -41671,6 +41893,11 @@ mod tests { }) .await; assert!(!app.help_visible, "a click should still close help"); + assert_eq!( + app.playbook_popup.as_ref().map(|popup| popup.cursor), + Some(2), + "Help mouse input must not reach the covered Playbook" + ); server.abort(); } diff --git a/crates/cli/src/ui.rs b/crates/cli/src/ui.rs index 908cc67e..441da088 100644 --- a/crates/cli/src/ui.rs +++ b/crates/cli/src/ui.rs @@ -3,8 +3,9 @@ use crate::app::{ feature_guidance, harness_guidance, harness_picker_entries, smith_method_guidance, App, ConfigureTab, FocusBorderTarget, HarnessHit, HintZone, ListItem as AppListItem, MainWindowTree, - PaneFocus, Prompt, PromptChoiceAction, PromptChoiceHit, PromptIntent, RemoteControlHit, - RemoteControlHitAction, ScreenPoint, Selection, OperatorTitleMenuAction, SessionTitleMenuAction, + ModalOwner, PaneFocus, Prompt, PromptChoiceAction, PromptChoiceHit, PromptIntent, + RemoteControlHit, RemoteControlHitAction, ScreenPoint, Selection, OperatorTitleMenuAction, + SessionTitleMenuAction, TextSelectionRange, TurnRowHit, ViewMode, WindowDividerHit, WindowPaneHit, WindowSplitDirection, ZoomMode, CONFIGURE_TABS, PLAYBOOK_AGENT_COLLAB_CURSOR_TTL_MS, PLAYBOOK_CLIP_HOVER_PREVIEW_COLS, PLAYBOOK_CLIP_HOVER_PREVIEW_ROWS, @@ -421,6 +422,7 @@ pub fn render(f: &mut Frame, app: &mut App) { app.layout.matrix_rain_area = None; app.layout.prompt_area = Some(prompt_area); app.layout.modal_area = None; + app.layout.modal_owner = None; app.layout.list_row_count = app.list_items().len(); app.layout.list_items_area = None; app.layout.list_scroll_offset = 0; @@ -477,6 +479,7 @@ pub fn render(f: &mut Frame, app: &mut App) { if app.help_visible { let help_popup = render_help(f, area, app); app.layout.modal_area = Some(help_popup); + app.layout.modal_owner = Some(ModalOwner::Help); } render_session_title_menu(f, app); render_operator_title_menu(f, app); @@ -2380,6 +2383,7 @@ fn render_zoomed_view(f: &mut Frame, area: Rect, app: &mut App) { app.layout.matrix_rain_area = None; app.layout.prompt_area = Some(prompt_area); app.layout.modal_area = None; + app.layout.modal_owner = None; app.layout.list_items_area = None; app.layout.list_scroll_offset = 0; @@ -2401,6 +2405,7 @@ fn render_zoomed_view(f: &mut Frame, area: Rect, app: &mut App) { if app.help_visible { let help_popup = render_help(f, area, app); app.layout.modal_area = Some(help_popup); + app.layout.modal_owner = Some(ModalOwner::Help); } } @@ -2420,6 +2425,7 @@ fn render_zoomed_list(f: &mut Frame, area: Rect, app: &mut App) { app.layout.matrix_rain_area = None; app.layout.prompt_area = Some(prompt_area); app.layout.modal_area = None; + app.layout.modal_owner = None; app.layout.list_row_count = app.list_items().len(); app.layout.list_items_area = None; app.layout.list_scroll_offset = 0; @@ -2432,6 +2438,7 @@ fn render_zoomed_list(f: &mut Frame, area: Rect, app: &mut App) { if app.help_visible { let help_popup = render_help(f, area, app); app.layout.modal_area = Some(help_popup); + app.layout.modal_owner = Some(ModalOwner::Help); } } @@ -14203,6 +14210,7 @@ fn render_configure_popup(f: &mut Frame, app: &mut App) { let inner = block.inner(popup_area); f.render_widget(block, popup_area); app.layout.modal_area = Some(popup_area); + app.layout.modal_owner = Some(ModalOwner::Configure); let sections = Layout::default() .direction(Direction::Vertical) @@ -16316,6 +16324,7 @@ fn render_tasks_popup(f: &mut Frame, app: &mut App) { height: h, }; app.layout.modal_area = Some(rect); + app.layout.modal_owner = Some(ModalOwner::Tasks); let title = format!( " tasks — session {} ({} entries) — Esc to close ", short_id(&popup.session_id), @@ -17850,6 +17859,7 @@ fn render_playbook_popup_at( // is visible there (a neighboring split, the exposed terminal). let pane_right = base_rect.right(); app.layout.modal_area = Some(rect.intersection(base_rect)); + app.layout.modal_owner = Some(ModalOwner::Playbook); app.layout.playbook_base_area = Some(base_rect); app.layout.playbook_resize_hit = Some( Rect { @@ -20207,6 +20217,8 @@ fn render_session_picker(f: &mut Frame, app: &mut App) { let inner = block.inner(rect); f.render_widget(Clear, rect); f.render_widget(block, rect); + app.layout.modal_area = Some(rect); + app.layout.modal_owner = Some(ModalOwner::SessionPicker); // The switcher splits its inner area into search / separator / body / footer; // the anchored variant is body-only. @@ -22925,6 +22937,7 @@ fn render_remote_control_popup(f: &mut Frame, app: &mut App) { height: h, }; app.layout.modal_area = Some(rect); + app.layout.modal_owner = Some(ModalOwner::RemoteControl); let block = Block::default() .borders(Borders::ALL) diff --git a/crates/e2e/tests/tui_smoke.rs b/crates/e2e/tests/tui_smoke.rs index 3706f778..20816451 100644 --- a/crates/e2e/tests/tui_smoke.rs +++ b/crates/e2e/tests/tui_smoke.rs @@ -19,6 +19,8 @@ use std::time::Duration; use construct_e2e::{Daemon, Tui}; +const OPEN_PLAYBOOK: &[u8] = b"\x18 "; + /// Minimal smoke: TUI starts, draws the modeline (IPC + render /// path), and quits cleanly on `q`. Keeps the bar low for the /// first TUI e2e — assertions on the slash-command popup go in @@ -125,6 +127,126 @@ async fn tui_remote_control_popup_via_palette() { ); } +/// Full PTY regression for modal precedence: open a real session Playbook, +/// place the remote-control dialog over it, then prove keyboard text does not +/// edit the document and a mouse click on the Construct provider reaches the +/// dialog's next step. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn tui_remote_control_owns_keyboard_and_mouse_over_playbook() { + let d = Daemon::spawn().await.expect("spawn daemon"); + let cwd = d.socket.parent().unwrap().to_string_lossy().to_string(); + let session = d + .client + .create(shell_session_params(&cwd, "modal-routing")) + .await + .expect("create session"); + d.client + .playbook_update(construct_protocol::PlaybookUpdateParams { + session_id: session.clone(), + markdown: "modal-routing-sentinel\n".to_string(), + base_version: None, + actor: construct_protocol::PlaybookUpdateActor::Human, + template_id: None, + note: None, + shimmer: None, + shimmer_tooltips: None, + }) + .await + .expect("seed playbook"); + + let mut tui = Tui::spawn_with_recording( + &d.socket, + "tui_remote_control_owns_keyboard_and_mouse_over_playbook", + ) + .expect("spawn TUI"); + tui.wait_for("construct focus:", Duration::from_secs(15)) + .await + .expect("modeline never rendered"); + tui.send(b"\x1b").expect("dismiss first-run configure if present"); + tokio::time::sleep(Duration::from_millis(300)).await; + tui.send(OPEN_PLAYBOOK).expect("open Playbook"); + tui.wait_for("modal-routing-sentinel", Duration::from_secs(10)) + .await + .expect("Playbook never rendered"); + + tui.send(b"\x18x").expect("open command palette"); + tokio::time::sleep(Duration::from_millis(200)).await; + tui.send(b"remote-control\r").expect("open remote control"); + tui.wait_for("/remote-connect", Duration::from_secs(15)) + .await + .expect("remote dialog never rendered over Playbook"); + + // This key is unused by the chooser but still belongs to the explicit + // modal. Before the fix it was inserted into the Playbook underneath. + tui.send(b"z").expect("send unused dialog key"); + tokio::time::sleep(Duration::from_millis(400)).await; + let stored = d + .client + .playbook_get(&session) + .await + .expect("read Playbook after key") + .playbook + .markdown; + assert_eq!(stored, "modal-routing-sentinel\n"); + + // Move focus to Cloudflare first. The pointer click below must move it + // back to the stable provider; otherwise Enter cannot reach the name form. + tui.send(b"\x1b[C").expect("select next provider"); + tokio::time::sleep(Duration::from_millis(100)).await; + let screen = tui.screen(); + let (row, col) = screen + .lines() + .enumerate() + .find_map(|(row, line)| { + line.find("tunnel.zarvis.ai") + .map(|byte_col| (row, line[..byte_col].chars().count())) + }) + .expect("stable provider coordinates"); + let mouse_down = format!("\x1b[<0;{};{}M", col + 1, row + 1); + let mouse_up = format!("\x1b[<0;{};{}m", col + 1, row + 1); + tui.send(mouse_down.as_bytes()).expect("provider mouse down"); + tui.send(mouse_up.as_bytes()).expect("provider mouse up"); + tui.send(b"\r").expect("activate clicked provider"); + tui.wait_for("Choose your tunnel name", Duration::from_secs(5)) + .await + .expect("provider click did not reach remote-control dialog"); + + tui.send(b"\x1b").expect("return to remote chooser"); + tui.wait_for_absence("Choose your tunnel name", Duration::from_secs(5)) + .await + .expect("remote name form did not go back"); + tui.send(b"\x1b").expect("close remote chooser"); + tui.wait_for_absence("/remote-connect", Duration::from_secs(5)) + .await + .expect("remote chooser did not close"); + tui.send(b"\x18\x03").expect("quit TUI"); + let status = tui + .wait_exit(Duration::from_secs(5)) + .await + .expect("TUI did not exit"); + assert!(status.success(), "TUI exited unsuccessfully: {status:?}"); +} + +fn shell_session_params(cwd: &str, title: &str) -> construct_protocol::CreateSessionParams { + construct_protocol::CreateSessionParams { + harness: "shell".to_string(), + cwd: cwd.to_string(), + prompt: None, + model: None, + title: Some(title.to_string()), + mode: None, + pty_size: None, + worktree: false, + env: std::collections::HashMap::new(), + args: Vec::new(), + kind: Default::default(), + parent_session_id: None, + group_id: None, + position_after_session_id: None, + forked_from: None, + } +} + /// The stopped remote-control status is a mouse-first discovery path. Clicking /// it starts the local listener and opens the same chooser as /// `/remote-connect`; after dismissing the dialog, the status remains visible diff --git a/specs/0205-tui-topmost-modal-owns-input.md b/specs/0205-tui-topmost-modal-owns-input.md new file mode 100644 index 00000000..19f4a8b1 --- /dev/null +++ b/specs/0205-tui-topmost-modal-owns-input.md @@ -0,0 +1,30 @@ +# 0205-tui-topmost-modal-owns-input + +Status: accepted +Date: 2026-08-19 +Area: tui +Scope: Input precedence when transient TUI surfaces overlap pane-local surfaces such as the rolled-down Playbook. + +## Decision + +The topmost rendered modal owns input before every surface painted beneath it. The frame records both the modal's bounds and its identity; keyboard, paste, and pointer routing must consult the same modal precedence represented by the render order instead of inferring ownership from whether an underlying surface exists. + +An explicit modal consumes inputs it does not use unless that modal's own established semantics say otherwise. In particular, the keyboard-only session picker consumes pointer events without acting on them, Help keeps its close-or-scroll behavior, and the remote-control dialog consumes every key while routing its registered pointer controls. + +Modal precedence does not alter dismissal semantics. A Playbook remains a pane-local roll-down surface; clicking outside it may interact with exposed panes without closing it. A dialog that closes on an outside click closes itself rather than a covered Playbook. A modal that can auto-open without user action keeps the close-and-reroute rule for every input it does not claim, so the dismissing input is processed exactly once by the next eligible surface. + +## Reason + +Render order and input order can diverge when one shared rectangle stores only the last modal geometry while event handlers separately test whether a Playbook or other underlying surface exists. The visible dialog then appears focused but keystrokes and clicks mutate the covered editor. Recording the owner alongside the geometry makes hit-testing follow what the user can actually see. + +## Consequences + +- New modal renderers must register their identity when they register modal bounds. +- Event paths must not give a covered Playbook, pane, or child PTY first refusal merely because it remains mounted. +- Pointer-down events on a modal cannot start focus, rename, resize, text-selection, or editor gestures underneath it. +- Existing per-modal close, fallthrough, scrolling, and keyboard-only behavior remains authoritative. + +## Non-Goals + +- Defining one universal dismissal key or outside-click behavior for every modal. +- Turning non-modal overlays, menus, or tutorial cards into modals. From 9f3a6e1b018ac96d8ec3890a0a1068fdf346de9e Mon Sep 17 00:00:00 2001 From: Edwin Date: Wed, 19 Aug 2026 09:36:55 -0700 Subject: [PATCH 2/2] keep modal routing fix scoped to direct input --- crates/cli/src/app.rs | 38 ++++------------------ specs/0205-tui-topmost-modal-owns-input.md | 2 +- 2 files changed, 8 insertions(+), 32 deletions(-) diff --git a/crates/cli/src/app.rs b/crates/cli/src/app.rs index 8955c5d0..e4677174 100644 --- a/crates/cli/src/app.rs +++ b/crates/cli/src/app.rs @@ -7169,33 +7169,6 @@ impl App { return; } - // True topmost modals get the same precedence for terminal paste - // events as for individual keys. The session picker owns paste as its - // typeahead; configure's auto-open semantics close and reroute the - // unclaimed text; Help closes and consumes; remote control consumes - // the paste (and its name form accepts the same valid characters as - // ordinary key events). - if self.session_picker_insert_text(&text) { - return; - } - if self.configure_popup.is_some() { - self.configure_popup = None; - } - if self.help_visible { - self.help_visible = false; - return; - } - if self.remote_control_popup.is_some() { - for ch in text.chars() { - self.handle_remote_control_key(KeyEvent::new( - KeyCode::Char(ch), - KeyModifiers::NONE, - )) - .await; - } - return; - } - // An inline title edit has the same modal precedence for terminal // paste events as it does for ordinary keys. Consume the whole paste // at the edit cursor instead of leaking it into the pane's PTY. @@ -7203,6 +7176,13 @@ impl App { return; } + // The session picker owns paste events at the same precedence as + // ordinary key events. Without this, `C-x b` accepted typed text in + // its search line but sent pasted text to the previously focused PTY. + if self.session_picker_insert_text(&text) { + return; + } + if self.insert_operator_dialog_text(&text) { return; } @@ -39984,10 +39964,6 @@ mod tests { if let Some(playbook) = app.playbook_popup.as_ref() { assert_eq!(playbook.buffer, "draft"); } - app.on_paste("pasted".to_string()).await; - if let Some(playbook) = app.playbook_popup.as_ref() { - assert_eq!(playbook.buffer, "draft"); - } if let Some(RemoteControlPopup::Choose(choose)) = app.remote_control_popup.as_mut() diff --git a/specs/0205-tui-topmost-modal-owns-input.md b/specs/0205-tui-topmost-modal-owns-input.md index 19f4a8b1..07344cd4 100644 --- a/specs/0205-tui-topmost-modal-owns-input.md +++ b/specs/0205-tui-topmost-modal-owns-input.md @@ -7,7 +7,7 @@ Scope: Input precedence when transient TUI surfaces overlap pane-local surfaces ## Decision -The topmost rendered modal owns input before every surface painted beneath it. The frame records both the modal's bounds and its identity; keyboard, paste, and pointer routing must consult the same modal precedence represented by the render order instead of inferring ownership from whether an underlying surface exists. +The topmost rendered modal owns input before every surface painted beneath it. The frame records both the modal's bounds and its identity; keyboard and pointer routing must consult the same modal precedence represented by the render order instead of inferring ownership from whether an underlying surface exists. An explicit modal consumes inputs it does not use unless that modal's own established semantics say otherwise. In particular, the keyboard-only session picker consumes pointer events without acting on them, Help keeps its close-or-scroll behavior, and the remote-control dialog consumes every key while routing its registered pointer controls.