From 27da8a68d37115d104785c062fca5fd5f5568acc Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Wed, 14 Jan 2026 12:28:18 -0800 Subject: [PATCH] fix(tui): disable double-press quit shortcut (#9220) Disables the default Ctrl+C/Ctrl+D double-press quit UX (keeps the code path behind a const) while we rethink the quit/interrupt flow. Tests: - just fmt - cargo clippy --fix --all-features --tests --allow-dirty --allow-no-vcs -p codex-tui - cargo test -p codex-tui --lib --- codex-rs/tui/src/bottom_pane/mod.rs | 15 +++++++++++-- codex-rs/tui/src/chatwidget.rs | 32 ++++++++++++++++++++++----- codex-rs/tui/src/chatwidget/tests.rs | 32 ++++----------------------- codex-rs/tui2/src/bottom_pane/mod.rs | 15 +++++++++++-- codex-rs/tui2/src/chatwidget.rs | 32 ++++++++++++++++++++++----- codex-rs/tui2/src/chatwidget/tests.rs | 32 ++++----------------------- 6 files changed, 88 insertions(+), 70 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 34aa2f958..f270145a0 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -71,6 +71,13 @@ pub(crate) use feedback_view::FeedbackNoteView; /// Keeping a single value ensures Ctrl+C and Ctrl+D behave identically. pub(crate) const QUIT_SHORTCUT_TIMEOUT: Duration = Duration::from_secs(1); +/// Whether Ctrl+C/Ctrl+D require a second press to quit. +/// +/// This UX experiment was enabled by default, but requiring a double press to quit feels janky in +/// practice (especially for users accustomed to shells and other TUIs). Disable it for now while we +/// rethink a better quit/interrupt design. +pub(crate) const DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED: bool = false; + /// The result of offering a cancellation key to a bottom-pane surface. /// /// This is primarily used for Ctrl+C routing: active views can consume the key to dismiss @@ -359,6 +366,10 @@ impl BottomPane { /// after [`QUIT_SHORTCUT_TIMEOUT`] so the hint disappears even if the user /// stops typing and no other events trigger a draw. pub(crate) fn show_quit_shortcut_hint(&mut self, key: KeyBinding) { + if !DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED { + return; + } + self.composer .show_quit_shortcut_hint(key, self.has_input_focus); let frame_requester = self.frame_requester.clone(); @@ -695,7 +706,7 @@ mod tests { } #[test] - fn ctrl_c_on_modal_consumes_and_shows_quit_hint() { + fn ctrl_c_on_modal_consumes_without_showing_quit_hint() { let (tx_raw, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); let features = Features::with_defaults(); @@ -711,7 +722,7 @@ mod tests { }); pane.push_approval_request(exec_request(), &features); assert_eq!(CancellationEvent::Handled, pane.on_ctrl_c()); - assert!(pane.quit_shortcut_hint_visible()); + assert!(!pane.quit_shortcut_hint_visible()); assert_eq!(CancellationEvent::NotHandled, pane.on_ctrl_c()); } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 6030e5b7e..9050b4758 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -124,6 +124,7 @@ use crate::bottom_pane::BetaFeatureItem; use crate::bottom_pane::BottomPane; use crate::bottom_pane::BottomPaneParams; use crate::bottom_pane::CancellationEvent; +use crate::bottom_pane::DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED; use crate::bottom_pane::ExperimentalFeaturesView; use crate::bottom_pane::InputResult; use crate::bottom_pane::QUIT_SHORTCUT_TIMEOUT; @@ -3989,12 +3990,23 @@ impl ChatWidget { let key = key_hint::ctrl(KeyCode::Char('c')); let modal_or_popup_active = !self.bottom_pane.no_modal_or_popup_active(); if self.bottom_pane.on_ctrl_c() == CancellationEvent::Handled { - if modal_or_popup_active { - self.quit_shortcut_expires_at = None; - self.quit_shortcut_key = None; - self.bottom_pane.clear_quit_shortcut_hint(); + if DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED { + if modal_or_popup_active { + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; + self.bottom_pane.clear_quit_shortcut_hint(); + } else { + self.arm_quit_shortcut(key); + } + } + return; + } + + if !DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED { + if self.is_cancellable_work_active() { + self.submit_op(Op::Interrupt); } else { - self.arm_quit_shortcut(key); + self.request_quit_without_confirmation(); } return; } @@ -4019,6 +4031,16 @@ impl ChatWidget { /// Otherwise it should be routed to the active view and not attempt to quit. fn on_ctrl_d(&mut self) -> bool { let key = key_hint::ctrl(KeyCode::Char('d')); + if !DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED { + if !self.bottom_pane.composer_is_empty() || !self.bottom_pane.no_modal_or_popup_active() + { + return false; + } + + self.request_quit_without_confirmation(); + return true; + } + if self.quit_shortcut_active_for(key) { self.quit_shortcut_expires_at = None; self.quit_shortcut_key = None; diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index ff06e0d73..78000adff 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -1109,46 +1109,22 @@ async fn streaming_final_answer_keeps_task_running_state() { Ok(Op::Interrupt) => {} other => panic!("expected Op::Interrupt, got {other:?}"), } - assert!(chat.bottom_pane.quit_shortcut_hint_visible()); + assert!(!chat.bottom_pane.quit_shortcut_hint_visible()); } #[tokio::test] -async fn ctrl_c_shutdown_ignores_caps_lock() { +async fn ctrl_c_shutdown_works_with_caps_lock() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; chat.handle_key_event(KeyEvent::new(KeyCode::Char('C'), KeyModifiers::CONTROL)); - assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); - - chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL)); - assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst))); } #[tokio::test] -async fn ctrl_d_double_press_quits_without_prompt() { +async fn ctrl_d_quits_without_prompt() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; - chat.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL)); - assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); - - let width = 100; - let height = chat.desired_height(width); - let mut terminal = - ratatui::Terminal::new(VT100Backend::new(width, height)).expect("create terminal"); - terminal.set_viewport_area(Rect::new(0, 0, width, height)); - terminal - .draw(|f| chat.render(f.area(), f.buffer_mut())) - .expect("draw after ctrl+d"); - assert!( - terminal - .backend() - .vt100() - .screen() - .contents() - .contains("ctrl + d again to quit") - ); - chat.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL)); assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst))); } @@ -1179,7 +1155,7 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() { chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL)); assert!(chat.bottom_pane.composer_text().is_empty()); assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty)); - assert!(chat.bottom_pane.quit_shortcut_hint_visible()); + assert!(!chat.bottom_pane.quit_shortcut_hint_visible()); chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); let restored_text = chat.bottom_pane.composer_text(); diff --git a/codex-rs/tui2/src/bottom_pane/mod.rs b/codex-rs/tui2/src/bottom_pane/mod.rs index c67129c5b..df9c0d84c 100644 --- a/codex-rs/tui2/src/bottom_pane/mod.rs +++ b/codex-rs/tui2/src/bottom_pane/mod.rs @@ -68,6 +68,13 @@ pub(crate) use feedback_view::FeedbackNoteView; /// Keeping a single value ensures Ctrl+C and Ctrl+D behave identically. pub(crate) const QUIT_SHORTCUT_TIMEOUT: Duration = Duration::from_secs(1); +/// Whether Ctrl+C/Ctrl+D require a second press to quit. +/// +/// This UX experiment was enabled by default, but requiring a double press to quit feels janky in +/// practice (especially for users accustomed to shells and other TUIs). Disable it for now while we +/// rethink a better quit/interrupt design. +pub(crate) const DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED: bool = false; + /// The result of offering a cancellation key to a bottom-pane surface. /// /// This is primarily used for Ctrl+C routing: active views can consume the key to dismiss @@ -337,6 +344,10 @@ impl BottomPane { /// after [`QUIT_SHORTCUT_TIMEOUT`] so the hint disappears even if the user /// stops typing and no other events trigger a draw. pub(crate) fn show_quit_shortcut_hint(&mut self, key: KeyBinding) { + if !DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED { + return; + } + self.composer .show_quit_shortcut_hint(key, self.has_input_focus); let frame_requester = self.frame_requester.clone(); @@ -681,7 +692,7 @@ mod tests { } #[test] - fn ctrl_c_on_modal_consumes_and_shows_quit_hint() { + fn ctrl_c_on_modal_consumes_without_showing_quit_hint() { let (tx_raw, _rx) = unbounded_channel::(); let tx = AppEventSender::new(tx_raw); let features = Features::with_defaults(); @@ -697,7 +708,7 @@ mod tests { }); pane.push_approval_request(exec_request(), &features); assert_eq!(CancellationEvent::Handled, pane.on_ctrl_c()); - assert!(pane.quit_shortcut_hint_visible()); + assert!(!pane.quit_shortcut_hint_visible()); assert_eq!(CancellationEvent::NotHandled, pane.on_ctrl_c()); } diff --git a/codex-rs/tui2/src/chatwidget.rs b/codex-rs/tui2/src/chatwidget.rs index f51a9d55c..c103e058a 100644 --- a/codex-rs/tui2/src/chatwidget.rs +++ b/codex-rs/tui2/src/chatwidget.rs @@ -119,6 +119,7 @@ use crate::bottom_pane::ApprovalRequest; use crate::bottom_pane::BottomPane; use crate::bottom_pane::BottomPaneParams; use crate::bottom_pane::CancellationEvent; +use crate::bottom_pane::DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED; use crate::bottom_pane::InputResult; use crate::bottom_pane::QUIT_SHORTCUT_TIMEOUT; use crate::bottom_pane::SelectionAction; @@ -3662,12 +3663,23 @@ impl ChatWidget { let key = key_hint::ctrl(KeyCode::Char('c')); let modal_or_popup_active = !self.bottom_pane.no_modal_or_popup_active(); if self.bottom_pane.on_ctrl_c() == CancellationEvent::Handled { - if modal_or_popup_active { - self.quit_shortcut_expires_at = None; - self.quit_shortcut_key = None; - self.bottom_pane.clear_quit_shortcut_hint(); + if DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED { + if modal_or_popup_active { + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; + self.bottom_pane.clear_quit_shortcut_hint(); + } else { + self.arm_quit_shortcut(key); + } + } + return; + } + + if !DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED { + if self.is_cancellable_work_active() { + self.submit_op(Op::Interrupt); } else { - self.arm_quit_shortcut(key); + self.request_quit_without_confirmation(); } return; } @@ -3692,6 +3704,16 @@ impl ChatWidget { /// Otherwise it should be routed to the active view and not attempt to quit. fn on_ctrl_d(&mut self) -> bool { let key = key_hint::ctrl(KeyCode::Char('d')); + if !DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED { + if !self.bottom_pane.composer_is_empty() || !self.bottom_pane.no_modal_or_popup_active() + { + return false; + } + + self.request_quit_without_confirmation(); + return true; + } + if self.quit_shortcut_active_for(key) { self.quit_shortcut_expires_at = None; self.quit_shortcut_key = None; diff --git a/codex-rs/tui2/src/chatwidget/tests.rs b/codex-rs/tui2/src/chatwidget/tests.rs index a8c7f86f5..82cf4b0ea 100644 --- a/codex-rs/tui2/src/chatwidget/tests.rs +++ b/codex-rs/tui2/src/chatwidget/tests.rs @@ -1061,46 +1061,22 @@ async fn streaming_final_answer_keeps_task_running_state() { Ok(Op::Interrupt) => {} other => panic!("expected Op::Interrupt, got {other:?}"), } - assert!(chat.bottom_pane.quit_shortcut_hint_visible()); + assert!(!chat.bottom_pane.quit_shortcut_hint_visible()); } #[tokio::test] -async fn ctrl_c_shutdown_ignores_caps_lock() { +async fn ctrl_c_shutdown_works_with_caps_lock() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; chat.handle_key_event(KeyEvent::new(KeyCode::Char('C'), KeyModifiers::CONTROL)); - assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); - - chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL)); - assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst))); } #[tokio::test] -async fn ctrl_d_double_press_quits_without_prompt() { +async fn ctrl_d_quits_without_prompt() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await; - chat.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL)); - assert_matches!(rx.try_recv(), Err(TryRecvError::Empty)); - - let width = 100; - let height = chat.desired_height(width); - let mut terminal = - ratatui::Terminal::new(VT100Backend::new(width, height)).expect("create terminal"); - terminal.set_viewport_area(Rect::new(0, 0, width, height)); - terminal - .draw(|f| chat.render(f.area(), f.buffer_mut())) - .expect("draw after ctrl+d"); - assert!( - terminal - .backend() - .vt100() - .screen() - .contents() - .contains("ctrl + d again to quit") - ); - chat.handle_key_event(KeyEvent::new(KeyCode::Char('d'), KeyModifiers::CONTROL)); assert_matches!(rx.try_recv(), Ok(AppEvent::Exit(ExitMode::ShutdownFirst))); } @@ -1131,7 +1107,7 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() { chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL)); assert!(chat.bottom_pane.composer_text().is_empty()); assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty)); - assert!(chat.bottom_pane.quit_shortcut_hint_visible()); + assert!(!chat.bottom_pane.quit_shortcut_hint_visible()); chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); let restored_text = chat.bottom_pane.composer_text();