From 4530a2826975716af59c0c7f0ef1abf6e2538169 Mon Sep 17 00:00:00 2001 From: Charles Cunningham Date: Fri, 6 Mar 2026 10:42:29 -0800 Subject: [PATCH] codex: split rollout response id events Replace the rollout-only response metadata event with separate ResponsesApiRequestId and ResponsesApiResponseId events so persisted logs do not carry unrelated null fields. Co-authored-by: Codex --- .../app-server/src/codex_message_processor.rs | 6 ++- .../src/endpoint/responses_websocket.rs | 2 +- codex-rs/core/src/codex.rs | 21 +++++----- codex-rs/core/src/rollout/policy.rs | 3 +- codex-rs/core/tests/suite/client.rs | 40 ++++++++----------- .../src/event_processor_with_human_output.rs | 6 ++- codex-rs/exec/src/lib.rs | 3 +- codex-rs/mcp-server/src/codex_tool_runner.rs | 3 +- codex-rs/protocol/src/protocol.rs | 30 ++++++++++---- codex-rs/tui/src/chatwidget.rs | 3 +- 10 files changed, 65 insertions(+), 52 deletions(-) diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 626411dbb3..298c2b2ba3 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -5838,7 +5838,11 @@ impl CodexMessageProcessor { } }; let event = event.clone(); - if matches!(event.msg, EventMsg::ResponseMetadata(_)) { + if matches!( + event.msg, + EventMsg::ResponsesApiRequestId(_) + | EventMsg::ResponsesApiResponseId(_) + ) { continue; } diff --git a/codex-rs/codex-api/src/endpoint/responses_websocket.rs b/codex-rs/codex-api/src/endpoint/responses_websocket.rs index 9ffac248e4..98d679112a 100644 --- a/codex-rs/codex-api/src/endpoint/responses_websocket.rs +++ b/codex-rs/codex-api/src/endpoint/responses_websocket.rs @@ -270,7 +270,7 @@ impl ResponsesWebsocketConnection { rx_event, // Websocket upgrade response headers are scoped to the connection, not // individual `stream_request` calls, so they cannot be used as per-turn - // request IDs in rollout `ResponseMetadata`. + // request IDs in rollout `ResponsesApiRequestId` events. request_id_for_rollout_log: None, }) } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 6df82f005c..ea184cf3c0 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -89,7 +89,8 @@ use codex_protocol::protocol::HasLegacyEvent; use codex_protocol::protocol::ItemCompletedEvent; use codex_protocol::protocol::ItemStartedEvent; use codex_protocol::protocol::RawResponseItemEvent; -use codex_protocol::protocol::ResponseMetadataEvent; +use codex_protocol::protocol::ResponsesApiRequestIdEvent; +use codex_protocol::protocol::ResponsesApiResponseIdEvent; use codex_protocol::protocol::ReviewRequest; use codex_protocol::protocol::RolloutItem; use codex_protocol::protocol::SessionSource; @@ -5987,7 +5988,8 @@ fn realtime_text_for_event(msg: &EventMsg) -> Option { | EventMsg::ShutdownComplete | EventMsg::EnteredReviewMode(_) | EventMsg::ExitedReviewMode(_) - | EventMsg::ResponseMetadata(_) + | EventMsg::ResponsesApiRequestId(_) + | EventMsg::ResponsesApiResponseId(_) | EventMsg::RawResponseItem(_) | EventMsg::ItemStarted(_) | EventMsg::AgentMessageContentDelta(_) @@ -6329,9 +6331,8 @@ async fn try_run_sampling_request( if let Some(request_id) = err.request_id() { sess.send_event( &turn_context, - EventMsg::ResponseMetadata(ResponseMetadataEvent { - request_id: Some(request_id.to_string()), - response_id: None, + EventMsg::ResponsesApiRequestId(ResponsesApiRequestIdEvent { + request_id: request_id.to_string(), }), ) .await; @@ -6343,10 +6344,7 @@ async fn try_run_sampling_request( if let Some(request_id) = stream.request_id_for_rollout_log.clone() { sess.send_event( &turn_context, - EventMsg::ResponseMetadata(ResponseMetadataEvent { - request_id: Some(request_id), - response_id: None, - }), + EventMsg::ResponsesApiRequestId(ResponsesApiRequestIdEvent { request_id }), ) .await; } @@ -6405,9 +6403,8 @@ async fn try_run_sampling_request( if let Some(response_id) = response_id { sess.send_event( &turn_context, - EventMsg::ResponseMetadata(ResponseMetadataEvent { - request_id: None, - response_id: Some(response_id), + EventMsg::ResponsesApiResponseId(ResponsesApiResponseIdEvent { + response_id, }), ) .await; diff --git a/codex-rs/core/src/rollout/policy.rs b/codex-rs/core/src/rollout/policy.rs index 8bf8964644..6f6f6c8465 100644 --- a/codex-rs/core/src/rollout/policy.rs +++ b/codex-rs/core/src/rollout/policy.rs @@ -91,7 +91,8 @@ fn event_msg_persistence_mode(ev: &EventMsg) -> Option { match ev { EventMsg::UserMessage(_) | EventMsg::AgentMessage(_) - | EventMsg::ResponseMetadata(_) + | EventMsg::ResponsesApiRequestId(_) + | EventMsg::ResponsesApiResponseId(_) | EventMsg::AgentReasoning(_) | EventMsg::AgentReasoningRawContent(_) | EventMsg::TokenCount(_) diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 4a5a4edf23..d4c414d287 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -180,29 +180,25 @@ async fn rollout_persists_responses_request_and_response_ids() { assert!(lines.iter().any(|line| { matches!( &line.item, - RolloutItem::EventMsg(EventMsg::ResponseMetadata(event)) - if event.request_id.as_deref() == Some("req_123") - && event.response_id.is_none() + RolloutItem::EventMsg(EventMsg::ResponsesApiRequestId(event)) + if event.request_id == "req_123" ) })); assert!(lines.iter().any(|line| { matches!( &line.item, - RolloutItem::EventMsg(EventMsg::ResponseMetadata(event)) - if event.request_id.is_none() - && event.response_id.as_deref() == Some("resp_123") + RolloutItem::EventMsg(EventMsg::ResponsesApiResponseId(event)) + if event.response_id == "resp_123" ) })); assert!(json_lines.iter().any(|line| { line["type"] == "event_msg" - && line["payload"]["type"] == "response_metadata" + && line["payload"]["type"] == "responses_api_request_id" && line["payload"]["request_id"] == "req_123" - && line["payload"]["response_id"].is_null() })); assert!(json_lines.iter().any(|line| { line["type"] == "event_msg" - && line["payload"]["type"] == "response_metadata" - && line["payload"]["request_id"].is_null() + && line["payload"]["type"] == "responses_api_response_id" && line["payload"]["response_id"] == "resp_123" })); } @@ -242,23 +238,21 @@ async fn rollout_persists_request_id_when_responses_request_errors() -> anyhow:: }) .await?; - let response_metadata = wait_for_event(&codex, |event| { + let request_id_event = wait_for_event(&codex, |event| { matches!( event, - EventMsg::ResponseMetadata(metadata) - if metadata.request_id.as_deref() == Some("req_error_123") - && metadata.response_id.is_none() + EventMsg::ResponsesApiRequestId(metadata) + if metadata.request_id == "req_error_123" ) }) .await; assert!( matches!( - &response_metadata, - EventMsg::ResponseMetadata(metadata) - if metadata.request_id.as_deref() == Some("req_error_123") - && metadata.response_id.is_none() + &request_id_event, + EventMsg::ResponsesApiRequestId(metadata) + if metadata.request_id == "req_error_123" ), - "expected response metadata with request id; got {response_metadata:?}" + "expected request id event; got {request_id_event:?}" ); let error_event = wait_for_event(&codex, |event| matches!(event, EventMsg::Error(_))).await; @@ -285,16 +279,14 @@ async fn rollout_persists_request_id_when_responses_request_errors() -> anyhow:: assert!(lines.iter().any(|line| { matches!( &line.item, - RolloutItem::EventMsg(EventMsg::ResponseMetadata(event)) - if event.request_id.as_deref() == Some("req_error_123") - && event.response_id.is_none() + RolloutItem::EventMsg(EventMsg::ResponsesApiRequestId(event)) + if event.request_id == "req_error_123" ) })); assert!(json_lines.iter().any(|line| { line["type"] == "event_msg" - && line["payload"]["type"] == "response_metadata" + && line["payload"]["type"] == "responses_api_request_id" && line["payload"]["request_id"] == "req_error_123" - && line["payload"]["response_id"].is_null() })); Ok(()) 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 fb4da6d1e6..f16c09bf83 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -848,7 +848,8 @@ impl EventProcessor for EventProcessorWithHumanOutput { | EventMsg::ListRemoteSkillsResponse(_) | EventMsg::RemoteSkillDownloaded(_) | EventMsg::RawResponseItem(_) - | EventMsg::ResponseMetadata(_) + | EventMsg::ResponsesApiRequestId(_) + | EventMsg::ResponsesApiResponseId(_) | EventMsg::UserMessage(_) | EventMsg::EnteredReviewMode(_) | EventMsg::ExitedReviewMode(_) @@ -930,7 +931,8 @@ impl EventProcessorWithHumanOutput { | EventMsg::ListRemoteSkillsResponse(_) | EventMsg::RemoteSkillDownloaded(_) | EventMsg::RawResponseItem(_) - | EventMsg::ResponseMetadata(_) + | EventMsg::ResponsesApiRequestId(_) + | EventMsg::ResponsesApiResponseId(_) | EventMsg::UserMessage(_) | EventMsg::EnteredReviewMode(_) | EventMsg::ExitedReviewMode(_) diff --git a/codex-rs/exec/src/lib.rs b/codex-rs/exec/src/lib.rs index 59479f5a0e..360bbbcf9f 100644 --- a/codex-rs/exec/src/lib.rs +++ b/codex-rs/exec/src/lib.rs @@ -789,7 +789,8 @@ fn should_suppress_agent_job_event(msg: &EventMsg) -> bool { | EventMsg::RequestUserInput(_) | EventMsg::DynamicToolCallRequest(_) | EventMsg::DynamicToolCallResponse(_) - | EventMsg::ResponseMetadata(_) + | EventMsg::ResponsesApiRequestId(_) + | EventMsg::ResponsesApiResponseId(_) | EventMsg::ElicitationRequest(_) | EventMsg::Error(_) | EventMsg::Warning(_) diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 2a3d13b6d6..99124fcbcb 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -355,7 +355,8 @@ async fn run_codex_tool_session_inner( | EventMsg::ImageGenerationBegin(_) | EventMsg::ImageGenerationEnd(_) | EventMsg::RawResponseItem(_) - | EventMsg::ResponseMetadata(_) + | EventMsg::ResponsesApiRequestId(_) + | EventMsg::ResponsesApiResponseId(_) | EventMsg::EnteredReviewMode(_) | EventMsg::ItemStarted(_) | EventMsg::ItemCompleted(_) diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 4944823eff..50effe26d7 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -1178,7 +1178,11 @@ pub enum EventMsg { #[schemars(skip)] #[ts(skip)] - ResponseMetadata(ResponseMetadataEvent), + ResponsesApiRequestId(ResponsesApiRequestIdEvent), + + #[schemars(skip)] + #[ts(skip)] + ResponsesApiResponseId(ResponsesApiResponseIdEvent), RawResponseItem(RawResponseItemEvent), @@ -1361,11 +1365,13 @@ impl CodexErrorInfo { } #[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)] -pub struct ResponseMetadataEvent { - #[serde(default)] - pub request_id: Option, - #[serde(default)] - pub response_id: Option, +pub struct ResponsesApiRequestIdEvent { + pub request_id: String, +} + +#[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)] +pub struct ResponsesApiResponseIdEvent { + pub response_id: String, } #[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)] @@ -1980,7 +1986,11 @@ impl InitialHistory { .iter() .filter_map(|ri| match ri { RolloutItem::EventMsg(ev) - if !matches!(ev, EventMsg::ResponseMetadata(_)) => + if !matches!( + ev, + EventMsg::ResponsesApiRequestId(_) + | EventMsg::ResponsesApiResponseId(_) + ) => { Some(ev.clone()) } @@ -1993,7 +2003,11 @@ impl InitialHistory { .iter() .filter_map(|ri| match ri { RolloutItem::EventMsg(ev) - if !matches!(ev, EventMsg::ResponseMetadata(_)) => + if !matches!( + ev, + EventMsg::ResponsesApiRequestId(_) + | EventMsg::ResponsesApiResponseId(_) + ) => { Some(ev.clone()) } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 817039fdeb..c6072b1ad7 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -4830,7 +4830,8 @@ impl ChatWidget { } } EventMsg::RawResponseItem(_) - | EventMsg::ResponseMetadata(_) + | EventMsg::ResponsesApiRequestId(_) + | EventMsg::ResponsesApiResponseId(_) | EventMsg::ItemStarted(_) | EventMsg::AgentMessageContentDelta(_) | EventMsg::ReasoningContentDelta(_)