Restore image attachments/text elements when recalling input history (Up/Down)

codex-rs/tui/src/bottom_pane/chat_composer.rs

@@ -15,6 +15,16 @@
 //! [`ChatComposer::handle_key_event_without_popup`]. After every handled key, we call
 //! [`ChatComposer::sync_popups`] so UI state follows the latest buffer/cursor.
 //!
+//! # History Navigation (↑/↓)
+//!
+//! The Up/Down history path is managed by [`ChatComposerHistory`]. It merges:
+//!
+//! - Persistent cross-session history (text-only; no element ranges or attachments).
+//! - Local in-session history (full text + text elements + local image paths).
+//!
+//! When recalling a local entry, the composer rehydrates text elements and image attachments.
+//! When recalling a persistent entry, only the text is restored.
+//!
 //! # Submission and Prompt Expansion
 //!
 //! On submit/queue paths, the composer:
@@ -89,6 +99,7 @@ use ratatui::widgets::StatefulWidgetRef;
 use ratatui::widgets::WidgetRef;
 
 use super::chat_composer_history::ChatComposerHistory;
+use super::chat_composer_history::HistoryEntry;
 use super::command_popup::CommandItem;
 use super::command_popup::CommandPopup;
 use super::command_popup::CommandPopupFlags;
@@ -460,11 +471,12 @@ impl ChatComposer {
         offset: usize,
         entry: Option<String>,
     ) -> bool {
-        let Some(text) = self.history.on_entry_response(log_id, offset, entry) else {
+        let Some(entry) = self.history.on_entry_response(log_id, offset, entry) else {
             return false;
         };
-        // Composer history (↑/↓) stores plain text only; no UI element ranges/attachments to restore here.
-        self.set_text_content(text, Vec::new(), Vec::new());
+        // Persistent ↑/↓ history is text-only (backwards-compatible and avoids persisting
+        // attachments), but local in-session ↑/↓ history can rehydrate elements and image paths.
+        self.set_text_content(entry.text, entry.text_elements, entry.local_image_paths);
         true
     }
 
@@ -707,9 +719,19 @@ impl ChatComposer {
             return None;
         }
         let previous = self.current_text();
+        let text_elements = self.textarea.text_elements();
+        let local_image_paths = self
+            .attached_images
+            .iter()
+            .map(|img| img.path.clone())
+            .collect();
         self.set_text_content(String::new(), Vec::new(), Vec::new());
         self.history.reset_navigation();
-        self.history.record_local_submission(&previous);
+        self.history.record_local_submission(HistoryEntry {
+            text: previous.clone(),
+            text_elements,
+            local_image_paths,
+        });
         Some(previous)
     }
 
@@ -1792,8 +1814,17 @@ impl ChatComposer {
         if text.is_empty() && self.attached_images.is_empty() {
             return None;
         }
-        if !text.is_empty() {
-            self.history.record_local_submission(&text);
+        if !text.is_empty() || !self.attached_images.is_empty() {
+            let local_image_paths = self
+                .attached_images
+                .iter()
+                .map(|img| img.path.clone())
+                .collect();
+            self.history.record_local_submission(HistoryEntry {
+                text: text.clone(),
+                text_elements: text_elements.clone(),
+                local_image_paths,
+            });
         }
         self.pending_pastes.clear();
         Some((text, text_elements))
@@ -1989,15 +2020,19 @@ impl ChatComposer {
                     .history
                     .should_handle_navigation(self.textarea.text(), self.textarea.cursor())
                 {
-                    let replace_text = match key_event.code {
+                    let replace_entry = match key_event.code {
                         KeyCode::Up => self.history.navigate_up(&self.app_event_tx),
                         KeyCode::Down => self.history.navigate_down(&self.app_event_tx),
                         KeyCode::Char('p') => self.history.navigate_up(&self.app_event_tx),
                         KeyCode::Char('n') => self.history.navigate_down(&self.app_event_tx),
                         _ => unreachable!(),
                     };
-                    if let Some(text) = replace_text {
-                        self.set_text_content(text, Vec::new(), Vec::new());
+                    if let Some(entry) = replace_entry {
+                        self.set_text_content(
+                            entry.text,
+                            entry.text_elements,
+                            entry.local_image_paths,
+                        );
                         return (InputResult::None, true);
                     }
                 }
@@ -3255,7 +3290,7 @@ mod tests {
 
         assert_eq!(
             composer.history.navigate_up(&composer.app_event_tx),
-            Some("draft text".to_string())
+            Some(HistoryEntry::from_text("draft text".to_string()))
         );
     }
 
@@ -4708,6 +4743,37 @@ mod tests {
         assert_eq!(vec![path], imgs);
     }
 
+    #[test]
+    fn history_navigation_restores_image_attachments() {
+        let (tx, _rx) = unbounded_channel::<AppEvent>();
+        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);
+        let path = PathBuf::from("/tmp/image1.png");
+        composer.attach_image(path.clone());
+
+        let (result, _needs_redraw) =
+            composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
+        assert!(matches!(result, InputResult::Submitted { .. }));
+
+        composer.set_text_content(String::new(), Vec::new(), Vec::new());
+
+        let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE));
+
+        let text = composer.current_text();
+        assert_eq!(text, "[Image #1]");
+        let text_elements = composer.text_elements();
+        assert_eq!(text_elements.len(), 1);
+        assert_eq!(text_elements[0].placeholder(&text), Some("[Image #1]"));
+        assert_eq!(composer.local_image_paths(), vec![path]);
+    }
+
     #[test]
     fn set_text_content_reattaches_images_without_placeholder_metadata() {
         let (tx, _rx) = unbounded_channel::<AppEvent>();

codex-rs/tui/src/bottom_pane/chat_composer_history.rs

@@ -1,8 +1,35 @@
 use std::collections::HashMap;
+use std::path::PathBuf;
 
 use crate::app_event::AppEvent;
 use crate::app_event_sender::AppEventSender;
 use codex_core::protocol::Op;
+use codex_protocol::user_input::TextElement;
+
+#[derive(Debug, Clone, PartialEq)]
+pub(crate) struct HistoryEntry {
+    pub(crate) text: String,
+    pub(crate) text_elements: Vec<TextElement>,
+    pub(crate) local_image_paths: Vec<PathBuf>,
+}
+
+impl HistoryEntry {
+    fn empty() -> Self {
+        Self {
+            text: String::new(),
+            text_elements: Vec::new(),
+            local_image_paths: Vec::new(),
+        }
+    }
+
+    pub(crate) fn from_text(text: String) -> Self {
+        Self {
+            text,
+            text_elements: Vec::new(),
+            local_image_paths: Vec::new(),
+        }
+    }
+}
 
 /// State machine that manages shell-style history navigation (Up/Down) inside
 /// the chat composer. This struct is intentionally decoupled from the
@@ -15,10 +42,10 @@ pub(crate) struct ChatComposerHistory {
     history_entry_count: usize,
 
     /// Messages submitted by the user *during this UI session* (newest at END).
-    local_history: Vec<String>,
+    local_history: Vec<HistoryEntry>,
 
     /// Cache of persistent history entries fetched on-demand.
-    fetched_history: HashMap<usize, String>,
+    fetched_history: HashMap<usize, HistoryEntry>,
 
     /// Current cursor within the combined (persistent + local) history. `None`
     /// indicates the user is *not* currently browsing history.
@@ -54,20 +81,20 @@ impl ChatComposerHistory {
 
     /// Record a message submitted by the user in the current session so it can
     /// be recalled later.
-    pub fn record_local_submission(&mut self, text: &str) {
-        if text.is_empty() {
+    pub fn record_local_submission(&mut self, entry: HistoryEntry) {
+        if entry.text.is_empty() && entry.local_image_paths.is_empty() {
             return;
         }
 
         self.history_cursor = None;
         self.last_history_text = None;
 
         // Avoid inserting a duplicate if identical to the previous entry.
-        if self.local_history.last().is_some_and(|prev| prev == text) {
+        if self.local_history.last().is_some_and(|prev| prev == &entry) {
             return;
         }
 
-        self.local_history.push(text.to_string());
+        self.local_history.push(entry);
     }
 
     /// Reset navigation tracking so the next Up key resumes from the latest entry.
@@ -99,7 +126,7 @@ impl ChatComposerHistory {
 
     /// Handle <Up>. Returns true when the key was consumed and the caller
     /// should request a redraw.
-    pub fn navigate_up(&mut self, app_event_tx: &AppEventSender) -> Option<String> {
+    pub fn navigate_up(&mut self, app_event_tx: &AppEventSender) -> Option<HistoryEntry> {
         let total_entries = self.history_entry_count + self.local_history.len();
         if total_entries == 0 {
             return None;
@@ -116,7 +143,7 @@ impl ChatComposerHistory {
     }
 
     /// Handle <Down>.
-    pub fn navigate_down(&mut self, app_event_tx: &AppEventSender) -> Option<String> {
+    pub fn navigate_down(&mut self, app_event_tx: &AppEventSender) -> Option<HistoryEntry> {
         let total_entries = self.history_entry_count + self.local_history.len();
         if total_entries == 0 {
             return None;
@@ -137,7 +164,7 @@ impl ChatComposerHistory {
                 // Past newest – clear and exit browsing mode.
                 self.history_cursor = None;
                 self.last_history_text = None;
-                Some(String::new())
+                Some(HistoryEntry::empty())
             }
         }
     }
@@ -148,16 +175,17 @@ impl ChatComposerHistory {
         log_id: u64,
         offset: usize,
         entry: Option<String>,
-    ) -> Option<String> {
+    ) -> Option<HistoryEntry> {
         if self.history_log_id != Some(log_id) {
             return None;
         }
         let text = entry?;
-        self.fetched_history.insert(offset, text.clone());
+        let entry = HistoryEntry::from_text(text);
+        self.fetched_history.insert(offset, entry.clone());
 
         if self.history_cursor == Some(offset as isize) {
-            self.last_history_text = Some(text.clone());
-            return Some(text);
+            self.last_history_text = Some(entry.text.clone());
+            return Some(entry);
         }
         None
     }
@@ -170,19 +198,20 @@ impl ChatComposerHistory {
         &mut self,
         global_idx: usize,
         app_event_tx: &AppEventSender,
-    ) -> Option<String> {
+    ) -> Option<HistoryEntry> {
         if global_idx >= self.history_entry_count {
             // Local entry.
-            if let Some(text) = self
+            if let Some(entry) = self
                 .local_history
                 .get(global_idx - self.history_entry_count)
+                .cloned()
             {
-                self.last_history_text = Some(text.clone());
-                return Some(text.clone());
+                self.last_history_text = Some(entry.text.clone());
+                return Some(entry);
             }
-        } else if let Some(text) = self.fetched_history.get(&global_idx) {
-            self.last_history_text = Some(text.clone());
-            return Some(text.clone());
+        } else if let Some(entry) = self.fetched_history.get(&global_idx).cloned() {
+            self.last_history_text = Some(entry.text.clone());
+            return Some(entry);
         } else if let Some(log_id) = self.history_log_id {
             let op = Op::GetHistoryEntryRequest {
                 offset: global_idx,
@@ -206,22 +235,28 @@ mod tests {
         let mut history = ChatComposerHistory::new();
 
         // Empty submissions are ignored.
-        history.record_local_submission("");
+        history.record_local_submission(HistoryEntry::from_text(String::new()));
         assert_eq!(history.local_history.len(), 0);
 
         // First entry is recorded.
-        history.record_local_submission("hello");
+        history.record_local_submission(HistoryEntry::from_text("hello".to_string()));
         assert_eq!(history.local_history.len(), 1);
-        assert_eq!(history.local_history.last().unwrap(), "hello");
+        assert_eq!(
+            history.local_history.last().unwrap(),
+            &HistoryEntry::from_text("hello".to_string())
+        );
 
         // Identical consecutive entry is skipped.
-        history.record_local_submission("hello");
+        history.record_local_submission(HistoryEntry::from_text("hello".to_string()));
         assert_eq!(history.local_history.len(), 1);
 
         // Different entry is recorded.
-        history.record_local_submission("world");
+        history.record_local_submission(HistoryEntry::from_text("world".to_string()));
         assert_eq!(history.local_history.len(), 2);
-        assert_eq!(history.local_history.last().unwrap(), "world");
+        assert_eq!(
+            history.local_history.last().unwrap(),
+            &HistoryEntry::from_text("world".to_string())
+        );
     }
 
     #[test]
@@ -252,7 +287,7 @@ mod tests {
 
         // Inject the async response.
         assert_eq!(
-            Some("latest".into()),
+            Some(HistoryEntry::from_text("latest".to_string())),
             history.on_entry_response(1, 2, Some("latest".into()))
         );
 
@@ -273,7 +308,7 @@ mod tests {
         );
 
         assert_eq!(
-            Some("older".into()),
+            Some(HistoryEntry::from_text("older".to_string())),
             history.on_entry_response(1, 1, Some("older".into()))
         );
     }
@@ -285,16 +320,29 @@ mod tests {
 
         let mut history = ChatComposerHistory::new();
         history.set_metadata(1, 3);
-        history.fetched_history.insert(1, "command2".into());
-        history.fetched_history.insert(2, "command3".into());
+        history
+            .fetched_history
+            .insert(1, HistoryEntry::from_text("command2".to_string()));
+        history
+            .fetched_history
+            .insert(2, HistoryEntry::from_text("command3".to_string()));
 
-        assert_eq!(Some("command3".into()), history.navigate_up(&tx));
-        assert_eq!(Some("command2".into()), history.navigate_up(&tx));
+        assert_eq!(
+            Some(HistoryEntry::from_text("command3".to_string())),
+            history.navigate_up(&tx)
+        );
+        assert_eq!(
+            Some(HistoryEntry::from_text("command2".to_string())),
+            history.navigate_up(&tx)
+        );
 
         history.reset_navigation();
         assert!(history.history_cursor.is_none());
         assert!(history.last_history_text.is_none());
 
-        assert_eq!(Some("command3".into()), history.navigate_up(&tx));
+        assert_eq!(
+            Some(HistoryEntry::from_text("command3".to_string())),
+            history.navigate_up(&tx)
+        );
     }
 }