From 1aa3a68e7b863557ae36ba5dc5f77e19a453059a Mon Sep 17 00:00:00 2001 From: Benjamin Carlsson Date: Mon, 17 Aug 2026 19:21:53 +0000 Subject: [PATCH] Restrict queued-message editing to its dedicated binding (#39064) ## What changed - Stop treating Vim normal-mode history-up as an alias for editing the latest queued message. - Continue to edit queued messages through the configured `chat.edit_queued_message` binding. GitOrigin-RevId: d830e0976a38725e948d420c1a5f374bb13fe945 --- codex-rs/tui/src/bottom_pane/chat_composer.rs | 5 -- codex-rs/tui/src/bottom_pane/mod.rs | 6 +- codex-rs/tui/src/chatwidget/interaction.rs | 13 +-- .../chatwidget/tests/composer_submission.rs | 86 ------------------- 4 files changed, 2 insertions(+), 108 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 29bffa9f13..9d9ef7d2bb 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -44,11 +44,6 @@ //! - Persistent cross-session history (text-only; no element ranges or attachments). //! - Local in-session history (full text + text elements + local/remote image attachments). //! -//! Before composer history handles Vim-normal history-up, `ChatWidget` restores the latest queued -//! follow-up when the composer is empty and no popup is active. It removes the message from the -//! queue before placing it in the composer, so editing and requeuing it replaces the prior -//! revision instead of creating duplicates. -//! //! When recalling a local entry, the composer rehydrates text elements and both attachment kinds //! (local image paths + remote image URLs). //! When recalling a persistent entry, only the text is restored. diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 06dfdb2a6a..690024f93a 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -1222,12 +1222,8 @@ impl BottomPane { popup_consts::standard_popup_hint_line_for_keymap(&self.keymap.list) } - pub(crate) fn runtime_keymap(&self) -> &RuntimeKeymap { - &self.keymap - } - pub(crate) fn list_keymap(&self) -> crate::keymap::ListKeymap { - self.runtime_keymap().list.clone() + self.keymap.list.clone() } /// Replace one or more active views whose IDs are in `view_ids` with a diff --git a/codex-rs/tui/src/chatwidget/interaction.rs b/codex-rs/tui/src/chatwidget/interaction.rs index 8906506594..e61ceadc0d 100644 --- a/codex-rs/tui/src/chatwidget/interaction.rs +++ b/codex-rs/tui/src/chatwidget/interaction.rs @@ -111,18 +111,7 @@ impl ChatWidget { } if key_event.kind == KeyEventKind::Press - && (self.chat_keymap.edit_queued_message.is_pressed(key_event) - || (self.bottom_pane.composer_is_empty() - && self - .bottom_pane - .keymap_contexts() - .contains(crate::keymap::KeymapContext::VimNormal) - && self - .bottom_pane - .runtime_keymap() - .vim_normal - .move_up - .is_pressed(key_event))) + && self.chat_keymap.edit_queued_message.is_pressed(key_event) && self.has_queued_follow_up_messages() && self.bottom_pane.no_modal_or_popup_active() { diff --git a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs index 825fa4b98b..8728b224b7 100644 --- a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs +++ b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs @@ -1676,92 +1676,6 @@ async fn alt_up_edits_most_recent_queued_message() { ); } -#[tokio::test] -async fn vim_normal_history_up_edits_queued_message_without_duplicating_it() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; - chat.thread_id = Some(ThreadId::new()); - chat.toggle_vim_mode_and_notify(); - chat.bottom_pane.set_task_running(/*running*/ true); - chat.bottom_pane - .set_composer_text("first queued message".to_string(), Vec::new(), Vec::new()); - chat.handle_key_event(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE)); - - for expected in ["first queued message", "first queued message edited"] { - chat.handle_key_event(KeyEvent::new(KeyCode::Char('k'), KeyModifiers::NONE)); - - assert_eq!(chat.bottom_pane.composer_text(), expected); - assert!(chat.input_queue.queued_user_messages.is_empty()); - assert!( - chat.input_queue - .queued_user_message_history_records - .is_empty() - ); - - chat.bottom_pane - .set_composer_text(format!("{expected} edited"), Vec::new(), Vec::new()); - chat.handle_key_event(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE)); - assert_eq!(chat.input_queue.queued_user_messages.len(), 1); - assert_eq!( - chat.input_queue.queued_user_message_history_records.len(), - 1 - ); - } - - assert_eq!( - chat.queued_user_message_texts(), - vec!["first queued message edited edited".to_string()] - ); -} - -#[tokio::test] -async fn vim_normal_queued_message_edit_uses_remapped_history_up() { - let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; - let mut keymap = crate::keymap::RuntimeKeymap::defaults(); - keymap.vim_normal.move_up = vec![crate::key_hint::plain(KeyCode::F(2))]; - chat.bottom_pane.set_keymap_bindings(&keymap); - chat.toggle_vim_mode_and_notify(); - chat.bottom_pane.set_task_running(/*running*/ true); - chat.input_queue - .queued_user_messages - .push_back(UserMessage::from("queued message").into()); - chat.input_queue - .queued_user_message_history_records - .push_back(UserMessageHistoryRecord::UserMessageText); - chat.refresh_pending_input_preview(); - - chat.handle_key_event(KeyEvent::new(KeyCode::Char('k'), KeyModifiers::NONE)); - - assert!(chat.bottom_pane.composer_is_empty()); - assert_eq!(chat.input_queue.queued_user_messages.len(), 1); - - chat.handle_key_event(KeyEvent::new(KeyCode::F(2), KeyModifiers::NONE)); - - assert_eq!(chat.bottom_pane.composer_text(), "queued message"); - assert!(chat.input_queue.queued_user_messages.is_empty()); - assert!( - chat.input_queue - .queued_user_message_history_records - .is_empty() - ); - - let width = 80; - let height = chat.desired_height(width); - let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(width, height)) - .expect("create terminal"); - terminal - .draw(|frame| chat.render(frame.area(), frame.buffer_mut())) - .expect("render restored queued message"); - insta::assert_snapshot!( - normalized_backend_snapshot(terminal.backend()) - .lines() - .filter(|line| line.contains("queued message")) - .map(|line| line.trim_matches('"').trim()) - .collect::>() - .join("\n"), - @"› queued message" - ); -} - #[tokio::test] async fn unbound_queued_message_edit_does_not_fall_back_to_alt_up() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;