From 0e10077fb2f06cafe65d1dc8267349a8e5ef55f6 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sat, 13 Jun 2026 20:56:51 +0000 Subject: [PATCH] path-uri: clarify API path string semantics --- codex-rs/exec-server/src/environment.rs | 25 ----- codex-rs/exec-server/src/protocol.rs | 39 -------- .../utils/path-uri/src/api_path_string.rs | 26 +++++ .../path-uri/src/api_path_string_tests.rs | 96 +++++++++++++++++-- 4 files changed, 113 insertions(+), 73 deletions(-) diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index eda4924e2f..688c421dd3 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -26,7 +26,6 @@ use crate::protocol::ShellInfo; use crate::remote_file_system::RemoteFileSystem; use crate::remote_process::RemoteProcess; use codex_shell_command::shell_detect::DetectedShell; -use codex_utils_path_uri::PathConvention; pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; @@ -474,7 +473,6 @@ impl EnvironmentInfo { pub(crate) fn local() -> Self { Self { shell: codex_shell_command::shell_detect::default_user_shell().into(), - path_convention: PathConvention::native(), } } } @@ -496,14 +494,10 @@ mod tests { use super::EnvironmentManager; use super::LOCAL_ENVIRONMENT_ID; use super::REMOTE_ENVIRONMENT_ID; - use crate::EnvironmentInfo; use crate::ExecServerRuntimePaths; use crate::ProcessId; - use crate::ShellInfo; use crate::environment_provider::EnvironmentDefault; use crate::environment_provider::EnvironmentProviderSnapshot; - use codex_utils_path_uri::PathConvention; - use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; fn test_runtime_paths() -> ExecServerRuntimePaths { @@ -518,25 +512,6 @@ mod tests { assert!(manager.try_local_environment().is_none()); } - #[test] - fn environment_info_renders_paths_using_its_convention() { - let info = EnvironmentInfo { - shell: ShellInfo { - name: "powershell".to_string(), - path: "powershell.exe".to_string(), - }, - path_convention: PathConvention::Windows, - }; - let path = PathUri::parse("file:///C:/workspace/src/main.rs").expect("valid file URI"); - - assert_eq!( - info.render_path(&path) - .expect("Windows path should render") - .into_string(), - r"C:\workspace\src\main.rs" - ); - } - #[tokio::test] async fn create_local_environment_does_not_connect() { let environment = Environment::create(/*exec_server_url*/ None, test_runtime_paths()) diff --git a/codex-rs/exec-server/src/protocol.rs b/codex-rs/exec-server/src/protocol.rs index 2c58f1cdcf..175d23fe0a 100644 --- a/codex-rs/exec-server/src/protocol.rs +++ b/codex-rs/exec-server/src/protocol.rs @@ -4,9 +4,6 @@ use std::path::PathBuf; use crate::FileSystemSandboxContext; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use codex_protocol::config_types::ShellEnvironmentPolicyInherit; -use codex_utils_path_uri::ApiPathString; -use codex_utils_path_uri::ApiPathStringError; -use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; use serde::Deserialize; use serde::Serialize; @@ -72,18 +69,6 @@ pub struct InitializeResponse { #[serde(rename_all = "camelCase")] pub struct EnvironmentInfo { pub shell: ShellInfo, - /// Native path syntax used by this environment. - pub path_convention: PathConvention, -} - -impl EnvironmentInfo { - /// Renders a path using this environment's native path syntax. - /// - /// TODO(anp): Once `PathUri` carries an environment identifier, invert this - /// relationship and use the URI to resolve the environment information. - pub fn render_path(&self, path: &PathUri) -> Result { - ApiPathString::from_path_uri(path, self.path_convention) - } } /// Shell detected for an execution/filesystem environment. @@ -464,13 +449,10 @@ mod base64_bytes { #[cfg(test)] mod tests { - use super::EnvironmentInfo; use super::FsReadFileParams; use super::HttpRequestParams; - use super::ShellInfo; use crate::FileSystemSandboxContext; use codex_protocol::models::PermissionProfile; - use codex_utils_path_uri::PathConvention; use codex_utils_path_uri::PathUri; use pretty_assertions::assert_eq; @@ -508,27 +490,6 @@ mod tests { ); } - #[test] - fn environment_info_serializes_path_convention() { - let current = EnvironmentInfo { - shell: ShellInfo { - name: "powershell".to_string(), - path: "powershell.exe".to_string(), - }, - path_convention: PathConvention::Windows, - }; - assert_eq!( - serde_json::to_value(current).expect("environment info should serialize"), - serde_json::json!({ - "shell": { - "name": "powershell", - "path": "powershell.exe", - }, - "pathConvention": "windows", - }) - ); - } - #[test] fn http_request_timeout_treats_omitted_and_null_as_no_timeout() { let omitted: HttpRequestParams = serde_json::from_value(serde_json::json!({ diff --git a/codex-rs/utils/path-uri/src/api_path_string.rs b/codex-rs/utils/path-uri/src/api_path_string.rs index 84222c6eb2..6b377cad4b 100644 --- a/codex-rs/utils/path-uri/src/api_path_string.rs +++ b/codex-rs/utils/path-uri/src/api_path_string.rs @@ -51,7 +51,12 @@ impl fmt::Display for PathConvention { /// accidentally applying the current host's path rules. Opaque fallback paths /// are decoded using the supplied convention and converted to UTF-8 lossily at /// this API boundary because the value is serialized as a JSON string. +/// +/// Deserialization accepts any UTF-8 string without interpreting or validating +/// it. Relative path text remains valid until an operation such as +/// [`Self::to_path_uri`] requires an absolute path. #[derive(Clone, Debug, PartialEq, Eq, Hash, Deserialize, TS)] +#[serde(transparent)] #[ts(type = "string")] pub struct ApiPathString(String); @@ -99,6 +104,27 @@ impl ApiPathString { }) } + /// Infers the path convention of an absolute API path from its spelling. + /// + /// Relative paths and ambiguous spellings return `None`. In particular, + /// slash-prefixed paths are treated as POSIX even when they could also be + /// interpreted as slash-delimited Windows UNC paths. + pub fn infer_absolute_path_convention(&self) -> Option { + let bytes = self.0.as_bytes(); + let has_windows_drive_root = matches!( + bytes, + [drive, b':', separator, ..] + if drive.is_ascii_alphabetic() && is_windows_separator_byte(*separator) + ); + if has_windows_drive_root || self.0.starts_with(r"\\") { + Some(PathConvention::Windows) + } else if self.0.starts_with('/') { + Some(PathConvention::Posix) + } else { + None + } + } + pub fn as_str(&self) -> &str { &self.0 } diff --git a/codex-rs/utils/path-uri/src/api_path_string_tests.rs b/codex-rs/utils/path-uri/src/api_path_string_tests.rs index c123568889..fab687f667 100644 --- a/codex-rs/utils/path-uri/src/api_path_string_tests.rs +++ b/codex-rs/utils/path-uri/src/api_path_string_tests.rs @@ -315,15 +315,20 @@ fn renders_native_paths_from_shared_cases() { }) } }; + let actual = ApiPathString::from_path_uri(&path, case.convention); - assert_eq!( - ApiPathString::from_path_uri(&path, case.convention), - expected, - "rendering {case:?}" - ); + assert_eq!(actual, expected, "rendering {case:?}"); + if let Ok(rendered) = &actual { + assert_eq!( + rendered.infer_absolute_path_convention(), + Some(case.convention), + "inferring {case:?}" + ); + } if let RenderExpectation::RoundTrip(rendered) = case.expected { - let api_path = ApiPathString(rendered.to_string()); + let api_path = serde_json::from_value::(serde_json::json!(rendered)) + .expect("native path should deserialize from API text"); let reparsed = api_path .to_path_uri(case.convention) .expect("native path should parse using its convention"); @@ -338,14 +343,44 @@ fn renders_native_paths_from_shared_cases() { } #[test] -fn rejects_relative_api_paths() { +fn relative_api_path_serializes_and_deserializes_unchanged() { + for raw_path in [".", "subdir", "subdir/file.rs"] { + let path = serde_json::from_value::(serde_json::json!(raw_path)) + .expect("relative API path should deserialize"); + + assert_eq!( + serde_json::to_value(path).expect("relative API path should serialize"), + serde_json::json!(raw_path) + ); + } +} + +#[test] +fn relative_api_path_is_invalid_when_converted_to_a_path_uri() { + let raw_path = "subdir"; + let path = serde_json::from_value::(serde_json::json!(raw_path)) + .expect("relative API path should deserialize"); + + assert_eq!(path.infer_absolute_path_convention(), None); + assert_eq!( + path.to_path_uri(PathConvention::Posix), + Err(ApiPathStringError::InvalidNativePath { + path: raw_path.to_string(), + convention: PathConvention::Posix, + }) + ); +} + +#[test] +fn other_non_absolute_api_paths_cannot_be_converted_to_path_uris() { for (raw_path, convention) in [ - ("workspace/file.rs", PathConvention::Posix), (r"workspace\file.rs", PathConvention::Windows), (r"C:file.rs", PathConvention::Windows), ] { - let path = ApiPathString(raw_path.to_string()); + let path = serde_json::from_value::(serde_json::json!(raw_path)) + .expect("API path should deserialize without validation"); + assert_eq!(path.infer_absolute_path_convention(), None); assert_eq!( path.to_path_uri(convention), Err(ApiPathStringError::InvalidNativePath { @@ -356,6 +391,49 @@ fn rejects_relative_api_paths() { } } +#[test] +fn infers_absolute_path_conventions_from_api_text() { + for (raw_path, expected) in [ + (r"C:\workspace\file.rs", Some(PathConvention::Windows)), + ("c:/workspace/file.rs", Some(PathConvention::Windows)), + (r"\\server\share\file.rs", Some(PathConvention::Windows)), + (r"\\?\C:\workspace\file.rs", Some(PathConvention::Windows)), + (r"\\.\COM1", Some(PathConvention::Windows)), + ("/workspace/file.rs", Some(PathConvention::Posix)), + ("/C:/workspace/file.rs", Some(PathConvention::Posix)), + ("//server/share/file.rs", Some(PathConvention::Posix)), + ("", None), + (".", None), + ("subdir/file.rs", None), + (r"subdir\file.rs", None), + (r"C:file.rs", None), + (r"\rooted-without-drive", None), + ] { + let path = serde_json::from_value::(serde_json::json!(raw_path)) + .expect("API path should deserialize without validation"); + + assert_eq!( + path.infer_absolute_path_convention(), + expected, + "inferring {raw_path:?}" + ); + } +} + +#[test] +fn foreign_absolute_syntax_deserializes_without_host_interpretation() { + for (raw_path, convention) in [ + (r"C:\workspace\file.rs", PathConvention::Windows), + ("/workspace/file.rs", PathConvention::Posix), + ] { + let path = serde_json::from_value::(serde_json::json!(raw_path)) + .expect("foreign API path should deserialize"); + + assert_eq!(path.as_str(), raw_path); + assert_eq!(path.infer_absolute_path_convention(), Some(convention)); + } +} + #[test] fn renders_an_absolute_path_using_the_explicit_convention() { #[cfg(unix)]