mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
MCP tool call approval (simplified version) (#10200)
Add elicitation approval request for MCP tool call requests.
This commit is contained in:
@@ -221,7 +221,11 @@ fn normalize_connector_value(value: Option<&str>) -> Option<String> {
|
||||
}
|
||||
|
||||
const ALLOWED_APPS_SDK_APPS: &[&str] = &["asdk_app_69781557cc1481919cf5e9824fa2e792"];
|
||||
const DISALLOWED_CONNECTOR_IDS: &[&str] = &["asdk_app_6938a94a61d881918ef32cb999ff937c"];
|
||||
const DISALLOWED_CONNECTOR_IDS: &[&str] = &[
|
||||
"asdk_app_6938a94a61d881918ef32cb999ff937c",
|
||||
"connector_2b0a9009c9c64bf9933a3dae3f2b1254",
|
||||
"connector_68de829bf7648191acd70a907364c67c",
|
||||
];
|
||||
const DISALLOWED_CONNECTOR_PREFIX: &str = "connector_openai_";
|
||||
|
||||
fn filter_disallowed_connectors(connectors: Vec<AppInfo>) -> Vec<AppInfo> {
|
||||
|
||||
@@ -769,6 +769,32 @@ fn filter_tools(tools: Vec<ToolInfo>, filter: ToolFilter) -> Vec<ToolInfo> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn normalize_codex_apps_tool_title(
|
||||
server_name: &str,
|
||||
connector_name: Option<&str>,
|
||||
value: &str,
|
||||
) -> String {
|
||||
if server_name != CODEX_APPS_MCP_SERVER_NAME {
|
||||
return value.to_string();
|
||||
}
|
||||
|
||||
let Some(connector_name) = connector_name
|
||||
.map(str::trim)
|
||||
.filter(|name| !name.is_empty())
|
||||
else {
|
||||
return value.to_string();
|
||||
};
|
||||
|
||||
let prefix = format!("{connector_name}_");
|
||||
if let Some(stripped) = value.strip_prefix(&prefix)
|
||||
&& !stripped.is_empty()
|
||||
{
|
||||
return stripped.to_string();
|
||||
}
|
||||
|
||||
value.to_string()
|
||||
}
|
||||
|
||||
fn resolve_bearer_token(
|
||||
server_name: &str,
|
||||
bearer_token_env_var: Option<&str>,
|
||||
@@ -926,12 +952,23 @@ async fn list_tools_for_client(
|
||||
Ok(resp
|
||||
.tools
|
||||
.into_iter()
|
||||
.map(|tool| ToolInfo {
|
||||
server_name: server_name.to_owned(),
|
||||
tool_name: tool.tool.name.clone(),
|
||||
tool: tool.tool,
|
||||
connector_id: tool.connector_id,
|
||||
connector_name: tool.connector_name,
|
||||
.map(|tool| {
|
||||
let connector_name = tool.connector_name;
|
||||
let mut tool_def = tool.tool;
|
||||
if let Some(title) = tool_def.title.as_deref() {
|
||||
let normalized_title =
|
||||
normalize_codex_apps_tool_title(server_name, connector_name.as_deref(), title);
|
||||
if tool_def.title.as_deref() != Some(normalized_title.as_str()) {
|
||||
tool_def.title = Some(normalized_title);
|
||||
}
|
||||
}
|
||||
ToolInfo {
|
||||
server_name: server_name.to_owned(),
|
||||
tool_name: tool_def.name.clone(),
|
||||
tool: tool_def,
|
||||
connector_id: tool.connector_id,
|
||||
connector_name,
|
||||
}
|
||||
})
|
||||
.collect())
|
||||
}
|
||||
|
||||
@@ -1,20 +1,30 @@
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
use tracing::error;
|
||||
|
||||
use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
|
||||
use crate::protocol::EventMsg;
|
||||
use crate::protocol::McpInvocation;
|
||||
use crate::protocol::McpToolCallBeginEvent;
|
||||
use crate::protocol::McpToolCallEndEvent;
|
||||
use codex_protocol::models::FunctionCallOutputPayload;
|
||||
use codex_protocol::models::ResponseInputItem;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::SandboxPolicy;
|
||||
use codex_protocol::request_user_input::RequestUserInputArgs;
|
||||
use codex_protocol::request_user_input::RequestUserInputQuestion;
|
||||
use codex_protocol::request_user_input::RequestUserInputQuestionOption;
|
||||
use codex_protocol::request_user_input::RequestUserInputResponse;
|
||||
use mcp_types::ToolAnnotations;
|
||||
use std::sync::Arc;
|
||||
|
||||
/// Handles the specified tool call dispatches the appropriate
|
||||
/// `McpToolCallBegin` and `McpToolCallEnd` events to the `Session`.
|
||||
pub(crate) async fn handle_mcp_tool_call(
|
||||
sess: &Session,
|
||||
sess: Arc<Session>,
|
||||
turn_context: &TurnContext,
|
||||
call_id: String,
|
||||
server: String,
|
||||
@@ -48,11 +58,79 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
arguments: arguments_value.clone(),
|
||||
};
|
||||
|
||||
if let Some(decision) =
|
||||
maybe_request_mcp_tool_approval(sess.as_ref(), turn_context, &call_id, &server, &tool_name)
|
||||
.await
|
||||
{
|
||||
let result = match decision {
|
||||
McpToolApprovalDecision::Accept => {
|
||||
let tool_call_begin_event = EventMsg::McpToolCallBegin(McpToolCallBeginEvent {
|
||||
call_id: call_id.clone(),
|
||||
invocation: invocation.clone(),
|
||||
});
|
||||
notify_mcp_tool_call_event(sess.as_ref(), turn_context, tool_call_begin_event)
|
||||
.await;
|
||||
|
||||
let start = Instant::now();
|
||||
let result = sess
|
||||
.call_tool(&server, &tool_name, arguments_value.clone())
|
||||
.await
|
||||
.map_err(|e| format!("tool call error: {e:?}"));
|
||||
if let Err(e) = &result {
|
||||
tracing::warn!("MCP tool call error: {e:?}");
|
||||
}
|
||||
let tool_call_end_event = EventMsg::McpToolCallEnd(McpToolCallEndEvent {
|
||||
call_id: call_id.clone(),
|
||||
invocation,
|
||||
duration: start.elapsed(),
|
||||
result: result.clone(),
|
||||
});
|
||||
notify_mcp_tool_call_event(
|
||||
sess.as_ref(),
|
||||
turn_context,
|
||||
tool_call_end_event.clone(),
|
||||
)
|
||||
.await;
|
||||
result
|
||||
}
|
||||
McpToolApprovalDecision::Decline => {
|
||||
let message = "user rejected MCP tool call".to_string();
|
||||
notify_mcp_tool_call_skip(
|
||||
sess.as_ref(),
|
||||
turn_context,
|
||||
&call_id,
|
||||
invocation,
|
||||
message,
|
||||
)
|
||||
.await
|
||||
}
|
||||
McpToolApprovalDecision::Cancel => {
|
||||
let message = "user cancelled MCP tool call".to_string();
|
||||
notify_mcp_tool_call_skip(
|
||||
sess.as_ref(),
|
||||
turn_context,
|
||||
&call_id,
|
||||
invocation,
|
||||
message,
|
||||
)
|
||||
.await
|
||||
}
|
||||
};
|
||||
|
||||
let status = if result.is_ok() { "ok" } else { "error" };
|
||||
turn_context
|
||||
.client
|
||||
.get_otel_manager()
|
||||
.counter("codex.mcp.call", 1, &[("status", status)]);
|
||||
|
||||
return ResponseInputItem::McpToolCallOutput { call_id, result };
|
||||
}
|
||||
|
||||
let tool_call_begin_event = EventMsg::McpToolCallBegin(McpToolCallBeginEvent {
|
||||
call_id: call_id.clone(),
|
||||
invocation: invocation.clone(),
|
||||
});
|
||||
notify_mcp_tool_call_event(sess, turn_context, tool_call_begin_event).await;
|
||||
notify_mcp_tool_call_event(sess.as_ref(), turn_context, tool_call_begin_event).await;
|
||||
|
||||
let start = Instant::now();
|
||||
// Perform the tool call.
|
||||
@@ -70,7 +148,7 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
result: result.clone(),
|
||||
});
|
||||
|
||||
notify_mcp_tool_call_event(sess, turn_context, tool_call_end_event.clone()).await;
|
||||
notify_mcp_tool_call_event(sess.as_ref(), turn_context, tool_call_end_event.clone()).await;
|
||||
|
||||
let status = if result.is_ok() { "ok" } else { "error" };
|
||||
turn_context
|
||||
@@ -84,3 +162,236 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
async fn notify_mcp_tool_call_event(sess: &Session, turn_context: &TurnContext, event: EventMsg) {
|
||||
sess.send_event(turn_context, event).await;
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum McpToolApprovalDecision {
|
||||
Accept,
|
||||
Decline,
|
||||
Cancel,
|
||||
}
|
||||
|
||||
struct McpToolApprovalMetadata {
|
||||
annotations: ToolAnnotations,
|
||||
connector_name: Option<String>,
|
||||
tool_title: Option<String>,
|
||||
}
|
||||
|
||||
const MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX: &str = "mcp_tool_call_approval";
|
||||
const MCP_TOOL_APPROVAL_ACCEPT: &str = "Accept";
|
||||
const MCP_TOOL_APPROVAL_DECLINE: &str = "Decline";
|
||||
const MCP_TOOL_APPROVAL_CANCEL: &str = "Cancel";
|
||||
|
||||
async fn maybe_request_mcp_tool_approval(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
call_id: &str,
|
||||
server: &str,
|
||||
tool_name: &str,
|
||||
) -> Option<McpToolApprovalDecision> {
|
||||
if is_full_access_mode(turn_context) {
|
||||
return None;
|
||||
}
|
||||
if server != CODEX_APPS_MCP_SERVER_NAME {
|
||||
return None;
|
||||
}
|
||||
|
||||
let metadata = lookup_mcp_tool_metadata(sess, server, tool_name).await?;
|
||||
if !requires_mcp_tool_approval(&metadata.annotations) {
|
||||
return None;
|
||||
}
|
||||
|
||||
let question_id = format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_{call_id}");
|
||||
let question = build_mcp_tool_approval_question(
|
||||
question_id.clone(),
|
||||
tool_name,
|
||||
metadata.tool_title.as_deref(),
|
||||
metadata.connector_name.as_deref(),
|
||||
&metadata.annotations,
|
||||
);
|
||||
let args = RequestUserInputArgs {
|
||||
questions: vec![question],
|
||||
};
|
||||
let response = sess
|
||||
.request_user_input(turn_context, call_id.to_string(), args)
|
||||
.await;
|
||||
Some(parse_mcp_tool_approval_response(response, &question_id))
|
||||
}
|
||||
|
||||
fn is_full_access_mode(turn_context: &TurnContext) -> bool {
|
||||
matches!(turn_context.approval_policy, AskForApproval::Never)
|
||||
&& matches!(
|
||||
turn_context.sandbox_policy,
|
||||
SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. }
|
||||
)
|
||||
}
|
||||
|
||||
async fn lookup_mcp_tool_metadata(
|
||||
sess: &Session,
|
||||
server: &str,
|
||||
tool_name: &str,
|
||||
) -> Option<McpToolApprovalMetadata> {
|
||||
let tools = sess
|
||||
.services
|
||||
.mcp_connection_manager
|
||||
.read()
|
||||
.await
|
||||
.list_all_tools()
|
||||
.await;
|
||||
|
||||
tools.into_values().find_map(|tool_info| {
|
||||
if tool_info.server_name == server && tool_info.tool_name == tool_name {
|
||||
tool_info
|
||||
.tool
|
||||
.annotations
|
||||
.map(|annotations| McpToolApprovalMetadata {
|
||||
annotations,
|
||||
connector_name: tool_info.connector_name,
|
||||
tool_title: tool_info.tool.title,
|
||||
})
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn build_mcp_tool_approval_question(
|
||||
question_id: String,
|
||||
tool_name: &str,
|
||||
tool_title: Option<&str>,
|
||||
connector_name: Option<&str>,
|
||||
annotations: &ToolAnnotations,
|
||||
) -> RequestUserInputQuestion {
|
||||
let destructive = annotations.destructive_hint == Some(true);
|
||||
let open_world = annotations.open_world_hint == Some(true);
|
||||
let reason = match (destructive, open_world) {
|
||||
(true, true) => "may modify data and access external systems",
|
||||
(true, false) => "may modify or delete data",
|
||||
(false, true) => "may access external systems",
|
||||
(false, false) => "may have side effects",
|
||||
};
|
||||
|
||||
let tool_label = tool_title.unwrap_or(tool_name);
|
||||
let app_label = connector_name
|
||||
.map(|name| format!("The {name} app"))
|
||||
.unwrap_or_else(|| "This app".to_string());
|
||||
let question = format!(
|
||||
"{app_label} wants to run the tool \"{tool_label}\", which {reason}. Allow this action?"
|
||||
);
|
||||
|
||||
RequestUserInputQuestion {
|
||||
id: question_id,
|
||||
header: "Approve app tool call?".to_string(),
|
||||
question,
|
||||
is_other: false,
|
||||
is_secret: false,
|
||||
options: Some(vec![
|
||||
RequestUserInputQuestionOption {
|
||||
label: MCP_TOOL_APPROVAL_ACCEPT.to_string(),
|
||||
description: "Run the tool and continue.".to_string(),
|
||||
},
|
||||
RequestUserInputQuestionOption {
|
||||
label: MCP_TOOL_APPROVAL_DECLINE.to_string(),
|
||||
description: "Decline this tool call and continue.".to_string(),
|
||||
},
|
||||
RequestUserInputQuestionOption {
|
||||
label: MCP_TOOL_APPROVAL_CANCEL.to_string(),
|
||||
description: "Cancel this tool call".to_string(),
|
||||
},
|
||||
]),
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_mcp_tool_approval_response(
|
||||
response: Option<RequestUserInputResponse>,
|
||||
question_id: &str,
|
||||
) -> McpToolApprovalDecision {
|
||||
let Some(response) = response else {
|
||||
return McpToolApprovalDecision::Cancel;
|
||||
};
|
||||
let answers = response
|
||||
.answers
|
||||
.get(question_id)
|
||||
.map(|answer| answer.answers.as_slice());
|
||||
let Some(answers) = answers else {
|
||||
return McpToolApprovalDecision::Cancel;
|
||||
};
|
||||
if answers
|
||||
.iter()
|
||||
.any(|answer| answer == MCP_TOOL_APPROVAL_ACCEPT)
|
||||
{
|
||||
McpToolApprovalDecision::Accept
|
||||
} else if answers
|
||||
.iter()
|
||||
.any(|answer| answer == MCP_TOOL_APPROVAL_CANCEL)
|
||||
{
|
||||
McpToolApprovalDecision::Cancel
|
||||
} else {
|
||||
McpToolApprovalDecision::Decline
|
||||
}
|
||||
}
|
||||
|
||||
fn requires_mcp_tool_approval(annotations: &ToolAnnotations) -> bool {
|
||||
annotations.read_only_hint == Some(false)
|
||||
&& (annotations.destructive_hint == Some(true) || annotations.open_world_hint == Some(true))
|
||||
}
|
||||
|
||||
async fn notify_mcp_tool_call_skip(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
call_id: &str,
|
||||
invocation: McpInvocation,
|
||||
message: String,
|
||||
) -> Result<mcp_types::CallToolResult, String> {
|
||||
let tool_call_begin_event = EventMsg::McpToolCallBegin(McpToolCallBeginEvent {
|
||||
call_id: call_id.to_string(),
|
||||
invocation: invocation.clone(),
|
||||
});
|
||||
notify_mcp_tool_call_event(sess, turn_context, tool_call_begin_event).await;
|
||||
|
||||
let tool_call_end_event = EventMsg::McpToolCallEnd(McpToolCallEndEvent {
|
||||
call_id: call_id.to_string(),
|
||||
invocation,
|
||||
duration: Duration::ZERO,
|
||||
result: Err(message.clone()),
|
||||
});
|
||||
notify_mcp_tool_call_event(sess, turn_context, tool_call_end_event).await;
|
||||
Err(message)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
fn annotations(
|
||||
read_only: Option<bool>,
|
||||
destructive: Option<bool>,
|
||||
open_world: Option<bool>,
|
||||
) -> ToolAnnotations {
|
||||
ToolAnnotations {
|
||||
destructive_hint: destructive,
|
||||
idempotent_hint: None,
|
||||
open_world_hint: open_world,
|
||||
read_only_hint: read_only,
|
||||
title: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_required_when_read_only_false_and_destructive() {
|
||||
let annotations = annotations(Some(false), Some(true), None);
|
||||
assert_eq!(requires_mcp_tool_approval(&annotations), true);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_required_when_read_only_false_and_open_world() {
|
||||
let annotations = annotations(Some(false), None, Some(true));
|
||||
assert_eq!(requires_mcp_tool_approval(&annotations), true);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_not_required_when_read_only_true() {
|
||||
let annotations = annotations(Some(true), Some(true), Some(true));
|
||||
assert_eq!(requires_mcp_tool_approval(&annotations), false);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use async_trait::async_trait;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::mcp_tool_call::handle_mcp_tool_call;
|
||||
@@ -42,7 +43,7 @@ impl ToolHandler for McpHandler {
|
||||
let arguments_str = raw_arguments;
|
||||
|
||||
let response = handle_mcp_tool_call(
|
||||
session.as_ref(),
|
||||
Arc::clone(&session),
|
||||
turn.as_ref(),
|
||||
call_id.clone(),
|
||||
server,
|
||||
|
||||
Reference in New Issue
Block a user