diff --git a/bazel/rules/testing/wine/src/lib.rs b/bazel/rules/testing/wine/src/lib.rs index 0cc4c6a0b..3b99f5870 100644 --- a/bazel/rules/testing/wine/src/lib.rs +++ b/bazel/rules/testing/wine/src/lib.rs @@ -328,11 +328,12 @@ fn install_powershell_runtime(prefix: &Path, runtime: &Path) -> Result<()> { /// Recursively reproduces a runfiles directory in a writable Wine prefix. /// -/// Bazel runfiles may be immutable and may contain the PowerShell distribution -/// on a different filesystem from the temporary prefix. Hard links avoid -/// repeatedly copying the roughly hundred-megabyte runtime when both locations -/// share a filesystem; the copy fallback preserves correctness for sandbox or -/// remote-execution layouts where cross-device hard links are unavailable. +/// Bazel runfiles may be immutable, represented by a symlink forest, and may +/// contain the PowerShell distribution on a different filesystem from the +/// temporary prefix. Hard links avoid repeatedly copying the roughly +/// hundred-megabyte runtime when both locations share a filesystem; the copy +/// fallback preserves correctness for sandbox or remote-execution layouts +/// where cross-device hard links are unavailable. fn materialize_runtime_directory(source: &Path, destination: &Path) -> Result<()> { fs::create_dir_all(destination).with_context(|| { format!( @@ -346,23 +347,34 @@ fn materialize_runtime_directory(source: &Path, destination: &Path) -> Result<() let entry = entry.context("read PowerShell runtime entry")?; let source_path = entry.path(); let destination_path = destination.join(entry.file_name()); - let file_type = entry - .file_type() - .with_context(|| format!("inspect PowerShell runtime entry {}", source_path.display()))?; + // Local Bazel runfiles trees expose external-repository files as + // symlinks. Resolve those trusted runfiles entries before inspecting + // or linking them so the writable prefix contains ordinary files. + let resolved_source_path = fs::canonicalize(&source_path).with_context(|| { + format!("resolve PowerShell runtime entry {}", source_path.display()) + })?; + let file_type = fs::metadata(&resolved_source_path) + .with_context(|| { + format!( + "inspect PowerShell runtime entry {}", + resolved_source_path.display() + ) + })? + .file_type(); if file_type.is_dir() { // PowerShell resolves assemblies and modules by their relative // locations, so flattening the archive is not an option. - materialize_runtime_directory(&source_path, &destination_path)?; + materialize_runtime_directory(&resolved_source_path, &destination_path)?; } else if file_type.is_file() { // A hard link gives each prefix the expected installation layout // without duplicating the large runtime in the common local case. - if fs::hard_link(&source_path, &destination_path).is_err() { + if fs::hard_link(&resolved_source_path, &destination_path).is_err() { // Cross-device links are common under Bazel sandboxing and // remote execution, where an ordinary copy is still valid. - fs::copy(&source_path, &destination_path).with_context(|| { + fs::copy(&resolved_source_path, &destination_path).with_context(|| { format!( "copy PowerShell runtime file {} to {}", - source_path.display(), + resolved_source_path.display(), destination_path.display() ) })?; diff --git a/bazel/rules/testing/wine/src/lib_tests.rs b/bazel/rules/testing/wine/src/lib_tests.rs index fd8e667fd..e32d17399 100644 --- a/bazel/rules/testing/wine/src/lib_tests.rs +++ b/bazel/rules/testing/wine/src/lib_tests.rs @@ -264,6 +264,28 @@ fn powershell_runtime_is_materialized_at_the_windows_fallback_path() -> Result<( Ok(()) } +#[test] +fn powershell_runtime_follows_runfiles_symlinks() -> Result<()> { + let prefix = TempDir::new()?; + let runtime = TempDir::new()?; + let backing = TempDir::new()?; + let backing_file = backing.path().join("pwsh.exe"); + fs::write(&backing_file, b"pwsh")?; + std::os::unix::fs::symlink(&backing_file, runtime.path().join("pwsh.exe"))?; + + install_powershell_runtime(prefix.path(), runtime.path())?; + + let installed = prefix + .path() + .join("drive_c") + .join("Program Files") + .join("PowerShell") + .join("7") + .join("pwsh.exe"); + assert_eq!(fs::read(installed)?, b"pwsh"); + Ok(()) +} + #[tokio::test] async fn pinned_powershell_runs_under_wine_with_a_pty() -> Result<()> { // Keep this integration smoke test local to the Wine support crate. The diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 941e28f32..063f513d1 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3583,6 +3583,7 @@ dependencies = [ "codex-network-proxy", "codex-utils-absolute-path", "codex-utils-image", + "codex-utils-path-uri", "codex-utils-string", "encoding_rs", "globset", 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 5c1092685..bd354753e 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor.rs @@ -4,6 +4,7 @@ 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; @@ -1292,7 +1293,7 @@ impl ThreadRequestProcessor { .into_iter() .map(|environment| TurnEnvironmentSelection { environment_id: environment.environment_id, - cwd: environment.cwd, + cwd: PathUri::from_abs_path(&environment.cwd), }) .collect::>() }); 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 b9d6656cc..c8074b8db 100644 --- a/codex-rs/app-server/src/request_processors/turn_processor.rs +++ b/codex-rs/app-server/src/request_processors/turn_processor.rs @@ -4,6 +4,7 @@ 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"; @@ -339,7 +340,7 @@ impl TurnRequestProcessor { .into_iter() .map(|environment| TurnEnvironmentSelection { environment_id: environment.environment_id, - cwd: environment.cwd, + cwd: PathUri::from_abs_path(&environment.cwd), }) .collect::>() }); @@ -523,7 +524,7 @@ impl TurnRequestProcessor { environment_selections .iter() .find(|selection| selection.environment_id == LOCAL_ENVIRONMENT_ID) - .map(|selection| selection.cwd.clone()) + .and_then(|selection| selection.cwd.to_abs_path().ok()) .unwrap_or_else(|| snapshot.cwd().clone()) }); Some(TurnEnvironmentSelections::new( diff --git a/codex-rs/core/src/environment_selection.rs b/codex-rs/core/src/environment_selection.rs index 67cd1ccac..31c7494ca 100644 --- a/codex-rs/core/src/environment_selection.rs +++ b/codex-rs/core/src/environment_selection.rs @@ -7,6 +7,7 @@ use codex_protocol::error::CodexErr; use codex_protocol::error::Result as CodexResult; use codex_protocol::protocol::TurnEnvironmentSelection; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use crate::session::turn_context::TurnEnvironment; use crate::shell::Shell; @@ -20,7 +21,7 @@ pub(crate) fn default_thread_environment_selections( .into_iter() .map(|environment_id| TurnEnvironmentSelection { environment_id, - cwd: cwd.clone(), + cwd: PathUri::from_abs_path(cwd), }) .collect() } @@ -99,7 +100,12 @@ pub(crate) async fn resolve_environment_selections( turn_environments.push(TurnEnvironment::new( environment_id, environment, - selected_environment.cwd.clone(), + 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 + )) + })?, shell, )); } @@ -114,6 +120,7 @@ mod tests { use codex_exec_server::REMOTE_ENVIRONMENT_ID; use codex_protocol::protocol::TurnEnvironmentSelection; use codex_utils_absolute_path::AbsolutePathBuf; + use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use super::*; @@ -129,6 +136,7 @@ mod tests { #[tokio::test] async fn default_thread_environment_selections_use_manager_default_id() { let cwd = AbsolutePathBuf::current_dir().expect("cwd"); + let cwd_uri = PathUri::from_abs_path(&cwd); let manager = EnvironmentManager::create_for_tests( Some("ws://127.0.0.1:8765".to_string()), Some(test_runtime_paths()), @@ -139,7 +147,7 @@ mod tests { default_thread_environment_selections(&manager, &cwd), vec![TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd, + cwd: cwd_uri, }] ); } @@ -157,6 +165,7 @@ url = "ws://127.0.0.1:8765" ) .expect("write environments.toml"); let cwd = AbsolutePathBuf::current_dir().expect("cwd"); + let cwd_uri = PathUri::from_abs_path(&cwd); let manager = EnvironmentManager::from_codex_home(temp_dir.path(), Some(test_runtime_paths())) .await @@ -167,11 +176,11 @@ url = "ws://127.0.0.1:8765" vec![ TurnEnvironmentSelection { environment_id: LOCAL_ENVIRONMENT_ID.to_string(), - cwd: cwd.clone(), + cwd: cwd_uri.clone(), }, TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd, + cwd: cwd_uri, }, ] ); @@ -191,6 +200,7 @@ url = "ws://127.0.0.1:8765" #[tokio::test] async fn resolve_environment_selections_rejects_duplicate_ids() { let cwd = AbsolutePathBuf::current_dir().expect("cwd"); + let cwd_uri = PathUri::from_abs_path(&cwd); let manager = EnvironmentManager::default_for_tests(); let err = resolve_environment_selections( @@ -198,11 +208,11 @@ url = "ws://127.0.0.1:8765" &[ TurnEnvironmentSelection { environment_id: "local".to_string(), - cwd: cwd.clone(), + cwd: cwd_uri.clone(), }, TurnEnvironmentSelection { environment_id: "local".to_string(), - cwd: cwd.join("other"), + cwd: cwd_uri.join("other").expect("other cwd URI"), }, ], ) @@ -216,13 +226,14 @@ url = "ws://127.0.0.1:8765" async fn resolved_environment_selections_use_first_selection_as_primary() { let cwd = AbsolutePathBuf::current_dir().expect("cwd"); let selected_cwd = cwd.join("selected"); + let selected_cwd_uri = PathUri::from_abs_path(&selected_cwd); let manager = EnvironmentManager::default_for_tests(); let resolved = resolve_environment_selections( &manager, &[TurnEnvironmentSelection { environment_id: "local".to_string(), - cwd: selected_cwd, + cwd: selected_cwd_uri, }], ) .await @@ -255,12 +266,13 @@ url = "ws://127.0.0.1:8765" #[tokio::test] async fn single_local_environment_cwd_requires_exactly_one_local_environment() { let cwd = AbsolutePathBuf::current_dir().expect("cwd"); + let cwd_uri = PathUri::from_abs_path(&cwd); let local_manager = EnvironmentManager::default_for_tests(); let local = resolve_environment_selections( &local_manager, &[TurnEnvironmentSelection { environment_id: LOCAL_ENVIRONMENT_ID.to_string(), - cwd: cwd.clone(), + cwd: cwd_uri, }], ) .await diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index c9a4d19fb..f22b1fb62 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -149,6 +149,7 @@ use codex_thread_store::ResumeThreadParams; use codex_thread_store::ThreadPersistenceMetadata; use codex_thread_store::ThreadStore; use codex_utils_output_truncation::TruncationPolicy; +use codex_utils_path_uri::PathUri; use futures::future::BoxFuture; use futures::future::Shared; use futures::prelude::*; @@ -2206,6 +2207,18 @@ impl Session { } let requested_permissions = args.permissions; + // TODO(anp): Migrate request_permissions to support paths from foreign environments. + let Ok(native_environment_cwd) = environment.cwd.to_abs_path() else { + warn!( + cwd = %environment.cwd, + "request_permissions requires a cwd native to the Codex host" + ); + return Some(RequestPermissionsResponse { + permissions: RequestPermissionProfile::default(), + scope: PermissionGrantScope::Turn, + strict_auto_review: false, + }); + }; if crate::guardian::routes_approval_to_guardian(turn_context.as_ref()) { let originating_turn_state = { @@ -2273,7 +2286,7 @@ impl Session { let response = Self::normalize_request_permissions_response( requested_permissions, response, - environment.cwd.as_path(), + native_environment_cwd.as_path(), ); self.record_granted_request_permissions_for_turn( &response, @@ -2313,7 +2326,7 @@ impl Session { started_at_ms: now_unix_timestamp_ms(), reason: args.reason, permissions: requested_permissions, - cwd: Some(environment.cwd), + cwd: Some(native_environment_cwd), }); self.send_event(turn_context.as_ref(), event).await; tokio::select! { @@ -2354,7 +2367,7 @@ impl Session { }); }; let mut environment = turn_environment.selection(); - environment.cwd = cwd; + environment.cwd = PathUri::from_abs_path(&cwd); self.request_permissions_for_environment( turn_context, call_id, @@ -2457,11 +2470,26 @@ impl Session { }; match entry { Some(entry) => { - let response = Self::normalize_request_permissions_response( - entry.requested_permissions, - response, - entry.environment.cwd.as_path(), - ); + // TODO(anp): Migrate request_permissions to support paths from foreign environments. + let response = match entry.environment.cwd.to_abs_path() { + Ok(native_environment_cwd) => Self::normalize_request_permissions_response( + entry.requested_permissions, + response, + native_environment_cwd.as_path(), + ), + Err(err) => { + warn!( + cwd = %entry.environment.cwd, + %err, + "request_permissions requires a cwd native to the Codex host" + ); + RequestPermissionsResponse { + permissions: RequestPermissionProfile::default(), + scope: PermissionGrantScope::Turn, + strict_auto_review: false, + } + } + }; self.record_granted_request_permissions_for_turn( &response, &entry.environment.environment_id, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 363490a6a..460b2b473 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -58,6 +58,7 @@ use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::TurnEnvironmentSelections; use codex_protocol::request_permissions::PermissionGrantScope; use codex_protocol::request_permissions::RequestPermissionProfile; +use codex_utils_path_uri::PathUri; use tracing::Span; use crate::rollout::recorder::RolloutRecorder; @@ -4705,7 +4706,7 @@ async fn cwd_update_rewrites_sticky_environment_cwd() { assert_eq!(state.session_configuration.cwd(), &updated_cwd); assert_eq!( state.session_configuration.environment_selections()[0].cwd, - updated_cwd + PathUri::from_abs_path(&updated_cwd) ); assert_ne!(environment_cwd, updated_cwd); } diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index fdd834b20..70b085fc5 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -79,7 +79,7 @@ impl TurnEnvironment { pub(crate) fn selection(&self) -> TurnEnvironmentSelection { TurnEnvironmentSelection { environment_id: self.environment_id.clone(), - cwd: self.cwd.clone(), + cwd: self.cwd_uri.clone(), } } } diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 790f5ab8c..06a6374e7 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -23,6 +23,7 @@ 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::PathUri; use core_test_support::PathBufExt; use core_test_support::PathExt; use core_test_support::responses::mount_models_once; @@ -331,7 +332,7 @@ async fn start_thread_rejects_explicit_local_environment_when_default_provider_i parent_trace: None, environments: vec![TurnEnvironmentSelection { environment_id: "local".to_string(), - cwd: config.cwd.clone(), + cwd: PathUri::from_abs_path(&config.cwd), }], thread_extension_init: Default::default(), }) @@ -594,7 +595,7 @@ async fn resume_and_fork_do_not_restore_thread_environments_from_rollout() { std::fs::create_dir_all(&selected_cwd).expect("create selected cwd"); let environments = vec![TurnEnvironmentSelection { environment_id: "local".to_string(), - cwd: selected_cwd.clone(), + cwd: PathUri::from_abs_path(&selected_cwd), }]; let default_cwd = config.cwd.clone(); let mut source_config = config.clone(); 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 55d3befee..92a37ae10 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 @@ -151,9 +151,16 @@ impl ExecCommandHandler { let process_id = manager.allocate_process_id().await; let shell_mode = shell_mode_for_environment(&turn.unified_exec_shell_mode, environment.as_ref()); + // Remote environments may use a different OS and must build commands with their native + // shell; fall back to the session shell when the environment did not report one. + let shell = turn_environment + .shell + .clone() + .map(Arc::new) + .unwrap_or_else(|| session.user_shell()); let resolved_command = get_command( &args, - session.user_shell(), + shell, &shell_mode, turn.config.permissions.allow_login_shell, ) diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index cd7541bb1..477e325f5 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -104,7 +104,7 @@ impl UserInstructionsProvider for RecordingUserInstructionsProvider { pub fn local(cwd: AbsolutePathBuf) -> TurnEnvironmentSelection { TurnEnvironmentSelection { environment_id: codex_exec_server::LOCAL_ENVIRONMENT_ID.to_string(), - cwd, + cwd: PathUri::from_abs_path(&cwd), } } diff --git a/codex-rs/core/tests/remote_env_windows/BUILD.bazel b/codex-rs/core/tests/remote_env_windows/BUILD.bazel index a620965c8..de7dcfaa7 100644 --- a/codex-rs/core/tests/remote_env_windows/BUILD.bazel +++ b/codex-rs/core/tests/remote_env_windows/BUILD.bazel @@ -6,19 +6,25 @@ wine_rust_test( srcs = ["remote_env_windows_test.rs"], crate_name = "remote_env_windows_test", crate_root = "remote_env_windows_test.rs", + host_binaries = { + "codex-app-server": "//codex-rs/app-server:codex-app-server", + }, windows_binaries = { "wine-windows-exec-server": "//codex-rs/exec-server/testing:windows-exec-server", }, deps = [ - "//bazel/rules/testing/wine:wine_test_support", + "//codex-rs/app-server-protocol", + "//codex-rs/app-server/tests/common", "//codex-rs/core/tests/common", "//codex-rs/exec-server", + "//codex-rs/exec-server/testing:wine-exec-server-test-support", "//codex-rs/features", "//codex-rs/protocol", - "//codex-rs/utils/cargo-bin", + "//codex-rs/utils/path-uri", "@crates//:anyhow", "@crates//:pretty_assertions", "@crates//:serde_json", + "@crates//:tempfile", "@crates//:tokio", ], ) 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 b505227b6..24efb8e7e 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,11 +2,16 @@ use anyhow::Context; use anyhow::Result; +use app_test_support::TestAppServer; +use codex_app_server_protocol::JSONRPCError; +use codex_app_server_protocol::RequestId; use codex_exec_server::REMOTE_ENVIRONMENT_ID; +use codex_exec_server::CODEX_EXEC_SERVER_URL_ENV_VAR; use codex_features::Feature; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::ExecCommandStatus; use codex_protocol::protocol::Op; use codex_protocol::protocol::TurnEnvironmentSelection; use codex_protocol::protocol::TurnEnvironmentSelections; @@ -21,40 +26,27 @@ 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::PathUri; +use pretty_assertions::assert_eq; use serde_json::json; -use tokio::io::AsyncBufReadExt; -use tokio::io::BufReader; -use wine_test_support::WineTestCommand; +use tempfile::TempDir; +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 = "echo WINE_BAZEL_OK&&cd"; +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_rejects_non_native_cwd_uri() -> Result<()> { - let executable = codex_utils_cargo_bin::cargo_bin("wine-windows-exec-server")?; - let mut exec_server = WineTestCommand::new(executable) - .env("CODEX_HOME", r"C:\codex-home") - .spawn()?; - let stdout = exec_server.take_stdout(); - - exec_server - .scope(async move { - let mut lines = BufReader::new(stdout).lines(); - let exec_server_url = loop { - let line = lines - .next_line() - .await? - .context("Wine exec-server exited before reporting its URL")?; - if line.starts_with("ws://") { - break line; - } - }; - +async fn windows_exec_server_runs_with_native_shell_and_cwd() -> Result<()> { + WineExecServer + .scope(|exec_server_url| async move { let server = start_mock_server().await; let arguments = serde_json::to_string(&json!({ "cmd": COMMAND, "login": false, - "yield_time_ms": 5_000, + "yield_time_ms": 10_000, }))?; let response_mock = mount_sse_sequence( &server, @@ -90,7 +82,7 @@ async fn windows_exec_server_rejects_non_native_cwd_uri() -> Result<()> { test.config.cwd.clone(), vec![TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: test.config.cwd.clone(), + cwd: PathUri::parse("file:///C:/windows")?, }], ); @@ -121,39 +113,92 @@ async fn windows_exec_server_rejects_non_native_cwd_uri() -> Result<()> { }) .await?; - let mut saw_exec_event = false; + let mut begin = None; + let mut end = None; + let mut turn_complete = false; loop { match wait_for_event(&test.codex, |_| true).await { EventMsg::ExecCommandBegin(event) if event.call_id == CALL_ID => { - saw_exec_event = true + begin = Some(event) } EventMsg::ExecCommandEnd(event) if event.call_id == CALL_ID => { - saw_exec_event = true + end = Some(event) } - EventMsg::TurnComplete(_) => break, + EventMsg::TurnComplete(_) => turn_complete = true, _ => {} } + if turn_complete && end.is_some() { + break; + } } + let begin = begin.context("exec_command should emit a begin event")?; assert!( - !saw_exec_event, - "a non-native cwd should be rejected before process lifecycle events", + begin.command.first().is_some_and(|command| command + .to_ascii_lowercase() + .ends_with("pwsh.exe")), + "unexpected command: {:?}", + begin.command ); + assert_eq!( + &begin.command[1..], + ["-NoProfile", "-Command", COMMAND] + ); + + let end = end.context("exec_command should emit an end event")?; + assert_eq!((end.exit_code, end.status), (0, ExecCommandStatus::Completed)); let request = response_mock .last_request() - .context("model should receive the rejected command output")?; - let (output, success) = request + .context("model should receive the command output")?; + let (_output, success) = request .function_call_output_content_and_success(CALL_ID) - .context("rejected command output should be present")?; - let output = output.context("rejected command output should contain text")?; - assert!( - output.contains("exec-server rejected request (-32602)") - && output.contains("cwd URI") - && output.contains("is not valid on this exec-server host"), - "unexpected command output: {output:?}", - ); - assert_ne!(success, Some(true)); + .context("command output should be present")?; + assert_ne!(success, Some(false)); + + Ok(()) + }) + .await +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn app_server_rejects_windows_environment_cwd() -> Result<()> { + WineExecServer + .scope(|exec_server_url| async move { + let codex_home = TempDir::new()?; + let mut app_server = TestAppServer::new_with_env( + codex_home.path(), + &[( + CODEX_EXEC_SERVER_URL_ENV_VAR, + Some(exec_server_url.as_str()), + )], + ) + .await?; + 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", + }], + })), + ) + .await?; + let error: JSONRPCError = timeout( + APP_SERVER_READ_TIMEOUT, + app_server.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 request: AbsolutePathBuf deserialized without a base path" + ); Ok(()) }) diff --git a/codex-rs/core/tests/suite/agents_md.rs b/codex-rs/core/tests/suite/agents_md.rs index f350195d8..05fea8415 100644 --- a/codex-rs/core/tests/suite/agents_md.rs +++ b/codex-rs/core/tests/suite/agents_md.rs @@ -646,11 +646,11 @@ async fn multi_environment_thread_loads_every_project_and_keeps_creation_snapsho environments: vec![ TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: test.config.cwd.clone(), + cwd: PathUri::from_abs_path(&test.config.cwd), }, TurnEnvironmentSelection { environment_id: LOCAL_ENVIRONMENT_ID.to_string(), - cwd: local_root.path().to_path_buf().try_into()?, + cwd: PathUri::from_path(local_root.path())?, }, ], thread_extension_init: Default::default(), diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index 7359f7f90..b417a38fc 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -1635,7 +1635,7 @@ async fn apply_patch_turn_diff_tracks_local_and_remote_environment_paths() -> Re local(shared_cwd.clone()), TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: shared_cwd.clone(), + cwd: PathUri::from_abs_path(&shared_cwd), }, ]; test.codex diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 3ec2301aa..12aa3876a 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -352,7 +352,7 @@ async fn exec_command_routes_to_selected_remote_environment() -> Result<()> { .await?; let remote_selection = TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: remote_cwd.clone(), + cwd: PathUri::from_abs_path(&remote_cwd), }; let multi_env_output = exec_command_routing_output( &test, @@ -508,7 +508,7 @@ async fn remote_request_permissions_grant_unblocks_later_remote_exec() -> Result local(local_cwd.path().abs()), TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: remote_cwd.clone(), + cwd: PathUri::from_abs_path(&remote_cwd), }, ], ) @@ -648,7 +648,7 @@ async fn apply_patch_freeform_routes_to_selected_remote_environment() -> Result< local(local_cwd.path().abs()), TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: remote_cwd.clone(), + cwd: PathUri::from_abs_path(&remote_cwd), }, ]), ) @@ -731,7 +731,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> { local(local_cwd.path().abs()), TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: remote_cwd.clone(), + cwd: PathUri::from_abs_path(&remote_cwd), }, ]; let local_patch = format!( @@ -929,7 +929,7 @@ async fn apply_patch_intercepted_exec_command_routes_to_selected_remote_environm local(local_cwd.path().abs()), TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: remote_cwd.clone(), + cwd: PathUri::from_abs_path(&remote_cwd), }, ]), ) diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index 6674dcb8d..bbce61084 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -624,7 +624,7 @@ async fn view_image_routes_to_selected_remote_environment() -> anyhow::Result<() .await?; let remote_selection = TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), - cwd: remote_cwd.clone(), + cwd: PathUri::from_abs_path(&remote_cwd), }; let call_id = "call-view-image-multi-env"; let response_mock = mount_sse_sequence( diff --git a/codex-rs/exec-server/testing/BUILD.bazel b/codex-rs/exec-server/testing/BUILD.bazel index ad0f5b855..bf50e0bce 100644 --- a/codex-rs/exec-server/testing/BUILD.bazel +++ b/codex-rs/exec-server/testing/BUILD.bazel @@ -1,4 +1,20 @@ -load("@rules_rust//rust:defs.bzl", "rust_binary") +load("@rules_rust//rust:defs.bzl", "rust_binary", "rust_library") + +rust_library( + name = "wine-exec-server-test-support", + testonly = True, + srcs = ["wine_exec_server.rs"], + crate_name = "wine_exec_server_test_support", + crate_root = "wine_exec_server.rs", + target_compatible_with = ["@platforms//os:linux"], + visibility = ["//codex-rs/core/tests/remote_env_windows:__pkg__"], + deps = [ + "//bazel/rules/testing/wine:wine_test_support", + "//codex-rs/utils/cargo-bin", + "@crates//:anyhow", + "@crates//:tokio", + ], +) rust_binary( name = "windows-exec-server", diff --git a/codex-rs/exec-server/testing/wine_exec_server.rs b/codex-rs/exec-server/testing/wine_exec_server.rs new file mode 100644 index 000000000..88857e785 --- /dev/null +++ b/codex-rs/exec-server/testing/wine_exec_server.rs @@ -0,0 +1,43 @@ +//! Test support for running the Windows exec-server under Wine. + +use std::future::Future; + +use anyhow::Context; +use anyhow::Result; +use tokio::io::AsyncBufReadExt; +use tokio::io::BufReader; +use wine_test_support::WineTestCommand; + +/// Runs the Windows exec-server under Wine for the duration of a scoped operation. +pub struct WineExecServer; + +impl WineExecServer { + /// Starts the server, passes its WebSocket URL to `operation`, and tears it down afterward. + pub async fn scope(self, operation: F) -> Result + where + F: FnOnce(String) -> Fut, + Fut: Future>, + { + let executable = codex_utils_cargo_bin::cargo_bin("wine-windows-exec-server")?; + let mut exec_server = WineTestCommand::new(executable) + .env("CODEX_HOME", r"C:\codex-home") + .spawn()?; + let stdout = exec_server.take_stdout(); + + exec_server + .scope(async move { + let mut lines = BufReader::new(stdout).lines(); + let exec_server_url = loop { + let line = lines + .next_line() + .await? + .context("Wine exec-server exited before reporting its URL")?; + if line.starts_with("ws://") { + break line; + } + }; + operation(exec_server_url).await + }) + .await + } +} diff --git a/codex-rs/protocol/Cargo.toml b/codex-rs/protocol/Cargo.toml index 89b10e383..5d1ff57f5 100644 --- a/codex-rs/protocol/Cargo.toml +++ b/codex-rs/protocol/Cargo.toml @@ -20,6 +20,7 @@ codex-execpolicy = { workspace = true } codex-network-proxy = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-image = { workspace = true } +codex-utils-path-uri = { workspace = true } codex-utils-string = { workspace = true } encoding_rs = { workspace = true } globset = { workspace = true } diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 48eb2352f..b41a02b88 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -52,6 +52,7 @@ use crate::request_permissions::RequestPermissionsResponse; use crate::request_user_input::RequestUserInputResponse; use crate::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; @@ -105,11 +106,14 @@ pub const COLLABORATION_MODE_CLOSE_TAG: &str = ""; pub const REALTIME_CONVERSATION_OPEN_TAG: &str = ""; pub const REALTIME_CONVERSATION_CLOSE_TAG: &str = ""; pub const USER_MESSAGE_BEGIN: &str = "## My request for Codex:"; +const LOCAL_ENVIRONMENT_ID: &str = "local"; +// TODO(anp): Replace `TurnEnvironmentSelection` with `PathUri` once path URIs carry environment +// identifiers. #[derive(Debug, Clone, PartialEq)] pub struct TurnEnvironmentSelection { pub environment_id: String, - pub cwd: AbsolutePathBuf, + pub cwd: PathUri, } #[derive(Debug, Clone, PartialEq)] @@ -132,10 +136,13 @@ impl TurnEnvironmentSelections { } fn sync_primary_environment_cwd(&mut self) { + let legacy_fallback_cwd = PathUri::from_abs_path(&self.legacy_fallback_cwd); + // Keep remote environments' native cwd instead of replacing it with the local fallback. if let Some(turn_environment) = self.environments.first_mut() - && turn_environment.cwd != self.legacy_fallback_cwd + && turn_environment.environment_id == LOCAL_ENVIRONMENT_ID + && turn_environment.cwd != legacy_fallback_cwd { - turn_environment.cwd = self.legacy_fallback_cwd.clone(); + turn_environment.cwd = legacy_fallback_cwd; } } }