mirror of
https://github.com/openai/codex.git
synced 2026-02-01 22:47:52 +00:00
feat: support login as an option on shell-tool-mcp (#7120)
The unified exec tool has a `login` option that defaults to `true`:
3bdcbc7292/codex-rs/core/src/tools/handlers/unified_exec.rs (L35-L36)
This updates the `ExecParams` for `shell-tool-mcp` to support the same
parameter. Note it is declared as `Option<bool>` to ensure it is marked
optional in the generated JSON schema.
This commit is contained in:
@@ -23,6 +23,7 @@ use crate::posix::escalate_protocol::EscalateResponse;
|
||||
use crate::posix::escalate_protocol::SuperExecMessage;
|
||||
use crate::posix::escalate_protocol::SuperExecResult;
|
||||
use crate::posix::escalation_policy::EscalationPolicy;
|
||||
use crate::posix::mcp::ExecParams;
|
||||
use crate::posix::socket::AsyncDatagramSocket;
|
||||
use crate::posix::socket::AsyncSocket;
|
||||
use codex_core::exec::ExecExpiration;
|
||||
@@ -47,9 +48,7 @@ impl EscalateServer {
|
||||
|
||||
pub async fn exec(
|
||||
&self,
|
||||
command: String,
|
||||
env: HashMap<String, String>,
|
||||
workdir: PathBuf,
|
||||
params: ExecParams,
|
||||
cancel_rx: CancellationToken,
|
||||
) -> anyhow::Result<ExecResult> {
|
||||
let (escalate_server, escalate_client) = AsyncDatagramSocket::pair()?;
|
||||
@@ -57,7 +56,7 @@ impl EscalateServer {
|
||||
client_socket.set_cloexec(false)?;
|
||||
|
||||
let escalate_task = tokio::spawn(escalate_task(escalate_server, self.policy.clone()));
|
||||
let mut env = env.clone();
|
||||
let mut env = std::env::vars().collect::<HashMap<String, String>>();
|
||||
env.insert(
|
||||
ESCALATE_SOCKET_ENV_VAR.to_string(),
|
||||
client_socket.as_raw_fd().to_string(),
|
||||
@@ -73,11 +72,21 @@ impl EscalateServer {
|
||||
let sandbox_policy = SandboxPolicy::ReadOnly;
|
||||
let sandbox_cwd = PathBuf::from("/__NONEXISTENT__");
|
||||
|
||||
let ExecParams {
|
||||
command,
|
||||
workdir,
|
||||
timeout_ms: _,
|
||||
login,
|
||||
} = params;
|
||||
let result = process_exec_tool_call(
|
||||
codex_core::exec::ExecParams {
|
||||
command: vec![
|
||||
self.bash_path.to_string_lossy().to_string(),
|
||||
"-c".to_string(),
|
||||
if login == Some(false) {
|
||||
"-c".to_string()
|
||||
} else {
|
||||
"-lc".to_string()
|
||||
},
|
||||
command,
|
||||
],
|
||||
cwd: PathBuf::from(&workdir),
|
||||
|
||||
@@ -41,6 +41,8 @@ pub struct ExecParams {
|
||||
pub workdir: String,
|
||||
/// The timeout for the command in milliseconds.
|
||||
pub timeout_ms: Option<u64>,
|
||||
/// Launch Bash with -lc instead of -c: defaults to true.
|
||||
pub login: Option<bool>,
|
||||
}
|
||||
|
||||
#[derive(Debug, serde::Serialize, schemars::JsonSchema)]
|
||||
@@ -101,13 +103,7 @@ impl ExecTool {
|
||||
McpEscalationPolicy::new(self.policy, context, stopwatch.clone()),
|
||||
);
|
||||
let result = escalate_server
|
||||
.exec(
|
||||
params.command,
|
||||
// TODO: use ShellEnvironmentPolicy
|
||||
std::env::vars().collect(),
|
||||
PathBuf::from(¶ms.workdir),
|
||||
cancel_token,
|
||||
)
|
||||
.exec(params, cancel_token)
|
||||
.await
|
||||
.map_err(|e| McpError::internal_error(e.to_string(), None))?;
|
||||
Ok(CallToolResult::success(vec![Content::json(
|
||||
@@ -147,3 +143,50 @@ pub(crate) async fn serve(
|
||||
let tool = ExecTool::new(bash_path, execve_wrapper, policy);
|
||||
tool.serve(stdio()).await
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::json;
|
||||
|
||||
/// Verify that the way we use serde does not compromise the desired JSON
|
||||
/// schema via schemars. In particular, ensure that the `login` and
|
||||
/// `timeout_ms` fields are optional.
|
||||
#[test]
|
||||
fn exec_params_json_schema_matches_expected() {
|
||||
let schema = schemars::schema_for!(ExecParams);
|
||||
let actual = serde_json::to_value(schema).expect("schema should serialize");
|
||||
|
||||
assert_eq!(
|
||||
actual,
|
||||
json!({
|
||||
"$schema": "https://json-schema.org/draft/2020-12/schema",
|
||||
"title": "ExecParams",
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"command": {
|
||||
"description": "The bash string to execute.",
|
||||
"type": "string"
|
||||
},
|
||||
"login": {
|
||||
"description": "Launch Bash with -lc instead of -c: defaults to true.",
|
||||
"type": ["boolean", "null"]
|
||||
},
|
||||
"timeout_ms": {
|
||||
"description": "The timeout for the command in milliseconds.",
|
||||
"format": "uint64",
|
||||
"minimum": 0,
|
||||
"type": ["integer", "null"]
|
||||
},
|
||||
"workdir": {
|
||||
"description":
|
||||
"The working directory to execute the command in. Must be an absolute path.",
|
||||
"type": "string"
|
||||
}
|
||||
},
|
||||
"required": ["command", "workdir"]
|
||||
})
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user