mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[codex] Cache plugin metadata for tool suggestions (#27812)
## Why `built_tools` runs for every sampling request, and local plugin discovery was repeatedly rereading plugin manifests, skills, MCP configuration, and app declarations to build the same tool-suggest metadata. That source-derived metadata is stable until the existing plugin manager reloads its cache. Runtime eligibility still needs to reflect the current install, disable, policy, app-overlap, and authentication state. ## What changed - Add a bounded, in-memory tool-suggest metadata cache owned by `PluginsManager`. - Key cached metadata by plugin identity and source, while applying authentication routing each time the metadata is projected. - Invalidate the metadata alongside the existing loaded-plugin cache, including its normal configuration, marketplace refresh, and remote-installed-plugin invalidation paths. - Guard against an in-flight load repopulating stale metadata after invalidation. - Keep marketplace membership and all runtime eligibility filtering live rather than introducing a separate catalog or revision model. ## Impact Repeated sampling requests reuse already-loaded plugin capability metadata while retaining the existing plugin-manager lifecycle as the single freshness boundary. ## Validation - `just test -p codex-core-plugins` — 252 passed - Added focused coverage for cache invalidation and authentication reprojection.
This commit is contained in:
committed by
GitHub
Unverified
parent
752ed90d78
commit
a52a3b5197
@@ -1,8 +1,8 @@
|
||||
use anyhow::Context;
|
||||
use codex_app_server_protocol::PluginAvailability;
|
||||
use codex_app_server_protocol::PluginInstallPolicy;
|
||||
use codex_core_skills::config_rules::skill_config_rules_from_stack;
|
||||
use codex_login::CodexAuth;
|
||||
use codex_plugin::PluginCapabilitySummary;
|
||||
use codex_plugin::PluginId;
|
||||
use std::collections::HashSet;
|
||||
use tracing::warn;
|
||||
@@ -93,6 +93,7 @@ impl PluginsManager {
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let skill_config_rules = skill_config_rules_from_stack(&input.plugins.config_layer_stack);
|
||||
|
||||
let mut discoverable_plugins = Vec::<ToolSuggestDiscoverablePlugin>::new();
|
||||
for marketplace in marketplaces {
|
||||
@@ -110,17 +111,15 @@ impl PluginsManager {
|
||||
}
|
||||
|
||||
let plugin_id = plugin.id.clone();
|
||||
|
||||
match self
|
||||
.read_plugin_detail_for_marketplace_plugin(
|
||||
&input.plugins,
|
||||
.tool_suggest_metadata_for_marketplace_plugin(
|
||||
&marketplace_name,
|
||||
plugin,
|
||||
&plugin,
|
||||
&skill_config_rules,
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(plugin) => {
|
||||
let plugin: PluginCapabilitySummary = plugin.into();
|
||||
discoverable_plugins.push(ToolSuggestDiscoverablePlugin {
|
||||
id: plugin.config_name,
|
||||
remote_plugin_id: None,
|
||||
|
||||
@@ -208,7 +208,7 @@ async fn includes_openai_curated_when_remote_enabled_without_auth() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn deduplicates_configured_marketplace_plugin() {
|
||||
async fn deduplicates_and_reprojects_cached_configured_marketplace_plugin() {
|
||||
let codex_home = tempdir().expect("tempdir should succeed");
|
||||
let plugin_name = "sample";
|
||||
let marketplace_name = OPENAI_BUNDLED_MARKETPLACE_NAME;
|
||||
@@ -229,6 +229,12 @@ async fn deduplicates_configured_marketplace_plugin() {
|
||||
),
|
||||
);
|
||||
write_curated_plugin(&marketplace_root, plugin_name);
|
||||
write_plugin_app(
|
||||
&marketplace_root,
|
||||
plugin_name,
|
||||
"sample-docs",
|
||||
"connector_sample",
|
||||
);
|
||||
write_file(
|
||||
&codex_home.path().join(CONFIG_TOML_FILE),
|
||||
&format!(
|
||||
@@ -241,18 +247,179 @@ source = "/tmp/{marketplace_name}"
|
||||
"#
|
||||
),
|
||||
);
|
||||
|
||||
let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await;
|
||||
let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf());
|
||||
let discoverable_plugins = list_discoverable_plugins(
|
||||
assert!(plugins_manager.set_auth_mode(Some(AuthMode::Chatgpt)));
|
||||
let chatgpt_projection = list_discoverable_plugins(
|
||||
&plugins_manager,
|
||||
discovery_input(plugins.clone(), &[plugin_id.as_str()], &[], &[]),
|
||||
/*auth*/ None,
|
||||
)
|
||||
.await;
|
||||
let expected = ToolSuggestDiscoverablePlugin {
|
||||
id: plugin_id.clone(),
|
||||
remote_plugin_id: None,
|
||||
name: "sample".to_string(),
|
||||
description: Some(
|
||||
"Plugin that includes skills, MCP servers, and app connectors".to_string(),
|
||||
),
|
||||
has_skills: true,
|
||||
mcp_server_names: Vec::new(),
|
||||
app_connector_ids: vec!["connector_sample".to_string()],
|
||||
};
|
||||
assert_eq!(chatgpt_projection, vec![expected.clone()]);
|
||||
|
||||
assert!(plugins_manager.set_auth_mode(Some(AuthMode::ApiKey)));
|
||||
let api_key_projection = list_discoverable_plugins(
|
||||
&plugins_manager,
|
||||
discovery_input(plugins, &[plugin_id.as_str()], &[], &[]),
|
||||
/*auth*/ None,
|
||||
)
|
||||
.await;
|
||||
assert_eq!(
|
||||
api_key_projection,
|
||||
vec![ToolSuggestDiscoverablePlugin {
|
||||
mcp_server_names: vec!["sample-docs".to_string()],
|
||||
app_connector_ids: Vec::new(),
|
||||
..expected
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
assert_eq!(discoverable_plugins.len(), 1);
|
||||
assert_eq!(discoverable_plugins[0].id, plugin_id);
|
||||
#[tokio::test]
|
||||
async fn reprojects_cached_skill_availability_for_current_config() {
|
||||
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"]);
|
||||
|
||||
let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await;
|
||||
let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf());
|
||||
let expected = ToolSuggestDiscoverablePlugin {
|
||||
id: "slack@openai-curated".to_string(),
|
||||
remote_plugin_id: None,
|
||||
name: "slack".to_string(),
|
||||
description: Some(
|
||||
"Plugin that includes skills, MCP servers, and app connectors".to_string(),
|
||||
),
|
||||
has_skills: true,
|
||||
mcp_server_names: vec!["sample-docs".to_string()],
|
||||
app_connector_ids: vec!["connector_calendar".to_string()],
|
||||
};
|
||||
let initial = list_discoverable_plugins(
|
||||
&plugins_manager,
|
||||
discovery_input(plugins, &[], &[], &[]),
|
||||
/*auth*/ None,
|
||||
)
|
||||
.await;
|
||||
assert_eq!(initial, vec![expected.clone()]);
|
||||
|
||||
write_file(
|
||||
&codex_home.path().join(CONFIG_TOML_FILE),
|
||||
r#"[[skills.config]]
|
||||
name = "slack:sample"
|
||||
enabled = false
|
||||
"#,
|
||||
);
|
||||
let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await;
|
||||
let after_skill_disabled = list_discoverable_plugins(
|
||||
&plugins_manager,
|
||||
discovery_input(plugins, &[], &[], &[]),
|
||||
/*auth*/ None,
|
||||
)
|
||||
.await;
|
||||
assert_eq!(
|
||||
after_skill_disabled,
|
||||
vec![ToolSuggestDiscoverablePlugin {
|
||||
has_skills: false,
|
||||
..expected
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn does_not_advertise_skills_when_skill_loading_fails() {
|
||||
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"]);
|
||||
write_file(
|
||||
&curated_root.join("plugins/slack/skills/SKILL.md"),
|
||||
"---\nname: bad",
|
||||
);
|
||||
|
||||
let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await;
|
||||
let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf());
|
||||
let discoverable_plugins = list_discoverable_plugins(
|
||||
&plugins_manager,
|
||||
discovery_input(plugins, &[], &[], &[]),
|
||||
/*auth*/ None,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
discoverable_plugins,
|
||||
vec![ToolSuggestDiscoverablePlugin {
|
||||
id: "slack@openai-curated".to_string(),
|
||||
remote_plugin_id: None,
|
||||
name: "slack".to_string(),
|
||||
description: Some(
|
||||
"Plugin that includes skills, MCP servers, and app connectors".to_string(),
|
||||
),
|
||||
has_skills: false,
|
||||
mcp_server_names: vec!["sample-docs".to_string()],
|
||||
app_connector_ids: vec!["connector_calendar".to_string()],
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn clear_cache_invalidates_cached_tool_suggest_metadata() {
|
||||
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"]);
|
||||
let plugin_manifest = curated_root.join("plugins/slack/.codex-plugin/plugin.json");
|
||||
write_file(
|
||||
&plugin_manifest,
|
||||
r#"{
|
||||
"name": "slack",
|
||||
"description": "Before reload"
|
||||
}"#,
|
||||
);
|
||||
|
||||
let plugins = load_plugins_config(codex_home.path(), codex_home.path()).await;
|
||||
let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf());
|
||||
let input = discovery_input(plugins, &[], &[], &[]);
|
||||
let expected_cached = vec![ToolSuggestDiscoverablePlugin {
|
||||
id: "slack@openai-curated".to_string(),
|
||||
remote_plugin_id: None,
|
||||
name: "slack".to_string(),
|
||||
description: Some("Before reload".to_string()),
|
||||
has_skills: true,
|
||||
mcp_server_names: vec!["sample-docs".to_string()],
|
||||
app_connector_ids: vec!["connector_calendar".to_string()],
|
||||
}];
|
||||
let initial = list_discoverable_plugins(&plugins_manager, input.clone(), /*auth*/ None).await;
|
||||
assert_eq!(initial, expected_cached);
|
||||
|
||||
write_file(
|
||||
&plugin_manifest,
|
||||
r#"{
|
||||
"name": "slack",
|
||||
"description": "After reload"
|
||||
}"#,
|
||||
);
|
||||
let before_reload =
|
||||
list_discoverable_plugins(&plugins_manager, input.clone(), /*auth*/ None).await;
|
||||
assert_eq!(before_reload, expected_cached);
|
||||
|
||||
plugins_manager.clear_cache();
|
||||
let after_reload = list_discoverable_plugins(&plugins_manager, input, /*auth*/ None).await;
|
||||
assert_eq!(
|
||||
after_reload,
|
||||
vec![ToolSuggestDiscoverablePlugin {
|
||||
description: Some("After reload".to_string()),
|
||||
..expected_cached[0].clone()
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -18,6 +18,7 @@ pub mod store;
|
||||
#[cfg(test)]
|
||||
mod test_support;
|
||||
pub mod toggles;
|
||||
mod tool_suggest_metadata;
|
||||
|
||||
pub const OPENAI_CURATED_MARKETPLACE_NAME: &str = "openai-curated";
|
||||
pub const OPENAI_API_CURATED_MARKETPLACE_NAME: &str = "openai-api-curated";
|
||||
|
||||
@@ -768,6 +768,29 @@ fn apply_plugin_mcp_server_policy(config: &mut McpServerConfig, policy: &PluginM
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) struct PluginSkillInventory {
|
||||
skills: Vec<SkillMetadata>,
|
||||
had_errors: bool,
|
||||
}
|
||||
|
||||
impl PluginSkillInventory {
|
||||
pub(crate) fn has_enabled_skills(&self, skill_config_rules: &SkillConfigRules) -> bool {
|
||||
contains_enabled_skill(
|
||||
&self.skills,
|
||||
&resolve_disabled_skill_paths(&self.skills, skill_config_rules),
|
||||
)
|
||||
}
|
||||
|
||||
fn resolve(self, skill_config_rules: &SkillConfigRules) -> ResolvedPluginSkills {
|
||||
let disabled_skill_paths = resolve_disabled_skill_paths(&self.skills, skill_config_rules);
|
||||
ResolvedPluginSkills {
|
||||
skills: self.skills,
|
||||
disabled_skill_paths,
|
||||
had_errors: self.had_errors,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct ResolvedPluginSkills {
|
||||
pub skills: Vec<SkillMetadata>,
|
||||
@@ -777,14 +800,19 @@ pub struct ResolvedPluginSkills {
|
||||
|
||||
impl ResolvedPluginSkills {
|
||||
pub fn has_enabled_skills(&self) -> bool {
|
||||
self.had_errors
|
||||
|| self
|
||||
.skills
|
||||
.iter()
|
||||
.any(|skill| !self.disabled_skill_paths.contains(&skill.path_to_skills_md))
|
||||
self.had_errors || contains_enabled_skill(&self.skills, &self.disabled_skill_paths)
|
||||
}
|
||||
}
|
||||
|
||||
fn contains_enabled_skill(
|
||||
skills: &[SkillMetadata],
|
||||
disabled_skill_paths: &HashSet<AbsolutePathBuf>,
|
||||
) -> bool {
|
||||
skills
|
||||
.iter()
|
||||
.any(|skill| !disabled_skill_paths.contains(&skill.path_to_skills_md))
|
||||
}
|
||||
|
||||
pub async fn load_plugin_skills(
|
||||
plugin_root: &AbsolutePathBuf,
|
||||
plugin_id: &PluginId,
|
||||
@@ -792,6 +820,17 @@ pub async fn load_plugin_skills(
|
||||
restriction_product: Option<Product>,
|
||||
skill_config_rules: &SkillConfigRules,
|
||||
) -> ResolvedPluginSkills {
|
||||
load_plugin_skill_inventory(plugin_root, plugin_id, manifest, restriction_product)
|
||||
.await
|
||||
.resolve(skill_config_rules)
|
||||
}
|
||||
|
||||
pub(crate) async fn load_plugin_skill_inventory(
|
||||
plugin_root: &AbsolutePathBuf,
|
||||
plugin_id: &PluginId,
|
||||
manifest: &PluginManifest,
|
||||
restriction_product: Option<Product>,
|
||||
) -> PluginSkillInventory {
|
||||
let roots = plugin_skill_roots(plugin_root, &manifest.paths)
|
||||
.into_iter()
|
||||
.map(|path| SkillRoot {
|
||||
@@ -810,13 +849,8 @@ pub async fn load_plugin_skills(
|
||||
.into_iter()
|
||||
.filter(|skill| skill.matches_product_restriction_for_product(restriction_product))
|
||||
.collect::<Vec<_>>();
|
||||
let disabled_skill_paths = resolve_disabled_skill_paths(&skills, skill_config_rules);
|
||||
|
||||
ResolvedPluginSkills {
|
||||
skills,
|
||||
disabled_skill_paths,
|
||||
had_errors,
|
||||
}
|
||||
PluginSkillInventory { skills, had_errors }
|
||||
}
|
||||
|
||||
fn plugin_skill_roots(
|
||||
|
||||
@@ -51,6 +51,7 @@ use crate::startup_sync::sync_openai_plugins_repo;
|
||||
use crate::store::PluginInstallResult as StorePluginInstallResult;
|
||||
use crate::store::PluginStore;
|
||||
use crate::store::PluginStoreError;
|
||||
use crate::tool_suggest_metadata::ToolSuggestMetadataCache;
|
||||
use codex_analytics::AnalyticsEventsClient;
|
||||
use codex_app_server_protocol::AuthMode;
|
||||
use codex_config::ConfigLayerStack;
|
||||
@@ -354,6 +355,7 @@ pub struct PluginsManager {
|
||||
// Keep the cache auth-independent so auth changes only need to resolve capabilities again.
|
||||
loaded_plugins_cache: RwLock<LoadedPluginsCache>,
|
||||
loaded_plugins_load_semaphore: Semaphore,
|
||||
tool_suggest_metadata_cache: ToolSuggestMetadataCache,
|
||||
remote_installed_plugins_cache: RwLock<Option<Vec<RemoteInstalledPlugin>>>,
|
||||
remote_installed_plugins_cache_refresh_state: RwLock<RemoteInstalledPluginsCacheRefreshState>,
|
||||
global_remote_catalog_cache_refresh_state: RwLock<GlobalRemoteCatalogCacheRefreshState>,
|
||||
@@ -410,6 +412,7 @@ impl PluginsManager {
|
||||
non_curated_cache_refresh_state: RwLock::new(NonCuratedCacheRefreshState::default()),
|
||||
loaded_plugins_cache: RwLock::new(LoadedPluginsCache::default()),
|
||||
loaded_plugins_load_semaphore: Semaphore::new(/*permits*/ 1),
|
||||
tool_suggest_metadata_cache: ToolSuggestMetadataCache::new(),
|
||||
remote_installed_plugins_cache: RwLock::new(None),
|
||||
remote_installed_plugins_cache_refresh_state: RwLock::new(
|
||||
RemoteInstalledPluginsCacheRefreshState::default(),
|
||||
@@ -551,6 +554,7 @@ impl PluginsManager {
|
||||
}
|
||||
|
||||
fn clear_loaded_plugins_cache(&self) {
|
||||
self.tool_suggest_metadata_cache.clear();
|
||||
let mut cache = match self.loaded_plugins_cache.write() {
|
||||
Ok(cache) => cache,
|
||||
Err(err) => err.into_inner(),
|
||||
@@ -559,6 +563,17 @@ impl PluginsManager {
|
||||
cache.entry = None;
|
||||
}
|
||||
|
||||
fn clear_caches_after_marketplace_source_refresh(
|
||||
&self,
|
||||
installed_plugin_cache_refreshed: bool,
|
||||
) {
|
||||
if installed_plugin_cache_refreshed {
|
||||
self.clear_cache();
|
||||
} else {
|
||||
self.tool_suggest_metadata_cache.clear();
|
||||
}
|
||||
}
|
||||
|
||||
/// Load plugins for a config layer stack without touching the plugins cache.
|
||||
pub async fn plugins_for_layer_stack(
|
||||
&self,
|
||||
@@ -1444,6 +1459,19 @@ impl PluginsManager {
|
||||
))
|
||||
}
|
||||
|
||||
pub(crate) async fn tool_suggest_metadata_for_marketplace_plugin(
|
||||
&self,
|
||||
marketplace_name: &str,
|
||||
plugin: &ConfiguredMarketplacePlugin,
|
||||
skill_config_rules: &SkillConfigRules,
|
||||
) -> Result<PluginCapabilitySummary, MarketplaceError> {
|
||||
let fragment = self
|
||||
.tool_suggest_metadata_cache
|
||||
.metadata_for_plugin(marketplace_name, plugin, self.restriction_product)
|
||||
.await?;
|
||||
Ok(fragment.project(skill_config_rules, self.auth_mode()))
|
||||
}
|
||||
|
||||
pub async fn read_plugin_for_config(
|
||||
&self,
|
||||
config: &PluginsConfigInput,
|
||||
@@ -1795,9 +1823,7 @@ impl PluginsManager {
|
||||
&outcome.upgraded_roots,
|
||||
) {
|
||||
Ok(cache_refreshed) => {
|
||||
if cache_refreshed {
|
||||
self.clear_cache();
|
||||
}
|
||||
self.clear_caches_after_marketplace_source_refresh(cache_refreshed);
|
||||
}
|
||||
Err(err) => {
|
||||
self.clear_cache();
|
||||
@@ -1977,9 +2003,8 @@ impl PluginsManager {
|
||||
&configured_curated_plugin_ids,
|
||||
) {
|
||||
Ok(cache_refreshed) => {
|
||||
if cache_refreshed {
|
||||
manager.clear_cache();
|
||||
}
|
||||
manager
|
||||
.clear_caches_after_marketplace_source_refresh(cache_refreshed);
|
||||
}
|
||||
Err(err) => {
|
||||
manager.clear_cache();
|
||||
@@ -2220,7 +2245,9 @@ impl PluginsManager {
|
||||
}
|
||||
}
|
||||
|
||||
fn remote_plugin_install_required_description(source: &MarketplacePluginSource) -> String {
|
||||
pub(crate) fn remote_plugin_install_required_description(
|
||||
source: &MarketplacePluginSource,
|
||||
) -> String {
|
||||
let source_description = match source {
|
||||
MarketplacePluginSource::Git {
|
||||
url,
|
||||
|
||||
@@ -3,6 +3,8 @@ use crate::LoadedPlugin;
|
||||
use crate::OPENAI_API_CURATED_MARKETPLACE_NAME;
|
||||
use crate::OPENAI_CURATED_MARKETPLACE_NAME;
|
||||
use crate::PluginLoadOutcome;
|
||||
use crate::ToolSuggestDiscoverablePlugin;
|
||||
use crate::ToolSuggestPluginDiscoveryInput;
|
||||
use crate::installed_marketplaces::marketplace_install_root;
|
||||
use crate::loader::load_plugin_skills;
|
||||
use crate::loader::load_plugins_from_layer_stack;
|
||||
@@ -18,6 +20,7 @@ use crate::startup_sync::curated_plugins_repo_path;
|
||||
use crate::test_support::TEST_CURATED_PLUGIN_CACHE_VERSION;
|
||||
use crate::test_support::TEST_CURATED_PLUGIN_SHA;
|
||||
use crate::test_support::load_plugins_config as load_plugins_config_input;
|
||||
use crate::test_support::write_curated_plugin;
|
||||
use crate::test_support::write_curated_plugin_sha_with as write_curated_plugin_sha;
|
||||
use crate::test_support::write_file;
|
||||
use crate::test_support::write_openai_api_curated_marketplace;
|
||||
@@ -3497,6 +3500,103 @@ source = "/tmp/debug"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn configured_marketplace_upgrade_invalidates_cached_tool_suggest_metadata() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let remote_repo = tmp.path().join("remote-marketplace");
|
||||
let remote_repo_url = url::Url::from_directory_path(&remote_repo)
|
||||
.unwrap()
|
||||
.to_string();
|
||||
write_file(
|
||||
&remote_repo.join(".agents/plugins/marketplace.json"),
|
||||
r#"{
|
||||
"name": "debug",
|
||||
"plugins": [
|
||||
{
|
||||
"name": "sample",
|
||||
"source": {
|
||||
"source": "local",
|
||||
"path": "./plugins/sample"
|
||||
}
|
||||
}
|
||||
]
|
||||
}"#,
|
||||
);
|
||||
write_curated_plugin(&remote_repo, "sample");
|
||||
write_file(
|
||||
&remote_repo.join("plugins/sample/.codex-plugin/plugin.json"),
|
||||
r#"{"name":"sample","description":"Before upgrade"}"#,
|
||||
);
|
||||
init_git_repo(&remote_repo);
|
||||
write_file(
|
||||
&tmp.path().join(CONFIG_TOML_FILE),
|
||||
&format!(
|
||||
r#"[features]
|
||||
plugins = true
|
||||
|
||||
[marketplaces.debug]
|
||||
source_type = "git"
|
||||
source = "{remote_repo_url}"
|
||||
"#
|
||||
),
|
||||
);
|
||||
|
||||
let manager = PluginsManager::new(tmp.path().to_path_buf());
|
||||
let config = load_config(tmp.path(), tmp.path()).await;
|
||||
let initial_upgrade = manager
|
||||
.upgrade_configured_marketplaces_for_config(&config, /*marketplace_name*/ None)
|
||||
.expect("initial marketplace install should succeed");
|
||||
assert_eq!(initial_upgrade.errors, Vec::new());
|
||||
assert_eq!(initial_upgrade.upgraded_roots.len(), 1);
|
||||
|
||||
let config = load_config(tmp.path(), tmp.path()).await;
|
||||
let input = ToolSuggestPluginDiscoveryInput {
|
||||
plugins: config.clone(),
|
||||
configured_plugin_ids: HashSet::from(["sample@debug".to_string()]),
|
||||
disabled_plugin_ids: HashSet::new(),
|
||||
loaded_plugin_app_connector_ids: HashSet::new(),
|
||||
};
|
||||
let expected = ToolSuggestDiscoverablePlugin {
|
||||
id: "sample@debug".to_string(),
|
||||
remote_plugin_id: None,
|
||||
name: "sample".to_string(),
|
||||
description: Some("Before upgrade".to_string()),
|
||||
has_skills: true,
|
||||
mcp_server_names: vec!["sample-docs".to_string()],
|
||||
app_connector_ids: vec!["connector_calendar".to_string()],
|
||||
};
|
||||
assert_eq!(
|
||||
manager
|
||||
.list_tool_suggest_discoverable_plugins(&input, /*auth*/ None)
|
||||
.await
|
||||
.expect("initial tool-suggest metadata should load"),
|
||||
vec![expected.clone()]
|
||||
);
|
||||
|
||||
write_file(
|
||||
&remote_repo.join("plugins/sample/.codex-plugin/plugin.json"),
|
||||
r#"{"name":"sample","description":"After upgrade"}"#,
|
||||
);
|
||||
run_git(&remote_repo, &["add", "."]);
|
||||
run_git(&remote_repo, &["commit", "-m", "update plugin"]);
|
||||
let upgrade = manager
|
||||
.upgrade_configured_marketplaces_for_config(&config, /*marketplace_name*/ None)
|
||||
.expect("marketplace upgrade should succeed");
|
||||
assert_eq!(upgrade.errors, Vec::new());
|
||||
assert_eq!(upgrade.upgraded_roots.len(), 1);
|
||||
|
||||
assert_eq!(
|
||||
manager
|
||||
.list_tool_suggest_discoverable_plugins(&input, /*auth*/ None)
|
||||
.await
|
||||
.expect("refreshed tool-suggest metadata should load"),
|
||||
vec![ToolSuggestDiscoverablePlugin {
|
||||
description: Some("After upgrade".to_string()),
|
||||
..expected
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn list_marketplaces_uses_config_when_known_registry_is_malformed() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
|
||||
@@ -66,7 +66,7 @@ pub struct MarketplacePlugin {
|
||||
pub keywords: Vec<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
|
||||
pub enum MarketplacePluginSource {
|
||||
Local {
|
||||
path: AbsolutePathBuf,
|
||||
|
||||
@@ -0,0 +1,232 @@
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use std::sync::RwLock;
|
||||
|
||||
use codex_app_server_protocol::AuthMode;
|
||||
use codex_core_skills::config_rules::SkillConfigRules;
|
||||
use codex_plugin::AppDeclaration;
|
||||
use codex_plugin::PluginCapabilitySummary;
|
||||
use codex_plugin::PluginId;
|
||||
use codex_plugin::PluginIdError;
|
||||
use codex_plugin::app_connector_ids_from_declarations;
|
||||
use codex_plugin::prompt_safe_plugin_description;
|
||||
use codex_protocol::protocol::Product;
|
||||
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_skill_inventory;
|
||||
use crate::manager::ConfiguredMarketplacePlugin;
|
||||
use crate::manager::remote_plugin_install_required_description;
|
||||
use crate::manifest::load_plugin_manifest;
|
||||
use crate::marketplace::MarketplaceError;
|
||||
use crate::marketplace::MarketplacePluginSource;
|
||||
|
||||
const MAX_TOOL_SUGGEST_METADATA_CACHE_ENTRIES: usize = 1024;
|
||||
|
||||
type ToolSuggestMetadataEntry = Result<Arc<ToolSuggestMetadataFragment>, String>;
|
||||
|
||||
/// Source-derived plugin metadata cached for tool suggestions.
|
||||
///
|
||||
/// `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 {
|
||||
state: RwLock<ToolSuggestMetadataCacheState>,
|
||||
load_semaphore: Semaphore,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
struct ToolSuggestMetadataCacheState {
|
||||
generation: u64,
|
||||
entries: HashMap<PluginArtifactIdentity, ToolSuggestMetadataEntry>,
|
||||
}
|
||||
|
||||
#[derive(Clone, PartialEq, Eq, Hash)]
|
||||
struct PluginArtifactIdentity {
|
||||
plugin_id: String,
|
||||
source: MarketplacePluginSource,
|
||||
}
|
||||
|
||||
pub(crate) struct ToolSuggestMetadataFragment {
|
||||
config_name: String,
|
||||
display_name: String,
|
||||
description: Option<String>,
|
||||
mcp_server_names: Vec<String>,
|
||||
app_declarations: Vec<AppDeclaration>,
|
||||
skill_inventory: Option<PluginSkillInventory>,
|
||||
}
|
||||
|
||||
impl ToolSuggestMetadataFragment {
|
||||
pub(crate) fn project(
|
||||
&self,
|
||||
skill_config_rules: &SkillConfigRules,
|
||||
auth_mode: Option<AuthMode>,
|
||||
) -> PluginCapabilitySummary {
|
||||
let mut app_declarations = self.app_declarations.clone();
|
||||
let mut mcp_servers = self
|
||||
.mcp_server_names
|
||||
.iter()
|
||||
.cloned()
|
||||
.map(|name| (name, ()))
|
||||
.collect::<HashMap<_, _>>();
|
||||
if auth_mode.is_some() {
|
||||
apply_app_mcp_routing_policy(
|
||||
&mut app_declarations,
|
||||
&mut mcp_servers,
|
||||
auth_mode,
|
||||
/*plugin_active*/ true,
|
||||
);
|
||||
}
|
||||
let mut mcp_server_names = mcp_servers.into_keys().collect::<Vec<_>>();
|
||||
mcp_server_names.sort_unstable();
|
||||
|
||||
PluginCapabilitySummary {
|
||||
config_name: self.config_name.clone(),
|
||||
display_name: self.display_name.clone(),
|
||||
description: self.description.clone(),
|
||||
has_skills: self
|
||||
.skill_inventory
|
||||
.as_ref()
|
||||
.is_some_and(|inventory| inventory.has_enabled_skills(skill_config_rules)),
|
||||
mcp_server_names,
|
||||
app_connector_ids: app_connector_ids_from_declarations(&app_declarations),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl ToolSuggestMetadataCache {
|
||||
pub(crate) fn new() -> Self {
|
||||
Self {
|
||||
state: RwLock::new(ToolSuggestMetadataCacheState::default()),
|
||||
load_semaphore: Semaphore::new(/*permits*/ 1),
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn clear(&self) {
|
||||
let mut state = match self.state.write() {
|
||||
Ok(state) => state,
|
||||
Err(err) => err.into_inner(),
|
||||
};
|
||||
state.generation = state.generation.wrapping_add(1);
|
||||
state.entries.clear();
|
||||
}
|
||||
|
||||
pub(crate) async fn metadata_for_plugin(
|
||||
&self,
|
||||
marketplace_name: &str,
|
||||
plugin: &ConfiguredMarketplacePlugin,
|
||||
restriction_product: Option<Product>,
|
||||
) -> Result<Arc<ToolSuggestMetadataFragment>, MarketplaceError> {
|
||||
let artifact = PluginArtifactIdentity {
|
||||
plugin_id: plugin.id.clone(),
|
||||
source: plugin.source.clone(),
|
||||
};
|
||||
loop {
|
||||
if let Some(entry) = self.cached_entry(&artifact) {
|
||||
return entry.map_err(MarketplaceError::InvalidPlugin);
|
||||
}
|
||||
|
||||
let _load_permit = self.load_semaphore.acquire().await.map_err(|_| {
|
||||
MarketplaceError::InvalidPlugin(
|
||||
"tool-suggest metadata cache loader closed".to_string(),
|
||||
)
|
||||
})?;
|
||||
if let Some(entry) = self.cached_entry(&artifact) {
|
||||
return entry.map_err(MarketplaceError::InvalidPlugin);
|
||||
}
|
||||
|
||||
let generation = self.generation();
|
||||
let entry = load_plugin_metadata(marketplace_name, plugin, restriction_product).await;
|
||||
if self.cache_entry_if_current(generation, artifact.clone(), entry.clone()) {
|
||||
return entry.map_err(MarketplaceError::InvalidPlugin);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn cached_entry(&self, artifact: &PluginArtifactIdentity) -> Option<ToolSuggestMetadataEntry> {
|
||||
match self.state.read() {
|
||||
Ok(state) => state.entries.get(artifact).cloned(),
|
||||
Err(err) => err.into_inner().entries.get(artifact).cloned(),
|
||||
}
|
||||
}
|
||||
|
||||
fn generation(&self) -> u64 {
|
||||
match self.state.read() {
|
||||
Ok(state) => state.generation,
|
||||
Err(err) => err.into_inner().generation,
|
||||
}
|
||||
}
|
||||
|
||||
fn cache_entry_if_current(
|
||||
&self,
|
||||
generation: u64,
|
||||
artifact: PluginArtifactIdentity,
|
||||
entry: ToolSuggestMetadataEntry,
|
||||
) -> bool {
|
||||
let mut state = match self.state.write() {
|
||||
Ok(state) => state,
|
||||
Err(err) => err.into_inner(),
|
||||
};
|
||||
if state.generation != generation {
|
||||
return false;
|
||||
}
|
||||
if state.entries.len() >= MAX_TOOL_SUGGEST_METADATA_CACHE_ENTRIES
|
||||
&& !state.entries.contains_key(&artifact)
|
||||
{
|
||||
state.entries.clear();
|
||||
}
|
||||
state.entries.insert(artifact, entry);
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
async fn load_plugin_metadata(
|
||||
marketplace_name: &str,
|
||||
plugin: &ConfiguredMarketplacePlugin,
|
||||
restriction_product: Option<Product>,
|
||||
) -> ToolSuggestMetadataEntry {
|
||||
let plugin_id = PluginId::new(plugin.name.clone(), marketplace_name.to_string()).map_err(
|
||||
|err| match err {
|
||||
PluginIdError::Invalid(message) => message,
|
||||
},
|
||||
)?;
|
||||
|
||||
let MarketplacePluginSource::Local { path: plugin_root } = &plugin.source else {
|
||||
return Ok(Arc::new(ToolSuggestMetadataFragment {
|
||||
config_name: plugin.id.clone(),
|
||||
display_name: plugin.name.clone(),
|
||||
description: prompt_safe_plugin_description(Some(
|
||||
&remote_plugin_install_required_description(&plugin.source),
|
||||
)),
|
||||
mcp_server_names: Vec::new(),
|
||||
app_declarations: Vec::new(),
|
||||
skill_inventory: None,
|
||||
}));
|
||||
};
|
||||
if !plugin_root.as_path().is_dir() {
|
||||
return Err("path does not exist or is not a directory".to_string());
|
||||
}
|
||||
let manifest = load_plugin_manifest(plugin_root.as_path())
|
||||
.ok_or_else(|| "missing or invalid plugin.json".to_string())?;
|
||||
let skill_inventory =
|
||||
load_plugin_skill_inventory(plugin_root, &plugin_id, &manifest, restriction_product).await;
|
||||
let mut mcp_server_names =
|
||||
load_plugin_mcp_servers(plugin_root.as_path(), /*auth_mode*/ None)
|
||||
.await
|
||||
.into_keys()
|
||||
.collect::<Vec<_>>();
|
||||
mcp_server_names.sort_unstable();
|
||||
mcp_server_names.dedup();
|
||||
let app_declarations = load_plugin_apps(plugin_root.as_path()).await;
|
||||
|
||||
Ok(Arc::new(ToolSuggestMetadataFragment {
|
||||
config_name: plugin.id.clone(),
|
||||
display_name: plugin.name.clone(),
|
||||
description: prompt_safe_plugin_description(manifest.description.as_deref()),
|
||||
mcp_server_names,
|
||||
app_declarations,
|
||||
skill_inventory: Some(skill_inventory),
|
||||
}))
|
||||
}
|
||||
Reference in New Issue
Block a user