From 12607163938054d7e90fa1d2a1c63772dece6689 Mon Sep 17 00:00:00 2001 From: Dmitriy Kovalenko Date: Mon, 28 Sep 2026 22:49:03 +0000 Subject: [PATCH] Cache parsed plugin manifests across plugin workflows (#49099) ## Why Repeated plugin discovery and loading reparse unchanged manifests and repeat warnings for invalid fields. ## What changed - Share a manifest cache through `PluginStore` across marketplace listing, plugin loading, capability inspection, and tool suggestion metadata. - Reuse successful parses by plugin root, manifest path, and content digests, including the Codex overlay. Reread files on each load so manifest edits and overlay changes or removal remain visible. - Bound the cache to 128 entries and bypass caching for inputs larger than 64 KiB. Retry failed parses and share entries across store clones while keeping independent stores isolated. - Reuse loaded manifest data when loading apps and MCP metadata. ## Testing Add unit coverage for parse reuse, warning deduplication, revision invalidation, repair after parse failures, equivalent Windows paths, store isolation, and oversized inputs. Add an app-server regression test that verifies repeated `plugin/list` requests observe manifest and overlay revisions in one running server. GitOrigin-RevId: ed082cd7d198015cd8ed35db7f2af74e13f49915 --- codex-rs/app-server/tests/suite/v2/mod.rs | 1 + .../tests/suite/v2/plugin_manifest_cache.rs | 95 ++++++ .../core-plugins/src/discoverable_tests.rs | 8 +- codex-rs/core-plugins/src/loader.rs | 11 +- codex-rs/core-plugins/src/manager.rs | 26 +- codex-rs/core-plugins/src/manager_tests.rs | 3 + codex-rs/core-plugins/src/manifest.rs | 73 ++--- .../src/manifest/manifest_cache.rs | 181 +++++++++++ .../src/manifest/manifest_cache_tests.rs | 303 ++++++++++++++++++ codex-rs/core-plugins/src/marketplace.rs | 66 ++-- .../src/remote/plugin_capabilities.rs | 3 +- codex-rs/core-plugins/src/store.rs | 4 + .../core-plugins/src/tool_suggest_metadata.rs | 32 +- 13 files changed, 713 insertions(+), 93 deletions(-) create mode 100644 codex-rs/app-server/tests/suite/v2/plugin_manifest_cache.rs create mode 100644 codex-rs/core-plugins/src/manifest/manifest_cache.rs create mode 100644 codex-rs/core-plugins/src/manifest/manifest_cache_tests.rs diff --git a/codex-rs/app-server/tests/suite/v2/mod.rs b/codex-rs/app-server/tests/suite/v2/mod.rs index 51f3e06b9a..d248969036 100644 --- a/codex-rs/app-server/tests/suite/v2/mod.rs +++ b/codex-rs/app-server/tests/suite/v2/mod.rs @@ -89,6 +89,7 @@ mod permission_profile_list; mod plan_item; mod plugin_install; mod plugin_list; +mod plugin_manifest_cache; mod plugin_read; mod plugin_reconcile; mod plugin_search; diff --git a/codex-rs/app-server/tests/suite/v2/plugin_manifest_cache.rs b/codex-rs/app-server/tests/suite/v2/plugin_manifest_cache.rs new file mode 100644 index 0000000000..854e8a533c --- /dev/null +++ b/codex-rs/app-server/tests/suite/v2/plugin_manifest_cache.rs @@ -0,0 +1,95 @@ +//! Exercise manifest and overlay invalidation across RPCs in one running app-server. + +use anyhow::Result; +use app_test_support::TestAppServer; +use codex_app_server_protocol::PluginListMarketplaceKind; +use codex_app_server_protocol::PluginListParams; +use codex_app_server_protocol::PluginListResponse; +use codex_utils_absolute_path::AbsolutePathBuf; +use pretty_assertions::assert_eq; +use serde_json::json; +use std::time::Duration; +use tempfile::TempDir; +use tokio::time::timeout; + +#[tokio::test] +async fn plugin_list_observes_manifest_and_overlay_revisions_in_one_server() -> Result<()> { + let codex_home = TempDir::new()?; + let repository = TempDir::new()?; + let root = repository.path(); + for directory in [".git", ".agents/plugins", "sample/.codex-plugin"] { + std::fs::create_dir_all(root.join(directory))?; + } + std::fs::write( + codex_home.path().join("config.toml"), + "[features]\nplugins = true\nremote_plugin = false\n", + )?; + std::fs::write( + root.join(".agents/plugins/marketplace.json"), + serde_json::to_vec(&json!({ + "name": "cache-test", "plugins": [{"name": "sample", "source": {"source": "local", "path": "./sample"}}] + }))?, + )?; + let mut server = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .build_initialized_with_timeout(Duration::from_secs(30)) + .await?; + let overlay_path = root.join("sample/.codex-plugin/plugin.json"); + for (version, overlay_name, expected_name) in [ + ("1.0.0", Some("First"), "First"), + ("2.0.0", Some("First"), "First"), + ("2.0.0", Some("Changed"), "Changed"), + ("2.0.0", None, "sample"), + ] { + std::fs::write( + root.join("sample/plugin.json"), + serde_json::to_vec(&json!({ + "$schema": "https://agent-plugins.org/schemas/1.0.0/plugin.schema.json", + "name": "sample", "version": version + }))?, + )?; + if let Some(display_name) = overlay_name { + std::fs::write( + &overlay_path, + serde_json::to_vec(&json!({"interface": {"displayName": display_name}}))?, + )?; + } else { + std::fs::remove_file(&overlay_path)?; + } + let mut previous = None; + for _ in 0..2 { + let id = server + .send_plugin_list_request(PluginListParams { + cwds: Some(vec![AbsolutePathBuf::try_from(root)?]), + marketplace_kinds: Some(vec![PluginListMarketplaceKind::Local]), + force_refetch: false, + }) + .await?; + let response: PluginListResponse = + timeout(Duration::from_secs(30), server.read_response(id)).await??; + assert_eq!(response.marketplace_load_errors, Vec::new()); + let marketplace = response + .marketplaces + .iter() + .find(|entry| entry.name == "cache-test") + .expect("local marketplace"); + let plugin = &marketplace.plugins[0]; + assert_eq!( + ( + plugin.id.as_str(), + plugin.local_version.as_deref(), + plugin + .interface + .as_ref() + .and_then(|value| value.display_name.as_deref()) + ), + ("sample@cache-test", Some(version), Some(expected_name)) + ); + if let Some(previous) = previous { + assert_eq!(marketplace, &previous); + } + previous = Some(marketplace.clone()); + } + } + Ok(()) +} diff --git a/codex-rs/core-plugins/src/discoverable_tests.rs b/codex-rs/core-plugins/src/discoverable_tests.rs index 3c2085c9e1..df227071fd 100644 --- a/codex-rs/core-plugins/src/discoverable_tests.rs +++ b/codex-rs/core-plugins/src/discoverable_tests.rs @@ -640,7 +640,7 @@ async fn omits_not_available_curated_plugins() { } #[tokio::test] -async fn does_not_reload_marketplace_per_plugin() { +async fn discovery_warns_once_per_invalid_manifest() { let codex_home = tempdir().expect("tempdir should succeed"); let curated_root = curated_plugins_repo_path(codex_home.path()); write_openai_curated_marketplace(&curated_root, &["slack", "gmail", "openai-developers"]); @@ -697,12 +697,12 @@ async fn does_not_reload_marketplace_per_plugin() { let logs = String::from_utf8(buffer.lock().expect("buffer lock").clone()) .expect("utf8 logs") .replace('\\', "/"); - assert_eq!(logs.matches("ignoring interface.defaultPrompt").count(), 8); - assert_eq!(logs.matches("gmail/.codex-plugin/plugin.json").count(), 4); + assert_eq!(logs.matches("ignoring interface.defaultPrompt").count(), 2); + assert_eq!(logs.matches("gmail/.codex-plugin/plugin.json").count(), 1); assert_eq!( logs.matches("openai-developers/.codex-plugin/plugin.json") .count(), - 4 + 1 ); } diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index d9f49eec4b..92500be8cc 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -2,6 +2,7 @@ use crate::PluginGitMode; use crate::app_mcp_routing::apply_app_mcp_routing_policy; use crate::app_mcp_routing::apps_route_available; use crate::is_openai_curated_marketplace_name; +use crate::manifest::ManifestCache; use crate::manifest::PluginManifest; use crate::manifest::PluginManifestFormat; use crate::manifest::PluginManifestHooks; @@ -889,7 +890,7 @@ async fn load_plugin( return loaded_plugin; } - let Some(loaded_manifest) = load_plugin_manifest_with_format(plugin_root.as_path()) else { + let Some(loaded_manifest) = store.manifest_cache.load(plugin_root.as_path()) else { loaded_plugin.error = Some("missing or invalid plugin.json".to_string()); return loaded_plugin; }; @@ -942,7 +943,8 @@ async fn load_plugin( ) .await; if loaded_manifest.format == PluginManifestFormat::Legacy { - loaded_plugin.apps = load_plugin_apps(plugin_root.as_path()).await; + loaded_plugin.apps = + load_plugin_apps_from_manifest(plugin_root.as_path(), manifest_paths).await; } } PluginLoadScope::HooksOnly => {} @@ -1328,12 +1330,13 @@ async fn load_apps_from_paths( app_declarations } -pub async fn plugin_capability_summary_from_root( +pub(crate) async fn plugin_capability_summary_from_root( plugin_id: &PluginId, plugin_root: &AbsolutePathBuf, skill_root_loader: &dyn SkillRootLoader, + manifest_cache: &ManifestCache, ) -> Option { - let loaded_manifest = load_plugin_manifest_with_format(plugin_root.as_path())?; + let loaded_manifest = manifest_cache.load(plugin_root.as_path())?; let manifest_format = loaded_manifest.format; let manifest = loaded_manifest.manifest; let plugin_identity = PluginIdentity { diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 3312b40120..397c17524a 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -38,8 +38,6 @@ use crate::loader::refresh_non_curated_plugin_cache_force_reinstall_detailed; use crate::loader::remote_installed_plugins_to_config; use crate::manifest::PluginManifestFormat; use crate::manifest::PluginManifestInterface; -use crate::manifest::load_plugin_manifest; -use crate::manifest::load_plugin_manifest_with_format; use crate::marketplace::MarketplaceError; use crate::marketplace::MarketplaceInterface; use crate::marketplace::MarketplaceListError; @@ -52,7 +50,7 @@ use crate::marketplace::ResolvedMarketplacePlugin; use crate::marketplace::find_installable_marketplace_plugin; use crate::marketplace::find_marketplace_plugin; use crate::marketplace::home_dir; -use crate::marketplace::list_marketplaces_with_home; +use crate::marketplace::list_marketplaces_with_cache; use crate::marketplace::plugin_interface_with_marketplace_category; use crate::marketplace_policy::MarketplacePolicy; use crate::marketplace_policy::configured_plugins_from_stack; @@ -677,9 +675,12 @@ impl PluginsManager { // This assumes a single CODEX_HOME is only used by one product. let remote_installed_plugin_bundle_sync_gate = crate::remote::remote_installed_plugin_bundle_sync_gate(&codex_home); + let store = PluginStore::new(codex_home.clone()); + let tool_suggest_metadata_cache = + ToolSuggestMetadataCache::new(Arc::clone(&store.manifest_cache)); Self { - codex_home: codex_home.clone(), - store: PluginStore::new(codex_home), + codex_home, + store, featured_plugin_ids_cache: RwLock::new(None), recommended_plugins_cache: RwLock::new(HashMap::new()), recommended_plugins_refreshes: RwLock::new(HashMap::new()), @@ -695,7 +696,7 @@ impl PluginsManager { loaded_plugins_cache: Mutex::new(LoadedPluginsCache::default()), loaded_plugins_load_semaphore: Semaphore::new(/*permits*/ 1), skill_root_loader, - tool_suggest_metadata_cache: ToolSuggestMetadataCache::new(), + tool_suggest_metadata_cache, remote_installed_plugins_cache: RwLock::new(RemoteInstalledPluginsCache::default()), remote_installed_plugin_bundle_sync_gate, remote_installed_plugins_cache_refresh_state: RwLock::new( @@ -1147,6 +1148,7 @@ impl PluginsManager { plugin_id, &plugin_root, self.skill_root_loader.as_ref(), + &self.store.manifest_cache, ) .await } @@ -1168,6 +1170,7 @@ impl PluginsManager { plugin_id, &plugin_root, self.skill_root_loader.as_ref(), + &self.store.manifest_cache, ) .await } @@ -2425,7 +2428,11 @@ impl PluginsManager { && plugin.source.is_install_materialized() && let Some(plugin_id) = plugin_id.as_ref() && let Some(plugin_root) = self.store.active_plugin_root(plugin_id) - && let Some(manifest) = load_plugin_manifest(plugin_root.as_path()) + && let Some(manifest) = self + .store + .manifest_cache + .load(plugin_root.as_path()) + .map(|loaded| loaded.manifest) { local_version = manifest.version.clone(); let marketplace_category = interface @@ -2656,7 +2663,7 @@ impl PluginsManager { } let loaded_manifest = if codex_utils_plugins::find_plugin_manifest_path(source_path.as_path()).is_some() { - load_plugin_manifest_with_format(source_path.as_path()) + self.store.manifest_cache.load(source_path.as_path()) } else { plugin .manifest_fallback @@ -3600,7 +3607,8 @@ impl PluginsManager { config: &PluginsConfigInput, roots: &[AbsolutePathBuf], ) -> Result { - let mut outcome = list_marketplaces_with_home(roots, home_dir().as_deref())?; + let mut outcome = + list_marketplaces_with_cache(roots, home_dir().as_deref(), &self.store.manifest_cache)?; let policy = MarketplacePolicy::from_requirements(config.config_layer_stack.requirements()); outcome.marketplaces.retain(|marketplace| { policy diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index d331e29396..2ae390470d 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -8,6 +8,7 @@ use crate::ToolSuggestPluginDiscoveryInput; use crate::installed_marketplaces::marketplace_install_root; use crate::loader::load_plugin_skill_inventory; use crate::loader::load_plugins_from_layer_stack; +use crate::loader::plugin_capability_summary_from_root; use crate::loader::refresh_non_curated_plugin_cache; use crate::loader::refresh_non_curated_plugin_cache_force_reinstall; use crate::marketplace::MarketplacePluginInstallPolicy; @@ -1925,6 +1926,7 @@ async fn plugin_telemetry_metadata_uses_default_mcp_config_path() { &PluginId::parse("sample@test").expect("plugin id should parse"), &plugin_root.abs(), test_skill_root_loader().as_ref(), + &crate::manifest::ManifestCache::disabled(), ) .await; @@ -1968,6 +1970,7 @@ async fn plugin_capability_summary_uses_manifest_mcp_server_objects() { &PluginId::parse("counter-sample@test").expect("plugin id should parse"), &plugin_root.abs(), test_skill_root_loader().as_ref(), + &crate::manifest::ManifestCache::disabled(), ) .await; diff --git a/codex-rs/core-plugins/src/manifest.rs b/codex-rs/core-plugins/src/manifest.rs index 9991031f60..a532660dde 100644 --- a/codex-rs/core-plugins/src/manifest.rs +++ b/codex-rs/core-plugins/src/manifest.rs @@ -10,6 +10,9 @@ use std::fs; use std::io; use std::path::Path; use std::path::PathBuf; + +mod manifest_cache; +pub(crate) use manifest_cache::ManifestCache; const MAX_DEFAULT_PROMPT_COUNT: usize = 3; const MAX_DEFAULT_PROMPT_LEN: usize = 128; @@ -165,41 +168,8 @@ pub fn is_agent_plugin_manifest(plugin_root: &Path) -> bool { } pub(crate) fn load_plugin_manifest_with_format(plugin_root: &Path) -> Option { - let manifest_path = find_plugin_manifest_path(plugin_root)?; - let contents = fs::read_to_string(&manifest_path).ok()?; - let is_agent_plugin = manifest_path == plugin_root.join(AGENT_PLUGIN_MANIFEST_RELATIVE_PATH); - let overlay = if is_agent_plugin { - let overlay_path = plugin_root.join(".codex-plugin/plugin.json"); - fs::read_to_string(&overlay_path) - .ok() - .map(|contents| (overlay_path, contents)) - } else { - None - }; - match parse_resolved_plugin_manifest( - plugin_root, - &manifest_path, - &contents, - overlay - .as_ref() - .map(|(path, contents)| (path.as_path(), contents.as_str())), - ) { - Ok(manifest) => Some(LoadedPluginManifest { - manifest, - format: if is_agent_plugin { - PluginManifestFormat::AgentPlugin - } else { - PluginManifestFormat::Legacy - }, - }), - Err(err) => { - tracing::warn!( - path = %manifest_path.display(), - "failed to parse plugin manifest: {err}" - ); - None - } - } + // TODO: accept an owner-provided cache when these standalone loaders join a repeated workflow. + ManifestCache::disabled().load(plugin_root) } pub(crate) fn load_plugin_command_paths(plugin_root: &Path) -> io::Result>> { @@ -220,15 +190,23 @@ pub(crate) fn load_plugin_command_paths(plugin_root: &Path) -> io::Result