diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 8bf46bdc666..38376a5278e 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -25,6 +25,8 @@ //! //! When recalling a local entry, the composer rehydrates text elements and image attachments. //! When recalling a persistent entry, only the text is restored. +//! Recalled entries move the cursor to end-of-line so repeated Up/Down presses keep shell-like +//! history traversal semantics instead of dropping to column 0. //! //! # Submission and Prompt Expansion //! @@ -532,9 +534,12 @@ impl ChatComposer { self.history.set_metadata(log_id, entry_count); } - /// Integrate an asynchronous response to an on-demand history lookup. If - /// the entry is present and the offset matches the current cursor we - /// immediately populate the textarea. + /// Integrate an asynchronous response to an on-demand history lookup. + /// + /// If the entry is present and the offset still matches the active history cursor, the + /// composer rehydrates the entry immediately. This path intentionally routes through + /// [`Self::apply_history_entry`] so cursor placement remains aligned with keyboard history + /// recall semantics. pub(crate) fn on_history_entry_response( &mut self, log_id: u64, @@ -783,6 +788,10 @@ impl ChatComposer { /// draft; if callers restore only text and images, mentions can appear /// intact to users while resolving to the wrong target or dropping on /// retry. + /// + /// This helper intentionally places the cursor at the start of the restored text. Callers + /// that need end-of-line restore behavior (for example shell-style history recall) should call + /// [`Self::move_cursor_to_end`] after this method. pub(crate) fn set_text_content_with_mention_bindings( &mut self, text: String, @@ -857,6 +866,13 @@ impl ChatComposer { self.textarea.text().to_string() } + /// Rehydrate a history entry into the composer with shell-like cursor placement. + /// + /// This path restores text, elements, images, mention bindings, and pending paste payloads, + /// then moves the cursor to end-of-line. If a caller reused + /// [`Self::set_text_content_with_mention_bindings`] directly for history recall and forgot the + /// final cursor move, repeated Up/Down would stop navigating history because cursor-gating + /// treats interior positions as normal editing mode. fn apply_history_entry(&mut self, entry: HistoryEntry) { let HistoryEntry { text, @@ -872,6 +888,7 @@ impl ChatComposer { mention_bindings, ); self.set_pending_pastes(pending_pastes); + self.move_cursor_to_end(); } pub(crate) fn text_elements(&self) -> Vec { @@ -5973,6 +5990,51 @@ mod tests { assert_eq!(text_elements.len(), 1); assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]")); assert_eq!(composer.local_image_paths(), vec![path]); + assert_eq!(composer.textarea.cursor(), composer.textarea.text().len()); + } + + #[test] + fn history_navigation_leaves_cursor_at_end_of_line() { + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + composer.set_steer_enabled(true); + + type_chars_humanlike(&mut composer, &['f', 'i', 'r', 's', 't']); + let (result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!(matches!(result, InputResult::Submitted { .. })); + + type_chars_humanlike(&mut composer, &['s', 'e', 'c', 'o', 'n', 'd']); + let (result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!(matches!(result, InputResult::Submitted { .. })); + + let (_result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); + assert_eq!(composer.textarea.text(), "second"); + assert_eq!(composer.textarea.cursor(), composer.textarea.text().len()); + + let (_result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); + assert_eq!(composer.textarea.text(), "first"); + assert_eq!(composer.textarea.cursor(), composer.textarea.text().len()); + + let (_result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)); + assert_eq!(composer.textarea.text(), "second"); + assert_eq!(composer.textarea.cursor(), composer.textarea.text().len()); + + let (_result, _needs_redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)); + assert!(composer.textarea.is_empty()); + assert_eq!(composer.textarea.cursor(), composer.textarea.text().len()); } #[test] diff --git a/codex-rs/tui/src/bottom_pane/chat_composer_history.rs b/codex-rs/tui/src/bottom_pane/chat_composer_history.rs index d080b79369c..e1c1d32ca1f 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer_history.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer_history.rs @@ -82,7 +82,8 @@ pub(crate) struct ChatComposerHistory { /// The text that was last inserted into the composer as a result of /// history navigation. Used to decide if further Up/Down presses should be - /// treated as navigation versus normal cursor movement. + /// treated as navigation versus normal cursor movement, together with the + /// "cursor at line boundary" check in [`Self::should_handle_navigation`]. last_history_text: Option, } @@ -136,8 +137,16 @@ impl ChatComposerHistory { self.last_history_text = None; } - /// Should Up/Down key presses be interpreted as history navigation given - /// the current content and cursor position of `textarea`? + /// Returns whether Up/Down should navigate history for the current textarea state. + /// + /// Empty text always enables history traversal. For non-empty text, this requires both: + /// + /// - the current text exactly matching the last recalled history entry, and + /// - the cursor being at a line boundary (start or end). + /// + /// This boundary gate keeps multiline cursor movement usable while preserving shell-like + /// history recall. If callers moved the cursor into the middle of a recalled entry and still + /// forced navigation, users would lose normal vertical movement within the draft. pub fn should_handle_navigation(&self, text: &str, cursor: usize) -> bool { if self.history_entry_count == 0 && self.local_history.is_empty() { return false; @@ -147,10 +156,11 @@ impl ChatComposerHistory { return true; } - // Textarea is not empty – only navigate when cursor is at start and - // text matches last recalled history entry so regular editing is not - // hijacked. - if cursor != 0 { + // Textarea is not empty – only navigate when text matches the last + // recalled history entry and the cursor is at a line boundary. This + // keeps shell-like Up/Down recall working while still allowing normal + // multiline cursor movement from interior positions. + if cursor != 0 && cursor != text.len() { return false; } @@ -378,4 +388,16 @@ mod tests { history.navigate_up(&tx) ); } + + #[test] + fn should_handle_navigation_when_cursor_is_at_line_boundaries() { + let mut history = ChatComposerHistory::new(); + history.record_local_submission(HistoryEntry::new("hello".to_string())); + history.last_history_text = Some("hello".to_string()); + + assert!(history.should_handle_navigation("hello", 0)); + assert!(history.should_handle_navigation("hello", "hello".len())); + assert!(!history.should_handle_navigation("hello", 1)); + assert!(!history.should_handle_navigation("other", 0)); + } } diff --git a/docs/tui-chat-composer.md b/docs/tui-chat-composer.md index 1e84a1b13b3..e4cf0c1b0c3 100644 --- a/docs/tui-chat-composer.md +++ b/docs/tui-chat-composer.md @@ -144,6 +144,10 @@ Local history entries capture: Persistent history entries only restore text. They intentionally do **not** rehydrate attachments or pending paste payloads. +For non-empty drafts, Up/Down navigation is only treated as history recall when the current text +matches the last recalled history entry and the cursor is at a boundary (start or end of the +line). This keeps multiline cursor movement intact while preserving shell-like history traversal. + ### Draft recovery (Ctrl+C) Ctrl+C clears the composer but stashes the full draft state (text elements, image paths, and @@ -157,6 +161,7 @@ ranges and local image paths. Pending paste payloads are cleared during submissi placeholders are expanded into their full text before being recorded. This means: - Up/Down recall of a submitted message restores image placeholders and their local paths. +- Recalled entries place the cursor at end-of-line to match typical shell history editing. - Large-paste placeholders are not expected in recalled submitted history; the text is the expanded paste content.