Bound remote installed-plugin inventory pagination (#55946)

## Why

Installed-plugin inventory fetches could keep following pagination cursors
without an overall bound, delaying release of the plugin mutation gate.

## What changed

- Limit each inventory fetch to 60 seconds and 100 pages, and reject repeated
  pagination cursors.
- Request 200 items per page for scoped inventory fetches as well as all-scope
  fetches.
- Report overall fetch timeouts as `remote_catalog_timeout`.

## Testing

Add coverage for cursor cycles, completion at the page limit, and overall
timeout cancellation after successful pages. Verify that failed pagination
preserves the cached inventory and plugin files and releases the mutation gate.

GitOrigin-RevId: e8429dd804581041d102defa9d19db02bcb1739b
This commit is contained in:
Matthew Zeng
2026-10-21 17:28:24 +00:00
committed by copyberry
parent a3b685331f
commit 9b39316997
6 changed files with 269 additions and 27 deletions
@@ -2394,6 +2394,7 @@ fn remote_plugin_catalog_error_type(err: &RemotePluginCatalogError) -> &'static
RemotePluginCatalogError::ArchiveTooLarge { .. } => "remote_catalog_archive_too_large",
RemotePluginCatalogError::MissingUploadEtag => "remote_catalog_missing_upload_etag",
RemotePluginCatalogError::UnexpectedResponse(_) => "remote_catalog_unexpected_response",
RemotePluginCatalogError::InstalledPluginsFetchTimeout => "remote_catalog_timeout",
RemotePluginCatalogError::CacheRemove(_) => "remote_catalog_cache_remove",
}
}
@@ -2463,6 +2464,7 @@ fn remote_plugin_catalog_error_to_jsonrpc(
| RemotePluginCatalogError::ArchiveJoin(_)
| RemotePluginCatalogError::MissingUploadEtag
| RemotePluginCatalogError::UnexpectedResponse(_)
| RemotePluginCatalogError::InstalledPluginsFetchTimeout
| RemotePluginCatalogError::CacheRemove(_) => internal_error(message),
}
}
@@ -2294,6 +2294,7 @@ async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled(
Mock::given(method("GET"))
.and(path("/backend-api/ps/plugins/installed"))
.and(query_param("scope", "GLOBAL"))
.and(query_param("limit", "200"))
.and(header("authorization", "Bearer chatgpt-token"))
.and(header("chatgpt-account-id", "account-123"))
.and(header("oai-product-sku", expected_product_sku))
@@ -2303,6 +2304,7 @@ async fn plugin_list_includes_remote_marketplaces_when_remote_plugin_enabled(
Mock::given(method("GET"))
.and(path("/backend-api/ps/plugins/installed"))
.and(query_param("scope", "WORKSPACE"))
.and(query_param("limit", "200"))
.and(header("authorization", "Bearer chatgpt-token"))
.and(header("chatgpt-account-id", "account-123"))
.and(header("oai-product-sku", expected_product_sku))
+1
View File
@@ -64,6 +64,7 @@ codex-utils-cargo-bin = { workspace = true }
libc = { workspace = true }
pretty_assertions = { workspace = true }
tempfile = { workspace = true }
tokio = { workspace = true, features = ["test-util"] }
tracing-subscriber = { workspace = true }
tracing-test = { workspace = true, features = ["no-env-filter"] }
wiremock = { workspace = true }
@@ -7744,6 +7744,85 @@ remote_plugin = true
assert!(!linear_metadata.exists());
}
#[tokio::test]
async fn failed_installed_plugin_pagination_preserves_inventory_and_releases_mutation_gate() {
let codex_home = TempDir::new().unwrap();
write_file(
&codex_home.path().join(CONFIG_TOML_FILE),
"[features]\nplugins = true\nremote_plugin = true\n",
);
write_cached_plugin(
codex_home.path(),
REMOTE_GLOBAL_MARKETPLACE_NAME,
"retained",
);
let server = MockServer::start().await;
// Both bundle sync and its metadata fallback must reject the incomplete inventory.
Mock::given(method("GET"))
.and(path("/backend-api/ps/plugins/installed"))
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
"plugins": [],
"pagination": {"next_page_token": "next"},
})))
.expect(4)
.mount(&server)
.await;
let mut config = load_config(codex_home.path(), codex_home.path()).await;
config.chatgpt_base_url = format!("{}/backend-api", server.uri());
let auth_manager = test_auth_manager(Some(AuthMode::Chatgpt));
let auth = auth_manager.auth_cached().expect("test ChatGPT auth");
let manager = Arc::new(test_plugins_manager_with_auth_manager(
codex_home.path().to_path_buf(),
Some(Product::Codex),
auth_manager,
));
let previous_inventory = vec![remote_installed_plugin("retained")];
manager.write_remote_installed_plugins_cache(previous_inventory.clone());
manager.maybe_start_remote_installed_plugin_bundle_sync(
&config,
Some(auth),
/*on_effective_plugins_changed*/ None,
);
// Starting the sync acquires the gate synchronously, so this waits for its release.
let _guard = tokio::time::timeout(
Duration::from_secs(5),
manager.acquire_remote_installed_plugin_sync_guard(),
)
.await
.expect("failed pagination should promptly release the mutation gate")
.expect("mutation gate should remain available");
tokio::time::timeout(Duration::from_secs(5), async {
while manager
.remote_installed_plugins_cache_refresh_state
.read()
.expect("refresh state lock")
.in_flight
{
tokio::task::yield_now().await;
}
})
.await
.expect("fallback metadata refresh should finish");
assert_eq!(
manager
.remote_installed_plugins_cache
.read()
.expect("installed cache lock")
.plugins
.clone(),
Some(previous_inventory),
);
assert!(
codex_home
.path()
.join("plugins/cache/openai-curated-remote/retained")
.exists()
);
server.verify().await;
}
#[test]
fn reconciliation_fences_refresh_publication_and_recovers_after_cancellation() {
let codex_home = TempDir::new().unwrap();
+45 -27
View File
@@ -109,6 +109,10 @@ const OPENAI_CURATED_REMOTE_COLLECTION_KEY: &str = "vertical";
const OAI_PRODUCT_SKU_HEADER: &str = "OAI-Product-Sku";
pub(crate) const CODEX_PRODUCT_SKU: &str = "codex";
const REMOTE_PLUGIN_CATALOG_TIMEOUT: Duration = Duration::from_secs(30);
// Leave room within the 120-second mutation-gate wait even when successful pages keep arriving.
const REMOTE_INSTALLED_PLUGINS_FETCH_TIMEOUT: Duration = Duration::from_secs(60);
// Allow 20,000 installed plugins without unbounded cursor history.
const MAX_REMOTE_INSTALLED_PLUGIN_PAGES: usize = 100;
const RECOMMENDED_PLUGINS_TIMEOUT: Duration = Duration::from_secs(5);
const REMOTE_PLUGIN_LIST_PAGE_LIMIT: u32 = 200;
const MAX_RECOMMENDED_PLUGINS: usize = 50;
@@ -482,6 +486,9 @@ pub enum RemotePluginCatalogError {
#[error("{0}")]
UnexpectedResponse(String),
#[error("installed plugin inventory fetch exceeded its overall time limit")]
InstalledPluginsFetchTimeout,
#[error("{0}")]
CacheRemove(String),
}
@@ -511,6 +518,7 @@ impl RemotePluginCatalogError {
| Self::ArchiveTooLarge { .. }
| Self::MissingUploadEtag
| Self::UnexpectedResponse(_)
| Self::InstalledPluginsFetchTimeout
| Self::CacheRemove(_) => None,
}
}
@@ -2177,24 +2185,38 @@ async fn fetch_installed_plugins(
scope: RemoteInstalledPluginScope,
include_download_urls: bool,
) -> Result<Vec<RemotePluginInstalledItem>, RemotePluginCatalogError> {
let mut plugins = Vec::new();
let mut page_token = None;
loop {
let response = get_remote_plugin_installed_page(
config,
auth,
scope,
page_token.as_deref(),
include_download_urls,
)
.await?;
plugins.extend(response.plugins);
let Some(next_page_token) = response.pagination.next_page_token else {
break;
};
page_token = Some(next_page_token);
}
Ok(plugins)
// Only a complete inventory is authoritative: every failure discards all fetched pages
// before callers can publish the snapshot or remove plugins absent from it.
tokio::time::timeout(REMOTE_INSTALLED_PLUGINS_FETCH_TIMEOUT, async {
let mut plugins = Vec::new();
let mut page_token = None;
let mut seen_tokens = HashSet::new();
for _ in 0..MAX_REMOTE_INSTALLED_PLUGIN_PAGES {
let response = get_remote_plugin_installed_page(
config,
auth,
scope,
page_token.as_deref(),
include_download_urls,
)
.await?;
plugins.extend(response.plugins);
let Some(next_page_token) = response.pagination.next_page_token else {
return Ok(plugins);
};
if !seen_tokens.insert(next_page_token.clone()) {
return Err(RemotePluginCatalogError::UnexpectedResponse(
"installed plugin inventory pagination repeated a cursor".to_string(),
));
}
page_token = Some(next_page_token);
}
Err(RemotePluginCatalogError::UnexpectedResponse(
"installed plugin inventory pagination exceeded its page limit".to_string(),
))
})
.await
.map_err(|_| RemotePluginCatalogError::InstalledPluginsFetchTimeout)?
}
async fn get_remote_plugin_list_page(
@@ -2257,16 +2279,12 @@ async fn get_remote_plugin_installed_page(
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)?;
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 let RemoteInstalledPluginScope::Single(scope) = scope {
url.query_pairs_mut()
.append_pair("scope", scope.api_value());
}
url.query_pairs_mut()
.append_pair("limit", &REMOTE_PLUGIN_LIST_PAGE_LIMIT.to_string());
if include_download_urls {
url.query_pairs_mut()
.append_pair("includeDownloadUrls", "true");
+140
View File
@@ -162,6 +162,146 @@ async fn remote_installed_plugins_paginate_across_all_scopes_without_download_ur
);
}
#[tokio::test]
async fn remote_installed_plugins_reject_cyclic_cursors() {
let server = MockServer::start().await;
for (cursor, next_cursor) in [
(None, "cursor-a"),
(Some("cursor-a"), "cursor-b"),
(Some("cursor-b"), "cursor-a"),
] {
let mock = Mock::given(method("GET")).and(path("/backend-api/ps/plugins/installed"));
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},
})))
.up_to_n_times(1)
.expect(1)
.mount(&server)
.await;
}
let (config, _) =
recording_remote_plugin_service_config(format!("{}/backend-api", server.uri()));
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
let error = fetch_remote_installed_plugins(&config, Some(&auth))
.await
.expect_err("a cursor cycle must not produce an installed inventory");
assert!(matches!(
error,
RemotePluginCatalogError::UnexpectedResponse(_)
));
}
#[tokio::test]
async fn remote_installed_plugins_require_a_complete_inventory_within_the_page_limit() {
for complete in [false, true] {
let server = MockServer::start().await;
Mock::given(method("GET"))
.and(path("/backend-api/ps/plugins/installed"))
.respond_with(move |request: &wiremock::Request| {
let page = request
.url
.query_pairs()
.find(|(name, _)| name == "pageToken")
.map_or(0, |(_, value)| value.parse::<usize>().unwrap());
let next_cursor = (!complete || page + 1 < MAX_REMOTE_INSTALLED_PLUGIN_PAGES)
.then(|| (page + 1).to_string());
ResponseTemplate::new(200).set_body_json(serde_json::json!({
"plugins": [],
"pagination": {"next_page_token": next_cursor},
}))
})
.up_to_n_times(MAX_REMOTE_INSTALLED_PLUGIN_PAGES as u64)
.expect(MAX_REMOTE_INSTALLED_PLUGIN_PAGES as u64)
.mount(&server)
.await;
let (config, _) =
recording_remote_plugin_service_config(format!("{}/backend-api", server.uri()));
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
let result = fetch_remote_installed_plugins(&config, Some(&auth)).await;
if complete {
assert_eq!(
result.expect("the last permitted page can complete the inventory"),
Vec::new()
);
} else {
assert!(matches!(
result,
Err(RemotePluginCatalogError::UnexpectedResponse(_))
));
}
}
}
#[tokio::test]
async fn remote_installed_plugins_timeout_cancels_a_late_page_after_successful_pages() {
let server = MockServer::start().await;
let request_started = Arc::new(tokio::sync::Notify::new());
let started = Arc::clone(&request_started);
let (release_response, released) = std::sync::mpsc::channel();
let released = std::sync::Mutex::new(released);
Mock::given(method("GET"))
.and(path("/backend-api/ps/plugins/installed"))
.respond_with(move |request: &wiremock::Request| {
let page = request
.url
.query_pairs()
.find(|(name, _)| name == "pageToken")
.map_or(0, |(_, value)| value.parse::<usize>().unwrap());
started.notify_one();
// Wiremock serves requests on its own runtime. Dropping the sender also releases it.
let _ = released.lock().unwrap().recv();
ResponseTemplate::new(200).set_body_json(serde_json::json!({
"plugins": [],
"pagination": {"next_page_token": (page + 1).to_string()},
}))
})
.up_to_n_times(3)
.expect(3)
.mount(&server)
.await;
let (config, _) =
recording_remote_plugin_service_config(format!("{}/backend-api", server.uri()));
let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing();
let fetch = fetch_remote_installed_plugins(&config, Some(&auth));
tokio::pin!(fetch);
// Each page stays below the individual request timeout, but together exceeds the fetch budget.
// Keep time running during socket I/O so Tokio cannot auto-advance past a response in flight.
for _ in 0..2 {
tokio::select! {
_ = request_started.notified() => {}
result = fetch.as_mut() => panic!("inventory fetch finished before its next page: {result:?}"),
}
tokio::time::pause();
tokio::time::advance(Duration::from_secs(21)).await;
tokio::time::resume();
release_response.send(()).unwrap();
}
tokio::select! {
_ = request_started.notified() => {}
result = fetch.as_mut() => panic!("inventory fetch finished before its late page: {result:?}"),
}
tokio::time::pause();
tokio::time::advance(Duration::from_secs(21)).await;
let result = fetch.await;
tokio::time::resume();
drop(release_response);
assert!(matches!(
result,
Err(RemotePluginCatalogError::InstalledPluginsFetchTimeout)
));
}
#[tokio::test]
async fn remote_catalog_cache_modes_control_refresh_and_persist_fetched_results() {
let server = MockServer::start().await;