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`
This commit is contained in:
Michael Bolin
2025-08-05 22:44:27 -07:00
committed by GitHub
parent f8d70d67b6
commit eaf2fb5b4f
2 changed files with 78 additions and 19 deletions

View File

@@ -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(_) => {}
}
}

View File

@@ -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 humanfriendly 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<Line<'static>> = 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<PathBuf, FileChange>) -> Vec<String> {