Include plugin id in plugin MCP tool metadata (#23353)

Adding the id of the plugin that contains the MCP (if any) so we can apply filters at plugin level. ## Summary - carry the plugin owner into MCP runtime provenance - attach `plugin_id` to outbound plugin-backed MCP tool-call `_meta` - avoid misattributing user-configured MCP servers that shadow plugin server names ## Testing - `just fmt` - `just fix -p codex-mcp` - `just fix -p codex-core` - `cargo test -p codex-mcp` - `cargo test -p codex-core plugin_mcp_tool_call_request_meta_includes_plugin_id` - `cargo test -p codex-core to_mcp_config_omits_plugin_id_when_user_server_shadows_plugin_mcp` - `cargo test -p codex-core rebuild_preserving_session_layers_refreshes_plugin_derived_mcp_config` - `git diff --check` ## Notes - Attempted `cargo test -p codex-core`; it aborted in `agent::control::tests::resume_agent_from_rollout_skips_descendants_when_parent_resume_fails` with a stack overflow before the full suite completed.
2026-07-01 00:31:56 +08:00 · 2026-05-18 15:33:33 -07:00
parent f2368b7de6
commit a66e0e9c4b
7 changed files with 155 additions and 13 deletions
@@ -71,6 +71,7 @@ use tracing::warn;
 pub struct McpConnectionManager {
    clients: HashMap<String, AsyncManagedClient>,
    server_metadata: HashMap<String, McpServerMetadata>,
+    tool_plugin_provenance: Arc<ToolPluginProvenance>,
    host_owned_codex_apps_enabled: bool,
    elicitation_requests: ElicitationRequestManager,
    startup_cancellation_token: CancellationToken,
@@ -91,6 +92,7 @@ impl McpConnectionManager {
        Self {
            clients: HashMap::new(),
            server_metadata: HashMap::new(),
+            tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()),
            host_owned_codex_apps_enabled: false,
            elicitation_requests: ElicitationRequestManager::new(
                approval_policy.value(),
@@ -136,6 +138,11 @@ impl McpConnectionManager {
            .is_none_or(|metadata| metadata.pollutes_memory)
    }

+    pub fn plugin_id_for_mcp_server_name(&self, server_name: &str) -> Option<&str> {
+        self.tool_plugin_provenance
+            .plugin_id_for_mcp_server_name(server_name)
+    }
+
    pub fn is_host_owned_codex_apps_server(&self, server_name: &str) -> bool {
        self.host_owned_codex_apps_enabled && server_name == CODEX_APPS_MCP_SERVER_NAME
    }
@@ -283,6 +290,7 @@ impl McpConnectionManager {
        let manager = Self {
            clients,
            server_metadata,
+            tool_plugin_provenance,
            host_owned_codex_apps_enabled,
            elicitation_requests: elicitation_requests.clone(),
            startup_cancellation_token: cancel_token.clone(),
@@ -138,6 +138,8 @@ pub struct McpConfig {
    ///
    /// Runtime-only additions are merged later by [`effective_mcp_servers`].
    pub configured_mcp_servers: HashMap<String, McpServerConfig>,
+    /// Winning plugin owner for plugin-provided MCP servers, keyed by server name.
+    pub plugin_ids_by_mcp_server_name: HashMap<String, String>,
    /// Plugin metadata used to attribute MCP tools/connectors to plugin display names.
    pub plugin_capability_summaries: Vec<PluginCapabilitySummary>,
 }
@@ -146,6 +148,7 @@ pub struct McpConfig {
 pub struct ToolPluginProvenance {
    plugin_display_names_by_connector_id: HashMap<String, Vec<String>>,
    plugin_display_names_by_mcp_server_name: HashMap<String, Vec<String>>,
+    plugin_ids_by_mcp_server_name: HashMap<String, String>,
 }

 impl ToolPluginProvenance {
@@ -163,9 +166,15 @@ impl ToolPluginProvenance {
            .unwrap_or(&[])
    }

-    fn from_capability_summaries(capability_summaries: &[PluginCapabilitySummary]) -> Self {
+    pub fn plugin_id_for_mcp_server_name(&self, server_name: &str) -> Option<&str> {
+        self.plugin_ids_by_mcp_server_name
+            .get(server_name)
+            .map(String::as_str)
+    }
+
+    fn from_config(config: &McpConfig) -> Self {
        let mut tool_plugin_provenance = Self::default();
-        for plugin in capability_summaries {
+        for plugin in &config.plugin_capability_summaries {
            for connector_id in &plugin.app_connector_ids {
                tool_plugin_provenance
                    .plugin_display_names_by_connector_id
@@ -195,6 +204,8 @@ impl ToolPluginProvenance {
            plugin_names.sort_unstable();
            plugin_names.dedup();
        }
+        tool_plugin_provenance.plugin_ids_by_mcp_server_name =
+            config.plugin_ids_by_mcp_server_name.clone();

        tool_plugin_provenance
    }
@@ -244,7 +255,7 @@ pub fn effective_mcp_servers_from_configured(
 }

 pub fn tool_plugin_provenance(config: &McpConfig) -> ToolPluginProvenance {
-    ToolPluginProvenance::from_capability_summaries(&config.plugin_capability_summaries)
+    ToolPluginProvenance::from_config(config)
 }

 pub async fn read_mcp_resource(
@@ -30,6 +30,7 @@ fn test_mcp_config(codex_home: PathBuf) -> McpConfig {
        apps_enabled: false,
        client_elicitation_capability: ElicitationCapability::default(),
        configured_mcp_servers: HashMap::new(),
+        plugin_ids_by_mcp_server_name: HashMap::new(),
        plugin_capability_summaries: Vec::new(),
    }
 }
@@ -124,7 +125,10 @@ fn mcp_prompt_auto_approval_rejects_auto_mode_in_default_permission_mode() {

 #[test]
 fn tool_plugin_provenance_collects_app_and_mcp_sources() {
-    let provenance = ToolPluginProvenance::from_capability_summaries(&[
+    let mut config = test_mcp_config(PathBuf::new());
+    config.plugin_ids_by_mcp_server_name =
+        HashMap::from([("alpha".to_string(), "alpha@test".to_string())]);
+    config.plugin_capability_summaries = vec![
        PluginCapabilitySummary {
            display_name: "alpha-plugin".to_string(),
            app_connector_ids: vec![AppConnectorId("connector_example".to_string())],
@@ -140,7 +144,8 @@ fn tool_plugin_provenance_collects_app_and_mcp_sources() {
            mcp_server_names: vec!["beta".to_string()],
            ..PluginCapabilitySummary::default()
        },
-    ]);
+    ];
+    let provenance = tool_plugin_provenance(&config);

    assert_eq!(
        provenance,
@@ -159,8 +164,17 @@ fn tool_plugin_provenance_collects_app_and_mcp_sources() {
                ("alpha".to_string(), vec!["alpha-plugin".to_string()]),
                ("beta".to_string(), vec!["beta-plugin".to_string()]),
            ]),
+            plugin_ids_by_mcp_server_name: HashMap::from([(
+                "alpha".to_string(),
+                "alpha@test".to_string(),
+            )]),
        }
    );
+    assert_eq!(
+        provenance.plugin_id_for_mcp_server_name("alpha"),
+        Some("alpha@test")
+    );
+    assert_eq!(provenance.plugin_id_for_mcp_server_name("beta"), None);
 }

 #[test]
@@ -3849,6 +3849,63 @@ async fn rebuild_preserving_session_layers_refreshes_plugin_derived_mcp_config()
        mcp_config.configured_mcp_servers.get("sample"),
        Some(&http_mcp("https://sample.example/mcp"))
    );
+    assert_eq!(
+        mcp_config.plugin_ids_by_mcp_server_name,
+        HashMap::from([("sample".to_string(), "sample@test".to_string())])
+    );
+
+    Ok(())
+}
+
+#[tokio::test]
+async fn to_mcp_config_omits_plugin_id_when_user_server_shadows_plugin_mcp() -> anyhow::Result<()> {
+    let codex_home = TempDir::new()?;
+    let plugin_root = codex_home
+        .path()
+        .join("plugins/cache")
+        .join("test/sample/local");
+    std::fs::create_dir_all(plugin_root.join(".codex-plugin"))?;
+    std::fs::write(
+        plugin_root.join(".codex-plugin/plugin.json"),
+        r#"{"name":"sample"}"#,
+    )?;
+    std::fs::write(
+        plugin_root.join(".mcp.json"),
+        r#"{
+  "mcpServers": {
+    "sample": {
+      "type": "http",
+      "url": "https://plugin.example/mcp"
+    }
+  }
+}"#,
+    )?;
+    std::fs::write(
+        codex_home.path().join(CONFIG_TOML_FILE),
+        r#"
+[features]
+plugins = true
+
+[mcp_servers.sample]
+url = "https://user.example/mcp"
+
+[plugins."sample@test"]
+enabled = true
+"#,
+    )?;
+
+    let config = ConfigBuilder::default()
+        .codex_home(codex_home.path().to_path_buf())
+        .build()
+        .await?;
+    let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf());
+    let mcp_config = config.to_mcp_config(&plugins_manager).await;
+
+    assert_eq!(
+        mcp_config.configured_mcp_servers.get("sample"),
+        Some(&http_mcp("https://user.example/mcp"))
+    );
+    assert!(mcp_config.plugin_ids_by_mcp_server_name.is_empty());

    Ok(())
 }
@@ -107,6 +107,7 @@ use serde::Serialize;
 use std::collections::BTreeMap;
 use std::collections::HashMap;
 use std::collections::HashSet;
+use std::collections::hash_map::Entry;
 use std::io::ErrorKind;
 use std::path::Path;
 use std::path::PathBuf;
@@ -1243,6 +1244,7 @@ impl Config {
        let plugins_input = self.plugins_config_input();
        let loaded_plugins = plugins_manager.plugins_for_config(&plugins_input).await;
        let mut configured_mcp_servers = self.mcp_servers.get().clone();
+        let mut plugin_ids_by_mcp_server_name = HashMap::new();
        for plugin in loaded_plugins
            .plugins()
            .iter()
@@ -1255,7 +1257,10 @@ impl Config {
                self.config_layer_stack.requirements().plugins.as_ref(),
            );
            for (name, plugin_server) in plugin_mcp_servers {
-                configured_mcp_servers.entry(name).or_insert(plugin_server);
+                if let Entry::Vacant(entry) = configured_mcp_servers.entry(name.clone()) {
+                    entry.insert(plugin_server);
+                    plugin_ids_by_mcp_server_name.insert(name, plugin.config_name.clone());
+                }
            }
        }
        if let Some(mcp_requirements) = self.config_layer_stack.requirements().mcp_servers.as_ref()
@@ -1265,6 +1270,8 @@ impl Config {
            // above.
            filter_mcp_servers_by_requirements(&mut configured_mcp_servers, Some(mcp_requirements));
        }
+        plugin_ids_by_mcp_server_name
+            .retain(|server_name, _| configured_mcp_servers.contains_key(server_name));

        McpConfig {
            chatgpt_base_url: self.chatgpt_base_url.clone(),
@@ -1292,6 +1299,7 @@ impl Config {
                ElicitationCapability::default()
            },
            configured_mcp_servers,
+            plugin_ids_by_mcp_server_name,
            plugin_capability_summaries: loaded_plugins.capability_summaries().to_vec(),
        }
    }
@@ -974,6 +974,7 @@ pub(crate) struct McpToolApprovalMetadata {
    connector_id: Option<String>,
    connector_name: Option<String>,
    connector_description: Option<String>,
+    plugin_id: Option<String>,
    tool_title: Option<String>,
    tool_description: Option<String>,
    mcp_app_resource_uri: Option<String>,
@@ -983,6 +984,7 @@ pub(crate) struct McpToolApprovalMetadata {

 const MCP_TOOL_OPENAI_OUTPUT_TEMPLATE_META_KEY: &str = "openai/outputTemplate";
 const MCP_TOOL_UI_RESOURCE_URI_META_KEY: &str = "ui/resourceUri";
+const MCP_TOOL_PLUGIN_ID_META_KEY: &str = "plugin_id";
 const MCP_TOOL_THREAD_ID_META_KEY: &str = "threadId";

 async fn custom_mcp_tool_approval_mode(
@@ -1068,6 +1070,12 @@ fn build_mcp_tool_call_request_meta(
            serde_json::Value::Object(codex_apps_meta),
        );
    }
+    if let Some(plugin_id) = metadata.and_then(|metadata| metadata.plugin_id.as_ref()) {
+        request_meta.insert(
+            MCP_TOOL_PLUGIN_ID_META_KEY.to_string(),
+            serde_json::Value::String(plugin_id.clone()),
+        );
+    }

    (!request_meta.is_empty()).then_some(serde_json::Value::Object(request_meta))
 }
@@ -1480,13 +1488,11 @@ pub(crate) async fn lookup_mcp_tool_metadata(
    server: &str,
    tool_name: &str,
 ) -> Option<McpToolApprovalMetadata> {
-    let tools = sess
-        .services
-        .mcp_connection_manager
-        .read()
-        .await
-        .list_all_tools()
-        .await;
+    let manager = sess.services.mcp_connection_manager.read().await;
+    let plugin_id = manager
+        .plugin_id_for_mcp_server_name(server)
+        .map(str::to_string);
+    let tools = manager.list_all_tools().await;
    let tool_info = tools
        .into_iter()
        .find(|tool_info| tool_info.server_name == server && tool_info.tool.name == tool_name)?;
@@ -1519,6 +1525,7 @@ pub(crate) async fn lookup_mcp_tool_metadata(
        connector_id: tool_info.connector_id,
        connector_name: tool_info.connector_name,
        connector_description,
+        plugin_id,
        tool_title: tool_info.tool.title,
        tool_description: tool_info.tool.description.map(std::borrow::Cow::into_owned),
        mcp_app_resource_uri: get_mcp_app_resource_uri(tool_info.tool.meta.as_deref()),
@@ -76,6 +76,7 @@ fn approval_metadata(
        connector_id: connector_id.map(str::to_string),
        connector_name: connector_name.map(str::to_string),
        connector_description: connector_description.map(str::to_string),
+        plugin_id: None,
        tool_title: tool_title.map(str::to_string),
        tool_description: tool_description.map(str::to_string),
        mcp_app_resource_uri: None,
@@ -1099,6 +1100,29 @@ async fn mcp_tool_call_request_meta_includes_turn_started_at_unix_ms() {
    );
 }

+#[tokio::test]
+async fn plugin_mcp_tool_call_request_meta_includes_plugin_id() {
+    let (_, turn_context) = make_session_and_context().await;
+    let expected_turn_metadata = turn_context
+        .turn_metadata_state
+        .current_meta_value_for_mcp_request(mcp_turn_metadata_context(&turn_context))
+        .expect("turn metadata");
+    let mut metadata = approval_metadata(
+        /*connector_id*/ None, /*connector_name*/ None,
+        /*connector_description*/ None, /*tool_title*/ None,
+        /*tool_description*/ None,
+    );
+    metadata.plugin_id = Some("sample@test".to_string());
+
+    assert_eq!(
+        build_mcp_tool_call_request_meta(&turn_context, "sample", "call-plugin", Some(&metadata),),
+        Some(serde_json::json!({
+            crate::X_CODEX_TURN_METADATA_HEADER: expected_turn_metadata,
+            MCP_TOOL_PLUGIN_ID_META_KEY: "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;
@@ -1111,6 +1135,7 @@ async fn codex_apps_tool_call_request_meta_includes_turn_metadata_and_codex_apps
        connector_id: Some("calendar".to_string()),
        connector_name: Some("Calendar".to_string()),
        connector_description: Some("Manage events".to_string()),
+        plugin_id: None,
        tool_title: Some("Create Event".to_string()),
        tool_description: Some("Create a calendar event.".to_string()),
        mcp_app_resource_uri: None,
@@ -1573,6 +1598,7 @@ fn guardian_mcp_review_request_includes_annotations_when_present() {
        connector_id: None,
        connector_name: None,
        connector_description: None,
+        plugin_id: None,
        tool_title: None,
        tool_description: None,
        mcp_app_resource_uri: None,
@@ -2290,6 +2316,7 @@ async fn approve_mode_skips_when_annotations_do_not_require_approval() {
        connector_id: None,
        connector_name: None,
        connector_description: None,
+        plugin_id: None,
        tool_title: Some("Read Only Tool".to_string()),
        tool_description: None,
        mcp_app_resource_uri: None,
@@ -2363,6 +2390,7 @@ async fn guardian_mode_skips_auto_when_annotations_do_not_require_approval() {
        connector_id: None,
        connector_name: None,
        connector_description: None,
+        plugin_id: None,
        tool_title: Some("Read Only Tool".to_string()),
        tool_description: None,
        mcp_app_resource_uri: None,
@@ -2419,6 +2447,7 @@ async fn permission_request_hook_allows_mcp_tool_call() {
        connector_id: None,
        connector_name: None,
        connector_description: None,
+        plugin_id: None,
        tool_title: Some("Create entities".to_string()),
        tool_description: None,
        mcp_app_resource_uri: None,
@@ -2554,6 +2583,7 @@ async fn permission_request_hook_runs_after_remembered_mcp_approval() {
        connector_id: None,
        connector_name: None,
        connector_description: None,
+        plugin_id: None,
        tool_title: Some("Create entities".to_string()),
        tool_description: None,
        mcp_app_resource_uri: None,
@@ -2640,6 +2670,7 @@ async fn guardian_mode_mcp_denial_returns_rationale_message() {
        connector_id: None,
        connector_name: None,
        connector_description: None,
+        plugin_id: None,
        tool_title: Some("Dangerous Tool".to_string()),
        tool_description: Some("Reads calendar data.".to_string()),
        mcp_app_resource_uri: None,
@@ -2693,6 +2724,7 @@ async fn prompt_mode_waits_for_approval_when_annotations_do_not_require_approval
        connector_id: None,
        connector_name: None,
        connector_description: None,
+        plugin_id: None,
        tool_title: Some("Read Only Tool".to_string()),
        tool_description: None,
        mcp_app_resource_uri: None,
@@ -2772,6 +2804,7 @@ async fn approve_mode_skips_arc_interrupt_for_model() {
        connector_id: Some("calendar".to_string()),
        connector_name: Some("Calendar".to_string()),
        connector_description: Some("Manage events".to_string()),
+        plugin_id: None,
        tool_title: Some("Dangerous Tool".to_string()),
        tool_description: Some("Performs a risky action.".to_string()),
        mcp_app_resource_uri: None,
@@ -2839,6 +2872,7 @@ async fn custom_approve_mode_skips_arc_interrupt_for_model() {
        connector_id: None,
        connector_name: None,
        connector_description: None,
+        plugin_id: None,
        tool_title: Some("Dangerous Tool".to_string()),
        tool_description: Some("Performs a risky action.".to_string()),
        mcp_app_resource_uri: None,
@@ -2906,6 +2940,7 @@ async fn approve_mode_skips_arc_interrupt_without_annotations() {
        connector_id: Some("calendar".to_string()),
        connector_name: Some("Calendar".to_string()),
        connector_description: Some("Manage events".to_string()),
+        plugin_id: None,
        tool_title: Some("Dangerous Tool".to_string()),
        tool_description: Some("Performs a risky action.".to_string()),
        mcp_app_resource_uri: None,
@@ -2978,6 +3013,7 @@ async fn full_access_mode_skips_arc_monitor_for_all_approval_modes() {
        connector_id: Some("calendar".to_string()),
        connector_name: Some("Calendar".to_string()),
        connector_description: Some("Manage events".to_string()),
+        plugin_id: None,
        tool_title: Some("Dangerous Tool".to_string()),
        tool_description: Some("Performs a risky action.".to_string()),
        mcp_app_resource_uri: None,
@@ -3046,6 +3082,7 @@ async fn approve_mode_skips_arc_and_guardian_in_every_permission_mode() {
        connector_id: Some("calendar".to_string()),
        connector_name: Some("Calendar".to_string()),
        connector_description: Some("Manage events".to_string()),
+        plugin_id: None,
        tool_title: Some("Dangerous Tool".to_string()),
        tool_description: Some("Performs a risky action.".to_string()),
        mcp_app_resource_uri: None,