From 7e735b59ce28e400d53bc636c2f8111fe6678930 Mon Sep 17 00:00:00 2001 From: Alex Daley Date: Tue, 16 Jun 2026 18:22:21 -0400 Subject: [PATCH] [codex] [1/4] Add recommended plugin endpoint cache (#28399) Summary - Add authenticated parsing for `/ps/plugins/suggested?scope=GLOBAL`, including remote plugin and connector app identities. - Validate, deduplicate, sort, and cap endpoint candidates before caching them by backend and account identity. - Deduplicate concurrent cache misses and warm recommendations from the existing remote-installed-plugin refresh path used at startup and after account changes. - Keep endpoint results model-invisible in this PR; failures and responses without `enabled: true` resolve to legacy mode. Stack - 1/3. Follow-up: #28400 generalizes plugin suggestion presentation without activating endpoint recommendations. - Final activation: #27704. Validation - `just test -p codex-core-plugins recommended_plugins` - `just fix -p codex-core-plugins` - `just fmt` - `git diff --check` --- .../request_processors/account_processor.rs | 13 +- codex-rs/core-plugins/src/lib.rs | 2 + codex-rs/core-plugins/src/manager.rs | 132 +++++++++- codex-rs/core-plugins/src/manager_tests.rs | 232 ++++++++++++++++++ codex-rs/core-plugins/src/remote.rs | 124 ++++++++++ codex-rs/core-plugins/src/remote_tests.rs | 203 +++++++++++++++ 6 files changed, 695 insertions(+), 11 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/account_processor.rs b/codex-rs/app-server/src/request_processors/account_processor.rs index 3dbef6773..1fa0961dd 100644 --- a/codex-rs/app-server/src/request_processors/account_processor.rs +++ b/codex-rs/app-server/src/request_processors/account_processor.rs @@ -173,7 +173,7 @@ impl AccountRequestProcessor { } } - async fn maybe_refresh_remote_installed_plugins_cache_for_current_config( + async fn maybe_refresh_plugin_caches_for_current_config( config_manager: &ConfigManager, thread_manager: &Arc, auth: Option, @@ -181,6 +181,9 @@ impl AccountRequestProcessor { thread_manager .plugins_manager() .set_auth_mode(auth.as_ref().map(CodexAuth::api_auth_mode)); + thread_manager + .plugins_manager() + .clear_recommended_plugins_cache(); match config_manager .load_latest_config(/*fallback_cwd*/ None) @@ -191,7 +194,7 @@ impl AccountRequestProcessor { let refresh_config_manager = config_manager.clone(); thread_manager .plugins_manager() - .maybe_start_remote_installed_plugins_cache_refresh( + .maybe_start_remote_plugin_caches_refresh( &config.plugins_config_input(), auth, Some(Arc::new(move || { @@ -618,7 +621,7 @@ impl AccountRequestProcessor { } async fn send_login_success_notifications(&self, login_id: Option) { - Self::maybe_refresh_remote_installed_plugins_cache_for_current_config( + Self::maybe_refresh_plugin_caches_for_current_config( &self.config_manager, &self.thread_manager, self.auth_manager.auth_cached(), @@ -671,7 +674,7 @@ impl AccountRequestProcessor { .await; let auth = auth_manager.auth_cached(); - Self::maybe_refresh_remote_installed_plugins_cache_for_current_config( + Self::maybe_refresh_plugin_caches_for_current_config( &config_manager, &thread_manager, auth.clone(), @@ -703,7 +706,7 @@ impl AccountRequestProcessor { } } - Self::maybe_refresh_remote_installed_plugins_cache_for_current_config( + Self::maybe_refresh_plugin_caches_for_current_config( &self.config_manager, &self.thread_manager, self.auth_manager.auth_cached(), diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index 56775d076..1b0f49223 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -54,3 +54,5 @@ pub use marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome as PluginMarket pub use provider::ExecutorPluginProvider; pub use provider::ExecutorPluginProviderError; pub use provider::ResolvedExecutorPlugin; +pub use remote::RecommendedPlugin; +pub use remote::RecommendedPluginsMode; diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 9ed4d82c5..e92b8d935 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -38,6 +38,7 @@ use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeError; use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome; use crate::marketplace_upgrade::configured_git_marketplace_names; use crate::marketplace_upgrade::upgrade_configured_git_marketplaces; +use crate::remote::RecommendedPluginsMode; use crate::remote::RemoteInstalledPlugin; use crate::remote::RemotePluginCatalogError; use crate::remote::RemotePluginServiceConfig; @@ -80,6 +81,7 @@ use std::sync::RwLock; use std::sync::atomic::AtomicBool; use std::sync::atomic::Ordering; use std::time::Instant; +use tokio::sync::OnceCell; use tokio::sync::Semaphore; use tracing::instrument; use tracing::warn; @@ -120,6 +122,11 @@ struct FeaturedPluginIdsCacheKey { is_workspace_account: bool, } +#[derive(Clone, Hash, PartialEq, Eq)] +struct RecommendedPluginsCacheKey { + chatgpt_base_url: String, +} + #[derive(Clone)] struct CachedFeaturedPluginIds { key: FeaturedPluginIdsCacheKey, @@ -209,6 +216,12 @@ fn featured_plugin_ids_cache_key( } } +fn recommended_plugins_cache_key(config: &PluginsConfigInput) -> RecommendedPluginsCacheKey { + RecommendedPluginsCacheKey { + chatgpt_base_url: config.chatgpt_base_url.clone(), + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct PluginInstallRequest { pub plugin_name: String, @@ -318,6 +331,9 @@ pub struct PluginsManager { codex_home: PathBuf, store: PluginStore, featured_plugin_ids_cache: RwLock>, + recommended_plugins_cache: RwLock>, + recommended_plugins_refreshes: + RwLock>>>, configured_marketplace_upgrade_state: RwLock, non_curated_cache_refresh_state: RwLock, // Keep the cache auth-independent so auth changes only need to resolve capabilities again. @@ -371,6 +387,8 @@ impl PluginsManager { codex_home: codex_home.clone(), store: PluginStore::new(codex_home), featured_plugin_ids_cache: RwLock::new(None), + recommended_plugins_cache: RwLock::new(HashMap::new()), + recommended_plugins_refreshes: RwLock::new(HashMap::new()), configured_marketplace_upgrade_state: RwLock::new( ConfiguredMarketplaceUpgradeState::default(), ), @@ -499,6 +517,19 @@ impl PluginsManager { *featured_plugin_ids_cache = None; } + pub fn clear_recommended_plugins_cache(&self) { + let mut refreshes = match self.recommended_plugins_refreshes.write() { + Ok(refreshes) => refreshes, + Err(err) => err.into_inner(), + }; + refreshes.clear(); + let mut cache = match self.recommended_plugins_cache.write() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + cache.clear(); + } + fn clear_loaded_plugins_cache(&self) { let mut cache = match self.loaded_plugins_cache.write() { Ok(cache) => cache, @@ -700,7 +731,7 @@ impl PluginsManager { true } - pub fn maybe_start_remote_installed_plugins_cache_refresh( + pub fn maybe_start_remote_plugin_caches_refresh( self: &Arc, config: &PluginsConfigInput, auth: Option, @@ -708,10 +739,18 @@ impl PluginsManager { ) { self.maybe_start_remote_installed_plugins_cache_refresh_with_notify( config, - auth, + auth.clone(), RemoteInstalledPluginsCacheRefreshNotify::IfCacheChanged, on_effective_plugins_changed, ); + + let manager = Arc::clone(self); + let config = config.clone(); + tokio::spawn(async move { + manager + .recommended_plugins_mode_for_config(&config, auth.as_ref()) + .await; + }); } pub fn maybe_start_remote_installed_plugins_cache_refresh_after_mutation( @@ -805,7 +844,7 @@ impl PluginsManager { if options.refresh_global_remote_catalog_cache { self.maybe_start_global_remote_catalog_cache_refresh(config, auth.clone()); } - self.maybe_start_remote_installed_plugins_cache_refresh( + self.maybe_start_remote_plugin_caches_refresh( config, auth.clone(), on_effective_plugins_changed.clone(), @@ -888,6 +927,87 @@ impl PluginsManager { Ok(featured_plugin_ids) } + pub async fn recommended_plugins_mode_for_config( + &self, + config: &PluginsConfigInput, + auth: Option<&CodexAuth>, + ) -> RecommendedPluginsMode { + if !config.plugins_enabled + || !config.remote_plugin_enabled + || !auth.is_some_and(CodexAuth::uses_codex_backend) + { + return RecommendedPluginsMode::Legacy; + } + + let cache_key = recommended_plugins_cache_key(config); + if let Some(cached) = self.cached_recommended_plugins_mode(&cache_key) { + return cached; + } + + let refresh = { + let mut refreshes = match self.recommended_plugins_refreshes.write() { + Ok(refreshes) => refreshes, + Err(err) => err.into_inner(), + }; + if let Some(cached) = self.cached_recommended_plugins_mode(&cache_key) { + return cached; + } + refreshes + .entry(cache_key.clone()) + .or_insert_with(|| Arc::new(OnceCell::new())) + .clone() + }; + + let mode = refresh + .get_or_init(|| async { + match crate::remote::fetch_recommended_plugins( + &remote_plugin_service_config(config), + auth, + ) + .await + { + Ok(mode) => { + let mut cache = match self.recommended_plugins_cache.write() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + cache.insert(cache_key.clone(), mode.clone()); + mode + } + Err(err) => { + warn!(error = %err, "failed to load recommended plugins"); + RecommendedPluginsMode::Legacy + } + } + }) + .await + .clone(); + + let mut refreshes = match self.recommended_plugins_refreshes.write() { + Ok(refreshes) => refreshes, + Err(err) => err.into_inner(), + }; + if refreshes + .get(&cache_key) + .is_some_and(|current| Arc::ptr_eq(current, &refresh)) + { + refreshes.remove(&cache_key); + } + + mode + } + + fn cached_recommended_plugins_mode( + &self, + cache_key: &RecommendedPluginsCacheKey, + ) -> Option { + let cache = match self.recommended_plugins_cache.read() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + cache.get(cache_key).cloned() + } + pub async fn install_plugin( &self, request: PluginInstallRequest, @@ -1420,7 +1540,7 @@ impl PluginsManager { let on_effective_plugins_changed = on_effective_plugins_changed.clone(); tokio::spawn(async move { let auth = auth_manager_for_remote_sync.auth().await; - manager.maybe_start_remote_installed_plugins_cache_refresh( + manager.maybe_start_remote_plugin_caches_refresh( &config_for_remote_sync, auth.clone(), on_effective_plugins_changed.clone(), @@ -1453,12 +1573,12 @@ impl PluginsManager { } }); - let config = config.clone(); + let config_for_featured_plugins = config.clone(); let manager = Arc::clone(self); tokio::spawn(async move { let auth = auth_manager.auth().await; if let Err(err) = manager - .featured_plugin_ids_for_config(&config, auth.as_ref()) + .featured_plugin_ids_for_config(&config_for_featured_plugins, auth.as_ref()) .await { warn!( diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 8ca3e0ac8..f219f5e76 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -11,6 +11,7 @@ use crate::marketplace::MarketplacePluginInstallPolicy; use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; use crate::remote::REMOTE_WORKSPACE_MARKETPLACE_NAME; use crate::remote::REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME; +use crate::remote::RecommendedPlugin; use crate::remote::RemoteInstalledPlugin; use crate::startup_sync::curated_plugins_repo_path; use crate::test_support::TEST_CURATED_PLUGIN_CACHE_VERSION; @@ -40,6 +41,7 @@ use codex_utils_absolute_path::test_support::PathBufExt; use pretty_assertions::assert_eq; use std::fs; use std::path::Path; +use std::time::Duration; use tempfile::TempDir; use toml::Value; use wiremock::Mock; @@ -3703,6 +3705,236 @@ plugins = true assert_eq!(featured_plugin_ids, vec!["codex-plugin".to_string()]); } +#[tokio::test] +async fn remote_plugin_caches_refresh_warms_recommended_plugins_cache() { + let tmp = tempfile::tempdir().unwrap(); + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +remote_plugin = true +"#, + ); + + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/ps/plugins/suggested")) + .and(query_param("scope", "GLOBAL")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "enabled": true, + "plugins": [] + }))) + .expect(1) + .mount(&server) + .await; + + let mut config = load_config(tmp.path(), tmp.path()).await; + config.chatgpt_base_url = server.uri(); + let manager = std::sync::Arc::new(PluginsManager::new(tmp.path().to_path_buf())); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let cache_key = recommended_plugins_cache_key(&config); + + manager.maybe_start_remote_plugin_caches_refresh( + &config, + Some(auth.clone()), + /*on_effective_plugins_changed*/ None, + ); + + let mode = tokio::time::timeout(Duration::from_secs(2), async { + loop { + if let Some(mode) = manager.cached_recommended_plugins_mode(&cache_key) { + break mode; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("recommended plugins cache should be warmed"); + assert_eq!( + mode, + RecommendedPluginsMode::Endpoint { + plugins: Vec::new() + } + ); + assert_eq!( + manager + .recommended_plugins_mode_for_config(&config, Some(&auth)) + .await, + mode + ); + manager.clear_recommended_plugins_cache(); + assert_eq!(manager.cached_recommended_plugins_mode(&cache_key), None); +} + +#[tokio::test] +async fn recommended_plugins_mode_deduplicates_concurrent_cache_misses() { + let tmp = tempfile::tempdir().unwrap(); + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +remote_plugin = true +"#, + ); + + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/ps/plugins/suggested")) + .and(query_param("scope", "GLOBAL")) + .and(header("authorization", "Bearer Access Token")) + .and(header("chatgpt-account-id", "account_id")) + .and(header("OAI-Product-Sku", "codex")) + .respond_with( + ResponseTemplate::new(200) + .set_body_json(serde_json::json!({ + "enabled": true, + "plugins": [ + { + "id": "plugin_slack", + "name": "slack", + "release": { + "display_name": "Slack", + "app_ids": ["connector_slack"] + } + }, + { + "id": "plugin_github", + "name": "github", + "release": {"display_name": "GitHub"} + } + ] + })) + .set_delay(Duration::from_millis(100)), + ) + .expect(1) + .mount(&server) + .await; + + let mut config = load_config(tmp.path(), tmp.path()).await; + config.chatgpt_base_url = server.uri(); + let manager = PluginsManager::new(tmp.path().to_path_buf()); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let expected = RecommendedPluginsMode::Endpoint { + plugins: vec![ + RecommendedPlugin { + config_id: "github@openai-curated-remote".to_string(), + remote_plugin_id: "plugin_github".to_string(), + display_name: "GitHub".to_string(), + app_connector_ids: Vec::new(), + }, + RecommendedPlugin { + config_id: "slack@openai-curated-remote".to_string(), + remote_plugin_id: "plugin_slack".to_string(), + display_name: "Slack".to_string(), + app_connector_ids: vec!["connector_slack".to_string()], + }, + ], + }; + + let (left, right) = tokio::join!( + manager.recommended_plugins_mode_for_config(&config, Some(&auth)), + manager.recommended_plugins_mode_for_config(&config, Some(&auth)), + ); + assert_eq!((left, right), (expected.clone(), expected.clone())); + assert_eq!( + manager + .recommended_plugins_mode_for_config(&config, Some(&auth)) + .await, + expected + ); +} + +#[tokio::test] +async fn recommended_plugins_mode_caches_explicit_false() { + let tmp = tempfile::tempdir().unwrap(); + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +remote_plugin = true +"#, + ); + + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/ps/plugins/suggested")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "enabled": false, + "plugins": [] + }))) + .expect(1) + .mount(&server) + .await; + + let mut config = load_config(tmp.path(), tmp.path()).await; + config.chatgpt_base_url = server.uri(); + let manager = PluginsManager::new(tmp.path().to_path_buf()); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + assert_eq!( + manager + .recommended_plugins_mode_for_config(&config, Some(&auth)) + .await, + RecommendedPluginsMode::Legacy + ); + assert_eq!( + manager + .recommended_plugins_mode_for_config(&config, Some(&auth)) + .await, + RecommendedPluginsMode::Legacy + ); +} + +#[tokio::test] +async fn recommended_plugins_mode_retries_after_fetch_failure() { + let tmp = tempfile::tempdir().unwrap(); + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +remote_plugin = true +"#, + ); + + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/ps/plugins/suggested")) + .respond_with(ResponseTemplate::new(500).set_body_string("unavailable")) + .expect(1) + .mount(&server) + .await; + + let mut config = load_config(tmp.path(), tmp.path()).await; + config.chatgpt_base_url = server.uri(); + let manager = PluginsManager::new(tmp.path().to_path_buf()); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + assert_eq!( + manager + .recommended_plugins_mode_for_config(&config, Some(&auth)) + .await, + RecommendedPluginsMode::Legacy + ); + + server.reset().await; + Mock::given(method("GET")) + .and(path("/ps/plugins/suggested")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "enabled": true, + "plugins": [] + }))) + .expect(1) + .mount(&server) + .await; + + assert_eq!( + manager + .recommended_plugins_mode_for_config(&config, Some(&auth)) + .await, + RecommendedPluginsMode::Endpoint { + plugins: Vec::new() + } + ); +} + #[test] fn refresh_curated_plugin_cache_replaces_existing_local_version_with_short_sha_version() { let tmp = tempfile::tempdir().unwrap(); diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index 1a712c08a..1f8d3f720 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -79,7 +79,11 @@ const OPENAI_CURATED_REMOTE_COLLECTION_KEY: &str = "vertical"; const OAI_PRODUCT_SKU_HEADER: &str = "OAI-Product-Sku"; const CODEX_PRODUCT_SKU: &str = "codex"; const REMOTE_PLUGIN_CATALOG_TIMEOUT: Duration = Duration::from_secs(30); +const RECOMMENDED_PLUGINS_TIMEOUT: Duration = Duration::from_secs(5); const REMOTE_PLUGIN_LIST_PAGE_LIMIT: u32 = 200; +const MAX_RECOMMENDED_PLUGINS: usize = 50; +const MAX_RECOMMENDED_PLUGIN_NAME_LEN: usize = 64; +const MAX_RECOMMENDED_PLUGIN_DISPLAY_NAME_LEN: usize = 64; const MAX_REMOTE_DEFAULT_PROMPT_COUNT: usize = 3; const MAX_REMOTE_DEFAULT_PROMPT_LEN: usize = 128; const INVALID_REQUEST_ERROR_CODE: i64 = -32600; @@ -237,6 +241,20 @@ pub struct RemoteDiscoverablePlugin { pub availability: PluginAvailability, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RecommendedPlugin { + pub config_id: String, + pub remote_plugin_id: String, + pub display_name: String, + pub app_connector_ids: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum RecommendedPluginsMode { + Legacy, + Endpoint { plugins: Vec }, +} + pub fn is_valid_remote_plugin_id(plugin_id: &str) -> bool { !plugin_id.is_empty() && plugin_id @@ -568,6 +586,32 @@ struct RemotePluginListResponse { pagination: RemotePluginPagination, } +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +struct RecommendedPluginsResponse { + #[serde(default)] + enabled: Option, + #[serde(default)] + plugins: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +struct RecommendedPluginItem { + id: String, + name: String, + #[serde(default)] + status: Option, + #[serde(default)] + installation_policy: Option, + release: RecommendedPluginRelease, +} + +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +struct RecommendedPluginRelease { + display_name: String, + #[serde(default)] + app_ids: Vec, +} + #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] struct RemotePluginInstalledResponse { plugins: Vec, @@ -761,6 +805,86 @@ pub async fn fetch_and_cache_global_remote_plugin_catalog( Ok(()) } +pub async fn fetch_recommended_plugins( + config: &RemotePluginServiceConfig, + auth: Option<&CodexAuth>, +) -> Result { + let auth = ensure_chatgpt_auth(auth)?; + let base_url = config.chatgpt_base_url.trim_end_matches('/'); + let url = format!("{base_url}/ps/plugins/suggested"); + let client = build_reqwest_client(); + let request = authenticated_request(client.get(&url), auth)? + .timeout(RECOMMENDED_PLUGINS_TIMEOUT) + .query(&[("scope", "GLOBAL")]); + let response: RecommendedPluginsResponse = send_and_decode(request, &url).await?; + Ok(recommended_plugins_mode(response)) +} + +fn recommended_plugins_mode(response: RecommendedPluginsResponse) -> RecommendedPluginsMode { + if response.enabled != Some(true) { + return RecommendedPluginsMode::Legacy; + } + + let mut plugins = BTreeMap::new(); + for plugin in response.plugins { + if !is_valid_remote_plugin_id(&plugin.id) + || plugin.name.chars().count() > MAX_RECOMMENDED_PLUGIN_NAME_LEN + || plugin + .status + .is_some_and(|status| status != PluginAvailability::Available) + || plugin + .installation_policy + .is_some_and(|policy| policy != PluginInstallPolicy::Available) + { + continue; + } + let plugin_id = match PluginId::new( + plugin.name.clone(), + REMOTE_GLOBAL_MARKETPLACE_NAME.to_string(), + ) { + Ok(plugin_id) => plugin_id, + Err(err) => { + tracing::warn!( + plugin_name = plugin.name, + error = %err, + "ignoring invalid recommended plugin" + ); + continue; + } + }; + let RecommendedPluginRelease { + display_name, + app_ids, + } = plugin.release; + let display_name = non_empty_string(Some(&display_name)) + .unwrap_or_else(|| plugin.name.clone()) + .chars() + .take(MAX_RECOMMENDED_PLUGIN_DISPLAY_NAME_LEN) + .collect(); + let mut seen_app_ids = HashSet::new(); + let app_connector_ids = app_ids + .into_iter() + .filter(|app_id| !app_id.is_empty() && seen_app_ids.insert(app_id.clone())) + .collect(); + let config_id = plugin_id.as_key(); + plugins + .entry(config_id.clone()) + .or_insert(RecommendedPlugin { + config_id, + remote_plugin_id: plugin.id, + display_name, + app_connector_ids, + }); + } + + RecommendedPluginsMode::Endpoint { + plugins: plugins + .into_values() + .take(MAX_RECOMMENDED_PLUGINS) + .collect(), + } +} + pub fn has_cached_global_remote_plugin_catalog( codex_home: &Path, config: &RemotePluginServiceConfig, diff --git a/codex-rs/core-plugins/src/remote_tests.rs b/codex-rs/core-plugins/src/remote_tests.rs index cff134b80..793a8a55b 100644 --- a/codex-rs/core-plugins/src/remote_tests.rs +++ b/codex-rs/core-plugins/src/remote_tests.rs @@ -76,3 +76,206 @@ fn directory_plugin(id: &str, name: &str) -> RemotePluginDirectoryItem { }, } } +fn item(name: &str, display_name: &str) -> RecommendedPluginItem { + RecommendedPluginItem { + id: format!("plugin_{name}"), + name: name.to_string(), + status: None, + installation_policy: None, + release: RecommendedPluginRelease { + display_name: display_name.to_string(), + app_ids: Vec::new(), + }, + } +} + +#[test] +fn recommended_plugins_enabled_flag_selects_endpoint_or_legacy_mode() { + let disabled: RecommendedPluginsResponse = serde_json::from_value(serde_json::json!({ + "enabled": false, + "plugins": [{"id": "plugin_github", "name": "github", "release": {"display_name": "GitHub"}}] + })) + .expect("response should deserialize"); + assert_eq!( + recommended_plugins_mode(disabled), + RecommendedPluginsMode::Legacy + ); + + for response in [ + serde_json::json!({"plugins": []}), + serde_json::json!({"enabled": null, "plugins": []}), + ] { + let response: RecommendedPluginsResponse = + serde_json::from_value(response).expect("response should deserialize"); + assert_eq!( + recommended_plugins_mode(response), + RecommendedPluginsMode::Legacy + ); + } + + let enabled: RecommendedPluginsResponse = serde_json::from_value(serde_json::json!({ + "enabled": true, + "plugins": [] + })) + .expect("response should deserialize"); + assert_eq!( + recommended_plugins_mode(enabled), + RecommendedPluginsMode::Endpoint { + plugins: Vec::new() + } + ); +} + +#[test] +fn recommended_plugins_require_remote_install_identity() { + let response = serde_json::from_value::(serde_json::json!({ + "enabled": true, + "plugins": [{ + "name": "github", + "release": {"display_name": "GitHub"} + }] + })); + + assert!(response.is_err()); +} + +#[test] +fn recommended_plugins_are_validated_deduplicated_sorted_and_capped() { + let mut plugins = (0..=52) + .rev() + .map(|index| item(&format!("plugin-{index:02}"), &format!("Plugin {index:02}"))) + .collect::>(); + plugins.push(item("plugin-00", "Duplicate")); + plugins.push(item("not/a/plugin", "Invalid")); + plugins.push(RecommendedPluginItem { + id: "plugin_disabled".to_string(), + name: "disabled".to_string(), + status: Some(PluginAvailability::DisabledByAdmin), + installation_policy: Some(PluginInstallPolicy::Available), + release: RecommendedPluginRelease { + display_name: "Disabled".to_string(), + app_ids: Vec::new(), + }, + }); + plugins.push(RecommendedPluginItem { + id: "plugin_not_available".to_string(), + name: "not-available".to_string(), + status: Some(PluginAvailability::Available), + installation_policy: Some(PluginInstallPolicy::NotAvailable), + release: RecommendedPluginRelease { + display_name: "Not Available".to_string(), + app_ids: Vec::new(), + }, + }); + + let mode = recommended_plugins_mode(RecommendedPluginsResponse { + enabled: Some(true), + plugins, + }); + let RecommendedPluginsMode::Endpoint { plugins } = mode else { + panic!("expected endpoint mode"); + }; + + assert_eq!(plugins.len(), MAX_RECOMMENDED_PLUGINS); + assert_eq!( + plugins.first(), + Some(&RecommendedPlugin { + config_id: "plugin-00@openai-curated-remote".to_string(), + remote_plugin_id: "plugin_plugin-00".to_string(), + display_name: "Plugin 00".to_string(), + app_connector_ids: Vec::new(), + }) + ); + assert_eq!( + plugins.last(), + Some(&RecommendedPlugin { + config_id: "plugin-49@openai-curated-remote".to_string(), + remote_plugin_id: "plugin_plugin-49".to_string(), + display_name: "Plugin 49".to_string(), + app_connector_ids: Vec::new(), + }) + ); +} + +#[test] +fn recommended_plugins_bound_model_visible_fields() { + let overlong_name = "n".repeat(MAX_RECOMMENDED_PLUGIN_NAME_LEN + 1); + let overlong_display_name = "D".repeat(MAX_RECOMMENDED_PLUGIN_DISPLAY_NAME_LEN + 1); + let mode = recommended_plugins_mode(RecommendedPluginsResponse { + enabled: Some(true), + plugins: vec![ + item(&overlong_name, "Ignored"), + item("bounded", &overlong_display_name), + ], + }); + + assert_eq!( + mode, + RecommendedPluginsMode::Endpoint { + plugins: vec![RecommendedPlugin { + config_id: "bounded@openai-curated-remote".to_string(), + remote_plugin_id: "plugin_bounded".to_string(), + display_name: "D".repeat(MAX_RECOMMENDED_PLUGIN_DISPLAY_NAME_LEN), + app_connector_ids: Vec::new(), + }], + } + ); +} + +#[test] +fn recommended_plugins_preserve_install_identity_and_normalize_app_ids() { + let mode = recommended_plugins_mode(RecommendedPluginsResponse { + enabled: Some(true), + plugins: vec![RecommendedPluginItem { + id: "plugin_connector_sample".to_string(), + name: "sample".to_string(), + status: Some(PluginAvailability::Available), + installation_policy: Some(PluginInstallPolicy::Available), + release: RecommendedPluginRelease { + display_name: "Sample".to_string(), + app_ids: vec![ + "connector_one".to_string(), + String::new(), + "connector_two".to_string(), + "connector_one".to_string(), + ], + }, + }], + }); + + assert_eq!( + mode, + RecommendedPluginsMode::Endpoint { + plugins: vec![RecommendedPlugin { + config_id: "sample@openai-curated-remote".to_string(), + remote_plugin_id: "plugin_connector_sample".to_string(), + display_name: "Sample".to_string(), + app_connector_ids: vec!["connector_one".to_string(), "connector_two".to_string(),], + }], + } + ); +} + +#[test] +fn recommended_plugins_ignore_invalid_remote_plugin_ids() { + let mode = recommended_plugins_mode(RecommendedPluginsResponse { + enabled: Some(true), + plugins: vec![RecommendedPluginItem { + id: "not/a/plugin".to_string(), + name: "sample".to_string(), + status: None, + installation_policy: None, + release: RecommendedPluginRelease { + display_name: "Sample".to_string(), + app_ids: Vec::new(), + }, + }], + }); + + assert_eq!( + mode, + RecommendedPluginsMode::Endpoint { + plugins: Vec::new(), + } + ); +}