From eaf2fb5b4f76f62ede52af9053d827c1959b8c68 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 5 Aug 2025 22:44:27 -0700 Subject: [PATCH] fix: fully enumerate EventMsg in chatwidget.rs (#1866) https://github.com/openai/codex/pull/1868 is a related fix that was in flight simultaenously, but after talking to @easong-openai, this: - logs instead of renders for `BackgroundEvent` - logs for `TurnDiff` - renders for `PatchApplyEnd` --- codex-rs/tui/src/chatwidget.rs | 17 ++++--- codex-rs/tui/src/history_cell.rs | 80 ++++++++++++++++++++++++++------ 2 files changed, 78 insertions(+), 19 deletions(-) diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 7751404f..86bf765f 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -12,6 +12,7 @@ use codex_core::protocol::AgentReasoningEvent; use codex_core::protocol::AgentReasoningRawContentDeltaEvent; use codex_core::protocol::AgentReasoningRawContentEvent; use codex_core::protocol::ApplyPatchApprovalRequestEvent; +use codex_core::protocol::BackgroundEventEvent; use codex_core::protocol::ErrorEvent; use codex_core::protocol::Event; use codex_core::protocol::EventMsg; @@ -25,6 +26,7 @@ use codex_core::protocol::Op; use codex_core::protocol::PatchApplyBeginEvent; use codex_core::protocol::TaskCompleteEvent; use codex_core::protocol::TokenUsage; +use codex_core::protocol::TurnDiffEvent; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; use ratatui::buffer::Buffer; @@ -33,6 +35,7 @@ use ratatui::widgets::Widget; use ratatui::widgets::WidgetRef; use tokio::sync::mpsc::UnboundedSender; use tokio::sync::mpsc::unbounded_channel; +use tracing::info; use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; @@ -435,6 +438,9 @@ impl ChatWidget<'_> { changes, )); } + EventMsg::PatchApplyEnd(patch_apply_end_event) => { + self.add_to_history(HistoryCell::new_patch_end_event(patch_apply_end_event)); + } EventMsg::ExecCommandEnd(ExecCommandEndEvent { call_id, exit_code, @@ -492,13 +498,12 @@ impl ChatWidget<'_> { EventMsg::ShutdownComplete => { self.app_event_tx.send(AppEvent::ExitRequest); } - EventMsg::BackgroundEvent(event) => { - let message = event.message; - self.add_to_history(HistoryCell::new_background_event(message.clone())); - self.update_latest_log(message); + EventMsg::TurnDiff(TurnDiffEvent { unified_diff }) => { + info!("TurnDiffEvent: {unified_diff}"); + } + EventMsg::BackgroundEvent(BackgroundEventEvent { message }) => { + info!("BackgroundEvent: {message}"); } - // TODO: Think of how are we going to render these events. - EventMsg::PatchApplyEnd(_) | EventMsg::TurnDiff(_) => {} } } diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 2fb0eecb..332d90a6 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -12,6 +12,7 @@ use codex_core::plan_tool::StepStatus; use codex_core::plan_tool::UpdatePlanArgs; use codex_core::protocol::FileChange; use codex_core::protocol::McpInvocation; +use codex_core::protocol::PatchApplyEndEvent; use codex_core::protocol::SessionConfiguredEvent; use image::DynamicImage; use image::ImageReader; @@ -61,23 +62,35 @@ fn line_to_static(line: &Line) -> Line<'static> { /// scrollable list. pub(crate) enum HistoryCell { /// Welcome message. - WelcomeMessage { view: TextBlock }, + WelcomeMessage { + view: TextBlock, + }, /// Message from the user. - UserPrompt { view: TextBlock }, + UserPrompt { + view: TextBlock, + }, // AgentMessage and AgentReasoning variants were unused and have been removed. /// An exec tool call that has not finished yet. - ActiveExecCommand { view: TextBlock }, + ActiveExecCommand { + view: TextBlock, + }, /// Completed exec tool call. - CompletedExecCommand { view: TextBlock }, + CompletedExecCommand { + view: TextBlock, + }, /// An MCP tool call that has not finished yet. - ActiveMcpToolCall { view: TextBlock }, + ActiveMcpToolCall { + view: TextBlock, + }, /// Completed MCP tool call where we show the result serialized as JSON. - CompletedMcpToolCall { view: TextBlock }, + CompletedMcpToolCall { + view: TextBlock, + }, /// Completed MCP tool call where the result is an image. /// Admittedly, [mcp_types::CallToolResult] can have multiple content types, @@ -87,28 +100,46 @@ pub(crate) enum HistoryCell { // resized version avoids doing the potentially expensive rescale twice // because the scroll-view first calls `height()` for layouting and then // `render_window()` for painting. - CompletedMcpToolCallWithImageOutput { _image: DynamicImage }, + CompletedMcpToolCallWithImageOutput { + _image: DynamicImage, + }, /// Background event. - BackgroundEvent { view: TextBlock }, + BackgroundEvent { + view: TextBlock, + }, /// Output from the `/diff` command. - GitDiffOutput { view: TextBlock }, + GitDiffOutput { + view: TextBlock, + }, /// Error event from the backend. - ErrorEvent { view: TextBlock }, + ErrorEvent { + view: TextBlock, + }, /// Info describing the newly-initialized session. - SessionInfo { view: TextBlock }, + SessionInfo { + view: TextBlock, + }, /// A pending code patch that is awaiting user approval. Mirrors the /// behaviour of `ActiveExecCommand` so the user sees *what* patch the /// model wants to apply before being prompted to approve or deny it. - PendingPatch { view: TextBlock }, + PendingPatch { + view: TextBlock, + }, + + PatchEventEnd { + view: TextBlock, + }, /// A human‑friendly rendering of the model's current plan and step /// statuses provided via the `update_plan` tool. - PlanUpdate { view: TextBlock }, + PlanUpdate { + view: TextBlock, + }, } const TOOL_CALL_MAX_LINES: usize = 5; @@ -128,6 +159,7 @@ impl HistoryCell { | HistoryCell::CompletedExecCommand { view } | HistoryCell::CompletedMcpToolCall { view } | HistoryCell::PendingPatch { view } + | HistoryCell::PatchEventEnd { view } | HistoryCell::PlanUpdate { view } | HistoryCell::ActiveExecCommand { view, .. } | HistoryCell::ActiveMcpToolCall { view, .. } => { @@ -598,6 +630,28 @@ impl HistoryCell { view: TextBlock::new(lines), } } + + pub(crate) fn new_patch_end_event(patch_apply_end_event: PatchApplyEndEvent) -> Self { + let PatchApplyEndEvent { + call_id: _, + stdout: _, + stderr, + success, + } = patch_apply_end_event; + + let mut lines: Vec> = if success { + vec![Line::from("patch applied successfully".italic())] + } else { + let mut lines = vec![Line::from("patch failed".italic())]; + lines.extend(stderr.lines().map(|l| Line::from(l.to_string()))); + lines + }; + lines.push(Line::from("")); + + HistoryCell::PatchEventEnd { + view: TextBlock::new(lines), + } + } } fn create_diff_summary(changes: HashMap) -> Vec {