mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Carry exec-server cwd as PathUri (#28032)
## Why This is the second-to-last place in the exec-server protocol that needs to migrate to URIs to support cross-OS operation. ## What - Change `ExecParams.cwd` to `PathUri`. - Keep the cwd URI-shaped through core and rmcp producers, converting it to `AbsolutePathBuf` only in `LocalProcess::start_process`. - Reject non-native cwd URIs before launch and update the affected protocol documentation and call sites.
This commit is contained in:
@@ -157,7 +157,7 @@ Request params:
|
||||
{
|
||||
"processId": "proc-1",
|
||||
"argv": ["bash", "-lc", "printf 'hello\\n'"],
|
||||
"cwd": "/absolute/working/directory",
|
||||
"cwd": "file:///absolute/working/directory",
|
||||
"env": {
|
||||
"PATH": "/usr/bin:/bin"
|
||||
},
|
||||
@@ -171,7 +171,7 @@ Field definitions:
|
||||
|
||||
- `processId`: caller-chosen stable id for this process within the connection.
|
||||
- `argv`: command vector. It must be non-empty.
|
||||
- `cwd`: absolute working directory used for the child process.
|
||||
- `cwd`: `file:` URI for the child process working directory.
|
||||
- `env`: environment variables passed to the child process.
|
||||
- `tty`: when `true`, spawn a PTY-backed interactive process.
|
||||
- `pipeStdin`: when `true`, keep non-PTY stdin writable via `process/write`.
|
||||
@@ -409,7 +409,7 @@ Initialize:
|
||||
Start a process:
|
||||
|
||||
```json
|
||||
{"id":2,"method":"process/start","params":{"processId":"proc-1","argv":["bash","-lc","printf 'ready\\n'; while IFS= read -r line; do printf 'echo:%s\\n' \"$line\"; done"],"cwd":"/tmp","env":{"PATH":"/usr/bin:/bin"},"tty":true,"pipeStdin":false,"arg0":null}}
|
||||
{"id":2,"method":"process/start","params":{"processId":"proc-1","argv":["bash","-lc","printf 'ready\\n'; while IFS= read -r line; do printf 'echo:%s\\n' \"$line\"; done"],"cwd":"file:///tmp","env":{"PATH":"/usr/bin:/bin"},"tty":true,"pipeStdin":false,"arg0":null}}
|
||||
{"id":2,"result":{"processId":"proc-1"}}
|
||||
{"method":"process/output","params":{"processId":"proc-1","seq":1,"stream":"stdout","chunk":"cmVhZHkK"}}
|
||||
```
|
||||
|
||||
@@ -498,6 +498,7 @@ mod tests {
|
||||
use crate::ProcessId;
|
||||
use crate::environment_provider::EnvironmentDefault;
|
||||
use crate::environment_provider::EnvironmentProviderSnapshot;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn test_runtime_paths() -> ExecServerRuntimePaths {
|
||||
@@ -862,7 +863,8 @@ mod tests {
|
||||
.start(crate::ExecParams {
|
||||
process_id: ProcessId::from("default-env-proc"),
|
||||
argv: vec!["true".to_string()],
|
||||
cwd: std::env::current_dir().expect("read current dir"),
|
||||
cwd: PathUri::from_path(std::env::current_dir().expect("read current dir"))
|
||||
.expect("cwd URI"),
|
||||
env_policy: None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
|
||||
@@ -156,6 +156,12 @@ 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 mut process_map = self.inner.processes.lock().await;
|
||||
@@ -172,7 +178,7 @@ impl LocalProcess {
|
||||
codex_utils_pty::spawn_pty_process(
|
||||
program,
|
||||
args,
|
||||
params.cwd.as_path(),
|
||||
native_cwd.as_path(),
|
||||
&env,
|
||||
¶ms.arg0,
|
||||
TerminalSize::default(),
|
||||
@@ -182,7 +188,7 @@ impl LocalProcess {
|
||||
codex_utils_pty::spawn_pipe_process(
|
||||
program,
|
||||
args,
|
||||
params.cwd.as_path(),
|
||||
native_cwd.as_path(),
|
||||
&env,
|
||||
¶ms.arg0,
|
||||
)
|
||||
@@ -191,7 +197,7 @@ impl LocalProcess {
|
||||
codex_utils_pty::spawn_pipe_process_no_stdin(
|
||||
program,
|
||||
args,
|
||||
params.cwd.as_path(),
|
||||
native_cwd.as_path(),
|
||||
&env,
|
||||
¶ms.arg0,
|
||||
)
|
||||
@@ -783,6 +789,7 @@ fn notification_sender(inner: &Inner) -> Option<RpcNotificationSender> {
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_protocol::config_types::ShellEnvironmentPolicyInherit;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use codex_utils_pty::ProcessDriver;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tokio::sync::oneshot;
|
||||
@@ -792,7 +799,7 @@ mod tests {
|
||||
ExecParams {
|
||||
process_id: ProcessId::from("env-test"),
|
||||
argv: vec!["true".to_string()],
|
||||
cwd: std::path::PathBuf::from("/tmp"),
|
||||
cwd: PathUri::from_path(std::env::current_dir().expect("cwd")).expect("cwd URI"),
|
||||
env_policy: None,
|
||||
env,
|
||||
tty: false,
|
||||
@@ -801,6 +808,30 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn start_process_rejects_non_native_cwd_before_launch() {
|
||||
#[cfg(unix)]
|
||||
let uri = "file://server/share/checkout";
|
||||
#[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 mut params = test_exec_params(HashMap::new());
|
||||
params.cwd = cwd;
|
||||
|
||||
let result = LocalProcess::default().start_process(params).await;
|
||||
let Err(error) = result else {
|
||||
panic!("non-native cwd should be rejected");
|
||||
};
|
||||
|
||||
assert_eq!(error, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn child_env_defaults_to_exact_env() {
|
||||
let params = test_exec_params(HashMap::from([("ONLY_THIS".to_string(), "1".to_string())]));
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use crate::FileSystemSandboxContext;
|
||||
use base64::engine::general_purpose::STANDARD as BASE64_STANDARD;
|
||||
@@ -77,7 +76,8 @@ pub struct EnvironmentInfo {
|
||||
pub struct ShellInfo {
|
||||
/// Stable shell name, for example `zsh`, `bash`, `powershell`, `sh`, or `cmd`.
|
||||
pub name: String,
|
||||
/// Path the exec server would use for that shell.
|
||||
/// Target-native shell executable path or command name. Fallbacks such as `cmd.exe` need not
|
||||
/// be absolute, so this is not a [`PathUri`].
|
||||
pub path: String,
|
||||
}
|
||||
|
||||
@@ -88,7 +88,8 @@ pub struct ExecParams {
|
||||
/// This is a protocol key, not an OS pid.
|
||||
pub process_id: ProcessId,
|
||||
pub argv: Vec<String>,
|
||||
pub cwd: PathBuf,
|
||||
/// Working directory URI, interpreted using the exec-server host's path rules at launch time.
|
||||
pub cwd: PathUri,
|
||||
#[serde(default)]
|
||||
pub env_policy: Option<ExecEnvPolicy>,
|
||||
pub env: HashMap<String, String>,
|
||||
@@ -96,6 +97,8 @@ pub struct ExecParams {
|
||||
/// Keep non-tty stdin writable through `process/write`.
|
||||
#[serde(default)]
|
||||
pub pipe_stdin: bool,
|
||||
/// Optional process-visible argv0 override. Values such as `codex-linux-sandbox` are command
|
||||
/// names rather than paths, so this is not a [`PathUri`].
|
||||
pub arg0: Option<String>,
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tokio::sync::mpsc;
|
||||
use uuid::Uuid;
|
||||
@@ -26,7 +27,7 @@ fn exec_params_with_argv(process_id: &str, argv: Vec<String>) -> ExecParams {
|
||||
ExecParams {
|
||||
process_id: ProcessId::from(process_id),
|
||||
argv,
|
||||
cwd: std::env::current_dir().expect("cwd"),
|
||||
cwd: PathUri::from_path(std::env::current_dir().expect("cwd")).expect("cwd URI"),
|
||||
env_policy: None,
|
||||
env: inherited_path_env(),
|
||||
tty: false,
|
||||
|
||||
@@ -195,6 +195,7 @@ mod tests {
|
||||
use codex_app_server_protocol::JSONRPCRequest;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use serde::Serialize;
|
||||
use serde::de::DeserializeOwned;
|
||||
use tokio::io::AsyncBufReadExt;
|
||||
@@ -396,7 +397,7 @@ mod tests {
|
||||
ExecParams {
|
||||
process_id,
|
||||
argv: sleep_then_print_argv(),
|
||||
cwd: std::env::current_dir().expect("cwd"),
|
||||
cwd: PathUri::from_path(std::env::current_dir().expect("cwd")).expect("cwd URI"),
|
||||
env_policy: None,
|
||||
env,
|
||||
tty: false,
|
||||
|
||||
@@ -15,6 +15,7 @@ use codex_exec_server::ProcessSignal;
|
||||
use codex_exec_server::ReadResponse;
|
||||
use codex_exec_server::StartedExecProcess;
|
||||
use codex_exec_server::WriteStatus;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
use test_case::test_case;
|
||||
@@ -72,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: std::env::current_dir()?,
|
||||
cwd: PathUri::from_path(std::env::current_dir()?)?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
@@ -213,7 +214,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: std::env::current_dir()?,
|
||||
cwd: PathUri::from_path(std::env::current_dir()?)?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
@@ -244,7 +245,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: std::env::current_dir()?,
|
||||
cwd: PathUri::from_path(std::env::current_dir()?)?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
@@ -291,7 +292,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: std::env::current_dir()?,
|
||||
cwd: PathUri::from_path(std::env::current_dir()?)?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
@@ -339,7 +340,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: std::env::current_dir()?,
|
||||
cwd: PathUri::from_path(std::env::current_dir()?)?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
@@ -412,7 +413,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: std::env::current_dir()?,
|
||||
cwd: PathUri::from_path(std::env::current_dir()?)?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: true,
|
||||
@@ -449,7 +450,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: std::env::current_dir()?,
|
||||
cwd: PathUri::from_path(std::env::current_dir()?)?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
@@ -482,7 +483,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: std::env::current_dir()?,
|
||||
cwd: PathUri::from_path(std::env::current_dir()?)?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
@@ -516,7 +517,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: std::env::current_dir()?,
|
||||
cwd: PathUri::from_path(std::env::current_dir()?)?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
@@ -569,7 +570,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: std::env::current_dir()?,
|
||||
cwd: PathUri::from_path(std::env::current_dir()?)?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
@@ -609,7 +610,7 @@ async fn assert_exec_process_preserves_queued_events_before_subscribe(
|
||||
"-c".to_string(),
|
||||
"printf 'queued output\\n'".to_string(),
|
||||
],
|
||||
cwd: std::env::current_dir()?,
|
||||
cwd: PathUri::from_path(std::env::current_dir()?)?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
@@ -644,7 +645,7 @@ async fn remote_exec_process_reports_transport_disconnect() -> Result<()> {
|
||||
"-c".to_string(),
|
||||
"sleep 10".to_string(),
|
||||
],
|
||||
cwd: std::env::current_dir()?,
|
||||
cwd: PathUri::from_path(std::env::current_dir()?)?,
|
||||
env_policy: /*env_policy*/ None,
|
||||
env: Default::default(),
|
||||
tty: false,
|
||||
|
||||
Reference in New Issue
Block a user