diff --git a/codex-rs/app-server/src/command_exec.rs b/codex-rs/app-server/src/command_exec.rs index 2f62f099e9..4274892318 100644 --- a/codex-rs/app-server/src/command_exec.rs +++ b/codex-rs/app-server/src/command_exec.rs @@ -250,6 +250,9 @@ impl CommandExecManager { let (program, args) = command .split_first() .ok_or_else(|| invalid_request("command must not be empty"))?; + let cwd = cwd + .to_abs_path() + .map_err(|err| invalid_request(format!("invalid command cwd: {err}")))?; { let mut sessions = self.sessions.lock().await; if sessions.contains_key(&process_key) { diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index bf9843c53e..6808b05852 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -548,14 +548,32 @@ async fn resolve_turn_environment_selections( let Some(environments) = environments else { return Ok(None); }; + let mut selections = Vec::with_capacity(environments.len()); + for environment in environments { + let environment_id = environment.environment_id; + let convention = environment + .cwd + .infer_absolute_path_convention() + .ok_or_else(|| { + invalid_request(format!( + "invalid cwd for environment `{environment_id}`: path `{}` does not use absolute POSIX or Windows path syntax", + environment.cwd + )) + })?; + let cwd = environment.cwd.to_path_uri(convention).map_err(|err| { + invalid_request(format!( + "invalid cwd for environment `{environment_id}`: {err}" + )) + })?; + selections.push(TurnEnvironmentSelection { + environment_id, + cwd, + }); + } thread_manager - .resolve_native_environment_selections( - environments - .into_iter() - .map(|environment| (environment.environment_id, environment.cwd)), - ) - .map(Some) - .map_err(environment_selection_error) + .validate_environment_selections(&selections) + .map_err(environment_selection_error)?; + Ok(Some(selections)) } fn resolve_runtime_workspace_roots(workspace_roots: Vec) -> Vec { 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 93dec41463..c0946ba679 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -33,7 +33,6 @@ use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR; use codex_protocol::config_types::SERVICE_TIER_DEFAULT_REQUEST_VALUE; use codex_protocol::config_types::TrustLevel; use codex_protocol::openai_models::ReasoningEffort; -use codex_utils_path_uri::ApiPathString; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; @@ -294,7 +293,10 @@ async fn thread_start_rejects_unknown_environment_as_invalid_request() -> Result .send_thread_start_request(ThreadStartParams { environments: Some(vec![TurnEnvironmentParams { environment_id: "missing".to_string(), - cwd: ApiPathString::from_abs_path(&codex_home.path().to_path_buf().try_into()?), + cwd: codex_utils_absolute_path::AbsolutePathBuf::try_from( + codex_home.path().to_path_buf(), + )? + .into(), }]), ..Default::default() }) diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 22c5269ac3..48c8f5cd23 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -76,7 +76,7 @@ use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS; use codex_protocol::models::ImageDetail; use codex_protocol::openai_models::ReasoningEffort; use codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS; -use codex_utils_path_uri::ApiPathString; +use codex_utils_absolute_path::AbsolutePathBuf; use core_test_support::responses; use core_test_support::skip_if_no_network; use pretty_assertions::assert_eq; @@ -1327,7 +1327,10 @@ async fn turn_start_rejects_unknown_environment_before_starting_turn() -> Result }], environments: Some(vec![TurnEnvironmentParams { environment_id: "missing".to_string(), - cwd: ApiPathString::from_abs_path(&codex_home.path().to_path_buf().try_into()?), + cwd: codex_utils_absolute_path::AbsolutePathBuf::try_from( + codex_home.path().to_path_buf(), + )? + .into(), }]), ..Default::default() }) @@ -2745,7 +2748,7 @@ fn environment_params( .map(|id| { Ok(TurnEnvironmentParams { environment_id: (*id).to_string(), - cwd: ApiPathString::from_abs_path(&cwd.to_path_buf().try_into()?), + cwd: AbsolutePathBuf::try_from(cwd.to_path_buf())?.into(), }) }) .collect() diff --git a/codex-rs/core/src/agents_md.rs b/codex-rs/core/src/agents_md.rs index 5cb70dc5fc..861e7b4ac7 100644 --- a/codex-rs/core/src/agents_md.rs +++ b/codex-rs/core/src/agents_md.rs @@ -53,11 +53,16 @@ pub(crate) async fn load_project_instructions( let mut loaded = LoadedAgentsMd::from_user_instructions(user_instructions); for turn_environment in &environments.turn_environments { let filesystem = turn_environment.environment.get_filesystem(); + // TODO(anp): Migrate AGENTS.md discovery to PathUri so instructions can be loaded from + // environment-native foreign working directories. + let Ok(cwd) = turn_environment.cwd().to_abs_path() else { + continue; + }; match read_agents_md( config, filesystem.as_ref(), &turn_environment.environment_id, - turn_environment.cwd(), + &cwd, ) .await { diff --git a/codex-rs/core/src/agents_md_tests.rs b/codex-rs/core/src/agents_md_tests.rs index 1138f84861..104293a059 100644 --- a/codex-rs/core/src/agents_md_tests.rs +++ b/codex-rs/core/src/agents_md_tests.rs @@ -256,7 +256,6 @@ fn resolved_local_environments( PathUri::from_abs_path(&cwd), /*shell*/ None, ) - .expect("turn environment") }) .collect(), } diff --git a/codex-rs/core/src/context/environment_context.rs b/codex-rs/core/src/context/environment_context.rs index 8aa07d1ed5..f0cc9f80bc 100644 --- a/codex-rs/core/src/context/environment_context.rs +++ b/codex-rs/core/src/context/environment_context.rs @@ -44,14 +44,18 @@ impl EnvironmentContextEnvironment { fn from_turn_environments(environments: &[TurnEnvironment], shell: &Shell) -> Vec { environments .iter() - .map(|environment| Self { - id: environment.environment_id.clone(), - cwd: environment.cwd().clone(), - shell: environment - .shell - .as_ref() - .map(|shell| shell.name().to_string()) - .unwrap_or_else(|| shell.name().to_string()), + .filter_map(|environment| { + // TODO(anp): Migrate EnvironmentContextEnvironment to PathUri so foreign + // environments remain visible in model context. + Some(Self { + id: environment.environment_id.clone(), + cwd: environment.cwd().to_abs_path().ok()?, + shell: environment + .shell + .as_ref() + .map(|shell| shell.name().to_string()) + .unwrap_or_else(|| shell.name().to_string()), + }) }) .collect() } diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index c1a5edd3ad..16b2c47b5d 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -97,7 +97,7 @@ impl ThreadEnvironments { } let turn_environment = match current.turn_environments.iter().find(|environment| { environment.environment_id == selected_environment.environment_id - && environment.cwd_uri() == &selected_environment.cwd + && environment.cwd() == &selected_environment.cwd }) { Some(environment) => environment.clone(), None => match Self::resolve_selection( @@ -159,13 +159,7 @@ impl ThreadEnvironments { environment, selected_environment.cwd.clone(), shell, - ) - .map_err(|err| { - CodexErr::InvalidRequest(format!( - "turn environment cwd `{}` is not valid on this host: {err}", - selected_environment.cwd - )) - })?; + ); let task = shell_snapshot .clone() .build(turn_environment.clone()) @@ -227,8 +221,10 @@ impl TurnEnvironmentSnapshot { (!environment.environment.is_remote()).then_some(environment) } - pub(crate) fn single_local_environment_cwd(&self) -> Option<&AbsolutePathBuf> { - self.single_local_environment().map(TurnEnvironment::cwd) + pub(crate) fn single_local_environment_cwd(&self) -> Option { + // TODO(anp): Migrate local-environment consumers to PathUri so this compatibility + // conversion can be removed. + self.single_local_environment()?.cwd().to_abs_path().ok() } } @@ -571,15 +567,12 @@ url = "ws://127.0.0.1:8765" .expect("remote environment"), ); let remote = TurnEnvironmentSnapshot { - turn_environments: vec![ - TurnEnvironment::new( - REMOTE_ENVIRONMENT_ID.to_string(), - remote_environment.clone(), - cwd_uri.clone(), - /*shell*/ None, - ) - .expect("remote turn environment"), - ], + turn_environments: vec![TurnEnvironment::new( + REMOTE_ENVIRONMENT_ID.to_string(), + remote_environment.clone(), + cwd_uri.clone(), + /*shell*/ None, + )], }; let multiple = TurnEnvironmentSnapshot { turn_environments: vec![ @@ -589,12 +582,11 @@ url = "ws://127.0.0.1:8765" remote_environment, cwd_uri, /*shell*/ None, - ) - .expect("remote turn environment"), + ), ], }; - assert_eq!(local.single_local_environment_cwd(), Some(&cwd)); + assert_eq!(local.single_local_environment_cwd(), Some(cwd)); assert_eq!(remote.single_local_environment_cwd(), None); assert_eq!(multiple.single_local_environment_cwd(), None); } diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 9468534d8f..be47d195f8 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -461,6 +461,7 @@ pub(crate) async fn execute_exec_request( windows_sandbox_filesystem_overrides, arg0, } = exec_request; + let cwd = cwd.to_abs_path()?; let params = ExecParams { command, diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 9d96a2d60b..7aa007891f 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -750,11 +750,13 @@ async fn run_review_on_session( .unwrap_or_default(); let guardian_permission_profile = PermissionProfile::read_only(); let parent_turn_environments = params.parent_turn.environments.to_selections(); + // TODO(anp): Migrate guardian review thread settings to a PathUri fallback cwd so foreign + // parent environments do not fall back to the host-native config cwd. let parent_turn_legacy_fallback_cwd = params .parent_turn .environments .primary() - .map(|environment| environment.cwd().clone()) + .and_then(|environment| environment.cwd().to_abs_path().ok()) .unwrap_or_else(|| params.parent_turn.config.cwd.clone()); let submit_result = run_before_review_deadline( diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index f1f53d80ef..259fd7031e 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -25,6 +25,7 @@ use codex_protocol::permissions::NetworkSandboxPolicy; use codex_sandboxing::SandboxExecRequest; use codex_sandboxing::SandboxType; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use std::collections::HashMap; #[derive(Debug)] @@ -42,7 +43,7 @@ pub(crate) struct ExecServerEnvConfig { #[derive(Debug)] pub struct ExecRequest { pub command: Vec, - pub cwd: AbsolutePathBuf, + pub cwd: PathUri, pub env: HashMap, pub(crate) exec_server_env_config: Option, pub network: Option, @@ -77,6 +78,7 @@ impl ExecRequest { arg0: Option, ) -> Self { let windows_sandbox_policy_cwd = cwd.clone(); + let cwd = PathUri::from_abs_path(&cwd); let (file_system_sandbox_policy, network_sandbox_policy) = permission_profile.to_runtime_permissions(); Self { @@ -135,7 +137,7 @@ impl ExecRequest { } Self { command, - cwd, + cwd: PathUri::from_abs_path(&cwd), env, exec_server_env_config: None, network, diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index cc6e42f852..d1b8982223 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -318,17 +318,18 @@ impl Session { ) .await; let environment_manager = self.services.turn_environments.environment_manager(); - let mcp_runtime_context = match turn_context.environments.primary() { - Some(turn_environment) => McpRuntimeContext::new( - Arc::clone(&environment_manager), - turn_environment.cwd().to_path_buf(), - ), - None => McpRuntimeContext::new( - environment_manager, + // TODO(anp): Migrate MCP runtime cwd plumbing to PathUri so foreign environment cwd + // values can be used without falling back to the legacy host cwd. + let cwd = turn_context + .environments + .primary() + .and_then(|turn_environment| turn_environment.cwd().to_abs_path().ok()) + .map(|cwd| cwd.to_path_buf()) + .unwrap_or_else(|| { #[allow(deprecated)] - turn_context.cwd.to_path_buf(), - ), - }; + turn_context.cwd.to_path_buf() + }); + let mcp_runtime_context = McpRuntimeContext::new(environment_manager, cwd); let mcp_startup_cancellation_token = { let mut guard = self.services.mcp_startup_cancellation_token.lock().await; guard.cancel(); diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 46f20eaf53..578152814b 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -1119,9 +1119,12 @@ impl Session { }; let mcp_runtime_context = { let turn_environments = sess.services.turn_environments.snapshot().await; + // TODO(anp): Migrate MCP runtime cwd plumbing to PathUri so foreign environment + // cwd values can be used without falling back to the session host cwd. let cwd = turn_environments .primary() - .map(|turn_environment| turn_environment.cwd().to_path_buf()) + .and_then(|turn_environment| turn_environment.cwd().to_abs_path().ok()) + .map(|cwd| cwd.to_path_buf()) .unwrap_or_else(|| session_configuration.cwd().to_path_buf()); McpRuntimeContext::new( sess.services.turn_environments.environment_manager(), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index dec6feceab..664377c4ce 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -979,8 +979,11 @@ async fn danger_full_access_tool_attempts_do_not_enforce_managed_network() -> an &'a mut self, _req: &'a (), _ctx: crate::tools::sandboxing::ApprovalCtx<'a>, - ) -> futures::future::BoxFuture<'a, ReviewDecision> { - Box::pin(async { ReviewDecision::Approved }) + ) -> futures::future::BoxFuture< + 'a, + Result, + > { + Box::pin(async { Ok(ReviewDecision::Approved) }) } } @@ -5697,10 +5700,9 @@ async fn request_permissions_tool_resolves_relative_paths_against_selected_envir turn_context_mut.environments.turn_environments[0] = TurnEnvironment::new( "remote".to_string(), current_environment.environment, - codex_utils_path_uri::PathUri::from_abs_path(&environment_cwd), + PathUri::from_abs_path(&environment_cwd), current_environment.shell, - ) - .expect("remote turn environment"); + ); let call_id = "call-1".to_string(); let handler = RequestPermissionsHandler; @@ -6317,15 +6319,16 @@ async fn primary_environment_uses_first_turn_environment() { let first_environment = turn_context.environments.turn_environments[0].clone(); #[allow(deprecated)] let second_cwd = turn_context.cwd.join("second"); - turn_context.environments.turn_environments.push( - TurnEnvironment::new( + let second_cwd_uri = codex_utils_path_uri::PathUri::from_abs_path(&second_cwd); + turn_context + .environments + .turn_environments + .push(TurnEnvironment::new( "second".to_string(), Arc::clone(&first_environment.environment), - codex_utils_path_uri::PathUri::from_abs_path(&second_cwd), + second_cwd_uri.clone(), /*shell*/ None, - ) - .expect("second turn environment"), - ); + )); assert_eq!( turn_context @@ -6343,12 +6346,12 @@ async fn primary_environment_uses_first_turn_environment() { .find(|environment| environment.environment_id == "second") .expect("second environment") .cwd(), - &second_cwd + &second_cwd_uri ); assert_eq!(turn_context.environments.turn_environments.len(), 2); assert_eq!( turn_context.environments.turn_environments[1].cwd(), - &second_cwd + &second_cwd_uri ); } diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 3c783eb486..a105aeffee 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -414,13 +414,16 @@ pub(crate) async fn run_turn( async fn turn_diff_display_roots(turn_context: &TurnContext) -> Vec<(String, PathBuf)> { let mut display_roots = Vec::new(); for turn_environment in &turn_context.environments.turn_environments { - let root = get_git_repo_root_with_fs( - turn_environment.environment.get_filesystem().as_ref(), - turn_environment.cwd(), - ) - .await - .unwrap_or_else(|| turn_environment.cwd().clone()) - .into_path_buf(); + // TODO(anp): Migrate git-root discovery and diff display roots to PathUri so foreign + // environment roots can participate without host-native conversion. + let Ok(cwd) = turn_environment.cwd().to_abs_path() else { + continue; + }; + let root = + get_git_repo_root_with_fs(turn_environment.environment.get_filesystem().as_ref(), &cwd) + .await + .unwrap_or(cwd) + .into_path_buf(); display_roots.push((turn_environment.environment_id.clone(), root)); } display_roots @@ -634,10 +637,14 @@ async fn build_extension_turn_input_items( .turn_environments .iter() .enumerate() - .map(|(index, environment)| TurnInputEnvironment { - environment_id: environment.environment_id.clone(), - cwd: environment.cwd().as_path().to_path_buf(), - is_primary: index == 0, + .filter_map(|(index, environment)| { + // TODO(anp): Migrate extension turn-input environments to PathUri so foreign cwd + // values are not omitted from extension context. + Some(TurnInputEnvironment { + environment_id: environment.environment_id.clone(), + cwd: environment.cwd().to_abs_path().ok()?.into_path_buf(), + is_primary: index == 0, + }) }) .collect::>(); diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 8df2a7514e..1e5ce3f8af 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -3,7 +3,6 @@ use crate::SkillLoadOutcome; use crate::agents_md::LoadedAgentsMd; use crate::config::GhostSnapshotConfig; use crate::environment_selection::TurnEnvironmentSnapshot; -use crate::path_utils; use crate::shell_snapshot::ShellSnapshotFile; use codex_core_skills::HostLoadedSkills; use codex_file_system::FileSystemSandboxContext; @@ -49,13 +48,7 @@ pub(crate) type ShellSnapshotTask = Shared, - // Keep both representations together while cwd consumers migrate to URI semantics. Keeping - // them synchronized means neither representation can be exposed through a mutable reference; - // updates must rebuild the validated pair through `TurnEnvironment::new`. Once - // `TurnEnvironment::cwd` itself becomes a `PathUri`, convert only at native filesystem and - // process-launch boundaries and remove this paired migration state. - cwd: AbsolutePathBuf, - cwd_uri: PathUri, + cwd: PathUri, pub(crate) shell: Option, pub(crate) shell_snapshot: ShellSnapshotTask, } @@ -64,22 +57,20 @@ impl TurnEnvironment { pub(crate) fn new( environment_id: String, environment: Arc, - cwd_uri: PathUri, + cwd: PathUri, shell: Option, - ) -> std::io::Result { - let cwd = cwd_uri.to_abs_path()?; - Ok(Self { + ) -> Self { + Self { environment_id, environment, cwd, - cwd_uri, shell, shell_snapshot: futures::future::ready(None).boxed().shared(), - }) + } } - pub(crate) fn shell_snapshot(&self, cwd: &AbsolutePathBuf) -> Option { - if !path_utils::paths_match_after_normalization(self.cwd.as_path(), cwd.as_path()) { + pub(crate) fn shell_snapshot(&self, cwd: &PathUri) -> Option { + if &self.cwd != cwd { return None; } self.shell_snapshot @@ -88,18 +79,14 @@ impl TurnEnvironment { .map(ShellSnapshotFile::path) } - pub(crate) fn cwd(&self) -> &AbsolutePathBuf { + pub(crate) fn cwd(&self) -> &PathUri { &self.cwd } - pub(crate) fn cwd_uri(&self) -> &PathUri { - &self.cwd_uri - } - pub(crate) fn selection(&self) -> TurnEnvironmentSelection { TurnEnvironmentSelection { environment_id: self.environment_id.clone(), - cwd: self.cwd_uri.clone(), + cwd: self.cwd.clone(), } } } @@ -110,7 +97,6 @@ impl std::fmt::Debug for TurnEnvironment { .field("environment_id", &self.environment_id) .field("environment", &self.environment) .field("cwd", &self.cwd) - .field("cwd_uri", &self.cwd_uri) .field("shell", &self.shell) .finish_non_exhaustive() } @@ -755,9 +741,11 @@ impl Session { ) -> Arc { let turn_environments = self.services.turn_environments.snapshot().await; let primary_turn_environment = turn_environments.primary().cloned(); + // TODO(anp): Migrate per-turn config and legacy TurnContext cwd consumers to PathUri so + // a foreign primary environment does not fall back to the session's host cwd. let cwd = primary_turn_environment .as_ref() - .map(|turn_environment| turn_environment.cwd().clone()) + .and_then(|turn_environment| turn_environment.cwd().to_abs_path().ok()) .unwrap_or_else(|| session_configuration.cwd().clone()); let per_turn_config = Self::build_per_turn_config(&session_configuration, cwd.clone()); { diff --git a/codex-rs/core/src/shell_snapshot.rs b/codex-rs/core/src/shell_snapshot.rs index 6e6c6cd515..6a95093003 100644 --- a/codex-rs/core/src/shell_snapshot.rs +++ b/codex-rs/core/src/shell_snapshot.rs @@ -76,7 +76,9 @@ impl ShellSnapshot { } let shell = environment.shell.clone()?; - let cwd = environment.cwd().clone(); + // TODO(anp): Migrate shell snapshot creation to accept PathUri and defer native + // conversion to the spawned shell process. + let cwd = environment.cwd().to_abs_path().ok()?; Self::build_for_cwd(Arc::clone(config), cwd, shell).await } diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index 6a01a87a0e..c3abf03e74 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -162,7 +162,17 @@ pub(crate) async fn execute_user_shell_command( let call_id = Uuid::new_v4().to_string(); let raw_command = command; - let cwd = turn_environment.cwd().clone(); + // TODO(anp): Migrate user-shell events and execution plumbing to PathUri so this local-only + // feature does not need to project the selected environment cwd onto the Codex host. + let Ok(cwd) = turn_environment.cwd().to_abs_path() else { + send_user_shell_error( + &session, + turn_context.as_ref(), + "shell working directory is not native to the Codex host", + ) + .await; + return; + }; let parsed_cmd = parse_command(&display_command); session @@ -185,7 +195,7 @@ pub(crate) async fn execute_user_shell_command( let permission_profile = PermissionProfile::Disabled; let exec_env = ExecRequest { command: exec_command.clone(), - cwd: cwd.clone(), + cwd: turn_environment.cwd().clone(), env: exec_env_map, exec_server_env_config: None, // `/shell` is the explicit full-access escape hatch, so it must not diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 091bbe3edf..2adb713a0e 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -66,7 +66,6 @@ use codex_thread_store::ThreadStore; use codex_thread_store::ThreadStoreError; use codex_thread_store::UpdateThreadMetadataParams; use codex_utils_absolute_path::AbsolutePathBuf; -use codex_utils_path_uri::ApiPathString; use futures::StreamExt; use futures::stream::FuturesUnordered; use std::collections::HashMap; @@ -473,33 +472,6 @@ impl ThreadManager { Ok(()) } - /// Resolves environment-native cwd strings without applying the Codex host's path rules. - pub fn resolve_native_environment_selections( - &self, - environments: impl IntoIterator, - ) -> CodexResult> { - let environments = environments.into_iter(); - let mut selections = Vec::with_capacity(environments.size_hint().0); - for (environment_id, cwd) in environments { - let convention = cwd.infer_absolute_path_convention().ok_or_else(|| { - CodexErr::InvalidRequest(format!( - "invalid cwd for environment `{environment_id}`: path `{cwd}` does not use absolute POSIX or Windows path syntax" - )) - })?; - let cwd = cwd.to_path_uri(convention).map_err(|err| { - CodexErr::InvalidRequest(format!( - "invalid cwd for environment `{environment_id}`: {err}" - )) - })?; - selections.push(TurnEnvironmentSelection { - environment_id, - cwd, - }); - } - self.validate_environment_selections(&selections)?; - Ok(selections) - } - pub fn get_models_manager(&self) -> SharedModelsManager { self.state.models_manager.clone() } diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 437575a41e..cec54b5e44 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -23,7 +23,6 @@ use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::ThreadSource; use codex_protocol::protocol::TurnStartedEvent; use codex_protocol::protocol::UserMessageEvent; -use codex_utils_path_uri::ApiPathString; use codex_utils_path_uri::PathUri; use core_test_support::PathBufExt; use core_test_support::PathExt; @@ -299,126 +298,6 @@ async fn shutdown_all_threads_bounded_submits_shutdown_to_every_thread() { assert!(manager.list_thread_ids().await.is_empty()); } -#[tokio::test] -async fn start_thread_rejects_explicit_local_environment_when_default_provider_is_disabled() { - let temp_dir = tempdir().expect("tempdir"); - let mut config = test_config().await; - config.codex_home = temp_dir.path().join("codex-home").abs(); - config.cwd = config.codex_home.abs(); - std::fs::create_dir_all(&config.codex_home).expect("create codex home"); - - let runtime_paths = codex_exec_server::ExecServerRuntimePaths::new( - std::env::current_exe().expect("current exe path"), - /*codex_linux_sandbox_exe*/ None, - ) - .expect("runtime paths"); - let environment_manager = Arc::new( - codex_exec_server::EnvironmentManager::create_for_tests( - Some("none".to_string()), - Some(runtime_paths), - ) - .await, - ); - let manager = ThreadManager::with_models_provider_and_home_for_tests( - CodexAuth::from_api_key("dummy"), - config.model_provider.clone(), - config.codex_home.to_path_buf(), - environment_manager, - ); - - let result = manager - .start_thread_with_options(StartThreadOptions { - config: config.clone(), - initial_history: InitialHistory::New, - session_source: None, - thread_source: None, - dynamic_tools: Vec::new(), - metrics_service_name: None, - parent_trace: None, - environments: vec![TurnEnvironmentSelection { - environment_id: "local".to_string(), - cwd: PathUri::from_abs_path(&config.cwd), - }], - thread_extension_init: Default::default(), - }) - .await; - let err = match result { - Ok(_) => panic!("explicit local environment should not resolve when provider is disabled"), - Err(err) => err, - }; - - assert_eq!(err.to_string(), "unknown turn environment id `local`"); - assert!(manager.list_thread_ids().await.is_empty()); -} - -#[tokio::test] -async fn native_environment_selections_preserve_selected_path_syntax() { - let temp_dir = tempdir().expect("tempdir"); - let mut config = test_config().await; - config.codex_home = temp_dir.path().join("codex-home").abs(); - config.cwd = temp_dir.path().join("workspace").abs(); - std::fs::create_dir_all(&config.codex_home).expect("create codex home"); - std::fs::create_dir_all(&config.cwd).expect("create workspace"); - - let manager = ThreadManager::with_models_provider_and_home_for_tests( - CodexAuth::from_api_key("dummy"), - config.model_provider.clone(), - config.codex_home.to_path_buf(), - Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), - ); - let selections = manager - .resolve_native_environment_selections([( - "local".to_string(), - ApiPathString::from_abs_path(&config.cwd), - )]) - .expect("resolve local selection"); - - assert_eq!( - selections, - vec![TurnEnvironmentSelection { - environment_id: "local".to_string(), - cwd: PathUri::from_abs_path(&config.cwd), - }] - ); -} - -#[tokio::test] -async fn native_environment_selections_reject_duplicate_ids() { - let temp_dir = tempdir().expect("tempdir"); - let mut config = test_config().await; - config.codex_home = temp_dir.path().join("codex-home").abs(); - config.cwd = temp_dir.path().join("workspace").abs(); - std::fs::create_dir_all(&config.codex_home).expect("create codex home"); - - let manager = ThreadManager::with_models_provider_and_home_for_tests( - CodexAuth::from_api_key("dummy"), - config.model_provider.clone(), - config.codex_home.to_path_buf(), - Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), - ); - let native_cwd = ApiPathString::from_abs_path(&config.cwd); - let error = manager - .resolve_native_environment_selections([ - ("local".to_string(), native_cwd.clone()), - ("local".to_string(), native_cwd), - ]) - .expect_err("duplicate ids must fail"); - - assert_eq!(error.to_string(), "duplicate turn environment id `local`"); - - let error = manager - .resolve_native_environment_selections([( - "local".to_string(), - serde_json::from_value(serde_json::json!("relative")) - .expect("API path should deserialize"), - )]) - .expect_err("relative cwd must fail at the environment boundary"); - assert_eq!( - error.to_string(), - "invalid cwd for environment `local`: path `relative` does not use absolute POSIX or Windows path syntax" - ); -} - #[tokio::test] async fn start_thread_keeps_internal_threads_hidden_from_normal_lookups() { let temp_dir = tempdir().expect("tempdir"); @@ -727,11 +606,11 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { assert_eq!(resumed_turn.environments.turn_environments.len(), 1); assert_eq!( resumed_turn.environments.turn_environments[0].cwd(), - &default_cwd + &PathUri::from_abs_path(&default_cwd) ); assert_ne!( resumed_turn.environments.turn_environments[0].cwd(), - &selected_cwd + &PathUri::from_abs_path(&selected_cwd) ); let forked = manager @@ -754,11 +633,11 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { assert_eq!(forked_turn.environments.turn_environments.len(), 1); assert_eq!( forked_turn.environments.turn_environments[0].cwd(), - &default_cwd + &PathUri::from_abs_path(&default_cwd) ); assert_ne!( forked_turn.environments.turn_environments[0].cwd(), - &selected_cwd + &PathUri::from_abs_path(&selected_cwd) ); } diff --git a/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs b/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs index 2556989621..a519e5db1b 100644 --- a/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs +++ b/codex-rs/core/src/tools/handlers/agent_jobs/spawn_agents_on_csv.rs @@ -299,7 +299,7 @@ pub async fn handle( Ok(FunctionToolOutput::from_text(content, Some(true))) } -fn single_local_environment_cwd(turn: &TurnContext) -> Result<&AbsolutePathBuf, FunctionCallError> { +fn single_local_environment_cwd(turn: &TurnContext) -> Result { let [turn_environment] = turn.environments.turn_environments.as_slice() else { return Err(FunctionCallError::RespondToModel( "spawn_agents_on_csv requires exactly one local environment".to_string(), @@ -312,5 +312,12 @@ fn single_local_environment_cwd(turn: &TurnContext) -> Result<&AbsolutePathBuf, )); } - Ok(turn_environment.cwd()) + // TODO(anp): Migrate spawn_agents_on_csv filesystem access to PathUri before enabling it for + // remote environments. + turn_environment.cwd().to_abs_path().map_err(|err| { + FunctionCallError::RespondToModel(format!( + "spawn_agents_on_csv cwd `{}` is not native to the Codex host: {err}", + turn_environment.cwd() + )) + }) } diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index a719e9acd9..2ce9119e35 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -359,11 +359,18 @@ impl ApplyPatchHandler { "apply_patch is unavailable in this session".to_string(), )); }; - let cwd = turn_environment.cwd().clone(); + // TODO(anp): Migrate apply-patch verification and permission accounting to PathUri so + // patches can target environment-native foreign paths without host projection. + let cwd = turn_environment.cwd().to_abs_path().map_err(|err| { + FunctionCallError::RespondToModel(format!( + "apply_patch cwd `{}` is not native to the Codex host: {err}", + turn_environment.cwd() + )) + })?; let fs = turn_environment.environment.get_filesystem(); let sandbox = turn.file_system_sandbox_context( /*additional_permissions*/ None, - turn_environment.cwd_uri(), + turn_environment.cwd(), ); match codex_apply_patch::verify_apply_patch_args(args, &cwd, fs.as_ref(), Some(&sandbox)) .await diff --git a/codex-rs/core/src/tools/handlers/extension_tools.rs b/codex-rs/core/src/tools/handlers/extension_tools.rs index f0522c7c17..b59bfaec02 100644 --- a/codex-rs/core/src/tools/handlers/extension_tools.rs +++ b/codex-rs/core/src/tools/handlers/extension_tools.rs @@ -114,10 +114,15 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall { ConversationHistory::new(invocation.session.clone_history().await.into_raw_items()); let mut environments = Vec::with_capacity(invocation.turn.environments.turn_environments.len()); for environment in &invocation.turn.environments.turn_environments { + // TODO(anp): Migrate extension ToolEnvironment and granted-permission lookup to PathUri + // so extensions can receive foreign environment cwd values. + let Ok(native_cwd) = environment.cwd().to_abs_path() else { + continue; + }; let additional_permissions = apply_granted_turn_permissions( invocation.session.as_ref(), &environment.environment_id, - environment.cwd().as_path(), + native_cwd.as_path(), SandboxPermissions::UseDefault, /*additional_permissions*/ None, ) @@ -125,10 +130,10 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall { .additional_permissions; let file_system_sandbox_context = invocation .turn - .file_system_sandbox_context(additional_permissions, environment.cwd_uri()); + .file_system_sandbox_context(additional_permissions, environment.cwd()); environments.push(ToolEnvironment { environment_id: environment.environment_id.clone(), - cwd: environment.cwd().clone(), + cwd: native_cwd, file_system: environment.environment.get_filesystem(), file_system_sandbox_context, }); @@ -315,7 +320,7 @@ mod tests { .environments .turn_environments .iter() - .map(|environment| Some(environment.cwd_uri().clone())) + .map(|environment| Some(environment.cwd().clone())) .collect::>(); let history_item = ResponseItem::Message { id: None, diff --git a/codex-rs/core/src/tools/handlers/request_permissions.rs b/codex-rs/core/src/tools/handlers/request_permissions.rs index 53341b14f5..afc15f9659 100644 --- a/codex-rs/core/src/tools/handlers/request_permissions.rs +++ b/codex-rs/core/src/tools/handlers/request_permissions.rs @@ -70,8 +70,16 @@ impl RequestPermissionsHandler { "request_permissions requires a primary environment".to_string(), )); }; + // TODO(anp): Migrate request_permissions parsing and permission profiles to PathUri so + // environment-native foreign paths do not require host conversion. + let native_cwd = turn_environment.cwd().to_abs_path().map_err(|err| { + FunctionCallError::RespondToModel(format!( + "request_permissions cwd `{}` is not native to the Codex host: {err}", + turn_environment.cwd() + )) + })?; let mut args: RequestPermissionsArgs = - parse_arguments_with_base_path(&arguments, turn_environment.cwd())?; + parse_arguments_with_base_path(&arguments, &native_cwd)?; args.permissions = normalize_additional_permissions(args.permissions.into()) .map(codex_protocol::request_permissions::RequestPermissionProfile::from) .map_err(FunctionCallError::RespondToModel)?; diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 71e776dbf0..35e42374cd 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -10,6 +10,7 @@ use crate::tools::registry::PostToolUsePayload; use codex_exec_server::Environment; use codex_protocol::models::AdditionalPermissionProfile; use codex_tools::UnifiedExecShellMode; +use codex_utils_path_uri::ApiPathString; use serde::Deserialize; use std::path::PathBuf; use std::sync::Arc; @@ -54,7 +55,7 @@ struct ExecCommandEnvironmentArgs { // Keep this raw until after environment selection; relative paths must be // resolved against the selected environment cwd, not the process cwd. #[serde(default)] - workdir: Option, + workdir: Option, } fn default_exec_yield_time_ms() -> u64 { diff --git a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs index eb89e56ef2..1d7d64e2f5 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec/exec_command.rs @@ -11,7 +11,6 @@ use crate::tools::handlers::apply_patch::intercept_apply_patch; use crate::tools::handlers::implicit_granted_permissions; use crate::tools::handlers::normalize_and_validate_additional_permissions; use crate::tools::handlers::parse_arguments; -use crate::tools::handlers::parse_arguments_with_base_path; use crate::tools::handlers::resolve_tool_environment; use crate::tools::handlers::rewrite_function_string_argument; use crate::tools::handlers::updated_hook_command; @@ -131,21 +130,26 @@ impl ExecCommandHandler { }; let cwd = environment_args .workdir - .as_deref() + .as_ref() .filter(|workdir| !workdir.is_empty()) - .map_or_else( - || turn_environment.cwd().clone(), - |workdir| turn_environment.cwd().join(workdir), - ); + .and_then(|workdir| turn_environment.cwd().join(workdir.as_str()).ok()) + .unwrap_or_else(|| turn_environment.cwd().clone()); + // TODO(anp): Migrate command approvals, granted permissions, and tool hooks to PathUri so + // foreign command working directories do not require a host-native projection here. + let native_cwd = cwd.to_abs_path().map_err(|err| { + FunctionCallError::RespondToModel(format!( + "command workdir `{cwd}` is not native to the Codex host: {err}" + )) + })?; let environment = Arc::clone(&turn_environment.environment); let fs = environment.get_filesystem(); - let args: ExecCommandArgs = parse_arguments_with_base_path(&arguments, &cwd)?; + let args: ExecCommandArgs = parse_arguments(&arguments)?; let hook_command = args.cmd.clone(); maybe_emit_implicit_skill_invocation( session.as_ref(), context.turn.as_ref(), &hook_command, - &cwd, + &native_cwd, ) .await; let process_id = manager.allocate_process_id().await; @@ -186,7 +190,7 @@ impl ExecCommandHandler { let effective_additional_permissions = apply_granted_turn_permissions( context.session.as_ref(), &turn_environment.environment_id, - cwd.as_path(), + native_cwd.as_path(), sandbox_permissions, additional_permissions, ) @@ -226,7 +230,7 @@ impl ExecCommandHandler { effective_additional_permissions.sandbox_permissions, effective_additional_permissions.additional_permissions, effective_additional_permissions.permissions_preapproved, - &cwd, + native_cwd.as_path(), ) }, |permissions| Ok(Some(permissions)), @@ -240,7 +244,7 @@ impl ExecCommandHandler { if let Some(output) = intercept_apply_patch( &command, - &cwd, + &native_cwd, fs.as_ref(), turn_environment.clone(), context.session.clone(), @@ -277,7 +281,14 @@ impl ExecCommandHandler { yield_time_ms, max_output_tokens, cwd, - sandbox_cwd: turn_environment.cwd().clone(), + // TODO(anp): Migrate the sandbox orchestration traits to PathUri so the + // selected environment root can remain foreign until sandbox enforcement. + sandbox_cwd: turn_environment.cwd().to_abs_path().map_err(|err| { + FunctionCallError::RespondToModel(format!( + "environment cwd `{}` is not native to the Codex host: {err}", + turn_environment.cwd() + )) + })?, turn_environment: turn_environment.clone(), shell_mode, network: context.turn.network.clone(), diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index f2e16f0f58..6a9c61069e 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -11,6 +11,7 @@ use codex_protocol::openai_models::InputModality; use codex_utils_image::PromptImageMode; use codex_utils_image::data_url_from_bytes; use codex_utils_image::load_for_prompt_bytes; +use codex_utils_path_uri::ApiPathString; use serde::Deserialize; use crate::function_tool::FunctionCallError; @@ -27,7 +28,6 @@ use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; use codex_tools::ToolName; use codex_tools::ToolSpec; -use codex_utils_path_uri::PathUri; pub struct ViewImageHandler { options: ViewImageToolOptions, @@ -55,7 +55,7 @@ const VIEW_IMAGE_UNSUPPORTED_MESSAGE: &str = #[derive(Deserialize)] struct ViewImageArgs { - path: String, + path: ApiPathString, #[serde(default)] environment_id: Option, detail: Option, @@ -143,29 +143,40 @@ impl ViewImageHandler { "view_image is unavailable in this session".to_string(), )); }; - let cwd = turn_environment.cwd().clone(); - let abs_path = cwd.join(path); + // Image paths may be absolute using the selected environment's native syntax or relative + // to its cwd. Resolve either form to a PathUri without parsing it as a Codex-host path. + let path_uri = match path.infer_absolute_path_convention() { + Some(convention) => path.to_path_uri(convention).map_err(|err| err.to_string()), + None => turn_environment + .cwd() + .join(path.as_str()) + .map_err(|err| err.to_string()), + } + .map_err(|err| { + FunctionCallError::RespondToModel(format!("invalid image path `{path}`: {err}")) + })?; + // Preserve the existing native error spelling for local paths while still rendering a + // meaningful URI when the selected environment path is foreign to the Codex host. + let display_path = path_uri + .to_abs_path() + .map_or_else(|_| path_uri.to_string(), |path| path.display().to_string()); let sandbox = turn.file_system_sandbox_context( /*additional_permissions*/ None, - turn_environment.cwd_uri(), + turn_environment.cwd(), ); let fs = turn_environment.environment.get_filesystem(); - let path_uri = PathUri::from_abs_path(&abs_path); - let metadata = fs .get_metadata(&path_uri, Some(&sandbox)) .await .map_err(|error| { FunctionCallError::RespondToModel(format!( - "unable to locate image at `{}`: {error}", - abs_path.display() + "unable to locate image at `{display_path}`: {error}" )) })?; if !metadata.is_file { return Err(FunctionCallError::RespondToModel(format!( - "image path `{}` is not a file", - abs_path.display() + "image path `{display_path}` is not a file" ))); } let file_bytes = fs @@ -173,11 +184,17 @@ impl ViewImageHandler { .await .map_err(|error| { FunctionCallError::RespondToModel(format!( - "unable to read image at `{}`: {error}", - abs_path.display() + "unable to read image at `{display_path}`: {error}" )) })?; - let event_path = abs_path.clone(); + // TODO(anp): Migrate image-view events and image decoding hints to PathUri so foreign + // environment paths do not require a host-native projection after remote file reads. + let native_path = path_uri.to_abs_path().map_err(|err| { + FunctionCallError::RespondToModel(format!( + "image path `{path_uri}` is not native to the Codex host: {err}" + )) + })?; + let event_path = native_path.clone(); let can_request_original_detail = can_request_original_image_detail(&turn.model_info); let use_original_detail = @@ -197,11 +214,11 @@ impl ViewImageHandler { } else { PromptImageMode::ResizeToFit }; - load_for_prompt_bytes(abs_path.as_path(), file_bytes, image_mode) + load_for_prompt_bytes(native_path.as_path(), file_bytes, image_mode) .map_err(|error| { FunctionCallError::RespondToModel(format!( "unable to process image at `{}`: {error}", - abs_path.display() + native_path.display() )) })? .into_data_url() @@ -272,6 +289,7 @@ mod tests { use crate::turn_diff_tracker::TurnDiffTracker; use codex_protocol::models::PermissionProfile; use codex_utils_absolute_path::AbsolutePathBuf; + use codex_utils_path_uri::PathUri; use core_test_support::TempDirExt; use pretty_assertions::assert_eq; use serde_json::json; @@ -290,8 +308,7 @@ mod tests { current.environment, PathUri::from_abs_path(&cwd), current.shell, - ) - .expect("turn environment"); + ); } #[test] diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 7e97e4ad51..29cecf71ed 100644 --- a/codex-rs/core/src/tools/orchestrator.rs +++ b/codex-rs/core/src/tools/orchestrator.rs @@ -537,7 +537,7 @@ impl ToolOrchestrator { } else { ToolDecisionSource::User }; - let decision = tool.start_approval_async(req, approval_ctx).await; + let decision = tool.start_approval_async(req, approval_ctx).await?; let tool_name = flat_tool_name(&tool_ctx.tool_name); otel.tool_decision( tool_name.as_ref(), diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index 9ad9d284c7..0624b75e43 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -133,7 +133,7 @@ impl Approvable for ApplyPatchRuntime { &'a mut self, req: &'a ApplyPatchRequest, ctx: ApprovalCtx<'a>, - ) -> BoxFuture<'a, ReviewDecision> { + ) -> BoxFuture<'a, Result> { let session = ctx.session; let turn = ctx.turn; let call_id = ctx.call_id.to_string(); @@ -144,11 +144,12 @@ impl Approvable for ApplyPatchRuntime { Box::pin(async move { if let Some(review_id) = guardian_review_id { let action = ApplyPatchRuntime::build_guardian_review_request(req, ctx.call_id); - return review_approval_request(session, turn, review_id, action, retry_reason) - .await; + return Ok( + review_approval_request(session, turn, review_id, action, retry_reason).await, + ); } if req.permissions_preapproved && retry_reason.is_none() { - return ReviewDecision::Approved; + return Ok(ReviewDecision::Approved); } if let Some(reason) = retry_reason { let rx_approve = session @@ -160,10 +161,10 @@ impl Approvable for ApplyPatchRuntime { /*grant_root*/ None, ) .await; - return rx_approve.await.unwrap_or_default(); + return Ok(rx_approve.await.unwrap_or_default()); } - with_cached_approval( + Ok(with_cached_approval( &session.services, "apply_patch", approval_keys, @@ -176,7 +177,7 @@ impl Approvable for ApplyPatchRuntime { rx_approve.await.unwrap_or_default() }, ) - .await + .await) }) } diff --git a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs index a181502f0b..51ca0b98c8 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -22,7 +22,6 @@ fn test_turn_environment(environment_id: &str) -> crate::session::turn_context:: PathUri::from_abs_path(&std::env::temp_dir().abs()), /*shell*/ None, ) - .expect("turn environment") } #[test] diff --git a/codex-rs/core/src/tools/runtimes/mod.rs b/codex-rs/core/src/tools/runtimes/mod.rs index 27f4ee5946..6c8cc15d87 100644 --- a/codex-rs/core/src/tools/runtimes/mod.rs +++ b/codex-rs/core/src/tools/runtimes/mod.rs @@ -33,22 +33,21 @@ pub(crate) mod apply_patch; pub(crate) mod shell; pub(crate) mod unified_exec; -/// Shared helper to construct sandbox transform inputs from a tokenized command line and native -/// working directory. Validates that at least a program is present. +/// Shared helper to construct sandbox transform inputs from a tokenized command line and working +/// directory. Validates that at least a program is present. pub(crate) fn build_sandbox_command( command: &[String], - cwd: &AbsolutePathBuf, + cwd: &PathUri, env: &HashMap, additional_permissions: Option, ) -> Result { let (program, args) = command .split_first() .ok_or_else(|| ToolError::Rejected("command args are empty".to_string()))?; - let cwd = PathUri::from_abs_path(cwd); Ok(SandboxCommand { program: program.clone().into(), args: args.to_vec(), - cwd, + cwd: cwd.clone(), env: env.clone(), additional_permissions, }) diff --git a/codex-rs/core/src/tools/runtimes/mod_tests.rs b/codex-rs/core/src/tools/runtimes/mod_tests.rs index 2b879e75ca..a42fbef8ab 100644 --- a/codex-rs/core/src/tools/runtimes/mod_tests.rs +++ b/codex-rs/core/src/tools/runtimes/mod_tests.rs @@ -91,7 +91,7 @@ async fn explicit_escalation_prepares_exec_without_managed_network() -> anyhow:: let command = vec!["/bin/echo".to_string(), "ok".to_string()]; let command = build_sandbox_command( &command, - &command_cwd, + &PathUri::from_abs_path(&command_cwd), &exec_env_for_sandbox_permissions(&env, SandboxPermissions::RequireEscalated), /*additional_permissions*/ None, ) @@ -129,7 +129,7 @@ async fn explicit_escalation_prepares_exec_without_managed_network() -> anyhow:: ) .expect("prepare exec request"); - assert_eq!(exec_request.cwd, command_cwd); + assert_eq!(exec_request.cwd, PathUri::from_abs_path(&command_cwd)); assert_eq!( exec_request.windows_sandbox_policy_cwd, native_sandbox_policy_cwd diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 6cfa3b2dfc..3ccd9e4c72 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -47,6 +47,7 @@ use codex_protocol::protocol::ReviewDecision; use codex_sandboxing::SandboxablePreference; use codex_shell_command::powershell::prefix_powershell_script_with_utf8; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use futures::future::BoxFuture; use std::collections::HashMap; use tokio_util::sync::CancellationToken; @@ -138,7 +139,7 @@ impl Approvable for ShellRuntime { &'a mut self, req: &'a ShellRequest, ctx: ApprovalCtx<'a>, - ) -> BoxFuture<'a, ReviewDecision> { + ) -> BoxFuture<'a, Result> { let keys = self.approval_keys(req); let command = req.command.clone(); let cwd = req.cwd.clone(); @@ -150,7 +151,7 @@ impl Approvable for ShellRuntime { let guardian_review_id = ctx.guardian_review_id.clone(); Box::pin(async move { if let Some(review_id) = guardian_review_id { - return review_approval_request( + return Ok(review_approval_request( session, turn, review_id, @@ -164,28 +165,30 @@ impl Approvable for ShellRuntime { }, retry_reason, ) - .await; + .await); } - with_cached_approval(&session.services, "shell", keys, move || async move { - let available_decisions = None; - session - .request_command_approval( - turn, - call_id, - /*approval_id*/ None, - command, - cwd, - reason, - ctx.network_approval_context.clone(), - req.exec_approval_requirement - .proposed_execpolicy_amendment() - .cloned(), - req.additional_permissions.clone(), - available_decisions, - ) - .await - }) - .await + Ok( + with_cached_approval(&session.services, "shell", keys, move || async move { + let available_decisions = None; + session + .request_command_approval( + turn, + call_id, + /*approval_id*/ None, + command, + cwd, + reason, + ctx.network_approval_context.clone(), + req.exec_approval_requirement + .proposed_execpolicy_amendment() + .cloned(), + req.additional_permissions.clone(), + available_decisions, + ) + .await + }) + .await, + ) }) } @@ -247,7 +250,9 @@ impl ToolRuntime for ShellRuntime { .shell .as_ref() .unwrap_or(session_shell.as_ref()); - let shell_snapshot_location = req.turn_environment.shell_snapshot(&req.cwd); + let shell_snapshot_location = req + .turn_environment + .shell_snapshot(&PathUri::from_abs_path(&req.cwd)); let (file_system_sandbox_policy, _) = attempt.permissions.to_runtime_permissions(); let sandbox_permissions = sandbox_permissions_preserving_denied_reads( req.sandbox_permissions, @@ -305,8 +310,12 @@ impl ToolRuntime for ShellRuntime { } } - let command = - build_sandbox_command(&command, &req.cwd, &env, req.additional_permissions.clone())?; + let command = build_sandbox_command( + &command, + &PathUri::from_abs_path(&req.cwd), + &env, + req.additional_permissions.clone(), + )?; let mut expiration: crate::exec::ExecExpiration = req.timeout_ms.into(); expiration = expiration.with_cancellation(req.cancellation_token.clone()); if let Some(cancellation) = attempt.network_denial_cancellation_token.clone() { diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index ea53e9a55c..3b843c7fae 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -132,8 +132,12 @@ pub(super) async fn try_run_zsh_fork( }; let mut env = exec_env_for_sandbox_permissions(&req.env, req.sandbox_permissions); prepend_zsh_fork_bin_to_path(&mut env, shell_zsh_path); - let command = - build_sandbox_command(command, &req.cwd, &env, req.additional_permissions.clone())?; + let command = build_sandbox_command( + command, + &PathUri::from_abs_path(&req.cwd), + &env, + req.additional_permissions.clone(), + )?; let options = ExecOptions { expiration: req.timeout_ms.into(), capture_policy: ExecCapturePolicy::ShellTool, @@ -172,6 +176,11 @@ pub(super) async fn try_run_zsh_fork( let exec_policy = Arc::new(RwLock::new( ctx.session.services.exec_policy.current().as_ref().clone(), )); + let sandbox_cwd = sandbox_cwd.to_abs_path().map_err(|err| { + ToolError::Rejected(format!( + "sandbox cwd `{sandbox_cwd}` is not native to the Codex host: {err}" + )) + })?; let command_executor = CoreShellCommandExecutor { command, cwd: sandbox_cwd, @@ -273,9 +282,15 @@ pub(crate) async fn prepare_unified_exec_zsh_fork( let exec_policy = Arc::new(RwLock::new( ctx.session.services.exec_policy.current().as_ref().clone(), )); + let native_cwd = exec_request.cwd.to_abs_path().map_err(|err| { + ToolError::Rejected(format!( + "zsh-fork cwd `{}` is not native to the Codex host: {err}", + exec_request.cwd + )) + })?; let command_executor = CoreShellCommandExecutor { command: exec_request.command.clone(), - cwd: exec_request.cwd.clone(), + cwd: native_cwd, permission_profile: exec_request.permission_profile.clone(), file_system_sandbox_policy: exec_request.file_system_sandbox_policy.clone(), network_sandbox_policy: exec_request.network_sandbox_policy, @@ -852,7 +867,7 @@ impl CoreShellCommandExecutor { let result = crate::sandboxing::execute_exec_request_with_after_spawn( crate::sandboxing::ExecRequest { command: self.command.clone(), - cwd: self.cwd.clone(), + cwd: PathUri::from_abs_path(&self.cwd), env: exec_env, exec_server_env_config: None, network: self.network.clone(), @@ -1003,9 +1018,15 @@ impl CoreShellCommandExecutor { network.apply_to_env(&mut exec_request.env); } + let cwd = exec_request.cwd.to_abs_path().map_err(|err| { + anyhow::anyhow!( + "escalated cwd `{}` is not native to the Codex host: {err}", + exec_request.cwd + ) + })?; Ok(PreparedExec { command: exec_request.command, - cwd: exec_request.cwd.to_path_buf(), + cwd: cwd.to_path_buf(), env: exec_request.env, arg0: exec_request.arg0, }) diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 25a1b932ce..2964f43ea7 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -51,6 +51,7 @@ use codex_sandboxing::SandboxablePreference; use codex_shell_command::powershell::prefix_powershell_script_with_utf8; use codex_tools::UnifiedExecShellMode; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use futures::future::BoxFuture; use std::collections::HashMap; use tokio_util::sync::CancellationToken; @@ -63,7 +64,7 @@ pub struct UnifiedExecRequest { pub shell_type: ShellType, pub hook_command: String, pub process_id: i32, - pub cwd: AbsolutePathBuf, + pub cwd: PathUri, pub sandbox_cwd: AbsolutePathBuf, pub turn_environment: TurnEnvironment, pub env: HashMap, @@ -84,7 +85,7 @@ pub struct UnifiedExecRequest { #[derive(serde::Serialize, Clone, Debug, Eq, PartialEq, Hash)] pub struct UnifiedExecApprovalKey { pub command: Vec, - pub cwd: AbsolutePathBuf, + pub cwd: PathUri, pub tty: bool, pub sandbox_permissions: SandboxPermissions, pub additional_permissions: Option, @@ -147,19 +148,26 @@ impl Approvable for UnifiedExecRuntime<'_> { &'b mut self, req: &'b UnifiedExecRequest, ctx: ApprovalCtx<'b>, - ) -> BoxFuture<'b, ReviewDecision> { + ) -> BoxFuture<'b, Result> { let keys = self.approval_keys(req); let session = ctx.session; let turn = ctx.turn; let call_id = ctx.call_id.to_string(); let command = req.command.clone(); - let cwd = req.cwd.clone(); let retry_reason = ctx.retry_reason.clone(); let reason = retry_reason.clone().or_else(|| req.justification.clone()); let guardian_review_id = ctx.guardian_review_id.clone(); Box::pin(async move { + // TODO(anp): Migrate command approval payloads to PathUri so approvals can represent + // working directories native to a remote environment. + let cwd = req.cwd.to_abs_path().map_err(|err| { + ToolError::Rejected(format!( + "command cwd `{}` is not native to the Codex host: {err}", + req.cwd + )) + })?; if let Some(review_id) = guardian_review_id { - return review_approval_request( + return Ok(review_approval_request( session, turn, review_id, @@ -174,28 +182,30 @@ impl Approvable for UnifiedExecRuntime<'_> { }, retry_reason, ) - .await; + .await); } - with_cached_approval(&session.services, "unified_exec", keys, || async move { - let available_decisions = None; - session - .request_command_approval( - turn, - call_id, - /*approval_id*/ None, - command, - cwd.clone(), - reason, - ctx.network_approval_context.clone(), - req.exec_approval_requirement - .proposed_execpolicy_amendment() - .cloned(), - req.additional_permissions.clone(), - available_decisions, - ) - .await - }) - .await + Ok( + with_cached_approval(&session.services, "unified_exec", keys, || async move { + let available_decisions = None; + session + .request_command_approval( + turn, + call_id, + /*approval_id*/ None, + command, + cwd.clone(), + reason, + ctx.network_approval_context.clone(), + req.exec_approval_requirement + .proposed_execpolicy_amendment() + .cloned(), + req.additional_permissions.clone(), + available_decisions, + ) + .await + }) + .await, + ) }) } @@ -238,6 +248,9 @@ impl<'a> ToolRuntime for UnifiedExecRunt ); let network = managed_network_for_sandbox_permissions(req.network.as_ref(), sandbox_permissions)?; + // TODO(anp): Migrate guardian network approval triggers to PathUri so remote working + // directories do not suppress managed-network approval context. + let cwd = req.cwd.to_abs_path().ok()?; Some(NetworkApprovalSpec { network: Some(network.clone()), mode: NetworkApprovalMode::Deferred, @@ -245,7 +258,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt call_id: ctx.call_id.clone(), tool_name: flat_tool_name(&ctx.tool_name).into_owned(), command: req.command.clone(), - cwd: req.cwd.clone(), + cwd, sandbox_permissions: req.sandbox_permissions, additional_permissions: req.additional_permissions.clone(), justification: req.justification.clone(), @@ -398,6 +411,9 @@ impl<'a> ToolRuntime for UnifiedExecRunt .env_for(command, options, managed_network) .map_err(ToolError::Codex)?; exec_env.exec_server_env_config = req.exec_server_env_config.clone(); + if req.turn_environment.environment.is_remote() { + exec_env.cwd = req.cwd.clone(); + } self.manager .open_session_with_exec_env( req.process_id, @@ -435,7 +451,7 @@ mod tests { TurnEnvironment::new( LOCAL_ENVIRONMENT_ID.to_string(), Arc::new(Environment::default_for_tests()), - cwd, + PathUri::from_abs_path(&cwd), /*shell*/ None, ) } @@ -477,7 +493,7 @@ mod tests { shell_type: ShellType::Sh, hook_command: "pwd".to_string(), process_id: 1000, - cwd, + cwd: PathUri::from_abs_path(&cwd), sandbox_cwd: sandbox_cwd.clone(), turn_environment: test_turn_environment(sandbox_cwd.clone()), env: HashMap::new(), @@ -576,7 +592,7 @@ mod tests { shell_type: ShellType::Zsh, hook_command: "echo hi".to_string(), process_id: 1000, - cwd: cwd.clone(), + cwd: PathUri::from_abs_path(&cwd), sandbox_cwd: cwd.clone(), turn_environment: test_turn_environment(cwd), env: HashMap::new(), diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 499cdfbdae..b072a51881 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -366,7 +366,7 @@ pub(crate) trait Approvable { &'a mut self, req: &'a Req, ctx: ApprovalCtx<'a>, - ) -> BoxFuture<'a, ReviewDecision>; + ) -> BoxFuture<'a, Result>; } pub(crate) trait Sandboxable { diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index cbe282ec39..5faee28682 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -597,10 +597,9 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava )) .expect("remote test environment"), ), - codex_utils_path_uri::PathUri::from_abs_path(&remote_cwd), + remote_cwd, /*shell*/ None, - ) - .expect("remote turn environment"), + ), ); }) .await; diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 23dd3bf1b5..54e9af09fe 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -32,6 +32,7 @@ use codex_protocol::models::AdditionalPermissionProfile; use codex_tools::UnifiedExecShellMode; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_path_uri::PathUri; use rand::Rng; use rand::rng; use tokio::sync::Mutex; @@ -96,7 +97,7 @@ pub(crate) struct ExecCommandRequest { pub process_id: i32, pub yield_time_ms: u64, pub max_output_tokens: Option, - pub cwd: AbsolutePathBuf, + pub cwd: PathUri, pub sandbox_cwd: AbsolutePathBuf, pub turn_environment: TurnEnvironment, pub shell_mode: UnifiedExecShellMode, diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 702a339e2e..be4f2abe2c 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -57,7 +57,6 @@ use codex_protocol::protocol::ExecCommandSource; use codex_tools::ToolName; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_output_truncation::approx_token_count; -use codex_utils_path_uri::PathUri; const UNIFIED_EXEC_ENV: [(&str, &str); 10] = [ ("NO_COLOR", "1"), @@ -157,7 +156,7 @@ fn exec_server_params_for_request( codex_exec_server::ExecParams { process_id: exec_server_process_id(process_id).into(), argv: request.command.clone(), - cwd: PathUri::from_abs_path(&request.cwd), + cwd: request.cwd.clone(), env_policy, env, tty, @@ -385,10 +384,15 @@ impl UnifiedExecProcessManager { request: ExecCommandRequest, context: &UnifiedExecContext, ) -> Result { - let cwd = request.cwd.clone(); - let process = self - .open_session_with_sandbox(&request, cwd.clone(), context) - .await; + // TODO(anp): Migrate unified-exec events and retained process metadata to PathUri so + // foreign working directories do not need a host-native projection here. + let event_cwd = request.cwd.to_abs_path().map_err(|err| { + UnifiedExecError::create_process(format!( + "command cwd `{}` is not native to the Codex host: {err}", + request.cwd + )) + })?; + let process = self.open_session_with_sandbox(&request, context).await; let (process, mut deferred_network_approval) = match process { Ok((process, deferred_network_approval)) => { @@ -416,7 +420,7 @@ impl UnifiedExecProcessManager { ); let emitter = ToolEmitter::unified_exec( &request.command, - cwd.clone(), + event_cwd.clone(), ExecCommandSource::UnifiedExecStartup, Some(request.process_id.to_string()), ); @@ -434,7 +438,7 @@ impl UnifiedExecProcessManager { context, &request.command, request.hook_command.clone(), - cwd.clone(), + event_cwd.clone(), start, request.process_id, request.tty, @@ -493,7 +497,7 @@ impl UnifiedExecProcessManager { process_started_alive, context, &request, - cwd.clone(), + event_cwd.clone(), Arc::clone(&transcript), text.clone(), message.clone(), @@ -513,7 +517,7 @@ impl UnifiedExecProcessManager { process_started_alive, context, &request, - cwd.clone(), + event_cwd.clone(), Arc::clone(&transcript), text.clone(), message.clone(), @@ -565,7 +569,7 @@ impl UnifiedExecProcessManager { process_started_alive, context, &request, - cwd.clone(), + event_cwd.clone(), Arc::clone(&transcript), text.clone(), message.clone(), @@ -582,7 +586,7 @@ impl UnifiedExecProcessManager { Arc::clone(&context.turn), context.call_id.clone(), request.command.clone(), - cwd.clone(), + event_cwd.clone(), Some(process_id.to_string()), Arc::clone(&transcript), text.clone(), @@ -899,6 +903,12 @@ impl UnifiedExecProcessManager { #[cfg(target_os = "windows")] if request.sandbox == codex_sandboxing::SandboxType::WindowsRestrictedToken { + let native_cwd = request.cwd.to_abs_path().map_err(|err| { + UnifiedExecError::create_process(format!( + "failed to convert command cwd `{}` for Windows sandbox execution: {err}", + request.cwd + )) + })?; let codex_home = crate::config::find_codex_home().map_err(|err| { UnifiedExecError::create_process(format!( "windows sandbox: failed to resolve codex_home: {err}" @@ -933,7 +943,7 @@ impl UnifiedExecProcessManager { request.windows_sandbox_workspace_roots.as_slice(), codex_home.as_ref(), request.command.clone(), - request.cwd.as_path(), + native_cwd.as_path(), request.env.clone(), request.network.is_some(), None, @@ -955,7 +965,7 @@ impl UnifiedExecProcessManager { request.windows_sandbox_workspace_roots.as_slice(), codex_home.as_ref(), request.command.clone(), - request.cwd.as_path(), + native_cwd.as_path(), request.env.clone(), None, &additional_deny_read_paths, @@ -995,11 +1005,17 @@ impl UnifiedExecProcessManager { .command .split_first() .ok_or(UnifiedExecError::MissingCommandLine)?; + let native_cwd = request.cwd.to_abs_path().map_err(|err| { + UnifiedExecError::create_process(format!( + "failed to convert command cwd `{}` for local execution: {err}", + request.cwd + )) + })?; let spawn_result = if tty { codex_utils_pty::pty::spawn_process_with_inherited_fds( program, args, - request.cwd.as_path(), + native_cwd.as_path(), &request.env, &request.arg0, codex_utils_pty::TerminalSize::default(), @@ -1010,7 +1026,7 @@ impl UnifiedExecProcessManager { codex_utils_pty::pipe::spawn_process_no_stdin_with_inherited_fds( program, args, - request.cwd.as_path(), + native_cwd.as_path(), &request.env, &request.arg0, &inherited_fds, @@ -1026,7 +1042,6 @@ impl UnifiedExecProcessManager { pub(super) async fn open_session_with_sandbox( &self, request: &ExecCommandRequest, - cwd: AbsolutePathBuf, context: &UnifiedExecContext, ) -> Result<(UnifiedExecProcess, Option), UnifiedExecError> { let local_policy_env = create_env( @@ -1067,7 +1082,7 @@ impl UnifiedExecProcessManager { shell_type: request.shell_type, hook_command: request.hook_command.clone(), process_id: request.process_id, - cwd, + cwd: request.cwd.clone(), sandbox_cwd: request.sandbox_cwd.clone(), turn_environment: request.turn_environment.clone(), env, diff --git a/codex-rs/core/src/unified_exec/process_manager_tests.rs b/codex-rs/core/src/unified_exec/process_manager_tests.rs index 49758ec799..993adf022a 100644 --- a/codex-rs/core/src/unified_exec/process_manager_tests.rs +++ b/codex-rs/core/src/unified_exec/process_manager_tests.rs @@ -1,5 +1,6 @@ use super::*; use crate::unified_exec::clamp_yield_time; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use tokio::time::Duration; use tokio::time::Instant; @@ -72,13 +73,14 @@ fn exec_server_params_use_path_uri_and_env_policy_overlay_contract() { .expect("current dir") .try_into() .expect("absolute path"); + let command_cwd = PathUri::parse("file://server/share/workspace").expect("valid UNC path URI"); let file_system_sandbox_policy = codex_protocol::permissions::FileSystemSandboxPolicy::unrestricted(); let network_sandbox_policy = codex_protocol::permissions::NetworkSandboxPolicy::Restricted; let permission_profile = codex_protocol::models::PermissionProfile::Disabled; let request = ExecRequest { command: vec!["bash".to_string(), "-lc".to_string(), "true".to_string()], - cwd: cwd.clone(), + cwd: command_cwd.clone(), env: HashMap::from([ ("HOME".to_string(), "/client-home".to_string()), ("PATH".to_string(), "/sandbox-path".to_string()), @@ -116,7 +118,7 @@ fn exec_server_params_use_path_uri_and_env_policy_overlay_contract() { exec_server_params_for_request(/*process_id*/ 123, &request, /*tty*/ true); assert_eq!(params.process_id.as_str(), "123"); - assert_eq!(params.cwd, PathUri::from_abs_path(&request.cwd)); + assert_eq!(params.cwd, command_cwd); assert!(params.env_policy.is_some()); assert_eq!( params.env, @@ -200,7 +202,7 @@ async fn failed_initial_end_for_unstored_process_uses_fallback_output() { yield_time_ms: 1000, max_output_tokens: None, #[allow(deprecated)] - cwd: turn.cwd.clone(), + cwd: PathUri::from_abs_path(&turn.cwd), #[allow(deprecated)] sandbox_cwd: turn.cwd.clone(), turn_environment: turn diff --git a/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs b/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs index 1ea7d293ce..6531f075a8 100644 --- a/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs +++ b/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs @@ -339,11 +339,11 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul .received_requests() .await .context("failed to fetch received requests")?; - let model_request_includes_remote_instructions = requests.iter().any(|request| { - request - .body_json::() - .is_ok_and(|body| body.to_string().contains(AGENTS_INSTRUCTIONS)) - }); + let initial_request = requests.first().context("missing initial model request")?; + let model_request_includes_remote_instructions = initial_request + .body_json::()? + .to_string() + .contains(AGENTS_INSTRUCTIONS); // TODO(anp): Load remote workspace instructions into the model context. assert!(!model_request_includes_remote_instructions); diff --git a/codex-rs/exec-server/src/local_process.rs b/codex-rs/exec-server/src/local_process.rs index 78a53897ee..3e0f4c03b8 100644 --- a/codex-rs/exec-server/src/local_process.rs +++ b/codex-rs/exec-server/src/local_process.rs @@ -156,10 +156,11 @@ impl LocalProcess { .argv .split_first() .ok_or_else(|| invalid_params("argv must not be empty".to_string()))?; - let native_cwd = params.cwd.to_abs_path().map_err(|err| { + let native_cwd = params.cwd.to_abs_path().map_err(|_| { invalid_params(format!( - "cwd URI `{}` is not valid on this exec-server host: {err}", - params.cwd + "'{}' is invalid on '{}'", + params.cwd, + std::env::consts::OS )) })?; @@ -815,12 +816,7 @@ mod tests { #[cfg(windows)] let uri = "file:///usr/local/checkout"; let cwd = PathUri::parse(uri).expect("non-native cwd URI"); - let source = cwd - .to_abs_path() - .expect_err("cwd should not be native to this host"); - let expected = invalid_params(format!( - "cwd URI `{cwd}` is not valid on this exec-server host: {source}" - )); + let expected = invalid_params(format!("'{cwd}' is invalid on '{}'", std::env::consts::OS)); let mut params = test_exec_params(HashMap::new()); params.cwd = cwd; diff --git a/codex-rs/exec/tests/suite/sandbox.rs b/codex-rs/exec/tests/suite/sandbox.rs index 4b8cecc515..a678436a4b 100644 --- a/codex-rs/exec/tests/suite/sandbox.rs +++ b/codex-rs/exec/tests/suite/sandbox.rs @@ -60,7 +60,7 @@ async fn spawn_command_under_sandbox( child.arg0(arg0); } child.args(args); - child.current_dir(exec_request.cwd); + child.current_dir(exec_request.cwd.to_abs_path()?.as_path()); child.env_clear(); child.envs(exec_request.env); diff --git a/codex-rs/utils/path-uri/src/api_path_string.rs b/codex-rs/utils/path-uri/src/api_path_string.rs index 0304777769..a8675c9a73 100644 --- a/codex-rs/utils/path-uri/src/api_path_string.rs +++ b/codex-rs/utils/path-uri/src/api_path_string.rs @@ -97,6 +97,10 @@ impl ApiPathString { &self.0 } + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + pub fn into_string(self) -> String { self.0 } diff --git a/codex-rs/utils/path-uri/src/lib.rs b/codex-rs/utils/path-uri/src/lib.rs index c37ad31b45..0c2d41bf09 100644 --- a/codex-rs/utils/path-uri/src/lib.rs +++ b/codex-rs/utils/path-uri/src/lib.rs @@ -198,7 +198,11 @@ impl PathUri { Some(Self(url)) } - /// Lexically joins a relative URI path onto this URI. + /// Lexically joins a relative path onto this URI. + /// + /// `/` separates components for every base URI. When the base URI is + /// inferred to use the Windows path convention, `\` is also treated as a + /// separator. A leading separator is rejected as an absolute path. /// /// Empty and `.` segments are ignored, while `..` removes one segment /// without escaping the URI root. Literal `%`, `?`, and `#` characters are @@ -207,7 +211,18 @@ impl PathUri { /// Opaque fallback URIs created by [`Self::from_abs_path`] reject non-empty /// joins. pub fn join(&self, path: &str) -> Result { - if path.starts_with('/') { + let uses_windows_path_convention = + self.infer_path_convention() == Some(PathConvention::Windows); + let starts_with_root_separator = path.chars().next().is_some_and(|character| { + if uses_windows_path_convention { + matches!(character, '/' | '\\') + } else { + character == '/' + } + }); + let has_windows_drive_prefix = uses_windows_path_convention + && matches!(path.as_bytes(), [drive, b':', ..] if drive.is_ascii_alphabetic()); + if starts_with_root_separator || has_windows_drive_prefix { return Err(PathUriParseError::JoinPathMustBeRelative(path.to_string())); } if path.contains('\0') { @@ -230,7 +245,14 @@ impl PathUri { unreachable!("validated file URLs support hierarchical path segments"); }; segments.pop_if_empty(); - for component in path.split('/') { + let is_separator = |character| { + if uses_windows_path_convention { + matches!(character, '/' | '\\') + } else { + character == '/' + } + }; + for component in path.split(is_separator) { match component { "" | "." => {} ".." => { @@ -378,6 +400,12 @@ impl FromStr for PathUri { } } +impl From for PathUri { + fn from(path: AbsolutePathBuf) -> Self { + Self::from_abs_path(&path) + } +} + impl fmt::Display for PathUri { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { self.0.fmt(f) diff --git a/codex-rs/utils/path-uri/src/tests.rs b/codex-rs/utils/path-uri/src/tests.rs index fe436476dd..f81c426819 100644 --- a/codex-rs/utils/path-uri/src/tests.rs +++ b/codex-rs/utils/path-uri/src/tests.rs @@ -481,11 +481,26 @@ fn join_normalizes_relative_uri_segments() { ), ("file:///", "../../etc", "file:///etc"), ("file:///C:/Users", "../Windows", "file:///C:/Windows"), + ( + "file:///C:/Users/Alice", + r"..\Bob\source", + "file:///C:/Users/Bob/source", + ), ( "file://server/share/src", "../tests", "file://server/share/tests", ), + ( + "file://server/share/src", + r"..\tests", + "file://server/share/tests", + ), + ( + "file:///workspace", + r"src\main.rs", + "file:///workspace/src%5Cmain.rs", + ), ( "file:///workspace", "a?b#c%d", @@ -502,6 +517,7 @@ fn join_normalizes_relative_uri_segments() { #[test] fn join_rejects_absolute_and_null_paths() { let base = PathUri::parse("file:///workspace").expect("valid base URI"); + let windows_base = PathUri::parse("file:///C:/workspace").expect("valid base URI"); assert!(matches!( base.join("/src"), @@ -513,6 +529,16 @@ fn join_rejects_absolute_and_null_paths() { path: "src\0file".to_string(), }) ); + assert!(matches!( + windows_base.join(r"\src"), + Err(PathUriParseError::JoinPathMustBeRelative(path)) if path == r"\src" + )); + for path in [r"C:\src", "C:src"] { + assert!(matches!( + windows_base.join(path), + Err(PathUriParseError::JoinPathMustBeRelative(rejected)) if rejected == path + )); + } } #[test]