From 5dea97bfaaab1283b2fb34f4d80d04a0cb4cc2ea Mon Sep 17 00:00:00 2001 From: Alex Daley Date: Mon, 15 Jun 2026 18:08:51 -0400 Subject: [PATCH 1/3] Add recommended plugin endpoint cache --- codex-rs/core-plugins/src/lib.rs | 2 + codex-rs/core-plugins/src/manager.rs | 121 ++++++++- codex-rs/core-plugins/src/manager_tests.rs | 273 +++++++++++++++++++++ codex-rs/core-plugins/src/remote.rs | 128 ++++++++++ codex-rs/core-plugins/src/remote_tests.rs | 178 ++++++++++++++ 5 files changed, 699 insertions(+), 3 deletions(-) diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index 56775d07626e..1b0f492231dc 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 9ed4d82c5c6f..0e14366c9144 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,14 @@ struct FeaturedPluginIdsCacheKey { is_workspace_account: bool, } +#[derive(Clone, Hash, PartialEq, Eq)] +struct RecommendedPluginsCacheKey { + chatgpt_base_url: String, + account_id: Option, + chatgpt_user_id: Option, + is_workspace_account: bool, +} + #[derive(Clone)] struct CachedFeaturedPluginIds { key: FeaturedPluginIdsCacheKey, @@ -209,6 +219,17 @@ fn featured_plugin_ids_cache_key( } } +fn recommended_plugins_cache_key( + config: &PluginsConfigInput, + auth: Option<&CodexAuth>, +) -> RecommendedPluginsCacheKey { + RecommendedPluginsCacheKey { + chatgpt_base_url: config.chatgpt_base_url.clone(), + account_id: auth.and_then(CodexAuth::get_account_id), + chatgpt_user_id: auth.and_then(CodexAuth::get_chatgpt_user_id), + is_workspace_account: auth.is_some_and(CodexAuth::is_workspace_account), + } +} #[derive(Debug, Clone, PartialEq, Eq)] pub struct PluginInstallRequest { pub plugin_name: String, @@ -318,6 +339,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 +395,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(), ), @@ -708,10 +734,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( @@ -888,6 +922,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, auth); + 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, @@ -1453,12 +1568,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 8ca3e0ac899c..11cdc5a335ad 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,277 @@ plugins = true assert_eq!(featured_plugin_ids, vec!["codex-plugin".to_string()]); } +#[tokio::test] +async fn remote_installed_plugins_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, Some(&auth)); + + manager.maybe_start_remote_installed_plugins_cache_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 + ); +} + +#[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_memory_cache_is_isolated_by_account() { + let tmp = tempfile::tempdir().unwrap(); + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +remote_plugin = true +"#, + ); + let config = load_config(tmp.path(), tmp.path()).await; + let manager = PluginsManager::new(tmp.path().to_path_buf()); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let mode = 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(), + }], + }; + { + let mut cache = manager.recommended_plugins_cache.write().unwrap(); + cache.insert( + recommended_plugins_cache_key(&config, Some(&auth)), + mode.clone(), + ); + } + + assert_eq!( + manager + .recommended_plugins_mode_for_config(&config, Some(&auth)) + .await, + mode + ); + assert_eq!( + manager + .recommended_plugins_mode_for_config(&config, /*auth*/ None) + .await, + RecommendedPluginsMode::Legacy + ); +} + +#[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 1a712c08a9a4..6d59007da95b 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 = 128; +const MAX_RECOMMENDED_PLUGIN_DISPLAY_NAME_LEN: usize = 256; 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, @@ -1732,3 +1856,7 @@ async fn send_and_decode Deserialize<'de>>( source, }) } + +#[cfg(test)] +#[path = "remote_tests.rs"] +mod tests; diff --git a/codex-rs/core-plugins/src/remote_tests.rs b/codex-rs/core-plugins/src/remote_tests.rs index cff134b80d01..a023aae4d161 100644 --- a/codex-rs/core-plugins/src/remote_tests.rs +++ b/codex-rs/core-plugins/src/remote_tests.rs @@ -76,3 +76,181 @@ 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_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(), + } + ); +} From 1abc796af38caa50a770974b17ea92756cec3dc6 Mon Sep 17 00:00:00 2001 From: Alex Daley Date: Mon, 15 Jun 2026 19:24:17 -0400 Subject: [PATCH 2/3] Address recommended plugin cache review feedback --- .../request_processors/account_processor.rs | 10 ++++---- codex-rs/core-plugins/src/manager.rs | 8 +++--- codex-rs/core-plugins/src/manager_tests.rs | 4 +-- codex-rs/core-plugins/src/remote.rs | 8 ++---- codex-rs/core-plugins/src/remote_tests.rs | 25 +++++++++++++++++++ 5 files changed, 39 insertions(+), 16 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 3dbef6773a68..bb136d4b26bd 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, @@ -191,7 +191,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 +618,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 +671,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 +703,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/manager.rs b/codex-rs/core-plugins/src/manager.rs index 0e14366c9144..3aa75bcd03b5 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -223,6 +223,8 @@ fn recommended_plugins_cache_key( config: &PluginsConfigInput, auth: Option<&CodexAuth>, ) -> RecommendedPluginsCacheKey { + // Account changes start a warmup but do not clear process-local entries. Keep install + // identities scoped to the auth that fetched them, including while a new warmup is in flight. RecommendedPluginsCacheKey { chatgpt_base_url: config.chatgpt_base_url.clone(), account_id: auth.and_then(CodexAuth::get_account_id), @@ -726,7 +728,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, @@ -839,7 +841,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(), @@ -1535,7 +1537,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(), diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 11cdc5a335ad..5a20c9d2cf70 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -3706,7 +3706,7 @@ plugins = true } #[tokio::test] -async fn remote_installed_plugins_refresh_warms_recommended_plugins_cache() { +async fn remote_plugin_caches_refresh_warms_recommended_plugins_cache() { let tmp = tempfile::tempdir().unwrap(); write_file( &tmp.path().join(CONFIG_TOML_FILE), @@ -3734,7 +3734,7 @@ remote_plugin = true let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let cache_key = recommended_plugins_cache_key(&config, Some(&auth)); - manager.maybe_start_remote_installed_plugins_cache_refresh( + manager.maybe_start_remote_plugin_caches_refresh( &config, Some(auth.clone()), /*on_effective_plugins_changed*/ None, diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index 6d59007da95b..1f8d3f7201d3 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -82,8 +82,8 @@ 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 = 128; -const MAX_RECOMMENDED_PLUGIN_DISPLAY_NAME_LEN: usize = 256; +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; @@ -1856,7 +1856,3 @@ async fn send_and_decode Deserialize<'de>>( source, }) } - -#[cfg(test)] -#[path = "remote_tests.rs"] -mod tests; diff --git a/codex-rs/core-plugins/src/remote_tests.rs b/codex-rs/core-plugins/src/remote_tests.rs index a023aae4d161..793a8a55bd21 100644 --- a/codex-rs/core-plugins/src/remote_tests.rs +++ b/codex-rs/core-plugins/src/remote_tests.rs @@ -197,6 +197,31 @@ fn recommended_plugins_are_validated_deduplicated_sorted_and_capped() { ); } +#[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 { From 6f17e4e845e0e4c65defe91c415a55bb112a483b Mon Sep 17 00:00:00 2001 From: Alex Daley Date: Mon, 15 Jun 2026 20:27:57 -0400 Subject: [PATCH 3/3] Reset recommended plugin cache on account changes --- .../request_processors/account_processor.rs | 3 ++ codex-rs/core-plugins/src/manager.rs | 29 +++++++----- codex-rs/core-plugins/src/manager_tests.rs | 47 ++----------------- 3 files changed, 22 insertions(+), 57 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 bb136d4b26bd..1fa0961ddc9b 100644 --- a/codex-rs/app-server/src/request_processors/account_processor.rs +++ b/codex-rs/app-server/src/request_processors/account_processor.rs @@ -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) diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 3aa75bcd03b5..e92b8d935bc0 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -125,9 +125,6 @@ struct FeaturedPluginIdsCacheKey { #[derive(Clone, Hash, PartialEq, Eq)] struct RecommendedPluginsCacheKey { chatgpt_base_url: String, - account_id: Option, - chatgpt_user_id: Option, - is_workspace_account: bool, } #[derive(Clone)] @@ -219,19 +216,12 @@ fn featured_plugin_ids_cache_key( } } -fn recommended_plugins_cache_key( - config: &PluginsConfigInput, - auth: Option<&CodexAuth>, -) -> RecommendedPluginsCacheKey { - // Account changes start a warmup but do not clear process-local entries. Keep install - // identities scoped to the auth that fetched them, including while a new warmup is in flight. +fn recommended_plugins_cache_key(config: &PluginsConfigInput) -> RecommendedPluginsCacheKey { RecommendedPluginsCacheKey { chatgpt_base_url: config.chatgpt_base_url.clone(), - account_id: auth.and_then(CodexAuth::get_account_id), - chatgpt_user_id: auth.and_then(CodexAuth::get_chatgpt_user_id), - is_workspace_account: auth.is_some_and(CodexAuth::is_workspace_account), } } + #[derive(Debug, Clone, PartialEq, Eq)] pub struct PluginInstallRequest { pub plugin_name: String, @@ -527,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, @@ -936,7 +939,7 @@ impl PluginsManager { return RecommendedPluginsMode::Legacy; } - let cache_key = recommended_plugins_cache_key(config, auth); + let cache_key = recommended_plugins_cache_key(config); if let Some(cached) = self.cached_recommended_plugins_mode(&cache_key) { return cached; } diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 5a20c9d2cf70..f219f5e7605e 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -3732,7 +3732,7 @@ remote_plugin = true 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, Some(&auth)); + let cache_key = recommended_plugins_cache_key(&config); manager.maybe_start_remote_plugin_caches_refresh( &config, @@ -3762,6 +3762,8 @@ remote_plugin = true .await, mode ); + manager.clear_recommended_plugins_cache(); + assert_eq!(manager.cached_recommended_plugins_mode(&cache_key), None); } #[tokio::test] @@ -3842,49 +3844,6 @@ remote_plugin = true ); } -#[tokio::test] -async fn recommended_plugins_memory_cache_is_isolated_by_account() { - let tmp = tempfile::tempdir().unwrap(); - write_file( - &tmp.path().join(CONFIG_TOML_FILE), - r#"[features] -plugins = true -remote_plugin = true -"#, - ); - let config = load_config(tmp.path(), tmp.path()).await; - let manager = PluginsManager::new(tmp.path().to_path_buf()); - let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); - let mode = 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(), - }], - }; - { - let mut cache = manager.recommended_plugins_cache.write().unwrap(); - cache.insert( - recommended_plugins_cache_key(&config, Some(&auth)), - mode.clone(), - ); - } - - assert_eq!( - manager - .recommended_plugins_mode_for_config(&config, Some(&auth)) - .await, - mode - ); - assert_eq!( - manager - .recommended_plugins_mode_for_config(&config, /*auth*/ None) - .await, - RecommendedPluginsMode::Legacy - ); -} - #[tokio::test] async fn recommended_plugins_mode_caches_explicit_false() { let tmp = tempfile::tempdir().unwrap();