mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] migrate exec-server filesystem protocol to PathUri (#27653)
Exec-server filesystem calls should preserve cross-platform `file:` URIs across the remote boundary instead of converting them through paths native to the client host. This changes the exec-server filesystem protocol DTOs to use `PathUri`, carries those values directly through remote and sandbox-helper transports, and keeps legacy native absolute-path request strings readable for compatibility. It also updates protocol documentation and coverage for URI serialization and non-native URI forwarding.
This commit is contained in:
@@ -53,10 +53,9 @@ impl FileSystemHandler {
|
||||
&self,
|
||||
params: FsReadFileParams,
|
||||
) -> Result<FsReadFileResponse, JSONRPCErrorError> {
|
||||
let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?;
|
||||
let bytes = self
|
||||
.file_system
|
||||
.read_file(&path, params.sandbox.as_ref())
|
||||
.read_file(¶ms.path, params.sandbox.as_ref())
|
||||
.await
|
||||
.map_err(map_fs_error)?;
|
||||
Ok(FsReadFileResponse {
|
||||
@@ -68,14 +67,13 @@ impl FileSystemHandler {
|
||||
&self,
|
||||
params: FsWriteFileParams,
|
||||
) -> Result<FsWriteFileResponse, JSONRPCErrorError> {
|
||||
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(&path, bytes, params.sandbox.as_ref())
|
||||
.write_file(¶ms.path, bytes, params.sandbox.as_ref())
|
||||
.await
|
||||
.map_err(map_fs_error)?;
|
||||
Ok(FsWriteFileResponse {})
|
||||
@@ -86,10 +84,9 @@ impl FileSystemHandler {
|
||||
params: FsCreateDirectoryParams,
|
||||
) -> Result<FsCreateDirectoryResponse, JSONRPCErrorError> {
|
||||
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(
|
||||
&path,
|
||||
¶ms.path,
|
||||
CreateDirectoryOptions { recursive },
|
||||
params.sandbox.as_ref(),
|
||||
)
|
||||
@@ -102,10 +99,9 @@ impl FileSystemHandler {
|
||||
&self,
|
||||
params: FsGetMetadataParams,
|
||||
) -> Result<FsGetMetadataResponse, JSONRPCErrorError> {
|
||||
let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?;
|
||||
let metadata = self
|
||||
.file_system
|
||||
.get_metadata(&path, params.sandbox.as_ref())
|
||||
.get_metadata(¶ms.path, params.sandbox.as_ref())
|
||||
.await
|
||||
.map_err(map_fs_error)?;
|
||||
Ok(FsGetMetadataResponse {
|
||||
@@ -121,13 +117,11 @@ 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(&requested_path, params.sandbox.as_ref())
|
||||
.canonicalize(¶ms.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 })
|
||||
}
|
||||
|
||||
@@ -136,7 +130,9 @@ impl FileSystemHandler {
|
||||
params: FsJoinParams,
|
||||
) -> Result<FsJoinResponse, JSONRPCErrorError> {
|
||||
// TODO(anp): remove and migrate callers to PathUri.
|
||||
let path = params.base_path.join(params.path);
|
||||
let base_path = params.base_path.to_abs_path().map_err(map_fs_error)?;
|
||||
let path = base_path.join(params.path);
|
||||
let path = PathUri::from_abs_path(&path).map_err(map_fs_error)?;
|
||||
Ok(FsJoinResponse { path })
|
||||
}
|
||||
|
||||
@@ -145,7 +141,14 @@ impl FileSystemHandler {
|
||||
params: FsParentParams,
|
||||
) -> Result<FsParentResponse, JSONRPCErrorError> {
|
||||
// TODO(anp): remove and migrate callers to PathUri.
|
||||
let path = params.path.parent();
|
||||
let path = params
|
||||
.path
|
||||
.to_abs_path()
|
||||
.map_err(map_fs_error)?
|
||||
.parent()
|
||||
.map(|path| PathUri::from_abs_path(&path))
|
||||
.transpose()
|
||||
.map_err(map_fs_error)?;
|
||||
Ok(FsParentResponse { path })
|
||||
}
|
||||
|
||||
@@ -153,10 +156,9 @@ impl FileSystemHandler {
|
||||
&self,
|
||||
params: FsReadDirectoryParams,
|
||||
) -> Result<FsReadDirectoryResponse, JSONRPCErrorError> {
|
||||
let path = PathUri::from_abs_path(¶ms.path).map_err(map_fs_error)?;
|
||||
let entries = self
|
||||
.file_system
|
||||
.read_directory(&path, params.sandbox.as_ref())
|
||||
.read_directory(¶ms.path, params.sandbox.as_ref())
|
||||
.await
|
||||
.map_err(map_fs_error)?
|
||||
.into_iter()
|
||||
@@ -175,10 +177,9 @@ impl FileSystemHandler {
|
||||
) -> Result<FsRemoveResponse, JSONRPCErrorError> {
|
||||
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(
|
||||
&path,
|
||||
¶ms.path,
|
||||
RemoveOptions { recursive, force },
|
||||
params.sandbox.as_ref(),
|
||||
)
|
||||
@@ -191,13 +192,10 @@ impl FileSystemHandler {
|
||||
&self,
|
||||
params: FsCopyParams,
|
||||
) -> Result<FsCopyResponse, JSONRPCErrorError> {
|
||||
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(
|
||||
&source_path,
|
||||
&destination_path,
|
||||
¶ms.source_path,
|
||||
¶ms.destination_path,
|
||||
CopyOptions {
|
||||
recursive: params.recursive,
|
||||
},
|
||||
@@ -224,6 +222,7 @@ mod tests {
|
||||
use codex_protocol::protocol::NetworkAccess;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::*;
|
||||
@@ -252,9 +251,7 @@ mod tests {
|
||||
},
|
||||
),
|
||||
] {
|
||||
let path =
|
||||
AbsolutePathBuf::from_absolute_path(temp_dir.path().join(file_name).as_path())
|
||||
.expect("absolute path");
|
||||
let path = PathUri::from_path(temp_dir.path().join(file_name)).expect("path URI");
|
||||
|
||||
handler
|
||||
.write_file(FsWriteFileParams {
|
||||
@@ -280,10 +277,10 @@ mod tests {
|
||||
.expect("canonicalize file");
|
||||
assert_eq!(
|
||||
canonicalized.path,
|
||||
AbsolutePathBuf::from_absolute_path(
|
||||
std::fs::canonicalize(path.as_path()).expect("canonical path"),
|
||||
PathUri::from_path(
|
||||
std::fs::canonicalize(temp_dir.path().join(file_name)).expect("canonical path"),
|
||||
)
|
||||
.expect("absolute canonical path"),
|
||||
.expect("canonical path URI"),
|
||||
);
|
||||
|
||||
let response = handler
|
||||
@@ -302,7 +299,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn protocol_join_and_parent_remain_native_path_operations() {
|
||||
async fn protocol_join_and_parent_preserve_native_path_operations() {
|
||||
let temp_dir = tempfile::tempdir().expect("tempdir");
|
||||
let runtime_paths = ExecServerRuntimePaths::new(
|
||||
std::env::current_exe().expect("current exe"),
|
||||
@@ -310,8 +307,9 @@ mod tests {
|
||||
)
|
||||
.expect("runtime paths");
|
||||
let handler = FileSystemHandler::new(runtime_paths);
|
||||
let base_path =
|
||||
let native_base =
|
||||
AbsolutePathBuf::from_absolute_path(temp_dir.path()).expect("absolute tempdir");
|
||||
let base_path = PathUri::from_abs_path(&native_base).expect("path URI");
|
||||
|
||||
let joined = handler
|
||||
.join(FsJoinParams {
|
||||
@@ -320,7 +318,10 @@ mod tests {
|
||||
})
|
||||
.await
|
||||
.expect("join path");
|
||||
assert_eq!(joined.path, base_path.join("nested/file.txt"));
|
||||
assert_eq!(
|
||||
joined.path,
|
||||
PathUri::from_abs_path(&native_base.join("nested/file.txt")).expect("joined path URI")
|
||||
);
|
||||
|
||||
let parent = handler
|
||||
.parent(FsParentParams {
|
||||
@@ -328,6 +329,36 @@ mod tests {
|
||||
})
|
||||
.await
|
||||
.expect("parent path");
|
||||
assert_eq!(parent.path, joined.path.parent());
|
||||
assert_eq!(
|
||||
parent.path,
|
||||
native_base
|
||||
.join("nested/file.txt")
|
||||
.parent()
|
||||
.map(|path| PathUri::from_abs_path(&path).expect("parent path URI"))
|
||||
);
|
||||
|
||||
let absolute_path = native_base.join("absolute.txt");
|
||||
let joined = handler
|
||||
.join(FsJoinParams {
|
||||
base_path,
|
||||
path: absolute_path.as_path().to_path_buf(),
|
||||
})
|
||||
.await
|
||||
.expect("join absolute path");
|
||||
assert_eq!(
|
||||
joined.path,
|
||||
PathUri::from_abs_path(&absolute_path).expect("absolute path URI")
|
||||
);
|
||||
|
||||
let native_root = native_base
|
||||
.ancestors()
|
||||
.last()
|
||||
.expect("absolute path should have a root");
|
||||
let root = PathUri::from_abs_path(&native_root).expect("root path URI");
|
||||
let parent = handler
|
||||
.parent(FsParentParams { path: root })
|
||||
.await
|
||||
.expect("root parent");
|
||||
assert_eq!(parent, FsParentResponse { path: None });
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user