This commit is contained in:
Matthew Zeng
2026-03-12 19:10:55 -07:00
parent e5995b1f70
commit d0f8f87960
3 changed files with 224 additions and 49 deletions

View File

@@ -1220,7 +1220,7 @@ fn normalize_codex_apps_tool_title(
value.to_string()
}
fn normalize_codex_apps_tool_name(
pub(crate) fn normalize_codex_apps_tool_name(
server_name: &str,
tool_name: &str,
connector_id: Option<&str>,

View File

@@ -26,13 +26,15 @@ use crate::guardian::guardian_approval_request_to_json;
use crate::guardian::review_approval_request;
use crate::guardian::routes_approval_to_guardian;
use crate::mcp::CODEX_APPS_MCP_SERVER_NAME;
use crate::mcp_connection_manager::ToolInfo;
use crate::mcp_connection_manager::normalize_codex_apps_tool_name;
use crate::mcp_tool_approval_templates::RenderedMcpToolApprovalParam;
use crate::mcp_tool_approval_templates::render_mcp_tool_approval_template;
use crate::protocol::EventMsg;
use crate::protocol::McpInvocation;
use crate::protocol::McpToolCallBeginEvent;
use crate::protocol::McpToolCallEndEvent;
use crate::slack_channel_names::slack_channel_name_from_profile_result;
use crate::slack_channel_names::slack_channel_name_from_tool_result;
use crate::slack_channel_names::slack_send_message_channel_id;
use crate::slack_channel_names::translated_slack_send_message_tool_params;
use crate::state_db;
@@ -694,11 +696,10 @@ async fn build_mcp_tool_approval_tool_params(
invocation: &McpInvocation,
metadata: Option<&McpToolApprovalMetadata>,
) -> Option<serde_json::Value> {
let connector_name = metadata.and_then(|metadata| metadata.connector_name.as_deref());
let tool_title = metadata.and_then(|metadata| metadata.tool_title.as_deref());
let connector_id = metadata.and_then(|metadata| metadata.connector_id.as_deref());
let channel_name = match slack_send_message_channel_id(
connector_name,
connector_id,
tool_title,
invocation.arguments.as_ref(),
) {
@@ -707,7 +708,7 @@ async fn build_mcp_tool_approval_tool_params(
};
translated_slack_send_message_tool_params(
connector_name,
connector_id,
tool_title,
invocation.arguments.as_ref(),
channel_name.as_deref(),
@@ -722,8 +723,8 @@ async fn maybe_record_slack_channel_name_from_tool_result(
let Ok(result) = result else {
return;
};
let Some(channel_name) = slack_channel_name_from_profile_result(
metadata.and_then(|metadata| metadata.connector_name.as_deref()),
let Some(channel_name) = slack_channel_name_from_tool_result(
metadata.and_then(|metadata| metadata.connector_id.as_deref()),
metadata.and_then(|metadata| metadata.tool_title.as_deref()),
result,
) else {
@@ -770,9 +771,7 @@ async fn lookup_mcp_tool_metadata(
.list_all_tools()
.await;
let tool_info = tools
.into_values()
.find(|tool_info| tool_info.server_name == server && tool_info.tool.name == tool_name)?;
let tool_info = find_mcp_tool_info(&tools, server, tool_name)?.clone();
let connector_description = if server == CODEX_APPS_MCP_SERVER_NAME {
let connectors = match connectors::list_cached_accessible_connectors_from_mcp_tools(
turn_context.config.as_ref(),
@@ -820,15 +819,33 @@ async fn lookup_mcp_app_usage_metadata(
.list_all_tools()
.await;
tools.into_values().find_map(|tool_info| {
if tool_info.server_name == server && tool_info.tool.name == tool_name {
Some(McpAppUsageMetadata {
connector_id: tool_info.connector_id,
app_name: tool_info.connector_name,
})
} else {
None
let tool_info = find_mcp_tool_info(&tools, server, tool_name)?;
Some(McpAppUsageMetadata {
connector_id: tool_info.connector_id.clone(),
app_name: tool_info.connector_name.clone(),
})
}
fn find_mcp_tool_info<'a>(
tools: &'a std::collections::HashMap<String, ToolInfo>,
server: &str,
raw_tool_name: &str,
) -> Option<&'a ToolInfo> {
tools.values().find(|tool_info| {
if tool_info.server_name != server {
return false;
}
tool_info.tool.name == raw_tool_name
|| tool_info.tool_name == raw_tool_name
|| (server == CODEX_APPS_MCP_SERVER_NAME
&& normalize_codex_apps_tool_name(
server,
raw_tool_name,
tool_info.connector_id.as_deref(),
tool_info.connector_name.as_deref(),
) == tool_info.tool_name)
})
}
@@ -1304,6 +1321,7 @@ mod tests {
use crate::config::types::AppToolConfig;
use crate::config::types::AppToolsConfig;
use crate::config::types::AppsConfigToml;
use crate::slack_channel_names::SLACK_CONNECTOR_ID;
use codex_config::CONFIG_TOML_FILE;
use pretty_assertions::assert_eq;
use serde::Deserialize;
@@ -1342,6 +1360,39 @@ mod tests {
}
}
fn test_codex_app_tool(
qualified_name: &str,
raw_tool_name: &str,
normalized_tool_name: &str,
tool_namespace: &str,
connector_id: &str,
connector_name: &str,
) -> (String, ToolInfo) {
(
qualified_name.to_string(),
ToolInfo {
server_name: CODEX_APPS_MCP_SERVER_NAME.to_string(),
tool_name: normalized_tool_name.to_string(),
tool_namespace: tool_namespace.to_string(),
tool: rmcp::model::Tool {
name: raw_tool_name.to_string().into(),
title: None,
description: None,
input_schema: Arc::new(rmcp::model::JsonObject::default()),
output_schema: None,
annotations: None,
execution: None,
icons: None,
meta: None,
},
connector_id: Some(connector_id.to_string()),
connector_name: Some(connector_name.to_string()),
plugin_display_names: Vec::new(),
connector_description: None,
},
)
}
fn prompt_options(
allow_session_remember: bool,
allow_persistent_approval: bool,
@@ -1388,6 +1439,40 @@ mod tests {
);
}
#[test]
fn find_mcp_tool_info_falls_back_to_connector_aware_tool_name_matching() {
let tools = HashMap::from([test_codex_app_tool(
"mcp__codex_apps__gmail-batch-read-email",
"gmail-batch-read-email",
"-batch-read-email",
"mcp__codex_apps__gmail",
"connector_gmail_456",
"Gmail",
)]);
let tool = find_mcp_tool_info(
&tools,
CODEX_APPS_MCP_SERVER_NAME,
"connector-gmail-456-batch-read-email",
)
.expect("tool");
assert_eq!(
(
tool.connector_id.as_deref(),
tool.connector_name.as_deref(),
tool.tool.name.as_ref(),
tool.tool_name.as_str(),
),
(
Some("connector_gmail_456"),
Some("Gmail"),
"gmail-batch-read-email",
"-batch-read-email",
)
);
}
#[test]
fn approval_question_text_prepends_safety_reason() {
assert_eq!(
@@ -1474,7 +1559,7 @@ mod tests {
async fn slack_profile_tool_result_records_channel_name_with_global_fallback() {
let (session, _turn_context) = make_session_and_context().await;
let metadata = approval_metadata(
Some("connector_slack_profile"),
Some(SLACK_CONNECTOR_ID),
Some("Slack Codex App"),
None,
Some("get_profile"),
@@ -1497,24 +1582,53 @@ mod tests {
assert_eq!(
session
.slack_channel_name(Some("connector_slack_profile"), "U123")
.slack_channel_name(Some(SLACK_CONNECTOR_ID), "U123")
.await,
Some("@mzeng".to_string())
);
assert_eq!(
session
.slack_channel_name(Some("connector_slack_send"), "U123")
.slack_channel_name(Some("connector_other"), "U123")
.await,
Some("@mzeng".to_string())
);
}
#[tokio::test]
async fn slack_search_users_result_records_channel_name() {
let (session, _turn_context) = make_session_and_context().await;
let metadata = approval_metadata(
Some(SLACK_CONNECTOR_ID),
Some("Slack Codex App"),
None,
Some("slack_search_users"),
None,
);
let result = Ok(CallToolResult {
content: Vec::new(),
structured_content: Some(serde_json::json!({
"results": "# Search Results for: mzeng@openai.com\n\n## Users (1 results)\n### Result 1 of 1\nName: Matthew Zeng\nUser ID: U07B9LBRPST\nTitle: Codexing agent\nEmail: mzeng@openai.com",
})),
is_error: Some(false),
meta: None,
});
maybe_record_slack_channel_name_from_tool_result(&session, Some(&metadata), &result).await;
assert_eq!(
session
.slack_channel_name(Some("connector_other"), "U07B9LBRPST")
.await,
Some("Matthew Zeng".to_string())
);
}
#[tokio::test]
async fn slack_send_message_approval_tool_params_include_translated_to_field() {
let (session, _turn_context) = make_session_and_context().await;
session
.record_slack_channel_name(
Some("connector_slack_profile"),
Some(SLACK_CONNECTOR_ID),
"U123".to_string(),
"@mzeng".to_string(),
)
@@ -1528,7 +1642,7 @@ mod tests {
})),
};
let metadata = approval_metadata(
Some("connector_slack_send"),
Some(SLACK_CONNECTOR_ID),
Some("Slack"),
None,
Some("slack_send_message"),

View File

@@ -7,10 +7,14 @@ const ID_FIELD_NAME: &str = "id";
const NAME_FIELD_NAME: &str = "name";
const NICKNAME_FIELD_NAME: &str = "nickname";
const RESULT_FIELD_NAME: &str = "result";
const RESULTS_FIELD_NAME: &str = "results";
const TO_FIELD_NAME: &str = "to";
pub(crate) const SLACK_CONNECTOR_ID: &str = "asdk_app_69a1d78e929881919bba0dbda1f6436d";
const SLACK_GET_PROFILE_TOOL_TITLE: &str = "get_profile";
const SLACK_READ_USER_PROFILE_TOOL_TITLE: &str = "slack_read_user_profile";
const SLACK_SEARCH_USERS_TOOL_TITLE: &str = "slack_search_users";
const SLACK_SEND_MESSAGE_TOOL_TITLE: &str = "slack_send_message";
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -19,12 +23,12 @@ pub(crate) struct SlackChannelName {
pub(crate) channel_name: String,
}
pub(crate) fn slack_channel_name_from_profile_result(
connector_name: Option<&str>,
pub(crate) fn slack_channel_name_from_tool_result(
connector_id: Option<&str>,
tool_title: Option<&str>,
result: &CallToolResult,
) -> Option<SlackChannelName> {
if !is_slack_profile_tool(connector_name, tool_title) {
if !is_slack_channel_lookup_tool(connector_id, tool_title) {
return None;
}
@@ -49,11 +53,11 @@ pub(crate) fn slack_channel_name_from_profile_result(
}
pub(crate) fn slack_send_message_channel_id<'a>(
connector_name: Option<&str>,
connector_id: Option<&str>,
tool_title: Option<&str>,
tool_params: Option<&'a Value>,
) -> Option<&'a str> {
if !is_slack_send_message_tool(connector_name, tool_title) {
if !is_slack_send_message_tool(connector_id, tool_title) {
return None;
}
@@ -64,13 +68,13 @@ pub(crate) fn slack_send_message_channel_id<'a>(
}
pub(crate) fn translated_slack_send_message_tool_params(
connector_name: Option<&str>,
connector_id: Option<&str>,
tool_title: Option<&str>,
tool_params: Option<&Value>,
channel_name: Option<&str>,
) -> Option<Value> {
let tool_params = tool_params?;
if !is_slack_send_message_tool(connector_name, tool_title) {
if !is_slack_send_message_tool(connector_id, tool_title) {
return Some(tool_params.clone());
}
@@ -89,26 +93,28 @@ pub(crate) fn translated_slack_send_message_tool_params(
Some(Value::Object(translated))
}
fn is_slack_profile_tool(connector_name: Option<&str>, tool_title: Option<&str>) -> bool {
is_slack_connector(connector_name)
fn is_slack_channel_lookup_tool(connector_id: Option<&str>, tool_title: Option<&str>) -> bool {
is_slack_connector(connector_id)
&& matches!(
tool_title.map(str::trim),
Some(SLACK_GET_PROFILE_TOOL_TITLE) | Some(SLACK_READ_USER_PROFILE_TOOL_TITLE)
Some(SLACK_GET_PROFILE_TOOL_TITLE)
| Some(SLACK_READ_USER_PROFILE_TOOL_TITLE)
| Some(SLACK_SEARCH_USERS_TOOL_TITLE)
)
}
fn is_slack_send_message_tool(connector_name: Option<&str>, tool_title: Option<&str>) -> bool {
is_slack_connector(connector_name)
fn is_slack_send_message_tool(connector_id: Option<&str>, tool_title: Option<&str>) -> bool {
is_slack_connector(connector_id)
&& matches!(
tool_title.map(str::trim),
Some(SLACK_SEND_MESSAGE_TOOL_TITLE)
)
}
fn is_slack_connector(connector_name: Option<&str>) -> bool {
connector_name
fn is_slack_connector(connector_id: Option<&str>) -> bool {
connector_id
.map(str::trim)
.is_some_and(|connector_name| connector_name.starts_with("Slack"))
.is_some_and(|connector_id| connector_id == SLACK_CONNECTOR_ID)
}
fn parse_payload_from_text_content(content: &[Value]) -> Option<Value> {
@@ -130,10 +136,13 @@ fn raw_text_content(content: &[Value]) -> Option<&str> {
fn slack_channel_name_from_payload(payload: &Value) -> Option<SlackChannelName> {
let payload = payload.as_object()?;
if let Some(result_text) = payload.get(RESULT_FIELD_NAME).and_then(nonempty_string) {
let result = payload
.get(RESULT_FIELD_NAME)
.or_else(|| payload.get(RESULTS_FIELD_NAME));
if let Some(result_text) = result.and_then(nonempty_string) {
return parse_slack_channel_name_from_text(result_text);
}
if let Some(result_object) = payload.get(RESULT_FIELD_NAME).and_then(Value::as_object) {
if let Some(result_object) = result.and_then(Value::as_object) {
return slack_channel_name_from_profile_object(result_object);
}
@@ -269,8 +278,8 @@ mod tests {
};
assert_eq!(
slack_channel_name_from_profile_result(
Some("Slack Codex App"),
slack_channel_name_from_tool_result(
Some(SLACK_CONNECTOR_ID),
Some("get_profile"),
&result
),
@@ -294,8 +303,8 @@ mod tests {
};
assert_eq!(
slack_channel_name_from_profile_result(
Some("Slack"),
slack_channel_name_from_tool_result(
Some(SLACK_CONNECTOR_ID),
Some("slack_read_user_profile"),
&result
),
@@ -318,8 +327,8 @@ mod tests {
};
assert_eq!(
slack_channel_name_from_profile_result(
Some("Slack"),
slack_channel_name_from_tool_result(
Some(SLACK_CONNECTOR_ID),
Some("slack_read_user_profile"),
&result
),
@@ -345,16 +354,68 @@ mod tests {
};
assert_eq!(
slack_channel_name_from_profile_result(Some("Linear"), Some("get_profile"), &result),
slack_channel_name_from_tool_result(
Some("connector_linear"),
Some("get_profile"),
&result
),
None
);
}
#[test]
fn extracts_slack_channel_name_from_search_users_result() {
let result = CallToolResult {
content: Vec::new(),
structured_content: Some(json!({
"results": "# Search Results for: mzeng@openai.com\n\n## Users (1 results)\n### Result 1 of 1\nName: Matthew Zeng\nUser ID: U07B9LBRPST\nTitle: Codexing agent\nEmail: mzeng@openai.com",
})),
is_error: Some(false),
meta: None,
};
assert_eq!(
slack_channel_name_from_tool_result(
Some(SLACK_CONNECTOR_ID),
Some("slack_search_users"),
&result
),
Some(SlackChannelName {
channel_id: "U07B9LBRPST".to_string(),
channel_name: "Matthew Zeng".to_string(),
})
);
}
#[test]
fn extracts_slack_channel_name_from_read_user_profile_results_result() {
let result = CallToolResult {
content: Vec::new(),
structured_content: Some(json!({
"results": "# Search Results for: mzeng@openai.com\n\n## Users (1 results)\n### Result 1 of 1\nName: Matthew Zeng\nUser ID: U07B9LBRPST\nTitle: Codexing agent\nEmail: mzeng@openai.com",
})),
is_error: Some(false),
meta: None,
};
assert_eq!(
slack_channel_name_from_tool_result(
Some(SLACK_CONNECTOR_ID),
Some("slack_read_user_profile"),
&result
),
Some(SlackChannelName {
channel_id: "U07B9LBRPST".to_string(),
channel_name: "Matthew Zeng".to_string(),
})
);
}
#[test]
fn translates_slack_send_message_tool_params() {
assert_eq!(
translated_slack_send_message_tool_params(
Some("Slack"),
Some(SLACK_CONNECTOR_ID),
Some("slack_send_message"),
Some(&json!({
"channel_id": "U123",
@@ -374,7 +435,7 @@ mod tests {
fn leaves_non_slack_send_message_tool_params_unchanged() {
assert_eq!(
translated_slack_send_message_tool_params(
Some("Slack"),
Some(SLACK_CONNECTOR_ID),
Some("slack_search_channels"),
Some(&json!({
"query": "eng",