From b2a4e3be271e1119ae802a412f941d770ab0376a Mon Sep 17 00:00:00 2001 From: "Adam Perry @ OpenAI" Date: Thu, 11 Jun 2026 11:44:18 -0700 Subject: [PATCH] [codex] migrate ExecutorFileSystem paths to PathUri (#27424) ## Why We're moving exec-server to use PathUri for its internal path representations. ## What Move `ExecutorFileSystem` APIs to use `PathUri` instead of `AbsolutePathBuf`. Future changes will convert higher-level parts of exec-server. --- codex-rs/Cargo.lock | 11 + codex-rs/Cargo.toml | 1 + codex-rs/app-server/Cargo.toml | 1 + .../src/request_processors/fs_processor.rs | 26 +- codex-rs/apply-patch/Cargo.toml | 1 + codex-rs/apply-patch/src/invocation.rs | 14 +- codex-rs/apply-patch/src/lib.rs | 44 +++- codex-rs/config/Cargo.toml | 1 + codex-rs/config/src/loader/layer_io.rs | 4 +- codex-rs/config/src/loader/mod.rs | 29 ++- codex-rs/config/src/loader/tests.rs | 39 ++- codex-rs/core-skills/Cargo.toml | 1 + codex-rs/core-skills/src/injection.rs | 20 +- codex-rs/core-skills/src/loader.rs | 90 ++++++- codex-rs/core-skills/src/model.rs | 5 +- codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/agents_md.rs | 36 ++- codex-rs/core/src/config/agent_roles.rs | 10 +- codex-rs/core/src/config/mod.rs | 4 +- .../core/src/tools/handlers/view_image.rs | 11 +- codex-rs/core/tests/common/Cargo.toml | 1 + codex-rs/core/tests/common/test_codex.rs | 33 ++- codex-rs/core/tests/suite/agents_md.rs | 51 ++-- codex-rs/core/tests/suite/apply_patch_cli.rs | 31 ++- .../core/tests/suite/hierarchical_agents.rs | 4 +- codex-rs/core/tests/suite/remote_env.rs | 90 ++++--- codex-rs/core/tests/suite/rmcp_client.rs | 5 +- codex-rs/core/tests/suite/skills.rs | 7 +- codex-rs/core/tests/suite/unified_exec.rs | 4 +- codex-rs/core/tests/suite/view_image.rs | 18 +- codex-rs/exec-server/Cargo.toml | 1 + codex-rs/exec-server/src/environment.rs | 1 + codex-rs/exec-server/src/fs_helper.rs | 111 +++++++-- codex-rs/exec-server/src/local_file_system.rs | 116 ++++----- .../src/local_file_system_path_uri_tests.rs | 27 +++ .../exec-server/src/remote_file_system.rs | 86 +++---- .../src/remote_file_system_path_uri_tests.rs | 225 ++++++++++++++++++ .../exec-server/src/sandboxed_file_system.rs | 59 ++--- .../sandboxed_file_system_path_uri_tests.rs | 43 ++++ .../src/server/file_system_handler.rs | 92 +++++-- .../exec-server/tests/file_system/shared.rs | 81 +++---- .../exec-server/tests/file_system/support.rs | 4 +- .../exec-server/tests/file_system_unix.rs | 58 ++--- codex-rs/ext/skills/Cargo.toml | 1 + codex-rs/ext/skills/src/provider/executor.rs | 9 +- .../tests/executor_file_system_authority.rs | 52 ++-- codex-rs/file-system/Cargo.toml | 1 + codex-rs/file-system/src/lib.rs | 33 +-- codex-rs/git-utils/Cargo.toml | 1 + codex-rs/git-utils/src/info.rs | 19 +- codex-rs/utils/plugins/Cargo.toml | 1 + .../utils/plugins/src/plugin_namespace.rs | 7 +- 52 files changed, 1126 insertions(+), 495 deletions(-) create mode 100644 codex-rs/exec-server/src/local_file_system_path_uri_tests.rs create mode 100644 codex-rs/exec-server/src/remote_file_system_path_uri_tests.rs create mode 100644 codex-rs/exec-server/src/sandboxed_file_system_path_uri_tests.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 90a4335dc..d22127877 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2004,6 +2004,7 @@ dependencies = [ "codex-utils-cargo-bin", "codex-utils-cli", "codex-utils-json-to-toml", + "codex-utils-path-uri", "codex-utils-pty", "codex-web-search-extension", "codex-windows-sandbox", @@ -2183,6 +2184,7 @@ dependencies = [ "codex-exec-server", "codex-utils-absolute-path", "codex-utils-cargo-bin", + "codex-utils-path-uri", "pretty_assertions", "similar", "tempfile", @@ -2515,6 +2517,7 @@ dependencies = [ "codex-protocol", "codex-utils-absolute-path", "codex-utils-path", + "codex-utils-path-uri", "core-foundation 0.9.4", "dns-lookup", "dunce", @@ -2634,6 +2637,7 @@ dependencies = [ "codex-utils-image", "codex-utils-output-truncation", "codex-utils-path", + "codex-utils-path-uri", "codex-utils-plugins", "codex-utils-pty", "codex-utils-stream-parser", @@ -2768,6 +2772,7 @@ dependencies = [ "codex-skills", "codex-utils-absolute-path", "codex-utils-output-truncation", + "codex-utils-path-uri", "codex-utils-plugins", "dirs", "dunce", @@ -2848,6 +2853,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", @@ -3004,6 +3010,7 @@ dependencies = [ "async-trait", "codex-protocol", "codex-utils-absolute-path", + "codex-utils-path-uri", "serde", ] @@ -3027,6 +3034,7 @@ dependencies = [ "codex-file-system", "codex-protocol", "codex-utils-absolute-path", + "codex-utils-path-uri", "futures", "gix", "once_cell", @@ -3822,6 +3830,7 @@ dependencies = [ "codex-protocol", "codex-tools", "codex-utils-absolute-path", + "codex-utils-path-uri", "codex-utils-string", "pretty_assertions", "schemars 0.8.22", @@ -4203,6 +4212,7 @@ dependencies = [ "codex-exec-server", "codex-login", "codex-utils-absolute-path", + "codex-utils-path-uri", "serde", "serde_json", "tempfile", @@ -4592,6 +4602,7 @@ dependencies = [ "codex-protocol", "codex-utils-absolute-path", "codex-utils-cargo-bin", + "codex-utils-path-uri", "ctor 0.6.3", "futures", "notify", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index b87b0cf39..ad4ebf7a7 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/app-server/Cargo.toml b/codex-rs/app-server/Cargo.toml index 55ea78e61..442fb5df2 100644 --- a/codex-rs/app-server/Cargo.toml +++ b/codex-rs/app-server/Cargo.toml @@ -76,6 +76,7 @@ codex-thread-store = { workspace = true } codex-tools = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-json-to-toml = { workspace = true } +codex-utils-path-uri = { workspace = true } chrono = { workspace = true } clap = { workspace = true, features = ["derive"] } futures = { workspace = true } diff --git a/codex-rs/app-server/src/request_processors/fs_processor.rs b/codex-rs/app-server/src/request_processors/fs_processor.rs index 99a8620b4..c0d93bb15 100644 --- a/codex-rs/app-server/src/request_processors/fs_processor.rs +++ b/codex-rs/app-server/src/request_processors/fs_processor.rs @@ -29,6 +29,7 @@ use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::EnvironmentManager; use codex_exec_server::ExecutorFileSystem; use codex_exec_server::RemoveOptions; +use codex_utils_path_uri::PathUri; use std::io; use std::sync::Arc; @@ -64,9 +65,10 @@ impl FsRequestProcessor { &self, params: FsReadFileParams, ) -> Result { + let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let bytes = self .file_system()? - .read_file(¶ms.path, /*sandbox*/ None) + .read_file(&path, /*sandbox*/ None) .await .map_err(map_fs_error)?; Ok(FsReadFileResponse { @@ -83,8 +85,9 @@ impl FsRequestProcessor { "fs/writeFile requires valid base64 dataBase64: {err}" )) })?; + let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; self.file_system()? - .write_file(¶ms.path, bytes, /*sandbox*/ None) + .write_file(&path, bytes, /*sandbox*/ None) .await .map_err(map_fs_error)?; Ok(FsWriteFileResponse {}) @@ -94,9 +97,10 @@ impl FsRequestProcessor { &self, params: FsCreateDirectoryParams, ) -> Result { + let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; self.file_system()? .create_directory( - ¶ms.path, + &path, CreateDirectoryOptions { recursive: params.recursive.unwrap_or(true), }, @@ -111,9 +115,10 @@ impl FsRequestProcessor { &self, params: FsGetMetadataParams, ) -> Result { + let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let metadata = self .file_system()? - .get_metadata(¶ms.path, /*sandbox*/ None) + .get_metadata(&path, /*sandbox*/ None) .await .map_err(map_fs_error)?; Ok(FsGetMetadataResponse { @@ -129,9 +134,10 @@ impl FsRequestProcessor { &self, params: FsReadDirectoryParams, ) -> Result { + let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let entries = self .file_system()? - .read_directory(¶ms.path, /*sandbox*/ None) + .read_directory(&path, /*sandbox*/ None) .await .map_err(map_fs_error)?; Ok(FsReadDirectoryResponse { @@ -150,9 +156,10 @@ impl FsRequestProcessor { &self, params: FsRemoveParams, ) -> Result { + let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; self.file_system()? .remove( - ¶ms.path, + &path, RemoveOptions { recursive: params.recursive.unwrap_or(true), force: params.force.unwrap_or(true), @@ -168,10 +175,13 @@ impl FsRequestProcessor { &self, params: FsCopyParams, ) -> Result { + let source_path = PathUri::from_abs_path(¶ms.source_path).map_err(map_fs_error)?; + let destination_path = + PathUri::from_abs_path(¶ms.destination_path).map_err(map_fs_error)?; self.file_system()? .copy( - ¶ms.source_path, - ¶ms.destination_path, + &source_path, + &destination_path, CopyOptions { recursive: params.recursive, }, diff --git a/codex-rs/apply-patch/Cargo.toml b/codex-rs/apply-patch/Cargo.toml index 258433861..8e1d7f651 100644 --- a/codex-rs/apply-patch/Cargo.toml +++ b/codex-rs/apply-patch/Cargo.toml @@ -20,6 +20,7 @@ workspace = true anyhow = { workspace = true } codex-exec-server = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } similar = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true, features = ["macros", "rt"] } diff --git a/codex-rs/apply-patch/src/invocation.rs b/codex-rs/apply-patch/src/invocation.rs index 1aa429e0d..b66ee2291 100644 --- a/codex-rs/apply-patch/src/invocation.rs +++ b/codex-rs/apply-patch/src/invocation.rs @@ -21,6 +21,7 @@ use crate::parser::Hunk; use crate::parser::ParseError; use crate::parser::parse_patch; use crate::unified_diff_from_chunks; +use codex_utils_path_uri::PathUri; use std::str::Utf8Error; use tree_sitter::LanguageError; @@ -185,7 +186,18 @@ pub async fn verify_apply_patch_args( ); } Hunk::DeleteFile { .. } => { - let content = match fs.read_file_text(&path, sandbox).await { + let path_uri = match PathUri::from_abs_path(&path) { + Ok(path_uri) => path_uri, + Err(e) => { + return MaybeApplyPatchVerified::CorrectnessError( + ApplyPatchError::IoError(IoError { + context: format!("Failed to read {}", path.display()), + source: e, + }), + ); + } + }; + let content = match fs.read_file_text(&path_uri, sandbox).await { Ok(content) => content, Err(e) => { return MaybeApplyPatchVerified::CorrectnessError( diff --git a/codex-rs/apply-patch/src/lib.rs b/codex-rs/apply-patch/src/lib.rs index 29d42e1c0..0800ed04b 100644 --- a/codex-rs/apply-patch/src/lib.rs +++ b/codex-rs/apply-patch/src/lib.rs @@ -16,6 +16,7 @@ use codex_exec_server::ExecutorFileSystem; use codex_exec_server::FileSystemSandboxContext; use codex_exec_server::RemoveOptions; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; pub use parser::Hunk; pub use parser::ParseError; use parser::ParseError::*; @@ -390,6 +391,7 @@ async fn apply_hunks_to_files( for hunk in hunks { let affected_path = hunk.path().to_path_buf(); let path_abs = hunk.resolve_path(cwd); + let path_uri = PathUri::from_abs_path(&path_abs)?; match hunk { Hunk::AddFile { contents, .. } => { let overwritten_content = @@ -415,7 +417,7 @@ async fn apply_hunks_to_files( } Hunk::DeleteFile { .. } => { note_existing_path_delta_support(&path_abs, fs, sandbox, &mut delta.exact).await; - let deleted_content = fs.read_file_text(&path_abs, sandbox).await.ok(); + let deleted_content = fs.read_file_text(&path_uri, sandbox).await.ok(); if deleted_content.is_none() { delta.exact = false; } @@ -424,7 +426,7 @@ async fn apply_hunks_to_files( .with_context(|| format!("Failed to delete file {}", path_abs.display()))?; if let Err(error) = fs .remove( - &path_abs, + &path_uri, RemoveOptions { recursive: false, force: false, @@ -488,7 +490,7 @@ async fn apply_hunks_to_files( })?; if let Err(error) = fs .remove( - &path_abs, + &path_uri, RemoveOptions { recursive: false, force: false, @@ -521,7 +523,7 @@ async fn apply_hunks_to_files( modified.push(affected_path); } else { try_write!( - fs.write_file(&path_abs, new_contents.clone().into_bytes(), sandbox) + fs.write_file(&path_uri, new_contents.clone().into_bytes(), sandbox) .await .with_context(|| format!( "Failed to write file {}", @@ -554,7 +556,8 @@ async fn ensure_not_directory( fs: &dyn ExecutorFileSystem, sandbox: Option<&FileSystemSandboxContext>, ) -> io::Result<()> { - let metadata = fs.get_metadata(path, sandbox).await?; + let path_uri = PathUri::from_abs_path(path)?; + let metadata = fs.get_metadata(&path_uri, sandbox).await?; if metadata.is_directory { return Err(io::Error::new( io::ErrorKind::InvalidInput, @@ -570,9 +573,12 @@ async fn remove_failure_was_side_effect_free( fs: &dyn ExecutorFileSystem, sandbox: Option<&FileSystemSandboxContext>, ) -> bool { + let Ok(path_uri) = PathUri::from_abs_path(path) else { + return false; + }; match expected_content { Some(expected_content) => fs - .read_file_text(path, sandbox) + .read_file_text(&path_uri, sandbox) .await .is_ok_and(|content| content == expected_content), None => false, @@ -586,7 +592,14 @@ async fn read_optional_file_text_for_delta( exact: &mut bool, ) -> Option { note_existing_path_delta_support(path, fs, sandbox, exact).await; - match fs.read_file_text(path, sandbox).await { + let path_uri = match PathUri::from_abs_path(path) { + Ok(path_uri) => path_uri, + Err(_) => { + *exact = false; + return None; + } + }; + match fs.read_file_text(&path_uri, sandbox).await { Ok(content) => Some(content), Err(source) if source.kind() == io::ErrorKind::NotFound => None, Err(_) => { @@ -602,7 +615,11 @@ async fn note_existing_path_delta_support( sandbox: Option<&FileSystemSandboxContext>, exact: &mut bool, ) { - match fs.get_metadata(path, sandbox).await { + let Ok(path_uri) = PathUri::from_abs_path(path) else { + *exact = false; + return; + }; + match fs.get_metadata(&path_uri, sandbox).await { Ok(metadata) if metadata.is_file && !metadata.is_symlink => {} Ok(_) => *exact = false, Err(source) if source.kind() == io::ErrorKind::NotFound => {} @@ -616,12 +633,14 @@ async fn write_file_with_missing_parent_retry( contents: Vec, sandbox: Option<&FileSystemSandboxContext>, ) -> anyhow::Result<()> { - match fs.write_file(path_abs, contents.clone(), sandbox).await { + let path_uri = PathUri::from_abs_path(path_abs)?; + match fs.write_file(&path_uri, contents.clone(), sandbox).await { Ok(()) => Ok(()), Err(err) if err.kind() == io::ErrorKind::NotFound => { if let Some(parent_abs) = path_abs.parent() { + let parent_uri = PathUri::from_abs_path(&parent_abs)?; fs.create_directory( - &parent_abs, + &parent_uri, CreateDirectoryOptions { recursive: true }, sandbox, ) @@ -633,7 +652,7 @@ async fn write_file_with_missing_parent_retry( ) })?; } - fs.write_file(path_abs, contents, sandbox) + fs.write_file(&path_uri, contents, sandbox) .await .with_context(|| format!("Failed to write file {}", path_abs.display()))?; Ok(()) @@ -657,7 +676,8 @@ async fn derive_new_contents_from_chunks( fs: &dyn ExecutorFileSystem, sandbox: Option<&FileSystemSandboxContext>, ) -> std::result::Result { - let original_contents = fs.read_file_text(path_abs, sandbox).await.map_err(|err| { + let path_uri = PathUri::from_abs_path(path_abs)?; + let original_contents = fs.read_file_text(&path_uri, sandbox).await.map_err(|err| { ApplyPatchError::IoError(IoError { context: format!("Failed to read file to update {}", path_abs.display()), source: err, diff --git a/codex-rs/config/Cargo.toml b/codex-rs/config/Cargo.toml index f77a76b76..e2c650dcc 100644 --- a/codex-rs/config/Cargo.toml +++ b/codex-rs/config/Cargo.toml @@ -25,6 +25,7 @@ codex-network-proxy = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-path = { workspace = true } +codex-utils-path-uri = { workspace = true } dunce = { workspace = true } futures = { workspace = true, features = ["alloc", "std"] } gethostname = { workspace = true } diff --git a/codex-rs/config/src/loader/layer_io.rs b/codex-rs/config/src/loader/layer_io.rs index 415d82e40..7cdb4f535 100644 --- a/codex-rs/config/src/loader/layer_io.rs +++ b/codex-rs/config/src/loader/layer_io.rs @@ -10,6 +10,7 @@ use crate::strict_config::config_error_from_ignored_toml_value_fields; use codex_file_system::ExecutorFileSystem; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; +use codex_utils_path_uri::PathUri; use std::io; use std::path::Path; use std::path::PathBuf; @@ -106,7 +107,8 @@ pub(super) async fn read_config_from_path( log_missing_as_info: bool, strict_config: bool, ) -> io::Result> { - match fs.read_file_text(path, /*sandbox*/ None).await { + let path_uri = PathUri::from_abs_path(path)?; + match fs.read_file_text(&path_uri, /*sandbox*/ None).await { Ok(contents) => match toml::from_str::(&contents) { Ok(value) => { if strict_config { diff --git a/codex-rs/config/src/loader/mod.rs b/codex-rs/config/src/loader/mod.rs index 1a79e984e..d47ad50b2 100644 --- a/codex-rs/config/src/loader/mod.rs +++ b/codex-rs/config/src/loader/mod.rs @@ -40,6 +40,7 @@ use codex_protocol::config_types::TrustLevel; use codex_protocol::protocol::AskForApproval; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; +use codex_utils_path_uri::PathUri; use dunce::canonicalize as normalize_path; use serde::Deserialize; use std::io; @@ -471,7 +472,8 @@ async fn load_config_toml_for_required_layer( strict_config: bool, create_entry: impl FnOnce(TomlValue) -> ConfigLayerEntry, ) -> io::Result { - let toml_value = match fs.read_file_text(toml_file, /*sandbox*/ None).await { + let toml_file_uri = PathUri::from_abs_path(toml_file)?; + let toml_value = match fs.read_file_text(&toml_file_uri, /*sandbox*/ None).await { Ok(contents) => { let config_parent = toml_file.as_path().parent().ok_or_else(|| { io::Error::new( @@ -566,8 +568,9 @@ pub async fn load_requirements_toml( fs: &dyn ExecutorFileSystem, requirements_toml_file: &AbsolutePathBuf, ) -> io::Result> { + let requirements_toml_file_uri = PathUri::from_abs_path(requirements_toml_file)?; match fs - .read_file_text(requirements_toml_file, /*sandbox*/ None) + .read_file_text(&requirements_toml_file_uri, /*sandbox*/ None) .await { Ok(contents) => { @@ -1135,8 +1138,9 @@ async fn find_project_root( for ancestor in cwd.ancestors() { for marker in project_root_markers { let marker_path = ancestor.join(marker); + let marker_path_uri = PathUri::from_abs_path(&marker_path)?; if fs - .get_metadata(&marker_path, /*sandbox*/ None) + .get_metadata(&marker_path_uri, /*sandbox*/ None) .await .is_ok() { @@ -1151,14 +1155,20 @@ async fn find_git_checkout_root( fs: &dyn ExecutorFileSystem, cwd: &AbsolutePathBuf, ) -> Option { - let base = match fs.get_metadata(cwd, /*sandbox*/ None).await { + let cwd_uri = PathUri::from_abs_path(cwd).ok()?; + let base = match fs.get_metadata(&cwd_uri, /*sandbox*/ None).await { Ok(metadata) if metadata.is_directory => cwd.clone(), _ => cwd.parent()?, }; for dir in base.ancestors() { let dot_git = dir.join(".git"); - if fs.get_metadata(&dot_git, /*sandbox*/ None).await.is_ok() { + let dot_git_uri = PathUri::from_abs_path(&dot_git).ok()?; + if fs + .get_metadata(&dot_git_uri, /*sandbox*/ None) + .await + .is_ok() + { return Some(dir); } } @@ -1206,8 +1216,9 @@ async fn load_project_layers( let mut startup_warnings = Vec::new(); for dir in dirs { let dot_codex_abs = dir.join(".codex"); + let dot_codex_uri = PathUri::from_abs_path(&dot_codex_abs)?; if !fs - .get_metadata(&dot_codex_abs, /*sandbox*/ None) + .get_metadata(&dot_codex_uri, /*sandbox*/ None) .await .map(|metadata| metadata.is_directory) .unwrap_or(false) @@ -1224,7 +1235,8 @@ async fn load_project_layers( continue; } let config_file = dot_codex_abs.join(CONFIG_TOML_FILE); - match fs.read_file_text(&config_file, /*sandbox*/ None).await { + let config_file_uri = PathUri::from_abs_path(&config_file)?; + match fs.read_file_text(&config_file_uri, /*sandbox*/ None).await { Ok(contents) => { let config: TomlValue = match toml::from_str(&contents) { Ok(config) => config, @@ -1327,8 +1339,9 @@ async fn merge_root_checkout_project_hooks( return Ok(config); }; let hooks_config_file = hooks_config_folder.join(CONFIG_TOML_FILE); + let hooks_config_file_uri = PathUri::from_abs_path(&hooks_config_file)?; let root_config = match fs - .read_file_text(&hooks_config_file, /*sandbox*/ None) + .read_file_text(&hooks_config_file_uri, /*sandbox*/ None) .await { Ok(contents) => { diff --git a/codex-rs/config/src/loader/tests.rs b/codex-rs/config/src/loader/tests.rs index 6492f9a0c..e349ba0c6 100644 --- a/codex-rs/config/src/loader/tests.rs +++ b/codex-rs/config/src/loader/tests.rs @@ -7,8 +7,8 @@ use codex_file_system::FileSystemResult; use codex_file_system::FileSystemSandboxContext; use codex_file_system::ReadDirectoryEntry; use codex_file_system::RemoveOptions; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; -use std::path::Path; use tempfile::tempdir; struct TestFileSystem; @@ -17,35 +17,26 @@ struct TestFileSystem; impl ExecutorFileSystem for TestFileSystem { async fn canonicalize( &self, - path: &AbsolutePathBuf, + path: &PathUri, _sandbox: Option<&FileSystemSandboxContext>, - ) -> FileSystemResult { - 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()) + ) -> FileSystemResult { + let path = path.to_abs_path()?; + let canonicalized = path.canonicalize()?; + PathUri::from_abs_path(&canonicalized) } async fn read_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { + let path = path.to_abs_path()?; tokio::fs::read(path.as_path()).await } async fn write_file( &self, - _path: &AbsolutePathBuf, + _path: &PathUri, _contents: Vec, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -54,7 +45,7 @@ impl ExecutorFileSystem for TestFileSystem { async fn create_directory( &self, - _path: &AbsolutePathBuf, + _path: &PathUri, _create_directory_options: CreateDirectoryOptions, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -63,7 +54,7 @@ impl ExecutorFileSystem for TestFileSystem { async fn get_metadata( &self, - _path: &AbsolutePathBuf, + _path: &PathUri, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult { unimplemented!("test filesystem only supports reads") @@ -71,7 +62,7 @@ impl ExecutorFileSystem for TestFileSystem { async fn read_directory( &self, - _path: &AbsolutePathBuf, + _path: &PathUri, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { unimplemented!("test filesystem only supports reads") @@ -79,7 +70,7 @@ impl ExecutorFileSystem for TestFileSystem { async fn remove( &self, - _path: &AbsolutePathBuf, + _path: &PathUri, _remove_options: RemoveOptions, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -88,8 +79,8 @@ impl ExecutorFileSystem for TestFileSystem { async fn copy( &self, - _source_path: &AbsolutePathBuf, - _destination_path: &AbsolutePathBuf, + _source_path: &PathUri, + _destination_path: &PathUri, _copy_options: CopyOptions, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { diff --git a/codex-rs/core-skills/Cargo.toml b/codex-rs/core-skills/Cargo.toml index 3c18bee60..9473fa8e4 100644 --- a/codex-rs/core-skills/Cargo.toml +++ b/codex-rs/core-skills/Cargo.toml @@ -26,6 +26,7 @@ codex-protocol = { workspace = true } codex-skills = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-output-truncation = { workspace = true } +codex-utils-path-uri = { workspace = true } codex-utils-plugins = { workspace = true } dirs = { workspace = true } dunce = { workspace = true } diff --git a/codex-rs/core-skills/src/injection.rs b/codex-rs/core-skills/src/injection.rs index a358ddc63..03bb0a6dd 100644 --- a/codex-rs/core-skills/src/injection.rs +++ b/codex-rs/core-skills/src/injection.rs @@ -13,6 +13,7 @@ use codex_exec_server::LOCAL_FS; use codex_otel::SessionTelemetry; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use codex_utils_plugins::mention_syntax::TOOL_MENTION_SIGIL; #[derive(Debug, Default)] @@ -75,10 +76,21 @@ pub async fn build_skill_injections( let fs = loaded_skills .and_then(|outcome| outcome.file_system_for_skill(skill)) .unwrap_or_else(|| Arc::clone(&LOCAL_FS)); - match fs - .read_file_text(&skill.path_to_skills_md, /*sandbox*/ None) - .await - { + // Skill metadata may point at a file that is absent, but a path the host + // cannot represent means the configured skill cannot be loaded correctly. + let path = match PathUri::from_abs_path(&skill.path_to_skills_md) { + Ok(path) => path, + Err(err) => { + emit_skill_injected_metric(otel, skill, "error"); + result.warnings.push(format!( + "Failed to load skill {name} at {path}: {err:#}", + name = skill.name, + path = skill.path_to_skills_md.display() + )); + continue; + } + }; + match fs.read_file_text(&path, /*sandbox*/ None).await { Ok(contents) => { emit_skill_injected_metric(otel, skill, "ok"); invocations.push(SkillInvocation { diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index e2c3896cb..813f13018 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -19,6 +19,7 @@ use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; +use codex_utils_path_uri::PathUri; use codex_utils_plugins::PluginSkillRoot; use codex_utils_plugins::plugin_namespace_for_skill_path; use dirs::home_dir; @@ -374,7 +375,17 @@ async fn repo_agents_skill_roots( let mut roots = Vec::new(); for dir in dirs { let agents_skills = dir.join(AGENTS_DIR_NAME).join(SKILLS_DIR_NAME); - match fs.get_metadata(&agents_skills, /*sandbox*/ None).await { + let agents_skills_uri = match PathUri::from_abs_path(&agents_skills) { + Ok(path) => path, + Err(err) => { + tracing::warn!( + "failed to convert repo skills root {} to URI: {err:#}", + agents_skills.display() + ); + continue; + } + }; + match fs.get_metadata(&agents_skills_uri, /*sandbox*/ None).await { Ok(metadata) if metadata.is_directory => roots.push(SkillRoot { path: agents_skills, scope: SkillScope::Repo, @@ -429,7 +440,17 @@ async fn find_project_root( for ancestor in cwd.ancestors() { for marker in project_root_markers { let marker_path = ancestor.join(marker); - match fs.get_metadata(&marker_path, /*sandbox*/ None).await { + let marker_path_uri = match PathUri::from_abs_path(&marker_path) { + Ok(path) => path, + Err(err) => { + tracing::warn!( + "failed to convert project root marker {} to URI: {err:#}", + marker_path.display() + ); + continue; + } + }; + match fs.get_metadata(&marker_path_uri, /*sandbox*/ None).await { Ok(_) => return ancestor, Err(err) if err.kind() == io::ErrorKind::NotFound => {} Err(err) => { @@ -475,8 +496,12 @@ async fn canonicalize_for_skill_identity( fs: &dyn ExecutorFileSystem, path: &AbsolutePathBuf, ) -> AbsolutePathBuf { - fs.canonicalize(path, /*sandbox*/ None) + let Ok(path_uri) = PathUri::from_abs_path(path) else { + return path.clone(); + }; + fs.canonicalize(&path_uri, /*sandbox*/ None) .await + .and_then(|path| path.to_abs_path()) .unwrap_or_else(|_| path.clone()) } @@ -494,7 +519,17 @@ async fn discover_skills_under_root( None => None, }; - match fs.get_metadata(&root, /*sandbox*/ None).await { + let root_uri = match PathUri::from_abs_path(&root) { + Ok(path) => path, + Err(err) => { + tracing::warn!( + "failed to convert skills root {} to URI: {err:#}", + root.display() + ); + return; + } + }; + match fs.get_metadata(&root_uri, /*sandbox*/ None).await { Ok(metadata) if metadata.is_directory => {} Ok(_) => return, Err(err) if err.kind() == io::ErrorKind::NotFound => return, @@ -536,7 +571,17 @@ async fn discover_skills_under_root( let mut truncated_by_dir_limit = false; while let Some((dir, depth)) = queue.pop_front() { - let entries = match fs.read_directory(&dir, /*sandbox*/ None).await { + let dir_uri = match PathUri::from_abs_path(&dir) { + Ok(path) => path, + Err(e) => { + tracing::warn!( + "failed to convert skills dir {} to URI: {e:#}", + dir.display() + ); + continue; + } + }; + let entries = match fs.read_directory(&dir_uri, /*sandbox*/ None).await { Ok(entries) => entries, Err(e) => { error!("failed to read skills dir {}: {e:#}", dir.display()); @@ -551,7 +596,17 @@ async fn discover_skills_under_root( } let path = dir.join(&file_name); - let metadata = match fs.get_metadata(&path, /*sandbox*/ None).await { + let path_uri = match PathUri::from_abs_path(&path) { + Ok(path) => path, + Err(e) => { + tracing::warn!( + "failed to convert skills path {} to URI: {e:#}", + path.display() + ); + continue; + } + }; + let metadata = match fs.get_metadata(&path_uri, /*sandbox*/ None).await { Ok(metadata) => metadata, Err(e) => { error!("failed to stat skills path {}: {e:#}", path.display()); @@ -563,7 +618,7 @@ async fn discover_skills_under_root( if !follow_symlinks { continue; } - match fs.read_directory(&path, /*sandbox*/ None).await { + match fs.read_directory(&path_uri, /*sandbox*/ None).await { Ok(_) => { let resolved_dir = canonicalize_for_skill_identity(fs, &path).await; enqueue_dir( @@ -635,8 +690,9 @@ async fn parse_skill_file( plugin_id: Option<&str>, plugin_root: Option<&AbsolutePathBuf>, ) -> Result { + let path_uri = PathUri::from_abs_path(path).map_err(SkillParseError::Read)?; let contents = fs - .read_file_text(path, /*sandbox*/ None) + .read_file_text(&path_uri, /*sandbox*/ None) .await .map_err(SkillParseError::Read)?; @@ -730,7 +786,18 @@ async fn load_skill_metadata( let metadata_path = skill_dir .join(SKILLS_METADATA_DIR) .join(SKILLS_METADATA_FILENAME); - match fs.get_metadata(&metadata_path, /*sandbox*/ None).await { + let metadata_path_uri = match PathUri::from_abs_path(&metadata_path) { + Ok(path) => path, + Err(error) => { + tracing::warn!( + "ignoring {path}: failed to convert {label} path to URI: {error}", + path = metadata_path.display(), + label = SKILLS_METADATA_FILENAME + ); + return LoadedSkillMetadata::default(); + } + }; + match fs.get_metadata(&metadata_path_uri, /*sandbox*/ None).await { Ok(metadata) if metadata.is_file => {} Ok(_) => return LoadedSkillMetadata::default(), Err(error) if error.kind() == io::ErrorKind::NotFound => { @@ -746,7 +813,10 @@ async fn load_skill_metadata( } } - let contents = match fs.read_file_text(&metadata_path, /*sandbox*/ None).await { + let contents = match fs + .read_file_text(&metadata_path_uri, /*sandbox*/ None) + .await + { Ok(contents) => contents, Err(error) => { tracing::warn!( diff --git a/codex-rs/core-skills/src/model.rs b/codex-rs/core-skills/src/model.rs index a9cc24306..5569655ad 100644 --- a/codex-rs/core-skills/src/model.rs +++ b/codex-rs/core-skills/src/model.rs @@ -9,6 +9,7 @@ use codex_exec_server::LOCAL_FS; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; #[derive(Debug, Clone, PartialEq)] pub struct SkillMetadata { @@ -152,8 +153,8 @@ impl HostLoadedSkills { .outcome .file_system_for_skill(skill) .unwrap_or_else(|| Arc::clone(&LOCAL_FS)); - fs.read_file_text(&skill.path_to_skills_md, /*sandbox*/ None) - .await + let path = PathUri::from_abs_path(&skill.path_to_skills_md)?; + fs.read_file_text(&path, /*sandbox*/ None).await } } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 352e8051e..60678db5d 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -70,6 +70,7 @@ codex-utils-image = { workspace = true } codex-utils-home-dir = { workspace = true } codex-utils-output-truncation = { workspace = true } codex-utils-path = { workspace = true } +codex-utils-path-uri = { workspace = true } codex-utils-plugins = { workspace = true } codex-utils-pty = { workspace = true } codex-utils-string = { workspace = true } diff --git a/codex-rs/core/src/agents_md.rs b/codex-rs/core/src/agents_md.rs index 5739e42f3..e60eacc55 100644 --- a/codex-rs/core/src/agents_md.rs +++ b/codex-rs/core/src/agents_md.rs @@ -26,6 +26,7 @@ use codex_exec_server::ExecutorFileSystem; use codex_features::Feature; use codex_prompts::HIERARCHICAL_AGENTS_MESSAGE; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use std::io; use toml::Value as TomlValue; use tracing::error; @@ -58,7 +59,19 @@ impl<'a> AgentsMdManager<'a> { let base = codex_dir?; for candidate in [LOCAL_AGENTS_MD_FILENAME, DEFAULT_AGENTS_MD_FILENAME] { let path = base.join(candidate); - let data = match fs.read_file(&path, /*sandbox*/ None).await { + // A missing global instructions file is normal, but an unrepresentable + // configured path means Codex cannot honor the workspace configuration. + let path_uri = match PathUri::from_abs_path(&path) { + Ok(path_uri) => path_uri, + Err(err) => { + startup_warnings.push(format!( + "Failed to read global AGENTS.md instructions from `{}`: {err}", + path.display() + )); + continue; + } + }; + let data = match fs.read_file(&path_uri, /*sandbox*/ None).await { Ok(data) => data, Err(err) if err.kind() == io::ErrorKind::NotFound => continue, Err(err) if err.kind() == io::ErrorKind::IsADirectory => continue, @@ -149,14 +162,15 @@ impl<'a> AgentsMdManager<'a> { break; } - match fs.get_metadata(&p, /*sandbox*/ None).await { + let path_uri = PathUri::from_abs_path(&p)?; + match fs.get_metadata(&path_uri, /*sandbox*/ None).await { Ok(metadata) if !metadata.is_file => continue, Ok(_) => {} Err(err) if err.kind() == io::ErrorKind::NotFound => continue, Err(err) => return Err(err), } - let mut data = match fs.read_file(&p, /*sandbox*/ None).await { + let mut data = match fs.read_file(&path_uri, /*sandbox*/ None).await { Ok(data) => data, Err(err) if err.kind() == io::ErrorKind::NotFound => continue, Err(err) => return Err(err), @@ -231,12 +245,13 @@ impl<'a> AgentsMdManager<'a> { for ancestor in dir.ancestors() { for marker in &project_root_markers { let marker_path = ancestor.join(marker); - let marker_exists = match fs.get_metadata(&marker_path, /*sandbox*/ None).await - { - Ok(_) => true, - Err(err) if err.kind() == io::ErrorKind::NotFound => false, - Err(err) => return Err(err), - }; + let marker_path_uri = PathUri::from_abs_path(&marker_path)?; + let marker_exists = + match fs.get_metadata(&marker_path_uri, /*sandbox*/ None).await { + Ok(_) => true, + Err(err) if err.kind() == io::ErrorKind::NotFound => false, + Err(err) => return Err(err), + }; if marker_exists { project_root = Some(ancestor.clone()); break; @@ -272,7 +287,8 @@ impl<'a> AgentsMdManager<'a> { for d in search_dirs { for name in &candidate_filenames { let candidate = d.join(name); - match fs.get_metadata(&candidate, /*sandbox*/ None).await { + let candidate_uri = PathUri::from_abs_path(&candidate)?; + match fs.get_metadata(&candidate_uri, /*sandbox*/ None).await { Ok(md) if md.is_file => { found.push(candidate); break; diff --git a/codex-rs/core/src/config/agent_roles.rs b/codex-rs/core/src/config/agent_roles.rs index abdef33e7..dfcec2eac 100644 --- a/codex-rs/core/src/config/agent_roles.rs +++ b/codex-rs/core/src/config/agent_roles.rs @@ -7,6 +7,7 @@ use codex_config::config_toml::ConfigToml; use codex_exec_server::ExecutorFileSystem; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; +use codex_utils_path_uri::PathUri; use serde::Deserialize; use std::collections::BTreeMap; use std::collections::BTreeSet; @@ -319,7 +320,8 @@ async fn read_resolved_agent_role_file( path: &AbsolutePathBuf, role_name_hint: Option<&str>, ) -> std::io::Result { - let contents = fs.read_file_text(path, /*sandbox*/ None).await?; + let path_uri = PathUri::from_abs_path(path)?; + let contents = fs.read_file_text(&path_uri, /*sandbox*/ None).await?; let config_base_dir = path.parent().unwrap_or_else(|| path.clone()); parse_agent_role_file_contents( &contents, @@ -391,8 +393,9 @@ async fn validate_agent_role_config_file( return Ok(()); }; + let config_file_uri = PathUri::from_abs_path(config_file)?; let metadata = fs - .get_metadata(config_file, /*sandbox*/ None) + .get_metadata(&config_file_uri, /*sandbox*/ None) .await .map_err(|e| { std::io::Error::new( @@ -522,7 +525,8 @@ async fn collect_agent_role_files( let mut files = Vec::new(); let mut dirs = vec![dir.clone()]; while let Some(dir) = dirs.pop() { - let entries = match fs.read_directory(&dir, /*sandbox*/ None).await { + let dir_uri = PathUri::from_abs_path(&dir)?; + let entries = match fs.read_directory(&dir_uri, /*sandbox*/ None).await { Ok(entries) => entries, Err(err) if err.kind() == ErrorKind::NotFound => continue, Err(err) => return Err(err), diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index 597033e4e..40895c39f 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -105,6 +105,7 @@ use codex_protocol::protocol::SandboxPolicy; pub use codex_thread_store::ExtraConfig; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; +use codex_utils_path_uri::PathUri; use rmcp::model::ElicitationCapability; use rmcp::model::FormElicitationCapability; use rmcp::model::UrlElicitationCapability; @@ -3667,8 +3668,9 @@ impl Config { return Ok(None); }; + let path_uri = PathUri::from_abs_path(path)?; let contents = fs - .read_file_text(path, /*sandbox*/ None) + .read_file_text(&path_uri, /*sandbox*/ None) .await .map_err(|e| { std::io::Error::new( diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index c3023b421..17708564b 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -27,6 +27,7 @@ use crate::tools::registry::CoreToolRuntime; use crate::tools::registry::ToolExecutor; use codex_tools::ToolName; use codex_tools::ToolSpec; +use codex_utils_path_uri::PathUri; pub struct ViewImageHandler { options: ViewImageToolOptions, @@ -146,9 +147,15 @@ impl ViewImageHandler { let abs_path = cwd.join(path); let sandbox = turn.file_system_sandbox_context(/*additional_permissions*/ None, &cwd); let fs = turn_environment.environment.get_filesystem(); + let path_uri = PathUri::from_abs_path(&abs_path).map_err(|error| { + FunctionCallError::RespondToModel(format!( + "unable to locate image at `{}`: {error}", + abs_path.display() + )) + })?; let metadata = fs - .get_metadata(&abs_path, Some(&sandbox)) + .get_metadata(&path_uri, Some(&sandbox)) .await .map_err(|error| { FunctionCallError::RespondToModel(format!( @@ -164,7 +171,7 @@ impl ViewImageHandler { ))); } let file_bytes = fs - .read_file(&abs_path, Some(&sandbox)) + .read_file(&path_uri, Some(&sandbox)) .await .map_err(|error| { FunctionCallError::RespondToModel(format!( diff --git a/codex-rs/core/tests/common/Cargo.toml b/codex-rs/core/tests/common/Cargo.toml index fb6221cfc..575985d2c 100644 --- a/codex-rs/core/tests/common/Cargo.toml +++ b/codex-rs/core/tests/common/Cargo.toml @@ -28,6 +28,7 @@ codex-models-manager = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-cargo-bin = { workspace = true } +codex-utils-path-uri = { workspace = true } ctor = { workspace = true } futures = { workspace = true } notify = { workspace = true } diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 62f422ba1..07f1a8a78 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -43,6 +43,7 @@ use codex_protocol::protocol::TurnEnvironmentSelection; use codex_protocol::protocol::TurnEnvironmentSelections; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use futures::future::BoxFuture; use serde_json::Value; use tempfile::TempDir; @@ -135,10 +136,11 @@ pub async fn test_env() -> Result { let environment = codex_exec_server::Environment::create_for_tests(Some(websocket_url.clone()))?; let cwd = remote_aware_cwd_path(); + let cwd_uri = PathUri::from_path(&cwd)?; environment .get_filesystem() .create_directory( - &cwd, + &cwd_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) @@ -907,35 +909,45 @@ impl TestCodexHarness { ) -> Result<()> { let abs_path = self.path_abs(rel); if let Some(parent) = abs_path.parent() { + let parent_uri = PathUri::from_path(&parent)?; self.test .fs() .create_directory( - &parent, + &parent_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) .await?; } + let abs_path_uri = PathUri::from_path(&abs_path)?; self.test .fs() - .write_file(&abs_path, contents.as_ref().to_vec(), /*sandbox*/ None) + .write_file( + &abs_path_uri, + contents.as_ref().to_vec(), + /*sandbox*/ None, + ) .await?; Ok(()) } pub async fn read_file_text(&self, rel: impl AsRef) -> Result { + let path = self.path_abs(rel); + let path_uri = PathUri::from_path(&path)?; Ok(self .test .fs() - .read_file_text(&self.path_abs(rel), /*sandbox*/ None) + .read_file_text(&path_uri, /*sandbox*/ None) .await?) } pub async fn create_dir_all(&self, rel: impl AsRef) -> Result<()> { + let path = self.path_abs(rel); + let path_uri = PathUri::from_path(&path)?; self.test .fs() .create_directory( - &self.path_abs(rel), + &path_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) @@ -948,10 +960,11 @@ impl TestCodexHarness { } pub async fn remove_abs_path(&self, path: &AbsolutePathBuf) -> Result<()> { + let path_uri = PathUri::from_abs_path(path)?; self.test .fs() .remove( - path, + &path_uri, RemoveOptions { recursive: false, force: true, @@ -963,7 +976,13 @@ impl TestCodexHarness { } pub async fn abs_path_exists(&self, path: &AbsolutePathBuf) -> Result { - match self.test.fs().get_metadata(path, /*sandbox*/ None).await { + let path_uri = PathUri::from_abs_path(path)?; + match self + .test + .fs() + .get_metadata(&path_uri, /*sandbox*/ None) + .await + { Ok(_) => Ok(true), Err(err) if err.kind() == ErrorKind::NotFound => Ok(false), Err(err) => Err(err.into()), diff --git a/codex-rs/core/tests/suite/agents_md.rs b/codex-rs/core/tests/suite/agents_md.rs index 88f014f75..810f73398 100644 --- a/codex-rs/core/tests/suite/agents_md.rs +++ b/codex-rs/core/tests/suite/agents_md.rs @@ -7,6 +7,7 @@ use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use core_test_support::PathBufExt; use core_test_support::create_directory_symlink; use core_test_support::load_default_config_for_test; @@ -112,10 +113,12 @@ async fn agents_override_is_preferred_over_agents_md() -> Result<()> { agents_instructions(test_codex().with_workspace_setup(|cwd, fs| async move { let agents_md = cwd.join("AGENTS.md"); let override_md = cwd.join("AGENTS.override.md"); - fs.write_file(&agents_md, b"base doc".to_vec(), /*sandbox*/ None) + let agents_md_uri = PathUri::from_path(&agents_md)?; + let override_md_uri = PathUri::from_path(&override_md)?; + fs.write_file(&agents_md_uri, b"base doc".to_vec(), /*sandbox*/ None) .await?; fs.write_file( - &override_md, + &override_md_uri, b"override doc".to_vec(), /*sandbox*/ None, ) @@ -146,14 +149,20 @@ async fn configured_fallback_is_used_when_agents_candidate_is_directory() -> Res .with_workspace_setup(|cwd, fs| async move { let agents_dir = cwd.join("AGENTS.md"); let fallback = cwd.join("WORKFLOW.md"); + let agents_dir_uri = PathUri::from_path(&agents_dir)?; + let fallback_uri = PathUri::from_path(&fallback)?; fs.create_directory( - &agents_dir, + &agents_dir_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) .await?; - fs.write_file(&fallback, b"fallback doc".to_vec(), /*sandbox*/ None) - .await?; + fs.write_file( + &fallback_uri, + b"fallback doc".to_vec(), + /*sandbox*/ None, + ) + .await?; Ok::<(), anyhow::Error>(()) }), ) @@ -183,23 +192,35 @@ async fn agents_docs_are_concatenated_from_project_root_to_cwd() -> Result<()> { let root_agents = root.join("AGENTS.md"); let git_marker = root.join(".git"); let nested_agents = nested.join("AGENTS.md"); + let nested_uri = PathUri::from_path(&nested)?; + let root_agents_uri = PathUri::from_path(&root_agents)?; + let git_marker_uri = PathUri::from_path(&git_marker)?; + let nested_agents_uri = PathUri::from_path(&nested_agents)?; fs.create_directory( - &nested, + &nested_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) .await?; - fs.write_file(&root_agents, b"root doc".to_vec(), /*sandbox*/ None) - .await?; fs.write_file( - &git_marker, + &root_agents_uri, + b"root doc".to_vec(), + /*sandbox*/ None, + ) + .await?; + fs.write_file( + &git_marker_uri, b"gitdir: /tmp/mock-git-dir\n".to_vec(), /*sandbox*/ None, ) .await?; - fs.write_file(&nested_agents, b"child doc".to_vec(), /*sandbox*/ None) - .await?; + fs.write_file( + &nested_agents_uri, + b"child doc".to_vec(), + /*sandbox*/ None, + ) + .await?; Ok::<(), anyhow::Error>(()) }), ) @@ -314,8 +335,9 @@ async fn selected_environment_sources_match_model_visible_instructions() -> Resu let mut builder = test_codex() .with_home(home) .with_workspace_setup(|cwd, fs| async move { + let agents_md_uri = PathUri::from_path(cwd.join("AGENTS.md"))?; fs.write_file( - &cwd.join("AGENTS.md"), + &agents_md_uri, b"project doc".to_vec(), /*sandbox*/ None, ) @@ -367,8 +389,9 @@ async fn fresh_thread_composes_global_before_project_and_reports_sources() -> Re let mut builder = test_codex() .with_home(Arc::clone(&home)) .with_workspace_setup(|cwd, fs| async move { + let agents_md_uri = PathUri::from_path(cwd.join("AGENTS.md"))?; fs.write_file( - &cwd.join("AGENTS.md"), + &agents_md_uri, PROJECT_INSTRUCTIONS.as_bytes().to_vec(), /*sandbox*/ None, ) @@ -392,7 +415,7 @@ async fn fresh_thread_composes_global_before_project_and_reports_sources() -> Re )?; test.fs() .write_file( - &project_source, + &PathUri::from_path(&project_source)?, NEW_PROJECT_INSTRUCTIONS.as_bytes().to_vec(), /*sandbox*/ None, ) diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index a9ebadceb..f320063a6 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -38,6 +38,7 @@ use codex_protocol::user_input::UserInput; #[cfg(target_os = "linux")] use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use core_test_support::PathBufExt; use core_test_support::assert_regex_match; use core_test_support::get_remote_test_env; @@ -1329,25 +1330,24 @@ async fn apply_patch_turn_diff_paths_stay_repo_relative_when_session_cwd_is_nest config.cwd = config.cwd.join("subdir"); }) .with_workspace_setup(|cwd, fs| async move { + let cwd_uri = PathUri::from_path(&cwd)?; fs.create_directory( - &cwd, + &cwd_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) .await?; let repo_root = cwd.parent().expect("nested cwd should have parent"); + let git_uri = PathUri::from_path(repo_root.join(".git"))?; + let repo_file_uri = PathUri::from_path(repo_root.join("repo.txt"))?; fs.write_file( - &repo_root.join(".git"), + &git_uri, b"gitdir: /tmp/fake-worktree\n".to_vec(), /*sandbox*/ None, ) .await?; - fs.write_file( - &repo_root.join("repo.txt"), - b"before\n".to_vec(), - /*sandbox*/ None, - ) - .await?; + fs.write_file(&repo_file_uri, b"before\n".to_vec(), /*sandbox*/ None) + .await?; Ok(()) }) }) @@ -1582,10 +1582,11 @@ async fn apply_patch_turn_diff_tracks_local_and_remote_environment_paths() -> Re SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis() )) .abs(); + let shared_cwd_uri = PathUri::from_path(&shared_cwd)?; let _ = fs::remove_dir_all(shared_cwd.as_path()); test.fs() .remove( - &shared_cwd, + &shared_cwd_uri, RemoveOptions { recursive: true, force: true, @@ -1596,7 +1597,7 @@ async fn apply_patch_turn_diff_tracks_local_and_remote_environment_paths() -> Re fs::create_dir_all(shared_cwd.as_path())?; test.fs() .create_directory( - &shared_cwd, + &shared_cwd_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) @@ -1683,7 +1684,10 @@ async fn apply_patch_turn_diff_tracks_local_and_remote_environment_paths() -> Re assert_eq!(fs::read_to_string(shared_cwd.join(file_name))?, "local\n"); assert_eq!( test.fs() - .read_file_text(&shared_cwd.join(file_name), /*sandbox*/ None) + .read_file_text( + &PathUri::from_path(shared_cwd.join(file_name))?, + /*sandbox*/ None, + ) .await?, "remote\n" ); @@ -1710,7 +1714,7 @@ index 0000000000000000000000000000000000000000..9c998f7b995a7327177b38a90d138517 let _ = fs::remove_dir_all(shared_cwd.as_path()); test.fs() .remove( - &shared_cwd, + &shared_cwd_uri, RemoveOptions { recursive: true, force: true, @@ -1851,8 +1855,9 @@ async fn apply_patch_clears_aggregated_diff_after_inexact_delta() -> Result<()> let harness = apply_patch_harness_with(|builder| { builder.with_workspace_setup(|cwd, fs| async move { + let binary_path_uri = PathUri::from_path(cwd.join("binary.dat"))?; fs.write_file( - &cwd.join("binary.dat"), + &binary_path_uri, vec![0xff, 0xfe, 0xfd], /*sandbox*/ None, ) diff --git a/codex-rs/core/tests/suite/hierarchical_agents.rs b/codex-rs/core/tests/suite/hierarchical_agents.rs index 751807623..276fd235f 100644 --- a/codex-rs/core/tests/suite/hierarchical_agents.rs +++ b/codex-rs/core/tests/suite/hierarchical_agents.rs @@ -1,4 +1,5 @@ use codex_features::Feature; +use codex_utils_path_uri::PathUri; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_response_created; use core_test_support::responses::mount_sse_once; @@ -27,7 +28,8 @@ async fn hierarchical_agents_appends_to_project_doc_in_user_instructions() { }) .with_workspace_setup(|cwd, fs| async move { let agents_md = cwd.join("AGENTS.md"); - fs.write_file(&agents_md, b"be nice".to_vec(), /*sandbox*/ None) + let agents_md_uri = PathUri::from_path(&agents_md)?; + fs.write_file(&agents_md_uri, b"be nice".to_vec(), /*sandbox*/ None) .await?; Ok::<(), anyhow::Error>(()) }); diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 00f5a0c51..64d5c7081 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -28,6 +28,7 @@ use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsResponse; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use core_test_support::PathBufExt; use core_test_support::PathExt; use core_test_support::get_remote_test_env; @@ -155,19 +156,20 @@ async fn remote_test_env_can_connect_and_use_filesystem() -> Result<()> { let file_system = test_env.environment().get_filesystem(); let file_path_abs = remote_test_file_path().abs(); + let file_path_uri = PathUri::from_path(&file_path_abs)?; let payload = b"remote-test-env-ok".to_vec(); file_system - .write_file(&file_path_abs, payload.clone(), /*sandbox*/ None) + .write_file(&file_path_uri, payload.clone(), /*sandbox*/ None) .await?; let actual = file_system - .read_file(&file_path_abs, /*sandbox*/ None) + .read_file(&file_path_uri, /*sandbox*/ None) .await?; assert_eq!(actual, payload); file_system .remove( - &file_path_abs, + &file_path_uri, RemoveOptions { recursive: false, force: true, @@ -295,16 +297,18 @@ async fn exec_command_routes_to_selected_remote_environment() -> Result<()> { )) .abs(); let remote_marker_name = "marker.txt"; + let remote_cwd_uri = PathUri::from_path(&remote_cwd)?; + let remote_marker_uri = PathUri::from_path(remote_cwd.join(remote_marker_name))?; test.fs() .create_directory( - &remote_cwd, + &remote_cwd_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) .await?; test.fs() .write_file( - &remote_cwd.join(remote_marker_name), + &remote_marker_uri, b"remote-routing".to_vec(), /*sandbox*/ None, ) @@ -338,7 +342,7 @@ async fn exec_command_routes_to_selected_remote_environment() -> Result<()> { test.fs() .remove( - &remote_cwd, + &remote_cwd_uri, RemoveOptions { recursive: true, force: true, @@ -390,9 +394,10 @@ async fn remote_request_permissions_grant_unblocks_later_remote_exec() -> Result let local_write_root = local_cwd.path().join(relative_write_root); let local_target_path = local_cwd.path().join(relative_target_path); fs::create_dir(&local_write_root)?; + let remote_write_root_uri = PathUri::from_path(&remote_write_root)?; test.fs() .create_directory( - &remote_write_root, + &remote_write_root_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) @@ -527,7 +532,10 @@ async fn remote_request_permissions_grant_unblocks_later_remote_exec() -> Result ); assert_eq!( test.fs() - .read_file_text(&remote_target_path, /*sandbox*/ None) + .read_file_text( + &PathUri::from_path(&remote_target_path)?, + /*sandbox*/ None, + ) .await?, "remote-request-permissions-ok" ); @@ -538,7 +546,7 @@ async fn remote_request_permissions_grant_unblocks_later_remote_exec() -> Result test.fs() .remove( - &remote_cwd, + &PathUri::from_abs_path(&remote_cwd)?, RemoveOptions { recursive: true, force: true, @@ -567,9 +575,10 @@ async fn apply_patch_freeform_routes_to_selected_remote_environment() -> Result< SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis() )) .abs(); + let remote_cwd_uri = PathUri::from_path(&remote_cwd)?; test.fs() .create_directory( - &remote_cwd, + &remote_cwd_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) @@ -610,7 +619,10 @@ async fn apply_patch_freeform_routes_to_selected_remote_environment() -> Result< let remote_contents = test .fs() - .read_file_text(&remote_cwd.join(file_name), /*sandbox*/ None) + .read_file_text( + &PathUri::from_path(remote_cwd.join(file_name))?, + /*sandbox*/ None, + ) .await?; assert_eq!(remote_contents, "patched remote freeform\n"); assert!( @@ -620,7 +632,7 @@ async fn apply_patch_freeform_routes_to_selected_remote_environment() -> Result< test.fs() .remove( - &remote_cwd, + &remote_cwd_uri, RemoveOptions { recursive: true, force: true, @@ -651,9 +663,10 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> { SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis() )) .abs(); + let remote_cwd_uri = PathUri::from_path(&remote_cwd)?; test.fs() .create_directory( - &remote_cwd, + &remote_cwd_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) @@ -664,10 +677,11 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> { SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis() )) .abs(); + let target_path_uri = PathUri::from_path(&target_path)?; let _ = fs::remove_file(&target_path); test.fs() .remove( - &target_path, + &target_path_uri, RemoveOptions { recursive: false, force: true, @@ -771,7 +785,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> { .await; assert_eq!( test.fs() - .read_file_text(&target_path, /*sandbox*/ None) + .read_file_text(&target_path_uri, /*sandbox*/ None) .await?, "remote\n" ); @@ -785,7 +799,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> { wait_for_completion_without_patch_approval(&test).await; assert_eq!( test.fs() - .read_file_text(&target_path, /*sandbox*/ None) + .read_file_text(&target_path_uri, /*sandbox*/ None) .await?, "remote updated\n" ); @@ -793,7 +807,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> { let _ = fs::remove_file(&target_path); test.fs() .remove( - &target_path, + &target_path_uri, RemoveOptions { recursive: false, force: true, @@ -803,7 +817,7 @@ async fn apply_patch_approvals_are_remembered_per_environment() -> Result<()> { .await?; test.fs() .remove( - &remote_cwd, + &remote_cwd_uri, RemoveOptions { recursive: true, force: true, @@ -832,9 +846,10 @@ async fn apply_patch_intercepted_exec_command_routes_to_selected_remote_environm SystemTime::now().duration_since(UNIX_EPOCH)?.as_millis() )) .abs(); + let remote_cwd_uri = PathUri::from_path(&remote_cwd)?; test.fs() .create_directory( - &remote_cwd, + &remote_cwd_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) @@ -885,7 +900,10 @@ async fn apply_patch_intercepted_exec_command_routes_to_selected_remote_environm let remote_contents = test .fs() - .read_file_text(&remote_cwd.join(file_name), /*sandbox*/ None) + .read_file_text( + &PathUri::from_path(remote_cwd.join(file_name))?, + /*sandbox*/ None, + ) .await?; assert_eq!(remote_contents, "patched remote exec\n"); assert!( @@ -895,7 +913,7 @@ async fn apply_patch_intercepted_exec_command_routes_to_selected_remote_environm test.fs() .remove( - &remote_cwd, + &remote_cwd_uri, RemoveOptions { recursive: true, force: true, @@ -919,16 +937,18 @@ async fn remote_test_env_sandboxed_read_allows_readable_root() -> Result<()> { let allowed_dir = PathBuf::from(format!("/tmp/codex-remote-readable-{}", std::process::id())); let file_path = allowed_dir.join("note.txt"); + let allowed_dir_uri = PathUri::from_path(&allowed_dir)?; + let file_path_uri = PathUri::from_path(&file_path)?; file_system .create_directory( - &absolute_path(allowed_dir.clone()), + &allowed_dir_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) .await?; file_system .write_file( - &absolute_path(file_path.clone()), + &file_path_uri, b"sandboxed hello".to_vec(), /*sandbox*/ None, ) @@ -936,13 +956,13 @@ async fn remote_test_env_sandboxed_read_allows_readable_root() -> Result<()> { let sandbox = read_only_sandbox(allowed_dir.clone()); let contents = file_system - .read_file(&absolute_path(file_path.clone()), Some(&sandbox)) + .read_file(&file_path_uri, Some(&sandbox)) .await?; assert_eq!(contents, b"sandboxed hello"); file_system .remove( - &absolute_path(allowed_dir), + &allowed_dir_uri, RemoveOptions { recursive: true, force: true, @@ -976,7 +996,8 @@ async fn remote_test_env_sandboxed_read_rejects_symlink_parent_dotdot_escape() - secret = secret_path.display(), ))?; - let requested_path = absolute_path(allowed_dir.join("link").join("..").join("secret.txt")); + let requested_path = + PathUri::from_path(allowed_dir.join("link").join("..").join("secret.txt"))?; let sandbox = read_only_sandbox(allowed_dir.clone()); let error = match file_system.read_file(&requested_path, Some(&sandbox)).await { Ok(_) => anyhow::bail!("read should fail after path normalization"), @@ -1023,7 +1044,7 @@ async fn remote_test_env_remove_removes_symlink_not_target() -> Result<()> { let sandbox = workspace_write_sandbox(allowed_dir.clone()); file_system .remove( - &absolute_path(symlink_path.clone()), + &PathUri::from_path(&symlink_path)?, RemoveOptions { recursive: false, force: false, @@ -1033,18 +1054,21 @@ async fn remote_test_env_remove_removes_symlink_not_target() -> Result<()> { .await?; let symlink_exists = file_system - .get_metadata(&absolute_path(symlink_path), /*sandbox*/ None) + .get_metadata( + &PathUri::from_abs_path(&absolute_path(symlink_path))?, + /*sandbox*/ None, + ) .await .is_ok(); assert!(!symlink_exists); let outside = file_system - .read_file_text(&absolute_path(outside_file.clone()), /*sandbox*/ None) + .read_file_text(&PathUri::from_path(&outside_file)?, /*sandbox*/ None) .await?; assert_eq!(outside, "outside"); file_system .remove( - &absolute_path(root), + &PathUri::from_path(&root)?, RemoveOptions { recursive: true, force: true, @@ -1085,8 +1109,8 @@ async fn remote_test_env_copy_preserves_symlink_source() -> Result<()> { let sandbox = workspace_write_sandbox(allowed_dir.clone()); file_system .copy( - &absolute_path(source_symlink), - &absolute_path(copied_symlink.clone()), + &PathUri::from_path(&source_symlink)?, + &PathUri::from_path(&copied_symlink)?, CopyOptions { recursive: false }, Some(&sandbox), ) @@ -1117,7 +1141,7 @@ async fn remote_test_env_copy_preserves_symlink_source() -> Result<()> { file_system .remove( - &absolute_path(root), + &PathUri::from_path(&root)?, RemoveOptions { recursive: true, force: true, diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 7f0493ea8..d9397cfc5 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -48,6 +48,7 @@ use codex_protocol::protocol::McpToolCallBeginEvent; use codex_protocol::protocol::Op; use codex_protocol::user_input::UserInput; use codex_utils_cargo_bin::cargo_bin; +use codex_utils_path_uri::PathUri; use core_test_support::assert_regex_match; use core_test_support::remote_env_env_var; use core_test_support::responses; @@ -625,8 +626,10 @@ async fn stdio_server_uses_configured_cwd_before_runtime_fallback() -> anyhow::R let fixture = test_codex() .with_workspace_setup(|cwd, fs| async move { + let configured_cwd = cwd.join("mcp-configured-cwd"); + let configured_cwd_uri = PathUri::from_path(&configured_cwd)?; fs.create_directory( - &cwd.join("mcp-configured-cwd"), + &configured_cwd_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) diff --git a/codex-rs/core/tests/suite/skills.rs b/codex-rs/core/tests/suite/skills.rs index 45b52c6ac..55c75016d 100644 --- a/codex-rs/core/tests/suite/skills.rs +++ b/codex-rs/core/tests/suite/skills.rs @@ -9,6 +9,7 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Op; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_response_created; @@ -29,15 +30,17 @@ async fn write_repo_skill( body: &str, ) -> Result<()> { let skill_dir = cwd.join(".agents").join("skills").join(name); + let skill_dir_uri = PathUri::from_path(&skill_dir)?; fs.create_directory( - &skill_dir, + &skill_dir_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) .await?; let contents = format!("---\nname: {name}\ndescription: {description}\n---\n\n{body}\n"); let path = skill_dir.join("SKILL.md"); - fs.write_file(&path, contents.into_bytes(), /*sandbox*/ None) + let path_uri = PathUri::from_path(&path)?; + fs.write_file(&path_uri, contents.into_bytes(), /*sandbox*/ None) .await?; Ok(()) } diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index eb8bf2955..06b5288cd 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -17,6 +17,7 @@ use codex_protocol::protocol::ExecCommandSource; use codex_protocol::protocol::ExecCommandStatus; use codex_protocol::protocol::Op; use codex_protocol::user_input::UserInput; +use codex_utils_path_uri::PathUri; use core_test_support::TempDirExt; use core_test_support::assert_regex_match; use core_test_support::managed_network_requirements_loader; @@ -228,9 +229,10 @@ async fn create_workspace_directory( rel_path: impl AsRef, ) -> Result { let abs_path = test.config.cwd.join(rel_path.as_ref()); + let abs_path_uri = PathUri::from_path(&abs_path)?; test.fs() .create_directory( - &abs_path, + &abs_path_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) diff --git a/codex-rs/core/tests/suite/view_image.rs b/codex-rs/core/tests/suite/view_image.rs index 240f386ec..3790eb19b 100644 --- a/codex-rs/core/tests/suite/view_image.rs +++ b/codex-rs/core/tests/suite/view_image.rs @@ -29,6 +29,7 @@ use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::Op; use codex_protocol::protocol::TurnEnvironmentSelection; use codex_protocol::user_input::UserInput; +use codex_utils_path_uri::PathUri; use core_test_support::PathBufExt; use core_test_support::PathExt; use core_test_support::get_remote_test_env; @@ -134,9 +135,10 @@ fn png_bytes(width: u32, height: u32, rgba: [u8; 4]) -> anyhow::Result> async fn create_workspace_directory(test: &TestCodex, rel_path: &str) -> anyhow::Result { let abs_path = test.config.cwd.join(rel_path); + let abs_path_uri = PathUri::from_path(&abs_path)?; test.fs() .create_directory( - &abs_path, + &abs_path_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) @@ -151,16 +153,18 @@ async fn write_workspace_file( ) -> anyhow::Result { let abs_path = test.config.cwd.join(rel_path); if let Some(parent) = abs_path.parent() { + let parent_uri = PathUri::from_path(&parent)?; test.fs() .create_directory( - &parent, + &parent_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) .await?; } + let abs_path_uri = PathUri::from_path(&abs_path)?; test.fs() - .write_file(&abs_path, contents, /*sandbox*/ None) + .write_file(&abs_path_uri, contents, /*sandbox*/ None) .await?; Ok(abs_path.into_path_buf()) } @@ -607,16 +611,18 @@ async fn view_image_routes_to_selected_remote_environment() -> anyhow::Result<() )) .abs(); let image_path = remote_cwd.join("remote.png"); + let remote_cwd_uri = PathUri::from_path(&remote_cwd)?; test.fs() .create_directory( - &remote_cwd, + &remote_cwd_uri, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) .await?; let png = png_bytes(/*width*/ 1, /*height*/ 1, [0, 255, 0, 255])?; + let image_path_uri = PathUri::from_path(&image_path)?; test.fs() - .write_file(&image_path, png, /*sandbox*/ None) + .write_file(&image_path_uri, png, /*sandbox*/ None) .await?; let remote_selection = TurnEnvironmentSelection { environment_id: REMOTE_ENVIRONMENT_ID.to_string(), @@ -675,7 +681,7 @@ async fn view_image_routes_to_selected_remote_environment() -> anyhow::Result<() test.fs() .remove( - &remote_cwd, + &remote_cwd_uri, RemoveOptions { recursive: true, force: true, diff --git a/codex-rs/exec-server/Cargo.toml b/codex-rs/exec-server/Cargo.toml index eadab83fe..3173ad1bd 100644 --- a/codex-rs/exec-server/Cargo.toml +++ b/codex-rs/exec-server/Cargo.toml @@ -24,6 +24,7 @@ codex-protocol = { workspace = true } codex-sandboxing = { workspace = true } codex-shell-command = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } codex-utils-pty = { workspace = true } codex-utils-rustls-provider = { workspace = true } futures = { workspace = true } diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 1dfd49d3f..bc100b942 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -882,6 +882,7 @@ mod tests { std::env::current_exe().expect("current exe").as_path(), ) .expect("absolute current exe"); + let path = codex_utils_path_uri::PathUri::from_abs_path(&path).expect("path URI"); let sandbox = crate::FileSystemSandboxContext::from_permission_profile( codex_protocol::models::PermissionProfile::from_runtime_permissions( &codex_protocol::permissions::FileSystemSandboxPolicy::restricted(Vec::new()), diff --git a/codex-rs/exec-server/src/fs_helper.rs b/codex-rs/exec-server/src/fs_helper.rs index 0a210175b..526e68297 100644 --- a/codex-rs/exec-server/src/fs_helper.rs +++ b/codex-rs/exec-server/src/fs_helper.rs @@ -189,8 +189,10 @@ pub(crate) async fn run_direct_request( let file_system = DirectFileSystem; match request { FsHelperRequest::ReadFile(params) => { + let path = + codex_utils_path_uri::PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let data = file_system - .read_file(¶ms.path, /*sandbox*/ None) + .read_file(&path, /*sandbox*/ None) .await .map_err(map_fs_error)?; Ok(FsHelperPayload::ReadFile(FsReadFileResponse { @@ -198,21 +200,25 @@ pub(crate) async fn run_direct_request( })) } FsHelperRequest::WriteFile(params) => { + let path = + codex_utils_path_uri::PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let bytes = STANDARD.decode(params.data_base64).map_err(|err| { invalid_request(format!( "{FS_WRITE_FILE_METHOD} requires valid base64 dataBase64: {err}" )) })?; file_system - .write_file(¶ms.path, bytes, /*sandbox*/ None) + .write_file(&path, bytes, /*sandbox*/ None) .await .map_err(map_fs_error)?; Ok(FsHelperPayload::WriteFile(FsWriteFileResponse {})) } FsHelperRequest::CreateDirectory(params) => { + let path = + codex_utils_path_uri::PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; file_system .create_directory( - ¶ms.path, + &path, CreateDirectoryOptions { recursive: params.recursive.unwrap_or(true), }, @@ -225,8 +231,10 @@ pub(crate) async fn run_direct_request( )) } FsHelperRequest::GetMetadata(params) => { + let path = + codex_utils_path_uri::PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let metadata = file_system - .get_metadata(¶ms.path, /*sandbox*/ None) + .get_metadata(&path, /*sandbox*/ None) .await .map_err(map_fs_error)?; Ok(FsHelperPayload::GetMetadata(FsGetMetadataResponse { @@ -238,17 +246,22 @@ pub(crate) async fn run_direct_request( })) } FsHelperRequest::Canonicalize(params) => { + let path = + codex_utils_path_uri::PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let path = file_system - .canonicalize(¶ms.path, /*sandbox*/ None) + .canonicalize(&path, /*sandbox*/ None) .await .map_err(map_fs_error)?; + let path = path.to_abs_path().map_err(map_fs_error)?; Ok(FsHelperPayload::Canonicalize(FsCanonicalizeResponse { path, })) } FsHelperRequest::ReadDirectory(params) => { + let path = + codex_utils_path_uri::PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let entries = file_system - .read_directory(¶ms.path, /*sandbox*/ None) + .read_directory(&path, /*sandbox*/ None) .await .map_err(map_fs_error)? .into_iter() @@ -263,9 +276,11 @@ pub(crate) async fn run_direct_request( })) } FsHelperRequest::Remove(params) => { + let path = + codex_utils_path_uri::PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; file_system .remove( - ¶ms.path, + &path, RemoveOptions { recursive: params.recursive.unwrap_or(true), force: params.force.unwrap_or(true), @@ -277,10 +292,15 @@ pub(crate) async fn run_direct_request( Ok(FsHelperPayload::Remove(FsRemoveResponse {})) } FsHelperRequest::Copy(params) => { + let source_path = codex_utils_path_uri::PathUri::from_abs_path(¶ms.source_path) + .map_err(map_fs_error)?; + let destination_path = + codex_utils_path_uri::PathUri::from_abs_path(¶ms.destination_path) + .map_err(map_fs_error)?; file_system .copy( - ¶ms.source_path, - ¶ms.destination_path, + &source_path, + &destination_path, CopyOptions { recursive: params.recursive, }, @@ -305,23 +325,72 @@ fn map_fs_error(err: io::Error) -> JSONRPCErrorError { #[cfg(test)] mod tests { + use codex_utils_absolute_path::AbsolutePathBuf; + use pretty_assertions::assert_eq; + use serde_json::json; + use super::*; #[test] - fn helper_requests_use_fs_method_names() -> serde_json::Result<()> { - assert_eq!( - serde_json::to_value(FsHelperRequest::WriteFile(FsWriteFileParams { - path: std::env::current_dir() - .expect("cwd") - .join("file") - .as_path() - .try_into() - .expect("absolute path"), + fn helper_protocol_keeps_native_absolute_paths() -> serde_json::Result<()> { + let local_path = + AbsolutePathBuf::from_absolute_path(std::env::current_dir().expect("cwd").join("file")) + .expect("absolute path"); + #[cfg(not(windows))] + let paths = [local_path]; + #[cfg(windows)] + let paths = [ + local_path, + AbsolutePathBuf::from_absolute_path(r"\\server\share\file").expect("absolute UNC path"), + ]; + + for path in paths { + let expected_path = path.to_string_lossy().into_owned(); + + let request = serde_json::to_value(FsHelperRequest::WriteFile(FsWriteFileParams { + path: path.clone(), data_base64: String::new(), sandbox: None, - }))?["operation"], - FS_WRITE_FILE_METHOD, - ); + }))?; + assert_eq!( + request, + json!({ + "operation": FS_WRITE_FILE_METHOD, + "params": { + "path": expected_path.as_str(), + "dataBase64": "", + "sandbox": null, + }, + }), + ); + let request_path = request["params"]["path"] + .as_str() + .expect("request path should be a string"); + assert_eq!(request_path, expected_path); + assert!(!request_path.starts_with("file:")); + + let response = serde_json::to_value(FsHelperResponse::Ok( + FsHelperPayload::Canonicalize(FsCanonicalizeResponse { path }), + ))?; + assert_eq!( + response, + json!({ + "status": "ok", + "payload": { + "operation": FS_CANONICALIZE_METHOD, + "response": { + "path": expected_path.as_str(), + }, + }, + }), + ); + let response_path = response["payload"]["response"]["path"] + .as_str() + .expect("canonicalize response path should be a string"); + assert_eq!(response_path, expected_path); + assert!(!response_path.starts_with("file:")); + } + Ok(()) } } diff --git a/codex-rs/exec-server/src/local_file_system.rs b/codex-rs/exec-server/src/local_file_system.rs index 3f410f926..ee38d7ff9 100644 --- a/codex-rs/exec-server/src/local_file_system.rs +++ b/codex-rs/exec-server/src/local_file_system.rs @@ -1,5 +1,6 @@ use async_trait::async_trait; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; @@ -81,28 +82,16 @@ impl LocalFileSystem { impl ExecutorFileSystem for LocalFileSystem { async fn canonicalize( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, - ) -> FileSystemResult { + ) -> FileSystemResult { let (file_system, sandbox) = self.file_system_for(sandbox)?; 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, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { let (file_system, sandbox) = self.file_system_for(sandbox)?; @@ -111,7 +100,7 @@ impl ExecutorFileSystem for LocalFileSystem { async fn write_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, contents: Vec, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -121,7 +110,7 @@ impl ExecutorFileSystem for LocalFileSystem { async fn create_directory( &self, - path: &AbsolutePathBuf, + path: &PathUri, options: CreateDirectoryOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -131,7 +120,7 @@ impl ExecutorFileSystem for LocalFileSystem { async fn get_metadata( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult { let (file_system, sandbox) = self.file_system_for(sandbox)?; @@ -140,7 +129,7 @@ impl ExecutorFileSystem for LocalFileSystem { async fn read_directory( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { let (file_system, sandbox) = self.file_system_for(sandbox)?; @@ -149,7 +138,7 @@ impl ExecutorFileSystem for LocalFileSystem { async fn remove( &self, - path: &AbsolutePathBuf, + path: &PathUri, options: RemoveOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -159,8 +148,8 @@ impl ExecutorFileSystem for LocalFileSystem { async fn copy( &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, + source_path: &PathUri, + destination_path: &PathUri, options: CopyOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -175,28 +164,16 @@ impl ExecutorFileSystem for LocalFileSystem { impl ExecutorFileSystem for UnsandboxedFileSystem { async fn canonicalize( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, - ) -> FileSystemResult { + ) -> FileSystemResult { reject_platform_sandbox_context(sandbox)?; 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, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { reject_platform_sandbox_context(sandbox)?; @@ -205,7 +182,7 @@ impl ExecutorFileSystem for UnsandboxedFileSystem { async fn write_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, contents: Vec, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -217,7 +194,7 @@ impl ExecutorFileSystem for UnsandboxedFileSystem { async fn create_directory( &self, - path: &AbsolutePathBuf, + path: &PathUri, options: CreateDirectoryOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -229,7 +206,7 @@ impl ExecutorFileSystem for UnsandboxedFileSystem { async fn get_metadata( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult { reject_platform_sandbox_context(sandbox)?; @@ -238,7 +215,7 @@ impl ExecutorFileSystem for UnsandboxedFileSystem { async fn read_directory( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { reject_platform_sandbox_context(sandbox)?; @@ -249,7 +226,7 @@ impl ExecutorFileSystem for UnsandboxedFileSystem { async fn remove( &self, - path: &AbsolutePathBuf, + path: &PathUri, options: RemoveOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -261,8 +238,8 @@ impl ExecutorFileSystem for UnsandboxedFileSystem { async fn copy( &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, + source_path: &PathUri, + destination_path: &PathUri, options: CopyOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -282,31 +259,23 @@ impl ExecutorFileSystem for UnsandboxedFileSystem { impl ExecutorFileSystem for DirectFileSystem { async fn canonicalize( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, - ) -> FileSystemResult { + ) -> FileSystemResult { reject_sandbox_context(sandbox)?; - 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()) + let path = path.to_abs_path()?; + let canonicalized = + AbsolutePathBuf::from_absolute_path(tokio::fs::canonicalize(path.as_path()).await?)?; + PathUri::from_abs_path(&canonicalized) } async fn read_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { reject_sandbox_context(sandbox)?; + let path = path.to_abs_path()?; let metadata = tokio::fs::metadata(path.as_path()).await?; if metadata.len() > MAX_READ_FILE_BYTES { return Err(io::Error::new( @@ -319,21 +288,23 @@ impl ExecutorFileSystem for DirectFileSystem { async fn write_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, contents: Vec, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { reject_sandbox_context(sandbox)?; + let path = path.to_abs_path()?; tokio::fs::write(path.as_path(), contents).await } async fn create_directory( &self, - path: &AbsolutePathBuf, + path: &PathUri, options: CreateDirectoryOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { reject_sandbox_context(sandbox)?; + let path = path.to_abs_path()?; if options.recursive { tokio::fs::create_dir_all(path.as_path()).await?; } else { @@ -344,10 +315,11 @@ impl ExecutorFileSystem for DirectFileSystem { async fn get_metadata( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult { reject_sandbox_context(sandbox)?; + let path = path.to_abs_path()?; let metadata = tokio::fs::metadata(path.as_path()).await?; let symlink_metadata = tokio::fs::symlink_metadata(path.as_path()).await?; Ok(FileMetadata { @@ -361,10 +333,11 @@ impl ExecutorFileSystem for DirectFileSystem { async fn read_directory( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { reject_sandbox_context(sandbox)?; + let path = path.to_abs_path()?; let mut entries = Vec::new(); let mut read_dir = tokio::fs::read_dir(path.as_path()).await?; while let Some(entry) = read_dir.next_entry().await? { @@ -382,11 +355,12 @@ impl ExecutorFileSystem for DirectFileSystem { async fn remove( &self, - path: &AbsolutePathBuf, + path: &PathUri, options: RemoveOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { reject_sandbox_context(sandbox)?; + let path = path.to_abs_path()?; match tokio::fs::symlink_metadata(path.as_path()).await { Ok(metadata) => { let file_type = metadata.file_type(); @@ -408,14 +382,14 @@ impl ExecutorFileSystem for DirectFileSystem { async fn copy( &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, + source_path: &PathUri, + destination_path: &PathUri, options: CopyOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { reject_sandbox_context(sandbox)?; - let source_path = source_path.to_path_buf(); - let destination_path = destination_path.to_path_buf(); + let source_path = source_path.to_abs_path()?.into_path_buf(); + let destination_path = destination_path.to_abs_path()?.into_path_buf(); tokio::task::spawn_blocking(move || -> FileSystemResult<()> { let metadata = std::fs::symlink_metadata(source_path.as_path())?; let file_type = metadata.file_type(); @@ -576,6 +550,10 @@ fn system_time_to_unix_ms(time: SystemTime) -> i64 { .unwrap_or(0) } +#[cfg(all(test, any(unix, windows)))] +#[path = "local_file_system_path_uri_tests.rs"] +mod path_uri_tests; + #[cfg(all(test, unix))] mod tests { use super::*; diff --git a/codex-rs/exec-server/src/local_file_system_path_uri_tests.rs b/codex-rs/exec-server/src/local_file_system_path_uri_tests.rs new file mode 100644 index 000000000..dcddd98e0 --- /dev/null +++ b/codex-rs/exec-server/src/local_file_system_path_uri_tests.rs @@ -0,0 +1,27 @@ +use codex_utils_path_uri::PathUri; +use pretty_assertions::assert_eq; +use tokio::io; + +use super::*; + +#[tokio::test] +async fn direct_file_system_rejects_non_native_uri_as_invalid_input() { + let error = DirectFileSystem + .read_file(&non_native_uri(), /*sandbox*/ None) + .await + .expect_err("non-native URI should be rejected"); + + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); +} + +fn non_native_uri() -> PathUri { + #[cfg(unix)] + let uri = "file://server/share/file.txt"; + #[cfg(windows)] + let uri = "file:///usr/local/file.txt"; + + match PathUri::parse(uri) { + Ok(uri) => uri, + Err(err) => panic!("valid non-native URI should parse: {err}"), + } +} diff --git a/codex-rs/exec-server/src/remote_file_system.rs b/codex-rs/exec-server/src/remote_file_system.rs index 54198c93b..9bcdcc5b0 100644 --- a/codex-rs/exec-server/src/remote_file_system.rs +++ b/codex-rs/exec-server/src/remote_file_system.rs @@ -1,8 +1,7 @@ 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 codex_utils_path_uri::PathUri; 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; @@ -45,58 +42,33 @@ impl RemoteFileSystem { impl ExecutorFileSystem for RemoteFileSystem { async fn canonicalize( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, - ) -> FileSystemResult { + ) -> FileSystemResult { trace!("remote fs canonicalize"); + let path = path.to_abs_path()?; let client = self.client.get().await.map_err(map_remote_error)?; let response = client .fs_canonicalize(FsCanonicalizeParams { - path: path.clone(), + path, sandbox: remote_sandbox_context(sandbox), }) .await .map_err(map_remote_error)?; - 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) + PathUri::from_abs_path(&response.path) } async fn read_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { trace!("remote fs read_file"); + let path = path.to_abs_path()?; let client = self.client.get().await.map_err(map_remote_error)?; let response = client .fs_read_file(FsReadFileParams { - path: path.clone(), + path, sandbox: remote_sandbox_context(sandbox), }) .await @@ -111,15 +83,16 @@ impl ExecutorFileSystem for RemoteFileSystem { async fn write_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, contents: Vec, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs write_file"); + let path = path.to_abs_path()?; let client = self.client.get().await.map_err(map_remote_error)?; client .fs_write_file(FsWriteFileParams { - path: path.clone(), + path, data_base64: STANDARD.encode(contents), sandbox: remote_sandbox_context(sandbox), }) @@ -130,15 +103,16 @@ impl ExecutorFileSystem for RemoteFileSystem { async fn create_directory( &self, - path: &AbsolutePathBuf, + path: &PathUri, options: CreateDirectoryOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs create_directory"); + let path = path.to_abs_path()?; let client = self.client.get().await.map_err(map_remote_error)?; client .fs_create_directory(FsCreateDirectoryParams { - path: path.clone(), + path, recursive: Some(options.recursive), sandbox: remote_sandbox_context(sandbox), }) @@ -149,14 +123,15 @@ impl ExecutorFileSystem for RemoteFileSystem { async fn get_metadata( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult { trace!("remote fs get_metadata"); + let path = path.to_abs_path()?; let client = self.client.get().await.map_err(map_remote_error)?; let response = client .fs_get_metadata(FsGetMetadataParams { - path: path.clone(), + path, sandbox: remote_sandbox_context(sandbox), }) .await @@ -172,14 +147,15 @@ impl ExecutorFileSystem for RemoteFileSystem { async fn read_directory( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { trace!("remote fs read_directory"); + let path = path.to_abs_path()?; let client = self.client.get().await.map_err(map_remote_error)?; let response = client .fs_read_directory(FsReadDirectoryParams { - path: path.clone(), + path, sandbox: remote_sandbox_context(sandbox), }) .await @@ -197,15 +173,16 @@ impl ExecutorFileSystem for RemoteFileSystem { async fn remove( &self, - path: &AbsolutePathBuf, + path: &PathUri, options: RemoveOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs remove"); + let path = path.to_abs_path()?; let client = self.client.get().await.map_err(map_remote_error)?; client .fs_remove(FsRemoveParams { - path: path.clone(), + path, recursive: Some(options.recursive), force: Some(options.force), sandbox: remote_sandbox_context(sandbox), @@ -217,17 +194,19 @@ impl ExecutorFileSystem for RemoteFileSystem { async fn copy( &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, + source_path: &PathUri, + destination_path: &PathUri, options: CopyOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { trace!("remote fs copy"); + let source_path = source_path.to_abs_path()?; + let destination_path = destination_path.to_abs_path()?; let client = self.client.get().await.map_err(map_remote_error)?; client .fs_copy(FsCopyParams { - source_path: source_path.clone(), - destination_path: destination_path.clone(), + source_path, + destination_path, recursive: options.recursive, sandbox: remote_sandbox_context(sandbox), }) @@ -261,6 +240,10 @@ fn map_remote_error(error: ExecServerError) -> io::Error { } } +#[cfg(all(test, any(unix, windows)))] +#[path = "remote_file_system_path_uri_tests.rs"] +mod path_uri_tests; + #[cfg(test)] mod tests { use codex_protocol::models::PermissionProfile; @@ -270,6 +253,7 @@ mod tests { use codex_protocol::permissions::FileSystemSandboxPolicy; use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; + use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use super::*; diff --git a/codex-rs/exec-server/src/remote_file_system_path_uri_tests.rs b/codex-rs/exec-server/src/remote_file_system_path_uri_tests.rs new file mode 100644 index 000000000..29d0edaff --- /dev/null +++ b/codex-rs/exec-server/src/remote_file_system_path_uri_tests.rs @@ -0,0 +1,225 @@ +#![allow(clippy::expect_used)] + +#[cfg(windows)] +use codex_app_server_protocol::JSONRPCMessage; +#[cfg(windows)] +use codex_app_server_protocol::JSONRPCResponse; +#[cfg(windows)] +use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; +#[cfg(windows)] +use futures::SinkExt; +#[cfg(windows)] +use futures::StreamExt; +use pretty_assertions::assert_eq; +use tokio::io; +#[cfg(windows)] +use tokio::net::TcpListener; +#[cfg(windows)] +use tokio::net::TcpStream; +#[cfg(windows)] +use tokio::sync::oneshot; +#[cfg(windows)] +use tokio::time::Duration; +#[cfg(windows)] +use tokio::time::timeout; +#[cfg(windows)] +use tokio_tungstenite::WebSocketStream; +#[cfg(windows)] +use tokio_tungstenite::accept_async; +#[cfg(windows)] +use tokio_tungstenite::tungstenite::Message; + +use super::*; +use crate::client_api::ExecServerTransportParams; +#[cfg(windows)] +use crate::protocol::FS_READ_FILE_METHOD; +#[cfg(windows)] +use crate::protocol::FsReadFileParams; +#[cfg(windows)] +use crate::protocol::FsReadFileResponse; +#[cfg(windows)] +use crate::protocol::INITIALIZE_METHOD; +#[cfg(windows)] +use crate::protocol::INITIALIZED_METHOD; +#[cfg(windows)] +use crate::protocol::InitializeResponse; + +#[tokio::test] +async fn non_native_uri_is_rejected_before_connecting() { + let file_system = RemoteFileSystem::new(LazyRemoteExecServerClient::new( + ExecServerTransportParams::websocket_url("not a websocket URL".to_string()), + )); + + let error = file_system + .read_file(&non_native_uri(), /*sandbox*/ None) + .await + .expect_err("non-native URI should be rejected"); + + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); +} + +#[cfg(windows)] +#[tokio::test] +async fn remote_file_system_sends_explicit_windows_native_paths() { + let (websocket_url, captured_paths, server) = record_read_file_paths(2).await; + let file_system = RemoteFileSystem::new(LazyRemoteExecServerClient::new( + ExecServerTransportParams::websocket_url(websocket_url), + )); + let paths = vec![ + ( + PathUri::parse("file:///C:/Users/Alice/src/main.rs").expect("valid drive URI"), + absolute_windows_path(r"C:\Users\Alice\src\main.rs"), + ), + ( + PathUri::parse("file://server/share/src/main.rs").expect("valid UNC URI"), + absolute_windows_path(r"\\server\share\src\main.rs"), + ), + ]; + let expected_paths = paths + .iter() + .map(|(_, expected_path)| expected_path.clone()) + .collect::>(); + + for (path, _) in paths { + assert_eq!( + file_system + .read_file(&path, /*sandbox*/ None) + .await + .expect("remote read should succeed"), + Vec::::new() + ); + } + + assert_eq!( + captured_paths.await.expect("captured paths"), + expected_paths + ); + server.await.expect("recording server should succeed"); +} + +fn non_native_uri() -> PathUri { + #[cfg(unix)] + let uri = "file://server/share/file.txt"; + #[cfg(windows)] + let uri = "file:///usr/local/file.txt"; + + PathUri::parse(uri).expect("valid non-native URI") +} + +#[cfg(windows)] +fn absolute_windows_path(path: &str) -> AbsolutePathBuf { + AbsolutePathBuf::from_absolute_path_checked(path).expect("absolute Windows path") +} + +#[cfg(windows)] +async fn record_read_file_paths( + expected_requests: usize, +) -> ( + String, + oneshot::Receiver>, + tokio::task::JoinHandle<()>, +) { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("listener should bind"); + let websocket_url = format!("ws://{}", listener.local_addr().expect("listener address")); + let (captured_paths_tx, captured_paths_rx) = oneshot::channel(); + let server = tokio::spawn(async move { + let (stream, _) = listener.accept().await.expect("listener should accept"); + let mut websocket = accept_async(stream) + .await + .expect("websocket handshake should succeed"); + complete_websocket_initialize(&mut websocket).await; + + let mut captured_paths = Vec::with_capacity(expected_requests); + for _ in 0..expected_requests { + let request = match read_jsonrpc_websocket(&mut websocket).await { + JSONRPCMessage::Request(request) if request.method == FS_READ_FILE_METHOD => { + request + } + other => panic!("expected fs/readFile request, got {other:?}"), + }; + let params: FsReadFileParams = + serde_json::from_value(request.params.expect("fs/readFile params should exist")) + .expect("fs/readFile params should deserialize"); + captured_paths.push(params.path); + write_jsonrpc_websocket( + &mut websocket, + JSONRPCMessage::Response(JSONRPCResponse { + id: request.id, + result: serde_json::to_value(FsReadFileResponse { + data_base64: String::new(), + }) + .expect("fs/readFile response should serialize"), + }), + ) + .await; + } + captured_paths_tx + .send(captured_paths) + .expect("captured paths receiver should stay open"); + }); + + (websocket_url, captured_paths_rx, server) +} + +#[cfg(windows)] +async fn complete_websocket_initialize(websocket: &mut WebSocketStream) { + let request = match read_jsonrpc_websocket(websocket).await { + JSONRPCMessage::Request(request) if request.method == INITIALIZE_METHOD => request, + other => panic!("expected initialize request, got {other:?}"), + }; + write_jsonrpc_websocket( + websocket, + JSONRPCMessage::Response(JSONRPCResponse { + id: request.id, + result: serde_json::to_value(InitializeResponse { + session_id: "session-1".to_string(), + }) + .expect("initialize response should serialize"), + }), + ) + .await; + + match read_jsonrpc_websocket(websocket).await { + JSONRPCMessage::Notification(notification) if notification.method == INITIALIZED_METHOD => { + } + other => panic!("expected initialized notification, got {other:?}"), + } +} + +#[cfg(windows)] +async fn read_jsonrpc_websocket(websocket: &mut WebSocketStream) -> JSONRPCMessage { + loop { + match timeout(Duration::from_secs(1), websocket.next()) + .await + .expect("json-rpc websocket read should not time out") + .expect("websocket should stay open") + .expect("websocket frame should read") + { + Message::Text(text) => { + return serde_json::from_str(text.as_ref()) + .expect("json-rpc text frame should parse"); + } + Message::Binary(bytes) => { + return serde_json::from_slice(bytes.as_ref()) + .expect("json-rpc binary frame should parse"); + } + Message::Ping(_) | Message::Pong(_) => {} + other => panic!("expected json-rpc websocket frame, got {other:?}"), + } + } +} + +#[cfg(windows)] +async fn write_jsonrpc_websocket( + websocket: &mut WebSocketStream, + message: JSONRPCMessage, +) { + let encoded = serde_json::to_string(&message).expect("json-rpc should serialize"); + websocket + .send(Message::Text(encoded.into())) + .await + .expect("json-rpc websocket frame should write"); +} diff --git a/codex-rs/exec-server/src/sandboxed_file_system.rs b/codex-rs/exec-server/src/sandboxed_file_system.rs index 00b6966c3..14f26e60b 100644 --- a/codex-rs/exec-server/src/sandboxed_file_system.rs +++ b/codex-rs/exec-server/src/sandboxed_file_system.rs @@ -2,8 +2,7 @@ use async_trait::async_trait; 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 codex_utils_path_uri::PathUri; use tokio::io; use crate::CopyOptions; @@ -55,39 +54,27 @@ impl SandboxedFileSystem { impl ExecutorFileSystem for SandboxedFileSystem { async fn canonicalize( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, - ) -> FileSystemResult { + ) -> FileSystemResult { let sandbox = require_platform_sandbox(sandbox)?; let response = self .run_sandboxed( sandbox, FsHelperRequest::Canonicalize(FsCanonicalizeParams { - path: path.clone(), + path: path.to_abs_path()?, sandbox: None, }), ) .await? .expect_canonicalize() .map_err(map_sandbox_error)?; - 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()) + PathUri::from_abs_path(&response.path) } async fn read_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { let sandbox = require_platform_sandbox(sandbox)?; @@ -95,7 +82,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { .run_sandboxed( sandbox, FsHelperRequest::ReadFile(FsReadFileParams { - path: path.clone(), + path: path.to_abs_path()?, sandbox: None, }), ) @@ -112,7 +99,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { async fn write_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, contents: Vec, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -120,7 +107,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { self.run_sandboxed( sandbox, FsHelperRequest::WriteFile(FsWriteFileParams { - path: path.clone(), + path: path.to_abs_path()?, data_base64: STANDARD.encode(contents), sandbox: None, }), @@ -133,7 +120,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { async fn create_directory( &self, - path: &AbsolutePathBuf, + path: &PathUri, options: CreateDirectoryOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -141,7 +128,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { self.run_sandboxed( sandbox, FsHelperRequest::CreateDirectory(FsCreateDirectoryParams { - path: path.clone(), + path: path.to_abs_path()?, recursive: Some(options.recursive), sandbox: None, }), @@ -154,7 +141,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { async fn get_metadata( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult { let sandbox = require_platform_sandbox(sandbox)?; @@ -162,7 +149,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { .run_sandboxed( sandbox, FsHelperRequest::GetMetadata(FsGetMetadataParams { - path: path.clone(), + path: path.to_abs_path()?, sandbox: None, }), ) @@ -180,7 +167,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { async fn read_directory( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { let sandbox = require_platform_sandbox(sandbox)?; @@ -188,7 +175,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { .run_sandboxed( sandbox, FsHelperRequest::ReadDirectory(FsReadDirectoryParams { - path: path.clone(), + path: path.to_abs_path()?, sandbox: None, }), ) @@ -208,7 +195,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { async fn remove( &self, - path: &AbsolutePathBuf, + path: &PathUri, remove_options: RemoveOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -216,7 +203,7 @@ impl ExecutorFileSystem for SandboxedFileSystem { self.run_sandboxed( sandbox, FsHelperRequest::Remove(FsRemoveParams { - path: path.clone(), + path: path.to_abs_path()?, recursive: Some(remove_options.recursive), force: Some(remove_options.force), sandbox: None, @@ -230,8 +217,8 @@ impl ExecutorFileSystem for SandboxedFileSystem { async fn copy( &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, + source_path: &PathUri, + destination_path: &PathUri, options: CopyOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -239,8 +226,8 @@ impl ExecutorFileSystem for SandboxedFileSystem { self.run_sandboxed( sandbox, FsHelperRequest::Copy(FsCopyParams { - source_path: source_path.clone(), - destination_path: destination_path.clone(), + source_path: source_path.to_abs_path()?, + destination_path: destination_path.to_abs_path()?, recursive: options.recursive, sandbox: None, }), @@ -272,3 +259,7 @@ fn map_sandbox_error(error: JSONRPCErrorError) -> io::Error { _ => io::Error::other(error.message), } } + +#[cfg(all(test, any(unix, windows)))] +#[path = "sandboxed_file_system_path_uri_tests.rs"] +mod path_uri_tests; diff --git a/codex-rs/exec-server/src/sandboxed_file_system_path_uri_tests.rs b/codex-rs/exec-server/src/sandboxed_file_system_path_uri_tests.rs new file mode 100644 index 000000000..eb1a0d5dc --- /dev/null +++ b/codex-rs/exec-server/src/sandboxed_file_system_path_uri_tests.rs @@ -0,0 +1,43 @@ +use codex_protocol::models::PermissionProfile; +use codex_protocol::permissions::FileSystemSandboxPolicy; +use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_utils_path_uri::PathUri; +use pretty_assertions::assert_eq; +use tokio::io; + +use super::*; + +#[tokio::test] +async fn sandboxed_file_system_rejects_non_native_uri_as_invalid_input() { + let runtime_paths = ExecServerRuntimePaths::new( + std::env::current_exe().expect("current exe"), + /*codex_linux_sandbox_exe*/ None, + ) + .expect("runtime paths"); + let file_system = SandboxedFileSystem::new(runtime_paths); + let sandbox = FileSystemSandboxContext::from_permission_profile( + PermissionProfile::from_runtime_permissions( + &FileSystemSandboxPolicy::restricted(Vec::new()), + NetworkSandboxPolicy::Restricted, + ), + ); + + let error = file_system + .read_file(&non_native_uri(), Some(&sandbox)) + .await + .expect_err("non-native URI should be rejected"); + + assert_eq!(error.kind(), io::ErrorKind::InvalidInput); +} + +fn non_native_uri() -> PathUri { + #[cfg(unix)] + let uri = "file://server/share/file.txt"; + #[cfg(windows)] + let uri = "file:///usr/local/file.txt"; + + match PathUri::parse(uri) { + Ok(uri) => uri, + Err(err) => panic!("valid non-native URI should parse: {err}"), + } +} 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 b60589d02..97d028f89 100644 --- a/codex-rs/exec-server/src/server/file_system_handler.rs +++ b/codex-rs/exec-server/src/server/file_system_handler.rs @@ -3,6 +3,7 @@ use std::io; use base64::Engine as _; use base64::engine::general_purpose::STANDARD; use codex_app_server_protocol::JSONRPCErrorError; +use codex_utils_path_uri::PathUri; use crate::CopyOptions; use crate::CreateDirectoryOptions; @@ -52,9 +53,10 @@ impl FileSystemHandler { &self, params: FsReadFileParams, ) -> Result { + let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let bytes = self .file_system - .read_file(¶ms.path, params.sandbox.as_ref()) + .read_file(&path, params.sandbox.as_ref()) .await .map_err(map_fs_error)?; Ok(FsReadFileResponse { @@ -66,13 +68,14 @@ impl FileSystemHandler { &self, params: FsWriteFileParams, ) -> Result { + let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let bytes = STANDARD.decode(params.data_base64).map_err(|err| { invalid_request(format!( "{FS_WRITE_FILE_METHOD} requires valid base64 dataBase64: {err}" )) })?; self.file_system - .write_file(¶ms.path, bytes, params.sandbox.as_ref()) + .write_file(&path, bytes, params.sandbox.as_ref()) .await .map_err(map_fs_error)?; Ok(FsWriteFileResponse {}) @@ -83,9 +86,10 @@ impl FileSystemHandler { params: FsCreateDirectoryParams, ) -> Result { let recursive = params.recursive.unwrap_or(true); + let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; self.file_system .create_directory( - ¶ms.path, + &path, CreateDirectoryOptions { recursive }, params.sandbox.as_ref(), ) @@ -98,9 +102,10 @@ impl FileSystemHandler { &self, params: FsGetMetadataParams, ) -> Result { + let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let metadata = self .file_system - .get_metadata(¶ms.path, params.sandbox.as_ref()) + .get_metadata(&path, params.sandbox.as_ref()) .await .map_err(map_fs_error)?; Ok(FsGetMetadataResponse { @@ -116,11 +121,13 @@ impl FileSystemHandler { &self, params: FsCanonicalizeParams, ) -> Result { + let requested_path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let path = self .file_system - .canonicalize(¶ms.path, params.sandbox.as_ref()) + .canonicalize(&requested_path, params.sandbox.as_ref()) .await .map_err(map_fs_error)?; + let path = path.to_abs_path().map_err(map_fs_error)?; Ok(FsCanonicalizeResponse { path }) } @@ -128,11 +135,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 +144,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 }) } @@ -152,9 +153,10 @@ impl FileSystemHandler { &self, params: FsReadDirectoryParams, ) -> Result { + let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; let entries = self .file_system - .read_directory(¶ms.path, params.sandbox.as_ref()) + .read_directory(&path, params.sandbox.as_ref()) .await .map_err(map_fs_error)? .into_iter() @@ -173,9 +175,10 @@ impl FileSystemHandler { ) -> Result { let recursive = params.recursive.unwrap_or(true); let force = params.force.unwrap_or(true); + let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?; self.file_system .remove( - ¶ms.path, + &path, RemoveOptions { recursive, force }, params.sandbox.as_ref(), ) @@ -188,10 +191,13 @@ impl FileSystemHandler { &self, params: FsCopyParams, ) -> Result { + let source_path = PathUri::from_abs_path(¶ms.source_path).map_err(map_fs_error)?; + let destination_path = + PathUri::from_abs_path(¶ms.destination_path).map_err(map_fs_error)?; self.file_system .copy( - ¶ms.source_path, - ¶ms.destination_path, + &source_path, + &destination_path, CopyOptions { recursive: params.recursive, }, @@ -262,6 +268,24 @@ mod tests { .await .expect("write file"); + let canonicalized = handler + .canonicalize(FsCanonicalizeParams { + path: path.clone(), + sandbox: Some(FileSystemSandboxContext::from_legacy_sandbox_policy( + sandbox_policy.clone(), + sandbox_cwd.clone(), + )), + }) + .await + .expect("canonicalize file"); + assert_eq!( + canonicalized.path, + AbsolutePathBuf::from_absolute_path( + std::fs::canonicalize(path.as_path()).expect("canonical path"), + ) + .expect("absolute canonical path"), + ); + let response = handler .read_file(FsReadFileParams { path, @@ -276,4 +300,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 34e3111d5..0bef4aa28 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; @@ -61,7 +62,7 @@ async fn file_system_get_metadata_reports_files_and_directories( std::fs::create_dir(&directory_path)?; let file_metadata = file_system - .get_metadata(&absolute_path(&file_path), /*sandbox*/ None) + .get_metadata(&PathUri::from_path(&file_path)?, /*sandbox*/ None) .await .with_context(|| format!("mode={implementation}"))?; assert_eq!(file_metadata.is_directory, false); @@ -70,7 +71,7 @@ async fn file_system_get_metadata_reports_files_and_directories( assert!(file_metadata.modified_at_ms > 0); let directory_metadata = file_system - .get_metadata(&absolute_path(&directory_path), /*sandbox*/ None) + .get_metadata(&PathUri::from_path(&directory_path)?, /*sandbox*/ None) .await .with_context(|| format!("mode={implementation}"))?; assert_eq!(directory_metadata.is_directory, true); @@ -95,7 +96,7 @@ async fn file_system_create_directory_creates_nested_directories( file_system .create_directory( - &absolute_path(&nested_dir), + &PathUri::from_path(&nested_dir)?, CreateDirectoryOptions { recursive: true }, /*sandbox*/ None, ) @@ -119,7 +120,7 @@ async fn file_system_write_file_writes_bytes( let file_path = tmp.path().join("note.txt"); file_system .write_file( - &absolute_path(&file_path), + &PathUri::from_path(&file_path)?, b"hello from trait".to_vec(), /*sandbox*/ None, ) @@ -130,42 +131,26 @@ 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(()) } @@ -183,7 +168,7 @@ async fn file_system_read_file_returns_bytes( std::fs::write(&file_path, "hello from trait")?; let contents = file_system - .read_file(&absolute_path(&file_path), /*sandbox*/ None) + .read_file(&PathUri::from_path(&file_path)?, /*sandbox*/ None) .await .with_context(|| format!("mode={implementation}"))?; assert_eq!(contents, b"hello from trait"); @@ -205,7 +190,7 @@ async fn file_system_read_file_text_returns_string( std::fs::write(&file_path, "hello from trait")?; let contents = file_system - .read_file_text(&absolute_path(&file_path), /*sandbox*/ None) + .read_file_text(&PathUri::from_path(&file_path)?, /*sandbox*/ None) .await .with_context(|| format!("mode={implementation}"))?; assert_eq!(contents, "hello from trait"); @@ -227,8 +212,8 @@ async fn file_system_copy_copies_file(implementation: FileSystemImplementation) file_system .copy( - &absolute_path(&source_file), - &absolute_path(&copied_file), + &PathUri::from_path(&source_file)?, + &PathUri::from_path(&copied_file)?, CopyOptions { recursive: false }, /*sandbox*/ None, ) @@ -258,8 +243,8 @@ async fn file_system_copy_copies_directory_recursively( file_system .copy( - &absolute_path(&source_dir), - &absolute_path(&copied_dir), + &PathUri::from_path(&source_dir)?, + &PathUri::from_path(&copied_dir)?, CopyOptions { recursive: true }, /*sandbox*/ None, ) @@ -288,7 +273,7 @@ async fn file_system_read_directory_lists_entries( std::fs::write(source_dir.join("root.txt"), "hello")?; let mut entries = file_system - .read_directory(&absolute_path(&source_dir), /*sandbox*/ None) + .read_directory(&PathUri::from_path(&source_dir)?, /*sandbox*/ None) .await .with_context(|| format!("mode={implementation}"))?; entries.sort_by(|left, right| left.file_name.cmp(&right.file_name)); @@ -326,7 +311,7 @@ async fn file_system_remove_removes_directory( file_system .remove( - &absolute_path(&directory_path), + &PathUri::from_path(&directory_path)?, RemoveOptions { recursive: true, force: true, @@ -354,7 +339,7 @@ async fn file_system_write_file_reports_missing_parent( let error = match file_system .write_file( - &absolute_path(&missing_parent_path), + &PathUri::from_path(&missing_parent_path)?, b"hello from trait".to_vec(), /*sandbox*/ None, ) @@ -388,8 +373,8 @@ async fn file_system_copy_rejects_directory_without_recursive( let error = file_system .copy( - &absolute_path(&source_dir), - &absolute_path(tmp.path().join("dest")), + &PathUri::from_path(&source_dir)?, + &PathUri::from_path(tmp.path().join("dest"))?, CopyOptions { recursive: false }, /*sandbox*/ None, ) @@ -424,7 +409,7 @@ async fn file_system_sandboxed_read_allows_readable_root( let sandbox = read_only_sandbox(allowed_dir); let contents = file_system - .read_file(&absolute_path(&file_path), Some(&sandbox)) + .read_file(&PathUri::from_path(&file_path)?, Some(&sandbox)) .await .with_context(|| format!("mode={implementation}"))?; assert_eq!(contents, b"sandboxed hello"); @@ -448,8 +433,8 @@ pub(crate) async fn assert_canonicalize_resolves_directory_alias( std::fs::write(&file_path, "canonical hello")?; create_directory_alias(&source_dir, &alias_dir)?; - let requested_path = absolute_path(alias_dir.join("nested").join("note.txt")); - let expected_path = absolute_path(std::fs::canonicalize(&file_path)?); + let requested_path = PathUri::from_path(alias_dir.join("nested").join("note.txt"))?; + let expected_path = PathUri::from_path(std::fs::canonicalize(&file_path)?)?; assert_ne!(requested_path, expected_path); let canonical_path = file_system @@ -478,8 +463,8 @@ pub(crate) async fn assert_sandboxed_canonicalize_resolves_directory_alias( create_directory_alias(&source_dir, &alias_dir)?; let sandbox = read_only_sandbox(tmp.path().to_path_buf()); - let requested_path = absolute_path(alias_dir.join("nested").join("note.txt")); - let expected_path = absolute_path(std::fs::canonicalize(&file_path)?); + let requested_path = PathUri::from_path(alias_dir.join("nested").join("note.txt"))?; + let expected_path = PathUri::from_path(std::fs::canonicalize(&file_path)?)?; assert_ne!(requested_path, expected_path); let canonical_path = file_system @@ -532,7 +517,7 @@ async fn file_system_sandboxed_write_allows_additional_write_root( file_system .write_file( - &absolute_path(&file_path), + &PathUri::from_path(&file_path)?, b"created".to_vec(), Some(&sandbox), ) @@ -558,8 +543,8 @@ async fn file_system_copy_rejects_copying_directory_into_descendant( let error = file_system .copy( - &absolute_path(&source_dir), - &absolute_path(source_dir.join("nested").join("copy")), + &PathUri::from_path(&source_dir)?, + &PathUri::from_path(source_dir.join("nested").join("copy"))?, CopyOptions { recursive: true }, /*sandbox*/ None, ) diff --git a/codex-rs/exec-server/tests/file_system/support.rs b/codex-rs/exec-server/tests/file_system/support.rs index 80d0cfc86..715faa996 100644 --- a/codex-rs/exec-server/tests/file_system/support.rs +++ b/codex-rs/exec-server/tests/file_system/support.rs @@ -1,5 +1,4 @@ use std::fmt; -use std::path::Path; use std::sync::Arc; use anyhow::Result; @@ -71,8 +70,7 @@ pub(crate) async fn create_file_system_context( } } -pub(crate) fn absolute_path(path: impl AsRef) -> AbsolutePathBuf { - let path = path.as_ref().to_path_buf(); +pub(crate) fn absolute_path(path: std::path::PathBuf) -> AbsolutePathBuf { assert!( path.is_absolute(), "path must be absolute: {}", diff --git a/codex-rs/exec-server/tests/file_system_unix.rs b/codex-rs/exec-server/tests/file_system_unix.rs index 054d59a39..3538f3236 100644 --- a/codex-rs/exec-server/tests/file_system_unix.rs +++ b/codex-rs/exec-server/tests/file_system_unix.rs @@ -22,6 +22,7 @@ use codex_exec_server::CreateDirectoryOptions; #[cfg(target_os = "linux")] use codex_exec_server::Environment; use codex_exec_server::RemoveOptions; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; use tempfile::TempDir; use test_case::test_case; @@ -30,7 +31,6 @@ use test_case::test_case; use crate::common::exec_server::exec_server_with_env; use crate::support::FileSystemImplementation; -use crate::support::absolute_path; use crate::support::create_file_system_context; use crate::support::read_only_sandbox; use crate::support::workspace_write_sandbox; @@ -185,7 +185,7 @@ async fn sandboxed_file_system_helper_finds_bwrap_on_preserved_path() -> Result< file_system .write_file( - &absolute_path(&file_path), + &PathUri::from_path(&file_path)?, b"written through fs helper".to_vec(), Some(&sandbox), ) @@ -219,7 +219,7 @@ async fn file_system_get_metadata_reports_symlink_targets( let symlink_path = tmp.path().join("note-link.txt"); symlink(&file_path, &symlink_path)?; let symlink_metadata = file_system - .get_metadata(&absolute_path(&symlink_path), /*sandbox*/ None) + .get_metadata(&PathUri::from_path(&symlink_path)?, /*sandbox*/ None) .await .with_context(|| format!("mode={implementation}"))?; assert_eq!(symlink_metadata.is_directory, false); @@ -232,7 +232,10 @@ async fn file_system_get_metadata_reports_symlink_targets( let dir_symlink_path = tmp.path().join("notes-link"); symlink(&dir_path, &dir_symlink_path)?; let dir_symlink_metadata = file_system - .get_metadata(&absolute_path(&dir_symlink_path), /*sandbox*/ None) + .get_metadata( + &PathUri::from_path(&dir_symlink_path)?, + /*sandbox*/ None, + ) .await .with_context(|| format!("mode={implementation}"))?; assert_eq!(dir_symlink_metadata.is_directory, true); @@ -257,7 +260,7 @@ async fn file_system_sandboxed_write_rejects_unwritable_path( let sandbox = read_only_sandbox(tmp.path().to_path_buf()); let error = match file_system .write_file( - &absolute_path(&blocked_path), + &PathUri::from_path(&blocked_path)?, b"nope".to_vec(), Some(&sandbox), ) @@ -293,7 +296,7 @@ async fn file_system_sandboxed_write_allows_explicit_alias_roots( file_system .write_file( - &absolute_path(&file_path), + &PathUri::from_path(&file_path)?, b"created".to_vec(), Some(&sandbox), ) @@ -324,7 +327,7 @@ async fn file_system_sandboxed_read_rejects_symlink_escape( let requested_path = allowed_dir.join("link").join("secret.txt"); let sandbox = read_only_sandbox(allowed_dir); let error = match file_system - .read_file(&absolute_path(&requested_path), Some(&sandbox)) + .read_file(&PathUri::from_path(&requested_path)?, Some(&sandbox)) .await { Ok(_) => anyhow::bail!("read should be blocked"), @@ -353,13 +356,14 @@ async fn file_system_sandboxed_read_rejects_symlink_parent_dotdot_escape( std::fs::write(&secret_path, "nope")?; symlink(&outside_dir, allowed_dir.join("link"))?; - let requested_path = absolute_path(allowed_dir.join("link").join("..").join("secret.txt")); + let requested_path = + PathUri::from_path(allowed_dir.join("link").join("..").join("secret.txt"))?; let sandbox = read_only_sandbox(allowed_dir); let error = match file_system.read_file(&requested_path, Some(&sandbox)).await { Ok(_) => anyhow::bail!("read should fail after path normalization"), Err(error) => error, }; - // AbsolutePathBuf normalizes `link/../secret.txt` to + // PathUri's native path constructor normalizes `link/../secret.txt` to // `allowed/secret.txt` before the request reaches the filesystem layer. // Depending on whether the platform/runtime resolves that normalized path // through a top-level symlink alias, the request can surface as either @@ -389,7 +393,7 @@ async fn file_system_sandboxed_write_rejects_symlink_escape( let sandbox = workspace_write_sandbox(allowed_dir); let error = match file_system .write_file( - &absolute_path(&requested_path), + &PathUri::from_path(&requested_path)?, b"nope".to_vec(), Some(&sandbox), ) @@ -427,7 +431,7 @@ async fn file_system_sandboxed_write_preserves_existing_hard_link( let sandbox = workspace_write_sandbox(allowed_dir); file_system .write_file( - &absolute_path(&hard_link), + &PathUri::from_path(&hard_link)?, b"updated through existing hard link\n".to_vec(), Some(&sandbox), ) @@ -473,7 +477,7 @@ async fn file_system_create_directory_rejects_symlink_escape( let sandbox = workspace_write_sandbox(allowed_dir); let error = match file_system .create_directory( - &absolute_path(&requested_path), + &PathUri::from_path(&requested_path)?, CreateDirectoryOptions { recursive: false }, Some(&sandbox), ) @@ -508,7 +512,7 @@ async fn file_system_read_directory_rejects_symlink_escape( let requested_path = allowed_dir.join("link"); let sandbox = read_only_sandbox(allowed_dir); let error = match file_system - .read_directory(&absolute_path(&requested_path), Some(&sandbox)) + .read_directory(&PathUri::from_path(&requested_path)?, Some(&sandbox)) .await { Ok(_) => anyhow::bail!("read_directory should be blocked"), @@ -540,8 +544,8 @@ async fn file_system_copy_rejects_symlink_escape_destination( let sandbox = workspace_write_sandbox(allowed_dir.clone()); let error = match file_system .copy( - &absolute_path(allowed_dir.join("source.txt")), - &absolute_path(&requested_destination), + &PathUri::from_path(allowed_dir.join("source.txt"))?, + &PathUri::from_path(&requested_destination)?, CopyOptions { recursive: false }, Some(&sandbox), ) @@ -578,7 +582,7 @@ async fn file_system_remove_removes_symlink_not_target( let sandbox = workspace_write_sandbox(allowed_dir); file_system .remove( - &absolute_path(&symlink_path), + &PathUri::from_path(&symlink_path)?, RemoveOptions { recursive: false, force: false, @@ -618,8 +622,8 @@ async fn file_system_copy_preserves_symlink_source( let sandbox = workspace_write_sandbox(allowed_dir.clone()); file_system .copy( - &absolute_path(&source_symlink), - &absolute_path(&copied_symlink), + &PathUri::from_path(&source_symlink)?, + &PathUri::from_path(&copied_symlink)?, CopyOptions { recursive: false }, Some(&sandbox), ) @@ -655,7 +659,7 @@ async fn file_system_remove_rejects_symlink_escape( let sandbox = workspace_write_sandbox(allowed_dir); let error = match file_system .remove( - &absolute_path(&requested_path), + &PathUri::from_path(&requested_path)?, RemoveOptions { recursive: false, force: false, @@ -696,8 +700,8 @@ async fn file_system_copy_rejects_symlink_escape_source( let sandbox = workspace_write_sandbox(allowed_dir); let error = match file_system .copy( - &absolute_path(&requested_source), - &absolute_path(&requested_destination), + &PathUri::from_path(&requested_source)?, + &PathUri::from_path(&requested_destination)?, CopyOptions { recursive: false }, Some(&sandbox), ) @@ -730,8 +734,8 @@ async fn file_system_copy_preserves_symlinks_in_recursive_copy( file_system .copy( - &absolute_path(&source_dir), - &absolute_path(&copied_dir), + &PathUri::from_path(&source_dir)?, + &PathUri::from_path(&copied_dir)?, CopyOptions { recursive: true }, /*sandbox*/ None, ) @@ -776,8 +780,8 @@ async fn file_system_copy_ignores_unknown_special_files_in_recursive_copy( file_system .copy( - &absolute_path(&source_dir), - &absolute_path(&copied_dir), + &PathUri::from_path(&source_dir)?, + &PathUri::from_path(&copied_dir)?, CopyOptions { recursive: true }, /*sandbox*/ None, ) @@ -815,8 +819,8 @@ async fn file_system_copy_rejects_standalone_fifo_source( let error = file_system .copy( - &absolute_path(&fifo_path), - &absolute_path(tmp.path().join("copied")), + &PathUri::from_path(&fifo_path)?, + &PathUri::from_path(tmp.path().join("copied"))?, CopyOptions { recursive: false }, /*sandbox*/ None, ) diff --git a/codex-rs/ext/skills/Cargo.toml b/codex-rs/ext/skills/Cargo.toml index 0a30dddb9..cc8dcb114 100644 --- a/codex-rs/ext/skills/Cargo.toml +++ b/codex-rs/ext/skills/Cargo.toml @@ -21,6 +21,7 @@ codex-mcp = { workspace = true } codex-protocol = { workspace = true } codex-tools = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } codex-utils-string = { workspace = true } schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } diff --git a/codex-rs/ext/skills/src/provider/executor.rs b/codex-rs/ext/skills/src/provider/executor.rs index d16b2b933..1ed1d11fa 100644 --- a/codex-rs/ext/skills/src/provider/executor.rs +++ b/codex-rs/ext/skills/src/provider/executor.rs @@ -10,6 +10,7 @@ use codex_protocol::capabilities::CapabilityRootLocation; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use crate::catalog::SkillAuthority; use crate::catalog::SkillCatalog; @@ -130,9 +131,15 @@ impl SkillProvider for ExecutorSkillProvider { "executor skill resource references unavailable environment `{environment_id}`" ))); }; + let resource_path = PathUri::from_abs_path(resource_path).map_err(|err| { + SkillProviderError::new(format!( + "failed to read executor skill resource {}: {err}", + request.resource.as_str() + )) + })?; let contents = environment .get_filesystem() - .read_file_text(resource_path, /*sandbox*/ None) + .read_file_text(&resource_path, /*sandbox*/ None) .await .map_err(|err| { SkillProviderError::new(format!( 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 1b2cb27e0..5b9a440d8 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; @@ -24,6 +23,7 @@ use codex_skills_extension::ExecutorSkillProvider; use codex_skills_extension::provider::SkillListQuery; use codex_skills_extension::provider::SkillProvider; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; const SKILL_CONTENTS: &str = @@ -60,34 +60,23 @@ impl SyntheticFileSystem { impl ExecutorFileSystem for SyntheticFileSystem { async fn canonicalize( &self, - path: &AbsolutePathBuf, + path: &PathUri, _sandbox: Option<&FileSystemSandboxContext>, - ) -> FileSystemResult { - if path == &self.alias_root { - return Ok(self.canonical_root.clone()); + ) -> FileSystemResult { + let path = path.to_abs_path()?; + if path == self.alias_root { + return PathUri::from_abs_path(&self.canonical_root); } - self.metadata(path)?; - 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()) + self.metadata(&path)?; + PathUri::from_abs_path(&path) } async fn read_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { - if path == &self.canonical_root.join("skill/SKILL.md") { + if path.to_abs_path()? == self.canonical_root.join("skill/SKILL.md") { Ok(SKILL_CONTENTS.as_bytes().to_vec()) } else { Err(io::Error::new(io::ErrorKind::NotFound, "not found")) @@ -96,7 +85,7 @@ impl ExecutorFileSystem for SyntheticFileSystem { async fn write_file( &self, - _path: &AbsolutePathBuf, + _path: &PathUri, _contents: Vec, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -105,7 +94,7 @@ impl ExecutorFileSystem for SyntheticFileSystem { async fn create_directory( &self, - _path: &AbsolutePathBuf, + _path: &PathUri, _options: CreateDirectoryOptions, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -114,24 +103,25 @@ impl ExecutorFileSystem for SyntheticFileSystem { async fn get_metadata( &self, - path: &AbsolutePathBuf, + path: &PathUri, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult { - self.metadata(path) + self.metadata(&path.to_abs_path()?) } async fn read_directory( &self, - path: &AbsolutePathBuf, + path: &PathUri, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult> { - if path == &self.canonical_root { + let path = path.to_abs_path()?; + if path == self.canonical_root { Ok(vec![ReadDirectoryEntry { file_name: "skill".to_string(), is_directory: true, is_file: false, }]) - } else if path == &self.canonical_root.join("skill") { + } else if path == self.canonical_root.join("skill") { Ok(vec![ReadDirectoryEntry { file_name: "SKILL.md".to_string(), is_directory: false, @@ -144,7 +134,7 @@ impl ExecutorFileSystem for SyntheticFileSystem { async fn remove( &self, - _path: &AbsolutePathBuf, + _path: &PathUri, _options: RemoveOptions, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { @@ -153,8 +143,8 @@ impl ExecutorFileSystem for SyntheticFileSystem { async fn copy( &self, - _source_path: &AbsolutePathBuf, - _destination_path: &AbsolutePathBuf, + _source_path: &PathUri, + _destination_path: &PathUri, _options: CopyOptions, _sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()> { diff --git a/codex-rs/file-system/Cargo.toml b/codex-rs/file-system/Cargo.toml index 85e083567..bae752757 100644 --- a/codex-rs/file-system/Cargo.toml +++ b/codex-rs/file-system/Cargo.toml @@ -11,6 +11,7 @@ workspace = true async-trait = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } serde = { workspace = true, features = ["derive"] } [lib] diff --git a/codex-rs/file-system/src/lib.rs b/codex-rs/file-system/src/lib.rs index 8fad1f5b6..95537402a 100644 --- a/codex-rs/file-system/src/lib.rs +++ b/codex-rs/file-system/src/lib.rs @@ -9,6 +9,7 @@ use codex_protocol::permissions::FileSystemSpecialPath; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use std::io; use std::path::Path; @@ -136,30 +137,20 @@ pub trait ExecutorFileSystem: Send + Sync { /// Resolves a path within this filesystem. async fn canonicalize( &self, - path: &AbsolutePathBuf, + path: &PathUri, 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>; + ) -> FileSystemResult; async fn read_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult>; /// Reads a file and decodes it as UTF-8 text. async fn read_file_text( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult { let bytes = self.read_file(path, sandbox).await?; @@ -168,41 +159,41 @@ pub trait ExecutorFileSystem: Send + Sync { async fn write_file( &self, - path: &AbsolutePathBuf, + path: &PathUri, contents: Vec, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()>; async fn create_directory( &self, - path: &AbsolutePathBuf, + path: &PathUri, create_directory_options: CreateDirectoryOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()>; async fn get_metadata( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult; async fn read_directory( &self, - path: &AbsolutePathBuf, + path: &PathUri, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult>; async fn remove( &self, - path: &AbsolutePathBuf, + path: &PathUri, remove_options: RemoveOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()>; async fn copy( &self, - source_path: &AbsolutePathBuf, - destination_path: &AbsolutePathBuf, + source_path: &PathUri, + destination_path: &PathUri, copy_options: CopyOptions, sandbox: Option<&FileSystemSandboxContext>, ) -> FileSystemResult<()>; diff --git a/codex-rs/git-utils/Cargo.toml b/codex-rs/git-utils/Cargo.toml index 9f9c6d0c1..18ba48825 100644 --- a/codex-rs/git-utils/Cargo.toml +++ b/codex-rs/git-utils/Cargo.toml @@ -14,6 +14,7 @@ chrono = { workspace = true } codex-file-system = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } futures = { workspace = true, features = ["alloc"] } gix = { workspace = true } once_cell = { workspace = true } diff --git a/codex-rs/git-utils/src/info.rs b/codex-rs/git-utils/src/info.rs index 7c0d59462..beff18b51 100644 --- a/codex-rs/git-utils/src/info.rs +++ b/codex-rs/git-utils/src/info.rs @@ -6,6 +6,7 @@ use std::path::PathBuf; use codex_file_system::ExecutorFileSystem; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use futures::future::join_all; use schemars::JsonSchema; use serde::Deserialize; @@ -46,7 +47,8 @@ pub async fn get_git_repo_root_with_fs( fs: &dyn ExecutorFileSystem, cwd: &AbsolutePathBuf, ) -> Option { - let base = match fs.get_metadata(cwd, /*sandbox*/ None).await { + let cwd_uri = PathUri::from_abs_path(cwd).ok()?; + let base = match fs.get_metadata(&cwd_uri, /*sandbox*/ None).await { Ok(metadata) if metadata.is_directory => cwd.clone(), _ => cwd.parent()?, }; @@ -803,8 +805,9 @@ pub async fn resolve_root_git_project_for_trust( ) -> Option { let repo_root = get_git_repo_root_with_fs(fs, cwd).await?; let dot_git = repo_root.join(".git"); + let dot_git_uri = PathUri::from_abs_path(&dot_git).ok()?; if fs - .get_metadata(&dot_git, /*sandbox*/ None) + .get_metadata(&dot_git_uri, /*sandbox*/ None) .await .ok()? .is_directory @@ -812,7 +815,10 @@ pub async fn resolve_root_git_project_for_trust( return Some(repo_root); } - let git_dir_s = fs.read_file_text(&dot_git, /*sandbox*/ None).await.ok()?; + let git_dir_s = fs + .read_file_text(&dot_git_uri, /*sandbox*/ None) + .await + .ok()?; let git_dir_rel = git_dir_s.trim().strip_prefix("gitdir:")?.trim(); if git_dir_rel.is_empty() { return None; @@ -853,7 +859,12 @@ async fn find_ancestor_git_entry_with_fs( ) -> Option<(AbsolutePathBuf, AbsolutePathBuf)> { for dir in base_dir.ancestors() { let dot_git = dir.join(".git"); - if fs.get_metadata(&dot_git, /*sandbox*/ None).await.is_ok() { + let dot_git_uri = PathUri::from_abs_path(&dot_git).ok()?; + if fs + .get_metadata(&dot_git_uri, /*sandbox*/ None) + .await + .is_ok() + { return Some((dir, dot_git)); } } diff --git a/codex-rs/utils/plugins/Cargo.toml b/codex-rs/utils/plugins/Cargo.toml index 983f4526e..2c56ab377 100644 --- a/codex-rs/utils/plugins/Cargo.toml +++ b/codex-rs/utils/plugins/Cargo.toml @@ -17,6 +17,7 @@ workspace = true codex-exec-server = { workspace = true } codex-login = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } diff --git a/codex-rs/utils/plugins/src/plugin_namespace.rs b/codex-rs/utils/plugins/src/plugin_namespace.rs index ea5caab52..d0d8bf685 100644 --- a/codex-rs/utils/plugins/src/plugin_namespace.rs +++ b/codex-rs/utils/plugins/src/plugin_namespace.rs @@ -2,6 +2,7 @@ use codex_exec_server::ExecutorFileSystem; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; use std::path::Path; use std::path::PathBuf; @@ -29,7 +30,8 @@ async fn plugin_manifest_name( let mut manifest_path = None; for relative_path in DISCOVERABLE_PLUGIN_MANIFEST_PATHS { let candidate = plugin_root.join(relative_path); - match fs.get_metadata(&candidate, /*sandbox*/ None).await { + let candidate_uri = PathUri::from_abs_path(&candidate).ok()?; + match fs.get_metadata(&candidate_uri, /*sandbox*/ None).await { Ok(metadata) if metadata.is_file => { manifest_path = Some(candidate); break; @@ -38,8 +40,9 @@ async fn plugin_manifest_name( } } let manifest_path = manifest_path?; + let manifest_path_uri = PathUri::from_abs_path(&manifest_path).ok()?; let contents = fs - .read_file_text(&manifest_path, /*sandbox*/ None) + .read_file_text(&manifest_path_uri, /*sandbox*/ None) .await .ok()?; let RawPluginManifestName { name: raw_name } = serde_json::from_str(&contents).ok()?;