mirror of
https://github.com/openai/codex.git
synced 2026-04-29 00:55:38 +00:00
Merge origin/main into exec-server-sandbox-0403
Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
@@ -14,44 +14,83 @@ use crate::remote_process::RemoteProcess;
|
||||
|
||||
pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL";
|
||||
|
||||
pub trait ExecutorEnvironment: Send + Sync {
|
||||
fn get_exec_backend(&self) -> Arc<dyn ExecBackend>;
|
||||
}
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
/// Lazily creates and caches the active environment for a session.
|
||||
///
|
||||
/// The manager keeps the session's environment selection stable so subagents
|
||||
/// and follow-up turns preserve an explicit disabled state.
|
||||
#[derive(Debug)]
|
||||
pub struct EnvironmentManager {
|
||||
exec_server_url: Option<String>,
|
||||
current_environment: OnceCell<Arc<Environment>>,
|
||||
disabled: bool,
|
||||
current_environment: OnceCell<Option<Arc<Environment>>>,
|
||||
}
|
||||
|
||||
impl Default for EnvironmentManager {
|
||||
fn default() -> Self {
|
||||
Self::new(/*exec_server_url*/ None)
|
||||
}
|
||||
}
|
||||
|
||||
impl EnvironmentManager {
|
||||
/// Builds a manager from the raw `CODEX_EXEC_SERVER_URL` value.
|
||||
pub fn new(exec_server_url: Option<String>) -> Self {
|
||||
let (exec_server_url, disabled) = normalize_exec_server_url(exec_server_url);
|
||||
Self {
|
||||
exec_server_url: normalize_exec_server_url(exec_server_url),
|
||||
exec_server_url,
|
||||
disabled,
|
||||
current_environment: OnceCell::new(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Builds a manager from process environment variables.
|
||||
pub fn from_env() -> Self {
|
||||
Self::new(std::env::var(CODEX_EXEC_SERVER_URL_ENV_VAR).ok())
|
||||
}
|
||||
|
||||
/// Builds a manager from the currently selected environment, or from the
|
||||
/// disabled mode when no environment is available.
|
||||
pub fn from_environment(environment: Option<&Environment>) -> Self {
|
||||
match environment {
|
||||
Some(environment) => Self {
|
||||
exec_server_url: environment.exec_server_url().map(str::to_owned),
|
||||
disabled: false,
|
||||
current_environment: OnceCell::new(),
|
||||
},
|
||||
None => Self {
|
||||
exec_server_url: None,
|
||||
disabled: true,
|
||||
current_environment: OnceCell::new(),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the remote exec-server URL when one is configured.
|
||||
pub fn exec_server_url(&self) -> Option<&str> {
|
||||
self.exec_server_url.as_deref()
|
||||
}
|
||||
|
||||
pub async fn current(&self) -> Result<Arc<Environment>, ExecServerError> {
|
||||
/// Returns the cached environment, creating it on first access.
|
||||
pub async fn current(&self) -> Result<Option<Arc<Environment>>, ExecServerError> {
|
||||
self.current_environment
|
||||
.get_or_try_init(|| async {
|
||||
Ok(Arc::new(
|
||||
Environment::create(self.exec_server_url.clone()).await?,
|
||||
))
|
||||
if self.disabled {
|
||||
Ok(None)
|
||||
} else {
|
||||
Ok(Some(Arc::new(
|
||||
Environment::create(self.exec_server_url.clone()).await?,
|
||||
)))
|
||||
}
|
||||
})
|
||||
.await
|
||||
.map(Arc::clone)
|
||||
.map(Option::as_ref)
|
||||
.map(std::option::Option::<&Arc<Environment>>::cloned)
|
||||
}
|
||||
}
|
||||
|
||||
/// Concrete execution/filesystem environment selected for a session.
|
||||
///
|
||||
/// This bundles the selected backend together with the corresponding remote
|
||||
/// client, if any.
|
||||
#[derive(Clone)]
|
||||
pub struct Environment {
|
||||
exec_server_url: Option<String>,
|
||||
@@ -86,12 +125,19 @@ impl std::fmt::Debug for Environment {
|
||||
}
|
||||
|
||||
impl Environment {
|
||||
/// Builds an environment from the raw `CODEX_EXEC_SERVER_URL` value.
|
||||
pub async fn create(exec_server_url: Option<String>) -> Result<Self, ExecServerError> {
|
||||
let exec_server_url = normalize_exec_server_url(exec_server_url);
|
||||
let remote_exec_server_client = if let Some(url) = &exec_server_url {
|
||||
let (exec_server_url, disabled) = normalize_exec_server_url(exec_server_url);
|
||||
if disabled {
|
||||
return Err(ExecServerError::Protocol(
|
||||
"disabled mode does not create an Environment".to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
let remote_exec_server_client = if let Some(exec_server_url) = &exec_server_url {
|
||||
Some(
|
||||
ExecServerClient::connect_websocket(RemoteExecServerConnectArgs {
|
||||
websocket_url: url.clone(),
|
||||
websocket_url: exec_server_url.clone(),
|
||||
client_name: "codex-environment".to_string(),
|
||||
connect_timeout: std::time::Duration::from_secs(5),
|
||||
initialize_timeout: std::time::Duration::from_secs(5),
|
||||
@@ -102,10 +148,14 @@ impl Environment {
|
||||
None
|
||||
};
|
||||
|
||||
let exec_backend: Arc<dyn ExecBackend> =
|
||||
if let Some(client) = remote_exec_server_client.clone() {
|
||||
Arc::new(RemoteProcess::new(client))
|
||||
} else {
|
||||
let exec_backend: Arc<dyn ExecBackend> = match remote_exec_server_client.clone() {
|
||||
Some(client) => Arc::new(RemoteProcess::new(client)),
|
||||
None if exec_server_url.is_some() => {
|
||||
return Err(ExecServerError::Protocol(
|
||||
"remote mode should have an exec-server client".to_string(),
|
||||
));
|
||||
}
|
||||
None => {
|
||||
let local_process = LocalProcess::default();
|
||||
local_process
|
||||
.initialize()
|
||||
@@ -114,7 +164,8 @@ impl Environment {
|
||||
.initialized()
|
||||
.map_err(ExecServerError::Protocol)?;
|
||||
Arc::new(local_process)
|
||||
};
|
||||
}
|
||||
};
|
||||
|
||||
Ok(Self {
|
||||
exec_server_url,
|
||||
@@ -123,6 +174,11 @@ impl Environment {
|
||||
})
|
||||
}
|
||||
|
||||
pub fn is_remote(&self) -> bool {
|
||||
self.exec_server_url.is_some()
|
||||
}
|
||||
|
||||
/// Returns the remote exec-server URL when this environment is remote.
|
||||
pub fn exec_server_url(&self) -> Option<&str> {
|
||||
self.exec_server_url.as_deref()
|
||||
}
|
||||
@@ -132,27 +188,20 @@ impl Environment {
|
||||
}
|
||||
|
||||
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)
|
||||
match self.remote_exec_server_client.clone() {
|
||||
Some(client) => Arc::new(RemoteFileSystem::new(client)),
|
||||
None => Arc::new(LocalFileSystem),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn normalize_exec_server_url(exec_server_url: Option<String>) -> Option<String> {
|
||||
exec_server_url.and_then(|url| {
|
||||
let url = url.trim();
|
||||
(!url.is_empty()).then(|| url.to_string())
|
||||
})
|
||||
}
|
||||
|
||||
impl ExecutorEnvironment for Environment {
|
||||
fn get_exec_backend(&self) -> Arc<dyn ExecBackend> {
|
||||
Arc::clone(&self.exec_backend)
|
||||
fn normalize_exec_server_url(exec_server_url: Option<String>) -> (Option<String>, bool) {
|
||||
match exec_server_url.as_deref().map(str::trim) {
|
||||
None | Some("") => (None, false),
|
||||
Some(url) if url.eq_ignore_ascii_case("none") => (None, true),
|
||||
Some(url) => (Some(url.to_string()), false),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::sync::Arc;
|
||||
@@ -164,7 +213,7 @@ mod tests {
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[tokio::test]
|
||||
async fn create_without_remote_exec_server_url_does_not_connect() {
|
||||
async fn create_local_environment_does_not_connect() {
|
||||
let environment = Environment::create(/*exec_server_url*/ None)
|
||||
.await
|
||||
.expect("create environment");
|
||||
@@ -177,6 +226,15 @@ mod tests {
|
||||
fn environment_manager_normalizes_empty_url() {
|
||||
let manager = EnvironmentManager::new(Some(String::new()));
|
||||
|
||||
assert!(!manager.disabled);
|
||||
assert_eq!(manager.exec_server_url(), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn environment_manager_treats_none_value_as_disabled() {
|
||||
let manager = EnvironmentManager::new(Some("none".to_string()));
|
||||
|
||||
assert!(manager.disabled);
|
||||
assert_eq!(manager.exec_server_url(), None);
|
||||
}
|
||||
|
||||
@@ -187,9 +245,25 @@ mod tests {
|
||||
let first = manager.current().await.expect("get current environment");
|
||||
let second = manager.current().await.expect("get current environment");
|
||||
|
||||
let first = first.expect("local environment");
|
||||
let second = second.expect("local environment");
|
||||
|
||||
assert!(Arc::ptr_eq(&first, &second));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn disabled_environment_manager_has_no_current_environment() {
|
||||
let manager = EnvironmentManager::new(Some("none".to_string()));
|
||||
|
||||
assert!(
|
||||
manager
|
||||
.current()
|
||||
.await
|
||||
.expect("get current environment")
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn default_environment_has_ready_local_executor() {
|
||||
let environment = Environment::default();
|
||||
|
||||
@@ -39,6 +39,12 @@ pub type FileSystemResult<T> = io::Result<T>;
|
||||
pub trait ExecutorFileSystem: Send + Sync {
|
||||
async fn read_file(&self, path: &AbsolutePathBuf) -> FileSystemResult<Vec<u8>>;
|
||||
|
||||
/// Reads a file and decodes it as UTF-8 text.
|
||||
async fn read_file_text(&self, path: &AbsolutePathBuf) -> FileSystemResult<String> {
|
||||
let bytes = self.read_file(path).await?;
|
||||
String::from_utf8(bytes).map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))
|
||||
}
|
||||
|
||||
async fn write_file(&self, path: &AbsolutePathBuf, contents: Vec<u8>) -> FileSystemResult<()>;
|
||||
|
||||
async fn create_directory(
|
||||
|
||||
@@ -34,7 +34,6 @@ pub use codex_app_server_protocol::FsWriteFileResponse;
|
||||
pub use environment::CODEX_EXEC_SERVER_URL_ENV_VAR;
|
||||
pub use environment::Environment;
|
||||
pub use environment::EnvironmentManager;
|
||||
pub use environment::ExecutorEnvironment;
|
||||
pub use file_system::CopyOptions;
|
||||
pub use file_system::CreateDirectoryOptions;
|
||||
pub use file_system::ExecutorFileSystem;
|
||||
@@ -42,6 +41,7 @@ pub use file_system::FileMetadata;
|
||||
pub use file_system::FileSystemResult;
|
||||
pub use file_system::ReadDirectoryEntry;
|
||||
pub use file_system::RemoveOptions;
|
||||
pub use local_file_system::LOCAL_FS;
|
||||
pub use process::ExecBackend;
|
||||
pub use process::ExecProcess;
|
||||
pub use process::StartedExecProcess;
|
||||
|
||||
@@ -3,6 +3,8 @@ use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use std::path::Component;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::LazyLock;
|
||||
use std::time::SystemTime;
|
||||
use std::time::UNIX_EPOCH;
|
||||
use tokio::io;
|
||||
@@ -17,6 +19,9 @@ use crate::RemoveOptions;
|
||||
|
||||
const MAX_READ_FILE_BYTES: u64 = 512 * 1024 * 1024;
|
||||
|
||||
pub static LOCAL_FS: LazyLock<Arc<dyn ExecutorFileSystem>> =
|
||||
LazyLock::new(|| -> Arc<dyn ExecutorFileSystem> { Arc::new(LocalFileSystem) });
|
||||
|
||||
#[derive(Clone, Default)]
|
||||
pub(crate) struct LocalFileSystem;
|
||||
|
||||
|
||||
@@ -23,6 +23,7 @@ use crate::ReadDirectoryEntry;
|
||||
use crate::RemoveOptions;
|
||||
|
||||
const INVALID_REQUEST_ERROR_CODE: i64 = -32600;
|
||||
const NOT_FOUND_ERROR_CODE: i64 = -32004;
|
||||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct RemoteFileSystem {
|
||||
@@ -151,6 +152,9 @@ impl ExecutorFileSystem for RemoteFileSystem {
|
||||
|
||||
fn map_remote_error(error: ExecServerError) -> io::Error {
|
||||
match error {
|
||||
ExecServerError::Server { code, message } if code == NOT_FOUND_ERROR_CODE => {
|
||||
io::Error::new(io::ErrorKind::NotFound, message)
|
||||
}
|
||||
ExecServerError::Server { code, message } if code == INVALID_REQUEST_ERROR_CODE => {
|
||||
io::Error::new(io::ErrorKind::InvalidInput, message)
|
||||
}
|
||||
|
||||
@@ -356,6 +356,14 @@ pub(crate) fn invalid_params(message: String) -> JSONRPCErrorError {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn not_found(message: String) -> JSONRPCErrorError {
|
||||
JSONRPCErrorError {
|
||||
code: -32004,
|
||||
data: None,
|
||||
message,
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn internal_error(message: String) -> JSONRPCErrorError {
|
||||
JSONRPCErrorError {
|
||||
code: -32603,
|
||||
|
||||
@@ -26,6 +26,7 @@ use crate::RemoveOptions;
|
||||
use crate::local_file_system::LocalFileSystem;
|
||||
use crate::rpc::internal_error;
|
||||
use crate::rpc::invalid_request;
|
||||
use crate::rpc::not_found;
|
||||
|
||||
#[derive(Clone, Default)]
|
||||
pub(crate) struct FileSystemHandler {
|
||||
@@ -153,7 +154,9 @@ impl FileSystemHandler {
|
||||
}
|
||||
|
||||
fn map_fs_error(err: io::Error) -> JSONRPCErrorError {
|
||||
if err.kind() == io::ErrorKind::InvalidInput {
|
||||
if err.kind() == io::ErrorKind::NotFound {
|
||||
not_found(err.to_string())
|
||||
} else if err.kind() == io::ErrorKind::InvalidInput {
|
||||
invalid_request(err.to_string())
|
||||
} else {
|
||||
internal_error(err.to_string())
|
||||
|
||||
Reference in New Issue
Block a user