From 34aa1991f1287f69ecead71c3fb2a9165ebb1afd Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 14 May 2025 13:36:43 -0700 Subject: [PATCH] chore: handle all cases for EventMsg (#936) For now, this removes the `#[non_exhaustive]` directive on `EventMsg` so that we are forced to handle all `EventMsg` by default. (We may revisit this if/when we publish `core/` as a `lib` crate.) For now, it is helpful to have this as a forcing function because we have effectively two UIs (`tui` and `exec`) and usually when we add a new variant to `EventMsg`, we want to be sure that we update both. --- codex-rs/core/src/protocol.rs | 1 - codex-rs/core/tests/live_agent.rs | 12 +++++++++--- codex-rs/core/tests/previous_response_id.rs | 4 +++- codex-rs/exec/src/event_processor.rs | 11 +++++++---- codex-rs/mcp-server/src/codex_tool_runner.rs | 18 +++++++++++++++++- 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/codex-rs/core/src/protocol.rs b/codex-rs/core/src/protocol.rs index 80087430..f7f772f1 100644 --- a/codex-rs/core/src/protocol.rs +++ b/codex-rs/core/src/protocol.rs @@ -298,7 +298,6 @@ pub struct Event { } /// Response event from the agent -#[non_exhaustive] #[derive(Debug, Clone, Deserialize, Serialize)] #[serde(tag = "type", rename_all = "snake_case")] pub enum EventMsg { diff --git a/codex-rs/core/tests/live_agent.rs b/codex-rs/core/tests/live_agent.rs index d6afb895..83880d34 100644 --- a/codex-rs/core/tests/live_agent.rs +++ b/codex-rs/core/tests/live_agent.rs @@ -99,7 +99,9 @@ async fn live_streaming_and_prev_id_reset() { EventMsg::Error(ErrorEvent { message }) => { panic!("agent reported error in task1: {message}") } - _ => (), + _ => { + // Ignore other events. + } } } @@ -135,7 +137,9 @@ async fn live_streaming_and_prev_id_reset() { EventMsg::Error(ErrorEvent { message }) => { panic!("agent reported error in task2: {message}") } - _ => (), + _ => { + // Ignore other events. + } } } @@ -201,7 +205,9 @@ async fn live_shell_function_call() { EventMsg::Error(codex_core::protocol::ErrorEvent { message }) => { panic!("agent error during shell test: {message}") } - _ => (), + _ => { + // Ignore other events. + } } } diff --git a/codex-rs/core/tests/previous_response_id.rs b/codex-rs/core/tests/previous_response_id.rs index 166e2be3..f0ee8405 100644 --- a/codex-rs/core/tests/previous_response_id.rs +++ b/codex-rs/core/tests/previous_response_id.rs @@ -155,7 +155,9 @@ async fn keeps_previous_response_id_between_tasks() { EventMsg::Error(ErrorEvent { message }) => { panic!("unexpected error: {message}") } - _ => (), + _ => { + // Ignore other events. + } } } } diff --git a/codex-rs/exec/src/event_processor.rs b/codex-rs/exec/src/event_processor.rs index 191d616b..263e08cb 100644 --- a/codex-rs/exec/src/event_processor.rs +++ b/codex-rs/exec/src/event_processor.rs @@ -12,6 +12,7 @@ use codex_core::protocol::McpToolCallBeginEvent; use codex_core::protocol::McpToolCallEndEvent; use codex_core::protocol::PatchApplyBeginEvent; use codex_core::protocol::PatchApplyEndEvent; +use codex_core::protocol::SessionConfiguredEvent; use owo_colors::OwoColorize; use owo_colors::Style; use shlex::try_join; @@ -180,8 +181,6 @@ impl EventProcessor { } println!("{}", truncated_output.style(self.dimmed)); } - - // Handle MCP tool calls (e.g. calling external functions via MCP). EventMsg::McpToolCallBegin(McpToolCallBeginEvent { call_id, server, @@ -372,8 +371,12 @@ impl EventProcessor { EventMsg::ApplyPatchApprovalRequest(_) => { // Should we exit? } - _ => { - // Ignore event. + EventMsg::AgentReasoning(agent_reasoning_event) => { + println!("thinking: {}", agent_reasoning_event.text); + } + EventMsg::SessionConfigured(session_configured_event) => { + let SessionConfiguredEvent { session_id, model } = session_configured_event; + println!("session {session_id} with model {model}"); } } } diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 34534809..b70b8e9c 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -157,7 +157,23 @@ pub async fn run_codex_tool_session( EventMsg::SessionConfigured(_) => { tracing::error!("unexpected SessionConfigured event"); } - _ => {} + EventMsg::Error(_) + | EventMsg::TaskStarted + | EventMsg::AgentReasoning(_) + | EventMsg::McpToolCallBegin(_) + | EventMsg::McpToolCallEnd(_) + | EventMsg::ExecCommandBegin(_) + | EventMsg::ExecCommandEnd(_) + | EventMsg::BackgroundEvent(_) + | EventMsg::PatchApplyBegin(_) + | EventMsg::PatchApplyEnd(_) => { + // For now, we do not do anything extra for these + // events. Note that + // send(codex_event_to_notification(&event)) above has + // already dispatched these events as notifications, + // though we may want to do give different treatment to + // individual events in the future. + } } } Err(e) => {