From faa5d4a5e28330a80b251f571ed3a03fae75fa87 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Fri, 8 May 2026 16:33:29 -0700 Subject: [PATCH] Increase exec-server environment transport timeouts (#21825) ## Why The environment-backed exec-server transport currently hardcodes 5 second connect and initialize timeouts in `client_transport.rs`. That is short for SSH-backed stdio environments and remote websocket environments, and there is currently no way to raise those values from `CODEX_HOME/environments.toml`. This stacked follow-up raises the default environment transport timeouts and lets each configured environment override them in `environments.toml`. ## What Changed - raise the default environment transport connect and initialize timeouts from 5s to 10s - store concrete timeout values on `ExecServerTransportParams` instead of hardcoding them in `connect_for_transport(...)` - add `connect_timeout_sec` and `initialize_timeout_sec` to `[[environments]]` entries in `environments.toml` - apply parse-time defaults so runtime transport code receives fully resolved timeout values - reject `connect_timeout_sec` on stdio environments because it only applies to websocket transports - extend parser tests to cover the new fields and defaults ## Stack - base: https://github.com/openai/codex/pull/21794 - this PR: configurable environment transport timeouts ## Validation - `cd /Users/starr/code/codex-worktrees/exec-env-timeouts-config-20260508/codex-rs && just fmt` - not run: tests --------- Co-authored-by: Codex --- codex-rs/exec-server/src/client.rs | 23 ++-- codex-rs/exec-server/src/client_api.rs | 24 +++- codex-rs/exec-server/src/client_transport.rs | 21 +-- codex-rs/exec-server/src/environment.rs | 11 +- codex-rs/exec-server/src/environment_toml.rs | 136 +++++++++++++++++-- 5 files changed, 176 insertions(+), 39 deletions(-) diff --git a/codex-rs/exec-server/src/client.rs b/codex-rs/exec-server/src/client.rs index ff3cf37904..9261a59542 100644 --- a/codex-rs/exec-server/src/client.rs +++ b/codex-rs/exec-server/src/client.rs @@ -895,6 +895,8 @@ mod tests { use super::ExecServerClientConnectOptions; use crate::ProcessId; #[cfg(not(windows))] + use crate::client_api::DEFAULT_REMOTE_EXEC_SERVER_INITIALIZE_TIMEOUT; + #[cfg(not(windows))] use crate::client_api::ExecServerTransportParams; use crate::client_api::StdioExecServerCommand; use crate::client_api::StdioExecServerConnectArgs; @@ -962,15 +964,18 @@ mod tests { #[tokio::test] async fn connect_for_transport_initializes_stdio_command() { let client = ExecServerClient::connect_for_transport( - ExecServerTransportParams::StdioCommand(StdioExecServerCommand { - program: "sh".to_string(), - args: vec![ - "-c".to_string(), - "read _line; printf '%s\\n' '{\"id\":1,\"result\":{\"sessionId\":\"stdio-test\"}}'; read _line; sleep 60".to_string(), - ], - env: HashMap::new(), - cwd: None, - }), + ExecServerTransportParams::StdioCommand { + command: StdioExecServerCommand { + program: "sh".to_string(), + args: vec![ + "-c".to_string(), + "read _line; printf '%s\\n' '{\"id\":1,\"result\":{\"sessionId\":\"stdio-test\"}}'; read _line; sleep 60".to_string(), + ], + env: HashMap::new(), + cwd: None, + }, + initialize_timeout: DEFAULT_REMOTE_EXEC_SERVER_INITIALIZE_TIMEOUT, + }, ) .await .expect("stdio transport should connect"); diff --git a/codex-rs/exec-server/src/client_api.rs b/codex-rs/exec-server/src/client_api.rs index 8adfadd6e7..899863723f 100644 --- a/codex-rs/exec-server/src/client_api.rs +++ b/codex-rs/exec-server/src/client_api.rs @@ -9,6 +9,9 @@ use crate::HttpRequestParams; use crate::HttpRequestResponse; use crate::HttpResponseBodyStream; +pub(crate) const DEFAULT_REMOTE_EXEC_SERVER_CONNECT_TIMEOUT: Duration = Duration::from_secs(10); +pub(crate) const DEFAULT_REMOTE_EXEC_SERVER_INITIALIZE_TIMEOUT: Duration = Duration::from_secs(10); + /// Connection options for any exec-server client transport. #[derive(Debug, Clone, PartialEq, Eq)] pub struct ExecServerClientConnectOptions { @@ -48,9 +51,26 @@ pub(crate) struct StdioExecServerCommand { /// Parameters used to connect to a remote exec-server environment. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) enum ExecServerTransportParams { - WebSocketUrl(String), + WebSocketUrl { + websocket_url: String, + connect_timeout: Duration, + initialize_timeout: Duration, + }, #[allow(dead_code)] - StdioCommand(StdioExecServerCommand), + StdioCommand { + command: StdioExecServerCommand, + initialize_timeout: Duration, + }, +} + +impl ExecServerTransportParams { + pub(crate) fn websocket_url(websocket_url: String) -> Self { + Self::WebSocketUrl { + websocket_url, + connect_timeout: DEFAULT_REMOTE_EXEC_SERVER_CONNECT_TIMEOUT, + initialize_timeout: DEFAULT_REMOTE_EXEC_SERVER_INITIALIZE_TIMEOUT, + } + } } /// Sends HTTP requests through a runtime-selected transport. diff --git a/codex-rs/exec-server/src/client_transport.rs b/codex-rs/exec-server/src/client_transport.rs index 3fccfa25c5..23dc0bc7b3 100644 --- a/codex-rs/exec-server/src/client_transport.rs +++ b/codex-rs/exec-server/src/client_transport.rs @@ -1,6 +1,4 @@ use std::process::Stdio; -use std::time::Duration; - use tokio::io::AsyncBufReadExt; use tokio::io::BufReader; use tokio::process::Command; @@ -17,29 +15,34 @@ use crate::client_api::StdioExecServerConnectArgs; use crate::connection::JsonRpcConnection; const ENVIRONMENT_CLIENT_NAME: &str = "codex-environment"; -const ENVIRONMENT_CONNECT_TIMEOUT: Duration = Duration::from_secs(5); -const ENVIRONMENT_INITIALIZE_TIMEOUT: Duration = Duration::from_secs(5); impl ExecServerClient { pub(crate) async fn connect_for_transport( transport_params: crate::client_api::ExecServerTransportParams, ) -> Result { match transport_params { - crate::client_api::ExecServerTransportParams::WebSocketUrl(websocket_url) => { + crate::client_api::ExecServerTransportParams::WebSocketUrl { + websocket_url, + connect_timeout, + initialize_timeout, + } => { Self::connect_websocket(RemoteExecServerConnectArgs { websocket_url, client_name: ENVIRONMENT_CLIENT_NAME.to_string(), - connect_timeout: ENVIRONMENT_CONNECT_TIMEOUT, - initialize_timeout: ENVIRONMENT_INITIALIZE_TIMEOUT, + connect_timeout, + initialize_timeout, resume_session_id: None, }) .await } - crate::client_api::ExecServerTransportParams::StdioCommand(command) => { + crate::client_api::ExecServerTransportParams::StdioCommand { + command, + initialize_timeout, + } => { Self::connect_stdio_command(StdioExecServerConnectArgs { command, client_name: ENVIRONMENT_CLIENT_NAME.to_string(), - initialize_timeout: ENVIRONMENT_INITIALIZE_TIMEOUT, + initialize_timeout, resume_session_id: None, }) .await diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 049a2d0229..7e4a3fb056 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -373,7 +373,7 @@ impl Environment { local_runtime_paths: Option, ) -> Self { Self::remote_with_transport( - ExecServerTransportParams::WebSocketUrl(exec_server_url), + ExecServerTransportParams::websocket_url(exec_server_url), local_runtime_paths, ) } @@ -383,10 +383,11 @@ impl Environment { local_runtime_paths: Option, ) -> Self { let exec_server_url = match &remote_transport { - ExecServerTransportParams::WebSocketUrl(exec_server_url) => { - Some(exec_server_url.clone()) - } - ExecServerTransportParams::StdioCommand(_) => None, + ExecServerTransportParams::WebSocketUrl { + websocket_url: exec_server_url, + .. + } => Some(exec_server_url.clone()), + ExecServerTransportParams::StdioCommand { .. } => None, }; let client = LazyRemoteExecServerClient::new(remote_transport.clone()); let exec_backend: Arc = Arc::new(RemoteProcess::new(client.clone())); diff --git a/codex-rs/exec-server/src/environment_toml.rs b/codex-rs/exec-server/src/environment_toml.rs index 2f5fd97790..90f4c78262 100644 --- a/codex-rs/exec-server/src/environment_toml.rs +++ b/codex-rs/exec-server/src/environment_toml.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; +use std::time::Duration; use async_trait::async_trait; use serde::Deserialize; @@ -11,6 +12,8 @@ use crate::DefaultEnvironmentProvider; use crate::Environment; use crate::EnvironmentProvider; use crate::ExecServerError; +use crate::client_api::DEFAULT_REMOTE_EXEC_SERVER_CONNECT_TIMEOUT; +use crate::client_api::DEFAULT_REMOTE_EXEC_SERVER_INITIALIZE_TIMEOUT; use crate::client_api::ExecServerTransportParams; use crate::client_api::StdioExecServerCommand; use crate::environment::LOCAL_ENVIRONMENT_ID; @@ -38,6 +41,10 @@ struct EnvironmentToml { args: Option>, env: Option>, cwd: Option, + #[serde(default, with = "option_duration_secs")] + connect_timeout_sec: Option, + #[serde(default, with = "option_duration_secs")] + initialize_timeout_sec: Option, } #[derive(Clone, Debug, PartialEq, Eq)] @@ -108,6 +115,8 @@ fn parse_environment_toml( args, env, cwd, + connect_timeout_sec, + initialize_timeout_sec, } = item; validate_environment_id(&id)?; if program.is_none() && (args.is_some() || env.is_some() || cwd.is_some()) { @@ -115,11 +124,24 @@ fn parse_environment_toml( "environment `{id}` args, env, and cwd require program" ))); } + if url.is_none() && connect_timeout_sec.is_some() { + return Err(ExecServerError::Protocol(format!( + "environment `{id}` connect_timeout_sec requires url" + ))); + } + + let connect_timeout = connect_timeout_sec.unwrap_or(DEFAULT_REMOTE_EXEC_SERVER_CONNECT_TIMEOUT); + let initialize_timeout = + initialize_timeout_sec.unwrap_or(DEFAULT_REMOTE_EXEC_SERVER_INITIALIZE_TIMEOUT); let transport_params = match (url, program) { (Some(url), None) => { let url = validate_websocket_url(url)?; - ExecServerTransportParams::WebSocketUrl(url) + ExecServerTransportParams::WebSocketUrl { + websocket_url: url, + connect_timeout, + initialize_timeout, + } } (None, Some(program)) => { let program = program.trim().to_string(); @@ -129,12 +151,15 @@ fn parse_environment_toml( ))); } let cwd = normalize_stdio_cwd(&id, cwd, config_dir)?; - ExecServerTransportParams::StdioCommand(StdioExecServerCommand { - program, - args: args.unwrap_or_default(), - env: env.unwrap_or_default(), - cwd, - }) + ExecServerTransportParams::StdioCommand { + command: StdioExecServerCommand { + program, + args: args.unwrap_or_default(), + env: env.unwrap_or_default(), + cwd, + }, + initialize_timeout, + } } (None, None) | (Some(_), Some(_)) => { return Err(ExecServerError::Protocol(format!( @@ -278,6 +303,22 @@ fn load_environments_toml(path: &Path) -> Result(deserializer: D) -> Result, D::Error> + where + D: Deserializer<'de>, + { + let secs = Option::::deserialize(deserializer)?; + secs.map(|secs| Duration::try_from_secs_f64(secs).map_err(serde::de::Error::custom)) + .transpose() + } +} + #[cfg(test)] mod tests { use pretty_assertions::assert_eq; @@ -424,6 +465,15 @@ mod tests { }, "environment `devbox` args, env, and cwd require program", ), + ( + EnvironmentToml { + id: "ssh-dev".to_string(), + program: Some("ssh".to_string()), + connect_timeout_sec: Some(Duration::from_secs(1)), + ..Default::default() + }, + "environment `ssh-dev` connect_timeout_sec requires url", + ), ]; for (item, expected) in cases { @@ -459,12 +509,59 @@ mod tests { assert_eq!( provider.environments[0].1, - ExecServerTransportParams::StdioCommand(StdioExecServerCommand { - program: "ssh".to_string(), - args: Vec::new(), - env: HashMap::new(), - cwd: Some(config_dir.path().join("workspace")), - }) + ExecServerTransportParams::StdioCommand { + command: StdioExecServerCommand { + program: "ssh".to_string(), + args: Vec::new(), + env: HashMap::new(), + cwd: Some(config_dir.path().join("workspace")), + }, + initialize_timeout: DEFAULT_REMOTE_EXEC_SERVER_INITIALIZE_TIMEOUT, + } + ); + } + + #[test] + fn toml_provider_parses_configured_transport_timeouts() { + let provider = TomlEnvironmentProvider::new(EnvironmentsToml { + default: None, + environments: vec![ + EnvironmentToml { + id: "devbox".to_string(), + url: Some("ws://127.0.0.1:8765".to_string()), + connect_timeout_sec: Some(Duration::from_secs(12)), + initialize_timeout_sec: Some(Duration::from_secs(34)), + ..Default::default() + }, + EnvironmentToml { + id: "ssh-dev".to_string(), + program: Some("ssh".to_string()), + initialize_timeout_sec: Some(Duration::from_secs(56)), + ..Default::default() + }, + ], + }) + .expect("provider"); + + assert_eq!( + provider.environments[0].1, + ExecServerTransportParams::WebSocketUrl { + websocket_url: "ws://127.0.0.1:8765".to_string(), + connect_timeout: Duration::from_secs(12), + initialize_timeout: Duration::from_secs(34), + } + ); + assert_eq!( + provider.environments[1].1, + ExecServerTransportParams::StdioCommand { + command: StdioExecServerCommand { + program: "ssh".to_string(), + args: Vec::new(), + env: HashMap::new(), + cwd: None, + }, + initialize_timeout: Duration::from_secs(56), + } ); } @@ -559,6 +656,8 @@ default = "ssh-dev" [[environments]] id = "devbox" url = "ws://127.0.0.1:4512" +connect_timeout_sec = 12.0 +initialize_timeout_sec = 34.0 [[environments]] id = "ssh-dev" @@ -575,7 +674,16 @@ CODEX_LOG = "debug" assert_eq!(environments.default.as_deref(), Some("ssh-dev")); assert_eq!(environments.environments.len(), 2); - assert_eq!(environments.environments[0].id, "devbox"); + assert_eq!( + environments.environments[0], + EnvironmentToml { + id: "devbox".to_string(), + url: Some("ws://127.0.0.1:4512".to_string()), + connect_timeout_sec: Some(Duration::from_secs(12)), + initialize_timeout_sec: Some(Duration::from_secs(34)), + ..Default::default() + } + ); assert_eq!( environments.environments[1], EnvironmentToml {