From bce5f2fcfcc32ae80b0e9d2eac5bca8d1895120e Mon Sep 17 00:00:00 2001 From: jif Date: Thu, 20 Aug 2026 18:19:32 +0000 Subject: [PATCH] Standardize shell execution on unified exec (#39772) ## What changed - Use `exec_command` and `write_stdin` as the shell tool surface. - Treat legacy `default`, `local`, and `shell_command` model metadata as `unified_exec`. - Remove obsolete shell-selection configuration and runtime paths while preserving the feature and policy gates for zsh fork execution. GitOrigin-RevId: d743cbe598630d73052f1fecad680c4cde17977d --- .../tests/suite/v2/turn_start_zsh_fork.rs | 8 - codex-rs/core/src/config/config_tests.rs | 18 +- codex-rs/core/src/config/mod.rs | 6 - codex-rs/core/src/exec.rs | 8 - codex-rs/core/src/guardian/review_session.rs | 2 - codex-rs/core/src/session/review.rs | 2 +- codex-rs/core/src/session/turn_context.rs | 2 +- codex-rs/core/src/tools/parallel.rs | 189 +------------ codex-rs/core/src/tools/registry.rs | 11 - codex-rs/core/src/tools/router.rs | 6 - codex-rs/core/src/tools/runtimes/mod.rs | 2 +- codex-rs/core/src/tools/runtimes/shell/mod.rs | 3 - .../core/src/tools/runtimes/unified_exec.rs | 12 +- .../zsh_fork_backend.rs => zsh_fork.rs} | 6 +- .../{shell => zsh_fork}/unix_escalation.rs | 0 .../unix_escalation_tests.rs | 0 codex-rs/core/src/tools/spec_plan.rs | 36 +-- codex-rs/core/src/tools/spec_plan_tests.rs | 34 +-- codex-rs/core/tests/common/zsh_fork.rs | 9 - .../remote_env_windows_test.rs | 10 +- codex-rs/core/tests/suite/agent_execution.rs | 3 +- .../core/tests/suite/cyber_exec_policy.rs | 79 +----- .../core/tests/suite/deprecation_notice.rs | 2 - codex-rs/core/tests/suite/hooks.rs | 97 +------ .../tests/suite/multi_exec_server_sandbox.rs | 9 +- codex-rs/core/tests/suite/network_approval.rs | 5 - codex-rs/core/tests/suite/remote_env.rs | 42 +-- .../core/tests/suite/request_permissions.rs | 49 +--- codex-rs/core/tests/suite/rmcp_client.rs | 2 +- codex-rs/core/tests/suite/shell_snapshot.rs | 10 - codex-rs/core/tests/suite/skills_extension.rs | 5 - codex-rs/core/tests/suite/unified_exec.rs | 250 ++---------------- .../suite/unified_exec_process_events.rs | 6 - codex-rs/core/tests/suite/workspace_roots.rs | 6 - codex-rs/models-manager/src/model_info.rs | 2 +- codex-rs/protocol/src/openai_models.rs | 15 +- codex-rs/thread-manager-sample/src/main.rs | 1 - codex-rs/tools/src/lib.rs | 3 - codex-rs/tools/src/tool_config.rs | 52 +--- codex-rs/tools/src/tool_config_tests.rs | 179 +++---------- 40 files changed, 189 insertions(+), 992 deletions(-) delete mode 100644 codex-rs/core/src/tools/runtimes/shell/mod.rs rename codex-rs/core/src/tools/runtimes/{shell/zsh_fork_backend.rs => zsh_fork.rs} (96%) rename codex-rs/core/src/tools/runtimes/{shell => zsh_fork}/unix_escalation.rs (100%) rename codex-rs/core/src/tools/runtimes/{shell => zsh_fork}/unix_escalation_tests.rs (100%) diff --git a/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs b/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs index f960491db1..863ccb901b 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs @@ -96,8 +96,6 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> { "never", &BTreeMap::from([ (Feature::ShellZshFork, true), - (Feature::UnifiedExec, true), - (Feature::UnifiedExecZshFork, true), (Feature::ShellSnapshot, false), ]), )?; @@ -212,8 +210,6 @@ async fn turn_start_shell_zsh_fork_exec_approval_decline_v2() -> Result<()> { "on-request", &BTreeMap::from([ (Feature::ShellZshFork, true), - (Feature::UnifiedExec, true), - (Feature::UnifiedExecZshFork, true), (Feature::ShellSnapshot, false), ]), )?; @@ -342,8 +338,6 @@ async fn turn_start_shell_zsh_fork_exec_approval_cancel_v2() -> Result<()> { "on-request", &BTreeMap::from([ (Feature::ShellZshFork, true), - (Feature::UnifiedExec, true), - (Feature::UnifiedExecZshFork, true), (Feature::ShellSnapshot, false), ]), )?; @@ -498,8 +492,6 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() "on-request", &BTreeMap::from([ (Feature::ShellZshFork, true), - (Feature::UnifiedExec, true), - (Feature::UnifiedExecZshFork, true), (Feature::ShellSnapshot, false), ]), )?; diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index f10f5f7831..57f96aaaa6 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -6440,8 +6440,6 @@ async fn legacy_toggles_map_to_features() -> std::io::Result<()> { assert!(config.features.enabled(Feature::UnifiedExec)); - assert!(config.use_experimental_unified_exec_tool); - Ok(()) } @@ -11033,6 +11031,7 @@ async fn feature_requirements_can_still_disable_unified_exec() -> std::io::Resul [features] unified_exec = false shell_tool = true +unified_exec_zsh_fork = false "#, ), ) @@ -11041,14 +11040,27 @@ shell_tool = true assert!(!config.features.enabled(Feature::UnifiedExec)); assert!(config.features.enabled(Feature::ShellTool)); + assert!(!config.features.enabled(Feature::UnifiedExecZshFork)); + assert!( + !config + .startup_warnings + .iter() + .any(|warning| warning.contains("Ignoring unknown `features` requirement")), + "{:?}", + config.startup_warnings + ); config .features .enable(Feature::UnifiedExec) .expect("managed feature mutations should normalize successfully"); - + config + .features + .enable(Feature::UnifiedExecZshFork) + .expect("managed feature updates should preserve administrator policy"); assert!(!config.features.enabled(Feature::UnifiedExec)); assert!(config.features.enabled(Feature::ShellTool)); + assert!(!config.features.enabled(Feature::UnifiedExecZshFork)); Ok(()) } diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index d5c2f18404..49e7f5a271 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -991,9 +991,6 @@ pub struct Config { /// Configuration for the experimental code-mode tool surface. pub code_mode: CodeModeConfig, - /// If set to `true`, used only the experimental unified exec tool. - pub use_experimental_unified_exec_tool: bool, - /// Maximum poll window for background terminal output (`write_stdin`), in milliseconds. /// Default: `300000` (5 minutes). pub background_terminal_max_timeout: u64, @@ -3774,8 +3771,6 @@ impl Config { config }; - let use_experimental_unified_exec_tool = features.enabled(Feature::UnifiedExec); - let forced_chatgpt_workspace_id = cfg .forced_chatgpt_workspace_id .clone() @@ -4175,7 +4170,6 @@ impl Config { update_plan_enabled, tool_registry, code_mode, - use_experimental_unified_exec_tool, background_terminal_max_timeout, ghost_snapshot, multi_agent_v2, diff --git a/codex-rs/core/src/exec.rs b/codex-rs/core/src/exec.rs index 4c8c8ce80a..8a8994bff3 100644 --- a/codex-rs/core/src/exec.rs +++ b/codex-rs/core/src/exec.rs @@ -161,14 +161,6 @@ pub enum ExecExpirationOutcome { Cancelled, } -impl From> for ExecExpiration { - fn from(timeout_ms: Option) -> Self { - timeout_ms.map_or(ExecExpiration::DefaultTimeout, |timeout_ms| { - ExecExpiration::Timeout(Duration::from_millis(timeout_ms)) - }) - } -} - impl From for ExecExpiration { fn from(timeout_ms: u64) -> Self { ExecExpiration::Timeout(Duration::from_millis(timeout_ms)) diff --git a/codex-rs/core/src/guardian/review_session.rs b/codex-rs/core/src/guardian/review_session.rs index 3a6090992b..afee14677d 100644 --- a/codex-rs/core/src/guardian/review_session.rs +++ b/codex-rs/core/src/guardian/review_session.rs @@ -210,7 +210,6 @@ struct GuardianReviewSessionReuseKey { main_execve_wrapper_exe: Option, zsh_path: Option, features: ManagedFeatures, - use_experimental_unified_exec_tool: bool, environment_ids: Vec, } @@ -249,7 +248,6 @@ impl GuardianReviewSessionReuseKey { main_execve_wrapper_exe: spawn_config.main_execve_wrapper_exe.clone(), zsh_path: spawn_config.zsh_path.clone(), features: spawn_config.features.clone(), - use_experimental_unified_exec_tool: spawn_config.use_experimental_unified_exec_tool, environment_ids: Vec::new(), } } diff --git a/codex-rs/core/src/session/review.rs b/codex-rs/core/src/session/review.rs index d431e602b5..320213eda2 100644 --- a/codex-rs/core/src/session/review.rs +++ b/codex-rs/core/src/session/review.rs @@ -33,7 +33,7 @@ pub(super) async fn spawn_review_thread( let _ = review_features.disable(Feature::Goals); let review_web_search_mode = WebSearchMode::Disabled; let unified_exec_shell_mode = UnifiedExecShellMode::for_session( - codex_tools::unified_exec_feature_mode_for_features(review_features.get()), + review_features.get(), crate::tools::tool_user_shell_type(sess.services.user_shell.as_ref()), sess.services.shell_zsh_path.as_ref(), sess.services.main_execve_wrapper_exe.as_ref(), diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index 728749a410..feb016d676 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -647,7 +647,7 @@ impl Session { let session_telemetry_for_context = session_telemetry; let available_models = models_manager.try_list_models().unwrap_or_default(); let unified_exec_shell_mode = UnifiedExecShellMode::for_session( - codex_tools::unified_exec_feature_mode_for_features(per_turn_config.features.get()), + per_turn_config.features.get(), crate::tools::tool_user_shell_type(user_shell), shell_zsh_path, main_execve_wrapper_exe, diff --git a/codex-rs/core/src/tools/parallel.rs b/codex-rs/core/src/tools/parallel.rs index b75e8943e5..191850c4eb 100644 --- a/codex-rs/core/src/tools/parallel.rs +++ b/codex-rs/core/src/tools/parallel.rs @@ -112,7 +112,6 @@ impl ToolCallRuntime { let router = &self.step_context.tool_router; let supports_parallel = router.tool_supports_parallel(&call); let tool_runtime = router.tool_runtime(&call); - let wait_for_runtime_cancellation = router.tool_waits_for_runtime_cancellation(&call); let router = Arc::clone(router); let session = Arc::clone(&self.session); let step_context = Arc::clone(&self.step_context); @@ -185,24 +184,11 @@ impl ToolCallRuntime { } else { let secs = started.elapsed().as_secs_f32().max(0.1); abort_dispatch_span.record("aborted", true); - if wait_for_runtime_cancellation { - if terminal_outcome_reached.swap(true, Ordering::AcqRel) { - return dispatch_handle.await.map_err(Self::tool_task_join_error)?; - } - // The abort owns the terminal outcome; await only so - // the runtime can finish process teardown. - match dispatch_handle.await { - Ok(_) => {} - Err(err) if err.is_cancelled() => {} - Err(err) => return Err(Self::tool_task_join_error(err)), - } - } else { - dispatch_handle.abort(); - match dispatch_handle.await { - Ok(result) => return result, - Err(err) if err.is_cancelled() => {} - Err(err) => return Err(Self::tool_task_join_error(err)), - } + dispatch_handle.abort(); + match dispatch_handle.await { + Ok(result) => return result, + Err(err) if err.is_cancelled() => {} + Err(err) => return Err(Self::tool_task_join_error(err)), } let response = Self::aborted_response(&call, secs); notify_tool_aborted( @@ -266,12 +252,7 @@ impl ToolCallRuntime { } fn abort_message(call: &ToolCall, secs: f32) -> String { - if call.tool_name.is_default_namespace() - && matches!( - call.tool_name.name.as_str(), - "exec_command" | "unified_exec" - ) - { + if call.tool_name.is_default_namespace() && call.tool_name.name == "exec_command" { format!("Wall time: {secs:.1} seconds\naborted by user") } else { format!("aborted by user after {secs:.1}s") @@ -568,90 +549,6 @@ mod tests { impl CoreToolRuntime for ImmediateHandler {} - struct CancellationCleanupHandler { - tool_name: codex_tools::ToolName, - started: std::sync::Mutex>>, - cleanup_started: std::sync::Mutex>>, - allow_cleanup: Arc, - } - - impl ToolExecutor for CancellationCleanupHandler { - fn tool_name(&self) -> codex_tools::ToolName { - self.tool_name.clone() - } - - fn spec(&self) -> codex_tools::ToolSpec { - codex_tools::ToolSpec::Function(codex_tools::ResponsesApiTool { - name: self.tool_name.name.clone(), - description: "Cancellation cleanup test tool.".to_string(), - strict: false, - defer_loading: None, - parameters: codex_tools::JsonSchema::default(), - output_schema: None, - }) - } - - fn handle(&self, invocation: ToolInvocation) -> codex_tools::ToolExecutorFuture<'_> { - Box::pin(self.handle_call(invocation)) - } - } - - impl CancellationCleanupHandler { - async fn handle_call( - &self, - invocation: ToolInvocation, - ) -> Result, FunctionCallError> { - let started = self - .started - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .take(); - if let Some(started) = started { - let _ = started.send(()); - } - invocation.cancellation_token.cancelled().await; - let cleanup_started = self - .cleanup_started - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .take(); - if let Some(cleanup_started) = cleanup_started { - let _ = cleanup_started.send(()); - } - self.allow_cleanup.notified().await; - Ok(Box::new(FunctionToolOutput::from_text( - "cleanup complete".to_string(), - Some(false), - )) as Box) - } - } - - impl CoreToolRuntime for CancellationCleanupHandler { - fn waits_for_runtime_cancellation(&self) -> bool { - true - } - } - - struct FinishRecorder { - records: Arc>>, - } - - impl codex_extension_api::ToolLifecycleContributor for FinishRecorder { - fn on_tool_finish<'a>( - &'a self, - input: codex_extension_api::ToolFinishInput<'a>, - ) -> codex_extension_api::ToolLifecycleFuture<'a> { - let records = Arc::clone(&self.records); - let outcome = input.outcome; - Box::pin(async move { - records - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .push(outcome); - }) - } - } - struct BlockingFinishContributor { records: Arc>>, finish_started: std::sync::Mutex>>, @@ -756,78 +653,4 @@ mod tests { Ok(()) } - - #[tokio::test] - async fn cancellation_waiting_for_runtime_cleanup_emits_only_aborted_lifecycle() - -> anyhow::Result<()> { - let (mut session, turn_context) = crate::session::tests::make_session_and_context().await; - let records = Arc::new(std::sync::Mutex::new(Vec::new())); - let mut builder = - codex_extension_api::ExtensionRegistryBuilder::::new(); - builder.tool_lifecycle_contributor(Arc::new(FinishRecorder { - records: Arc::clone(&records), - })); - session.services.extensions = Arc::new(builder.build()); - - let session = Arc::new(session); - let turn_context = Arc::new(turn_context); - let tool_name = codex_tools::ToolName::plain("cleanup_tool"); - let (started_tx, started_rx) = oneshot::channel(); - let (cleanup_started_tx, cleanup_started_rx) = oneshot::channel(); - let allow_cleanup = Arc::new(Notify::new()); - let handler = Arc::new(CancellationCleanupHandler { - tool_name: tool_name.clone(), - started: std::sync::Mutex::new(Some(started_tx)), - cleanup_started: std::sync::Mutex::new(Some(cleanup_started_tx)), - allow_cleanup: Arc::clone(&allow_cleanup), - }) as Arc; - let step_context = StepContext::for_test(Arc::clone(&turn_context)); - let router = Arc::new(ToolRouter::from_parts( - ToolRegistry::from_tools([handler]), - Vec::new(), - )); - let step_context = step_context.with_tool_router_for_test(router); - let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())); - let runtime = ToolCallRuntime::new(session, step_context, tracker); - let cancellation_token = CancellationToken::new(); - let call = ToolCall { - tool_name, - call_id: "call-1".to_string(), - payload: ToolPayload::Function { - arguments: "{}".to_string(), - }, - encrypted_function_args: None, - }; - - let response_task = - tokio::spawn(runtime.handle_tool_call(call, cancellation_token.clone())); - started_rx.await.expect("handler should start"); - cancellation_token.cancel(); - cleanup_started_rx - .await - .expect("handler should start cleanup"); - tokio::time::sleep(Duration::from_millis(10)).await; - allow_cleanup.notify_one(); - - let response = tokio::time::timeout(Duration::from_secs(1), response_task) - .await - .expect("timed out waiting for tool response") - .expect("tool response task should join")?; - let ResponseInputItem::FunctionCallOutput { output, .. } = response else { - anyhow::bail!("cancelled tool should return function output"); - }; - let FunctionCallOutputBody::Text(text) = output.body else { - anyhow::bail!("cancelled tool output should be text"); - }; - assert!(text.contains("aborted by user")); - - let actual = records - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .drain(..) - .collect::>(); - assert_eq!(vec![ToolCallOutcome::Aborted], actual); - - Ok(()) - } } diff --git a/codex-rs/core/src/tools/registry.rs b/codex-rs/core/src/tools/registry.rs index 48322c3b0b..5ae1d9d054 100644 --- a/codex-rs/core/src/tools/registry.rs +++ b/codex-rs/core/src/tools/registry.rs @@ -85,12 +85,6 @@ pub(crate) trait CoreToolRuntime: ToolExecutor { ) } - /// Whether cancellation should let the handler finish teardown before the - /// host returns an aborted tool response. - fn waits_for_runtime_cancellation(&self) -> bool { - false - } - fn telemetry_tags(&self, _invocation: &ToolInvocation) -> ToolTelemetryTags { Vec::new() } @@ -478,11 +472,6 @@ impl ToolRegistry { Some(tool.exposure != ToolExposure::Hidden && tool.runtime.supports_parallel_tool_calls()) } - pub(crate) fn waits_for_runtime_cancellation(&self, name: &ToolName) -> Option { - let tool = self.tool(name)?; - Some(tool.waits_for_runtime_cancellation()) - } - #[expect( clippy::await_holding_invalid_type, reason = "tool dispatch must keep active-turn accounting atomic" diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index b3e60a5986..e5e6124b44 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -144,12 +144,6 @@ impl ToolRouter { self.registry.tool(&call.tool_name) } - pub fn tool_waits_for_runtime_cancellation(&self, call: &ToolCall) -> bool { - self.registry - .waits_for_runtime_cancellation(&call.tool_name) - .unwrap_or(false) - } - #[instrument(level = "trace", skip_all, err)] pub fn build_tool_call(item: ResponseItem) -> Result, FunctionCallError> { match item { diff --git a/codex-rs/core/src/tools/runtimes/mod.rs b/codex-rs/core/src/tools/runtimes/mod.rs index 368bc3c418..929ef6c563 100644 --- a/codex-rs/core/src/tools/runtimes/mod.rs +++ b/codex-rs/core/src/tools/runtimes/mod.rs @@ -31,8 +31,8 @@ use std::collections::HashMap; use std::path::Path; pub(crate) mod apply_patch; -pub(crate) mod shell; pub(crate) mod unified_exec; +pub(crate) mod zsh_fork; pub(crate) fn exec_env_for_sandbox_permissions( env: &HashMap, diff --git a/codex-rs/core/src/tools/runtimes/shell/mod.rs b/codex-rs/core/src/tools/runtimes/shell/mod.rs deleted file mode 100644 index 9c4f6bc4a4..0000000000 --- a/codex-rs/core/src/tools/runtimes/shell/mod.rs +++ /dev/null @@ -1,3 +0,0 @@ -#[cfg(unix)] -pub(crate) mod unix_escalation; -pub(crate) mod zsh_fork_backend; diff --git a/codex-rs/core/src/tools/runtimes/unified_exec.rs b/codex-rs/core/src/tools/runtimes/unified_exec.rs index fe4110f36d..25a21b7990 100644 --- a/codex-rs/core/src/tools/runtimes/unified_exec.rs +++ b/codex-rs/core/src/tools/runtimes/unified_exec.rs @@ -23,7 +23,7 @@ use crate::tools::runtimes::apply_zsh_fork_path_prepend; use crate::tools::runtimes::disable_powershell_profile_for_elevated_windows_sandbox; use crate::tools::runtimes::exec_env_for_sandbox_permissions; use crate::tools::runtimes::maybe_wrap_shell_lc_with_snapshot; -use crate::tools::runtimes::shell::zsh_fork_backend; +use crate::tools::runtimes::zsh_fork; use crate::tools::sandboxing::Approvable; use crate::tools::sandboxing::ApprovalAction; use crate::tools::sandboxing::ExecApprovalRequirement; @@ -422,14 +422,8 @@ impl<'a> ToolRuntime for UnifiedExecRunt ) .map_err(ToolError::Codex)?; exec_env.exec_server_env_config = req.exec_server_env_config.clone(); - match zsh_fork_backend::maybe_prepare_unified_exec( - req, - attempt, - ctx, - exec_env, - zsh_fork_config, - ) - .await? + match zsh_fork::maybe_prepare_unified_exec(req, attempt, ctx, exec_env, zsh_fork_config) + .await? { Some(prepared) => { if req.turn_environment.environment.is_remote() { diff --git a/codex-rs/core/src/tools/runtimes/shell/zsh_fork_backend.rs b/codex-rs/core/src/tools/runtimes/zsh_fork.rs similarity index 96% rename from codex-rs/core/src/tools/runtimes/shell/zsh_fork_backend.rs rename to codex-rs/core/src/tools/runtimes/zsh_fork.rs index a6aede84f6..4daa0f18b0 100644 --- a/codex-rs/core/src/tools/runtimes/shell/zsh_fork_backend.rs +++ b/codex-rs/core/src/tools/runtimes/zsh_fork.rs @@ -27,10 +27,14 @@ pub(crate) async fn maybe_prepare_unified_exec( imp::maybe_prepare_unified_exec(req, attempt, ctx, exec_request, zsh_fork_config).await } +#[cfg(unix)] +#[path = "zsh_fork/unix_escalation.rs"] +mod unix_escalation; + #[cfg(unix)] mod imp { use super::*; - use crate::tools::runtimes::shell::unix_escalation; + use crate::tools::runtimes::zsh_fork::unix_escalation; use crate::unified_exec::SpawnLifecycle; use codex_shell_escalation::ESCALATE_SOCKET_ENV_VAR; use codex_shell_escalation::EscalationSession; diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/zsh_fork/unix_escalation.rs similarity index 100% rename from codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs rename to codex-rs/core/src/tools/runtimes/zsh_fork/unix_escalation.rs diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/zsh_fork/unix_escalation_tests.rs similarity index 100% rename from codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs rename to codex-rs/core/src/tools/runtimes/zsh_fork/unix_escalation_tests.rs diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 162781173b..f0ab350319 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -90,7 +90,6 @@ use codex_tools::collect_code_mode_exec_prompt_tool_definitions; use codex_tools::collect_request_plugin_install_entries; use codex_tools::default_namespace_description; use codex_tools::request_user_input_available_modes; -use codex_tools::shell_type_for_model_and_features; use futures::future::BoxFuture; use std::collections::BTreeMap; use std::collections::HashMap; @@ -960,29 +959,30 @@ fn add_shell_tools(context: &CoreToolPlanContext<'_>, registry: &mut ToolRegistr let turn_context = context.turn_context; let features = turn_context.config.features.get(); let environment_mode = tool_environment_mode(context.environments); - if !environment_mode.has_environment() { + if !environment_mode.has_environment() + || !features.enabled(Feature::ShellTool) + || !features.enabled(Feature::UnifiedExec) + || matches!( + turn_context.model_info.shell_type, + ConfigShellToolType::Disabled + ) + { return; } let allow_login_shell = any_environment_allows_login_shell(context.environments); let exec_permission_approvals_enabled = features.enabled(Feature::ExecPermissionApprovals); let include_environment_id = matches!(environment_mode, ToolEnvironmentMode::Multiple); - match shell_type_for_model_and_features(&turn_context.model_info, features) { - ConfigShellToolType::UnifiedExec => { - registry.add(ExecCommandHandler::new(ExecCommandHandlerOptions { - allow_login_shell, - exec_permission_approvals_enabled, - include_environment_id, - include_shell_parameter: unified_exec_should_include_shell_parameter( - turn_context, - context.environments, - ), - })); - registry.add(WriteStdinHandler); - } - ConfigShellToolType::Disabled => {} - ConfigShellToolType::Default | ConfigShellToolType::Local => {} - } + registry.add(ExecCommandHandler::new(ExecCommandHandlerOptions { + allow_login_shell, + exec_permission_approvals_enabled, + include_environment_id, + include_shell_parameter: unified_exec_should_include_shell_parameter( + turn_context, + context.environments, + ), + })); + registry.add(WriteStdinHandler); } fn unified_exec_should_include_shell_parameter( diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index a2fcc037c6..a31e576788 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -662,7 +662,7 @@ async fn request_user_input_stays_direct_in_code_mode_only() { #[tokio::test] async fn shell_family_registers_only_unified_exec_tools() { let plan = probe(|turn| { - set_features(turn, &[Feature::ShellTool, Feature::UnifiedExec]); + set_features(turn, &[Feature::ShellTool]); set_feature(turn, Feature::ShellZshFork, /*enabled*/ false); Arc::make_mut(&mut turn.model_info).shell_type = ConfigShellToolType::UnifiedExec; }) @@ -679,7 +679,6 @@ async fn login_shell_parameter_follows_selected_environment() { for allow_login_shell in [false, true] { let plan = probe(|turn| { set_feature(turn, Feature::ShellTool, /*enabled*/ true); - set_feature(turn, Feature::UnifiedExec, /*enabled*/ true); set_feature(turn, Feature::ShellZshFork, /*enabled*/ false); Arc::make_mut(&mut turn.model_info).shell_type = ConfigShellToolType::UnifiedExec; update_config(turn, |config| { @@ -715,7 +714,7 @@ async fn login_shell_parameter_follows_selected_environment() { #[tokio::test] async fn login_shell_parameter_is_available_when_any_environment_allows_it() { let plan = probe(|turn| { - set_features(turn, &[Feature::ShellTool, Feature::UnifiedExec]); + set_features(turn, &[Feature::ShellTool]); set_feature(turn, Feature::ShellZshFork, /*enabled*/ false); update_config(turn, |config| { config.permissions.allow_login_shell = false; @@ -756,8 +755,8 @@ async fn disabling_shell_tools_disables_command_tools_for_all_environments() { ); }) .await; - remote_environment.assert_visible_lacks(&["shell_command", "exec_command", "write_stdin"]); - remote_environment.assert_registered_lacks(&["shell_command", "exec_command", "write_stdin"]); + remote_environment.assert_visible_lacks(&["exec_command", "write_stdin"]); + remote_environment.assert_registered_lacks(&["exec_command", "write_stdin"]); let multiple_local_environments = probe(|turn| { set_feature(turn, Feature::ShellTool, /*enabled*/ false); @@ -765,16 +764,8 @@ async fn disabling_shell_tools_disables_command_tools_for_all_environments() { duplicate_primary_environment(turn); }) .await; - multiple_local_environments.assert_visible_lacks(&[ - "shell_command", - "exec_command", - "write_stdin", - ]); - multiple_local_environments.assert_registered_lacks(&[ - "shell_command", - "exec_command", - "write_stdin", - ]); + multiple_local_environments.assert_visible_lacks(&["exec_command", "write_stdin"]); + multiple_local_environments.assert_registered_lacks(&["exec_command", "write_stdin"]); } #[tokio::test] @@ -808,7 +799,7 @@ async fn dynamic_tools_cannot_reclaim_the_reserved_exec_command_name() { #[tokio::test] async fn shell_zsh_fork_keeps_unified_exec_available() { let without_composition = probe(|turn| { - set_features(turn, &[Feature::ShellTool, Feature::UnifiedExec]); + set_features(turn, &[Feature::ShellTool]); set_feature(turn, Feature::ShellZshFork, /*enabled*/ true); set_feature(turn, Feature::UnifiedExecZshFork, /*enabled*/ false); Arc::make_mut(&mut turn.model_info).shell_type = ConfigShellToolType::UnifiedExec; @@ -823,7 +814,6 @@ async fn shell_zsh_fork_keeps_unified_exec_available() { turn, &[ Feature::ShellTool, - Feature::UnifiedExec, Feature::ShellZshFork, Feature::UnifiedExecZshFork, ], @@ -847,7 +837,6 @@ async fn zsh_fork_unified_exec_hides_shell_parameter() { turn, &[ Feature::ShellTool, - Feature::UnifiedExec, Feature::ShellZshFork, Feature::UnifiedExecZshFork, ], @@ -872,7 +861,6 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava turn, &[ Feature::ShellTool, - Feature::UnifiedExec, Feature::ShellZshFork, Feature::UnifiedExecZshFork, ], @@ -923,8 +911,6 @@ async fn zsh_fork_unified_exec_keeps_shell_parameter_when_remote_environment_ava .await; plan.assert_visible_contains(&["exec_command", "write_stdin"]); - plan.assert_visible_lacks(&["shell_command"]); - plan.assert_registered_lacks(&["shell_command"]); assert!(has_parameter(plan.visible_spec("exec_command"), "shell")); assert!(has_parameter( plan.visible_spec("exec_command"), @@ -960,7 +946,6 @@ async fn environment_count_controls_environment_backed_tools() { let multiple_environments = probe(|turn| { duplicate_primary_environment(turn); set_feature(turn, Feature::ShellTool, /*enabled*/ true); - set_feature(turn, Feature::UnifiedExec, /*enabled*/ true); set_feature(turn, Feature::RequestPermissionsTool, /*enabled*/ true); Arc::make_mut(&mut turn.model_info).apply_patch_tool_type = Some(ApplyPatchToolType::Freeform); @@ -972,8 +957,6 @@ async fn environment_count_controls_environment_backed_tools() { "view_image", "request_permissions", ]); - multiple_environments.assert_visible_lacks(&["shell_command"]); - multiple_environments.assert_registered_lacks(&["shell_command"]); assert!(has_parameter( multiple_environments.visible_spec("exec_command"), "environment_id" @@ -990,7 +973,6 @@ async fn environment_count_controls_environment_backed_tools() { #[tokio::test] async fn environment_tools_follow_the_step_context() { let (_session, mut turn) = make_session_and_context().await; - set_feature(&mut turn, Feature::UnifiedExec, /*enabled*/ true); Arc::make_mut(&mut turn.model_info).apply_patch_tool_type = Some(ApplyPatchToolType::Freeform); let environments = turn.environments.clone(); @@ -1346,7 +1328,7 @@ async fn strict_namespace_ownership_requires_tool_namespace_inventory_opt_in() { async fn unified_tool_runtimes_preserve_source_order_and_collision_priority() { let plan = probe_with( |turn| { - set_features(turn, &[Feature::ShellTool, Feature::UnifiedExec]); + set_features(turn, &[Feature::ShellTool]); set_feature(turn, Feature::ShellZshFork, /*enabled*/ false); Arc::make_mut(&mut turn.model_info).shell_type = ConfigShellToolType::UnifiedExec; }, diff --git a/codex-rs/core/tests/common/zsh_fork.rs b/codex-rs/core/tests/common/zsh_fork.rs index 8736da72c3..0a0fb14d85 100644 --- a/codex-rs/core/tests/common/zsh_fork.rs +++ b/codex-rs/core/tests/common/zsh_fork.rs @@ -106,15 +106,6 @@ where .permissions .set_permission_profile(permission_profile) .expect("set permission profile"); - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - config - .features - .enable(Feature::UnifiedExecZshFork) - .expect("test config should allow feature update"); }); builder.build(server).await } diff --git a/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs b/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs index 53531a2a31..a99c891421 100644 --- a/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs +++ b/codex-rs/core/tests/remote_env_windows/remote_env_windows_test.rs @@ -3,7 +3,6 @@ use anyhow::Context; use anyhow::Result; use codex_exec_server::REMOTE_ENVIRONMENT_ID; -use codex_features::Feature; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; @@ -97,14 +96,7 @@ async fn windows_exec_server_runs_with_native_shell_and_cwd() -> Result<()> { let mut builder = test_codex() .with_model("gpt-5.2") - .with_exec_server_url(exec_server_url) - .with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + .with_exec_server_url(exec_server_url); let test = builder.build(&server).await?; let (sandbox_policy, permission_profile) = turn_permission_fields(PermissionProfile::Disabled, test.config.cwd.as_path()); diff --git a/codex-rs/core/tests/suite/agent_execution.rs b/codex-rs/core/tests/suite/agent_execution.rs index 353ca07633..dd77cdb547 100644 --- a/codex-rs/core/tests/suite/agent_execution.rs +++ b/codex-rs/core/tests/suite/agent_execution.rs @@ -240,13 +240,12 @@ async fn v2_residency_reload_preserves_inherited_environment_and_tools() -> Resu .with_model("gpt-5.6-sol") .with_exec_server_url("none") .with_config(|config| { - for feature in [Feature::Collab, Feature::MultiAgentV2, Feature::UnifiedExec] { + for feature in [Feature::Collab, Feature::MultiAgentV2] { config .features .enable(feature) .expect("test config should allow feature update"); } - config.use_experimental_unified_exec_tool = true; config.multi_agent_v2.max_concurrent_threads_per_session = 2; config .permissions diff --git a/codex-rs/core/tests/suite/cyber_exec_policy.rs b/codex-rs/core/tests/suite/cyber_exec_policy.rs index 10ff99db2c..f3e37df9a9 100644 --- a/codex-rs/core/tests/suite/cyber_exec_policy.rs +++ b/codex-rs/core/tests/suite/cyber_exec_policy.rs @@ -3,7 +3,6 @@ use codex_core::TurnInputRequest; use codex_core::config::Config; use codex_core::config::Constrained; use codex_core::sandboxing::SandboxPermissions; -use codex_features::Feature; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::openai_models::MODEL_SPECIALTY_CYBER; use codex_protocol::protocol::AskForApproval; @@ -39,23 +38,6 @@ use test_case::test_case; const TEST_COMMAND: &str = "git version"; const SAVED_PREFIX: &str = r#"["git", "version"]"#; -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -enum CommandTool { - UnifiedExec, -} - -impl CommandTool { - fn configure(self, config: &mut Config) { - if self == Self::UnifiedExec { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("enable unified exec"); - } - } -} - #[derive(Clone, Copy, Debug, Eq, PartialEq)] enum ModelSpecialty { Cyber, @@ -89,19 +71,17 @@ fn configure_saved_prefix_and_guardian(config: &mut Config) { .expect("set workspace sandbox policy"); } -fn command_response(response_id: &str, call_id: &str, command_tool: CommandTool) -> Result { - let (tool_name, command_key) = match command_tool { - CommandTool::UnifiedExec => ("exec_command", "cmd"), - }; - let mut args = json!({ +fn command_response(response_id: &str, call_id: &str) -> Result { + let args = json!({ + "cmd": TEST_COMMAND, "sandbox_permissions": SandboxPermissions::RequireEscalated, "justification": "Check whether a saved prefix bypasses Guardian.", "prefix_rule": ["git", "version"], }); - args[command_key] = json!(TEST_COMMAND); + Ok(sse(vec![ ev_response_created(response_id), - ev_function_call(call_id, tool_name, &serde_json::to_string(&args)?), + ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?), ev_completed(response_id), ])) } @@ -128,12 +108,11 @@ async fn submit_model_turn(test: &TestCodex, model: &str, prompt: &str) -> Resul test.submit_text_turn(prompt).await } -#[test_case(CommandTool::UnifiedExec, ModelSpecialty::Cyber, ShellBackend::Standard; "cyber unified exec is reviewed")] -#[test_case(CommandTool::UnifiedExec, ModelSpecialty::Cyber, ShellBackend::ZshFork; "cyber zsh unified exec is reviewed")] -#[test_case(CommandTool::UnifiedExec, ModelSpecialty::General, ShellBackend::Standard; "general unified exec keeps saved approval")] +#[test_case(ModelSpecialty::Cyber, ShellBackend::Standard; "cyber unified exec is reviewed")] +#[test_case(ModelSpecialty::Cyber, ShellBackend::ZshFork; "cyber zsh unified exec is reviewed")] +#[test_case(ModelSpecialty::General, ShellBackend::Standard; "general unified exec keeps saved approval")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn saved_prefix_only_bypasses_guardian_for_general_models( - command_tool: CommandTool, model_specialty: ModelSpecialty, shell_backend: ShellBackend, ) -> Result<()> { @@ -158,16 +137,7 @@ async fn saved_prefix_only_bypasses_guardian_for_general_models( model.model_specialty = Some(MODEL_SPECIALTY_CYBER.to_string()); } }) - .with_config(move |config| { - configure_saved_prefix_and_guardian(config); - command_tool.configure(config); - if shell_backend == ShellBackend::ZshFork && command_tool == CommandTool::UnifiedExec { - config - .features - .enable(Feature::UnifiedExecZshFork) - .expect("enable unified-exec zsh fork"); - } - }); + .with_config(configure_saved_prefix_and_guardian); let test = builder.build_with_auto_env(&server).await?; let expected_guardian_review_count = match (model_specialty, shell_backend) { (ModelSpecialty::General, _) => 0, @@ -178,7 +148,6 @@ async fn saved_prefix_only_bypasses_guardian_for_general_models( let mut response_bodies = vec![command_response( "parent-saved-prefix-command", "saved-prefix-command", - command_tool, )?]; for review_index in 0..expected_guardian_review_count { response_bodies.push(guardian_allow_response(&format!( @@ -217,11 +186,8 @@ async fn saved_prefix_only_bypasses_guardian_for_general_models( Ok(()) } -#[test_case(CommandTool::UnifiedExec; "unified exec")] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn cyber_model_user_approval_never_offers_a_reusable_prefix( - command_tool: CommandTool, -) -> Result<()> { +async fn cyber_model_user_approval_never_offers_a_reusable_prefix() -> Result<()> { skip_if_no_network!(Ok(())); skip_if_sandbox!(Ok(())); skip_if_wine_exec!(Ok(()), "command approval requires host-native paths"); @@ -233,7 +199,6 @@ async fn cyber_model_user_approval_never_offers_a_reusable_prefix( }) .with_config(move |config| { configure_saved_prefix_and_guardian(config); - command_tool.configure(config); config.approvals_reviewer = ApprovalsReviewer::User; }); let test = builder.build_with_auto_env(&server).await?; @@ -242,11 +207,7 @@ async fn cyber_model_user_approval_never_offers_a_reusable_prefix( let responses = mount_sse_sequence( &server, vec![ - command_response( - "parent-one-time-approval", - "one-time-approval", - command_tool, - )?, + command_response("parent-one-time-approval", "one-time-approval")?, sse_completed("parent-one-time-complete"), ], ) @@ -322,24 +283,12 @@ async fn switching_models_suppresses_and_restores_saved_prefix_approvals() -> Re let responses = mount_sse_sequence( &server, vec![ - command_response( - "parent-general-first-command", - "general-first-command", - CommandTool::UnifiedExec, - )?, + command_response("parent-general-first-command", "general-first-command")?, sse_completed("parent-general-first-complete"), - command_response( - "parent-cyber-command", - "cyber-command", - CommandTool::UnifiedExec, - )?, + command_response("parent-cyber-command", "cyber-command")?, guardian_allow_response("guardian-cyber-review"), sse_completed("parent-cyber-complete"), - command_response( - "parent-general-last-command", - "general-last-command", - CommandTool::UnifiedExec, - )?, + command_response("parent-general-last-command", "general-last-command")?, sse_completed("parent-general-last-complete"), ], ) diff --git a/codex-rs/core/tests/suite/deprecation_notice.rs b/codex-rs/core/tests/suite/deprecation_notice.rs index 9248486f53..6858986f32 100644 --- a/codex-rs/core/tests/suite/deprecation_notice.rs +++ b/codex-rs/core/tests/suite/deprecation_notice.rs @@ -20,14 +20,12 @@ async fn emits_deprecation_notice_for_legacy_feature_flag() -> anyhow::Result<() let mut builder = test_codex().with_config(|config| { let mut features = config.features.get().clone(); - features.enable(Feature::UnifiedExec); features .record_legacy_usage_force("use_experimental_unified_exec_tool", Feature::UnifiedExec); config .features .set(features) .expect("test config should allow managed feature metadata updates"); - config.use_experimental_unified_exec_tool = true; }); let TestCodex { codex, .. } = builder.build(&server).await?; diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 1cfc7bc228..1e88b17147 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -3097,12 +3097,7 @@ async fn permission_request_hook_sees_raw_exec_command_input() -> Result<()> { .expect("failed to write permission request hook test fixture"); }) .with_config(|config| { - config.use_experimental_unified_exec_tool = true; trust_discovered_hooks(config); - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); }); let test = builder.build(&server).await?; @@ -3684,73 +3679,28 @@ Path(r"{hook_finished_path}").write_text("finished", encoding="utf-8") Ok(()) } -#[derive(Clone, Copy)] -enum BashRewriteSurface { - ExecCommand, -} - -impl BashRewriteSurface { - fn slug(self) -> &'static str { - match self { - BashRewriteSurface::ExecCommand => "exec-command", - } - } - - fn tool_call(self, call_id: &str, command_text: &str) -> Result { - match self { - BashRewriteSurface::ExecCommand => Ok(ev_function_call( - call_id, - "exec_command", - &serde_json::to_string(&serde_json::json!({ "cmd": command_text }))?, - )), - } - } - - fn original_command(self, marker: &Path) -> String { - match self { - BashRewriteSurface::ExecCommand => { - format!("git init --quiet {}", marker.display()) - } - } - } - - fn rewritten_command(self, marker: &Path) -> String { - match self { - BashRewriteSurface::ExecCommand => { - format!("git init {}", marker.display()) - } - } - } - - fn configure(self, config: &mut Config) { - trust_discovered_hooks(config); - if matches!(self, BashRewriteSurface::ExecCommand) { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - } - } -} - -async fn assert_pre_tool_use_rewrites_bash_surface(surface: BashRewriteSurface) -> Result<()> { +#[tokio::test] +async fn pre_tool_use_rewrites_exec_command_before_execution() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; - let slug = surface.slug(); + let slug = "exec-command"; let call_id = format!("pretooluse-{slug}-rewrite"); let marker_dir = TempDir::new()?; let original_marker = marker_dir.path().join("original"); let rewritten_marker = marker_dir.path().join("rewritten"); - let original_command = surface.original_command(&original_marker); - let rewritten_command = surface.rewritten_command(&rewritten_marker); + let original_command = format!("git init --quiet {}", original_marker.display()); + let rewritten_command = format!("git init {}", rewritten_marker.display()); let responses = mount_sse_sequence( &server, vec![ sse(vec![ ev_response_created("resp-1"), - surface.tool_call(&call_id, &original_command)?, + ev_function_call( + &call_id, + "exec_command", + &serde_json::to_string(&serde_json::json!({ "cmd": original_command }))?, + ), ev_completed("resp-1"), ]), sse(vec![ @@ -3768,7 +3718,7 @@ async fn assert_pre_tool_use_rewrites_bash_surface(surface: BashRewriteSurface) write_updating_pre_tool_use_hook(home, "^Bash$", &updated_input) .expect("failed to write updating pre tool use hook fixture"); }) - .with_config(move |config| surface.configure(config)); + .with_config(trust_discovered_hooks); let test = builder.build(&server).await?; test.submit_turn_with_permission_profile( @@ -3796,11 +3746,6 @@ async fn assert_pre_tool_use_rewrites_bash_surface(surface: BashRewriteSurface) Ok(()) } -#[tokio::test] -async fn pre_tool_use_rewrites_exec_command_before_execution() -> Result<()> { - assert_pre_tool_use_rewrites_bash_surface(BashRewriteSurface::ExecCommand).await -} - #[tokio::test] async fn pre_tool_use_rewrites_code_mode_nested_exec_command_before_execution() -> Result<()> { skip_if_no_network!(Ok(())); @@ -4433,12 +4378,7 @@ async fn pre_tool_use_blocks_exec_command_before_execution() -> Result<()> { .expect("failed to write pre tool use hook test fixture"); }) .with_config(|config| { - config.use_experimental_unified_exec_tool = true; trust_discovered_hooks(config); - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); }); let test = builder.build(&server).await?; @@ -5057,12 +4997,7 @@ async fn post_tool_use_exit_two_replaces_one_shot_exec_command_output_with_feedb .expect("failed to write post tool use hook test fixture"); }) .with_config(|config| { - config.use_experimental_unified_exec_tool = true; trust_discovered_hooks(config); - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); }); let test = builder.build(&server).await?; @@ -5129,12 +5064,7 @@ async fn post_tool_use_spills_large_feedback_message() -> Result<()> { } }) .with_config(|config| { - config.use_experimental_unified_exec_tool = true; trust_discovered_hooks(config); - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); }); let test = builder.build(&server).await?; @@ -5213,12 +5143,7 @@ async fn post_tool_use_blocks_when_exec_session_completes_via_write_stdin() -> R .expect("failed to write tool use hook test fixture"); }) .with_config(|config| { - config.use_experimental_unified_exec_tool = true; trust_discovered_hooks(config); - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); }); let test = builder.build(&server).await?; diff --git a/codex-rs/core/tests/suite/multi_exec_server_sandbox.rs b/codex-rs/core/tests/suite/multi_exec_server_sandbox.rs index 5842575fa1..840bda2797 100644 --- a/codex-rs/core/tests/suite/multi_exec_server_sandbox.rs +++ b/codex-rs/core/tests/suite/multi_exec_server_sandbox.rs @@ -5,7 +5,6 @@ use std::time::Duration; use anyhow::Context; use anyhow::Result; use anyhow::bail; -use codex_features::Feature; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_protocol::protocol::AskForApproval; @@ -108,13 +107,7 @@ async fn two_exec_servers_isolate_workspace_write_roots() -> Result<()> { let second_workspace = TempDir::new()?; let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow unified exec"); - }); + let mut builder = test_codex(); let test = builder.build(&server).await?; let environment_manager = test.thread_manager.environment_manager(); environment_manager.upsert_environment( diff --git a/codex-rs/core/tests/suite/network_approval.rs b/codex-rs/core/tests/suite/network_approval.rs index 001c2936e1..5642216432 100644 --- a/codex-rs/core/tests/suite/network_approval.rs +++ b/codex-rs/core/tests/suite/network_approval.rs @@ -2078,11 +2078,6 @@ allow_local_binding = true .with_home(home) .with_cloud_config_bundle(managed_network_requirements_loader()) .with_config(move |config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); for feature in &features { config .features diff --git a/codex-rs/core/tests/suite/remote_env.rs b/codex-rs/core/tests/suite/remote_env.rs index 959abfeedb..3bcd720afd 100644 --- a/codex-rs/core/tests/suite/remote_env.rs +++ b/codex-rs/core/tests/suite/remote_env.rs @@ -204,14 +204,7 @@ fn test_codex_with_wait_for_environment() -> TestCodexBuilder { } async fn unified_exec_test(server: &wiremock::MockServer) -> Result { - let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - let result = config.features.enable(Feature::UnifiedExec); - assert!( - result.is_ok(), - "unified exec should enable for test: {result:?}", - ); - }); + let mut builder = test_codex(); builder.build_with_remote_and_local_env(server).await } @@ -405,13 +398,7 @@ async fn explicit_remote_shell_runs_in_remote_cwd() -> Result<()> { "login": false, "yield_time_ms": 10_000, }))?; - let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let response_mock = mount_sse_sequence( &server, @@ -468,11 +455,6 @@ async fn environment_permissions_follow_configuration_ownership() -> Result<()> let server = start_mock_server().await; let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); config .permissions .set_permission_profile(PermissionProfile::workspace_write()) @@ -1229,12 +1211,7 @@ async fn shared_executor_keeps_ready_capability_roots_scoped_to_each_attachment( let server = start_mock_server().await; let mut extensions = ExtensionRegistryBuilder::new(); extensions.prompt_contributor(Arc::new(ReadyCapabilityRootsTestExtension::default())); - let mut builder = test_codex() - .with_extensions(Arc::new(extensions.build())) - .with_config(|config| { - config.use_experimental_unified_exec_tool = true; - assert!(config.features.enable(Feature::UnifiedExec).is_ok()); - }); + let mut builder = test_codex().with_extensions(Arc::new(extensions.build())); let test = builder.build_with_auto_env(&server).await?; let selection = test .codex @@ -1498,9 +1475,7 @@ async fn pending_attachment_installs_configuration_before_waiting_turn_resumes() let mut builder = test_codex() .with_extensions(Arc::new(extensions.build())) .with_config(|config| { - config.use_experimental_unified_exec_tool = true; assert!(config.features.enable(Feature::DeferredExecutor).is_ok()); - assert!(config.features.enable(Feature::UnifiedExec).is_ok()); config .permissions .set_permission_profile(PermissionProfile::read_only()) @@ -1827,9 +1802,7 @@ async fn ready_before_selection_resolves_resumed_thread_capability_root_after_wa .with_extensions(Arc::new(extensions.build())) .with_config(|config| { config.project_doc_max_bytes = 0; - config.use_experimental_unified_exec_tool = true; assert!(config.features.enable(Feature::DeferredExecutor).is_ok()); - assert!(config.features.enable(Feature::UnifiedExec).is_ok()); }); let test = builder.build(&server).await?; let refreshed_root = SelectedCapabilityRoot { @@ -2018,9 +1991,7 @@ async fn deferred_executor_stays_pending_after_materialization() -> Result<()> { ) .await; let mut builder = test_codex_with_wait_for_environment().with_config(|config| { - config.use_experimental_unified_exec_tool = true; assert!(config.features.enable(Feature::DeferredExecutor).is_ok()); - assert!(config.features.enable(Feature::UnifiedExec).is_ok()); }); let test = timeout(Duration::from_secs(5), builder.build(&server)) .await @@ -2310,11 +2281,9 @@ async fn deferred_executor_guardian_uses_newly_ready_step_environment() -> Resul .with_exec_server_url(format!("ws://{}", listener.local_addr()?)) .with_config(|config| { config.project_doc_max_bytes = 0; - config.use_experimental_unified_exec_tool = true; config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); config.approvals_reviewer = ApprovalsReviewer::AutoReview; assert!(config.features.enable(Feature::DeferredExecutor).is_ok()); - assert!(config.features.enable(Feature::UnifiedExec).is_ok()); }); let (attach_tx, attach_rx) = tokio::sync::oneshot::channel(); let (shutdown_tx, shutdown_rx) = tokio::sync::oneshot::channel(); @@ -3049,13 +3018,8 @@ async fn remote_request_permissions_grant_unblocks_later_remote_exec() -> Result let server = start_mock_server().await; let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; config.permissions.approval_policy = Constrained::allow_any(AskForApproval::OnRequest); config.approvals_reviewer = ApprovalsReviewer::User; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); config .features .enable(Feature::ExecPermissionApprovals) diff --git a/codex-rs/core/tests/suite/request_permissions.rs b/codex-rs/core/tests/suite/request_permissions.rs index cced118981..fa48e01442 100644 --- a/codex-rs/core/tests/suite/request_permissions.rs +++ b/codex-rs/core/tests/suite/request_permissions.rs @@ -108,21 +108,6 @@ fn parse_result(item: &Value) -> CommandResult { } } -fn command_event_with_request_permissions( - call_id: &str, - command: &str, - additional_permissions: &S, -) -> Result { - let args = json!({ - "cmd": command, - "yield_time_ms": 10_000_u64, - "sandbox_permissions": SandboxPermissions::WithAdditionalPermissions, - "additional_permissions": additional_permissions, - }); - let args_str = serde_json::to_string(&args)?; - Ok(ev_function_call(call_id, "exec_command", &args_str)) -} - fn request_permissions_tool_event( call_id: &str, reason: &str, @@ -136,19 +121,10 @@ fn request_permissions_tool_event( Ok(ev_function_call(call_id, "request_permissions", &args_str)) } -fn command_event(call_id: &str, command: &str) -> Result { - let args = json!({ - "cmd": command, - "yield_time_ms": 10_000_u64, - }); - let args_str = serde_json::to_string(&args)?; - Ok(ev_function_call(call_id, "exec_command", &args_str)) -} - fn exec_command_event(call_id: &str, command: &str) -> Result { let args = json!({ "cmd": command, - "yield_time_ms": 1_000_u64, + "yield_time_ms": 10_000_u64, }); let args_str = serde_json::to_string(&args)?; Ok(ev_function_call(call_id, "exec_command", &args_str)) @@ -161,7 +137,7 @@ fn exec_command_event_with_request_permissions( ) -> Result { let args = json!({ "cmd": command, - "yield_time_ms": 1_000_u64, + "yield_time_ms": 10_000_u64, "sandbox_permissions": SandboxPermissions::WithAdditionalPermissions, "additional_permissions": additional_permissions, }); @@ -362,7 +338,8 @@ async fn with_additional_permissions_requires_approval_under_on_request() -> Res )), ..Default::default() }; - let event = command_event_with_request_permissions(call_id, command, &requested_permissions)?; + let event = + exec_command_event_with_request_permissions(call_id, command, &requested_permissions)?; let _ = mount_sse_once( &server, @@ -911,7 +888,8 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_cwd )), ..Default::default() }; - let event = command_event_with_request_permissions(call_id, &command, &requested_permissions)?; + let event = + exec_command_event_with_request_permissions(call_id, &command, &requested_permissions)?; let _ = mount_sse_once( &server, @@ -1015,7 +993,8 @@ async fn read_only_with_additional_permissions_does_not_widen_to_unrequested_tmp )), ..Default::default() }; - let event = command_event_with_request_permissions(call_id, &command, &requested_permissions)?; + let event = + exec_command_event_with_request_permissions(call_id, &command, &requested_permissions)?; let _ = mount_sse_once( &server, @@ -1126,7 +1105,8 @@ async fn workspace_write_with_additional_permissions_can_write_outside_cwd() -> )), ..RequestPermissionProfile::default() }; - let event = command_event_with_request_permissions(call_id, &command, &requested_permissions)?; + let event = + exec_command_event_with_request_permissions(call_id, &command, &requested_permissions)?; let _ = mount_sse_once( &server, @@ -1231,7 +1211,8 @@ async fn with_additional_permissions_denied_approval_blocks_execution() -> Resul )), ..Default::default() }; - let event = command_event_with_request_permissions(call_id, &command, &requested_permissions)?; + let event = + exec_command_event_with_request_permissions(call_id, &command, &requested_permissions)?; let _ = mount_sse_once( &server, @@ -1584,7 +1565,7 @@ async fn request_permissions_grants_apply_to_later_exec_command_calls_without_in ]), sse(vec![ ev_response_created("resp-sticky-shell-independent-2"), - command_event("shell-call", &command)?, + exec_command_event("shell-call", &command)?, ev_completed("resp-sticky-shell-independent-2"), ]), sse(vec![ @@ -2122,10 +2103,6 @@ async fn denied_child_permissions_require_fresh_approval( .permissions .set_permission_profile(CorePermissionProfile::read_only()) .expect("set permission profile"); - config - .features - .enable(Feature::UnifiedExec) - .expect("enable unified exec"); let inline_permissions = if mode == ApprovalMode::InlineFeatureDisabled { config.features.disable(Feature::ExecPermissionApprovals) } else { diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index 06c631a370..3897f43966 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -2447,7 +2447,7 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re effort: codex_protocol::openai_models::ReasoningEffort::Medium, description: "Medium".to_string(), }], - shell_type: ConfigShellToolType::Default, + shell_type: ConfigShellToolType::UnifiedExec, visibility: ModelVisibility::List, supported_in_api: true, priority: 1, diff --git a/codex-rs/core/tests/suite/shell_snapshot.rs b/codex-rs/core/tests/suite/shell_snapshot.rs index 140606d107..d24e15377b 100644 --- a/codex-rs/core/tests/suite/shell_snapshot.rs +++ b/codex-rs/core/tests/suite/shell_snapshot.rs @@ -127,11 +127,6 @@ async fn run_snapshot_command_with_options( shell_environment_set, } = options; let builder = test_codex().with_config(move |config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); config .features .enable(Feature::ShellSnapshot) @@ -320,11 +315,6 @@ async fn linux_unified_exec_uses_shell_snapshot() -> Result<()> { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn unified_exec_snapshot_preserves_shell_environment_policy_set() -> Result<()> { let builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); config .features .enable(Feature::ShellSnapshot) diff --git a/codex-rs/core/tests/suite/skills_extension.rs b/codex-rs/core/tests/suite/skills_extension.rs index e7c3091df4..fe929e0a59 100644 --- a/codex-rs/core/tests/suite/skills_extension.rs +++ b/codex-rs/core/tests/suite/skills_extension.rs @@ -1315,11 +1315,6 @@ async fn executor_skill_invocation_is_environment_scoped_and_deduplicated() -> R .with_config(move |config| { configure_catalog_test(config); config.chatgpt_base_url = chatgpt_base_url; - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("unified exec should be configurable in tests"); }); let test = builder.build_with_auto_env(&server).await?; test.submit_turn("Read the executor skill twice.").await?; diff --git a/codex-rs/core/tests/suite/unified_exec.rs b/codex-rs/core/tests/suite/unified_exec.rs index e94cedadb6..3b6cabfdef 100644 --- a/codex-rs/core/tests/suite/unified_exec.rs +++ b/codex-rs/core/tests/suite/unified_exec.rs @@ -9,7 +9,6 @@ use std::sync::OnceLock; use anyhow::Context; use anyhow::Result; use codex_exec_server::CreateDirectoryOptions; -use codex_features::Feature; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::EnvironmentVariablePattern; use codex_protocol::config_types::ModeKind; @@ -259,11 +258,6 @@ async fn exec_command_hides_and_rejects_login_when_disabled() -> Result<()> { let builder = test_codex().with_model("gpt-5.4").with_config(|config| { config.permissions.allow_login_shell = false; - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); }); let harness = TestCodexHarness::with_builder(builder).await?; let call_id = "exec-command-login-disabled"; @@ -369,11 +363,6 @@ async fn exec_command_uses_installed_environment_shell_policy_with_explicit_over skip_if_no_network!(Ok(())); let builder = test_codex().with_model("gpt-5.4").with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); config.permissions.shell_environment_policy = ShellEnvironmentPolicy { inherit: ShellEnvironmentPolicyInherit::None, include_only: vec![EnvironmentVariablePattern::new_case_insensitive("DROP")], @@ -468,13 +457,7 @@ async fn unified_exec_intercepts_apply_patch_exec_command() -> Result<()> { skip_if_sandbox!(Ok(())); skip_if_host_windows!(Ok(())); - let builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let builder = test_codex(); let harness = TestCodexHarness::with_builder(builder).await?; let patch = @@ -607,13 +590,7 @@ async fn unified_exec_rejects_justification_without_sandbox_permissions() -> Res skip_if_no_network!(Ok(())); let server = start_mock_server().await; - let mut builder = test_codex().with_model("gpt-5.2").with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex().with_model("gpt-5.2"); let test = builder.build_with_auto_env(&server).await?; let call_id = "uexec-missing-sandbox-permissions"; @@ -686,13 +663,7 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_model("gpt-5.2").with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex().with_model("gpt-5.2"); let test = builder.build_with_auto_env(&server).await?; let cwd = test.config.cwd.to_path_buf(); @@ -749,13 +720,7 @@ async fn unified_exec_resolves_relative_workdir() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_model("gpt-5.2").with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex().with_model("gpt-5.2"); let test = builder.build_with_auto_env(&server).await?; let workdir_rel = std::path::PathBuf::from("uexec_relative_workdir"); @@ -819,13 +784,7 @@ async fn unified_exec_respects_workdir_override() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_model("gpt-5.2").with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex().with_model("gpt-5.2"); let test = builder.build_with_auto_env(&server).await?; let workdir = create_workspace_directory(&test, "uexec_workdir_test").await?; @@ -885,13 +844,7 @@ async fn unified_exec_emits_exec_command_end_event() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let call_id = "uexec-end-event"; @@ -959,13 +912,7 @@ async fn unified_exec_emits_output_delta_for_exec_command() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let call_id = "uexec-delta-1"; @@ -1018,13 +965,7 @@ async fn unified_exec_full_lifecycle_with_background_end_event() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let call_id = "uexec-full-lifecycle"; @@ -1262,11 +1203,6 @@ allow_local_binding = true .with_home(home) .with_cloud_config_bundle(managed_network_requirements_loader()) .with_config(move |config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); config.permissions.approval_policy = Constrained::allow_any(AskForApproval::Never); config .permissions @@ -1351,13 +1287,7 @@ async fn unified_exec_emits_terminal_interaction_for_write_stdin() -> Result<()> let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let open_call_id = "uexec-open"; @@ -1435,13 +1365,7 @@ async fn unified_exec_terminal_interaction_captures_delayed_output() -> Result<( let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let open_call_id = "uexec-delayed-open"; @@ -1616,13 +1540,7 @@ async fn unified_exec_emits_one_begin_and_one_end_event() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let open_call_id = "uexec-open-session"; @@ -1740,12 +1658,7 @@ async fn exec_command_reports_chunk_and_exit_metadata() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let call_id = "uexec-metadata"; @@ -1835,12 +1748,7 @@ async fn exec_command_clamps_model_requested_max_output_tokens_to_policy() -> Re let server = start_mock_server().await; let mut builder = test_codex().with_model("gpt-5.4").with_config(|config| { - config.use_experimental_unified_exec_tool = true; config.tool_output_token_limit = Some(50); - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); }); let test = builder.build_with_auto_env(&server).await?; @@ -1899,12 +1807,7 @@ async fn write_stdin_clamps_model_requested_max_output_tokens_to_policy() -> Res let server = start_mock_server().await; let mut builder = test_codex().with_model("gpt-5.4").with_config(|config| { - config.use_experimental_unified_exec_tool = true; config.tool_output_token_limit = Some(50); - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); }); let test = builder.build_with_auto_env(&server).await?; @@ -1989,12 +1892,7 @@ async fn unified_exec_defaults_to_pipe() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let call_id = "uexec-default-pipe"; @@ -2059,12 +1957,7 @@ async fn unified_exec_can_enable_tty() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let call_id = "uexec-tty-enabled"; @@ -2126,12 +2019,7 @@ async fn unified_exec_respects_early_exit_notifications() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let call_id = "uexec-early-exit"; @@ -2210,12 +2098,7 @@ async fn write_stdin_returns_exit_metadata_and_clears_session() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let start_call_id = "uexec-cat-start"; @@ -2418,12 +2301,7 @@ async fn assert_write_stdin_ctrl_c_interrupts_non_tty_session( let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let start_call_id = format!("uexec-non-tty-interrupt-{test_name}-start"); @@ -2541,12 +2419,7 @@ async fn write_stdin_ctrl_c_terminates_non_tty_session_on_windows() -> Result<() let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let start_call_id = "uexec-windows-interrupt-start"; @@ -2635,13 +2508,7 @@ async fn unified_exec_emits_end_event_when_session_dies_via_stdin() -> Result<() let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let start_call_id = "uexec-end-on-exit-start"; @@ -2727,13 +2594,7 @@ async fn unified_exec_keeps_long_running_session_after_turn_end() -> Result<()> let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let TestCodex { codex, cwd, @@ -2834,13 +2695,7 @@ async fn unified_exec_interrupt_preserves_long_running_session() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let TestCodex { codex, cwd, @@ -2930,12 +2785,7 @@ async fn unified_exec_reuses_session_via_stdin() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let first_call_id = "uexec-start"; @@ -3029,13 +2879,7 @@ async fn unified_exec_streams_after_lagged_output() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let script = r#"python3 - <<'PY' @@ -3146,12 +2990,7 @@ async fn unified_exec_timeout_and_followup_poll() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let first_call_id = "uexec-timeout"; @@ -3239,12 +3078,7 @@ async fn unified_exec_formats_large_output_summary() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let output_line = "token token \n"; @@ -3335,12 +3169,7 @@ async fn unified_exec_runs_under_sandbox() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let TestCodex { codex, cwd, @@ -3428,10 +3257,6 @@ async fn unified_exec_enforces_glob_deny_read_policy() -> Result<()> { let server = start_mock_server().await; let mut builder = test_codex().with_config(move |config| { - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); let mut file_system_sandbox_policy = FileSystemSandboxPolicy::default(); file_system_sandbox_policy .entries @@ -3567,13 +3392,7 @@ async fn unified_exec_python_prompt_under_seatbelt() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let TestCodex { codex, cwd, @@ -3703,12 +3522,7 @@ async fn unified_exec_runs_on_all_platforms() -> Result<()> { let server = start_mock_server().await; - let mut builder = test_codex().with_config(|config| { - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex(); let test = builder.build_with_auto_env(&server).await?; let call_id = "uexec"; @@ -3766,13 +3580,7 @@ async fn write_stdin_calls_run_in_parallel_across_sessions() -> Result<()> { skip_if_target_windows!(Ok(()), "uses bash and POSIX file rendezvous commands"); let server = start_mock_server().await; - let mut builder = test_codex().with_model("gpt-5.4").with_config(|config| { - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); - }); + let mut builder = test_codex().with_model("gpt-5.4"); let test = builder.build_with_auto_env(&server).await?; let start_args = serde_json::to_string(&json!({ diff --git a/codex-rs/core/tests/suite/unified_exec_process_events.rs b/codex-rs/core/tests/suite/unified_exec_process_events.rs index f092a1af34..d0f9045965 100644 --- a/codex-rs/core/tests/suite/unified_exec_process_events.rs +++ b/codex-rs/core/tests/suite/unified_exec_process_events.rs @@ -4,7 +4,6 @@ use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use codex_config::test_support::CloudConfigBundleFixture; use codex_core::TurnInputRequest; -use codex_features::Feature; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; @@ -651,7 +650,6 @@ timeout = 900 } let mut builder = builder.with_config(move |config| { config.project_doc_max_bytes = 0; - config.use_experimental_unified_exec_tool = true; if matches!(scenario, PushedExecScenario::ElevatedPowerShell) { config.set_windows_elevated_sandbox_enabled(/*value*/ true); } @@ -661,10 +659,6 @@ timeout = 900 config.approvals_reviewer = ApprovalsReviewer::AutoReview; config.bypass_hook_trust = true; } - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); }); let test = timeout(Duration::from_secs(5), builder.build(&server)) .await diff --git a/codex-rs/core/tests/suite/workspace_roots.rs b/codex-rs/core/tests/suite/workspace_roots.rs index c83525647f..c3fdae0fb6 100644 --- a/codex-rs/core/tests/suite/workspace_roots.rs +++ b/codex-rs/core/tests/suite/workspace_roots.rs @@ -2,7 +2,6 @@ use anyhow::Context; use anyhow::Result; use codex_exec_server::CreateDirectoryOptions; use codex_exec_server::RemoveOptions; -use codex_features::Feature; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::NetworkSandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; @@ -46,11 +45,6 @@ async fn workspace_roots_test(server: &MockServer) -> Result { .expect("test workspace should be canonicalizable") .abs(); } - config.use_experimental_unified_exec_tool = true; - config - .features - .enable(Feature::UnifiedExec) - .expect("test config should allow feature update"); config.workspace_roots = vec![config.cwd.clone()]; config.set_windows_sandbox_enabled(/*value*/ true); }); diff --git a/codex-rs/models-manager/src/model_info.rs b/codex-rs/models-manager/src/model_info.rs index eaf8d3274a..0f41823b31 100644 --- a/codex-rs/models-manager/src/model_info.rs +++ b/codex-rs/models-manager/src/model_info.rs @@ -145,7 +145,7 @@ pub fn model_info_from_slug(slug: &str) -> ModelInfo { description: None, default_reasoning_level: None, supported_reasoning_levels: Vec::new(), - shell_type: ConfigShellToolType::Default, + shell_type: ConfigShellToolType::UnifiedExec, visibility: ModelVisibility::None, supported_in_api: true, priority: 99, diff --git a/codex-rs/protocol/src/openai_models.rs b/codex-rs/protocol/src/openai_models.rs index 30c9c3b118..3dc5aa6261 100644 --- a/codex-rs/protocol/src/openai_models.rs +++ b/codex-rs/protocol/src/openai_models.rs @@ -297,9 +297,7 @@ pub enum ModelVisibility { #[serde(rename_all = "snake_case")] #[strum(serialize_all = "snake_case")] pub enum ConfigShellToolType { - Default, - Local, - #[serde(alias = "shell_command")] + #[serde(alias = "default", alias = "local", alias = "shell_command")] UnifiedExec, Disabled, } @@ -888,10 +886,13 @@ mod tests { #[test] fn legacy_shell_model_metadata_deserializes_as_unified_exec() { - assert_eq!( - from_str::("\"shell_command\"").expect("legacy shell type"), - ConfigShellToolType::UnifiedExec - ); + for legacy_shell_type in ["default", "local", "shell_command"] { + assert_eq!( + from_str::(&format!("\"{legacy_shell_type}\"")) + .expect("legacy shell type"), + ConfigShellToolType::UnifiedExec + ); + } assert_eq!( to_string(&ConfigShellToolType::UnifiedExec).expect("serialize unified shell type"), "\"unified_exec\"" diff --git a/codex-rs/thread-manager-sample/src/main.rs b/codex-rs/thread-manager-sample/src/main.rs index 155e3026a0..32461981b9 100644 --- a/codex-rs/thread-manager-sample/src/main.rs +++ b/codex-rs/thread-manager-sample/src/main.rs @@ -296,7 +296,6 @@ fn new_config(model: Option, arg0_paths: Arg0DispatchPaths) -> anyhow::R update_plan_enabled: true, tool_registry: Default::default(), code_mode: Default::default(), - use_experimental_unified_exec_tool: false, background_terminal_max_timeout: 300_000, ghost_snapshot: GhostSnapshotConfig::default(), multi_agent_v2: MultiAgentV2Config::default(), diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 879318f80d..86491196b3 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -73,12 +73,9 @@ pub use tool_call::TurnItemEmissionFuture; pub use tool_call::TurnItemEmitter; pub use tool_config::ToolEnvironmentMode; pub use tool_config::ToolUserShellType; -pub use tool_config::UnifiedExecFeatureMode; pub use tool_config::UnifiedExecShellMode; pub use tool_config::ZshForkConfig; pub use tool_config::request_user_input_available_modes; -pub use tool_config::shell_type_for_model_and_features; -pub use tool_config::unified_exec_feature_mode_for_features; pub use tool_definition::ToolDefinition; pub use tool_discovery::DiscoverablePluginInfo; pub use tool_discovery::DiscoverableTool; diff --git a/codex-rs/tools/src/tool_config.rs b/codex-rs/tools/src/tool_config.rs index 1f525d4181..ea2a9688a7 100644 --- a/codex-rs/tools/src/tool_config.rs +++ b/codex-rs/tools/src/tool_config.rs @@ -2,20 +2,9 @@ use codex_features::Feature; use codex_features::Features; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::TUI_VISIBLE_COLLABORATION_MODES; -use codex_protocol::openai_models::ConfigShellToolType; -use codex_protocol::openai_models::ModelInfo; use codex_utils_absolute_path::AbsolutePathBuf; use std::path::PathBuf; -#[derive(Debug, Clone, Copy, Eq, PartialEq)] -pub enum UnifiedExecFeatureMode { - /// Unified exec should not be selected by this feature set. - /// - Disabled, - Direct, - ZshFork, -} - #[derive(Debug, Clone, Copy, Eq, PartialEq)] pub enum ToolUserShellType { Zsh, @@ -36,40 +25,6 @@ pub fn request_user_input_available_modes(features: &Features) -> Vec .collect() } -/// Returns the unified-exec mode requested by feature policy, before runtime -/// session inputs such as platform, user shell, and zsh-fork binary paths are -/// resolved. -/// -/// Disabling unified exec keeps command execution disabled. The legacy -/// composition flag can still disable zsh-fork interception independently. -pub fn unified_exec_feature_mode_for_features(features: &Features) -> UnifiedExecFeatureMode { - if !features.enabled(Feature::ShellTool) || !features.enabled(Feature::UnifiedExec) { - UnifiedExecFeatureMode::Disabled - } else if features.enabled(Feature::ShellZshFork) { - if features.enabled(Feature::UnifiedExecZshFork) { - UnifiedExecFeatureMode::ZshFork - } else { - UnifiedExecFeatureMode::Direct - } - } else { - UnifiedExecFeatureMode::Direct - } -} - -pub fn shell_type_for_model_and_features( - model_info: &ModelInfo, - features: &Features, -) -> ConfigShellToolType { - if !features.enabled(Feature::ShellTool) - || !features.enabled(Feature::UnifiedExec) - || matches!(model_info.shell_type, ConfigShellToolType::Disabled) - { - ConfigShellToolType::Disabled - } else { - ConfigShellToolType::UnifiedExec - } -} - #[derive(Debug, Clone, Eq, PartialEq)] pub enum UnifiedExecShellMode { Direct, @@ -84,13 +39,16 @@ pub struct ZshForkConfig { impl UnifiedExecShellMode { pub fn for_session( - feature_mode: UnifiedExecFeatureMode, + features: &Features, user_shell_type: ToolUserShellType, shell_zsh_path: Option<&PathBuf>, main_execve_wrapper_exe: Option<&PathBuf>, ) -> Self { if cfg!(unix) - && matches!(feature_mode, UnifiedExecFeatureMode::ZshFork) + && features.enabled(Feature::ShellTool) + && features.enabled(Feature::UnifiedExec) + && features.enabled(Feature::ShellZshFork) + && features.enabled(Feature::UnifiedExecZshFork) && matches!(user_shell_type, ToolUserShellType::Zsh) && let (Some(shell_zsh_path), Some(main_execve_wrapper_exe)) = (shell_zsh_path, main_execve_wrapper_exe) diff --git a/codex-rs/tools/src/tool_config_tests.rs b/codex-rs/tools/src/tool_config_tests.rs index e9ab1ed842..69e6c13117 100644 --- a/codex-rs/tools/src/tool_config_tests.rs +++ b/codex-rs/tools/src/tool_config_tests.rs @@ -1,142 +1,10 @@ use codex_features::Feature; use codex_features::Features; use codex_protocol::config_types::ModeKind; -use codex_protocol::openai_models::ConfigShellToolType; -use codex_protocol::openai_models::ModelInfo; -use codex_protocol::openai_models::ModelVisibility; -use codex_protocol::openai_models::TruncationPolicyConfig; use pretty_assertions::assert_eq; use super::*; -fn model_with_shell_type(shell_type: ConfigShellToolType) -> ModelInfo { - ModelInfo { - slug: "test-model".to_string(), - display_name: "Test Model".to_string(), - description: None, - default_reasoning_level: None, - supported_reasoning_levels: Vec::new(), - shell_type, - visibility: ModelVisibility::List, - supported_in_api: true, - priority: 0, - additional_speed_tiers: Vec::new(), - service_tiers: Vec::new(), - default_service_tier: None, - availability_nux: None, - upgrade: None, - model_messages: None, - include_skills_usage_instructions: false, - include_plugin_usage_instructions: false, - include_apps_usage_instructions: false, - supports_reasoning_summary_parameter: true, - default_reasoning_summary: Default::default(), - support_verbosity: false, - default_verbosity: None, - apply_patch_tool_type: None, - web_search_tool_type: Default::default(), - truncation_policy: TruncationPolicyConfig::tokens(/*limit*/ 1024), - supports_image_detail_original: false, - context_window: None, - max_context_window: None, - auto_compact_token_limit: None, - comp_hash: None, - effective_context_window_percent: 95, - experimental_supported_tools: Vec::new(), - input_modalities: codex_protocol::openai_models::default_input_modalities(), - used_fallback_model_metadata: false, - supports_search_tool: false, - use_responses_lite: false, - node_repl_auto_review_required: false, - node_repl_disabled: false, - auto_review_model_override: None, - model_specialty: None, - tool_mode: None, - multi_agent_version: None, - } -} - -fn shell_features() -> Features { - let mut features = Features::with_defaults(); - features.enable(Feature::ShellTool); - features.disable(Feature::ShellZshFork); - features.disable(Feature::UnifiedExec); - features.disable(Feature::UnifiedExecZshFork); - features -} - -#[test] -fn shell_type_is_derived_from_model_and_feature_gates() { - let model = model_with_shell_type(ConfigShellToolType::UnifiedExec); - let mut features = shell_features(); - features.enable(Feature::UnifiedExec); - assert_eq!( - shell_type_for_model_and_features(&model, &features), - ConfigShellToolType::UnifiedExec - ); - features.disable(Feature::ShellTool); - assert_eq!( - shell_type_for_model_and_features(&model, &features), - ConfigShellToolType::Disabled - ); - - features.enable(Feature::ShellTool); - features.disable(Feature::UnifiedExec); - assert_eq!( - shell_type_for_model_and_features(&model, &features), - ConfigShellToolType::Disabled - ); -} - -#[test] -fn shell_type_respects_disabled_model_capability() { - let model = model_with_shell_type(ConfigShellToolType::Disabled); - assert_eq!( - shell_type_for_model_and_features(&model, &shell_features()), - ConfigShellToolType::Disabled - ); -} - -#[test] -fn unified_exec_feature_mode_follows_composition_dependencies() { - let mut features = shell_features(); - assert_eq!( - unified_exec_feature_mode_for_features(&features), - UnifiedExecFeatureMode::Disabled - ); - - features.enable(Feature::UnifiedExec); - assert_eq!( - unified_exec_feature_mode_for_features(&features), - UnifiedExecFeatureMode::Direct - ); - - features.enable(Feature::UnifiedExecZshFork); - assert_eq!( - unified_exec_feature_mode_for_features(&features), - UnifiedExecFeatureMode::Direct - ); - - features.enable(Feature::ShellZshFork); - features.disable(Feature::UnifiedExecZshFork); - assert_eq!( - unified_exec_feature_mode_for_features(&features), - UnifiedExecFeatureMode::Direct - ); - - features.enable(Feature::UnifiedExecZshFork); - assert_eq!( - unified_exec_feature_mode_for_features(&features), - UnifiedExecFeatureMode::ZshFork - ); - - features.disable(Feature::ShellTool); - assert_eq!( - unified_exec_feature_mode_for_features(&features), - UnifiedExecFeatureMode::Disabled - ); -} - #[test] fn request_user_input_modes_follow_default_mode_feature() { let mut features = Features::with_defaults(); @@ -154,12 +22,14 @@ fn request_user_input_modes_follow_default_mode_feature() { } #[test] -fn unified_exec_shell_mode_uses_zsh_fork_only_when_all_inputs_match() { +fn unified_exec_shell_mode_respects_feature_and_policy_gates() { let exe = std::env::current_exe().expect("current exe path"); let shell = exe.clone(); - + let mut features = Features::with_defaults(); + features.enable(Feature::ShellTool); + features.enable(Feature::ShellZshFork); let mode = UnifiedExecShellMode::for_session( - UnifiedExecFeatureMode::ZshFork, + &features, ToolUserShellType::Zsh, Some(&shell), Some(&exe), @@ -170,9 +40,46 @@ fn unified_exec_shell_mode_uses_zsh_fork_only_when_all_inputs_match() { assert_eq!(mode, UnifiedExecShellMode::Direct); } + features.disable(Feature::ShellZshFork); assert_eq!( UnifiedExecShellMode::for_session( - UnifiedExecFeatureMode::Direct, + &features, + ToolUserShellType::Zsh, + Some(&shell), + Some(&exe), + ), + UnifiedExecShellMode::Direct + ); + + features.enable(Feature::ShellZshFork); + features.disable(Feature::UnifiedExecZshFork); + assert_eq!( + UnifiedExecShellMode::for_session( + &features, + ToolUserShellType::Zsh, + Some(&shell), + Some(&exe), + ), + UnifiedExecShellMode::Direct + ); + + features.enable(Feature::UnifiedExecZshFork); + features.disable(Feature::UnifiedExec); + assert_eq!( + UnifiedExecShellMode::for_session( + &features, + ToolUserShellType::Zsh, + Some(&shell), + Some(&exe), + ), + UnifiedExecShellMode::Direct + ); + + features.enable(Feature::UnifiedExec); + features.disable(Feature::ShellTool); + assert_eq!( + UnifiedExecShellMode::for_session( + &features, ToolUserShellType::Zsh, Some(&shell), Some(&exe),