From 0d337e5fe78860b25053bb2eb37c3876e566c428 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Mon, 19 Oct 2026 20:13:49 +0000 Subject: [PATCH] Fix recall of command-line initial prompts in the TUI (#55372) ## Why Command-line initial prompts bypass the composer and were missing from local recall history, so pressing Up could skip them and retrieve cross-session history instead. ## What changed Record accepted initial prompts in local recall history with their draft metadata. Add image placeholders where needed so recalled local attachments survive resubmission. Leave synchronously rejected prompts out of recall history. ## Testing Add regression tests for recall order before cross-session history, ordering after transcript replay, queued prompts appearing once, rejected submissions, and image attachment recall and resubmission. Add a snapshot of the recalled initial prompt. GitOrigin-RevId: 0a12221ae285f01bff4b5860e546c8f7ee3c71e3 --- codex-rs/tui/src/app/tests.rs | 5 + codex-rs/tui/src/bottom_pane/chat_composer.rs | 6 + codex-rs/tui/src/bottom_pane/mod.rs | 4 + codex-rs/tui/src/chatwidget/input_restore.rs | 43 +++- ...sts__initial_prompt_recalled_composer.snap | 7 + .../tests/startup_submission_tests.rs | 231 ++++++++++++++++++ 6 files changed, 295 insertions(+), 1 deletion(-) create mode 100644 codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__initial_prompt_recalled_composer.snap diff --git a/codex-rs/tui/src/app/tests.rs b/codex-rs/tui/src/app/tests.rs index 2e03905e1c..235139f802 100644 --- a/codex-rs/tui/src/app/tests.rs +++ b/codex-rs/tui/src/app/tests.rs @@ -963,6 +963,11 @@ async fn enqueue_primary_thread_session_replays_turns_before_initial_prompt_subm }]) ); + for expected in ["follow-up after replay", "earlier prompt"] { + app.chat_widget.handle_key_event(KeyCode::Up.into()); + assert_eq!(app.chat_widget.composer_text_with_pending(), expected); + } + Ok(()) } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 756c2d6840..57d5b49ac0 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -5,6 +5,8 @@ //! bursts, especially on Windows. Paste timing uses Tokio's clock so asynchronous flush deadlines //! and input classification share a clock, including in paused-time tests. Copy shortcuts and right //! clicks preserve selected draft text. +//! Local recall retains full draft metadata for composer submissions and accepted command-line +//! initial prompts; transcript replay seeds earlier prompts separately. //! When enabled, fullscreen right-click paste requires an editable composer without a selection, //! search, or blocking view. The app reads clipboard text asynchronously and delivers a normal //! paste only while the same thread, draft, and cursor remain eligible. Intervening input or focus @@ -1243,6 +1245,10 @@ impl ChatComposer { true } + pub(crate) fn record_local_user_message_history(&mut self, entry: HistoryEntry) { + self.history.record_local_submission(entry); + } + pub(crate) fn record_replayed_user_message_history(&mut self, entry: HistoryEntry) { self.history.record_replayed_submission(entry); } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 90ed97e811..ca77db8794 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -2331,6 +2331,10 @@ impl BottomPane { } } + pub(crate) fn record_local_user_message_history(&mut self, entry: HistoryEntry) { + self.composer.record_local_user_message_history(entry); + } + pub(crate) fn record_replayed_user_message_history(&mut self, entry: HistoryEntry) { self.composer.record_replayed_user_message_history(entry); } diff --git a/codex-rs/tui/src/chatwidget/input_restore.rs b/codex-rs/tui/src/chatwidget/input_restore.rs index 1f21ca9a7a..b4114ec109 100644 --- a/codex-rs/tui/src/chatwidget/input_restore.rs +++ b/codex-rs/tui/src/chatwidget/input_restore.rs @@ -2,6 +2,8 @@ use std::collections::HashSet; +use codex_protocol::models::local_image_label_text; + use crate::bottom_pane::ComposerDraftSnapshot; use crate::bottom_pane::KillBufferSnapshot; @@ -177,7 +179,46 @@ impl ChatWidget { return; } if let Some(user_message) = self.initial_user_message.take() { - self.submit_user_message(user_message); + // Command-line input bypasses the composer, which normally records local recall. + let mut entry = HistoryEntry { + text: user_message.text.clone(), + text_elements: user_message.text_elements.clone(), + local_image_paths: user_message + .local_images + .iter() + .map(|image| image.path.clone()) + .collect(), + remote_image_urls: user_message.remote_image_urls.clone(), + mention_bindings: user_message.mention_bindings.clone(), + pending_pastes: Vec::new(), + }; + // CLI images have no composer elements. Give recalled attachments owned placeholders + // so the composer retains them when the user submits the recalled prompt again. + for index in 0..entry.local_image_paths.len() { + let placeholder = local_image_label_text(entry.remote_image_urls.len() + index + 1); + if entry + .text_elements + .iter() + .any(|element| element.placeholder(&entry.text) == Some(placeholder.as_str())) + { + continue; + } + if !entry.text.is_empty() { + entry.text.push(' '); + } + let start = entry.text.len(); + entry.text.push_str(&placeholder); + entry.text_elements.push(TextElement::new( + (start..entry.text.len()).into(), + Some(placeholder), + )); + } + if self.submit_user_message_with_history_record( + user_message, + UserMessageHistoryRecord::UserMessageText, + ) { + self.bottom_pane.record_local_user_message_history(entry); + } } } diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__initial_prompt_recalled_composer.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__initial_prompt_recalled_composer.snap new file mode 100644 index 0000000000..1ce688b471 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__initial_prompt_recalled_composer.snap @@ -0,0 +1,7 @@ +--- +source: tui/src/chatwidget/tests/startup_submission_tests.rs +expression: "super::helpers::normalize_snapshot_paths(render_bottom_popup(&chat, 80))" +--- +› CURRENT_A + + GPT-5.6-Sol default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/tests/startup_submission_tests.rs b/codex-rs/tui/src/chatwidget/tests/startup_submission_tests.rs index cebcd5650c..68737acb03 100644 --- a/codex-rs/tui/src/chatwidget/tests/startup_submission_tests.rs +++ b/codex-rs/tui/src/chatwidget/tests/startup_submission_tests.rs @@ -1,6 +1,237 @@ use super::*; +use crate::app_event::HistoryLookupResponse; use pretty_assertions::assert_eq; +#[tokio::test] +async fn initial_prompt_recall_precedes_cross_session_history() { + for submit_follow_up in [false, true] { + let (mut chat, mut events, mut ops) = make_chatwidget_manual(/*model_override*/ None).await; + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); + chat.bottom_pane + .set_history_metadata(thread_id, /*log_id*/ 7, /*entry_count*/ 1); + chat.initial_user_message = + create_initial_user_message(Some("CURRENT_A".into()), Vec::new(), Vec::new()); + + chat.submit_initial_user_message_if_pending(); + chat.submit_initial_user_message_if_pending(); + assert_matches!(next_submit_op(&mut ops), Op::UserTurn { items, .. } + if items == vec![UserInput::Text { text: "CURRENT_A".into(), text_elements: Vec::new() }]); + assert_no_submit_op(&mut ops); + handle_turn_started(&mut chat, "initial"); + handle_turn_completed(&mut chat, "initial", /*duration_ms*/ None); + + let mut expected = vec!["CURRENT_A"]; + if submit_follow_up { + chat.handle_paste("CURRENT_B".into()); + chat.handle_key_event(KeyCode::Enter.into()); + assert_matches!(next_submit_op(&mut ops), Op::UserTurn { .. }); + handle_turn_started(&mut chat, "follow-up"); + handle_turn_completed(&mut chat, "follow-up", /*duration_ms*/ None); + expected.insert(0, "CURRENT_B"); + } + let persisted = std::iter::from_fn(|| events.try_recv().ok()) + .filter_map(|event| match event { + AppEvent::AppendMessageHistoryEntry { + thread_id: owner, + text, + } => { + assert_eq!(owner, thread_id); + Some(text) + } + _ => None, + }) + .collect::>(); + assert_eq!( + persisted, + expected.iter().rev().copied().collect::>() + ); + + for text in expected { + chat.handle_key_event(KeyCode::Up.into()); + assert_eq!(chat.bottom_pane.composer_text(), text); + assert!( + events.try_recv().is_err(), + "local recall must not fetch shared history" + ); + } + if submit_follow_up { + assert_chatwidget_snapshot!( + "initial_prompt_recalled_composer", + super::helpers::normalize_snapshot_paths(render_bottom_popup( + &chat, /*width*/ 80 + )) + ); + } + chat.handle_key_event(KeyCode::Up.into()); + let AppEvent::LookupMessageHistoryEntry { + thread_id: owner, + log_id, + offset, + } = events + .try_recv() + .expect("shared history lookup after current prompts") + else { + panic!("expected shared history lookup"); + }; + assert_eq!((owner, log_id, offset), (thread_id, 7, 0)); + chat.bottom_pane + .on_history_lookup_response(HistoryLookupResponse::Entry { + log_id, + offset, + entry: Some("FOREIGN_X".into()), + }); + assert_eq!(chat.bottom_pane.composer_text(), "FOREIGN_X"); + } +} + +#[tokio::test] +async fn initial_prompt_image_preparation_preserves_recall_metadata() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("image.png"); + image::RgbImage::new(/*width*/ 1, /*height*/ 1) + .save(&path) + .unwrap(); + let placeholder = "[Image #1]"; + for (input, elements, recalled_text, placeholder_start) in [ + ( + "describe", + Vec::new(), + "describe [Image #1]", + "describe ".len(), + ), + ( + "[Image #1] describe", + vec![TextElement::new( + (0..placeholder.len()).into(), + Some(placeholder.into()), + )], + "[Image #1] describe", + 0, + ), + ] { + let (mut chat, mut events, mut ops) = make_chatwidget_manual(/*model_override*/ None).await; + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); + chat.bottom_pane + .set_history_metadata(thread_id, /*log_id*/ 7, /*entry_count*/ 1); + chat.snapshot_local_images = true; + let text_elements = vec![TextElement::new( + (placeholder_start..placeholder_start + placeholder.len()).into(), + Some(placeholder.into()), + )]; + chat.initial_user_message = + create_initial_user_message(Some(input.into()), vec![path.clone()], elements); + + chat.submit_initial_user_message_if_pending(); + assert!(chat.pending_image_submission.is_some()); + tokio::time::timeout(Duration::from_secs(10), async { + loop { + if let AppEvent::ImagesPrepared(id) = events.recv().await.unwrap() { + chat.on_images_prepared(id); + break; + } + } + }) + .await + .expect("initial image preparation"); + assert_matches!(next_submit_op(&mut ops), Op::UserTurn { items, .. } + if items.iter().any(|item| matches!(item, UserInput::Text { text, .. } if text == input))); + chat.submit_initial_user_message_if_pending(); + assert_no_submit_op(&mut ops); + handle_turn_started(&mut chat, "initial"); + handle_turn_completed(&mut chat, "initial", /*duration_ms*/ None); + while events.try_recv().is_ok() {} + + chat.handle_key_event(KeyCode::Up.into()); + let recalled = chat.bottom_pane.composer_draft_snapshot(); + assert_eq!( + ( + recalled.text, + recalled.text_elements, + recalled.local_images, + recalled.pending_pastes + ), + ( + recalled_text.into(), + text_elements, + vec![LocalImageAttachment { + placeholder: placeholder.into(), + path: path.clone() + }], + Vec::new() + ), + ); + chat.handle_key_event(KeyCode::Up.into()); + assert_matches!( + events.try_recv(), + Ok(AppEvent::LookupMessageHistoryEntry { offset: 0, .. }) + ); + + chat.handle_key_event(KeyCode::Enter.into()); + tokio::time::timeout(Duration::from_secs(10), async { + loop { + if let AppEvent::ImagesPrepared(id) = events.recv().await.unwrap() { + chat.on_images_prepared(id); + break; + } + } + }) + .await + .expect("recalled image preparation"); + assert_matches!(next_submit_op(&mut ops), Op::UserTurn { items, .. } + if items.iter().any(|item| matches!(item, UserInput::Image { .. }))); + } +} + +#[tokio::test] +async fn initial_prompt_queued_before_session_is_recalled_once_after_dispatch() { + let (mut chat, mut events, mut ops) = make_chatwidget_manual(/*model_override*/ None).await; + chat.initial_user_message = Some("CURRENT_A".into()); + chat.submit_initial_user_message_if_pending(); + assert_no_submit_op(&mut ops); + assert_eq!(chat.queued_user_message_texts(), vec!["CURRENT_A"]); + + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); + chat.bottom_pane + .set_history_metadata(thread_id, /*log_id*/ 7, /*entry_count*/ 1); + assert!(chat.maybe_send_next_queued_input()); + assert_matches!(next_submit_op(&mut ops), Op::UserTurn { .. }); + handle_turn_started(&mut chat, "initial"); + handle_turn_completed(&mut chat, "initial", /*duration_ms*/ None); + while events.try_recv().is_ok() {} + + chat.handle_key_event(KeyCode::Up.into()); + assert_eq!(chat.bottom_pane.composer_text(), "CURRENT_A"); + chat.handle_key_event(KeyCode::Up.into()); + assert_matches!( + events.try_recv(), + Ok(AppEvent::LookupMessageHistoryEntry { offset: 0, .. }) + ); +} + +#[tokio::test] +async fn rejected_initial_prompt_is_not_recorded_in_recall_history() { + let (mut chat, mut events, mut ops) = make_chatwidget_manual(/*model_override*/ None).await; + let thread_id = ThreadId::new(); + chat.thread_id = Some(thread_id); + chat.bottom_pane + .set_history_metadata(thread_id, /*log_id*/ 7, /*entry_count*/ 1); + // A disconnected submission channel rejects the initial prompt synchronously. + ops.close(); + chat.initial_user_message = Some("CURRENT_A".into()); + chat.submit_initial_user_message_if_pending(); + assert_no_submit_op(&mut ops); + while events.try_recv().is_ok() {} + + chat.handle_key_event(KeyCode::Up.into()); + assert_matches!( + events.try_recv(), + Ok(AppEvent::LookupMessageHistoryEntry { offset: 0, .. }) + ); +} + #[tokio::test] async fn startup_submission_waits_for_session_and_sends_exactly_once() { let (mut source, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;