From 870cd77f427d33da227bdb2a548e277f172ab929 Mon Sep 17 00:00:00 2001 From: Tino Caer Date: Wed, 22 Apr 2026 11:17:15 -0700 Subject: [PATCH] Allow plugin-declared first-party connectors --- codex-rs/Cargo.lock | 2 +- .../app-server/src/codex_message_processor.rs | 4 + .../apps_list_helpers.rs | 9 +- codex-rs/app-server/src/message_processor.rs | 3 + codex-rs/chatgpt/src/connectors.rs | 96 +++++++-- codex-rs/codex-mcp/Cargo.toml | 1 + codex-rs/codex-mcp/src/mcp/mod.rs | 18 +- codex-rs/codex-mcp/src/mcp/mod_tests.rs | 11 + .../codex-mcp/src/mcp_connection_manager.rs | 85 ++++++-- .../src/mcp_connection_manager_tests.rs | 94 +++++++- codex-rs/connectors/src/filter.rs | 109 +++++++++- codex-rs/core/src/connectors.rs | 28 ++- codex-rs/core/src/connectors_tests.rs | 204 ++++++++++++++++-- .../core/src/tools/handlers/tool_suggest.rs | 3 +- codex-rs/tui/src/chatwidget.rs | 10 +- codex-rs/utils/plugins/Cargo.toml | 1 - codex-rs/utils/plugins/src/mcp_connector.rs | 30 --- 17 files changed, 588 insertions(+), 120 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 25c12ac154..6d9e6014d8 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2448,6 +2448,7 @@ dependencies = [ "async-channel", "codex-async-utils", "codex-config", + "codex-connectors", "codex-exec-server", "codex-login", "codex-otel", @@ -3225,7 +3226,6 @@ name = "codex-utils-plugins" version = "0.0.0" dependencies = [ "codex-exec-server", - "codex-login", "codex-utils-absolute-path", "serde", "serde_json", diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 3b92d4199f..424966ba11 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -6268,6 +6268,8 @@ impl CodexMessageProcessor { connectors::list_cached_all_connectors(&config) ); let cached_all_connectors = all_connectors.clone(); + let plugin_declared_connector_ids = + connectors::plugin_declared_connector_ids_for_config(&config).await; let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel(); @@ -6301,6 +6303,7 @@ impl CodexMessageProcessor { apps_list_helpers::merge_loaded_apps( all_connectors.as_deref(), accessible_connectors.as_deref(), + &plugin_declared_connector_ids, ), &config, ); @@ -6387,6 +6390,7 @@ impl CodexMessageProcessor { apps_list_helpers::merge_loaded_apps( all_connectors_for_update, accessible_connectors_for_update, + &plugin_declared_connector_ids, ), &config, ); diff --git a/codex-rs/app-server/src/codex_message_processor/apps_list_helpers.rs b/codex-rs/app-server/src/codex_message_processor/apps_list_helpers.rs index b0a6df4a80..015f7d58df 100644 --- a/codex-rs/app-server/src/codex_message_processor/apps_list_helpers.rs +++ b/codex-rs/app-server/src/codex_message_processor/apps_list_helpers.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use std::sync::Arc; use codex_app_server_protocol::AppInfo; @@ -13,11 +14,17 @@ use crate::outgoing_message::OutgoingMessageSender; pub(super) fn merge_loaded_apps( all_connectors: Option<&[AppInfo]>, accessible_connectors: Option<&[AppInfo]>, + plugin_declared_connector_ids: &HashSet, ) -> Vec { let all_connectors_loaded = all_connectors.is_some(); let all = all_connectors.map_or_else(Vec::new, <[AppInfo]>::to_vec); let accessible = accessible_connectors.map_or_else(Vec::new, <[AppInfo]>::to_vec); - connectors::merge_connectors_with_accessible(all, accessible, all_connectors_loaded) + connectors::merge_connectors_with_accessible( + all, + accessible, + all_connectors_loaded, + plugin_declared_connector_ids, + ) } pub(super) fn should_send_app_list_updated_notification( diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 53d9f2df4c..62506a34ed 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -1066,12 +1066,15 @@ impl MessageProcessor { return; } }; + let plugin_declared_connector_ids = + connectors::plugin_declared_connector_ids_for_config(&config).await; let data = connectors::with_app_enabled_state( connectors::merge_connectors_with_accessible( all_connectors, accessible_connectors, /*all_connectors_loaded*/ true, + &plugin_declared_connector_ids, ), &config, ); diff --git a/codex-rs/chatgpt/src/connectors.rs b/codex-rs/chatgpt/src/connectors.rs index 5f6efbc124..57a038e2c1 100644 --- a/codex-rs/chatgpt/src/connectors.rs +++ b/codex-rs/chatgpt/src/connectors.rs @@ -47,9 +47,13 @@ pub async fn list_connectors(config: &Config) -> anyhow::Result> { ); let connectors = connectors_result?; let accessible = accessible_result?; + let plugin_declared_connector_ids = plugin_declared_connector_ids_for_config(config).await; Ok(with_app_enabled_state( merge_connectors_with_accessible( - connectors, accessible, /*all_connectors_loaded*/ true, + connectors, + accessible, + /*all_connectors_loaded*/ true, + &plugin_declared_connector_ids, ), config, )) @@ -73,16 +77,16 @@ pub async fn list_cached_all_connectors(config: &Config) -> Option> let token_data = get_chatgpt_token_data()?; let cache_key = all_connectors_cache_key(config, &token_data); let connectors = codex_connectors::cached_all_connectors(&cache_key)?; + let plugin_apps = plugin_apps_for_config(config).await; + let plugin_declared_connector_ids = plugin_declared_connector_ids(&plugin_apps); let connectors = merge_plugin_connectors( connectors, - plugin_apps_for_config(config) - .await - .into_iter() - .map(|connector_id| connector_id.0), + plugin_apps.into_iter().map(|connector_id| connector_id.0), ); Some(filter_disallowed_connectors( connectors, originator().value.as_str(), + &plugin_declared_connector_ids, )) } @@ -113,16 +117,16 @@ pub async fn list_all_connectors_with_options( }, ) .await?; + let plugin_apps = plugin_apps_for_config(config).await; + let plugin_declared_connector_ids = plugin_declared_connector_ids(&plugin_apps); let connectors = merge_plugin_connectors( connectors, - plugin_apps_for_config(config) - .await - .into_iter() - .map(|connector_id| connector_id.0), + plugin_apps.into_iter().map(|connector_id| connector_id.0), ); Ok(filter_disallowed_connectors( connectors, originator().value.as_str(), + &plugin_declared_connector_ids, )) } @@ -142,6 +146,17 @@ async fn plugin_apps_for_config(config: &Config) -> Vec HashSet { + plugin_declared_connector_ids(&plugin_apps_for_config(config).await) +} + +fn plugin_declared_connector_ids(plugin_apps: &[AppConnectorId]) -> HashSet { + plugin_apps + .iter() + .map(|connector_id| connector_id.0.clone()) + .collect() +} + pub fn connectors_for_plugin_apps( connectors: Vec, plugin_apps: &[AppConnectorId], @@ -150,6 +165,7 @@ pub fn connectors_for_plugin_apps( .iter() .map(|connector_id| connector_id.0.as_str()) .collect::>(); + let plugin_declared_connector_ids = plugin_declared_connector_ids(plugin_apps); let connectors = merge_plugin_connectors( connectors, @@ -157,16 +173,21 @@ pub fn connectors_for_plugin_apps( .iter() .map(|connector_id| connector_id.0.clone()), ); - filter_disallowed_connectors(connectors, originator().value.as_str()) - .into_iter() - .filter(|connector| plugin_app_ids.contains(connector.id.as_str())) - .collect() + filter_disallowed_connectors( + connectors, + originator().value.as_str(), + &plugin_declared_connector_ids, + ) + .into_iter() + .filter(|connector| plugin_app_ids.contains(connector.id.as_str())) + .collect() } pub fn merge_connectors_with_accessible( connectors: Vec, accessible_connectors: Vec, all_connectors_loaded: bool, + plugin_declared_connector_ids: &HashSet, ) -> Vec { let accessible_connectors = if all_connectors_loaded { let connector_ids: HashSet<&str> = connectors @@ -181,7 +202,11 @@ pub fn merge_connectors_with_accessible( accessible_connectors }; let merged = merge_connectors(connectors, accessible_connectors); - filter_disallowed_connectors(merged, originator().value.as_str()) + filter_disallowed_connectors( + merged, + originator().value.as_str(), + plugin_declared_connector_ids, + ) } #[cfg(test)] @@ -233,6 +258,7 @@ mod tests { vec![app("alpha")], vec![app("alpha"), app("beta")], /*all_connectors_loaded*/ true, + &HashSet::new(), ); assert_eq!(merged, vec![merged_app("alpha", /*is_accessible*/ true)]); } @@ -243,6 +269,7 @@ mod tests { vec![app("alpha")], vec![app("alpha"), app("beta")], /*all_connectors_loaded*/ false, + &HashSet::new(), ); assert_eq!( merged, @@ -253,6 +280,31 @@ mod tests { ); } + #[test] + fn filters_disallowed_accessible_connectors_while_all_loading() { + let plugin_declared_connector_ids = + HashSet::from(["connector_openai_appgarden".to_string()]); + let merged = merge_connectors_with_accessible( + Vec::new(), + vec![ + app("connector_openai_appgarden"), + app("connector_openai_hidden"), + app("asdk_app_6938a94a61d881918ef32cb999ff937c"), + app("beta"), + ], + /*all_connectors_loaded*/ false, + &plugin_declared_connector_ids, + ); + + assert_eq!( + merged, + vec![ + merged_app("beta", /*is_accessible*/ true), + merged_app("connector_openai_appgarden", /*is_accessible*/ true), + ] + ); + } + #[test] fn connectors_for_plugin_apps_returns_only_requested_plugin_apps() { let connectors = connectors_for_plugin_apps( @@ -278,4 +330,20 @@ mod tests { ); assert_eq!(connectors, Vec::::new()); } + + #[test] + fn connectors_for_plugin_apps_returns_openai_prefixed_plugin_apps() { + let connectors = connectors_for_plugin_apps( + Vec::new(), + &[AppConnectorId("connector_openai_appgarden".to_string())], + ); + + assert_eq!( + connectors, + vec![merged_app( + "connector_openai_appgarden", + /*is_accessible*/ false + )] + ); + } } diff --git a/codex-rs/codex-mcp/Cargo.toml b/codex-rs/codex-mcp/Cargo.toml index 0aec1f3aaf..dfe74a85ca 100644 --- a/codex-rs/codex-mcp/Cargo.toml +++ b/codex-rs/codex-mcp/Cargo.toml @@ -16,6 +16,7 @@ anyhow = { workspace = true } async-channel = { workspace = true } codex-async-utils = { workspace = true } codex-config = { workspace = true } +codex-connectors = { workspace = true } codex-exec-server = { workspace = true } codex-login = { workspace = true } codex-otel = { workspace = true } diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 3118c7c171..55029a73df 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -14,6 +14,7 @@ pub use skill_dependencies::canonical_mcp_server_key; pub use skill_dependencies::collect_missing_mcp_dependencies; use std::collections::HashMap; +use std::collections::HashSet; use std::env; use std::path::PathBuf; use std::time::Duration; @@ -152,6 +153,19 @@ impl ToolPluginProvenance { .unwrap_or(&[]) } + pub fn connector_id_is_plugin_declared(&self, connector_id: &str) -> bool { + !self + .plugin_display_names_for_connector_id(connector_id) + .is_empty() + } + + pub fn plugin_declared_connector_ids(&self) -> HashSet { + self.plugin_display_names_by_connector_id + .keys() + .cloned() + .collect() + } + pub fn plugin_display_names_for_mcp_server_name(&self, server_name: &str) -> &[String] { self.plugin_display_names_by_mcp_server_name .get(server_name) @@ -159,7 +173,9 @@ impl ToolPluginProvenance { .unwrap_or(&[]) } - fn from_capability_summaries(capability_summaries: &[PluginCapabilitySummary]) -> Self { + pub(crate) fn from_capability_summaries( + capability_summaries: &[PluginCapabilitySummary], + ) -> Self { let mut tool_plugin_provenance = Self::default(); for plugin in capability_summaries { for connector_id in &plugin.app_connector_ids { diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 8db52a9d83..1b78983f43 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -6,6 +6,7 @@ use codex_plugin::PluginCapabilitySummary; use codex_protocol::protocol::AskForApproval; use pretty_assertions::assert_eq; use std::collections::HashMap; +use std::collections::HashSet; use std::path::PathBuf; fn test_mcp_config(codex_home: PathBuf) -> McpConfig { @@ -123,6 +124,16 @@ fn tool_plugin_provenance_collects_app_and_mcp_sources() { ]), } ); + assert!(provenance.connector_id_is_plugin_declared("connector_example")); + assert!(provenance.connector_id_is_plugin_declared("connector_gmail")); + assert!(!provenance.connector_id_is_plugin_declared("connector_missing")); + assert_eq!( + provenance.plugin_declared_connector_ids(), + HashSet::from([ + "connector_example".to_string(), + "connector_gmail".to_string() + ]) + ); } #[test] diff --git a/codex-rs/codex-mcp/src/mcp_connection_manager.rs b/codex-rs/codex-mcp/src/mcp_connection_manager.rs index 10c48e040e..9bfbf6d7b2 100644 --- a/codex-rs/codex-mcp/src/mcp_connection_manager.rs +++ b/codex-rs/codex-mcp/src/mcp_connection_manager.rs @@ -93,8 +93,9 @@ use url::Url; use codex_config::McpServerConfig; use codex_config::McpServerTransportConfig; +use codex_connectors::filter::connector_id_passes_disallow_filter; use codex_login::CodexAuth; -use codex_utils_plugins::mcp_connector::is_connector_id_allowed; +use codex_login::default_client::originator; use codex_utils_plugins::mcp_connector::sanitize_name; /// Delimiter used to separate MCP tool-name parts. @@ -448,11 +449,11 @@ struct ManagedClient { } impl ManagedClient { - fn listed_tools(&self) -> Vec { + fn listed_tools(&self, tool_plugin_provenance: &ToolPluginProvenance) -> Vec { let total_start = Instant::now(); if let Some(cache_context) = self.codex_apps_tools_cache_context.as_ref() && let CachedCodexAppsToolsLoad::Hit(tools) = - load_cached_codex_apps_tools(cache_context) + load_cached_codex_apps_tools(cache_context, tool_plugin_provenance) { emit_duration( MCP_TOOLS_LIST_DURATION_METRIC, @@ -501,9 +502,11 @@ impl AsyncManagedClient { let startup_snapshot = load_startup_cached_codex_apps_tools_snapshot( &server_name, codex_apps_tools_cache_context.as_ref(), + tool_plugin_provenance.as_ref(), ) .map(|tools| filter_tools(tools, &tool_filter)); let startup_tool_filter = tool_filter; + let startup_tool_plugin_provenance = Arc::clone(&tool_plugin_provenance); let startup_complete = Arc::new(AtomicBool::new(false)); let startup_complete_for_fut = Arc::clone(&startup_complete); let fut = async move { @@ -533,6 +536,7 @@ impl AsyncManagedClient { tx_event, elicitation_requests, codex_apps_tools_cache_context, + tool_plugin_provenance: startup_tool_plugin_provenance, }, ) .or_cancel(&cancel_token) @@ -631,7 +635,7 @@ impl AsyncManagedClient { Some(startup_tools) } else { match self.client().await { - Ok(client) => Some(client.listed_tools()), + Ok(client) => Some(client.listed_tools(self.tool_plugin_provenance.as_ref())), Err(_) => self.startup_snapshot.clone(), } }; @@ -939,10 +943,12 @@ impl McpConnectionManager { /// latest filtered tool map is returned directly to the caller. On /// failure, the existing cache remains unchanged. pub async fn hard_refresh_codex_apps_tools_cache(&self) -> Result> { - let managed_client = self + let async_managed_client = self .clients .get(CODEX_APPS_MCP_SERVER_NAME) - .ok_or_else(|| anyhow!("unknown MCP server '{CODEX_APPS_MCP_SERVER_NAME}'"))? + .ok_or_else(|| anyhow!("unknown MCP server '{CODEX_APPS_MCP_SERVER_NAME}'"))?; + let tool_plugin_provenance = Arc::clone(&async_managed_client.tool_plugin_provenance); + let managed_client = async_managed_client .client() .await .context("failed to get client")?; @@ -954,6 +960,7 @@ impl McpConnectionManager { &managed_client.client, managed_client.tool_timeout, managed_client.server_instructions.as_deref(), + tool_plugin_provenance.as_ref(), ) .await .with_context(|| { @@ -1438,6 +1445,7 @@ async fn start_server_task( tx_event, elicitation_requests, codex_apps_tools_cache_context, + tool_plugin_provenance, } = params; let elicitation = elicitation_capability_for_server(&server_name); let params = InitializeRequestParams { @@ -1481,6 +1489,7 @@ async fn start_server_task( &client, startup_timeout, initialize_result.instructions.as_deref(), + tool_plugin_provenance.as_ref(), ) .await .map_err(StartupOutcomeError::from)?; @@ -1523,6 +1532,7 @@ struct StartServerTaskParams { tx_event: Sender, elicitation_requests: ElicitationRequestManager, codex_apps_tools_cache_context: Option, + tool_plugin_provenance: Arc, } async fn make_rmcp_client( @@ -1648,6 +1658,7 @@ fn write_cached_codex_apps_tools_if_needed( fn load_startup_cached_codex_apps_tools_snapshot( server_name: &str, cache_context: Option<&CodexAppsToolsCacheContext>, + tool_plugin_provenance: &ToolPluginProvenance, ) -> Option> { if server_name != CODEX_APPS_MCP_SERVER_NAME { return None; @@ -1655,7 +1666,7 @@ fn load_startup_cached_codex_apps_tools_snapshot( let cache_context = cache_context?; - match load_cached_codex_apps_tools(cache_context) { + match load_cached_codex_apps_tools(cache_context, tool_plugin_provenance) { CachedCodexAppsToolsLoad::Hit(tools) => Some(tools), CachedCodexAppsToolsLoad::Missing | CachedCodexAppsToolsLoad::Invalid => None, } @@ -1664,8 +1675,9 @@ fn load_startup_cached_codex_apps_tools_snapshot( #[cfg(test)] fn read_cached_codex_apps_tools( cache_context: &CodexAppsToolsCacheContext, + tool_plugin_provenance: &ToolPluginProvenance, ) -> Option> { - match load_cached_codex_apps_tools(cache_context) { + match load_cached_codex_apps_tools(cache_context, tool_plugin_provenance) { CachedCodexAppsToolsLoad::Hit(tools) => Some(tools), CachedCodexAppsToolsLoad::Missing | CachedCodexAppsToolsLoad::Invalid => None, } @@ -1673,6 +1685,19 @@ fn read_cached_codex_apps_tools( fn load_cached_codex_apps_tools( cache_context: &CodexAppsToolsCacheContext, + tool_plugin_provenance: &ToolPluginProvenance, +) -> CachedCodexAppsToolsLoad { + match load_raw_cached_codex_apps_tools(cache_context) { + CachedCodexAppsToolsLoad::Hit(tools) => CachedCodexAppsToolsLoad::Hit( + filter_disallowed_codex_apps_tools(tools, tool_plugin_provenance), + ), + CachedCodexAppsToolsLoad::Missing => CachedCodexAppsToolsLoad::Missing, + CachedCodexAppsToolsLoad::Invalid => CachedCodexAppsToolsLoad::Invalid, + } +} + +fn load_raw_cached_codex_apps_tools( + cache_context: &CodexAppsToolsCacheContext, ) -> CachedCodexAppsToolsLoad { let cache_path = cache_context.cache_path(); let bytes = match std::fs::read(cache_path) { @@ -1689,7 +1714,7 @@ fn load_cached_codex_apps_tools( if cache.schema_version != CODEX_APPS_TOOLS_CACHE_SCHEMA_VERSION { return CachedCodexAppsToolsLoad::Invalid; } - CachedCodexAppsToolsLoad::Hit(filter_disallowed_codex_apps_tools(cache.tools)) + CachedCodexAppsToolsLoad::Hit(cache.tools) } fn write_cached_codex_apps_tools(cache_context: &CodexAppsToolsCacheContext, tools: &[ToolInfo]) { @@ -1699,23 +1724,43 @@ fn write_cached_codex_apps_tools(cache_context: &CodexAppsToolsCacheContext, too { return; } - let tools = filter_disallowed_codex_apps_tools(tools.to_vec()); let Ok(bytes) = serde_json::to_vec_pretty(&CodexAppsToolsDiskCache { schema_version: CODEX_APPS_TOOLS_CACHE_SCHEMA_VERSION, - tools, + tools: tools.to_vec(), }) else { return; }; let _ = std::fs::write(cache_path, bytes); } -fn filter_disallowed_codex_apps_tools(tools: Vec) -> Vec { +fn filter_disallowed_codex_apps_tools_for_server( + server_name: &str, + tools: Vec, + tool_plugin_provenance: &ToolPluginProvenance, +) -> Vec { + if server_name == CODEX_APPS_MCP_SERVER_NAME { + filter_disallowed_codex_apps_tools(tools, tool_plugin_provenance) + } else { + tools + } +} + +fn filter_disallowed_codex_apps_tools( + tools: Vec, + tool_plugin_provenance: &ToolPluginProvenance, +) -> Vec { + let plugin_declared_connector_ids = tool_plugin_provenance.plugin_declared_connector_ids(); + let originator_value = originator(); tools .into_iter() .filter(|tool| { - tool.connector_id - .as_deref() - .is_none_or(is_connector_id_allowed) + tool.connector_id.as_deref().is_none_or(|connector_id| { + connector_id_passes_disallow_filter( + connector_id, + originator_value.value.as_str(), + &plugin_declared_connector_ids, + ) + }) }) .collect() } @@ -1741,6 +1786,7 @@ async fn list_tools_for_client_uncached( client: &Arc, timeout: Option, server_instructions: Option<&str>, + tool_plugin_provenance: &ToolPluginProvenance, ) -> Result> { let resp = client .list_tools_with_connector_ids(/*params*/ None, timeout) @@ -1782,10 +1828,11 @@ async fn list_tools_for_client_uncached( } }) .collect(); - if server_name == CODEX_APPS_MCP_SERVER_NAME { - return Ok(filter_disallowed_codex_apps_tools(tools)); - } - Ok(tools) + Ok(filter_disallowed_codex_apps_tools_for_server( + server_name, + tools, + tool_plugin_provenance, + )) } fn validate_mcp_server_name(server_name: &str) -> Result<()> { diff --git a/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs b/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs index cf2889ccde..88f1e1dc9d 100644 --- a/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/mcp_connection_manager_tests.rs @@ -1,4 +1,6 @@ use super::*; +use codex_plugin::AppConnectorId; +use codex_plugin::PluginCapabilitySummary; use codex_protocol::ToolName; use codex_protocol::protocol::GranularApprovalConfig; use codex_protocol::protocol::McpAuthStatus; @@ -62,6 +64,14 @@ fn create_codex_apps_tools_cache_context( } } +fn provenance_for_connector(connector_id: &str) -> ToolPluginProvenance { + ToolPluginProvenance::from_capability_summaries(&[PluginCapabilitySummary { + display_name: "sample-plugin".to_string(), + app_connector_ids: vec![AppConnectorId(connector_id.to_string())], + ..PluginCapabilitySummary::default() + }]) +} + #[test] fn declared_openai_file_fields_treat_names_literally() { let meta = serde_json::json!({ @@ -480,12 +490,14 @@ fn codex_apps_tools_cache_is_overwritten_by_last_write() { write_cached_codex_apps_tools(&cache_context, &tools_gateway_1); let cached_gateway_1 = - read_cached_codex_apps_tools(&cache_context).expect("cache entry exists for first write"); + read_cached_codex_apps_tools(&cache_context, &ToolPluginProvenance::default()) + .expect("cache entry exists for first write"); assert_eq!(cached_gateway_1[0].callable_name, "one"); write_cached_codex_apps_tools(&cache_context, &tools_gateway_2); let cached_gateway_2 = - read_cached_codex_apps_tools(&cache_context).expect("cache entry exists for second write"); + read_cached_codex_apps_tools(&cache_context, &ToolPluginProvenance::default()) + .expect("cache entry exists for second write"); assert_eq!(cached_gateway_2[0].callable_name, "two"); } @@ -509,9 +521,11 @@ fn codex_apps_tools_cache_is_scoped_per_user() { write_cached_codex_apps_tools(&cache_context_user_2, &tools_user_2); let read_user_1 = - read_cached_codex_apps_tools(&cache_context_user_1).expect("cache entry for user one"); + read_cached_codex_apps_tools(&cache_context_user_1, &ToolPluginProvenance::default()) + .expect("cache entry for user one"); let read_user_2 = - read_cached_codex_apps_tools(&cache_context_user_2).expect("cache entry for user two"); + read_cached_codex_apps_tools(&cache_context_user_2, &ToolPluginProvenance::default()) + .expect("cache entry for user two"); assert_eq!(read_user_1[0].callable_name, "one"); assert_eq!(read_user_2[0].callable_name, "two"); @@ -546,13 +560,74 @@ fn codex_apps_tools_cache_filters_disallowed_connectors() { ]; write_cached_codex_apps_tools(&cache_context, &tools); - let cached = read_cached_codex_apps_tools(&cache_context).expect("cache entry exists for user"); + let cached = read_cached_codex_apps_tools(&cache_context, &ToolPluginProvenance::default()) + .expect("cache entry exists for user"); assert_eq!(cached.len(), 1); assert_eq!(cached[0].callable_name, "allowed_tool"); assert_eq!(cached[0].connector_id.as_deref(), Some("calendar")); } +#[test] +fn codex_apps_tools_cache_keeps_plugin_declared_openai_connector() { + let codex_home = tempdir().expect("tempdir"); + let cache_context = create_codex_apps_tools_cache_context( + codex_home.path().to_path_buf(), + Some("account-one"), + Some("user-one"), + ); + let tools = vec![ + create_test_tool_with_connector( + CODEX_APPS_MCP_SERVER_NAME, + "appgarden_tool", + "connector_openai_appgarden", + Some("Appgen"), + ), + create_test_tool_with_connector( + CODEX_APPS_MCP_SERVER_NAME, + "hidden_tool", + "connector_openai_hidden", + Some("Hidden"), + ), + ]; + + write_cached_codex_apps_tools(&cache_context, &tools); + let cached = read_cached_codex_apps_tools( + &cache_context, + &provenance_for_connector("connector_openai_appgarden"), + ) + .expect("cache entry exists for user"); + + assert_eq!(cached.len(), 1); + assert_eq!(cached[0].callable_name, "appgarden_tool"); + assert_eq!( + cached[0].connector_id.as_deref(), + Some("connector_openai_appgarden") + ); +} + +#[test] +fn codex_apps_tools_cache_hides_cached_openai_connector_without_current_provenance() { + let codex_home = tempdir().expect("tempdir"); + let cache_context = create_codex_apps_tools_cache_context( + codex_home.path().to_path_buf(), + Some("account-one"), + Some("user-one"), + ); + let tools = vec![create_test_tool_with_connector( + CODEX_APPS_MCP_SERVER_NAME, + "appgarden_tool", + "connector_openai_appgarden", + Some("Appgen"), + )]; + + write_cached_codex_apps_tools(&cache_context, &tools); + let cached = read_cached_codex_apps_tools(&cache_context, &ToolPluginProvenance::default()) + .expect("cache entry exists for user"); + + assert!(cached.is_empty()); +} + #[test] fn codex_apps_tools_cache_is_ignored_when_schema_version_mismatches() { let codex_home = tempdir().expect("tempdir"); @@ -572,7 +647,9 @@ fn codex_apps_tools_cache_is_ignored_when_schema_version_mismatches() { .expect("serialize"); std::fs::write(cache_path, bytes).expect("write"); - assert!(read_cached_codex_apps_tools(&cache_context).is_none()); + assert!( + read_cached_codex_apps_tools(&cache_context, &ToolPluginProvenance::default(),).is_none() + ); } #[test] @@ -589,7 +666,9 @@ fn codex_apps_tools_cache_is_ignored_when_json_is_invalid() { } std::fs::write(cache_path, b"{not json").expect("write"); - assert!(read_cached_codex_apps_tools(&cache_context).is_none()); + assert!( + read_cached_codex_apps_tools(&cache_context, &ToolPluginProvenance::default(),).is_none() + ); } #[test] @@ -609,6 +688,7 @@ fn startup_cached_codex_apps_tools_loads_from_disk_cache() { let startup_snapshot = load_startup_cached_codex_apps_tools_snapshot( CODEX_APPS_MCP_SERVER_NAME, Some(&cache_context), + &ToolPluginProvenance::default(), ); let startup_tools = startup_snapshot.expect("expected startup snapshot to load from cache"); diff --git a/codex-rs/connectors/src/filter.rs b/codex-rs/connectors/src/filter.rs index 82c334f82d..6986e76ef7 100644 --- a/codex-rs/connectors/src/filter.rs +++ b/codex-rs/connectors/src/filter.rs @@ -7,6 +7,7 @@ pub fn filter_tool_suggest_discoverable_connectors( accessible_connectors: &[AppInfo], discoverable_connector_ids: &HashSet, originator_value: &str, + plugin_declared_connector_ids: &HashSet, ) -> Vec { let accessible_connector_ids: HashSet<&str> = accessible_connectors .iter() @@ -14,11 +15,15 @@ pub fn filter_tool_suggest_discoverable_connectors( .map(|connector| connector.id.as_str()) .collect(); - let mut connectors = filter_disallowed_connectors(directory_connectors, originator_value) - .into_iter() - .filter(|connector| !accessible_connector_ids.contains(connector.id.as_str())) - .filter(|connector| discoverable_connector_ids.contains(connector.id.as_str())) - .collect::>(); + let mut connectors = filter_disallowed_connectors( + directory_connectors, + originator_value, + plugin_declared_connector_ids, + ) + .into_iter() + .filter(|connector| !accessible_connector_ids.contains(connector.id.as_str())) + .filter(|connector| discoverable_connector_ids.contains(connector.id.as_str())) + .collect::>(); connectors.sort_by(|left, right| { left.name .cmp(&right.name) @@ -42,12 +47,16 @@ const DISALLOWED_CONNECTOR_PREFIX: &str = "connector_openai_"; pub fn filter_disallowed_connectors( connectors: Vec, originator_value: &str, + plugin_declared_connector_ids: &HashSet, ) -> Vec { - let first_party_chat_originator = is_first_party_chat_originator(originator_value); connectors .into_iter() .filter(|connector| { - is_connector_id_allowed(connector.id.as_str(), first_party_chat_originator) + connector_id_passes_disallow_filter( + connector.id.as_str(), + originator_value, + plugin_declared_connector_ids, + ) }) .collect() } @@ -56,13 +65,95 @@ fn is_first_party_chat_originator(originator_value: &str) -> bool { originator_value == "codex_atlas" || originator_value == "codex_chatgpt_desktop" } -fn is_connector_id_allowed(connector_id: &str, first_party_chat_originator: bool) -> bool { +pub fn connector_id_passes_disallow_filter( + connector_id: &str, + originator_value: &str, + plugin_declared_connector_ids: &HashSet, +) -> bool { + let first_party_chat_originator = is_first_party_chat_originator(originator_value); let disallowed_connector_ids = if first_party_chat_originator { FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS } else { DISALLOWED_CONNECTOR_IDS }; + if disallowed_connector_ids.contains(&connector_id) { + return false; + } + !connector_id.starts_with(DISALLOWED_CONNECTOR_PREFIX) - && !disallowed_connector_ids.contains(&connector_id) + || plugin_declared_connector_ids.contains(connector_id) +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + fn app(id: &str) -> AppInfo { + AppInfo { + id: id.to_string(), + name: id.to_string(), + description: None, + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: None, + is_accessible: false, + is_enabled: true, + plugin_display_names: Vec::new(), + } + } + + #[test] + fn filter_disallowed_connectors_filters_openai_prefix_without_plugin_provenance() { + let filtered = filter_disallowed_connectors( + vec![app("connector_openai_foo"), app("gamma")], + "codex_cli", + &HashSet::new(), + ); + + assert_eq!(filtered, vec![app("gamma")]); + } + + #[test] + fn filter_disallowed_connectors_allows_plugin_declared_openai_prefix() { + let plugin_declared_connector_ids = + HashSet::from(["connector_openai_appgarden".to_string()]); + + let filtered = filter_disallowed_connectors( + vec![ + app("connector_openai_appgarden"), + app("connector_openai_hidden"), + app("gamma"), + ], + "codex_cli", + &plugin_declared_connector_ids, + ); + + assert_eq!( + filtered, + vec![app("connector_openai_appgarden"), app("gamma")] + ); + } + + #[test] + fn filter_disallowed_connectors_keeps_explicit_denylist_over_plugin_provenance() { + let plugin_declared_connector_ids = + HashSet::from(["asdk_app_6938a94a61d881918ef32cb999ff937c".to_string()]); + + let filtered = filter_disallowed_connectors( + vec![ + app("asdk_app_6938a94a61d881918ef32cb999ff937c"), + app("gamma"), + ], + "codex_cli", + &plugin_declared_connector_ids, + ); + + assert_eq!(filtered, vec![app("gamma")]); + } } diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 965521ae3d..1e59e13674 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -119,13 +119,15 @@ pub(crate) async fn list_tool_suggest_discoverable_tools_with_auth( ) -> anyhow::Result> { let directory_connectors = list_directory_connectors_for_tool_suggest_with_auth(config, auth).await?; - let connector_ids = tool_suggest_connector_ids(config).await; + let plugin_declared_connector_ids = plugin_declared_connector_ids_for_config(config).await; + let connector_ids = tool_suggest_connector_ids(config, &plugin_declared_connector_ids); let discoverable_connectors = codex_connectors::filter::filter_tool_suggest_discoverable_connectors( directory_connectors, accessible_connectors, &connector_ids, originator().value.as_str(), + &plugin_declared_connector_ids, ) .into_iter() .map(DiscoverableTool::from); @@ -151,15 +153,17 @@ pub async fn list_cached_accessible_connectors_from_mcp_tools( return Some(Vec::new()); } let cache_key = accessible_connectors_cache_key(config, auth.as_ref()); + let plugin_declared_connector_ids = plugin_declared_connector_ids_for_config(config).await; read_cached_accessible_connectors(&cache_key).map(|connectors| { codex_connectors::filter::filter_disallowed_connectors( connectors, originator().value.as_str(), + &plugin_declared_connector_ids, ) }) } -pub(crate) fn refresh_accessible_connectors_cache_from_mcp_tools( +pub(crate) async fn refresh_accessible_connectors_cache_from_mcp_tools( config: &Config, auth: Option<&CodexAuth>, mcp_tools: &HashMap, @@ -169,9 +173,11 @@ pub(crate) fn refresh_accessible_connectors_cache_from_mcp_tools( } let cache_key = accessible_connectors_cache_key(config, auth); + let plugin_declared_connector_ids = plugin_declared_connector_ids_for_config(config).await; let accessible_connectors = codex_connectors::filter::filter_disallowed_connectors( accessible_connectors_from_mcp_tools(mcp_tools), originator().value.as_str(), + &plugin_declared_connector_ids, ); write_cached_accessible_connectors(cache_key, &accessible_connectors); } @@ -207,11 +213,13 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status( let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.to_path_buf())); let mcp_manager = McpManager::new(Arc::clone(&plugins_manager)); let tool_plugin_provenance = mcp_manager.tool_plugin_provenance(config).await; + let plugin_declared_connector_ids = tool_plugin_provenance.plugin_declared_connector_ids(); if !force_refetch && let Some(cached_connectors) = read_cached_accessible_connectors(&cache_key) { let cached_connectors = codex_connectors::filter::filter_disallowed_connectors( cached_connectors, originator().value.as_str(), + &plugin_declared_connector_ids, ); let cached_connectors = with_app_plugin_sources(cached_connectors, &tool_plugin_provenance); return Ok(AccessibleConnectorsStatus { @@ -258,7 +266,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status( McpRuntimeEnvironment::new(Arc::new(Environment::default()), config.cwd.to_path_buf()), config.codex_home.to_path_buf(), codex_apps_tools_cache_key(auth.as_ref()), - ToolPluginProvenance::default(), + tool_plugin_provenance.clone(), ) .await; @@ -319,6 +327,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status( let accessible_connectors = codex_connectors::filter::filter_disallowed_connectors( accessible_connectors_from_mcp_tools(&tools), originator().value.as_str(), + &plugin_declared_connector_ids, ); if codex_apps_ready || !accessible_connectors.is_empty() { write_cached_accessible_connectors(cache_key, &accessible_connectors); @@ -387,15 +396,22 @@ fn write_cached_accessible_connectors( }); } -async fn tool_suggest_connector_ids(config: &Config) -> HashSet { - let mut connector_ids = PluginsManager::new(config.codex_home.to_path_buf()) +async fn plugin_declared_connector_ids_for_config(config: &Config) -> HashSet { + PluginsManager::new(config.codex_home.to_path_buf()) .plugins_for_config(config) .await .capability_summaries() .iter() .flat_map(|plugin| plugin.app_connector_ids.iter()) .map(|connector_id| connector_id.0.clone()) - .collect::>(); + .collect::>() +} + +fn tool_suggest_connector_ids( + config: &Config, + plugin_declared_connector_ids: &HashSet, +) -> HashSet { + let mut connector_ids = plugin_declared_connector_ids.clone(); connector_ids.extend( config .tool_suggest diff --git a/codex-rs/core/src/connectors_tests.rs b/codex-rs/core/src/connectors_tests.rs index 0f9e834d8d..f8c8506d7a 100644 --- a/codex-rs/core/src/connectors_tests.rs +++ b/codex-rs/core/src/connectors_tests.rs @@ -28,8 +28,15 @@ use rmcp::model::Tool; use std::collections::BTreeMap; use std::collections::HashMap; use std::collections::HashSet; +use std::future::Future; +use std::path::Path; use std::sync::Arc; +use std::sync::LazyLock; use tempfile::tempdir; +use tokio::sync::Mutex as TokioMutex; + +static ACCESSIBLE_CONNECTORS_CACHE_TEST_LOCK: LazyLock> = + LazyLock::new(|| TokioMutex::new(())); fn annotations(destructive_hint: Option, open_world_hint: Option) -> ToolAnnotations { ToolAnnotations { @@ -128,14 +135,51 @@ fn codex_app_tool( } } -fn with_accessible_connectors_cache_cleared(f: impl FnOnce() -> R) -> R { +fn write_installed_plugin_app(codex_home: &Path, connector_id: &str) { + let plugin_root = codex_home.join("plugins/cache/test/sample/local"); + std::fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create plugin manifest dir"); + std::fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ) + .expect("write plugin manifest"); + std::fs::write( + plugin_root.join(".app.json"), + format!( + r#"{{ + "apps": {{ + "sample": {{ + "id": "{connector_id}" + }} + }} +}}"# + ), + ) + .expect("write plugin app config"); + std::fs::write( + codex_home.join(CONFIG_TOML_FILE), + r#"[features] +plugins = true + +[plugins."sample@test"] +enabled = true +"#, + ) + .expect("write config"); +} + +async fn with_accessible_connectors_cache_cleared(f: impl FnOnce() -> F) -> R +where + F: Future, +{ + let _guard = ACCESSIBLE_CONNECTORS_CACHE_TEST_LOCK.lock().await; let previous = { let mut cache_guard = ACCESSIBLE_CONNECTORS_CACHE .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); cache_guard.take() }; - let result = f(); + let result = f().await; let mut cache_guard = ACCESSIBLE_CONNECTORS_CACHE .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); @@ -233,6 +277,7 @@ fn accessible_connectors_from_mcp_tools_carries_plugin_display_names() { #[tokio::test] async fn refresh_accessible_connectors_cache_from_mcp_tools_writes_latest_installed_apps() { let codex_home = tempdir().expect("tempdir should succeed"); + write_installed_plugin_app(codex_home.path(), "connector_openai_appgarden"); let mut config = ConfigBuilder::default() .codex_home(codex_home.path().to_path_buf()) .build() @@ -250,6 +295,15 @@ async fn refresh_accessible_connectors_cache_from_mcp_tools_writes_latest_instal &["calendar-plugin"], ), ), + ( + "mcp__codex_apps__appgarden_create_project".to_string(), + codex_app_tool( + "appgarden_create_project", + "connector_openai_appgarden", + Some("Appgen"), + &["appgarden-plugin"], + ), + ), ( "mcp__codex_apps__openai_hidden".to_string(), codex_app_tool( @@ -261,31 +315,92 @@ async fn refresh_accessible_connectors_cache_from_mcp_tools_writes_latest_instal ), ]); - let cached = with_accessible_connectors_cache_cleared(|| { - refresh_accessible_connectors_cache_from_mcp_tools(&config, /*auth*/ None, &tools); + let cached = with_accessible_connectors_cache_cleared(|| async { + refresh_accessible_connectors_cache_from_mcp_tools(&config, /*auth*/ None, &tools).await; read_cached_accessible_connectors(&cache_key).expect("cache should be populated") - }); + }) + .await; assert_eq!( cached, - vec![AppInfo { - id: "calendar".to_string(), - name: "Google Calendar".to_string(), - description: None, - logo_url: None, - logo_url_dark: None, - distribution_channel: None, - install_url: Some(connector_install_url("Google Calendar", "calendar")), - branding: None, - app_metadata: None, - labels: None, - is_accessible: true, - is_enabled: true, - plugin_display_names: plugin_names(&["calendar-plugin"]), - }] + vec![ + AppInfo { + id: "connector_openai_appgarden".to_string(), + name: "Appgen".to_string(), + description: None, + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + install_url: Some(connector_install_url( + "Appgen", + "connector_openai_appgarden" + )), + branding: None, + app_metadata: None, + labels: None, + is_accessible: true, + is_enabled: true, + plugin_display_names: plugin_names(&["appgarden-plugin"]), + }, + AppInfo { + id: "calendar".to_string(), + name: "Google Calendar".to_string(), + description: None, + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + install_url: Some(connector_install_url("Google Calendar", "calendar")), + branding: None, + app_metadata: None, + labels: None, + is_accessible: true, + is_enabled: true, + plugin_display_names: plugin_names(&["calendar-plugin"]), + }, + ] ); } +#[tokio::test] +async fn list_cached_accessible_connectors_applies_current_plugin_provenance() { + let codex_home = tempdir().expect("tempdir should succeed"); + let mut config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("config should load"); + let _ = config.features.set_enabled(Feature::Apps, /*enabled*/ true); + let cache_key = accessible_connectors_cache_key(&config, /*auth*/ None); + let cached_connectors = vec![AppInfo { + id: "connector_openai_appgarden".to_string(), + name: "Appgen".to_string(), + description: None, + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + install_url: Some(connector_install_url( + "Appgen", + "connector_openai_appgarden", + )), + branding: None, + app_metadata: None, + labels: None, + is_accessible: true, + is_enabled: true, + plugin_display_names: Vec::new(), + }]; + + let filtered = with_accessible_connectors_cache_cleared(|| async { + write_cached_accessible_connectors(cache_key, &cached_connectors); + list_cached_accessible_connectors_from_mcp_tools(&config) + .await + .expect("cache should be readable") + }) + .await; + + assert_eq!(filtered, Vec::::new()); +} + #[test] fn merge_connectors_unions_and_dedupes_plugin_display_names() { let mut plugin = plugin_connector_to_app_info("calendar".to_string()); @@ -1038,8 +1153,11 @@ fn app_tool_policy_matches_prefix_stripped_tool_name_for_tool_config() { #[test] fn filter_disallowed_connectors_allows_non_disallowed_connectors() { - let filtered = - filter_disallowed_connectors(vec![app("asdk_app_hidden"), app("alpha")], "codex_cli"); + let filtered = filter_disallowed_connectors( + vec![app("asdk_app_hidden"), app("alpha")], + "codex_cli", + &HashSet::new(), + ); assert_eq!(filtered, vec![app("asdk_app_hidden"), app("alpha")]); } @@ -1052,6 +1170,7 @@ fn filter_disallowed_connectors_filters_openai_prefix() { app("gamma"), ], "codex_cli", + &HashSet::new(), ); assert_eq!(filtered, vec![app("gamma")]); } @@ -1065,6 +1184,7 @@ fn filter_disallowed_connectors_filters_disallowed_connector_ids() { app("delta"), ], "codex_cli", + &HashSet::new(), ); assert_eq!(filtered, vec![app("delta")]); } @@ -1078,6 +1198,7 @@ fn first_party_chat_originator_filters_target_and_openai_prefixed_connectors() { app("connector_0f9c9d4592e54d0a9a12b3f44a1e2010"), ], "codex_atlas", + &HashSet::new(), ); assert_eq!( filtered, @@ -1107,7 +1228,7 @@ discoverables = [ .expect("config should load"); assert_eq!( - tool_suggest_connector_ids(&config).await, + tool_suggest_connector_ids(&config, &HashSet::new()), HashSet::from(["connector_2128aebfecb84f64a069897515042a44".to_string()]) ); } @@ -1135,6 +1256,7 @@ fn filter_tool_suggest_discoverable_connectors_keeps_only_plugin_backed_uninstal "connector_68df038e0ba48191908c8434991bbac2".to_string(), ]), "codex_cli", + &HashSet::new(), ); assert_eq!( @@ -1146,6 +1268,41 @@ fn filter_tool_suggest_discoverable_connectors_keeps_only_plugin_backed_uninstal ); } +#[test] +fn filter_tool_suggest_discoverable_connectors_allows_plugin_declared_openai_prefix() { + let filtered = filter_tool_suggest_discoverable_connectors( + vec![ + named_app("connector_openai_appgarden", "Appgen"), + named_app("connector_openai_hidden", "Hidden"), + ], + &[], + &HashSet::from([ + "connector_openai_appgarden".to_string(), + "connector_openai_hidden".to_string(), + ]), + "codex_cli", + &HashSet::from(["connector_openai_appgarden".to_string()]), + ); + + assert_eq!( + filtered, + vec![named_app("connector_openai_appgarden", "Appgen")] + ); +} + +#[test] +fn filter_tool_suggest_discoverable_connectors_blocks_config_only_openai_prefix() { + let filtered = filter_tool_suggest_discoverable_connectors( + vec![named_app("connector_openai_hidden", "Hidden")], + &[], + &HashSet::from(["connector_openai_hidden".to_string()]), + "codex_cli", + &HashSet::new(), + ); + + assert_eq!(filtered, Vec::::new()); +} + #[test] fn filter_tool_suggest_discoverable_connectors_excludes_accessible_apps_even_when_disabled() { let filtered = filter_tool_suggest_discoverable_connectors( @@ -1175,6 +1332,7 @@ fn filter_tool_suggest_discoverable_connectors_excludes_accessible_apps_even_whe "connector_68df038e0ba48191908c8434991bbac2".to_string(), ]), "codex_cli", + &HashSet::new(), ); assert_eq!(filtered, Vec::::new()); diff --git a/codex-rs/core/src/tools/handlers/tool_suggest.rs b/codex-rs/core/src/tools/handlers/tool_suggest.rs index 5c51c90d66..7d9888ba12 100644 --- a/codex-rs/core/src/tools/handlers/tool_suggest.rs +++ b/codex-rs/core/src/tools/handlers/tool_suggest.rs @@ -232,7 +232,8 @@ async fn refresh_missing_suggested_connectors( &turn.config, auth, &mcp_tools, - ); + ) + .await; Some(accessible_connectors) } Err(err) => { diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index be258fae4a..2122e715ac 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -7830,12 +7830,15 @@ impl ChatWidget { }); let result: Result = async { + let plugin_declared_connector_ids = + connectors::plugin_declared_connector_ids_for_config(&config).await; let all_connectors = connectors::list_all_connectors_with_options(&config, force_refetch).await?; let connectors = connectors::merge_connectors_with_accessible( all_connectors, accessible_connectors, /*all_connectors_loaded*/ true, + &plugin_declared_connector_ids, ); Ok(ConnectorsSnapshot { connectors }) } @@ -10902,13 +10905,6 @@ impl ChatWidget { match result { Ok(mut snapshot) => { - if !is_final { - snapshot.connectors = connectors::merge_connectors_with_accessible( - Vec::new(), - snapshot.connectors, - /*all_connectors_loaded*/ false, - ); - } snapshot.connectors = connectors::with_app_enabled_state(snapshot.connectors, &self.config); if let ConnectorsCacheState::Ready(existing_snapshot) = &self.connectors_cache { diff --git a/codex-rs/utils/plugins/Cargo.toml b/codex-rs/utils/plugins/Cargo.toml index 983f4526ea..a7992bf97b 100644 --- a/codex-rs/utils/plugins/Cargo.toml +++ b/codex-rs/utils/plugins/Cargo.toml @@ -15,7 +15,6 @@ workspace = true [dependencies] codex-exec-server = { workspace = true } -codex-login = { workspace = true } codex-utils-absolute-path = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } diff --git a/codex-rs/utils/plugins/src/mcp_connector.rs b/codex-rs/utils/plugins/src/mcp_connector.rs index 40fb0d4bbf..96fd72eb24 100644 --- a/codex-rs/utils/plugins/src/mcp_connector.rs +++ b/codex-rs/utils/plugins/src/mcp_connector.rs @@ -1,33 +1,3 @@ -use codex_login::default_client::is_first_party_chat_originator; -use codex_login::default_client::originator; - -const DISALLOWED_CONNECTOR_IDS: &[&str] = &[ - "asdk_app_6938a94a61d881918ef32cb999ff937c", - "connector_2b0a9009c9c64bf9933a3dae3f2b1254", - "connector_3f8d1a79f27c4c7ba1a897ab13bf37dc", - "connector_68de829bf7648191acd70a907364c67c", - "connector_68e004f14af881919eb50893d3d9f523", - "connector_69272cb413a081919685ec3c88d1744e", -]; -const FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS: &[&str] = - &["connector_0f9c9d4592e54d0a9a12b3f44a1e2010"]; -const DISALLOWED_CONNECTOR_PREFIX: &str = "connector_openai_"; - -pub fn is_connector_id_allowed(connector_id: &str) -> bool { - is_connector_id_allowed_for_originator(connector_id, originator().value.as_str()) -} - -fn is_connector_id_allowed_for_originator(connector_id: &str, originator_value: &str) -> bool { - let disallowed_connector_ids = if is_first_party_chat_originator(originator_value) { - FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS - } else { - DISALLOWED_CONNECTOR_IDS - }; - - !connector_id.starts_with(DISALLOWED_CONNECTOR_PREFIX) - && !disallowed_connector_ids.contains(&connector_id) -} - pub fn sanitize_name(name: &str) -> String { sanitize_slug(name).replace("-", "_") }