From 5089f0533ee63022b397bc68686c263c710c5827 Mon Sep 17 00:00:00 2001 From: viyatb-oai Date: Mon, 15 Jun 2026 22:41:32 -0700 Subject: [PATCH] fix(exec-server): default omitted cwd to server directory Co-authored-by: Codex --- .../core/src/unified_exec/process_manager.rs | 2 +- .../src/unified_exec/process_manager_tests.rs | 2 +- codex-rs/exec-server/src/environment.rs | 6 +- codex-rs/exec-server/src/local_process.rs | 21 ++++--- codex-rs/exec-server/src/protocol.rs | 3 +- .../exec-server/src/server/handler/tests.rs | 2 +- codex-rs/exec-server/src/server/processor.rs | 2 +- codex-rs/exec-server/tests/exec_process.rs | 59 +++++++++++++++---- codex-rs/exec-server/tests/relay.rs | 4 +- .../rmcp-client/src/stdio_server_launcher.rs | 2 +- 10 files changed, 73 insertions(+), 30 deletions(-) diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index 702a339e2e..caf5aa2a67 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -157,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: PathUri::from_abs_path(&request.cwd), + cwd: Some(PathUri::from_abs_path(&request.cwd)), env_policy, env, tty, diff --git a/codex-rs/core/src/unified_exec/process_manager_tests.rs b/codex-rs/core/src/unified_exec/process_manager_tests.rs index 49758ec799..76cc9bfd28 100644 --- a/codex-rs/core/src/unified_exec/process_manager_tests.rs +++ b/codex-rs/core/src/unified_exec/process_manager_tests.rs @@ -116,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, PathUri::from_abs_path(&request.cwd)); + assert_eq!(params.cwd, Some(PathUri::from_abs_path(&request.cwd))); assert!(params.env_policy.is_some()); assert_eq!( params.env, diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 0c703ecef3..eeb0372b2f 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -897,8 +897,10 @@ mod tests { .start(crate::ExecParams { process_id: ProcessId::from("default-env-proc"), argv: vec!["true".to_string()], - cwd: PathUri::from_path(std::env::current_dir().expect("read current dir")) - .expect("cwd URI"), + cwd: Some( + PathUri::from_path(std::env::current_dir().expect("read current dir")) + .expect("cwd URI"), + ), env_policy: None, env: Default::default(), tty: false, diff --git a/codex-rs/exec-server/src/local_process.rs b/codex-rs/exec-server/src/local_process.rs index 78a53897ee..8e4a4b1590 100644 --- a/codex-rs/exec-server/src/local_process.rs +++ b/codex-rs/exec-server/src/local_process.rs @@ -156,12 +156,17 @@ 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| { - invalid_params(format!( - "cwd URI `{}` is not valid on this exec-server host: {err}", - params.cwd - )) - })?; + let native_cwd = match ¶ms.cwd { + Some(cwd) => cwd.to_abs_path().map_err(|err| { + invalid_params(format!( + "cwd URI `{cwd}` is not valid on this exec-server host: {err}" + )) + })?, + None => std::env::current_dir() + .map_err(|err| internal_error(format!("failed to read exec-server cwd: {err}")))? + .try_into() + .map_err(|err| internal_error(format!("exec-server cwd is not absolute: {err}")))?, + }; { let mut process_map = self.inner.processes.lock().await; @@ -799,7 +804,7 @@ mod tests { ExecParams { process_id: ProcessId::from("env-test"), argv: vec!["true".to_string()], - cwd: PathUri::from_path(std::env::current_dir().expect("cwd")).expect("cwd URI"), + cwd: Some(PathUri::from_path(std::env::current_dir().expect("cwd")).expect("cwd URI")), env_policy: None, env, tty: false, @@ -822,7 +827,7 @@ mod tests { "cwd URI `{cwd}` is not valid on this exec-server host: {source}" )); let mut params = test_exec_params(HashMap::new()); - params.cwd = cwd; + params.cwd = Some(cwd); let result = LocalProcess::default().start_process(params).await; let Err(error) = result else { diff --git a/codex-rs/exec-server/src/protocol.rs b/codex-rs/exec-server/src/protocol.rs index 49b96b9e50..d3568bc8df 100644 --- a/codex-rs/exec-server/src/protocol.rs +++ b/codex-rs/exec-server/src/protocol.rs @@ -89,7 +89,8 @@ pub struct ExecParams { pub process_id: ProcessId, pub argv: Vec, /// Working directory URI, interpreted using the exec-server host's path rules at launch time. - pub cwd: PathUri, + /// Defaults to the exec-server process working directory when omitted. + pub cwd: Option, #[serde(default)] pub env_policy: Option, pub env: HashMap, diff --git a/codex-rs/exec-server/src/server/handler/tests.rs b/codex-rs/exec-server/src/server/handler/tests.rs index 97bfba5344..b6a63b8eb1 100644 --- a/codex-rs/exec-server/src/server/handler/tests.rs +++ b/codex-rs/exec-server/src/server/handler/tests.rs @@ -27,7 +27,7 @@ fn exec_params_with_argv(process_id: &str, argv: Vec) -> ExecParams { ExecParams { process_id: ProcessId::from(process_id), argv, - cwd: PathUri::from_path(std::env::current_dir().expect("cwd")).expect("cwd URI"), + cwd: Some(PathUri::from_path(std::env::current_dir().expect("cwd")).expect("cwd URI")), env_policy: None, env: inherited_path_env(), tty: false, diff --git a/codex-rs/exec-server/src/server/processor.rs b/codex-rs/exec-server/src/server/processor.rs index 952f414213..dd82325245 100644 --- a/codex-rs/exec-server/src/server/processor.rs +++ b/codex-rs/exec-server/src/server/processor.rs @@ -397,7 +397,7 @@ mod tests { ExecParams { process_id, argv: sleep_then_print_argv(), - cwd: PathUri::from_path(std::env::current_dir().expect("cwd")).expect("cwd URI"), + cwd: Some(PathUri::from_path(std::env::current_dir().expect("cwd")).expect("cwd URI")), env_policy: None, env, tty: false, diff --git a/codex-rs/exec-server/tests/exec_process.rs b/codex-rs/exec-server/tests/exec_process.rs index e5522f4073..d9a9dc6225 100644 --- a/codex-rs/exec-server/tests/exec_process.rs +++ b/codex-rs/exec-server/tests/exec_process.rs @@ -73,7 +73,7 @@ async fn assert_exec_process_starts_and_exits(use_remote: bool) -> Result<()> { .start(ExecParams { process_id: ProcessId::from("proc-1"), argv: vec!["true".to_string()], - cwd: PathUri::from_path(std::env::current_dir()?)?, + cwd: Some(PathUri::from_path(std::env::current_dir()?)?), env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -91,6 +91,31 @@ async fn assert_exec_process_starts_and_exits(use_remote: bool) -> Result<()> { Ok(()) } +async fn assert_exec_process_defaults_to_server_cwd(use_remote: bool) -> Result<()> { + let context = create_process_context(use_remote).await?; + let session = context + .backend + .start(ExecParams { + process_id: ProcessId::from("proc-default-cwd"), + argv: vec!["pwd".to_string()], + cwd: None, + env_policy: /*env_policy*/ None, + env: Default::default(), + tty: false, + pipe_stdin: false, + arg0: None, + }) + .await?; + let wake_rx = session.process.subscribe_wake(); + let (output, exit_code, closed) = + collect_process_output_from_reads(session.process, wake_rx).await?; + + assert_eq!(output.trim(), std::env::current_dir()?.to_string_lossy()); + assert_eq!(exit_code, Some(0)); + assert!(closed); + Ok(()) +} + async fn read_process_until_change( session: Arc, wake_rx: &mut watch::Receiver, @@ -214,7 +239,7 @@ async fn assert_exec_process_streams_output(use_remote: bool) -> Result<()> { "-c".to_string(), "sleep 0.05; printf 'session output\\n'".to_string(), ], - cwd: PathUri::from_path(std::env::current_dir()?)?, + cwd: Some(PathUri::from_path(std::env::current_dir()?)?), env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -245,7 +270,7 @@ async fn assert_exec_process_pushes_events(use_remote: bool) -> Result<()> { "-c".to_string(), "printf 'event output\\n'; sleep 0.1; printf 'event err\\n' >&2; sleep 0.1; exit 7".to_string(), ], - cwd: PathUri::from_path(std::env::current_dir()?)?, + cwd: Some(PathUri::from_path(std::env::current_dir()?)?), env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -292,7 +317,7 @@ async fn assert_exec_process_replays_events_after_close(use_remote: bool) -> Res "-c".to_string(), "printf 'late one\\n'; printf 'late two\\n'".to_string(), ], - cwd: PathUri::from_path(std::env::current_dir()?)?, + cwd: Some(PathUri::from_path(std::env::current_dir()?)?), env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -340,7 +365,7 @@ async fn assert_exec_process_retains_output_after_exit_until_streams_close( DELAYED_OUTPUT_AFTER_EXIT_PARENT_ARG.to_string(), release_path.to_string_lossy().into_owned(), ], - cwd: PathUri::from_path(std::env::current_dir()?)?, + cwd: Some(PathUri::from_path(std::env::current_dir()?)?), env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -413,7 +438,7 @@ async fn assert_exec_process_write_then_read(use_remote: bool) -> Result<()> { "-c".to_string(), "IFS= read line; printf 'from-stdin:%s\\n' \"$line\"".to_string(), ], - cwd: PathUri::from_path(std::env::current_dir()?)?, + cwd: Some(PathUri::from_path(std::env::current_dir()?)?), env_policy: /*env_policy*/ None, env: Default::default(), tty: true, @@ -450,7 +475,7 @@ async fn assert_exec_process_write_then_read_without_tty(use_remote: bool) -> Re "-c".to_string(), "IFS= read line; printf 'from-stdin:%s\\n' \"$line\"".to_string(), ], - cwd: PathUri::from_path(std::env::current_dir()?)?, + cwd: Some(PathUri::from_path(std::env::current_dir()?)?), env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -483,7 +508,7 @@ async fn assert_exec_process_rejects_write_without_pipe_stdin(use_remote: bool) "-c".to_string(), "sleep 0.3; if IFS= read -r line; then printf 'read:%s\\n' \"$line\"; else printf 'eof\\n'; fi".to_string(), ], - cwd: PathUri::from_path(std::env::current_dir()?)?, + cwd: Some(PathUri::from_path(std::env::current_dir()?)?), env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -517,7 +542,7 @@ async fn assert_exec_process_signal_interrupts_process(use_remote: bool) -> Resu "-c".to_string(), "trap 'printf \"signal:2\\n\"; exit 7' INT; printf 'ready\\n'; while :; do :; done".to_string(), ], - cwd: PathUri::from_path(std::env::current_dir()?)?, + cwd: Some(PathUri::from_path(std::env::current_dir()?)?), env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -570,7 +595,7 @@ async fn assert_exec_process_signal_reports_unsupported_on_windows(use_remote: b "/C".to_string(), "echo ready && ping -n 30 127.0.0.1 >NUL".to_string(), ], - cwd: PathUri::from_path(std::env::current_dir()?)?, + cwd: Some(PathUri::from_path(std::env::current_dir()?)?), env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -610,7 +635,7 @@ async fn assert_exec_process_preserves_queued_events_before_subscribe( "-c".to_string(), "printf 'queued output\\n'".to_string(), ], - cwd: PathUri::from_path(std::env::current_dir()?)?, + cwd: Some(PathUri::from_path(std::env::current_dir()?)?), env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -645,7 +670,7 @@ async fn remote_exec_process_reports_transport_disconnect() -> Result<()> { "-c".to_string(), "sleep 10".to_string(), ], - cwd: PathUri::from_path(std::env::current_dir()?)?, + cwd: Some(PathUri::from_path(std::env::current_dir()?)?), env_policy: /*env_policy*/ None, env: Default::default(), tty: false, @@ -731,6 +756,16 @@ async fn exec_process_starts_and_exits(use_remote: bool) -> Result<()> { assert_exec_process_starts_and_exits(use_remote).await } +#[test_case(false ; "local")] +#[test_case(true ; "remote")] +#[cfg_attr(not(unix), ignore = "Unix-only exec-server process test")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +// Serialize tests that launch a real exec-server process through the full CLI. +#[serial_test::serial(remote_exec_server)] +async fn exec_process_defaults_to_server_cwd(use_remote: bool) -> Result<()> { + assert_exec_process_defaults_to_server_cwd(use_remote).await +} + #[test_case(false ; "local")] #[test_case(true ; "remote")] #[cfg_attr(not(unix), ignore = "Unix-only exec-server process test")] diff --git a/codex-rs/exec-server/tests/relay.rs b/codex-rs/exec-server/tests/relay.rs index 114fe122a2..234b186a46 100644 --- a/codex-rs/exec-server/tests/relay.rs +++ b/codex-rs/exec-server/tests/relay.rs @@ -115,7 +115,7 @@ async fn noise_environment_refreshes_bundle_for_each_connection_attempt() -> Res .start(ExecParams { process_id: ProcessId::new(format!("proc-{attempt}")), argv: vec!["true".to_string()], - cwd: PathUri::from_path(std::env::current_dir()?)?, + cwd: Some(PathUri::from_path(std::env::current_dir()?)?), env_policy: None, env: HashMap::new(), tty: false, @@ -209,7 +209,7 @@ async fn remote_environment_routes_encrypted_exec_server_rpc() -> Result<()> { .exec(ExecParams { process_id: ProcessId::from("proc-1"), argv: vec!["true".to_string()], - cwd: PathUri::from_path(std::env::current_dir()?)?, + cwd: Some(PathUri::from_path(std::env::current_dir()?)?), env_policy: None, env: HashMap::new(), tty: false, diff --git a/codex-rs/rmcp-client/src/stdio_server_launcher.rs b/codex-rs/rmcp-client/src/stdio_server_launcher.rs index 8ada806ff7..beb05b289f 100644 --- a/codex-rs/rmcp-client/src/stdio_server_launcher.rs +++ b/codex-rs/rmcp-client/src/stdio_server_launcher.rs @@ -497,7 +497,7 @@ impl ExecutorStdioServerLauncher { .start(ExecParams { process_id, argv, - cwd, + cwd: Some(cwd), env_policy: Some(Self::remote_env_policy(&remote_env_vars)), env, tty: false,