mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] exec-server: stream files in chunks (#28354)
## Why `fs/readFile` buffers the entire file in one response, which makes large remote reads expensive and prevents callers from applying backpressure. We need an opt-in streaming path with bounded block sizes while preserving the existing single-call API for small and sandboxed reads. ## What changed - Add `ExecServerClient::stream`, returning a named `FileReadStream` that implements `futures::Stream` and yields immutable 1 MiB byte blocks. - Add internal `fs/open`, `fs/readBlock`, and `fs/close` RPCs. `fs/readBlock` accepts an explicit offset and length. - Keep unsandboxed files open between block reads, cap open handles per connection, and clean them up on EOF, error, stream drop, explicit close, or connection shutdown. - Reject platform-sandboxed streaming opens instead of turning the one-shot sandbox helper into a persistent server. Existing `fs/readFile` behavior is unchanged. ## Testing - `just test -p codex-exec-server` - Integration coverage for 1 MiB chunking, exact block-boundary EOF, sandbox rejection, and continued reads from the opened file after path replacement. - Handle-manager coverage for non-sequential offsets, variable block lengths, the 128-handle limit, and capacity release after close.
This commit is contained in:
@@ -9,16 +9,23 @@ use crate::CreateDirectoryOptions;
|
||||
use crate::ExecServerRuntimePaths;
|
||||
use crate::ExecutorFileSystem;
|
||||
use crate::RemoveOptions;
|
||||
use crate::file_read::FileReadHandleManager;
|
||||
use crate::local_file_system::LocalFileSystem;
|
||||
use crate::protocol::FS_WRITE_FILE_METHOD;
|
||||
use crate::protocol::FsCanonicalizeParams;
|
||||
use crate::protocol::FsCanonicalizeResponse;
|
||||
use crate::protocol::FsCloseParams;
|
||||
use crate::protocol::FsCloseResponse;
|
||||
use crate::protocol::FsCopyParams;
|
||||
use crate::protocol::FsCopyResponse;
|
||||
use crate::protocol::FsCreateDirectoryParams;
|
||||
use crate::protocol::FsCreateDirectoryResponse;
|
||||
use crate::protocol::FsGetMetadataParams;
|
||||
use crate::protocol::FsGetMetadataResponse;
|
||||
use crate::protocol::FsOpenParams;
|
||||
use crate::protocol::FsOpenResponse;
|
||||
use crate::protocol::FsReadBlockParams;
|
||||
use crate::protocol::FsReadBlockResponse;
|
||||
use crate::protocol::FsReadDirectoryEntry;
|
||||
use crate::protocol::FsReadDirectoryParams;
|
||||
use crate::protocol::FsReadDirectoryResponse;
|
||||
@@ -32,18 +39,69 @@ use crate::rpc::internal_error;
|
||||
use crate::rpc::invalid_request;
|
||||
use crate::rpc::not_found;
|
||||
|
||||
const MAX_FILE_READ_HANDLE_ID_BYTES: usize = 32;
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct FileSystemHandler {
|
||||
file_system: LocalFileSystem,
|
||||
file_reads: FileReadHandleManager,
|
||||
}
|
||||
|
||||
impl FileSystemHandler {
|
||||
pub(crate) fn new(runtime_paths: ExecServerRuntimePaths) -> Self {
|
||||
Self {
|
||||
file_system: LocalFileSystem::with_runtime_paths(runtime_paths),
|
||||
file_reads: FileReadHandleManager::default(),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) async fn shutdown(&self) {
|
||||
self.file_reads.close_all().await;
|
||||
}
|
||||
|
||||
pub(crate) async fn open(
|
||||
&self,
|
||||
params: FsOpenParams,
|
||||
) -> Result<FsOpenResponse, JSONRPCErrorError> {
|
||||
validate_file_read_handle_id(¶ms.handle_id)?;
|
||||
let file = self
|
||||
.file_system
|
||||
.open_file_for_read(¶ms.path, params.sandbox.as_ref())
|
||||
.await
|
||||
.map_err(map_fs_error)?;
|
||||
let handle_id = self
|
||||
.file_reads
|
||||
.open(params.handle_id, file)
|
||||
.await
|
||||
.map_err(map_fs_error)?;
|
||||
Ok(FsOpenResponse { handle_id })
|
||||
}
|
||||
|
||||
pub(crate) async fn read_block(
|
||||
&self,
|
||||
params: FsReadBlockParams,
|
||||
) -> Result<FsReadBlockResponse, JSONRPCErrorError> {
|
||||
validate_file_read_handle_id(¶ms.handle_id)?;
|
||||
let block = self
|
||||
.file_reads
|
||||
.read_block(¶ms.handle_id, params.offset, params.len)
|
||||
.await
|
||||
.map_err(map_fs_error)?;
|
||||
Ok(FsReadBlockResponse {
|
||||
chunk: block.bytes.into(),
|
||||
eof: block.eof,
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) async fn close(
|
||||
&self,
|
||||
params: FsCloseParams,
|
||||
) -> Result<FsCloseResponse, JSONRPCErrorError> {
|
||||
validate_file_read_handle_id(¶ms.handle_id)?;
|
||||
self.file_reads.close(¶ms.handle_id).await;
|
||||
Ok(FsCloseResponse {})
|
||||
}
|
||||
|
||||
pub(crate) async fn read_file(
|
||||
&self,
|
||||
params: FsReadFileParams,
|
||||
@@ -176,6 +234,15 @@ impl FileSystemHandler {
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_file_read_handle_id(handle_id: &str) -> Result<(), JSONRPCErrorError> {
|
||||
if handle_id.len() > MAX_FILE_READ_HANDLE_ID_BYTES {
|
||||
return Err(invalid_request(format!(
|
||||
"file read handle ID must not exceed {MAX_FILE_READ_HANDLE_ID_BYTES} bytes"
|
||||
)));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn map_fs_error(err: io::Error) -> JSONRPCErrorError {
|
||||
match err.kind() {
|
||||
io::ErrorKind::NotFound => not_found(err.to_string()),
|
||||
|
||||
@@ -19,12 +19,18 @@ use crate::protocol::ExecParams;
|
||||
use crate::protocol::ExecResponse;
|
||||
use crate::protocol::FsCanonicalizeParams;
|
||||
use crate::protocol::FsCanonicalizeResponse;
|
||||
use crate::protocol::FsCloseParams;
|
||||
use crate::protocol::FsCloseResponse;
|
||||
use crate::protocol::FsCopyParams;
|
||||
use crate::protocol::FsCopyResponse;
|
||||
use crate::protocol::FsCreateDirectoryParams;
|
||||
use crate::protocol::FsCreateDirectoryResponse;
|
||||
use crate::protocol::FsGetMetadataParams;
|
||||
use crate::protocol::FsGetMetadataResponse;
|
||||
use crate::protocol::FsOpenParams;
|
||||
use crate::protocol::FsOpenResponse;
|
||||
use crate::protocol::FsReadBlockParams;
|
||||
use crate::protocol::FsReadBlockResponse;
|
||||
use crate::protocol::FsReadDirectoryParams;
|
||||
use crate::protocol::FsReadDirectoryResponse;
|
||||
use crate::protocol::FsReadFileParams;
|
||||
@@ -87,6 +93,7 @@ impl ExecServerHandler {
|
||||
self.background_task_shutdown.cancel();
|
||||
self.background_tasks.close();
|
||||
self.background_tasks.wait().await;
|
||||
self.file_system.shutdown().await;
|
||||
if let Some(session) = self.session() {
|
||||
session.detach().await;
|
||||
}
|
||||
@@ -234,6 +241,30 @@ impl ExecServerHandler {
|
||||
self.file_system.read_file(params).await
|
||||
}
|
||||
|
||||
pub(crate) async fn fs_open(
|
||||
&self,
|
||||
params: FsOpenParams,
|
||||
) -> Result<FsOpenResponse, JSONRPCErrorError> {
|
||||
self.require_initialized_for("filesystem")?;
|
||||
self.file_system.open(params).await
|
||||
}
|
||||
|
||||
pub(crate) async fn fs_read_block(
|
||||
&self,
|
||||
params: FsReadBlockParams,
|
||||
) -> Result<FsReadBlockResponse, JSONRPCErrorError> {
|
||||
self.require_initialized_for("filesystem")?;
|
||||
self.file_system.read_block(params).await
|
||||
}
|
||||
|
||||
pub(crate) async fn fs_close(
|
||||
&self,
|
||||
params: FsCloseParams,
|
||||
) -> Result<FsCloseResponse, JSONRPCErrorError> {
|
||||
self.require_initialized_for("filesystem")?;
|
||||
self.file_system.close(params).await
|
||||
}
|
||||
|
||||
pub(crate) async fn fs_write_file(
|
||||
&self,
|
||||
params: FsWriteFileParams,
|
||||
|
||||
@@ -8,17 +8,23 @@ use crate::protocol::EXEC_TERMINATE_METHOD;
|
||||
use crate::protocol::EXEC_WRITE_METHOD;
|
||||
use crate::protocol::ExecParams;
|
||||
use crate::protocol::FS_CANONICALIZE_METHOD;
|
||||
use crate::protocol::FS_CLOSE_METHOD;
|
||||
use crate::protocol::FS_COPY_METHOD;
|
||||
use crate::protocol::FS_CREATE_DIRECTORY_METHOD;
|
||||
use crate::protocol::FS_GET_METADATA_METHOD;
|
||||
use crate::protocol::FS_OPEN_METHOD;
|
||||
use crate::protocol::FS_READ_BLOCK_METHOD;
|
||||
use crate::protocol::FS_READ_DIRECTORY_METHOD;
|
||||
use crate::protocol::FS_READ_FILE_METHOD;
|
||||
use crate::protocol::FS_REMOVE_METHOD;
|
||||
use crate::protocol::FS_WRITE_FILE_METHOD;
|
||||
use crate::protocol::FsCanonicalizeParams;
|
||||
use crate::protocol::FsCloseParams;
|
||||
use crate::protocol::FsCopyParams;
|
||||
use crate::protocol::FsCreateDirectoryParams;
|
||||
use crate::protocol::FsGetMetadataParams;
|
||||
use crate::protocol::FsOpenParams;
|
||||
use crate::protocol::FsReadBlockParams;
|
||||
use crate::protocol::FsReadDirectoryParams;
|
||||
use crate::protocol::FsReadFileParams;
|
||||
use crate::protocol::FsRemoveParams;
|
||||
@@ -93,6 +99,24 @@ pub(crate) fn build_router() -> RpcRouter<ExecServerHandler> {
|
||||
handler.fs_read_file(params).await
|
||||
},
|
||||
);
|
||||
router.request(
|
||||
FS_OPEN_METHOD,
|
||||
|handler: Arc<ExecServerHandler>, params: FsOpenParams| async move {
|
||||
handler.fs_open(params).await
|
||||
},
|
||||
);
|
||||
router.request(
|
||||
FS_READ_BLOCK_METHOD,
|
||||
|handler: Arc<ExecServerHandler>, params: FsReadBlockParams| async move {
|
||||
handler.fs_read_block(params).await
|
||||
},
|
||||
);
|
||||
router.request(
|
||||
FS_CLOSE_METHOD,
|
||||
|handler: Arc<ExecServerHandler>, params: FsCloseParams| async move {
|
||||
handler.fs_close(params).await
|
||||
},
|
||||
);
|
||||
router.request(
|
||||
FS_WRITE_FILE_METHOD,
|
||||
|handler: Arc<ExecServerHandler>, params: FsWriteFileParams| async move {
|
||||
|
||||
Reference in New Issue
Block a user