mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Add plugin id to MCP tool call items (#23737)
Add owning plugin id to MCP tool call items so we can better filter them at plugin level. ## Summary - add optional `plugin_id` to MCP tool-call items and legacy begin/end events - propagate plugin metadata into emitted core items and app-server v2 `ThreadItem::McpToolCall` - preserve plugin ids through app-server replay/redaction paths and regenerate v2 schema fixtures ## Testing - `just write-app-server-schema` - `just fmt` - `just fix -p codex-core` - `cargo test -p codex-protocol -p codex-app-server-protocol` - `cargo test -p codex-app-server-protocol` - `cargo test -p codex-core mcp_tool_call_item_includes_plugin_id --lib` - `cargo check -p codex-tui --tests` - `cargo check -p codex-app-server --tests` - `git diff --check` ## Notes - `just fix -p codex-core` completed with two non-fatal `too_many_arguments` warnings on the touched MCP notification helpers. - A broader `cargo test -p codex-core` run passed core unit tests, then hit shell/sandbox/snapshot failures in the integration target. - A broader app-server downstream run hit the existing `in_process::tests::in_process_start_clamps_zero_channel_capacity` stack overflow; `cargo test -p codex-exec` also hit the existing sandbox expectation mismatch in `thread_lifecycle_params_include_legacy_sandbox_when_no_active_profile`.
This commit is contained in:
committed by
GitHub
Unverified
parent
0b5cf85b64
commit
0a4179bb19
@@ -138,9 +138,14 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
|
||||
let metadata =
|
||||
lookup_mcp_tool_metadata(sess.as_ref(), turn_context.as_ref(), &server, &tool_name).await;
|
||||
let mcp_app_resource_uri = metadata
|
||||
.as_ref()
|
||||
.and_then(|metadata| metadata.mcp_app_resource_uri.clone());
|
||||
let item_metadata = McpToolCallItemMetadata {
|
||||
mcp_app_resource_uri: metadata
|
||||
.as_ref()
|
||||
.and_then(|metadata| metadata.mcp_app_resource_uri.clone()),
|
||||
plugin_id: metadata
|
||||
.as_ref()
|
||||
.and_then(|metadata| metadata.plugin_id.clone()),
|
||||
};
|
||||
let app_tool_policy = if server == CODEX_APPS_MCP_SERVER_NAME {
|
||||
connectors::app_tool_policy(
|
||||
&turn_context.config,
|
||||
@@ -171,7 +176,7 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
turn_context.as_ref(),
|
||||
&call_id,
|
||||
invocation,
|
||||
mcp_app_resource_uri.clone(),
|
||||
item_metadata.clone(),
|
||||
"MCP tool call blocked by app configuration".to_string(),
|
||||
/*already_started*/ false,
|
||||
)
|
||||
@@ -200,7 +205,7 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
turn_context.as_ref(),
|
||||
&call_id,
|
||||
invocation.clone(),
|
||||
mcp_app_resource_uri.clone(),
|
||||
item_metadata.clone(),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -225,7 +230,7 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
&call_id,
|
||||
invocation,
|
||||
metadata.as_ref(),
|
||||
mcp_app_resource_uri,
|
||||
item_metadata,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
@@ -236,7 +241,7 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
turn_context.as_ref(),
|
||||
&call_id,
|
||||
invocation,
|
||||
mcp_app_resource_uri.clone(),
|
||||
item_metadata.clone(),
|
||||
message,
|
||||
/*already_started*/ true,
|
||||
)
|
||||
@@ -249,7 +254,7 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
turn_context.as_ref(),
|
||||
&call_id,
|
||||
invocation,
|
||||
mcp_app_resource_uri.clone(),
|
||||
item_metadata.clone(),
|
||||
message,
|
||||
/*already_started*/ true,
|
||||
)
|
||||
@@ -280,7 +285,7 @@ pub(crate) async fn handle_mcp_tool_call(
|
||||
&call_id,
|
||||
invocation,
|
||||
metadata.as_ref(),
|
||||
mcp_app_resource_uri,
|
||||
item_metadata,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -290,13 +295,19 @@ pub(crate) struct HandledMcpToolCall {
|
||||
pub(crate) tool_input: JsonValue,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
struct McpToolCallItemMetadata {
|
||||
mcp_app_resource_uri: Option<String>,
|
||||
plugin_id: Option<String>,
|
||||
}
|
||||
|
||||
async fn handle_approved_mcp_tool_call(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
call_id: &str,
|
||||
invocation: McpInvocation,
|
||||
metadata: Option<&McpToolApprovalMetadata>,
|
||||
mcp_app_resource_uri: Option<String>,
|
||||
item_metadata: McpToolCallItemMetadata,
|
||||
) -> HandledMcpToolCall {
|
||||
let server = invocation.server.clone();
|
||||
maybe_mark_thread_memory_mode_polluted(sess, turn_context, &server).await;
|
||||
@@ -365,7 +376,7 @@ async fn handle_approved_mcp_tool_call(
|
||||
turn_context,
|
||||
call_id,
|
||||
invocation,
|
||||
mcp_app_resource_uri,
|
||||
item_metadata,
|
||||
duration,
|
||||
truncate_mcp_tool_result_for_event(&result),
|
||||
)
|
||||
@@ -840,7 +851,7 @@ async fn notify_mcp_tool_call_started(
|
||||
turn_context: &TurnContext,
|
||||
call_id: &str,
|
||||
invocation: McpInvocation,
|
||||
mcp_app_resource_uri: Option<String>,
|
||||
item_metadata: McpToolCallItemMetadata,
|
||||
) {
|
||||
let McpInvocation {
|
||||
server,
|
||||
@@ -852,7 +863,8 @@ async fn notify_mcp_tool_call_started(
|
||||
server,
|
||||
tool,
|
||||
arguments: arguments.unwrap_or(JsonValue::Null),
|
||||
mcp_app_resource_uri,
|
||||
mcp_app_resource_uri: item_metadata.mcp_app_resource_uri,
|
||||
plugin_id: item_metadata.plugin_id,
|
||||
status: McpToolCallStatus::InProgress,
|
||||
result: None,
|
||||
error: None,
|
||||
@@ -866,7 +878,7 @@ async fn notify_mcp_tool_call_completed(
|
||||
turn_context: &TurnContext,
|
||||
call_id: &str,
|
||||
invocation: McpInvocation,
|
||||
mcp_app_resource_uri: Option<String>,
|
||||
item_metadata: McpToolCallItemMetadata,
|
||||
duration: Duration,
|
||||
result: Result<CallToolResult, String>,
|
||||
) {
|
||||
@@ -891,7 +903,8 @@ async fn notify_mcp_tool_call_completed(
|
||||
server,
|
||||
tool,
|
||||
arguments: arguments.unwrap_or(JsonValue::Null),
|
||||
mcp_app_resource_uri,
|
||||
mcp_app_resource_uri: item_metadata.mcp_app_resource_uri,
|
||||
plugin_id: item_metadata.plugin_id,
|
||||
status,
|
||||
result,
|
||||
error,
|
||||
@@ -2092,7 +2105,7 @@ async fn notify_mcp_tool_call_skip(
|
||||
turn_context: &TurnContext,
|
||||
call_id: &str,
|
||||
invocation: McpInvocation,
|
||||
mcp_app_resource_uri: Option<String>,
|
||||
item_metadata: McpToolCallItemMetadata,
|
||||
message: String,
|
||||
already_started: bool,
|
||||
) -> Result<CallToolResult, String> {
|
||||
@@ -2102,7 +2115,7 @@ async fn notify_mcp_tool_call_skip(
|
||||
turn_context,
|
||||
call_id,
|
||||
invocation.clone(),
|
||||
mcp_app_resource_uri.clone(),
|
||||
item_metadata.clone(),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
@@ -2112,7 +2125,7 @@ async fn notify_mcp_tool_call_skip(
|
||||
turn_context,
|
||||
call_id,
|
||||
invocation,
|
||||
mcp_app_resource_uri,
|
||||
item_metadata,
|
||||
Duration::ZERO,
|
||||
truncate_mcp_tool_result_for_event(&Err(message.clone())),
|
||||
)
|
||||
|
||||
@@ -1112,6 +1112,40 @@ async fn plugin_mcp_tool_call_request_meta_includes_plugin_id() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_tool_call_item_includes_plugin_id() {
|
||||
let (session, turn_context, rx_event) = make_session_and_context_with_rx().await;
|
||||
|
||||
notify_mcp_tool_call_started(
|
||||
&session,
|
||||
&turn_context,
|
||||
"call-plugin",
|
||||
McpInvocation {
|
||||
server: "sample".to_string(),
|
||||
tool: "echo".to_string(),
|
||||
arguments: None,
|
||||
},
|
||||
McpToolCallItemMetadata {
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: Some("sample@test".to_string()),
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let event = tokio::time::timeout(std::time::Duration::from_secs(1), rx_event.recv())
|
||||
.await
|
||||
.expect("tool call item timed out")
|
||||
.expect("tool call item event");
|
||||
let EventMsg::ItemStarted(item_started) = event.msg else {
|
||||
panic!("expected ItemStarted event");
|
||||
};
|
||||
let TurnItem::McpToolCall(item) = item_started.item else {
|
||||
panic!("expected MCP tool call item");
|
||||
};
|
||||
|
||||
assert_eq!(item.plugin_id.as_deref(), Some("sample@test"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps_meta() {
|
||||
let (_, turn_context) = make_session_and_context().await;
|
||||
|
||||
@@ -202,6 +202,7 @@ async fn emit_tool_call_begin(
|
||||
tool,
|
||||
arguments: arguments.unwrap_or(Value::Null),
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
status: McpToolCallStatus::InProgress,
|
||||
result: None,
|
||||
error: None,
|
||||
@@ -240,6 +241,7 @@ async fn emit_tool_call_end(
|
||||
tool,
|
||||
arguments: arguments.unwrap_or(Value::Null),
|
||||
mcp_app_resource_uri: None,
|
||||
plugin_id: None,
|
||||
status,
|
||||
result,
|
||||
error,
|
||||
|
||||
Reference in New Issue
Block a user