diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 51a6cd08cc..471ce1558d 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2848,6 +2848,7 @@ dependencies = [ "codex-shell-command", "codex-test-binary-support", "codex-utils-absolute-path", + "codex-utils-path-uri", "codex-utils-pty", "codex-utils-rustls-provider", "ctor 0.6.3", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index b87b0cf39f..ad4ebf7a79 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -237,6 +237,7 @@ codex-utils-json-to-toml = { path = "utils/json-to-toml" } codex-utils-oss = { path = "utils/oss" } codex-utils-output-truncation = { path = "utils/output-truncation" } codex-utils-path = { path = "utils/path-utils" } +codex-utils-path-uri = { path = "utils/path-uri" } codex-utils-plugins = { path = "utils/plugins" } codex-utils-pty = { path = "utils/pty" } codex-utils-rustls-provider = { path = "utils/rustls-provider" } diff --git a/codex-rs/config/src/loader/tests.rs b/codex-rs/config/src/loader/tests.rs index 6492f9a0c0..635e524e92 100644 --- a/codex-rs/config/src/loader/tests.rs +++ b/codex-rs/config/src/loader/tests.rs @@ -8,7 +8,6 @@ use codex_file_system::FileSystemSandboxContext; use codex_file_system::ReadDirectoryEntry; use codex_file_system::RemoveOptions; use pretty_assertions::assert_eq; -use std::path::Path; use tempfile::tempdir; struct TestFileSystem; @@ -23,18 +22,6 @@ impl ExecutorFileSystem for TestFileSystem { path.canonicalize() } - async fn join( - &self, - base_path: &AbsolutePathBuf, - path: &Path, - ) -> FileSystemResult { - Ok(base_path.join(path)) - } - - async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { - Ok(path.parent()) - } - async fn read_file( &self, path: &AbsolutePathBuf, diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index eadab83fe7..7e45834bab 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -52,6 +52,7 @@ uuid = { workspace = true, features = ["v4"] } [dev-dependencies] anyhow = { workspace = true } codex-test-binary-support = { workspace = true } +codex-utils-path-uri = { workspace = true } ctor = { workspace = true } http = { workspace = true } pretty_assertions = { workspace = true } diff --git a/codex-rs/exec-server/src/local_file_system.rs b/codex-rs/exec-server/src/local_file_system.rs index 3f410f926d..848762625f 100644 --- a/codex-rs/exec-server/src/local_file_system.rs +++ b/codex-rs/exec-server/src/local_file_system.rs @@ -88,18 +88,6 @@ impl ExecutorFileSystem for LocalFileSystem { file_system.canonicalize(path, sandbox).await } - async fn join( - &self, - base_path: &AbsolutePathBuf, - path: &Path, - ) -> FileSystemResult { - self.unsandboxed.join(base_path, path).await - } - - async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { - self.unsandboxed.parent(path).await - } - async fn read_file( &self, path: &AbsolutePathBuf, @@ -182,18 +170,6 @@ impl ExecutorFileSystem for UnsandboxedFileSystem { self.file_system.canonicalize(path, /*sandbox*/ None).await } - async fn join( - &self, - base_path: &AbsolutePathBuf, - path: &Path, - ) -> FileSystemResult { - self.file_system.join(base_path, path).await - } - - async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { - self.file_system.parent(path).await - } - async fn read_file( &self, path: &AbsolutePathBuf, @@ -289,18 +265,6 @@ impl ExecutorFileSystem for DirectFileSystem { AbsolutePathBuf::from_absolute_path(tokio::fs::canonicalize(path.as_path()).await?) } - async fn join( - &self, - base_path: &AbsolutePathBuf, - path: &Path, - ) -> FileSystemResult { - Ok(base_path.join(path)) - } - - async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { - Ok(path.parent()) - } - async fn read_file( &self, path: &AbsolutePathBuf, diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index 54198c93ba..f5b0f7e666 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -2,7 +2,6 @@ use async_trait::async_trait; use base64::Engine as _; use base64::engine::general_purpose::STANDARD; use codex_utils_absolute_path::AbsolutePathBuf; -use std::path::Path; use tokio::io; use tracing::trace; @@ -20,8 +19,6 @@ use crate::protocol::FsCanonicalizeParams; use crate::protocol::FsCopyParams; use crate::protocol::FsCreateDirectoryParams; use crate::protocol::FsGetMetadataParams; -use crate::protocol::FsJoinParams; -use crate::protocol::FsParentParams; use crate::protocol::FsReadDirectoryParams; use crate::protocol::FsReadFileParams; use crate::protocol::FsRemoveParams; @@ -60,33 +57,6 @@ impl ExecutorFileSystem for RemoteFileSystem { Ok(response.path) } - async fn join( - &self, - base_path: &AbsolutePathBuf, - path: &Path, - ) -> FileSystemResult { - trace!("remote fs join"); - let client = self.client.get().await.map_err(map_remote_error)?; - let response = client - .fs_join(FsJoinParams { - base_path: base_path.clone(), - path: path.to_path_buf(), - }) - .await - .map_err(map_remote_error)?; - Ok(response.path) - } - - async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { - trace!("remote fs parent"); - let client = self.client.get().await.map_err(map_remote_error)?; - let response = client - .fs_parent(FsParentParams { path: path.clone() }) - .await - .map_err(map_remote_error)?; - Ok(response.path) - } - async fn read_file( &self, path: &AbsolutePathBuf, diff --git a/codex-rs/exec-server/src/sandboxed_file_system.rs b/codex-rs/exec-server/src/sandboxed_file_system.rs index 00b6966c3b..a6851c3b69 100644 --- a/codex-rs/exec-server/src/sandboxed_file_system.rs +++ b/codex-rs/exec-server/src/sandboxed_file_system.rs @@ -3,7 +3,6 @@ use base64::Engine as _; use base64::engine::general_purpose::STANDARD; use codex_app_server_protocol::JSONRPCErrorError; use codex_utils_absolute_path::AbsolutePathBuf; -use std::path::Path; use tokio::io; use crate::CopyOptions; @@ -73,18 +72,6 @@ impl ExecutorFileSystem for SandboxedFileSystem { Ok(response.path) } - async fn join( - &self, - base_path: &AbsolutePathBuf, - path: &Path, - ) -> FileSystemResult { - Ok(base_path.join(path)) - } - - async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { - Ok(path.parent()) - } - async fn read_file( &self, path: &AbsolutePathBuf, diff --git a/codex-rs/exec-server/src/server/file_system_handler.rs b/codex-rs/exec-server/src/server/file_system_handler.rs index b60589d020..b42c8a379e 100644 --- a/codex-rs/exec-server/src/server/file_system_handler.rs +++ b/codex-rs/exec-server/src/server/file_system_handler.rs @@ -128,11 +128,8 @@ impl FileSystemHandler { &self, params: FsJoinParams, ) -> Result { - let path = self - .file_system - .join(¶ms.base_path, ¶ms.path) - .await - .map_err(map_fs_error)?; + // TODO(anp): remove and migrate callers to PathUri. + let path = params.base_path.join(params.path); Ok(FsJoinResponse { path }) } @@ -140,11 +137,8 @@ impl FileSystemHandler { &self, params: FsParentParams, ) -> Result { - let path = self - .file_system - .parent(¶ms.path) - .await - .map_err(map_fs_error)?; + // TODO(anp): remove and migrate callers to PathUri. + let path = params.path.parent(); Ok(FsParentResponse { path }) } @@ -276,4 +270,34 @@ mod tests { assert_eq!(response.data_base64, STANDARD.encode("ok")); } } + + #[tokio::test] + async fn protocol_join_and_parent_remain_native_path_operations() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let runtime_paths = ExecServerRuntimePaths::new( + std::env::current_exe().expect("current exe"), + /*codex_linux_sandbox_exe*/ None, + ) + .expect("runtime paths"); + let handler = FileSystemHandler::new(runtime_paths); + let base_path = + AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute tempdir"); + + let joined = handler + .join(FsJoinParams { + base_path: base_path.clone(), + path: "nested/file.txt".into(), + }) + .await + .expect("join path"); + assert_eq!(joined.path, base_path.join("nested/file.txt")); + + let parent = handler + .parent(FsParentParams { + path: joined.path.clone(), + }) + .await + .expect("parent path"); + assert_eq!(parent.path, joined.path.parent()); + } } diff --git a/codex-rs/exec-server/tests/file_system/shared.rs b/codex-rs/exec-server/tests/file_system/shared.rs index 34e3111d55..700bd25919 100644 --- a/codex-rs/exec-server/tests/file_system/shared.rs +++ b/codex-rs/exec-server/tests/file_system/shared.rs @@ -9,6 +9,7 @@ use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::PermissionProfile; use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy; use codex_sandboxing::policy_transforms::effective_network_sandbox_policy; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use std::path::Path; use tempfile::TempDir; @@ -130,40 +131,25 @@ async fn file_system_write_file_writes_bytes( Ok(()) } -#[test_case(FileSystemImplementation::Local ; "local")] -#[test_case(FileSystemImplementation::Remote ; "remote")] -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn file_system_join_and_parent_preserve_lexical_paths( - implementation: FileSystemImplementation, -) -> Result<()> { - let context = create_file_system_context(implementation).await?; - let file_system = context.file_system; - +#[test] +fn path_uri_join_and_parent_preserve_lexical_paths() -> Result<()> { let tmp = TempDir::new()?; let source_dir = tmp.path().join("source"); - let joined_nested = file_system - .join(&absolute_path(&source_dir), Path::new("nested/note.txt")) - .await - .with_context(|| format!("mode={implementation}"))?; + let source_dir_uri = PathUri::from_path(&source_dir)?; + let joined_nested = source_dir_uri.join("nested/note.txt")?; assert_eq!( joined_nested, - absolute_path(source_dir.join("nested").join("note.txt")) + PathUri::from_path(source_dir.join("nested").join("note.txt"))? ); - let joined_parent = file_system - .parent(&joined_nested) - .await - .with_context(|| format!("mode={implementation}"))?; + let joined_parent = joined_nested.parent(); assert_eq!( joined_parent, - Some(absolute_path(source_dir.join("nested"))) + Some(PathUri::from_path(source_dir.join("nested"))?) ); - let joined_parent_traversal = file_system - .join(&absolute_path(&source_dir), Path::new("../outside")) - .await - .with_context(|| format!("mode={implementation}"))?; + let joined_parent_traversal = source_dir_uri.join("../outside")?; assert_eq!( joined_parent_traversal, - absolute_path(source_dir.join("../outside")) + PathUri::from_path(source_dir.join("../outside"))? ); Ok(()) diff --git a/codex-rs/ext/skills/tests/executor_file_system_authority.rs b/codex-rs/ext/skills/tests/executor_file_system_authority.rs index a8bcf60273..19795065a4 100644 --- a/codex-rs/ext/skills/tests/executor_file_system_authority.rs +++ b/codex-rs/ext/skills/tests/executor_file_system_authority.rs @@ -1,5 +1,4 @@ use std::io; -use std::path::Path; use std::sync::Arc; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; @@ -70,18 +69,6 @@ impl ExecutorFileSystem for SyntheticFileSystem { Ok(path.clone()) } - async fn join( - &self, - base_path: &AbsolutePathBuf, - path: &Path, - ) -> FileSystemResult { - Ok(base_path.join(path)) - } - - async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult> { - Ok(path.parent()) - } - async fn read_file( &self, path: &AbsolutePathBuf, diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index 8fad1f5b62..3fd9cce9a8 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -140,16 +140,6 @@ pub trait ExecutorFileSystem: Send + Sync { sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult; - /// Lexically joins a path onto an existing bound path. - async fn join( - &self, - base_path: &AbsolutePathBuf, - path: &Path, - ) -> FileSystemResult; - - /// Returns the parent directory of a bound path. - async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult>; - async fn read_file( &self, path: &AbsolutePathBuf,