Reduce remote plugin catalog cache probing overhead (#57258)

## Why

Cache presence and freshness checks deserialize full plugin records even when callers only need to decide whether a cached catalog is available.

## What changed

- Probe cache availability without deserializing plugin records, while still requiring a valid catalog envelope and a `plugins` array.
- Let `PreferFreshCache` reject stale catalogs from a 256-byte prefix when the header provides enough information; fall back to a full read otherwise.
- Limit discovery cache probes to the user and workspace scopes that need them, skipping the global cache probe.
- Serialize cached plugin slices directly without cloning them.

## Testing

Add coverage for malformed cache headers and payloads, reordered fields, stale-prefix rejection, and deferring plugin-record validation until a full catalog load.

GitOrigin-RevId: 1e5ec95917f38c84bba30dbd3bb8c2f91cc7e3a8
This commit is contained in:
Charlie Marsh
2026-10-26 21:13:03 +00:00
committed by copyberry
parent eb3572f9d7
commit a8bca62978
5 changed files with 275 additions and 50 deletions
+7 -2
View File
@@ -2983,15 +2983,20 @@ impl PluginsManager {
if !prefetch_discovery {
return;
}
let scopes_to_probe: &[RemotePluginScope] =
if config_for_remote_sync.remote_plugin_enabled {
&[RemotePluginScope::User, RemotePluginScope::Workspace]
} else {
&[RemotePluginScope::Workspace]
};
let mut scopes = crate::remote::cached_remote_plugin_catalog_scopes(
manager.codex_home.as_path(),
&remote_plugin_service_config(&config_for_remote_sync),
auth.as_ref(),
scopes_to_probe,
);
if config_for_remote_sync.remote_plugin_enabled {
scopes.insert(RemotePluginScope::Global);
} else {
scopes.retain(|scope| *scope == RemotePluginScope::Workspace);
}
manager.maybe_start_remote_catalog_cache_refresh(
&config_for_remote_sync,
+19 -20
View File
@@ -541,8 +541,6 @@ enum RemoteInstalledPluginScope {
}
impl RemotePluginScope {
const CATALOG_CACHE_SCOPES: [Self; 3] = [Self::Global, Self::User, Self::Workspace];
fn api_value(self) -> &'static str {
match self {
Self::Global => "GLOBAL",
@@ -1155,32 +1153,35 @@ pub(crate) fn has_fresh_cached_remote_plugin_catalog(
let Ok(auth) = ensure_chatgpt_auth(auth) else {
return false;
};
catalog_cache::load_cached_directory_plugins(
codex_home, config, auth, scope, /*collection*/ None,
catalog_cache::has_cached_directory_plugins(
codex_home,
config,
auth,
scope,
RemotePluginCatalogCacheMode::PreferFreshCache,
)
.is_some_and(|cached| {
matches!(
cached.freshness,
catalog_cache::RemotePluginCatalogCacheFreshness::Fresh
)
})
}
pub(crate) fn cached_remote_plugin_catalog_scopes(
codex_home: &Path,
config: &RemotePluginServiceConfig,
auth: Option<&CodexAuth>,
scopes: &[RemotePluginScope],
) -> BTreeSet<RemotePluginScope> {
let Ok(auth) = ensure_chatgpt_auth(auth) else {
return BTreeSet::new();
};
RemotePluginScope::CATALOG_CACHE_SCOPES
.into_iter()
scopes
.iter()
.copied()
.filter(|scope| {
catalog_cache::load_cached_directory_plugins(
codex_home, config, auth, *scope, /*collection*/ None,
catalog_cache::has_cached_directory_plugins(
codex_home,
config,
auth,
*scope,
RemotePluginCatalogCacheMode::PreferCache,
)
.is_some()
})
.collect()
}
@@ -1196,6 +1197,7 @@ pub fn cached_global_remote_discoverable_plugins(
auth,
RemotePluginScope::Global,
/*collection*/ None,
RemotePluginCatalogCacheMode::PreferCache,
)
.map(|cached| cached.plugins)
.unwrap_or_default()
@@ -2097,13 +2099,10 @@ async fn fetch_directory_plugins_for_scope_with_cache(
collection: Option<&str>,
cache_mode: RemotePluginCatalogCacheMode,
) -> Result<DirectoryPluginsFetchOutcome, RemotePluginCatalogError> {
if cache_mode != RemotePluginCatalogCacheMode::ForceRefetch
&& let Some(codex_home) = codex_home
if let Some(codex_home) = codex_home
&& let Some(cached) = catalog_cache::load_cached_directory_plugins(
codex_home, config, auth, scope, collection,
codex_home, config, auth, scope, collection, cache_mode,
)
&& (cache_mode == RemotePluginCatalogCacheMode::PreferCache
|| cached.freshness == catalog_cache::RemotePluginCatalogCacheFreshness::Fresh)
{
return Ok(DirectoryPluginsFetchOutcome {
plugins: cached.plugins,
+149 -12
View File
@@ -1,3 +1,4 @@
use super::RemotePluginCatalogCacheMode;
use super::RemotePluginDirectoryItem;
use super::RemotePluginScope;
use super::RemotePluginServiceConfig;
@@ -5,7 +6,18 @@ use chrono::DateTime;
use chrono::Utc;
use codex_login::CodexAuth;
use serde::Deserialize;
use serde::Deserializer;
use serde::Serialize;
use serde::de::DeserializeOwned;
use serde::de::DeserializeSeed;
use serde::de::IgnoredAny;
use serde::de::MapAccess;
use serde::de::SeqAccess;
use serde::de::Visitor;
use std::fmt;
use std::fs::File;
use std::io;
use std::io::Read;
use std::path::Path;
use std::path::PathBuf;
use std::time::Duration;
@@ -58,12 +70,12 @@ impl RemotePluginCatalogCacheKey {
}
}
#[derive(Debug, Clone, Serialize, Deserialize)]
struct RemotePluginCatalogDiskCache {
#[derive(Serialize, Deserialize)]
struct RemotePluginCatalogDiskCache<Plugins> {
schema_version: u8,
#[serde(default, skip_serializing_if = "Option::is_none")]
fetched_at: Option<DateTime<Utc>>,
plugins: Vec<RemotePluginDirectoryItem>,
plugins: Plugins,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -72,23 +84,40 @@ pub(super) enum RemotePluginCatalogCacheFreshness {
Stale,
}
#[derive(Debug, Clone)]
pub(super) struct CachedDirectoryPlugins {
pub plugins: Vec<RemotePluginDirectoryItem>,
pub(super) struct CachedDirectoryPlugins<T> {
pub plugins: Vec<T>,
pub freshness: RemotePluginCatalogCacheFreshness,
}
pub(crate) fn load_cached_directory_plugins(
pub(super) fn has_cached_directory_plugins(
codex_home: &Path,
config: &RemotePluginServiceConfig,
auth: &CodexAuth,
scope: RemotePluginScope,
cache_mode: RemotePluginCatalogCacheMode,
) -> bool {
// Ignore each entry so Serde still requires `plugins` to be an array.
load_cached_directory_plugins::<IgnoredAny>(
codex_home, config, auth, scope, /*collection*/ None, cache_mode,
)
.is_some()
}
pub(super) fn load_cached_directory_plugins<T: DeserializeOwned>(
codex_home: &Path,
config: &RemotePluginServiceConfig,
auth: &CodexAuth,
scope: RemotePluginScope,
collection: Option<&str>,
) -> Option<CachedDirectoryPlugins> {
cache_mode: RemotePluginCatalogCacheMode,
) -> Option<CachedDirectoryPlugins<T>> {
if cache_mode == RemotePluginCatalogCacheMode::ForceRefetch {
return None;
}
let cache_key = RemotePluginCatalogCacheKey::new(config, auth, scope, collection)?;
let cache_path = cache_path(codex_home, &cache_key);
let bytes = match std::fs::read(&cache_path) {
Ok(bytes) => bytes,
let bytes = match read_catalog_bytes(&cache_path, cache_mode) {
Ok(bytes) => bytes?,
Err(err) if err.kind() == std::io::ErrorKind::NotFound => return None,
Err(err) => {
warn!(
@@ -98,7 +127,7 @@ pub(crate) fn load_cached_directory_plugins(
return None;
}
};
let cache: RemotePluginCatalogDiskCache = match serde_json::from_slice(&bytes) {
let cache: RemotePluginCatalogDiskCache<Vec<T>> = match serde_json::from_slice(&bytes) {
Ok(cache) => cache,
Err(err) => {
warn!(
@@ -116,6 +145,8 @@ pub(crate) fn load_cached_directory_plugins(
let freshness = if is_fresh(cache.fetched_at, Utc::now()) {
RemotePluginCatalogCacheFreshness::Fresh
} else if cache_mode == RemotePluginCatalogCacheMode::PreferFreshCache {
return None;
} else {
RemotePluginCatalogCacheFreshness::Stale
};
@@ -125,6 +156,112 @@ pub(crate) fn load_cached_directory_plugins(
})
}
fn read_catalog_bytes(
path: &Path,
cache_mode: RemotePluginCatalogCacheMode,
) -> io::Result<Option<Vec<u8>>> {
let mut file = File::open(path)?;
let mut bytes = Vec::new();
if cache_mode == RemotePluginCatalogCacheMode::PreferFreshCache {
const PREFIX_LEN: usize = 256;
bytes.reserve_exact(PREFIX_LEN);
file.by_ref()
.take(PREFIX_LEN as u64)
.read_to_end(&mut bytes)?;
if bytes.len() < PREFIX_LEN {
return Ok(Some(bytes));
}
if CatalogHeader::is_stale_prefix(&bytes, Utc::now()) {
return Ok(None);
}
}
file.read_to_end(&mut bytes)?;
Ok(Some(bytes))
}
#[derive(Default)]
struct CatalogHeader {
schema_version: Option<u8>,
fetched_at: Option<DateTime<Utc>>,
metadata_before_plugins: bool,
}
impl CatalogHeader {
fn is_stale_prefix(bytes: &[u8], now: DateTime<Utc>) -> bool {
let mut header = Self::default();
let mut json = serde_json::Deserializer::from_slice(bytes);
// A bounded read can end inside the payload. Only fully decoded fields count.
let parsed = json.deserialize_map(&mut header).and_then(|()| json.end());
if parsed.is_err_and(|err| !err.is_eof()) {
return false;
}
header.metadata_before_plugins && !is_fresh(header.fetched_at, now)
}
}
// Only the `plugins` field uses the seed. Serde calls `visit_seq` when it enters
// that array, so a missing or wrongly typed payload still goes through the full reader.
impl<'de> DeserializeSeed<'de> for &mut CatalogHeader {
type Value = ();
fn deserialize<D: Deserializer<'de>>(self, deserializer: D) -> Result<(), D::Error> {
deserializer.deserialize_seq(self)
}
}
impl<'de> Visitor<'de> for &mut CatalogHeader {
type Value = ();
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
formatter.write_str("a plugin catalog object or its plugins array")
}
fn visit_map<A: MapAccess<'de>>(self, mut map: A) -> Result<(), A::Error> {
#[derive(Deserialize)]
#[serde(field_identifier, rename_all = "snake_case")]
enum Field {
SchemaVersion,
FetchedAt,
Plugins,
#[serde(other)]
Other,
}
let mut saw_plugins = false;
while let Some(field) = map.next_key()? {
match field {
Field::SchemaVersion if self.schema_version.is_none() => {
self.schema_version = Some(map.next_value()?);
}
Field::FetchedAt if self.fetched_at.is_none() => {
self.fetched_at = Some(map.next_value()?);
}
Field::Plugins if !saw_plugins => {
saw_plugins = true;
map.next_value_seed(&mut *self)?;
}
Field::SchemaVersion => {
return Err(serde::de::Error::duplicate_field("schema_version"));
}
Field::FetchedAt => return Err(serde::de::Error::duplicate_field("fetched_at")),
Field::Plugins => return Err(serde::de::Error::duplicate_field("plugins")),
Field::Other => {
map.next_value::<IgnoredAny>()?;
}
}
}
Ok(())
}
fn visit_seq<A: SeqAccess<'de>>(self, mut plugins: A) -> Result<(), A::Error> {
self.metadata_before_plugins = self.schema_version
== Some(REMOTE_PLUGIN_CATALOG_DISK_CACHE_SCHEMA_VERSION)
&& self.fetched_at.is_some();
while plugins.next_element::<IgnoredAny>()?.is_some() {}
Ok(())
}
}
pub(crate) fn write_cached_directory_plugins(
codex_home: &Path,
config: &RemotePluginServiceConfig,
@@ -140,7 +277,7 @@ pub(crate) fn write_cached_directory_plugins(
let Ok(contents) = serde_json::to_string_pretty(&RemotePluginCatalogDiskCache {
schema_version: REMOTE_PLUGIN_CATALOG_DISK_CACHE_SCHEMA_VERSION,
fetched_at: Some(Utc::now()),
plugins: plugins.to_vec(),
plugins,
}) else {
return;
};
@@ -62,15 +62,16 @@ fn global_catalog_cache_reuses_legacy_cache_file() {
"schema_version": REMOTE_PLUGIN_CATALOG_DISK_CACHE_SCHEMA_VERSION,
"plugins": [],
});
let contents = serde_json::to_string_pretty(&legacy_cache).expect("serialize legacy cache");
codex_utils_path::write_atomically(&legacy_cache_path, &contents).expect("write legacy cache");
codex_utils_path::write_atomically(&legacy_cache_path, &legacy_cache.to_string())
.expect("write legacy cache");
let cached = load_cached_directory_plugins(
let cached = load_cached_directory_plugins::<RemotePluginDirectoryItem>(
codex_home.path(),
&config,
&auth,
RemotePluginScope::Global,
/*collection*/ None,
RemotePluginCatalogCacheMode::PreferCache,
)
.expect("load legacy global cache");
assert!(cached.plugins.is_empty());
@@ -84,11 +85,90 @@ fn global_catalog_cache_reuses_legacy_cache_file() {
/*collection*/ None,
&[],
);
let refreshed_cache: RemotePluginCatalogDiskCache = serde_json::from_slice(
&std::fs::read(&legacy_cache_path).expect("read refreshed legacy cache"),
)
.expect("parse refreshed legacy cache");
assert!(refreshed_cache.fetched_at.is_some());
assert!(has_cached_directory_plugins(
codex_home.path(),
&config,
&auth,
RemotePluginScope::Global,
RemotePluginCatalogCacheMode::PreferFreshCache,
));
}
#[test]
fn freshness_probe_defers_plugin_validation_until_full_read() {
let codex_home = tempfile::tempdir().expect("create codex home");
let config = RemotePluginServiceConfig::new(
"https://chatgpt.com/backend-api".to_string(),
crate::test_support::test_http_client_factory(),
/*product_sku*/ None,
);
let auth = CodexAuth::Headers(AuthHeaders::new(http::HeaderMap::new()));
let scope = RemotePluginScope::Global;
let key = RemotePluginCatalogCacheKey::new(&config, &auth, scope, /*collection*/ None)
.expect("global cache key");
let path = cache_path(codex_home.path(), &key);
let probe = |mode| has_cached_directory_plugins(codex_home.path(), &config, &auth, scope, mode);
let version = REMOTE_PLUGIN_CATALOG_DISK_CACHE_SCHEMA_VERSION;
let now = serde_json::to_string(&Utc::now()).expect("serialize timestamp");
let expired_at = "2000-01-01T00:00:00Z";
let padding = " ".repeat(512);
for contents in [
serde_json::json!({"schema_version": version + 1, "plugins": []}).to_string(),
serde_json::json!({"schema_version": version}).to_string(),
serde_json::json!({"schema_version": version, "plugins": {}}).to_string(),
format!(r#"{{"schema_version":{version},"plugins":[{{"#),
format!(r#"{{"schema_version":{version},"fetched_at":{now},"plugins":[{padding}"#),
format!(
r#"{{"details":{{"fetched_at":"{expired_at}"}},"schema_version":{version},"plugins":[{padding}"#
),
format!(
r#"{{"schema_version":{version},"fetched_at":"{expired_at}","fetched_at":{now},"plugins":[{padding}"#
),
format!(
r#"{{"schema_version":{version},"fetched_at":"{expired_at}","plugins":{{}}}}{padding}"#
),
] {
codex_utils_path::write_atomically(&path, &contents).expect("write invalid cache");
assert!(
!probe(RemotePluginCatalogCacheMode::PreferFreshCache),
"{contents}"
);
assert!(!path.exists());
}
let expired = format!(
r#"{{"note":"[","fetched\u005fat":"{expired_at}","schema_version":{version},"plugins":[{padding}"#
);
codex_utils_path::write_atomically(&path, &expired).expect("write expired cache");
assert!(!probe(RemotePluginCatalogCacheMode::PreferFreshCache));
assert!(path.exists());
assert!(!probe(RemotePluginCatalogCacheMode::PreferCache));
assert!(!path.exists());
let reordered =
format!(r#"{{"plugins":[{padding}],"schema_version":{version},"fetched_at":{now}}}"#);
codex_utils_path::write_atomically(&path, &reordered).expect("write reordered cache");
assert!(probe(RemotePluginCatalogCacheMode::PreferFreshCache));
let contents = serde_json::json!({
"schema_version": version,
"fetched_at": Utc::now(),
"plugins": [{}],
});
codex_utils_path::write_atomically(&path, &contents.to_string()).expect("write invalid plugin");
assert!(probe(RemotePluginCatalogCacheMode::PreferFreshCache));
assert!(
load_cached_directory_plugins::<RemotePluginDirectoryItem>(
codex_home.path(),
&config,
&auth,
scope,
/*collection*/ None,
RemotePluginCatalogCacheMode::PreferCache,
)
.is_none()
);
assert!(!path.exists());
}
#[test]
@@ -110,12 +190,13 @@ fn header_auth_does_not_cache_private_catalogs_without_a_stable_identity() {
&[],
);
assert!(
load_cached_directory_plugins(
load_cached_directory_plugins::<RemotePluginDirectoryItem>(
codex_home.path(),
&config,
&auth,
RemotePluginScope::Global,
/*collection*/ None,
RemotePluginCatalogCacheMode::PreferCache,
)
.is_some()
);
@@ -144,19 +225,20 @@ fn header_auth_does_not_cache_private_catalogs_without_a_stable_identity() {
let insecure_cache = RemotePluginCatalogDiskCache {
schema_version: REMOTE_PLUGIN_CATALOG_DISK_CACHE_SCHEMA_VERSION,
fetched_at: Some(Utc::now()),
plugins: Vec::new(),
plugins: Vec::<RemotePluginDirectoryItem>::new(),
};
let contents = serde_json::to_string_pretty(&insecure_cache).expect("serialize cache");
codex_utils_path::write_atomically(&insecure_cache_path, &contents)
.expect("write insecure cache");
assert!(
load_cached_directory_plugins(
load_cached_directory_plugins::<RemotePluginDirectoryItem>(
codex_home.path(),
&config,
&auth,
scope,
/*collection*/ None,
RemotePluginCatalogCacheMode::PreferCache,
)
.is_none()
);
+7 -5
View File
@@ -468,7 +468,8 @@ fn catalog_cache_invalidation_clears_global_collections_and_preserves_other_scop
/*product_sku*/ None,
);
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
for scope in [RemotePluginScope::Global, RemotePluginScope::Workspace] {
let scopes = [RemotePluginScope::Global, RemotePluginScope::Workspace];
for scope in scopes {
catalog_cache::write_cached_directory_plugins(
codex_home.path(),
&config,
@@ -480,8 +481,8 @@ fn catalog_cache_invalidation_clears_global_collections_and_preserves_other_scop
}
assert_eq!(
cached_remote_plugin_catalog_scopes(codex_home.path(), &config, Some(&auth)),
BTreeSet::from([RemotePluginScope::Global, RemotePluginScope::Workspace])
cached_remote_plugin_catalog_scopes(codex_home.path(), &config, Some(&auth), &scopes),
BTreeSet::from(scopes)
);
catalog_cache::write_cached_directory_plugins(
codex_home.path(),
@@ -500,16 +501,17 @@ fn catalog_cache_invalidation_clears_global_collections_and_preserves_other_scop
);
assert_eq!(
cached_remote_plugin_catalog_scopes(codex_home.path(), &config, Some(&auth)),
cached_remote_plugin_catalog_scopes(codex_home.path(), &config, Some(&auth), &scopes),
BTreeSet::from([RemotePluginScope::Workspace]),
);
assert!(
catalog_cache::load_cached_directory_plugins(
catalog_cache::load_cached_directory_plugins::<RemotePluginDirectoryItem>(
codex_home.path(),
&config,
&auth,
RemotePluginScope::Global,
Some(OPENAI_CURATED_REMOTE_COLLECTION_KEY),
RemotePluginCatalogCacheMode::PreferCache,
)
.is_none()
);