mirror of
https://github.com/openai/codex.git
synced 2026-10-24 18:18:41 +00:00
Reuse plugin discovery during startup
This commit is contained in:
@@ -57,8 +57,9 @@ use codex_config::apply_user_plugin_config_edits;
|
||||
use codex_config::clear_user_plugin;
|
||||
use codex_config::set_user_plugin_enabled;
|
||||
use codex_config::types::PluginConfig;
|
||||
use codex_config::version_for_toml;
|
||||
use codex_core_skills::SkillMetadata;
|
||||
use codex_core_skills::config_rules::SkillConfigRules;
|
||||
use codex_core_skills::config_rules::skill_config_rules_from_stack;
|
||||
use codex_hooks::plugin_hook_declarations;
|
||||
use codex_login::AuthManager;
|
||||
use codex_login::CodexAuth;
|
||||
@@ -77,6 +78,7 @@ use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::RwLock;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
use std::sync::atomic::AtomicU64;
|
||||
use std::sync::atomic::Ordering;
|
||||
use std::time::Instant;
|
||||
use tokio::sync::Semaphore;
|
||||
@@ -404,6 +406,8 @@ pub struct PluginsManager {
|
||||
configured_marketplace_upgrade_state: RwLock<ConfiguredMarketplaceUpgradeState>,
|
||||
non_curated_cache_refresh_state: RwLock<NonCuratedCacheRefreshState>,
|
||||
cached_enabled_outcome: RwLock<Option<CachedPluginLoadOutcome>>,
|
||||
enabled_outcome_load_semaphore: Semaphore,
|
||||
enabled_outcome_cache_generation: AtomicU64,
|
||||
remote_installed_plugins_cache: RwLock<Option<Vec<RemoteInstalledPlugin>>>,
|
||||
remote_installed_plugins_cache_refresh_state: RwLock<RemoteInstalledPluginsCacheRefreshState>,
|
||||
remote_sync_lock: Semaphore,
|
||||
@@ -413,10 +417,17 @@ pub struct PluginsManager {
|
||||
|
||||
#[derive(Clone)]
|
||||
struct CachedPluginLoadOutcome {
|
||||
config_version: String,
|
||||
key: PluginLoadCacheKey,
|
||||
outcome: PluginLoadOutcome,
|
||||
}
|
||||
|
||||
#[derive(Clone, PartialEq, Eq)]
|
||||
struct PluginLoadCacheKey {
|
||||
configured_plugins: HashMap<String, PluginConfig>,
|
||||
skill_config_rules: SkillConfigRules,
|
||||
remote_plugin_enabled: bool,
|
||||
}
|
||||
|
||||
impl PluginsManager {
|
||||
pub fn new(codex_home: PathBuf) -> Self {
|
||||
Self::new_with_restriction_product(codex_home, Some(Product::Codex))
|
||||
@@ -442,6 +453,8 @@ impl PluginsManager {
|
||||
),
|
||||
non_curated_cache_refresh_state: RwLock::new(NonCuratedCacheRefreshState::default()),
|
||||
cached_enabled_outcome: RwLock::new(None),
|
||||
enabled_outcome_load_semaphore: Semaphore::new(/*permits*/ 1),
|
||||
enabled_outcome_cache_generation: AtomicU64::new(0),
|
||||
remote_installed_plugins_cache: RwLock::new(None),
|
||||
remote_installed_plugins_cache_refresh_state: RwLock::new(
|
||||
RemoteInstalledPluginsCacheRefreshState::default(),
|
||||
@@ -484,11 +497,25 @@ impl PluginsManager {
|
||||
return PluginLoadOutcome::default();
|
||||
}
|
||||
|
||||
let config_version = version_for_toml(&config.config_layer_stack.effective_config());
|
||||
if !force_reload && let Some(outcome) = self.cached_enabled_outcome(&config_version) {
|
||||
let cache_key = PluginLoadCacheKey {
|
||||
configured_plugins: configured_plugins_from_stack(&config.config_layer_stack),
|
||||
skill_config_rules: skill_config_rules_from_stack(&config.config_layer_stack),
|
||||
remote_plugin_enabled: config.remote_plugin_enabled,
|
||||
};
|
||||
if !force_reload && let Some(outcome) = self.cached_enabled_outcome(&cache_key) {
|
||||
return outcome;
|
||||
}
|
||||
|
||||
let Ok(_load_permit) = self.enabled_outcome_load_semaphore.acquire().await else {
|
||||
warn!("plugin load semaphore closed");
|
||||
return PluginLoadOutcome::default();
|
||||
};
|
||||
if !force_reload && let Some(outcome) = self.cached_enabled_outcome(&cache_key) {
|
||||
return outcome;
|
||||
}
|
||||
let cache_generation = self
|
||||
.enabled_outcome_cache_generation
|
||||
.load(Ordering::Acquire);
|
||||
let outcome = load_plugins_from_layer_stack(
|
||||
&config.config_layer_stack,
|
||||
self.remote_installed_plugin_configs(),
|
||||
@@ -498,14 +525,20 @@ impl PluginsManager {
|
||||
)
|
||||
.await;
|
||||
log_plugin_load_errors(&outcome);
|
||||
let mut cache = match self.cached_enabled_outcome.write() {
|
||||
Ok(cache) => cache,
|
||||
Err(err) => err.into_inner(),
|
||||
};
|
||||
*cache = Some(CachedPluginLoadOutcome {
|
||||
config_version,
|
||||
outcome: outcome.clone(),
|
||||
});
|
||||
if self
|
||||
.enabled_outcome_cache_generation
|
||||
.load(Ordering::Acquire)
|
||||
== cache_generation
|
||||
{
|
||||
let mut cache = match self.cached_enabled_outcome.write() {
|
||||
Ok(cache) => cache,
|
||||
Err(err) => err.into_inner(),
|
||||
};
|
||||
*cache = Some(CachedPluginLoadOutcome {
|
||||
key: cache_key,
|
||||
outcome: outcome.clone(),
|
||||
});
|
||||
}
|
||||
outcome
|
||||
}
|
||||
|
||||
@@ -519,6 +552,8 @@ impl PluginsManager {
|
||||
}
|
||||
|
||||
fn clear_enabled_outcome_cache(&self) {
|
||||
self.enabled_outcome_cache_generation
|
||||
.fetch_add(1, Ordering::AcqRel);
|
||||
let mut cached_enabled_outcome = match self.cached_enabled_outcome.write() {
|
||||
Ok(cache) => cache,
|
||||
Err(err) => err.into_inner(),
|
||||
@@ -574,16 +609,16 @@ impl PluginsManager {
|
||||
.effective_plugin_skill_roots()
|
||||
}
|
||||
|
||||
fn cached_enabled_outcome(&self, config_version: &str) -> Option<PluginLoadOutcome> {
|
||||
fn cached_enabled_outcome(&self, key: &PluginLoadCacheKey) -> Option<PluginLoadOutcome> {
|
||||
match self.cached_enabled_outcome.read() {
|
||||
Ok(cache) => cache
|
||||
.as_ref()
|
||||
.filter(|cached| cached.config_version == config_version)
|
||||
.filter(|cached| cached.key == *key)
|
||||
.map(|cached| cached.outcome.clone()),
|
||||
Err(err) => err
|
||||
.into_inner()
|
||||
.as_ref()
|
||||
.filter(|cached| cached.config_version == config_version)
|
||||
.filter(|cached| cached.key == *key)
|
||||
.map(|cached| cached.outcome.clone()),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1300,6 +1300,76 @@ async fn load_plugins_returns_empty_when_feature_disabled() {
|
||||
assert_eq!(outcome, PluginLoadOutcome::default());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn plugin_cache_ignores_unrelated_session_overrides() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let plugin_root = codex_home
|
||||
.path()
|
||||
.join("plugins/cache")
|
||||
.join("test/sample/local");
|
||||
write_plugin(
|
||||
codex_home.path().join("plugins/cache/test").as_path(),
|
||||
"sample/local",
|
||||
"sample",
|
||||
);
|
||||
write_file(
|
||||
&plugin_root.join(".mcp.json"),
|
||||
r#"{
|
||||
"mcpServers": {
|
||||
"sample": {
|
||||
"url": "https://sample.example/mcp"
|
||||
}
|
||||
}
|
||||
}"#,
|
||||
);
|
||||
|
||||
let user_file = codex_home.path().join(CONFIG_TOML_FILE).abs();
|
||||
let user_config: toml::Value = toml::from_str(&plugin_config_toml(
|
||||
/*enabled*/ true, /*plugins_feature_enabled*/ true,
|
||||
))
|
||||
.expect("user config should parse");
|
||||
let stack = |session_config: &str| {
|
||||
ConfigLayerStack::new(
|
||||
vec![
|
||||
ConfigLayerEntry::new(
|
||||
ConfigLayerSource::User {
|
||||
file: user_file.clone(),
|
||||
profile: None,
|
||||
},
|
||||
user_config.clone(),
|
||||
),
|
||||
ConfigLayerEntry::new(
|
||||
ConfigLayerSource::SessionFlags,
|
||||
toml::from_str(session_config).expect("session config should parse"),
|
||||
),
|
||||
],
|
||||
ConfigRequirements::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)
|
||||
.expect("config layer stack should build")
|
||||
};
|
||||
let config = |session_config| {
|
||||
PluginsConfigInput::new(
|
||||
stack(session_config),
|
||||
/*plugins_enabled*/ true,
|
||||
/*remote_plugin_enabled*/ false,
|
||||
"https://chatgpt.com".to_string(),
|
||||
)
|
||||
};
|
||||
let manager = PluginsManager::new(codex_home.path().to_path_buf());
|
||||
|
||||
let first = manager
|
||||
.plugins_for_config(&config(r#"model = "first""#))
|
||||
.await;
|
||||
std::fs::remove_file(plugin_root.join(".mcp.json")).unwrap();
|
||||
let second = manager
|
||||
.plugins_for_config(&config(r#"model = "second""#))
|
||||
.await;
|
||||
|
||||
assert_eq!(second, first);
|
||||
assert_eq!(second.plugins()[0].mcp_servers.len(), 1);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn load_plugins_rejects_invalid_plugin_keys() {
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
|
||||
Reference in New Issue
Block a user