From 4c65f47c2dae337467bb583fbc27fb055cc63da0 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 14 May 2026 15:15:45 -0700 Subject: [PATCH] tui: remove Esc side conversation dismissal Refs #22599 --- codex-rs/tui/src/app/input.rs | 21 +++++++- codex-rs/tui/src/app/platform_actions.rs | 32 ++++-------- codex-rs/tui/src/app/side.rs | 4 +- codex-rs/tui/src/app/tests.rs | 51 ++++++++++++++++++- codex-rs/tui/src/chatwidget/side.rs | 4 ++ codex-rs/tui/src/chatwidget/slash_dispatch.rs | 3 +- ...e_context_label_preserves_status_line.snap | 2 +- ...ide_context_label_shows_parent_status.snap | 2 +- codex-rs/tui/src/chatwidget/tests/side.rs | 8 +-- codex-rs/tui/src/keymap.rs | 5 +- ...rejection_reports_unavailable_message.snap | 5 ++ 11 files changed, 100 insertions(+), 37 deletions(-) create mode 100644 codex-rs/tui/src/snapshots/codex_tui__app__tests__side_backtrack_rejection_reports_unavailable_message.snap diff --git a/codex-rs/tui/src/app/input.rs b/codex-rs/tui/src/app/input.rs index 905f62f86f2a..f6530cad26a2 100644 --- a/codex-rs/tui/src/app/input.rs +++ b/codex-rs/tui/src/app/input.rs @@ -5,6 +5,9 @@ use super::*; +const SIDE_EDIT_PREVIOUS_UNAVAILABLE_MESSAGE: &str = + "Editing previous prompts is unavailable in side conversations."; + impl App { pub(super) async fn launch_external_editor(&mut self, tui: &mut tui::Tui) { let editor_cmd = match external_editor::resolve_editor_command() { @@ -197,6 +200,8 @@ impl App { // handles it. if self.should_handle_backtrack_esc(key_event) { self.handle_backtrack_esc_key(tui); + } else if self.should_reject_side_backtrack_esc(key_event) { + self.reject_side_backtrack_esc(); } else { self.chat_widget.handle_key_event(key_event); } @@ -252,11 +257,25 @@ impl App { } pub(super) fn should_handle_backtrack_esc(&self, key_event: KeyEvent) -> bool { - self.chat_widget.is_normal_backtrack_mode() + !self.chat_widget.side_conversation_active() + && self.chat_widget.is_normal_backtrack_mode() + && self.chat_widget.composer_is_empty() + && !self.chat_widget.should_handle_vim_insert_escape(key_event) + } + + pub(super) fn should_reject_side_backtrack_esc(&self, key_event: KeyEvent) -> bool { + self.chat_widget.side_conversation_active() + && self.chat_widget.is_normal_backtrack_mode() && self.chat_widget.composer_is_empty() && !self.chat_widget.should_handle_vim_insert_escape(key_event) } + pub(super) fn reject_side_backtrack_esc(&mut self) { + self.reset_backtrack_state(); + self.chat_widget + .add_error_message(SIDE_EDIT_PREVIOUS_UNAVAILABLE_MESSAGE.to_string()); + } + fn app_keymap_shortcuts_available(&self) -> bool { self.overlay.is_none() && self.chat_widget.no_modal_or_popup_active() } diff --git a/codex-rs/tui/src/app/platform_actions.rs b/codex-rs/tui/src/app/platform_actions.rs index f19ed0bb600b..11289bc2aaee 100644 --- a/codex-rs/tui/src/app/platform_actions.rs +++ b/codex-rs/tui/src/app/platform_actions.rs @@ -54,24 +54,16 @@ fn send_world_writable_scan_failed(tx: &AppEventSender) { } pub(super) fn side_return_shortcut_matches(key_event: KeyEvent) -> bool { - match key_event { - KeyEvent { - code: KeyCode::Esc, - kind: KeyEventKind::Press | KeyEventKind::Repeat, - .. - } => true, + matches!( + key_event, KeyEvent { code: KeyCode::Char(c), modifiers, kind: KeyEventKind::Press, .. } if modifiers.contains(KeyModifiers::CONTROL) - && (c.eq_ignore_ascii_case(&'c') || c.eq_ignore_ascii_case(&'d')) => - { - true - } - _ => false, - } + && (c.eq_ignore_ascii_case(&'c') || c.eq_ignore_ascii_case(&'d')) + ) } #[cfg(test)] @@ -79,16 +71,7 @@ mod tests { use super::*; #[test] - fn side_return_shortcuts_match_esc_ctrl_c_and_ctrl_d() { - assert!(side_return_shortcut_matches(KeyEvent::new( - KeyCode::Esc, - KeyModifiers::NONE, - ))); - assert!(side_return_shortcut_matches(KeyEvent::new_with_kind( - KeyCode::Esc, - KeyModifiers::NONE, - KeyEventKind::Repeat, - ))); + fn side_return_shortcuts_match_ctrl_c_and_ctrl_d() { assert!(side_return_shortcut_matches(KeyEvent::new( KeyCode::Char('c'), KeyModifiers::CONTROL, @@ -105,6 +88,11 @@ mod tests { KeyCode::Char('D'), KeyModifiers::CONTROL, ))); + assert!(!side_return_shortcut_matches(KeyEvent::new_with_kind( + KeyCode::Esc, + KeyModifiers::NONE, + KeyEventKind::Press, + ))); assert!(!side_return_shortcut_matches(KeyEvent::new_with_kind( KeyCode::Esc, KeyModifiers::NONE, diff --git a/codex-rs/tui/src/app/side.rs b/codex-rs/tui/src/app/side.rs index 6414e610476c..044cb9b3910d 100644 --- a/codex-rs/tui/src/app/side.rs +++ b/codex-rs/tui/src/app/side.rs @@ -20,7 +20,7 @@ const SIDE_NO_STARTED_CONVERSATION_MESSAGE: &str = concat!( "Send a message first, then try /side again." ); const SIDE_ALREADY_OPEN_MESSAGE: &str = - "A side conversation is already open. Press Esc to return before starting another."; + "A side conversation is already open. Press Ctrl+C to return before starting another."; const SIDE_BOUNDARY_PROMPT: &str = r#"Side conversation boundary. Everything before this boundary is inherited history from the parent thread. It is reference context only. It is not your current task. @@ -247,7 +247,7 @@ impl App { if let Some(parent_status) = parent_status { label_parts.push(parent_status.label(parent_is_main).to_string()); } - label_parts.push("Esc to return".to_string()); + label_parts.push("Ctrl+C to return".to_string()); self.chat_widget .set_side_conversation_context_label(Some(format!("Side {}", label_parts.join(" · ")))); } diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 2abecc3585a8..397f65403ee3 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -3168,7 +3168,9 @@ async fn side_start_block_message_tracks_open_side_conversation() { assert_eq!( app.side_start_block_message(), - Some("A side conversation is already open. Press Esc to return before starting another.") + Some( + "A side conversation is already open. Press Ctrl+C to return before starting another." + ) ); app.side_threads.remove(&side_thread_id); @@ -5247,3 +5249,50 @@ async fn backtrack_esc_does_not_steal_empty_vim_insert_escape() { assert!(!app.chat_widget.should_handle_vim_insert_escape(esc)); assert!(app.should_handle_backtrack_esc(esc)); } + +#[tokio::test] +async fn side_conversations_reject_backtrack_esc_without_stealing_vim_insert_escape() { + let mut app = make_test_app().await; + let esc = crossterm::event::KeyEvent::new(crossterm::event::KeyCode::Esc, KeyModifiers::NONE); + + app.chat_widget + .set_side_conversation_active(/*active*/ true); + assert!(app.chat_widget.composer_is_empty()); + assert!(!app.should_handle_backtrack_esc(esc)); + assert!(app.should_reject_side_backtrack_esc(esc)); + + app.chat_widget.toggle_vim_mode_and_notify(); + app.chat_widget + .handle_key_event(crossterm::event::KeyEvent::new( + crossterm::event::KeyCode::Char('i'), + KeyModifiers::NONE, + )); + + assert!(app.chat_widget.should_handle_vim_insert_escape(esc)); + assert!(!app.should_handle_backtrack_esc(esc)); + assert!(!app.should_reject_side_backtrack_esc(esc)); +} + +#[tokio::test] +async fn side_backtrack_rejection_reports_unavailable_message_snapshot() { + let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await; + app.backtrack.primed = true; + + app.reject_side_backtrack_esc(); + + assert!(!app.backtrack.primed); + let cell = match app_event_rx.try_recv() { + Ok(AppEvent::InsertHistoryCell(cell)) => cell, + other => panic!("expected InsertHistoryCell event, got {other:?}"), + }; + let rendered = cell + .display_lines(/*width*/ 80) + .into_iter() + .map(|line| line.to_string()) + .collect::>() + .join("\n"); + assert_app_snapshot!( + "side_backtrack_rejection_reports_unavailable_message", + rendered + ); +} diff --git a/codex-rs/tui/src/chatwidget/side.rs b/codex-rs/tui/src/chatwidget/side.rs index 0d6c2f5a8ed9..e379fe360628 100644 --- a/codex-rs/tui/src/chatwidget/side.rs +++ b/codex-rs/tui/src/chatwidget/side.rs @@ -25,6 +25,10 @@ impl ChatWidget { self.bottom_pane.set_side_conversation_active(active); } + pub(crate) fn side_conversation_active(&self) -> bool { + self.active_side_conversation + } + pub(crate) fn set_side_conversation_context_label(&mut self, label: Option) { self.bottom_pane.set_side_conversation_context_label(label); } diff --git a/codex-rs/tui/src/chatwidget/slash_dispatch.rs b/codex-rs/tui/src/chatwidget/slash_dispatch.rs index 41989c51748a..9d8d49a77cf0 100644 --- a/codex-rs/tui/src/chatwidget/slash_dispatch.rs +++ b/codex-rs/tui/src/chatwidget/slash_dispatch.rs @@ -32,7 +32,8 @@ struct PreparedSlashCommandArgs { const SIDE_STARTING_CONTEXT_LABEL: &str = "Side starting..."; const SIDE_REVIEW_UNAVAILABLE_MESSAGE: &str = "'/side' is unavailable while code review is running."; -const SIDE_SLASH_COMMAND_UNAVAILABLE_HINT: &str = "Press Esc to return to the main thread first."; +const SIDE_SLASH_COMMAND_UNAVAILABLE_HINT: &str = + "Press Ctrl+C to return to the main thread first."; const GOAL_USAGE: &str = "Usage: /goal "; const GOAL_USAGE_HINT: &str = "Example: /goal improve benchmark coverage"; const RAW_USAGE: &str = "Usage: /raw [on|off]"; diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_preserves_status_line.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_preserves_status_line.snap index 168dca809156..5757bf8dacfc 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_preserves_status_line.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_preserves_status_line.snap @@ -6,4 +6,4 @@ expression: terminal.backend() " " "› Check recently modified functions for compatibility " " " -" gpt-5.5 Side from main thread · Esc to return " +" gpt-5.5 Side from main thread · Ctrl+C to return " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_shows_parent_status.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_shows_parent_status.snap index 1f140765733d..a1b8af6052cf 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_shows_parent_status.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__side_context_label_shows_parent_status.snap @@ -6,4 +6,4 @@ expression: terminal.backend() " " "› Check recently modified functions for compatibility " " " -" gpt-5.5 default · … Side from main thread · main needs input · Esc to return " +" gpt-5.5 default… Side from main thread · main needs input · Ctrl+C to return " diff --git a/codex-rs/tui/src/chatwidget/tests/side.rs b/codex-rs/tui/src/chatwidget/tests/side.rs index 2f2a6d33561b..906deb3b566d 100644 --- a/codex-rs/tui/src/chatwidget/tests/side.rs +++ b/codex-rs/tui/src/chatwidget/tests/side.rs @@ -106,7 +106,7 @@ async fn slash_commands_without_side_flag_are_rejected_for_side_threads() { let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 80)); assert!( rendered.contains( - "'/review' is unavailable in side conversations. Press Esc to return to the main thread first." + "'/review' is unavailable in side conversations. Press Ctrl+C to return to the main thread first." ), "expected side conversation slash command error, got {rendered:?}" ); @@ -132,7 +132,7 @@ async fn slash_side_is_rejected_for_side_threads() { let rendered = lines_to_single_string(&cell.display_lines(/*width*/ 80)); assert!( rendered.contains( - "'/side' is unavailable in side conversations. Press Esc to return to the main thread first." + "'/side' is unavailable in side conversations. Press Ctrl+C to return to the main thread first." ), "expected side conversation slash command error, got {rendered:?}" ); @@ -276,7 +276,7 @@ async fn side_context_label_preserves_status_line_snapshot() { chat.refresh_status_line(); chat.set_side_conversation_active(/*active*/ true); chat.set_side_conversation_context_label(Some( - "Side from main thread · Esc to return".to_string(), + "Side from main thread · Ctrl+C to return".to_string(), )); let width = 80; @@ -297,7 +297,7 @@ async fn side_context_label_shows_parent_status_snapshot() { chat.show_welcome_banner = false; chat.set_side_conversation_active(/*active*/ true); chat.set_side_conversation_context_label(Some( - "Side from main thread · main needs input · Esc to return".to_string(), + "Side from main thread · main needs input · Ctrl+C to return".to_string(), )); let width = 80; diff --git a/codex-rs/tui/src/keymap.rs b/codex-rs/tui/src/keymap.rs index fbac16096722..533e28f3a23b 100644 --- a/codex-rs/tui/src/keymap.rs +++ b/codex-rs/tui/src/keymap.rs @@ -1445,10 +1445,7 @@ const MAIN_RESERVED_BINDINGS: &[(&str, KeyBinding)] = &[ "fixed.cycle_collaboration_mode", key_hint::shift(KeyCode::Tab), ), - ( - "fixed.return_from_side_or_backtrack", - key_hint::plain(KeyCode::Esc), - ), + ("fixed.backtrack", key_hint::plain(KeyCode::Esc)), ("fixed.previous_agent", key_hint::alt(KeyCode::Left)), ("fixed.next_agent", key_hint::alt(KeyCode::Right)), ("fixed.slash_command", key_hint::plain(KeyCode::Char('/'))), diff --git a/codex-rs/tui/src/snapshots/codex_tui__app__tests__side_backtrack_rejection_reports_unavailable_message.snap b/codex-rs/tui/src/snapshots/codex_tui__app__tests__side_backtrack_rejection_reports_unavailable_message.snap new file mode 100644 index 000000000000..841e0c0f59d7 --- /dev/null +++ b/codex-rs/tui/src/snapshots/codex_tui__app__tests__side_backtrack_rejection_reports_unavailable_message.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/app/tests.rs +expression: rendered +--- +■ Editing previous prompts is unavailable in side conversations.