mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
feat: shell_command tool (#6510)
This adds support for a new variant of the shell tool behind a flag. To
test, run `codex` with `--enable shell_command_tool`, which will
register the tool with Codex under the name `shell_command` that accepts
the following shape:
```python
{
command: str
workdir: str | None,
timeout_ms: int | None,
with_escalated_permissions: bool | None,
justification: str | None,
}
```
This is comparable to the existing tool registered under
`shell`/`container.exec`. The primary difference is that it accepts
`command` as a `str` instead of a `str[]`. The `shell_command` tool
executes by running `execvp(["bash", "-lc", command])`, though the exact
arguments to `execvp(3)` depend on the user's default shell.
The hypothesis is that this will simplify things for the model. For
example, on Windows, instead of generating:
```json
{"command": ["pwsh.exe", "-NoLogo", "-Command", "ls -Name"]}
```
The model could simply generate:
```json
{"command": "ls -Name"}
```
As part of this change, I extracted some logic out of `user_shell.rs` as
`Shell::derive_exec_args()` so that it can be reused in
`codex-rs/core/src/tools/handlers/shell.rs`. Note the original code
generated exec arg lists like:
```javascript
["bash", "-lc", command]
["zsh", "-lc", command]
["pwsh.exe", "-NoProfile", "-Command", command]
```
Using `-l` for Bash and Zsh, but then specifying `-NoProfile` for
PowerShell seemed inconsistent to me, so I changed this in the new
implementation while also adding a `use_login_shell: bool` option to
make this explicit. If we decide to add a `login: bool` to
`ShellCommandToolCallParams` like we have for unified exec:
https://github.com/openai/codex/blob/807e2c27f0a9f2e85c50e7e6df5533f0d9b853c7/codex-rs/core/src/tools/handlers/unified_exec.rs#L33-L34
Then this should make it straightforward to support.
This commit is contained in:
committed by
GitHub
Unverified
parent
530db0ad73
commit
29364f3a9b
@@ -104,7 +104,7 @@ pub fn extract_bash_command(command: &[String]) -> Option<(&str, &str)> {
|
||||
let [shell, flag, script] = command else {
|
||||
return None;
|
||||
};
|
||||
if flag != "-lc" || !is_well_known_sh_shell(shell) {
|
||||
if !matches!(flag.as_str(), "-lc" | "-c") || !is_well_known_sh_shell(shell) {
|
||||
return None;
|
||||
}
|
||||
Some((shell, script))
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -31,16 +31,37 @@ pub enum Shell {
|
||||
impl Shell {
|
||||
pub fn name(&self) -> Option<String> {
|
||||
match self {
|
||||
Shell::Zsh(zsh) => std::path::Path::new(&zsh.shell_path)
|
||||
.file_name()
|
||||
.map(|s| s.to_string_lossy().to_string()),
|
||||
Shell::Bash(bash) => std::path::Path::new(&bash.shell_path)
|
||||
.file_name()
|
||||
.map(|s| s.to_string_lossy().to_string()),
|
||||
Shell::Zsh(ZshShell { shell_path, .. }) | Shell::Bash(BashShell { shell_path, .. }) => {
|
||||
std::path::Path::new(shell_path)
|
||||
.file_name()
|
||||
.map(|s| s.to_string_lossy().to_string())
|
||||
}
|
||||
Shell::PowerShell(ps) => Some(ps.exe.clone()),
|
||||
Shell::Unknown => None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Takes a string of shell and returns the full list of command args to
|
||||
/// use with `exec()` to run the shell command.
|
||||
pub fn derive_exec_args(&self, command: &str, use_login_shell: bool) -> Vec<String> {
|
||||
match self {
|
||||
Shell::Zsh(ZshShell { shell_path, .. }) | Shell::Bash(BashShell { shell_path, .. }) => {
|
||||
let arg = if use_login_shell { "-lc" } else { "-c" };
|
||||
vec![shell_path.clone(), arg.to_string(), command.to_string()]
|
||||
}
|
||||
Shell::PowerShell(ps) => {
|
||||
let mut args = vec![ps.exe.clone(), "-NoLogo".to_string()];
|
||||
if !use_login_shell {
|
||||
args.push("-NoProfile".to_string());
|
||||
}
|
||||
|
||||
args.push("-Command".to_string());
|
||||
args.push(command.to_string());
|
||||
args
|
||||
}
|
||||
Shell::Unknown => shlex::split(command).unwrap_or_else(|| vec![command.to_string()]),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
|
||||
@@ -63,27 +63,10 @@ impl SessionTask for UserShellCommandTask {
|
||||
// Execute the user's script under their default shell when known; this
|
||||
// allows commands that use shell features (pipes, &&, redirects, etc.).
|
||||
// We do not source rc files or otherwise reformat the script.
|
||||
let shell_invocation = match session.user_shell() {
|
||||
crate::shell::Shell::Zsh(zsh) => vec![
|
||||
zsh.shell_path.clone(),
|
||||
"-lc".to_string(),
|
||||
self.command.clone(),
|
||||
],
|
||||
crate::shell::Shell::Bash(bash) => vec![
|
||||
bash.shell_path.clone(),
|
||||
"-lc".to_string(),
|
||||
self.command.clone(),
|
||||
],
|
||||
crate::shell::Shell::PowerShell(ps) => vec![
|
||||
ps.exe.clone(),
|
||||
"-NoProfile".to_string(),
|
||||
"-Command".to_string(),
|
||||
self.command.clone(),
|
||||
],
|
||||
crate::shell::Shell::Unknown => {
|
||||
shlex::split(&self.command).unwrap_or_else(|| vec![self.command.clone()])
|
||||
}
|
||||
};
|
||||
let use_login_shell = true;
|
||||
let shell_invocation = session
|
||||
.user_shell()
|
||||
.derive_exec_args(&self.command, use_login_shell);
|
||||
|
||||
let call_id = Uuid::new_v4().to_string();
|
||||
let raw_command = self.command.clone();
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use async_trait::async_trait;
|
||||
use codex_protocol::models::ShellCommandToolCallParams;
|
||||
use codex_protocol::models::ShellToolCallParams;
|
||||
use std::sync::Arc;
|
||||
|
||||
@@ -25,6 +26,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 +42,28 @@ impl ShellHandler {
|
||||
}
|
||||
}
|
||||
|
||||
impl ShellCommandHandler {
|
||||
fn to_exec_params(
|
||||
params: ShellCommandToolCallParams,
|
||||
session: &crate::codex::Session,
|
||||
turn_context: &TurnContext,
|
||||
) -> ExecParams {
|
||||
let shell = session.user_shell();
|
||||
let use_login_shell = true;
|
||||
let command = shell.derive_exec_args(¶ms.command, use_login_shell);
|
||||
|
||||
ExecParams {
|
||||
command,
|
||||
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,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl ToolHandler for ShellHandler {
|
||||
fn kind(&self) -> ToolKind {
|
||||
@@ -102,6 +127,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<ToolOutput, FunctionCallError> {
|
||||
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 +308,49 @@ impl ShellHandler {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crate::is_safe_command::is_known_safe_command;
|
||||
use crate::shell::BashShell;
|
||||
use crate::shell::Shell;
|
||||
use crate::shell::ZshShell;
|
||||
|
||||
/// The logic for is_known_safe_command() has heuristics for known shells,
|
||||
/// so we must ensure the commands generated by [ShellCommandHandler] can be
|
||||
/// recognized as safe if the `command` is safe.
|
||||
#[test]
|
||||
fn commands_generated_by_shell_command_handler_can_be_matched_by_is_known_safe_command() {
|
||||
let bash_shell = Shell::Bash(BashShell {
|
||||
shell_path: "/bin/bash".to_string(),
|
||||
bashrc_path: "/home/user/.bashrc".to_string(),
|
||||
});
|
||||
assert_safe(&bash_shell, "ls -la");
|
||||
|
||||
let zsh_shell = Shell::Zsh(ZshShell {
|
||||
shell_path: "/bin/zsh".to_string(),
|
||||
zshrc_path: "/home/user/.zshrc".to_string(),
|
||||
});
|
||||
assert_safe(&zsh_shell, "ls -la");
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
{
|
||||
use crate::shell::PowerShellConfig;
|
||||
|
||||
let powershell = Shell::PowerShell(PowerShellConfig {
|
||||
exe: "pwsh.exe".to_string(),
|
||||
bash_exe_fallback: None,
|
||||
});
|
||||
assert_safe(&powershell, "ls -Name");
|
||||
}
|
||||
}
|
||||
|
||||
fn assert_safe(shell: &Shell, command: &str) {
|
||||
assert!(is_known_safe_command(
|
||||
&shell.derive_exec_args(command, /* use_login_shell */ true)
|
||||
));
|
||||
assert!(is_known_safe_command(
|
||||
&shell.derive_exec_args(command, /* use_login_shell */ false)
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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()
|
||||
};
|
||||
@@ -302,6 +306,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();
|
||||
@@ -891,6 +942,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;
|
||||
@@ -906,6 +958,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 => {
|
||||
@@ -920,12 +973,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);
|
||||
@@ -1061,6 +1118,7 @@ mod tests {
|
||||
ConfigShellToolType::Default => Some("shell"),
|
||||
ConfigShellToolType::Local => Some("local_shell"),
|
||||
ConfigShellToolType::UnifiedExec => None,
|
||||
ConfigShellToolType::ShellCommand => Some("shell_command"),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1293,6 +1351,22 @@ mod tests {
|
||||
assert_contains_tool_names(&tools, &subset);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_specs_shell_command_present() {
|
||||
assert_model_tools(
|
||||
"codex-mini-latest",
|
||||
Features::with_defaults().enable(Feature::ShellCommandTool),
|
||||
&[
|
||||
"shell_command",
|
||||
"list_mcp_resources",
|
||||
"list_mcp_resource_templates",
|
||||
"read_mcp_resource",
|
||||
"update_plan",
|
||||
"view_image",
|
||||
],
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[ignore]
|
||||
fn test_parallel_support_flags() {
|
||||
@@ -1748,6 +1822,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")
|
||||
|
||||
@@ -292,7 +292,7 @@ impl From<Vec<UserInput>> for ResponseInputItem {
|
||||
}
|
||||
|
||||
/// If the `name` of a `ResponseItem::FunctionCall` is either `container.exec`
|
||||
/// or shell`, the `arguments` field should deserialize to this struct.
|
||||
/// or `shell`, the `arguments` field should deserialize to this struct.
|
||||
#[derive(Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
pub struct ShellToolCallParams {
|
||||
pub command: Vec<String>,
|
||||
@@ -307,6 +307,22 @@ pub struct ShellToolCallParams {
|
||||
pub justification: Option<String>,
|
||||
}
|
||||
|
||||
/// 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<String>,
|
||||
|
||||
/// This is the maximum time in milliseconds that the command is allowed to run.
|
||||
#[serde(alias = "timeout")]
|
||||
pub timeout_ms: Option<u64>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub with_escalated_permissions: Option<bool>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub justification: Option<String>,
|
||||
}
|
||||
|
||||
/// 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)]
|
||||
|
||||
Reference in New Issue
Block a user