From a46c5be77eac0dc856f677a099a6a7702edd578b Mon Sep 17 00:00:00 2001 From: Alex Zamoshchin Date: Fri, 13 Nov 2026 18:47:03 +0000 Subject: [PATCH] Delegate installed plugin recommendation filtering to the endpoint (#61374) ## What changed Request plugin suggestions with `includeInstalled=false` and use the endpoint results without filtering against loaded plugins or the local installed-plugin cache. Retain disabled-tool and client eligibility filtering, and remove plugin loading that was only needed for recommendation filtering. ## Testing Update tests to verify the query parameter and confirm that endpoint recommendations survive local installed-cache filtering while disabled tools remain excluded. GitOrigin-RevId: a1b199f9ac8da7dae40abe46b6ccfaf1d8230a5c --- codex-rs/core-plugins/src/manager.rs | 30 +------------------ codex-rs/core-plugins/src/manager_tests.rs | 16 +++------- codex-rs/core-plugins/src/remote.rs | 4 ++- codex-rs/core-plugins/src/remote_tests.rs | 5 ++-- codex-rs/core/src/session/mod.rs | 7 ----- codex-rs/core/src/session/turn.rs | 7 ----- .../tests/suite/request_plugin_install.rs | 7 +---- 7 files changed, 12 insertions(+), 64 deletions(-) diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 1841741d5c..dc4fe35dcd 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -70,7 +70,6 @@ use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome; use crate::marketplace_upgrade::upgrade_configured_git_marketplaces_with_mode; use crate::remote::CODEX_PRODUCT_SKU; use crate::remote::InstalledPluginDisplayAssets; -use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; use crate::remote::RecommendedPluginsMode; use crate::remote::RemoteInstalledPlugin; use crate::remote::RemoteInstalledPluginBundleSyncError; @@ -295,7 +294,6 @@ pub struct EffectivePluginsChange { /// Inputs used to select endpoint-backed plugin install candidates. pub struct RecommendedPluginCandidatesInput<'a> { pub plugins_config: &'a PluginsConfigInput, - pub loaded_plugins: &'a PluginLoadOutcome, pub auth: Option<&'a CodexAuth>, pub disabled_tools: &'a [ToolSuggestDisabledTool], pub app_server_client_name: Option<&'a str>, @@ -2201,28 +2199,6 @@ impl PluginsManager { return Some(Vec::new()); } - let installed_plugin_ids = input - .loaded_plugins - .plugins() - .iter() - .map(|plugin| plugin.config_name.as_str()) - .collect::>(); - let installed_remote_plugin_ids = { - let cache = match self.remote_installed_plugins_cache.read() { - Ok(cache) => cache, - Err(err) => err.into_inner(), - }; - let plugins = self - .remote_installed_plugins_cache_matches_current_auth(&cache) - .then_some(cache.plugins.as_deref()) - .flatten() - .unwrap_or_default(); - plugins - .iter() - .filter(|plugin| plugin.marketplace_name == REMOTE_GLOBAL_MARKETPLACE_NAME) - .map(|plugin| plugin.id.clone()) - .collect::>() - }; let disabled_plugin_ids = input .disabled_tools .iter() @@ -2232,11 +2208,7 @@ impl PluginsManager { let candidates = plugins .into_iter() - .filter(|plugin| { - !installed_plugin_ids.contains(plugin.config_id.as_str()) - && !installed_remote_plugin_ids.contains(plugin.remote_plugin_id.as_str()) - && !disabled_plugin_ids.contains(plugin.config_id.as_str()) - }) + .filter(|plugin| !disabled_plugin_ids.contains(plugin.config_id.as_str())) .map(|plugin| { DiscoverableTool::from(DiscoverablePluginInfo { id: plugin.config_id, diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index c12ce00bea..f1a2cbc8a3 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -6351,7 +6351,7 @@ async fn recommended_plugins_cache_clear_rejects_stale_fetch_completion() { } #[tokio::test] -async fn recommended_plugin_candidates_filter_installed_and_disabled_plugins() { +async fn recommended_plugin_candidates_use_endpoint_results_and_filter_disabled_tools() { let tmp = tempfile::tempdir().unwrap(); write_file( &tmp.path().join(CONFIG_TOML_FILE), @@ -6374,11 +6374,6 @@ plugins = true "id": "plugin_github", "name": "github", "display_name": "GitHub" - }, - { - "id": "plugin_slack", - "name": "slack", - "display_name": "Slack" } ] }))) @@ -6396,12 +6391,9 @@ plugins = true let disabled_tools = [ToolSuggestDisabledTool::plugin( "github@openai-curated-remote", )]; - let loaded_plugins = manager.plugins_for_config(&config).await; - let candidates = manager .recommended_plugin_candidates_for_config(RecommendedPluginCandidatesInput { plugins_config: &config, - loaded_plugins: &loaded_plugins, auth: Some(&auth), disabled_tools: &disabled_tools, app_server_client_name: None, @@ -6411,9 +6403,9 @@ plugins = true assert_eq!( candidates, Some(vec![DiscoverableTool::from(DiscoverablePluginInfo { - id: "slack@openai-curated-remote".to_string(), - remote_plugin_id: Some("plugin_slack".to_string()), - name: "Slack".to_string(), + id: "linear@openai-curated-remote".to_string(), + remote_plugin_id: Some("plugin_linear".to_string()), + name: "Linear".to_string(), description: None, has_skills: false, mcp_server_names: Vec::new(), diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index 957d0d6eff..e1961f9432 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -1156,7 +1156,9 @@ pub async fn fetch_recommended_plugins( let base_url = config.chatgpt_base_url.trim_end_matches('/'); let mut url = Url::parse(&format!("{base_url}/ps/plugins/suggested/codex")) .map_err(RemotePluginCatalogError::InvalidBaseUrl)?; - url.query_pairs_mut().append_pair("scope", "GLOBAL"); + url.query_pairs_mut() + .append_pair("scope", "GLOBAL") + .append_pair("includeInstalled", "false"); let url = url.to_string(); let request = authenticated_request(config, Method::GET, &url, auth) .await? diff --git a/codex-rs/core-plugins/src/remote_tests.rs b/codex-rs/core-plugins/src/remote_tests.rs index f53e5eb3f5..89ab677826 100644 --- a/codex-rs/core-plugins/src/remote_tests.rs +++ b/codex-rs/core-plugins/src/remote_tests.rs @@ -49,11 +49,12 @@ async fn remote_plugin_list_routes_the_complete_query_url() { } #[tokio::test] -async fn recommended_plugins_requests_codex_suggestions_endpoint() { +async fn recommended_plugins_requests_uninstalled_codex_suggestions() { let server = MockServer::start().await; Mock::given(method("GET")) .and(path("/backend-api/ps/plugins/suggested/codex")) .and(query_param("scope", "GLOBAL")) + .and(query_param("includeInstalled", "false")) .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ "enabled": true, "plugins": [], @@ -78,7 +79,7 @@ async fn recommended_plugins_requests_codex_suggestions_endpoint() { assert_eq!( recorded_http_client_urls(&selected_urls), vec![format!( - "{}/backend-api/ps/plugins/suggested/codex?scope=GLOBAL", + "{}/backend-api/ps/plugins/suggested/codex?scope=GLOBAL&includeInstalled=false", server.uri() )] ); diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index f5b3c0b595..bc6462c155 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -5134,12 +5134,6 @@ impl Session { developer_sections .push(DeveloperInstructions::new(developer_instructions).render_fragment()); } - let loaded_plugins = self - .services - .plugins_manager - .plugins_for_config(&turn_context.config.plugins_config_input()) - .await - .without_plugins(&turn_context.disabled_plugin_ids); let recommended_plugin_candidates = if turn_context .config .features @@ -5151,7 +5145,6 @@ impl Session { .plugins_manager .recommended_plugin_candidates_for_config(RecommendedPluginCandidatesInput { plugins_config: &plugins_config, - loaded_plugins: &loaded_plugins, auth: auth.as_ref(), disabled_tools: &turn_context.config.tool_suggest.disabled_tools, app_server_client_name: turn_context.app_server_client_name.as_deref(), diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index fb40b7e763..59db966818 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -2105,12 +2105,6 @@ pub(crate) async fn prepare_tool_recommendations( sess: &Session, turn_context: &TurnContext, ) -> PreparedToolRecommendations { - let loaded_plugins = sess - .services - .plugins_manager - .plugins_for_config(&turn_context.config.plugins_config_input()) - .await - .without_plugins(&turn_context.disabled_plugin_ids); let tool_suggest_is_enabled = tool_suggest_enabled(turn_context); let auth = if tool_suggest_is_enabled { sess.services.auth_manager.auth().await @@ -2123,7 +2117,6 @@ pub(crate) async fn prepare_tool_recommendations( .plugins_manager .recommended_plugin_candidates_for_config(RecommendedPluginCandidatesInput { plugins_config: &plugins_config, - loaded_plugins: &loaded_plugins, auth: auth.as_ref(), disabled_tools: &turn_context.config.tool_suggest.disabled_tools, app_server_client_name: turn_context.app_server_client_name.as_deref(), diff --git a/codex-rs/core/tests/suite/request_plugin_install.rs b/codex-rs/core/tests/suite/request_plugin_install.rs index 834e338eff..ffcc6a6a12 100644 --- a/codex-rs/core/tests/suite/request_plugin_install.rs +++ b/codex-rs/core/tests/suite/request_plugin_install.rs @@ -452,6 +452,7 @@ async fn mount_remote_calendar_installed_plugins(server: &wiremock::MockServer) }, "enabled": true })])) + .expect(1..) .with_priority(1) .mount(server) .await; @@ -1616,12 +1617,6 @@ async fn run_remote_plugin_install_refresh_case(refreshed_tools: RefreshedAppsTo completed, "the resumed router should reflect the refreshed Apps tools" ); - assert!( - !tool_names(&requests[1].body_json()) - .iter() - .any(|name| name == REQUEST_PLUGIN_INSTALL_TOOL_NAME), - "the refreshed installed-plugin cache should filter the cached recommendation" - ); drop(requests); let current_config = test.codex.config().await; let _ = test