diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 0f61148d63..b6aff710ee 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -941,7 +941,7 @@ impl TurnContext { sandbox_policy: self.sandbox_policy.get(), windows_sandbox_level: self.windows_sandbox_level, }) - .with_attached_executor(self.environment.attached_executor()) + .with_executor_attachment(self.environment.executor_attachment()) .with_unified_exec_shell_mode(self.tools_config.unified_exec_shell_mode.clone()) .with_web_search_config(self.tools_config.web_search_config.clone()) .with_allow_login_shell(self.tools_config.allow_login_shell) @@ -1396,7 +1396,7 @@ impl Session { sandbox_policy: session_configuration.sandbox_policy.get(), windows_sandbox_level: session_configuration.windows_sandbox_level, }) - .with_attached_executor(environment.attached_executor()) + .with_executor_attachment(environment.executor_attachment()) .with_unified_exec_shell_mode_for_session( user_shell, shell_zsh_path, @@ -5474,7 +5474,7 @@ async fn spawn_review_thread( sess.services.shell_zsh_path.as_ref(), sess.services.main_execve_wrapper_exe.as_ref(), ) - .with_attached_executor(parent_turn_context.environment.attached_executor()) + .with_executor_attachment(parent_turn_context.environment.executor_attachment()) .with_web_search_config(/*web_search_config*/ None) .with_allow_login_shell(config.permissions.allow_login_shell) .with_agent_roles(config.agent_roles.clone()); diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index f5e5d4463e..2ff9e7767e 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -5259,7 +5259,12 @@ async fn unified_exec_rejects_escalated_permissions_when_policy_not_on_request() let turn_context = Arc::new(turn_context_raw); let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())); - let handler = UnifiedExecHandler; + let handler = UnifiedExecHandler::new( + turn_context + .environment + .executor_attachment() + .expect("test environment has an executor attachment"), + ); let resp = handler .handle(ToolInvocation { session: Arc::clone(&session), diff --git a/codex-rs/core/src/codex_tests_guardian.rs b/codex-rs/core/src/codex_tests_guardian.rs index f5698d58ff..72de22f7e5 100644 --- a/codex-rs/core/src/codex_tests_guardian.rs +++ b/codex-rs/core/src/codex_tests_guardian.rs @@ -207,7 +207,12 @@ async fn guardian_allows_unified_exec_additional_permissions_requests_past_polic let turn_context = Arc::new(turn_context_raw); let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())); - let handler = UnifiedExecHandler; + let handler = UnifiedExecHandler::new( + turn_context + .environment + .executor_attachment() + .expect("test environment has an executor attachment"), + ); let resp = handler .handle(ToolInvocation { session: Arc::clone(&session), diff --git a/codex-rs/core/src/tools/handlers/apply_patch.rs b/codex-rs/core/src/tools/handlers/apply_patch.rs index 92aed375e1..5b36c8521f 100644 --- a/codex-rs/core/src/tools/handlers/apply_patch.rs +++ b/codex-rs/core/src/tools/handlers/apply_patch.rs @@ -31,7 +31,7 @@ use crate::tools::spec::JsonSchema; use async_trait::async_trait; use codex_apply_patch::ApplyPatchAction; use codex_apply_patch::ApplyPatchFileChange; -use codex_exec_server::AttachedExecutor; +use codex_exec_server::ExecutorAttachment; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::PermissionProfile; use codex_sandboxing::policy_transforms::effective_file_system_sandbox_policy; @@ -42,13 +42,13 @@ use std::collections::BTreeSet; use std::sync::Arc; pub struct ApplyPatchHandler { - _attached_executor: Arc, + executor_attachment: Arc, } impl ApplyPatchHandler { - pub fn new(attached_executor: Arc) -> Self { + pub fn new(executor_attachment: Arc) -> Self { Self { - _attached_executor: attached_executor, + executor_attachment, } } } @@ -224,7 +224,8 @@ impl ToolHandler for ApplyPatchHandler { }; let mut orchestrator = ToolOrchestrator::new(); - let mut runtime = ApplyPatchRuntime::new(); + let mut runtime = + ApplyPatchRuntime::new(Arc::clone(&self.executor_attachment)); let tool_ctx = ToolCtx { session: session.clone(), turn: turn.clone(), @@ -303,6 +304,12 @@ pub(crate) async fn intercept_apply_patch( Ok(Some(FunctionToolOutput::from_text(content, Some(true)))) } InternalApplyPatchInvocation::DelegateToExec(apply) => { + let Some(executor_attachment) = turn.environment.executor_attachment() else { + return Err(FunctionCallError::RespondToModel( + "apply_patch interception requires an executor attachment".to_string(), + )); + }; + let changes = convert_apply_patch_to_protocol(&apply.action); let emitter = ToolEmitter::apply_patch(changes.clone(), apply.auto_approved); let event_ctx = ToolEventCtx::new( @@ -326,7 +333,7 @@ pub(crate) async fn intercept_apply_patch( }; let mut orchestrator = ToolOrchestrator::new(); - let mut runtime = ApplyPatchRuntime::new(); + let mut runtime = ApplyPatchRuntime::new(executor_attachment); let tool_ctx = ToolCtx { session: session.clone(), turn: turn.clone(), diff --git a/codex-rs/core/src/tools/handlers/list_dir.rs b/codex-rs/core/src/tools/handlers/list_dir.rs index faf43785c1..340345effe 100644 --- a/codex-rs/core/src/tools/handlers/list_dir.rs +++ b/codex-rs/core/src/tools/handlers/list_dir.rs @@ -6,7 +6,9 @@ use std::path::PathBuf; use std::sync::Arc; use async_trait::async_trait; -use codex_exec_server::AttachedExecutor; +use codex_exec_server::ExecutorAttachment; +use codex_exec_server::ReadDirectoryEntry; +use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_string::take_bytes_at_char_boundary; use serde::Deserialize; use tokio::fs; @@ -20,13 +22,13 @@ use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; pub struct ListDirHandler { - _attached_executor: Arc, + executor_attachment: Arc, } impl ListDirHandler { - pub fn new(attached_executor: Arc) -> Self { + pub fn new(executor_attachment: Arc) -> Self { Self { - _attached_executor: attached_executor, + executor_attachment, } } } @@ -111,7 +113,8 @@ impl ToolHandler for ListDirHandler { )); } - let entries = list_dir_slice(&path, offset, limit, depth).await?; + let entries = + list_dir_slice(&self.executor_attachment, &path, offset, limit, depth).await?; let mut output = Vec::with_capacity(entries.len() + 1); output.push(format!("Absolute path: {}", path.display())); output.extend(entries); @@ -120,13 +123,21 @@ impl ToolHandler for ListDirHandler { } async fn list_dir_slice( + executor_attachment: &ExecutorAttachment, path: &Path, offset: usize, limit: usize, depth: usize, ) -> Result, FunctionCallError> { let mut entries = Vec::new(); - collect_entries(path, Path::new(""), depth, &mut entries).await?; + collect_entries( + executor_attachment, + path, + Path::new(""), + depth, + &mut entries, + ) + .await?; if entries.is_empty() { return Ok(Vec::new()); @@ -159,6 +170,7 @@ async fn list_dir_slice( } async fn collect_entries( + executor_attachment: &ExecutorAttachment, dir_path: &Path, relative_prefix: &Path, depth: usize, @@ -168,41 +180,18 @@ async fn collect_entries( queue.push_back((dir_path.to_path_buf(), relative_prefix.to_path_buf(), depth)); while let Some((current_dir, prefix, remaining_depth)) = queue.pop_front() { - let mut read_dir = fs::read_dir(¤t_dir).await.map_err(|err| { - FunctionCallError::RespondToModel(format!("failed to read directory: {err}")) - })?; - let mut dir_entries = Vec::new(); - while let Some(entry) = read_dir.next_entry().await.map_err(|err| { - FunctionCallError::RespondToModel(format!("failed to read directory: {err}")) - })? { - let file_type = entry.file_type().await.map_err(|err| { - FunctionCallError::RespondToModel(format!("failed to inspect entry: {err}")) - })?; - - let file_name = entry.file_name(); - let relative_path = if prefix.as_os_str().is_empty() { - PathBuf::from(&file_name) - } else { - prefix.join(&file_name) - }; - - let display_name = format_entry_component(&file_name); - let display_depth = prefix.components().count(); - let sort_key = format_entry_name(&relative_path); - let kind = DirEntryKind::from(&file_type); - dir_entries.push(( - entry.path(), - relative_path, - kind, - DirEntry { - name: sort_key, - display_name, - depth: display_depth, - kind, - }, - )); + if executor_attachment.exec_server_url().is_some() { + collect_remote_dir_entries( + executor_attachment, + ¤t_dir, + &prefix, + &mut dir_entries, + ) + .await?; + } else { + collect_local_dir_entries(¤t_dir, &prefix, &mut dir_entries).await?; } dir_entries.sort_unstable_by(|a, b| a.3.name.cmp(&b.3.name)); @@ -218,6 +207,96 @@ async fn collect_entries( Ok(()) } +async fn collect_local_dir_entries( + current_dir: &Path, + prefix: &Path, + dir_entries: &mut Vec<(PathBuf, PathBuf, DirEntryKind, DirEntry)>, +) -> Result<(), FunctionCallError> { + let mut read_dir = fs::read_dir(current_dir).await.map_err(|err| { + FunctionCallError::RespondToModel(format!("failed to read directory: {err}")) + })?; + + while let Some(entry) = read_dir.next_entry().await.map_err(|err| { + FunctionCallError::RespondToModel(format!("failed to read directory: {err}")) + })? { + let file_type = entry.file_type().await.map_err(|err| { + FunctionCallError::RespondToModel(format!("failed to inspect entry: {err}")) + })?; + let file_name = entry.file_name(); + let relative_path = relative_child_path(prefix, &file_name); + let kind = DirEntryKind::from(&file_type); + dir_entries.push(( + entry.path(), + relative_path.clone(), + kind, + to_dir_entry(relative_path, &file_name, kind), + )); + } + + Ok(()) +} + +async fn collect_remote_dir_entries( + executor_attachment: &ExecutorAttachment, + current_dir: &Path, + prefix: &Path, + dir_entries: &mut Vec<(PathBuf, PathBuf, DirEntryKind, DirEntry)>, +) -> Result<(), FunctionCallError> { + let current_dir = AbsolutePathBuf::from_absolute_path(current_dir).map_err(|err| { + FunctionCallError::RespondToModel(format!("failed to inspect directory path: {err}")) + })?; + let entries = executor_attachment + .get_filesystem() + .read_directory(¤t_dir) + .await + .map_err(|err| { + FunctionCallError::RespondToModel(format!("failed to read directory: {err}")) + })?; + + for entry in entries { + let file_name = OsStr::new(entry.file_name.as_str()); + let relative_path = relative_child_path(prefix, file_name); + let kind = DirEntryKind::from(&entry); + dir_entries.push(( + current_dir + .join(entry.file_name.as_str()) + .map_err(|err| { + FunctionCallError::RespondToModel(format!( + "failed to inspect entry path: {err}" + )) + })? + .into_path_buf(), + relative_path.clone(), + kind, + to_dir_entry(relative_path, file_name, kind), + )); + } + + Ok(()) +} + +fn relative_child_path(prefix: &Path, file_name: &OsStr) -> PathBuf { + if prefix.as_os_str().is_empty() { + PathBuf::from(file_name) + } else { + prefix.join(file_name) + } +} + +fn to_dir_entry(relative_path: PathBuf, file_name: &OsStr, kind: DirEntryKind) -> DirEntry { + let display_name = format_entry_component(file_name); + let display_depth = relative_path + .parent() + .map_or(0, |prefix| prefix.components().count()); + let sort_key = format_entry_name(&relative_path); + DirEntry { + name: sort_key, + display_name, + depth: display_depth, + kind, + } +} + fn format_entry_name(path: &Path) -> String { let normalized = path.to_string_lossy().replace("\\", "/"); if normalized.len() > MAX_ENTRY_LENGTH { @@ -278,6 +357,20 @@ impl From<&FileType> for DirEntryKind { } } +impl From<&ReadDirectoryEntry> for DirEntryKind { + fn from(entry: &ReadDirectoryEntry) -> Self { + // The remote directory API currently exposes directory/file booleans, but not a symlink + // bit, so remote listings preserve entries but cannot render the local "@" suffix. + if entry.is_directory { + DirEntryKind::Directory + } else if entry.is_file { + DirEntryKind::File + } else { + DirEntryKind::Other + } + } +} + #[cfg(test)] #[path = "list_dir_tests.rs"] mod tests; diff --git a/codex-rs/core/src/tools/handlers/list_dir_tests.rs b/codex-rs/core/src/tools/handlers/list_dir_tests.rs index ccc30b5326..b136b56b3b 100644 --- a/codex-rs/core/src/tools/handlers/list_dir_tests.rs +++ b/codex-rs/core/src/tools/handlers/list_dir_tests.rs @@ -1,7 +1,16 @@ use super::*; +use codex_exec_server::Environment; +use codex_exec_server::ExecutorAttachment; use pretty_assertions::assert_eq; +use std::sync::Arc; use tempfile::tempdir; +fn test_executor_attachment() -> Arc { + Environment::default() + .executor_attachment() + .expect("default environment should have an executor attachment") +} + #[tokio::test] async fn lists_directory_entries() { let temp = tempdir().expect("create tempdir"); @@ -34,8 +43,13 @@ async fn lists_directory_entries() { symlink(dir_path.join("entry.txt"), &link_path).expect("create symlink"); } + let executor_attachment = test_executor_attachment(); let entries = list_dir_slice( - dir_path, /*offset*/ 1, /*limit*/ 20, /*depth*/ 3, + executor_attachment.as_ref(), + dir_path, + /*offset*/ 1, + /*limit*/ 20, + /*depth*/ 3, ) .await .expect("list directory"); @@ -70,8 +84,13 @@ async fn errors_when_offset_exceeds_entries() { .await .expect("create sub dir"); + let executor_attachment = test_executor_attachment(); let err = list_dir_slice( - dir_path, /*offset*/ 10, /*limit*/ 1, /*depth*/ 2, + executor_attachment.as_ref(), + dir_path, + /*offset*/ 10, + /*limit*/ 1, + /*depth*/ 2, ) .await .expect_err("offset exceeds entries"); @@ -99,8 +118,13 @@ async fn respects_depth_parameter() { .await .expect("write deeper"); + let executor_attachment = test_executor_attachment(); let entries_depth_one = list_dir_slice( - dir_path, /*offset*/ 1, /*limit*/ 10, /*depth*/ 1, + executor_attachment.as_ref(), + dir_path, + /*offset*/ 1, + /*limit*/ 10, + /*depth*/ 1, ) .await .expect("list depth 1"); @@ -110,7 +134,11 @@ async fn respects_depth_parameter() { ); let entries_depth_two = list_dir_slice( - dir_path, /*offset*/ 1, /*limit*/ 20, /*depth*/ 2, + executor_attachment.as_ref(), + dir_path, + /*offset*/ 1, + /*limit*/ 20, + /*depth*/ 2, ) .await .expect("list depth 2"); @@ -125,7 +153,11 @@ async fn respects_depth_parameter() { ); let entries_depth_three = list_dir_slice( - dir_path, /*offset*/ 1, /*limit*/ 30, /*depth*/ 3, + executor_attachment.as_ref(), + dir_path, + /*offset*/ 1, + /*limit*/ 30, + /*depth*/ 3, ) .await .expect("list depth 3"); @@ -158,8 +190,13 @@ async fn paginates_in_sorted_order() { .await .expect("write b child"); + let executor_attachment = test_executor_attachment(); let first_page = list_dir_slice( - dir_path, /*offset*/ 1, /*limit*/ 2, /*depth*/ 2, + executor_attachment.as_ref(), + dir_path, + /*offset*/ 1, + /*limit*/ 2, + /*depth*/ 2, ) .await .expect("list page one"); @@ -173,7 +210,11 @@ async fn paginates_in_sorted_order() { ); let second_page = list_dir_slice( - dir_path, /*offset*/ 3, /*limit*/ 2, /*depth*/ 2, + executor_attachment.as_ref(), + dir_path, + /*offset*/ 3, + /*limit*/ 2, + /*depth*/ 2, ) .await .expect("list page two"); @@ -197,9 +238,16 @@ async fn handles_large_limit_without_overflow() { .await .expect("write gamma"); - let entries = list_dir_slice(dir_path, /*offset*/ 2, usize::MAX, /*depth*/ 1) - .await - .expect("list without overflow"); + let executor_attachment = test_executor_attachment(); + let entries = list_dir_slice( + executor_attachment.as_ref(), + dir_path, + /*offset*/ 2, + usize::MAX, + /*depth*/ 1, + ) + .await + .expect("list without overflow"); assert_eq!( entries, vec!["beta.txt".to_string(), "gamma.txt".to_string(),] @@ -218,8 +266,13 @@ async fn indicates_truncated_results() { .expect("write file"); } + let executor_attachment = test_executor_attachment(); let entries = list_dir_slice( - dir_path, /*offset*/ 1, /*limit*/ 25, /*depth*/ 1, + executor_attachment.as_ref(), + dir_path, + /*offset*/ 1, + /*limit*/ 25, + /*depth*/ 1, ) .await .expect("list directory"); @@ -242,8 +295,13 @@ async fn truncation_respects_sorted_order() -> anyhow::Result<()> { tokio::fs::write(nested.join("child.txt"), b"child").await?; tokio::fs::write(deeper.join("grandchild.txt"), b"deep").await?; + let executor_attachment = test_executor_attachment(); let entries_depth_three = list_dir_slice( - dir_path, /*offset*/ 1, /*limit*/ 3, /*depth*/ 3, + executor_attachment.as_ref(), + dir_path, + /*offset*/ 1, + /*limit*/ 3, + /*depth*/ 3, ) .await?; assert_eq!( diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index bebb7c78d2..5f30f1e17e 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -27,7 +27,7 @@ use crate::unified_exec::UnifiedExecContext; use crate::unified_exec::UnifiedExecProcessManager; use crate::unified_exec::WriteStdinRequest; use async_trait::async_trait; -use codex_exec_server::AttachedExecutor; +use codex_exec_server::ExecutorAttachment; use codex_features::Feature; use codex_otel::SessionTelemetry; use codex_otel::metrics::names::TOOL_CALL_UNIFIED_EXEC_METRIC; @@ -37,12 +37,14 @@ use std::path::PathBuf; use std::sync::Arc; pub struct UnifiedExecHandler { - attached_executor: Arc, + executor_attachment: Arc, } impl UnifiedExecHandler { - pub fn new(attached_executor: Arc) -> Self { - Self { attached_executor } + pub fn new(executor_attachment: Arc) -> Self { + Self { + executor_attachment, + } } } @@ -192,7 +194,7 @@ impl ToolHandler for UnifiedExecHandler { session.clone(), turn.clone(), call_id.clone(), - Arc::clone(&self.attached_executor), + Arc::clone(&self.executor_attachment), ); let response = match tool_name.as_str() { diff --git a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs index 11b75b28d4..18d99dc9a2 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec_tests.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec_tests.rs @@ -17,8 +17,17 @@ use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::registry::ToolHandler; use crate::turn_diff_tracker::TurnDiffTracker; +use codex_exec_server::Environment; use tokio::sync::Mutex; +fn unified_exec_handler() -> UnifiedExecHandler { + UnifiedExecHandler::new( + Environment::default() + .executor_attachment() + .expect("default environment has an executor attachment"), + ) +} + #[test] fn test_get_command_uses_default_shell_when_unspecified() -> anyhow::Result<()> { let json = r#"{"cmd": "echo hello"}"#; @@ -202,7 +211,7 @@ async fn exec_command_pre_tool_use_payload_uses_raw_command() { arguments: serde_json::json!({ "cmd": "printf exec command" }).to_string(), }; let (session, turn) = make_session_and_context().await; - let handler = UnifiedExecHandler; + let handler = unified_exec_handler(); assert_eq!( handler.pre_tool_use_payload(&ToolInvocation { @@ -226,7 +235,7 @@ async fn exec_command_pre_tool_use_payload_skips_write_stdin() { arguments: serde_json::json!({ "chars": "echo hi" }).to_string(), }; let (session, turn) = make_session_and_context().await; - let handler = UnifiedExecHandler; + let handler = unified_exec_handler(); assert_eq!( handler.pre_tool_use_payload(&ToolInvocation { @@ -263,8 +272,10 @@ fn exec_command_post_tool_use_payload_uses_output_for_noninteractive_one_shot_co ]), }; + let handler = unified_exec_handler(); + assert_eq!( - UnifiedExecHandler.post_tool_use_payload("call-43", &payload, &output), + handler.post_tool_use_payload("call-43", &payload, &output), Some(crate::tools::registry::PostToolUsePayload { command: "echo three".to_string(), tool_response: serde_json::json!("three"), @@ -293,8 +304,10 @@ fn exec_command_post_tool_use_payload_skips_interactive_exec() { ]), }; + let handler = unified_exec_handler(); + assert_eq!( - UnifiedExecHandler.post_tool_use_payload("call-44", &payload, &output), + handler.post_tool_use_payload("call-44", &payload, &output), None ); } @@ -320,8 +333,10 @@ fn exec_command_post_tool_use_payload_skips_running_sessions() { ]), }; + let handler = unified_exec_handler(); + assert_eq!( - UnifiedExecHandler.post_tool_use_payload("call-45", &payload, &output), + handler.post_tool_use_payload("call-45", &payload, &output), None ); } diff --git a/codex-rs/core/src/tools/handlers/view_image.rs b/codex-rs/core/src/tools/handlers/view_image.rs index 0da22b2f6c..001a94f4de 100644 --- a/codex-rs/core/src/tools/handlers/view_image.rs +++ b/codex-rs/core/src/tools/handlers/view_image.rs @@ -21,15 +21,17 @@ use crate::tools::context::ToolPayload; use crate::tools::handlers::parse_arguments; use crate::tools::registry::ToolHandler; use crate::tools::registry::ToolKind; -use codex_exec_server::AttachedExecutor; +use codex_exec_server::ExecutorAttachment; pub struct ViewImageHandler { - attached_executor: Arc, + executor_attachment: Arc, } impl ViewImageHandler { - pub fn new(attached_executor: Arc) -> Self { - Self { attached_executor } + pub fn new(executor_attachment: Arc) -> Self { + Self { + executor_attachment, + } } } @@ -105,7 +107,7 @@ impl ToolHandler for ViewImageHandler { })?; let metadata = self - .attached_executor + .executor_attachment .get_filesystem() .get_metadata(&abs_path) .await @@ -123,7 +125,7 @@ impl ToolHandler for ViewImageHandler { ))); } let file_bytes = self - .attached_executor + .executor_attachment .get_filesystem() .read_file(&abs_path) .await diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index 65a3981cca..d4c86aee09 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -4,12 +4,17 @@ //! decision to avoid re-prompting, builds the self-invocation command for //! `codex --codex-run-as-apply-patch`, and runs under the current //! `SandboxAttempt` with a minimal environment. +use crate::error::CodexErr; +use crate::error::SandboxErr; use crate::exec::ExecCapturePolicy; use crate::exec::ExecToolCallOutput; +use crate::exec::StreamOutput; +use crate::exec::is_likely_sandbox_denied; use crate::guardian::GuardianApprovalRequest; use crate::guardian::review_approval_request; use crate::guardian::routes_approval_to_guardian; use crate::sandboxing::ExecOptions; +use crate::sandboxing::ExecRequest; use crate::sandboxing::execute_env; use crate::tools::sandboxing::Approvable; use crate::tools::sandboxing::ApprovalCtx; @@ -22,6 +27,10 @@ use crate::tools::sandboxing::ToolRuntime; use crate::tools::sandboxing::with_cached_approval; use codex_apply_patch::ApplyPatchAction; use codex_apply_patch::CODEX_CORE_APPLY_PATCH_ARG1; +use codex_exec_server::ExecOutputStream as ExecutorOutputStream; +use codex_exec_server::ExecParams as ExecutorExecParams; +use codex_exec_server::ExecutorAttachment; +use codex_exec_server::ProcessId; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::FileChange; @@ -31,7 +40,13 @@ use codex_sandboxing::SandboxablePreference; use codex_utils_absolute_path::AbsolutePathBuf; use futures::future::BoxFuture; use std::collections::HashMap; +use std::io; use std::path::PathBuf; +use std::sync::Arc; +use std::time::Duration; +use std::time::Instant; + +const EXEC_TIMEOUT_EXIT_CODE: i32 = 124; #[derive(Debug)] pub struct ApplyPatchRequest { @@ -44,12 +59,15 @@ pub struct ApplyPatchRequest { pub timeout_ms: Option, } -#[derive(Default)] -pub struct ApplyPatchRuntime; +pub struct ApplyPatchRuntime { + executor_attachment: Arc, +} impl ApplyPatchRuntime { - pub fn new() -> Self { - Self + pub fn new(executor_attachment: Arc) -> Self { + Self { + executor_attachment, + } } fn build_guardian_review_request( @@ -116,6 +134,141 @@ impl ApplyPatchRuntime { tx_event: ctx.session.get_tx_event(), }) } + + async fn execute_request( + &self, + env: ExecRequest, + ctx: &ToolCtx, + ) -> Result { + let start = Instant::now(); + let out = if self.executor_attachment.exec_server_url().is_some() { + self.execute_request_remote(env, ctx).await? + } else { + execute_env(env, Self::stdout_stream(ctx)).await? + }; + let duration = start.elapsed(); + + let mut out = out; + out.duration = duration; + Ok(out) + } + + async fn execute_request_remote( + &self, + env: ExecRequest, + ctx: &ToolCtx, + ) -> Result { + let started = self + .executor_attachment + .get_exec_backend() + .start(ExecutorExecParams { + process_id: ProcessId::new(format!("apply-patch-{}", ctx.call_id)), + argv: env.command.clone(), + cwd: env.cwd.clone(), + env: env.env.clone(), + tty: false, + arg0: env.arg0.clone(), + }) + .await + .map_err(|err| CodexErr::Io(io::Error::other(err)))?; + + let process = started.process; + let mut wake_rx = process.subscribe_wake(); + let mut after_seq = None; + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + let mut aggregated_output = Vec::new(); + let mut exit_code = None; + let mut timed_out = false; + let expiration = env.expiration.clone(); + let capture_policy = env.capture_policy; + let expiration_wait = async move { + if matches!(capture_policy, ExecCapturePolicy::ShellTool) { + expiration.wait().await; + } else { + std::future::pending::<()>().await; + } + }; + tokio::pin!(expiration_wait); + + loop { + let response = process + .read(after_seq, /*max_bytes*/ None, /*wait_ms*/ Some(0)) + .await + .map_err(|err| CodexErr::Io(io::Error::other(err)))?; + + for chunk in response.chunks { + let bytes = chunk.chunk.into_inner(); + match chunk.stream { + ExecutorOutputStream::Stdout | ExecutorOutputStream::Pty => { + stdout.extend_from_slice(&bytes); + } + ExecutorOutputStream::Stderr => { + stderr.extend_from_slice(&bytes); + } + } + aggregated_output.extend_from_slice(&bytes); + } + + if let Some(message) = response.failure { + return Err(CodexErr::Io(io::Error::other(message))); + } + + if response.exited { + exit_code = response.exit_code; + } + + if response.closed { + break; + } + + after_seq = response.next_seq.checked_sub(1); + tokio::select! { + wake_result = wake_rx.changed() => { + if wake_result.is_err() { + return Err(CodexErr::Io(io::Error::other( + "exec-server wake channel closed", + ))); + } + } + _ = &mut expiration_wait, if !timed_out => { + process + .terminate() + .await + .map_err(|err| CodexErr::Io(io::Error::other(err)))?; + timed_out = true; + exit_code = Some(EXEC_TIMEOUT_EXIT_CODE); + break; + } + } + } + + let output = ExecToolCallOutput { + exit_code: exit_code.unwrap_or(-1), + stdout: StreamOutput::new(String::from_utf8_lossy(&stdout).to_string()), + stderr: StreamOutput::new(String::from_utf8_lossy(&stderr).to_string()), + aggregated_output: StreamOutput::new( + String::from_utf8_lossy(&aggregated_output).to_string(), + ), + duration: Duration::ZERO, + timed_out, + }; + + if timed_out { + return Err(CodexErr::Sandbox(SandboxErr::Timeout { + output: Box::new(output), + })); + } + + if is_likely_sandbox_denied(env.sandbox, &output) { + return Err(CodexErr::Sandbox(SandboxErr::Denied { + output: Box::new(output), + network_policy_decision: None, + })); + } + + Ok(output) + } } impl Sandboxable for ApplyPatchRuntime { @@ -223,7 +376,8 @@ impl ToolRuntime for ApplyPatchRuntime { let env = attempt .env_for(command, options, /*network*/ None) .map_err(|err| ToolError::Codex(err.into()))?; - let out = execute_env(env, Self::stdout_stream(ctx)) + let out = self + .execute_request(env, ctx) .await .map_err(ToolError::Codex)?; Ok(out) diff --git a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs index a94f1f99b8..40d630f788 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch_tests.rs @@ -1,13 +1,20 @@ use super::*; +use codex_exec_server::Environment; use codex_protocol::protocol::GranularApprovalConfig; use pretty_assertions::assert_eq; use std::collections::HashMap; #[cfg(not(target_os = "windows"))] use std::path::PathBuf; -#[test] -fn wants_no_sandbox_approval_granular_respects_sandbox_flag() { - let runtime = ApplyPatchRuntime::new(); +#[tokio::test] +async fn wants_no_sandbox_approval_granular_respects_sandbox_flag() { + let runtime = ApplyPatchRuntime::new( + Environment::create(/*exec_server_url*/ None) + .await + .expect("create environment") + .executor_attachment() + .expect("default environment should have an executor attachment"), + ); assert!(runtime.wants_no_sandbox_approval(AskForApproval::OnRequest)); assert!( !runtime.wants_no_sandbox_approval(AskForApproval::Granular(GranularApprovalConfig { diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index 383a72f69b..238d927d3e 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -37,7 +37,7 @@ use crate::unified_exec::NoopSpawnLifecycle; use crate::unified_exec::UnifiedExecError; use crate::unified_exec::UnifiedExecProcess; use crate::unified_exec::UnifiedExecProcessManager; -use codex_exec_server::AttachedExecutor; +use codex_exec_server::ExecutorAttachment; use codex_network_proxy::NetworkProxy; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::ReviewDecision; @@ -81,7 +81,7 @@ pub struct UnifiedExecApprovalKey { /// unified-exec side while delegating process startup to the manager. pub struct UnifiedExecRuntime<'a> { manager: &'a UnifiedExecProcessManager, - attached_executor: Arc, + executor_attachment: Arc, shell_mode: UnifiedExecShellMode, } @@ -89,12 +89,12 @@ impl<'a> UnifiedExecRuntime<'a> { /// Creates a runtime bound to the shared unified-exec process manager. pub fn new( manager: &'a UnifiedExecProcessManager, - attached_executor: Arc, + executor_attachment: Arc, shell_mode: UnifiedExecShellMode, ) -> Self { Self { manager, - attached_executor, + executor_attachment, shell_mode, } } @@ -247,7 +247,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt .await? { Some(prepared) => { - if self.attached_executor.exec_server_url().is_some() { + if self.executor_attachment.exec_server_url().is_some() { return Err(ToolError::Rejected( "unified_exec zsh-fork is not supported when exec_server_url is configured".to_string(), )); @@ -259,7 +259,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt &prepared.exec_request, req.tty, prepared.spawn_lifecycle, - self.attached_executor.as_ref(), + self.executor_attachment.as_ref(), ) .await .map_err(|err| match err { @@ -295,7 +295,7 @@ impl<'a> ToolRuntime for UnifiedExecRunt &exec_env, req.tty, Box::new(NoopSpawnLifecycle), - self.attached_executor.as_ref(), + self.executor_attachment.as_ref(), ) .await .map_err(|err| match err { diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index 8e26997de5..dc77d684fc 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -21,7 +21,7 @@ use crate::tools::handlers::request_permissions_tool_description; use crate::tools::handlers::request_user_input_tool_description; use crate::tools::registry::ToolRegistryBuilder; use crate::tools::registry::tool_handler_key; -use codex_exec_server::AttachedExecutor; +use codex_exec_server::ExecutorAttachment; use codex_features::Feature; use codex_features::Features; use codex_protocol::config_types::WebSearchConfig; @@ -173,7 +173,7 @@ pub(crate) struct ToolsConfig { pub experimental_supported_tools: Vec, pub agent_jobs_tools: bool, pub agent_jobs_worker_tools: bool, - pub attached_executor: Option>, + pub executor_attachment: Option>, } pub(crate) struct ToolsConfigParams<'a> { @@ -308,15 +308,15 @@ impl ToolsConfig { experimental_supported_tools: model_info.experimental_supported_tools.clone(), agent_jobs_tools: include_agent_jobs, agent_jobs_worker_tools, - attached_executor: None, + executor_attachment: None, } } - pub fn with_attached_executor( + pub fn with_executor_attachment( mut self, - attached_executor: Option>, + executor_attachment: Option>, ) -> Self { - self.attached_executor = attached_executor; + self.executor_attachment = executor_attachment; self } @@ -403,8 +403,9 @@ pub(crate) fn build_specs( dynamic_tools: &[DynamicToolSpec], ) -> ToolRegistryBuilder { let mut config = config.clone(); - if config.attached_executor.is_none() { - config.attached_executor = codex_exec_server::Environment::default().attached_executor(); + if config.executor_attachment.is_none() { + config.executor_attachment = + codex_exec_server::Environment::default().executor_attachment(); } build_specs_with_discoverable_tools( &config, @@ -504,8 +505,9 @@ pub(crate) fn build_specs_with_discoverable_tools( builder.register_handler(WAIT_TOOL_NAME, code_mode_wait_handler); } - if let Some(attached_executor) = config.attached_executor.as_ref() { - let unified_exec_handler = Arc::new(UnifiedExecHandler::new(Arc::clone(attached_executor))); + if let Some(executor_attachment) = config.executor_attachment.as_ref() { + let unified_exec_handler = + Arc::new(UnifiedExecHandler::new(Arc::clone(executor_attachment))); match &config.shell_type { ConfigShellToolType::Default => { push_tool_spec( @@ -601,7 +603,7 @@ pub(crate) fn build_specs_with_discoverable_tools( ); builder.register_handler("update_plan", plan_handler); - if config.attached_executor.is_some() && config.js_repl_enabled { + if config.executor_attachment.is_some() && config.js_repl_enabled { push_tool_spec( &mut builder, create_js_repl_tool(), @@ -675,10 +677,10 @@ pub(crate) fn build_specs_with_discoverable_tools( builder.register_handler(TOOL_SUGGEST_TOOL_NAME, tool_suggest_handler); } - if let Some(attached_executor) = config.attached_executor.as_ref() + if let Some(executor_attachment) = config.executor_attachment.as_ref() && let Some(apply_patch_tool_type) = &config.apply_patch_tool_type { - let apply_patch_handler = Arc::new(ApplyPatchHandler::new(Arc::clone(attached_executor))); + let apply_patch_handler = Arc::new(ApplyPatchHandler::new(Arc::clone(executor_attachment))); match apply_patch_tool_type { ApplyPatchToolType::Freeform => { push_tool_spec( @@ -700,13 +702,13 @@ pub(crate) fn build_specs_with_discoverable_tools( builder.register_handler("apply_patch", apply_patch_handler); } - if let Some(attached_executor) = config.attached_executor.as_ref() + if let Some(executor_attachment) = config.executor_attachment.as_ref() && config .experimental_supported_tools .iter() .any(|tool| tool == "list_dir") { - let list_dir_handler = Arc::new(ListDirHandler::new(Arc::clone(attached_executor))); + let list_dir_handler = Arc::new(ListDirHandler::new(Arc::clone(executor_attachment))); push_tool_spec( &mut builder, create_list_dir_tool(), @@ -781,8 +783,8 @@ pub(crate) fn build_specs_with_discoverable_tools( ); } - if let Some(attached_executor) = config.attached_executor.as_ref() { - let view_image_handler = Arc::new(ViewImageHandler::new(Arc::clone(attached_executor))); + if let Some(executor_attachment) = config.executor_attachment.as_ref() { + let view_image_handler = Arc::new(ViewImageHandler::new(Arc::clone(executor_attachment))); push_tool_spec( &mut builder, create_view_image_tool(ViewImageToolOptions { diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index fae2d7889b..101a39b8a5 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -1003,7 +1003,7 @@ fn no_attached_executor_hides_executor_backed_tools() { sandbox_policy: &SandboxPolicy::DangerFullAccess, windows_sandbox_level: WindowsSandboxLevel::Disabled, }) - .with_attached_executor(None); + .with_executor_attachment(None); tools_config .experimental_supported_tools .push("list_dir".to_string()); diff --git a/codex-rs/core/src/unified_exec/mod.rs b/codex-rs/core/src/unified_exec/mod.rs index 18a7fa4901..154e2806b2 100644 --- a/codex-rs/core/src/unified_exec/mod.rs +++ b/codex-rs/core/src/unified_exec/mod.rs @@ -28,7 +28,7 @@ use std::path::PathBuf; use std::sync::Arc; use std::sync::Weak; -use codex_exec_server::AttachedExecutor; +use codex_exec_server::ExecutorAttachment; use codex_network_proxy::NetworkProxy; use codex_protocol::models::PermissionProfile; use rand::Rng; @@ -74,7 +74,7 @@ pub(crate) struct UnifiedExecContext { pub session: Arc, pub turn: Arc, pub call_id: String, - pub attached_executor: Arc, + pub executor_attachment: Arc, } impl UnifiedExecContext { @@ -82,13 +82,13 @@ impl UnifiedExecContext { session: Arc, turn: Arc, call_id: String, - attached_executor: Arc, + executor_attachment: Arc, ) -> Self { Self { session, turn, call_id, - attached_executor, + executor_attachment, } } } diff --git a/codex-rs/core/src/unified_exec/mod_tests.rs b/codex-rs/core/src/unified_exec/mod_tests.rs index ec2cac4585..3e4b540e93 100644 --- a/codex-rs/core/src/unified_exec/mod_tests.rs +++ b/codex-rs/core/src/unified_exec/mod_tests.rs @@ -99,8 +99,8 @@ async fn exec_command_with_tty( tty, Box::new(NoopSpawnLifecycle), turn.environment - .attached_executor() - .expect("test turn has attached executor") + .executor_attachment() + .expect("test turn has executor attachment") .as_ref(), ) .await?, @@ -110,8 +110,8 @@ async fn exec_command_with_tty( Arc::clone(turn), "call".to_string(), turn.environment - .attached_executor() - .expect("test turn has attached executor"), + .executor_attachment() + .expect("test turn has executor attachment"), ); let started_at = Instant::now(); let process_started_alive = !process.has_exited() && process.exit_code().is_none(); @@ -516,13 +516,16 @@ async fn completed_pipe_commands_preserve_exit_code() -> anyhow::Result<()> { ); let environment = codex_exec_server::Environment::default(); + let executor_attachment = environment + .executor_attachment() + .expect("default environment has an executor attachment"); let process = UnifiedExecProcessManager::default() .open_session_with_exec_env( /*process_id*/ 1234, &request, /*tty*/ false, Box::new(NoopSpawnLifecycle), - &environment, + executor_attachment.as_ref(), ) .await?; @@ -550,6 +553,9 @@ async fn unified_exec_uses_remote_exec_server_when_configured() -> anyhow::Resul let remote_test_env = remote_test_env().await?; let environment = remote_test_env.environment(); + let executor_attachment = environment + .executor_attachment() + .expect("remote environment has an executor attachment"); let (_, turn) = make_session_and_context().await; let request = test_exec_request( &turn, @@ -565,7 +571,7 @@ async fn unified_exec_uses_remote_exec_server_when_configured() -> anyhow::Resul &request, /*tty*/ true, Box::new(NoopSpawnLifecycle), - environment, + executor_attachment.as_ref(), ) .await?; @@ -604,6 +610,10 @@ async fn remote_exec_server_rejects_inherited_fd_launches() -> anyhow::Result<() let remote_test_env = remote_test_env().await?; let (_, mut turn) = make_session_and_context().await; turn.environment = Arc::new(remote_test_env.environment().clone()); + let executor_attachment = turn + .environment + .executor_attachment() + .expect("remote environment has an executor attachment"); let request = test_exec_request( &turn, @@ -621,7 +631,7 @@ async fn remote_exec_server_rejects_inherited_fd_launches() -> anyhow::Result<() Box::new(TestSpawnLifecycle { inherited_fds: vec![42], }), - turn.environment.as_ref(), + executor_attachment.as_ref(), ) .await .expect_err("expected inherited fd rejection"); diff --git a/codex-rs/core/src/unified_exec/process_manager.rs b/codex-rs/core/src/unified_exec/process_manager.rs index abb2c12406..6521c11036 100644 --- a/codex-rs/core/src/unified_exec/process_manager.rs +++ b/codex-rs/core/src/unified_exec/process_manager.rs @@ -49,7 +49,7 @@ use crate::unified_exec::process::OutputBuffer; use crate::unified_exec::process::OutputHandles; use crate::unified_exec::process::SpawnLifecycleHandle; use crate::unified_exec::process::UnifiedExecProcess; -use codex_exec_server::AttachedExecutor; +use codex_exec_server::ExecutorAttachment; use codex_utils_output_truncation::approx_token_count; const UNIFIED_EXEC_ENV: [(&str, &str); 10] = [ @@ -586,7 +586,7 @@ impl UnifiedExecProcessManager { env: &ExecRequest, tty: bool, mut spawn_lifecycle: SpawnLifecycleHandle, - attached_executor: &AttachedExecutor, + executor_attachment: &ExecutorAttachment, ) -> Result { let (program, args) = env .command @@ -594,14 +594,14 @@ impl UnifiedExecProcessManager { .ok_or(UnifiedExecError::MissingCommandLine)?; let inherited_fds = spawn_lifecycle.inherited_fds(); - if attached_executor.exec_server_url().is_some() { + if executor_attachment.exec_server_url().is_some() { if !inherited_fds.is_empty() { return Err(UnifiedExecError::create_process( "remote exec-server does not support inherited file descriptors".to_string(), )); } - let started = attached_executor + let started = executor_attachment .get_exec_backend() .start(codex_exec_server::ExecParams { process_id: exec_server_process_id(process_id).into(), @@ -657,7 +657,7 @@ impl UnifiedExecProcessManager { let mut orchestrator = ToolOrchestrator::new(); let mut runtime = UnifiedExecRuntime::new( self, - Arc::clone(&context.attached_executor), + Arc::clone(&context.executor_attachment), context.turn.tools_config.unified_exec_shell_mode.clone(), ); let exec_approval_requirement = context diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 2e37df237c..d079679239 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -132,6 +132,8 @@ pub async fn test_env() -> Result { let environment = codex_exec_server::Environment::create(Some(websocket_url)).await?; let cwd = remote_aware_cwd_path(); environment + .executor_attachment() + .expect("remote test environment has an executor attachment") .get_filesystem() .create_directory( &absolute_path(&cwd)?, @@ -663,7 +665,11 @@ impl TestCodex { } pub fn fs(&self) -> Arc { - self._test_env.environment().get_filesystem() + self._test_env + .environment() + .executor_attachment() + .expect("test environment has an executor attachment") + .get_filesystem() } pub async fn submit_turn(&self, prompt: &str) -> Result<()> { diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 0dd7718d3a..a01e78157d 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -15,7 +15,11 @@ async fn remote_test_env_can_connect_and_use_filesystem() -> Result<()> { }; let test_env = test_env().await?; - let file_system = test_env.environment().get_filesystem(); + let file_system = test_env + .environment() + .executor_attachment() + .expect("remote test environment has an executor attachment") + .get_filesystem(); let file_path = remote_test_file_path(); let file_path_abs = absolute_path(file_path.clone())?; diff --git a/codex-rs/exec-server/src/environment.rs b/codex-rs/exec-server/src/environment.rs index 9bb1f49425..e9937e14e4 100644 --- a/codex-rs/exec-server/src/environment.rs +++ b/codex-rs/exec-server/src/environment.rs @@ -14,26 +14,32 @@ use crate::remote_process::RemoteProcess; pub const CODEX_EXEC_SERVER_URL_ENV_VAR: &str = "CODEX_EXEC_SERVER_URL"; +/// Exec backend access for a concrete executor attachment. +/// +/// Implementations should return a backend bound to the same environment +/// instance so process execution and filesystem operations stay on one +/// transport. pub trait ExecutorEnvironment: Send + Sync { fn get_exec_backend(&self) -> Arc; } +/// Executor and filesystem transports attached to one environment. #[derive(Clone)] -pub struct AttachedExecutor { +pub struct ExecutorAttachment { exec_server_url: Option, exec_backend: Arc, file_system: Arc, } -impl std::fmt::Debug for AttachedExecutor { +impl std::fmt::Debug for ExecutorAttachment { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("AttachedExecutor") + f.debug_struct("ExecutorAttachment") .field("exec_server_url", &self.exec_server_url) .finish_non_exhaustive() } } -impl AttachedExecutor { +impl ExecutorAttachment { fn new_local(exec_backend: Arc) -> Self { Self { exec_server_url: None, @@ -63,7 +69,7 @@ impl AttachedExecutor { } } -impl ExecutorEnvironment for AttachedExecutor { +impl ExecutorEnvironment for ExecutorAttachment { fn get_exec_backend(&self) -> Arc { self.get_exec_backend() } @@ -113,30 +119,31 @@ impl EnvironmentManager { #[derive(Clone, Debug, Default, Eq, PartialEq)] enum ExecutorMode { #[default] - LocalExecutor, - RemoteExecutor { + Local, + Remote { url: String, }, - NoExecutor, + None, } impl ExecutorMode { fn remote_exec_server_url(&self) -> Option<&str> { match self { - Self::RemoteExecutor { url } => Some(url.as_str()), - Self::LocalExecutor | Self::NoExecutor => None, + Self::Remote { url } => Some(url.as_str()), + Self::Local | Self::None => None, } } fn has_attached_executor(&self) -> bool { - !matches!(self, Self::NoExecutor) + !matches!(self, Self::None) } } +/// Session/turn environment plus an optional executor attachment. #[derive(Clone)] pub struct Environment { executor_mode: ExecutorMode, - attached_executor: Option>, + executor_attachment: Option>, } impl Default for Environment { @@ -150,8 +157,8 @@ impl Default for Environment { } Self { - executor_mode: ExecutorMode::LocalExecutor, - attached_executor: Some(Arc::new(AttachedExecutor::new_local(Arc::new( + executor_mode: ExecutorMode::Local, + executor_attachment: Some(Arc::new(ExecutorAttachment::new_local(Arc::new( local_process, )))), } @@ -172,7 +179,7 @@ impl Environment { } async fn create_with_mode(executor_mode: ExecutorMode) -> Result { - let attached_executor = if let Some(url) = executor_mode.remote_exec_server_url() { + let executor_attachment = if let Some(url) = executor_mode.remote_exec_server_url() { let client = ExecServerClient::connect_websocket(RemoteExecServerConnectArgs { websocket_url: url.to_string(), client_name: "codex-environment".to_string(), @@ -180,11 +187,11 @@ impl Environment { initialize_timeout: std::time::Duration::from_secs(5), }) .await?; - Some(Arc::new(AttachedExecutor::new_remote( + Some(Arc::new(ExecutorAttachment::new_remote( url.to_string(), client, ))) - } else if matches!(executor_mode, ExecutorMode::NoExecutor) { + } else if matches!(executor_mode, ExecutorMode::None) { None } else { let local_process = LocalProcess::default(); @@ -194,14 +201,14 @@ impl Environment { local_process .initialized() .map_err(ExecServerError::Protocol)?; - Some(Arc::new(AttachedExecutor::new_local(Arc::new( + Some(Arc::new(ExecutorAttachment::new_local(Arc::new( local_process, )))) }; Ok(Self { executor_mode, - attached_executor, + executor_attachment, }) } @@ -213,16 +220,16 @@ impl Environment { self.executor_mode.has_attached_executor() } - pub fn attached_executor(&self) -> Option> { - self.attached_executor.clone() + pub fn executor_attachment(&self) -> Option> { + self.executor_attachment.clone() } } fn parse_executor_mode(exec_server_url: Option) -> ExecutorMode { match exec_server_url.as_deref().map(str::trim) { - None | Some("") => ExecutorMode::LocalExecutor, - Some(url) if url.eq_ignore_ascii_case("none") => ExecutorMode::NoExecutor, - Some(url) => ExecutorMode::RemoteExecutor { + None | Some("") => ExecutorMode::Local, + Some(url) if url.eq_ignore_ascii_case("none") => ExecutorMode::None, + Some(url) => ExecutorMode::Remote { url: url.to_string(), }, } @@ -247,42 +254,42 @@ mod tests { assert_eq!(environment.exec_server_url(), None); assert!(environment.has_attached_executor()); - assert_eq!(environment.executor_mode, ExecutorMode::LocalExecutor); - assert!(environment.attached_executor().is_some()); + assert_eq!(environment.executor_mode, ExecutorMode::Local); + assert!(environment.executor_attachment().is_some()); } #[test] fn environment_manager_normalizes_empty_url() { let manager = EnvironmentManager::new(Some(String::new())); - assert_eq!(manager.executor_mode, ExecutorMode::LocalExecutor); + assert_eq!(manager.executor_mode, ExecutorMode::Local); } #[test] fn environment_manager_preserves_no_executor_setting() { let manager = EnvironmentManager::new(Some("none".to_string())); - assert_eq!(manager.executor_mode, ExecutorMode::NoExecutor); + assert_eq!(manager.executor_mode, ExecutorMode::None); } #[test] fn parse_executor_mode_preserves_no_executor_semantics() { - assert_eq!(parse_executor_mode(None), ExecutorMode::LocalExecutor); + assert_eq!(parse_executor_mode(None), ExecutorMode::Local); assert_eq!( parse_executor_mode(Some(String::new())), - ExecutorMode::LocalExecutor + ExecutorMode::Local ); assert_eq!( parse_executor_mode(Some("none".to_string())), - ExecutorMode::NoExecutor + ExecutorMode::None ); assert_eq!( parse_executor_mode(Some("NONE".to_string())), - ExecutorMode::NoExecutor + ExecutorMode::None ); assert_eq!( parse_executor_mode(Some("ws://localhost:1234".to_string())), - ExecutorMode::RemoteExecutor { + ExecutorMode::Remote { url: "ws://localhost:1234".to_string(), } ); @@ -303,8 +310,8 @@ mod tests { let environment = Environment::default(); let response = environment - .attached_executor() - .expect("default environment has attached executor") + .executor_attachment() + .expect("default environment has executor attachment") .get_exec_backend() .start(crate::ExecParams { process_id: ProcessId::from("default-env-proc"), @@ -321,23 +328,23 @@ mod tests { } #[tokio::test] - async fn no_executor_environment_disables_attached_executor() { + async fn no_executor_environment_disables_executor_attachment() { let environment = Environment::create(Some("none".to_string())) .await .expect("create environment"); assert_eq!(environment.exec_server_url(), None); assert!(!environment.has_attached_executor()); - assert_eq!(environment.executor_mode, ExecutorMode::NoExecutor); - assert!(environment.attached_executor().is_none()); + assert_eq!(environment.executor_mode, ExecutorMode::None); + assert!(environment.executor_attachment().is_none()); } #[tokio::test] - async fn no_executor_environment_has_no_executor_capability() { + async fn no_executor_environment_has_no_executor_attachment() { let environment = Environment::create(Some("none".to_string())) .await .expect("create environment"); - assert!(environment.attached_executor().is_none()); + assert!(environment.executor_attachment().is_none()); } } diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs index 8eedbffe7e..25092d583e 100644 --- a/codex-rs/exec-server/src/lib.rs +++ b/codex-rs/exec-server/src/lib.rs @@ -31,10 +31,10 @@ pub use codex_app_server_protocol::FsRemoveParams; pub use codex_app_server_protocol::FsRemoveResponse; pub use codex_app_server_protocol::FsWriteFileParams; pub use codex_app_server_protocol::FsWriteFileResponse; -pub use environment::AttachedExecutor; pub use environment::CODEX_EXEC_SERVER_URL_ENV_VAR; pub use environment::Environment; pub use environment::EnvironmentManager; +pub use environment::ExecutorAttachment; pub use environment::ExecutorEnvironment; pub use file_system::CopyOptions; pub use file_system::CreateDirectoryOptions; diff --git a/codex-rs/exec-server/tests/exec_process.rs b/codex-rs/exec-server/tests/exec_process.rs index ab232ccd3e..10610549d1 100644 --- a/codex-rs/exec-server/tests/exec_process.rs +++ b/codex-rs/exec-server/tests/exec_process.rs @@ -31,13 +31,19 @@ async fn create_process_context(use_remote: bool) -> Result { let server = exec_server().await?; let environment = Environment::create(Some(server.websocket_url().to_string())).await?; Ok(ProcessContext { - backend: environment.get_exec_backend(), + backend: environment + .executor_attachment() + .expect("remote environment has an executor attachment") + .get_exec_backend(), server: Some(server), }) } else { let environment = Environment::create(/*exec_server_url*/ None).await?; Ok(ProcessContext { - backend: environment.get_exec_backend(), + backend: environment + .executor_attachment() + .expect("local environment has an executor attachment") + .get_exec_backend(), server: None, }) } diff --git a/codex-rs/exec-server/tests/file_system.rs b/codex-rs/exec-server/tests/file_system.rs index dea47e8fcc..70a47af087 100644 --- a/codex-rs/exec-server/tests/file_system.rs +++ b/codex-rs/exec-server/tests/file_system.rs @@ -32,13 +32,19 @@ async fn create_file_system_context(use_remote: bool) -> Result