From 0d44bd708eef1fe0a00f601ffe2ecab89a77434e Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 26 Mar 2026 10:55:18 -0600 Subject: [PATCH] Fix duplicate /review messages in app-server TUI (#15839) ## Symptoms When `/review` ran through `tui_app_server`, the TUI could show duplicate review content: - the `>> Code review started: ... <<` banner appeared twice - the final review body could also appear twice ## Problem `tui_app_server` was treating review lifecycle items as renderable content on more than one delivery path. Specifically: - `EnteredReviewMode` was rendered both when the item started and again when it completed - `ExitedReviewMode` rendered the review text itself, even though the same review text was also delivered later as the assistant message item That meant the same logical review event was committed into history multiple times. ## Solution Make review lifecycle items control state transitions only once, and keep the final review body sourced from the assistant message item: - render the review-start banner from the live `ItemStarted` path, while still allowing replay to restore it once - treat `ExitedReviewMode` as a mode-exit/finish-banner event instead of rendering the review body from it - preserve the existing assistant-message rendering path as the single source of final review text --- codex-rs/tui_app_server/src/chatwidget.rs | 69 ++++++++++++----------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/codex-rs/tui_app_server/src/chatwidget.rs b/codex-rs/tui_app_server/src/chatwidget.rs index deb378ea2..7f6a9b4c6 100644 --- a/codex-rs/tui_app_server/src/chatwidget.rs +++ b/codex-rs/tui_app_server/src/chatwidget.rs @@ -850,7 +850,6 @@ pub(crate) struct ChatWidget { quit_shortcut_key: Option, // Simple review mode flag; used to adjust layout and banners. is_review_mode: bool, - #[cfg(test)] // Snapshot of token usage to restore after review mode exits. pre_review_token_info: Option>, // Whether the next streamed assistant content should be preceded by a final message separator. @@ -2509,7 +2508,6 @@ impl ChatWidget { Some(info.total_token_usage.tokens_in_context_window()) } - #[cfg(test)] fn restore_pre_review_token_info(&mut self) { if let Some(saved) = self.pre_review_token_info.take() { match saved { @@ -4467,7 +4465,6 @@ impl ChatWidget { quit_shortcut_expires_at: None, quit_shortcut_key: None, is_review_mode: false, - #[cfg(test)] pre_review_token_info: None, needs_final_message_separator: false, had_work_activity: false, @@ -5962,17 +5959,12 @@ impl ChatWidget { }); } ThreadItem::EnteredReviewMode { review, .. } => { - self.add_to_history(history_cell::new_review_status_line(format!( - ">> Code review started: {review} <<" - ))); - if !self.bottom_pane.is_task_running() { - self.bottom_pane.set_task_running(/*running*/ true); + if from_replay { + self.enter_review_mode_with_hint(review, /*from_replay*/ true); } - self.is_review_mode = true; } - ThreadItem::ExitedReviewMode { review, .. } => { - self.on_agent_message(review); - self.is_review_mode = false; + ThreadItem::ExitedReviewMode { .. } => { + self.exit_review_mode_after_item(); } ThreadItem::ContextCompaction { .. } => { self.on_agent_message("Context compacted".to_owned()); @@ -6097,7 +6089,7 @@ impl ChatWidget { self.handle_turn_completed_notification(notification, replay_kind); } ServerNotification::ItemStarted(notification) => { - self.handle_item_started_notification(notification); + self.handle_item_started_notification(notification, replay_kind.is_some()); } ServerNotification::ItemCompleted(notification) => { self.handle_item_completed_notification(notification, replay_kind); @@ -6381,7 +6373,11 @@ impl ChatWidget { } } - fn handle_item_started_notification(&mut self, notification: ItemStartedNotification) { + fn handle_item_started_notification( + &mut self, + notification: ItemStartedNotification, + from_replay: bool, + ) { match notification.item { ThreadItem::CommandExecution { id, @@ -6458,10 +6454,9 @@ impl ChatWidget { agents_states, }), ThreadItem::EnteredReviewMode { review, .. } => { - self.add_to_history(history_cell::new_review_status_line(format!( - ">> Code review started: {review} <<" - ))); - self.is_review_mode = true; + if !from_replay { + self.enter_review_mode_with_hint(review, /*from_replay*/ false); + } } _ => {} } @@ -6861,28 +6856,41 @@ impl ChatWidget { } } - #[cfg(test)] - fn on_entered_review_mode(&mut self, review: ReviewRequest, from_replay: bool) { - // Enter review mode and emit a concise banner + fn enter_review_mode_with_hint(&mut self, hint: String, from_replay: bool) { if self.pre_review_token_info.is_none() { self.pre_review_token_info = Some(self.token_info.clone()); } - // Avoid toggling running state for replayed history events on resume. if !from_replay && !self.bottom_pane.is_task_running() { self.bottom_pane.set_task_running(/*running*/ true); } self.is_review_mode = true; - let hint = review - .user_facing_hint - .unwrap_or_else(|| codex_core::review_prompts::user_facing_hint(&review.target)); let banner = format!(">> Code review started: {hint} <<"); self.add_to_history(history_cell::new_review_status_line(banner)); self.request_redraw(); } + fn exit_review_mode_after_item(&mut self) { + self.flush_answer_stream_with_separator(); + self.flush_interrupt_queue(); + self.flush_active_cell(); + self.is_review_mode = false; + self.restore_pre_review_token_info(); + self.add_to_history(history_cell::new_review_status_line( + "<< Code review finished >>".to_string(), + )); + self.request_redraw(); + } + + #[cfg(test)] + fn on_entered_review_mode(&mut self, review: ReviewRequest, from_replay: bool) { + let hint = review + .user_facing_hint + .unwrap_or_else(|| codex_core::review_prompts::user_facing_hint(&review.target)); + self.enter_review_mode_with_hint(hint, from_replay); + } + #[cfg(test)] fn on_exited_review_mode(&mut self, review: ExitedReviewModeEvent) { - // Leave review mode; if output is present, flush pending stream + show results. if let Some(output) = review.review_output { self.flush_answer_stream_with_separator(); self.flush_interrupt_queue(); @@ -6911,14 +6919,7 @@ impl ChatWidget { } // Final message is rendered as part of the AgentMessage. } - - self.is_review_mode = false; - self.restore_pre_review_token_info(); - // Append a finishing banner at the end of this turn. - self.add_to_history(history_cell::new_review_status_line( - "<< Code review finished >>".to_string(), - )); - self.request_redraw(); + self.exit_review_mode_after_item(); } fn on_user_message_event(&mut self, event: UserMessageEvent) {