From d4c72ea829cf2e170d261583d846846589bc3ae7 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 20 Mar 2026 00:48:55 -0600 Subject: [PATCH] codex: address PR review feedback (#15106) --- .../src/event_processor_with_human_output.rs | 46 +++++++++++++++++-- .../src/event_processor_with_jsonl_output.rs | 7 +-- ...event_processor_with_jsonl_output_tests.rs | 33 +++++++++++++ 3 files changed, 77 insertions(+), 9 deletions(-) diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index eae285730b..d95e19e63a 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -313,10 +313,8 @@ impl EventProcessor for EventProcessorWithHumanOutput { } TypedExecEvent::TurnCompleted(notification) => match notification.turn.status { TurnStatus::Completed => { - if self.final_message.is_none() { - self.final_message = - final_message_from_turn_items(notification.turn.items.as_slice()); - } + self.final_message = + final_message_from_turn_items(notification.turn.items.as_slice()); self.print_usage(); CodexStatus::InitiateShutdown } @@ -561,4 +559,44 @@ mod tests { ); assert_eq!(processor.final_message.as_deref(), Some("final answer")); } + + #[test] + fn turn_completed_overwrites_stale_final_message_from_turn_items() { + let mut processor = EventProcessorWithHumanOutput { + bold: Style::new(), + cyan: Style::new(), + dimmed: Style::new(), + green: Style::new(), + red: Style::new(), + yellow: Style::new(), + show_agent_reasoning: true, + show_raw_agent_reasoning: false, + last_message_path: None, + final_message: Some("stale answer".to_string()), + last_total_token_usage: None, + }; + + let status = processor.process_event(TypedExecEvent::TurnCompleted( + codex_app_server_protocol::TurnCompletedNotification { + thread_id: "thread-1".to_string(), + turn: Turn { + id: "turn-1".to_string(), + items: vec![ThreadItem::AgentMessage { + id: "msg-1".to_string(), + text: "final answer".to_string(), + phase: None, + memory_citation: None, + }], + status: TurnStatus::Completed, + error: None, + }, + }, + )); + + assert_eq!( + status, + crate::event_processor::CodexStatus::InitiateShutdown + ); + assert_eq!(processor.final_message.as_deref(), Some("final answer")); + } } diff --git a/codex-rs/exec/src/event_processor_with_jsonl_output.rs b/codex-rs/exec/src/event_processor_with_jsonl_output.rs index 9dbdcf9093..878e7619d1 100644 --- a/codex-rs/exec/src/event_processor_with_jsonl_output.rs +++ b/codex-rs/exec/src/event_processor_with_jsonl_output.rs @@ -406,11 +406,8 @@ impl EventProcessorWithJsonOutput { } match notification.turn.status { TurnStatus::Completed => { - if self.final_message.is_none() { - self.final_message = Self::final_message_from_turn_items( - notification.turn.items.as_slice(), - ); - } + self.final_message = + Self::final_message_from_turn_items(notification.turn.items.as_slice()); events.push(ThreadEvent::TurnCompleted(TurnCompletedEvent { usage: self.usage_from_last_total(), })); diff --git a/codex-rs/exec/src/event_processor_with_jsonl_output_tests.rs b/codex-rs/exec/src/event_processor_with_jsonl_output_tests.rs index ad1b6803ca..0793af6fc3 100644 --- a/codex-rs/exec/src/event_processor_with_jsonl_output_tests.rs +++ b/codex-rs/exec/src/event_processor_with_jsonl_output_tests.rs @@ -482,6 +482,39 @@ fn turn_completion_recovers_final_message_from_turn_items() { assert_eq!(processor.final_message.as_deref(), Some("final answer")); } +#[test] +fn turn_completion_overwrites_stale_final_message_from_turn_items() { + let mut processor = EventProcessorWithJsonOutput::new(None); + processor.final_message = Some("stale answer".to_string()); + + let completed = + processor.collect_thread_events(TypedExecEvent::TurnCompleted(TurnCompletedNotification { + thread_id: "thread-1".to_string(), + turn: Turn { + id: "turn-1".to_string(), + items: vec![ThreadItem::AgentMessage { + id: "msg-1".to_string(), + text: "final answer".to_string(), + phase: None, + memory_citation: None, + }], + status: TurnStatus::Completed, + error: None, + }, + })); + + assert_eq!( + completed, + CollectedThreadEvents { + events: vec![ThreadEvent::TurnCompleted(TurnCompletedEvent { + usage: Usage::default(), + })], + status: CodexStatus::InitiateShutdown, + } + ); + assert_eq!(processor.final_message.as_deref(), Some("final answer")); +} + #[test] fn turn_completion_falls_back_to_final_plan_text() { let mut processor = EventProcessorWithJsonOutput::new(None);