Allow installed plugin metadata to skip display enrichment (#56483)

## Why

Runtime metadata and tool suggestions do not require resolved image URLs or creator names. Callers that need display enrichment must still be able to distinguish a complete display snapshot from one fetched without it.

## What changed

- Add `InstalledPluginDisplayAssets` to select whether installed plugin requests include display enrichment, preserving `Include` as the default.
- Send `includeDisplayAssets=false` on every request page when `Skip` is selected.
- Track display enrichment in the installed plugin cache. Allow tool suggestions to use snapshots without enrichment, while treating those snapshots as cache misses for display callers.
- Avoid invalidating effective runtime metadata when only the cache's display enrichment state changes.

## Testing

Add coverage for paginated requests without display enrichment, cache eligibility and enrichment state transitions, and discovery using snapshots fetched with or without display assets.

GitOrigin-RevId: 1fdfd6443a300a03753416f2f6c633b52a97d0fb
This commit is contained in:
bkotsopoulos
2026-10-23 14:15:03 +00:00
committed by copyberry
parent 7d592f6dd9
commit de06fd0be4
8 changed files with 238 additions and 13 deletions
+5 -3
View File
@@ -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
};
@@ -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");
+56 -5
View File
@@ -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<Vec<RemoteInstalledPlugin>>,
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<Vec<crate::remote::RemoteMarketplace>> {
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<Vec<crate::remote::RemoteMarketplace>> {
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<Vec<crate::remote::RemoteMarketplace>> {
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<EffectivePluginsChangedCallback>,
) -> Result<Vec<crate::remote::RemoteMarketplace>, 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<EffectivePluginsChangedCallback>,
display_assets: InstalledPluginDisplayAssets,
) -> Result<Vec<crate::remote::RemoteMarketplace>, 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;
};
+67 -2
View File
@@ -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)
);
@@ -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| {
+32
View File
@@ -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<Vec<RemoteInstalledPlugin>, 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<Vec<RemoteInstalledPlugin>, 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<Vec<RemotePluginInstalledItem>, 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<RemotePluginInstalledResponse, RemotePluginCatalogError> {
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);
}
@@ -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
+49
View File
@@ -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;