mirror of
https://github.com/openai/codex.git
synced 2026-09-06 15:29:32 +00:00
Remove ExecutorFileSystem join and parent
This commit is contained in:
1
codex-rs/Cargo.lock
generated
1
codex-rs/Cargo.lock
generated
@@ -2848,6 +2848,7 @@ dependencies = [
|
||||
"codex-shell-command",
|
||||
"codex-test-binary-support",
|
||||
"codex-utils-absolute-path",
|
||||
"codex-utils-path-uri",
|
||||
"codex-utils-pty",
|
||||
"codex-utils-rustls-provider",
|
||||
"ctor 0.6.3",
|
||||
|
||||
@@ -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" }
|
||||
|
||||
@@ -8,7 +8,6 @@ use codex_file_system::FileSystemSandboxContext;
|
||||
use codex_file_system::ReadDirectoryEntry;
|
||||
use codex_file_system::RemoveOptions;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::Path;
|
||||
use tempfile::tempdir;
|
||||
|
||||
struct TestFileSystem;
|
||||
@@ -23,18 +22,6 @@ impl ExecutorFileSystem for TestFileSystem {
|
||||
path.canonicalize()
|
||||
}
|
||||
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
Ok(base_path.join(path))
|
||||
}
|
||||
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
|
||||
Ok(path.parent())
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
|
||||
@@ -52,6 +52,7 @@ uuid = { workspace = true, features = ["v4"] }
|
||||
[dev-dependencies]
|
||||
anyhow = { workspace = true }
|
||||
codex-test-binary-support = { workspace = true }
|
||||
codex-utils-path-uri = { workspace = true }
|
||||
ctor = { workspace = true }
|
||||
http = { workspace = true }
|
||||
pretty_assertions = { workspace = true }
|
||||
|
||||
@@ -88,18 +88,6 @@ impl ExecutorFileSystem for LocalFileSystem {
|
||||
file_system.canonicalize(path, sandbox).await
|
||||
}
|
||||
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
self.unsandboxed.join(base_path, path).await
|
||||
}
|
||||
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
|
||||
self.unsandboxed.parent(path).await
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
@@ -182,18 +170,6 @@ impl ExecutorFileSystem for UnsandboxedFileSystem {
|
||||
self.file_system.canonicalize(path, /*sandbox*/ None).await
|
||||
}
|
||||
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
self.file_system.join(base_path, path).await
|
||||
}
|
||||
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
|
||||
self.file_system.parent(path).await
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
@@ -289,18 +265,6 @@ impl ExecutorFileSystem for DirectFileSystem {
|
||||
AbsolutePathBuf::from_absolute_path(tokio::fs::canonicalize(path.as_path()).await?)
|
||||
}
|
||||
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
Ok(base_path.join(path))
|
||||
}
|
||||
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
|
||||
Ok(path.parent())
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
|
||||
@@ -2,7 +2,6 @@ use async_trait::async_trait;
|
||||
use base64::Engine as _;
|
||||
use base64::engine::general_purpose::STANDARD;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::path::Path;
|
||||
use tokio::io;
|
||||
use tracing::trace;
|
||||
|
||||
@@ -20,8 +19,6 @@ use crate::protocol::FsCanonicalizeParams;
|
||||
use crate::protocol::FsCopyParams;
|
||||
use crate::protocol::FsCreateDirectoryParams;
|
||||
use crate::protocol::FsGetMetadataParams;
|
||||
use crate::protocol::FsJoinParams;
|
||||
use crate::protocol::FsParentParams;
|
||||
use crate::protocol::FsReadDirectoryParams;
|
||||
use crate::protocol::FsReadFileParams;
|
||||
use crate::protocol::FsRemoveParams;
|
||||
@@ -60,33 +57,6 @@ impl ExecutorFileSystem for RemoteFileSystem {
|
||||
Ok(response.path)
|
||||
}
|
||||
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
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<Option<AbsolutePathBuf>> {
|
||||
trace!("remote fs parent");
|
||||
let client = self.client.get().await.map_err(map_remote_error)?;
|
||||
let response = client
|
||||
.fs_parent(FsParentParams { path: path.clone() })
|
||||
.await
|
||||
.map_err(map_remote_error)?;
|
||||
Ok(response.path)
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
|
||||
@@ -3,7 +3,6 @@ use base64::Engine as _;
|
||||
use base64::engine::general_purpose::STANDARD;
|
||||
use codex_app_server_protocol::JSONRPCErrorError;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::path::Path;
|
||||
use tokio::io;
|
||||
|
||||
use crate::CopyOptions;
|
||||
@@ -73,18 +72,6 @@ impl ExecutorFileSystem for SandboxedFileSystem {
|
||||
Ok(response.path)
|
||||
}
|
||||
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
Ok(base_path.join(path))
|
||||
}
|
||||
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
|
||||
Ok(path.parent())
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
|
||||
@@ -128,11 +128,8 @@ impl FileSystemHandler {
|
||||
&self,
|
||||
params: FsJoinParams,
|
||||
) -> Result<FsJoinResponse, JSONRPCErrorError> {
|
||||
let path = self
|
||||
.file_system
|
||||
.join(¶ms.base_path, ¶ms.path)
|
||||
.await
|
||||
.map_err(map_fs_error)?;
|
||||
// TODO(anp): remove and migrate callers to PathUri.
|
||||
let path = params.base_path.join(params.path);
|
||||
Ok(FsJoinResponse { path })
|
||||
}
|
||||
|
||||
@@ -140,11 +137,8 @@ impl FileSystemHandler {
|
||||
&self,
|
||||
params: FsParentParams,
|
||||
) -> Result<FsParentResponse, JSONRPCErrorError> {
|
||||
let path = self
|
||||
.file_system
|
||||
.parent(¶ms.path)
|
||||
.await
|
||||
.map_err(map_fs_error)?;
|
||||
// TODO(anp): remove and migrate callers to PathUri.
|
||||
let path = params.path.parent();
|
||||
Ok(FsParentResponse { path })
|
||||
}
|
||||
|
||||
@@ -276,4 +270,34 @@ mod tests {
|
||||
assert_eq!(response.data_base64, STANDARD.encode("ok"));
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn protocol_join_and_parent_remain_native_path_operations() {
|
||||
let temp_dir = tempfile::tempdir().expect("tempdir");
|
||||
let runtime_paths = ExecServerRuntimePaths::new(
|
||||
std::env::current_exe().expect("current exe"),
|
||||
/*codex_linux_sandbox_exe*/ None,
|
||||
)
|
||||
.expect("runtime paths");
|
||||
let handler = FileSystemHandler::new(runtime_paths);
|
||||
let base_path =
|
||||
AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute tempdir");
|
||||
|
||||
let joined = handler
|
||||
.join(FsJoinParams {
|
||||
base_path: base_path.clone(),
|
||||
path: "nested/file.txt".into(),
|
||||
})
|
||||
.await
|
||||
.expect("join path");
|
||||
assert_eq!(joined.path, base_path.join("nested/file.txt"));
|
||||
|
||||
let parent = handler
|
||||
.parent(FsParentParams {
|
||||
path: joined.path.clone(),
|
||||
})
|
||||
.await
|
||||
.expect("parent path");
|
||||
assert_eq!(parent.path, joined.path.parent());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ use codex_protocol::models::FileSystemPermissions;
|
||||
use codex_protocol::models::PermissionProfile;
|
||||
use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy;
|
||||
use codex_sandboxing::policy_transforms::effective_network_sandbox_policy;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::path::Path;
|
||||
use tempfile::TempDir;
|
||||
@@ -130,40 +131,25 @@ async fn file_system_write_file_writes_bytes(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case(FileSystemImplementation::Local ; "local")]
|
||||
#[test_case(FileSystemImplementation::Remote ; "remote")]
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn file_system_join_and_parent_preserve_lexical_paths(
|
||||
implementation: FileSystemImplementation,
|
||||
) -> Result<()> {
|
||||
let context = create_file_system_context(implementation).await?;
|
||||
let file_system = context.file_system;
|
||||
|
||||
#[test]
|
||||
fn path_uri_join_and_parent_preserve_lexical_paths() -> Result<()> {
|
||||
let tmp = TempDir::new()?;
|
||||
let source_dir = tmp.path().join("source");
|
||||
let joined_nested = file_system
|
||||
.join(&absolute_path(&source_dir), Path::new("nested/note.txt"))
|
||||
.await
|
||||
.with_context(|| format!("mode={implementation}"))?;
|
||||
let source_dir_uri = PathUri::from_path(&source_dir)?;
|
||||
let joined_nested = source_dir_uri.join("nested/note.txt")?;
|
||||
assert_eq!(
|
||||
joined_nested,
|
||||
absolute_path(source_dir.join("nested").join("note.txt"))
|
||||
PathUri::from_path(source_dir.join("nested").join("note.txt"))?
|
||||
);
|
||||
let joined_parent = file_system
|
||||
.parent(&joined_nested)
|
||||
.await
|
||||
.with_context(|| format!("mode={implementation}"))?;
|
||||
let joined_parent = joined_nested.parent();
|
||||
assert_eq!(
|
||||
joined_parent,
|
||||
Some(absolute_path(source_dir.join("nested")))
|
||||
Some(PathUri::from_path(source_dir.join("nested"))?)
|
||||
);
|
||||
let joined_parent_traversal = file_system
|
||||
.join(&absolute_path(&source_dir), Path::new("../outside"))
|
||||
.await
|
||||
.with_context(|| format!("mode={implementation}"))?;
|
||||
let joined_parent_traversal = source_dir_uri.join("../outside")?;
|
||||
assert_eq!(
|
||||
joined_parent_traversal,
|
||||
absolute_path(source_dir.join("../outside"))
|
||||
PathUri::from_path(source_dir.join("../outside"))?
|
||||
);
|
||||
|
||||
Ok(())
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
use std::io;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
use std::sync::atomic::AtomicUsize;
|
||||
use std::sync::atomic::Ordering;
|
||||
@@ -70,18 +69,6 @@ impl ExecutorFileSystem for SyntheticFileSystem {
|
||||
Ok(path.clone())
|
||||
}
|
||||
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf> {
|
||||
Ok(base_path.join(path))
|
||||
}
|
||||
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>> {
|
||||
Ok(path.parent())
|
||||
}
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
|
||||
@@ -140,16 +140,6 @@ pub trait ExecutorFileSystem: Send + Sync {
|
||||
sandbox: Option<&FileSystemSandboxContext>,
|
||||
) -> FileSystemResult<AbsolutePathBuf>;
|
||||
|
||||
/// Lexically joins a path onto an existing bound path.
|
||||
async fn join(
|
||||
&self,
|
||||
base_path: &AbsolutePathBuf,
|
||||
path: &Path,
|
||||
) -> FileSystemResult<AbsolutePathBuf>;
|
||||
|
||||
/// Returns the parent directory of a bound path.
|
||||
async fn parent(&self, path: &AbsolutePathBuf) -> FileSystemResult<Option<AbsolutePathBuf>>;
|
||||
|
||||
async fn read_file(
|
||||
&self,
|
||||
path: &AbsolutePathBuf,
|
||||
|
||||
Reference in New Issue
Block a user