fix: change EventMsg enum so every variant takes a single struct (#925)

https://github.com/openai/codex/pull/922 did this for the `SessionConfigured` enum variant, and I think it is generally helpful to be able to work with the values as each enum variant as their own type, so this converts the remaining variants and updates all of the callsites. Added a simple unit test to verify that the JSON-serialized version of `Event` does not have any unexpected nesting.
2025-05-13 20:44:42 -07:00
parent e6c206d19d
commit a5f3a34827
9 changed files with 288 additions and 183 deletions
--- a/codex-rs/core/src/protocol.rs
+++ b/codex-rs/core/src/protocol.rs
@@ -303,9 +303,7 @@ pub struct Event {
 #[serde(tag = "type", rename_all = "snake_case")]
 pub enum EventMsg {
    /// Error while executing a submission
-    Error {
-        message: String,
-    },
+    Error(ErrorEvent),

    /// Agent has started a task
    TaskStarted,
@@ -314,117 +312,145 @@ pub enum EventMsg {
    TaskComplete,

    /// Agent text output message
-    AgentMessage {
-        message: String,
-    },
+    AgentMessage(AgentMessageEvent),

    /// Reasoning event from agent.
-    AgentReasoning {
-        text: String,
-    },
+    AgentReasoning(AgentReasoningEvent),

    /// Ack the client's configure message.
    SessionConfigured(SessionConfiguredEvent),

-    McpToolCallBegin {
-        /// Identifier so this can be paired with the McpToolCallEnd event.
-        call_id: String,
+    McpToolCallBegin(McpToolCallBeginEvent),

-        /// Name of the MCP server as defined in the config.
-        server: String,
-
-        /// Name of the tool as given by the MCP server.
-        tool: String,
-
-        /// Arguments to the tool call.
-        arguments: Option<serde_json::Value>,
-    },
-
-    McpToolCallEnd {
-        /// Identifier for the McpToolCallBegin that finished.
-        call_id: String,
-
-        /// Whether the tool call was successful. If `false`, `result` might
-        /// not be present.
-        success: bool,
-
-        /// Result of the tool call. Note this could be an error.
-        result: Option<CallToolResult>,
-    },
+    McpToolCallEnd(McpToolCallEndEvent),

    /// Notification that the server is about to execute a command.
-    ExecCommandBegin {
-        /// Identifier so this can be paired with the ExecCommandEnd event.
-        call_id: String,
-        /// The command to be executed.
-        command: Vec<String>,
-        /// The command's working directory if not the default cwd for the
-        /// agent.
-        cwd: PathBuf,
-    },
+    ExecCommandBegin(ExecCommandBeginEvent),

-    ExecCommandEnd {
-        /// Identifier for the ExecCommandBegin that finished.
-        call_id: String,
-        /// Captured stdout
-        stdout: String,
-        /// Captured stderr
-        stderr: String,
-        /// The command's exit code.
-        exit_code: i32,
-    },
+    ExecCommandEnd(ExecCommandEndEvent),

-    ExecApprovalRequest {
-        /// The command to be executed.
-        command: Vec<String>,
-        /// The command's working directory.
-        cwd: PathBuf,
-        /// Optional human‑readable reason for the approval (e.g. retry without
-        /// sandbox).
-        #[serde(skip_serializing_if = "Option::is_none")]
-        reason: Option<String>,
-    },
+    ExecApprovalRequest(ExecApprovalRequestEvent),

-    ApplyPatchApprovalRequest {
-        changes: HashMap<PathBuf, FileChange>,
-        /// Optional explanatory reason (e.g. request for extra write access).
-        #[serde(skip_serializing_if = "Option::is_none")]
-        reason: Option<String>,
+    ApplyPatchApprovalRequest(ApplyPatchApprovalRequestEvent),

-        /// When set, the agent is asking the user to allow writes under this
-        /// root for the remainder of the session.
-        #[serde(skip_serializing_if = "Option::is_none")]
-        grant_root: Option<PathBuf>,
-    },
-
-    BackgroundEvent {
-        message: String,
-    },
+    BackgroundEvent(BackgroundEventEvent),

    /// Notification that the agent is about to apply a code patch. Mirrors
    /// `ExecCommandBegin` so front‑ends can show progress indicators.
-    PatchApplyBegin {
-        /// Identifier so this can be paired with the PatchApplyEnd event.
-        call_id: String,
-
-        /// If true, there was no ApplyPatchApprovalRequest for this patch.
-        auto_approved: bool,
-
-        /// The changes to be applied.
-        changes: HashMap<PathBuf, FileChange>,
-    },
+    PatchApplyBegin(PatchApplyBeginEvent),

    /// Notification that a patch application has finished.
-    PatchApplyEnd {
-        /// Identifier for the PatchApplyBegin that finished.
-        call_id: String,
-        /// Captured stdout (summary printed by apply_patch).
-        stdout: String,
-        /// Captured stderr (parser errors, IO failures, etc.).
-        stderr: String,
-        /// Whether the patch was applied successfully.
-        success: bool,
-    },
+    PatchApplyEnd(PatchApplyEndEvent),
+}
+
+// Individual event payload types matching each `EventMsg` variant.
+
+#[derive(Debug, Clone, Deserialize, Serialize)]
+pub struct ErrorEvent {
+    pub message: String,
+}
+
+#[derive(Debug, Clone, Deserialize, Serialize)]
+pub struct AgentMessageEvent {
+    pub message: String,
+}
+
+#[derive(Debug, Clone, Deserialize, Serialize)]
+pub struct AgentReasoningEvent {
+    pub text: String,
+}
+
+#[derive(Debug, Clone, Deserialize, Serialize)]
+pub struct McpToolCallBeginEvent {
+    /// Identifier so this can be paired with the McpToolCallEnd event.
+    pub call_id: String,
+    /// Name of the MCP server as defined in the config.
+    pub server: String,
+    /// Name of the tool as given by the MCP server.
+    pub tool: String,
+    /// Arguments to the tool call.
+    pub arguments: Option<serde_json::Value>,
+}
+
+#[derive(Debug, Clone, Deserialize, Serialize)]
+pub struct McpToolCallEndEvent {
+    /// Identifier for the corresponding McpToolCallBegin that finished.
+    pub call_id: String,
+    /// Whether the tool call was successful. If `false`, `result` might not be present.
+    pub success: bool,
+    /// Result of the tool call. Note this could be an error.
+    pub result: Option<CallToolResult>,
+}
+
+#[derive(Debug, Clone, Deserialize, Serialize)]
+pub struct ExecCommandBeginEvent {
+    /// Identifier so this can be paired with the ExecCommandEnd event.
+    pub call_id: String,
+    /// The command to be executed.
+    pub command: Vec<String>,
+    /// The command's working directory if not the default cwd for the agent.
+    pub cwd: PathBuf,
+}
+
+#[derive(Debug, Clone, Deserialize, Serialize)]
+pub struct ExecCommandEndEvent {
+    /// Identifier for the ExecCommandBegin that finished.
+    pub call_id: String,
+    /// Captured stdout
+    pub stdout: String,
+    /// Captured stderr
+    pub stderr: String,
+    /// The command's exit code.
+    pub exit_code: i32,
+}
+
+#[derive(Debug, Clone, Deserialize, Serialize)]
+pub struct ExecApprovalRequestEvent {
+    /// The command to be executed.
+    pub command: Vec<String>,
+    /// The command's working directory.
+    pub cwd: PathBuf,
+    /// Optional human-readable reason for the approval (e.g. retry without sandbox).
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub reason: Option<String>,
+}
+
+#[derive(Debug, Clone, Deserialize, Serialize)]
+pub struct ApplyPatchApprovalRequestEvent {
+    pub changes: HashMap<PathBuf, FileChange>,
+    /// Optional explanatory reason (e.g. request for extra write access).
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub reason: Option<String>,
+    /// When set, the agent is asking the user to allow writes under this root for the remainder of the session.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub grant_root: Option<PathBuf>,
+}
+
+#[derive(Debug, Clone, Deserialize, Serialize)]
+pub struct BackgroundEventEvent {
+    pub message: String,
+}
+
+#[derive(Debug, Clone, Deserialize, Serialize)]
+pub struct PatchApplyBeginEvent {
+    /// Identifier so this can be paired with the PatchApplyEnd event.
+    pub call_id: String,
+    /// If true, there was no ApplyPatchApprovalRequest for this patch.
+    pub auto_approved: bool,
+    /// The changes to be applied.
+    pub changes: HashMap<PathBuf, FileChange>,
+}
+
+#[derive(Debug, Clone, Deserialize, Serialize)]
+pub struct PatchApplyEndEvent {
+    /// Identifier for the PatchApplyBegin that finished.
+    pub call_id: String,
+    /// Captured stdout (summary printed by apply_patch).
+    pub stdout: String,
+    /// Captured stderr (parser errors, IO failures, etc.).
+    pub stderr: String,
+    /// Whether the patch was applied successfully.
+    pub success: bool,
 }

 #[derive(Debug, Default, Clone, Deserialize, Serialize)]
@@ -478,3 +504,28 @@ pub struct Chunk {
    pub deleted_lines: Vec<String>,
    pub inserted_lines: Vec<String>,
 }
+
+#[cfg(test)]
+mod tests {
+    #![allow(clippy::unwrap_used)]
+    use super::*;
+
+    /// Serialize Event to verify that its JSON representation has the expected
+    /// amount of nesting.
+    #[test]
+    fn serialize_event() {
+        let session_id: Uuid = uuid::uuid!("67e55044-10b1-426f-9247-bb680e5fe0c8");
+        let event = Event {
+            id: "1234".to_string(),
+            msg: EventMsg::SessionConfigured(SessionConfiguredEvent {
+                session_id,
+                model: "o4-mini".to_string(),
+            }),
+        };
+        let serialized = serde_json::to_string(&event).unwrap();
+        assert_eq!(
+            serialized,
+            r#"{"id":"1234","msg":{"type":"session_configured","session_id":"67e55044-10b1-426f-9247-bb680e5fe0c8","model":"o4-mini"}}"#
+        );
+    }
+}