mirror of
https://github.com/openai/codex.git
synced 2026-11-13 21:30:47 +00:00
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
This commit is contained in:
committed by
copyberry
parent
7194b21776
commit
a46c5be77e
@@ -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::<HashSet<_>>();
|
||||
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::<HashSet<_>>()
|
||||
};
|
||||
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,
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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?
|
||||
|
||||
@@ -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()
|
||||
)]
|
||||
);
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user