From 964aeda6273f67f633a2f0f198158471ef095547 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sun, 25 Oct 2026 05:00:45 +0000 Subject: [PATCH] Fix transcript selection input routing and preserve composer drafts (#57224) ## Why Leaving transcript selection with a held key can send repeated input to the composer and submit a preserved draft. ## What changed - Route the transcript selection shortcut and selection controls in both the owned transcript and transcript overlay, preserving drafts and ignoring pastes outside search. - Give fixed selection controls and pager motions precedence over conflicting key chords while allowing search and voice controls. - Consume held exit keys across selection dismissal and asynchronous copy completion, and require fresh presses for composer submit and queue actions. - Initialize overlay selection against the refreshed transcript frame and complete selection navigation when older history finishes loading. ## Testing Add regression coverage for draft preservation, repeated `Enter` events, selection and search key precedence, refreshed-frame selection, and pending copy exit-key cleanup. GitOrigin-RevId: 6cca9f399af1d49ff79337ae745cfc1564a14b88 --- codex-rs/tui/src/app.rs | 24 ++- codex-rs/tui/src/app/clipboard.rs | 7 + codex-rs/tui/src/app/history_pagination.rs | 9 +- codex-rs/tui/src/app/input.rs | 28 +++- codex-rs/tui/src/app/input/select_mode.rs | 146 ++++++++++++++++++ codex-rs/tui/src/app/owned_transcript.rs | 75 +++++++-- .../tui/src/app/owned_transcript_tests.rs | 132 ++++++++++++++++ codex-rs/tui/src/app/tests/key_chords.rs | 80 +++++++++- codex-rs/tui/src/app_backtrack.rs | 88 ++++++++++- codex-rs/tui/src/bottom_pane/chat_composer.rs | 7 + codex-rs/tui/src/keymap.rs | 2 + codex-rs/tui/src/keymap/chords.rs | 4 +- codex-rs/tui/src/pager_overlay.rs | 2 +- codex-rs/tui/src/pager_overlay/transcript.rs | 35 +++-- codex-rs/tui/src/tui.rs | 4 + codex-rs/tui/src/tui/alternate_screen.rs | 8 + codex-rs/tui/src/tui/keyboard_modes.rs | 40 +++-- 17 files changed, 649 insertions(+), 42 deletions(-) create mode 100644 codex-rs/tui/src/app/input/select_mode.rs diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index f3326b44bb..d2bb38e890 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -887,6 +887,23 @@ impl App { self.chat_widget.acknowledge_program_status(); } let idle_draw = matches!(event, TuiEvent::Draw); + if let TuiEvent::Key(key) = &event { + self.backtrack.observe_pending_select_exit_key(*key); + } + if let TuiEvent::Key(key) = &event + && let Some(binding) = self.backtrack.select_exit_key + { + if binding.parts() == crate::key_hint::KeyBinding::from_event(*key).parts() { + if key.kind == KeyEventKind::Release + || !self.enhanced_keys_supported + || !crate::tui::reports_key_releases() + { + self.backtrack.select_exit_key = None; + } + return Ok(AppRunControl::Continue); + } + self.backtrack.select_exit_key = None; + } if self.handle_rendered_selection_event(tui, &event)? { return Ok(AppRunControl::Continue); } @@ -993,9 +1010,12 @@ impl App { }) && app.keymap.chat.toggle_voice.is_pressed(key) }; - // Find consumes otherwise-unhandled keys; let enabled voice controls reach App. + // Transcript interactions consume otherwise-unhandled keys; let enabled voice controls + // reach App. if !matches!(&event, TuiEvent::Key(key) - if voice_toggle(self, *key) && !self.transcript_view.owns_interaction_key(*key)) + if voice_toggle(self, *key) + && (!self.transcript_view.owns_interaction_key(*key) + || self.transcript_view.is_select_mode())) && self.handle_owned_transcript_event(tui, app_server, &event)? { return Ok(AppRunControl::Continue); diff --git a/codex-rs/tui/src/app/clipboard.rs b/codex-rs/tui/src/app/clipboard.rs index 5b25a0f299..40b882ed18 100644 --- a/codex-rs/tui/src/app/clipboard.rs +++ b/codex-rs/tui/src/app/clipboard.rs @@ -69,11 +69,18 @@ impl App { let follow = self.transcript_view .finish_copy(&self.transcript_cells, completion, current); + let pending_exit_key = if current { + self.backtrack + .finish_pending_select_exit_key(completion.0, /*select_mode_closed*/ true) + } else { + None + }; if follow == Some(true) { if self.backtrack.overlay_preview_active { self.close_transcript_overlay(tui); } self.transcript_view.jump_to_latest(); + self.backtrack.select_exit_key = pending_exit_key; } } if !current || matches!(event, TuiEvent::FocusLost | TuiEvent::Resume) { diff --git a/codex-rs/tui/src/app/history_pagination.rs b/codex-rs/tui/src/app/history_pagination.rs index 0c9f0053fb..f7210795b2 100644 --- a/codex-rs/tui/src/app/history_pagination.rs +++ b/codex-rs/tui/src/app/history_pagination.rs @@ -287,8 +287,13 @@ impl App { }; let continue_to_start = previous == TranscriptHistoryState::LoadingBeginning; if continue_to_start && !self.scrollback_has_older_history { - self.transcript_view - .jump_to_entry(&self.transcript_cells, /*index*/ 0); + if self.transcript_view.is_select_mode() { + self.transcript_view + .complete_select_beginning(&self.transcript_cells); + } else { + self.transcript_view + .jump_to_entry(&self.transcript_cells, /*index*/ 0); + } } if self.scrollback_has_older_history && (continue_to_start diff --git a/codex-rs/tui/src/app/input.rs b/codex-rs/tui/src/app/input.rs index 2ceb510c23..d2be9ee5d1 100644 --- a/codex-rs/tui/src/app/input.rs +++ b/codex-rs/tui/src/app/input.rs @@ -9,6 +9,8 @@ use crate::app_backtrack::SIDE_EDIT_PREVIOUS_UNAVAILABLE_MESSAGE; use crate::keymap::bindings_for_action; use crate::keymap::keymap_action_ids; +mod select_mode; + impl App { pub(super) fn should_recover_vim_insert_escape(&self, key_event: KeyEvent) -> bool { let active_contexts = self.active_keymap_contexts(); @@ -90,14 +92,16 @@ impl App { if self.overlay.is_none() && self.chat_widget.handle_quick_dial_menu(key_event) { return None; } - if matches!(&self.overlay, Some(Overlay::Transcript(overlay)) if overlay.owns_interaction_key(key_event)) + let select_mode_chord = self.select_mode_owns_chord(tui, key_event); + if (matches!(&self.overlay, Some(Overlay::Transcript(overlay)) if overlay.owns_interaction_key(key_event)) || (tui.is_owned_screen() && self.overlay.is_none() && self.chat_widget.no_modal_or_popup_active() && self.transcript_view.owns_interaction_key(key_event) && (self.transcript_view.has_active_interaction() || self.backtrack.overlay_preview_active - || crate::transcript_view::JumpTarget::from_key(key_event).is_none())) + || crate::transcript_view::JumpTarget::from_key(key_event).is_none()))) + && !select_mode_chord { let close_chord = tui.is_owned_screen() && self.overlay.is_none() @@ -232,7 +236,13 @@ impl App { } else { KeymapContext::Pager }; - let contexts = KeymapContextSet::new(context).with_voice_toggle(&self.keymap); + let contexts = + if matches!(overlay, Overlay::Transcript(_)) && context == KeymapContext::Pager { + KeymapContextSet::transcript() + } else { + KeymapContextSet::new(context) + } + .with_voice_toggle(&self.keymap); return if self.backtrack.overlay_preview_active && context == KeymapContext::Pager { KeymapContextSet::browsing().with_voice_toggle(&self.keymap) } else { @@ -242,6 +252,9 @@ impl App { if self.transcript_view.is_search_editing() && self.chat_widget.no_modal_or_popup_active() { return KeymapContextSet::new(KeymapContext::Editor).with_voice_toggle(&self.keymap); } + if self.transcript_view.is_select_mode() && self.chat_widget.no_modal_or_popup_active() { + return KeymapContextSet::transcript().with_voice_toggle(&self.keymap); + } if self.transcript_view.is_activity_focused() && self.chat_widget.no_modal_or_popup_active() { return KeymapContextSet::activity().with_voice_toggle(&self.keymap); @@ -583,8 +596,11 @@ impl App { } let find_transcript = self.keymap.app.find_transcript.is_pressed(key_event); + let select_transcript = self.keymap.app.select_transcript.is_pressed(key_event); if app_keymap_shortcuts_available - && (self.keymap.app.open_transcript.is_pressed(key_event) || find_transcript) + && (self.keymap.app.open_transcript.is_pressed(key_event) + || find_transcript + || select_transcript) { self.scrollback_has_older_history = self .chat_widget @@ -595,6 +611,10 @@ impl App { overlay.set_keymap_bindings(&self.keymap); overlay.begin_search(); } + if select_transcript && let Some(Overlay::Transcript(overlay)) = &mut self.overlay { + overlay.set_keymap_bindings(&self.keymap); + overlay.begin_select_mode(); + } return; } diff --git a/codex-rs/tui/src/app/input/select_mode.rs b/codex-rs/tui/src/app/input/select_mode.rs new file mode 100644 index 0000000000..1664dd2b3b --- /dev/null +++ b/codex-rs/tui/src/app/input/select_mode.rs @@ -0,0 +1,146 @@ +//! Transcript selection chord routing kept outside the central application input path. + +use super::super::*; + +impl App { + pub(crate) fn transcript_select_exit_key(event: &TuiEvent) -> Option { + let TuiEvent::Key(key) = event else { + return None; + }; + let (code, modifiers) = crate::key_hint::normalize_key_parts(key.code, key.modifiers); + (key.kind == KeyEventKind::Press + && modifiers == KeyModifiers::NONE + && matches!(code, KeyCode::Esc | KeyCode::Enter | KeyCode::Char('q'))) + .then(|| KeyBinding::from_event(*key)) + } + + pub(super) fn select_mode_owns_chord(&self, tui: &tui::Tui, key_event: KeyEvent) -> bool { + let select_mode = matches!( + &self.overlay, + Some(Overlay::Transcript(overlay)) if overlay.is_select_mode() + ) || (tui.is_owned_screen() + && self.overlay.is_none() + && self.transcript_view.is_select_mode()); + if !select_mode { + return false; + } + let contexts = self.active_keymap_contexts(); + let search_editing = matches!( + &self.overlay, + Some(Overlay::Transcript(overlay)) if overlay.is_search_editing() + ) || (self.overlay.is_none() + && self.transcript_view.is_search_editing()); + let (code, modifiers) = + crate::key_hint::normalize_key_parts(key_event.code, key_event.modifiers); + let fixed_search_control = search_editing + && (code == KeyCode::Esc + || (modifiers == KeyModifiers::NONE + && matches!(code, KeyCode::Enter | KeyCode::PageUp | KeyCode::PageDown)) + || (modifiers == KeyModifiers::CONTROL + && matches!(code, KeyCode::Char('c' | 'n' | 'p')))); + let fixed_select_control = !search_editing + && key_event.kind != KeyEventKind::Release + && (crate::text_selection::is_copy_key(key_event) + || matches!( + (code, modifiers), + ( + KeyCode::Enter | KeyCode::Esc | KeyCode::Char('q' | '/' | 'o' | 'y'), + KeyModifiers::NONE + ) | ( + KeyCode::Char(' '), + KeyModifiers::NONE | KeyModifiers::CONTROL + ) | ( + KeyCode::Char('v' | 'n' | 'g' | '$'), + KeyModifiers::NONE | KeyModifiers::SHIFT + ) | ( + KeyCode::Left | KeyCode::Right | KeyCode::Up | KeyCode::Down, + KeyModifiers::NONE | KeyModifiers::SHIFT + ) | ( + KeyCode::Char('h' | 'l' | 'k' | 'j' | 'w' | 'b' | 'e' | '0'), + KeyModifiers::NONE + ) | ( + KeyCode::Home | KeyCode::End, + KeyModifiers::NONE | KeyModifiers::CONTROL + ) | (KeyCode::PageUp | KeyCode::PageDown, KeyModifiers::NONE) + | (KeyCode::Char('b' | 'f' | 'u' | 'd'), KeyModifiers::CONTROL) + )); + let pager_motion = !search_editing + && [ + &self.keymap.pager.scroll_up, + &self.keymap.pager.scroll_down, + &self.keymap.pager.page_up, + &self.keymap.pager.page_down, + &self.keymap.pager.half_page_up, + &self.keymap.pager.half_page_down, + &self.keymap.pager.jump_top, + &self.keymap.pager.jump_bottom, + ] + .into_iter() + .any(|bindings| bindings.is_pressed(key_event)); + let active_action = |action: crate::keymap::KeymapActionId| { + contexts.contains_action(action) + && if search_editing { + action.context == crate::keymap::KeymapContext::Editor + || matches!( + (action.context, action.action), + (crate::keymap::KeymapContext::Chat, "toggle_voice") + ) + } else { + matches!( + (action.context, action.action), + (crate::keymap::KeymapContext::Global, "find_transcript") + | (crate::keymap::KeymapContext::Chat, "toggle_voice") + | ( + crate::keymap::KeymapContext::Pager, + "find" + | "scroll_up" + | "scroll_down" + | "page_up" + | "page_down" + | "half_page_up" + | "half_page_down" + | "jump_top" + | "jump_bottom" + ) + ) || (self.overlay.is_some() + && action.context == crate::keymap::KeymapContext::Pager + && matches!(action.action, "close" | "close_transcript")) + } + }; + let completed_action = |dispatch| { + self.keymap.chords.bindings.iter().any(|binding| { + active_action(binding.action) + && crate::keymap::dispatch_event(binding.action) == Some(dispatch) + }) + }; + if fixed_search_control || fixed_select_control || pager_motion { + false + } else if self.key_chord_matcher.is_pending() { + match self + .key_chord_matcher + .clone() + .advance(key_event, &self.keymap.chords, contexts) + { + crate::keymap::KeyChordMatch::Completed(dispatch) => completed_action(dispatch), + crate::keymap::KeyChordMatch::Pending(prefix) => { + let (code, modifiers) = prefix.parts(); + self.keymap.chords.bindings.iter().any(|binding| { + active_action(binding.action) + && crate::keymap::chord_stroke_matches( + binding.chord.prefix, + KeyEvent::new(code, modifiers), + ) + }) + } + crate::keymap::KeyChordMatch::Ignored => true, + crate::keymap::KeyChordMatch::PassThrough + | crate::keymap::KeyChordMatch::Cancelled => false, + } + } else { + self.keymap.chords.bindings.iter().any(|binding| { + active_action(binding.action) + && crate::keymap::chord_stroke_matches(binding.chord.prefix, key_event) + }) + } + } +} diff --git a/codex-rs/tui/src/app/owned_transcript.rs b/codex-rs/tui/src/app/owned_transcript.rs index 7c43e1799b..e976c2bce1 100644 --- a/codex-rs/tui/src/app/owned_transcript.rs +++ b/codex-rs/tui/src/app/owned_transcript.rs @@ -485,6 +485,7 @@ impl App { | crossterm::event::MouseEventKind::ScrollDown ) { self.transcript_view.end_selection(&self.transcript_cells); + self.transcript_view.end_select_mode(); self.transcript_view.cancel_search(); self.transcript_view.clear_activity_focus(); } @@ -532,6 +533,16 @@ impl App { { self.chat_widget.end_composer_drag(); } + if let TuiEvent::Key(key) = event + && self.chat_widget.no_modal_or_popup_active() + && !self.transcript_view.is_select_mode() + && self.keymap.app.select_transcript.is_pressed(*key) + { + self.transcript_view + .begin_select_mode(&self.transcript_cells); + tui.frame_requester().schedule_frame(); + return Ok(true); + } // Read-only Escape belongs to session navigation, even when the transcript is scrolled. // Search and selection still consume Escape first to dismiss their interaction. if let TuiEvent::Key(key) = event @@ -627,7 +638,24 @@ impl App { { return Ok(false); } + let was_select_mode = self.transcript_view.is_select_mode(); + let select_exit_key = Self::transcript_select_exit_key(event); let action = match event { + TuiEvent::Key(key) if was_select_mode && !self.transcript_view.is_search_editing() => { + if let Some(action) = self + .transcript_view + .handle_select_mode_key(*key, &self.transcript_cells) + { + Some(action) + } else { + if self.keymap.app.find_transcript.is_pressed(*key) + || self.keymap.pager.find.is_pressed(*key) + { + self.transcript_view.begin_search(); + } + Some(ViewAction::Changed) + } + } TuiEvent::Key(key) if !self.transcript_view.is_search_editing() && self.keymap.app.find_transcript.is_pressed(*key) => @@ -645,6 +673,11 @@ impl App { .transcript_view .handle_mouse(*mouse, &self.transcript_cells), TuiEvent::Paste(text) => { + if self.transcript_view.is_select_mode() + && !self.transcript_view.is_search_editing() + { + return Ok(true); + } self.transcript_view.end_selection(&self.transcript_cells); if self.transcript_view.paste_search(text) { Some(ViewAction::Changed) @@ -677,15 +710,39 @@ impl App { } return self.handle_owned_backtrack_event(tui, event); }; - if let Some((characters, result)) = self.transcript_view.copy_action( - tui, - &self.transcript_cells, - &action, - crate::clipboard_copy::CopyFormat::Markdown, - ) { - self.transcript_view.show_copy_feedback(&result, characters); - } else if let ViewAction::OpenLink(url) = action { - self.open_url_in_browser(url); + let resume_following = matches!(action, ViewAction::CopyAndFollow(_)); + if !matches!(action, ViewAction::ExitSelectMode) { + if let Some((characters, result)) = self.transcript_view.copy_action( + tui, + &self.transcript_cells, + &action, + crate::clipboard_copy::CopyFormat::Markdown, + ) { + self.transcript_view.show_copy_feedback(&result, characters); + if resume_following { + match result { + Ok(crate::clipboard_copy::CopyStatus::Confirmed) => { + self.transcript_view.jump_to_latest(); + } + Ok(crate::clipboard_copy::CopyStatus::Pending(id)) => { + if let Some(key) = select_exit_key { + self.backtrack.pending_select_exit_key = Some((id, key)); + } + } + Ok( + crate::clipboard_copy::CopyStatus::Unconfirmed + | crate::clipboard_copy::CopyStatus::Busy, + ) + | Err(_) => {} + } + } + } else if let ViewAction::OpenLink(url) = action { + self.open_url_in_browser(url); + } + } + if was_select_mode && !self.transcript_view.is_select_mode() { + self.backtrack.pending_select_exit_key = None; + self.backtrack.select_exit_key = select_exit_key; } self.request_owned_history(tui, app_server); tui.frame_requester().schedule_frame(); diff --git a/codex-rs/tui/src/app/owned_transcript_tests.rs b/codex-rs/tui/src/app/owned_transcript_tests.rs index a68938e517..7195d7697a 100644 --- a/codex-rs/tui/src/app/owned_transcript_tests.rs +++ b/codex-rs/tui/src/app/owned_transcript_tests.rs @@ -867,6 +867,138 @@ async fn owned_search_and_selection_consume_input_before_composer_and_backtrack( Ok(()) } +#[tokio::test] +async fn keyboard_select_mode_preserves_the_draft_and_absorbs_enter_repeat() -> Result<()> { + let mut app = crate::app::test_support::make_test_app().await; + attach_thread(&mut app, ThreadId::new()); + app.transcript_cells = vec![user_cell("select this transcript")]; + app.chat_widget + .apply_external_edit("composer draft".to_string()); + app.keymap.app.find_transcript = vec![crate::key_hint::KeyBinding::from_event( + KeyCode::Char('v').into(), + )]; + app.keymap.pager.find = vec![crate::key_hint::KeyBinding::from_event( + KeyCode::Enter.into(), + )]; + let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?; + let mut tui = crate::tui::test_support::make_test_tui()?; + tui.set_owned_screen(/*owned*/ true)?; + app.render_owned_transcript(&mut tui, Size::new(/*width*/ 80, /*height*/ 24))?; + + for key in [ + KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL), + KeyCode::Char('[').into(), + ] { + app.handle_tui_event(&mut tui, &mut app_server, TuiEvent::Key(key)) + .await?; + } + assert!(app.transcript_view.is_select_mode()); + + app.handle_tui_event( + &mut tui, + &mut app_server, + TuiEvent::Key(KeyCode::Char('v').into()), + ) + .await?; + assert!(app.transcript_view.is_select_mode()); + assert!(!app.transcript_view.is_search_editing()); + for event in [ + TuiEvent::Paste("ignored".to_string()), + TuiEvent::Key(KeyCode::Esc.into()), + ] { + app.handle_tui_event(&mut tui, &mut app_server, event) + .await?; + } + assert!(app.transcript_view.is_select_mode()); + assert!(app.backtrack.select_exit_key.is_none()); + + for (kind, exit_guard) in [ + (KeyEventKind::Press, true), + (KeyEventKind::Repeat, false), + (KeyEventKind::Release, false), + ] { + app.handle_tui_event( + &mut tui, + &mut app_server, + TuiEvent::Key(KeyEvent::new_with_kind( + KeyCode::Enter, + KeyModifiers::NONE, + kind, + )), + ) + .await?; + assert_eq!(app.backtrack.select_exit_key.is_some(), exit_guard); + } + assert!(!app.transcript_view.is_select_mode()); + assert_eq!( + app.chat_widget.composer_text_with_pending(), + "composer draft" + ); + + tui.set_owned_screen(/*owned*/ false)?; + app_server.shutdown().await?; + Ok(()) +} + +#[tokio::test] +async fn inline_select_mode_routes_find_and_returns_to_the_preserved_draft() -> Result<()> { + let mut app = crate::app::test_support::make_test_app().await; + attach_thread(&mut app, ThreadId::new()); + app.transcript_cells = vec![user_cell("needle in history")]; + app.chat_widget + .apply_external_edit("preserved inline draft".to_string()); + app.keymap = RuntimeKeymap::from_config(&serde_json::from_value(serde_json::json!({ + "global": {"find_transcript": "f4"}, + "pager": {"scroll_down": "f4"} + }))?) + .expect("pager motion takes precedence in select mode"); + let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?; + let mut tui = crate::tui::test_support::make_test_tui()?; + app.open_transcript_overlay(&mut tui); + + for key in [ + KeyEvent::new(KeyCode::Char('x'), KeyModifiers::CONTROL), + KeyCode::Char('[').into(), + ] { + app.handle_tui_event(&mut tui, &mut app_server, TuiEvent::Key(key)) + .await?; + } + app.handle_tui_event( + &mut tui, + &mut app_server, + TuiEvent::Key(KeyCode::F(4).into()), + ) + .await?; + assert!(matches!( + &app.overlay, + Some(Overlay::Transcript(overlay)) + if overlay.is_select_mode() && !overlay.is_search_editing() + )); + app.handle_tui_event( + &mut tui, + &mut app_server, + TuiEvent::Key(KeyCode::F(3).into()), + ) + .await?; + assert!(matches!( + &app.overlay, + Some(Overlay::Transcript(overlay)) if overlay.is_search_editing() + )); + + for key in [KeyCode::Esc, KeyCode::Char('q')] { + app.handle_tui_event(&mut tui, &mut app_server, TuiEvent::Key(key.into())) + .await?; + } + assert!(app.overlay.is_none()); + assert_eq!( + app.chat_widget.composer_text_with_pending(), + "preserved inline draft" + ); + + app_server.shutdown().await?; + Ok(()) +} + #[tokio::test] async fn inline_transcript_search_draws_and_escape_precedes_backtrack() -> Result<()> { let mut app = crate::app::test_support::make_test_app().await; diff --git a/codex-rs/tui/src/app/tests/key_chords.rs b/codex-rs/tui/src/app/tests/key_chords.rs index 7151dafd25..447a3c5b4f 100644 --- a/codex-rs/tui/src/app/tests/key_chords.rs +++ b/codex-rs/tui/src/app/tests/key_chords.rs @@ -868,7 +868,10 @@ async fn preset_menu_keys_take_precedence_over_transcript_find() -> Result<()> { async fn transcript_fixed_keys_take_precedence_over_pager_chord_prefixes() -> Result<()> { let (mut app, mut tui, mut app_server) = chord_app().await?; app.keymap = RuntimeKeymap::from_config(&serde_json::from_value(serde_json::json!({ - "global": {"copy": ["ctrl-x ctrl-u"]}, + "global": { + "copy": ["ctrl-x ctrl-u"], + "find_transcript": ["ctrl-space f"] + }, "pager": { "scroll_up": ["ctrl-space x", "ctrl-home x", "ctrl-end x"], "close_transcript": ["ctrl-x ctrl-u"] @@ -884,6 +887,19 @@ async fn transcript_fixed_keys_take_precedence_over_pager_chord_prefixes() -> Re assert_eq!(app.route_key_chord_event(&mut tui, key), Some(key)); assert!(!app.key_chord_matcher.is_pending()); } + if let Some(crate::pager_overlay::Overlay::Transcript(overlay)) = &mut app.overlay { + overlay.begin_select_mode(); + let fixed = KeyEvent::new(KeyCode::Char(' '), KeyModifiers::CONTROL); + assert_eq!(app.route_key_chord_event(&mut tui, fixed), Some(fixed)); + assert!(!app.key_chord_matcher.is_pending()); + press( + &mut app, + &mut tui, + &mut app_server, + KeyCode::Char('q').into(), + ) + .await?; + } press(&mut app, &mut tui, &mut app_server, ctrl('x')).await?; press(&mut app, &mut tui, &mut app_server, ctrl('u')).await?; assert!(app.overlay.is_none()); @@ -938,6 +954,40 @@ async fn transcript_fixed_keys_take_precedence_over_pager_chord_prefixes() -> Re assert!(!app.transcript_view.is_detailed()); assert!(!app.key_chord_matcher.is_pending()); } + app.keymap = RuntimeKeymap::from_config(&toml::from_str( + "[global]\nfind_transcript = 'f6 f'\n[pager]\nscroll_down = 'f6'", + )?) + .expect("valid pager motion and global chord"); + app.transcript_cells = vec![std::sync::Arc::new( + crate::history_cell::PlainHistoryCell::new(vec!["first".into(), "second".into()]), + )]; + tui.set_owned_screen(/*owned*/ true)?; + app.render_owned_transcript( + &mut tui, + ratatui::layout::Size::new(/*width*/ 80, /*height*/ 12), + )?; + assert!(app.transcript_view.begin_select_mode(&app.transcript_cells)); + press( + &mut app, + &mut tui, + &mut app_server, + KeyCode::Char('g').into(), + ) + .await?; + press( + &mut app, + &mut tui, + &mut app_server, + KeyCode::Char('v').into(), + ) + .await?; + let before = app.transcript_view.selected_text(&app.transcript_cells); + press(&mut app, &mut tui, &mut app_server, KeyCode::F(6).into()).await?; + assert_ne!( + app.transcript_view.selected_text(&app.transcript_cells), + before + ); + assert!(!app.key_chord_matcher.is_pending()); tui.set_owned_screen(/*owned*/ false)?; app_server.shutdown().await?; Ok(()) @@ -946,6 +996,13 @@ async fn transcript_fixed_keys_take_precedence_over_pager_chord_prefixes() -> Re #[tokio::test] async fn selection_after_live_commit_uses_the_refreshed_frame() -> Result<()> { let (mut app, mut tui, mut app_server) = chord_app().await?; + let config: TuiKeymap = toml::from_str( + "[global]\ntoggle_side_conversation = []\nfind_transcript = 'ctrl-7 j'\n[pager]\nscroll_down = 'ctrl-7 j'", + )?; + let runtime = + RuntimeKeymap::from_config(&config).map_err(|error| color_eyre::eyre::eyre!(error))?; + app.chat_widget.apply_keymap_update(config, &runtime); + app.keymap = runtime; let area = ratatui::layout::Rect::new( /*x*/ 0, /*y*/ 0, /*width*/ 40, /*height*/ 12, ); @@ -960,18 +1017,33 @@ async fn selection_after_live_commit_uses_the_refreshed_frame() -> Result<()> { overlay.insert_cell(std::sync::Arc::new( crate::history_cell::PlainHistoryCell::new(vec!["committed output".into()]), )); + overlay.begin_select_mode(); app.overlay = Some(crate::pager_overlay::Overlay::Transcript(overlay)); // The active cell has completed, but no Draw has reached the overlay yet. - app.handle_tui_event( + assert!(matches!( + &app.overlay, + Some(crate::pager_overlay::Overlay::Transcript(overlay)) if overlay.is_select_mode() + )); + press(&mut app, &mut tui, &mut app_server, ctrl('/')).await?; + assert!(app.key_chord_matcher.is_pending()); + let repeat = KeyEvent::new_with_kind( + KeyCode::Char('/'), + KeyModifiers::CONTROL, + KeyEventKind::Repeat, + ); + press(&mut app, &mut tui, &mut app_server, repeat).await?; + assert!(app.key_chord_matcher.is_pending()); + press( + &mut app, &mut tui, &mut app_server, - TuiEvent::Key(KeyEvent::new(KeyCode::Char(' '), KeyModifiers::CONTROL)), + KeyCode::Char('j').into(), ) .await?; let Some(crate::pager_overlay::Overlay::Transcript(overlay)) = &mut app.overlay else { panic!("overlay closed") }; - assert!(overlay.has_active_interaction()); + assert!(overlay.has_active_interaction() && !overlay.is_search_editing()); let mut buffer = ratatui::buffer::Buffer::empty(area); overlay.render(area, &mut buffer); let text = buffer diff --git a/codex-rs/tui/src/app_backtrack.rs b/codex-rs/tui/src/app_backtrack.rs index f3e5d5a844..56176cd353 100644 --- a/codex-rs/tui/src/app_backtrack.rs +++ b/codex-rs/tui/src/app_backtrack.rs @@ -75,9 +75,43 @@ pub(crate) struct BacktrackState { pub(crate) nth_user_message: usize, /// True when the transcript overlay is showing a backtrack preview. pub(crate) overlay_preview_active: bool, + /// Consume held-key input after a fresh press acts on transcript select mode. + pub(crate) select_exit_key: Option, + /// Preserve the initiating key while a select-and-follow copy is pending. + pub(crate) pending_select_exit_key: Option<(u64, crate::key_hint::KeyBinding)>, origin: Option, } +impl BacktrackState { + pub(crate) fn observe_pending_select_exit_key(&mut self, event: KeyEvent) { + if event.kind == KeyEventKind::Release + && self.pending_select_exit_key.is_some_and(|(_, binding)| { + binding.parts() == crate::key_hint::KeyBinding::from_event(event).parts() + }) + { + self.pending_select_exit_key = None; + } + } + + pub(crate) fn finish_pending_select_exit_key( + &mut self, + completed_id: u64, + select_mode_closed: bool, + ) -> Option { + match self.pending_select_exit_key { + Some((id, key)) if id == completed_id => { + self.pending_select_exit_key = None; + select_mode_closed.then_some(key) + } + Some((id, _)) if id < completed_id => { + self.pending_select_exit_key = None; + None + } + _ => None, + } + } +} + /// A user-visible backtrack choice that can be reopened in the current thread. #[derive(Debug, Clone, PartialEq)] pub(crate) struct BacktrackSelection { @@ -357,6 +391,7 @@ impl App { /// source of truth for the active cell and its cache invalidation key, and because `App` owns /// overlay lifecycle and frame scheduling for animations. fn overlay_forward_event(&mut self, tui: &mut tui::Tui, event: TuiEvent) -> Result<()> { + let select_exit_key = Self::transcript_select_exit_key(&event); if matches!( &event, TuiEvent::Draw | TuiEvent::Resume | TuiEvent::Resize(_) | TuiEvent::FocusGained @@ -378,6 +413,8 @@ impl App { )) && let Some(Overlay::Transcript(t)) = &mut self.overlay { + let was_select_mode = t.is_select_mode(); + let pending_follow_before = t.pending_follow_id(); tui.set_overlay_input(tui::OverlayInput::Transcript)?; t.motion = crate::motion::MotionMode::from_animations_enabled( self.local_settings.tui.animations && self.local_settings.tui.effects.shimmer, @@ -407,8 +444,10 @@ impl App { if result.is_err() { let _ = tui.set_overlay_input(tui::OverlayInput::Default); } - result?; + let completed_copy = result?; let close_overlay = t.is_done(); + let select_mode_closed = was_select_mode && (!t.is_select_mode() || close_overlay); + let pending_follow_id = t.pending_follow_id(); if !close_overlay && active_key.is_some_and(|key| key.animation_tick.is_some()) && t.is_scrolled_to_bottom() @@ -416,6 +455,21 @@ impl App { tui.frame_requester() .schedule_frame_in(std::time::Duration::from_millis(50)); } + if pending_follow_id != pending_follow_before + && let (Some(id), Some(key)) = (pending_follow_id, select_exit_key) + { + self.backtrack.pending_select_exit_key = Some((id, key)); + } + let completed_exit_key = completed_copy.and_then(|completed_id| { + self.backtrack + .finish_pending_select_exit_key(completed_id, select_mode_closed) + }); + if select_mode_closed { + self.backtrack.select_exit_key = select_exit_key.or(completed_exit_key); + if select_exit_key.is_some() { + self.backtrack.pending_select_exit_key = None; + } + } if close_overlay { self.close_transcript_overlay(tui); tui.frame_requester().schedule_frame(); @@ -740,6 +794,38 @@ mod tests { .collect() } + #[test] + fn pending_select_exit_key_ends_on_completion_or_release() { + let key = crate::key_hint::KeyBinding::from_event(KeyCode::Enter.into()); + let mut state = BacktrackState { + pending_select_exit_key: Some((7, key)), + ..BacktrackState::default() + }; + + assert_eq!( + state.finish_pending_select_exit_key( + /*completed_id*/ 7, /*select_mode_closed*/ false, + ), + None + ); + assert_eq!(state.pending_select_exit_key, None); + state.pending_select_exit_key = Some((8, key)); + state.observe_pending_select_exit_key(KeyEvent::new_with_kind( + KeyCode::Enter, + crossterm::event::KeyModifiers::NONE, + KeyEventKind::Release, + )); + assert_eq!(state.pending_select_exit_key, None); + state.pending_select_exit_key = Some((9, key)); + assert_eq!( + state.finish_pending_select_exit_key( + /*completed_id*/ 9, /*select_mode_closed*/ true, + ), + Some(key) + ); + assert_eq!(state.pending_select_exit_key, None); + } + fn turn(turn_id: &str, status: TurnStatus, user_messages: usize) -> Turn { Turn { id: turn_id.to_string(), diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 90e854d128..ce17880041 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -129,6 +129,8 @@ //! Multiline pastes continue blockquote prefixes. //! `Enter` submits immediately. `Tab` requests queuing while a task is running; if no task is //! running, `Tab` submits just like Enter so input is never dropped. +//! Submit and queue bindings act only on fresh key presses; repeated events are consumed so a held +//! key cannot dispatch a preserved draft after another input layer closes. //! Vim Replace shares Insert's composer actions; only textarea editing differs. //! Literal completion/paste edits discard stale Replace recovery; subsequent typing records anew. //! Token markers added during Replace are removed when restoring overwritten characters. @@ -3528,6 +3530,11 @@ impl ChatComposer { } else { self.footer.mode = reset_mode_after_activity(self.footer.mode); } + if key_event.kind == KeyEventKind::Repeat + && (self.queue_keys.is_pressed(key_event) || self.submit_keys.is_pressed(key_event)) + { + return (InputResult::None, true); + } if self.queue_keys.is_pressed(key_event) && (self.is_task_running || self.queue_submissions || !self.is_bang_shell_command()) { diff --git a/codex-rs/tui/src/keymap.rs b/codex-rs/tui/src/keymap.rs index 884cfe34ba..6ac0a2678a 100644 --- a/codex-rs/tui/src/keymap.rs +++ b/codex-rs/tui/src/keymap.rs @@ -66,6 +66,8 @@ pub(crate) use chords::KeyChordMatch; pub(crate) use chords::KeyChordMatcher; pub(crate) use chords::KeymapContextSet; pub(crate) use chords::RuntimeChordKeymap; +pub(crate) use chords::chord_stroke_matches; +pub(crate) use chords::dispatch_event; pub(crate) use chords::is_dispatch_token; pub(crate) use chords::is_dispatch_token_event; diff --git a/codex-rs/tui/src/keymap/chords.rs b/codex-rs/tui/src/keymap/chords.rs index 40733cf61b..cebea5f85e 100644 --- a/codex-rs/tui/src/keymap/chords.rs +++ b/codex-rs/tui/src/keymap/chords.rs @@ -404,6 +404,8 @@ Reduce `tui.keymap.global.leader` alternatives or `leader ...` bindings." !(contexts.0 & TRANSCRIPT_CLOSE != 0 && binding.action.context == KeymapContext::Pager && binding.action.action == "close_transcript"), + !(contexts.0 & TRANSCRIPT_ACTIONS != 0 + && binding.action.context == KeymapContext::Pager), contexts.is_warnings() && binding.action.context != KeymapContext::List, contexts.is_warnings() && binding.action.context == KeymapContext::Global, ) @@ -690,7 +692,7 @@ pub(crate) fn is_dispatch_token_event(event: KeyEvent) -> bool { ) } -fn chord_stroke_matches(binding: KeyBinding, key_event: KeyEvent) -> bool { +pub(crate) fn chord_stroke_matches(binding: KeyBinding, key_event: KeyEvent) -> bool { let ctrl_slash = crate::key_hint::ctrl(KeyCode::Char('/')); let ctrl_seven = crate::key_hint::ctrl(KeyCode::Char('7')); diff --git a/codex-rs/tui/src/pager_overlay.rs b/codex-rs/tui/src/pager_overlay.rs index d77247cb78..48d4d9b216 100644 --- a/codex-rs/tui/src/pager_overlay.rs +++ b/codex-rs/tui/src/pager_overlay.rs @@ -92,7 +92,7 @@ impl Overlay { }; tui.set_overlay_input(input)?; let result = match self { - Overlay::Transcript(o) => o.handle_event(tui, event), + Overlay::Transcript(o) => o.handle_event(tui, event).map(drop), Overlay::Static(o) => o.handle_event(tui, event), Overlay::Analytics(o) => o.handle_event(tui, event), }; diff --git a/codex-rs/tui/src/pager_overlay/transcript.rs b/codex-rs/tui/src/pager_overlay/transcript.rs index 62e27083e5..c8af27c6e5 100644 --- a/codex-rs/tui/src/pager_overlay/transcript.rs +++ b/codex-rs/tui/src/pager_overlay/transcript.rs @@ -78,6 +78,7 @@ impl TranscriptOverlay { pub(crate) fn render(&mut self, area: Rect, buf: &mut Buffer) { Clear.render(area, buf); let browsing = self.browsing_footer.is_some() + && !self.pending_select_mode && !self.view.has_active_interaction() && self.view.history != TranscriptHistoryState::Failed; let chrome_height = if browsing { 2 } else { 5 }; @@ -190,10 +191,18 @@ impl TranscriptOverlay { Ok(()) } - pub(crate) fn handle_event(&mut self, tui: &mut tui::Tui, event: TuiEvent) -> Result<()> { - if let Some(completion) = tui.clipboard.poll() { + pub(crate) fn handle_event( + &mut self, + tui: &mut tui::Tui, + event: TuiEvent, + ) -> Result> { + let completed_copy = if let Some(completion) = tui.clipboard.poll() { + let id = completion.0; self.finish_pending_copy(completion); - } + Some(id) + } else { + None + }; if matches!(event, TuiEvent::Resume | TuiEvent::FocusLost) { self.view.cancel_primary(); self.view.end_drag(); @@ -219,7 +228,7 @@ impl TranscriptOverlay { } TuiEvent::Paste(text) => { if self.view.is_select_mode() && !self.view.is_search_editing() { - return Ok(()); + return Ok(completed_copy); } if self.view.is_search_editing() { self.view.end_selection(&self.cells); @@ -232,7 +241,7 @@ impl TranscriptOverlay { .schedule_frame_in(crate::tui::TARGET_FRAME_INTERVAL); } self.draw(tui)?; - return Ok(()); + return Ok(completed_copy); } TuiEvent::FocusLost => None, }; @@ -240,7 +249,7 @@ impl TranscriptOverlay { self.apply_action(tui, action); tui.frame_requester().schedule_frame(); } - Ok(()) + Ok(completed_copy) } pub(crate) fn is_done(&self) -> bool { @@ -263,19 +272,27 @@ impl TranscriptOverlay { } pub(crate) fn begin_select_mode(&mut self) { - self.pending_select_mode = !self.view.begin_select_mode(&self.cells); + self.pending_select_mode = true; } pub(crate) fn is_search_editing(&self) -> bool { self.view.is_search_editing() } + pub(crate) fn is_select_mode(&self) -> bool { + self.pending_select_mode || self.view.is_select_mode() + } + pub(crate) fn owns_interaction_key(&self, key: KeyEvent) -> bool { - self.view.owns_interaction_key(key) + self.pending_select_mode || self.view.owns_interaction_key(key) } pub(crate) fn has_active_interaction(&self) -> bool { - self.view.has_active_interaction() + self.pending_select_mode || self.view.has_active_interaction() + } + + pub(crate) fn pending_follow_id(&self) -> Option { + self.pending_follow_close.map(|(id, _)| id) } pub(crate) fn needs_history(&mut self) -> bool { diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 2a573f1e97..97eed4a6e2 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -116,6 +116,10 @@ pub(crate) fn running_in_vscode_terminal() -> bool { keyboard_modes::running_in_vscode_terminal() } +pub(crate) fn reports_key_releases() -> bool { + keyboard_modes::reports_key_releases() +} + fn should_emit_notification(condition: NotificationCondition, terminal_focused: bool) -> bool { match condition { NotificationCondition::Unfocused => !terminal_focused, diff --git a/codex-rs/tui/src/tui/alternate_screen.rs b/codex-rs/tui/src/tui/alternate_screen.rs index 2313da69df..3d1fc5cb1e 100644 --- a/codex-rs/tui/src/tui/alternate_screen.rs +++ b/codex-rs/tui/src/tui/alternate_screen.rs @@ -33,6 +33,7 @@ pub(super) static ALTERNATE_SCREEN: AlternateScreen = AlternateScreen { mouse_capture_disabled: AtomicBool::new(/*v*/ false), input_configured: AtomicBool::new(/*v*/ false), keyboard_active: AtomicBool::new(/*v*/ false), + parent_reports_key_releases: AtomicBool::new(/*v*/ false), link_pointer: super::link_pointer::LinkPointer::new(), }; @@ -47,6 +48,8 @@ pub(super) struct AlternateScreen { input_configured: AtomicBool, // An editor can take over the screen after Codex has popped its keyboard mode. keyboard_active: AtomicBool, + // Restore the main screen's input classification when this screen's keyboard frame is popped. + parent_reports_key_releases: AtomicBool, } /// Report pointer motion so the owned transcript can update its return-to-bottom hover state. @@ -87,6 +90,8 @@ impl AlternateScreen { // Stdout retains queued bytes on a flush error; cleanup must follow that pending entry. self.active.store(/*val*/ true, Ordering::Relaxed); writer.flush()?; + self.parent_reports_key_releases + .store(keyboard_modes::reports_key_releases(), Ordering::Relaxed); let mouse_capture = keyboard_modes::enable_keyboard_enhancement(writer); self.keyboard_active.store( !cfg!(windows) && !keyboard_modes::keyboard_enhancement_disabled(), @@ -174,6 +179,9 @@ impl AlternateScreen { let result = execute!(writer, PopKeyboardEnhancementFlags); if result.is_ok() { self.keyboard_active.store(/*val*/ false, Ordering::Relaxed); + keyboard_modes::set_reports_key_releases( + self.parent_reports_key_releases.load(Ordering::Relaxed), + ); } result } else { diff --git a/codex-rs/tui/src/tui/keyboard_modes.rs b/codex-rs/tui/src/tui/keyboard_modes.rs index 4dddb97f78..abf8cdd584 100644 --- a/codex-rs/tui/src/tui/keyboard_modes.rs +++ b/codex-rs/tui/src/tui/keyboard_modes.rs @@ -8,6 +8,8 @@ use std::fmt; use std::io::Write; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; use codex_terminal_detection::TerminalName; use codex_terminal_detection::terminal_info; @@ -18,6 +20,7 @@ use crossterm::event::PushKeyboardEnhancementFlags; use ratatui::crossterm::execute; const DISABLE_KEYBOARD_ENHANCEMENT_ENV_VAR: &str = "CODEX_TUI_DISABLE_KEYBOARD_ENHANCEMENT"; +static REPORTS_KEY_RELEASES: AtomicBool = AtomicBool::new(/*v*/ false); #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum VscodeDetection { @@ -210,6 +213,7 @@ mod windows_term_program_tests; pub(super) fn enable_keyboard_enhancement(writer: &mut impl Write) -> super::tmux::MouseCapture { let tmux_options = super::tmux::options(); if keyboard_enhancement_disabled() { + set_reports_key_releases(/*reports*/ false); return tmux_options.mouse_capture; } @@ -220,14 +224,18 @@ pub(super) fn enable_keyboard_enhancement(writer: &mut impl Write) -> super::tmu None }; - let _ = execute!( + let flags = keyboard_enhancement_flags( + terminal_info().name, + running_in_tmux_session, + tmux_extended_keys_format, + ); + let result = execute!( writer, DisableModifyOtherKeys, - PushKeyboardEnhancementFlags(keyboard_enhancement_flags( - terminal_info().name, - running_in_tmux_session, - tmux_extended_keys_format - )) + PushKeyboardEnhancementFlags(flags) + ); + set_reports_key_releases( + result.is_ok() && flags.contains(KeyboardEnhancementFlags::REPORT_EVENT_TYPES), ); if tmux_should_enable_modify_other_keys_for(running_in_tmux_session, tmux_extended_keys_format) @@ -258,6 +266,14 @@ fn keyboard_enhancement_flags( } } +pub(super) fn reports_key_releases() -> bool { + REPORTS_KEY_RELEASES.load(Ordering::Relaxed) +} + +pub(super) fn set_reports_key_releases(reports: bool) { + REPORTS_KEY_RELEASES.store(reports, Ordering::Relaxed); +} + fn running_in_tmux_session() -> bool { tmux_session_detected( std::env::var("TMUX").ok().as_deref(), @@ -280,16 +296,22 @@ fn tmux_should_enable_modify_other_keys_for( } pub(super) fn restore_keyboard_enhancement_stack(writer: &mut impl Write) { - let _ = execute!(writer, PopKeyboardEnhancementFlags, DisableModifyOtherKeys); + if execute!(writer, PopKeyboardEnhancementFlags, DisableModifyOtherKeys).is_ok() { + set_reports_key_releases(/*reports*/ false); + } } pub(super) fn reset_keyboard_reporting_after_exit(writer: &mut impl Write) { - let _ = execute!( + if execute!( writer, PopKeyboardEnhancementFlags, ResetKeyboardEnhancementFlags, DisableModifyOtherKeys - ); + ) + .is_ok() + { + set_reports_key_releases(/*reports*/ false); + } } #[derive(Debug, Clone, Copy, PartialEq, Eq)]