From 1944388f63193c08e80c9e55959004b1227992e9 Mon Sep 17 00:00:00 2001 From: Eric Ning Date: Fri, 29 May 2026 18:47:21 -0700 Subject: [PATCH 1/2] [codex] Suggest plugins from remote vertical catalog --- .../src/request_processors/plugins.rs | 65 ++++---- .../app-server/tests/suite/v2/plugin_list.rs | 3 +- codex-rs/core-plugins/src/manager.rs | 48 ++++++ codex-rs/core-plugins/src/manager_tests.rs | 66 ++++++++ codex-rs/core-plugins/src/remote.rs | 9 ++ .../core-plugins/src/remote/share/tests.rs | 6 + codex-rs/core/src/connectors.rs | 15 +- codex-rs/core/src/connectors_tests.rs | 16 +- codex-rs/core/src/plugins/discoverable.rs | 61 ++++++- .../core/src/plugins/discoverable_tests.rs | 150 +++++++++++++----- codex-rs/core/src/session/turn.rs | 1 + .../tools/handlers/request_plugin_install.rs | 27 +++- .../handlers/request_plugin_install_tests.rs | 92 +++++++++-- 13 files changed, 469 insertions(+), 90 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/plugins.rs b/codex-rs/app-server/src/request_processors/plugins.rs index fd45a8a732c..a63312cae0a 100644 --- a/codex-rs/app-server/src/request_processors/plugins.rs +++ b/codex-rs/app-server/src/request_processors/plugins.rs @@ -565,47 +565,56 @@ impl PluginRequestProcessor { (Vec::new(), Vec::new()) }; - // TODO(remote plugins): Remove this once remote plugins are ready and vertical plugins are - // served directly from the normal remote catalog. - if include_vertical && !config.features.enabled(Feature::RemotePlugin) { - let remote_plugin_service_config = RemotePluginServiceConfig { - chatgpt_base_url: config.chatgpt_base_url.clone(), - }; - match codex_core_plugins::remote::fetch_openai_curated_remote_collection_marketplace( - &remote_plugin_service_config, - auth.as_ref(), - ) - .await + let mut remote_sources = Vec::new(); + if !explicit_marketplace_kinds && config.features.enabled(Feature::RemotePlugin) { + remote_sources.push(RemoteMarketplaceSource::Global); + } + if marketplace_kinds.contains(&PluginListMarketplaceKind::WorkspaceDirectory) { + remote_sources.push(RemoteMarketplaceSource::WorkspaceDirectory); + } + if marketplace_kinds.contains(&PluginListMarketplaceKind::SharedWithMe) + && config.features.enabled(Feature::PluginSharing) + { + remote_sources.push(RemoteMarketplaceSource::SharedWithMe); + } + // Tool suggestions stay limited to the server-selected vertical collection, even when + // the full remote marketplace is available in the plugin browser. Cache that collection + // while plugin/list is loading remote catalog data so turn construction is network-free. + let remote_plugin_enabled = config.features.enabled(Feature::RemotePlugin); + let should_cache_remote_tool_suggest_marketplace = (include_vertical + && !remote_plugin_enabled) + || (!explicit_marketplace_kinds && remote_plugin_enabled); + let remote_tool_suggest_marketplace = if should_cache_remote_tool_suggest_marketplace { + match plugins_manager + .remote_tool_suggest_marketplace_for_config(&plugins_input, auth.as_ref()) + .await { - Ok(Some(remote_marketplace)) => { - data.push(remote_marketplace_to_info(remote_marketplace)); - } - Ok(None) => {} + Ok(remote_marketplace) => remote_marketplace, Err( RemotePluginCatalogError::AuthRequired | RemotePluginCatalogError::UnsupportedAuthMode, - ) => {} + ) => None, Err(err) => { warn!( error = %err, - "plugin/list openai-curated-remote collection fetch failed; returning local marketplaces only" + "plugin/list remote plugin suggestion catalog fetch failed; returning other marketplaces only" ); + None } } - } + } else { + None + }; - let mut remote_sources = Vec::new(); - if !explicit_marketplace_kinds && config.features.enabled(Feature::RemotePlugin) { - remote_sources.push(RemoteMarketplaceSource::Global); - } - if marketplace_kinds.contains(&PluginListMarketplaceKind::WorkspaceDirectory) { - remote_sources.push(RemoteMarketplaceSource::WorkspaceDirectory); - } - if marketplace_kinds.contains(&PluginListMarketplaceKind::SharedWithMe) - && config.features.enabled(Feature::PluginSharing) + // TODO(remote plugins): Remove this compatibility response once clients no longer request + // the vertical marketplace explicitly. The same result remains cached for suggestions. + if include_vertical + && !remote_plugin_enabled + && let Some(remote_marketplace) = remote_tool_suggest_marketplace { - remote_sources.push(RemoteMarketplaceSource::SharedWithMe); + data.push(remote_marketplace_to_info(remote_marketplace)); } + if !remote_sources.is_empty() { let remote_plugin_service_config = RemotePluginServiceConfig { chatgpt_base_url: config.chatgpt_base_url.clone(), diff --git a/codex-rs/app-server/tests/suite/v2/plugin_list.rs b/codex-rs/app-server/tests/suite/v2/plugin_list.rs index 93dbfb47736..d743992b3de 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_list.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_list.rs @@ -1712,6 +1712,7 @@ async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled() - .respond_with(ResponseTemplate::new(200).set_body_string(global_directory_body)) .mount(&server) .await; + mount_openai_curated_remote_collection_plugin_list(&server, empty_page_body).await; Mock::given(method("GET")) .and(path("/backend-api/ps/plugins/list")) .and(query_param("scope", "WORKSPACE")) @@ -1801,7 +1802,7 @@ async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled() - ); assert_eq!(response.featured_plugin_ids, Vec::::new()); assert!( - !server + server .received_requests() .await .expect("wiremock should record requests") diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index ec3e74dd16c..276edc22bf2 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -37,9 +37,11 @@ use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome; use crate::marketplace_upgrade::configured_git_marketplace_names; use crate::marketplace_upgrade::upgrade_configured_git_marketplaces; use crate::remote::RemoteInstalledPlugin; +use crate::remote::RemoteMarketplace; use crate::remote::RemotePluginCatalogError; use crate::remote::RemotePluginScope; use crate::remote::RemotePluginServiceConfig; +use crate::remote::fetch_openai_curated_remote_collection_marketplace; use crate::remote_legacy::RemotePluginFetchError; use crate::remote_legacy::RemotePluginMutationError; use crate::startup_sync::curated_plugins_repo_path; @@ -124,6 +126,12 @@ struct CachedFeaturedPluginIds { featured_plugin_ids: Vec, } +#[derive(Clone)] +struct CachedRemoteToolSuggestMarketplace { + key: FeaturedPluginIdsCacheKey, + marketplace: Option, +} + struct RemoteInstalledPluginsCacheRefreshRequest { service_config: RemotePluginServiceConfig, auth: Option, @@ -402,6 +410,7 @@ pub struct PluginsManager { configured_marketplace_upgrade_state: RwLock, non_curated_cache_refresh_state: RwLock, cached_enabled_outcome: RwLock>, + remote_tool_suggest_marketplace_cache: RwLock>, remote_installed_plugins_cache: RwLock>>, remote_installed_plugins_cache_refresh_state: RwLock, remote_sync_lock: Semaphore, @@ -440,6 +449,7 @@ impl PluginsManager { ), non_curated_cache_refresh_state: RwLock::new(NonCuratedCacheRefreshState::default()), cached_enabled_outcome: RwLock::new(None), + remote_tool_suggest_marketplace_cache: RwLock::new(None), remote_installed_plugins_cache: RwLock::new(None), remote_installed_plugins_cache_refresh_state: RwLock::new( RemoteInstalledPluginsCacheRefreshState::default(), @@ -513,6 +523,12 @@ impl PluginsManager { Err(err) => err.into_inner(), }; *featured_plugin_ids_cache = None; + let mut remote_tool_suggest_marketplace_cache = + match self.remote_tool_suggest_marketplace_cache.write() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + *remote_tool_suggest_marketplace_cache = None; } fn clear_enabled_outcome_cache(&self) { @@ -590,6 +606,38 @@ impl PluginsManager { Some(crate::remote::group_remote_installed_plugins_by_marketplaces(plugins, visible_scopes)) } + pub async fn remote_tool_suggest_marketplace_for_config( + &self, + config: &PluginsConfigInput, + auth: Option<&CodexAuth>, + ) -> Result, RemotePluginCatalogError> { + let key = featured_plugin_ids_cache_key(config, auth); + { + let cache = match self.remote_tool_suggest_marketplace_cache.read() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + if let Some(cached) = cache.as_ref().filter(|cached| cached.key == key) { + return Ok(cached.marketplace.clone()); + } + } + + let marketplace = fetch_openai_curated_remote_collection_marketplace( + &remote_plugin_service_config(config), + auth, + ) + .await?; + let mut cache = match self.remote_tool_suggest_marketplace_cache.write() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + *cache = Some(CachedRemoteToolSuggestMarketplace { + key, + marketplace: marketplace.clone(), + }); + Ok(marketplace) + } + pub async fn build_and_cache_remote_installed_plugin_marketplaces( &self, config: &PluginsConfigInput, diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 0131ef22ed1..ea821a3eec5 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -425,6 +425,72 @@ async fn build_remote_installed_plugin_marketplaces_from_cache_uses_remote_metad ); } +#[tokio::test] +async fn remote_tool_suggest_marketplace_for_config_caches_vertical_collection() { + let codex_home = TempDir::new().unwrap(); + write_file( + &codex_home.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +"#, + ); + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/list")) + .and(query_param("scope", "GLOBAL")) + .and(query_param("limit", "200")) + .and(query_param("collection", "vertical")) + .respond_with(ResponseTemplate::new(200).set_body_string( + r#"{ + "plugins": [{ + "id": "plugins~Plugin_data_analytics", + "name": "data-analytics", + "scope": "GLOBAL", + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "status": "ENABLED", + "release": { + "display_name": "Data Analytics", + "description": "Analyze metrics", + "app_ids": ["asdk_app_databricks_workspace"], + "interface": {}, + "skills": [] + } + }], + "pagination": { "next_page_token": null } +}"#, + )) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param("scope", "GLOBAL")) + .respond_with( + ResponseTemplate::new(200) + .set_body_string(r#"{"plugins": [], "pagination": {"next_page_token": null}}"#), + ) + .expect(1) + .mount(&server) + .await; + + let mut config = load_config(codex_home.path(), codex_home.path()).await; + config.chatgpt_base_url = format!("{}/backend-api", server.uri()); + let manager = PluginsManager::new(codex_home.path().to_path_buf()); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + + let first = manager + .remote_tool_suggest_marketplace_for_config(&config, Some(&auth)) + .await + .expect("first suggestion marketplace fetch should succeed"); + let second = manager + .remote_tool_suggest_marketplace_for_config(&config, Some(&auth)) + .await + .expect("cached suggestion marketplace fetch should succeed"); + + assert_eq!(second, first); +} + #[tokio::test] async fn load_plugins_resolves_disabled_skill_names_against_loaded_plugin_skills() { 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 16ae512931c..9f7e0a038a3 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -125,6 +125,9 @@ pub struct RemotePluginSummary { pub id: String, pub remote_plugin_id: String, pub name: String, + pub description: Option, + pub has_skills: bool, + pub app_ids: Vec, pub share_context: Option, pub installed: bool, pub enabled: bool, @@ -723,6 +726,9 @@ pub fn group_remote_installed_plugins_by_marketplaces( id: plugin_id.as_key(), remote_plugin_id: plugin.id.clone(), name: plugin.name.clone(), + description: None, + has_skills: false, + app_ids: Vec::new(), share_context: None, installed: true, enabled: plugin.enabled, @@ -1042,6 +1048,9 @@ fn build_remote_plugin_summary( id: plugin_id.as_key(), remote_plugin_id: plugin.id.clone(), name: plugin.name.clone(), + description: non_empty_string(Some(&plugin.release.description)), + has_skills: !plugin.release.skills.is_empty(), + app_ids: plugin.release.app_ids.clone(), share_context: remote_plugin_share_context(plugin)?, installed: installed_plugin.is_some(), enabled: installed_plugin.is_some_and(|plugin| plugin.enabled), diff --git a/codex-rs/core-plugins/src/remote/share/tests.rs b/codex-rs/core-plugins/src/remote/share/tests.rs index e33d3c5d512..f3257fe8966 100644 --- a/codex-rs/core-plugins/src/remote/share/tests.rs +++ b/codex-rs/core-plugins/src/remote/share/tests.rs @@ -617,6 +617,9 @@ async fn list_remote_plugin_shares_fetches_created_workspace_plugins() { id: "demo-plugin@workspace-shared-with-me".to_string(), remote_plugin_id: "plugins_123".to_string(), name: "demo-plugin".to_string(), + description: Some("Demo plugin description".to_string()), + has_skills: false, + app_ids: Vec::new(), share_context: Some(RemotePluginShareContext { remote_plugin_id: "plugins_123".to_string(), remote_version: Some("0.1.0".to_string()), @@ -656,6 +659,9 @@ async fn list_remote_plugin_shares_fetches_created_workspace_plugins() { id: "demo-plugin@workspace-shared-with-me".to_string(), remote_plugin_id: "plugins_456".to_string(), name: "demo-plugin".to_string(), + description: Some("Demo plugin description".to_string()), + has_skills: false, + app_ids: Vec::new(), share_context: Some(RemotePluginShareContext { remote_plugin_id: "plugins_456".to_string(), remote_version: Some("0.1.0".to_string()), diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 55a941e8c4d..a12a2a44ff9 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -114,6 +114,7 @@ pub(crate) async fn list_tool_suggest_discoverable_tools_with_auth( auth: Option<&CodexAuth>, accessible_connectors: &[AppInfo], loaded_plugin_app_connector_ids: &[String], + plugins_manager: &PluginsManager, ) -> anyhow::Result> { let connector_ids = tool_suggest_connector_ids(config, loaded_plugin_app_connector_ids); let directory_connectors = codex_connectors::merge::merge_plugin_connectors( @@ -129,11 +130,15 @@ pub(crate) async fn list_tool_suggest_discoverable_tools_with_auth( ) .into_iter() .map(DiscoverableTool::from); - let discoverable_plugins = - list_tool_suggest_discoverable_plugins(config, loaded_plugin_app_connector_ids) - .await? - .into_iter() - .map(DiscoverableTool::from); + let discoverable_plugins = list_tool_suggest_discoverable_plugins( + config, + auth, + plugins_manager, + loaded_plugin_app_connector_ids, + ) + .await? + .into_iter() + .map(DiscoverableTool::from); Ok(discoverable_connectors .chain(discoverable_plugins) .collect()) diff --git a/codex-rs/core/src/connectors_tests.rs b/codex-rs/core/src/connectors_tests.rs index 285df9a29a6..54b5f36a568 100644 --- a/codex-rs/core/src/connectors_tests.rs +++ b/codex-rs/core/src/connectors_tests.rs @@ -1237,10 +1237,16 @@ discoverables = [ .expect("config should load"); let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); - let discoverable_tools = - list_tool_suggest_discoverable_tools_with_auth(&config, Some(&auth), &[], &[]) - .await - .expect("discoverable tools should load"); + let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let discoverable_tools = list_tool_suggest_discoverable_tools_with_auth( + &config, + Some(&auth), + &[], + &[], + &plugins_manager, + ) + .await + .expect("discoverable tools should load"); assert_eq!( discoverable_tools, @@ -1268,12 +1274,14 @@ apps = true .expect("config should load"); let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let loaded_plugin_app_connector_ids = vec!["asdk_app_databricks_workspace".to_string()]; + let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); let discoverable_tools = list_tool_suggest_discoverable_tools_with_auth( &config, Some(&auth), &[], &loaded_plugin_app_connector_ids, + &plugins_manager, ) .await .expect("discoverable tools should load"); diff --git a/codex-rs/core/src/plugins/discoverable.rs b/codex-rs/core/src/plugins/discoverable.rs index 4f0d88b512f..b641d534262 100644 --- a/codex-rs/core/src/plugins/discoverable.rs +++ b/codex-rs/core/src/plugins/discoverable.rs @@ -4,13 +4,18 @@ use tracing::warn; use super::PluginCapabilitySummary; use crate::config::Config; +use codex_app_server_protocol::PluginAvailability; +use codex_app_server_protocol::PluginInstallPolicy; use codex_config::types::ToolSuggestDiscoverableType; use codex_core_plugins::OPENAI_BUNDLED_MARKETPLACE_NAME; use codex_core_plugins::OPENAI_CURATED_MARKETPLACE_NAME; use codex_core_plugins::PluginsManager; use codex_core_plugins::TOOL_SUGGEST_DISCOVERABLE_PLUGIN_ALLOWLIST as TOOL_SUGGEST_DISCOVERABLE_PLUGIN_FALLBACK_ALLOWLIST; use codex_core_plugins::marketplace::MarketplacePluginInstallPolicy; +use codex_core_plugins::remote::RemoteMarketplace; +use codex_core_plugins::remote::RemotePluginCatalogError; use codex_features::Feature; +use codex_login::CodexAuth; use codex_tools::DiscoverablePluginInfo; const TOOL_SUGGEST_DISCOVERABLE_MARKETPLACE_ALLOWLIST: &[&str] = &[ @@ -20,13 +25,14 @@ const TOOL_SUGGEST_DISCOVERABLE_MARKETPLACE_ALLOWLIST: &[&str] = &[ pub(crate) async fn list_tool_suggest_discoverable_plugins( config: &Config, + auth: Option<&CodexAuth>, + plugins_manager: &PluginsManager, loaded_plugin_app_connector_ids: &[String], ) -> anyhow::Result> { if !config.features.enabled(Feature::Plugins) { return Ok(Vec::new()); } - let plugins_manager = PluginsManager::new(config.codex_home.to_path_buf()); let plugins_input = config.plugins_config_input(); let configured_plugin_ids = config .tool_suggest @@ -113,6 +119,20 @@ pub(crate) async fn list_tool_suggest_discoverable_plugins( } } } + match plugins_manager + .remote_tool_suggest_marketplace_for_config(&plugins_input, auth) + .await + { + Ok(remote_tool_suggest_marketplace) => append_remote_discoverable_plugins( + remote_tool_suggest_marketplace.as_ref(), + &disabled_plugin_ids, + &mut discoverable_plugins, + ), + Err( + RemotePluginCatalogError::AuthRequired | RemotePluginCatalogError::UnsupportedAuthMode, + ) => {} + Err(err) => warn!("failed to load remote discoverable plugin suggestions: {err}"), + } discoverable_plugins.sort_by(|left, right| { left.name .cmp(&right.name) @@ -121,6 +141,45 @@ pub(crate) async fn list_tool_suggest_discoverable_plugins( Ok(discoverable_plugins) } +fn append_remote_discoverable_plugins( + remote_tool_suggest_marketplace: Option<&RemoteMarketplace>, + disabled_plugin_ids: &HashSet<&str>, + discoverable_plugins: &mut Vec, +) { + let Some(remote_marketplace) = remote_tool_suggest_marketplace else { + return; + }; + + for plugin in &remote_marketplace.plugins { + if plugin.installed + || plugin.install_policy == PluginInstallPolicy::NotAvailable + || plugin.availability == PluginAvailability::DisabledByAdmin + || disabled_plugin_ids.contains(plugin.id.as_str()) + { + continue; + } + + let name = plugin + .interface + .as_ref() + .and_then(|interface| interface.display_name.clone()) + .unwrap_or_else(|| plugin.name.clone()); + let description = plugin + .interface + .as_ref() + .and_then(|interface| interface.short_description.clone()) + .or_else(|| plugin.description.clone()); + discoverable_plugins.push(DiscoverablePluginInfo { + id: plugin.id.clone(), + name, + description, + has_skills: plugin.has_skills, + mcp_server_names: Vec::new(), + app_connector_ids: plugin.app_ids.clone(), + }); + } +} + #[cfg(test)] #[path = "discoverable_tests.rs"] mod tests; diff --git a/codex-rs/core/src/plugins/discoverable_tests.rs b/codex-rs/core/src/plugins/discoverable_tests.rs index 21203aea23f..477fe0dd463 100644 --- a/codex-rs/core/src/plugins/discoverable_tests.rs +++ b/codex-rs/core/src/plugins/discoverable_tests.rs @@ -8,6 +8,7 @@ use crate::plugins::test_support::write_plugins_feature_config; use codex_core_plugins::OPENAI_BUNDLED_MARKETPLACE_NAME; use codex_core_plugins::PluginInstallRequest; use codex_core_plugins::startup_sync::curated_plugins_repo_path; +use codex_login::CodexAuth; use codex_tools::DiscoverablePluginInfo; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; @@ -16,6 +17,12 @@ use tempfile::tempdir; use tracing::Level; use tracing_subscriber::fmt::format::FmtSpan; use tracing_test::internal::MockWriter; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::ResponseTemplate; +use wiremock::matchers::method; +use wiremock::matchers::path; +use wiremock::matchers::query_param; #[tokio::test] async fn list_tool_suggest_discoverable_plugins_returns_fallback_plugins_without_installed_apps() { @@ -25,9 +32,7 @@ async fn list_tool_suggest_discoverable_plugins_returns_fallback_plugins_without write_plugins_feature_config(codex_home.path()); let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) - .await - .unwrap(); + let discoverable_plugins = list_local_discoverable_plugins(&config, &[]).await.unwrap(); assert_eq!( discoverable_plugins @@ -41,6 +46,83 @@ async fn list_tool_suggest_discoverable_plugins_returns_fallback_plugins_without ); } +#[tokio::test] +async fn list_tool_suggest_discoverable_plugins_includes_remote_vertical_plugins() { + let codex_home = tempdir().expect("tempdir should succeed"); + write_plugins_feature_config(codex_home.path()); + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/list")) + .and(query_param("scope", "GLOBAL")) + .and(query_param("limit", "200")) + .and(query_param("collection", "vertical")) + .respond_with(ResponseTemplate::new(200).set_body_string( + r#"{ + "plugins": [{ + "id": "plugins~Plugin_data_analytics", + "name": "data-analytics", + "scope": "GLOBAL", + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "status": "ENABLED", + "release": { + "display_name": "Data Analytics", + "description": "Analyze product and business metrics", + "app_ids": ["asdk_app_databricks_workspace"], + "interface": {}, + "skills": [{ + "name": "build-report", + "description": "Build an analytics report", + "plugin_release_skill_id": "skill-1", + "interface": {} + }] + } + }], + "pagination": { "next_page_token": null } +}"#, + )) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param("scope", "GLOBAL")) + .respond_with( + ResponseTemplate::new(200) + .set_body_string(r#"{"plugins": [], "pagination": {"next_page_token": null}}"#), + ) + .expect(1) + .mount(&server) + .await; + + let mut config = load_plugins_config(codex_home.path()).await; + config.chatgpt_base_url = format!("{}/backend-api", server.uri()); + let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let discoverable_plugins = + list_tool_suggest_discoverable_plugins(&config, Some(&auth), &plugins_manager, &[]) + .await + .expect("remote discoverable plugins should load"); + let cached_discoverable_plugins = + list_tool_suggest_discoverable_plugins(&config, Some(&auth), &plugins_manager, &[]) + .await + .expect("cached remote discoverable plugins should load"); + + assert_eq!( + discoverable_plugins, + vec![DiscoverablePluginInfo { + id: "data-analytics@openai-curated-remote".to_string(), + name: "Data Analytics".to_string(), + description: Some("Analyze product and business metrics".to_string()), + has_skills: true, + mcp_server_names: Vec::new(), + app_connector_ids: vec!["asdk_app_databricks_workspace".to_string()], + }] + ); + assert_eq!(cached_discoverable_plugins, discoverable_plugins); + server.verify().await; +} + #[tokio::test] async fn list_tool_suggest_discoverable_plugins_filters_non_fallback_by_installed_apps() { let codex_home = tempdir().expect("tempdir should succeed"); @@ -51,9 +133,7 @@ async fn list_tool_suggest_discoverable_plugins_filters_non_fallback_by_installe install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "slack").await; let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) - .await - .unwrap(); + let discoverable_plugins = list_local_discoverable_plugins(&config, &[]).await.unwrap(); assert_eq!( discoverable_plugins @@ -77,7 +157,7 @@ async fn list_tool_suggest_discoverable_plugins_filters_by_loaded_plugin_apps() let config = load_plugins_config(codex_home.path()).await; let discoverable_plugins = - list_tool_suggest_discoverable_plugins(&config, &[hubspot_app_id.to_string()]) + list_local_discoverable_plugins(&config, &[hubspot_app_id.to_string()]) .await .unwrap(); @@ -102,9 +182,7 @@ async fn list_tool_suggest_discoverable_plugins_filters_microsoft_by_installed_a install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "teams").await; let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) - .await - .unwrap(); + let discoverable_plugins = list_local_discoverable_plugins(&config, &[]).await.unwrap(); assert_eq!( discoverable_plugins @@ -179,9 +257,7 @@ source = "/tmp/{sales_marketplace_name}" install_marketplace_plugin(codex_home.path(), sales_marketplace_root.as_path(), "sales").await; let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) - .await - .unwrap(); + let discoverable_plugins = list_local_discoverable_plugins(&config, &[]).await.unwrap(); assert_eq!( discoverable_plugins @@ -234,9 +310,7 @@ discoverables = [{{ type = "plugin", id = "{plugin_id}" }}] ); let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) - .await - .unwrap(); + let discoverable_plugins = list_local_discoverable_plugins(&config, &[]).await.unwrap(); assert_eq!(discoverable_plugins.len(), 1); assert_eq!(discoverable_plugins[0].id, plugin_id.as_str()); @@ -278,9 +352,7 @@ source = "/tmp/{marketplace_name}" install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "installed").await; let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) - .await - .unwrap(); + let discoverable_plugins = list_local_discoverable_plugins(&config, &[]).await.unwrap(); assert_eq!(discoverable_plugins.len(), 1); assert_eq!(discoverable_plugins[0].id, "slack@openai-curated"); @@ -299,9 +371,7 @@ plugins = false ); let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) - .await - .unwrap(); + let discoverable_plugins = list_local_discoverable_plugins(&config, &[]).await.unwrap(); assert_eq!(discoverable_plugins, Vec::::new()); } @@ -322,9 +392,7 @@ async fn list_tool_suggest_discoverable_plugins_normalizes_description() { install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "installed").await; let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) - .await - .unwrap(); + let discoverable_plugins = list_local_discoverable_plugins(&config, &[]).await.unwrap(); assert_eq!( discoverable_plugins, @@ -359,7 +427,7 @@ async fn list_tool_suggest_discoverable_plugins_omits_installed_curated_plugins( .expect("plugin should install"); let refreshed_config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&refreshed_config, &[]) + let discoverable_plugins = list_local_discoverable_plugins(&refreshed_config, &[]) .await .unwrap(); @@ -384,9 +452,7 @@ disabled_tools = [ ); let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) - .await - .unwrap(); + let discoverable_plugins = list_local_discoverable_plugins(&config, &[]).await.unwrap(); assert_eq!(discoverable_plugins, Vec::::new()); } @@ -435,9 +501,7 @@ async fn list_tool_suggest_discoverable_plugins_omits_not_available_curated_plug install_marketplace_plugin(codex_home.path(), curated_root.as_path(), "installed").await; let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) - .await - .unwrap(); + let discoverable_plugins = list_local_discoverable_plugins(&config, &[]).await.unwrap(); assert_eq!( discoverable_plugins @@ -464,9 +528,7 @@ discoverables = [{ type = "plugin", id = "sample@openai-curated" }] ); let config = load_plugins_config(codex_home.path()).await; - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) - .await - .unwrap(); + let discoverable_plugins = list_local_discoverable_plugins(&config, &[]).await.unwrap(); assert_eq!( discoverable_plugins, @@ -519,9 +581,7 @@ async fn list_tool_suggest_discoverable_plugins_does_not_reload_marketplace_per_ .finish(); let _guard = tracing::subscriber::set_default(subscriber); - let discoverable_plugins = list_tool_suggest_discoverable_plugins(&config, &[]) - .await - .unwrap(); + let discoverable_plugins = list_local_discoverable_plugins(&config, &[]).await.unwrap(); assert_eq!( discoverable_plugins @@ -579,3 +639,17 @@ fn write_plugin_app(root: &Path, plugin_name: &str, app_name: &str, app_id: &str ), ); } + +async fn list_local_discoverable_plugins( + config: &crate::config::Config, + loaded_plugin_app_connector_ids: &[String], +) -> anyhow::Result> { + let plugins_manager = PluginsManager::new(config.codex_home.to_path_buf()); + list_tool_suggest_discoverable_plugins( + config, + /*auth*/ None, + &plugins_manager, + loaded_plugin_app_connector_ids, + ) + .await +} diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index f808134f99b..99138e61e48 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -1083,6 +1083,7 @@ pub(crate) async fn built_tools( auth.as_ref(), accessible_connectors.as_slice(), &loaded_plugin_app_connector_ids, + sess.services.plugins_manager.as_ref(), ) .await .map(|discoverable_tools| { diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install.rs b/codex-rs/core/src/tools/handlers/request_plugin_install.rs index c134eae079a..804214535aa 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install.rs @@ -2,6 +2,7 @@ use std::collections::HashSet; use codex_app_server_protocol::AppInfo; use codex_config::types::ToolSuggestDisabledTool; +use codex_core_plugins::remote::RemotePluginScope; use codex_mcp::CODEX_APPS_MCP_SERVER_NAME; use codex_rmcp_client::ElicitationAction; use codex_rmcp_client::ElicitationResponse; @@ -279,7 +280,9 @@ async fn verify_request_plugin_install_completed( plugin.id.as_str(), config.as_ref(), session.services.plugins_manager.as_ref(), - ); + auth, + ) + .await; let _ = refresh_missing_requested_connectors( session, turn, @@ -340,18 +343,36 @@ async fn refresh_missing_requested_connectors( } } -fn verified_plugin_install_completed( +async fn verified_plugin_install_completed( tool_id: &str, config: &crate::config::Config, plugins_manager: &codex_core_plugins::PluginsManager, + auth: Option<&codex_login::CodexAuth>, ) -> bool { let plugins_input = config.plugins_config_input(); - plugins_manager + let is_locally_installed = plugins_manager .list_marketplaces_for_config(&plugins_input, &[]) .ok() .into_iter() .flat_map(|outcome| outcome.marketplaces) .flat_map(|marketplace| marketplace.plugins.into_iter()) + .any(|plugin| plugin.id == tool_id && plugin.installed); + if is_locally_installed { + return true; + } + + plugins_manager + .build_and_cache_remote_installed_plugin_marketplaces( + &plugins_input, + auth, + &[RemotePluginScope::Global, RemotePluginScope::Workspace], + /*on_effective_plugins_changed*/ None, + ) + .await + .ok() + .into_iter() + .flatten() + .flat_map(|marketplace| marketplace.plugins) .any(|plugin| plugin.id == tool_id && plugin.installed) } diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs b/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs index 1a8caf0dceb..aef48386987 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs @@ -12,6 +12,7 @@ use codex_config::types::ToolSuggestDiscoverableType; use codex_core_plugins::PluginInstallRequest; use codex_core_plugins::PluginsManager; use codex_core_plugins::startup_sync::curated_plugins_repo_path; +use codex_login::CodexAuth; use codex_rmcp_client::ElicitationResponse; use codex_tools::DiscoverablePluginInfo; use codex_utils_absolute_path::AbsolutePathBuf; @@ -20,6 +21,12 @@ use pretty_assertions::assert_eq; use rmcp::model::ElicitationAction; use serde_json::json; use tempfile::tempdir; +use wiremock::Mock; +use wiremock::MockServer; +use wiremock::ResponseTemplate; +use wiremock::matchers::method; +use wiremock::matchers::path; +use wiremock::matchers::query_param; #[tokio::test] async fn verified_plugin_install_completed_requires_installed_plugin() { @@ -32,11 +39,15 @@ async fn verified_plugin_install_completed_requires_installed_plugin() { let config = load_plugins_config(codex_home.path()).await; let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); - assert!(!verified_plugin_install_completed( - "sample@openai-curated", - &config, - &plugins_manager, - )); + assert!( + !verified_plugin_install_completed( + "sample@openai-curated", + &config, + &plugins_manager, + None, + ) + .await + ); plugins_manager .install_plugin(PluginInstallRequest { @@ -50,11 +61,72 @@ async fn verified_plugin_install_completed_requires_installed_plugin() { .expect("plugin should install"); let refreshed_config = load_plugins_config(codex_home.path()).await; - assert!(verified_plugin_install_completed( - "sample@openai-curated", - &refreshed_config, - &plugins_manager, - )); + assert!( + verified_plugin_install_completed( + "sample@openai-curated", + &refreshed_config, + &plugins_manager, + None, + ) + .await + ); +} + +#[tokio::test] +async fn verified_plugin_install_completed_checks_remote_installed_state() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param("scope", "GLOBAL")) + .respond_with(ResponseTemplate::new(200).set_body_string( + r#"{ + "plugins": [{ + "id": "plugins~Plugin_data_analytics", + "name": "data-analytics", + "scope": "GLOBAL", + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "status": "ENABLED", + "release": { + "display_name": "Data Analytics", + "description": "Analyze metrics", + "interface": {}, + "skills": [] + }, + "enabled": true, + "disabled_skill_names": [] + }], + "pagination": { "next_page_token": null } +}"#, + )) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param("scope", "WORKSPACE")) + .respond_with( + ResponseTemplate::new(200) + .set_body_string(r#"{"plugins": [], "pagination": {"next_page_token": null}}"#), + ) + .mount(&server) + .await; + + let codex_home = tempdir().expect("tempdir should succeed"); + write_plugins_feature_config(codex_home.path()); + let mut config = load_plugins_config(codex_home.path()).await; + config.chatgpt_base_url = format!("{}/backend-api", server.uri()); + let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + + assert!( + verified_plugin_install_completed( + "data-analytics@openai-curated-remote", + &config, + &plugins_manager, + Some(&auth), + ) + .await + ); } #[test] From ee22b7c72b3034f30dc1092d6cd717d405008bee Mon Sep 17 00:00:00 2001 From: Eric Ning Date: Fri, 29 May 2026 18:51:23 -0700 Subject: [PATCH 2/2] update tests --- codex-rs/core-plugins/src/manager_tests.rs | 10 ++++----- .../core/src/plugins/discoverable_tests.rs | 22 +++++++++---------- .../handlers/request_plugin_install_tests.rs | 10 ++++----- codex-rs/tui/src/lib.rs | 4 ++-- 4 files changed, 23 insertions(+), 23 deletions(-) diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index ea821a3eec5..84be403f298 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -443,16 +443,16 @@ plugins = true .respond_with(ResponseTemplate::new(200).set_body_string( r#"{ "plugins": [{ - "id": "plugins~Plugin_data_analytics", - "name": "data-analytics", + "id": "plugins~Plugin_sample_remote", + "name": "sample-remote", "scope": "GLOBAL", "installation_policy": "AVAILABLE", "authentication_policy": "ON_USE", "status": "ENABLED", "release": { - "display_name": "Data Analytics", - "description": "Analyze metrics", - "app_ids": ["asdk_app_databricks_workspace"], + "display_name": "Sample Remote", + "description": "Sample remote plugin", + "app_ids": ["asdk_app_sample_source"], "interface": {}, "skills": [] } diff --git a/codex-rs/core/src/plugins/discoverable_tests.rs b/codex-rs/core/src/plugins/discoverable_tests.rs index 477fe0dd463..9e36893c756 100644 --- a/codex-rs/core/src/plugins/discoverable_tests.rs +++ b/codex-rs/core/src/plugins/discoverable_tests.rs @@ -59,20 +59,20 @@ async fn list_tool_suggest_discoverable_plugins_includes_remote_vertical_plugins .respond_with(ResponseTemplate::new(200).set_body_string( r#"{ "plugins": [{ - "id": "plugins~Plugin_data_analytics", - "name": "data-analytics", + "id": "plugins~Plugin_sample_remote", + "name": "sample-remote", "scope": "GLOBAL", "installation_policy": "AVAILABLE", "authentication_policy": "ON_USE", "status": "ENABLED", "release": { - "display_name": "Data Analytics", - "description": "Analyze product and business metrics", - "app_ids": ["asdk_app_databricks_workspace"], + "display_name": "Sample Remote", + "description": "Sample remote plugin", + "app_ids": ["asdk_app_sample_source"], "interface": {}, "skills": [{ - "name": "build-report", - "description": "Build an analytics report", + "name": "sample-skill", + "description": "Run a sample workflow", "plugin_release_skill_id": "skill-1", "interface": {} }] @@ -111,12 +111,12 @@ async fn list_tool_suggest_discoverable_plugins_includes_remote_vertical_plugins assert_eq!( discoverable_plugins, vec![DiscoverablePluginInfo { - id: "data-analytics@openai-curated-remote".to_string(), - name: "Data Analytics".to_string(), - description: Some("Analyze product and business metrics".to_string()), + id: "sample-remote@openai-curated-remote".to_string(), + name: "Sample Remote".to_string(), + description: Some("Sample remote plugin".to_string()), has_skills: true, mcp_server_names: Vec::new(), - app_connector_ids: vec!["asdk_app_databricks_workspace".to_string()], + app_connector_ids: vec!["asdk_app_sample_source".to_string()], }] ); assert_eq!(cached_discoverable_plugins, discoverable_plugins); diff --git a/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs b/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs index aef48386987..b732142dd05 100644 --- a/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs +++ b/codex-rs/core/src/tools/handlers/request_plugin_install_tests.rs @@ -81,15 +81,15 @@ async fn verified_plugin_install_completed_checks_remote_installed_state() { .respond_with(ResponseTemplate::new(200).set_body_string( r#"{ "plugins": [{ - "id": "plugins~Plugin_data_analytics", - "name": "data-analytics", + "id": "plugins~Plugin_sample_remote", + "name": "sample-remote", "scope": "GLOBAL", "installation_policy": "AVAILABLE", "authentication_policy": "ON_USE", "status": "ENABLED", "release": { - "display_name": "Data Analytics", - "description": "Analyze metrics", + "display_name": "Sample Remote", + "description": "Sample remote plugin", "interface": {}, "skills": [] }, @@ -120,7 +120,7 @@ async fn verified_plugin_install_completed_checks_remote_installed_state() { assert!( verified_plugin_install_completed( - "data-analytics@openai-curated-remote", + "sample-remote@openai-curated-remote", &config, &plugins_manager, Some(&auth), diff --git a/codex-rs/tui/src/lib.rs b/codex-rs/tui/src/lib.rs index 36b800145e2..2a276beac4a 100644 --- a/codex-rs/tui/src/lib.rs +++ b/codex-rs/tui/src/lib.rs @@ -453,7 +453,7 @@ async fn connect_remote_app_server( ) -> color_eyre::Result { let app_server = RemoteAppServerClient::connect(RemoteAppServerConnectArgs { endpoint, - client_name: "codex-tui".to_string(), + client_name: "codex-plugin-suggestion-test".to_string(), client_version: env!("CARGO_PKG_VERSION").to_string(), experimental_api: true, opt_out_notification_methods: Vec::new(), @@ -616,7 +616,7 @@ where session_source: serde_json::from_value(serde_json::json!("cli")) .unwrap_or_else(|err| panic!("cli session source should deserialize: {err}")), enable_codex_api_key_env: false, - client_name: "codex-tui".to_string(), + client_name: "codex-plugin-suggestion-test".to_string(), client_version: env!("CARGO_PKG_VERSION").to_string(), experimental_api: true, opt_out_notification_methods: Vec::new(),