Refactor ExecServer filesystem split between local and remote

codex-rs/app-server/src/fs_api.rs

@@ -34,7 +34,7 @@ pub(crate) struct FsApi {
 impl Default for FsApi {
     fn default() -> Self {
         Self {
-            file_system: Arc::new(Environment::default().get_filesystem()),
+            file_system: Environment::default().get_filesystem(),
         }
     }
 }

codex-rs/core/src/tools/handlers/view_image.rs

@@ -1,5 +1,4 @@
 use async_trait::async_trait;
-use codex_exec_server::ExecutorFileSystem;
 use codex_protocol::models::FunctionCallOutputBody;
 use codex_protocol::models::FunctionCallOutputContentItem;
 use codex_protocol::models::FunctionCallOutputPayload;

codex-rs/exec-server/Cargo.toml

@@ -44,3 +44,4 @@ anyhow = { workspace = true }
 codex-utils-cargo-bin = { workspace = true }
 pretty_assertions = { workspace = true }
 tempfile = { workspace = true }
+test-case = "3.3.1"

codex-rs/exec-server/src/environment.rs

@@ -1,8 +1,10 @@
 use crate::ExecServerClient;
 use crate::ExecServerError;
 use crate::RemoteExecServerConnectArgs;
-use crate::fs;
-use crate::fs::ExecutorFileSystem;
+use crate::file_system::ExecutorFileSystem;
+use crate::local_file_system::LocalFileSystem;
+use crate::remote_file_system::RemoteFileSystem;
+use std::sync::Arc;
 
 #[derive(Clone, Default)]
 pub struct Environment {
@@ -56,8 +58,12 @@ impl Environment {
         self.remote_exec_server_client.as_ref()
     }
 
-    pub fn get_filesystem(&self) -> impl ExecutorFileSystem + use<> {
-        fs::LocalFileSystem
+    pub fn get_filesystem(&self) -> Arc<dyn ExecutorFileSystem> {
+        if let Some(client) = self.remote_exec_server_client.clone() {
+            Arc::new(RemoteFileSystem::new(client))
+        } else {
+            Arc::new(LocalFileSystem)
+        }
     }
 }
 

codex-rs/exec-server/src/file_system.rs

@@ -0,0 +1,65 @@
+use async_trait::async_trait;
+use codex_utils_absolute_path::AbsolutePathBuf;
+use tokio::io;
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub struct CreateDirectoryOptions {
+    pub recursive: bool,
+}
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub struct RemoveOptions {
+    pub recursive: bool,
+    pub force: bool,
+}
+
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub struct CopyOptions {
+    pub recursive: bool,
+}
+
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct FileMetadata {
+    pub is_directory: bool,
+    pub is_file: bool,
+    pub created_at_ms: i64,
+    pub modified_at_ms: i64,
+}
+
+#[derive(Clone, Debug, Eq, PartialEq)]
+pub struct ReadDirectoryEntry {
+    pub file_name: String,
+    pub is_directory: bool,
+    pub is_file: bool,
+}
+
+pub type FileSystemResult<T> = io::Result<T>;
+
+#[async_trait]
+pub trait ExecutorFileSystem: Send + Sync {
+    async fn read_file(&self, path: &AbsolutePathBuf) -> FileSystemResult<Vec<u8>>;
+
+    async fn write_file(&self, path: &AbsolutePathBuf, contents: Vec<u8>) -> FileSystemResult<()>;
+
+    async fn create_directory(
+        &self,
+        path: &AbsolutePathBuf,
+        options: CreateDirectoryOptions,
+    ) -> FileSystemResult<()>;
+
+    async fn get_metadata(&self, path: &AbsolutePathBuf) -> FileSystemResult<FileMetadata>;
+
+    async fn read_directory(
+        &self,
+        path: &AbsolutePathBuf,
+    ) -> FileSystemResult<Vec<ReadDirectoryEntry>>;
+
+    async fn remove(&self, path: &AbsolutePathBuf, options: RemoveOptions) -> FileSystemResult<()>;
+
+    async fn copy(
+        &self,
+        source_path: &AbsolutePathBuf,
+        destination_path: &AbsolutePathBuf,
+        options: CopyOptions,
+    ) -> FileSystemResult<()>;
+}

codex-rs/exec-server/src/lib.rs

@@ -2,8 +2,10 @@ mod client;
 mod client_api;
 mod connection;
 mod environment;
-mod fs;
+mod file_system;
+mod local_file_system;
 mod protocol;
+mod remote_file_system;
 mod rpc;
 mod server;
 
@@ -28,13 +30,13 @@ pub use codex_app_server_protocol::FsRemoveResponse;
 pub use codex_app_server_protocol::FsWriteFileParams;
 pub use codex_app_server_protocol::FsWriteFileResponse;
 pub use environment::Environment;
-pub use fs::CopyOptions;
-pub use fs::CreateDirectoryOptions;
-pub use fs::ExecutorFileSystem;
-pub use fs::FileMetadata;
-pub use fs::FileSystemResult;
-pub use fs::ReadDirectoryEntry;
-pub use fs::RemoveOptions;
+pub use file_system::CopyOptions;
+pub use file_system::CreateDirectoryOptions;
+pub use file_system::ExecutorFileSystem;
+pub use file_system::FileMetadata;
+pub use file_system::FileSystemResult;
+pub use file_system::ReadDirectoryEntry;
+pub use file_system::RemoveOptions;
 pub use protocol::ExecExitedNotification;
 pub use protocol::ExecOutputDeltaNotification;
 pub use protocol::ExecOutputStream;

codex-rs/exec-server/src/fs.rs → codex-rs/exec-server/src/local_file_system.rs

@@ -7,69 +7,15 @@ use std::time::SystemTime;
 use std::time::UNIX_EPOCH;
 use tokio::io;
 
-const MAX_READ_FILE_BYTES: u64 = 512 * 1024 * 1024;
-
-#[derive(Clone, Copy, Debug, Eq, PartialEq)]
-pub struct CreateDirectoryOptions {
-    pub recursive: bool,
-}
-
-#[derive(Clone, Copy, Debug, Eq, PartialEq)]
-pub struct RemoveOptions {
-    pub recursive: bool,
-    pub force: bool,
-}
-
-#[derive(Clone, Copy, Debug, Eq, PartialEq)]
-pub struct CopyOptions {
-    pub recursive: bool,
-}
-
-#[derive(Clone, Debug, Eq, PartialEq)]
-pub struct FileMetadata {
-    pub is_directory: bool,
-    pub is_file: bool,
-    pub created_at_ms: i64,
-    pub modified_at_ms: i64,
-}
+use crate::CopyOptions;
+use crate::CreateDirectoryOptions;
+use crate::ExecutorFileSystem;
+use crate::FileMetadata;
+use crate::FileSystemResult;
+use crate::ReadDirectoryEntry;
+use crate::RemoveOptions;
 
-#[derive(Clone, Debug, Eq, PartialEq)]
-pub struct ReadDirectoryEntry {
-    pub file_name: String,
-    pub is_directory: bool,
-    pub is_file: bool,
-}
-
-pub type FileSystemResult<T> = io::Result<T>;
-
-#[async_trait]
-pub trait ExecutorFileSystem: Send + Sync {
-    async fn read_file(&self, path: &AbsolutePathBuf) -> FileSystemResult<Vec<u8>>;
-
-    async fn write_file(&self, path: &AbsolutePathBuf, contents: Vec<u8>) -> FileSystemResult<()>;
-
-    async fn create_directory(
-        &self,
-        path: &AbsolutePathBuf,
-        options: CreateDirectoryOptions,
-    ) -> FileSystemResult<()>;
-
-    async fn get_metadata(&self, path: &AbsolutePathBuf) -> FileSystemResult<FileMetadata>;
-
-    async fn read_directory(
-        &self,
-        path: &AbsolutePathBuf,
-    ) -> FileSystemResult<Vec<ReadDirectoryEntry>>;
-
-    async fn remove(&self, path: &AbsolutePathBuf, options: RemoveOptions) -> FileSystemResult<()>;
-
-    async fn copy(
-        &self,
-        source_path: &AbsolutePathBuf,
-        destination_path: &AbsolutePathBuf,
-        options: CopyOptions,
-    ) -> FileSystemResult<()>;
-}
+const MAX_READ_FILE_BYTES: u64 = 512 * 1024 * 1024;
 
 #[derive(Clone, Default)]
 pub(crate) struct LocalFileSystem;

codex-rs/exec-server/src/remote_file_system.rs

@@ -0,0 +1,154 @@
+use async_trait::async_trait;
+use base64::Engine as _;
+use base64::engine::general_purpose::STANDARD;
+use codex_app_server_protocol::FsCopyParams;
+use codex_app_server_protocol::FsCreateDirectoryParams;
+use codex_app_server_protocol::FsGetMetadataParams;
+use codex_app_server_protocol::FsReadDirectoryParams;
+use codex_app_server_protocol::FsReadFileParams;
+use codex_app_server_protocol::FsRemoveParams;
+use codex_app_server_protocol::FsWriteFileParams;
+use codex_utils_absolute_path::AbsolutePathBuf;
+use tokio::io;
+
+use crate::CopyOptions;
+use crate::CreateDirectoryOptions;
+use crate::ExecServerClient;
+use crate::ExecServerError;
+use crate::ExecutorFileSystem;
+use crate::FileMetadata;
+use crate::FileSystemResult;
+use crate::ReadDirectoryEntry;
+use crate::RemoveOptions;
+
+const INVALID_REQUEST_ERROR_CODE: i64 = -32600;
+
+#[derive(Clone)]
+pub(crate) struct RemoteFileSystem {
+    client: ExecServerClient,
+}
+
+impl RemoteFileSystem {
+    pub(crate) fn new(client: ExecServerClient) -> Self {
+        Self { client }
+    }
+}
+
+#[async_trait]
+impl ExecutorFileSystem for RemoteFileSystem {
+    async fn read_file(&self, path: &AbsolutePathBuf) -> FileSystemResult<Vec<u8>> {
+        let response = self
+            .client
+            .fs_read_file(FsReadFileParams { path: path.clone() })
+            .await
+            .map_err(map_remote_error)?;
+        STANDARD.decode(response.data_base64).map_err(|err| {
+            io::Error::new(
+                io::ErrorKind::InvalidData,
+                format!("remote fs/readFile returned invalid base64 dataBase64: {err}"),
+            )
+        })
+    }
+
+    async fn write_file(&self, path: &AbsolutePathBuf, contents: Vec<u8>) -> FileSystemResult<()> {
+        self.client
+            .fs_write_file(FsWriteFileParams {
+                path: path.clone(),
+                data_base64: STANDARD.encode(contents),
+            })
+            .await
+            .map_err(map_remote_error)?;
+        Ok(())
+    }
+
+    async fn create_directory(
+        &self,
+        path: &AbsolutePathBuf,
+        options: CreateDirectoryOptions,
+    ) -> FileSystemResult<()> {
+        self.client
+            .fs_create_directory(FsCreateDirectoryParams {
+                path: path.clone(),
+                recursive: Some(options.recursive),
+            })
+            .await
+            .map_err(map_remote_error)?;
+        Ok(())
+    }
+
+    async fn get_metadata(&self, path: &AbsolutePathBuf) -> FileSystemResult<FileMetadata> {
+        let response = self
+            .client
+            .fs_get_metadata(FsGetMetadataParams { path: path.clone() })
+            .await
+            .map_err(map_remote_error)?;
+        Ok(FileMetadata {
+            is_directory: response.is_directory,
+            is_file: response.is_file,
+            created_at_ms: response.created_at_ms,
+            modified_at_ms: response.modified_at_ms,
+        })
+    }
+
+    async fn read_directory(
+        &self,
+        path: &AbsolutePathBuf,
+    ) -> FileSystemResult<Vec<ReadDirectoryEntry>> {
+        let response = self
+            .client
+            .fs_read_directory(FsReadDirectoryParams { path: path.clone() })
+            .await
+            .map_err(map_remote_error)?;
+        Ok(response
+            .entries
+            .into_iter()
+            .map(|entry| ReadDirectoryEntry {
+                file_name: entry.file_name,
+                is_directory: entry.is_directory,
+                is_file: entry.is_file,
+            })
+            .collect())
+    }
+
+    async fn remove(&self, path: &AbsolutePathBuf, options: RemoveOptions) -> FileSystemResult<()> {
+        self.client
+            .fs_remove(FsRemoveParams {
+                path: path.clone(),
+                recursive: Some(options.recursive),
+                force: Some(options.force),
+            })
+            .await
+            .map_err(map_remote_error)?;
+        Ok(())
+    }
+
+    async fn copy(
+        &self,
+        source_path: &AbsolutePathBuf,
+        destination_path: &AbsolutePathBuf,
+        options: CopyOptions,
+    ) -> FileSystemResult<()> {
+        self.client
+            .fs_copy(FsCopyParams {
+                source_path: source_path.clone(),
+                destination_path: destination_path.clone(),
+                recursive: options.recursive,
+            })
+            .await
+            .map_err(map_remote_error)?;
+        Ok(())
+    }
+}
+
+fn map_remote_error(error: ExecServerError) -> io::Error {
+    match error {
+        ExecServerError::Server { code, message } if code == INVALID_REQUEST_ERROR_CODE => {
+            io::Error::new(io::ErrorKind::InvalidInput, message)
+        }
+        ExecServerError::Server { message, .. } => io::Error::other(message),
+        ExecServerError::Closed => {
+            io::Error::new(io::ErrorKind::BrokenPipe, "exec-server transport closed")
+        }
+        _ => io::Error::other(error.to_string()),
+    }
+}

codex-rs/exec-server/src/server.rs

@@ -1,4 +1,4 @@
-mod filesystem;
+mod file_system_handler;
 mod handler;
 mod processor;
 mod registry;