From 8f3431054fe44dbf0d901bb12c522bb87c64fd47 Mon Sep 17 00:00:00 2001 From: "Adam Perry @ OpenAI" Date: Mon, 6 Jul 2026 19:08:50 -0700 Subject: [PATCH 1/7] fix: restore Codex environment setup table (#31337) ## Why Creating a Codex worktree skips local environment setup because `.codex/environments/environment.toml` omits the required `setup` object. ## What Restore an empty `[setup]` table with `script = ""`, preserving the intended no-op setup while satisfying the environment parser schema. ## Validation - Parsed the final TOML and verified `setup.script` is present as a string. --- .codex/environments/environment.toml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.codex/environments/environment.toml b/.codex/environments/environment.toml index b3c5910fbc..ae0bd24ca8 100644 --- a/.codex/environments/environment.toml +++ b/.codex/environments/environment.toml @@ -2,6 +2,10 @@ version = 1 name = "codex" +# TODO(anp) make it optional to specify this field +[setup] +script = "" + [[actions]] name = "Run" icon = "run" From 831c14fc39f7810703263c31a4a712842dba35f4 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 6 Jul 2026 19:28:44 -0700 Subject: [PATCH 2/7] Preserve managed exec policy after rules parse errors (#31188) ## Why Cleanup for #31179 exposed a core fallback bug that the TUI had been handling locally. When a custom `.rules` file fails to parse, nonfatal clients warn and continue, but `load_exec_policy_with_warning` replaced the entire policy with `Policy::empty()` before managed requirements were merged. App-server and desktop clients could therefore silently lose required prompt and forbidden rules. This fix is intentionally separate from #31179 so the TUI cleanup does not depend on it. ## What changed - Preserve the managed requirements exec policy when custom file rules fail to parse, while returning the existing warning and discarding the file-based policy. - Use the same nonfatal fallback when loading network proxy policy. - Keep parse errors fatal for strict clients such as `codex exec`. --- codex-rs/core/src/exec_policy.rs | 11 ++- codex-rs/core/src/exec_policy_tests.rs | 36 +++++++++ codex-rs/core/src/network_proxy_loader.rs | 27 +++---- .../core/src/network_proxy_loader_tests.rs | 50 ++++++++++++ codex-rs/core/tests/suite/exec_policy.rs | 80 +++++++++++++++++++ 5 files changed, 188 insertions(+), 16 deletions(-) diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index cbffcd7e43..8f31f150b3 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -554,12 +554,19 @@ pub fn format_exec_policy_error_with_source(error: &ExecPolicyError) -> String { } } -async fn load_exec_policy_with_warning( +pub(crate) async fn load_exec_policy_with_warning( config_stack: &ConfigLayerStack, ) -> Result<(Policy, Option), ExecPolicyError> { match load_exec_policy(config_stack).await { Ok(policy) => Ok((policy, None)), - Err(err @ ExecPolicyError::ParsePolicy { .. }) => Ok((Policy::empty(), Some(err))), + Err(err @ ExecPolicyError::ParsePolicy { .. }) => { + let policy = config_stack + .requirements() + .exec_policy + .as_deref() + .map_or_else(Policy::empty, |policy| policy.as_ref().clone()); + Ok((policy, Some(err))) + } Err(err) => Err(err), } } diff --git a/codex-rs/core/src/exec_policy_tests.rs b/codex-rs/core/src/exec_policy_tests.rs index cf17387ef5..95d29e0bac 100644 --- a/codex-rs/core/src/exec_policy_tests.rs +++ b/codex-rs/core/src/exec_policy_tests.rs @@ -374,6 +374,42 @@ async fn merges_requirements_exec_policy_network_rules() -> anyhow::Result<()> { Ok(()) } +#[tokio::test] +async fn malformed_custom_rules_preserve_requirements_exec_policy() -> anyhow::Result<()> { + let temp_dir = tempdir()?; + let policy_dir = temp_dir.path().join(RULES_DIR_NAME); + fs::create_dir_all(&policy_dir)?; + fs::write(policy_dir.join("broken.rules"), "prefix_rule(")?; + + let mut requirements_exec_policy = Policy::empty(); + requirements_exec_policy.add_prefix_rule(&["rm".to_string()], Decision::Forbidden)?; + let requirements = ConfigRequirements { + exec_policy: Some(Sourced::new( + RequirementsExecPolicy::new(requirements_exec_policy), + RequirementSource::Unknown, + )), + ..ConfigRequirements::default() + }; + let dot_codex_folder = AbsolutePathBuf::from_absolute_path(temp_dir.path())?; + let layer = ConfigLayerEntry::new( + ConfigLayerSource::Project { dot_codex_folder }, + TomlValue::Table(Default::default()), + ); + let config_stack = + ConfigLayerStack::new(vec![layer], requirements, ConfigRequirementsToml::default())?; + + let (policy, warning) = load_exec_policy_with_warning(&config_stack).await?; + + assert!(matches!(warning, Some(ExecPolicyError::ParsePolicy { .. }))); + assert_eq!( + policy + .check_multiple([vec!["rm".to_string()]].iter(), &|_| Decision::Allow) + .decision, + Decision::Forbidden + ); + Ok(()) +} + #[tokio::test] async fn preserves_host_executables_when_requirements_overlay_is_present() -> anyhow::Result<()> { let temp_dir = tempdir()?; diff --git a/codex-rs/core/src/network_proxy_loader.rs b/codex-rs/core/src/network_proxy_loader.rs index 0a4fe2d7c2..62ed44ad81 100644 --- a/codex-rs/core/src/network_proxy_loader.rs +++ b/codex-rs/core/src/network_proxy_loader.rs @@ -2,9 +2,8 @@ use crate::config::find_codex_home; use crate::config::is_builtin_permission_profile_name; use crate::config::reject_unknown_builtin_permission_profile; use crate::config::resolve_permission_profile; -use crate::exec_policy::ExecPolicyError; use crate::exec_policy::format_exec_policy_error_with_source; -use crate::exec_policy::load_exec_policy; +use crate::exec_policy::load_exec_policy_with_warning; use anyhow::Context; use anyhow::Result; use codex_config::CONFIG_TOML_FILE; @@ -64,13 +63,15 @@ async fn build_config_state_with_mtimes() -> Result<(ConfigState, Vec (policy, None), - Err(err @ ExecPolicyError::ParsePolicy { .. }) => { - (codex_execpolicy::Policy::empty(), Some(err)) - } - Err(err) => return Err(err.into()), - }; + let layer_mtimes = collect_layer_mtimes(&config_layer_stack); + let state = build_config_state_from_layers(&config_layer_stack).await?; + Ok((state, layer_mtimes)) +} + +async fn build_config_state_from_layers( + config_layer_stack: &ConfigLayerStack, +) -> Result { + let (exec_policy, warning) = load_exec_policy_with_warning(config_layer_stack).await?; if let Some(err) = warning.as_ref() { tracing::warn!( "failed to parse execpolicy while building network proxy state: {}", @@ -78,12 +79,10 @@ async fn build_config_state_with_mtimes() -> Result<(ConfigState, Vec Vec { diff --git a/codex-rs/core/src/network_proxy_loader_tests.rs b/codex-rs/core/src/network_proxy_loader_tests.rs index d6c8efc9ae..f7c7c2e452 100644 --- a/codex-rs/core/src/network_proxy_loader_tests.rs +++ b/codex-rs/core/src/network_proxy_loader_tests.rs @@ -5,6 +5,9 @@ use codex_config::ConfigLayerSource; use codex_config::ConfigLayerStack; use codex_config::ConfigRequirements; use codex_config::ConfigRequirementsToml; +use codex_config::RequirementSource; +use codex_config::RequirementsExecPolicy; +use codex_config::Sourced; use codex_config::permissions_toml::NetworkDomainPermissionToml; use codex_config::permissions_toml::NetworkDomainPermissionsToml; use codex_execpolicy::Decision; @@ -12,6 +15,8 @@ use codex_execpolicy::NetworkRuleProtocol; use codex_execpolicy::Policy; use pretty_assertions::assert_eq; use std::collections::BTreeMap; +use std::fs; +use tempfile::tempdir; #[test] fn higher_precedence_profile_network_overlays_domain_entries() { @@ -225,6 +230,51 @@ fn execpolicy_network_rules_overlay_network_lists() { ); } +#[tokio::test] +async fn malformed_custom_rules_preserve_managed_denied_domain() { + let temp_dir = tempdir().expect("create temp dir"); + let policy_dir = temp_dir.path().join("rules"); + fs::create_dir_all(&policy_dir).expect("create policy dir"); + fs::write(policy_dir.join("broken.rules"), "prefix_rule(").expect("write malformed policy"); + + let mut requirements_exec_policy = Policy::empty(); + requirements_exec_policy + .add_network_rule( + "blocked.example.com", + NetworkRuleProtocol::Https, + Decision::Forbidden, + /*justification*/ None, + ) + .expect("managed network rule should be valid"); + let requirements = ConfigRequirements { + exec_policy: Some(Sourced::new( + RequirementsExecPolicy::new(requirements_exec_policy), + RequirementSource::Unknown, + )), + ..ConfigRequirements::default() + }; + let dot_codex_folder = AbsolutePathBuf::from_absolute_path(temp_dir.path()) + .expect("dot codex folder should be absolute"); + let layers = ConfigLayerStack::new( + vec![ConfigLayerEntry::new( + ConfigLayerSource::Project { dot_codex_folder }, + toml::Value::Table(Default::default()), + )], + requirements, + ConfigRequirementsToml::default(), + ) + .expect("layer stack should be valid"); + + let state = build_config_state_from_layers(&layers) + .await + .expect("proxy state should tolerate malformed custom rules"); + + assert_eq!( + state.config.network.denied_domains(), + Some(vec!["blocked.example.com".to_string()]) + ); +} + #[test] fn apply_network_constraints_includes_allow_all_unix_sockets_flag() { let config: toml::Value = toml::from_str( diff --git a/codex-rs/core/tests/suite/exec_policy.rs b/codex-rs/core/tests/suite/exec_policy.rs index 4e64c66b0a..0c73b74ab2 100644 --- a/codex-rs/core/tests/suite/exec_policy.rs +++ b/codex-rs/core/tests/suite/exec_policy.rs @@ -1,6 +1,7 @@ #![allow(clippy::unwrap_used)] use anyhow::Result; +use codex_config::test_support::CloudConfigBundleFixture; use codex_features::Feature; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; @@ -17,6 +18,7 @@ use core_test_support::responses::ev_response_created; use core_test_support::responses::mount_sse_once; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; +use core_test_support::skip_if_target_windows; use core_test_support::test_codex::local_selections; use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; @@ -255,6 +257,84 @@ async fn execpolicy_blocks_shell_invocation() -> Result<()> { Ok(()) } +#[tokio::test] +async fn malformed_custom_rules_preserve_managed_forbidden_prefix() -> Result<()> { + skip_if_target_windows!( + Ok(()), + "managed prefix fixture uses POSIX executable semantics" + ); + + let mut builder = test_codex() + .with_cloud_config_bundle( + CloudConfigBundleFixture::loader_with_enterprise_requirement( + r#" +[rules] +prefix_rules = [ + { pattern = [{ token = "echo" }], decision = "forbidden" }, +] +"#, + ), + ) + .with_config(|config| { + config + .features + .enable(Feature::UnifiedExec) + .expect("test config should allow feature update"); + let policy_path = config.codex_home.join("rules").join("broken.rules"); + fs::create_dir_all( + policy_path + .parent() + .expect("policy directory must have a parent"), + ) + .expect("create policy directory"); + fs::write(policy_path, "prefix_rule(").expect("write malformed policy file"); + }); + let server = start_mock_server().await; + let test = builder.build_with_auto_env(&server).await?; + let call_id = "managed-shell-forbidden"; + let args = json!({ + "cmd": "echo blocked", + "yield_time_ms": 1_000, + }); + + mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-managed-1"), + ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?), + ev_completed("resp-managed-1"), + ]), + ) + .await; + let results_mock = mount_sse_once( + &server, + sse(vec![ + ev_assistant_message("msg-managed-1", "done"), + ev_completed("resp-managed-2"), + ]), + ) + .await; + + test.submit_turn_with_approval_and_permission_profile( + "run shell command", + AskForApproval::Never, + PermissionProfile::Disabled, + ) + .await?; + + let output_item = results_mock.single_request().function_call_output(call_id); + let output = output_item + .get("output") + .and_then(Value::as_str) + .expect("function call output should include a string output payload"); + assert!( + output.contains("policy forbids commands starting with `echo`"), + "unexpected output: {output}" + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn shell_command_empty_script_with_collaboration_mode_does_not_panic() -> Result<()> { let server = start_mock_server().await; From 775ef7dcc7d50069e759f3e11454f412a3e39d95 Mon Sep 17 00:00:00 2001 From: ashwinnathan-openai Date: Mon, 6 Jul 2026 23:23:56 -0400 Subject: [PATCH 3/7] [codex] Support sequential cutoff reasoning summaries (#31306) ## Summary When enabled for the OpenAI provider, Codex sends `stream_options.reasoning_summary_delivery = "sequential_cutoff"` on HTTP and WebSocket requests, including prewarm, and renders completed summary sections from `reasoning_summary_text.done`. Flag-off and non-OpenAI behavior is unchanged. ## Expected rollout ```text reasoning 0 added summary 0 done summary 1 done summary 2 starts summary 2 cancelled / incomplete reasoning 0 done <-- cancel summary 2 work and mark it incomplete message 1 added message 1 text streams message 1 completed ``` Depends on [openai/openai#1096660](https://github.com/openai/openai/pull/1096660). --- codex-rs/codex-api/src/common.rs | 21 +++++ .../src/endpoint/responses_websocket.rs | 1 + codex-rs/codex-api/src/lib.rs | 2 + codex-rs/codex-api/src/sse/responses.rs | 38 ++++++++ codex-rs/codex-api/tests/clients.rs | 3 + codex-rs/core/config.schema.json | 6 ++ codex-rs/core/src/client.rs | 16 +++- codex-rs/core/src/client_common_tests.rs | 4 + codex-rs/core/src/client_tests.rs | 3 + codex-rs/core/src/session/session.rs | 3 + codex-rs/core/src/session/tests.rs | 9 ++ codex-rs/core/src/session/turn.rs | 45 +++++++++ codex-rs/core/src/turn_timing.rs | 1 + codex-rs/core/tests/responses_headers.rs | 3 + codex-rs/core/tests/suite/client.rs | 58 ++++++++++++ .../core/tests/suite/client_websockets.rs | 42 ++++++++- codex-rs/core/tests/suite/items.rs | 93 +++++++++++++++++++ codex-rs/features/src/lib.rs | 8 ++ codex-rs/memories/write/src/runtime.rs | 1 + codex-rs/otel/src/events/session_telemetry.rs | 1 + 20 files changed, 352 insertions(+), 6 deletions(-) diff --git a/codex-rs/codex-api/src/common.rs b/codex-rs/codex-api/src/common.rs index 254a5cb38f..e4600e26aa 100644 --- a/codex-rs/codex-api/src/common.rs +++ b/codex-rs/codex-api/src/common.rs @@ -104,6 +104,11 @@ pub enum ResponseEvent { delta: String, summary_index: i64, }, + ReasoningSummaryDone { + item_id: String, + text: String, + summary_index: i64, + }, ReasoningContentDelta { delta: String, content_index: i64, @@ -148,6 +153,17 @@ pub struct Reasoning { pub context: Option, } +#[derive(Debug, Serialize, Clone, PartialEq)] +#[serde(rename_all = "snake_case")] +pub enum ReasoningSummaryDelivery { + SequentialCutoff, +} + +#[derive(Debug, Serialize, Clone, PartialEq)] +pub struct StreamOptions { + pub reasoning_summary_delivery: ReasoningSummaryDelivery, +} + #[derive(Debug, Serialize, Default, Clone, PartialEq)] #[serde(rename_all = "snake_case")] pub enum TextFormatType { @@ -209,6 +225,8 @@ pub struct ResponsesApiRequest { pub reasoning: Option, pub store: bool, pub stream: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub stream_options: Option, pub include: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub service_tier: Option, @@ -233,6 +251,7 @@ impl From<&ResponsesApiRequest> for ResponseCreateWsRequest { reasoning: request.reasoning.clone(), store: request.store, stream: request.stream, + stream_options: request.stream_options.clone(), include: request.include.clone(), service_tier: request.service_tier.clone(), prompt_cache_key: request.prompt_cache_key.clone(), @@ -258,6 +277,8 @@ pub struct ResponseCreateWsRequest { pub reasoning: Option, pub store: bool, pub stream: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub stream_options: Option, pub include: Vec, #[serde(skip_serializing_if = "Option::is_none")] pub service_tier: Option, diff --git a/codex-rs/codex-api/src/endpoint/responses_websocket.rs b/codex-rs/codex-api/src/endpoint/responses_websocket.rs index 447450fae1..cce056f638 100644 --- a/codex-rs/codex-api/src/endpoint/responses_websocket.rs +++ b/codex-rs/codex-api/src/endpoint/responses_websocket.rs @@ -851,6 +851,7 @@ mod tests { reasoning: None, store: false, stream: true, + stream_options: None, include: vec!["reasoning.encrypted_content".to_string()], service_tier: Some("priority".to_string()), prompt_cache_key: Some("cache-key".to_string()), diff --git a/codex-rs/codex-api/src/lib.rs b/codex-rs/codex-api/src/lib.rs index 72de4d0585..1d1a4c8c6e 100644 --- a/codex-rs/codex-api/src/lib.rs +++ b/codex-rs/codex-api/src/lib.rs @@ -34,11 +34,13 @@ pub use crate::common::RawMemory; pub use crate::common::RawMemoryMetadata; pub use crate::common::Reasoning; pub use crate::common::ReasoningContext; +pub use crate::common::ReasoningSummaryDelivery; pub use crate::common::ResponseCreateWsRequest; pub use crate::common::ResponseEvent; pub use crate::common::ResponseStream; pub use crate::common::ResponsesApiRequest; pub use crate::common::ResponsesWsRequest; +pub use crate::common::StreamOptions; pub use crate::common::TextControls; pub use crate::common::WS_REQUEST_HEADER_TRACEPARENT_CLIENT_METADATA_KEY; pub use crate::common::WS_REQUEST_HEADER_TRACESTATE_CLIENT_METADATA_KEY; diff --git a/codex-rs/codex-api/src/sse/responses.rs b/codex-rs/codex-api/src/sse/responses.rs index 17d31a02ee..265d3ced5c 100644 --- a/codex-rs/codex-api/src/sse/responses.rs +++ b/codex-rs/codex-api/src/sse/responses.rs @@ -167,6 +167,7 @@ pub struct ResponsesStreamEvent { item_id: Option, call_id: Option, delta: Option, + text: Option, summary_index: Option, content_index: Option, safety_buffering: Option, @@ -358,6 +359,17 @@ pub fn process_responses_event( })); } } + "response.reasoning_summary_text.done" => { + if let (Some(item_id), Some(text), Some(summary_index)) = + (event.item_id, event.text, event.summary_index) + { + return Ok(Some(ResponseEvent::ReasoningSummaryDone { + item_id, + text, + summary_index, + })); + } + } "response.reasoning_text.delta" => { if let (Some(delta), Some(content_index)) = (event.delta, event.content_index) { return Ok(Some(ResponseEvent::ReasoningContentDelta { @@ -796,6 +808,32 @@ mod tests { } } + #[tokio::test] + async fn parses_reasoning_summary_done() { + let events = run_sse(vec![ + json!({ + "type": "response.reasoning_summary_text.done", + "item_id": "reasoning-1", + "summary_index": 0, + "text": "Checking", + }), + json!({ + "type": "response.completed", + "response": { "id": "resp1" }, + }), + ]) + .await; + + assert_matches!( + &events[0], + ResponseEvent::ReasoningSummaryDone { + item_id, + text, + summary_index: 0, + } if item_id == "reasoning-1" && text == "Checking" + ); + } + #[tokio::test] async fn error_when_missing_completed() { let item1 = json!({ diff --git a/codex-rs/codex-api/tests/clients.rs b/codex-rs/codex-api/tests/clients.rs index 60a9852cfa..ab4ab1d08f 100644 --- a/codex-rs/codex-api/tests/clients.rs +++ b/codex-rs/codex-api/tests/clients.rs @@ -321,6 +321,7 @@ async fn responses_client_stream_request_preserves_item_ids() -> Result<()> { reasoning: None, store: false, stream: true, + stream_options: None, include: Vec::new(), service_tier: None, prompt_cache_key: None, @@ -407,6 +408,7 @@ async fn streaming_client_retries_on_transport_error() -> Result<()> { reasoning: None, store: false, stream: true, + stream_options: None, include: Vec::new(), service_tier: None, prompt_cache_key: None, @@ -526,6 +528,7 @@ async fn azure_store_sends_ids_and_headers() -> Result<()> { reasoning: None, store: true, stream: true, + stream_options: None, include: Vec::new(), service_tier: None, prompt_cache_key: None, diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 876180dd7e..0a48882b09 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -469,6 +469,9 @@ "computer_use": { "type": "boolean" }, + "concurrent_reasoning_summaries": { + "type": "boolean" + }, "connectors": { "type": "boolean" }, @@ -4838,6 +4841,9 @@ "computer_use": { "type": "boolean" }, + "concurrent_reasoning_summaries": { + "type": "boolean" + }, "connectors": { "type": "boolean" }, diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 43a758b0e8..716466b189 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -56,6 +56,7 @@ use codex_api::ResponsesWebsocketConnection as ApiWebSocketConnection; use codex_api::ResponsesWsRequest; use codex_api::SharedAuthProvider; use codex_api::SseTelemetry; +use codex_api::StreamOptions; use codex_api::TransportError; use codex_api::WebsocketTelemetry; use codex_api::auth_header_telemetry; @@ -203,6 +204,7 @@ struct ModelClientState { include_timing_metrics: bool, beta_features_header: Option, item_ids_enabled: bool, + concurrent_reasoning_summaries_enabled: bool, include_attestation: bool, attestation_provider: Option>, disable_websockets: AtomicBool, @@ -311,6 +313,7 @@ fn responses_request_properties_match( reasoning: previous_reasoning, store: previous_store, stream: previous_stream, + stream_options: _, include: previous_include, service_tier: previous_service_tier, prompt_cache_key: previous_prompt_cache_key, @@ -327,6 +330,7 @@ fn responses_request_properties_match( reasoning: current_reasoning, store: current_store, stream: current_stream, + stream_options: _, include: current_include, service_tier: current_service_tier, prompt_cache_key: current_prompt_cache_key, @@ -342,6 +346,8 @@ fn responses_request_properties_match( && previous_reasoning == current_reasoning && previous_store == current_store && previous_stream == current_stream + // Stream options control delivery for this response, not the context + // referenced by `previous_response_id`. && previous_include == current_include && previous_service_tier == current_service_tier && previous_prompt_cache_key == current_prompt_cache_key @@ -409,6 +415,7 @@ impl ModelClient { include_timing_metrics: bool, beta_features_header: Option, item_ids_enabled: bool, + concurrent_reasoning_summaries_enabled: bool, attestation_provider: Option>, ) -> Self { let model_provider = create_model_provider(provider_info, auth_manager); @@ -431,6 +438,7 @@ impl ModelClient { include_timing_metrics, beta_features_header, item_ids_enabled, + concurrent_reasoning_summaries_enabled, include_attestation, attestation_provider, disable_websockets: AtomicBool::new(false), @@ -820,7 +828,8 @@ impl ModelClient { responses_metadata: &CodexResponsesMetadata, ) -> Result { let mut input = prompt.get_formatted_input_for_request(model_info.use_responses_lite); - if !self.state.provider.info().is_openai() { + let is_openai = self.state.provider.info().is_openai(); + if !is_openai { input .iter_mut() .for_each(ResponseItem::clear_internal_chat_message_metadata_passthrough); @@ -848,6 +857,10 @@ impl ModelClient { } else { (prompt.base_instructions.text.clone(), Some(tools)) }; + let stream_options = (self.state.concurrent_reasoning_summaries_enabled && is_openai) + .then_some(StreamOptions { + reasoning_summary_delivery: codex_api::ReasoningSummaryDelivery::SequentialCutoff, + }); let reasoning = Self::build_reasoning(model_info, effort, summary); let include = if reasoning.is_some() { vec!["reasoning.encrypted_content".to_string()] @@ -882,6 +895,7 @@ impl ModelClient { reasoning, store: provider.is_azure_responses_endpoint(), stream: true, + stream_options, include, service_tier, prompt_cache_key, diff --git a/codex-rs/core/src/client_common_tests.rs b/codex-rs/core/src/client_common_tests.rs index 38c4aaecb5..b8d0a364a3 100644 --- a/codex-rs/core/src/client_common_tests.rs +++ b/codex-rs/core/src/client_common_tests.rs @@ -116,6 +116,7 @@ fn serializes_text_verbosity_when_set() { reasoning: None, store: false, stream: true, + stream_options: None, include: vec![], prompt_cache_key: None, service_tier: None, @@ -163,6 +164,7 @@ fn serializes_text_schema_with_strict_format() { reasoning: None, store: false, stream: true, + stream_options: None, include: vec![], prompt_cache_key: None, service_tier: None, @@ -224,6 +226,7 @@ fn omits_text_when_not_set() { reasoning: None, store: false, stream: true, + stream_options: None, include: vec![], prompt_cache_key: None, service_tier: None, @@ -247,6 +250,7 @@ fn serializes_flex_service_tier_when_set() { reasoning: None, store: false, stream: true, + stream_options: None, include: vec![], prompt_cache_key: None, service_tier: Some(ServiceTier::Flex.to_string()), diff --git a/codex-rs/core/src/client_tests.rs b/codex-rs/core/src/client_tests.rs index 82c464625d..c05f56b713 100644 --- a/codex-rs/core/src/client_tests.rs +++ b/codex-rs/core/src/client_tests.rs @@ -103,6 +103,7 @@ fn test_model_client_with_thread_id( /*include_timing_metrics*/ false, /*beta_features_header*/ None, /*item_ids_enabled*/ false, + /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, ) } @@ -147,6 +148,7 @@ async fn compact_uses_bearer_after_agent_identity_session_fallback() -> anyhow:: /*include_timing_metrics*/ false, /*beta_features_header*/ None, /*item_ids_enabled*/ false, + /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, ); let prompt = Prompt { @@ -821,6 +823,7 @@ fn model_client_with_counting_attestation( /*include_timing_metrics*/ false, /*beta_features_header*/ None, /*item_ids_enabled*/ false, + /*concurrent_reasoning_summaries_enabled*/ false, Some(Arc::new(CountingAttestationProvider { calls: attestation_calls.clone(), })), diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 2261eaa495..78a7f02588 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1111,6 +1111,9 @@ impl Session { config.features.enabled(Feature::RuntimeMetrics), Self::build_model_client_beta_features_header(config.as_ref()), /*item_ids_enabled*/ config.features.enabled(Feature::ItemIds), + /*concurrent_reasoning_summaries_enabled*/ config + .features + .enabled(Feature::ConcurrentReasoningSummaries), attestation_provider, ) .with_prompt_cache_key_override( diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 6cc223cd7d..1ff469cf4c 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -490,6 +490,7 @@ fn test_model_client_session() -> crate::client::ModelClientSession { /*include_timing_metrics*/ false, /*beta_features_header*/ None, /*item_ids_enabled*/ false, + /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, ) .new_session() @@ -5460,6 +5461,10 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { config.features.enabled(Feature::RuntimeMetrics), Session::build_model_client_beta_features_header(config.as_ref()), /*item_ids_enabled*/ config.features.enabled(Feature::ItemIds), + /*concurrent_reasoning_summaries_enabled*/ + config + .features + .enabled(Feature::ConcurrentReasoningSummaries), /*attestation_provider*/ None, ), code_mode_service: crate::tools::code_mode::CodeModeService::new(Arc::new( @@ -7586,6 +7591,10 @@ where config.features.enabled(Feature::RuntimeMetrics), Session::build_model_client_beta_features_header(config.as_ref()), /*item_ids_enabled*/ config.features.enabled(Feature::ItemIds), + /*concurrent_reasoning_summaries_enabled*/ + config + .features + .enabled(Feature::ConcurrentReasoningSummaries), /*attestation_provider*/ None, ), code_mode_service: crate::tools::code_mode::CodeModeService::new(Arc::new( diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index f16b525002..708f42f0ff 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -1909,6 +1909,11 @@ async fn try_run_sampling_request( turn_context.provider.info().name.as_str(), ); let sampling_timing_guard = turn_context.turn_timing_state.begin_sampling(); + let uses_sequential_cutoff_reasoning_summaries = turn_context + .config + .features + .enabled(Feature::ConcurrentReasoningSummaries) + && turn_context.provider.info().is_openai(); let mut stream = client_session .stream( prompt, @@ -2291,6 +2296,9 @@ async fn try_run_sampling_request( delta, summary_index, } => { + if uses_sequential_cutoff_reasoning_summaries { + continue; + } if let Some(active) = active_item.as_ref() { if !active_item_is_streaming_to_client { continue; @@ -2309,6 +2317,9 @@ async fn try_run_sampling_request( } } ResponseEvent::ReasoningSummaryPartAdded { summary_index } => { + if uses_sequential_cutoff_reasoning_summaries { + continue; + } if let Some(active) = active_item.as_ref() { if !active_item_is_streaming_to_client { continue; @@ -2323,6 +2334,40 @@ async fn try_run_sampling_request( error_or_panic("ReasoningSummaryPartAdded without active item".to_string()); } } + ResponseEvent::ReasoningSummaryDone { + item_id, + text, + summary_index, + } => { + if !uses_sequential_cutoff_reasoning_summaries { + continue; + } + let Some(active) = active_item.as_ref() else { + continue; + }; + if !active_item_is_streaming_to_client || active.id() != item_id { + continue; + } + if summary_index > 0 { + sess.send_event( + &turn_context, + EventMsg::AgentReasoningSectionBreak(AgentReasoningSectionBreakEvent { + item_id: item_id.clone(), + summary_index, + }), + ) + .await; + } + let event = ReasoningContentDeltaEvent { + thread_id: sess.thread_id.to_string(), + turn_id: turn_context.sub_id.clone(), + item_id, + delta: text, + summary_index, + }; + sess.send_event(&turn_context, EventMsg::ReasoningContentDelta(event)) + .await; + } ResponseEvent::ReasoningContentDelta { delta, content_index, diff --git a/codex-rs/core/src/turn_timing.rs b/codex-rs/core/src/turn_timing.rs index 04f7aeb69b..d0f4f46cc3 100644 --- a/codex-rs/core/src/turn_timing.rs +++ b/codex-rs/core/src/turn_timing.rs @@ -334,6 +334,7 @@ fn response_event_records_turn_ttft(event: &ResponseEvent) -> bool { } ResponseEvent::OutputTextDelta(_) | ResponseEvent::ReasoningSummaryDelta { .. } + | ResponseEvent::ReasoningSummaryDone { .. } | ResponseEvent::ReasoningContentDelta { .. } => true, ResponseEvent::Created | ResponseEvent::ServerModel(_) diff --git a/codex-rs/core/tests/responses_headers.rs b/codex-rs/core/tests/responses_headers.rs index 5b7c2c65b8..4c5f906160 100644 --- a/codex-rs/core/tests/responses_headers.rs +++ b/codex-rs/core/tests/responses_headers.rs @@ -131,6 +131,7 @@ async fn responses_stream_includes_subagent_header_on_review() { /*include_timing_metrics*/ false, /*beta_features_header*/ None, /*item_ids_enabled*/ false, + /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, ); let responses_metadata = test_turn_responses_metadata(&client, thread_id, &session_source); @@ -265,6 +266,7 @@ async fn responses_stream_includes_subagent_header_on_other() { /*include_timing_metrics*/ false, /*beta_features_header*/ None, /*item_ids_enabled*/ false, + /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, ); let responses_metadata = test_turn_responses_metadata(&client, thread_id, &session_source); @@ -385,6 +387,7 @@ async fn responses_respects_model_info_overrides_from_config() { /*include_timing_metrics*/ false, /*beta_features_header*/ None, /*item_ids_enabled*/ false, + /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, ); let responses_metadata = test_turn_responses_metadata(&client, thread_id, &session_source); diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index c87539f940..92624bfcb3 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1334,6 +1334,10 @@ async fn send_provider_auth_request(server: &MockServer, auth: ModelProviderAuth /*include_timing_metrics*/ false, /*beta_features_header*/ None, /*item_ids_enabled*/ config.features.enabled(Feature::ItemIds), + /*concurrent_reasoning_summaries_enabled*/ + config + .features + .enabled(Feature::ConcurrentReasoningSummaries), /*attestation_provider*/ None, ); let responses_metadata = test_turn_responses_metadata(&client, thread_id); @@ -2376,6 +2380,9 @@ async fn configured_reasoning_summary_is_sent() -> anyhow::Result<()> { let TestCodex { codex, .. } = test_codex() .with_config(|config| { config.model_reasoning_summary = Some(ReasoningSummary::Concise); + let _ = config + .features + .enable(Feature::ConcurrentReasoningSummaries); }) .build(&server) .await?; @@ -2412,6 +2419,55 @@ async fn configured_reasoning_summary_is_sent() -> anyhow::Result<()> { .and_then(|reasoning| reasoning.get("context")), None ); + pretty_assertions::assert_eq!( + request_body + .get("stream_options") + .and_then(|stream_options| stream_options.get("reasoning_summary_delivery")) + .and_then(|value| value.as_str()), + Some("sequential_cutoff") + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn sequential_cutoff_is_omitted_for_non_openai_provider() -> anyhow::Result<()> { + skip_if_no_network!(Ok(())); + let server = MockServer::start().await; + + let resp_mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp1"), ev_completed("resp1")]), + ) + .await; + let TestCodex { codex, .. } = test_codex() + .with_config(|config| { + config.model_provider.name = "mock".to_string(); + let _ = config + .features + .enable(Feature::ConcurrentReasoningSummaries); + }) + .build(&server) + .await?; + + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "hello".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: Default::default(), + }) + .await + .unwrap(); + + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + let request_body = resp_mock.single_request().body_json(); + pretty_assertions::assert_eq!(request_body.get("stream_options"), None); Ok(()) } @@ -2583,6 +2639,7 @@ async fn reasoning_summary_is_omitted_when_disabled() -> anyhow::Result<()> { .and_then(|reasoning| reasoning.get("summary")), None ); + pretty_assertions::assert_eq!(request_body.get("stream_options"), None); Ok(()) } @@ -2948,6 +3005,7 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() { /*include_timing_metrics*/ false, /*beta_features_header*/ None, /*item_ids_enabled*/ false, + /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, ); let responses_metadata = test_turn_responses_metadata(&client, thread_id); diff --git a/codex-rs/core/tests/suite/client_websockets.rs b/codex-rs/core/tests/suite/client_websockets.rs index d5218cdcab..c56a8680ab 100755 --- a/codex-rs/core/tests/suite/client_websockets.rs +++ b/codex-rs/core/tests/suite/client_websockets.rs @@ -386,7 +386,13 @@ async fn responses_websocket_request_prewarm_reuses_connection() { ]]) .await; - let harness = websocket_harness_with_options(&server, /*runtime_metrics_enabled*/ true).await; + let mut provider = websocket_provider(&server); + provider.name = ModelProviderInfo::create_openai_provider(/*base_url*/ None).name; + let harness = websocket_harness_with_provider_options( + provider, /*runtime_metrics_enabled*/ true, + /*concurrent_reasoning_summaries_enabled*/ true, + ) + .await; let mut client_session = harness.client.new_session(); let prompt = prompt_with_input(vec![message_item("hello")]); let responses_metadata = prewarm_metadata(&harness, /*turn_id*/ None); @@ -422,6 +428,10 @@ async fn responses_websocket_request_prewarm_reuses_connection() { assert_eq!(warmup["type"].as_str(), Some("response.create")); assert_eq!(warmup["generate"].as_bool(), Some(false)); + assert_eq!( + warmup["stream_options"]["reasoning_summary_delivery"].as_str(), + Some("sequential_cutoff") + ); assert_eq!(warmup["tools"], serde_json::json!([])); let warmup_turn_metadata: serde_json::Value = serde_json::from_str( warmup["client_metadata"]["x-codex-turn-metadata"] @@ -436,6 +446,10 @@ async fn responses_websocket_request_prewarm_reuses_connection() { assert_eq!(follow_up["type"].as_str(), Some("response.create")); assert_eq!(follow_up["previous_response_id"].as_str(), Some("warm-1")); assert_eq!(follow_up["input"], serde_json::json!([])); + assert_eq!( + follow_up["stream_options"]["reasoning_summary_delivery"].as_str(), + Some("sequential_cutoff") + ); server.shutdown().await; } @@ -990,8 +1004,11 @@ async fn responses_websocket_v2_incremental_requests_are_reused_across_turns() { // use OpenAI provider to check metadata logic let mut provider = websocket_provider(&server); provider.name = ModelProviderInfo::create_openai_provider(/*base_url*/ None).name; - let harness = - websocket_harness_with_provider_options(provider, /*runtime_metrics_enabled*/ false).await; + let harness = websocket_harness_with_provider_options( + provider, /*runtime_metrics_enabled*/ false, + /*concurrent_reasoning_summaries_enabled*/ false, + ) + .await; // Turn one: initiate let prompt_one = prompt_with_input(vec![message_item("hello")]); @@ -2195,13 +2212,18 @@ async fn websocket_harness_with_options( server: &WebSocketTestServer, runtime_metrics_enabled: bool, ) -> WebsocketTestHarness { - websocket_harness_with_provider_options(websocket_provider(server), runtime_metrics_enabled) - .await + websocket_harness_with_provider_options( + websocket_provider(server), + runtime_metrics_enabled, + /*concurrent_reasoning_summaries_enabled*/ false, + ) + .await } async fn websocket_harness_with_provider_options( provider: ModelProviderInfo, runtime_metrics_enabled: bool, + concurrent_reasoning_summaries_enabled: bool, ) -> WebsocketTestHarness { let codex_home = TempDir::new().unwrap(); let mut config = load_default_config_for_test(&codex_home).await; @@ -2212,6 +2234,12 @@ async fn websocket_harness_with_provider_options( .enable(Feature::RuntimeMetrics) .expect("test config should allow feature update"); } + if concurrent_reasoning_summaries_enabled { + config + .features + .enable(Feature::ConcurrentReasoningSummaries) + .expect("test config should allow feature update"); + } let config = Arc::new(config); let model_info = codex_core::test_support::construct_model_info_offline(MODEL, &config); let thread_id = ThreadId::new(); @@ -2251,6 +2279,10 @@ async fn websocket_harness_with_provider_options( runtime_metrics_enabled, /*beta_features_header*/ None, /*item_ids_enabled*/ config.features.enabled(Feature::ItemIds), + /*concurrent_reasoning_summaries_enabled*/ + config + .features + .enabled(Feature::ConcurrentReasoningSummaries), /*attestation_provider*/ None, ); diff --git a/codex-rs/core/tests/suite/items.rs b/codex-rs/core/tests/suite/items.rs index 515d496b8b..196277394a 100644 --- a/codex-rs/core/tests/suite/items.rs +++ b/codex-rs/core/tests/suite/items.rs @@ -1,6 +1,7 @@ #![cfg(not(target_os = "windows"))] use anyhow::Ok; +use codex_features::Feature; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; @@ -1134,6 +1135,98 @@ async fn reasoning_content_delta_has_item_metadata() -> anyhow::Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn sequential_cutoff_renders_done_summaries_for_active_reasoning_item() -> anyhow::Result<()> +{ + skip_if_no_network!(Ok(())); + + let server = start_mock_server().await; + + let TestCodex { codex, .. } = test_codex() + .with_config(|config| { + config + .features + .enable(Feature::ConcurrentReasoningSummaries) + .expect("test config should allow feature update"); + }) + .build(&server) + .await?; + let summary_done = |item_id: &str, summary_index: i64, text: &str| { + serde_json::json!({ + "type": "response.reasoning_summary_text.done", + "item_id": item_id, + "summary_index": summary_index, + "text": text, + }) + }; + mount_sse_once( + &server, + sse(vec![ + ev_response_created("resp-1"), + ev_reasoning_item_added("reasoning-1", &[""]), + // Sequential cutoff renders atomic done events, not partial legacy events. + ev_reasoning_summary_text_delta("partial"), + summary_done("reasoning-1", 0, "step one"), + summary_done("reasoning-1", 1, "step two"), + ev_reasoning_item("reasoning-1", &["step one", "step two"], &[]), + ev_message_item_added("message-1", ""), + // Drop a stale completion rather than attaching it to the active message. + summary_done("reasoning-1", 2, "late step"), + ev_output_text_delta("Done"), + ev_assistant_message("message-1", "Done"), + ev_completed("resp-1"), + ]), + ) + .await; + + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "reason through it".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: Default::default(), + }) + .await?; + + let reasoning_item = wait_for_event_match(&codex, |ev| match ev { + EventMsg::ItemStarted(ItemStartedEvent { + item: TurnItem::Reasoning(item), + .. + }) => Some(item.clone()), + _ => None, + }) + .await; + + let mut summary_deltas = Vec::new(); + let mut summary_sections = Vec::new(); + loop { + match wait_for_event(&codex, |_| true).await { + EventMsg::ReasoningContentDelta(event) => { + summary_deltas.push((event.item_id, event.delta, event.summary_index)); + } + EventMsg::AgentReasoningSectionBreak(event) => { + summary_sections.push((event.item_id, event.summary_index)); + } + EventMsg::TurnComplete(_) => break, + _ => {} + } + } + + assert_eq!( + summary_deltas, + vec![ + (reasoning_item.id.clone(), "step one".to_string(), 0), + (reasoning_item.id, "step two".to_string(), 1), + ] + ); + assert_eq!(summary_sections, vec![("reasoning-1".to_string(), 1)]); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn reasoning_raw_content_delta_respects_flag() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index 7ab7df18d1..ce5ca459b4 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -204,6 +204,8 @@ pub enum Feature { ResizeAllImages, /// Generate Responses API item IDs for client-created history items. ItemIds, + /// Request sequential cutoff reasoning summary delivery. + ConcurrentReasoningSummaries, /// Allow prompting and installing missing MCP dependencies. SkillMcpDependencyInstall, /// Removed compatibility flag for deleted skill env var dependency prompting. @@ -1184,6 +1186,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::UnderDevelopment, default_enabled: false, }, + FeatureSpec { + id: Feature::ConcurrentReasoningSummaries, + key: "concurrent_reasoning_summaries", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::SkillMcpDependencyInstall, key: "skill_mcp_dependency_install", diff --git a/codex-rs/memories/write/src/runtime.rs b/codex-rs/memories/write/src/runtime.rs index 6c7ad3978f..ae0903cfdc 100644 --- a/codex-rs/memories/write/src/runtime.rs +++ b/codex-rs/memories/write/src/runtime.rs @@ -261,6 +261,7 @@ impl MemoryStartupContext { config.features.enabled(Feature::RuntimeMetrics), /*beta_features_header*/ None, config.features.enabled(Feature::ItemIds), + /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, ); diff --git a/codex-rs/otel/src/events/session_telemetry.rs b/codex-rs/otel/src/events/session_telemetry.rs index 440f8f04a5..c470f8dc7c 100644 --- a/codex-rs/otel/src/events/session_telemetry.rs +++ b/codex-rs/otel/src/events/session_telemetry.rs @@ -1195,6 +1195,7 @@ impl SessionTelemetry { ResponseEvent::OutputTextDelta(_) => "text_delta".into(), ResponseEvent::ToolCallInputDelta { .. } => "tool_input_delta".into(), ResponseEvent::ReasoningSummaryDelta { .. } => "reasoning_summary_delta".into(), + ResponseEvent::ReasoningSummaryDone { .. } => "reasoning_summary_done".into(), ResponseEvent::ReasoningContentDelta { .. } => "reasoning_content_delta".into(), ResponseEvent::ReasoningSummaryPartAdded { .. } => { "reasoning_summary_part_added".into() From 9365b08467b79a116ff78f00931306d3dfca004f Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 6 Jul 2026 20:25:33 -0700 Subject: [PATCH 4/7] exec-server: use virtual time in Noise relay test (#31344) ## Why `fragmented_writes_yield_to_keepalive_and_queued_pong` deliberately blocks WebSocket writes while exercising keepalive and queued-Pong scheduling. It previously advanced those states with wall-clock sleeps. Under a sufficiently delayed CI worker, those sleeps and scheduling gaps could consume the test-only 100 ms Pong-watchdog budget, causing the relay to exit and the next write-permit send to fail with `TrySendError::Disconnected`. The failure was therefore a timing flake in the harness test, not evidence that the production relay mishandled a Pong. ## What changed - Run this test with Tokio time paused. - Advance the virtual clock through its two keepalive transitions instead of sleeping in wall-clock time. - Enable Tokio's `test-util` feature only for `codex-exec-server` dev dependencies. No production code or timeout values change. ## Review guide The behavioral change is confined to `noise_relay/harness_tests.rs`; the `Cargo.toml` change only exposes Tokio's paused-clock test APIs. ## Validation - `just test -p codex-exec-server fragmented_writes_yield_to_keepalive_and_queued_pong` - `just fix -p codex-exec-server` - `just bazel-lock-update` (no lockfile changes) --- codex-rs/exec-server/Cargo.toml | 1 + codex-rs/exec-server/src/noise_relay/harness_tests.rs | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index ba9058f0c6..f303ac70ee 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -74,6 +74,7 @@ rustls = { workspace = true } serial_test = { workspace = true } tempfile = { workspace = true } test-case = "3.3.1" +tokio = { workspace = true, features = ["test-util"] } tracing-opentelemetry = { workspace = true } tracing-subscriber = { workspace = true } wiremock = { workspace = true } diff --git a/codex-rs/exec-server/src/noise_relay/harness_tests.rs b/codex-rs/exec-server/src/noise_relay/harness_tests.rs index ce26a8ca8c..348351e61c 100644 --- a/codex-rs/exec-server/src/noise_relay/harness_tests.rs +++ b/codex-rs/exec-server/src/noise_relay/harness_tests.rs @@ -31,7 +31,7 @@ use crate::noise_channel::PendingResponderHandshake; const ENVIRONMENT_ID: &str = "environment-1"; const EXECUTOR_REGISTRATION_ID: &str = "registration-1"; -#[tokio::test] +#[tokio::test(start_paused = true)] async fn fragmented_writes_yield_to_keepalive_and_queued_pong() -> Result<()> { let (connection, mut control, mut outbound_rx) = connected_controlled_harness().await?; @@ -48,7 +48,7 @@ async fn fragmented_writes_yield_to_keepalive_and_queued_pong() -> Result<()> { .await?; control.wait_for_blocked_write(/*expected*/ 1).await?; - tokio::time::sleep(WEBSOCKET_KEEPALIVE_INTERVAL + Duration::from_millis(10)).await; + tokio::time::advance(WEBSOCKET_KEEPALIVE_INTERVAL + Duration::from_millis(10)).await; control.grant_writes(/*count*/ 1); let first_data = read_outbound_data(&mut outbound_rx).await?; assert_eq!(first_data.seq, 0); @@ -64,7 +64,7 @@ async fn fragmented_writes_yield_to_keepalive_and_queued_pong() -> Result<()> { control.wait_for_blocked_write(/*expected*/ 3).await?; control.send_inbound(Message::Pong(ping_payload))?; - tokio::time::sleep(WEBSOCKET_KEEPALIVE_INTERVAL + Duration::from_millis(10)).await; + tokio::time::advance(WEBSOCKET_KEEPALIVE_INTERVAL + Duration::from_millis(10)).await; control.grant_writes(/*count*/ 1); let second_data = read_outbound_data(&mut outbound_rx).await?; assert_eq!(second_data.seq, 1); From b9b934e99b57aa1908a7cd83adee9faf6d0b8f7d Mon Sep 17 00:00:00 2001 From: Owen Lin Date: Mon, 6 Jul 2026 20:41:57 -0700 Subject: [PATCH 5/7] refactor(protocol): map canonical tool items to legacy events (#31296) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This PR adds legacy `EventMsg` mappings for the `TurnItem` types introduced in [#30282](https://github.com/openai/codex/pull/30282): - `CommandExecution` - `DynamicToolCall` - `CollabAgentToolCall` - `SubAgentActivity` When their producers move to canonical `ItemStarted` / `ItemCompleted`, raw core event consumers can still receive the existing begin/end-style events. The canonical item lifecycle remains the live source of truth. We also record the mapped legacy events in rollout trace so the producer migration preserves the existing tool-runtime trace entries. ## Why This is the compatibility layer for the follow-up producer migrations. Splitting it out first keeps each producer PR small and keeps the legacy mapping in one place. ## What changed - Added `TurnItem` → legacy `EventMsg` mappings in `protocol/src/legacy_events.rs`. - Added the command execution status conversion used by the exec mapping. - Added focused coverage for command execution and dynamic tool mappings. --- codex-rs/core/src/session/mod.rs | 3 + codex-rs/protocol/src/items.rs | 11 + codex-rs/protocol/src/legacy_events.rs | 304 +++++++++++++++++++++++++ codex-rs/protocol/src/protocol.rs | 137 +++++++++++ 4 files changed, 455 insertions(+) diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 91a21ef731..fa240b890b 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -1794,6 +1794,9 @@ impl Session { let show_raw_agent_reasoning = self.show_raw_agent_reasoning(); for legacy in legacy_source.as_legacy_events(show_raw_agent_reasoning) { + self.services + .rollout_thread_trace + .record_tool_call_event(turn_context.sub_id.clone(), &legacy); let legacy_event = Event { id: turn_context.sub_id.clone(), msg: legacy, diff --git a/codex-rs/protocol/src/items.rs b/codex-rs/protocol/src/items.rs index 820c3b774b..2c9d073f0d 100644 --- a/codex-rs/protocol/src/items.rs +++ b/codex-rs/protocol/src/items.rs @@ -13,6 +13,7 @@ use crate::parse_command::ParsedCommand; use crate::protocol::AgentStatus; use crate::protocol::CollabAgentRef; use crate::protocol::ExecCommandSource; +use crate::protocol::ExecCommandStatus; use crate::protocol::FileChange; use crate::protocol::PatchApplyStatus; use crate::protocol::SubAgentActivityKind; @@ -137,6 +138,16 @@ pub enum CommandExecutionStatus { Declined, } +impl From for CommandExecutionStatus { + fn from(value: ExecCommandStatus) -> Self { + match value { + ExecCommandStatus::Completed => Self::Completed, + ExecCommandStatus::Failed => Self::Failed, + ExecCommandStatus::Declined => Self::Declined, + } + } +} + #[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema, PartialEq)] pub struct CommandExecutionItem { pub id: String, diff --git a/codex-rs/protocol/src/legacy_events.rs b/codex-rs/protocol/src/legacy_events.rs index f4a511d367..c12a2747a2 100644 --- a/codex-rs/protocol/src/legacy_events.rs +++ b/codex-rs/protocol/src/legacy_events.rs @@ -1,10 +1,20 @@ +use crate::ThreadId; +use crate::dynamic_tools::DynamicToolCallRequest; use crate::items::AgentMessageContent; use crate::items::AgentMessageItem; +use crate::items::CollabAgentTool; +use crate::items::CollabAgentToolCallItem; +use crate::items::CollabAgentToolCallStatus; +use crate::items::CommandExecutionItem; +use crate::items::CommandExecutionStatus; use crate::items::ContextCompactionItem; +use crate::items::DynamicToolCallItem; +use crate::items::DynamicToolCallStatus; use crate::items::FileChangeItem; use crate::items::ImageGenerationItem; use crate::items::McpToolCallItem; use crate::items::ReasoningItem; +use crate::items::SubAgentActivityItem; use crate::items::TurnItem; use crate::items::UserMessageItem; use crate::items::WebSearchItem; @@ -12,8 +22,24 @@ use crate::protocol::AgentMessageContentDeltaEvent; use crate::protocol::AgentMessageEvent; use crate::protocol::AgentReasoningEvent; use crate::protocol::AgentReasoningRawContentEvent; +use crate::protocol::AgentStatus; +use crate::protocol::CollabAgentInteractionBeginEvent; +use crate::protocol::CollabAgentInteractionEndEvent; +use crate::protocol::CollabAgentSpawnBeginEvent; +use crate::protocol::CollabAgentSpawnEndEvent; +use crate::protocol::CollabAgentStatusEntry; +use crate::protocol::CollabCloseBeginEvent; +use crate::protocol::CollabCloseEndEvent; +use crate::protocol::CollabResumeBeginEvent; +use crate::protocol::CollabResumeEndEvent; +use crate::protocol::CollabWaitingBeginEvent; +use crate::protocol::CollabWaitingEndEvent; use crate::protocol::ContextCompactedEvent; +use crate::protocol::DynamicToolCallResponseEvent; use crate::protocol::EventMsg; +use crate::protocol::ExecCommandBeginEvent; +use crate::protocol::ExecCommandEndEvent; +use crate::protocol::ExecCommandStatus; use crate::protocol::ImageGenerationBeginEvent; use crate::protocol::ImageGenerationEndEvent; use crate::protocol::ItemCompletedEvent; @@ -26,6 +52,7 @@ use crate::protocol::PatchApplyEndEvent; use crate::protocol::PatchApplyStatus; use crate::protocol::ReasoningContentDeltaEvent; use crate::protocol::ReasoningRawContentDeltaEvent; +use crate::protocol::SubAgentActivityEvent; use crate::protocol::UserMessageEvent; use crate::protocol::ViewImageToolCallEvent; use crate::protocol::WebSearchBeginEvent; @@ -97,6 +124,258 @@ impl ReasoningItem { } } +impl CommandExecutionItem { + pub(crate) fn as_legacy_begin_event(&self, turn_id: String, started_at_ms: i64) -> EventMsg { + EventMsg::ExecCommandBegin(ExecCommandBeginEvent { + call_id: self.id.clone(), + process_id: self.process_id.clone(), + turn_id, + started_at_ms, + command: self.command.clone(), + cwd: self.cwd.clone(), + parsed_cmd: self.parsed_cmd.clone(), + source: self.source, + interaction_input: self.interaction_input.clone(), + }) + } + + pub(crate) fn as_legacy_end_event( + &self, + turn_id: String, + completed_at_ms: i64, + ) -> Option { + let status = match self.status { + CommandExecutionStatus::InProgress => return None, + CommandExecutionStatus::Completed => ExecCommandStatus::Completed, + CommandExecutionStatus::Failed => ExecCommandStatus::Failed, + CommandExecutionStatus::Declined => ExecCommandStatus::Declined, + }; + Some(EventMsg::ExecCommandEnd(ExecCommandEndEvent { + call_id: self.id.clone(), + process_id: self.process_id.clone(), + turn_id, + completed_at_ms, + command: self.command.clone(), + cwd: self.cwd.clone(), + parsed_cmd: self.parsed_cmd.clone(), + source: self.source, + interaction_input: self.interaction_input.clone(), + stdout: self.stdout.clone().unwrap_or_default(), + stderr: self.stderr.clone().unwrap_or_default(), + aggregated_output: self.aggregated_output.clone().unwrap_or_default(), + exit_code: self.exit_code.unwrap_or_default(), + duration: self.duration.unwrap_or_default(), + formatted_output: self.formatted_output.clone().unwrap_or_default(), + status, + })) + } +} + +impl DynamicToolCallItem { + pub(crate) fn as_legacy_request_event(&self, turn_id: String, started_at_ms: i64) -> EventMsg { + EventMsg::DynamicToolCallRequest(DynamicToolCallRequest { + call_id: self.id.clone(), + turn_id, + started_at_ms, + namespace: self.namespace.clone(), + tool: self.tool.clone(), + arguments: self.arguments.clone(), + }) + } + + pub(crate) fn as_legacy_response_event( + &self, + turn_id: String, + completed_at_ms: i64, + ) -> Option { + if matches!(self.status, DynamicToolCallStatus::InProgress) { + return None; + } + Some(EventMsg::DynamicToolCallResponse( + DynamicToolCallResponseEvent { + call_id: self.id.clone(), + turn_id, + completed_at_ms, + namespace: self.namespace.clone(), + tool: self.tool.clone(), + arguments: self.arguments.clone(), + content_items: self.content_items.clone().unwrap_or_default(), + success: self.success.unwrap_or(false), + error: self.error.clone(), + duration: self.duration.unwrap_or_default(), + }, + )) + } +} + +impl CollabAgentToolCallItem { + pub(crate) fn as_legacy_begin_event(&self, started_at_ms: i64) -> Option { + let receiver_thread_id = self.receiver_thread_ids.first().copied(); + match self.tool { + CollabAgentTool::SpawnAgent => Some(EventMsg::CollabAgentSpawnBegin( + CollabAgentSpawnBeginEvent { + call_id: self.id.clone(), + started_at_ms, + sender_thread_id: self.sender_thread_id, + prompt: self.prompt.clone().unwrap_or_default(), + model: self.model.clone().unwrap_or_default(), + reasoning_effort: self.reasoning_effort.clone().unwrap_or_default(), + }, + )), + CollabAgentTool::SendInput => receiver_thread_id.map(|receiver_thread_id| { + EventMsg::CollabAgentInteractionBegin(CollabAgentInteractionBeginEvent { + call_id: self.id.clone(), + started_at_ms, + sender_thread_id: self.sender_thread_id, + receiver_thread_id, + prompt: self.prompt.clone().unwrap_or_default(), + }) + }), + CollabAgentTool::ResumeAgent => receiver_thread_id.map(|receiver_thread_id| { + let (receiver_agent_nickname, receiver_agent_role) = + self.receiver_agent_identity(receiver_thread_id); + EventMsg::CollabResumeBegin(CollabResumeBeginEvent { + call_id: self.id.clone(), + started_at_ms, + sender_thread_id: self.sender_thread_id, + receiver_thread_id, + receiver_agent_nickname, + receiver_agent_role, + }) + }), + CollabAgentTool::Wait => Some(EventMsg::CollabWaitingBegin(CollabWaitingBeginEvent { + started_at_ms, + sender_thread_id: self.sender_thread_id, + receiver_thread_ids: self.receiver_thread_ids.clone(), + receiver_agents: self.receiver_agents.clone(), + call_id: self.id.clone(), + })), + CollabAgentTool::CloseAgent => receiver_thread_id.map(|receiver_thread_id| { + EventMsg::CollabCloseBegin(CollabCloseBeginEvent { + call_id: self.id.clone(), + started_at_ms, + sender_thread_id: self.sender_thread_id, + receiver_thread_id, + }) + }), + } + } + + pub(crate) fn as_legacy_end_event(&self, completed_at_ms: i64) -> Option { + if matches!(self.status, CollabAgentToolCallStatus::InProgress) { + return None; + } + let receiver_thread_id = self.receiver_thread_ids.first().copied(); + match self.tool { + CollabAgentTool::SpawnAgent => { + let (new_agent_nickname, new_agent_role) = receiver_thread_id + .map(|thread_id| self.receiver_agent_identity(thread_id)) + .unwrap_or_default(); + Some(EventMsg::CollabAgentSpawnEnd(CollabAgentSpawnEndEvent { + call_id: self.id.clone(), + completed_at_ms, + sender_thread_id: self.sender_thread_id, + new_thread_id: receiver_thread_id, + new_agent_nickname, + new_agent_role, + prompt: self.prompt.clone().unwrap_or_default(), + model: self.model.clone().unwrap_or_default(), + reasoning_effort: self.reasoning_effort.clone().unwrap_or_default(), + status: receiver_thread_id + .map(|thread_id| self.agent_status(thread_id)) + .unwrap_or(AgentStatus::NotFound), + })) + } + CollabAgentTool::SendInput => receiver_thread_id.map(|receiver_thread_id| { + let (receiver_agent_nickname, receiver_agent_role) = + self.receiver_agent_identity(receiver_thread_id); + EventMsg::CollabAgentInteractionEnd(CollabAgentInteractionEndEvent { + call_id: self.id.clone(), + completed_at_ms, + sender_thread_id: self.sender_thread_id, + receiver_thread_id, + receiver_agent_nickname, + receiver_agent_role, + prompt: self.prompt.clone().unwrap_or_default(), + status: self.agent_status(receiver_thread_id), + }) + }), + CollabAgentTool::ResumeAgent => receiver_thread_id.map(|receiver_thread_id| { + let (receiver_agent_nickname, receiver_agent_role) = + self.receiver_agent_identity(receiver_thread_id); + EventMsg::CollabResumeEnd(CollabResumeEndEvent { + call_id: self.id.clone(), + completed_at_ms, + sender_thread_id: self.sender_thread_id, + receiver_thread_id, + receiver_agent_nickname, + receiver_agent_role, + status: self.agent_status(receiver_thread_id), + }) + }), + CollabAgentTool::Wait => Some(EventMsg::CollabWaitingEnd(CollabWaitingEndEvent { + sender_thread_id: self.sender_thread_id, + call_id: self.id.clone(), + completed_at_ms, + agent_statuses: self + .receiver_agents + .iter() + .map(|agent| CollabAgentStatusEntry { + thread_id: agent.thread_id, + agent_nickname: agent.agent_nickname.clone(), + agent_role: agent.agent_role.clone(), + status: self.agent_status(agent.thread_id), + }) + .collect(), + statuses: self.agents_states.clone(), + })), + CollabAgentTool::CloseAgent => receiver_thread_id.map(|receiver_thread_id| { + let (receiver_agent_nickname, receiver_agent_role) = + self.receiver_agent_identity(receiver_thread_id); + EventMsg::CollabCloseEnd(CollabCloseEndEvent { + call_id: self.id.clone(), + completed_at_ms, + sender_thread_id: self.sender_thread_id, + receiver_thread_id, + receiver_agent_nickname, + receiver_agent_role, + status: self.agent_status(receiver_thread_id), + }) + }), + } + } + + fn receiver_agent_identity(&self, thread_id: ThreadId) -> (Option, Option) { + let receiver_agent = self + .receiver_agents + .iter() + .find(|agent| agent.thread_id == thread_id); + ( + receiver_agent.and_then(|agent| agent.agent_nickname.clone()), + receiver_agent.and_then(|agent| agent.agent_role.clone()), + ) + } + + fn agent_status(&self, thread_id: ThreadId) -> AgentStatus { + self.agents_states + .get(&thread_id) + .cloned() + .unwrap_or(AgentStatus::NotFound) + } +} + +impl SubAgentActivityItem { + pub(crate) fn as_legacy_event(&self, occurred_at_ms: i64) -> EventMsg { + EventMsg::SubAgentActivity(SubAgentActivityEvent { + event_id: self.id.clone(), + occurred_at_ms, + agent_thread_id: self.agent_thread_id, + agent_path: self.agent_path.clone(), + kind: self.kind, + }) + } +} + impl WebSearchItem { pub fn as_legacy_event(&self) -> EventMsg { EventMsg::WebSearchEnd(WebSearchEndEvent { @@ -234,6 +513,16 @@ impl HasLegacyEvent for ItemStartedEvent { } TurnItem::FileChange(item) => vec![item.as_legacy_begin_event(self.turn_id.clone())], TurnItem::McpToolCall(item) => vec![item.as_legacy_begin_event()], + TurnItem::CommandExecution(item) => { + vec![item.as_legacy_begin_event(self.turn_id.clone(), self.started_at_ms)] + } + TurnItem::DynamicToolCall(item) => { + vec![item.as_legacy_request_event(self.turn_id.clone(), self.started_at_ms)] + } + TurnItem::CollabAgentToolCall(item) => item + .as_legacy_begin_event(self.started_at_ms) + .into_iter() + .collect(), _ => Vec::new(), } } @@ -246,6 +535,21 @@ impl HasLegacyEvent for ItemCompletedEvent { .as_legacy_end_event(self.turn_id.clone()) .into_iter() .collect(), + TurnItem::CommandExecution(item) => item + .as_legacy_end_event(self.turn_id.clone(), self.completed_at_ms) + .into_iter() + .collect(), + TurnItem::DynamicToolCall(item) => item + .as_legacy_response_event(self.turn_id.clone(), self.completed_at_ms) + .into_iter() + .collect(), + TurnItem::CollabAgentToolCall(item) => item + .as_legacy_end_event(self.completed_at_ms) + .into_iter() + .collect(), + TurnItem::SubAgentActivity(item) => { + vec![item.as_legacy_event(self.completed_at_ms)] + } _ => self.item.as_legacy_events(show_raw_agent_reasoning), } } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index ca509a5a17..b1b845fe28 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -4309,6 +4309,10 @@ pub struct CollabResumeEndEvent { #[cfg(test)] mod tests { use super::*; + use crate::items::CommandExecutionItem; + use crate::items::CommandExecutionStatus; + use crate::items::DynamicToolCallItem; + use crate::items::DynamicToolCallStatus; use crate::items::FileChangeItem; use crate::items::ImageGenerationItem; use crate::items::McpToolCallItem; @@ -5290,6 +5294,139 @@ mod tests { } } + #[test] + fn command_execution_item_lifecycle_emits_legacy_exec_events() { + let cwd = PathUri::from_abs_path(&test_path_buf("/tmp").abs()); + let started = ItemStartedEvent { + thread_id: ThreadId::new(), + turn_id: "turn-1".into(), + started_at_ms: 10, + item: TurnItem::CommandExecution(CommandExecutionItem { + id: "exec-1".into(), + process_id: Some("pid-1".into()), + command: vec!["echo".into(), "done".into()], + cwd: cwd.clone(), + parsed_cmd: vec![ParsedCommand::Unknown { + cmd: "echo done".into(), + }], + source: ExecCommandSource::Agent, + interaction_input: None, + status: CommandExecutionStatus::InProgress, + stdout: None, + stderr: None, + aggregated_output: None, + exit_code: None, + duration: None, + formatted_output: None, + }), + }; + let completed = ItemCompletedEvent { + thread_id: ThreadId::new(), + turn_id: "turn-1".into(), + completed_at_ms: 20, + item: TurnItem::CommandExecution(CommandExecutionItem { + id: "exec-1".into(), + process_id: Some("pid-1".into()), + command: vec!["echo".into(), "done".into()], + cwd, + parsed_cmd: vec![ParsedCommand::Unknown { + cmd: "echo done".into(), + }], + source: ExecCommandSource::Agent, + interaction_input: None, + status: CommandExecutionStatus::Completed, + stdout: Some("done\n".into()), + stderr: Some(String::new()), + aggregated_output: Some("done\n".into()), + exit_code: Some(0), + duration: Some(Duration::from_millis(5)), + formatted_output: Some("done\n".into()), + }), + }; + + assert!(matches!( + started.as_legacy_events(/*show_raw_agent_reasoning*/ false).as_slice(), + [EventMsg::ExecCommandBegin(ExecCommandBeginEvent { + call_id, + turn_id, + started_at_ms: 10, + .. + })] if call_id == "exec-1" && turn_id == "turn-1" + )); + assert!(matches!( + completed + .as_legacy_events(/*show_raw_agent_reasoning*/ false) + .as_slice(), + [EventMsg::ExecCommandEnd(ExecCommandEndEvent { + call_id, + turn_id, + completed_at_ms: 20, + aggregated_output, + .. + })] if call_id == "exec-1" && turn_id == "turn-1" && aggregated_output == "done\n" + )); + } + + #[test] + fn dynamic_tool_call_item_lifecycle_emits_legacy_dynamic_tool_events() { + let started = ItemStartedEvent { + thread_id: ThreadId::new(), + turn_id: "turn-1".into(), + started_at_ms: 10, + item: TurnItem::DynamicToolCall(DynamicToolCallItem { + id: "dynamic-1".into(), + namespace: Some("apps".into()), + tool: "lookup".into(), + arguments: json!({"id": "123"}), + status: DynamicToolCallStatus::InProgress, + content_items: None, + success: None, + error: None, + duration: None, + }), + }; + let completed = ItemCompletedEvent { + thread_id: ThreadId::new(), + turn_id: "turn-1".into(), + completed_at_ms: 20, + item: TurnItem::DynamicToolCall(DynamicToolCallItem { + id: "dynamic-1".into(), + namespace: Some("apps".into()), + tool: "lookup".into(), + arguments: json!({"id": "123"}), + status: DynamicToolCallStatus::Completed, + content_items: Some(vec![DynamicToolCallOutputContentItem::InputText { + text: "ok".into(), + }]), + success: Some(true), + error: None, + duration: Some(Duration::from_millis(5)), + }), + }; + + assert!(matches!( + started.as_legacy_events(/*show_raw_agent_reasoning*/ false).as_slice(), + [EventMsg::DynamicToolCallRequest(DynamicToolCallRequest { + call_id, + turn_id, + started_at_ms: 10, + .. + })] if call_id == "dynamic-1" && turn_id == "turn-1" + )); + assert!(matches!( + completed + .as_legacy_events(/*show_raw_agent_reasoning*/ false) + .as_slice(), + [EventMsg::DynamicToolCallResponse(DynamicToolCallResponseEvent { + call_id, + turn_id, + completed_at_ms: 20, + success: true, + .. + })] if call_id == "dynamic-1" && turn_id == "turn-1" + )); + } + #[test] fn item_started_event_requires_started_at_ms() { let mut value = serde_json::to_value(ItemStartedEvent { From 6afcf26d5d76c2f88b9096caa758931ffa673745 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 6 Jul 2026 20:49:53 -0700 Subject: [PATCH 6/7] core: route Responses API through system proxy (#31335) ## Why `features.respect_system_proxy` already routes authentication traffic through the OS proxy APIs, but it does not affect the primary inference path. That leaves users behind OS-managed proxies unable to send normal Responses API requests even after login succeeds. This PR is the first product-path migration onto the route-aware transport introduced in #31323 and refined in #31331. It also establishes the construction pattern for later migrations: the effective feature state is resolved once into a required HTTP client factory rather than represented by an optional per-call setting. The scope remains limited to the two HTTP Responses endpoints; WebSockets, model discovery, memories, realtime, and file uploads remain follow-up migrations. ## What changed - Replace the optional proxy marker with an explicit `OutboundProxyPolicy::{ReqwestDefault, RespectSystemProxy}` and a required `HttpClientFactory`. The policy has no default, and the lower-level route-aware reqwest builder is now private. - Have `Config` construct the factory from the effective feature state and require every `ModelClient` constructor to receive it. There is no optional setter or implicit `None` fallback. - Build HTTP clients for `/responses` and `/responses/compact` with `ClientRouteClass::Api`, using the complete destination URL so PAC rules can make URL-specific decisions. - Layer route-aware selection onto Codex's existing default headers, Cloudflare cookie store, custom CA handling, and sandbox no-proxy behavior. - Add an integration test that loads `features.respect_system_proxy` through `config.toml`, creates a real Codex session, and verifies that both a normal Responses turn and remote compaction reach an isolated local proxy. ## Review guide 1. `http-client/src/outbound_proxy.rs` defines the mandatory policy/factory boundary and keeps route resolution private. 2. `core/src/config/mod.rs`, `core/src/session/session.rs`, and `core/src/client.rs` show the compile-time invariant: effective config creates the factory, and `ModelClient` cannot be constructed without one. 3. `login/src/auth/default_client.rs` preserves existing default-client behavior while accepting the required factory for migrated routes. 4. `core/src/client.rs` switches only streaming Responses and remote compaction HTTP transports to the API route class. 5. `core/tests/suite/responses_api_system_proxy.rs` is the behavioral regression boundary. Its Linux subprocess deliberately sets the CGI marker that disables reqwest's implicit environment-proxy handling, so the test fails if session wiring or either Responses call site falls back to the default client. ## Test plan - `cargo check --tests -p codex-http-client -p codex-login -p codex-core` - `just test -p codex-login` - `just test -p codex-core respect_system_proxy_feature_resolves_enabled` - Existing `compact_uses_bearer_after_agent_identity_session_fallback` coverage passes with the new transport construction. - New Linux integration coverage: `responses_and_compact_use_enabled_system_proxy` - `just bazel-lock-check` --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/openai/codex/pull/31335). * #31342 * __->__ #31335 --- codex-rs/Cargo.lock | 1 + codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/client.rs | 32 +++++++++- codex-rs/core/src/client_tests.rs | 5 ++ codex-rs/core/src/config/config_tests.rs | 4 ++ codex-rs/core/src/config/mod.rs | 12 ++++ codex-rs/core/src/session/session.rs | 1 + codex-rs/core/src/session/tests.rs | 5 ++ codex-rs/core/tests/responses_headers.rs | 3 + codex-rs/core/tests/suite/client.rs | 2 + .../core/tests/suite/client_websockets.rs | 1 + codex-rs/http-client/src/lib.rs | 4 +- codex-rs/http-client/src/outbound_proxy.rs | 62 +++++++++++++++---- .../http-client/src/outbound_proxy_tests.rs | 3 +- codex-rs/login/src/auth/default_client.rs | 56 ++++++++++++----- codex-rs/login/src/outbound_proxy.rs | 11 ++-- codex-rs/memories/write/src/runtime.rs | 1 + 17 files changed, 166 insertions(+), 38 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 9d89f8eb1f..0d4d3c33bf 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2647,6 +2647,7 @@ dependencies = [ "codex-git-utils", "codex-home", "codex-hooks", + "codex-http-client", "codex-image-generation-extension", "codex-install-context", "codex-login", diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 4435a2eb63..c617684f78 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -49,6 +49,7 @@ codex-shell-command = { workspace = true } codex-execpolicy = { workspace = true } codex-git-utils = { workspace = true } codex-hooks = { workspace = true } +codex-http-client = { workspace = true } codex-install-context = { workspace = true } codex-network-proxy = { workspace = true } codex-otel = { workspace = true } diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 716466b189..0bf33b3993 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -63,10 +63,13 @@ use codex_api::auth_header_telemetry; use codex_api::build_session_headers; use codex_api::create_text_param_for_request; use codex_api::response_create_client_metadata; +use codex_http_client::ClientRouteClass; +use codex_http_client::HttpClientFactory; use codex_login::AuthManager; use codex_login::CodexAuth; use codex_login::RefreshTokenError; use codex_login::UnauthorizedRecovery; +use codex_login::default_client::build_default_reqwest_client_for_route; use codex_login::default_client::build_reqwest_client; use codex_otel::SessionTelemetry; use codex_otel::current_span_w3c_trace_context; @@ -250,6 +253,7 @@ pub struct ModelClient { state: Arc, agent_identity_policy: AgentIdentityAuthPolicy, prompt_cache_key_override: Option, + http_client_factory: HttpClientFactory, } /// A turn-scoped streaming session created from a [`ModelClient`]. @@ -402,7 +406,9 @@ impl ModelClient { /// Creates a new session-scoped `ModelClient`. /// /// All arguments are expected to be stable for the lifetime of a Codex session. Per-turn values - /// are passed to [`ModelClientSession::stream`] (and other turn-scoped methods) explicitly. + /// are passed to [`ModelClientSession::stream`] (and other turn-scoped methods) explicitly. The + /// HTTP client factory must come from the effective session configuration so every transport + /// observes the resolved outbound proxy policy. pub fn new( auth_manager: Option>, agent_identity_policy: AgentIdentityAuthPolicy, @@ -417,6 +423,7 @@ impl ModelClient { item_ids_enabled: bool, concurrent_reasoning_summaries_enabled: bool, attestation_provider: Option>, + http_client_factory: HttpClientFactory, ) -> Self { let model_provider = create_model_provider(provider_info, auth_manager); let codex_api_key_env_enabled = model_provider @@ -447,6 +454,7 @@ impl ModelClient { }), agent_identity_policy, prompt_cache_key_override: None, + http_client_factory, } } @@ -540,7 +548,8 @@ impl ModelClient { return Ok(Vec::new()); } let client_setup = self.current_client_setup().await?; - let transport = ReqwestTransport::new(build_reqwest_client()); + let transport = + self.build_responses_transport(&client_setup.api_provider, RESPONSES_COMPACT_ENDPOINT)?; let request_telemetry = Self::build_request_telemetry( session_telemetry, AuthRequestTelemetryContext::new( @@ -952,6 +961,21 @@ impl ModelClient { }) } + fn build_responses_transport( + &self, + api_provider: &ApiProvider, + endpoint: &str, + ) -> Result { + let request_url = api_provider.url_for_path(endpoint); + let client = build_default_reqwest_client_for_route( + &self.http_client_factory, + &request_url, + ClientRouteClass::Api, + ) + .map_err(std::io::Error::from)?; + Ok(ReqwestTransport::new(client)) + } + pub(crate) async fn prewarm_auth(&self) -> Result<()> { self.current_client_setup().await.map(|_| ()) } @@ -1386,7 +1410,9 @@ impl ModelClientSession { let mut pending_retry = PendingUnauthorizedRetry::default(); loop { let client_setup = self.client.current_client_setup().await?; - let transport = ReqwestTransport::new(build_reqwest_client()); + let transport = self + .client + .build_responses_transport(&client_setup.api_provider, RESPONSES_ENDPOINT)?; let request_auth_context = AuthRequestTelemetryContext::new( client_setup.auth.as_ref().map(CodexAuth::auth_mode), client_setup.api_auth.as_ref(), diff --git a/codex-rs/core/src/client_tests.rs b/codex-rs/core/src/client_tests.rs index c05f56b713..ddf9c4ebec 100644 --- a/codex-rs/core/src/client_tests.rs +++ b/codex-rs/core/src/client_tests.rs @@ -19,6 +19,8 @@ use codex_api::AgentIdentityTelemetry; use codex_api::ApiError; use codex_api::ResponseEvent; use codex_api::TransportError; +use codex_http_client::HttpClientFactory; +use codex_http_client::OutboundProxyPolicy; use codex_login::AuthCredentialsStoreMode; use codex_login::AuthKeyringBackendKind; use codex_login::AuthManager; @@ -105,6 +107,7 @@ fn test_model_client_with_thread_id( /*item_ids_enabled*/ false, /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), ) } @@ -150,6 +153,7 @@ async fn compact_uses_bearer_after_agent_identity_session_fallback() -> anyhow:: /*item_ids_enabled*/ false, /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), ); let prompt = Prompt { input: vec![ResponseItem::Message { @@ -827,6 +831,7 @@ fn model_client_with_counting_attestation( Some(Arc::new(CountingAttestationProvider { calls: attestation_calls.clone(), })), + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), ); (model_client, attestation_calls) } diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index 6156b9b9ea..40ebc64a0b 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -1590,6 +1590,10 @@ respect_system_proxy = true .await?; assert!(config.respect_system_proxy); + assert_eq!( + config.http_client_factory().outbound_proxy_policy(), + codex_http_client::OutboundProxyPolicy::RespectSystemProxy + ); Ok(()) } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 54fbb97f69..8c0a165564 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -70,6 +70,8 @@ use codex_features::MultiAgentV2ConfigToml; use codex_features::NetworkProxyConfigToml; use codex_features::TokenBudgetConfigToml; use codex_git_utils::resolve_root_git_project_for_trust; +use codex_http_client::HttpClientFactory; +use codex_http_client::OutboundProxyPolicy; use codex_install_context::InstallContext; use codex_login::AuthManagerConfig; use codex_login::AuthRouteConfig; @@ -1481,6 +1483,16 @@ impl Config { .then(AuthRouteConfig::respect_system_proxy) } + /// Creates the HTTP client factory resolved from the effective feature configuration. + pub fn http_client_factory(&self) -> HttpClientFactory { + let outbound_proxy_policy = if self.respect_system_proxy { + OutboundProxyPolicy::RespectSystemProxy + } else { + OutboundProxyPolicy::ReqwestDefault + }; + HttpClientFactory::new(outbound_proxy_policy) + } + /// Build the plugin-manager input from the effective config. pub fn plugins_config_input(&self) -> PluginsConfigInput { PluginsConfigInput::new( diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 78a7f02588..08abb46377 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1115,6 +1115,7 @@ impl Session { .features .enabled(Feature::ConcurrentReasoningSummaries), attestation_provider, + config.http_client_factory(), ) .with_prompt_cache_key_override( crate::guardian::prompt_cache_key_override_for_review_session( diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 1ff469cf4c..5ebce9aa42 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -33,6 +33,8 @@ use codex_core_skills::HostSkillsSnapshot; use core_test_support::test_codex::local_selections; use codex_features::Feature; +use codex_http_client::HttpClientFactory; +use codex_http_client::OutboundProxyPolicy; use codex_login::CodexAuth; use codex_login::auth::AgentIdentityAuthPolicy; use codex_model_provider_info::ModelProviderInfo; @@ -492,6 +494,7 @@ fn test_model_client_session() -> crate::client::ModelClientSession { /*item_ids_enabled*/ false, /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), ) .new_session() } @@ -5466,6 +5469,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { .features .enabled(Feature::ConcurrentReasoningSummaries), /*attestation_provider*/ None, + config.http_client_factory(), ), code_mode_service: crate::tools::code_mode::CodeModeService::new(Arc::new( codex_code_mode::InProcessCodeModeSessionProvider, @@ -7596,6 +7600,7 @@ where .features .enabled(Feature::ConcurrentReasoningSummaries), /*attestation_provider*/ None, + config.http_client_factory(), ), code_mode_service: crate::tools::code_mode::CodeModeService::new(Arc::new( codex_code_mode::InProcessCodeModeSessionProvider, diff --git a/codex-rs/core/tests/responses_headers.rs b/codex-rs/core/tests/responses_headers.rs index 4c5f906160..b31b6d5682 100644 --- a/codex-rs/core/tests/responses_headers.rs +++ b/codex-rs/core/tests/responses_headers.rs @@ -133,6 +133,7 @@ async fn responses_stream_includes_subagent_header_on_review() { /*item_ids_enabled*/ false, /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, + config.http_client_factory(), ); let responses_metadata = test_turn_responses_metadata(&client, thread_id, &session_source); let mut client_session = client.new_session(); @@ -268,6 +269,7 @@ async fn responses_stream_includes_subagent_header_on_other() { /*item_ids_enabled*/ false, /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, + config.http_client_factory(), ); let responses_metadata = test_turn_responses_metadata(&client, thread_id, &session_source); let mut client_session = client.new_session(); @@ -389,6 +391,7 @@ async fn responses_respects_model_info_overrides_from_config() { /*item_ids_enabled*/ false, /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, + config.http_client_factory(), ); let responses_metadata = test_turn_responses_metadata(&client, thread_id, &session_source); let mut client_session = client.new_session(); diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 92624bfcb3..4d298998af 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1339,6 +1339,7 @@ async fn send_provider_auth_request(server: &MockServer, auth: ModelProviderAuth .features .enabled(Feature::ConcurrentReasoningSummaries), /*attestation_provider*/ None, + config.http_client_factory(), ); let responses_metadata = test_turn_responses_metadata(&client, thread_id); let mut client_session = client.new_session(); @@ -3007,6 +3008,7 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() { /*item_ids_enabled*/ false, /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, + config.http_client_factory(), ); let responses_metadata = test_turn_responses_metadata(&client, thread_id); let mut client_session = client.new_session(); diff --git a/codex-rs/core/tests/suite/client_websockets.rs b/codex-rs/core/tests/suite/client_websockets.rs index c56a8680ab..05a93807b2 100755 --- a/codex-rs/core/tests/suite/client_websockets.rs +++ b/codex-rs/core/tests/suite/client_websockets.rs @@ -2284,6 +2284,7 @@ async fn websocket_harness_with_provider_options( .features .enabled(Feature::ConcurrentReasoningSummaries), /*attestation_provider*/ None, + config.http_client_factory(), ); WebsocketTestHarness { diff --git a/codex-rs/http-client/src/lib.rs b/codex-rs/http-client/src/lib.rs index 5f90435452..07e579ecb3 100644 --- a/codex-rs/http-client/src/lib.rs +++ b/codex-rs/http-client/src/lib.rs @@ -25,9 +25,9 @@ pub use crate::error::StreamError; pub use crate::error::TransportError; pub use crate::outbound_proxy::BuildRouteAwareHttpClientError; pub use crate::outbound_proxy::ClientRouteClass; -pub use crate::outbound_proxy::OutboundProxyConfig; +pub use crate::outbound_proxy::HttpClientFactory; +pub use crate::outbound_proxy::OutboundProxyPolicy; pub use crate::outbound_proxy::RouteFailureClass; -pub use crate::outbound_proxy::build_reqwest_client_for_route; pub use crate::request::EncodedJsonBody; pub use crate::request::PreparedRequestBody; pub use crate::request::Request; diff --git a/codex-rs/http-client/src/outbound_proxy.rs b/codex-rs/http-client/src/outbound_proxy.rs index e643698a8c..5506f3d40b 100644 --- a/codex-rs/http-client/src/outbound_proxy.rs +++ b/codex-rs/http-client/src/outbound_proxy.rs @@ -84,14 +84,54 @@ impl fmt::Display for RouteFailureClass { } } -/// Marker enabling fixed system/PAC/WPAD, environment, then direct routing. -/// Resolved endpoints and platform details remain internal to the client builder. +/// Resolved outbound proxy behavior for HTTP clients. +/// +/// Callers must choose a policy explicitly so omitting feature resolution cannot silently select +/// legacy behavior. #[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct OutboundProxyConfig; +pub enum OutboundProxyPolicy { + /// Preserve reqwest's built-in proxy behavior. + ReqwestDefault, + /// Resolve system/PAC/WPAD settings, then environment settings, then direct routing. + RespectSystemProxy, +} -impl OutboundProxyConfig { - pub const fn respect_system_proxy() -> Self { - Self +/// Builds route-specific HTTP clients using one resolved outbound proxy policy. +/// +/// Construct this once from the effective application configuration and carry it with the +/// session or component that owns outbound requests. Individual request paths should supply only +/// their destination and route class rather than resolving feature state themselves. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct HttpClientFactory { + outbound_proxy_policy: OutboundProxyPolicy, +} + +impl HttpClientFactory { + /// Creates a factory from the outbound proxy policy resolved by the application. + pub const fn new(outbound_proxy_policy: OutboundProxyPolicy) -> Self { + Self { + outbound_proxy_policy, + } + } + + /// Returns the outbound proxy policy used for clients built by this factory. + pub const fn outbound_proxy_policy(&self) -> OutboundProxyPolicy { + self.outbound_proxy_policy + } + + /// Builds a reqwest client for a concrete outbound route. + pub fn build_reqwest_client( + &self, + builder: reqwest::ClientBuilder, + request_url: &str, + route_class: ClientRouteClass, + ) -> Result { + build_reqwest_client_for_route( + builder, + request_url, + route_class, + self.outbound_proxy_policy, + ) } } @@ -120,18 +160,18 @@ impl From for io::Error { /// a route is selected are returned without trying another route. Ordered PAC candidates are /// currently collapsed to one route on both Windows and macOS; later proxy or `DIRECT` candidates /// are not retried after a connection failure. -pub fn build_reqwest_client_for_route( +fn build_reqwest_client_for_route( builder: reqwest::ClientBuilder, request_url: &str, route_class: ClientRouteClass, - config: Option<&OutboundProxyConfig>, + outbound_proxy_policy: OutboundProxyPolicy, ) -> Result { let builder = configure_proxy_for_route( &ProcessEnv, builder, request_url, route_class, - config, + outbound_proxy_policy, resolve_system_proxy, )?; build_reqwest_client_with_custom_ca(builder).map_err(Into::into) @@ -142,10 +182,10 @@ fn configure_proxy_for_route( builder: reqwest::ClientBuilder, request_url: &str, route_class: ClientRouteClass, - config: Option<&OutboundProxyConfig>, + outbound_proxy_policy: OutboundProxyPolicy, resolve_system_proxy: impl FnOnce(&str, &RequestOrigin) -> SystemProxyDecision, ) -> Result { - if config.is_none() { + if matches!(outbound_proxy_policy, OutboundProxyPolicy::ReqwestDefault) { return Ok(builder); } let origin = RequestOrigin::parse(request_url); diff --git a/codex-rs/http-client/src/outbound_proxy_tests.rs b/codex-rs/http-client/src/outbound_proxy_tests.rs index 60a1d56396..734d5483ec 100644 --- a/codex-rs/http-client/src/outbound_proxy_tests.rs +++ b/codex-rs/http-client/src/outbound_proxy_tests.rs @@ -79,13 +79,12 @@ async fn enabled_environment_proxy_routes_request_through_proxy() { values: HashMap::from([("HTTP_PROXY".to_string(), format!("http://{proxy_addr}"))]), }; let request_url = "http://enabled-proxy.test/proxy-check"; - let config = OutboundProxyConfig::respect_system_proxy(); let builder = configure_proxy_for_route( &env, reqwest::Client::builder().timeout(Duration::from_secs(2)), request_url, ClientRouteClass::Auth, - Some(&config), + OutboundProxyPolicy::RespectSystemProxy, |_, _| SystemProxyDecision::Unavailable { failure: RouteFailureClass::ProxyResolutionUnavailable, }, diff --git a/codex-rs/login/src/auth/default_client.rs b/codex-rs/login/src/auth/default_client.rs index 1146a89d96..089e78340b 100644 --- a/codex-rs/login/src/auth/default_client.rs +++ b/codex-rs/login/src/auth/default_client.rs @@ -8,8 +8,9 @@ use codex_http_client::BuildCustomCaTransportError; use codex_http_client::BuildRouteAwareHttpClientError; use codex_http_client::ClientRouteClass; use codex_http_client::HttpClient; +use codex_http_client::HttpClientFactory; +use codex_http_client::OutboundProxyPolicy; pub use codex_http_client::RequestBuilder as CodexRequestBuilder; -use codex_http_client::build_reqwest_client_for_route; use codex_http_client::build_reqwest_client_with_custom_ca; use codex_http_client::with_chatgpt_cloudflare_cookie_store; use codex_terminal_detection::user_agent; @@ -235,6 +236,35 @@ pub fn try_build_reqwest_client() -> Result Result { + if matches!( + http_client_factory.outbound_proxy_policy(), + OutboundProxyPolicy::ReqwestDefault + ) { + return Ok(build_reqwest_client()); + } + if is_sandboxed() { + // Preserve the sandbox's existing no-proxy policy; sandboxed command egress is routed + // separately through network-proxy. + return Ok(build_reqwest_client()); + } + + http_client_factory.build_reqwest_client( + default_reqwest_client_builder(), + request_url, + route_class, + ) +} + fn default_reqwest_client_builder() -> reqwest::ClientBuilder { let mut builder = reqwest::Client::builder().default_headers(default_headers()); if is_sandboxed() { @@ -248,11 +278,10 @@ pub(crate) fn build_raw_auth_reqwest_client( endpoint: &str, auth_route_config: Option<&AuthRouteConfig>, ) -> Result { - build_reqwest_client_for_route( + auth_http_client_factory(auth_route_config).build_reqwest_client( reqwest::Client::builder(), endpoint, ClientRouteClass::Auth, - auth_route_config.map(AuthRouteConfig::route_config), ) } @@ -261,20 +290,10 @@ pub(crate) fn build_default_auth_reqwest_client( endpoint: &str, auth_route_config: Option<&AuthRouteConfig>, ) -> Result { - let Some(route_config) = auth_route_config.map(AuthRouteConfig::route_config) else { - return Ok(build_reqwest_client()); - }; - - if is_sandboxed() { - // Preserve the sandbox's existing no-proxy policy; sandboxed command egress is routed - // separately through network-proxy. - return Ok(build_reqwest_client()); - } - build_reqwest_client_for_route( - default_reqwest_client_builder(), + build_default_reqwest_client_for_route( + &auth_http_client_factory(auth_route_config), endpoint, ClientRouteClass::Auth, - Some(route_config), ) } @@ -286,6 +305,13 @@ pub(crate) fn create_default_auth_client( build_default_auth_reqwest_client(endpoint, auth_route_config).map(HttpClient::new) } +fn auth_http_client_factory(auth_route_config: Option<&AuthRouteConfig>) -> HttpClientFactory { + auth_route_config.map_or_else( + || HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + |config| config.http_client_factory().clone(), + ) +} + pub fn default_headers() -> HeaderMap { let mut headers = HeaderMap::new(); headers.insert("originator", originator().header_value); diff --git a/codex-rs/login/src/outbound_proxy.rs b/codex-rs/login/src/outbound_proxy.rs index 66441e3637..79fcf76de1 100644 --- a/codex-rs/login/src/outbound_proxy.rs +++ b/codex-rs/login/src/outbound_proxy.rs @@ -1,4 +1,5 @@ -use codex_http_client::OutboundProxyConfig; +use codex_http_client::HttpClientFactory; +use codex_http_client::OutboundProxyPolicy; /// Auth-layer adapter around client-owned proxy policy. /// @@ -6,17 +7,17 @@ use codex_http_client::OutboundProxyConfig; /// client layer. #[derive(Debug, Clone, PartialEq, Eq)] pub struct AuthRouteConfig { - route_config: OutboundProxyConfig, + http_client_factory: HttpClientFactory, } impl AuthRouteConfig { pub fn respect_system_proxy() -> Self { Self { - route_config: OutboundProxyConfig::respect_system_proxy(), + http_client_factory: HttpClientFactory::new(OutboundProxyPolicy::RespectSystemProxy), } } - pub(crate) fn route_config(&self) -> &OutboundProxyConfig { - &self.route_config + pub(crate) fn http_client_factory(&self) -> &HttpClientFactory { + &self.http_client_factory } } diff --git a/codex-rs/memories/write/src/runtime.rs b/codex-rs/memories/write/src/runtime.rs index ae0903cfdc..903a1743ff 100644 --- a/codex-rs/memories/write/src/runtime.rs +++ b/codex-rs/memories/write/src/runtime.rs @@ -263,6 +263,7 @@ impl MemoryStartupContext { config.features.enabled(Feature::ItemIds), /*concurrent_reasoning_summaries_enabled*/ false, /*attestation_provider*/ None, + config.http_client_factory(), ); let mut client_session = model_client.new_session(); From e061022857e4890212444fc05c19230ecbd5964c Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Mon, 6 Jul 2026 20:50:51 -0700 Subject: [PATCH 7/7] core: use HTTP Responses with system proxy --- codex-rs/core/src/client.rs | 10 +++- .../core/tests/suite/websocket_fallback.rs | 55 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 0bf33b3993..f13e633d91 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -65,6 +65,7 @@ use codex_api::create_text_param_for_request; use codex_api::response_create_client_metadata; use codex_http_client::ClientRouteClass; use codex_http_client::HttpClientFactory; +use codex_http_client::OutboundProxyPolicy; use codex_login::AuthManager; use codex_login::CodexAuth; use codex_login::RefreshTokenError; @@ -926,9 +927,14 @@ impl ModelClient { /// Returns whether the Responses-over-WebSocket transport is active for this session. /// - /// WebSocket use is controlled by provider capability and session-scoped fallback state. + /// WebSocket use is controlled by outbound proxy policy, provider capability, and + /// session-scoped fallback state. System-proxy sessions conservatively use HTTP until the + /// WebSocket transport supports proxy-aware dialing. pub fn responses_websocket_enabled(&self) -> bool { - if !self.state.provider.info().supports_websockets + if matches!( + self.http_client_factory.outbound_proxy_policy(), + OutboundProxyPolicy::RespectSystemProxy + ) || !self.state.provider.info().supports_websockets || self.state.disable_websockets.load(Ordering::Relaxed) { return false; diff --git a/codex-rs/core/tests/suite/websocket_fallback.rs b/codex-rs/core/tests/suite/websocket_fallback.rs index cdf77694c2..c7923fe260 100644 --- a/codex-rs/core/tests/suite/websocket_fallback.rs +++ b/codex-rs/core/tests/suite/websocket_fallback.rs @@ -1,4 +1,5 @@ use anyhow::Result; +use codex_config::CONFIG_TOML_FILE; use codex_model_provider_info::WireApi; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; @@ -26,6 +27,60 @@ use wiremock::http::Method; use wiremock::matchers::method; use wiremock::matchers::path_regex; +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn system_proxy_policy_uses_http_without_websocket_handshake() -> Result<()> { + skip_if_no_network!(Ok(())); + + let server = responses::start_mock_server().await; + let response_mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + + let mut builder = test_codex() + .with_pre_build_hook(|codex_home| { + std::fs::write( + codex_home.join(CONFIG_TOML_FILE), + "[features]\nrespect_system_proxy = true\n", + ) + .expect("system-proxy feature config should be written"); + }) + .with_config({ + let base_url = format!("{}/v1", server.uri()); + move |config| { + config.model_provider.base_url = Some(base_url); + config.model_provider.wire_api = WireApi::Responses; + config.model_provider.supports_websockets = true; + config.model_provider.request_max_retries = Some(0); + } + }); + let test = builder.build_with_auto_env(&server).await?; + assert!(test.config.respect_system_proxy); + + test.submit_turn("hello").await?; + + let requests = server.received_requests().await.unwrap_or_default(); + let websocket_attempts = requests + .iter() + .filter(|request| { + request.method == Method::GET && request.url.path().ends_with("/responses") + }) + .count(); + let http_attempts = requests + .iter() + .filter(|request| { + request.method == Method::POST && request.url.path().ends_with("/responses") + }) + .count(); + + assert_eq!(websocket_attempts, 0); + assert_eq!(http_attempts, 1); + assert_eq!(response_mock.requests().len(), 1); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn websocket_fallback_switches_to_http_on_upgrade_required_connect() -> Result<()> { skip_if_no_network!(Ok(()));