From f8b6b1db81120660b809ab9104b7c80cf26b69d7 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 17 May 2025 09:00:23 -0700 Subject: [PATCH] fix: ensure the first user message always displays after the session info (#988) Previously, if the first user message was sent with the command invocation, e.g.: ``` $ cargo run --bin codex 'hello' ``` Then the user message was added as the first entry in the history and then `is_first_event` would be `false` here: https://github.com/openai/codex/blob/031df77dfb9248fe47b40ec52bf8b05052231722/codex-rs/tui/src/conversation_history_widget.rs#L178-L179 which would prevent the "welcome" message with things like the the model version from displaying. The fix in this PR is twofold: * Reorganize the logic so the `ChatWidget` constructor stores `initial_user_message` rather than sending it right away. Now inside `handle_codex_event()`, it waits for the `SessionConfigured` event and sends the `initial_user_message`, if it exists. * In `conversation_history_widget.rs`, `add_session_info()` checks to see whether a `WelcomeMessage` exists in the history when determining the value of `has_welcome_message`. By construction, we expect that `WelcomeMessage` is always the first message (in which case the existing `let is_first_event = self.entries.is_empty();` logic would be sound), but we decide to be extra defensive in case an `EventMsg::Error` is processed before `EventMsg::SessionConfigured`. --- codex-rs/tui/src/chatwidget.rs | 53 +++++++++++++------ .../tui/src/conversation_history_widget.rs | 14 ++++- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 17f57fc0..8ceef95b 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -44,6 +44,7 @@ pub(crate) struct ChatWidget<'a> { bottom_pane: BottomPane<'a>, input_focus: InputFocus, config: Config, + initial_user_message: Option, } #[derive(Clone, Copy, Eq, PartialEq)] @@ -52,6 +53,28 @@ enum InputFocus { BottomPane, } +struct UserMessage { + text: String, + image_paths: Vec, +} + +impl From for UserMessage { + fn from(text: String) -> Self { + Self { + text, + image_paths: Vec::new(), + } + } +} + +fn create_initial_user_message(text: String, image_paths: Vec) -> Option { + if text.is_empty() && image_paths.is_empty() { + None + } else { + Some(UserMessage { text, image_paths }) + } +} + impl ChatWidget<'_> { pub(crate) fn new( config: Config, @@ -93,7 +116,7 @@ impl ChatWidget<'_> { } }); - let mut chat_widget = Self { + Self { app_event_tx: app_event_tx.clone(), codex_op_tx, conversation_history: ConversationHistoryWidget::new(), @@ -103,14 +126,11 @@ impl ChatWidget<'_> { }), input_focus: InputFocus::BottomPane, config, - }; - - if initial_prompt.is_some() || !initial_images.is_empty() { - let text = initial_prompt.unwrap_or_default(); - chat_widget.submit_user_message_with_images(text, initial_images); + initial_user_message: create_initial_user_message( + initial_prompt.unwrap_or_default(), + initial_images, + ), } - - chat_widget } pub(crate) fn handle_key_event(&mut self, key_event: KeyEvent) { @@ -141,19 +161,15 @@ impl ChatWidget<'_> { } InputFocus::BottomPane => match self.bottom_pane.handle_key_event(key_event) { InputResult::Submitted(text) => { - self.submit_user_message(text); + self.submit_user_message(text.into()); } InputResult::None => {} }, } } - fn submit_user_message(&mut self, text: String) { - // Forward to codex and update conversation history. - self.submit_user_message_with_images(text, vec![]); - } - - fn submit_user_message_with_images(&mut self, text: String, image_paths: Vec) { + fn submit_user_message(&mut self, user_message: UserMessage) { + let UserMessage { text, image_paths } = user_message; let mut items: Vec = Vec::new(); if !text.is_empty() { @@ -207,6 +223,13 @@ impl ChatWidget<'_> { // composer can navigate through past messages. self.bottom_pane .set_history_metadata(event.history_log_id, event.history_entry_count); + + if let Some(user_message) = self.initial_user_message.take() { + // If the user provided an initial message, add it to the + // conversation history. + self.submit_user_message(user_message); + } + self.request_redraw(); } EventMsg::AgentMessage(AgentMessageEvent { message }) => { diff --git a/codex-rs/tui/src/conversation_history_widget.rs b/codex-rs/tui/src/conversation_history_widget.rs index 16fc3f48..3246fe72 100644 --- a/codex-rs/tui/src/conversation_history_widget.rs +++ b/codex-rs/tui/src/conversation_history_widget.rs @@ -175,8 +175,18 @@ impl ConversationHistoryWidget { /// Note `model` could differ from `config.model` if the agent decided to /// use a different model than the one requested by the user. pub fn add_session_info(&mut self, config: &Config, event: SessionConfiguredEvent) { - let is_first_event = self.entries.is_empty(); - self.add_to_history(HistoryCell::new_session_info(config, event, is_first_event)); + // In practice, SessionConfiguredEvent should always be the first entry + // in the history, but it is possible that an error could be sent + // before the session info. + let has_welcome_message = self + .entries + .iter() + .any(|entry| matches!(entry.cell, HistoryCell::WelcomeMessage { .. })); + self.add_to_history(HistoryCell::new_session_info( + config, + event, + !has_welcome_message, + )); } pub fn add_user_message(&mut self, message: String) {