Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions codex-rs/exec-server/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -338,13 +338,15 @@ Params:

## Filesystem RPCs

Filesystem methods use absolute paths and return JSON-RPC errors for invalid
or unavailable paths:
Filesystem methods use canonical `file:` URIs and return JSON-RPC errors for
invalid or unavailable paths. For compatibility, requests also accept native
absolute path strings and normalize them to `file:` URIs:

- `fs/readFile`
- `fs/writeFile`
- `fs/createDirectory`
- `fs/getMetadata`
- `fs/canonicalize`
- `fs/readDirectory`
- `fs/remove`
- `fs/copy`
Expand Down
58 changes: 17 additions & 41 deletions codex-rs/exec-server/src/fs_helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,36 +189,30 @@ pub(crate) async fn run_direct_request(
let file_system = DirectFileSystem;
match request {
FsHelperRequest::ReadFile(params) => {
let path =
codex_utils_path_uri::PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
let data = file_system
.read_file(&path, /*sandbox*/ None)
.read_file(&params.path, /*sandbox*/ None)
.await
.map_err(map_fs_error)?;
Ok(FsHelperPayload::ReadFile(FsReadFileResponse {
data_base64: STANDARD.encode(data),
}))
}
FsHelperRequest::WriteFile(params) => {
let path =
codex_utils_path_uri::PathUri::from_abs_path(&params.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}"
))
})?;
file_system
.write_file(&path, bytes, /*sandbox*/ None)
.write_file(&params.path, bytes, /*sandbox*/ None)
.await
.map_err(map_fs_error)?;
Ok(FsHelperPayload::WriteFile(FsWriteFileResponse {}))
}
FsHelperRequest::CreateDirectory(params) => {
let path =
codex_utils_path_uri::PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
file_system
.create_directory(
&path,
&params.path,
CreateDirectoryOptions {
recursive: params.recursive.unwrap_or(true),
},
Expand All @@ -231,10 +225,8 @@ pub(crate) async fn run_direct_request(
))
}
FsHelperRequest::GetMetadata(params) => {
let path =
codex_utils_path_uri::PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
let metadata = file_system
.get_metadata(&path, /*sandbox*/ None)
.get_metadata(&params.path, /*sandbox*/ None)
.await
.map_err(map_fs_error)?;
Ok(FsHelperPayload::GetMetadata(FsGetMetadataResponse {
Expand All @@ -246,22 +238,17 @@ pub(crate) async fn run_direct_request(
}))
}
FsHelperRequest::Canonicalize(params) => {
let path =
codex_utils_path_uri::PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
let path = file_system
.canonicalize(&path, /*sandbox*/ None)
.canonicalize(&params.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,
}))
}
FsHelperRequest::ReadDirectory(params) => {
let path =
codex_utils_path_uri::PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
let entries = file_system
.read_directory(&path, /*sandbox*/ None)
.read_directory(&params.path, /*sandbox*/ None)
.await
.map_err(map_fs_error)?
.into_iter()
Expand All @@ -276,11 +263,9 @@ pub(crate) async fn run_direct_request(
}))
}
FsHelperRequest::Remove(params) => {
let path =
codex_utils_path_uri::PathUri::from_abs_path(&params.path).map_err(map_fs_error)?;
file_system
.remove(
&path,
&params.path,
RemoveOptions {
recursive: params.recursive.unwrap_or(true),
force: params.force.unwrap_or(true),
Expand All @@ -292,15 +277,10 @@ pub(crate) async fn run_direct_request(
Ok(FsHelperPayload::Remove(FsRemoveResponse {}))
}
FsHelperRequest::Copy(params) => {
let source_path = codex_utils_path_uri::PathUri::from_abs_path(&params.source_path)
.map_err(map_fs_error)?;
let destination_path =
codex_utils_path_uri::PathUri::from_abs_path(&params.destination_path)
.map_err(map_fs_error)?;
file_system
.copy(
&source_path,
&destination_path,
&params.source_path,
&params.destination_path,
CopyOptions {
recursive: params.recursive,
},
Expand All @@ -325,27 +305,23 @@ fn map_fs_error(err: io::Error) -> JSONRPCErrorError {

#[cfg(test)]
mod tests {
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use pretty_assertions::assert_eq;
use serde_json::json;

use super::*;

#[test]
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)]
fn helper_protocol_uses_path_uris() -> serde_json::Result<()> {
let local_path = PathUri::from_path(std::env::current_dir().expect("cwd").join("file"))
.expect("path URI");
let paths = [
local_path,
AbsolutePathBuf::from_absolute_path(r"\\server\share\file").expect("absolute UNC path"),
PathUri::parse("file://server/share/file").expect("path URI"),
];

for path in paths {
let expected_path = path.to_string_lossy().into_owned();
let expected_path = path.to_string();

let request = serde_json::to_value(FsHelperRequest::WriteFile(FsWriteFileParams {
path: path.clone(),
Expand All @@ -367,7 +343,7 @@ mod tests {
.as_str()
.expect("request path should be a string");
assert_eq!(request_path, expected_path);
assert!(!request_path.starts_with("file:"));
assert!(request_path.starts_with("file:"));

let response = serde_json::to_value(FsHelperResponse::Ok(
FsHelperPayload::Canonicalize(FsCanonicalizeResponse { path }),
Expand All @@ -388,7 +364,7 @@ mod tests {
.as_str()
.expect("canonicalize response path should be a string");
assert_eq!(response_path, expected_path);
assert!(!response_path.starts_with("file:"));
assert!(response_path.starts_with("file:"));
}

Ok(())
Expand Down
59 changes: 44 additions & 15 deletions codex-rs/exec-server/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ 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_absolute_path::AbsolutePathBuf;
use codex_utils_path_uri::PathUri;
use serde::Deserialize;
use serde::Serialize;

Expand Down Expand Up @@ -199,7 +199,7 @@ pub struct TerminateResponse {
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FsReadFileParams {
pub path: AbsolutePathBuf,
pub path: PathUri,
pub sandbox: Option<FileSystemSandboxContext>,
}

Expand All @@ -212,7 +212,7 @@ pub struct FsReadFileResponse {
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FsWriteFileParams {
pub path: AbsolutePathBuf,
pub path: PathUri,
pub data_base64: String,
pub sandbox: Option<FileSystemSandboxContext>,
}
Expand All @@ -224,7 +224,7 @@ pub struct FsWriteFileResponse {}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FsCreateDirectoryParams {
pub path: AbsolutePathBuf,
pub path: PathUri,
pub recursive: Option<bool>,
pub sandbox: Option<FileSystemSandboxContext>,
}
Expand All @@ -236,7 +236,7 @@ pub struct FsCreateDirectoryResponse {}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FsGetMetadataParams {
pub path: AbsolutePathBuf,
pub path: PathUri,
pub sandbox: Option<FileSystemSandboxContext>,
}

Expand All @@ -253,45 +253,47 @@ pub struct FsGetMetadataResponse {
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FsCanonicalizeParams {
pub path: AbsolutePathBuf,
pub path: PathUri,
pub sandbox: Option<FileSystemSandboxContext>,
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FsCanonicalizeResponse {
pub path: AbsolutePathBuf,
pub path: PathUri,
Comment thread
anp-oai marked this conversation as resolved.
}

// TODO(anp): remove fs/join from the protocol.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FsJoinParams {
pub base_path: AbsolutePathBuf,
pub base_path: PathUri,
pub path: PathBuf,
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FsJoinResponse {
pub path: AbsolutePathBuf,
pub path: PathUri,
}

// TODO(anp): remove fs/parent from the protocol.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FsParentParams {
pub path: AbsolutePathBuf,
pub path: PathUri,
Comment thread
anp-oai marked this conversation as resolved.
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FsParentResponse {
pub path: Option<AbsolutePathBuf>,
pub path: Option<PathUri>,
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FsReadDirectoryParams {
pub path: AbsolutePathBuf,
pub path: PathUri,
pub sandbox: Option<FileSystemSandboxContext>,
}

Expand All @@ -312,7 +314,7 @@ pub struct FsReadDirectoryResponse {
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FsRemoveParams {
pub path: AbsolutePathBuf,
pub path: PathUri,
pub recursive: Option<bool>,
pub force: Option<bool>,
pub sandbox: Option<FileSystemSandboxContext>,
Expand All @@ -325,8 +327,8 @@ pub struct FsRemoveResponse {}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct FsCopyParams {
pub source_path: AbsolutePathBuf,
pub destination_path: AbsolutePathBuf,
pub source_path: PathUri,
pub destination_path: PathUri,
pub recursive: bool,
pub sandbox: Option<FileSystemSandboxContext>,
}
Expand Down Expand Up @@ -475,9 +477,36 @@ mod base64_bytes {

#[cfg(test)]
mod tests {
use super::FsReadFileParams;
use super::HttpRequestParams;
use codex_utils_path_uri::PathUri;
use pretty_assertions::assert_eq;

#[test]
fn filesystem_protocol_accepts_legacy_absolute_paths_and_serializes_path_uris() {
let legacy_path = std::env::current_dir()
.expect("current directory")
.join("legacy-file.txt");
let params: FsReadFileParams = serde_json::from_value(serde_json::json!({
"path": legacy_path.to_string_lossy(),
"sandbox": null,
}))
.expect("legacy absolute path should deserialize");
let expected = FsReadFileParams {
path: PathUri::from_path(legacy_path).expect("path URI"),
sandbox: None,
};

assert_eq!(params, expected);
assert_eq!(
serde_json::to_value(params).expect("params should serialize"),
serde_json::json!({
"path": expected.path.to_string(),
"sandbox": null,
})
);
}

#[test]
fn http_request_timeout_treats_omitted_and_null_as_no_timeout() {
let omitted: HttpRequestParams = serde_json::from_value(serde_json::json!({
Expand Down
Loading
Loading