From 4e757cfe785e416dfec18643200a81a728a693df Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sat, 13 Jun 2026 01:00:01 +0000 Subject: [PATCH 01/15] app-server: accept environment-native cwd --- .../schema/json/ClientRequest.json | 10 +- .../codex_app_server_protocol.schemas.json | 7 +- .../codex_app_server_protocol.v2.schemas.json | 7 +- .../schema/json/v2/ThreadStartParams.json | 10 +- .../schema/json/v2/TurnStartParams.json | 10 +- .../typescript/v2/TurnEnvironmentParams.ts | 8 +- .../src/protocol/v2/tests.rs | 18 +-- .../src/protocol/v2/turn.rs | 4 +- codex-rs/app-server/README.md | 2 +- codex-rs/app-server/src/request_processors.rs | 18 +++ .../request_processors/thread_processor.rs | 24 +--- .../src/request_processors/turn_processor.rs | 25 +--- .../app-server/tests/suite/v2/thread_start.rs | 3 +- .../app-server/tests/suite/v2/turn_start.rs | 5 +- codex-rs/core/src/thread_manager.rs | 47 +++++++ codex-rs/core/src/thread_manager_tests.rs | 127 ++++++++++++++++++ 16 files changed, 261 insertions(+), 64 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 3ed2791c8551..7812e9c202ca 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -78,6 +78,9 @@ } ] }, + "ApiPathString": { + "type": "string" + }, "ApprovalsReviewer": { "description": "Configures who approval requests are routed to for review. Examples include sandbox escapes, blocked network access, MCP approval prompts, and ARC escalations. Defaults to `user`. `auto_review` uses a carefully prompted subagent to gather relevant context and apply a risk-based decision framework before approving or denying the request. The legacy value `guardian_subagent` is accepted for compatibility.", "enum": [ @@ -4331,7 +4334,12 @@ "TurnEnvironmentParams": { "properties": { "cwd": { - "$ref": "#/definitions/AbsolutePathBuf" + "allOf": [ + { + "$ref": "#/definitions/ApiPathString" + } + ], + "description": "Absolute cwd rendered using the selected environment's path convention." }, "environmentId": { "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 97c4eee3cff2..558c8b7f85eb 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -19517,7 +19517,12 @@ "TurnEnvironmentParams": { "properties": { "cwd": { - "$ref": "#/definitions/v2/AbsolutePathBuf" + "allOf": [ + { + "$ref": "#/definitions/v2/ApiPathString" + } + ], + "description": "Absolute cwd rendered using the selected environment's path convention." }, "environmentId": { "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 1f5e7ac2bbc9..4ac01e318d3c 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -17325,7 +17325,12 @@ "TurnEnvironmentParams": { "properties": { "cwd": { - "$ref": "#/definitions/AbsolutePathBuf" + "allOf": [ + { + "$ref": "#/definitions/ApiPathString" + } + ], + "description": "Absolute cwd rendered using the selected environment's path convention." }, "environmentId": { "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json index 59e05c773b88..95e4c706b9c0 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json @@ -5,6 +5,9 @@ "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", "type": "string" }, + "ApiPathString": { + "type": "string" + }, "ApprovalsReviewer": { "description": "Configures who approval requests are routed to for review. Examples include sandbox escapes, blocked network access, MCP approval prompts, and ARC escalations. Defaults to `user`. `auto_review` uses a carefully prompted subagent to gather relevant context and apply a risk-based decision framework before approving or denying the request. The legacy value `guardian_subagent` is accepted for compatibility.", "enum": [ @@ -242,7 +245,12 @@ "TurnEnvironmentParams": { "properties": { "cwd": { - "$ref": "#/definitions/AbsolutePathBuf" + "allOf": [ + { + "$ref": "#/definitions/ApiPathString" + } + ], + "description": "Absolute cwd rendered using the selected environment's path convention." }, "environmentId": { "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json index 070944a28b19..c1db63db4808 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json @@ -27,6 +27,9 @@ ], "type": "string" }, + "ApiPathString": { + "type": "string" + }, "ApprovalsReviewer": { "description": "Configures who approval requests are routed to for review. Examples include sandbox escapes, blocked network access, MCP approval prompts, and ARC escalations. Defaults to `user`. `auto_review` uses a carefully prompted subagent to gather relevant context and apply a risk-based decision framework before approving or denying the request. The legacy value `guardian_subagent` is accepted for compatibility.", "enum": [ @@ -331,7 +334,12 @@ "TurnEnvironmentParams": { "properties": { "cwd": { - "$ref": "#/definitions/AbsolutePathBuf" + "allOf": [ + { + "$ref": "#/definitions/ApiPathString" + } + ], + "description": "Absolute cwd rendered using the selected environment's path convention." }, "environmentId": { "type": "string" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/TurnEnvironmentParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/TurnEnvironmentParams.ts index bb981b0ac973..f5c91a9af11c 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/TurnEnvironmentParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/TurnEnvironmentParams.ts @@ -1,6 +1,10 @@ // GENERATED CODE! DO NOT MODIFY BY HAND! // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -import type { AbsolutePathBuf } from "../AbsolutePathBuf"; +import type { ApiPathString } from "../ApiPathString"; -export type TurnEnvironmentParams = { environmentId: string, cwd: AbsolutePathBuf, }; +export type TurnEnvironmentParams = { environmentId: string, +/** + * Absolute cwd rendered using the selected environment's path convention. + */ +cwd: ApiPathString, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index c690871689ce..c702f439e2ab 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -3723,7 +3723,7 @@ fn thread_settings_update_params_preserve_field_level_experimental_gates() { #[test] fn turn_start_params_round_trip_environments() { - let cwd = test_absolute_path(); + let cwd = ApiPathString::new(r"C:\workspace"); let params: TurnStartParams = serde_json::from_value(json!({ "threadId": "thread_123", "input": [], @@ -3806,8 +3806,8 @@ fn turn_start_params_treat_null_or_omitted_environments_as_default() { } #[test] -fn turn_start_params_reject_relative_environment_cwd() { - let err = serde_json::from_value::(json!({ +fn turn_start_params_preserve_relative_environment_cwd_for_boundary_validation() { + let params = serde_json::from_value::(json!({ "threadId": "thread_123", "input": [], "environments": [ @@ -3817,12 +3817,14 @@ fn turn_start_params_reject_relative_environment_cwd() { } ], })) - .expect_err("relative environment cwd should fail"); + .expect("native cwd should deserialize without applying host rules"); - assert!( - err.to_string() - .contains("AbsolutePathBuf deserialized without a base path"), - "unexpected error: {err}" + assert_eq!( + params.environments, + Some(vec![TurnEnvironmentParams { + environment_id: "local".to_string(), + cwd: ApiPathString::new("relative"), + }]) ); } diff --git a/codex-rs/app-server-protocol/src/protocol/v2/turn.rs b/codex-rs/app-server-protocol/src/protocol/v2/turn.rs index 836c6d4e9693..0767a8ce6d93 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/turn.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/turn.rs @@ -14,6 +14,7 @@ use codex_protocol::user_input::ByteRange as CoreByteRange; use codex_protocol::user_input::TextElement as CoreTextElement; use codex_protocol::user_input::UserInput as CoreUserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::ApiPathString; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -38,7 +39,8 @@ pub enum TurnStatus { #[ts(export_to = "v2/")] pub struct TurnEnvironmentParams { pub environment_id: String, - pub cwd: AbsolutePathBuf, + /// Absolute cwd rendered using the selected environment's path convention. + pub cwd: ApiPathString, } #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 336f769d7155..706b706d2e5e 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -709,7 +709,7 @@ Turns attach user input (text or images) to a thread and trigger Codex generatio - `{"type":"image","url":"https://…png"}` - `{"type":"localImage","path":"/tmp/screenshot.png"}` -You can optionally specify config overrides on the new turn. If specified, these settings become the default for subsequent turns on the same thread. `outputSchema` applies only to the current turn. Experimental `environments` is turn-scoped: omit it to inherit the thread's sticky environments, pass `[]` to run the turn with no environments, or pass explicit environment ids to override the sticky selection for this turn only. +You can optionally specify config overrides on the new turn. If specified, these settings become the default for subsequent turns on the same thread. `outputSchema` applies only to the current turn. Experimental `environments` is turn-scoped: omit it to inherit the thread's sticky environments, pass `[]` to run the turn with no environments, or pass explicit environment ids to override the sticky selection for this turn only. Each environment `cwd` must be absolute and use that environment's native path convention, so a Windows environment uses a value such as `C:\\workspace` even when app-server runs on Linux. `approvalsReviewer` accepts: diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index cfc411e13080..c731b9a9b1a2 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -541,6 +541,24 @@ fn resolve_request_cwd(cwd: Option) -> Result, .transpose() } +async fn resolve_turn_environment_selections( + thread_manager: &ThreadManager, + environments: Option>, +) -> Result>, JSONRPCErrorError> { + let Some(environments) = environments else { + return Ok(None); + }; + thread_manager + .resolve_native_environment_selections( + environments + .into_iter() + .map(|environment| (environment.environment_id, environment.cwd)), + ) + .await + .map(Some) + .map_err(environment_selection_error) +} + fn resolve_runtime_workspace_roots(workspace_roots: Vec) -> Vec { let mut resolved_roots = Vec::new(); for root in workspace_roots { diff --git a/codex-rs/app-server/src/request_processors/thread_processor.rs b/codex-rs/app-server/src/request_processors/thread_processor.rs index 09ff0dd6981f..89b7259d4887 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -906,7 +906,8 @@ impl ThreadRequestProcessor { "`permissions` cannot be combined with `sandbox`", )); } - let environment_selections = self.parse_environment_selections(environments)?; + let environment_selections = + resolve_turn_environment_selections(self.thread_manager.as_ref(), environments).await?; let runtime_workspace_roots = runtime_workspace_roots.map(resolve_runtime_workspace_roots); let mut typesafe_overrides = self.build_thread_config_overrides( model, @@ -1311,27 +1312,6 @@ impl ThreadRequestProcessor { } } - fn parse_environment_selections( - &self, - environments: Option>, - ) -> Result>, JSONRPCErrorError> { - let environment_selections = environments.map(|environments| { - environments - .into_iter() - .map(|environment| TurnEnvironmentSelection { - environment_id: environment.environment_id, - cwd: PathUri::from_abs_path(&environment.cwd), - }) - .collect::>() - }); - if let Some(environment_selections) = environment_selections.as_ref() { - self.thread_manager - .validate_environment_selections(environment_selections) - .map_err(environment_selection_error)?; - } - Ok(environment_selections) - } - async fn thread_archive_inner( &self, params: ThreadArchiveParams, diff --git a/codex-rs/app-server/src/request_processors/turn_processor.rs b/codex-rs/app-server/src/request_processors/turn_processor.rs index 8d1810d87f3a..c15a4e05ab73 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -341,27 +341,6 @@ impl TurnRequestProcessor { Ok((review_request, hint)) } - fn parse_environment_selections( - &self, - environments: Option>, - ) -> Result>, JSONRPCErrorError> { - let environment_selections = environments.map(|environments| { - environments - .into_iter() - .map(|environment| TurnEnvironmentSelection { - environment_id: environment.environment_id, - cwd: PathUri::from_abs_path(&environment.cwd), - }) - .collect::>() - }); - if let Some(environment_selections) = environment_selections.as_ref() { - self.thread_manager - .validate_environment_selections(environment_selections) - .map_err(environment_selection_error)?; - } - Ok(environment_selections) - } - async fn request_trace_context( &self, request_id: &ConnectionRequestId, @@ -433,7 +412,9 @@ impl TurnRequestProcessor { self.track_error_response(&request_id, error, /*error_type*/ None); })?; - let environment_selections = self.parse_environment_selections(params.environments)?; + let environment_selections = + resolve_turn_environment_selections(self.thread_manager.as_ref(), params.environments) + .await?; // Map v2 input items to core input items. let mapped_items: Vec = params 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 82a2b6b2c070..cbe0f07dc150 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -33,6 +33,7 @@ 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; @@ -293,7 +294,7 @@ 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: codex_home.path().to_path_buf().try_into()?, + cwd: ApiPathString::new(codex_home.path().to_string_lossy()), }]), ..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 6567b06be1c1..663a8685191a 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -76,6 +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 core_test_support::responses; use core_test_support::skip_if_no_network; use pretty_assertions::assert_eq; @@ -1326,7 +1327,7 @@ async fn turn_start_rejects_unknown_environment_before_starting_turn() -> Result }], environments: Some(vec![TurnEnvironmentParams { environment_id: "missing".to_string(), - cwd: codex_home.path().to_path_buf().try_into()?, + cwd: ApiPathString::new(codex_home.path().to_string_lossy()), }]), ..Default::default() }) @@ -2744,7 +2745,7 @@ fn environment_params( .map(|id| { Ok(TurnEnvironmentParams { environment_id: (*id).to_string(), - cwd: cwd.to_path_buf().try_into()?, + cwd: ApiPathString::new(cwd.to_string_lossy()), }) }) .collect() diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 2adb713a0e51..a04113ffa5b6 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -66,6 +66,7 @@ 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; @@ -472,6 +473,52 @@ impl ThreadManager { Ok(()) } + /// Resolves environment-native cwd strings using metadata from each selected environment. + pub async fn resolve_native_environment_selections( + &self, + environments: impl IntoIterator, + ) -> CodexResult> { + let environments = environments.into_iter(); + let mut environment_ids = HashSet::new(); + let mut selections = Vec::with_capacity(environments.size_hint().0); + for (environment_id, cwd) in environments { + if !environment_ids.insert(environment_id.clone()) { + return Err(CodexErr::InvalidRequest(format!( + "duplicate turn environment id `{environment_id}`" + ))); + } + let environment = self + .state + .environment_manager + .get_environment(&environment_id) + .ok_or_else(|| { + CodexErr::InvalidRequest(format!( + "unknown turn environment id `{environment_id}`" + )) + })?; + let info = environment.info().await.map_err(|err| { + CodexErr::InvalidRequest(format!( + "failed to get info for environment `{environment_id}`: {err}" + )) + })?; + crate::shell::Shell::from_environment_shell_info(info.shell).map_err(|err| { + CodexErr::InvalidRequest(format!( + "failed to resolve shell for environment `{environment_id}`: {err}" + )) + })?; + let cwd = cwd.to_path_uri(info.path_convention).map_err(|err| { + CodexErr::InvalidRequest(format!( + "invalid cwd for environment `{environment_id}`: {err}" + )) + })?; + selections.push(TurnEnvironmentSelection { + environment_id, + cwd, + }); + } + 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 ddfa5e701123..df68a30c32fb 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -23,6 +23,8 @@ 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::PathConvention; use codex_utils_path_uri::PathUri; use core_test_support::PathBufExt; use core_test_support::PathExt; @@ -298,6 +300,131 @@ 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_use_selected_environment_metadata() { + 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::new(config.cwd.display().to_string()), + )]) + .await + .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::new(config.cwd.display().to_string()); + let error = manager + .resolve_native_environment_selections([ + ("local".to_string(), native_cwd.clone()), + ("local".to_string(), native_cwd), + ]) + .await + .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(), + ApiPathString::new("relative"), + )]) + .await + .expect_err("relative cwd must fail at the environment boundary"); + assert_eq!( + error.to_string(), + format!( + "invalid cwd for environment `local`: path `relative` is not absolute using {} path syntax", + PathConvention::native() + ) + ); +} + #[tokio::test] async fn start_thread_keeps_internal_threads_hidden_from_normal_lookups() { let temp_dir = tempdir().expect("tempdir"); From 2e01ad4114fc86999aa9a3174d852bd8da49d617 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sun, 14 Jun 2026 00:56:16 +0000 Subject: [PATCH 02/15] app-server: preserve foreign environment cwd syntax --- .../app-server-protocol/src/protocol/v2/tests.rs | 5 +++-- .../app-server/tests/suite/v2/thread_start.rs | 6 +++++- codex-rs/app-server/tests/suite/v2/turn_start.rs | 11 +++++++++-- codex-rs/core/src/thread_manager.rs | 16 +++++----------- codex-rs/core/src/thread_manager_tests.rs | 16 ++++++++-------- .../remote_env_windows_test.rs | 2 +- 6 files changed, 31 insertions(+), 25 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index c702f439e2ab..484c6eb85f60 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -3723,7 +3723,8 @@ fn thread_settings_update_params_preserve_field_level_experimental_gates() { #[test] fn turn_start_params_round_trip_environments() { - let cwd = ApiPathString::new(r"C:\workspace"); + let cwd: ApiPathString = + serde_json::from_value(json!(r"C:\workspace")).expect("API path should deserialize"); let params: TurnStartParams = serde_json::from_value(json!({ "threadId": "thread_123", "input": [], @@ -3823,7 +3824,7 @@ fn turn_start_params_preserve_relative_environment_cwd_for_boundary_validation() params.environments, Some(vec![TurnEnvironmentParams { environment_id: "local".to_string(), - cwd: ApiPathString::new("relative"), + cwd: serde_json::from_value(json!("relative")).expect("API path should deserialize"), }]) ); } 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 cbe0f07dc150..6b6464fd2702 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -34,6 +34,7 @@ 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 codex_utils_path_uri::PathConvention; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; @@ -294,7 +295,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::new(codex_home.path().to_string_lossy()), + cwd: ApiPathString::from_abs_path( + &codex_home.path().to_path_buf().try_into()?, + PathConvention::native(), + )?, }]), ..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 663a8685191a..e82c97bacaf5 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -77,6 +77,7 @@ 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_path_uri::PathConvention; use core_test_support::responses; use core_test_support::skip_if_no_network; use pretty_assertions::assert_eq; @@ -1327,7 +1328,10 @@ async fn turn_start_rejects_unknown_environment_before_starting_turn() -> Result }], environments: Some(vec![TurnEnvironmentParams { environment_id: "missing".to_string(), - cwd: ApiPathString::new(codex_home.path().to_string_lossy()), + cwd: ApiPathString::from_abs_path( + &codex_home.path().to_path_buf().try_into()?, + PathConvention::native(), + )?, }]), ..Default::default() }) @@ -2745,7 +2749,10 @@ fn environment_params( .map(|id| { Ok(TurnEnvironmentParams { environment_id: (*id).to_string(), - cwd: ApiPathString::new(cwd.to_string_lossy()), + cwd: ApiPathString::from_abs_path( + &cwd.to_path_buf().try_into()?, + PathConvention::native(), + )?, }) }) .collect() diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index a04113ffa5b6..74ebac84feda 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -473,7 +473,7 @@ impl ThreadManager { Ok(()) } - /// Resolves environment-native cwd strings using metadata from each selected environment. + /// Resolves environment-native cwd strings without applying the Codex host's path rules. pub async fn resolve_native_environment_selections( &self, environments: impl IntoIterator, @@ -487,8 +487,7 @@ impl ThreadManager { "duplicate turn environment id `{environment_id}`" ))); } - let environment = self - .state + self.state .environment_manager .get_environment(&environment_id) .ok_or_else(|| { @@ -496,17 +495,12 @@ impl ThreadManager { "unknown turn environment id `{environment_id}`" )) })?; - let info = environment.info().await.map_err(|err| { - CodexErr::InvalidRequest(format!( - "failed to get info for environment `{environment_id}`: {err}" - )) - })?; - crate::shell::Shell::from_environment_shell_info(info.shell).map_err(|err| { + let convention = cwd.infer_absolute_path_convention().ok_or_else(|| { CodexErr::InvalidRequest(format!( - "failed to resolve shell for environment `{environment_id}`: {err}" + "invalid cwd for environment `{environment_id}`: path `{cwd}` does not use absolute POSIX or Windows path syntax" )) })?; - let cwd = cwd.to_path_uri(info.path_convention).map_err(|err| { + let cwd = cwd.to_path_uri(convention).map_err(|err| { CodexErr::InvalidRequest(format!( "invalid cwd for environment `{environment_id}`: {err}" )) diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index df68a30c32fb..13f9d6738f5d 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -353,7 +353,7 @@ async fn start_thread_rejects_explicit_local_environment_when_default_provider_i } #[tokio::test] -async fn native_environment_selections_use_selected_environment_metadata() { +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(); @@ -370,7 +370,8 @@ async fn native_environment_selections_use_selected_environment_metadata() { let selections = manager .resolve_native_environment_selections([( "local".to_string(), - ApiPathString::new(config.cwd.display().to_string()), + ApiPathString::from_abs_path(&config.cwd, PathConvention::native()) + .expect("render native cwd"), )]) .await .expect("resolve local selection"); @@ -398,7 +399,8 @@ async fn native_environment_selections_reject_duplicate_ids() { config.codex_home.to_path_buf(), Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ); - let native_cwd = ApiPathString::new(config.cwd.display().to_string()); + let native_cwd = ApiPathString::from_abs_path(&config.cwd, PathConvention::native()) + .expect("render native cwd"); let error = manager .resolve_native_environment_selections([ ("local".to_string(), native_cwd.clone()), @@ -412,16 +414,14 @@ async fn native_environment_selections_reject_duplicate_ids() { let error = manager .resolve_native_environment_selections([( "local".to_string(), - ApiPathString::new("relative"), + serde_json::from_value(serde_json::json!("relative")) + .expect("API path should deserialize"), )]) .await .expect_err("relative cwd must fail at the environment boundary"); assert_eq!( error.to_string(), - format!( - "invalid cwd for environment `local`: path `relative` is not absolute using {} path syntax", - PathConvention::native() - ) + "invalid cwd for environment `local`: path `relative` does not use absolute POSIX or Windows path syntax" ); } 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 24efb8e7ed92..26de7d3a8130 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 @@ -162,7 +162,7 @@ async fn windows_exec_server_runs_with_native_shell_and_cwd() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn app_server_rejects_windows_environment_cwd() -> Result<()> { +async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Result<()> { WineExecServer .scope(|exec_server_url| async move { let codex_home = TempDir::new()?; From f2bbee127d5779f9b026cdf5bde4344938839ed2 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sun, 14 Jun 2026 01:20:31 +0000 Subject: [PATCH 03/15] app-server: address environment cwd review feedback --- .../src/protocol/v2/tests.rs | 31 ++--- codex-rs/app-server/README.md | 2 +- codex-rs/app-server/src/request_processors.rs | 1 - .../app-server/tests/suite/v2/thread_start.rs | 36 ++++++ codex-rs/core/src/thread_manager.rs | 17 +-- codex-rs/core/src/thread_manager_tests.rs | 3 - .../remote_env_windows_test.rs | 113 ++++++++++++++++-- 7 files changed, 150 insertions(+), 53 deletions(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index 484c6eb85f60..1359a47be152 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -3723,8 +3723,14 @@ fn thread_settings_update_params_preserve_field_level_experimental_gates() { #[test] fn turn_start_params_round_trip_environments() { + // Use a path foreign to the test host so this exercises syntax preservation instead of the + // host-native conversion performed by test_absolute_path(). + #[cfg(windows)] + let raw_cwd = "/workspace"; + #[cfg(not(windows))] + let raw_cwd = r"C:\workspace"; let cwd: ApiPathString = - serde_json::from_value(json!(r"C:\workspace")).expect("API path should deserialize"); + serde_json::from_value(json!(raw_cwd)).expect("API path should deserialize"); let params: TurnStartParams = serde_json::from_value(json!({ "threadId": "thread_123", "input": [], @@ -3806,29 +3812,6 @@ fn turn_start_params_treat_null_or_omitted_environments_as_default() { ); } -#[test] -fn turn_start_params_preserve_relative_environment_cwd_for_boundary_validation() { - let params = serde_json::from_value::(json!({ - "threadId": "thread_123", - "input": [], - "environments": [ - { - "environmentId": "local", - "cwd": "relative" - } - ], - })) - .expect("native cwd should deserialize without applying host rules"); - - assert_eq!( - params.environments, - Some(vec![TurnEnvironmentParams { - environment_id: "local".to_string(), - cwd: serde_json::from_value(json!("relative")).expect("API path should deserialize"), - }]) - ); -} - #[test] fn realtime_append_text_defaults_role_to_user() { let params = serde_json::from_value::(json!({ diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 706b706d2e5e..5bf23c6c1044 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -709,7 +709,7 @@ Turns attach user input (text or images) to a thread and trigger Codex generatio - `{"type":"image","url":"https://…png"}` - `{"type":"localImage","path":"/tmp/screenshot.png"}` -You can optionally specify config overrides on the new turn. If specified, these settings become the default for subsequent turns on the same thread. `outputSchema` applies only to the current turn. Experimental `environments` is turn-scoped: omit it to inherit the thread's sticky environments, pass `[]` to run the turn with no environments, or pass explicit environment ids to override the sticky selection for this turn only. Each environment `cwd` must be absolute and use that environment's native path convention, so a Windows environment uses a value such as `C:\\workspace` even when app-server runs on Linux. +You can optionally specify config overrides on the new turn. If specified, these settings become the default for subsequent turns on the same thread. `outputSchema` applies only to the current turn. Experimental `environments` is turn-scoped: omit it to inherit the thread's sticky environments, pass `[]` to run the turn with no environments, or pass explicit environment objects to override the sticky selection for this turn only. A `cwd` is allowed on each environment object. `approvalsReviewer` accepts: diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index c731b9a9b1a2..bf9843c53ed8 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -554,7 +554,6 @@ async fn resolve_turn_environment_selections( .into_iter() .map(|environment| (environment.environment_id, environment.cwd)), ) - .await .map(Some) .map_err(environment_selection_error) } 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 6b6464fd2702..e11869adc8bb 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -317,6 +317,42 @@ async fn thread_start_rejects_unknown_environment_as_invalid_request() -> Result Ok(()) } +#[tokio::test] +async fn thread_start_rejects_relative_environment_cwd_as_invalid_request() -> 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 mut mcp = TestAppServer::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_raw_request( + "thread/start", + Some(json!({ + "environments": [{ + "environmentId": "local", + "cwd": "relative", + }], + })), + ) + .await?; + let error: JSONRPCError = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_error_message(RequestId::Integer(request_id)), + ) + .await??; + + assert_eq!(error.id, RequestId::Integer(request_id)); + assert_eq!(error.error.code, INVALID_REQUEST_ERROR_CODE); + assert_eq!( + error.error.message, + "invalid cwd for environment `local`: path `relative` does not use absolute POSIX or Windows path syntax" + ); + + Ok(()) +} + #[tokio::test] async fn thread_start_response_includes_loaded_instruction_sources() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 74ebac84feda..091bbe3edf8f 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -474,27 +474,13 @@ impl ThreadManager { } /// Resolves environment-native cwd strings without applying the Codex host's path rules. - pub async fn resolve_native_environment_selections( + pub fn resolve_native_environment_selections( &self, environments: impl IntoIterator, ) -> CodexResult> { let environments = environments.into_iter(); - let mut environment_ids = HashSet::new(); let mut selections = Vec::with_capacity(environments.size_hint().0); for (environment_id, cwd) in environments { - if !environment_ids.insert(environment_id.clone()) { - return Err(CodexErr::InvalidRequest(format!( - "duplicate turn environment id `{environment_id}`" - ))); - } - self.state - .environment_manager - .get_environment(&environment_id) - .ok_or_else(|| { - CodexErr::InvalidRequest(format!( - "unknown turn environment id `{environment_id}`" - )) - })?; 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" @@ -510,6 +496,7 @@ impl ThreadManager { cwd, }); } + self.validate_environment_selections(&selections)?; Ok(selections) } diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 13f9d6738f5d..79217968d239 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -373,7 +373,6 @@ async fn native_environment_selections_preserve_selected_path_syntax() { ApiPathString::from_abs_path(&config.cwd, PathConvention::native()) .expect("render native cwd"), )]) - .await .expect("resolve local selection"); assert_eq!( @@ -406,7 +405,6 @@ async fn native_environment_selections_reject_duplicate_ids() { ("local".to_string(), native_cwd.clone()), ("local".to_string(), native_cwd), ]) - .await .expect_err("duplicate ids must fail"); assert_eq!(error.to_string(), "duplicate turn environment id `local`"); @@ -417,7 +415,6 @@ async fn native_environment_selections_reject_duplicate_ids() { serde_json::from_value(serde_json::json!("relative")) .expect("API path should deserialize"), )]) - .await .expect_err("relative cwd must fail at the environment boundary"); assert_eq!( error.to_string(), 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 26de7d3a8130..404bcaef4678 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 @@ -3,8 +3,15 @@ use anyhow::Context; use anyhow::Result; use app_test_support::TestAppServer; -use codex_app_server_protocol::JSONRPCError; +use app_test_support::create_mock_responses_server_sequence; +use app_test_support::to_response; +use app_test_support::write_mock_responses_config_toml; +use codex_app_server_protocol::CommandExecutionStatus; +use codex_app_server_protocol::ItemCompletedNotification; +use codex_app_server_protocol::JSONRPCMessage; use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ThreadItem; +use codex_app_server_protocol::ThreadStartResponse; use codex_exec_server::REMOTE_ENVIRONMENT_ID; use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_features::Feature; @@ -29,6 +36,7 @@ use core_test_support::wait_for_event; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use serde_json::json; +use std::collections::BTreeMap; use tempfile::TempDir; use tokio::time::timeout; use wine_exec_server_test_support::WineExecServer; @@ -166,6 +174,36 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul WineExecServer .scope(|exec_server_url| async move { let codex_home = TempDir::new()?; + let responses = vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_function_call( + CALL_ID, + "exec_command", + &serde_json::to_string(&json!({ + "cmd": COMMAND, + "login": false, + "yield_time_ms": 5_000, + }))?, + ), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]), + ]; + let server = create_mock_responses_server_sequence(responses).await; + write_mock_responses_config_toml( + codex_home.path(), + &server.uri(), + &BTreeMap::from([(Feature::UnifiedExec, true)]), + 100_000, + /*requires_openai_auth*/ None, + "mock", + "compact", + )?; let mut app_server = TestAppServer::new_with_env( codex_home.path(), &[( @@ -187,18 +225,75 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul })), ) .await?; - let error: JSONRPCError = timeout( + let request_id = RequestId::Integer(request_id); + let message = timeout(APP_SERVER_READ_TIMEOUT, async { + loop { + let message = app_server.read_next_message().await?; + match &message { + JSONRPCMessage::Response(response) if response.id == request_id => { + return Ok::(message); + } + JSONRPCMessage::Error(error) if error.id == request_id => { + return Ok(message); + } + _ => {} + } + } + }) + .await??; + let JSONRPCMessage::Response(response) = message else { + panic!("thread/start should succeed, got {message:?}"); + }; + let response: ThreadStartResponse = to_response(response)?; + assert!(!response.thread.id.is_empty()); + + let turn_request_id = app_server + .send_raw_request( + "turn/start", + Some(json!({ + "threadId": response.thread.id, + "input": [{"type": "text", "text": "run the Windows smoke command"}], + "approvalPolicy": "never", + "sandboxPolicy": {"type": "dangerFullAccess"}, + })), + ) + .await?; + timeout( APP_SERVER_READ_TIMEOUT, - app_server.read_stream_until_error_message(RequestId::Integer(request_id)), + app_server.read_stream_until_response_message(RequestId::Integer(turn_request_id)), ) .await??; - assert_eq!(error.id, RequestId::Integer(request_id)); - assert_eq!(error.error.code, INVALID_REQUEST_ERROR_CODE); - assert_eq!( - error.error.message, - "Invalid request: AbsolutePathBuf deserialized without a base path" - ); + let completed = timeout(APP_SERVER_READ_TIMEOUT, async { + loop { + let notification = app_server + .read_stream_until_notification_message("item/completed") + .await?; + let completed: ItemCompletedNotification = serde_json::from_value( + notification.params.context("item/completed params")?, + )?; + if matches!(completed.item, ThreadItem::CommandExecution { .. }) { + return Ok::(completed.item); + } + } + }) + .await??; + let ThreadItem::CommandExecution { + id, + status, + exit_code, + .. + } = completed + else { + unreachable!("loop returns only command execution items"); + }; + assert_eq!(id, CALL_ID); + assert_eq!((status, exit_code), (CommandExecutionStatus::Completed, Some(0))); + timeout( + APP_SERVER_READ_TIMEOUT, + app_server.read_stream_until_notification_message("turn/completed"), + ) + .await??; Ok(()) }) From 5af27916d7a17db5a23fb2f1ecd6544b4ba91af6 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sun, 14 Jun 2026 01:27:12 +0000 Subject: [PATCH 04/15] tests: use typed remote environment thread start --- .../remote_env_windows_test.rs | 42 +++++++------------ 1 file changed, 14 insertions(+), 28 deletions(-) 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 404bcaef4678..a94cf8e15bd2 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 @@ -8,10 +8,11 @@ use app_test_support::to_response; use app_test_support::write_mock_responses_config_toml; use codex_app_server_protocol::CommandExecutionStatus; use codex_app_server_protocol::ItemCompletedNotification; -use codex_app_server_protocol::JSONRPCMessage; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::ThreadItem; +use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; +use codex_app_server_protocol::TurnEnvironmentParams; use codex_exec_server::REMOTE_ENVIRONMENT_ID; use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_features::Feature; @@ -33,6 +34,7 @@ use core_test_support::responses::start_mock_server; use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; +use codex_utils_path_uri::ApiPathString; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use serde_json::json; @@ -215,35 +217,19 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul timeout(APP_SERVER_READ_TIMEOUT, app_server.initialize()).await??; let request_id = app_server - .send_raw_request( - "thread/start", - Some(json!({ - "environments": [{ - "environmentId": REMOTE_ENVIRONMENT_ID, - "cwd": r"C:\windows", - }], - })), - ) + .send_thread_start_request(ThreadStartParams { + environments: Some(vec![TurnEnvironmentParams { + environment_id: REMOTE_ENVIRONMENT_ID.to_string(), + cwd: serde_json::from_value::(json!(r"C:\windows"))?, + }]), + ..Default::default() + }) .await?; - let request_id = RequestId::Integer(request_id); - let message = timeout(APP_SERVER_READ_TIMEOUT, async { - loop { - let message = app_server.read_next_message().await?; - match &message { - JSONRPCMessage::Response(response) if response.id == request_id => { - return Ok::(message); - } - JSONRPCMessage::Error(error) if error.id == request_id => { - return Ok(message); - } - _ => {} - } - } - }) + let response = timeout( + APP_SERVER_READ_TIMEOUT, + app_server.read_stream_until_response_message(RequestId::Integer(request_id)), + ) .await??; - let JSONRPCMessage::Response(response) = message else { - panic!("thread/start should succeed, got {message:?}"); - }; let response: ThreadStartResponse = to_response(response)?; assert!(!response.thread.id.is_empty()); From f50a181c985761ba17f0c8f5cca82d546a06e5fa Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sun, 14 Jun 2026 01:36:47 +0000 Subject: [PATCH 05/15] tests: type remote Windows turn start --- .../remote_env_windows_test.rs | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) 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 a94cf8e15bd2..d91199c36d1a 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 @@ -2,17 +2,23 @@ use anyhow::Context; use anyhow::Result; +use app_test_support::PathBufExt; use app_test_support::TestAppServer; use app_test_support::create_mock_responses_server_sequence; use app_test_support::to_response; use app_test_support::write_mock_responses_config_toml; +use codex_app_server_protocol::AskForApproval as AppAskForApproval; use codex_app_server_protocol::CommandExecutionStatus; use codex_app_server_protocol::ItemCompletedNotification; use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::SandboxPolicy; use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnEnvironmentParams; +use codex_app_server_protocol::TurnStartParams; +use codex_app_server_protocol::TurnStartResponse; +use codex_app_server_protocol::UserInput as V2UserInput; use codex_exec_server::REMOTE_ENVIRONMENT_ID; use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_features::Feature; @@ -44,12 +50,12 @@ use tokio::time::timeout; use wine_exec_server_test_support::WineExecServer; const APP_SERVER_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); -const CALL_ID: &str = "wine-cmd-smoke"; -const COMMAND: &str = r#"if ((Get-Location).Path -ne 'C:\windows') { exit 1 }"#; -const INVALID_REQUEST_ERROR_CODE: i64 = -32600; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn windows_exec_server_runs_with_native_shell_and_cwd() -> Result<()> { + const CALL_ID: &str = "wine-cmd-smoke"; + const COMMAND: &str = r#"if ((Get-Location).Path -ne 'C:\windows') { exit 1 }"#; + WineExecServer .scope(|exec_server_url| async move { let server = start_mock_server().await; @@ -173,6 +179,9 @@ async fn windows_exec_server_runs_with_native_shell_and_cwd() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Result<()> { + const CALL_ID: &str = "wine-cmd-smoke"; + const COMMAND: &str = r#"if ((Get-Location).Path -ne 'C:\windows') { exit 1 }"#; + WineExecServer .scope(|exec_server_url| async move { let codex_home = TempDir::new()?; @@ -232,23 +241,33 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul .await??; let response: ThreadStartResponse = to_response(response)?; assert!(!response.thread.id.is_empty()); + let host_cwd = codex_home.path().to_path_buf().abs(); + // TODO(anp): Return environment-native values here once these app-server response + // fields support foreign paths and remote instruction/profile discovery. + assert_eq!(response.cwd, host_cwd); + assert_eq!(response.runtime_workspace_roots, vec![host_cwd]); + assert_eq!(response.instruction_sources, Vec::new()); + assert_eq!(response.active_permission_profile, None); let turn_request_id = app_server - .send_raw_request( - "turn/start", - Some(json!({ - "threadId": response.thread.id, - "input": [{"type": "text", "text": "run the Windows smoke command"}], - "approvalPolicy": "never", - "sandboxPolicy": {"type": "dangerFullAccess"}, - })), - ) + .send_turn_start_request(TurnStartParams { + thread_id: response.thread.id, + client_user_message_id: None, + input: vec![V2UserInput::Text { + text: "run the Windows smoke command".to_string(), + text_elements: Vec::new(), + }], + approval_policy: Some(AppAskForApproval::Never), + sandbox_policy: Some(SandboxPolicy::DangerFullAccess), + ..Default::default() + }) .await?; - timeout( + let turn_response = timeout( APP_SERVER_READ_TIMEOUT, app_server.read_stream_until_response_message(RequestId::Integer(turn_request_id)), ) .await??; + let _: TurnStartResponse = to_response(turn_response)?; let completed = timeout(APP_SERVER_READ_TIMEOUT, async { loop { From fc8e49af873aa832abe7faa72a2034c82dc6ed1f Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sun, 14 Jun 2026 01:41:02 +0000 Subject: [PATCH 06/15] tests: seed remote Windows workspace instructions --- .../core/tests/remote_env_windows/BUILD.bazel | 1 + .../remote_env_windows_test.rs | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/codex-rs/core/tests/remote_env_windows/BUILD.bazel b/codex-rs/core/tests/remote_env_windows/BUILD.bazel index 9b5ce52bd922..5e213ac14be4 100644 --- a/codex-rs/core/tests/remote_env_windows/BUILD.bazel +++ b/codex-rs/core/tests/remote_env_windows/BUILD.bazel @@ -22,6 +22,7 @@ wine_rust_test( "//codex-rs/protocol", "//codex-rs/utils/path-uri", "@crates//:anyhow", + "@crates//:base64", "@crates//:pretty_assertions", "@crates//:serde_json", "@crates//:tempfile", 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 d91199c36d1a..ede1bac9339e 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 @@ -7,6 +7,8 @@ use app_test_support::TestAppServer; use app_test_support::create_mock_responses_server_sequence; use app_test_support::to_response; use app_test_support::write_mock_responses_config_toml; +use base64::Engine as _; +use base64::engine::general_purpose::STANDARD; use codex_app_server_protocol::AskForApproval as AppAskForApproval; use codex_app_server_protocol::CommandExecutionStatus; use codex_app_server_protocol::ItemCompletedNotification; @@ -21,6 +23,9 @@ use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::UserInput as V2UserInput; use codex_exec_server::REMOTE_ENVIRONMENT_ID; use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; +use codex_exec_server::ExecServerClient; +use codex_exec_server::FsWriteFileParams; +use codex_exec_server::RemoteExecServerConnectArgs; use codex_features::Feature; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; @@ -179,11 +184,30 @@ async fn windows_exec_server_runs_with_native_shell_and_cwd() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Result<()> { + const AGENTS_INSTRUCTIONS: &str = "remote Windows workspace instructions"; const CALL_ID: &str = "wine-cmd-smoke"; const COMMAND: &str = r#"if ((Get-Location).Path -ne 'C:\windows') { exit 1 }"#; WineExecServer .scope(|exec_server_url| async move { + let exec_server_client = ExecServerClient::connect_websocket( + RemoteExecServerConnectArgs { + websocket_url: exec_server_url.clone(), + client_name: "remote-env-windows-test".to_string(), + connect_timeout: APP_SERVER_READ_TIMEOUT, + initialize_timeout: APP_SERVER_READ_TIMEOUT, + resume_session_id: None, + }, + ) + .await?; + exec_server_client + .fs_write_file(FsWriteFileParams { + path: PathUri::parse("file:///C:/windows/AGENTS.md")?, + data_base64: STANDARD.encode(AGENTS_INSTRUCTIONS), + sandbox: None, + }) + .await?; + let codex_home = TempDir::new()?; let responses = vec![ sse(vec![ From 629edc98f197cfeaacd185e1fcb04a9ccd18fa96 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sun, 14 Jun 2026 01:46:40 +0000 Subject: [PATCH 07/15] tests: split remote environment follow-up TODOs --- .../tests/remote_env_windows/remote_env_windows_test.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 ede1bac9339e..0237c38b3018 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 @@ -266,11 +266,13 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul let response: ThreadStartResponse = to_response(response)?; assert!(!response.thread.id.is_empty()); let host_cwd = codex_home.path().to_path_buf().abs(); - // TODO(anp): Return environment-native values here once these app-server response - // fields support foreign paths and remote instruction/profile discovery. + // TODO(anp): Return the selected environment's native cwd from thread/start. assert_eq!(response.cwd, host_cwd); + // TODO(anp): Derive runtime workspace roots from the selected remote environment. assert_eq!(response.runtime_workspace_roots, vec![host_cwd]); + // TODO(anp): Discover and report instruction sources from the remote filesystem. assert_eq!(response.instruction_sources, Vec::new()); + // TODO(anp): Report the implicit built-in permission profile instead of None. assert_eq!(response.active_permission_profile, None); let turn_request_id = app_server From e050805529c127a3cf5c1d2eaad7fad70b39bb4a Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sun, 14 Jun 2026 02:28:38 +0000 Subject: [PATCH 08/15] codex: address PR review feedback (#28146) --- codex-rs/app-server/README.md | 2 +- .../app-server/tests/suite/v2/thread_start.rs | 16 ++++---- codex-rs/core/src/agents_md_tests.rs | 3 +- codex-rs/core/src/environment_selection.rs | 37 +++++++++++-------- codex-rs/core/src/session/tests.rs | 17 +++++---- codex-rs/core/src/session/turn_context.rs | 10 ++--- .../core/src/tools/handlers/view_image.rs | 5 ++- .../src/tools/runtimes/apply_patch_tests.rs | 3 +- codex-rs/core/src/tools/spec_plan_tests.rs | 5 ++- .../remote_env_windows_test.rs | 11 +++++- 10 files changed, 63 insertions(+), 46 deletions(-) diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index 5bf23c6c1044..336f769d7155 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -709,7 +709,7 @@ Turns attach user input (text or images) to a thread and trigger Codex generatio - `{"type":"image","url":"https://…png"}` - `{"type":"localImage","path":"/tmp/screenshot.png"}` -You can optionally specify config overrides on the new turn. If specified, these settings become the default for subsequent turns on the same thread. `outputSchema` applies only to the current turn. Experimental `environments` is turn-scoped: omit it to inherit the thread's sticky environments, pass `[]` to run the turn with no environments, or pass explicit environment objects to override the sticky selection for this turn only. A `cwd` is allowed on each environment object. +You can optionally specify config overrides on the new turn. If specified, these settings become the default for subsequent turns on the same thread. `outputSchema` applies only to the current turn. Experimental `environments` is turn-scoped: omit it to inherit the thread's sticky environments, pass `[]` to run the turn with no environments, or pass explicit environment ids to override the sticky selection for this turn only. `approvalsReviewer` accepts: 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 e11869adc8bb..480058783042 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -327,15 +327,13 @@ async fn thread_start_rejects_relative_environment_cwd_as_invalid_request() -> R timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; let request_id = mcp - .send_raw_request( - "thread/start", - Some(json!({ - "environments": [{ - "environmentId": "local", - "cwd": "relative", - }], - })), - ) + .send_thread_start_request(ThreadStartParams { + environments: Some(vec![TurnEnvironmentParams { + environment_id: "local".to_string(), + cwd: serde_json::from_value(json!("relative"))?, + }]), + ..Default::default() + }) .await?; let error: JSONRPCError = timeout( DEFAULT_READ_TIMEOUT, diff --git a/codex-rs/core/src/agents_md_tests.rs b/codex-rs/core/src/agents_md_tests.rs index ffe12d52cc8e..be3afdc3adb0 100644 --- a/codex-rs/core/src/agents_md_tests.rs +++ b/codex-rs/core/src/agents_md_tests.rs @@ -267,9 +267,10 @@ fn resolved_local_environments( Environment::create_for_tests(/*exec_server_url*/ None) .expect("local environment"), ), - cwd, + PathUri::from_abs_path(&cwd), /*shell*/ None, ) + .expect("turn environment") }) .collect(), } diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index 8fb59ba92532..c1a5edd3ad0e 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -157,14 +157,15 @@ impl ThreadEnvironments { let mut turn_environment = TurnEnvironment::new( environment_id, environment, - selected_environment.cwd.to_abs_path().map_err(|err| { - CodexErr::InvalidRequest(format!( - "turn environment cwd `{}` is not valid on this host: {err}", - selected_environment.cwd - )) - })?, + 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()) @@ -560,7 +561,7 @@ url = "ws://127.0.0.1:8765" Arc::clone(&local_manager), &[TurnEnvironmentSelection { environment_id: LOCAL_ENVIRONMENT_ID.to_string(), - cwd: cwd_uri, + cwd: cwd_uri.clone(), }], ) .await; @@ -570,12 +571,15 @@ 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.clone(), - /*shell*/ None, - )], + turn_environments: vec![ + TurnEnvironment::new( + REMOTE_ENVIRONMENT_ID.to_string(), + remote_environment.clone(), + cwd_uri.clone(), + /*shell*/ None, + ) + .expect("remote turn environment"), + ], }; let multiple = TurnEnvironmentSnapshot { turn_environments: vec![ @@ -583,9 +587,10 @@ url = "ws://127.0.0.1:8765" TurnEnvironment::new( REMOTE_ENVIRONMENT_ID.to_string(), remote_environment, - cwd.clone(), + cwd_uri, /*shell*/ None, - ), + ) + .expect("remote turn environment"), ], }; diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 5086fd63bd57..dec6feceab7a 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -5697,9 +5697,10 @@ 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, - environment_cwd.clone(), + codex_utils_path_uri::PathUri::from_abs_path(&environment_cwd), current_environment.shell, - ); + ) + .expect("remote turn environment"); let call_id = "call-1".to_string(); let handler = RequestPermissionsHandler; @@ -6316,15 +6317,15 @@ 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( + turn_context.environments.turn_environments.push( + TurnEnvironment::new( "second".to_string(), Arc::clone(&first_environment.environment), - second_cwd.clone(), + codex_utils_path_uri::PathUri::from_abs_path(&second_cwd), /*shell*/ None, - )); + ) + .expect("second turn environment"), + ); assert_eq!( turn_context diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 042eca89295e..8df2a7514eab 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -64,18 +64,18 @@ impl TurnEnvironment { pub(crate) fn new( environment_id: String, environment: Arc, - cwd: AbsolutePathBuf, + cwd_uri: PathUri, shell: Option, - ) -> Self { - let cwd_uri = PathUri::from_abs_path(&cwd); - Self { + ) -> std::io::Result { + let cwd = cwd_uri.to_abs_path()?; + Ok(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 { diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index dc52e7e336c7..f2e16f0f58d0 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -288,9 +288,10 @@ mod tests { turn.environments.turn_environments[0] = TurnEnvironment::new( current.environment_id, current.environment, - cwd, + PathUri::from_abs_path(&cwd), current.shell, - ); + ) + .expect("turn environment"); } #[test] 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 4100b662ee25..a181502f0b43 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -19,9 +19,10 @@ fn test_turn_environment(environment_id: &str) -> crate::session::turn_context:: crate::session::turn_context::TurnEnvironment::new( environment_id.to_string(), std::sync::Arc::new(codex_exec_server::Environment::default_for_tests()), - std::env::temp_dir().abs(), + PathUri::from_abs_path(&std::env::temp_dir().abs()), /*shell*/ None, ) + .expect("turn environment") } #[test] diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 5faee28682bd..cbe282ec393c 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -597,9 +597,10 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava )) .expect("remote test environment"), ), - remote_cwd, + codex_utils_path_uri::PathUri::from_abs_path(&remote_cwd), /*shell*/ None, - ), + ) + .expect("remote turn environment"), ); }) .await; 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 0237c38b3018..7ab29309b82b 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 @@ -10,6 +10,7 @@ use app_test_support::write_mock_responses_config_toml; use base64::Engine as _; use base64::engine::general_purpose::STANDARD; use codex_app_server_protocol::AskForApproval as AppAskForApproval; +use codex_app_server_protocol::CommandAction; use codex_app_server_protocol::CommandExecutionStatus; use codex_app_server_protocol::ItemCompletedNotification; use codex_app_server_protocol::RequestId; @@ -186,7 +187,7 @@ async fn windows_exec_server_runs_with_native_shell_and_cwd() -> Result<()> { async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Result<()> { const AGENTS_INSTRUCTIONS: &str = "remote Windows workspace instructions"; const CALL_ID: &str = "wine-cmd-smoke"; - const COMMAND: &str = r#"if ((Get-Location).Path -ne 'C:\windows') { exit 1 }"#; + const COMMAND: &str = "Get-Content AGENTS.md -ErrorAction Stop"; WineExecServer .scope(|exec_server_url| async move { @@ -310,6 +311,8 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul }) .await??; let ThreadItem::CommandExecution { + command_actions, + cwd, id, status, exit_code, @@ -319,6 +322,12 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul unreachable!("loop returns only command execution items"); }; assert_eq!(id, CALL_ID); + // TODO(anp): Make command execution cwd a PathUri so this stays `C:\windows`. + assert_eq!(cwd, std::path::PathBuf::from("/C:/windows").abs()); + // TODO(anp): Parse command actions using the selected environment's path convention so + // their paths remain Windows-native instead of degrading the action to Unknown. + assert_eq!(command_actions.len(), 1); + assert!(matches!(command_actions[0], CommandAction::Unknown { .. })); assert_eq!((status, exit_code), (CommandExecutionStatus::Completed, Some(0))); timeout( APP_SERVER_READ_TIMEOUT, From 5f128f9dba77b40cf0abd7e5ad46d2d63f8fee18 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sun, 14 Jun 2026 02:53:24 +0000 Subject: [PATCH 09/15] tests: record missing remote workspace instructions --- .../remote_env_windows/remote_env_windows_test.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) 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 7ab29309b82b..1ea7d293cede 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 @@ -335,6 +335,18 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul ) .await??; + let requests = server + .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)) + }); + // TODO(anp): Load remote workspace instructions into the model context. + assert!(!model_request_includes_remote_instructions); + Ok(()) }) .await From 1e8b407276608a6379d5cafd24e68b4430d07092 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sun, 14 Jun 2026 06:34:17 +0000 Subject: [PATCH 10/15] codex: fix rebase against #28122 --- .../src/request_processors/thread_processor.rs | 1 - .../src/request_processors/turn_processor.rs | 1 - codex-rs/app-server/tests/suite/v2/thread_start.rs | 6 +----- codex-rs/app-server/tests/suite/v2/turn_start.rs | 11 ++--------- codex-rs/core/src/thread_manager_tests.rs | 7 ++----- 5 files changed, 5 insertions(+), 21 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/thread_processor.rs b/codex-rs/app-server/src/request_processors/thread_processor.rs index 89b7259d4887..747e1a091b08 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -4,7 +4,6 @@ use codex_app_server_protocol::SelectedCapabilityRoot; use codex_extension_api::ExtensionDataInit; use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_DANGER_FULL_ACCESS; use codex_protocol::models::BUILT_IN_PERMISSION_PROFILE_WORKSPACE; -use codex_utils_path_uri::PathUri; const THREAD_LIST_DEFAULT_LIMIT: usize = 25; const THREAD_LIST_MAX_LIMIT: usize = 100; diff --git a/codex-rs/app-server/src/request_processors/turn_processor.rs b/codex-rs/app-server/src/request_processors/turn_processor.rs index c15a4e05ab73..66c4ca895658 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -4,7 +4,6 @@ use codex_protocol::protocol::AdditionalContextKind as CoreAdditionalContextKind use codex_protocol::protocol::MultiAgentVersion; use codex_protocol::protocol::SessionSource; use codex_protocol::protocol::SubAgentSource; -use codex_utils_path_uri::PathUri; const DIRECT_INPUT_TO_MULTI_AGENT_V2_SUBAGENT_ERROR: &str = "direct app-server input is not allowed for multi-agent v2 sub-agents"; 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 480058783042..93dec41463c3 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_start.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_start.rs @@ -34,7 +34,6 @@ 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 codex_utils_path_uri::PathConvention; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; @@ -295,10 +294,7 @@ 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()?, - PathConvention::native(), - )?, + cwd: ApiPathString::from_abs_path(&codex_home.path().to_path_buf().try_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 e82c97bacaf5..22c5269ac3ac 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -77,7 +77,6 @@ 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_path_uri::PathConvention; use core_test_support::responses; use core_test_support::skip_if_no_network; use pretty_assertions::assert_eq; @@ -1328,10 +1327,7 @@ 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()?, - PathConvention::native(), - )?, + cwd: ApiPathString::from_abs_path(&codex_home.path().to_path_buf().try_into()?), }]), ..Default::default() }) @@ -2749,10 +2745,7 @@ fn environment_params( .map(|id| { Ok(TurnEnvironmentParams { environment_id: (*id).to_string(), - cwd: ApiPathString::from_abs_path( - &cwd.to_path_buf().try_into()?, - PathConvention::native(), - )?, + cwd: ApiPathString::from_abs_path(&cwd.to_path_buf().try_into()?), }) }) .collect() diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 79217968d239..437575a41e09 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -24,7 +24,6 @@ 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::PathConvention; use codex_utils_path_uri::PathUri; use core_test_support::PathBufExt; use core_test_support::PathExt; @@ -370,8 +369,7 @@ async fn native_environment_selections_preserve_selected_path_syntax() { let selections = manager .resolve_native_environment_selections([( "local".to_string(), - ApiPathString::from_abs_path(&config.cwd, PathConvention::native()) - .expect("render native cwd"), + ApiPathString::from_abs_path(&config.cwd), )]) .expect("resolve local selection"); @@ -398,8 +396,7 @@ async fn native_environment_selections_reject_duplicate_ids() { config.codex_home.to_path_buf(), Arc::new(codex_exec_server::EnvironmentManager::default_for_tests()), ); - let native_cwd = ApiPathString::from_abs_path(&config.cwd, PathConvention::native()) - .expect("render native cwd"); + let native_cwd = ApiPathString::from_abs_path(&config.cwd); let error = manager .resolve_native_environment_selections([ ("local".to_string(), native_cwd.clone()), From 3d9446fe68679a709efc5f31edfc8d8bbd08f7e5 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Tue, 16 Jun 2026 00:03:25 +0000 Subject: [PATCH 11/15] codex: address PR review feedback (#28146) --- codex-rs/app-server/src/command_exec.rs | 3 + codex-rs/app-server/src/request_processors.rs | 32 ++++- .../app-server/tests/suite/v2/thread_start.rs | 6 +- .../app-server/tests/suite/v2/turn_start.rs | 9 +- codex-rs/core/src/agents_md.rs | 7 +- codex-rs/core/src/agents_md_tests.rs | 1 - .../core/src/context/environment_context.rs | 20 +-- codex-rs/core/src/environment_selection.rs | 36 ++--- codex-rs/core/src/exec.rs | 1 + codex-rs/core/src/guardian/review_session.rs | 4 +- codex-rs/core/src/sandboxing/mod.rs | 6 +- codex-rs/core/src/session/mcp.rs | 21 +-- codex-rs/core/src/session/session.rs | 5 +- codex-rs/core/src/session/tests.rs | 29 ++-- codex-rs/core/src/session/turn.rs | 29 ++-- codex-rs/core/src/session/turn_context.rs | 36 ++--- codex-rs/core/src/shell_snapshot.rs | 4 +- codex-rs/core/src/tasks/user_shell.rs | 14 +- codex-rs/core/src/thread_manager.rs | 28 ---- codex-rs/core/src/thread_manager_tests.rs | 129 +----------------- .../agent_jobs/spawn_agents_on_csv.rs | 11 +- .../core/src/tools/handlers/apply_patch.rs | 11 +- .../src/tools/handlers/extension_tools.rs | 13 +- .../src/tools/handlers/request_permissions.rs | 10 +- .../core/src/tools/handlers/unified_exec.rs | 3 +- .../handlers/unified_exec/exec_command.rs | 35 +++-- .../core/src/tools/handlers/view_image.rs | 53 ++++--- codex-rs/core/src/tools/orchestrator.rs | 2 +- .../core/src/tools/runtimes/apply_patch.rs | 15 +- .../src/tools/runtimes/apply_patch_tests.rs | 1 - codex-rs/core/src/tools/runtimes/mod.rs | 9 +- codex-rs/core/src/tools/runtimes/mod_tests.rs | 4 +- codex-rs/core/src/tools/runtimes/shell.rs | 61 +++++---- .../tools/runtimes/shell/unix_escalation.rs | 31 ++++- .../core/src/tools/runtimes/unified_exec.rs | 76 +++++++---- codex-rs/core/src/tools/sandboxing.rs | 2 +- codex-rs/core/src/tools/spec_plan_tests.rs | 5 +- codex-rs/core/src/unified_exec/mod.rs | 3 +- .../core/src/unified_exec/process_manager.rs | 51 ++++--- .../src/unified_exec/process_manager_tests.rs | 8 +- .../remote_env_windows_test.rs | 10 +- codex-rs/exec-server/src/local_process.rs | 14 +- codex-rs/exec/tests/suite/sandbox.rs | 2 +- .../utils/path-uri/src/api_path_string.rs | 4 + codex-rs/utils/path-uri/src/lib.rs | 34 ++++- codex-rs/utils/path-uri/src/tests.rs | 26 ++++ 46 files changed, 491 insertions(+), 423 deletions(-) diff --git a/codex-rs/app-server/src/command_exec.rs b/codex-rs/app-server/src/command_exec.rs index 2f62f099e95d..427489231819 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 bf9843c53ed8..6808b0585216 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 93dec41463c3..c0946ba6790f 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 22c5269ac3ac..48c8f5cd23ae 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 5cb70dc5fc4e..861e7b4ac74a 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 be3afdc3adb0..42604629a04c 100644 --- a/codex-rs/core/src/agents_md_tests.rs +++ b/codex-rs/core/src/agents_md_tests.rs @@ -270,7 +270,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 8aa07d1ed5a5..f0cc9f80bc6b 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 c1a5edd3ad0e..16b2c47b5dd7 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 9468534d8fd3..be47d195f87e 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 9d96a2d60bb8..7aa007891f24 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 f1f53d80efbe..259fd7031ed7 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 cc6e42f85222..d1b89822232d 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 46f20eaf53de..578152814b66 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 dec6feceab7a..664377c4ce69 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 3c783eb486b4..a105aeffeed4 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 8df2a7514eab..1e5ce3f8af6c 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 6e6c6cd51510..6a9509300301 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 6a01a87a0e06..c3abf03e745a 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 091bbe3edf8f..2adb713a0e51 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 437575a41e09..cec54b5e4446 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 2556989621cb..a519e5db1b08 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 a719e9acd9f6..2ce9119e3580 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 f0522c7c1763..b59bfaec02db 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 53341b14f5a6..afc15f965972 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 71e776dbf033..35e42374cdd9 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 eb89e56ef23d..1d7d64e2f5fe 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 f2e16f0f58d0..6a9c61069e9f 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 7e97e4ad5144..29cecf71ed59 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 9ad9d284c786..0624b75e43da 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 a181502f0b43..51ca0b98c813 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 27f4ee594659..6c8cc15d87bb 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 2b879e75cad6..a42fbef8ab97 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 6cfa3b2dfcad..3ccd9e4c726a 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 ea53e9a55c89..3b843c7faeae 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 25a1b932ce43..2964f43ea75c 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 499cdfbdaeda..b072a5188133 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 cbe282ec393c..5faee28682bd 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 23dd3bf1b5fd..54e9af09fe7a 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 702a339e2edb..be4f2abe2cff 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 49758ec7998f..993adf022a60 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 1ea7d293cede..6531f075a8ed 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 78a53897ee48..3e0f4c03b816 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 4b8cecc515fe..a678436a4ba7 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 f9ae8690c8fd..a8c056f7f6a1 100644 --- a/codex-rs/utils/path-uri/src/api_path_string.rs +++ b/codex-rs/utils/path-uri/src/api_path_string.rs @@ -100,6 +100,10 @@ impl LegacyAppPathString { &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 b835530d5b44..02d15be7e3f7 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 fe436476ddd1..f81c426819cd 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] From a873df8a60ac04237925fd81b0eb8bcddc71a3d3 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Tue, 16 Jun 2026 18:17:40 +0000 Subject: [PATCH 12/15] codex: address PR review feedback (#28146) --- codex-rs/app-server/src/request_processors.rs | 10 +++------- .../src/request_processors/thread_processor.rs | 2 +- .../src/request_processors/turn_processor.rs | 3 +-- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/codex-rs/app-server/src/request_processors.rs b/codex-rs/app-server/src/request_processors.rs index 6808b0585216..b5295b9b74d4 100644 --- a/codex-rs/app-server/src/request_processors.rs +++ b/codex-rs/app-server/src/request_processors.rs @@ -541,7 +541,7 @@ fn resolve_request_cwd(cwd: Option) -> Result, .transpose() } -async fn resolve_turn_environment_selections( +fn resolve_turn_environment_selections( thread_manager: &ThreadManager, environments: Option>, ) -> Result>, JSONRPCErrorError> { @@ -551,20 +551,16 @@ async fn resolve_turn_environment_selections( let mut selections = Vec::with_capacity(environments.len()); for environment in environments { let environment_id = environment.environment_id; - let convention = environment + let cwd = environment .cwd .infer_absolute_path_convention() + .and_then(|convention| environment.cwd.to_path_uri(convention).ok()) .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, diff --git a/codex-rs/app-server/src/request_processors/thread_processor.rs b/codex-rs/app-server/src/request_processors/thread_processor.rs index 747e1a091b08..a5299f2c72d9 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -906,7 +906,7 @@ impl ThreadRequestProcessor { )); } let environment_selections = - resolve_turn_environment_selections(self.thread_manager.as_ref(), environments).await?; + resolve_turn_environment_selections(self.thread_manager.as_ref(), environments)?; let runtime_workspace_roots = runtime_workspace_roots.map(resolve_runtime_workspace_roots); let mut typesafe_overrides = self.build_thread_config_overrides( model, diff --git a/codex-rs/app-server/src/request_processors/turn_processor.rs b/codex-rs/app-server/src/request_processors/turn_processor.rs index 66c4ca895658..d48e8c04b55a 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -412,8 +412,7 @@ impl TurnRequestProcessor { })?; let environment_selections = - resolve_turn_environment_selections(self.thread_manager.as_ref(), params.environments) - .await?; + resolve_turn_environment_selections(self.thread_manager.as_ref(), params.environments)?; // Map v2 input items to core input items. let mapped_items: Vec = params From 94d6e21ff25498bfd4a68411fc9f407926307325 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Tue, 16 Jun 2026 18:20:50 +0000 Subject: [PATCH 13/15] codex: address PR review feedback (#28146) --- .../app-server/tests/suite/v2/turn_start.rs | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) 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 48c8f5cd23ae..76b02a6d6e5a 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_absolute_path::AbsolutePathBuf; +use codex_utils_absolute_path::test_support::PathExt; use core_test_support::responses; use core_test_support::skip_if_no_network; use pretty_assertions::assert_eq; @@ -2673,7 +2673,7 @@ async fn run_environment_selection_case( .send_thread_start_request(ThreadStartParams { model: Some("mock-model".to_string()), cwd: Some(workspace.to_string_lossy().into_owned()), - environments: environment_params(case.sticky, workspace)?, + environments: environment_params(case.sticky, workspace), ..Default::default() }) .await?; @@ -2692,7 +2692,7 @@ async fn run_environment_selection_case( text: format!("run {}", case.name), text_elements: Vec::new(), }], - environments: environment_params(case.turn, workspace)?, + environments: environment_params(case.turn, workspace), cwd: Some(workspace.to_path_buf()), model: Some("mock-model".to_string()), ..Default::default() @@ -2739,21 +2739,15 @@ async fn run_environment_selection_case( Ok(()) } -fn environment_params( - ids: Option<&[&str]>, - cwd: &Path, -) -> Result>> { +fn environment_params(ids: Option<&[&str]>, cwd: &Path) -> Option> { ids.map(|ids| { ids.iter() - .map(|id| { - Ok(TurnEnvironmentParams { - environment_id: (*id).to_string(), - cwd: AbsolutePathBuf::try_from(cwd.to_path_buf())?.into(), - }) + .map(|id| TurnEnvironmentParams { + environment_id: (*id).to_string(), + cwd: cwd.abs().into(), }) .collect() }) - .transpose() } #[tokio::test] From 986dc06d90dafa74b4a34d7f65984f8d69221efe Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Tue, 16 Jun 2026 18:34:48 +0000 Subject: [PATCH 14/15] codex: address PR review feedback (#28146) --- codex-rs/app-server-protocol/src/protocol/v2/turn.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/codex-rs/app-server-protocol/src/protocol/v2/turn.rs b/codex-rs/app-server-protocol/src/protocol/v2/turn.rs index 0767a8ce6d93..e786cbd35424 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/turn.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/turn.rs @@ -39,7 +39,6 @@ pub enum TurnStatus { #[ts(export_to = "v2/")] pub struct TurnEnvironmentParams { pub environment_id: String, - /// Absolute cwd rendered using the selected environment's path convention. pub cwd: ApiPathString, } From fa970c1c0f16045476d63270dae578c9601cce46 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Tue, 16 Jun 2026 19:47:45 +0000 Subject: [PATCH 15/15] codex: narrow remote environment cwd support (#28146) --- .../schema/json/ClientRequest.json | 13 +- .../codex_app_server_protocol.schemas.json | 7 +- .../codex_app_server_protocol.v2.schemas.json | 7 +- .../schema/json/v2/ThreadStartParams.json | 13 +- .../schema/json/v2/TurnStartParams.json | 13 +- .../typescript/v2/TurnEnvironmentParams.ts | 8 +- .../src/protocol/v2/tests.rs | 2 +- .../src/protocol/v2/turn.rs | 4 +- codex-rs/app-server/src/command_exec.rs | 3 - codex-rs/core/src/exec.rs | 1 - codex-rs/core/src/mcp_openai_file.rs | 11 +- codex-rs/core/src/sandboxing/mod.rs | 6 +- codex-rs/core/src/session/tests.rs | 7 +- codex-rs/core/src/session/turn_context.rs | 4 +- codex-rs/core/src/tasks/user_shell.rs | 26 ++-- .../core/src/tools/handlers/unified_exec.rs | 3 +- .../handlers/unified_exec/exec_command.rs | 43 ++++--- .../core/src/tools/handlers/view_image.rs | 52 ++++---- codex-rs/core/src/tools/orchestrator.rs | 2 +- .../core/src/tools/runtimes/apply_patch.rs | 15 ++- codex-rs/core/src/tools/runtimes/mod.rs | 9 +- codex-rs/core/src/tools/runtimes/mod_tests.rs | 4 +- codex-rs/core/src/tools/runtimes/shell.rs | 61 ++++------ .../tools/runtimes/shell/unix_escalation.rs | 31 +---- .../core/src/tools/runtimes/unified_exec.rs | 75 +++++------- codex-rs/core/src/tools/sandboxing.rs | 2 +- codex-rs/core/src/unified_exec/mod.rs | 3 +- .../core/src/unified_exec/process_manager.rs | 51 +++----- .../src/unified_exec/process_manager_tests.rs | 8 +- .../remote_env_windows_test.rs | 113 +----------------- codex-rs/exec-server/src/local_process.rs | 14 ++- codex-rs/exec/tests/suite/sandbox.rs | 2 +- .../utils/path-uri/src/api_path_string.rs | 4 - codex-rs/utils/path-uri/src/lib.rs | 34 +----- codex-rs/utils/path-uri/src/tests.rs | 26 ---- 35 files changed, 207 insertions(+), 470 deletions(-) diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 7812e9c202ca..3358ea9d63c0 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -78,9 +78,6 @@ } ] }, - "ApiPathString": { - "type": "string" - }, "ApprovalsReviewer": { "description": "Configures who approval requests are routed to for review. Examples include sandbox escapes, blocked network access, MCP approval prompts, and ARC escalations. Defaults to `user`. `auto_review` uses a carefully prompted subagent to gather relevant context and apply a risk-based decision framework before approving or denying the request. The legacy value `guardian_subagent` is accepted for compatibility.", "enum": [ @@ -1306,6 +1303,9 @@ ], "type": "object" }, + "LegacyAppPathString": { + "type": "string" + }, "ListMcpServerStatusParams": { "properties": { "cursor": { @@ -4334,12 +4334,7 @@ "TurnEnvironmentParams": { "properties": { "cwd": { - "allOf": [ - { - "$ref": "#/definitions/ApiPathString" - } - ], - "description": "Absolute cwd rendered using the selected environment's path convention." + "$ref": "#/definitions/LegacyAppPathString" }, "environmentId": { "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 558c8b7f85eb..1e11c76ae43f 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -19517,12 +19517,7 @@ "TurnEnvironmentParams": { "properties": { "cwd": { - "allOf": [ - { - "$ref": "#/definitions/v2/ApiPathString" - } - ], - "description": "Absolute cwd rendered using the selected environment's path convention." + "$ref": "#/definitions/v2/LegacyAppPathString" }, "environmentId": { "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 4ac01e318d3c..326bfdfc9aad 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -17325,12 +17325,7 @@ "TurnEnvironmentParams": { "properties": { "cwd": { - "allOf": [ - { - "$ref": "#/definitions/ApiPathString" - } - ], - "description": "Absolute cwd rendered using the selected environment's path convention." + "$ref": "#/definitions/LegacyAppPathString" }, "environmentId": { "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json index 95e4c706b9c0..d4291f6dc1a4 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json @@ -5,9 +5,6 @@ "description": "A path that is guaranteed to be absolute and normalized (though it is not guaranteed to be canonicalized or exist on the filesystem).\n\nIMPORTANT: When deserializing an `AbsolutePathBuf`, a base path must be set using [AbsolutePathBufGuard::new]. If no base path is set, the deserialization will fail unless the path being deserialized is already absolute.", "type": "string" }, - "ApiPathString": { - "type": "string" - }, "ApprovalsReviewer": { "description": "Configures who approval requests are routed to for review. Examples include sandbox escapes, blocked network access, MCP approval prompts, and ARC escalations. Defaults to `user`. `auto_review` uses a carefully prompted subagent to gather relevant context and apply a risk-based decision framework before approving or denying the request. The legacy value `guardian_subagent` is accepted for compatibility.", "enum": [ @@ -194,6 +191,9 @@ } ] }, + "LegacyAppPathString": { + "type": "string" + }, "Personality": { "enum": [ "none", @@ -245,12 +245,7 @@ "TurnEnvironmentParams": { "properties": { "cwd": { - "allOf": [ - { - "$ref": "#/definitions/ApiPathString" - } - ], - "description": "Absolute cwd rendered using the selected environment's path convention." + "$ref": "#/definitions/LegacyAppPathString" }, "environmentId": { "type": "string" diff --git a/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json index c1db63db4808..b1c72bb1f26e 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json @@ -27,9 +27,6 @@ ], "type": "string" }, - "ApiPathString": { - "type": "string" - }, "ApprovalsReviewer": { "description": "Configures who approval requests are routed to for review. Examples include sandbox escapes, blocked network access, MCP approval prompts, and ARC escalations. Defaults to `user`. `auto_review` uses a carefully prompted subagent to gather relevant context and apply a risk-based decision framework before approving or denying the request. The legacy value `guardian_subagent` is accepted for compatibility.", "enum": [ @@ -133,6 +130,9 @@ ], "type": "string" }, + "LegacyAppPathString": { + "type": "string" + }, "ModeKind": { "description": "Initial collaboration mode to use when the TUI starts.", "enum": [ @@ -334,12 +334,7 @@ "TurnEnvironmentParams": { "properties": { "cwd": { - "allOf": [ - { - "$ref": "#/definitions/ApiPathString" - } - ], - "description": "Absolute cwd rendered using the selected environment's path convention." + "$ref": "#/definitions/LegacyAppPathString" }, "environmentId": { "type": "string" diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/TurnEnvironmentParams.ts b/codex-rs/app-server-protocol/schema/typescript/v2/TurnEnvironmentParams.ts index f5c91a9af11c..cb93ba396411 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/TurnEnvironmentParams.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/TurnEnvironmentParams.ts @@ -1,10 +1,6 @@ // GENERATED CODE! DO NOT MODIFY BY HAND! // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -import type { ApiPathString } from "../ApiPathString"; +import type { LegacyAppPathString } from "../LegacyAppPathString"; -export type TurnEnvironmentParams = { environmentId: string, -/** - * Absolute cwd rendered using the selected environment's path convention. - */ -cwd: ApiPathString, }; +export type TurnEnvironmentParams = { environmentId: string, cwd: LegacyAppPathString, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs index 1359a47be152..de138d5c2e8d 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/tests.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/tests.rs @@ -3729,7 +3729,7 @@ fn turn_start_params_round_trip_environments() { let raw_cwd = "/workspace"; #[cfg(not(windows))] let raw_cwd = r"C:\workspace"; - let cwd: ApiPathString = + let cwd: LegacyAppPathString = serde_json::from_value(json!(raw_cwd)).expect("API path should deserialize"); let params: TurnStartParams = serde_json::from_value(json!({ "threadId": "thread_123", diff --git a/codex-rs/app-server-protocol/src/protocol/v2/turn.rs b/codex-rs/app-server-protocol/src/protocol/v2/turn.rs index e786cbd35424..77da0ea8ad3e 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/turn.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/turn.rs @@ -14,7 +14,7 @@ use codex_protocol::user_input::ByteRange as CoreByteRange; use codex_protocol::user_input::TextElement as CoreTextElement; use codex_protocol::user_input::UserInput as CoreUserInput; use codex_utils_absolute_path::AbsolutePathBuf; -use codex_utils_path_uri::ApiPathString; +use codex_utils_path_uri::LegacyAppPathString; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -39,7 +39,7 @@ pub enum TurnStatus { #[ts(export_to = "v2/")] pub struct TurnEnvironmentParams { pub environment_id: String, - pub cwd: ApiPathString, + pub cwd: LegacyAppPathString, } #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] diff --git a/codex-rs/app-server/src/command_exec.rs b/codex-rs/app-server/src/command_exec.rs index 427489231819..2f62f099e95d 100644 --- a/codex-rs/app-server/src/command_exec.rs +++ b/codex-rs/app-server/src/command_exec.rs @@ -250,9 +250,6 @@ 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/core/src/exec.rs b/codex-rs/core/src/exec.rs index be47d195f87e..9468534d8fd3 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -461,7 +461,6 @@ 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/mcp_openai_file.rs b/codex-rs/core/src/mcp_openai_file.rs index 7c1e4d7c8d85..4dee0da717a4 100644 --- a/codex-rs/core/src/mcp_openai_file.rs +++ b/codex-rs/core/src/mcp_openai_file.rs @@ -122,7 +122,13 @@ async fn build_uploaded_argument_value( "no primary turn environment is available".to_string(), )); }; - let resolved_path = turn_environment.cwd().join(file_path); + // TODO(anp): Resolve app tool file arguments using the selected environment's native path + // convention so uploads can read relative paths from foreign environments. + let native_environment_cwd = turn_environment + .cwd() + .to_abs_path() + .map_err(|error| contextualize_error(error.to_string()))?; + let resolved_path = native_environment_cwd.join(file_path); let path_uri = PathUri::from_abs_path(&resolved_path); let fs = turn_environment.environment.get_filesystem(); let metadata = fs @@ -178,6 +184,7 @@ mod tests { use crate::session::tests::make_session_and_context; use crate::session::turn_context::TurnEnvironment; use codex_utils_absolute_path::AbsolutePathBuf; + use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use std::path::Path; use std::sync::Arc; @@ -194,7 +201,7 @@ mod tests { *primary = TurnEnvironment::new( primary.environment_id.clone(), Arc::clone(&primary.environment), - cwd, + PathUri::from_abs_path(&cwd), primary.shell.clone(), ); } diff --git a/codex-rs/core/src/sandboxing/mod.rs b/codex-rs/core/src/sandboxing/mod.rs index 259fd7031ed7..f1f53d80efbe 100644 --- a/codex-rs/core/src/sandboxing/mod.rs +++ b/codex-rs/core/src/sandboxing/mod.rs @@ -25,7 +25,6 @@ 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)] @@ -43,7 +42,7 @@ pub(crate) struct ExecServerEnvConfig { #[derive(Debug)] pub struct ExecRequest { pub command: Vec, - pub cwd: PathUri, + pub cwd: AbsolutePathBuf, pub env: HashMap, pub(crate) exec_server_env_config: Option, pub network: Option, @@ -78,7 +77,6 @@ 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 { @@ -137,7 +135,7 @@ impl ExecRequest { } Self { command, - cwd: PathUri::from_abs_path(&cwd), + cwd, env, exec_server_env_config: None, network, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 664377c4ce69..b4b786c902a1 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -979,11 +979,8 @@ 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, - Result, - > { - Box::pin(async { Ok(ReviewDecision::Approved) }) + ) -> futures::future::BoxFuture<'a, ReviewDecision> { + Box::pin(async { ReviewDecision::Approved }) } } diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 1e5ce3f8af6c..aed5fae681f3 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -69,8 +69,8 @@ impl TurnEnvironment { } } - pub(crate) fn shell_snapshot(&self, cwd: &PathUri) -> Option { - if &self.cwd != cwd { + pub(crate) fn shell_snapshot(&self, cwd: &AbsolutePathBuf) -> Option { + if self.cwd != PathUri::from_abs_path(cwd) { return None; } self.shell_snapshot diff --git a/codex-rs/core/src/tasks/user_shell.rs b/codex-rs/core/src/tasks/user_shell.rs index c3abf03e745a..4c3049351729 100644 --- a/codex-rs/core/src/tasks/user_shell.rs +++ b/codex-rs/core/src/tasks/user_shell.rs @@ -144,7 +144,18 @@ pub(crate) async fn execute_user_shell_command( // We do not source rc files or otherwise reformat the script. let use_login_shell = true; let display_command = environment_shell.derive_exec_args(&command, use_login_shell); - let shell_snapshot_location = turn_environment.shell_snapshot(turn_environment.cwd()); + // 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 shell_snapshot_location = turn_environment.shell_snapshot(&cwd); let mut exec_env_map = create_env( &turn_context.shell_environment_policy, Some(session.thread_id), @@ -162,17 +173,6 @@ pub(crate) async fn execute_user_shell_command( let call_id = Uuid::new_v4().to_string(); let raw_command = command; - // 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 @@ -195,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: turn_environment.cwd().clone(), + cwd: 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/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 35e42374cdd9..71e776dbf033 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -10,7 +10,6 @@ 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; @@ -55,7 +54,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 1d7d64e2f5fe..996ae9b52b04 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,6 +11,7 @@ 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; @@ -128,28 +129,31 @@ impl ExecCommandHandler { "unified exec is unavailable in this session".to_string(), )); }; - let cwd = environment_args - .workdir - .as_ref() - .filter(|workdir| !workdir.is_empty()) - .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| { + // TODO(anp): Resolve tool paths using the selected environment's native path convention + // so unified exec can support relative paths in foreign environments. + let native_environment_cwd = turn_environment.cwd().to_abs_path().map_err(|err| { FunctionCallError::RespondToModel(format!( - "command workdir `{cwd}` is not native to the Codex host: {err}" + "environment cwd `{}` is not native to the Codex host: {err}", + turn_environment.cwd() )) })?; + let cwd = environment_args + .workdir + .as_deref() + .filter(|workdir| !workdir.is_empty()) + .map_or_else( + || native_environment_cwd.clone(), + |workdir| native_environment_cwd.join(workdir), + ); let environment = Arc::clone(&turn_environment.environment); let fs = environment.get_filesystem(); - let args: ExecCommandArgs = parse_arguments(&arguments)?; + let args: ExecCommandArgs = parse_arguments_with_base_path(&arguments, &cwd)?; let hook_command = args.cmd.clone(); maybe_emit_implicit_skill_invocation( session.as_ref(), context.turn.as_ref(), &hook_command, - &native_cwd, + &cwd, ) .await; let process_id = manager.allocate_process_id().await; @@ -190,7 +194,7 @@ impl ExecCommandHandler { let effective_additional_permissions = apply_granted_turn_permissions( context.session.as_ref(), &turn_environment.environment_id, - native_cwd.as_path(), + cwd.as_path(), sandbox_permissions, additional_permissions, ) @@ -230,7 +234,7 @@ impl ExecCommandHandler { effective_additional_permissions.sandbox_permissions, effective_additional_permissions.additional_permissions, effective_additional_permissions.permissions_preapproved, - native_cwd.as_path(), + &cwd, ) }, |permissions| Ok(Some(permissions)), @@ -244,7 +248,7 @@ impl ExecCommandHandler { if let Some(output) = intercept_apply_patch( &command, - &native_cwd, + &cwd, fs.as_ref(), turn_environment.clone(), context.session.clone(), @@ -281,14 +285,7 @@ impl ExecCommandHandler { yield_time_ms, max_output_tokens, cwd, - // 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() - )) - })?, + sandbox_cwd: native_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 6a9c61069e9f..45a1adc2467d 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -11,7 +11,6 @@ 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; @@ -28,6 +27,7 @@ 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: ApiPathString, + path: String, #[serde(default)] environment_id: Option, detail: Option, @@ -143,40 +143,36 @@ impl ViewImageHandler { "view_image is unavailable in this session".to_string(), )); }; - // 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}")) + // TODO(anp): Resolve tool paths using the selected environment's native path convention + // so view_image can support relative paths in foreign environments. + let 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() + )) })?; - // 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 abs_path = cwd.join(path); let sandbox = turn.file_system_sandbox_context( /*additional_permissions*/ None, 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 `{display_path}`: {error}" + "unable to locate image at `{}`: {error}", + abs_path.display() )) })?; if !metadata.is_file { return Err(FunctionCallError::RespondToModel(format!( - "image path `{display_path}` is not a file" + "image path `{}` is not a file", + abs_path.display() ))); } let file_bytes = fs @@ -184,17 +180,11 @@ impl ViewImageHandler { .await .map_err(|error| { FunctionCallError::RespondToModel(format!( - "unable to read image at `{display_path}`: {error}" + "unable to read image at `{}`: {error}", + abs_path.display() )) })?; - // 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 event_path = abs_path.clone(); let can_request_original_detail = can_request_original_image_detail(&turn.model_info); let use_original_detail = @@ -214,11 +204,11 @@ impl ViewImageHandler { } else { PromptImageMode::ResizeToFit }; - load_for_prompt_bytes(native_path.as_path(), file_bytes, image_mode) + load_for_prompt_bytes(abs_path.as_path(), file_bytes, image_mode) .map_err(|error| { FunctionCallError::RespondToModel(format!( "unable to process image at `{}`: {error}", - native_path.display() + abs_path.display() )) })? .into_data_url() diff --git a/codex-rs/core/src/tools/orchestrator.rs b/codex-rs/core/src/tools/orchestrator.rs index 29cecf71ed59..7e97e4ad5144 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 0624b75e43da..9ad9d284c786 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, Result> { + ) -> BoxFuture<'a, ReviewDecision> { let session = ctx.session; let turn = ctx.turn; let call_id = ctx.call_id.to_string(); @@ -144,12 +144,11 @@ 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 Ok( - review_approval_request(session, turn, review_id, action, retry_reason).await, - ); + return review_approval_request(session, turn, review_id, action, retry_reason) + .await; } if req.permissions_preapproved && retry_reason.is_none() { - return Ok(ReviewDecision::Approved); + return ReviewDecision::Approved; } if let Some(reason) = retry_reason { let rx_approve = session @@ -161,10 +160,10 @@ impl Approvable for ApplyPatchRuntime { /*grant_root*/ None, ) .await; - return Ok(rx_approve.await.unwrap_or_default()); + return rx_approve.await.unwrap_or_default(); } - Ok(with_cached_approval( + with_cached_approval( &session.services, "apply_patch", approval_keys, @@ -177,7 +176,7 @@ impl Approvable for ApplyPatchRuntime { rx_approve.await.unwrap_or_default() }, ) - .await) + .await }) } diff --git a/codex-rs/core/src/tools/runtimes/mod.rs b/codex-rs/core/src/tools/runtimes/mod.rs index 6c8cc15d87bb..27f4ee594659 100644 --- a/codex-rs/core/src/tools/runtimes/mod.rs +++ b/codex-rs/core/src/tools/runtimes/mod.rs @@ -33,21 +33,22 @@ 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 working -/// directory. Validates that at least a program is present. +/// Shared helper to construct sandbox transform inputs from a tokenized command line and native +/// working directory. Validates that at least a program is present. pub(crate) fn build_sandbox_command( command: &[String], - cwd: &PathUri, + cwd: &AbsolutePathBuf, 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.clone(), + cwd, 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 a42fbef8ab97..2b879e75cad6 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, - &PathUri::from_abs_path(&command_cwd), + &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, PathUri::from_abs_path(&command_cwd)); + assert_eq!(exec_request.cwd, 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 3ccd9e4c726a..6cfa3b2dfcad 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -47,7 +47,6 @@ 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; @@ -139,7 +138,7 @@ impl Approvable for ShellRuntime { &'a mut self, req: &'a ShellRequest, ctx: ApprovalCtx<'a>, - ) -> BoxFuture<'a, Result> { + ) -> BoxFuture<'a, ReviewDecision> { let keys = self.approval_keys(req); let command = req.command.clone(); let cwd = req.cwd.clone(); @@ -151,7 +150,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 Ok(review_approval_request( + return review_approval_request( session, turn, review_id, @@ -165,30 +164,28 @@ impl Approvable for ShellRuntime { }, retry_reason, ) - .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, - ) + 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 }) } @@ -250,9 +247,7 @@ impl ToolRuntime for ShellRuntime { .shell .as_ref() .unwrap_or(session_shell.as_ref()); - let shell_snapshot_location = req - .turn_environment - .shell_snapshot(&PathUri::from_abs_path(&req.cwd)); + let shell_snapshot_location = req.turn_environment.shell_snapshot(&req.cwd); let (file_system_sandbox_policy, _) = attempt.permissions.to_runtime_permissions(); let sandbox_permissions = sandbox_permissions_preserving_denied_reads( req.sandbox_permissions, @@ -310,12 +305,8 @@ impl ToolRuntime for ShellRuntime { } } - let command = build_sandbox_command( - &command, - &PathUri::from_abs_path(&req.cwd), - &env, - req.additional_permissions.clone(), - )?; + let command = + build_sandbox_command(&command, &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 3b843c7faeae..ea53e9a55c89 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -132,12 +132,8 @@ 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, - &PathUri::from_abs_path(&req.cwd), - &env, - req.additional_permissions.clone(), - )?; + let command = + build_sandbox_command(command, &req.cwd, &env, req.additional_permissions.clone())?; let options = ExecOptions { expiration: req.timeout_ms.into(), capture_policy: ExecCapturePolicy::ShellTool, @@ -176,11 +172,6 @@ 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, @@ -282,15 +273,9 @@ 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: native_cwd, + cwd: exec_request.cwd.clone(), 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, @@ -867,7 +852,7 @@ impl CoreShellCommandExecutor { let result = crate::sandboxing::execute_exec_request_with_after_spawn( crate::sandboxing::ExecRequest { command: self.command.clone(), - cwd: PathUri::from_abs_path(&self.cwd), + cwd: self.cwd.clone(), env: exec_env, exec_server_env_config: None, network: self.network.clone(), @@ -1018,15 +1003,9 @@ 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: cwd.to_path_buf(), + cwd: exec_request.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 2964f43ea75c..eb23fc0cedc6 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -51,7 +51,6 @@ 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; @@ -64,7 +63,7 @@ pub struct UnifiedExecRequest { pub shell_type: ShellType, pub hook_command: String, pub process_id: i32, - pub cwd: PathUri, + pub cwd: AbsolutePathBuf, pub sandbox_cwd: AbsolutePathBuf, pub turn_environment: TurnEnvironment, pub env: HashMap, @@ -85,7 +84,7 @@ pub struct UnifiedExecRequest { #[derive(serde::Serialize, Clone, Debug, Eq, PartialEq, Hash)] pub struct UnifiedExecApprovalKey { pub command: Vec, - pub cwd: PathUri, + pub cwd: AbsolutePathBuf, pub tty: bool, pub sandbox_permissions: SandboxPermissions, pub additional_permissions: Option, @@ -148,26 +147,19 @@ impl Approvable for UnifiedExecRuntime<'_> { &'b mut self, req: &'b UnifiedExecRequest, ctx: ApprovalCtx<'b>, - ) -> BoxFuture<'b, Result> { + ) -> BoxFuture<'b, ReviewDecision> { 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 Ok(review_approval_request( + return review_approval_request( session, turn, review_id, @@ -182,30 +174,28 @@ impl Approvable for UnifiedExecRuntime<'_> { }, retry_reason, ) - .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, - ) + 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 }) } @@ -248,9 +238,6 @@ 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, @@ -258,7 +245,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, + cwd: req.cwd.clone(), sandbox_permissions: req.sandbox_permissions, additional_permissions: req.additional_permissions.clone(), justification: req.justification.clone(), @@ -411,9 +398,6 @@ 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, @@ -443,6 +427,7 @@ mod tests { use codex_exec_server::Environment; use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_tools::ZshForkConfig; + use codex_utils_path_uri::PathUri; use std::sync::Arc; use std::time::Duration; use tempfile::tempdir; @@ -493,7 +478,7 @@ mod tests { shell_type: ShellType::Sh, hook_command: "pwd".to_string(), process_id: 1000, - cwd: PathUri::from_abs_path(&cwd), + cwd, sandbox_cwd: sandbox_cwd.clone(), turn_environment: test_turn_environment(sandbox_cwd.clone()), env: HashMap::new(), @@ -592,7 +577,7 @@ mod tests { shell_type: ShellType::Zsh, hook_command: "echo hi".to_string(), process_id: 1000, - cwd: PathUri::from_abs_path(&cwd), + cwd: cwd.clone(), 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 b072a5188133..499cdfbdaeda 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, Result>; + ) -> BoxFuture<'a, ReviewDecision>; } pub(crate) trait Sandboxable { diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 54e9af09fe7a..23dd3bf1b5fd 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -32,7 +32,6 @@ 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; @@ -97,7 +96,7 @@ pub(crate) struct ExecCommandRequest { pub process_id: i32, pub yield_time_ms: u64, pub max_output_tokens: Option, - pub cwd: PathUri, + pub cwd: AbsolutePathBuf, 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 be4f2abe2cff..702a339e2edb 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -57,6 +57,7 @@ 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"), @@ -156,7 +157,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: request.cwd.clone(), + cwd: PathUri::from_abs_path(&request.cwd), env_policy, env, tty, @@ -384,15 +385,10 @@ impl UnifiedExecProcessManager { request: ExecCommandRequest, context: &UnifiedExecContext, ) -> Result { - // 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 cwd = request.cwd.clone(); + let process = self + .open_session_with_sandbox(&request, cwd.clone(), context) + .await; let (process, mut deferred_network_approval) = match process { Ok((process, deferred_network_approval)) => { @@ -420,7 +416,7 @@ impl UnifiedExecProcessManager { ); let emitter = ToolEmitter::unified_exec( &request.command, - event_cwd.clone(), + cwd.clone(), ExecCommandSource::UnifiedExecStartup, Some(request.process_id.to_string()), ); @@ -438,7 +434,7 @@ impl UnifiedExecProcessManager { context, &request.command, request.hook_command.clone(), - event_cwd.clone(), + cwd.clone(), start, request.process_id, request.tty, @@ -497,7 +493,7 @@ impl UnifiedExecProcessManager { process_started_alive, context, &request, - event_cwd.clone(), + cwd.clone(), Arc::clone(&transcript), text.clone(), message.clone(), @@ -517,7 +513,7 @@ impl UnifiedExecProcessManager { process_started_alive, context, &request, - event_cwd.clone(), + cwd.clone(), Arc::clone(&transcript), text.clone(), message.clone(), @@ -569,7 +565,7 @@ impl UnifiedExecProcessManager { process_started_alive, context, &request, - event_cwd.clone(), + cwd.clone(), Arc::clone(&transcript), text.clone(), message.clone(), @@ -586,7 +582,7 @@ impl UnifiedExecProcessManager { Arc::clone(&context.turn), context.call_id.clone(), request.command.clone(), - event_cwd.clone(), + cwd.clone(), Some(process_id.to_string()), Arc::clone(&transcript), text.clone(), @@ -903,12 +899,6 @@ 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}" @@ -943,7 +933,7 @@ impl UnifiedExecProcessManager { request.windows_sandbox_workspace_roots.as_slice(), codex_home.as_ref(), request.command.clone(), - native_cwd.as_path(), + request.cwd.as_path(), request.env.clone(), request.network.is_some(), None, @@ -965,7 +955,7 @@ impl UnifiedExecProcessManager { request.windows_sandbox_workspace_roots.as_slice(), codex_home.as_ref(), request.command.clone(), - native_cwd.as_path(), + request.cwd.as_path(), request.env.clone(), None, &additional_deny_read_paths, @@ -1005,17 +995,11 @@ 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, - native_cwd.as_path(), + request.cwd.as_path(), &request.env, &request.arg0, codex_utils_pty::TerminalSize::default(), @@ -1026,7 +1010,7 @@ impl UnifiedExecProcessManager { codex_utils_pty::pipe::spawn_process_no_stdin_with_inherited_fds( program, args, - native_cwd.as_path(), + request.cwd.as_path(), &request.env, &request.arg0, &inherited_fds, @@ -1042,6 +1026,7 @@ 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( @@ -1082,7 +1067,7 @@ impl UnifiedExecProcessManager { shell_type: request.shell_type, hook_command: request.hook_command.clone(), process_id: request.process_id, - cwd: request.cwd.clone(), + cwd, 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 993adf022a60..49758ec7998f 100644 --- a/codex-rs/core/src/unified_exec/process_manager_tests.rs +++ b/codex-rs/core/src/unified_exec/process_manager_tests.rs @@ -1,6 +1,5 @@ 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; @@ -73,14 +72,13 @@ 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: command_cwd.clone(), + cwd: cwd.clone(), env: HashMap::from([ ("HOME".to_string(), "/client-home".to_string()), ("PATH".to_string(), "/sandbox-path".to_string()), @@ -118,7 +116,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, command_cwd); + assert_eq!(params.cwd, PathUri::from_abs_path(&request.cwd)); assert!(params.env_policy.is_some()); assert_eq!( params.env, @@ -202,7 +200,7 @@ async fn failed_initial_end_for_unstored_process_uses_fallback_output() { yield_time_ms: 1000, max_output_tokens: None, #[allow(deprecated)] - cwd: PathUri::from_abs_path(&turn.cwd), + cwd: turn.cwd.clone(), #[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 6531f075a8ed..208dfa554d61 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 @@ -4,18 +4,10 @@ use anyhow::Context; use anyhow::Result; use app_test_support::PathBufExt; use app_test_support::TestAppServer; -use app_test_support::create_mock_responses_server_sequence; +use app_test_support::create_mock_responses_server_repeating_assistant; use app_test_support::to_response; use app_test_support::write_mock_responses_config_toml; -use base64::Engine as _; -use base64::engine::general_purpose::STANDARD; -use codex_app_server_protocol::AskForApproval as AppAskForApproval; -use codex_app_server_protocol::CommandAction; -use codex_app_server_protocol::CommandExecutionStatus; -use codex_app_server_protocol::ItemCompletedNotification; use codex_app_server_protocol::RequestId; -use codex_app_server_protocol::SandboxPolicy; -use codex_app_server_protocol::ThreadItem; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnEnvironmentParams; @@ -24,9 +16,6 @@ use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::UserInput as V2UserInput; use codex_exec_server::REMOTE_ENVIRONMENT_ID; use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; -use codex_exec_server::ExecServerClient; -use codex_exec_server::FsWriteFileParams; -use codex_exec_server::RemoteExecServerConnectArgs; use codex_features::Feature; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; @@ -46,7 +35,7 @@ use core_test_support::responses::start_mock_server; use core_test_support::test_codex::test_codex; use core_test_support::test_codex::turn_permission_fields; use core_test_support::wait_for_event; -use codex_utils_path_uri::ApiPathString; +use codex_utils_path_uri::LegacyAppPathString; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use serde_json::json; @@ -185,56 +174,14 @@ async fn windows_exec_server_runs_with_native_shell_and_cwd() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Result<()> { - const AGENTS_INSTRUCTIONS: &str = "remote Windows workspace instructions"; - const CALL_ID: &str = "wine-cmd-smoke"; - const COMMAND: &str = "Get-Content AGENTS.md -ErrorAction Stop"; - WineExecServer .scope(|exec_server_url| async move { - let exec_server_client = ExecServerClient::connect_websocket( - RemoteExecServerConnectArgs { - websocket_url: exec_server_url.clone(), - client_name: "remote-env-windows-test".to_string(), - connect_timeout: APP_SERVER_READ_TIMEOUT, - initialize_timeout: APP_SERVER_READ_TIMEOUT, - resume_session_id: None, - }, - ) - .await?; - exec_server_client - .fs_write_file(FsWriteFileParams { - path: PathUri::parse("file:///C:/windows/AGENTS.md")?, - data_base64: STANDARD.encode(AGENTS_INSTRUCTIONS), - sandbox: None, - }) - .await?; - let codex_home = TempDir::new()?; - let responses = vec![ - sse(vec![ - ev_response_created("resp-1"), - ev_function_call( - CALL_ID, - "exec_command", - &serde_json::to_string(&json!({ - "cmd": COMMAND, - "login": false, - "yield_time_ms": 5_000, - }))?, - ), - ev_completed("resp-1"), - ]), - sse(vec![ - ev_response_created("resp-2"), - ev_assistant_message("msg-1", "done"), - ev_completed("resp-2"), - ]), - ]; - let server = create_mock_responses_server_sequence(responses).await; + let server = create_mock_responses_server_repeating_assistant("done").await; write_mock_responses_config_toml( codex_home.path(), &server.uri(), - &BTreeMap::from([(Feature::UnifiedExec, true)]), + &BTreeMap::new(), 100_000, /*requires_openai_auth*/ None, "mock", @@ -254,7 +201,7 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul .send_thread_start_request(ThreadStartParams { environments: Some(vec![TurnEnvironmentParams { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: serde_json::from_value::(json!(r"C:\windows"))?, + cwd: serde_json::from_value::(json!(r"C:\windows"))?, }]), ..Default::default() }) @@ -281,11 +228,9 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul thread_id: response.thread.id, client_user_message_id: None, input: vec![V2UserInput::Text { - text: "run the Windows smoke command".to_string(), + text: "say done".to_string(), text_elements: Vec::new(), }], - approval_policy: Some(AppAskForApproval::Never), - sandbox_policy: Some(SandboxPolicy::DangerFullAccess), ..Default::default() }) .await?; @@ -295,58 +240,12 @@ async fn app_server_starts_thread_with_windows_environment_native_cwd() -> Resul ) .await??; let _: TurnStartResponse = to_response(turn_response)?; - - let completed = timeout(APP_SERVER_READ_TIMEOUT, async { - loop { - let notification = app_server - .read_stream_until_notification_message("item/completed") - .await?; - let completed: ItemCompletedNotification = serde_json::from_value( - notification.params.context("item/completed params")?, - )?; - if matches!(completed.item, ThreadItem::CommandExecution { .. }) { - return Ok::(completed.item); - } - } - }) - .await??; - let ThreadItem::CommandExecution { - command_actions, - cwd, - id, - status, - exit_code, - .. - } = completed - else { - unreachable!("loop returns only command execution items"); - }; - assert_eq!(id, CALL_ID); - // TODO(anp): Make command execution cwd a PathUri so this stays `C:\windows`. - assert_eq!(cwd, std::path::PathBuf::from("/C:/windows").abs()); - // TODO(anp): Parse command actions using the selected environment's path convention so - // their paths remain Windows-native instead of degrading the action to Unknown. - assert_eq!(command_actions.len(), 1); - assert!(matches!(command_actions[0], CommandAction::Unknown { .. })); - assert_eq!((status, exit_code), (CommandExecutionStatus::Completed, Some(0))); timeout( APP_SERVER_READ_TIMEOUT, app_server.read_stream_until_notification_message("turn/completed"), ) .await??; - let requests = server - .received_requests() - .await - .context("failed to fetch received requests")?; - 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); - Ok(()) }) .await diff --git a/codex-rs/exec-server/src/local_process.rs b/codex-rs/exec-server/src/local_process.rs index 3e0f4c03b816..78a53897ee48 100644 --- a/codex-rs/exec-server/src/local_process.rs +++ b/codex-rs/exec-server/src/local_process.rs @@ -156,11 +156,10 @@ 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(|_| { + let native_cwd = params.cwd.to_abs_path().map_err(|err| { invalid_params(format!( - "'{}' is invalid on '{}'", - params.cwd, - std::env::consts::OS + "cwd URI `{}` is not valid on this exec-server host: {err}", + params.cwd )) })?; @@ -816,7 +815,12 @@ mod tests { #[cfg(windows)] let uri = "file:///usr/local/checkout"; let cwd = PathUri::parse(uri).expect("non-native cwd URI"); - let expected = invalid_params(format!("'{cwd}' is invalid on '{}'", std::env::consts::OS)); + 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 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 a678436a4ba7..4b8cecc515fe 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.to_abs_path()?.as_path()); + child.current_dir(exec_request.cwd); 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 a8c056f7f6a1..f9ae8690c8fd 100644 --- a/codex-rs/utils/path-uri/src/api_path_string.rs +++ b/codex-rs/utils/path-uri/src/api_path_string.rs @@ -100,10 +100,6 @@ impl LegacyAppPathString { &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 02d15be7e3f7..b835530d5b44 100644 --- a/codex-rs/utils/path-uri/src/lib.rs +++ b/codex-rs/utils/path-uri/src/lib.rs @@ -198,11 +198,7 @@ impl PathUri { Some(Self(url)) } - /// 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. + /// Lexically joins a relative URI path onto this URI. /// /// Empty and `.` segments are ignored, while `..` removes one segment /// without escaping the URI root. Literal `%`, `?`, and `#` characters are @@ -211,18 +207,7 @@ impl PathUri { /// Opaque fallback URIs created by [`Self::from_abs_path`] reject non-empty /// joins. pub fn join(&self, path: &str) -> Result { - 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 { + if path.starts_with('/') { return Err(PathUriParseError::JoinPathMustBeRelative(path.to_string())); } if path.contains('\0') { @@ -245,14 +230,7 @@ impl PathUri { unreachable!("validated file URLs support hierarchical path segments"); }; segments.pop_if_empty(); - let is_separator = |character| { - if uses_windows_path_convention { - matches!(character, '/' | '\\') - } else { - character == '/' - } - }; - for component in path.split(is_separator) { + for component in path.split('/') { match component { "" | "." => {} ".." => { @@ -400,12 +378,6 @@ 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 f81c426819cd..fe436476ddd1 100644 --- a/codex-rs/utils/path-uri/src/tests.rs +++ b/codex-rs/utils/path-uri/src/tests.rs @@ -481,26 +481,11 @@ 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", @@ -517,7 +502,6 @@ 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"), @@ -529,16 +513,6 @@ 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]