mirror of
https://github.com/openai/codex.git
synced 2026-10-26 18:38:47 +00:00
path-uri: clarify API path string semantics
This commit is contained in:
@@ -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())
|
||||
|
||||
@@ -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, ApiPathStringError> {
|
||||
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!({
|
||||
|
||||
@@ -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<PathConvention> {
|
||||
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
|
||||
}
|
||||
|
||||
@@ -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::<ApiPathString>(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::<ApiPathString>(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::<ApiPathString>(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::<ApiPathString>(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::<ApiPathString>(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::<ApiPathString>(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)]
|
||||
|
||||
Reference in New Issue
Block a user