From 031df77dfb9248fe47b40ec52bf8b05052231722 Mon Sep 17 00:00:00 2001 From: Christoph K <53318155+christophkogler@users.noreply.github.com> Date: Fri, 16 May 2025 19:48:11 -0700 Subject: [PATCH 1/3] Remove unnecessary console log from test (#970) When running `npm test` on `codex-cli`, the test `agent-cancel-prev-response.test.ts` logs a significant body of text to console for no obvious reason. This is not helpful, as it makes test logs messy and far longer. This change deletes the `console.log(...)` that produces the behavior. --- codex-cli/tests/agent-cancel-prev-response.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/codex-cli/tests/agent-cancel-prev-response.test.ts b/codex-cli/tests/agent-cancel-prev-response.test.ts index 08d37e7b76..9e7c397425 100644 --- a/codex-cli/tests/agent-cancel-prev-response.test.ts +++ b/codex-cli/tests/agent-cancel-prev-response.test.ts @@ -132,8 +132,6 @@ describe("cancel clears previous_response_id", () => { ] as any); const bodies = _test.getBodies(); - // eslint-disable-next-line no-console - console.log(JSON.stringify(bodies, null, 2)); expect(bodies.length).toBeGreaterThanOrEqual(2); // The *last* invocation belongs to the second run (after cancellation). From f8b6b1db81120660b809ab9104b7c80cf26b69d7 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 17 May 2025 09:00:23 -0700 Subject: [PATCH 2/3] 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 17f57fc053..8ceef95b4a 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 16fc3f4874..3246fe7263 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) { From 56a06203bbfe6c3fc33a7b6dbd7e35e1f3905597 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Sat, 17 May 2025 09:10:43 -0700 Subject: [PATCH 3/3] fix: clear scrollable view before drawing next frame --- codex-rs/tui/src/conversation_history_widget.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/codex-rs/tui/src/conversation_history_widget.rs b/codex-rs/tui/src/conversation_history_widget.rs index 3246fe7263..64fa4f6dd3 100644 --- a/codex-rs/tui/src/conversation_history_widget.rs +++ b/codex-rs/tui/src/conversation_history_widget.rs @@ -438,6 +438,8 @@ impl WidgetRef for ConversationHistoryWidget { .wrap(wrap_cfg()) .scroll((offset_into_first as u16, 0)); + // Clear the widget area to avoid visual artifacts from previous frames. + Clear.render(area, buf); paragraph.render(area, buf); // Draw scrollbar if necessary.