From 83decfa3009cc575403bf935415eccb0a552d8f2 Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Wed, 13 May 2026 09:43:25 -0700 Subject: [PATCH] [codex] Remove unused legacy shell tools (#22246) ## Why Recent session history showed no active use of the raw `shell`, `local_shell`, or `container.exec` execution surfaces. Keeping those handlers/specs wired into core leaves duplicate shell execution paths alongside the supported `shell_command` and unified exec tools. ## What changed - Removed the raw `shell` handler/spec and its `ShellToolCallParams` protocol helper. - Removed the legacy `local_shell` and `container.exec` handler/spec plumbing while preserving persisted-history compatibility for old response items. - Normalized model/config `default` and `local` shell selections to `shell_command`. - Pruned tests that exercised removed raw-shell/local-shell/apply-patch variants and kept coverage on `shell_command`, unified exec, and freeform `apply_patch`. ## Verification - `git diff --check` - `cargo test -p codex-protocol` - `cargo test -p codex-tools` - `cargo test -p codex-core tools::handlers::shell` - `cargo test -p codex-core tools::spec` - `cargo test -p codex-core tools::router` - `cargo test -p codex-core active_call_preserves_triggering_command_context` - `cargo test -p codex-core guardian_tests` - `cargo test -p codex-core --test all shell_serialization` - `cargo test -p codex-core --test all apply_patch_cli` - `cargo test -p codex-core --test all shell_command_` - `cargo test -p codex-core --test all local_shell` - `cargo test -p codex-core --test all otel::` - `cargo test -p codex-core --test all hooks::` - `just fix -p codex-core` - `just fix -p codex-tools` --- codex-rs/config/src/types.rs | 3 +- codex-rs/core/config.schema.json | 2 +- codex-rs/core/src/client_common.rs | 2 +- codex-rs/core/src/compact_remote_v2.rs | 2 +- codex-rs/core/src/memory_usage.rs | 9 - codex-rs/core/src/session/tests.rs | 63 ++-- .../core/src/session/tests/guardian_tests.rs | 131 ++------ codex-rs/core/src/stream_events_utils.rs | 28 -- codex-rs/core/src/tools/handlers/mod.rs | 3 - codex-rs/core/src/tools/handlers/shell.rs | 91 +----- .../tools/handlers/shell/container_exec.rs | 97 ------ .../src/tools/handlers/shell/local_shell.rs | 131 -------- .../src/tools/handlers/shell/shell_handler.rs | 146 --------- .../core/src/tools/handlers/shell_spec.rs | 72 ----- .../src/tools/handlers/shell_spec_tests.rs | 156 ---------- .../core/src/tools/handlers/shell_tests.rs | 39 --- .../core/src/tools/network_approval_tests.rs | 4 +- codex-rs/core/src/tools/parallel.rs | 2 +- codex-rs/core/src/tools/router.rs | 32 -- codex-rs/core/src/tools/router_tests.rs | 3 +- codex-rs/core/src/tools/runtimes/shell.rs | 20 +- codex-rs/core/src/tools/spec_plan.rs | 43 +-- codex-rs/core/src/tools/spec_plan_tests.rs | 1 - codex-rs/core/src/tools/spec_tests.rs | 4 +- .../core/src/tools/tool_dispatch_trace.rs | 9 - codex-rs/core/src/tools/tool_search_entry.rs | 1 - codex-rs/core/tests/common/responses.rs | 19 -- codex-rs/core/tests/common/test_codex.rs | 8 +- codex-rs/core/tests/suite/apply_patch_cli.rs | 49 --- codex-rs/core/tests/suite/compact.rs | 63 ++-- codex-rs/core/tests/suite/compact_remote.rs | 9 +- codex-rs/core/tests/suite/hooks.rs | 226 +------------- .../core/tests/suite/models_etag_responses.rs | 6 +- codex-rs/core/tests/suite/otel.rs | 177 +++-------- .../core/tests/suite/shell_serialization.rs | 288 +----------------- ...user_input_no_preempt_after_reasoning.snap | 2 +- codex-rs/core/tests/suite/tool_harness.rs | 15 +- codex-rs/core/tests/suite/tools.rs | 160 +++------- codex-rs/protocol/src/models.rs | 48 --- codex-rs/tools/src/code_mode.rs | 3 +- codex-rs/tools/src/function_call_error.rs | 2 - codex-rs/tools/src/tool_config.rs | 3 + codex-rs/tools/src/tool_payload.rs | 3 - codex-rs/tools/src/tool_spec.rs | 3 - codex-rs/tools/src/tool_spec_tests.rs | 1 - sdk/typescript/tests/abort.test.ts | 2 +- sdk/typescript/tests/responsesProxy.ts | 5 +- 47 files changed, 205 insertions(+), 1981 deletions(-) delete mode 100644 codex-rs/core/src/tools/handlers/shell/container_exec.rs delete mode 100644 codex-rs/core/src/tools/handlers/shell/local_shell.rs delete mode 100644 codex-rs/core/src/tools/handlers/shell/shell_handler.rs diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 21d9b44752..74e3394f93 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -892,8 +892,7 @@ impl From for codex_app_server_protocol::SandboxSettings } } -/// Policy for building the `env` when spawning a process via either the -/// `shell` or `local_shell` tool. +/// Policy for building the `env` when spawning a process via shell-like tools. #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct ShellEnvironmentPolicyToml { diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 8a7cb01b5f..5c0100c981 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2344,7 +2344,7 @@ }, "ShellEnvironmentPolicyToml": { "additionalProperties": false, - "description": "Policy for building the `env` when spawning a process via either the `shell` or `local_shell` tool.", + "description": "Policy for building the `env` when spawning a process via shell-like tools.", "properties": { "exclude": { "description": "List of regular expressions.", diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index efe2670652..2061d53c80 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -126,7 +126,7 @@ fn reserialize_shell_outputs(items: &mut [ResponseItem]) { } fn is_shell_tool_name(name: &str) -> bool { - matches!(name, "shell" | "container.exec") + name == "shell" } #[derive(Deserialize)] diff --git a/codex-rs/core/src/compact_remote_v2.rs b/codex-rs/core/src/compact_remote_v2.rs index 7f6ea61a5b..4dafb95f8e 100644 --- a/codex-rs/core/src/compact_remote_v2.rs +++ b/codex-rs/core/src/compact_remote_v2.rs @@ -401,7 +401,7 @@ mod tests { message("assistant", "final", Some(MessagePhase::FinalAnswer)), ResponseItem::FunctionCall { id: None, - name: "shell".to_string(), + name: "shell_command".to_string(), namespace: None, arguments: "{}".to_string(), call_id: "call_1".to_string(), diff --git a/codex-rs/core/src/memory_usage.rs b/codex-rs/core/src/memory_usage.rs index fd54044f21..bd856d5e6f 100644 --- a/codex-rs/core/src/memory_usage.rs +++ b/codex-rs/core/src/memory_usage.rs @@ -5,7 +5,6 @@ use crate::tools::handlers::unified_exec::ExecCommandArgs; use codex_memories_read::usage::MEMORIES_USAGE_METRIC; use codex_memories_read::usage::memories_usage_kinds_from_command; use codex_protocol::models::ShellCommandToolCallParams; -use codex_protocol::models::ShellToolCallParams; use std::path::PathBuf; pub(crate) async fn emit_metric_for_tool_read(invocation: &ToolInvocation, success: bool) { @@ -41,14 +40,6 @@ fn shell_command_for_invocation(invocation: &ToolInvocation) -> Option<(Vec serde_json::from_str::(arguments) - .ok() - .map(|params| { - ( - params.command, - invocation.turn.resolve_path(params.workdir).to_path_buf(), - ) - }), (None, "shell_command") => serde_json::from_str::(arguments) .ok() .map(|params| { diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index b4ba561873..9ef67bcb8e 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -4,7 +4,6 @@ use crate::config::ConfigBuilder; use crate::config::test_config; use crate::context::ContextualUserFragment; use crate::context::TurnAborted; -use crate::exec::ExecCapturePolicy; use crate::function_tool::FunctionCallError; use crate::shell::default_user_shell; use crate::skills::SkillRenderSideEffects; @@ -69,7 +68,7 @@ use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; use crate::tools::handlers::CreateGoalHandler; use crate::tools::handlers::ExecCommandHandler; -use crate::tools::handlers::ShellHandler; +use crate::tools::handlers::ShellCommandHandler; use crate::tools::handlers::UpdateGoalHandler; use crate::tools::registry::ToolExecutor; use crate::tools::router::ToolCallSource; @@ -8317,7 +8316,7 @@ async fn budget_limited_accounting_steers_active_turn_without_aborting() -> anyh sess.goal_runtime_apply(GoalRuntimeEvent::ToolCompleted { turn_context: tc.as_ref(), - tool_name: "shell", + tool_name: "shell_command", }) .await?; @@ -8553,7 +8552,7 @@ async fn external_active_goal_set_marks_current_turn_for_accounting() -> anyhow: .await; sess.goal_runtime_apply(GoalRuntimeEvent::ToolCompleted { turn_context: tc.as_ref(), - tool_name: "shell", + tool_name: "shell_command", }) .await?; @@ -8984,7 +8983,7 @@ async fn fatal_tool_error_stops_turn_and_reports_error() { id: None, status: None, call_id: "call-1".to_string(), - name: "shell".to_string(), + name: "shell_command".to_string(), input: "{}".to_string(), }; @@ -9007,7 +9006,10 @@ async fn fatal_tool_error_stops_turn_and_reports_error() { match err { FunctionCallError::Fatal(message) => { - assert_eq!(message, "tool shell invoked with incompatible payload"); + assert_eq!( + message, + "tool shell_command invoked with incompatible payload" + ); } other => panic!("expected FunctionCallError::Fatal, got {other:?}"), } @@ -9353,13 +9355,12 @@ async fn update_goal_tool_marks_goal_complete() { #[tokio::test] async fn rejects_escalated_permissions_when_policy_not_on_request() { - use crate::exec::ExecParams; use crate::exec_policy::ExecApprovalRequest; use crate::sandboxing::SandboxPermissions; use crate::tools::sandboxing::ExecApprovalRequirement; use crate::turn_diff_tracker::TurnDiffTracker; use codex_protocol::protocol::AskForApproval; - use std::collections::HashMap; + use codex_tools::ShellCommandBackendConfig; let (session, mut turn_context_raw) = make_session_and_context().await; // Ensure policy is NOT OnRequest so the early rejection path triggers @@ -9370,43 +9371,16 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() { let session = Arc::new(session); let mut turn_context = Arc::new(turn_context_raw); + let command_script = "echo hi"; let timeout_ms = 1000; let sandbox_permissions = SandboxPermissions::RequireEscalated; - let params = ExecParams { - command: if cfg!(windows) { - vec![ - "cmd.exe".to_string(), - "/C".to_string(), - "echo hi".to_string(), - ] - } else { - vec![ - "/bin/sh".to_string(), - "-c".to_string(), - "echo hi".to_string(), - ] - }, - cwd: turn_context.cwd.clone(), - expiration: timeout_ms.into(), - capture_policy: ExecCapturePolicy::ShellTool, - env: HashMap::new(), - network: None, - sandbox_permissions, - windows_sandbox_level: turn_context.windows_sandbox_level, - windows_sandbox_private_desktop: turn_context - .config - .permissions - .windows_sandbox_private_desktop, - justification: Some("test".to_string()), - arg0: None, - }; let turn_diff_tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())); - let tool_name = "shell"; + let tool_name = "shell_command"; let call_id = "test-call".to_string(); - let handler = ShellHandler::default(); + let handler = ShellCommandHandler::from(ShellCommandBackendConfig::Classic); let resp = handler .handle(ToolInvocation { session: Arc::clone(&session), @@ -9418,11 +9392,11 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() { source: crate::tools::context::ToolCallSource::Direct, payload: ToolPayload::Function { arguments: serde_json::json!({ - "command": params.command.clone(), + "command": command_script, "workdir": Some(turn_context.cwd.to_string_lossy().to_string()), - "timeout_ms": params.expiration.timeout_ms(), - "sandbox_permissions": params.sandbox_permissions, - "justification": params.justification.clone(), + "timeout_ms": timeout_ms, + "sandbox_permissions": sandbox_permissions, + "justification": Some("test"), }) .to_string(), }, @@ -9448,11 +9422,14 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() { turn_context_mut.permission_profile = PermissionProfile::Disabled; let file_system_sandbox_policy = turn_context.file_system_sandbox_policy(); + let command = session + .user_shell() + .derive_exec_args(command_script, turn_context.tools_config.allow_login_shell); let exec_approval_requirement = session .services .exec_policy .create_exec_approval_requirement_for_command(ExecApprovalRequest { - command: ¶ms.command, + command: &command, approval_policy: turn_context.approval_policy.value(), permission_profile: turn_context.permission_profile(), file_system_sandbox_policy: &file_system_sandbox_policy, diff --git a/codex-rs/core/src/session/tests/guardian_tests.rs b/codex-rs/core/src/session/tests/guardian_tests.rs index 730b9f5e87..ffb0d94d80 100644 --- a/codex-rs/core/src/session/tests/guardian_tests.rs +++ b/codex-rs/core/src/session/tests/guardian_tests.rs @@ -1,8 +1,6 @@ use super::*; use crate::compact::InitialContextInjection; use crate::environment_selection::ResolvedTurnEnvironments; -use crate::exec::ExecCapturePolicy; -use crate::exec::ExecParams; use crate::exec_policy::ExecPolicyManager; use crate::guardian::GUARDIAN_REVIEWER_NAME; use crate::sandboxing::SandboxPermissions; @@ -43,8 +41,6 @@ use core_test_support::responses::sse; use core_test_support::responses::sse_response; use core_test_support::responses::start_mock_server; use pretty_assertions::assert_eq; -use serde::Deserialize; -use std::collections::HashMap; use std::fs; use std::sync::Arc; use std::time::Duration; @@ -238,7 +234,7 @@ async fn request_permissions_guardian_review_stops_when_cancelled() { } #[tokio::test] -async fn guardian_allows_shell_additional_permissions_requests_past_policy_validation() { +async fn guardian_allows_shell_command_additional_permissions_requests_past_policy_validation() { let server = start_mock_server().await; let _request_log = mount_sse_once( &server, @@ -292,38 +288,9 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid let turn_context = Arc::new(turn_context_raw); let expiration_ms: u64 = if cfg!(windows) { 2_500 } else { 1_000 }; - let params = ExecParams { - command: if cfg!(windows) { - vec![ - "cmd.exe".to_string(), - "/Q".to_string(), - "/D".to_string(), - "/C".to_string(), - "echo hi".to_string(), - ] - } else { - vec![ - "/bin/sh".to_string(), - "-c".to_string(), - "echo hi".to_string(), - ] - }, - cwd: turn_context.cwd.clone(), - expiration: expiration_ms.into(), - capture_policy: ExecCapturePolicy::ShellTool, - env: HashMap::new(), - network: None, - sandbox_permissions: SandboxPermissions::WithAdditionalPermissions, - windows_sandbox_level: turn_context.windows_sandbox_level, - windows_sandbox_private_desktop: turn_context - .config - .permissions - .windows_sandbox_private_desktop, - justification: Some("test".to_string()), - arg0: None, - }; - - let handler = ShellHandler::default(); + let handler = crate::tools::handlers::ShellCommandHandler::from( + codex_tools::ShellCommandBackendConfig::Classic, + ); let resp = handler .handle(ToolInvocation { session: Arc::clone(&session), @@ -331,21 +298,22 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid cancellation_token: CancellationToken::new(), tracker: Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())), call_id: "test-call".to_string(), - tool_name: codex_tools::ToolName::plain("shell"), + tool_name: codex_tools::ToolName::plain("shell_command"), source: crate::tools::context::ToolCallSource::Direct, payload: ToolPayload::Function { arguments: serde_json::json!({ - "command": params.command.clone(), + "command": "echo hi", + "login": false, "workdir": Some(turn_context.cwd.to_string_lossy().to_string()), - "timeout_ms": params.expiration.timeout_ms(), - "sandbox_permissions": params.sandbox_permissions, + "timeout_ms": expiration_ms, + "sandbox_permissions": SandboxPermissions::WithAdditionalPermissions, "additional_permissions": PermissionProfile { network: Some(NetworkPermissions { enabled: Some(true), }), file_system: None, }, - "justification": params.justification.clone(), + "justification": Some("test"), }) .to_string(), }, @@ -353,27 +321,11 @@ async fn guardian_allows_shell_additional_permissions_requests_past_policy_valid .await; let output = expect_text_output(&resp.expect("expected Ok result")); - - #[derive(Deserialize, PartialEq, Eq, Debug)] - struct ResponseExecMetadata { - exit_code: i32, - } - - #[derive(Deserialize)] - struct ResponseExecOutput { - output: String, - metadata: ResponseExecMetadata, - } - - let exec_output: ResponseExecOutput = - serde_json::from_str(&output).expect("valid exec output json"); - - assert_eq!(exec_output.metadata, ResponseExecMetadata { exit_code: 0 }); - assert!(exec_output.output.contains("hi")); + assert!(output.contains("hi")); } #[tokio::test] -async fn strict_auto_review_turn_grant_forces_guardian_for_shell_policy_skip() { +async fn strict_auto_review_turn_grant_forces_guardian_for_shell_command_policy_skip() { let server = start_mock_server().await; let guardian_request_log = mount_sse_once( &server, @@ -437,34 +389,22 @@ async fn strict_auto_review_turn_grant_forces_guardian_for_shell_policy_skip() { let session = Arc::new(session); let turn_context = Arc::new(turn_context_raw); - let handler = ShellHandler::default(); - let command = if cfg!(windows) { - vec![ - "cmd.exe".to_string(), - "/Q".to_string(), - "/D".to_string(), - "/C".to_string(), - "echo hi".to_string(), - ] - } else { - vec![ - "/bin/sh".to_string(), - "-c".to_string(), - "echo hi".to_string(), - ] - }; + let handler = crate::tools::handlers::ShellCommandHandler::from( + codex_tools::ShellCommandBackendConfig::Classic, + ); let resp = handler .handle(ToolInvocation { session: Arc::clone(&session), turn: Arc::clone(&turn_context), cancellation_token: CancellationToken::new(), tracker: Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())), - call_id: "strict-shell-call".to_string(), - tool_name: codex_tools::ToolName::plain("shell"), + call_id: "strict-shell-command-call".to_string(), + tool_name: codex_tools::ToolName::plain("shell_command"), source: ToolCallSource::Direct, payload: ToolPayload::Function { arguments: serde_json::json!({ - "command": command, + "command": "echo hi", + "login": false, "workdir": Some(turn_context.cwd.to_string_lossy().to_string()), "timeout_ms": 1_000_u64, }) @@ -593,7 +533,7 @@ async fn process_compacted_history_preserves_separate_guardian_developer_message clippy::await_holding_invalid_type, reason = "test mutates active turn state directly to seed granted permissions" )] -async fn shell_handler_allows_sticky_turn_permissions_without_inline_request_permissions_feature() { +async fn shell_command_allows_sticky_turn_permissions_without_inline_request_permissions_feature() { let (mut session, turn_context_raw) = make_session_and_context().await; session .features @@ -615,7 +555,9 @@ async fn shell_handler_allows_sticky_turn_permissions_without_inline_request_per let session = Arc::new(session); let turn_context = Arc::new(turn_context_raw); - let handler = ShellHandler::default(); + let handler = crate::tools::handlers::ShellCommandHandler::from( + codex_tools::ShellCommandBackendConfig::Classic, + ); let resp = handler .handle(ToolInvocation { session: Arc::clone(&session), @@ -623,15 +565,12 @@ async fn shell_handler_allows_sticky_turn_permissions_without_inline_request_per cancellation_token: CancellationToken::new(), tracker: Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())), call_id: "sticky-turn-grant".to_string(), - tool_name: codex_tools::ToolName::plain("shell"), + tool_name: codex_tools::ToolName::plain("shell_command"), source: crate::tools::context::ToolCallSource::Direct, payload: ToolPayload::Function { arguments: serde_json::json!({ - "command": [ - "/bin/sh", - "-c", - "echo hi", - ], + "command": "echo hi", + "login": false, "timeout_ms": 1_000_u64, "workdir": Some(turn_context.cwd.to_string_lossy().to_string()), }) @@ -643,23 +582,7 @@ async fn shell_handler_allows_sticky_turn_permissions_without_inline_request_per match resp { Ok(output) => { let output = expect_text_output(&output); - - #[derive(Deserialize, PartialEq, Eq, Debug)] - struct ResponseExecMetadata { - exit_code: i32, - } - - #[derive(Deserialize)] - struct ResponseExecOutput { - output: String, - metadata: ResponseExecMetadata, - } - - let exec_output: ResponseExecOutput = - serde_json::from_str(&output).expect("valid exec output json"); - - assert_eq!(exec_output.metadata, ResponseExecMetadata { exit_code: 0 }); - assert!(exec_output.output.contains("hi")); + assert!(output.contains("hi")); } Err(FunctionCallError::RespondToModel(output)) => { assert!( diff --git a/codex-rs/core/src/stream_events_utils.rs b/codex-rs/core/src/stream_events_utils.rs index b4f00ee864..25d7dada95 100644 --- a/codex-rs/core/src/stream_events_utils.rs +++ b/codex-rs/core/src/stream_events_utils.rs @@ -289,34 +289,6 @@ pub(crate) async fn handle_output_item_done( output.last_agent_message = last_agent_message; } - // Guardrail: the model issued a LocalShellCall without an id; surface the error back into history. - Err(FunctionCallError::MissingLocalShellCallId) => { - let msg = "LocalShellCall without call_id or id"; - ctx.turn_context - .session_telemetry - .log_tool_failed("local_shell", msg); - tracing::error!(msg); - - let response = ResponseInputItem::FunctionCallOutput { - call_id: String::new(), - output: FunctionCallOutputPayload { - body: FunctionCallOutputBody::Text(msg.to_string()), - ..Default::default() - }, - }; - record_completed_response_item(ctx.sess.as_ref(), ctx.turn_context.as_ref(), &item) - .await; - if let Some(response_item) = response_input_to_response_item(&response) { - ctx.sess - .record_conversation_items( - &ctx.turn_context, - std::slice::from_ref(&response_item), - ) - .await; - } - - output.needs_follow_up = true; - } // The tool request should be answered directly (or was denied); push that response into the transcript. Err(FunctionCallError::RespondToModel(message)) => { let response = ResponseInputItem::FunctionCallOutput { diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index 712ed0f1a6..69281acc7d 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -62,11 +62,8 @@ pub use plan::PlanHandler; pub use request_permissions::RequestPermissionsHandler; pub use request_plugin_install::RequestPluginInstallHandler; pub use request_user_input::RequestUserInputHandler; -pub use shell::ContainerExecHandler; -pub use shell::LocalShellHandler; pub use shell::ShellCommandHandler; pub(crate) use shell::ShellCommandHandlerOptions; -pub use shell::ShellHandler; pub use test_sync::TestSyncHandler; pub use tool_search::ToolSearchHandler; pub use unified_exec::ExecCommandHandler; diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 6f8ba4d632..e6ce6908f3 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -1,6 +1,5 @@ use codex_features::Feature; use codex_protocol::models::ShellCommandToolCallParams; -use codex_protocol::models::ShellToolCallParams; use serde_json::Value as JsonValue; use std::sync::Arc; @@ -9,8 +8,6 @@ use crate::exec_policy::ExecApprovalRequest; use crate::function_tool::FunctionCallError; use crate::session::turn_context::TurnContext; use crate::tools::context::FunctionToolOutput; -use crate::tools::context::ToolInvocation; -use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; use crate::tools::events::ToolEmitter; use crate::tools::events::ToolEventCtx; @@ -19,12 +16,7 @@ use crate::tools::handlers::apply_patch::intercept_apply_patch; use crate::tools::handlers::implicit_granted_permissions; use crate::tools::handlers::normalize_and_validate_additional_permissions; use crate::tools::handlers::parse_arguments; -use crate::tools::handlers::rewrite_function_arguments; -use crate::tools::handlers::updated_hook_command; -use crate::tools::hook_names::HookToolName; use crate::tools::orchestrator::ToolOrchestrator; -use crate::tools::registry::PostToolUsePayload; -use crate::tools::registry::PreToolUsePayload; use crate::tools::runtimes::shell::ShellRequest; use crate::tools::runtimes::shell::ShellRuntime; use crate::tools::runtimes::shell::ShellRuntimeBackend; @@ -33,36 +25,10 @@ use codex_protocol::models::AdditionalPermissionProfile; use codex_protocol::protocol::ExecCommandSource; use codex_tools::ToolName; -mod container_exec; -mod local_shell; mod shell_command; -mod shell_handler; -pub use container_exec::ContainerExecHandler; -pub use local_shell::LocalShellHandler; pub use shell_command::ShellCommandHandler; pub(crate) use shell_command::ShellCommandHandlerOptions; -pub use shell_handler::ShellHandler; - -fn shell_function_payload_command(payload: &ToolPayload) -> Option { - let ToolPayload::Function { arguments } = payload else { - return None; - }; - - parse_arguments::(arguments) - .ok() - .map(|params| codex_shell_command::parse_command::shlex_join(¶ms.command)) -} - -fn local_shell_payload_command(payload: &ToolPayload) -> Option { - let ToolPayload::LocalShell { params } = payload else { - return None; - }; - - Some(codex_shell_command::parse_command::shlex_join( - ¶ms.command, - )) -} fn shell_command_payload_command(payload: &ToolPayload) -> Option { let ToolPayload::Function { arguments } = payload else { @@ -88,53 +54,6 @@ struct RunExecLikeArgs { shell_runtime_backend: ShellRuntimeBackend, } -fn shell_function_pre_tool_use_payload(invocation: &ToolInvocation) -> Option { - shell_function_payload_command(&invocation.payload).map(|command| PreToolUsePayload { - tool_name: HookToolName::bash(), - tool_input: serde_json::json!({ "command": command }), - }) -} - -fn rewrite_shell_function_updated_hook_input( - mut invocation: ToolInvocation, - updated_input: JsonValue, - tool_name: &str, -) -> Result { - let ToolPayload::Function { arguments } = invocation.payload else { - return Err(FunctionCallError::RespondToModel(format!( - "hook input rewrite received unsupported {tool_name} payload" - ))); - }; - let command = shlex::split(updated_hook_command(&updated_input)?).ok_or_else(|| { - FunctionCallError::RespondToModel( - "hook returned shell input with an invalid command string".to_string(), - ) - })?; - invocation.payload = ToolPayload::Function { - arguments: rewrite_function_arguments(&arguments, tool_name, |arguments| { - arguments.insert( - "command".to_string(), - JsonValue::Array(command.into_iter().map(JsonValue::String).collect()), - ); - })?, - }; - Ok(invocation) -} - -fn shell_function_post_tool_use_payload( - invocation: &ToolInvocation, - result: &FunctionToolOutput, -) -> Option { - let tool_response = result.post_tool_use_response(&invocation.call_id, &invocation.payload)?; - let command = shell_function_payload_command(&invocation.payload)?; - Some(PostToolUsePayload { - tool_name: HookToolName::bash(), - tool_use_id: invocation.call_id.clone(), - tool_input: serde_json::json!({ "command": command }), - tool_response, - }) -} - async fn run_exec_like(args: RunExecLikeArgs) -> Result { let RunExecLikeArgs { tool_name, @@ -289,15 +208,7 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result ShellRuntime::new(), - backend @ (ShellCommandClassic | ShellCommandZshFork) => { - ShellRuntime::for_shell_command(backend) - } - } - }; + let mut runtime = ShellRuntime::for_shell_command(shell_runtime_backend); let tool_ctx = ToolCtx { session: session.clone(), turn: turn.clone(), diff --git a/codex-rs/core/src/tools/handlers/shell/container_exec.rs b/codex-rs/core/src/tools/handlers/shell/container_exec.rs deleted file mode 100644 index ee3dd8221d..0000000000 --- a/codex-rs/core/src/tools/handlers/shell/container_exec.rs +++ /dev/null @@ -1,97 +0,0 @@ -use codex_protocol::models::ShellToolCallParams; -use codex_tools::ToolName; - -use crate::function_tool::FunctionCallError; -use crate::tools::context::FunctionToolOutput; -use crate::tools::context::ToolInvocation; -use crate::tools::context::ToolPayload; -use crate::tools::handlers::parse_arguments_with_base_path; -use crate::tools::handlers::resolve_workdir_base_path; -use crate::tools::registry::PostToolUsePayload; -use crate::tools::registry::PreToolUsePayload; -use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; -use crate::tools::runtimes::shell::ShellRuntimeBackend; - -use super::RunExecLikeArgs; -use super::rewrite_shell_function_updated_hook_input; -use super::run_exec_like; -use super::shell_function_post_tool_use_payload; -use super::shell_function_pre_tool_use_payload; -use super::shell_handler::ShellHandler; - -pub struct ContainerExecHandler; - -impl ToolExecutor for ContainerExecHandler { - type Output = FunctionToolOutput; - - fn tool_name(&self) -> ToolName { - ToolName::plain("container.exec") - } - - async fn handle(&self, invocation: ToolInvocation) -> Result { - let ToolInvocation { - session, - turn, - tracker, - call_id, - payload, - .. - } = invocation; - - let arguments = match payload { - ToolPayload::Function { arguments } => arguments, - _ => { - return Err(FunctionCallError::RespondToModel( - "unsupported payload for container.exec handler".to_string(), - )); - } - }; - - let cwd = resolve_workdir_base_path(&arguments, &turn.cwd)?; - let params: ShellToolCallParams = parse_arguments_with_base_path(&arguments, &cwd)?; - let prefix_rule = params.prefix_rule.clone(); - let exec_params = - ShellHandler::to_exec_params(¶ms, turn.as_ref(), session.conversation_id); - run_exec_like(RunExecLikeArgs { - tool_name: ToolName::plain("container.exec"), - exec_params, - hook_command: codex_shell_command::parse_command::shlex_join(¶ms.command), - additional_permissions: params.additional_permissions.clone(), - prefix_rule, - session, - turn, - tracker, - call_id, - freeform: false, - shell_runtime_backend: ShellRuntimeBackend::Generic, - }) - .await - } -} - -impl ToolHandler for ContainerExecHandler { - fn matches_kind(&self, payload: &ToolPayload) -> bool { - matches!(payload, ToolPayload::Function { .. }) - } - - fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { - shell_function_pre_tool_use_payload(invocation) - } - - fn with_updated_hook_input( - &self, - invocation: ToolInvocation, - updated_input: serde_json::Value, - ) -> Result { - rewrite_shell_function_updated_hook_input(invocation, updated_input, "container.exec") - } - - fn post_tool_use_payload( - &self, - invocation: &ToolInvocation, - result: &Self::Output, - ) -> Option { - shell_function_post_tool_use_payload(invocation, result) - } -} diff --git a/codex-rs/core/src/tools/handlers/shell/local_shell.rs b/codex-rs/core/src/tools/handlers/shell/local_shell.rs deleted file mode 100644 index 608d748d85..0000000000 --- a/codex-rs/core/src/tools/handlers/shell/local_shell.rs +++ /dev/null @@ -1,131 +0,0 @@ -use codex_tools::ToolName; - -use crate::function_tool::FunctionCallError; -use crate::tools::context::FunctionToolOutput; -use crate::tools::context::ToolInvocation; -use crate::tools::context::ToolOutput; -use crate::tools::context::ToolPayload; -use crate::tools::handlers::updated_hook_command; -use crate::tools::hook_names::HookToolName; -use crate::tools::registry::PostToolUsePayload; -use crate::tools::registry::PreToolUsePayload; -use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; -use crate::tools::runtimes::shell::ShellRuntimeBackend; -use codex_tools::ToolSpec; - -use super::super::shell_spec::create_local_shell_tool; -use super::RunExecLikeArgs; -use super::local_shell_payload_command; -use super::run_exec_like; -use super::shell_handler::ShellHandler; - -#[derive(Default)] -pub struct LocalShellHandler { - include_spec: bool, -} - -impl LocalShellHandler { - pub(crate) fn new() -> Self { - Self { include_spec: true } - } -} - -impl ToolExecutor for LocalShellHandler { - type Output = FunctionToolOutput; - - fn tool_name(&self) -> ToolName { - ToolName::plain("local_shell") - } - - fn spec(&self) -> Option { - self.include_spec.then(create_local_shell_tool) - } - - fn supports_parallel_tool_calls(&self) -> bool { - self.include_spec - } - - async fn handle(&self, invocation: ToolInvocation) -> Result { - let ToolInvocation { - session, - turn, - tracker, - call_id, - payload, - .. - } = invocation; - - let ToolPayload::LocalShell { params } = payload else { - return Err(FunctionCallError::RespondToModel( - "unsupported payload for local_shell handler".to_string(), - )); - }; - - let exec_params = - ShellHandler::to_exec_params(¶ms, turn.as_ref(), session.conversation_id); - run_exec_like(RunExecLikeArgs { - tool_name: ToolName::plain("local_shell"), - exec_params, - hook_command: codex_shell_command::parse_command::shlex_join(¶ms.command), - additional_permissions: None, - prefix_rule: None, - session, - turn, - tracker, - call_id, - freeform: false, - shell_runtime_backend: ShellRuntimeBackend::Generic, - }) - .await - } -} - -impl ToolHandler for LocalShellHandler { - fn matches_kind(&self, payload: &ToolPayload) -> bool { - matches!(payload, ToolPayload::LocalShell { .. }) - } - - fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { - local_shell_payload_command(&invocation.payload).map(|command| PreToolUsePayload { - tool_name: HookToolName::bash(), - tool_input: serde_json::json!({ "command": command }), - }) - } - - fn with_updated_hook_input( - &self, - mut invocation: ToolInvocation, - updated_input: serde_json::Value, - ) -> Result { - let command = updated_hook_command(&updated_input)?; - invocation.payload = match invocation.payload { - ToolPayload::LocalShell { mut params } => { - params.command = shlex::split(command).ok_or_else(|| { - FunctionCallError::RespondToModel( - "hook returned shell input with an invalid command string".to_string(), - ) - })?; - ToolPayload::LocalShell { params } - } - payload => payload, - }; - Ok(invocation) - } - - fn post_tool_use_payload( - &self, - invocation: &ToolInvocation, - result: &Self::Output, - ) -> Option { - let tool_response = - result.post_tool_use_response(&invocation.call_id, &invocation.payload)?; - let command = local_shell_payload_command(&invocation.payload)?; - Some(PostToolUsePayload { - tool_name: HookToolName::bash(), - tool_use_id: invocation.call_id.clone(), - tool_input: serde_json::json!({ "command": command }), - tool_response, - }) - } -} diff --git a/codex-rs/core/src/tools/handlers/shell/shell_handler.rs b/codex-rs/core/src/tools/handlers/shell/shell_handler.rs deleted file mode 100644 index 8875e96024..0000000000 --- a/codex-rs/core/src/tools/handlers/shell/shell_handler.rs +++ /dev/null @@ -1,146 +0,0 @@ -use codex_protocol::ThreadId; -use codex_protocol::models::ShellToolCallParams; -use codex_tools::ToolName; - -use crate::exec::ExecCapturePolicy; -use crate::exec::ExecParams; -use crate::exec_env::create_env; -use crate::function_tool::FunctionCallError; -use crate::session::turn_context::TurnContext; -use crate::tools::context::FunctionToolOutput; -use crate::tools::context::ToolInvocation; -use crate::tools::context::ToolPayload; -use crate::tools::handlers::parse_arguments_with_base_path; -use crate::tools::handlers::resolve_workdir_base_path; -use crate::tools::registry::PostToolUsePayload; -use crate::tools::registry::PreToolUsePayload; -use crate::tools::registry::ToolExecutor; -use crate::tools::registry::ToolHandler; -use crate::tools::runtimes::shell::ShellRuntimeBackend; -use codex_tools::ToolSpec; - -use super::super::shell_spec::ShellToolOptions; -use super::super::shell_spec::create_shell_tool; -use super::RunExecLikeArgs; -use super::rewrite_shell_function_updated_hook_input; -use super::run_exec_like; -use super::shell_function_post_tool_use_payload; -use super::shell_function_pre_tool_use_payload; - -#[derive(Default)] -pub struct ShellHandler { - options: Option, -} - -impl ShellHandler { - pub(crate) fn new(options: ShellToolOptions) -> Self { - Self { - options: Some(options), - } - } - - pub(super) fn to_exec_params( - params: &ShellToolCallParams, - turn_context: &TurnContext, - thread_id: ThreadId, - ) -> ExecParams { - ExecParams { - command: params.command.clone(), - cwd: turn_context.resolve_path(params.workdir.clone()), - expiration: params.timeout_ms.into(), - capture_policy: ExecCapturePolicy::ShellTool, - env: create_env(&turn_context.shell_environment_policy, Some(thread_id)), - network: turn_context.network.clone(), - sandbox_permissions: params.sandbox_permissions.unwrap_or_default(), - windows_sandbox_level: turn_context.windows_sandbox_level, - windows_sandbox_private_desktop: turn_context - .config - .permissions - .windows_sandbox_private_desktop, - justification: params.justification.clone(), - arg0: None, - } - } -} - -impl ToolExecutor for ShellHandler { - type Output = FunctionToolOutput; - - fn tool_name(&self) -> ToolName { - ToolName::plain("shell") - } - - fn spec(&self) -> Option { - self.options.map(create_shell_tool) - } - - fn supports_parallel_tool_calls(&self) -> bool { - self.options.is_some() - } - - async fn handle(&self, invocation: ToolInvocation) -> Result { - let ToolInvocation { - session, - turn, - tracker, - call_id, - payload, - .. - } = invocation; - - let arguments = match payload { - ToolPayload::Function { arguments } => arguments, - _ => { - return Err(FunctionCallError::RespondToModel( - "unsupported payload for shell handler".to_string(), - )); - } - }; - - let cwd = resolve_workdir_base_path(&arguments, &turn.cwd)?; - let params: ShellToolCallParams = parse_arguments_with_base_path(&arguments, &cwd)?; - let prefix_rule = params.prefix_rule.clone(); - let exec_params = - ShellHandler::to_exec_params(¶ms, turn.as_ref(), session.conversation_id); - run_exec_like(RunExecLikeArgs { - tool_name: ToolName::plain("shell"), - exec_params, - hook_command: codex_shell_command::parse_command::shlex_join(¶ms.command), - additional_permissions: params.additional_permissions.clone(), - prefix_rule, - session, - turn, - tracker, - call_id, - freeform: false, - shell_runtime_backend: ShellRuntimeBackend::Generic, - }) - .await - } -} - -impl ToolHandler for ShellHandler { - fn matches_kind(&self, payload: &ToolPayload) -> bool { - matches!(payload, ToolPayload::Function { .. }) - } - - fn pre_tool_use_payload(&self, invocation: &ToolInvocation) -> Option { - shell_function_pre_tool_use_payload(invocation) - } - - fn with_updated_hook_input( - &self, - invocation: ToolInvocation, - updated_input: serde_json::Value, - ) -> Result { - rewrite_shell_function_updated_hook_input(invocation, updated_input, "shell") - } - - fn post_tool_use_payload( - &self, - invocation: &ToolInvocation, - result: &Self::Output, - ) -> Option { - shell_function_post_tool_use_payload(invocation, result) - } -} diff --git a/codex-rs/core/src/tools/handlers/shell_spec.rs b/codex-rs/core/src/tools/handlers/shell_spec.rs index dc46290bfa..43b78e3afe 100644 --- a/codex-rs/core/src/tools/handlers/shell_spec.rs +++ b/codex-rs/core/src/tools/handlers/shell_spec.rs @@ -11,20 +11,11 @@ pub struct CommandToolOptions { pub exec_permission_approvals_enabled: bool, } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct ShellToolOptions { - pub exec_permission_approvals_enabled: bool, -} - #[cfg(test)] pub fn create_exec_command_tool(options: CommandToolOptions) -> ToolSpec { create_exec_command_tool_with_environment_id(options, /*include_environment_id*/ false) } -pub fn create_local_shell_tool() -> ToolSpec { - ToolSpec::LocalShell {} -} - pub(crate) fn create_exec_command_tool_with_environment_id( options: CommandToolOptions, include_environment_id: bool, @@ -153,69 +144,6 @@ pub fn create_write_stdin_tool() -> ToolSpec { }) } -pub fn create_shell_tool(options: ShellToolOptions) -> ToolSpec { - let mut properties = BTreeMap::from([ - ( - "command".to_string(), - JsonSchema::array( - JsonSchema::string(/*description*/ None), - Some("The command to execute".to_string()), - ), - ), - ( - "workdir".to_string(), - JsonSchema::string(Some( - "The working directory to execute the command in".to_string(), - )), - ), - ( - "timeout_ms".to_string(), - JsonSchema::number(Some( - "The timeout for the command in milliseconds".to_string(), - )), - ), - ]); - properties.extend(create_approval_parameters( - options.exec_permission_approvals_enabled, - )); - - let description = if cfg!(windows) { - format!( - r#"Runs a Powershell command (Windows) and returns its output. Arguments to `shell` will be passed to CreateProcessW(). Most commands should be prefixed with ["powershell.exe", "-Command"]. - -Examples of valid command strings: - -- ls -a (show hidden): ["powershell.exe", "-Command", "Get-ChildItem -Force"] -- recursive find by name: ["powershell.exe", "-Command", "Get-ChildItem -Recurse -Filter *.py"] -- recursive grep: ["powershell.exe", "-Command", "Get-ChildItem -Path C:\\myrepo -Recurse | Select-String -Pattern 'TODO' -CaseSensitive"] -- ps aux | grep python: ["powershell.exe", "-Command", "Get-Process | Where-Object {{ $_.ProcessName -like '*python*' }}"] -- setting an env var: ["powershell.exe", "-Command", "$env:FOO='bar'; echo $env:FOO"] -- running an inline Python script: ["powershell.exe", "-Command", "@'\\nprint('Hello, world!')\\n'@ | python -"] - -{}"#, - windows_shell_guidance() - ) - } else { - r#"Runs a shell command and returns its output. -- The arguments to `shell` will be passed to execvp(). Most terminal commands should be prefixed with ["bash", "-lc"]. -- Always set the `workdir` param when using the shell function. Do not use `cd` unless absolutely necessary."# - .to_string() - }; - - ToolSpec::Function(ResponsesApiTool { - name: "shell".to_string(), - description, - strict: false, - defer_loading: None, - parameters: JsonSchema::object( - properties, - Some(vec!["command".to_string()]), - Some(false.into()), - ), - output_schema: None, - }) -} - pub fn create_shell_command_tool(options: CommandToolOptions) -> ToolSpec { let mut properties = BTreeMap::from([ ( diff --git a/codex-rs/core/src/tools/handlers/shell_spec_tests.rs b/codex-rs/core/src/tools/handlers/shell_spec_tests.rs index a219263e1a..4774815fb2 100644 --- a/codex-rs/core/src/tools/handlers/shell_spec_tests.rs +++ b/codex-rs/core/src/tools/handlers/shell_spec_tests.rs @@ -6,91 +6,6 @@ fn windows_shell_guidance_description() -> String { format!("\n\n{}", windows_shell_guidance()) } -#[test] -fn shell_tool_matches_expected_spec() { - let tool = create_shell_tool(ShellToolOptions { - exec_permission_approvals_enabled: false, - }); - - let description = if cfg!(windows) { - r#"Runs a Powershell command (Windows) and returns its output. Arguments to `shell` will be passed to CreateProcessW(). Most commands should be prefixed with ["powershell.exe", "-Command"]. - -Examples of valid command strings: - -- ls -a (show hidden): ["powershell.exe", "-Command", "Get-ChildItem -Force"] -- recursive find by name: ["powershell.exe", "-Command", "Get-ChildItem -Recurse -Filter *.py"] -- recursive grep: ["powershell.exe", "-Command", "Get-ChildItem -Path C:\\myrepo -Recurse | Select-String -Pattern 'TODO' -CaseSensitive"] -- ps aux | grep python: ["powershell.exe", "-Command", "Get-Process | Where-Object { $_.ProcessName -like '*python*' }"] -- setting an env var: ["powershell.exe", "-Command", "$env:FOO='bar'; echo $env:FOO"] -- running an inline Python script: ["powershell.exe", "-Command", "@'\\nprint('Hello, world!')\\n'@ | python -"]"# - .to_string() - + &windows_shell_guidance_description() - } else { - r#"Runs a shell command and returns its output. -- The arguments to `shell` will be passed to execvp(). Most terminal commands should be prefixed with ["bash", "-lc"]. -- Always set the `workdir` param when using the shell function. Do not use `cd` unless absolutely necessary."# - .to_string() - }; - - let properties = BTreeMap::from([ - ( - "command".to_string(), - JsonSchema::array(JsonSchema::string(/*description*/ None), Some("The command to execute".to_string())), - ), - ( - "workdir".to_string(), - JsonSchema::string(Some("The working directory to execute the command in".to_string())), - ), - ( - "timeout_ms".to_string(), - JsonSchema::number(Some("The timeout for the command in milliseconds".to_string())), - ), - ( - "sandbox_permissions".to_string(), - JsonSchema::string(Some( - "Sandbox permissions for the command. Set to \"require_escalated\" to request running without sandbox restrictions; defaults to \"use_default\"." - .to_string(), - )), - ), - ( - "justification".to_string(), - JsonSchema::string(Some( - r#"Only set if sandbox_permissions is \"require_escalated\". - Request approval from the user to run this command outside the sandbox. - Phrased as a simple question that summarizes the purpose of the - command as it relates to the task at hand - e.g. 'Do you want to - fetch and pull the latest version of this git branch?'"# - .to_string(), - )), - ), - ( - "prefix_rule".to_string(), - JsonSchema::array(JsonSchema::string(/*description*/ None), Some( - r#"Only specify when sandbox_permissions is `require_escalated`. - Suggest a prefix command pattern that will allow you to fulfill similar requests from the user in the future. - Should be a short but reasonable prefix, e.g. [\"git\", \"pull\"] or [\"uv\", \"run\"] or [\"pytest\"]."# - .to_string(), - )), - ), - ]); - - assert_eq!( - tool, - ToolSpec::Function(ResponsesApiTool { - name: "shell".to_string(), - description, - strict: false, - defer_loading: None, - parameters: JsonSchema::object( - properties, - Some(vec!["command".to_string()]), - Some(false.into()) - ), - output_schema: None, - }) - ); -} - #[test] fn exec_command_tool_matches_expected_spec() { let tool = create_exec_command_tool(CommandToolOptions { @@ -224,77 +139,6 @@ fn write_stdin_tool_matches_expected_spec() { ); } -#[test] -fn shell_tool_with_request_permission_includes_additional_permissions() { - let tool = create_shell_tool(ShellToolOptions { - exec_permission_approvals_enabled: true, - }); - - let mut properties = BTreeMap::from([ - ( - "command".to_string(), - JsonSchema::array( - JsonSchema::string(/*description*/ None), - Some("The command to execute".to_string()), - ), - ), - ( - "workdir".to_string(), - JsonSchema::string(Some( - "The working directory to execute the command in".to_string(), - )), - ), - ( - "timeout_ms".to_string(), - JsonSchema::number(Some( - "The timeout for the command in milliseconds".to_string(), - )), - ), - ]); - properties.extend(create_approval_parameters( - /*exec_permission_approvals_enabled*/ true, - )); - - let description = if cfg!(windows) { - format!( - r#"Runs a Powershell command (Windows) and returns its output. Arguments to `shell` will be passed to CreateProcessW(). Most commands should be prefixed with ["powershell.exe", "-Command"]. - -Examples of valid command strings: - -- ls -a (show hidden): ["powershell.exe", "-Command", "Get-ChildItem -Force"] -- recursive find by name: ["powershell.exe", "-Command", "Get-ChildItem -Recurse -Filter *.py"] -- recursive grep: ["powershell.exe", "-Command", "Get-ChildItem -Path C:\\myrepo -Recurse | Select-String -Pattern 'TODO' -CaseSensitive"] -- ps aux | grep python: ["powershell.exe", "-Command", "Get-Process | Where-Object {{ $_.ProcessName -like '*python*' }}"] -- setting an env var: ["powershell.exe", "-Command", "$env:FOO='bar'; echo $env:FOO"] -- running an inline Python script: ["powershell.exe", "-Command", "@'\\nprint('Hello, world!')\\n'@ | python -"] - -{}"#, - windows_shell_guidance() - ) - } else { - r#"Runs a shell command and returns its output. -- The arguments to `shell` will be passed to execvp(). Most terminal commands should be prefixed with ["bash", "-lc"]. -- Always set the `workdir` param when using the shell function. Do not use `cd` unless absolutely necessary."# - .to_string() - }; - - assert_eq!( - tool, - ToolSpec::Function(ResponsesApiTool { - name: "shell".to_string(), - description, - strict: false, - defer_loading: None, - parameters: JsonSchema::object( - properties, - Some(vec!["command".to_string()]), - Some(false.into()) - ), - output_schema: None, - }) - ); -} - #[test] fn request_permissions_tool_includes_full_permission_schema() { let tool = diff --git a/codex-rs/core/src/tools/handlers/shell_tests.rs b/codex-rs/core/src/tools/handlers/shell_tests.rs index ce97b8317e..9db561d577 100644 --- a/codex-rs/core/src/tools/handlers/shell_tests.rs +++ b/codex-rs/core/src/tools/handlers/shell_tests.rs @@ -16,7 +16,6 @@ use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; -use crate::tools::handlers::LocalShellHandler; use crate::tools::handlers::ShellCommandHandler; use crate::tools::hook_names::HookToolName; use crate::tools::registry::ToolHandler; @@ -203,44 +202,6 @@ fn shell_command_handler_rejects_login_when_disallowed() { ); } -#[tokio::test] -async fn local_shell_pre_tool_use_payload_uses_joined_command() { - let payload = ToolPayload::LocalShell { - params: codex_protocol::models::ShellToolCallParams { - command: vec![ - "bash".to_string(), - "-lc".to_string(), - "printf hi".to_string(), - ], - workdir: None, - timeout_ms: None, - sandbox_permissions: None, - additional_permissions: None, - prefix_rule: None, - justification: None, - }, - }; - let (session, turn) = make_session_and_context().await; - let handler = LocalShellHandler::default(); - - assert_eq!( - handler.pre_tool_use_payload(&ToolInvocation { - session: session.into(), - turn: turn.into(), - cancellation_token: tokio_util::sync::CancellationToken::new(), - tracker: Arc::new(Mutex::new(TurnDiffTracker::new())), - call_id: "call-41".to_string(), - tool_name: codex_tools::ToolName::plain("local_shell"), - source: crate::tools::context::ToolCallSource::Direct, - payload, - }), - Some(crate::tools::registry::PreToolUsePayload { - tool_name: HookToolName::bash(), - tool_input: json!({ "command": "bash -lc 'printf hi'" }), - }) - ); -} - #[tokio::test] async fn shell_command_pre_tool_use_payload_uses_raw_command() { let payload = ToolPayload::Function { diff --git a/codex-rs/core/src/tools/network_approval_tests.rs b/codex-rs/core/src/tools/network_approval_tests.rs index 37a28b1255..a98d650688 100644 --- a/codex-rs/core/src/tools/network_approval_tests.rs +++ b/codex-rs/core/src/tools/network_approval_tests.rs @@ -229,7 +229,7 @@ async fn register_call_with_default_shell_trigger( "turn-1".to_string(), GuardianNetworkAccessTrigger { call_id: "call-1".to_string(), - tool_name: "shell".to_string(), + tool_name: "shell_command".to_string(), command: vec!["curl".to_string(), "https://example.com".to_string()], cwd: test_path_buf("/tmp").abs(), sandbox_permissions: SandboxPermissions::UseDefault, @@ -249,7 +249,7 @@ async fn active_call_preserves_triggering_command_context() { let service = NetworkApprovalService::default(); let expected = GuardianNetworkAccessTrigger { call_id: "call-1".to_string(), - tool_name: "shell".to_string(), + tool_name: "shell_command".to_string(), command: vec!["curl".to_string(), "https://example.com".to_string()], cwd: test_path_buf("/repo").abs(), sandbox_permissions: SandboxPermissions::UseDefault, diff --git a/codex-rs/core/src/tools/parallel.rs b/codex-rs/core/src/tools/parallel.rs index 2c62e6c15d..4c79e4b168 100644 --- a/codex-rs/core/src/tools/parallel.rs +++ b/codex-rs/core/src/tools/parallel.rs @@ -180,7 +180,7 @@ impl ToolCallRuntime { if call.tool_name.namespace.is_none() && matches!( call.tool_name.name.as_str(), - "shell" | "container.exec" | "local_shell" | "shell_command" | "unified_exec" + "shell_command" | "unified_exec" ) { format!("Wall time: {secs:.1} seconds\naborted by user") diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index d386d6dae3..d69e41ab19 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -1,5 +1,4 @@ use crate::function_tool::FunctionCallError; -use crate::sandboxing::SandboxPermissions; use crate::session::session::Session; use crate::session::turn_context::TurnContext; use crate::tools::context::SharedTurnDiffTracker; @@ -12,10 +11,8 @@ use crate::tools::spec::build_specs_with_discoverable_tools; use codex_extension_api::ExtensionToolExecutor; use codex_mcp::ToolInfo; use codex_protocol::dynamic_tools::DynamicToolSpec; -use codex_protocol::models::LocalShellAction; use codex_protocol::models::ResponseItem; use codex_protocol::models::SearchToolCallParams; -use codex_protocol::models::ShellToolCallParams; use codex_tools::DiscoverableTool; use codex_tools::ToolName; use codex_tools::ToolSpec; @@ -141,35 +138,6 @@ impl ToolRouter { call_id, payload: ToolPayload::Custom { input }, })), - ResponseItem::LocalShellCall { - id, - call_id, - action, - .. - } => { - let call_id = call_id - .or(id) - .ok_or(FunctionCallError::MissingLocalShellCallId)?; - - match action { - LocalShellAction::Exec(exec) => { - let params = ShellToolCallParams { - command: exec.command, - workdir: exec.working_directory, - timeout_ms: exec.timeout_ms, - sandbox_permissions: Some(SandboxPermissions::UseDefault), - additional_permissions: None, - prefix_rule: None, - justification: None, - }; - Ok(Some(ToolCall { - tool_name: ToolName::plain("local_shell"), - call_id, - payload: ToolPayload::LocalShell { params }, - })) - } - } - } _ => Ok(None), } } diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index b4e935efb1..b154585419 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -110,7 +110,7 @@ async fn parallel_support_does_not_match_namespaced_local_tool_names() -> anyhow }, ); - let parallel_tool_name = ["shell", "local_shell", "exec_command", "shell_command"] + let parallel_tool_name = ["exec_command", "shell_command"] .into_iter() .find(|name| { router.tool_supports_parallel(&ToolCall { @@ -399,7 +399,6 @@ fn namespace_function_names(specs: &[ToolSpec], namespace_name: &str) -> Vec None, diff --git a/codex-rs/core/src/tools/runtimes/shell.rs b/codex-rs/core/src/tools/runtimes/shell.rs index 4f6eb620aa..ad17c47596 100644 --- a/codex-rs/core/src/tools/runtimes/shell.rs +++ b/codex-rs/core/src/tools/runtimes/shell.rs @@ -62,19 +62,8 @@ pub struct ShellRequest { } /// Selects `ShellRuntime` behavior for different callers. -/// -/// Note: `Generic` is not the same as `ShellCommandClassic`. -/// `Generic` means "no `shell_command`-specific backend behavior" (used by the -/// generic `shell` tool path). The `ShellCommand*` variants are only for the -/// `shell_command` tool family. -#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) enum ShellRuntimeBackend { - /// Tool-agnostic/default runtime path. - /// - /// Uses the normal `ShellRuntime` execution flow without enabling any - /// `shell_command`-specific backend selection. - #[default] - Generic, /// Legacy backend for the `shell_command` tool. /// /// Keeps `shell_command` on the standard shell runtime flow without the @@ -88,7 +77,6 @@ pub(crate) enum ShellRuntimeBackend { ShellCommandZshFork, } -#[derive(Default)] pub struct ShellRuntime { backend: ShellRuntimeBackend, } @@ -102,12 +90,6 @@ pub(crate) struct ApprovalKey { } impl ShellRuntime { - pub fn new() -> Self { - Self { - backend: ShellRuntimeBackend::Generic, - } - } - pub(crate) fn for_shell_command(backend: ShellRuntimeBackend) -> Self { Self { backend } } diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index 0ac670921b..c4db408c55 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -2,7 +2,6 @@ use crate::tools::code_mode::execute_spec::create_code_mode_tool; use crate::tools::handlers::ApplyPatchHandler; use crate::tools::handlers::CodeModeExecuteHandler; use crate::tools::handlers::CodeModeWaitHandler; -use crate::tools::handlers::ContainerExecHandler; use crate::tools::handlers::CreateGoalHandler; use crate::tools::handlers::DynamicToolHandler; use crate::tools::handlers::ExecCommandHandler; @@ -10,7 +9,6 @@ use crate::tools::handlers::ExecCommandHandlerOptions; use crate::tools::handlers::GetGoalHandler; use crate::tools::handlers::ListMcpResourceTemplatesHandler; use crate::tools::handlers::ListMcpResourcesHandler; -use crate::tools::handlers::LocalShellHandler; use crate::tools::handlers::McpHandler; use crate::tools::handlers::PlanHandler; use crate::tools::handlers::ReadMcpResourceHandler; @@ -19,7 +17,6 @@ use crate::tools::handlers::RequestPluginInstallHandler; use crate::tools::handlers::RequestUserInputHandler; use crate::tools::handlers::ShellCommandHandler; use crate::tools::handlers::ShellCommandHandlerOptions; -use crate::tools::handlers::ShellHandler; use crate::tools::handlers::TestSyncHandler; use crate::tools::handlers::ToolSearchHandler; use crate::tools::handlers::UpdateGoalHandler; @@ -39,7 +36,6 @@ use crate::tools::handlers::multi_agents_v2::ListAgentsHandler as ListAgentsHand use crate::tools::handlers::multi_agents_v2::SendMessageHandler as SendMessageHandlerV2; use crate::tools::handlers::multi_agents_v2::SpawnAgentHandler as SpawnAgentHandlerV2; use crate::tools::handlers::multi_agents_v2::WaitAgentHandler as WaitAgentHandlerV2; -use crate::tools::handlers::shell_spec::ShellToolOptions; use crate::tools::handlers::view_image_spec::ViewImageToolOptions; use crate::tools::hosted_spec::WebSearchToolOptions; use crate::tools::hosted_spec::create_image_generation_tool; @@ -252,14 +248,6 @@ fn collect_handler_tools( let include_environment_id = matches!(config.environment_mode, ToolEnvironmentMode::Multiple); match &config.shell_type { - ConfigShellToolType::Default => { - handlers.push(Arc::new(ShellHandler::new(ShellToolOptions { - exec_permission_approvals_enabled, - }))); - } - ConfigShellToolType::Local => { - handlers.push(Arc::new(LocalShellHandler::new())); - } ConfigShellToolType::UnifiedExec => { handlers.push(Arc::new(ExecCommandHandler::new( ExecCommandHandlerOptions { @@ -271,7 +259,9 @@ fn collect_handler_tools( handlers.push(Arc::new(WriteStdinHandler)); } ConfigShellToolType::Disabled => {} - ConfigShellToolType::ShellCommand => { + ConfigShellToolType::Default + | ConfigShellToolType::Local + | ConfigShellToolType::ShellCommand => { handlers.push(Arc::new(ShellCommandHandler::new( ShellCommandHandlerOptions { backend_config: config.shell_command_backend, @@ -287,34 +277,15 @@ fn collect_handler_tools( && config.shell_type != ConfigShellToolType::Disabled { match &config.shell_type { - ConfigShellToolType::Default => { - handlers.push(Arc::new(ContainerExecHandler)); - handlers.push(Arc::new(LocalShellHandler::default())); - handlers.push(Arc::new(ShellCommandHandler::from( - config.shell_command_backend, - ))); - } - ConfigShellToolType::Local => { - handlers.push(Arc::new(ShellHandler::default())); - handlers.push(Arc::new(ContainerExecHandler)); - handlers.push(Arc::new(ShellCommandHandler::from( - config.shell_command_backend, - ))); - } ConfigShellToolType::UnifiedExec => { - handlers.push(Arc::new(ShellHandler::default())); - handlers.push(Arc::new(ContainerExecHandler)); - handlers.push(Arc::new(LocalShellHandler::default())); handlers.push(Arc::new(ShellCommandHandler::from( config.shell_command_backend, ))); } - ConfigShellToolType::ShellCommand => { - handlers.push(Arc::new(ShellHandler::default())); - handlers.push(Arc::new(ContainerExecHandler)); - handlers.push(Arc::new(LocalShellHandler::default())); - } - ConfigShellToolType::Disabled => {} + ConfigShellToolType::Default + | ConfigShellToolType::Local + | ConfigShellToolType::ShellCommand + | ConfigShellToolType::Disabled => {} } } diff --git a/codex-rs/core/src/tools/spec_plan_tests.rs b/codex-rs/core/src/tools/spec_plan_tests.rs index 207aa79234..044c445046 100644 --- a/codex-rs/core/src/tools/spec_plan_tests.rs +++ b/codex-rs/core/src/tools/spec_plan_tests.rs @@ -2770,7 +2770,6 @@ fn strip_descriptions_tool(spec: &mut ToolSpec) { } } ToolSpec::Freeform(FreeformTool { .. }) - | ToolSpec::LocalShell {} | ToolSpec::ImageGeneration { .. } | ToolSpec::WebSearch { .. } => {} } diff --git a/codex-rs/core/src/tools/spec_tests.rs b/codex-rs/core/src/tools/spec_tests.rs index 560f87b2f0..38645e69bb 100644 --- a/codex-rs/core/src/tools/spec_tests.rs +++ b/codex-rs/core/src/tools/spec_tests.rs @@ -182,8 +182,8 @@ fn assert_contains_tool_names(tools: &[ToolSpec], expected_subset: &[&str]) { fn shell_tool_name(config: &ToolsConfig) -> Option<&'static str> { match config.shell_type { - ConfigShellToolType::Default => Some("shell"), - ConfigShellToolType::Local => Some("local_shell"), + ConfigShellToolType::Default => Some("shell_command"), + ConfigShellToolType::Local => Some("shell_command"), ConfigShellToolType::UnifiedExec => None, ConfigShellToolType::Disabled => None, ConfigShellToolType::ShellCommand => Some("shell_command"), diff --git a/codex-rs/core/src/tools/tool_dispatch_trace.rs b/codex-rs/core/src/tools/tool_dispatch_trace.rs index 6a0e635792..522e3cd1d8 100644 --- a/codex-rs/core/src/tools/tool_dispatch_trace.rs +++ b/codex-rs/core/src/tools/tool_dispatch_trace.rs @@ -111,15 +111,6 @@ fn tool_dispatch_payload(payload: &ToolPayload) -> ToolDispatchPayload { ToolPayload::Custom { input } => ToolDispatchPayload::Custom { input: input.clone(), }, - ToolPayload::LocalShell { params } => ToolDispatchPayload::LocalShell { - command: params.command.clone(), - workdir: params.workdir.clone(), - timeout_ms: params.timeout_ms, - sandbox_permissions: params.sandbox_permissions, - prefix_rule: params.prefix_rule.clone(), - additional_permissions: params.additional_permissions.clone(), - justification: params.justification.clone(), - }, } } diff --git a/codex-rs/core/src/tools/tool_search_entry.rs b/codex-rs/core/src/tools/tool_search_entry.rs index 7ecaf91a4d..8e37cdcfff 100644 --- a/codex-rs/core/src/tools/tool_search_entry.rs +++ b/codex-rs/core/src/tools/tool_search_entry.rs @@ -40,7 +40,6 @@ impl ToolSearchInfo { LoadableToolSpec::Namespace(namespace) } ToolSpec::ToolSearch { .. } - | ToolSpec::LocalShell {} | ToolSpec::ImageGeneration { .. } | ToolSpec::WebSearch { .. } | ToolSpec::Freeform(_) => return None, diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs index eab90c5aba..6c1f581f9f 100644 --- a/codex-rs/core/tests/common/responses.rs +++ b/codex-rs/core/tests/common/responses.rs @@ -890,10 +890,6 @@ pub fn ev_apply_patch_call( ) -> Value { match output_type { ApplyPatchModelOutput::Freeform => ev_apply_patch_custom_tool_call(call_id, patch), - ApplyPatchModelOutput::Shell => ev_apply_patch_shell_call(call_id, patch), - ApplyPatchModelOutput::ShellViaHeredoc => { - ev_apply_patch_shell_call_via_heredoc(call_id, patch) - } ApplyPatchModelOutput::ShellCommandViaHeredoc => { ev_apply_patch_shell_command_call_via_heredoc(call_id, patch) } @@ -925,21 +921,6 @@ pub fn ev_shell_command_call_with_args(call_id: &str, args: &serde_json::Value) ev_function_call(call_id, "shell_command", &arguments) } -pub fn ev_apply_patch_shell_call(call_id: &str, patch: &str) -> Value { - let args = serde_json::json!({ "command": ["apply_patch", patch] }); - let arguments = serde_json::to_string(&args).expect("serialize apply_patch arguments"); - - ev_function_call(call_id, "shell", &arguments) -} - -pub fn ev_apply_patch_shell_call_via_heredoc(call_id: &str, patch: &str) -> Value { - let script = format!("apply_patch <<'EOF'\n{patch}\nEOF\n"); - let args = serde_json::json!({ "command": ["bash", "-lc", script] }); - let arguments = serde_json::to_string(&args).expect("serialize apply_patch arguments"); - - ev_function_call(call_id, "shell", &arguments) -} - pub fn ev_apply_patch_shell_command_call_via_heredoc(call_id: &str, patch: &str) -> Value { let args = serde_json::json!({ "command": format!("apply_patch <<'EOF'\n{patch}\nEOF\n") }); let arguments = serde_json::to_string(&args).expect("serialize apply_patch arguments"); diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 24c25d6364..e7db3f4044 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -189,17 +189,13 @@ fn docker_command_capture_stdout(args: [&str; N]) -> Result { Box::pin(self.custom_tool_call_output(call_id)).await } - ApplyPatchModelOutput::Shell - | ApplyPatchModelOutput::ShellViaHeredoc - | ApplyPatchModelOutput::ShellCommandViaHeredoc => { + ApplyPatchModelOutput::ShellCommandViaHeredoc => { Box::pin(self.function_call_stdout(call_id)).await } } diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index 82113ee494..e42057215a 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -258,8 +258,6 @@ async fn apply_patch_cli_uses_codex_self_exe_with_linux_sandbox_helper_alias() - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] async fn apply_patch_cli_multiple_operations_integration( output_type: ApplyPatchModelOutput, ) -> Result<()> { @@ -302,8 +300,6 @@ D delete.txt #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_multiple_chunks(model_output: ApplyPatchModelOutput) -> Result<()> { skip_if_no_network!(Ok(())); @@ -329,8 +325,6 @@ async fn apply_patch_cli_multiple_chunks(model_output: ApplyPatchModelOutput) -> #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_moves_file_to_new_directory( model_output: ApplyPatchModelOutput, @@ -357,8 +351,6 @@ async fn apply_patch_cli_moves_file_to_new_directory( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_updates_file_appends_trailing_newline( model_output: ApplyPatchModelOutput, @@ -385,8 +377,6 @@ async fn apply_patch_cli_updates_file_appends_trailing_newline( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_insert_only_hunk_modifies_file( model_output: ApplyPatchModelOutput, @@ -414,8 +404,6 @@ async fn apply_patch_cli_insert_only_hunk_modifies_file( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_move_overwrites_existing_destination( model_output: ApplyPatchModelOutput, @@ -445,8 +433,6 @@ async fn apply_patch_cli_move_overwrites_existing_destination( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_move_without_content_change_has_no_turn_diff( model_output: ApplyPatchModelOutput, @@ -484,8 +470,6 @@ async fn apply_patch_cli_move_without_content_change_has_no_turn_diff( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_add_overwrites_existing_file( model_output: ApplyPatchModelOutput, @@ -511,8 +495,6 @@ async fn apply_patch_cli_add_overwrites_existing_file( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_rejects_invalid_hunk_header( model_output: ApplyPatchModelOutput, @@ -542,8 +524,6 @@ async fn apply_patch_cli_rejects_invalid_hunk_header( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_reports_missing_context( model_output: ApplyPatchModelOutput, @@ -577,8 +557,6 @@ async fn apply_patch_cli_reports_missing_context( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_reports_missing_target_file( model_output: ApplyPatchModelOutput, @@ -612,8 +590,6 @@ async fn apply_patch_cli_reports_missing_target_file( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_delete_missing_file_reports_error( model_output: ApplyPatchModelOutput, @@ -648,8 +624,6 @@ async fn apply_patch_cli_delete_missing_file_reports_error( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_rejects_empty_patch(model_output: ApplyPatchModelOutput) -> Result<()> { skip_if_no_network!(Ok(())); @@ -672,8 +646,6 @@ async fn apply_patch_cli_rejects_empty_patch(model_output: ApplyPatchModelOutput #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_delete_directory_reports_verification_error( model_output: ApplyPatchModelOutput, @@ -698,8 +670,6 @@ async fn apply_patch_cli_delete_directory_reports_verification_error( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_rejects_path_traversal_outside_workspace( model_output: ApplyPatchModelOutput, @@ -744,8 +714,6 @@ async fn apply_patch_cli_rejects_path_traversal_outside_workspace( #[cfg(unix)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -#[test_case(ApplyPatchModelOutput::Shell ; "shell")] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc ; "shell_heredoc")] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc ; "shell_command_heredoc")] async fn intercepted_apply_patch_verification_uses_local_sandbox( model_output: ApplyPatchModelOutput, @@ -797,8 +765,6 @@ async fn intercepted_apply_patch_verification_uses_local_sandbox( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform ; "freeform")] -#[test_case(ApplyPatchModelOutput::Shell ; "shell")] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc ; "shell_heredoc")] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc ; "shell_command_heredoc")] async fn apply_patch_cli_does_not_write_through_symlink_escape_outside_workspace( model_output: ApplyPatchModelOutput, @@ -868,8 +834,6 @@ async fn apply_patch_cli_does_not_write_through_symlink_escape_outside_workspace #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform ; "freeform")] -#[test_case(ApplyPatchModelOutput::Shell ; "shell")] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc ; "shell_heredoc")] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc ; "shell_command_heredoc")] async fn apply_patch_cli_preserves_existing_hard_link_outside_workspace( model_output: ApplyPatchModelOutput, @@ -972,8 +936,6 @@ async fn apply_patch_cli_preserves_existing_hard_link_outside_workspace( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace( model_output: ApplyPatchModelOutput, @@ -1021,8 +983,6 @@ async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_verification_failure_has_no_side_effects( model_output: ApplyPatchModelOutput, @@ -1535,7 +1495,6 @@ async fn apply_patch_shell_command_failure_propagates_error_and_skips_diff() -> } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_shell_accepts_lenient_heredoc_wrapped_patch( model_output: ApplyPatchModelOutput, @@ -1558,8 +1517,6 @@ async fn apply_patch_shell_accepts_lenient_heredoc_wrapped_patch( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_end_of_file_anchor(model_output: ApplyPatchModelOutput) -> Result<()> { skip_if_no_network!(Ok(())); @@ -1579,8 +1536,6 @@ async fn apply_patch_cli_end_of_file_anchor(model_output: ApplyPatchModelOutput) #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_cli_missing_second_chunk_context_rejected( model_output: ApplyPatchModelOutput, @@ -1615,8 +1570,6 @@ async fn apply_patch_cli_missing_second_chunk_context_rejected( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_emits_turn_diff_event_with_unified_diff( model_output: ApplyPatchModelOutput, @@ -1847,8 +1800,6 @@ async fn apply_patch_clears_aggregated_diff_after_inexact_delta() -> Result<()> #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)] async fn apply_patch_change_context_disambiguates_target( model_output: ApplyPatchModelOutput, diff --git a/codex-rs/core/tests/suite/compact.rs b/codex-rs/core/tests/suite/compact.rs index b1620ee36b..68ddf1691b 100644 --- a/codex-rs/core/tests/suite/compact.rs +++ b/codex-rs/core/tests/suite/compact.rs @@ -26,7 +26,6 @@ use core_test_support::context_snapshot; use core_test_support::context_snapshot::ContextSnapshotOptions; use core_test_support::context_snapshot::ContextSnapshotRenderMode; use core_test_support::hooks::trust_discovered_hooks; -use core_test_support::responses::ev_local_shell_call; use core_test_support::responses::ev_reasoning_item; use core_test_support::responses::mount_models_once; use core_test_support::skip_if_no_network; @@ -78,6 +77,14 @@ const PRETURN_CONTEXT_DIFF_CWD: &str = "/tmp/PRETURN_CONTEXT_DIFF_CWD"; pub(super) const COMPACT_WARNING_MESSAGE: &str = "Heads up: Long threads and multiple compactions can cause the model to be less accurate. Start a new thread when possible to keep threads small and targeted."; +fn ev_shell_command_call(call_id: &str, command: &str) -> serde_json::Value { + ev_function_call( + call_id, + "shell_command", + &json!({ "command": command }).to_string(), + ) +} + fn disabled_permission_user_turn(text: impl Into, cwd: PathBuf, model: String) -> Op { let (sandbox_policy, permission_profile) = turn_permission_fields(PermissionProfile::Disabled, cwd.as_path()); @@ -954,7 +961,7 @@ async fn multiple_auto_compact_per_task_runs_after_token_limit_hit() { // first chunk of work let model_reasoning_response_1_sse = sse(vec![ reasoning_response_1.clone(), - ev_local_shell_call("r1-shell", "completed", vec!["echo", "make-react"]), + ev_shell_command_call("r1-shell", "echo make-react"), ev_completed_with_tokens("r1", token_count_used), ]); @@ -972,7 +979,7 @@ async fn multiple_auto_compact_per_task_runs_after_token_limit_hit() { // second chunk of work let model_reasoning_response_2_sse = sse(vec![ reasoning_response_2.clone(), - ev_local_shell_call("r3-shell", "completed", vec!["echo", "make-node"]), + ev_shell_command_call("r3-shell", "echo make-node"), ev_completed_with_tokens("r3", token_count_used), ]); @@ -990,7 +997,7 @@ async fn multiple_auto_compact_per_task_runs_after_token_limit_hit() { // third chunk of work let model_reasoning_response_3_sse = sse(vec![ ev_reasoning_item("m6", &["I will create a python app"], &[]), - ev_local_shell_call("r6-shell", "completed", vec!["echo", "make-python"]), + ev_shell_command_call("r6-shell", "echo make-python"), ev_completed_with_tokens("r6", token_count_used), ]); @@ -1186,20 +1193,10 @@ async fn multiple_auto_compact_per_task_runs_after_token_limit_hit() { "type": "reasoning" }, { - "action": { - "command": [ - "echo", - "make-react" - ], - "env": null, - "timeout_ms": null, - "type": "exec", - "user": null, - "working_directory": null - }, + "arguments": "{\"command\":\"echo make-react\"}", "call_id": "r1-shell", - "status": "completed", - "type": "local_shell_call" + "name": "shell_command", + "type": "function_call" }, { "call_id": "r1-shell", @@ -1296,20 +1293,10 @@ async fn multiple_auto_compact_per_task_runs_after_token_limit_hit() { "type": "reasoning" }, { - "action": { - "command": [ - "echo", - "make-node" - ], - "env": null, - "timeout_ms": null, - "type": "exec", - "user": null, - "working_directory": null - }, + "arguments": "{\"command\":\"echo make-node\"}", "call_id": "r3-shell", - "status": "completed", - "type": "local_shell_call" + "name": "shell_command", + "type": "function_call" }, { "call_id": "r3-shell", @@ -1406,20 +1393,10 @@ async fn multiple_auto_compact_per_task_runs_after_token_limit_hit() { "type": "reasoning" }, { - "action": { - "command": [ - "echo", - "make-python" - ], - "env": null, - "timeout_ms": null, - "type": "exec", - "user": null, - "working_directory": null - }, + "arguments": "{\"command\":\"echo make-python\"}", "call_id": "r6-shell", - "status": "completed", - "type": "local_shell_call" + "name": "shell_command", + "type": "function_call" }, { "call_id": "r6-shell", diff --git a/codex-rs/core/tests/suite/compact_remote.rs b/codex-rs/core/tests/suite/compact_remote.rs index 88aafd26ac..8849d80773 100644 --- a/codex-rs/core/tests/suite/compact_remote.rs +++ b/codex-rs/core/tests/suite/compact_remote.rs @@ -479,10 +479,9 @@ async fn assert_remote_manual_compact_request_parity( responses::ev_completed("turn-three-final-response"), ]), responses::sse(vec![ - responses::ev_local_shell_call( - "turn-four-local-shell", - "completed", - vec!["/bin/echo", "TURN_FOUR_LOCAL_SHELL"], + responses::ev_shell_command_call( + "turn-four-shell-command", + "echo TURN_FOUR_LOCAL_SHELL", ), responses::ev_completed("turn-four-local-shell-response"), ]), @@ -589,7 +588,7 @@ async fn assert_remote_manual_compact_request_parity( assert_eq!( response_requests.len(), 7, - "expected five turns with one unsupported tool continuation and one local shell continuation" + "expected five turns with one unsupported tool continuation and one shell command continuation" ); assert_eq!( compact_mock.requests().len(), diff --git a/codex-rs/core/tests/suite/hooks.rs b/codex-rs/core/tests/suite/hooks.rs index 563719cec2..635aeaf488 100644 --- a/codex-rs/core/tests/suite/hooks.rs +++ b/codex-rs/core/tests/suite/hooks.rs @@ -2135,48 +2135,25 @@ async fn blocked_pre_tool_use_records_additional_context_for_shell_command() -> #[derive(Clone, Copy)] enum BashRewriteSurface { - ContainerExec, ExecCommand, - LocalShell, - Shell, ShellCommand, } impl BashRewriteSurface { fn slug(self) -> &'static str { match self { - BashRewriteSurface::ContainerExec => "container-exec", BashRewriteSurface::ExecCommand => "exec-command", - BashRewriteSurface::LocalShell => "local-shell", - BashRewriteSurface::Shell => "shell", BashRewriteSurface::ShellCommand => "shell-command", } } - fn tool_call(self, call_id: &str, command: &[String], command_text: &str) -> Result { + fn tool_call(self, call_id: &str, command_text: &str) -> Result { match self { - BashRewriteSurface::ContainerExec => Ok(ev_function_call( - call_id, - "container.exec", - &serde_json::to_string(&serde_json::json!({ "command": command }))?, - )), BashRewriteSurface::ExecCommand => Ok(ev_function_call( call_id, "exec_command", &serde_json::to_string(&serde_json::json!({ "cmd": command_text }))?, )), - BashRewriteSurface::LocalShell => { - Ok(core_test_support::responses::ev_local_shell_call( - call_id, - "completed", - command.iter().map(String::as_str).collect(), - )) - } - BashRewriteSurface::Shell => Ok(ev_function_call( - call_id, - "shell", - &serde_json::to_string(&serde_json::json!({ "command": command }))?, - )), BashRewriteSurface::ShellCommand => Ok(ev_function_call( call_id, "shell_command", @@ -2185,33 +2162,19 @@ impl BashRewriteSurface { } } - fn original_command(self, marker: &Path) -> (Vec, String) { - let command_text = format!("printf original > {}", marker.display()); + fn original_command(self, marker: &Path) -> String { match self { - BashRewriteSurface::ContainerExec - | BashRewriteSurface::LocalShell - | BashRewriteSurface::Shell => { - let command = vec!["/bin/sh".to_string(), "-c".to_string(), command_text]; - let command_text = codex_shell_command::parse_command::shlex_join(&command); - (command, command_text) - } BashRewriteSurface::ExecCommand | BashRewriteSurface::ShellCommand => { - (Vec::new(), command_text) + format!("printf original > {}", marker.display()) } } } fn rewritten_command(self, marker: &Path) -> String { - let command_text = format!("printf rewritten > {}", marker.display()); match self { - BashRewriteSurface::ContainerExec - | BashRewriteSurface::LocalShell - | BashRewriteSurface::Shell => codex_shell_command::parse_command::shlex_join(&[ - "/bin/sh".to_string(), - "-c".to_string(), - command_text, - ]), - BashRewriteSurface::ExecCommand | BashRewriteSurface::ShellCommand => command_text, + BashRewriteSurface::ExecCommand | BashRewriteSurface::ShellCommand => { + format!("printf rewritten > {}", marker.display()) + } } } @@ -2234,14 +2197,14 @@ async fn assert_pre_tool_use_rewrites_bash_surface(surface: BashRewriteSurface) let call_id = format!("pretooluse-{slug}-rewrite"); let original_marker = std::env::temp_dir().join(format!("pretooluse-{slug}-original-marker")); let rewritten_marker = std::env::temp_dir().join(format!("pretooluse-{slug}-rewritten-marker")); - let (tool_command, original_command) = surface.original_command(&original_marker); + let original_command = surface.original_command(&original_marker); let rewritten_command = surface.rewritten_command(&rewritten_marker); let responses = mount_sse_sequence( &server, vec![ sse(vec![ ev_response_created("resp-1"), - surface.tool_call(&call_id, &tool_command, &original_command)?, + surface.tool_call(&call_id, &original_command)?, ev_completed("resp-1"), ]), sse(vec![ @@ -2295,21 +2258,6 @@ async fn assert_pre_tool_use_rewrites_bash_surface(surface: BashRewriteSurface) Ok(()) } -#[tokio::test] -async fn pre_tool_use_rewrites_shell_before_execution() -> Result<()> { - assert_pre_tool_use_rewrites_bash_surface(BashRewriteSurface::Shell).await -} - -#[tokio::test] -async fn pre_tool_use_rewrites_container_exec_before_execution() -> Result<()> { - assert_pre_tool_use_rewrites_bash_surface(BashRewriteSurface::ContainerExec).await -} - -#[tokio::test] -async fn pre_tool_use_rewrites_local_shell_before_execution() -> Result<()> { - assert_pre_tool_use_rewrites_bash_surface(BashRewriteSurface::LocalShell).await -} - #[tokio::test] async fn pre_tool_use_rewrites_shell_command_before_execution() -> Result<()> { assert_pre_tool_use_rewrites_bash_surface(BashRewriteSurface::ShellCommand).await @@ -2745,95 +2693,6 @@ async fn pre_tool_use_merges_hooks_json_and_config_toml() -> Result<()> { Ok(()) } -#[tokio::test] -async fn pre_tool_use_blocks_local_shell_before_execution() -> Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let call_id = "pretooluse-local-shell"; - let marker = std::env::temp_dir().join("pretooluse-local-shell-marker"); - let command = vec![ - "/bin/sh".to_string(), - "-c".to_string(), - format!("printf blocked > {}", marker.display()), - ]; - let responses = mount_sse_sequence( - &server, - vec![ - sse(vec![ - ev_response_created("resp-1"), - core_test_support::responses::ev_local_shell_call( - call_id, - "completed", - command.iter().map(String::as_str).collect(), - ), - ev_completed("resp-1"), - ]), - sse(vec![ - ev_response_created("resp-2"), - ev_assistant_message("msg-1", "local shell blocked"), - ev_completed("resp-2"), - ]), - ], - ) - .await; - - let mut builder = test_codex() - .with_pre_build_hook(|home| { - if let Err(error) = - write_pre_tool_use_hook(home, Some("^Bash$"), "json_deny", "blocked local shell") - { - panic!("failed to write pre tool use hook test fixture: {error}"); - } - }) - .with_config(trust_discovered_hooks); - let test = builder.build(&server).await?; - - if marker.exists() { - fs::remove_file(&marker).context("remove leftover local shell marker")?; - } - - test.submit_turn("run the blocked local shell command") - .await?; - - let requests = responses.requests(); - assert_eq!(requests.len(), 2); - let output_item = requests[1].function_call_output(call_id); - let output = output_item - .get("output") - .and_then(Value::as_str) - .expect("local shell output string"); - assert!( - output.contains("Command blocked by PreToolUse hook: blocked local shell"), - "blocked local shell output should surface the hook reason", - ); - assert!( - output.contains(&format!( - "Command: {}", - codex_shell_command::parse_command::shlex_join(&command) - )), - "blocked local shell output should surface the blocked command", - ); - assert!( - !marker.exists(), - "blocked local shell command should not execute" - ); - - let hook_inputs = read_pre_tool_use_hook_inputs(test.codex_home_path())?; - assert_eq!(hook_inputs.len(), 1); - assert_eq!( - hook_inputs[0]["tool_input"]["command"], - codex_shell_command::parse_command::shlex_join(&command), - ); - assert!( - hook_inputs[0]["turn_id"] - .as_str() - .is_some_and(|turn_id| !turn_id.is_empty()) - ); - - Ok(()) -} - #[tokio::test] async fn pre_tool_use_blocks_exec_command_before_execution() -> Result<()> { skip_if_no_network!(Ok(())); @@ -3424,75 +3283,6 @@ async fn post_tool_use_continue_false_replaces_shell_command_output_with_stop_re Ok(()) } -#[tokio::test] -async fn post_tool_use_records_additional_context_for_local_shell() -> Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let call_id = "posttooluse-local-shell"; - let command = vec![ - "/bin/sh".to_string(), - "-c".to_string(), - "printf local-post-tool-output".to_string(), - ]; - let responses = mount_sse_sequence( - &server, - vec![ - sse(vec![ - ev_response_created("resp-1"), - core_test_support::responses::ev_local_shell_call( - call_id, - "completed", - command.iter().map(String::as_str).collect(), - ), - ev_completed("resp-1"), - ]), - sse(vec![ - ev_response_created("resp-2"), - ev_assistant_message("msg-1", "local shell post hook context observed"), - ev_completed("resp-2"), - ]), - ], - ) - .await; - - let post_context = "Remember the local shell post-tool note."; - let mut builder = test_codex() - .with_pre_build_hook(|home| { - if let Err(error) = - write_post_tool_use_hook(home, Some("^Bash$"), "context", post_context) - { - panic!("failed to write post tool use hook test fixture: {error}"); - } - }) - .with_config(trust_discovered_hooks); - let test = builder.build(&server).await?; - - test.submit_turn("run the local shell command with post hook") - .await?; - - let requests = responses.requests(); - assert_eq!(requests.len(), 2); - assert!( - requests[1] - .message_input_texts("developer") - .contains(&post_context.to_string()), - "follow-up request should include local shell post tool use additional context", - ); - let hook_inputs = read_post_tool_use_hook_inputs(test.codex_home_path())?; - assert_eq!(hook_inputs.len(), 1); - assert_eq!( - hook_inputs[0]["tool_input"]["command"], - codex_shell_command::parse_command::shlex_join(&command), - ); - assert_eq!( - hook_inputs[0]["tool_response"], - Value::String("local-post-tool-output".to_string()), - ); - - Ok(()) -} - #[tokio::test] async fn post_tool_use_exit_two_replaces_one_shot_exec_command_output_with_feedback() -> Result<()> { diff --git a/codex-rs/core/tests/suite/models_etag_responses.rs b/codex-rs/core/tests/suite/models_etag_responses.rs index a9aa843ad1..346c503c25 100644 --- a/codex-rs/core/tests/suite/models_etag_responses.rs +++ b/codex-rs/core/tests/suite/models_etag_responses.rs @@ -15,8 +15,8 @@ use codex_protocol::user_input::UserInput; use core_test_support::responses; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; -use core_test_support::responses::ev_local_shell_call; use core_test_support::responses::ev_response_created; +use core_test_support::responses::ev_shell_command_call; use core_test_support::responses::sse; use core_test_support::responses::sse_response; use core_test_support::skip_if_no_network; @@ -32,7 +32,7 @@ async fn refresh_models_on_models_etag_mismatch_and_avoid_duplicate_models_fetch const ETAG_1: &str = "\"models-etag-1\""; const ETAG_2: &str = "\"models-etag-2\""; - const CALL_ID: &str = "local-shell-call-1"; + const CALL_ID: &str = "shell-command-call-1"; let server = MockServer::start().await; @@ -81,7 +81,7 @@ async fn refresh_models_on_models_etag_mismatch_and_avoid_duplicate_models_fetch // It also includes a mismatched X-Models-Etag, which should trigger a /models refresh. let first_response_body = sse(vec![ ev_response_created("resp-1"), - ev_local_shell_call(CALL_ID, "completed", vec!["/bin/echo", "etag ok"]), + ev_shell_command_call(CALL_ID, "/bin/echo 'etag ok'"), ev_completed("resp-1"), ]); responses::mount_response_once( diff --git a/codex-rs/core/tests/suite/otel.rs b/codex-rs/core/tests/suite/otel.rs index eb05b8a645..03692ac547 100644 --- a/codex-rs/core/tests/suite/otel.rs +++ b/codex-rs/core/tests/suite/otel.rs @@ -11,7 +11,6 @@ use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_custom_tool_call; use core_test_support::responses::ev_function_call; -use core_test_support::responses::ev_local_shell_call; use core_test_support::responses::ev_message_item_added; use core_test_support::responses::ev_output_text_delta; use core_test_support::responses::ev_reasoning_item; @@ -72,6 +71,19 @@ fn assert_empty_mcp_tool_fields(line: &str) -> Result<(), String> { Ok(()) } +fn shell_command_call(call_id: &str, command: &str) -> serde_json::Value { + let args = serde_json::json!({ "command": command }).to_string(); + ev_function_call(call_id, "shell_command", &args) +} + +fn touch_command(path: &str) -> String { + if cfg!(windows) { + format!("New-Item -ItemType File -Path {path} -Force | Out-Null") + } else { + format!("/usr/bin/touch {path}") + } +} + #[test] fn extract_log_field_handles_empty_bare_values() { let line = "event.name=\"codex.tool_result\" mcp_server= mcp_server_origin="; @@ -996,23 +1008,13 @@ async fn handle_response_item_records_tool_result_for_function_call() { #[tokio::test] #[traced_test] -async fn handle_response_item_records_tool_result_for_local_shell_missing_ids() { +async fn handle_response_item_records_tool_result_for_shell_command_call() { let server = start_mock_server().await; mount_sse_once( &server, sse(vec![ - serde_json::json!({ - "type": "response.output_item.done", - "item": { - "type": "local_shell_call", - "status": "completed", - "action": { - "type": "exec", - "command": vec!["/bin/echo", "hello"], - } - } - }), + shell_command_call("shell-call", "echo shell"), ev_completed("done"), ]), ) @@ -1021,76 +1023,7 @@ async fn handle_response_item_records_tool_result_for_local_shell_missing_ids() mount_sse_once( &server, sse(vec![ - ev_assistant_message("msg-1", "local shell done"), - ev_completed("done"), - ]), - ) - .await; - - let TestCodex { codex, .. } = test_codex() - .with_config(move |config| { - config - .features - .disable(Feature::GhostCommit) - .expect("test config should allow feature update"); - }) - .build(&server) - .await - .unwrap(); - - codex - .submit(Op::UserInput { - environments: None, - items: vec![UserInput::Text { - text: "hello".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - responsesapi_client_metadata: None, - }) - .await - .unwrap(); - - wait_for_event(&codex, |ev| matches!(ev, EventMsg::TokenCount(_))).await; - - logs_assert(|lines: &[&str]| { - let line = lines - .iter() - .find(|line| { - line.contains("codex.tool_result") - && line.contains(&"tool_name=local_shell".to_string()) - && line.contains("output=LocalShellCall without call_id or id") - }) - .ok_or_else(|| "missing codex.tool_result event".to_string())?; - - if !line.contains("success=false") { - return Err("missing success field".to_string()); - } - assert_empty_mcp_tool_fields(line)?; - - Ok(()) - }); -} - -#[cfg(target_os = "macos")] -#[tokio::test] -#[traced_test] -async fn handle_response_item_records_tool_result_for_local_shell_call() { - let server = start_mock_server().await; - - mount_sse_once( - &server, - sse(vec![ - ev_local_shell_call("shell-call", "completed", vec!["/bin/echo", "shell"]), - ev_completed("done"), - ]), - ) - .await; - - mount_sse_once( - &server, - sse(vec![ - ev_assistant_message("msg-1", "local shell done"), + ev_assistant_message("msg-1", "shell command done"), ev_completed("done"), ]), ) @@ -1129,10 +1062,10 @@ async fn handle_response_item_records_tool_result_for_local_shell_call() { .find(|line| line.contains("codex.tool_result") && line.contains("call_id=shell-call")) .ok_or_else(|| "missing codex.tool_result event".to_string())?; - if !line.contains("tool_name=local_shell") { + if !line.contains("tool_name=shell_command") { return Err("missing tool_name field".to_string()); } - if !line.contains("arguments=/bin/echo shell") { + if !line.contains("arguments={\"command\":\"echo shell\"}") { return Err("missing arguments field".to_string()); } let output_idx = line @@ -1168,8 +1101,8 @@ fn tool_decision_assertion<'a>( .ok_or_else(|| format!("missing codex.tool_decision event for {call_id}"))?; let lower = line.to_lowercase(); - if !lower.contains("tool_name=local_shell") { - return Err("missing tool_name for local_shell".to_string()); + if !lower.contains("tool_name=shell_command") { + return Err("missing tool_name for shell_command".to_string()); } if !lower.contains(&format!("decision={expected_decision}")) { return Err(format!("unexpected decision for {call_id}")); @@ -1184,16 +1117,12 @@ fn tool_decision_assertion<'a>( #[tokio::test] #[traced_test] -async fn handle_container_exec_autoapprove_from_config_records_tool_decision() { +async fn handle_shell_command_autoapprove_from_config_records_tool_decision() { let server = start_mock_server().await; mount_sse_once( &server, sse(vec![ - ev_local_shell_call( - "auto_config_call", - "completed", - vec!["/bin/echo", "local shell"], - ), + shell_command_call("auto_config_call", "echo local shell"), ev_completed("done"), ]), ) @@ -1202,7 +1131,7 @@ async fn handle_container_exec_autoapprove_from_config_records_tool_decision() { mount_sse_once( &server, sse(vec![ - ev_assistant_message("msg-1", "local shell done"), + ev_assistant_message("msg-1", "shell command done"), ev_completed("done"), ]), ) @@ -1244,16 +1173,13 @@ async fn handle_container_exec_autoapprove_from_config_records_tool_decision() { #[tokio::test] #[traced_test] -async fn handle_container_exec_user_approved_records_tool_decision() { +async fn handle_shell_command_user_approved_records_tool_decision() { let server = start_mock_server().await; + let command = touch_command("codex-otel-approval-test"); mount_sse_once( &server, sse(vec![ - ev_local_shell_call( - "user_approved_call", - "completed", - vec!["/usr/bin/touch", "codex-otel-approval-test"], - ), + shell_command_call("user_approved_call", &command), ev_completed("done"), ]), ) @@ -1262,7 +1188,7 @@ async fn handle_container_exec_user_approved_records_tool_decision() { mount_sse_once( &server, sse(vec![ - ev_assistant_message("msg-1", "local shell done"), + ev_assistant_message("msg-1", "shell command done"), ev_completed("done"), ]), ) @@ -1316,17 +1242,14 @@ async fn handle_container_exec_user_approved_records_tool_decision() { #[tokio::test] #[traced_test] -async fn handle_container_exec_user_approved_for_session_records_tool_decision() { +async fn handle_shell_command_user_approved_for_session_records_tool_decision() { let server = start_mock_server().await; + let command = touch_command("codex-otel-approval-test"); mount_sse_once( &server, sse(vec![ - ev_local_shell_call( - "user_approved_session_call", - "completed", - vec!["/usr/bin/touch", "codex-otel-approval-test"], - ), + shell_command_call("user_approved_session_call", &command), ev_completed("done"), ]), ) @@ -1334,7 +1257,7 @@ async fn handle_container_exec_user_approved_for_session_records_tool_decision() mount_sse_once( &server, sse(vec![ - ev_assistant_message("msg-1", "local shell done"), + ev_assistant_message("msg-1", "shell command done"), ev_completed("done"), ]), ) @@ -1390,15 +1313,12 @@ async fn handle_container_exec_user_approved_for_session_records_tool_decision() #[traced_test] async fn handle_sandbox_error_user_approves_retry_records_tool_decision() { let server = start_mock_server().await; + let command = touch_command("codex-otel-approval-test"); mount_sse_once( &server, sse(vec![ - ev_local_shell_call( - "sandbox_retry_call", - "completed", - vec!["/usr/bin/touch", "codex-otel-approval-test"], - ), + shell_command_call("sandbox_retry_call", &command), ev_completed("done"), ]), ) @@ -1406,7 +1326,7 @@ async fn handle_sandbox_error_user_approves_retry_records_tool_decision() { mount_sse_once( &server, sse(vec![ - ev_assistant_message("msg-1", "local shell done"), + ev_assistant_message("msg-1", "shell command done"), ev_completed("done"), ]), ) @@ -1460,17 +1380,14 @@ async fn handle_sandbox_error_user_approves_retry_records_tool_decision() { #[tokio::test] #[traced_test] -async fn handle_container_exec_user_denies_records_tool_decision() { +async fn handle_shell_command_user_denies_records_tool_decision() { let server = start_mock_server().await; + let command = touch_command("codex-otel-approval-test"); mount_sse_once( &server, sse(vec![ - ev_local_shell_call( - "user_denied_call", - "completed", - vec!["/usr/bin/touch", "codex-otel-approval-test"], - ), + shell_command_call("user_denied_call", &command), ev_completed("done"), ]), ) @@ -1479,7 +1396,7 @@ async fn handle_container_exec_user_denies_records_tool_decision() { mount_sse_once( &server, sse(vec![ - ev_assistant_message("msg-1", "local shell done"), + ev_assistant_message("msg-1", "shell command done"), ev_completed("done"), ]), ) @@ -1534,15 +1451,12 @@ async fn handle_container_exec_user_denies_records_tool_decision() { #[traced_test] async fn handle_sandbox_error_user_approves_for_session_records_tool_decision() { let server = start_mock_server().await; + let command = touch_command("codex-otel-approval-test"); mount_sse_once( &server, sse(vec![ - ev_local_shell_call( - "sandbox_session_call", - "completed", - vec!["/usr/bin/touch", "codex-otel-approval-test"], - ), + shell_command_call("sandbox_session_call", &command), ev_completed("done"), ]), ) @@ -1550,7 +1464,7 @@ async fn handle_sandbox_error_user_approves_for_session_records_tool_decision() mount_sse_once( &server, sse(vec![ - ev_assistant_message("msg-1", "local shell done"), + ev_assistant_message("msg-1", "shell command done"), ev_completed("done"), ]), ) @@ -1606,15 +1520,12 @@ async fn handle_sandbox_error_user_approves_for_session_records_tool_decision() #[traced_test] async fn handle_sandbox_error_user_denies_records_tool_decision() { let server = start_mock_server().await; + let command = touch_command("codex-otel-approval-test"); mount_sse_once( &server, sse(vec![ - ev_local_shell_call( - "sandbox_deny_call", - "completed", - vec!["/usr/bin/touch", "codex-otel-approval-test"], - ), + shell_command_call("sandbox_deny_call", &command), ev_completed("done"), ]), ) @@ -1623,7 +1534,7 @@ async fn handle_sandbox_error_user_denies_records_tool_decision() { mount_sse_once( &server, sse(vec![ - ev_assistant_message("msg-1", "local shell done"), + ev_assistant_message("msg-1", "shell command done"), ev_completed("done"), ]), ) diff --git a/codex-rs/core/tests/suite/shell_serialization.rs b/codex-rs/core/tests/suite/shell_serialization.rs index 01b1563a6b..56afa59b73 100644 --- a/codex-rs/core/tests/suite/shell_serialization.rs +++ b/codex-rs/core/tests/suite/shell_serialization.rs @@ -7,7 +7,6 @@ use core_test_support::assert_regex_match; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; -use core_test_support::responses::ev_local_shell_call; use core_test_support::responses::ev_response_created; use core_test_support::responses::mount_sse_sequence; use core_test_support::responses::sse; @@ -67,34 +66,6 @@ fn shell_responses( ]), ]) } - ShellModelOutput::Shell => { - let parameters = json!({ - "command": command, - "timeout_ms": 2_000, - }); - Ok(vec![ - sse(vec![ - ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(¶meters)?), - ev_completed("resp-1"), - ]), - sse(vec![ - ev_assistant_message("msg-1", "done"), - ev_completed("resp-2"), - ]), - ]) - } - ShellModelOutput::LocalShell => Ok(vec![ - sse(vec![ - ev_response_created("resp-1"), - ev_local_shell_call(call_id, "completed", command), - ev_completed("resp-1"), - ]), - sse(vec![ - ev_assistant_message("msg-1", "done"), - ev_completed("resp-2"), - ]), - ]), } } @@ -103,12 +74,8 @@ fn configure_shell_model( output_type: ShellModelOutput, include_apply_patch_tool: bool, ) -> TestCodexBuilder { - let builder = match (output_type, include_apply_patch_tool) { - (ShellModelOutput::ShellCommand, _) => builder.with_model("test-gpt-5-codex"), - (ShellModelOutput::LocalShell, true) => builder.with_model("gpt-5.4"), - (ShellModelOutput::Shell, true) => builder.with_model("gpt-5.4"), - (ShellModelOutput::LocalShell, false) => builder.with_model("test-local-shell-json"), - (ShellModelOutput::Shell, false) => builder.with_model("test-shell-json"), + let builder = match output_type { + ShellModelOutput::ShellCommand => builder.with_model("test-gpt-5-codex"), }; builder.with_config(move |config| { @@ -117,64 +84,7 @@ fn configure_shell_model( } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -#[test_case(ShellModelOutput::Shell)] -#[test_case(ShellModelOutput::LocalShell)] -async fn shell_output_stays_json_without_freeform_apply_patch( - output_type: ShellModelOutput, -) -> Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let mut builder = configure_shell_model( - test_codex(), - output_type, - /*include_apply_patch_tool*/ false, - ); - let test = builder.build(&server).await?; - - let call_id = "shell-json"; - let responses = shell_responses(call_id, vec!["/bin/echo", "shell json"], output_type)?; - let mock = mount_sse_sequence(&server, responses).await; - - test.submit_turn_with_permission_profile( - "run the json shell command", - PermissionProfile::Disabled, - ) - .await?; - - let req = mock.last_request().expect("shell output request recorded"); - let output_item = req.function_call_output(call_id); - let output = output_item - .get("output") - .and_then(Value::as_str) - .expect("shell output string"); - - let mut parsed: Value = serde_json::from_str(output)?; - if let Some(metadata) = parsed.get_mut("metadata").and_then(Value::as_object_mut) { - let _ = metadata.remove("duration_seconds"); - } - - assert_eq!( - parsed - .get("metadata") - .and_then(|metadata| metadata.get("exit_code")) - .and_then(Value::as_i64), - Some(0), - "expected zero exit code in unformatted JSON output", - ); - let stdout = parsed - .get("output") - .and_then(Value::as_str) - .unwrap_or_default(); - assert_regex_match(r"(?s)^shell json\n?$", stdout); - - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -#[test_case(ShellModelOutput::Shell)] #[test_case(ShellModelOutput::ShellCommand)] -#[test_case(ShellModelOutput::LocalShell)] async fn shell_output_is_structured_with_freeform_apply_patch( output_type: ShellModelOutput, ) -> Result<()> { @@ -222,76 +132,7 @@ freeform shell } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -#[test_case(ShellModelOutput::Shell)] -#[test_case(ShellModelOutput::LocalShell)] -async fn shell_output_preserves_fixture_json_without_serialization( - output_type: ShellModelOutput, -) -> Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let mut builder = configure_shell_model( - test_codex(), - output_type, - /*include_apply_patch_tool*/ false, - ); - let test = builder.build(&server).await?; - - let fixture_path = test.cwd.path().join("fixture.json"); - fs::write(&fixture_path, FIXTURE_JSON)?; - let fixture_path_str = fixture_path.to_string_lossy().to_string(); - - let call_id = "shell-json-fixture"; - let responses = shell_responses( - call_id, - vec!["/usr/bin/sed", "-n", "p", fixture_path_str.as_str()], - output_type, - )?; - let mock = mount_sse_sequence(&server, responses).await; - - test.submit_turn_with_permission_profile( - "read the fixture JSON with sed", - PermissionProfile::Disabled, - ) - .await?; - - let req = mock.last_request().expect("shell output request recorded"); - let output_item = req.function_call_output(call_id); - let output = output_item - .get("output") - .and_then(Value::as_str) - .expect("shell output string"); - - let mut parsed: Value = serde_json::from_str(output)?; - if let Some(metadata) = parsed.get_mut("metadata").and_then(Value::as_object_mut) { - let _ = metadata.remove("duration_seconds"); - } - - assert_eq!( - parsed - .get("metadata") - .and_then(|metadata| metadata.get("exit_code")) - .and_then(Value::as_i64), - Some(0), - "expected zero exit code when serialization is disabled", - ); - let stdout = parsed - .get("output") - .and_then(Value::as_str) - .unwrap_or_default() - .to_string(); - assert_eq!( - stdout, FIXTURE_JSON, - "expected shell output to match the fixture contents" - ); - - Ok(()) -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -#[test_case(ShellModelOutput::Shell)] #[test_case(ShellModelOutput::ShellCommand)] -#[test_case(ShellModelOutput::LocalShell)] async fn shell_output_structures_fixture_with_serialization( output_type: ShellModelOutput, ) -> Result<()> { @@ -352,9 +193,7 @@ async fn shell_output_structures_fixture_with_serialization( } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -#[test_case(ShellModelOutput::Shell)] #[test_case(ShellModelOutput::ShellCommand)] -#[test_case(ShellModelOutput::LocalShell)] async fn shell_output_for_freeform_tool_records_duration( output_type: ShellModelOutput, ) -> Result<()> { @@ -408,72 +247,8 @@ $"#; Ok(()) } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -#[test_case(ShellModelOutput::Shell)] -#[test_case(ShellModelOutput::LocalShell)] -async fn shell_output_reserializes_truncated_content(output_type: ShellModelOutput) -> Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let mut builder = configure_shell_model( - test_codex(), - output_type, - /*include_apply_patch_tool*/ true, - ) - .with_config(move |config| { - config.tool_output_token_limit = Some(200); - }); - let test = builder.build(&server).await?; - - let call_id = "shell-truncated"; - let responses = shell_responses(call_id, vec!["/bin/sh", "-c", "seq 1 400"], output_type)?; - let mock = mount_sse_sequence(&server, responses).await; - - test.submit_turn_with_permission_profile( - "run the truncation shell command", - PermissionProfile::Disabled, - ) - .await?; - - let req = mock - .last_request() - .expect("truncated output request recorded"); - let output_item = req.function_call_output(call_id); - let output = output_item - .get("output") - .and_then(Value::as_str) - .expect("truncated output string"); - - assert!( - serde_json::from_str::(output).is_err(), - "expected truncated shell output to be plain text", - ); - let truncated_pattern = r#"(?s)^Exit code: 0 -Wall time: [0-9]+(?:\.[0-9]+)? seconds -Total output lines: 400 -Output: -1 -2 -3 -4 -5 -6 -.*…46 tokens truncated….* -396 -397 -398 -399 -400 -$"#; - assert_regex_match(truncated_pattern, output); - - Ok(()) -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] async fn apply_patch_custom_tool_output_is_structured( output_type: ApplyPatchModelOutput, ) -> Result<()> { @@ -517,8 +292,6 @@ A {file_name} #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] async fn apply_patch_custom_tool_call_creates_file( output_type: ApplyPatchModelOutput, ) -> Result<()> { @@ -564,8 +337,6 @@ A {file_name} #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] async fn apply_patch_custom_tool_call_updates_existing_file( output_type: ApplyPatchModelOutput, ) -> Result<()> { @@ -616,8 +387,6 @@ M {file_name} #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] async fn apply_patch_custom_tool_call_reports_failure_output( output_type: ApplyPatchModelOutput, ) -> Result<()> { @@ -660,8 +429,6 @@ async fn apply_patch_custom_tool_call_reports_failure_output( #[tokio::test(flavor = "multi_thread", worker_threads = 2)] #[test_case(ApplyPatchModelOutput::Freeform)] -#[test_case(ApplyPatchModelOutput::Shell)] -#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)] async fn apply_patch_tool_output_is_structured(output_type: ApplyPatchModelOutput) -> Result<()> { skip_if_no_network!(Ok(())); @@ -702,9 +469,7 @@ A {file_name} } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -#[test_case(ShellModelOutput::Shell)] #[test_case(ShellModelOutput::ShellCommand)] -#[test_case(ShellModelOutput::LocalShell)] async fn shell_output_is_structured_for_nonzero_exit(output_type: ShellModelOutput) -> Result<()> { skip_if_no_network!(Ok(())); @@ -897,52 +662,3 @@ Output: Ok(()) } - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn local_shell_call_output_is_structured() -> Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let mut builder = test_codex().with_model("gpt-5.4").with_config(|config| { - config.include_apply_patch_tool = true; - }); - let test = builder.build(&server).await?; - - let call_id = "local-shell-call"; - let responses = vec![ - sse(vec![ - json!({"type": "response.created", "response": {"id": "resp-1"}}), - ev_local_shell_call(call_id, "completed", vec!["/bin/echo", "local shell"]), - ev_completed("resp-1"), - ]), - sse(vec![ - ev_assistant_message("msg-1", "local shell done"), - ev_completed("resp-2"), - ]), - ]; - let mock = mount_sse_sequence(&server, responses).await; - - test.submit_turn_with_permission_profile( - "run the local shell command", - PermissionProfile::Disabled, - ) - .await?; - - let req = mock - .last_request() - .expect("local shell output request recorded"); - let output_item = req.function_call_output(call_id); - let output = output_item - .get("output") - .and_then(Value::as_str) - .expect("local shell output string"); - - let expected_pattern = r"(?s)^Exit code: 0 -Wall time: [0-9]+(?:\.[0-9]+)? seconds -Output: -local shell -?$"; - assert_regex_match(expected_pattern, output); - - Ok(()) -} diff --git a/codex-rs/core/tests/suite/snapshots/all__suite__pending_input__pending_input_user_input_no_preempt_after_reasoning.snap b/codex-rs/core/tests/suite/snapshots/all__suite__pending_input__pending_input_user_input_no_preempt_after_reasoning.snap index 724efd706a..d6bd5a7b22 100644 --- a/codex-rs/core/tests/suite/snapshots/all__suite__pending_input__pending_input_user_input_no_preempt_after_reasoning.snap +++ b/codex-rs/core/tests/suite/snapshots/all__suite__pending_input__pending_input_user_input_no_preempt_after_reasoning.snap @@ -16,5 +16,5 @@ Scenario: /responses POST bodies (input only, redacted like other suite snapshot 03:reasoning:summary=thinking:encrypted=true 04:function_call/shell 05:message/assistant:first answer -06:function_call_output:failed to parse function arguments: invalid type: string "echo preserved tool call", expected a sequence at line 1 column 37 +06:function_call_output:unsupported call: shell 07:message/user:second prompt diff --git a/codex-rs/core/tests/suite/tool_harness.rs b/codex-rs/core/tests/suite/tool_harness.rs index a2017c99b3..ac41d06c3f 100644 --- a/codex-rs/core/tests/suite/tool_harness.rs +++ b/codex-rs/core/tests/suite/tool_harness.rs @@ -18,7 +18,6 @@ use core_test_support::responses::ev_apply_patch_custom_tool_call; use core_test_support::responses::ev_assistant_message; use core_test_support::responses::ev_completed; use core_test_support::responses::ev_function_call; -use core_test_support::responses::ev_local_shell_call; use core_test_support::responses::ev_response_created; use core_test_support::responses::sse; use core_test_support::responses::start_mock_server; @@ -66,12 +65,12 @@ fn custom_call_output(req: &ResponsesRequest, call_id: &str) -> (String, Option< } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn shell_tool_executes_command_and_streams_output() -> anyhow::Result<()> { +async fn shell_command_tool_executes_command_and_streams_output() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; - let mut builder = test_codex().with_model("test-local-shell-json"); + let mut builder = test_codex().with_model("test-gpt-5-codex"); let TestCodex { codex, cwd, @@ -79,11 +78,15 @@ async fn shell_tool_executes_command_and_streams_output() -> anyhow::Result<()> .. } = builder.build(&server).await?; - let call_id = "shell-tool-call"; - let command = vec!["/bin/echo", "tool harness"]; + let call_id = "shell-command-tool-call"; + let command_args = json!({ + "command": "echo tool harness", + "login": false, + }) + .to_string(); let first_response = sse(vec![ ev_response_created("resp-1"), - ev_local_shell_call(call_id, "completed", command), + ev_function_call(call_id, "shell_command", &command_args), ev_completed("resp-1"), ]); responses::mount_sse_once(&server, first_response).await; diff --git a/codex-rs/core/tests/suite/tools.rs b/codex-rs/core/tests/suite/tools.rs index 737ea122d4..0f46e50386 100644 --- a/codex-rs/core/tests/suite/tools.rs +++ b/codex-rs/core/tests/suite/tools.rs @@ -182,24 +182,26 @@ async fn custom_tool_unknown_returns_custom_output_error() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn shell_escalated_permissions_rejected_then_ok() -> Result<()> { +async fn shell_command_escalated_permissions_rejected_then_ok() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; - let mut builder = test_codex().with_model("test-shell-json"); + let mut builder = test_codex().with_model("test-gpt-5-codex"); let test = builder.build(&server).await?; - let command = ["/bin/echo", "shell ok"]; - let call_id_blocked = "shell-blocked"; - let call_id_success = "shell-success"; + let command = "echo shell ok"; + let call_id_blocked = "shell-command-blocked"; + let call_id_success = "shell-command-success"; let first_args = json!({ "command": command, + "login": false, "timeout_ms": 1_000, "sandbox_permissions": SandboxPermissions::RequireEscalated, }); let second_args = json!({ "command": command, + "login": false, "timeout_ms": 1_000, }); @@ -209,7 +211,7 @@ async fn shell_escalated_permissions_rejected_then_ok() -> Result<()> { ev_response_created("resp-1"), ev_function_call( call_id_blocked, - "shell", + "shell_command", &serde_json::to_string(&first_args)?, ), ev_completed("resp-1"), @@ -222,7 +224,7 @@ async fn shell_escalated_permissions_rejected_then_ok() -> Result<()> { ev_response_created("resp-2"), ev_function_call( call_id_success, - "shell", + "shell_command", &serde_json::to_string(&second_args)?, ), ev_completed("resp-2"), @@ -239,7 +241,7 @@ async fn shell_escalated_permissions_rejected_then_ok() -> Result<()> { .await; test.submit_turn_with_approval_and_permission_profile( - "run the shell command", + "run the shell_command script", AskForApproval::Never, PermissionProfile::Disabled, ) @@ -274,35 +276,32 @@ async fn shell_escalated_permissions_rejected_then_ok() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn sandbox_denied_shell_returns_original_output() -> Result<()> { +async fn sandbox_denied_shell_command_returns_original_output() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; let mut builder = test_codex().with_model("gpt-5.4"); let fixture = builder.build(&server).await?; - let call_id = "sandbox-denied-shell"; + let call_id = "sandbox-denied-shell-command"; let target_path = fixture.workspace_path("sandbox-denied.txt"); let sentinel = "sandbox-denied sentinel output"; - let command = vec![ - "/bin/sh".to_string(), - "-c".to_string(), - format!( - "printf {sentinel:?}; printf {content:?} > {path:?}", - sentinel = format!("{sentinel}\n"), - content = "sandbox denied", - path = &target_path - ), - ]; + let command = format!( + "printf {sentinel:?}; printf {content:?} > {path:?}", + sentinel = format!("{sentinel}\n"), + content = "sandbox denied", + path = &target_path + ); let args = json!({ "command": command, + "login": false, "timeout_ms": 5_000, }); let responses = vec![ sse(vec![ ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), ev_completed("resp-1"), ]), sse(vec![ @@ -367,7 +366,7 @@ async fn sandbox_denied_shell_returns_original_output() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn shell_enforces_glob_deny_read_policy() -> Result<()> { +async fn shell_command_enforces_glob_deny_read_policy() -> Result<()> { skip_if_no_network!(Ok(())); skip_if_sandbox!(Ok(())); @@ -403,24 +402,22 @@ async fn shell_enforces_glob_deny_read_policy() -> Result<()> { fs::write(&denied_path, format!("{secret}\n")).context("write denied fixture")?; fs::write(&allowed_path, format!("{allowed}\n")).context("write allowed fixture")?; - let call_id = "shell-glob-deny-read"; - let command = vec![ - "/bin/sh".to_string(), - "-c".to_string(), - "status=0; cat \"$1\" || status=$?; cat \"$2\"; exit \"$status\"".to_string(), - "sh".to_string(), - denied_path.to_string_lossy().into_owned(), - allowed_path.to_string_lossy().into_owned(), - ]; + let call_id = "shell-command-glob-deny-read"; + let command = format!( + "rc=0; cat {denied_path:?} || rc=$?; cat {allowed_path:?}; exit \"$rc\"", + denied_path = denied_path.to_string_lossy(), + allowed_path = allowed_path.to_string_lossy(), + ); let args = json!({ "command": command, + "login": false, "timeout_ms": 1_000, }); let responses = vec![ sse(vec![ ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), ev_completed("resp-1"), ]), sse(vec![ @@ -537,17 +534,18 @@ async fn unified_exec_spec_toggle_end_to_end() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn shell_timeout_includes_timeout_prefix_and_metadata() -> Result<()> { +async fn shell_command_timeout_includes_timeout_prefix_and_metadata() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; - let mut builder = test_codex().with_model("test-shell-json"); + let mut builder = test_codex().with_model("test-gpt-5-codex"); let test = builder.build(&server).await?; - let call_id = "shell-timeout"; + let call_id = "shell-command-timeout"; let timeout_ms = 50u64; let args = json!({ - "command": ["/bin/sh", "-c", "yes line | head -n 400; sleep 1"], + "command": "yes line | head -n 400; sleep 1", + "login": false, "timeout_ms": timeout_ms, }); @@ -555,7 +553,7 @@ async fn shell_timeout_includes_timeout_prefix_and_metadata() -> Result<()> { &server, sse(vec![ ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), ev_completed("resp-1"), ]), ) @@ -622,7 +620,7 @@ async fn shell_timeout_includes_timeout_prefix_and_metadata() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn shell_timeout_handles_background_grandchild_stdout() -> Result<()> { +async fn shell_command_timeout_handles_background_grandchild_stdout() -> Result<()> { skip_if_no_network!(Ok(())); let server = start_mock_server().await; @@ -634,7 +632,7 @@ async fn shell_timeout_handles_background_grandchild_stdout() -> Result<()> { }); let test = builder.build(&server).await?; - let call_id = "shell-grandchild-timeout"; + let call_id = "shell-command-grandchild-timeout"; let pid_path = test.cwd.path().join("grandchild_pid.txt"); let script_path = test.cwd.path().join("spawn_detached.py"); let script = format!( @@ -651,7 +649,8 @@ time.sleep(60) fs::write(&script_path, script)?; let args = json!({ - "command": ["python3", script_path.to_string_lossy()], + "command": format!("python3 {:?}", script_path.to_string_lossy()), + "login": false, "timeout_ms": 200, }); @@ -659,7 +658,7 @@ time.sleep(60) &server, sse(vec![ ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), + ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?), ev_completed("resp-1"), ]), ) @@ -716,82 +715,3 @@ time.sleep(60) Ok(()) } - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn shell_spawn_failure_truncates_exec_error() -> Result<()> { - skip_if_no_network!(Ok(())); - - let server = start_mock_server().await; - let mut builder = test_codex().with_config(|cfg| { - cfg.permissions - .set_permission_profile(PermissionProfile::Disabled) - .expect("set permission profile"); - }); - let test = builder.build(&server).await?; - - let call_id = "shell-spawn-failure"; - let bogus_component = "missing-bin-".repeat(700); - let bogus_exe = test - .cwd - .path() - .join(bogus_component) - .to_string_lossy() - .into_owned(); - - let args = json!({ - "command": [bogus_exe], - "timeout_ms": 1_000, - }); - - mount_sse_once( - &server, - sse(vec![ - ev_response_created("resp-1"), - ev_function_call(call_id, "shell", &serde_json::to_string(&args)?), - ev_completed("resp-1"), - ]), - ) - .await; - let second_mock = mount_sse_once( - &server, - sse(vec![ - ev_assistant_message("msg-1", "done"), - ev_completed("resp-2"), - ]), - ) - .await; - - test.submit_turn_with_approval_and_permission_profile( - "spawn a missing binary", - AskForApproval::Never, - PermissionProfile::Disabled, - ) - .await?; - - let failure_item = second_mock.single_request().function_call_output(call_id); - - let output = failure_item - .get("output") - .and_then(Value::as_str) - .expect("spawn failure output string"); - - let spawn_error_pattern = r#"(?s)^Exit code: -?\d+ -Wall time: [0-9]+(?:\.[0-9]+)? seconds -Output: -execution error: .*$"#; - let spawn_truncated_pattern = r#"(?s)^Exit code: -?\d+ -Wall time: [0-9]+(?:\.[0-9]+)? seconds -Total output lines: \d+ -Output: - -execution error: .*$"#; - let spawn_error_regex = Regex::new(spawn_error_pattern)?; - let spawn_truncated_regex = Regex::new(spawn_truncated_pattern)?; - if !spawn_error_regex.is_match(output) && !spawn_truncated_regex.is_match(output) { - let fallback_pattern = r"(?s)^execution error: .*$"; - assert_regex_match(fallback_pattern, output); - } - assert!(output.len() <= 10 * 1024); - - Ok(()) -} diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 5a0fafad94..13a4f6e009 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -1253,30 +1253,6 @@ pub struct SearchToolCallParams { pub limit: Option, } -/// If the `name` of a `ResponseItem::FunctionCall` is either `container.exec` -/// or `shell`, the `arguments` field should deserialize to this struct. -#[derive(Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] -pub struct ShellToolCallParams { - pub command: Vec, - pub workdir: Option, - - /// This is the maximum time in milliseconds that the command is allowed to run. - #[serde(alias = "timeout")] - pub timeout_ms: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] - pub sandbox_permissions: Option, - /// Suggests a command prefix to persist for future sessions - #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] - pub prefix_rule: Option>, - #[serde(default, skip_serializing_if = "Option::is_none")] - #[ts(optional)] - pub additional_permissions: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub justification: Option, -} - /// If the `name` of a `ResponseItem::FunctionCall` is `shell_command`, the /// `arguments` field should deserialize to this struct. #[derive(Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] @@ -2545,30 +2521,6 @@ mod tests { Ok(()) } - #[test] - fn deserialize_shell_tool_call_params() -> Result<()> { - let json = r#"{ - "command": ["ls", "-l"], - "workdir": "/tmp", - "timeout": 1000 - }"#; - - let params: ShellToolCallParams = serde_json::from_str(json)?; - assert_eq!( - ShellToolCallParams { - command: vec!["ls".to_string(), "-l".to_string()], - workdir: Some("/tmp".to_string()), - timeout_ms: Some(1000), - sandbox_permissions: None, - prefix_rule: None, - additional_permissions: None, - justification: None, - }, - params - ); - Ok(()) - } - #[test] fn wraps_image_user_input_with_tags() -> Result<()> { let image_url = "data:image/png;base64,abc".to_string(); diff --git a/codex-rs/tools/src/code_mode.rs b/codex-rs/tools/src/code_mode.rs index a0c2173cac..052edbac59 100644 --- a/codex-rs/tools/src/code_mode.rs +++ b/codex-rs/tools/src/code_mode.rs @@ -136,8 +136,7 @@ fn code_mode_tool_definitions_for_spec(spec: &ToolSpec) -> Vec Vec::new(), } diff --git a/codex-rs/tools/src/function_call_error.rs b/codex-rs/tools/src/function_call_error.rs index 3881c7af1b..1ab8335b25 100644 --- a/codex-rs/tools/src/function_call_error.rs +++ b/codex-rs/tools/src/function_call_error.rs @@ -5,8 +5,6 @@ use thiserror::Error; pub enum FunctionCallError { #[error("{0}")] RespondToModel(String), - #[error("LocalShellCall without call_id or id")] - MissingLocalShellCallId, #[error("Fatal error: {0}")] Fatal(String), } diff --git a/codex-rs/tools/src/tool_config.rs b/codex-rs/tools/src/tool_config.rs index 4639b404b1..17e0dccaae 100644 --- a/codex-rs/tools/src/tool_config.rs +++ b/codex-rs/tools/src/tool_config.rs @@ -203,6 +203,9 @@ impl ToolsConfig { ConfigShellToolType::UnifiedExec if !unified_exec_enabled => { ConfigShellToolType::ShellCommand } + ConfigShellToolType::Default | ConfigShellToolType::Local => { + ConfigShellToolType::ShellCommand + } other => other, }; let shell_type = if !features.enabled(Feature::ShellTool) { diff --git a/codex-rs/tools/src/tool_payload.rs b/codex-rs/tools/src/tool_payload.rs index b335e58372..71af457743 100644 --- a/codex-rs/tools/src/tool_payload.rs +++ b/codex-rs/tools/src/tool_payload.rs @@ -1,7 +1,6 @@ use std::borrow::Cow; use codex_protocol::models::SearchToolCallParams; -use codex_protocol::models::ShellToolCallParams; /// Canonical payload shapes accepted by model-visible tool runtimes. #[derive(Clone, Debug)] @@ -9,7 +8,6 @@ pub enum ToolPayload { Function { arguments: String }, ToolSearch { arguments: SearchToolCallParams }, Custom { input: String }, - LocalShell { params: ShellToolCallParams }, } impl ToolPayload { @@ -18,7 +16,6 @@ impl ToolPayload { ToolPayload::Function { arguments } => Cow::Borrowed(arguments), ToolPayload::ToolSearch { arguments } => Cow::Owned(arguments.query.clone()), ToolPayload::Custom { input } => Cow::Borrowed(input), - ToolPayload::LocalShell { params } => Cow::Owned(params.command.join(" ")), } } } diff --git a/codex-rs/tools/src/tool_spec.rs b/codex-rs/tools/src/tool_spec.rs index 2a0183a905..98f705363c 100644 --- a/codex-rs/tools/src/tool_spec.rs +++ b/codex-rs/tools/src/tool_spec.rs @@ -25,8 +25,6 @@ pub enum ToolSpec { description: String, parameters: JsonSchema, }, - #[serde(rename = "local_shell")] - LocalShell {}, #[serde(rename = "image_generation")] ImageGeneration { output_format: String }, // TODO: Understand why we get an error on web_search although the API docs @@ -58,7 +56,6 @@ impl ToolSpec { ToolSpec::Function(tool) => tool.name.as_str(), ToolSpec::Namespace(namespace) => namespace.name.as_str(), ToolSpec::ToolSearch { .. } => "tool_search", - ToolSpec::LocalShell {} => "local_shell", ToolSpec::ImageGeneration { .. } => "image_generation", ToolSpec::WebSearch { .. } => "web_search", ToolSpec::Freeform(tool) => tool.name.as_str(), diff --git a/codex-rs/tools/src/tool_spec_tests.rs b/codex-rs/tools/src/tool_spec_tests.rs index 6d2017e553..a181cd247b 100644 --- a/codex-rs/tools/src/tool_spec_tests.rs +++ b/codex-rs/tools/src/tool_spec_tests.rs @@ -57,7 +57,6 @@ fn tool_spec_name_covers_all_variants() { .name(), "tool_search" ); - assert_eq!(ToolSpec::LocalShell {}.name(), "local_shell"); assert_eq!( ToolSpec::ImageGeneration { output_format: "png".to_string(), diff --git a/sdk/typescript/tests/abort.test.ts b/sdk/typescript/tests/abort.test.ts index 0af318272b..ca93b1e89d 100644 --- a/sdk/typescript/tests/abort.test.ts +++ b/sdk/typescript/tests/abort.test.ts @@ -127,7 +127,7 @@ describe("AbortSignal support", () => { void event; // Consume the event eventCount++; // Abort after first event - if (eventCount === 5) { + if (eventCount === 1) { controller.abort("Aborted during iteration"); } // Continue iterating - should eventually throw diff --git a/sdk/typescript/tests/responsesProxy.ts b/sdk/typescript/tests/responsesProxy.ts index e9d3b29146..012cbf2320 100644 --- a/sdk/typescript/tests/responsesProxy.ts +++ b/sdk/typescript/tests/responsesProxy.ts @@ -181,15 +181,14 @@ export function assistantMessage(text: string, itemId: string = DEFAULT_MESSAGE_ } export function shell_call(): SseEvent { - const command = ["bash", "-lc", "echo 'Hello, world!'"]; return { type: "response.output_item.done", item: { type: "function_call", call_id: `call_id${Math.random().toString(36).slice(2)}`, - name: "shell", + name: "shell_command", arguments: JSON.stringify({ - command, + command: "echo 'Hello, world!'", timeout_ms: 100, }), },