diff --git a/codex-rs/connectors/src/metadata.rs b/codex-rs/connectors/src/metadata.rs index 64becb766..0a7eebe4e 100644 --- a/codex-rs/connectors/src/metadata.rs +++ b/codex-rs/connectors/src/metadata.rs @@ -5,7 +5,11 @@ pub fn connector_display_label(connector: &AppInfo) -> String { } pub fn connector_mention_slug(connector: &AppInfo) -> String { - crate::connector_name_slug(&connector_display_label(connector)) + connector_mention_slug_from_name(&connector_display_label(connector)) +} + +pub fn connector_mention_slug_from_name(name: &str) -> String { + crate::connector_name_slug(name) } pub fn connector_install_url(name: &str, connector_id: &str) -> String { diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 21208de94..721824ed6 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -858,6 +858,10 @@ impl ChatComposer { } } + pub(crate) fn record_replayed_user_message_history(&mut self, entry: HistoryEntry) { + self.history.record_replayed_submission(entry); + } + /// Integrate pasted text into the composer. /// /// Acts as the only place where paste text is integrated, both for: 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 23b553fa0..ff6dd9983 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer_history.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer_history.rs @@ -121,6 +121,8 @@ pub(crate) struct ChatComposerHistory { /// Messages submitted by the user *during this UI session* (newest at END). /// Local entries retain full draft state (text elements, image paths, pending pastes, remote image URLs). local_history: Vec, + /// Local entries seeded from resumed transcript replay. + replay_seeded_history: Vec, /// Cache of persistent history entries fetched on-demand (text-only). fetched_history: HashMap, @@ -128,6 +130,7 @@ pub(crate) struct ChatComposerHistory { /// Current cursor within the combined (persistent + local) history. `None` /// indicates the user is *not* currently browsing history. history_cursor: Option, + pending_navigation_direction: Option, /// 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 @@ -229,8 +232,10 @@ impl ChatComposerHistory { persistent_log_id: None, persistent_entry_count: 0, local_history: Vec::new(), + replay_seeded_history: Vec::new(), fetched_history: HashMap::new(), history_cursor: None, + pending_navigation_direction: None, last_history_text: None, search: None, at_mention_restore_enabled: false, @@ -259,7 +264,9 @@ impl ChatComposerHistory { self.persistent_entry_count = entry_count; self.fetched_history.clear(); self.local_history.clear(); + self.replay_seeded_history.clear(); self.history_cursor = None; + self.pending_navigation_direction = None; self.last_history_text = None; self.search = None; } @@ -269,6 +276,16 @@ impl ChatComposerHistory { /// Empty submissions are ignored, adjacent duplicates are collapsed, and active navigation or /// search state is reset because a new newest entry changes the combined history offset space. pub fn record_local_submission(&mut self, entry: HistoryEntry) { + self.record_local_submission_inner(entry); + } + + pub fn record_replayed_submission(&mut self, entry: HistoryEntry) { + if self.record_local_submission_inner(entry.clone()) { + self.replay_seeded_history.push(entry); + } + } + + fn record_local_submission_inner(&mut self, entry: HistoryEntry) -> bool { if entry.text.is_empty() && entry.text_elements.is_empty() && entry.local_image_paths.is_empty() @@ -276,18 +293,20 @@ impl ChatComposerHistory { && entry.mention_bindings.is_empty() && entry.pending_pastes.is_empty() { - return; + return false; } self.history_cursor = None; + self.pending_navigation_direction = None; self.last_history_text = None; self.search = None; // Avoid inserting a duplicate if identical to the previous entry. if self.local_history.last().is_some_and(|prev| prev == &entry) { - return; + return false; } self.local_history.push(entry); + true } /// Resets normal history navigation so the next Up key resumes from the newest entry. @@ -297,6 +316,7 @@ impl ChatComposerHistory { /// influence later Up/Down recall. pub fn reset_navigation(&mut self) { self.history_cursor = None; + self.pending_navigation_direction = None; self.last_history_text = None; self.search = None; } @@ -359,7 +379,11 @@ impl ChatComposerHistory { }; self.history_cursor = Some(next_idx); - self.populate_history_at_index(next_idx as usize, app_event_tx) + self.populate_history_at_index( + next_idx as usize, + HistorySearchDirection::Older, + app_event_tx, + ) } /// Handles Down by moving toward newer entries or clearing the composer past the newest entry. @@ -383,11 +407,16 @@ impl ChatComposerHistory { match next_idx_opt { Some(idx) => { self.history_cursor = Some(idx); - self.populate_history_at_index(idx as usize, app_event_tx) + self.populate_history_at_index( + idx as usize, + HistorySearchDirection::Newer, + app_event_tx, + ) } None => { // Past newest – clear and exit browsing mode. self.history_cursor = None; + self.pending_navigation_direction = None; self.last_history_text = None; Some(HistoryEntry::new(String::new())) } @@ -449,9 +478,22 @@ impl ChatComposerHistory { } if self.history_cursor == Some(offset as isize) { + let direction = self.pending_navigation_direction.take(); let Some(entry) = entry else { return HistoryEntryResponse::Ignored; }; + if self.persistent_entry_duplicates_local(&entry) + && let Some(direction) = direction + { + let Some(offset) = self.next_history_offset(offset, direction) else { + return HistoryEntryResponse::Ignored; + }; + self.history_cursor = Some(offset as isize); + return self + .populate_history_at_index(offset, direction, app_event_tx) + .map(HistoryEntryResponse::Found) + .unwrap_or(HistoryEntryResponse::Ignored); + } self.last_history_text = Some(entry.text.clone()); return HistoryEntryResponse::Found(entry); } @@ -729,29 +771,61 @@ impl ChatComposerHistory { fn populate_history_at_index( &mut self, global_idx: usize, + direction: HistorySearchDirection, app_event_tx: &AppEventSender, ) -> Option { - if global_idx >= self.persistent_entry_count { - // Local entry. - if let Some(entry) = self - .local_history - .get(global_idx - self.persistent_entry_count) - .cloned() - { + let mut global_idx = global_idx; + loop { + if let Some(entry) = self.entry_at_cached_offset(global_idx) { + if global_idx < self.persistent_entry_count + && self.persistent_entry_duplicates_local(&entry) + { + let Some(next_idx) = self.next_history_offset(global_idx, direction) else { + self.pending_navigation_direction = None; + return None; + }; + self.history_cursor = Some(next_idx as isize); + global_idx = next_idx; + continue; + } + self.pending_navigation_direction = None; self.last_history_text = Some(entry.text.clone()); return Some(entry); } - } 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(thread_id), Some(log_id)) = (self.thread_id, self.persistent_log_id) { - app_event_tx.send(AppEvent::LookupMessageHistoryEntry { - thread_id, - offset: global_idx, - log_id, - }); + + if global_idx >= self.persistent_entry_count { + return None; + } + + if let (Some(thread_id), Some(log_id)) = (self.thread_id, self.persistent_log_id) { + self.pending_navigation_direction = Some(direction); + app_event_tx.send(AppEvent::LookupMessageHistoryEntry { + thread_id, + offset: global_idx, + log_id, + }); + } + return None; } - None + } + + fn next_history_offset( + &self, + offset: usize, + direction: HistorySearchDirection, + ) -> Option { + match direction { + HistorySearchDirection::Older => offset.checked_sub(1), + HistorySearchDirection::Newer => offset + .checked_add(1) + .filter(|next| *next < self.total_entries()), + } + } + + fn persistent_entry_duplicates_local(&self, entry: &HistoryEntry) -> bool { + self.replay_seeded_history.iter().any(|local_entry| { + local_entry.text == entry.text && local_entry.mention_bindings == entry.mention_bindings + }) } } @@ -939,9 +1013,16 @@ mod tests { // Pretend there are 3 persistent entries. let thread_id = test_thread_id(); history.set_metadata(thread_id, /*log_id*/ 1, /*entry_count*/ 3); + history.record_local_submission(HistoryEntry::new("latest".to_string())); - // First Up should request offset 2 (latest) and await async data. + // First Up should recall current-session local history. assert!(history.should_handle_navigation("", /*cursor*/ 0)); + assert_eq!( + Some(HistoryEntry::new("latest".to_string())), + history.navigate_up(&tx) + ); + + // Next Up should request offset 2 and await async data. assert!(history.navigate_up(&tx).is_none()); // don't replace the text yet // Verify that a history lookup request was sent. diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 7ef385570..fa9291467 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -184,6 +184,7 @@ pub(crate) use chat_composer::ChatComposer; pub(crate) use chat_composer::ChatComposerConfig; pub(crate) use chat_composer::InputResult; pub(crate) use chat_composer::QueuedInputAction; +pub(crate) use chat_composer_history::HistoryEntry; use crate::status_indicator_widget::StatusDetailsCapitalization; use crate::status_indicator_widget::StatusIndicatorWidget; @@ -1543,6 +1544,10 @@ impl BottomPane { } } + pub(crate) fn record_replayed_user_message_history(&mut self, entry: HistoryEntry) { + self.composer.record_replayed_user_message_history(entry); + } + pub(crate) fn on_file_search_result(&mut self, query: String, matches: Vec) { self.composer.on_file_search_result(query, matches); self.request_redraw(); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index d3e5cf68e..7d879a09c 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -169,6 +169,8 @@ use codex_terminal_detection::TerminalName; use codex_terminal_detection::terminal_info; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_cli::resume_hint; +use codex_utils_plugins::mention_syntax::PLUGIN_TEXT_MENTION_SIGIL; +use codex_utils_plugins::mention_syntax::TOOL_MENTION_SIGIL; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; @@ -274,6 +276,7 @@ use crate::bottom_pane::DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED; use crate::bottom_pane::ExperimentalFeatureItem; use crate::bottom_pane::ExperimentalFeaturesView; use crate::bottom_pane::GoalStatusIndicator; +use crate::bottom_pane::HistoryEntry; use crate::bottom_pane::InputResult; use crate::bottom_pane::LocalImageAttachment; use crate::bottom_pane::McpServerElicitationFormRequest; @@ -1223,9 +1226,75 @@ impl ChatWidget { fn on_committed_user_message(&mut self, items: &[UserInput], from_replay: bool) { let display = Self::user_message_display_from_inputs(items); if from_replay { - if !self.review.is_review_mode { - self.on_user_message_display(display); + if self.review.is_review_mode { + return; } + let message = display.message.as_str(); + let mention_start = |sigil: char, mention: &str| { + let token = format!("{sigil}{mention}"); + message.match_indices(&token).find_map(|(start, _)| { + let end = start + token.len(); + message + .as_bytes() + .get(end) + .is_none_or(|byte| { + !byte.is_ascii_alphanumeric() && !matches!(byte, b'_' | b'-') + }) + .then_some(start) + }) + }; + let mut mention_bindings: Vec = items + .iter() + .filter_map(|item| match item { + UserInput::Skill { name, path } => Some(MentionBinding { + sigil: TOOL_MENTION_SIGIL, + mention: name.clone(), + path: path.to_string_lossy().into_owned(), + }), + UserInput::Mention { name, path } => { + let plugin_id = path.strip_prefix("plugin://"); + let mention = if let Some(plugin_id) = plugin_id { + plugin_id + .split_once('@') + .map(|(plugin_name, _)| plugin_name) + .unwrap_or(plugin_id) + .to_string() + } else if path.starts_with("app://") { + codex_connectors::metadata::connector_mention_slug_from_name(name) + } else { + name.clone() + }; + let sigil = if plugin_id.is_some() + && mention_start(PLUGIN_TEXT_MENTION_SIGIL, &mention).is_some() + { + PLUGIN_TEXT_MENTION_SIGIL + } else { + TOOL_MENTION_SIGIL + }; + Some(MentionBinding { + sigil, + mention, + path: path.clone(), + }) + } + UserInput::Text { .. } + | UserInput::Image { .. } + | UserInput::LocalImage { .. } => None, + }) + .collect(); + mention_bindings.sort_by_key(|binding| { + mention_start(binding.sigil, &binding.mention).unwrap_or(usize::MAX) + }); + self.bottom_pane + .record_replayed_user_message_history(HistoryEntry { + text: display.message.clone(), + text_elements: display.text_elements.clone(), + local_image_paths: display.local_images.clone(), + remote_image_urls: display.remote_image_urls.clone(), + mention_bindings, + pending_pastes: Vec::new(), + }); + self.on_user_message_display(display); return; } diff --git a/codex-rs/tui/src/chatwidget/tests/history_replay.rs b/codex-rs/tui/src/chatwidget/tests/history_replay.rs index 58f99231f..e6b9d4ede 100644 --- a/codex-rs/tui/src/chatwidget/tests/history_replay.rs +++ b/codex-rs/tui/src/chatwidget/tests/history_replay.rs @@ -1,4 +1,5 @@ use super::*; +use crate::app_event::HistoryLookupResponse; use codex_app_server_protocol::NetworkAccess; use codex_app_server_protocol::SandboxPolicy; use codex_protocol::models::ManagedFileSystemPermissions; @@ -74,6 +75,120 @@ async fn resumed_initial_messages_render_history() { ); } +#[tokio::test] +async fn replayed_user_messages_seed_composer_history() { + let (mut chat, mut rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await; + chat.bottom_pane.set_history_metadata( + ThreadId::new(), + /*log_id*/ 1, + /*entry_count*/ 3, + ); + + let mut replay_mention = |id: &str, text: &str, name: &str, path: &str| { + replay_user_message_inputs( + &mut chat, + id, + vec![ + AppServerUserInput::Text { + text: text.to_string(), + text_elements: Vec::new(), + }, + AppServerUserInput::Mention { + name: name.to_string(), + path: path.to_string(), + }, + ], + ReplayKind::ResumeInitialMessages, + ); + }; + replay_mention( + "user-1", + "use $sample", + "Sample Plugin", + "plugin://sample@test", + ); + replay_mention( + "user-2", + "use $google-calendar", + "Google Calendar", + "app://google_calendar", + ); + drain_insert_history(&mut rx); + + chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); + assert_eq!(chat.bottom_pane.composer_text(), "use $google-calendar"); + assert_eq!( + chat.bottom_pane.take_mention_bindings(), + vec![MentionBinding { + sigil: '$', + mention: "google-calendar".to_string(), + path: "app://google_calendar".to_string(), + }] + ); + + chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); + assert_eq!(chat.bottom_pane.composer_text(), "use $sample"); + assert_eq!( + chat.bottom_pane.take_mention_bindings(), + vec![MentionBinding { + sigil: '$', + mention: "sample".to_string(), + path: "plugin://sample@test".to_string(), + }] + ); + + let mut next_lookup_offset = || { + let AppEvent::LookupMessageHistoryEntry { offset, .. } = + rx.try_recv().expect("expected lookup") + else { + panic!("unexpected event variant"); + }; + offset + }; + let response = |offset, entry: &str| HistoryLookupResponse { + offset, + log_id: 1, + entry: Some(entry.to_string()), + }; + + chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); + chat.handle_history_entry_response(response( + next_lookup_offset(), + "use [$google-calendar](app://google_calendar)", + )); + + assert_eq!(next_lookup_offset(), 1); + chat.handle_history_entry_response(response(1, "use [$sample](plugin://sample@test)")); + + assert_eq!(next_lookup_offset(), 0); + chat.handle_history_entry_response(response(0, "/rename smoke-1")); + assert_eq!(chat.bottom_pane.composer_text(), "/rename smoke-1"); +} + +#[tokio::test] +async fn replayed_review_prompt_does_not_seed_composer_history() { + let (mut chat, mut rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await; + + chat.replay_thread_item( + AppServerThreadItem::EnteredReviewMode { + id: "review-start".to_string(), + review: "changes against main".to_string(), + }, + "turn-1".to_string(), + ReplayKind::ResumeInitialMessages, + ); + replay_user_message_text( + &mut chat, + "review-prompt", + "Review the code changes against the base branch 'main'.", + ReplayKind::ResumeInitialMessages, + ); + drain_insert_history(&mut rx); + + chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); + assert_eq!(chat.bottom_pane.composer_text(), ""); +} + #[tokio::test] async fn replayed_user_message_preserves_text_elements_and_local_images() { let (mut chat, mut rx, _ops) = make_chatwidget_manual(/*model_override*/ None).await;