mirror of
https://github.com/openai/codex.git
synced 2026-10-21 17:48:44 +00:00
Migrate ExecutorFileSystem canonicalize to PathUri
This commit is contained in:
Generated
+4
@@ -2515,6 +2515,7 @@ dependencies = [
|
||||
"codex-protocol",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-path",
|
||||
"codex-utils-path-uri",
|
||||
"core-foundation 0.9.4",
|
||||
"dns-lookup",
|
||||
"dunce",
|
||||
@@ -2768,6 +2769,7 @@ dependencies = [
|
||||
"codex-skills",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-output-truncation",
|
||||
"codex-utils-path-uri",
|
||||
"codex-utils-plugins",
|
||||
"dirs",
|
||||
"dunce",
|
||||
@@ -3005,6 +3007,7 @@ dependencies = [
|
||||
"async-trait",
|
||||
"codex-protocol",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-path-uri",
|
||||
"serde",
|
||||
]
|
||||
|
||||
@@ -3821,6 +3824,7 @@ dependencies = [
|
||||
"codex-extension-api",
|
||||
"codex-protocol",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-path-uri",
|
||||
"codex-utils-string",
|
||||
"pretty_assertions",
|
||||
"tokio",
|
||||
|
||||
@@ -62,6 +62,7 @@ windows-sys = { version = "0.52", features = [
|
||||
] }
|
||||
|
||||
[dev-dependencies]
|
||||
codex-utils-path-uri = { workspace = true }
|
||||
pretty_assertions = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
tokio = { workspace = true, features = ["full"] }
|
||||
|
||||
@@ -7,6 +7,7 @@ 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 tempfile::tempdir;
|
||||
|
||||
@@ -16,10 +17,12 @@ struct TestFileSystem;
|
||||
impl ExecutorFileSystem for TestFileSystem {
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
path: &PathUri,
|
||||
_sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
path.canonicalize()
|
||||
) -> FileSystemResult<PathUri> {
|
||||
let path = path.to_abs_path()?;
|
||||
let canonicalized = path.canonicalize()?;
|
||||
PathUri::from_abs_path(&canonicalized)
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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;
|
||||
@@ -475,8 +476,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())
|
||||
}
|
||||
|
||||
|
||||
@@ -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 }
|
||||
@@ -52,7 +53,6 @@ uuid = { workspace = true, features = ["v4"] }
|
||||
[dev-dependencies]
|
||||
anyhow = { workspace = true }
|
||||
codex-test-binary-support = { workspace = true }
|
||||
codex-utils-path-uri = { workspace = true }
|
||||
ctor = { workspace = true }
|
||||
http = { workspace = true }
|
||||
pretty_assertions = { workspace = true }
|
||||
|
||||
@@ -238,10 +238,13 @@ 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,
|
||||
}))
|
||||
@@ -305,23 +308,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(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,9 +82,9 @@ impl LocalFileSystem {
|
||||
impl ExecutorFileSystem for LocalFileSystem {
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
path: &PathUri,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
) -> FileSystemResult<PathUri> {
|
||||
let (file_system, sandbox) = self.file_system_for(sandbox)?;
|
||||
file_system.canonicalize(path, sandbox).await
|
||||
}
|
||||
@@ -163,9 +164,9 @@ impl ExecutorFileSystem for LocalFileSystem {
|
||||
impl ExecutorFileSystem for UnsandboxedFileSystem {
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
path: &PathUri,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
) -> FileSystemResult<PathUri> {
|
||||
reject_platform_sandbox_context(sandbox)?;
|
||||
self.file_system.canonicalize(path, /*sandbox*/ None).await
|
||||
}
|
||||
@@ -258,11 +259,14 @@ impl ExecutorFileSystem for UnsandboxedFileSystem {
|
||||
impl ExecutorFileSystem for DirectFileSystem {
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
path: &PathUri,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
) -> FileSystemResult<PathUri> {
|
||||
reject_sandbox_context(sandbox)?;
|
||||
AbsolutePathBuf::from_absolute_path(tokio::fs::canonicalize(path.as_path()).await?)
|
||||
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(
|
||||
|
||||
@@ -2,6 +2,7 @@ use async_trait::async_trait;
|
||||
use base64::Engine as _;
|
||||
use base64::engine::general_purpose::STANDARD;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use tokio::io;
|
||||
use tracing::trace;
|
||||
|
||||
@@ -42,19 +43,20 @@ impl RemoteFileSystem {
|
||||
impl ExecutorFileSystem for RemoteFileSystem {
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
path: &PathUri,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
) -> FileSystemResult<PathUri> {
|
||||
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)
|
||||
PathUri::from_abs_path(&response.path)
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
|
||||
@@ -3,6 +3,7 @@ use base64::Engine as _;
|
||||
use base64::engine::general_purpose::STANDARD;
|
||||
use codex_app_server_protocol::JSONRPCErrorError;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use tokio::io;
|
||||
|
||||
use crate::CopyOptions;
|
||||
@@ -54,22 +55,22 @@ impl SandboxedFileSystem {
|
||||
impl ExecutorFileSystem for SandboxedFileSystem {
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
path: &PathUri,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
) -> FileSystemResult<PathUri> {
|
||||
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)
|
||||
PathUri::from_abs_path(&response.path)
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
|
||||
@@ -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;
|
||||
@@ -116,11 +117,13 @@ impl FileSystemHandler {
|
||||
&self,
|
||||
params: FsCanonicalizeParams,
|
||||
) -> Result<FsCanonicalizeResponse, JSONRPCErrorError> {
|
||||
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 })
|
||||
}
|
||||
|
||||
@@ -256,6 +259,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,
|
||||
|
||||
@@ -434,8 +434,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
|
||||
@@ -464,8 +464,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
|
||||
|
||||
@@ -24,5 +24,6 @@ codex-utils-string = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
async-trait = { workspace = true }
|
||||
codex-utils-path-uri = { workspace = true }
|
||||
pretty_assertions = { workspace = true }
|
||||
tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
|
||||
|
||||
@@ -23,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 =
|
||||
@@ -59,14 +60,15 @@ impl SyntheticFileSystem {
|
||||
impl ExecutorFileSystem for SyntheticFileSystem {
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
path: &PathUri,
|
||||
_sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
if path == &self.alias_root {
|
||||
return Ok(self.canonical_root.clone());
|
||||
) -> FileSystemResult<PathUri> {
|
||||
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())
|
||||
self.metadata(&path)?;
|
||||
PathUri::from_abs_path(&path)
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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,9 +137,9 @@ pub trait ExecutorFileSystem: Send + Sync {
|
||||
/// Resolves a path within this filesystem.
|
||||
async fn canonicalize(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
path: &PathUri,
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf>;
|
||||
) -> FileSystemResult<PathUri>;
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
|
||||
Reference in New Issue
Block a user