diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index d297c04203..a0cceba2b8 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -2542,6 +2542,7 @@ impl CodexMessageProcessor { service_name, request_trace, environment_id, + requested_cwd, ) .instrument(tracing::info_span!( "app_server.thread_start.create_thread", diff --git a/codex-rs/app-server/tests/suite/v2/thread_start.rs b/codex-rs/app-server/tests/suite/v2/thread_start.rs index 10ec30e778..7b9ca39422 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -46,6 +46,7 @@ use super::analytics::wait_for_analytics_payload; const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; +const CODEX_EXEC_SERVER_CWD_ENV_VAR: &str = "CODEX_EXEC_SERVER_CWD"; #[tokio::test] async fn thread_start_creates_thread_and_emits_started() -> Result<()> { @@ -196,6 +197,87 @@ async fn thread_start_accepts_explicit_local_environment_when_default_is_remote( Ok(()) } +#[tokio::test] +async fn thread_start_uses_remote_default_cwd_when_request_omits_cwd() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + + let codex_home = TempDir::new()?; + create_config_toml_without_approval_policy(codex_home.path(), &server.uri())?; + + let remote_default_cwd = TempDir::new()?; + let remote_default_cwd = remote_default_cwd.path().to_string_lossy().into_owned(); + + let mut mcp = McpProcess::new_with_env( + codex_home.path(), + &[ + (CODEX_EXEC_SERVER_URL_ENV_VAR, Some("ws://127.0.0.1:1")), + ( + CODEX_EXEC_SERVER_CWD_ENV_VAR, + Some(remote_default_cwd.as_str()), + ), + ], + ) + .await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let req_id = mcp + .send_thread_start_request(ThreadStartParams::default()) + .await?; + + let resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(req_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(resp)?; + + assert_eq!(thread.cwd.as_path(), Path::new(&remote_default_cwd)); + Ok(()) +} + +#[tokio::test] +async fn thread_start_explicit_cwd_overrides_remote_default_cwd() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + + let codex_home = TempDir::new()?; + create_config_toml_without_approval_policy(codex_home.path(), &server.uri())?; + + let remote_default_cwd = TempDir::new()?; + let remote_default_cwd = remote_default_cwd.path().to_string_lossy().into_owned(); + let requested_cwd = TempDir::new()?; + let requested_cwd = requested_cwd.path().to_string_lossy().into_owned(); + + let mut mcp = McpProcess::new_with_env( + codex_home.path(), + &[ + (CODEX_EXEC_SERVER_URL_ENV_VAR, Some("ws://127.0.0.1:1")), + ( + CODEX_EXEC_SERVER_CWD_ENV_VAR, + Some(remote_default_cwd.as_str()), + ), + ], + ) + .await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let req_id = mcp + .send_thread_start_request(ThreadStartParams { + cwd: Some(requested_cwd.clone()), + ..Default::default() + }) + .await?; + + let resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(req_id)), + ) + .await??; + let ThreadStartResponse { thread, .. } = to_response::(resp)?; + + assert_eq!(thread.cwd.as_path(), Path::new(&requested_cwd)); + Ok(()) +} + #[tokio::test] async fn thread_start_rejects_explicit_remote_environment_when_not_configured() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index abcb23f314..5de65a8398 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -431,6 +431,7 @@ pub(crate) struct CodexSpawnArgs { pub(crate) models_manager: Arc, pub(crate) environment_manager: Arc, pub(crate) environment_id: Option, + pub(crate) requested_cwd: Option, pub(crate) skills_manager: Arc, pub(crate) plugins_manager: Arc, pub(crate) mcp_manager: Arc, @@ -485,6 +486,7 @@ impl Codex { models_manager, environment_manager, environment_id, + requested_cwd, skills_manager, plugins_manager, mcp_manager, @@ -508,6 +510,20 @@ impl Codex { .environment(environment_id.as_deref()) .await .map_err(|err| CodexErr::Fatal(format!("failed to create environment: {err}")))?; + if requested_cwd.is_none() + && let Some(default_cwd) = environment + .as_ref() + .and_then(|environment| environment.default_cwd()) + { + config.cwd = + codex_utils_absolute_path::AbsolutePathBuf::from_absolute_path(default_cwd) + .map_err(|err| { + CodexErr::Fatal(format!( + "failed to resolve environment default cwd {}: {err}", + default_cwd.display() + )) + })?; + } let fs = environment .as_ref() .map(|environment| environment.get_filesystem()); diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index eda1c13d18..9f02398ccc 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -83,6 +83,7 @@ pub(crate) async fn run_codex_thread_interactive( parent_ctx.environment.as_deref(), )), environment_id: None, + requested_cwd: None, skills_manager: Arc::clone(&parent_session.services.skills_manager), plugins_manager: Arc::clone(&parent_session.services.plugins_manager), mcp_manager: Arc::clone(&parent_session.services.mcp_manager), diff --git a/codex-rs/core/src/codex_tests_guardian.rs b/codex-rs/core/src/codex_tests_guardian.rs index 2de4ed613f..6b60eab58e 100644 --- a/codex-rs/core/src/codex_tests_guardian.rs +++ b/codex-rs/core/src/codex_tests_guardian.rs @@ -436,6 +436,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() { models_manager, environment_manager: Arc::new(EnvironmentManager::new(/*exec_server_url*/ None)), environment_id: None, + requested_cwd: None, skills_manager, plugins_manager, mcp_manager, diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 9b37d61c67..47adeebceb 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -489,6 +489,7 @@ impl ThreadManager { /*metrics_service_name*/ None, /*parent_trace*/ None, /*environment_id*/ None, + /*requested_cwd*/ None, )) .await } @@ -502,6 +503,7 @@ impl ThreadManager { metrics_service_name: Option, parent_trace: Option, environment_id: Option, + requested_cwd: Option, ) -> CodexResult { Box::pin(self.state.spawn_thread( config, @@ -514,6 +516,7 @@ impl ThreadManager { parent_trace, /*user_shell_override*/ None, environment_id, + requested_cwd, )) .await } @@ -555,6 +558,7 @@ impl ThreadManager { parent_trace, /*user_shell_override*/ None, /*environment_id*/ None, + /*requested_cwd*/ None, )) .await } @@ -575,6 +579,7 @@ impl ThreadManager { /*parent_trace*/ None, /*user_shell_override*/ Some(user_shell_override), /*environment_id*/ None, + /*requested_cwd*/ None, )) .await } @@ -598,6 +603,7 @@ impl ThreadManager { /*parent_trace*/ None, /*user_shell_override*/ Some(user_shell_override), /*environment_id*/ None, + /*requested_cwd*/ None, )) .await } @@ -707,6 +713,7 @@ impl ThreadManager { parent_trace, /*user_shell_override*/ None, /*environment_id*/ None, + /*requested_cwd*/ None, )) .await } @@ -809,6 +816,7 @@ impl ThreadManagerState { /*parent_trace*/ None, /*user_shell_override*/ None, /*environment_id*/ None, + /*requested_cwd*/ None, )) .await } @@ -837,6 +845,7 @@ impl ThreadManagerState { /*parent_trace*/ None, /*user_shell_override*/ None, /*environment_id*/ None, + /*requested_cwd*/ None, )) .await } @@ -866,6 +875,7 @@ impl ThreadManagerState { /*parent_trace*/ None, /*user_shell_override*/ None, /*environment_id*/ None, + /*requested_cwd*/ None, )) .await } @@ -884,6 +894,7 @@ impl ThreadManagerState { parent_trace: Option, user_shell_override: Option, environment_id: Option, + requested_cwd: Option, ) -> CodexResult { Box::pin(self.spawn_thread_with_source( config, @@ -899,6 +910,7 @@ impl ThreadManagerState { parent_trace, user_shell_override, environment_id, + requested_cwd, )) .await } @@ -919,6 +931,7 @@ impl ThreadManagerState { parent_trace: Option, user_shell_override: Option, environment_id: Option, + requested_cwd: Option, ) -> CodexResult { let environment = self .environment_manager @@ -946,6 +959,7 @@ impl ThreadManagerState { models_manager: Arc::clone(&self.models_manager), environment_manager: Arc::clone(&self.environment_manager), environment_id, + requested_cwd, skills_manager: Arc::clone(&self.skills_manager), plugins_manager: Arc::clone(&self.plugins_manager), mcp_manager: Arc::clone(&self.mcp_manager), diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index a5efc6925f..1c3ccfc9b8 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -1,3 +1,5 @@ +use std::path::Path; +use std::path::PathBuf; use std::sync::Arc; use tokio::sync::OnceCell; @@ -14,6 +16,7 @@ use crate::remote_file_system::RemoteFileSystem; use crate::remote_process::RemoteProcess; pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; +pub const CODEX_EXEC_SERVER_CWD_ENV_VAR: &str = "CODEX_EXEC_SERVER_CWD"; const LOCAL_ENVIRONMENT_ID: &str = "local"; const REMOTE_ENVIRONMENT_ID: &str = "remote"; @@ -24,13 +27,21 @@ enum ExplicitEnvironmentId { Remote, } +#[derive(Clone, Debug, PartialEq, Eq)] +struct EnvironmentConfig { + exec_server_url: Option, + default_cwd: Option, +} + /// Lazily creates and caches the active environment for a session. /// /// The manager keeps the session's environment selection stable so subagents /// and follow-up turns preserve an explicit disabled state. #[derive(Debug)] pub struct EnvironmentManager { - exec_server_url: Option, + current_environment_config: Option, + local_environment_config: Option, + remote_environment_config: Option, local_runtime_paths: Option, disabled: bool, current_environment: OnceCell>>, @@ -47,18 +58,43 @@ impl Default for EnvironmentManager { impl EnvironmentManager { /// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value. pub fn new(exec_server_url: Option) -> Self { - Self::new_with_runtime_paths(exec_server_url, /*local_runtime_paths*/ None) + Self::new_with_runtime_paths( + exec_server_url, + /*exec_server_cwd*/ None, + /*local_runtime_paths*/ None, + ) } /// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value and local /// runtime paths used when creating local filesystem helpers. pub fn new_with_runtime_paths( exec_server_url: Option, + exec_server_cwd: Option, local_runtime_paths: Option, ) -> Self { let (exec_server_url, disabled) = normalize_exec_server_url(exec_server_url); + let local_environment_config = (!disabled).then_some(EnvironmentConfig { + exec_server_url: None, + default_cwd: None, + }); + let remote_environment_config = + exec_server_url + .clone() + .map(|exec_server_url| EnvironmentConfig { + exec_server_url: Some(exec_server_url), + default_cwd: exec_server_cwd, + }); + let current_environment_config = if disabled { + None + } else if remote_environment_config.is_some() { + remote_environment_config.clone() + } else { + local_environment_config.clone() + }; Self { - exec_server_url, + current_environment_config, + local_environment_config, + remote_environment_config, local_runtime_paths, disabled, current_environment: OnceCell::new(), @@ -79,6 +115,9 @@ impl EnvironmentManager { ) -> Self { Self::new_with_runtime_paths( std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok(), + std::env::var(CODEX_EXEC_SERVER_CWD_ENV_VAR) + .ok() + .map(PathBuf::from), local_runtime_paths, ) } @@ -88,7 +127,20 @@ impl EnvironmentManager { pub fn from_environment(environment: Option<&Environment>) -> Self { match environment { Some(environment) => Self { - exec_server_url: environment.exec_server_url().map(str::to_owned), + current_environment_config: Some(EnvironmentConfig { + exec_server_url: environment.exec_server_url().map(str::to_owned), + default_cwd: environment.default_cwd().map(PathBuf::from), + }), + local_environment_config: Some(EnvironmentConfig { + exec_server_url: None, + default_cwd: None, + }), + remote_environment_config: environment.exec_server_url().map(|exec_server_url| { + EnvironmentConfig { + exec_server_url: Some(exec_server_url.to_owned()), + default_cwd: environment.default_cwd().map(PathBuf::from), + } + }), local_runtime_paths: environment.local_runtime_paths().cloned(), disabled: false, current_environment: OnceCell::new(), @@ -96,7 +148,9 @@ impl EnvironmentManager { remote_environment: OnceCell::new(), }, None => Self { - exec_server_url: None, + current_environment_config: None, + local_environment_config: None, + remote_environment_config: None, local_runtime_paths: None, disabled: true, current_environment: OnceCell::new(), @@ -108,12 +162,14 @@ impl EnvironmentManager { /// Returns the remote exec-server URL when one is configured. pub fn exec_server_url(&self) -> Option<&str> { - self.exec_server_url.as_deref() + self.current_environment_config + .as_ref() + .and_then(|config| config.exec_server_url.as_deref()) } /// Returns true when this manager is configured to use a remote exec server. pub fn is_remote(&self) -> bool { - self.exec_server_url.is_some() + self.exec_server_url().is_some() } /// Returns the cached environment, creating it on first access. @@ -123,9 +179,12 @@ impl EnvironmentManager { if self.disabled { Ok(None) } else { + let Some(environment_config) = self.current_environment_config.clone() else { + return Ok(None); + }; Ok(Some(Arc::new( - Environment::create_with_runtime_paths( - self.exec_server_url.clone(), + Environment::create_with_config( + environment_config, self.local_runtime_paths.clone(), ) .await?, @@ -154,11 +213,16 @@ impl EnvironmentManager { "environments are disabled for this session".to_string(), )); } + let Some(environment_config) = self.local_environment_config.clone() else { + return Err(ExecServerError::Protocol( + "local environment is not configured".to_string(), + )); + }; self.local_environment .get_or_try_init(|| async { - Environment::create_with_runtime_paths( - /*exec_server_url*/ None, + Environment::create_with_config( + environment_config, self.local_runtime_paths.clone(), ) .await @@ -174,7 +238,7 @@ impl EnvironmentManager { "environments are disabled for this session".to_string(), )); } - let Some(exec_server_url) = &self.exec_server_url else { + let Some(environment_config) = self.remote_environment_config.clone() else { return Err(ExecServerError::Protocol( "remote environment is not configured".to_string(), )); @@ -182,8 +246,8 @@ impl EnvironmentManager { self.remote_environment .get_or_try_init(|| async { - Environment::create_with_runtime_paths( - Some(exec_server_url.clone()), + Environment::create_with_config( + environment_config, self.local_runtime_paths.clone(), ) .await @@ -201,6 +265,7 @@ impl EnvironmentManager { #[derive(Clone)] pub struct Environment { exec_server_url: Option, + default_cwd: Option, remote_exec_server_client: Option, exec_backend: Arc, local_runtime_paths: Option, @@ -210,6 +275,7 @@ impl Default for Environment { fn default() -> Self { Self { exec_server_url: None, + default_cwd: None, remote_exec_server_client: None, exec_backend: Arc::new(LocalProcess::default()), local_runtime_paths: None, @@ -237,7 +303,22 @@ impl Environment { exec_server_url: Option, local_runtime_paths: Option, ) -> Result { - let (exec_server_url, disabled) = normalize_exec_server_url(exec_server_url); + Self::create_with_config( + EnvironmentConfig { + exec_server_url, + default_cwd: None, + }, + local_runtime_paths, + ) + .await + } + + async fn create_with_config( + environment_config: EnvironmentConfig, + local_runtime_paths: Option, + ) -> Result { + let (exec_server_url, disabled) = + normalize_exec_server_url(environment_config.exec_server_url); if disabled { return Err(ExecServerError::Protocol( "disabled mode does not create an Environment".to_string(), @@ -268,6 +349,7 @@ impl Environment { Ok(Self { exec_server_url, + default_cwd: environment_config.default_cwd, remote_exec_server_client, exec_backend, local_runtime_paths, @@ -287,6 +369,10 @@ impl Environment { self.local_runtime_paths.as_ref() } + pub fn default_cwd(&self) -> Option<&Path> { + self.default_cwd.as_deref() + } + pub fn get_exec_backend(&self) -> Arc { Arc::clone(&self.exec_backend) } @@ -372,6 +458,23 @@ mod tests { assert_eq!(manager.exec_server_url(), Some("ws://127.0.0.1:8765")); } + #[test] + fn environment_manager_carries_remote_default_cwd() { + let manager = EnvironmentManager::new_with_runtime_paths( + Some("ws://127.0.0.1:8765".to_string()), + Some(PathBuf::from("/tmp/devbox")), + /*local_runtime_paths*/ None, + ); + + assert_eq!( + manager + .remote_environment_config + .as_ref() + .and_then(|config| config.default_cwd.as_deref()), + Some(Path::new("/tmp/devbox")) + ); + } + #[tokio::test] async fn environment_manager_current_caches_environment() { let manager = EnvironmentManager::new(/*exec_server_url*/ None); @@ -410,6 +513,31 @@ mod tests { ); } + #[tokio::test] + async fn environment_manager_preserves_default_cwd_from_environment() { + let environment = Environment::create_with_config( + EnvironmentConfig { + exec_server_url: None, + default_cwd: Some(PathBuf::from("/tmp/session")), + }, + /*local_runtime_paths*/ None, + ) + .await + .expect("create environment"); + + let manager = EnvironmentManager::from_environment(Some(&environment)); + + assert_eq!( + manager + .current() + .await + .expect("get current environment") + .expect("local environment") + .default_cwd(), + Some(Path::new("/tmp/session")) + ); + } + #[tokio::test] async fn disabled_environment_manager_has_no_current_environment() { let manager = EnvironmentManager::new(Some("none".to_string()));