From 81387f9896489bb1e538e1146956f14ae9604cd1 Mon Sep 17 00:00:00 2001 From: Chummy Date: Sat, 28 Feb 2026 02:38:41 +0000 Subject: [PATCH] fix(codex): preserve transport overrides across runtimes --- docs/config-reference.md | 2 +- src/channels/mod.rs | 2 +- src/config/schema.rs | 14 ++++++++- src/gateway/mod.rs | 2 +- src/providers/mod.rs | 15 ++------- src/providers/openai_codex.rs | 52 ++++++++++++++++++++++++++++--- src/tools/mod.rs | 2 +- src/tools/model_routing_config.rs | 14 +++++++++ 8 files changed, 80 insertions(+), 23 deletions(-) diff --git a/docs/config-reference.md b/docs/config-reference.md index ccc8c33c1..ff24a7c89 100644 --- a/docs/config-reference.md +++ b/docs/config-reference.md @@ -395,7 +395,7 @@ Notes: - Transport override precedence for OpenAI Codex: 1. `[[model_routes]].transport` (route-specific) 2. `provider.transport` - 3. `ZEROCLAW_CODEX_TRANSPORT` / `ZEROCLAW_PROVIDER_TRANSPORT` + 3. `PROVIDER_TRANSPORT` / `ZEROCLAW_CODEX_TRANSPORT` / `ZEROCLAW_PROVIDER_TRANSPORT` 4. legacy `ZEROCLAW_RESPONSES_WEBSOCKET` (boolean) ## `[skills]` diff --git a/src/channels/mod.rs b/src/channels/mod.rs index decf6f017..02d434654 100644 --- a/src/channels/mod.rs +++ b/src/channels/mod.rs @@ -4805,7 +4805,7 @@ pub async fn start_channels(config: Config) -> Result<()> { let provider_runtime_options = providers::ProviderRuntimeOptions { auth_profile_override: None, provider_api_url: config.api_url.clone(), - provider_transport: None, + provider_transport: config.effective_provider_transport(), zeroclaw_dir: config.config_path.parent().map(std::path::PathBuf::from), secrets_encrypt: config.secrets.encrypt, reasoning_enabled: config.runtime.reasoning_enabled, diff --git a/src/config/schema.rs b/src/config/schema.rs index 162f3f5b9..b8319ed11 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -311,6 +311,14 @@ pub struct ProviderConfig { pub reasoning_level: Option, /// Optional transport override for providers that support multiple transports. /// Supported values: "auto", "websocket", "sse". + /// + /// Resolution order: + /// 1) `model_routes[].transport` (route-specific) + /// 2) `provider.transport` + /// 3) env overrides (`PROVIDER_TRANSPORT`, `ZEROCLAW_PROVIDER_TRANSPORT`, `ZEROCLAW_CODEX_TRANSPORT`) + /// 4) runtime default (`auto`, WebSocket-first with SSE fallback for OpenAI Codex) + /// + /// Existing configs that omit `provider.transport` remain valid and fall back to defaults. #[serde(default)] pub transport: Option, } @@ -3199,8 +3207,12 @@ pub struct ModelRouteConfig { /// Optional API key override for this route's provider #[serde(default)] pub api_key: Option, - /// Optional provider transport override for this route. + /// Optional route-specific transport override for this route. /// Supported values: "auto", "websocket", "sse". + /// + /// When `model_routes[].transport` is unset, the route inherits `provider.transport`. + /// If both are unset, runtime defaults are used (`auto` for OpenAI Codex). + /// Existing configs without this field remain valid. #[serde(default)] pub transport: Option, } diff --git a/src/gateway/mod.rs b/src/gateway/mod.rs index 4df567bfb..ff7a5d6f5 100644 --- a/src/gateway/mod.rs +++ b/src/gateway/mod.rs @@ -362,7 +362,7 @@ pub async fn run_gateway(host: &str, port: u16, config: Config) -> Result<()> { &providers::ProviderRuntimeOptions { auth_profile_override: None, provider_api_url: config.api_url.clone(), - provider_transport: None, + provider_transport: config.effective_provider_transport(), zeroclaw_dir: config.config_path.parent().map(std::path::PathBuf::from), secrets_encrypt: config.secrets.encrypt, reasoning_enabled: config.runtime.reasoning_enabled, diff --git a/src/providers/mod.rs b/src/providers/mod.rs index bca02f915..b9aec3b85 100644 --- a/src/providers/mod.rs +++ b/src/providers/mod.rs @@ -1552,19 +1552,8 @@ pub fn create_routed_provider_with_options( } } - // Build route table - let routes: Vec<(String, router::Route)> = model_routes - .iter() - .map(|r| { - ( - r.hint.clone(), - router::Route { - provider_name: r.provider.clone(), - model: r.model.clone(), - }, - ) - }) - .collect(); + // Keep only successfully initialized routed providers and preserve + // their provider-id bindings (e.g. "#"). Ok(Box::new( router::RouterProvider::new(providers, routes, default_model.to_string()) diff --git a/src/providers/openai_codex.rs b/src/providers/openai_codex.rs index aedaa481a..39a6b593e 100644 --- a/src/providers/openai_codex.rs +++ b/src/providers/openai_codex.rs @@ -9,6 +9,8 @@ use reqwest::Client; use serde::{Deserialize, Serialize}; use serde_json::Value; use std::path::PathBuf; +use std::time::Duration; +use tokio::time::timeout; use tokio_tungstenite::{ connect_async, tungstenite::{ @@ -29,6 +31,9 @@ const CODEX_PROVIDER_TRANSPORT_ENV: &str = "ZEROCLAW_PROVIDER_TRANSPORT"; const CODEX_RESPONSES_WEBSOCKET_ENV_LEGACY: &str = "ZEROCLAW_RESPONSES_WEBSOCKET"; const DEFAULT_CODEX_INSTRUCTIONS: &str = "You are ZeroClaw, a concise and helpful coding assistant."; +const CODEX_WS_CONNECT_TIMEOUT: Duration = Duration::from_secs(20); +const CODEX_WS_SEND_TIMEOUT: Duration = Duration::from_secs(15); +const CODEX_WS_READ_TIMEOUT: Duration = Duration::from_secs(60); #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum CodexTransport { @@ -714,16 +719,50 @@ impl OpenAiCodexProvider { "parallel_tool_calls": request.parallel_tool_calls, }); - let (mut ws_stream, _) = connect_async(ws_request).await?; - ws_stream - .send(WsMessage::Text(serde_json::to_string(&payload)?.into())) - .await?; + let (mut ws_stream, _) = timeout(CODEX_WS_CONNECT_TIMEOUT, connect_async(ws_request)) + .await + .map_err(|_| { + anyhow::anyhow!( + "OpenAI Codex websocket connect timed out after {}s", + CODEX_WS_CONNECT_TIMEOUT.as_secs() + ) + })??; + timeout( + CODEX_WS_SEND_TIMEOUT, + ws_stream.send(WsMessage::Text(serde_json::to_string(&payload)?.into())), + ) + .await + .map_err(|_| { + anyhow::anyhow!( + "OpenAI Codex websocket send timed out after {}s", + CODEX_WS_SEND_TIMEOUT.as_secs() + ) + })??; let mut saw_delta = false; let mut delta_accumulator = String::new(); let mut fallback_text: Option = None; + let mut timed_out = false; - while let Some(frame) = ws_stream.next().await { + loop { + let frame = match timeout(CODEX_WS_READ_TIMEOUT, ws_stream.next()).await { + Ok(frame) => frame, + Err(_) => { + let _ = ws_stream.close(None).await; + if saw_delta || fallback_text.is_some() { + timed_out = true; + break; + } + anyhow::bail!( + "OpenAI Codex websocket stream timed out after {}s waiting for events", + CODEX_WS_READ_TIMEOUT.as_secs() + ); + } + }; + + let Some(frame) = frame else { + break; + }; let frame = frame?; let event: Value = match frame { WsMessage::Text(text) => serde_json::from_str(text.as_ref())?, @@ -786,6 +825,9 @@ impl OpenAiCodexProvider { if let Some(text) = fallback_text { return Ok(text); } + if timed_out { + anyhow::bail!("No response from OpenAI Codex websocket stream before timeout"); + } anyhow::bail!("No response from OpenAI Codex websocket stream"); } diff --git a/src/tools/mod.rs b/src/tools/mod.rs index 7ddc341d8..7a915ce00 100644 --- a/src/tools/mod.rs +++ b/src/tools/mod.rs @@ -428,7 +428,7 @@ pub fn all_tools_with_runtime( let provider_runtime_options = crate::providers::ProviderRuntimeOptions { auth_profile_override: None, provider_api_url: root_config.api_url.clone(), - provider_transport: None, + provider_transport: root_config.effective_provider_transport(), zeroclaw_dir: root_config .config_path .parent() diff --git a/src/tools/model_routing_config.rs b/src/tools/model_routing_config.rs index 6ff23ebb9..68abc97aa 100644 --- a/src/tools/model_routing_config.rs +++ b/src/tools/model_routing_config.rs @@ -217,6 +217,7 @@ impl ModelRoutingConfigTool { "hint": route.hint, "provider": route.provider, "model": route.model, + "transport": route.transport, "api_key_configured": has_provider_credential(&route.provider, route.api_key.as_deref()), "classification": classification, }) @@ -429,6 +430,7 @@ impl ModelRoutingConfigTool { let provider = Self::parse_non_empty_string(args, "provider")?; let model = Self::parse_non_empty_string(args, "model")?; let api_key_update = Self::parse_optional_string_update(args, "api_key")?; + let transport_update = Self::parse_optional_string_update(args, "transport")?; let keywords_update = if let Some(raw) = args.get("keywords") { Some(Self::parse_string_list(raw, "keywords")?) @@ -479,6 +481,12 @@ impl ModelRoutingConfigTool { MaybeSet::Unset => {} } + match transport_update { + MaybeSet::Set(transport) => next_route.transport = Some(transport), + MaybeSet::Null => next_route.transport = None, + MaybeSet::Unset => {} + } + cfg.model_routes.retain(|route| route.hint != hint); cfg.model_routes.push(next_route); Self::normalize_and_sort_routes(&mut cfg.model_routes); @@ -783,6 +791,10 @@ impl Tool for ModelRoutingConfigTool { "type": ["string", "null"], "description": "Optional API key override for scenario route or delegate agent" }, + "transport": { + "type": ["string", "null"], + "description": "Optional route transport override for upsert_scenario (auto, websocket, sse)" + }, "keywords": { "description": "Classification keywords for upsert_scenario (string or string array)", "oneOf": [ @@ -1005,6 +1017,7 @@ mod tests { "hint": "coding", "provider": "openai", "model": "gpt-5.3-codex", + "transport": "websocket", "classification_enabled": true, "keywords": ["code", "bug", "refactor"], "patterns": ["```"], @@ -1026,6 +1039,7 @@ mod tests { item["hint"] == json!("coding") && item["provider"] == json!("openai") && item["model"] == json!("gpt-5.3-codex") + && item["transport"] == json!("websocket") })); }