mirror of
https://github.com/openai/codex.git
synced 2026-10-21 17:48:44 +00:00
fix(exec-server): default omitted cwd to server directory
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -89,7 +89,8 @@ pub struct ExecParams {
|
||||
pub process_id: ProcessId,
|
||||
pub argv: Vec<String>,
|
||||
/// 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<PathUri>,
|
||||
#[serde(default)]
|
||||
pub env_policy: Option<ExecEnvPolicy>,
|
||||
pub env: HashMap<String, String>,
|
||||
|
||||
@@ -27,7 +27,7 @@ fn exec_params_with_argv(process_id: &str, argv: Vec<String>) -> 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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<dyn ExecProcess>,
|
||||
wake_rx: &mut watch::Receiver<u64>,
|
||||
@@ -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")]
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user