From 0d848f3cc92d8eb0bfcbf4ee7621d3745ca45a46 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 11 Nov 2025 09:04:05 -0800 Subject: [PATCH] feat: shell_command tool --- codex-rs/Cargo.lock | 10 +- codex-rs/core/src/bash.rs | 29 +++++ codex-rs/core/src/features.rs | 9 ++ codex-rs/core/src/tools/handlers/mod.rs | 1 + codex-rs/core/src/tools/handlers/shell.rs | 138 ++++++++++++++++++++++ codex-rs/core/src/tools/spec.rs | 96 +++++++++++++++ codex-rs/protocol/src/models.rs | 16 +++ 7 files changed, 294 insertions(+), 5 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index cf5f961a62..8794f54af0 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4450,7 +4450,7 @@ checksum = "3af6b589e163c5a788fab00ce0c0366f6efbb9959c2f9874b224936af7fce7e1" dependencies = [ "base64", "indexmap 2.12.0", - "quick-xml", + "quick-xml 0.38.0", "serde", "time", ] @@ -7093,7 +7093,7 @@ version = "0.31.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c66a47e840dc20793f2264eb4b3e4ecb4b75d91c0dd4af04b456128e0bdd449d" dependencies = [ - "bitflags 2.9.1", + "bitflags 2.10.0", "rustix 1.0.8", "wayland-backend", "wayland-scanner", @@ -7105,7 +7105,7 @@ version = "0.32.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "efa790ed75fbfd71283bd2521a1cfdc022aabcc28bdcff00851f9e4ae88d9901" dependencies = [ - "bitflags 2.9.1", + "bitflags 2.10.0", "wayland-backend", "wayland-client", "wayland-scanner", @@ -7117,7 +7117,7 @@ version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "efd94963ed43cf9938a090ca4f7da58eb55325ec8200c3848963e98dc25b78ec" dependencies = [ - "bitflags 2.9.1", + "bitflags 2.10.0", "wayland-backend", "wayland-client", "wayland-protocols", @@ -7726,7 +7726,7 @@ dependencies = [ "os_pipe", "rustix 0.38.44", "tempfile", - "thiserror 2.0.16", + "thiserror 2.0.17", "tree_magic_mini", "wayland-backend", "wayland-client", diff --git a/codex-rs/core/src/bash.rs b/codex-rs/core/src/bash.rs index e85ce76a5e..7232cacd1b 100644 --- a/codex-rs/core/src/bash.rs +++ b/codex-rs/core/src/bash.rs @@ -120,6 +120,20 @@ pub fn parse_shell_lc_plain_commands(command: &[String]) -> Option Option { + let tree = try_parse_shell(command)?; + let commands = try_parse_word_only_commands_sequence(&tree, command)?; + let [command_words] = commands.as_slice() else { + return None; + }; + let (_, script) = extract_bash_command(command_words)?; + Some(script.to_string()) +} + fn parse_plain_command_from_node(cmd: tree_sitter::Node, src: &str) -> Option> { if cmd.kind() != "command" { return None; @@ -258,4 +272,19 @@ mod tests { let parsed = parse_shell_lc_plain_commands(&command).unwrap(); assert_eq!(parsed, vec![vec!["ls".to_string()]]); } + + #[test] + fn extracts_bash_lc_script_from_string() { + let script = extract_shell_lc_script_from_str(r#"bash -lc "ls -a""#).unwrap(); + assert_eq!(script, "ls -a"); + + let zsh_script = extract_shell_lc_script_from_str("zsh -lc 'echo hello && pwd'").unwrap(); + assert_eq!(zsh_script, "echo hello && pwd"); + } + + #[test] + fn ignores_non_bash_lc_strings() { + assert!(extract_shell_lc_script_from_str("echo hi").is_none()); + assert!(extract_shell_lc_script_from_str("bash -lc 'ls'; echo hi").is_none()); + } } diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index e34128f362..8a399d6552 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -29,6 +29,9 @@ pub enum Stage { pub enum Feature { /// Use the single unified PTY-backed exec tool. UnifiedExec, + /// Use the shell command tool that takes `command` as a single string of + /// shell instead of an array of args passed to `execvp(3)`. + ShellCommandTool, /// Enable experimental RMCP features such as OAuth login. RmcpClient, /// Include the freeform apply_patch tool. @@ -250,6 +253,12 @@ pub const FEATURES: &[FeatureSpec] = &[ stage: Stage::Experimental, default_enabled: false, }, + FeatureSpec { + id: Feature::ShellCommandTool, + key: "shell_command_tool", + stage: Stage::Experimental, + default_enabled: false, + }, FeatureSpec { id: Feature::RmcpClient, key: "rmcp_client", diff --git a/codex-rs/core/src/tools/handlers/mod.rs b/codex-rs/core/src/tools/handlers/mod.rs index 187b444414..dcf848e376 100644 --- a/codex-rs/core/src/tools/handlers/mod.rs +++ b/codex-rs/core/src/tools/handlers/mod.rs @@ -19,6 +19,7 @@ pub use mcp::McpHandler; pub use mcp_resource::McpResourceHandler; pub use plan::PlanHandler; pub use read_file::ReadFileHandler; +pub use shell::ShellCommandHandler; pub use shell::ShellHandler; pub use test_sync::TestSyncHandler; pub use unified_exec::UnifiedExecHandler; diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index b97242a9a3..1269898a58 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -1,14 +1,17 @@ use async_trait::async_trait; +use codex_protocol::models::ShellCommandToolCallParams; use codex_protocol::models::ShellToolCallParams; use std::sync::Arc; use crate::apply_patch; use crate::apply_patch::InternalApplyPatchInvocation; use crate::apply_patch::convert_apply_patch_to_protocol; +use crate::bash::extract_shell_lc_script_from_str; use crate::codex::TurnContext; use crate::exec::ExecParams; use crate::exec_env::create_env; use crate::function_tool::FunctionCallError; +use crate::shell::Shell; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; @@ -25,6 +28,8 @@ use crate::tools::sandboxing::ToolCtx; pub struct ShellHandler; +pub struct ShellCommandHandler; + impl ShellHandler { fn to_exec_params(params: ShellToolCallParams, turn_context: &TurnContext) -> ExecParams { ExecParams { @@ -39,6 +44,57 @@ impl ShellHandler { } } +impl ShellCommandHandler { + fn to_exec_params( + params: ShellCommandToolCallParams, + session: &crate::codex::Session, + turn_context: &TurnContext, + ) -> ExecParams { + let shell = session.user_shell(); + let script = script_for_shell(shell, params.command.as_str()); + ExecParams { + command: command_for_shell(shell, &script), + cwd: turn_context.resolve_path(params.workdir.clone()), + timeout_ms: params.timeout_ms, + env: create_env(&turn_context.shell_environment_policy), + with_escalated_permissions: params.with_escalated_permissions, + justification: params.justification, + arg0: None, + } + } +} + +fn script_for_shell(shell: &Shell, raw_command: &str) -> String { + if let Some(inner) = extract_shell_lc_script_from_str(raw_command) + && matches!(shell, Shell::Bash(_) | Shell::Zsh(_)) + { + return inner; + } + raw_command.to_string() +} + +fn command_for_shell(shell: &Shell, script: &str) -> Vec { + match shell { + Shell::Zsh(zsh) => vec![ + zsh.shell_path.clone(), + "-lc".to_string(), + script.to_string(), + ], + Shell::Bash(bash) => vec![ + bash.shell_path.clone(), + "-lc".to_string(), + script.to_string(), + ], + Shell::PowerShell(ps) => vec![ + ps.exe.clone(), + "-NoLogo".to_string(), + "-Command".to_string(), + script.to_string(), + ], + Shell::Unknown => shlex::split(script).unwrap_or_else(|| vec![script.to_string()]), + } +} + #[async_trait] impl ToolHandler for ShellHandler { fn kind(&self) -> ToolKind { @@ -102,6 +158,49 @@ impl ToolHandler for ShellHandler { } } +#[async_trait] +impl ToolHandler for ShellCommandHandler { + fn kind(&self) -> ToolKind { + ToolKind::Function + } + + fn matches_kind(&self, payload: &ToolPayload) -> bool { + matches!(payload, ToolPayload::Function { .. }) + } + + async fn handle(&self, invocation: ToolInvocation) -> Result { + let ToolInvocation { + session, + turn, + tracker, + call_id, + tool_name, + payload, + } = invocation; + + let ToolPayload::Function { arguments } = payload else { + return Err(FunctionCallError::RespondToModel(format!( + "unsupported payload for shell_command handler: {tool_name}" + ))); + }; + + let params: ShellCommandToolCallParams = serde_json::from_str(&arguments).map_err(|e| { + FunctionCallError::RespondToModel(format!("failed to parse function arguments: {e:?}")) + })?; + let exec_params = Self::to_exec_params(params, session.as_ref(), turn.as_ref()); + ShellHandler::run_exec_like( + tool_name.as_str(), + exec_params, + session, + turn, + tracker, + call_id, + false, + ) + .await + } +} + impl ShellHandler { async fn run_exec_like( tool_name: &str, @@ -240,3 +339,42 @@ impl ShellHandler { }) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::shell::BashShell; + + #[test] + fn unwraps_nested_bash_lc_for_bash_shell() { + let shell = Shell::Bash(BashShell { + shell_path: "/bin/bash".to_string(), + bashrc_path: "/home/user/.bashrc".to_string(), + }); + let script = script_for_shell(&shell, r#"bash -lc "echo hi""#); + assert_eq!(script, "echo hi"); + + let command = command_for_shell(&shell, &script); + assert_eq!( + command, + vec![ + "/bin/bash".to_string(), + "-lc".to_string(), + "echo hi".to_string() + ] + ); + } + + #[test] + fn keeps_raw_command_for_unknown_shell() { + let shell = Shell::Unknown; + let script = script_for_shell(&shell, r#"bash -lc "echo hi""#); + assert_eq!(script, r#"bash -lc "echo hi""#); + + let command = command_for_shell(&shell, &script); + assert_eq!( + command, + vec!["bash".to_string(), "-lc".to_string(), "echo hi".to_string()] + ); + } +} diff --git a/codex-rs/core/src/tools/spec.rs b/codex-rs/core/src/tools/spec.rs index cb1aeafdb2..4a1de4b04a 100644 --- a/codex-rs/core/src/tools/spec.rs +++ b/codex-rs/core/src/tools/spec.rs @@ -20,6 +20,8 @@ pub enum ConfigShellToolType { Default, Local, UnifiedExec, + /// Takes a command as a single string to be run in the user's default shell. + ShellCommand, } #[derive(Debug, Clone)] @@ -48,6 +50,8 @@ impl ToolsConfig { let shell_type = if features.enabled(Feature::UnifiedExec) { ConfigShellToolType::UnifiedExec + } else if features.enabled(Feature::ShellCommandTool) { + ConfigShellToolType::ShellCommand } else { model_family.shell_type.clone() }; @@ -284,6 +288,53 @@ fn create_shell_tool() -> ToolSpec { }) } +fn create_shell_command_tool() -> ToolSpec { + let mut properties = BTreeMap::new(); + properties.insert( + "command".to_string(), + JsonSchema::String { + description: Some( + "The shell script to execute in the user's default shell".to_string(), + ), + }, + ); + properties.insert( + "workdir".to_string(), + JsonSchema::String { + description: Some("The working directory to execute the command in".to_string()), + }, + ); + properties.insert( + "timeout_ms".to_string(), + JsonSchema::Number { + description: Some("The timeout for the command in milliseconds".to_string()), + }, + ); + properties.insert( + "with_escalated_permissions".to_string(), + JsonSchema::Boolean { + description: Some("Whether to request escalated permissions. Set to true if command needs to be run without sandbox restrictions".to_string()), + }, + ); + properties.insert( + "justification".to_string(), + JsonSchema::String { + description: Some("Only set if with_escalated_permissions is true. 1-sentence explanation of why we want to run this command.".to_string()), + }, + ); + + ToolSpec::Function(ResponsesApiTool { + name: "shell_command".to_string(), + description: "Runs a shell command string and returns its output.".to_string(), + strict: false, + parameters: JsonSchema::Object { + properties, + required: Some(vec!["command".to_string()]), + additional_properties: Some(false.into()), + }, + }) +} + fn create_view_image_tool() -> ToolSpec { // Support only local filesystem path. let mut properties = BTreeMap::new(); @@ -873,6 +924,7 @@ pub(crate) fn build_specs( use crate::tools::handlers::McpResourceHandler; use crate::tools::handlers::PlanHandler; use crate::tools::handlers::ReadFileHandler; + use crate::tools::handlers::ShellCommandHandler; use crate::tools::handlers::ShellHandler; use crate::tools::handlers::TestSyncHandler; use crate::tools::handlers::UnifiedExecHandler; @@ -888,6 +940,7 @@ pub(crate) fn build_specs( let view_image_handler = Arc::new(ViewImageHandler); let mcp_handler = Arc::new(McpHandler); let mcp_resource_handler = Arc::new(McpResourceHandler); + let shell_command_handler = Arc::new(ShellCommandHandler); match &config.shell_type { ConfigShellToolType::Default => { @@ -902,12 +955,16 @@ pub(crate) fn build_specs( builder.register_handler("exec_command", unified_exec_handler.clone()); builder.register_handler("write_stdin", unified_exec_handler); } + ConfigShellToolType::ShellCommand => { + builder.push_spec(create_shell_command_tool()); + } } // Always register shell aliases so older prompts remain compatible. builder.register_handler("shell", shell_handler.clone()); builder.register_handler("container.exec", shell_handler.clone()); builder.register_handler("local_shell", shell_handler); + builder.register_handler("shell_command", shell_command_handler); builder.push_spec_with_parallel_support(create_list_mcp_resources_tool(), true); builder.push_spec_with_parallel_support(create_list_mcp_resource_templates_tool(), true); @@ -1043,6 +1100,7 @@ mod tests { ConfigShellToolType::Default => Some("shell"), ConfigShellToolType::Local => Some("local_shell"), ConfigShellToolType::UnifiedExec => None, + ConfigShellToolType::ShellCommand => Some("shell_command"), } } @@ -1275,6 +1333,29 @@ mod tests { assert_contains_tool_names(&tools, &subset); } + #[test] + fn test_build_specs_shell_command_present() { + let mut model_family = + find_family_for_model("o3").expect("o3 should be a valid model family"); + model_family.shell_type = ConfigShellToolType::ShellCommand; + let features = Features::with_defaults(); + let config = ToolsConfig::new(&ToolsConfigParams { + model_family: &model_family, + features: &features, + }); + let (tools, _) = build_specs(&config, Some(HashMap::new())).build(); + + // Only check the shell variant and a couple of core tools. + let subset = vec![ + "shell_command", + "list_mcp_resources", + "list_mcp_resource_templates", + "read_mcp_resource", + "update_plan", + ]; + assert_contains_tool_names(&tools, &subset); + } + #[test] #[ignore] fn test_parallel_support_flags() { @@ -1730,6 +1811,21 @@ mod tests { assert_eq!(description, expected); } + #[test] + fn test_shell_command_tool() { + let tool = super::create_shell_command_tool(); + let ToolSpec::Function(ResponsesApiTool { + description, name, .. + }) = &tool + else { + panic!("expected function tool"); + }; + assert_eq!(name, "shell_command"); + + let expected = "Runs a shell command string and returns its output."; + assert_eq!(description, expected); + } + #[test] fn test_get_openai_tools_mcp_tools_with_additional_properties_schema() { let model_family = find_family_for_model("gpt-5-codex") diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index a824ee91e4..9cb579add5 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -307,6 +307,22 @@ pub struct ShellToolCallParams { 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)] +pub struct ShellCommandToolCallParams { + pub command: String, + 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(skip_serializing_if = "Option::is_none")] + pub with_escalated_permissions: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub justification: Option, +} + /// Responses API compatible content items that can be returned by a tool call. /// This is a subset of ContentItem with the types we support as function call outputs. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)]