mirror of
https://github.com/openai/codex.git
synced 2026-10-28 18:58:46 +00:00
Add capability-gated writable file streams to the exec-server client (#49805)
## Why Older executors can ignore the file open mode and silently return a read-only handle for a writable open. ## What changed Add `ExecServerClient::fs_write_block` and require the `file_write_streaming` capability for both block writes and `FsOpenMode::Replace` opens. Return a protocol error before sending either request when the executor lacks support, while allowing read-only opens. ## Testing Add a WebSocket regression test verifying that unsupported writable opens and block writes are rejected locally and read-only opens still reach the executor. GitOrigin-RevId: f206e1da80c12af76ead14c75f8ffe71f82cce89
This commit is contained in:
committed by
copyberry
parent
d2f2c40095
commit
e7798c9944
@@ -80,6 +80,7 @@ use crate::protocol::FS_READ_DIRECTORY_METHOD;
|
||||
use crate::protocol::FS_READ_FILE_METHOD;
|
||||
use crate::protocol::FS_REMOVE_METHOD;
|
||||
use crate::protocol::FS_WALK_METHOD;
|
||||
use crate::protocol::FS_WRITE_BLOCK_METHOD;
|
||||
use crate::protocol::FS_WRITE_FILE_METHOD;
|
||||
use crate::protocol::FsCanonicalizeParams;
|
||||
use crate::protocol::FsCanonicalizeResponse;
|
||||
@@ -91,6 +92,7 @@ use crate::protocol::FsCreateDirectoryParams;
|
||||
use crate::protocol::FsCreateDirectoryResponse;
|
||||
use crate::protocol::FsGetMetadataParams;
|
||||
use crate::protocol::FsGetMetadataResponse;
|
||||
use crate::protocol::FsOpenMode;
|
||||
use crate::protocol::FsOpenParams;
|
||||
use crate::protocol::FsOpenResponse;
|
||||
use crate::protocol::FsReadBlockParams;
|
||||
@@ -103,6 +105,8 @@ use crate::protocol::FsRemoveParams;
|
||||
use crate::protocol::FsRemoveResponse;
|
||||
use crate::protocol::FsWalkParams;
|
||||
use crate::protocol::FsWalkResponse;
|
||||
use crate::protocol::FsWriteBlockParams;
|
||||
use crate::protocol::FsWriteBlockResponse;
|
||||
use crate::protocol::FsWriteFileParams;
|
||||
use crate::protocol::FsWriteFileResponse;
|
||||
use crate::protocol::HTTP_REQUEST_BODY_DELTA_METHOD;
|
||||
@@ -952,6 +956,18 @@ impl ExecServerClient {
|
||||
}
|
||||
|
||||
pub async fn fs_open(&self, params: FsOpenParams) -> Result<FsOpenResponse, ExecServerError> {
|
||||
// Older executors ignore the mode field and could silently return a read-only handle.
|
||||
if params.mode == FsOpenMode::Replace
|
||||
&& !self
|
||||
.environment_info()
|
||||
.await?
|
||||
.capabilities
|
||||
.file_write_streaming
|
||||
{
|
||||
return Err(ExecServerError::Protocol(
|
||||
"exec-server does not support writable file streams".to_string(),
|
||||
));
|
||||
}
|
||||
self.call(FS_OPEN_METHOD, &WireFsOpenParams::from(params))
|
||||
.await
|
||||
}
|
||||
@@ -963,6 +979,23 @@ impl ExecServerClient {
|
||||
self.call(FS_READ_BLOCK_METHOD, ¶ms).await
|
||||
}
|
||||
|
||||
pub async fn fs_write_block(
|
||||
&self,
|
||||
params: FsWriteBlockParams,
|
||||
) -> Result<FsWriteBlockResponse, ExecServerError> {
|
||||
if !self
|
||||
.environment_info()
|
||||
.await?
|
||||
.capabilities
|
||||
.file_write_streaming
|
||||
{
|
||||
return Err(ExecServerError::Protocol(
|
||||
"exec-server does not support writable file streams".to_string(),
|
||||
));
|
||||
}
|
||||
self.call(FS_WRITE_BLOCK_METHOD, ¶ms).await
|
||||
}
|
||||
|
||||
pub async fn fs_close(
|
||||
&self,
|
||||
params: FsCloseParams,
|
||||
@@ -1920,6 +1953,7 @@ mod tests {
|
||||
|
||||
use super::ExecServerClient;
|
||||
use super::ExecServerClientConnectOptions;
|
||||
use super::ExecServerError;
|
||||
use super::LazyRemoteExecServerClient;
|
||||
use crate::EnvironmentObservedStatus;
|
||||
use crate::ProcessId;
|
||||
@@ -1944,6 +1978,10 @@ mod tests {
|
||||
use crate::protocol::ExecOutputStream;
|
||||
use crate::protocol::ExecParams;
|
||||
use crate::protocol::ExecResponse;
|
||||
use crate::protocol::FS_OPEN_METHOD;
|
||||
use crate::protocol::FsOpenMode;
|
||||
use crate::protocol::FsOpenParams;
|
||||
use crate::protocol::FsWriteBlockParams;
|
||||
use crate::protocol::INITIALIZE_METHOD;
|
||||
use crate::protocol::INITIALIZED_METHOD;
|
||||
use crate::protocol::InitializeResponse;
|
||||
@@ -2678,6 +2716,79 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Old executors must never receive writable opens they would silently treat as reads.
|
||||
#[tokio::test]
|
||||
async fn writable_file_streams_require_executor_capability() -> anyhow::Result<()> {
|
||||
let listener = TcpListener::bind("127.0.0.1:0").await?;
|
||||
let websocket_url = format!("ws://{}", listener.local_addr()?);
|
||||
let read_open = FsOpenParams {
|
||||
handle_id: "read-handle".to_string(),
|
||||
path: PathUri::parse("file:///tmp/existing.txt")?,
|
||||
mode: FsOpenMode::Read,
|
||||
sandbox: None,
|
||||
};
|
||||
let server = tokio::spawn(async move {
|
||||
let mut websocket = accept_websocket(&listener).await;
|
||||
let mut info = EnvironmentInfo::local();
|
||||
info.capabilities.file_write_streaming = false;
|
||||
complete_websocket_initialize_with_environment_info(
|
||||
&mut websocket,
|
||||
"session-1",
|
||||
/*expected_resume_session_id*/ None,
|
||||
Some(info),
|
||||
)
|
||||
.await;
|
||||
let JSONRPCMessage::Request(request) = read_jsonrpc_websocket(&mut websocket).await
|
||||
else {
|
||||
panic!("expected read-only open");
|
||||
};
|
||||
assert_eq!(request.method, FS_OPEN_METHOD);
|
||||
assert_eq!(
|
||||
request.params,
|
||||
Some(serde_json::json!({
|
||||
"handleId": "read-handle",
|
||||
"path": "file:///tmp/existing.txt",
|
||||
"mode": "read",
|
||||
"sandbox": null,
|
||||
}))
|
||||
);
|
||||
write_jsonrpc_websocket(
|
||||
&mut websocket,
|
||||
JSONRPCMessage::Response(JSONRPCResponse {
|
||||
id: request.id,
|
||||
result: serde_json::json!({ "handleId": "read-handle" }),
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
});
|
||||
let client = ExecServerClient::connect_websocket(RemoteExecServerConnectArgs::new(
|
||||
websocket_url,
|
||||
"file-stream-test".to_string(),
|
||||
HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault),
|
||||
))
|
||||
.await?;
|
||||
let error = client
|
||||
.fs_open(FsOpenParams {
|
||||
mode: FsOpenMode::Replace,
|
||||
..read_open.clone()
|
||||
})
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(matches!(error, ExecServerError::Protocol(_)));
|
||||
let error = client
|
||||
.fs_write_block(FsWriteBlockParams {
|
||||
handle_id: "write-handle".to_string(),
|
||||
offset: 0,
|
||||
chunk: vec![1].into(),
|
||||
})
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(matches!(error, ExecServerError::Protocol(_)));
|
||||
client.fs_open(read_open).await?;
|
||||
server.await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test_case::test_case(false; "socket_closed")]
|
||||
#[test_case::test_case(true; "health_check_timed_out")]
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user