diff --git a/codex-rs/connectors/src/snapshot.rs b/codex-rs/connectors/src/snapshot.rs index 7011a74d3c..4129f03988 100644 --- a/codex-rs/connectors/src/snapshot.rs +++ b/codex-rs/connectors/src/snapshot.rs @@ -79,10 +79,11 @@ pub struct ConnectorSnapshot { impl ConnectorSnapshot { /// Builds the final selection from all plugin sources, preserving contribution order. - /// Pass host and selected sources together so any enabled shared owner keeps a connector. + /// An enabled contributor preserves a shared connector unless its canonical owner is disabled. pub fn from_plugin_sources( sources: impl IntoIterator, disabled_plugin_ids: &[String], + canonical_disabled_connector_ids: HashSet, ) -> Self { let mut connector_ids = Vec::new(); let mut seen_connector_ids = HashSet::new(); @@ -95,6 +96,9 @@ impl ConnectorSnapshot { continue; } for connector_id in source.connector_ids() { + if canonical_disabled_connector_ids.contains(&connector_id.0) { + continue; + } if seen_connector_ids.insert(connector_id.0.clone()) { connector_ids.push(connector_id.clone()); } @@ -109,6 +113,7 @@ impl ConnectorSnapshot { plugin_names.dedup(); } disabled_connector_ids.retain(|id| !seen_connector_ids.contains(id)); + disabled_connector_ids.extend(canonical_disabled_connector_ids); Self { connector_ids, @@ -119,7 +124,11 @@ impl ConnectorSnapshot { /// Adapts the current host plugin summaries to the connector-owned snapshot. pub fn from_plugin_capability_summaries(summaries: &[PluginCapabilitySummary]) -> Self { - Self::from_plugin_sources(summaries.iter().map(PluginConnectorSource::from), &[]) + Self::from_plugin_sources( + summaries.iter().map(PluginConnectorSource::from), + &[], + HashSet::new(), + ) } /// Returns the connector IDs in source contribution order. @@ -127,7 +136,7 @@ impl ConnectorSnapshot { &self.connector_ids } - /// Connector tools excluded because no enabled plugin still contributes them. + /// Connector tools explicitly excluded or excluded by all contributing plugins. pub fn disabled_connector_ids(&self) -> &HashSet { &self.disabled_connector_ids } diff --git a/codex-rs/connectors/src/snapshot_tests.rs b/codex-rs/connectors/src/snapshot_tests.rs index e4f0bc2714..e6eb2b6b07 100644 --- a/codex-rs/connectors/src/snapshot_tests.rs +++ b/codex-rs/connectors/src/snapshot_tests.rs @@ -1,3 +1,5 @@ +use std::collections::HashSet; + use codex_plugin::AppConnectorId; use pretty_assertions::assert_eq; @@ -15,7 +17,11 @@ fn snapshot_merges_sources_in_order_and_dedupes_provenance() { source("selected-b", "Alpha", &["calendar"]), ]; - let merged = ConnectorSnapshot::from_plugin_sources(host.into_iter().chain(selected), &[]); + let merged = ConnectorSnapshot::from_plugin_sources( + host.into_iter().chain(selected), + &[], + HashSet::new(), + ); assert_eq!( merged.connector_ids(), @@ -40,13 +46,18 @@ fn disabled_plugins_preserve_shared_connectors() { source("alpha", "Alpha", &["exclusive", "shared"]), source("beta", "Beta", &["other", "shared"]), ]; - let filtered = ConnectorSnapshot::from_plugin_sources(sources.clone(), &["alpha".to_string()]); + let filtered = ConnectorSnapshot::from_plugin_sources( + sources.clone(), + &["alpha".to_string()], + HashSet::new(), + ); let expected = ConnectorSnapshot { - disabled_connector_ids: std::collections::HashSet::from(["exclusive".to_string()]), + disabled_connector_ids: HashSet::from(["exclusive".to_string()]), ..ConnectorSnapshot::from_plugin_sources( [source("beta", "Beta", &["other", "shared"])], &[], + HashSet::new(), ) }; assert_eq!(filtered, expected); @@ -54,13 +65,18 @@ fn disabled_plugins_preserve_shared_connectors() { ConnectorSnapshot::from_plugin_sources( sources.iter().rev().cloned(), &["alpha".to_string()], + HashSet::new(), ), expected ); assert_eq!( - ConnectorSnapshot::from_plugin_sources(sources, &["alpha".to_string(), "beta".to_string()],), + ConnectorSnapshot::from_plugin_sources( + sources, + &["alpha".to_string(), "beta".to_string()], + HashSet::new(), + ), ConnectorSnapshot { - disabled_connector_ids: std::collections::HashSet::from([ + disabled_connector_ids: HashSet::from([ "exclusive".to_string(), "other".to_string(), "shared".to_string(), diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 69269e67ed..ab56913d94 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -99,6 +99,8 @@ use codex_config::skill_config_rules_from_stack; use codex_config::types::PluginConfig; use codex_config::types::ToolSuggestDisabledTool; use codex_config::types::ToolSuggestDiscoverableType; +use codex_connectors::ConnectorSnapshot; +use codex_connectors::PluginConnectorSource; use codex_hooks::plugin_hook_declarations; use codex_http_client::HttpClientFactory; use codex_login::AuthManager; @@ -1024,6 +1026,40 @@ impl PluginsManager { } } + /// Applies plugin exclusions and canonical ownership from the current account's installed cache. + /// A canonical owner's bundle need not be installed on this host. + pub fn connector_snapshot( + &self, + sources: impl IntoIterator, + disabled_plugin_ids: &[String], + ) -> ConnectorSnapshot { + let mut canonical_app_ids = HashSet::new(); + if !disabled_plugin_ids.is_empty() { + let cache = match self.remote_installed_plugins_cache.read() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + if self.remote_installed_plugins_cache_matches_current_auth(&cache) { + canonical_app_ids = cache + .plugins + .as_deref() + .unwrap_or_default() + .iter() + .filter_map(|plugin| { + let app_id = plugin.canonical_app_id.as_ref()?; + let plugin_id = + PluginId::new(plugin.name.clone(), plugin.marketplace_name.clone()) + .ok()?; + disabled_plugin_ids + .contains(&plugin_id.as_key()) + .then(|| app_id.clone()) + }) + .collect(); + } + } + ConnectorSnapshot::from_plugin_sources(sources, disabled_plugin_ids, canonical_app_ids) + } + fn remote_installed_plugin_configs(&self) -> HashMap { let cache = match self.remote_installed_plugins_cache.read() { Ok(cache) => cache, diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 50d3598399..26930a88ba 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -864,6 +864,7 @@ fn remote_installed_plugin_in_marketplace( marketplace_name: &str, ) -> RemoteInstalledPlugin { RemoteInstalledPlugin { + canonical_app_id: None, marketplace_name: marketplace_name.to_string(), id: format!("plugins~Plugin_{name}"), version: None, @@ -3102,6 +3103,55 @@ enabled = true } } +#[tokio::test] +async fn connector_snapshot_combines_plugin_exclusions_with_current_account_ownership() { + let codex_home = TempDir::new().unwrap(); + let auth_manager = test_auth_manager(Some(AuthMode::Chatgpt)); + let manager = test_plugins_manager_with_auth_manager( + codex_home.path().to_path_buf(), + Some(Product::Codex), + Arc::clone(&auth_manager), + ); + let sources = [PluginConnectorSource::from_connector_ids( + "local@test", + "Local", + [AppConnectorId("local-connector".to_string())], + )]; + let disabled = vec![ + "linear@openai-curated-remote".to_string(), + "local@test".to_string(), + ]; + let local_exclusion = HashSet::from(["local-connector".to_string()]); + assert_eq!( + manager + .connector_snapshot(sources.clone(), &disabled) + .disabled_connector_ids(), + &local_exclusion, + ); + let mut plugin = remote_installed_linear_plugin(); + plugin.canonical_app_id = Some("linear".to_string()); + manager.write_remote_installed_plugins_cache(vec![plugin]); + assert_eq!( + manager + .connector_snapshot(sources.clone(), &disabled) + .disabled_connector_ids(), + &HashSet::from(["linear".to_string(), "local-connector".to_string()]), + ); + assert!( + manager + .connector_snapshot(sources.clone(), &["linear@another-marketplace".to_string()]) + .disabled_connector_ids() + .is_empty() + ); + set_test_auth_mode(&auth_manager, Some(AuthMode::ApiKey)).await; + assert_eq!( + manager + .connector_snapshot(sources, &disabled) + .disabled_connector_ids(), + &local_exclusion, + ); +} + #[test] fn loaded_plugins_cache_evicts_least_recently_used_configuration() { let codex_home = TempDir::new().unwrap(); diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index 18cd4ff967..86ba6960f4 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -230,6 +230,7 @@ pub struct RemoteInstalledPlugin { pub id: String, pub version: Option, pub name: String, + pub canonical_app_id: Option, pub installed_at: Option>, pub enabled: bool, pub install_policy: PluginInstallPolicy, @@ -682,6 +683,8 @@ impl RemotePluginInstallPolicySource { struct RemotePluginDirectoryItem { id: String, name: String, + #[serde(default)] + canonical_app_id: Option, scope: RemotePluginScope, #[serde(default)] discoverability: Option, @@ -1879,6 +1882,7 @@ fn remote_installed_plugin_to_cache_entry( id: plugin.id.clone(), version: plugin.release.version.clone(), name: plugin.name.clone(), + canonical_app_id: plugin.canonical_app_id.clone(), installed_at: installed_plugin.installed_at, enabled: installed_plugin.enabled, install_policy: plugin.installation_policy, diff --git a/codex-rs/core-plugins/src/remote_tests.rs b/codex-rs/core-plugins/src/remote_tests.rs index 1d8bc682ea..1dbc7b8b37 100644 --- a/codex-rs/core-plugins/src/remote_tests.rs +++ b/codex-rs/core-plugins/src/remote_tests.rs @@ -596,6 +596,7 @@ fn workspace_share_context_preserves_publish_capability() { fn directory_plugin(id: &str, name: &str) -> RemotePluginDirectoryItem { RemotePluginDirectoryItem { + canonical_app_id: None, id: id.to_string(), name: name.to_string(), scope: RemotePluginScope::Global, diff --git a/codex-rs/core/src/mcp.rs b/codex-rs/core/src/mcp.rs index 8b7617f035..9766373ee8 100644 --- a/codex-rs/core/src/mcp.rs +++ b/codex-rs/core/src/mcp.rs @@ -6,7 +6,6 @@ use crate::environment_selection::ThreadEnvironments; use codex_config::DEFAULT_MCP_SERVER_ENVIRONMENT_ID; use codex_config::McpServerConfig; use codex_connectors::ConnectorRuntimeManager; -use codex_connectors::ConnectorSnapshot; use codex_connectors::PluginConnectorSource; use codex_core_plugins::PluginsManager; use codex_exec_server::ExecutorCapabilityDiscoverySnapshot; @@ -273,10 +272,14 @@ impl McpManager { .capability_summaries() .iter() .map(PluginConnectorSource::from); - let connector_snapshot = ConnectorSnapshot::from_plugin_sources( - host_plugin_connector_sources.chain(selected_plugin_connector_sources), - disabled_plugin_ids, - ); + let connector_snapshot = if config.features.enabled(Feature::Plugins) { + self.plugins_manager.connector_snapshot( + host_plugin_connector_sources.chain(selected_plugin_connector_sources), + disabled_plugin_ids, + ) + } else { + Default::default() + }; let loaded_plugins = loaded_plugins.without_plugins(disabled_plugin_ids); let plugins_available = selected_plugin_available || !loaded_plugins.capability_summaries().is_empty(); diff --git a/codex-rs/core/tests/suite/canonical_plugin_connectors.rs b/codex-rs/core/tests/suite/canonical_plugin_connectors.rs new file mode 100644 index 0000000000..7ff9977907 --- /dev/null +++ b/codex-rs/core/tests/suite/canonical_plugin_connectors.rs @@ -0,0 +1,142 @@ +//! Canonical connector ownership overrides other plugins' shared contributions. + +use std::sync::Arc; + +use anyhow::Result; +use codex_core::TurnInputRequest; +use codex_features::Feature; +use codex_login::CodexAuth; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::ThreadSettingsOverrides; +use codex_protocol::user_input::UserInput; +use core_test_support::apps_test_server::AppsTestServer; +use core_test_support::apps_test_server::SEARCH_CALENDAR_CREATE_TOOL; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::ev_tool_search_call; +use core_test_support::responses::mount_sse_sequence; +use core_test_support::responses::namespace_child_tool; +use core_test_support::responses::sse; +use core_test_support::responses::start_mock_server; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; +use pretty_assertions::assert_eq; +use tempfile::TempDir; +use wiremock::Mock; +use wiremock::ResponseTemplate; +use wiremock::matchers::method; +use wiremock::matchers::path; + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn canonical_plugin_disable_overrides_shared_connector_and_can_be_cleared() -> Result<()> { + skip_if_no_network!(Ok(())); + let server = start_mock_server().await; + let apps = AppsTestServer::mount_with_connector_name(&server, "Google Calendar").await?; + Mock::given(method("GET")) + .and(path("/ps/plugins/installed")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "plugins": [{ + "id": "plugins~Plugin_calendar", + "name": "calendar", + "scope": "GLOBAL", + "status": "ENABLED", + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "canonical_app_id": "calendar", + "release": { + "version": "local", + "display_name": "Calendar", + "description": "Calendar connector", + "interface": {}, + }, + "enabled": true, + }], + "pagination": {"next_page_token": null}, + }))) + .mount(&server) + .await; + let home = Arc::new(TempDir::new()?); + std::fs::write( + home.path().join("config.toml"), + "[features]\nplugins = true\nremote_plugin = true\n[plugins.\"shared@test\"]\nenabled = true\n", + )?; + for (marketplace, name) in [("test", "shared"), ("openai-curated-remote", "calendar")] { + let root = home + .path() + .join(format!("plugins/cache/{marketplace}/{name}/local")); + std::fs::create_dir_all(root.join(".codex-plugin"))?; + std::fs::write( + root.join(".codex-plugin/plugin.json"), + format!(r#"{{"name":"{name}"}}"#), + )?; + std::fs::write( + root.join(".app.json"), + r#"{"apps":{"calendar":{"id":"calendar"}}}"#, + )?; + } + let mut builder = test_codex() + .with_home(home) + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(move |config| { + config.features.enable(Feature::Apps).unwrap(); + config.chatgpt_base_url = apps.chatgpt_base_url; + }); + let test = builder.build_with_auto_env(&server).await?; + let manager = test.thread_manager.plugins_manager(); + let auth = test.thread_manager.auth_manager().auth().await; + manager + .reconcile_remote_installed_plugins(&test.config.plugins_config_input(), auth.as_ref()) + .await?; + for (phase, (disabled, visible)) in [ + (vec!["shared@test"], true), + (vec!["calendar@openai-curated-remote"], false), + (vec![], true), + ] + .into_iter() + .enumerate() + { + let call_id = format!("calendar-search-{phase}"); + let mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("search"), + ev_tool_search_call( + &call_id, + &serde_json::json!({"query":"create calendar event"}), + ), + ev_completed("search"), + ]), + sse(vec![ev_response_created("done"), ev_completed("done")]), + ], + ) + .await; + test.codex + .start_or_steer_turn( + TurnInputRequest::user_input(vec![UserInput::Text { + text: "Find the calendar tool.".to_string(), + text_elements: Vec::new(), + }]) + .with_thread_settings(ThreadSettingsOverrides { + disabled_plugin_ids: Some(disabled.into_iter().map(str::to_string).collect()), + ..Default::default() + }), + ) + .await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + assert_eq!( + namespace_child_tool( + &mock.requests()[1].tool_search_output(&call_id), + "mcp__codex_apps__google_calendar", + SEARCH_CALENDAR_CREATE_TOOL, + ) + .is_some(), + visible + ); + } + Ok(()) +} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 67cfcb47eb..6e08c580dc 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -74,6 +74,7 @@ mod fork_thread; mod git_enrichment; mod guardian_authorization; // Uses the same command-approval harness as guardian_review below. +mod canonical_plugin_connectors; #[cfg(not(target_os = "windows"))] mod guardian_context_budget; mod guardian_history;