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
This commit is contained in:
Dmitriy Kovalenko
2026-09-28 23:32:45 +00:00
committed by copyberry
parent f5430515a8
commit 1260716393
13 changed files with 713 additions and 93 deletions
@@ -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;
@@ -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(())
}
@@ -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
);
}
+7 -4
View File
@@ -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<PluginSkillRoot>,
manifest_cache: &ManifestCache,
) -> Option<PluginCapabilitySummary> {
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 {
+17 -9
View File
@@ -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<MarketplaceListOutcome, MarketplaceError> {
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
@@ -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;
+30 -43
View File
@@ -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<LoadedPluginManifest> {
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<Option<Vec<PathBuf>>> {
@@ -220,15 +190,23 @@ pub(crate) fn load_plugin_command_paths(plugin_root: &Path) -> io::Result<Option
.map(Some)
}
// TODO: share an owner-provided cache if inline fallback manifests are parsed repeatedly.
pub(crate) fn parse_plugin_manifest(
plugin_root: &Path,
manifest_path: &Path,
contents: &str,
) -> Result<PluginManifest, serde_json::Error> {
parse_resolved_plugin_manifest(plugin_root, manifest_path, contents, /*overlay*/ None)
parse_resolved_plugin_manifest(
&ManifestCache::disabled(),
plugin_root,
manifest_path,
contents,
/*overlay*/ None,
)
}
fn parse_resolved_plugin_manifest(
cache: &ManifestCache,
plugin_root: &Path,
manifest_path: &Path,
contents: &str,
@@ -245,13 +223,22 @@ fn parse_resolved_plugin_manifest(
.map_err(serde_json::Error::io)
})
.transpose()?;
parse_resolved_plugin_manifest_uri(
&plugin_root_uri,
&manifest_path_uri,
contents,
overlay.as_ref().map(|(path, contents)| (path, *contents)),
)?
.try_map_resources(|path| path.to_abs_path().map_err(serde_json::Error::io))
cache
.parse(
&plugin_root_uri,
&manifest_path_uri,
contents,
overlay.as_ref().map(|(path, contents)| (path, *contents)),
|| {
parse_resolved_plugin_manifest_uri(
&plugin_root_uri,
&manifest_path_uri,
contents,
overlay.as_ref().map(|(path, contents)| (path, *contents)),
)
},
)?
.try_map_resources(|path| path.to_abs_path().map_err(serde_json::Error::io))
}
pub fn parse_plugin_manifest_uri(
@@ -0,0 +1,181 @@
//! Reuse parsed manifest revisions within an explicitly owned plugin store.
//! Store clones share the cache; independently created stores do not share entries or warnings.
use super::LoadedPluginManifest;
use super::PluginManifestFormat;
use super::UriPluginManifest;
use super::parse_resolved_plugin_manifest;
use codex_utils_path_uri::PathUri;
use codex_utils_plugins::AGENT_PLUGIN_MANIFEST_RELATIVE_PATH;
use codex_utils_plugins::find_plugin_manifest_path;
use sha2::Digest;
use sha2::Sha256;
use std::collections::HashMap;
use std::collections::VecDeque;
use std::fs;
use std::path::Path;
use std::sync::Mutex;
const CAPACITY: usize = 128;
const MAX_CACHABLE_INPUT_LEN: usize = 64 * 1024;
pub(crate) struct ManifestCache {
state: Option<Mutex<ManifestCacheState>>,
}
#[derive(Default)]
struct ManifestCacheState {
entries: HashMap<(PathUri, PathUri), Entry>,
recency: VecDeque<(PathUri, PathUri)>,
}
struct Entry {
contents_digest: [u8; 32],
overlay: Option<(PathUri, [u8; 32])>,
manifest: UriPluginManifest,
}
impl std::fmt::Debug for ManifestCache {
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
formatter
.debug_struct("ManifestCache")
.finish_non_exhaustive()
}
}
impl Default for ManifestCache {
fn default() -> Self {
Self {
state: Some(Mutex::default()),
}
}
}
impl ManifestCache {
pub(crate) fn disabled() -> Self {
Self { state: None }
}
pub(crate) fn load(&self, plugin_root: &Path) -> Option<LoadedPluginManifest> {
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(
self,
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
}
}
}
pub(crate) fn parse(
&self,
root: &PathUri,
path: &PathUri,
contents: &str,
overlay: Option<(&PathUri, &str)>,
parse: impl FnOnce() -> Result<UriPluginManifest, serde_json::Error>,
) -> Result<UriPluginManifest, serde_json::Error> {
let Some(state) = &self.state else {
return parse();
};
if contents
.len()
.saturating_add(overlay.map_or(0, |(_, value)| value.len()))
> MAX_CACHABLE_INPUT_LEN
{
state
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.remove(&(root.clone(), path.clone()));
// Unbounded inputs must not serialize unrelated parses behind the cache lock.
return parse();
}
// Parsing is synchronous and does no filesystem I/O. Serializing cache misses also
// prevents concurrent capability loaders from validating the same revision twice.
state
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.parse(root, path, contents, overlay, parse)
}
}
impl ManifestCacheState {
fn remove(&mut self, key: &(PathUri, PathUri)) -> Option<Entry> {
let entry = self.entries.remove(key)?;
self.recency.retain(|existing| existing != key);
Some(entry)
}
fn parse(
&mut self,
root: &PathUri,
path: &PathUri,
contents: &str,
overlay: Option<(&PathUri, &str)>,
parse: impl FnOnce() -> Result<UriPluginManifest, serde_json::Error>,
) -> Result<UriPluginManifest, serde_json::Error> {
let key = (root.clone(), path.clone());
let previous = self.remove(&key);
let contents_digest = Sha256::digest(contents.as_bytes()).into();
let overlay = overlay
.map(|(path, contents)| (path.clone(), Sha256::digest(contents.as_bytes()).into()));
if let Some(entry) = previous
&& entry.contents_digest == contents_digest
&& entry.overlay == overlay
{
let manifest = entry.manifest.clone();
self.recency.push_front(key.clone());
self.entries.insert(key, entry);
return Ok(manifest);
}
let manifest = parse()?;
if self.entries.len() == CAPACITY
&& let Some(oldest) = self.recency.pop_back()
{
self.entries.remove(&oldest);
}
self.recency.push_front(key.clone());
self.entries.insert(
key,
Entry {
contents_digest,
overlay,
manifest: manifest.clone(),
},
);
Ok(manifest)
}
}
#[cfg(test)]
#[path = "manifest_cache_tests.rs"]
mod tests;
@@ -0,0 +1,303 @@
//! Verify parse reuse, revision invalidation, and diagnostic preservation.
use super::super::parse_legacy_plugin_manifest_uri;
use super::super::parse_resolved_plugin_manifest;
use super::ManifestCache;
use codex_utils_path_uri::PathUri;
use pretty_assertions::assert_eq;
use std::cell::Cell;
use std::sync::Mutex;
use tracing_test::internal::MockWriter;
#[test]
fn repeated_revision_runs_parser_once_and_changes_run_it_again() {
let directory = tempfile::tempdir().unwrap();
let root = PathUri::from_host_native_path(directory.path()).unwrap();
let path = root.join(".codex-plugin/plugin.json").unwrap();
let first = r#"{"name":"demo","description":"first"}"#;
let changed = r#"{"name":"demo","description":"changed"}"#;
let parses = Cell::new(0);
let cache = ManifestCache::default();
let mut results = Vec::new();
for contents in std::iter::repeat_n(first, 220).chain([changed, first]) {
results.push(
cache
.parse(&root, &path, contents, /*overlay*/ None, || {
parses.set(parses.get() + 1);
parse_legacy_plugin_manifest_uri(&root, &path, contents)
})
.unwrap(),
);
}
let original = results[0].clone();
let mut revised = original.clone();
revised.description = Some("changed".to_string());
assert_eq!(
results,
std::iter::repeat_n(original.clone(), 220)
.chain([revised, original])
.collect::<Vec<_>>()
);
assert_eq!(parses.get(), 3);
}
#[test]
fn invalid_fields_warn_once_until_the_manifest_changes() {
let directory = tempfile::tempdir().unwrap();
let path = directory.path().join(".codex-plugin/plugin.json");
let buffer: &'static Mutex<Vec<u8>> = Box::leak(Box::new(Mutex::new(Vec::new())));
let subscriber = tracing_subscriber::fmt()
.without_time()
.with_ansi(/*ansi*/ false)
.with_writer(MockWriter::new(buffer))
.finish();
let _guard = tracing::subscriber::set_default(subscriber);
let original =
r#"{"name":"demo","interface":{"defaultPrompt":" ","composerIcon":"../outside.svg"}}"#;
let changed = r#"{"name":"demo","description":"changed","interface":{"defaultPrompt":" ","composerIcon":"../outside.svg"}}"#;
let cache = ManifestCache::default();
let parse = |contents| {
parse_resolved_plugin_manifest(
&cache,
directory.path(),
&path,
contents,
/*overlay*/ None,
)
.unwrap()
};
let first = parse(original);
for _ in 0..220 {
assert_eq!(parse(original), first);
}
let mut expected = first.clone();
expected.description = Some("changed".to_string());
assert_eq!(parse(changed), expected);
assert_eq!(parse(original), first);
let logs = String::from_utf8(buffer.lock().unwrap().clone()).unwrap();
assert_eq!(logs.matches("ignoring interface.defaultPrompt").count(), 3);
assert_eq!(logs.matches("ignoring interface.composerIcon").count(), 3);
}
#[test]
fn overlay_revision_and_resource_root_are_part_of_cached_parse() {
let directory = tempfile::tempdir().unwrap();
let root = PathUri::from_host_native_path(directory.path()).unwrap();
let other_root = root.join("other").unwrap();
let path = root.join("plugin.json").unwrap();
let overlay_path = root.join(".codex-plugin/plugin.json").unwrap();
let contents = serde_json::json!({
"$schema": codex_utils_plugins::AGENT_PLUGIN_SCHEMA_URI,
"name": "demo"
})
.to_string();
let first = r#"{"interface":{"defaultPrompt":"first"}}"#;
let changed = r#"{"interface":{"defaultPrompt":"changed"}}"#;
let parses = Cell::new(0);
let cache = ManifestCache::default();
for (plugin_root, overlay) in [
(&root, Some((&overlay_path, first))),
(&root, Some((&overlay_path, first))),
(&root, Some((&overlay_path, changed))),
(&root, None),
(&other_root, None),
] {
let manifest = cache
.parse(plugin_root, &path, &contents, overlay, || {
parses.set(parses.get() + 1);
super::super::agent_plugin_manifest::parse_agent_plugin_manifest_uri(
plugin_root,
&path,
&contents,
overlay,
)
})
.unwrap();
let expected = super::super::agent_plugin_manifest::parse_agent_plugin_manifest_uri(
plugin_root,
&path,
&contents,
overlay,
)
.unwrap();
assert_eq!(manifest, expected);
}
assert_eq!(parses.get(), 4);
}
#[test]
fn failed_revision_is_retried_and_repair_is_visible() {
let directory = tempfile::tempdir().unwrap();
let root = PathUri::from_host_native_path(directory.path()).unwrap();
let path = root.join(".codex-plugin/plugin.json").unwrap();
let cache = ManifestCache::default();
for contents in ["{", "{"] {
assert!(
cache
.parse(&root, &path, contents, /*overlay*/ None, || {
parse_legacy_plugin_manifest_uri(&root, &path, contents)
})
.is_err()
);
}
let repaired = r#"{"name":"repaired"}"#;
assert_eq!(
cache
.parse(&root, &path, repaired, /*overlay*/ None, || {
parse_legacy_plugin_manifest_uri(&root, &path, repaired)
})
.unwrap(),
parse_legacy_plugin_manifest_uri(&root, &path, repaired).unwrap()
);
}
#[test]
fn equivalent_windows_paths_share_a_cached_revision() {
let cache = ManifestCache::default();
let parses = Cell::new(0);
let contents = r#"{"name":"demo"}"#;
let mut manifests = Vec::new();
for value in ["file:///C:/plugins/Demo", "file:///C:/plugins/demo"] {
let root = PathUri::parse(value).unwrap();
let path = root.join(".codex-plugin/plugin.json").unwrap();
manifests.push(
cache
.parse(&root, &path, contents, /*overlay*/ None, || {
parses.set(parses.get() + 1);
parse_legacy_plugin_manifest_uri(&root, &path, contents)
})
.unwrap(),
);
}
assert_eq!(manifests[0], manifests[1]);
assert_eq!(parses.get(), 1);
}
#[test]
fn oversized_revision_invalidates_previous_result_without_being_cached() {
let directory = tempfile::tempdir().unwrap();
let root = PathUri::from_host_native_path(directory.path()).unwrap();
let path = root.join(".codex-plugin/plugin.json").unwrap();
let small = r#"{"name":"demo"}"#;
let large = format!("{}{small}", " ".repeat(super::MAX_CACHABLE_INPUT_LEN));
let parses = Cell::new(0);
let cache = ManifestCache::default();
for contents in [small, small, &large, &large, small] {
cache
.parse(&root, &path, contents, /*overlay*/ None, || {
parses.set(parses.get() + 1);
parse_legacy_plugin_manifest_uri(&root, &path, contents)
})
.unwrap();
}
assert_eq!(parses.get(), 4);
}
#[test]
fn oversized_parse_does_not_block_unrelated_manifest() {
use std::sync::mpsc;
use std::time::Duration;
let directory = tempfile::tempdir().unwrap();
let root = PathUri::from_host_native_path(directory.path()).unwrap();
let large_path = root.join("large.json").unwrap();
let small_path = root.join("small.json").unwrap();
let large = format!("{}{{}}", " ".repeat(super::MAX_CACHABLE_INPUT_LEN));
let (started_tx, started_rx) = mpsc::channel();
let (release_tx, release_rx) = mpsc::channel();
let (completed_tx, completed_rx) = mpsc::channel();
let root = &root;
let large_path = &large_path;
let small_path = &small_path;
let large = &large;
let cache = &ManifestCache::default();
std::thread::scope(|scope| {
scope.spawn(move || {
cache
.parse(root, large_path, large, /*overlay*/ None, || {
started_tx.send(()).unwrap();
release_rx.recv().unwrap();
parse_legacy_plugin_manifest_uri(root, large_path, large)
})
.unwrap();
});
started_rx.recv_timeout(Duration::from_secs(10)).unwrap();
scope.spawn(move || {
cache
.parse(root, small_path, "{}", /*overlay*/ None, || {
parse_legacy_plugin_manifest_uri(root, small_path, "{}")
})
.unwrap();
completed_tx.send(()).unwrap();
});
let completed = completed_rx.recv_timeout(Duration::from_secs(10));
release_tx.send(()).unwrap();
assert_eq!(completed, Ok(()));
});
}
#[test]
fn store_clones_reuse_validation_but_independent_stores_warn_again() {
use crate::store::PluginStore;
let directory = tempfile::tempdir().unwrap();
let root = directory.path().join("plugin");
std::fs::create_dir_all(root.join(".codex-plugin")).unwrap();
std::fs::write(
root.join(".codex-plugin/plugin.json"),
r#"{"name":"demo","interface":{"defaultPrompt":" "}}"#,
)
.unwrap();
let buffer: &'static Mutex<Vec<u8>> = Box::leak(Box::new(Mutex::new(Vec::new())));
let subscriber = tracing_subscriber::fmt()
.without_time()
.with_ansi(/*ansi*/ false)
.with_writer(MockWriter::new(buffer))
.finish();
let _guard = tracing::subscriber::set_default(subscriber);
let store = PluginStore::new(directory.path().to_path_buf());
let cloned_store = store.clone();
let first = store.manifest_cache.load(&root).unwrap().manifest;
for _ in 0..220 {
assert_eq!(
cloned_store.manifest_cache.load(&root).unwrap().manifest,
first
);
}
let independent_store = PluginStore::new(directory.path().to_path_buf());
assert_eq!(
independent_store
.manifest_cache
.load(&root)
.unwrap()
.manifest,
first
);
let logs = String::from_utf8(buffer.lock().unwrap().clone()).unwrap();
assert_eq!(
logs.matches("ignoring interface.defaultPrompt: prompt must not be empty")
.count(),
2
);
}
#[test]
fn disabled_cache_parses_each_load() {
let directory = tempfile::tempdir().unwrap();
let root = PathUri::from_host_native_path(directory.path()).unwrap();
let path = root.join(".codex-plugin/plugin.json").unwrap();
let contents = r#"{"name":"demo"}"#;
let expected = parse_legacy_plugin_manifest_uri(&root, &path, contents).unwrap();
let cache = ManifestCache::disabled();
let parses = Cell::new(0);
for _ in 0..3 {
let result = cache
.parse(&root, &path, contents, /*overlay*/ None, || {
parses.set(parses.get() + 1);
parse_legacy_plugin_manifest_uri(&root, &path, contents)
})
.unwrap();
assert_eq!(result, expected);
}
assert_eq!(parses.get(), 3);
}
+46 -20
View File
@@ -1,5 +1,5 @@
use crate::manifest::ManifestCache;
use crate::manifest::PluginManifestInterface;
use crate::manifest::load_plugin_manifest;
use codex_app_server_protocol::PluginAuthPolicy;
use codex_app_server_protocol::PluginInstallPolicy;
use codex_git_utils::get_git_repo_root;
@@ -244,6 +244,7 @@ impl MarketplaceError {
}
}
// TODO: thread an owner-provided manifest cache through standalone install/read workflows.
pub fn find_marketplace_plugin(
marketplace_path: &AbsolutePathBuf,
plugin_name: &str,
@@ -256,9 +257,12 @@ pub fn find_marketplace_plugin(
continue;
}
if let Some(plugin) =
resolve_marketplace_plugin_entry(marketplace_path, &marketplace_name, plugin)?
{
if let Some(plugin) = resolve_marketplace_plugin_entry(
marketplace_path,
&marketplace_name,
plugin,
&ManifestCache::disabled(),
)? {
return Ok(plugin);
}
}
@@ -368,25 +372,35 @@ fn marketplace_root_from_layout(marketplace_path: &Path, relative_path: &str) ->
Some(current.to_path_buf())
}
// TODO: allow standalone callers to share a cache across marketplace loads.
pub fn load_marketplace(path: &AbsolutePathBuf) -> Result<Marketplace, MarketplaceError> {
load_marketplace_with_cache(path, &ManifestCache::disabled())
}
fn load_marketplace_with_cache(
path: &AbsolutePathBuf,
manifest_cache: &ManifestCache,
) -> Result<Marketplace, MarketplaceError> {
let marketplace = load_raw_marketplace_manifest(path)?;
let mut plugins = Vec::new();
for plugin in marketplace.plugins {
let plugin = match resolve_marketplace_plugin_entry(path, &marketplace.name, plugin) {
Ok(Some(plugin)) => plugin,
Ok(None) => continue,
Err(MarketplaceError::InvalidPlugin(message)) => {
warn!(
path = %path.display(),
marketplace = %marketplace.name,
error = %message,
"skipping invalid marketplace plugin"
);
continue;
}
Err(err) => return Err(err),
};
let plugin =
match resolve_marketplace_plugin_entry(path, &marketplace.name, plugin, manifest_cache)
{
Ok(Some(plugin)) => plugin,
Ok(None) => continue,
Err(MarketplaceError::InvalidPlugin(message)) => {
warn!(
path = %path.display(),
marketplace = %marketplace.name,
error = %message,
"skipping invalid marketplace plugin"
);
continue;
}
Err(err) => return Err(err),
};
let manifest_fallback = plugin
.manifest_fallback
@@ -420,15 +434,24 @@ pub fn load_marketplace(path: &AbsolutePathBuf) -> Result<Marketplace, Marketpla
})
}
// TODO: pass an owner-provided cache from remaining callers; the manager already does.
#[doc(hidden)]
pub fn list_marketplaces_with_home(
additional_roots: &[AbsolutePathBuf],
home_dir: Option<&Path>,
) -> Result<MarketplaceListOutcome, MarketplaceError> {
list_marketplaces_with_cache(additional_roots, home_dir, &ManifestCache::disabled())
}
pub(crate) fn list_marketplaces_with_cache(
additional_roots: &[AbsolutePathBuf],
home_dir: Option<&Path>,
manifest_cache: &ManifestCache,
) -> Result<MarketplaceListOutcome, MarketplaceError> {
let mut outcome = MarketplaceListOutcome::default();
for marketplace_path in discover_marketplace_paths_from_roots(additional_roots, home_dir) {
match load_marketplace(&marketplace_path) {
match load_marketplace_with_cache(&marketplace_path, manifest_cache) {
Ok(marketplace) => outcome.marketplaces.push(marketplace),
Err(err) => {
warn!(
@@ -508,6 +531,7 @@ fn resolve_marketplace_plugin_entry(
marketplace_path: &AbsolutePathBuf,
marketplace_name: &str,
plugin: RawMarketplaceManifestPlugin,
manifest_cache: &ManifestCache,
) -> Result<Option<ResolvedMarketplacePlugin>, MarketplaceError> {
let RawMarketplaceManifestPlugin {
name,
@@ -525,7 +549,9 @@ fn resolve_marketplace_plugin_entry(
let manifest = match &source {
MarketplacePluginSource::Local { path } => {
if codex_utils_plugins::find_plugin_manifest_path(path.as_path()).is_some() {
load_plugin_manifest(path.as_path())
manifest_cache
.load(path.as_path())
.map(|loaded| loaded.manifest)
} else if manifest_fallback.has_metadata {
manifest_fallback.parse_for_plugin_root(path.as_path())
} else {
@@ -8,7 +8,6 @@ use crate::loader::load_plugin_hooks;
use crate::loader::load_plugin_mcp_servers_from_manifest_with_format;
use crate::loader::plugin_skill_roots;
use crate::manifest::PluginManifestFormat;
use crate::manifest::load_plugin_manifest_with_format;
use crate::store::PluginStore;
use codex_hooks::plugin_hook_declarations;
use codex_plugin::PluginId;
@@ -31,7 +30,7 @@ impl RemotePluginCapabilities {
let Some(root) = store.active_plugin_root(plugin_id) else {
return;
};
let Some(loaded) = load_plugin_manifest_with_format(root.as_path()) else {
let Some(loaded) = store.manifest_cache.load(root.as_path()) else {
return;
};
let paths = &loaded.manifest.paths;
+4
View File
@@ -1,4 +1,5 @@
use crate::command_migration::migrate_plugin_commands;
use crate::manifest::ManifestCache;
use crate::manifest::PluginManifest;
use crate::manifest::PluginManifestFormat;
use crate::manifest::load_plugin_manifest;
@@ -19,6 +20,7 @@ use std::io;
use std::io::Write;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
pub use codex_core_plugin_common::installed::DEFAULT_PLUGIN_VERSION;
pub use codex_core_plugin_common::installed::PLUGINS_CACHE_DIR;
@@ -39,6 +41,7 @@ pub struct PluginInstallResult {
#[derive(Debug, Clone)]
pub struct PluginStore {
pub(crate) manifest_cache: Arc<ManifestCache>,
codex_home: AbsolutePathBuf,
root: AbsolutePathBuf,
data_root: AbsolutePathBuf,
@@ -91,6 +94,7 @@ impl PluginStore {
.map_err(|err| PluginStoreError::io("failed to resolve Codex home", err))?;
Ok(Self {
manifest_cache: Arc::new(ManifestCache::default()),
codex_home,
root,
data_root,
@@ -18,13 +18,13 @@ use tokio::sync::Semaphore;
use crate::app_mcp_routing::apply_app_mcp_routing_policy;
use crate::loader::PluginSkillInventory;
use crate::loader::load_plugin_apps;
use crate::loader::load_plugin_mcp_servers;
use crate::loader::load_plugin_apps_from_manifest;
use crate::loader::load_plugin_mcp_servers_from_manifest_with_format;
use crate::loader::load_plugin_skill_inventory;
use crate::manager::ConfiguredMarketplacePlugin;
use crate::manager::remote_plugin_install_required_description;
use crate::manifest::ManifestCache;
use crate::manifest::PluginManifestFormat;
use crate::manifest::load_plugin_manifest_with_format;
use crate::marketplace::MarketplaceError;
use crate::marketplace::MarketplacePluginSource;
@@ -37,6 +37,7 @@ type ToolSuggestMetadataEntry = Result<Arc<ToolSuggestMetadataFragment>, String>
/// `PluginsManager` clears these entries alongside its loaded-plugin cache. Current skill config
/// and auth routing are projected after each lookup and are not part of this cache.
pub(crate) struct ToolSuggestMetadataCache {
manifest_cache: Arc<ManifestCache>,
state: RwLock<ToolSuggestMetadataCacheState>,
load_semaphore: Semaphore,
}
@@ -103,8 +104,9 @@ impl ToolSuggestMetadataFragment {
}
impl ToolSuggestMetadataCache {
pub(crate) fn new() -> Self {
pub(crate) fn new(manifest_cache: Arc<ManifestCache>) -> Self {
Self {
manifest_cache,
state: RwLock::new(ToolSuggestMetadataCacheState::default()),
load_semaphore: Semaphore::new(/*permits*/ 1),
}
@@ -146,6 +148,7 @@ impl ToolSuggestMetadataCache {
let generation = self.generation();
let entry = load_plugin_metadata(
&self.manifest_cache,
marketplace_name,
plugin,
restriction_product,
@@ -196,6 +199,7 @@ impl ToolSuggestMetadataCache {
}
async fn load_plugin_metadata(
manifest_cache: &ManifestCache,
marketplace_name: &str,
plugin: &ConfiguredMarketplacePlugin,
restriction_product: Option<Product>,
@@ -223,7 +227,8 @@ async fn load_plugin_metadata(
if !plugin_root.as_path().is_dir() {
return Err("path does not exist or is not a directory".to_string());
}
let loaded_manifest = load_plugin_manifest_with_format(plugin_root.as_path())
let loaded_manifest = manifest_cache
.load(plugin_root.as_path())
.ok_or_else(|| "missing or invalid plugin.json".to_string())?;
let plugin_identity = PluginIdentity {
plugin_id: plugin_id.as_key(),
@@ -240,17 +245,22 @@ async fn load_plugin_metadata(
skill_root_loader,
)
.await;
let mut mcp_server_names =
load_plugin_mcp_servers(plugin_root.as_path(), /*auth_mode*/ None)
.await
.into_keys()
.collect::<Vec<_>>();
let mut mcp_server_names = load_plugin_mcp_servers_from_manifest_with_format(
plugin_root.as_path(),
&manifest.paths,
/*plugin_policy*/ None,
/*plugin_data_root*/ None,
loaded_manifest.format,
)
.await
.into_keys()
.collect::<Vec<_>>();
mcp_server_names.sort_unstable();
mcp_server_names.dedup();
let app_declarations = if loaded_manifest.format == PluginManifestFormat::AgentPlugin {
Vec::new()
} else {
load_plugin_apps(plugin_root.as_path()).await
load_plugin_apps_from_manifest(plugin_root.as_path(), &manifest.paths).await
};
Ok(Arc::new(ToolSuggestMetadataFragment {