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 0139c68272..c69e544f67 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_list.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_list.rs @@ -1,3 +1,6 @@ +use std::collections::BTreeMap; +use std::sync::Mutex; +use std::sync::OnceLock; use std::time::Duration; use anyhow::Result; @@ -55,6 +58,9 @@ const TEST_ALLOW_HTTP_REMOTE_PLUGIN_BUNDLE_DOWNLOADS: &str = "CODEX_TEST_ALLOW_HTTP_REMOTE_PLUGIN_BUNDLE_DOWNLOADS"; const ALTERNATE_MARKETPLACE_RELATIVE_PATH: &str = ".claude-plugin/marketplace.json"; const ALTERNATE_PLUGIN_MANIFEST_RELATIVE_PATH: &str = ".claude-plugin/plugin.json"; +type RemoteInstalledPluginFixtures = BTreeMap>>; +static REMOTE_INSTALLED_PLUGIN_FIXTURES: OnceLock> = + OnceLock::new(); fn write_plugins_enabled_config(codex_home: &std::path::Path) -> std::io::Result<()> { std::fs::write( @@ -3379,8 +3385,7 @@ plugin_sharing = true ) ] ); - wait_for_remote_installed_scope_request(&server, "WORKSPACE").await?; - wait_for_remote_installed_scope_request(&server, "GLOBAL").await?; + wait_for_remote_installed_snapshot_request(&server).await?; Ok(()) } @@ -3466,8 +3471,7 @@ plugin_sharing = false true )] ); - wait_for_remote_installed_scope_request(&server, "WORKSPACE").await?; - wait_for_remote_installed_scope_request(&server, "GLOBAL").await?; + wait_for_remote_installed_snapshot_request(&server).await?; Ok(()) } @@ -3553,7 +3557,7 @@ plugin_sharing = false ), ) .await?; - wait_for_remote_installed_scope_request(&server, "USER").await?; + wait_for_remote_installed_snapshot_request(&server).await?; Ok(()) } @@ -4271,8 +4275,7 @@ async fn plugin_list_fetches_shared_with_me_kind() -> Result<()> { share_context.discoverability, Some(PluginShareDiscoverability::Unlisted) ); - wait_for_remote_installed_scope_request(&server, "WORKSPACE").await?; - wait_for_remote_installed_scope_request(&server, "GLOBAL").await?; + wait_for_remote_installed_snapshot_request(&server).await?; wait_for_remote_plugin_request_count(&server, "/ps/plugins/list", /*expected_count*/ 0).await?; Ok(()) } @@ -4751,7 +4754,7 @@ async fn wait_for_remote_plugin_list_scope_request_count( Ok(()) } -async fn wait_for_remote_installed_scope_request(server: &MockServer, scope: &str) -> Result<()> { +async fn wait_for_remote_installed_snapshot_request(server: &MockServer) -> Result<()> { timeout(DEFAULT_TIMEOUT, async { loop { let Some(requests) = server.received_requests().await else { @@ -4760,10 +4763,7 @@ async fn wait_for_remote_installed_scope_request(server: &MockServer, scope: &st if requests.iter().any(|request| { request.method == "GET" && request.url.path().ends_with("/ps/plugins/installed") - && request - .url - .query_pairs() - .any(|(name, value)| name == "scope" && value == scope) + && request.url.query_pairs().all(|(name, _)| name != "scope") }) { return Ok::<(), anyhow::Error>(()); } @@ -4998,6 +4998,19 @@ async fn mount_shared_workspace_plugins(server: &MockServer, body: &str) { } async fn mount_remote_installed_plugins(server: &MockServer, scope: &str, body: &str) { + let plugins = serde_json::from_str::(body) + .expect("installed plugin fixture should be valid JSON")["plugins"] + .as_array() + .expect("installed plugin fixture should contain plugins") + .clone(); + REMOTE_INSTALLED_PLUGIN_FIXTURES + .get_or_init(Default::default) + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .entry(server.uri()) + .or_default() + .insert(scope.to_string(), plugins); + Mock::given(method("GET")) .and(path("/backend-api/ps/plugins/installed")) .and(query_param("scope", scope)) @@ -5006,6 +5019,34 @@ async fn mount_remote_installed_plugins(server: &MockServer, scope: &str, body: .respond_with(ResponseTemplate::new(200).set_body_string(body)) .mount(server) .await; + + let server_uri = server.uri(); + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param_is_missing("scope")) + .and(header("authorization", "Bearer chatgpt-token")) + .and(header("chatgpt-account-id", "account-123")) + .respond_with(move |_request: &wiremock::Request| { + let fixtures = REMOTE_INSTALLED_PLUGIN_FIXTURES + .get() + .expect("installed plugin fixtures should exist") + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let scoped_plugins = fixtures + .get(&server_uri) + .expect("installed plugin fixtures should exist for this server"); + let plugins = ["GLOBAL", "WORKSPACE", "USER"] + .into_iter() + .flat_map(|scope| scoped_plugins.get(scope).into_iter().flatten()) + .cloned() + .collect::>(); + ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "plugins": plugins, + "pagination": {"limit": 50, "next_page_token": null}, + })) + }) + .mount(server) + .await; } async fn mount_empty_user_installed_plugins(server: &MockServer) { diff --git a/codex-rs/app-server/tests/suite/v2/skills_list.rs b/codex-rs/app-server/tests/suite/v2/skills_list.rs index ff4d1e23f2..16079ddfcf 100644 --- a/codex-rs/app-server/tests/suite/v2/skills_list.rs +++ b/codex-rs/app-server/tests/suite/v2/skills_list.rs @@ -37,6 +37,7 @@ use wiremock::matchers::header; use wiremock::matchers::method; use wiremock::matchers::path; use wiremock::matchers::query_param; +use wiremock::matchers::query_param_is_missing; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(30); const WATCHER_TIMEOUT: Duration = Duration::from_secs(20); @@ -579,6 +580,14 @@ async fn skills_list_loads_remote_installed_plugin_skills_from_cache() -> Result .mount(&server) .await; } + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param_is_missing("scope")) + .and(header("authorization", "Bearer chatgpt-token")) + .and(header("chatgpt-account-id", "account-123")) + .respond_with(ResponseTemplate::new(200).set_body_string(global_installed_body)) + .mount(&server) + .await; let plugin_list_request_id = mcp .send_plugin_list_request(PluginListParams { diff --git a/codex-rs/core-plugins/src/discoverable_tests.rs b/codex-rs/core-plugins/src/discoverable_tests.rs index 1dd88969ac..672f53fce4 100644 --- a/codex-rs/core-plugins/src/discoverable_tests.rs +++ b/codex-rs/core-plugins/src/discoverable_tests.rs @@ -33,6 +33,7 @@ use wiremock::ResponseTemplate; use wiremock::matchers::method; use wiremock::matchers::path; use wiremock::matchers::query_param; +use wiremock::matchers::query_param_is_missing; #[tokio::test] async fn returns_fallback_plugins_when_remote_disabled_for_codex_auth() { @@ -786,7 +787,7 @@ source = "/tmp/{sales_marketplace_name}" } #[tokio::test] -async fn expands_cached_remote_plugins_by_loaded_apps() { +async fn cached_remote_discovery_requires_installed_cache_and_filters_candidates() { let codex_home = tempdir().expect("tempdir should succeed"); write_file( &codex_home.path().join(CONFIG_TOML_FILE), @@ -801,6 +802,25 @@ plugins = true .and(query_param("scope", "GLOBAL")) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "plugins": [ + { + "id": "plugins~Plugin_remote_github", + "name": "github", + "scope": "GLOBAL", + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "status": "AVAILABLE", + "release": { + "display_name": "Remote GitHub", + "description": "Remote GitHub long", + "app_ids": ["github"], + "interface": {"short_description": "Remote GitHub short"}, + "skills": [{ + "name": "github", + "description": "Use GitHub", + "interface": null + }] + } + }, { "id": "plugins~Plugin_remote_unlisted", "name": "remote-unlisted", @@ -835,6 +855,32 @@ plugins = true } ] } + }, + { + "id": "plugins~Plugin_remote_slack_not_available", + "name": "slack", + "scope": "GLOBAL", + "installation_policy": "NOT_AVAILABLE", + "authentication_policy": "ON_USE", + "status": "AVAILABLE", + "release": { + "display_name": "Remote Slack", + "description": "Remote Slack long", + "interface": {"short_description": "Remote Slack short"} + } + }, + { + "id": "plugins~Plugin_remote_figma_admin_disabled", + "name": "figma", + "scope": "GLOBAL", + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "status": "DISABLED_BY_ADMIN", + "release": { + "display_name": "Remote Figma", + "description": "Remote Figma long", + "interface": {"short_description": "Remote Figma short"} + } } ], "pagination": { @@ -860,20 +906,29 @@ plugins = true .await .expect("remote plugin catalog cache should write"); - for scope in ["GLOBAL", "USER", "WORKSPACE"] { - Mock::given(method("GET")) - .and(path("/backend-api/ps/plugins/installed")) - .and(query_param("scope", scope)) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({ - "plugins": [], - "pagination": { - "next_page_token": null - } - }))) - .expect(1) - .mount(&server) - .await; - } + assert_eq!( + list_discoverable_plugins( + &plugins_manager, + discovery_input(plugins.clone(), &[], &[], &["remote-unlisted-app"]), + Some(&auth), + ) + .await, + Vec::new() + ); + + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param_is_missing("scope")) + .and(query_param_is_missing("includeDownloadUrls")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "plugins": [], + "pagination": { + "next_page_token": null + } + }))) + .expect(1) + .mount(&server) + .await; plugins_manager .build_and_cache_remote_installed_plugin_marketplaces( &plugins, @@ -884,24 +939,55 @@ plugins = true .await .expect("remote installed plugin cache should write"); + let expected_github = ToolSuggestDiscoverablePlugin { + id: "github@openai-curated-remote".to_string(), + remote_plugin_id: Some("plugins~Plugin_remote_github".to_string()), + name: "Remote GitHub".to_string(), + description: Some("Remote GitHub short".to_string()), + has_skills: true, + mcp_server_names: Vec::new(), + app_connector_ids: vec!["github".to_string()], + }; + assert_eq!( + list_discoverable_plugins( + &plugins_manager, + discovery_input(plugins.clone(), &[], &[], &[]), + Some(&auth), + ) + .await, + vec![expected_github.clone()] + ); + let discoverable_plugins = list_discoverable_plugins( &plugins_manager, - discovery_input(plugins, &[], &[], &["remote-unlisted-app"]), + discovery_input(plugins.clone(), &[], &[], &["remote-unlisted-app"]), Some(&auth), ) .await; assert_eq!( discoverable_plugins, - vec![ToolSuggestDiscoverablePlugin { - id: "remote-unlisted@openai-curated-remote".to_string(), - remote_plugin_id: Some("plugins~Plugin_remote_unlisted".to_string()), - name: "Remote Unlisted".to_string(), - description: Some("Remote Unlisted short".to_string()), - has_skills: true, - mcp_server_names: Vec::new(), - app_connector_ids: vec!["remote-unlisted-app".to_string()], - }] + vec![ + expected_github, + ToolSuggestDiscoverablePlugin { + id: "remote-unlisted@openai-curated-remote".to_string(), + remote_plugin_id: Some("plugins~Plugin_remote_unlisted".to_string()), + name: "Remote Unlisted".to_string(), + description: Some("Remote Unlisted short".to_string()), + has_skills: true, + mcp_server_names: Vec::new(), + app_connector_ids: vec!["remote-unlisted-app".to_string()], + }, + ] + ); + assert_eq!( + list_discoverable_plugins( + &plugins_manager, + discovery_input(plugins, &[], &["github@openai-curated-remote"], &[]), + Some(&auth), + ) + .await, + Vec::new() ); } diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index ed3f4df8d9..457de514b2 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -468,6 +468,12 @@ pub enum RemotePluginScope { Workspace, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum RemoteInstalledPluginScope { + All, + Single(RemotePluginScope), +} + impl RemotePluginScope { const CATALOG_CACHE_SCOPES: [Self; 3] = [Self::Global, Self::User, Self::Workspace]; @@ -1162,28 +1168,16 @@ pub(crate) async fn fetch_remote_installed_plugins( auth: Option<&CodexAuth>, ) -> Result, RemotePluginCatalogError> { let auth = ensure_chatgpt_auth(auth)?; - let global = async { - let scope = RemotePluginScope::Global; - let installed_plugins = fetch_installed_plugins_for_scope(config, auth, scope).await?; - Ok::<_, RemotePluginCatalogError>((scope, installed_plugins)) - }; - let workspace = async { - let scope = RemotePluginScope::Workspace; - let installed_plugins = fetch_installed_plugins_for_scope(config, auth, scope).await?; - Ok::<_, RemotePluginCatalogError>((scope, installed_plugins)) - }; - let user = async { - let scope = RemotePluginScope::User; - let installed_plugins = fetch_installed_plugins_for_scope(config, auth, scope).await?; - Ok::<_, RemotePluginCatalogError>((scope, installed_plugins)) - }; - - let (global, workspace, user) = tokio::try_join!(global, workspace, user)?; - let mut installed_plugins = [global, workspace, user] - .into_iter() - .flat_map(|(_scope, plugins)| plugins) - .map(|plugin| remote_installed_plugin_to_cache_entry(&plugin)) - .collect::, _>>()?; + let mut installed_plugins = fetch_installed_plugins( + config, + auth, + RemoteInstalledPluginScope::All, + /*include_download_urls*/ false, + ) + .await? + .into_iter() + .map(|plugin| remote_installed_plugin_to_cache_entry(&plugin)) + .collect::, _>>()?; installed_plugins.sort_by(|left, right| { left.marketplace_name .cmp(&right.marketplace_name) @@ -1989,16 +1983,19 @@ async fn fetch_installed_plugins_for_scope( auth: &CodexAuth, scope: RemotePluginScope, ) -> Result, RemotePluginCatalogError> { - fetch_installed_plugins_for_scope_with_download_url( - config, auth, scope, /*include_download_urls*/ false, + fetch_installed_plugins( + config, + auth, + RemoteInstalledPluginScope::Single(scope), + /*include_download_urls*/ false, ) .await } -async fn fetch_installed_plugins_for_scope_with_download_url( +async fn fetch_installed_plugins( config: &RemotePluginServiceConfig, auth: &CodexAuth, - scope: RemotePluginScope, + scope: RemoteInstalledPluginScope, include_download_urls: bool, ) -> Result, RemotePluginCatalogError> { let mut plugins = Vec::new(); @@ -2066,15 +2063,23 @@ async fn get_remote_shared_workspace_plugins_page( async fn get_remote_plugin_installed_page( config: &RemotePluginServiceConfig, auth: &CodexAuth, - scope: RemotePluginScope, + scope: RemoteInstalledPluginScope, page_token: Option<&str>, include_download_urls: bool, ) -> Result { let base_url = config.chatgpt_base_url.trim_end_matches('/'); let mut url = Url::parse(&format!("{base_url}/ps/plugins/installed")) .map_err(RemotePluginCatalogError::InvalidBaseUrl)?; - url.query_pairs_mut() - .append_pair("scope", scope.api_value()); + match scope { + RemoteInstalledPluginScope::All => { + url.query_pairs_mut() + .append_pair("limit", &REMOTE_PLUGIN_LIST_PAGE_LIMIT.to_string()); + } + RemoteInstalledPluginScope::Single(scope) => { + url.query_pairs_mut() + .append_pair("scope", scope.api_value()); + } + } if include_download_urls { url.query_pairs_mut() .append_pair("includeDownloadUrls", "true"); diff --git a/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs b/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs index 361707cfd5..a0a4504125 100644 --- a/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs +++ b/codex-rs/core-plugins/src/remote/remote_installed_plugin_sync.rs @@ -4,12 +4,13 @@ use super::REMOTE_WORKSPACE_MARKETPLACE_NAME; use super::REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME; use super::REMOTE_WORKSPACE_SHARED_WITH_ME_PRIVATE_MARKETPLACE_NAME; use super::REMOTE_WORKSPACE_SHARED_WITH_ME_UNLISTED_MARKETPLACE_NAME; +use super::RemoteInstalledPluginScope; use super::RemotePluginCatalogError; use super::RemotePluginScope; use super::RemotePluginServiceConfig; use super::RemotePluginShareDiscoverability; use super::ensure_chatgpt_auth; -use super::fetch_installed_plugins_for_scope_with_download_url; +use super::fetch_installed_plugins; use super::remote_plugin_canonical_marketplace_name; use crate::store::PLUGINS_CACHE_DIR; use crate::store::PluginStore; @@ -136,32 +137,13 @@ pub async fn sync_remote_installed_plugin_bundles_once( ) -> Result { let auth = ensure_chatgpt_auth(auth)?; let authenticated_account_id = auth.get_account_id(); - let global = async { - let scope = RemotePluginScope::Global; - let installed_plugins = fetch_installed_plugins_for_scope_with_download_url( - config, auth, scope, /*include_download_urls*/ true, - ) - .await?; - Ok::<_, RemotePluginCatalogError>((scope, installed_plugins)) - }; - let workspace = async { - let scope = RemotePluginScope::Workspace; - let installed_plugins = fetch_installed_plugins_for_scope_with_download_url( - config, auth, scope, /*include_download_urls*/ true, - ) - .await?; - Ok::<_, RemotePluginCatalogError>((scope, installed_plugins)) - }; - let user = async { - let scope = RemotePluginScope::User; - let installed_plugins = fetch_installed_plugins_for_scope_with_download_url( - config, auth, scope, /*include_download_urls*/ true, - ) - .await?; - Ok::<_, RemotePluginCatalogError>((scope, installed_plugins)) - }; - - let (global, workspace, user) = tokio::try_join!(global, workspace, user)?; + let installed_plugins = fetch_installed_plugins( + config, + auth, + RemoteInstalledPluginScope::All, + /*include_download_urls*/ true, + ) + .await?; let store = PluginStore::try_new(codex_home.clone())?; let mut installed_plugin_names_by_marketplace = BTreeMap::>::from_iter([ @@ -190,101 +172,99 @@ pub async fn sync_remote_installed_plugin_bundles_once( let mut materialized_remote_plugins = BTreeMap::new(); let mut failed_remote_plugin_ids = BTreeSet::new(); - for (_scope, installed_plugins) in [global, workspace, user] { - for installed_plugin in installed_plugins { - let plugin = installed_plugin.plugin; - let scope = plugin.scope; - let discoverability = plugin.discoverability; - let marketplace_name = remote_plugin_canonical_marketplace_name(&plugin)?.to_string(); - installed_plugin_names_by_marketplace - .entry(marketplace_name.clone()) - .or_default() - .insert(plugin.name.clone()); - let plugin_id = match PluginId::new(plugin.name.clone(), marketplace_name.clone()) { - Ok(plugin_id) => plugin_id, - Err(err) => { - warn!( - remote_plugin_id = %plugin.id, - plugin = %plugin.name, - marketplace = %marketplace_name, - error = %err, - "skipping remote installed plugin with invalid local cache id" - ); - failed_remote_plugin_ids.insert(plugin.id); - continue; - } - }; - let release_version = plugin - .release - .version - .as_deref() - .map(str::trim) - .filter(|version| !version.is_empty()); - if store.active_plugin_version(&plugin_id).as_deref() == release_version { - if let Err(err) = store.write_remote_plugin_id(&plugin_id, &plugin.id) { - warn!( - remote_plugin_id = %plugin.id, - plugin = %plugin.name, - marketplace = %marketplace_name, - error = %err, - "failed to persist identity for cached remote installed plugin" - ); - failed_remote_plugin_ids.insert(plugin.id); - } + for installed_plugin in installed_plugins { + let plugin = installed_plugin.plugin; + let scope = plugin.scope; + let discoverability = plugin.discoverability; + let marketplace_name = remote_plugin_canonical_marketplace_name(&plugin)?.to_string(); + installed_plugin_names_by_marketplace + .entry(marketplace_name.clone()) + .or_default() + .insert(plugin.name.clone()); + let plugin_id = match PluginId::new(plugin.name.clone(), marketplace_name.clone()) { + Ok(plugin_id) => plugin_id, + Err(err) => { + warn!( + remote_plugin_id = %plugin.id, + plugin = %plugin.name, + marketplace = %marketplace_name, + error = %err, + "skipping remote installed plugin with invalid local cache id" + ); + failed_remote_plugin_ids.insert(plugin.id); continue; } + }; + let release_version = plugin + .release + .version + .as_deref() + .map(str::trim) + .filter(|version| !version.is_empty()); + if store.active_plugin_version(&plugin_id).as_deref() == release_version { + if let Err(err) = store.write_remote_plugin_id(&plugin_id, &plugin.id) { + warn!( + remote_plugin_id = %plugin.id, + plugin = %plugin.name, + marketplace = %marketplace_name, + error = %err, + "failed to persist identity for cached remote installed plugin" + ); + failed_remote_plugin_ids.insert(plugin.id); + } + continue; + } - let bundle = match crate::remote_bundle::validate_remote_plugin_bundle( - &plugin.id, - &marketplace_name, - &plugin.name, - release_version, - plugin.release.bundle_download_url.as_deref(), - plugin.release.app_manifest.clone(), - ) { - Ok(bundle) => bundle, - Err(err) => { - warn!( - remote_plugin_id = %plugin.id, - plugin = %plugin.name, - marketplace = %marketplace_name, - error = %err, - "skipping remote installed plugin bundle download" - ); - failed_remote_plugin_ids.insert(plugin.id); - continue; - } - }; + let bundle = match crate::remote_bundle::validate_remote_plugin_bundle( + &plugin.id, + &marketplace_name, + &plugin.name, + release_version, + plugin.release.bundle_download_url.as_deref(), + plugin.release.app_manifest.clone(), + ) { + Ok(bundle) => bundle, + Err(err) => { + warn!( + remote_plugin_id = %plugin.id, + plugin = %plugin.name, + marketplace = %marketplace_name, + error = %err, + "skipping remote installed plugin bundle download" + ); + failed_remote_plugin_ids.insert(plugin.id); + continue; + } + }; - match crate::remote_bundle::download_and_install_remote_plugin_bundle( - config, - codex_home.clone(), - bundle, - ) - .await - { - Ok(result) => { - let plugin_id = result.plugin_id; - materialized_remote_plugins.insert( - plugin_id.as_key(), - RemotePluginMaterialization { - plugin_id, - scope, - discoverability, - authenticated_account_id: authenticated_account_id.clone(), - }, - ); - } - Err(err) => { - warn!( - remote_plugin_id = %plugin.id, - plugin = %plugin.name, - marketplace = %marketplace_name, - error = %err, - "failed to download remote installed plugin bundle" - ); - failed_remote_plugin_ids.insert(plugin.id); - } + match crate::remote_bundle::download_and_install_remote_plugin_bundle( + config, + codex_home.clone(), + bundle, + ) + .await + { + Ok(result) => { + let plugin_id = result.plugin_id; + materialized_remote_plugins.insert( + plugin_id.as_key(), + RemotePluginMaterialization { + plugin_id, + scope, + discoverability, + authenticated_account_id: authenticated_account_id.clone(), + }, + ); + } + Err(err) => { + warn!( + remote_plugin_id = %plugin.id, + plugin = %plugin.name, + marketplace = %marketplace_name, + error = %err, + "failed to download remote installed plugin bundle" + ); + failed_remote_plugin_ids.insert(plugin.id); } } } @@ -485,6 +465,7 @@ mod tests { use wiremock::matchers::method; use wiremock::matchers::path; use wiremock::matchers::query_param; + use wiremock::matchers::query_param_is_missing; #[test] fn remote_installed_plugin_sync_in_flight_dedupes_by_cache_root() { @@ -526,7 +507,8 @@ mod tests { let remote_plugin_id = "plugins~Plugin_linear"; Mock::given(method("GET")) .and(path("/backend-api/ps/plugins/installed")) - .and(query_param("scope", "GLOBAL")) + .and(query_param_is_missing("scope")) + .and(query_param("limit", "200")) .and(query_param("includeDownloadUrls", "true")) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "plugins": [{ @@ -549,28 +531,6 @@ mod tests { .expect(1) .mount(&server) .await; - Mock::given(method("GET")) - .and(path("/backend-api/ps/plugins/installed")) - .and(query_param("scope", "USER")) - .and(query_param("includeDownloadUrls", "true")) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({ - "plugins": [], - "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", "WORKSPACE")) - .and(query_param("includeDownloadUrls", "true")) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({ - "plugins": [], - "pagination": {"next_page_token": null}, - }))) - .expect(1) - .mount(&server) - .await; let config = RemotePluginServiceConfig::new( format!("{}/backend-api", server.uri()), crate::test_support::test_http_client_factory(), @@ -607,6 +567,177 @@ mod tests { ); } + #[tokio::test] + async fn sync_all_scopes_paginates_and_reconciles_each_marketplace() { + let server = MockServer::start().await; + let codex_home = tempfile::tempdir().expect("create codex home"); + let cached_plugins = [ + ( + REMOTE_GLOBAL_MARKETPLACE_NAME, + "global-plugin", + "GLOBAL", + None, + ), + ( + REMOTE_CREATED_BY_ME_MARKETPLACE_NAME, + "user-plugin", + "USER", + None, + ), + ( + REMOTE_WORKSPACE_MARKETPLACE_NAME, + "workspace-plugin", + "WORKSPACE", + Some("LISTED"), + ), + ( + REMOTE_WORKSPACE_SHARED_WITH_ME_MARKETPLACE_NAME, + "shared-plugin", + "WORKSPACE", + Some("PRIVATE"), + ), + ]; + for (marketplace_name, plugin_name, _, _) in cached_plugins { + for cached_plugin_name in [plugin_name, "stale"] { + let manifest = codex_home + .path() + .join(PLUGINS_CACHE_DIR) + .join(marketplace_name) + .join(cached_plugin_name) + .join("1.2.3") + .join(".codex-plugin") + .join("plugin.json"); + std::fs::create_dir_all(manifest.parent().expect("manifest parent")) + .expect("create cached plugin manifest parent"); + std::fs::write( + &manifest, + format!(r#"{{"name":"{cached_plugin_name}","version":"1.2.3"}}"#), + ) + .expect("write cached plugin manifest"); + } + } + let installed_plugins = cached_plugins + .iter() + .map(|(_, plugin_name, scope, discoverability)| { + let mut plugin = json!({ + "id": format!("plugins~Plugin_{plugin_name}"), + "name": plugin_name, + "scope": scope, + "installation_policy": "AVAILABLE", + "authentication_policy": "ON_USE", + "status": "ENABLED", + "release": { + "version": "1.2.3", + "display_name": plugin_name, + "description": "Installed plugin", + "interface": {}, + }, + "enabled": true, + }); + if let Some(discoverability) = discoverability { + plugin["discoverability"] = json!(discoverability); + } + plugin + }) + .collect::>(); + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param_is_missing("scope")) + .and(query_param("limit", "200")) + .and(query_param("includeDownloadUrls", "true")) + .and(query_param_is_missing("pageToken")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "plugins": &installed_plugins[..2], + "pagination": {"next_page_token": "page-2"}, + }))) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param_is_missing("scope")) + .and(query_param("limit", "200")) + .and(query_param("includeDownloadUrls", "true")) + .and(query_param("pageToken", "page-2")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "plugins": &installed_plugins[2..], + "pagination": {"next_page_token": null}, + }))) + .expect(1) + .mount(&server) + .await; + let (config, selected_urls) = crate::test_support::recording_remote_plugin_service_config( + format!("{}/backend-api", server.uri()), + ); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + + let outcome = sync_remote_installed_plugin_bundles_once( + codex_home.path().to_path_buf(), + &config, + Some(&auth), + ) + .await + .expect("sync installed plugins across every marketplace"); + let mut removed_cache_plugin_ids = cached_plugins + .iter() + .map(|(marketplace_name, _, _, _)| format!("stale@{marketplace_name}")) + .collect::>(); + removed_cache_plugin_ids.sort(); + + assert_eq!( + outcome, + RemoteInstalledPluginBundleSyncOutcome { + materialized_remote_plugins: Vec::new(), + removed_cache_plugin_ids, + failed_remote_plugin_ids: Vec::new(), + } + ); + assert_eq!( + crate::test_support::recorded_http_client_urls(&selected_urls), + vec![ + format!( + "{}/backend-api/ps/plugins/installed?limit=200&includeDownloadUrls=true", + server.uri() + ), + format!( + "{}/backend-api/ps/plugins/installed?limit=200&includeDownloadUrls=true&pageToken=page-2", + server.uri() + ), + ] + ); + for (marketplace_name, plugin_name, _, _) in cached_plugins { + let plugin_root = codex_home + .path() + .join(PLUGINS_CACHE_DIR) + .join(marketplace_name) + .join(plugin_name); + assert!( + plugin_root + .join("1.2.3/.codex-plugin/plugin.json") + .is_file() + ); + assert_eq!( + serde_json::from_str::( + &std::fs::read_to_string(plugin_root.join(".codex-remote-plugin-install.json")) + .expect("read remote plugin install metadata") + ) + .expect("parse remote plugin install metadata"), + json!({ + "schema_version": 1, + "remote_plugin_id": format!("plugins~Plugin_{plugin_name}"), + }) + ); + assert!( + !codex_home + .path() + .join(PLUGINS_CACHE_DIR) + .join(marketplace_name) + .join("stale") + .exists() + ); + } + } + #[test] fn stale_remote_plugin_cleanup_skips_cache_mutations_in_progress() { let codex_home = tempfile::tempdir().expect("create codex home"); diff --git a/codex-rs/core-plugins/src/remote_tests.rs b/codex-rs/core-plugins/src/remote_tests.rs index 4af981b6eb..eac1c6e03d 100644 --- a/codex-rs/core-plugins/src/remote_tests.rs +++ b/codex-rs/core-plugins/src/remote_tests.rs @@ -8,6 +8,8 @@ use wiremock::ResponseTemplate; use wiremock::matchers::header_exists; use wiremock::matchers::method; use wiremock::matchers::path; +use wiremock::matchers::query_param; +use wiremock::matchers::query_param_is_missing; #[tokio::test] async fn remote_plugin_list_routes_the_complete_query_url() { @@ -46,6 +48,84 @@ async fn remote_plugin_list_routes_the_complete_query_url() { ); } +#[tokio::test] +async fn remote_installed_plugins_paginate_across_all_scopes_without_download_urls() { + let server = MockServer::start().await; + let installed_plugin = |scope: RemotePluginScope, id: &str, name: &str| { + let mut plugin = directory_plugin(id, name); + plugin.scope = scope; + if scope == RemotePluginScope::Workspace { + plugin.discoverability = Some(RemotePluginShareDiscoverability::Listed); + } + let mut plugin = serde_json::to_value(plugin).expect("serialize installed plugin"); + plugin["enabled"] = serde_json::json!(true); + plugin + }; + let global = installed_plugin(RemotePluginScope::Global, "plugin-global", "global-plugin"); + let user = installed_plugin(RemotePluginScope::User, "plugin-user", "user-plugin"); + let workspace = installed_plugin( + RemotePluginScope::Workspace, + "plugin-workspace", + "workspace-plugin", + ); + + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param_is_missing("scope")) + .and(query_param("limit", "200")) + .and(query_param_is_missing("includeDownloadUrls")) + .and(query_param_is_missing("pageToken")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "plugins": [user.clone(), workspace.clone()], + "pagination": {"next_page_token": "next page/+"}, + }))) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param_is_missing("scope")) + .and(query_param("limit", "200")) + .and(query_param_is_missing("includeDownloadUrls")) + .and(query_param("pageToken", "next page/+")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "plugins": [global.clone()], + "pagination": {"next_page_token": null}, + }))) + .expect(1) + .mount(&server) + .await; + let (config, selected_urls) = + recording_remote_plugin_service_config(format!("{}/backend-api", server.uri())); + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + + let installed_plugins = fetch_remote_installed_plugins(&config, Some(&auth)) + .await + .expect("all-scopes installed plugin request should succeed"); + let expected_plugins = [user, global, workspace] + .into_iter() + .map(|plugin| { + let plugin = serde_json::from_value(plugin).expect("deserialize installed plugin"); + remote_installed_plugin_to_cache_entry(&plugin).expect("valid installed plugin") + }) + .collect::>(); + + assert_eq!(installed_plugins, expected_plugins); + assert_eq!( + recorded_http_client_urls(&selected_urls), + vec![ + format!( + "{}/backend-api/ps/plugins/installed?limit=200", + server.uri() + ), + format!( + "{}/backend-api/ps/plugins/installed?limit=200&pageToken=next+page%2F%2B", + server.uri() + ), + ] + ); +} + #[test] fn cached_remote_plugin_catalog_scopes_returns_existing_scopes() { let codex_home = tempfile::tempdir().expect("create codex home"); diff --git a/codex-rs/core/src/plugins/discoverable_tests.rs b/codex-rs/core/src/plugins/discoverable_tests.rs index a5472f18db..12ca2fd3d4 100644 --- a/codex-rs/core/src/plugins/discoverable_tests.rs +++ b/codex-rs/core/src/plugins/discoverable_tests.rs @@ -2,11 +2,7 @@ use crate::plugins::test_support::load_plugins_config; use crate::plugins::test_support::write_file; use crate::plugins::test_support::write_openai_curated_marketplace; use codex_core_plugins::PluginsManager; -use codex_core_plugins::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; -use codex_core_plugins::remote::RemotePluginServiceConfig; -use codex_core_plugins::remote::fetch_and_cache_global_remote_plugin_catalog; use codex_core_plugins::startup_sync::curated_plugins_repo_path; -use codex_protocol::protocol::Product; use codex_tools::DiscoverablePluginInfo; use pretty_assertions::assert_eq; use tempfile::tempdir; @@ -15,321 +11,16 @@ async fn list_discoverable_plugins( config: &crate::config::Config, loaded_plugin_app_connector_ids: &[String], ) -> anyhow::Result> { - list_discoverable_plugins_with_auth(config, /*auth*/ None, loaded_plugin_app_connector_ids) - .await -} - -async fn list_discoverable_plugins_with_auth( - config: &crate::config::Config, - auth: Option<&codex_login::CodexAuth>, - loaded_plugin_app_connector_ids: &[String], -) -> anyhow::Result> { - let plugins_manager = PluginsManager::new_with_options( - config.codex_home.to_path_buf(), - Some(Product::Codex), - auth.map(codex_login::CodexAuth::api_auth_mode), - ); - list_discoverable_plugins_with_manager_and_auth( - config, - &plugins_manager, - auth, - loaded_plugin_app_connector_ids, - ) - .await -} - -async fn list_discoverable_plugins_with_manager_and_auth( - config: &crate::config::Config, - plugins_manager: &PluginsManager, - auth: Option<&codex_login::CodexAuth>, - loaded_plugin_app_connector_ids: &[String], -) -> anyhow::Result> { + let plugins_manager = PluginsManager::new(config.codex_home.to_path_buf()); super::list_tool_suggest_discoverable_plugins( config, - plugins_manager, - auth, + &plugins_manager, + /*auth*/ None, loaded_plugin_app_connector_ids, ) .await } -#[tokio::test] -async fn list_tool_suggest_discoverable_plugins_includes_cached_remote_global_plugins() { - use codex_login::CodexAuth; - use serde_json::json; - use wiremock::Mock; - use wiremock::MockServer; - use wiremock::ResponseTemplate; - use wiremock::matchers::method; - use wiremock::matchers::path; - use wiremock::matchers::query_param; - - let codex_home = tempdir().expect("tempdir should succeed"); - write_file( - &codex_home.path().join(crate::config::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")) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({ - "plugins": [ - { - "id": "plugins~Plugin_remote_github", - "name": "github", - "scope": "GLOBAL", - "installation_policy": "AVAILABLE", - "authentication_policy": "ON_USE", - "status": "AVAILABLE", - "release": { - "display_name": "Remote GitHub", - "description": "Remote GitHub long", - "app_ids": ["github"], - "interface": { - "short_description": "Remote GitHub short", - "long_description": null, - "developer_name": null, - "category": null, - "capabilities": [], - "website_url": null, - "privacy_policy_url": null, - "terms_of_service_url": null, - "brand_color": null, - "default_prompt": null, - "composer_icon_url": null, - "logo_url": null, - "screenshot_urls": [] - }, - "skills": [ - { - "name": "github", - "description": "Use GitHub", - "interface": null - } - ] - } - }, - { - "id": "plugins~Plugin_remote_unlisted", - "name": "remote-unlisted", - "scope": "GLOBAL", - "installation_policy": "AVAILABLE", - "authentication_policy": "ON_USE", - "status": "AVAILABLE", - "release": { - "display_name": "Remote Unlisted", - "description": "Remote Unlisted long", - "app_ids": [], - "interface": { - "short_description": "Remote Unlisted short", - "long_description": null, - "developer_name": null, - "category": null, - "capabilities": [], - "website_url": null, - "privacy_policy_url": null, - "terms_of_service_url": null, - "brand_color": null, - "default_prompt": null, - "composer_icon_url": null, - "logo_url": null, - "screenshot_urls": [] - }, - "skills": [ - { - "name": "remote-unlisted", - "description": "Use unlisted remote plugin", - "interface": null - } - ] - } - }, - { - "id": "plugins~Plugin_remote_slack_not_available", - "name": "slack", - "scope": "GLOBAL", - "installation_policy": "NOT_AVAILABLE", - "authentication_policy": "ON_USE", - "status": "AVAILABLE", - "release": { - "display_name": "Remote Slack", - "description": "Remote Slack long", - "app_ids": [], - "interface": { - "short_description": "Remote Slack short", - "long_description": null, - "developer_name": null, - "category": null, - "capabilities": [], - "website_url": null, - "privacy_policy_url": null, - "terms_of_service_url": null, - "brand_color": null, - "default_prompt": null, - "composer_icon_url": null, - "logo_url": null, - "screenshot_urls": [] - }, - "skills": [] - } - }, - { - "id": "plugins~Plugin_remote_figma_admin_disabled", - "name": "figma", - "scope": "GLOBAL", - "installation_policy": "AVAILABLE", - "authentication_policy": "ON_USE", - "status": "DISABLED_BY_ADMIN", - "release": { - "display_name": "Remote Figma", - "description": "Remote Figma long", - "app_ids": [], - "interface": { - "short_description": "Remote Figma short", - "long_description": null, - "developer_name": null, - "category": null, - "capabilities": [], - "website_url": null, - "privacy_policy_url": null, - "terms_of_service_url": null, - "brand_color": null, - "default_prompt": null, - "composer_icon_url": null, - "logo_url": null, - "screenshot_urls": [] - }, - "skills": [] - } - } - ], - "pagination": { - "next_page_token": null - } - }))) - .expect(1) - .mount(&server) - .await; - - let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); - let mut config = load_plugins_config(codex_home.path()).await; - config.chatgpt_base_url = format!("{}/backend-api", server.uri()); - let plugins_manager = PluginsManager::new(config.codex_home.to_path_buf()); - fetch_and_cache_global_remote_plugin_catalog( - codex_home.path(), - &RemotePluginServiceConfig::new( - config.chatgpt_base_url.clone(), - config.http_client_factory(), - ), - Some(&auth), - ) - .await - .expect("remote plugin catalog cache should write"); - - let discoverable_plugins = list_discoverable_plugins_with_manager_and_auth( - &config, - &plugins_manager, - Some(&auth), - &[], - ) - .await - .unwrap(); - assert!( - discoverable_plugins - .iter() - .all(|plugin| plugin.id != "github@openai-curated-remote") - ); - - for scope in ["GLOBAL", "USER", "WORKSPACE"] { - Mock::given(method("GET")) - .and(path("/backend-api/ps/plugins/installed")) - .and(query_param("scope", scope)) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({ - "plugins": [], - "pagination": { - "next_page_token": null - } - }))) - .expect(1) - .mount(&server) - .await; - } - plugins_manager - .build_and_cache_remote_installed_plugin_marketplaces( - &config.plugins_config_input(), - Some(&auth), - &[REMOTE_GLOBAL_MARKETPLACE_NAME], - /*on_effective_plugins_changed*/ None, - ) - .await - .expect("remote installed plugin cache should write"); - - let discoverable_plugins = list_discoverable_plugins_with_manager_and_auth( - &config, - &plugins_manager, - Some(&auth), - &[], - ) - .await - .unwrap(); - assert_eq!( - discoverable_plugins - .iter() - .filter(|plugin| plugin.id.ends_with("@openai-curated-remote")) - .map(|plugin| plugin.id.as_str()) - .collect::>(), - vec!["github@openai-curated-remote"] - ); - let remote_plugins = discoverable_plugins - .into_iter() - .filter(|plugin| plugin.id == "github@openai-curated-remote") - .collect::>(); - - assert_eq!( - remote_plugins, - vec![DiscoverablePluginInfo { - id: "github@openai-curated-remote".to_string(), - remote_plugin_id: Some("plugins~Plugin_remote_github".to_string()), - name: "Remote GitHub".to_string(), - description: Some("Remote GitHub short".to_string()), - has_skills: true, - mcp_server_names: Vec::new(), - app_connector_ids: vec!["github".to_string()], - }] - ); - - write_file( - &codex_home.path().join(crate::config::CONFIG_TOML_FILE), - r#"[features] -plugins = true - -[tool_suggest] -disabled_tools = [ - { type = "plugin", id = "github@openai-curated-remote" } -] -"#, - ); - let mut config_with_disabled_remote_plugin = load_plugins_config(codex_home.path()).await; - config_with_disabled_remote_plugin.chatgpt_base_url = config.chatgpt_base_url.clone(); - let discoverable_plugins = list_discoverable_plugins_with_manager_and_auth( - &config_with_disabled_remote_plugin, - &plugins_manager, - Some(&auth), - &[], - ) - .await - .unwrap(); - assert!( - discoverable_plugins - .iter() - .all(|plugin| plugin.id != "github@openai-curated-remote") - ); -} - #[tokio::test] async fn list_tool_suggest_discoverable_plugins_returns_empty_when_plugins_feature_disabled() { let codex_home = tempdir().expect("tempdir should succeed"); diff --git a/codex-rs/core/tests/suite/request_plugin_install.rs b/codex-rs/core/tests/suite/request_plugin_install.rs index 735d1b20c5..95eb37aafb 100644 --- a/codex-rs/core/tests/suite/request_plugin_install.rs +++ b/codex-rs/core/tests/suite/request_plugin_install.rs @@ -355,7 +355,6 @@ async fn mount_empty_remote_installed_plugins(server: &wiremock::MockServer) -> async fn mount_remote_calendar_installed_plugins(server: &wiremock::MockServer) { Mock::given(method("GET")) .and(path("/ps/plugins/installed")) - .and(query_param("scope", "GLOBAL")) .respond_with(remote_installed_plugins_response(vec![json!({ "id": REMOTE_CALENDAR_PLUGIN_ID, "name": "calendar", @@ -373,15 +372,6 @@ async fn mount_remote_calendar_installed_plugins(server: &wiremock::MockServer) .with_priority(1) .mount(server) .await; - for scope in ["WORKSPACE", "USER"] { - Mock::given(method("GET")) - .and(path("/ps/plugins/installed")) - .and(query_param("scope", scope)) - .respond_with(remote_installed_plugins_response(Vec::new())) - .with_priority(1) - .mount(server) - .await; - } } #[tokio::test(flavor = "multi_thread", worker_threads = 2)]