diff --git a/codex-rs/core-plugins/src/discoverable.rs b/codex-rs/core-plugins/src/discoverable.rs index baad28b53f..1da0ac251e 100644 --- a/codex-rs/core-plugins/src/discoverable.rs +++ b/codex-rs/core-plugins/src/discoverable.rs @@ -12,6 +12,7 @@ use crate::OPENAI_CURATED_MARKETPLACE_NAME; use crate::PluginsConfigInput; use crate::PluginsManager; use crate::marketplace::MarketplacePluginInstallPolicy; +use crate::remote::InstalledPluginDisplayAssets; use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; const TOOL_SUGGEST_DISCOVERABLE_PLUGIN_ALLOWLIST: &[&str] = &[ @@ -87,9 +88,10 @@ impl PluginsManager { .context("failed to list plugin marketplaces for tool suggestions")? .marketplaces; let remote_installed_marketplaces = if use_remote_global_catalog { - self.build_remote_installed_plugin_marketplaces_from_cache(&[ - REMOTE_GLOBAL_MARKETPLACE_NAME, - ]) + self.build_remote_installed_plugin_marketplaces_from_cache_with_display_assets( + &[REMOTE_GLOBAL_MARKETPLACE_NAME], + InstalledPluginDisplayAssets::Skip, + ) } else { None }; diff --git a/codex-rs/core-plugins/src/discoverable_tests.rs b/codex-rs/core-plugins/src/discoverable_tests.rs index df227071fd..1821f6d823 100644 --- a/codex-rs/core-plugins/src/discoverable_tests.rs +++ b/codex-rs/core-plugins/src/discoverable_tests.rs @@ -4,6 +4,7 @@ use crate::OPENAI_BUNDLED_MARKETPLACE_NAME; use crate::PluginInstallRequest; use crate::PluginsConfigInput; use crate::PluginsManager; +use crate::remote::InstalledPluginDisplayAssets; use crate::remote::REMOTE_GLOBAL_MARKETPLACE_NAME; use crate::remote::RemotePluginServiceConfig; use crate::remote::fetch_and_cache_global_remote_plugin_catalog; @@ -840,6 +841,18 @@ source = "/tmp/{sales_marketplace_name}" #[tokio::test] async fn cached_remote_discovery_requires_installed_cache_and_filters_candidates() { + assert_cached_remote_discovery_filters_candidates(InstalledPluginDisplayAssets::Include).await; +} + +/// Tool suggestions need installed IDs even when display enrichment was skipped. +#[tokio::test] +async fn cached_remote_discovery_accepts_snapshot_without_display_assets() { + assert_cached_remote_discovery_filters_candidates(InstalledPluginDisplayAssets::Skip).await; +} + +async fn assert_cached_remote_discovery_filters_candidates( + display_assets: InstalledPluginDisplayAssets, +) { let codex_home = tempdir().expect("tempdir should succeed"); write_file( &codex_home.path().join(CONFIG_TOML_FILE), @@ -973,10 +986,19 @@ plugins = true Vec::new() ); - Mock::given(method("GET")) + let installed_request = Mock::given(method("GET")) .and(path("/backend-api/ps/plugins/installed")) .and(query_param_is_missing("scope")) - .and(query_param_is_missing("includeDownloadUrls")) + .and(query_param_is_missing("includeDownloadUrls")); + let installed_request = match display_assets { + InstalledPluginDisplayAssets::Include => { + installed_request.and(query_param_is_missing("includeDisplayAssets")) + } + InstalledPluginDisplayAssets::Skip => { + installed_request.and(query_param("includeDisplayAssets", "false")) + } + }; + installed_request .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "plugins": [], "pagination": { @@ -987,11 +1009,12 @@ plugins = true .mount(&server) .await; plugins_manager - .build_and_cache_remote_installed_plugin_marketplaces( + .build_and_cache_remote_installed_plugin_marketplaces_with_display_assets( &plugins, Some(&auth), &[REMOTE_GLOBAL_MARKETPLACE_NAME], /*on_effective_plugins_changed*/ None, + display_assets, ) .await .expect("remote installed plugin cache should write"); diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 9a1c0ee236..bad8481127 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -61,6 +61,7 @@ use crate::marketplace_upgrade::ConfiguredMarketplaceUpgradeError; 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; @@ -280,10 +281,11 @@ struct RemoteInstalledPluginsCache { // A cancelled pass may mutate bundles before publication; retry must notify consumers. needs_effective_plugins_refresh: bool, plugins: Option>, + display_assets: InstalledPluginDisplayAssets, } enum RemoteInstalledPluginsCachePublication { - Refresh, + Refresh(InstalledPluginDisplayAssets), Reconcile, } @@ -1247,11 +1249,24 @@ impl PluginsManager { pub fn build_remote_installed_plugin_marketplaces_from_cache( &self, visible_marketplaces: &[&str], + ) -> Option> { + self.build_remote_installed_plugin_marketplaces_from_cache_with_display_assets( + visible_marketplaces, + InstalledPluginDisplayAssets::Include, + ) + } + + /// A runtime snapshot cannot satisfy a caller that requires display enrichment. + pub fn build_remote_installed_plugin_marketplaces_from_cache_with_display_assets( + &self, + visible_marketplaces: &[&str], + display_assets: InstalledPluginDisplayAssets, ) -> Option> { let auth = self.auth_manager.auth_cached(); self.build_remote_installed_plugin_marketplaces_from_cache_for_auth( auth.as_ref(), visible_marketplaces, + display_assets, ) } @@ -1259,6 +1274,7 @@ impl PluginsManager { &self, auth: Option<&CodexAuth>, visible_marketplaces: &[&str], + display_assets: InstalledPluginDisplayAssets, ) -> Option> { let cache = match self.remote_installed_plugins_cache.read() { Ok(cache) => cache, @@ -1269,6 +1285,11 @@ impl PluginsManager { { return None; } + if display_assets == InstalledPluginDisplayAssets::Include + && cache.display_assets == InstalledPluginDisplayAssets::Skip + { + return None; + } let plugins = cache.plugins.as_ref()?; Some( crate::remote::group_remote_installed_plugins_by_marketplaces( @@ -1309,13 +1330,33 @@ impl PluginsManager { auth: Option<&CodexAuth>, visible_marketplaces: &[&str], on_effective_plugins_changed: Option, + ) -> Result, RemotePluginCatalogError> { + self.build_and_cache_remote_installed_plugin_marketplaces_with_display_assets( + config, + auth, + visible_marketplaces, + on_effective_plugins_changed, + InstalledPluginDisplayAssets::Include, + ) + .await + } + + /// Fetch installed runtime metadata without optional display enrichment. + pub async fn build_and_cache_remote_installed_plugin_marketplaces_with_display_assets( + &self, + config: &PluginsConfigInput, + auth: Option<&CodexAuth>, + visible_marketplaces: &[&str], + on_effective_plugins_changed: Option, + display_assets: InstalledPluginDisplayAssets, ) -> Result, RemotePluginCatalogError> { let generation = self .prepare_remote_installed_plugins_cache_generation(auth) .ok_or(RemotePluginCatalogError::AuthRequired)?; - let plugins = crate::remote::fetch_remote_installed_plugins( + let plugins = crate::remote::fetch_remote_installed_plugins_with_display_assets( &remote_plugin_service_config(config), auth, + display_assets, ) .await?; let marketplaces = crate::remote::group_remote_installed_plugins_by_marketplaces( @@ -1326,7 +1367,7 @@ impl PluginsManager { generation, plugins, auth, - RemoteInstalledPluginsCachePublication::Refresh, + RemoteInstalledPluginsCachePublication::Refresh(display_assets), ) else { let auth_identity = RemoteInstalledPluginsAuthIdentity::from_auth(auth); if !self.remote_installed_plugins_auth_is_current(&auth_identity) { @@ -1336,6 +1377,7 @@ impl PluginsManager { .build_remote_installed_plugin_marketplaces_from_cache_for_auth( auth, visible_marketplaces, + display_assets, ) .unwrap_or(marketplaces)); }; @@ -1346,6 +1388,7 @@ impl PluginsManager { .build_remote_installed_plugin_marketplaces_from_cache_for_auth( auth, visible_marketplaces, + display_assets, ) .unwrap_or(marketplaces)) } @@ -1463,6 +1506,12 @@ impl PluginsManager { } let needs_effective_plugins_refresh = is_reconcile && std::mem::take(&mut cache.needs_effective_plugins_refresh); + cache.display_assets = match publication { + RemoteInstalledPluginsCachePublication::Refresh(display_assets) => display_assets, + RemoteInstalledPluginsCachePublication::Reconcile => { + InstalledPluginDisplayAssets::Include + } + }; if cache.plugins.as_ref() == Some(&plugins) { drop(cache); if needs_effective_plugins_refresh { @@ -1492,7 +1541,7 @@ impl PluginsManager { generation, plugins, auth.as_ref(), - RemoteInstalledPluginsCachePublication::Refresh, + RemoteInstalledPluginsCachePublication::Refresh(InstalledPluginDisplayAssets::Include), ) .expect("test cache generation should remain current") } @@ -3382,7 +3431,9 @@ impl PluginsManager { request.generation, plugins, request.auth.as_ref(), - RemoteInstalledPluginsCachePublication::Refresh, + RemoteInstalledPluginsCachePublication::Refresh( + InstalledPluginDisplayAssets::Include, + ), ) else { continue; }; diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 525d4faef4..39ac515871 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -1848,6 +1848,71 @@ async fn build_remote_installed_plugin_marketplaces_from_cache_uses_remote_metad ); } +// A reduced snapshot remains usable for execution, but display callers must fetch enrichment. +// Display-resolution changes alone must not invalidate effective runtime metadata. +#[tokio::test] +async fn unresolved_display_assets_does_not_satisfy_display_cache_reads() { + let codex_home = TempDir::new().unwrap(); + let manager = test_plugins_manager(codex_home.path().to_path_buf()); + let auth = manager.auth_manager.auth_cached(); + let plugins = vec![remote_installed_linear_plugin()]; + let generation = manager + .prepare_remote_installed_plugins_cache_generation(auth.as_ref()) + .unwrap(); + assert_eq!( + manager.write_remote_installed_plugins_cache_snapshot( + generation, + plugins.clone(), + auth.as_ref(), + RemoteInstalledPluginsCachePublication::Refresh(InstalledPluginDisplayAssets::Skip), + ), + Some(true) + ); + assert!( + manager + .build_remote_installed_plugin_marketplaces_from_cache(&[ + REMOTE_GLOBAL_MARKETPLACE_NAME + ],) + .is_none() + ); + let runtime = manager + .build_remote_installed_plugin_marketplaces_from_cache_with_display_assets( + &[REMOTE_GLOBAL_MARKETPLACE_NAME], + InstalledPluginDisplayAssets::Skip, + ) + .expect("runtime metadata is ready"); + assert_eq!(runtime[0].plugins.len(), 1); + assert_eq!( + manager.write_remote_installed_plugins_cache_snapshot( + generation, + plugins.clone(), + auth.as_ref(), + RemoteInstalledPluginsCachePublication::Refresh(InstalledPluginDisplayAssets::Include), + ), + Some(false) + ); + assert_eq!( + manager.build_remote_installed_plugin_marketplaces_from_cache(&[ + REMOTE_GLOBAL_MARKETPLACE_NAME + ],), + Some(runtime) + ); + assert_eq!( + manager.write_remote_installed_plugins_cache_snapshot( + generation, + plugins, + auth.as_ref(), + RemoteInstalledPluginsCachePublication::Refresh(InstalledPluginDisplayAssets::Skip), + ), + Some(false) + ); + assert!( + manager + .build_remote_installed_plugin_marketplaces_from_cache(&[]) + .is_none() + ); +} + #[tokio::test] async fn build_remote_installed_plugin_marketplaces_from_cache_filters_by_marketplace_name() { let codex_home = TempDir::new().unwrap(); @@ -7839,7 +7904,7 @@ fn reconciliation_fences_refresh_publication_and_recovers_after_cancellation() { stale_refresh_generation, Vec::new(), /*auth*/ None, - RemoteInstalledPluginsCachePublication::Refresh, + RemoteInstalledPluginsCachePublication::Refresh(InstalledPluginDisplayAssets::Include), ), None ); @@ -7871,7 +7936,7 @@ fn reconciliation_fences_refresh_publication_and_recovers_after_cancellation() { refresh_generation, vec![remote_installed_plugin("beta")], /*auth*/ None, - RemoteInstalledPluginsCachePublication::Refresh, + RemoteInstalledPluginsCachePublication::Refresh(InstalledPluginDisplayAssets::Include), ), Some(true) ); diff --git a/codex-rs/core-plugins/src/plugin_measurement_catalog.rs b/codex-rs/core-plugins/src/plugin_measurement_catalog.rs index b961b9d8c4..dc1400f31a 100644 --- a/codex-rs/core-plugins/src/plugin_measurement_catalog.rs +++ b/codex-rs/core-plugins/src/plugin_measurement_catalog.rs @@ -17,6 +17,7 @@ pub(crate) async fn fetch_measurement_reference_bundle( auth, RemoteInstalledPluginScope::Single(RemotePluginScope::Global), /*include_download_urls*/ true, + InstalledPluginDisplayAssets::Include, ) .await?; let mut matching = installed.into_iter().filter(|installed| { diff --git a/codex-rs/core-plugins/src/remote.rs b/codex-rs/core-plugins/src/remote.rs index b1e3d783ee..2e3e4ae00a 100644 --- a/codex-rs/core-plugins/src/remote.rs +++ b/codex-rs/core-plugins/src/remote.rs @@ -1282,9 +1282,32 @@ fn build_remote_marketplace( })) } +/// Selects whether installed-plugin metadata needs display enrichment. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum InstalledPluginDisplayAssets { + /// Resolve image URLs and creator names. + #[default] + Include, + /// Skip display lookups; retain stored display fields and execution metadata. + Skip, +} + pub(crate) async fn fetch_remote_installed_plugins( config: &RemotePluginServiceConfig, auth: Option<&CodexAuth>, +) -> Result, RemotePluginCatalogError> { + fetch_remote_installed_plugins_with_display_assets( + config, + auth, + InstalledPluginDisplayAssets::Include, + ) + .await +} + +pub(crate) async fn fetch_remote_installed_plugins_with_display_assets( + config: &RemotePluginServiceConfig, + auth: Option<&CodexAuth>, + display_assets: InstalledPluginDisplayAssets, ) -> Result, RemotePluginCatalogError> { let auth = ensure_chatgpt_auth(auth)?; let mut installed_plugins = fetch_installed_plugins( @@ -1292,6 +1315,7 @@ pub(crate) async fn fetch_remote_installed_plugins( auth, RemoteInstalledPluginScope::All, /*include_download_urls*/ false, + display_assets, ) .await? .into_iter() @@ -2175,6 +2199,7 @@ async fn fetch_installed_plugins_for_scope( auth, RemoteInstalledPluginScope::Single(scope), /*include_download_urls*/ false, + InstalledPluginDisplayAssets::Include, ) .await } @@ -2184,6 +2209,7 @@ async fn fetch_installed_plugins( auth: &CodexAuth, scope: RemoteInstalledPluginScope, include_download_urls: bool, + display_assets: InstalledPluginDisplayAssets, ) -> Result, RemotePluginCatalogError> { // Only a complete inventory is authoritative: every failure discards all fetched pages // before callers can publish the snapshot or remove plugins absent from it. @@ -2198,6 +2224,7 @@ async fn fetch_installed_plugins( scope, page_token.as_deref(), include_download_urls, + display_assets, ) .await?; plugins.extend(response.plugins); @@ -2275,6 +2302,7 @@ async fn get_remote_plugin_installed_page( scope: RemoteInstalledPluginScope, page_token: Option<&str>, include_download_urls: bool, + display_assets: InstalledPluginDisplayAssets, ) -> Result { let base_url = config.chatgpt_base_url.trim_end_matches('/'); let mut url = Url::parse(&format!("{base_url}/ps/plugins/installed")) @@ -2289,6 +2317,10 @@ async fn get_remote_plugin_installed_page( url.query_pairs_mut() .append_pair("includeDownloadUrls", "true"); } + if display_assets == InstalledPluginDisplayAssets::Skip { + url.query_pairs_mut() + .append_pair("includeDisplayAssets", "false"); + } if let Some(page_token) = page_token { url.query_pairs_mut().append_pair("pageToken", page_token); } 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 92a2ba8697..89f5681d29 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 @@ -13,6 +13,7 @@ use super::RemotePluginServiceConfig; use super::RemotePluginShareDiscoverability; use super::ensure_chatgpt_auth; use super::fetch_installed_plugins; +use crate::remote::InstalledPluginDisplayAssets; use crate::store::PLUGINS_CACHE_DIR; use crate::store::PluginStore; use crate::store::PluginStoreError; @@ -141,6 +142,7 @@ pub(crate) async fn sync_remote_installed_plugin_bundles_once_with_snapshot( auth, RemoteInstalledPluginScope::All, /*include_download_urls*/ true, + InstalledPluginDisplayAssets::Include, ) .await?; // `/installed` is an authoritative full snapshot. If any row cannot be diff --git a/codex-rs/core-plugins/src/remote_tests.rs b/codex-rs/core-plugins/src/remote_tests.rs index b12bb7ce7b..580baabc85 100644 --- a/codex-rs/core-plugins/src/remote_tests.rs +++ b/codex-rs/core-plugins/src/remote_tests.rs @@ -162,6 +162,55 @@ async fn remote_installed_plugins_paginate_across_all_scopes_without_download_ur ); } +// Skipping display assets must survive pagination without requesting bundle downloads. +#[tokio::test] +async fn installed_plugins_skip_display_resolution_on_every_page() { + let server = MockServer::start().await; + for (cursor, next_cursor) in [(None, Some("next page/+")), (Some("next page/+"), None)] { + let mock = Mock::given(method("GET")) + .and(path("/backend-api/ps/plugins/installed")) + .and(query_param("includeDisplayAssets", "false")) + .and(query_param_is_missing("includeDownloadUrls")); + let mock = match cursor { + Some(cursor) => mock.and(query_param("pageToken", cursor)), + None => mock.and(query_param_is_missing("pageToken")), + }; + mock.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "plugins": [], + "pagination": {"next_page_token": next_cursor}, + }))) + .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 plugins = fetch_remote_installed_plugins_with_display_assets( + &config, + Some(&auth), + InstalledPluginDisplayAssets::Skip, + ) + .await + .expect("runtime metadata request succeeds"); + + assert_eq!(plugins, Vec::new()); + assert_eq!( + recorded_http_client_urls(&selected_urls), + vec![ + format!( + "{}/backend-api/ps/plugins/installed?limit=200&includeDisplayAssets=false", + server.uri() + ), + format!( + "{}/backend-api/ps/plugins/installed?limit=200&includeDisplayAssets=false&pageToken=next+page%2F%2B", + server.uri() + ), + ] + ); +} + #[tokio::test] async fn remote_installed_plugins_reject_cyclic_cursors() { let server = MockServer::start().await;