Honor layered configuration when loading plugins (#40954)

## What changed

- Resolve plugin activation, MCP server policy, and marketplace definitions from the effective configuration stack, including system settings and trusted project overrides.
- Load plugin skills independently for each working directory passed to `skills/list`.
- Exclude project configuration from catalog requests whose `cwds` are omitted or empty.
- Cache up to eight loaded plugin configurations with least-recently-used eviction so requests for different working directories can reuse their results.

## Testing

- Cover layered plugin and MCP policy across trusted and untrusted projects.
- Cover per-directory skill and plugin catalog results, cache reuse and eviction, and system-defined marketplaces.

GitOrigin-RevId: cb68b2b1bdab1da48e05ca7562bc05014431fc71
This commit is contained in:
willwang-openai
2026-08-26 19:02:02 +00:00
committed by copyberry
parent 7276d67081
commit 6ac012a0d4
18 changed files with 490 additions and 132 deletions
+10
View File
@@ -297,6 +297,16 @@ Example with notification opt-out:
### Plugin configuration scope
Plugin activation and MCP settings use the existing merged configuration, including
system settings and trusted project overrides. `skills/list` resolves plugin skills
independently for each requested working directory.
Omitted or empty catalog `cwds` exclude project configuration, including the
app-server process's project.
Marketplace definitions can come from system configuration, but configured Git
marketplaces currently require an existing downloaded snapshot.
`marketplace/remove` rejects removal when the marketplace name is defined in another
enabled layer of the operation's loaded config stack. Otherwise it removes the
snapshot and any base-user entry; a base-user entry is not required for cleanup.
@@ -144,6 +144,14 @@ impl ConfigManager {
.await
}
/// Loads system, user, and runtime settings without discovering a project
/// from the app-server process's working directory.
pub(crate) async fn load_non_project_config(&self) -> std::io::Result<Config> {
let mut manager = self.clone();
manager.loader_overrides.ignore_project_config = true;
manager.load_latest_config(/*fallback_cwd*/ None).await
}
pub(crate) async fn load_latest_config_for_thread(
&self,
thread_config: &Config,
@@ -479,19 +479,12 @@ impl CatalogRequestProcessor {
cwds
};
let config = self.load_latest_config(/*fallback_cwd*/ None).await?;
let skills_service = self.thread_manager.skills_service();
let plugins_manager = self.thread_manager.plugins_manager();
if force_reload && config.features.enabled(Feature::Plugins) {
if force_reload {
plugins_manager.clear_cache();
skills_service.clear_cache();
}
// Plugin configuration is user-scoped; workspace skill rules are applied below.
let plugins_input = config.plugins_config_input();
let plugins = plugins_manager.plugins_for_config(&plugins_input).await;
let effective_skill_roots = plugins.effective_plugin_skill_roots();
let plugin_skill_snapshots =
plugins_manager.plugin_skill_snapshots_for_config(&plugins_input);
let fs = self
.thread_manager
.environment_manager()
@@ -502,12 +495,11 @@ impl CatalogRequestProcessor {
.map(|(index, cwd)| {
let fs = fs.clone();
let skills_request = &skills_request;
let effective_skill_roots = effective_skill_roots.clone();
let plugin_skill_snapshots = plugin_skill_snapshots.clone();
let plugins_manager = Arc::clone(&plugins_manager);
async move {
let (cwd_abs, config_layer_stack) = match self.resolve_cwd_config(&cwd).await {
let config = match self.load_latest_config(Some(cwd.clone())).await {
Ok(resolved) => resolved,
Err(message) => {
Err(error) => {
let error_path = cwd.clone();
return (
index,
@@ -516,16 +508,20 @@ impl CatalogRequestProcessor {
skills: Vec::new(),
errors: vec![codex_app_server_protocol::SkillErrorInfo {
path: error_path,
message,
message: error.message,
}],
},
);
}
};
let plugins_input = config.plugins_config_input();
let plugins = plugins_manager.plugins_for_config(&plugins_input).await;
let plugin_skill_snapshots =
plugins_manager.plugin_skill_snapshots_for_config(&plugins_input);
let skills_input = codex_skills_extension::HostSkillsLoadInput::new(
cwd_abs.clone(),
effective_skill_roots,
config_layer_stack,
config.cwd.clone(),
plugins.effective_plugin_skill_roots(),
config.config_layer_stack,
)
.with_plugin_skill_snapshots(plugin_skill_snapshots);
let snapshot = skills_request
@@ -38,6 +38,7 @@ use codex_rmcp_client::OAuthDiscoveryTimeout;
use codex_rmcp_client::StreamableHttpRedirectMode;
use codex_rmcp_client::perform_oauth_login_silent;
mod local;
mod search;
fn plugin_redirect_mode(plugin_root: &Path) -> StreamableHttpRedirectMode {
@@ -557,7 +558,7 @@ impl PluginRequestProcessor {
let include_local = marketplace_kinds.contains(&PluginListMarketplaceKind::Local);
let include_vertical = marketplace_kinds.contains(&PluginListMarketplaceKind::Vertical);
let config = self.load_latest_config(/*fallback_cwd*/ None).await?;
let config = self.load_catalog_config(&roots).await?;
let empty_response = || PluginListResponse {
marketplaces: Vec::new(),
marketplace_load_errors: Vec::new(),
@@ -815,7 +816,7 @@ impl PluginRequestProcessor {
marketplaces: Vec::new(),
marketplace_load_errors: Vec::new(),
};
let config = self.load_latest_config(/*fallback_cwd*/ None).await?;
let config = self.load_catalog_config(&roots).await?;
if !config.features.enabled(Feature::Plugins) {
return Ok(empty_response());
}
@@ -0,0 +1,17 @@
use super::*;
impl PluginRequestProcessor {
pub(super) async fn load_catalog_config(
&self,
cwds: &[AbsolutePathBuf],
) -> Result<Config, JSONRPCErrorError> {
if cwds.is_empty() {
self.config_manager
.load_non_project_config()
.await
.map_err(|err| internal_error(format!("failed to reload config: {err}")))
} else {
self.load_latest_config(/*fallback_cwd*/ None).await
}
}
}
@@ -42,7 +42,9 @@ impl PluginRequestProcessor {
return Ok(empty_response());
}
let config = self.load_latest_config(/*fallback_cwd*/ None).await?;
let config = self
.load_catalog_config(cwds.as_deref().unwrap_or_default())
.await?;
if !config.features.enabled(Feature::Plugins) {
return Ok(empty_response());
}
@@ -834,14 +834,22 @@ async fn plugin_list_uses_alternate_discoverable_manifest_and_keeps_undiscoverab
}
#[tokio::test]
async fn plugin_list_accepts_omitted_cwds() -> Result<()> {
async fn plugin_list_omitted_cwds_excludes_server_project_config() -> Result<()> {
let codex_home = TempDir::new()?;
let project_marketplace = TempDir::new()?;
std::fs::create_dir_all(codex_home.path().join(".agents/plugins"))?;
write_plugins_enabled_config(codex_home.path())?;
std::fs::create_dir_all(codex_home.path().join(".git"))?;
std::fs::create_dir_all(codex_home.path().join(".codex"))?;
std::fs::create_dir_all(project_marketplace.path().join(".agents/plugins"))?;
write_installed_plugin(&codex_home, "home-marketplace", "home-plugin")?;
std::fs::write(
codex_home.path().join("config.toml"),
"[features]\nplugins = true\n[plugins.\"home-plugin@home-marketplace\"]\nenabled = true\n",
)?;
std::fs::write(
codex_home.path().join(".agents/plugins/marketplace.json"),
r#"{
"name": "codex-curated",
"name": "home-marketplace",
"plugins": [
{
"name": "home-plugin",
@@ -853,6 +861,20 @@ async fn plugin_list_accepts_omitted_cwds() -> Result<()> {
]
}"#,
)?;
std::fs::write(
project_marketplace
.path()
.join(".agents/plugins/marketplace.json"),
r#"{"name":"project-marketplace","plugins":[{"name":"project-plugin","source":{"source":"local","path":"./project-plugin"}}]}"#,
)?;
let source = serde_json::to_string(&project_marketplace.path().to_string_lossy())?;
std::fs::write(
codex_home.path().join(".codex/config.toml"),
format!(
"[marketplaces.project-marketplace]\nsource_type = \"local\"\nsource = {source}\n\n[plugins.\"home-plugin@home-marketplace\"]\nenabled = false\n"
),
)?;
set_project_trust_level(codex_home.path(), codex_home.path(), TrustLevel::Trusted)?;
let home = codex_home.path().to_string_lossy().into_owned();
let mut mcp = TestAppServer::builder()
.with_codex_home(codex_home.path())
@@ -863,15 +885,39 @@ async fn plugin_list_accepts_omitted_cwds() -> Result<()> {
.build_initialized_with_timeout(DEFAULT_TIMEOUT)
.await?;
let request_id = mcp
.send_plugin_list_request(PluginListParams {
cwds: None,
marketplace_kinds: None,
force_refetch: false,
})
.await?;
let _: PluginListResponse = timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??;
for (cwds, expected) in [
(None, vec![("home-plugin@home-marketplace", true, true)]),
(
Some(Vec::new()),
vec![("home-plugin@home-marketplace", true, true)],
),
(
Some(vec![AbsolutePathBuf::try_from(codex_home.path())?]),
vec![
("home-plugin@home-marketplace", true, false),
("project-plugin@project-marketplace", false, false),
],
),
] {
let request_id = mcp
.send_plugin_list_request(PluginListParams {
cwds,
marketplace_kinds: Some(vec![PluginListMarketplaceKind::Local]),
force_refetch: false,
})
.await?;
let response: PluginListResponse =
timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??;
let mut plugins = response
.marketplaces
.iter()
.flat_map(|marketplace| &marketplace.plugins)
.map(|plugin| (plugin.id.as_str(), plugin.installed, plugin.enabled))
.collect::<Vec<_>>();
plugins.sort_unstable();
assert_eq!(plugins, expected);
assert_eq!(response.marketplace_load_errors, Vec::new());
}
Ok(())
}
@@ -39,6 +39,7 @@ use core_test_support::skip_if_remote;
use core_test_support::skip_if_wine_exec;
use pretty_assertions::assert_eq;
use tempfile::TempDir;
use test_case::test_case;
use tokio::time::timeout;
use wiremock::Mock;
use wiremock::MockServer;
@@ -794,8 +795,10 @@ async fn skills_list_accepts_relative_cwds() -> Result<()> {
Ok(())
}
#[test_case(false; "plugins disabled by default")]
#[test_case(true; "plugins enabled by default")]
#[tokio::test]
async fn skills_list_preserves_requested_cwd_order() -> Result<()> {
async fn skills_list_preserves_requested_cwd_order(home_plugins_enabled: bool) -> Result<()> {
skip_if_wine_exec!(
Ok(()),
"skills/list currently requires host-native cwd paths for workspace config"
@@ -803,25 +806,32 @@ async fn skills_list_preserves_requested_cwd_order() -> Result<()> {
let codex_home = TempDir::new()?;
let first_cwd = TempDir::new()?;
let second_cwd = TempDir::new()?;
let third_cwd = TempDir::new()?;
write_skill(&codex_home, "shared-skill")?;
write_cached_local_curated_plugin_with_skill(codex_home.path(), "openai-api-curated")?;
std::fs::write(
codex_home.path().join("config.toml"),
r#"[features]
plugins = true
format!(
r#"[features]
plugins = {home_plugins_enabled}
[plugins."google-calendar@openai-api-curated"]
enabled = true
"#,
enabled = false
"#
),
)?;
for (cwd, plugin_enabled) in [(first_cwd.path(), true), (second_cwd.path(), false)] {
for (cwd, plugins_enabled, plugin_enabled) in [
(first_cwd.path(), true, true),
(second_cwd.path(), false, true),
(third_cwd.path(), true, false),
] {
std::fs::create_dir_all(cwd.join(".git"))?;
std::fs::create_dir_all(cwd.join(".codex"))?;
std::fs::write(
cwd.join(".codex/config.toml"),
format!(
"[plugins.\"google-calendar@openai-api-curated\"]\nenabled = {plugin_enabled}\n"
"[features]\nplugins = {plugins_enabled}\n[plugins.\"google-calendar@openai-api-curated\"]\nenabled = {plugin_enabled}\n"
),
)?;
set_project_trust_level(codex_home.path(), cwd, TrustLevel::Trusted)?;
@@ -836,6 +846,7 @@ enabled = true
for (cwd, name) in [
(first_cwd.path(), "first-project-skill"),
(second_cwd.path(), "second-project-skill"),
(third_cwd.path(), "third-project-skill"),
] {
let cwd = AbsolutePathBuf::try_from(cwd)?;
let git_dir = PathUri::from_abs_path(&cwd.join(".git"));
@@ -862,29 +873,30 @@ enabled = true
.await?;
}
for (request_index, force_reload) in [false, false, true].into_iter().enumerate() {
if request_index == 1 {
write_skill(&codex_home, "new-skill")?;
}
for force_reload in [false, false, true] {
let request_id = mcp
.send_skills_list_request(SkillsListParams {
cwds: vec![
first_cwd.path().to_path_buf(),
second_cwd.path().to_path_buf(),
third_cwd.path().to_path_buf(),
],
force_reload,
})
.await?;
let SkillsListResponse { data } =
timeout(DEFAULT_TIMEOUT, mcp.read_response(request_id)).await??;
assert_eq!(data.len(), 2);
assert_eq!(data[0].cwd, first_cwd.path());
assert_eq!(data[1].cwd, second_cwd.path());
for (entry, project_skill) in data
.iter()
.zip(["first-project-skill", "second-project-skill"])
{
assert_eq!(
data.iter()
.map(|entry| entry.cwd.as_path())
.collect::<Vec<_>>(),
vec![first_cwd.path(), second_cwd.path(), third_cwd.path()],
);
for (entry, (project_skill, plugin_enabled)) in data.iter().zip([
("first-project-skill", true),
("second-project-skill", false),
("third-project-skill", false),
]) {
assert_eq!(entry.errors, Vec::new());
assert!(
entry
@@ -892,6 +904,10 @@ enabled = true
.iter()
.any(|skill| skill.name == "shared-skill")
);
let mut expected_skills = vec![project_skill];
if plugin_enabled {
expected_skills.push("google-calendar:meeting-prep");
}
assert_eq!(
entry
.skills
@@ -902,11 +918,7 @@ enabled = true
})
.map(|skill| skill.name.as_str())
.collect::<Vec<_>>(),
vec![project_skill, "google-calendar:meeting-prep"]
);
assert_eq!(
entry.skills.iter().any(|skill| skill.name == "new-skill"),
force_reload
expected_skills
);
}
}
+2 -4
View File
@@ -335,10 +335,8 @@ fn configured_marketplace_sources_by_root(
plugins_input: &PluginsConfigInput,
) -> HashMap<PathBuf, JsonMarketplaceSource> {
let marketplace_sources = configured_marketplace_sources(plugins_input, codex_home);
let Some(user_config) = plugins_input.config_layer_stack.effective_user_config() else {
return HashMap::new();
};
let Some(marketplaces) = user_config
let effective_config = plugins_input.config_layer_stack.effective_config();
let Some(marketplaces) = effective_config
.get("marketplaces")
.and_then(toml::Value::as_table)
else {
+4 -8
View File
@@ -483,10 +483,8 @@ pub(crate) fn configured_marketplace_sources(
plugins_input: &PluginsConfigInput,
codex_home: &Path,
) -> HashMap<String, JsonMarketplaceSource> {
let Some(user_config) = plugins_input.config_layer_stack.effective_user_config() else {
return HashMap::new();
};
let Some(marketplaces) = user_config
let effective_config = plugins_input.config_layer_stack.effective_config();
let Some(marketplaces) = effective_config
.get("marketplaces")
.and_then(toml::Value::as_table)
else {
@@ -726,10 +724,8 @@ pub(crate) fn configured_marketplace_snapshot_issues(
load_errors: &[MarketplaceListError],
marketplace_name: Option<&str>,
) -> Vec<ConfiguredMarketplaceSnapshotIssue> {
let Some(user_config) = plugins_input.config_layer_stack.effective_user_config() else {
return Vec::new();
};
let Some(configured_marketplaces) = user_config
let effective_config = plugins_input.config_layer_stack.effective_config();
let Some(configured_marketplaces) = effective_config
.get("marketplaces")
.and_then(toml::Value::as_table)
else {
@@ -7,7 +7,7 @@ use codex_utils_absolute_path::AbsolutePathBuf;
use tracing::warn;
use crate::marketplace::find_marketplace_manifest_path;
use crate::marketplace_policy::project_effective_user_config;
use crate::marketplace_policy::policy_filtered_plugin_config;
pub const INSTALLED_MARKETPLACES_DIR: &str = ".tmp/marketplaces";
@@ -19,10 +19,11 @@ pub fn installed_marketplace_roots_from_layer_stack(
config_layer_stack: &ConfigLayerStack,
codex_home: &Path,
) -> Vec<AbsolutePathBuf> {
let Some(user_config) = project_effective_user_config(config_layer_stack, codex_home) else {
let Some(effective_config) = policy_filtered_plugin_config(config_layer_stack, codex_home)
else {
return Vec::new();
};
let Some(marketplaces_value) = user_config.get("marketplaces") else {
let Some(marketplaces_value) = effective_config.get("marketplaces") else {
return Vec::new();
};
let Some(marketplaces) = marketplaces_value.as_table() else {
+4 -7
View File
@@ -723,7 +723,7 @@ fn is_full_git_sha(value: &str) -> bool {
value.len() == 40 && value.chars().all(|ch| ch.is_ascii_hexdigit())
}
fn configured_plugins_from_user_config_value(
fn configured_plugins_from_config_value(
user_config: &toml::Value,
) -> HashMap<String, PluginConfig> {
let Some(plugins_value) = user_config.get("plugins") else {
@@ -769,7 +769,7 @@ fn configured_plugins_from_codex_home(
}
};
configured_plugins_from_user_config_value(&user_config)
configured_plugins_from_config_value(&user_config)
}
fn configured_plugin_ids(
@@ -1391,7 +1391,7 @@ pub async fn load_plugin_mcp_servers(
load_plugin_mcp_servers_with_policy(plugin_root, auth_mode, /*plugin_policy*/ None).await
}
/// Loads plugin MCP servers with the effective user policy for an installed plugin.
/// Loads plugin MCP servers with the effective configuration policy for an installed plugin.
pub async fn load_configured_plugin_mcp_servers(
plugin_root: &Path,
auth_mode: Option<AuthMode>,
@@ -1412,10 +1412,7 @@ pub async fn load_configured_plugin_mcp_servers(
pub fn configured_plugin_mcp_server_policies(
config_layer_stack: &ConfigLayerStack,
) -> HashMap<String, HashMap<String, PluginMcpServerConfig>> {
config_layer_stack
.effective_user_config()
.map(|config| configured_plugins_from_user_config_value(&config))
.unwrap_or_default()
configured_plugins_from_config_value(&config_layer_stack.effective_config())
.into_iter()
.map(|(plugin_id, plugin)| (plugin_id, plugin.mcp_servers))
.collect()
+43 -2
View File
@@ -314,10 +314,16 @@ async fn installed_agent_plugin_uses_isolated_data_root_for_stdio_mcp() {
}
#[test]
fn configured_plugins_from_stack_merges_user_layers() {
fn configured_plugins_from_stack_merges_enabled_effective_layers() {
let temp_dir = TempDir::new().expect("tempdir");
let stack = ConfigLayerStack::new(
vec![
ConfigLayerEntry::new(
ConfigLayerSource::System {
file: user_config_path(&temp_dir, "system.toml"),
},
toml::from_str("[plugins.system]\nenabled = true\n").expect("system config toml"),
),
user_layer(
user_config_path(&temp_dir, "config.toml"),
"[plugins.base]\nenabled = true\n",
@@ -326,12 +332,36 @@ fn configured_plugins_from_stack_merges_user_layers() {
user_config_path(&temp_dir, "work.config.toml"),
"[plugins.profile]\nenabled = false\n",
),
ConfigLayerEntry::new(
ConfigLayerSource::Project {
dot_codex_folder: user_config_path(&temp_dir, "project/.codex"),
},
toml::from_str(
"[plugins.profile]\nenabled = true\n[plugins.profile.mcp_servers.example]\nenabled = false\n",
)
.expect("project config toml"),
),
ConfigLayerEntry::new_disabled(
ConfigLayerSource::Project {
dot_codex_folder: user_config_path(&temp_dir, "project/untrusted/.codex"),
},
toml::from_str("[plugins.untrusted]\nenabled = true\n")
.expect("untrusted project config toml"),
"project is untrusted",
),
],
ConfigRequirements::default(),
ConfigRequirementsToml::default(),
)
.expect("valid config layer stack");
let project_mcp_servers = HashMap::from([(
"example".to_string(),
PluginMcpServerConfig {
enabled: false,
..PluginMcpServerConfig::default()
},
)]);
let plugins = configured_plugins_from_stack(&stack, temp_dir.path());
assert_eq!(
@@ -347,12 +377,23 @@ fn configured_plugins_from_stack_merges_user_layers() {
(
"profile".to_string(),
PluginConfig {
enabled: false,
enabled: true,
mcp_servers: project_mcp_servers.clone(),
},
),
(
"system".to_string(),
PluginConfig {
enabled: true,
mcp_servers: HashMap::new(),
},
),
])
);
assert_eq!(
configured_plugin_mcp_server_policies(&stack).get("profile"),
Some(&project_mcp_servers)
);
}
#[tokio::test]
+27 -17
View File
@@ -109,6 +109,7 @@ use std::collections::VecDeque;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;
use std::sync::Mutex;
use std::sync::RwLock;
use std::sync::atomic::AtomicBool;
use std::sync::atomic::Ordering;
@@ -125,6 +126,7 @@ static CURATED_REPO_SYNC_STARTED: AtomicBool = AtomicBool::new(false);
const FEATURED_PLUGIN_IDS_CACHE_TTL: std::time::Duration =
std::time::Duration::from_secs(60 * 60 * 3);
const REMOTE_INSTALLED_PLUGIN_SYNC_WAIT_TIMEOUT: Duration = Duration::from_secs(120);
const LOADED_PLUGINS_CACHE_CAPACITY: usize = 8;
type EffectivePluginsChangedCallback = Arc<dyn Fn(EffectivePluginsChange) + Send + Sync + 'static>;
@@ -503,8 +505,8 @@ pub struct PluginsManager {
non_curated_cache_refresh_lock: Semaphore,
non_curated_cache_refresh_state: RwLock<NonCuratedCacheRefreshState>,
non_curated_cache_refresh_completion: watch::Sender<NonCuratedCacheRefreshCompletion>,
// Remote installed plugins vary by account, so loaded capabilities are cached per auth.
loaded_plugins_cache: RwLock<LoadedPluginsCache>,
// Loaded capabilities vary by effective configuration and, for remote plugins, account.
loaded_plugins_cache: Mutex<LoadedPluginsCache>,
loaded_plugins_load_semaphore: Semaphore,
skill_root_loader: Arc<dyn SkillRootLoader<PluginSkillRoot>>,
tool_suggest_metadata_cache: ToolSuggestMetadataCache,
@@ -528,7 +530,17 @@ struct LoadedPluginsCacheEntry {
#[derive(Default)]
struct LoadedPluginsCache {
generation: u64,
entry: Option<LoadedPluginsCacheEntry>,
// Most recently used first.
entries: VecDeque<LoadedPluginsCacheEntry>,
}
impl LoadedPluginsCache {
fn get(&mut self, key: &PluginLoadCacheKey) -> Option<&LoadedPluginsCacheEntry> {
let index = self.entries.iter().position(|entry| &entry.key == key)?;
let entry = self.entries.remove(index)?;
self.entries.push_front(entry);
self.entries.front()
}
}
#[derive(Clone, PartialEq, Eq)]
@@ -611,7 +623,7 @@ impl PluginsManager {
NonCuratedCacheRefreshCompletion::default(),
)
.0,
loaded_plugins_cache: RwLock::new(LoadedPluginsCache::default()),
loaded_plugins_cache: Mutex::new(LoadedPluginsCache::default()),
loaded_plugins_load_semaphore: Semaphore::new(/*permits*/ 1),
skill_root_loader,
tool_suggest_metadata_cache: ToolSuggestMetadataCache::new(),
@@ -696,11 +708,9 @@ impl PluginsManager {
RemoteInstalledPluginsAuthIdentity::from_auth(auth.as_ref()),
);
self.loaded_plugins_cache
.read()
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.entry
.as_ref()
.filter(|cached| cached.key == key)
.get(&key)
.map(|cached| cached.plugin_skill_snapshots.clone())
}
@@ -855,12 +865,12 @@ impl PluginsManager {
fn clear_loaded_plugins_cache(&self) {
self.tool_suggest_metadata_cache.clear();
let mut cache = match self.loaded_plugins_cache.write() {
let mut cache = match self.loaded_plugins_cache.lock() {
Ok(cache) => cache,
Err(err) => err.into_inner(),
};
cache.generation = cache.generation.wrapping_add(1);
cache.entry = None;
cache.entries.clear();
}
fn clear_caches_after_marketplace_source_refresh(
@@ -900,16 +910,14 @@ impl PluginsManager {
fn cached_loaded_plugins(&self, key: &PluginLoadCacheKey) -> Option<Vec<LoadedPlugin>> {
self.loaded_plugins_cache
.read()
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.entry
.as_ref()
.filter(|cached| cached.key == *key)
.get(key)
.map(|cached| cached.plugins.clone())
}
fn loaded_plugins_cache_generation(&self) -> u64 {
match self.loaded_plugins_cache.read() {
match self.loaded_plugins_cache.lock() {
Ok(cache) => cache.generation,
Err(err) => err.into_inner().generation,
}
@@ -922,16 +930,18 @@ impl PluginsManager {
plugins: Vec<LoadedPlugin>,
plugin_skill_snapshots: SkillRootSnapshots<PluginSkillRoot>,
) {
let mut cache = match self.loaded_plugins_cache.write() {
let mut cache = match self.loaded_plugins_cache.lock() {
Ok(cache) => cache,
Err(err) => err.into_inner(),
};
if cache.generation == generation {
cache.entry = Some(LoadedPluginsCacheEntry {
cache.entries.retain(|entry| entry.key != key);
cache.entries.push_front(LoadedPluginsCacheEntry {
key,
plugins,
plugin_skill_snapshots,
});
cache.entries.truncate(LOADED_PLUGINS_CACHE_CAPACITY);
}
}
+96 -11
View File
@@ -2903,7 +2903,7 @@ async fn load_plugins_returns_empty_when_feature_disabled() {
}
#[tokio::test]
async fn plugin_cache_ignores_unrelated_session_overrides() {
async fn plugin_cache_reuses_effective_configurations() {
let codex_home = TempDir::new().unwrap();
let plugin_root = codex_home
.path()
@@ -2962,16 +2962,44 @@ async fn plugin_cache_ignores_unrelated_session_overrides() {
};
let manager = test_plugins_manager(codex_home.path().to_path_buf());
let first = manager
.plugins_for_config(&config(r#"model = "first""#))
.await;
let first_config = config(r#"model = "first""#);
let second_config = config(
r#"[plugins."sample@test".mcp_servers.sample]
enabled = false"#,
);
let first = manager.plugins_for_config(&first_config).await;
let first_snapshots = manager
.plugin_skill_snapshots_for_config(&first_config)
.expect("first configuration snapshots");
let second = manager.plugins_for_config(&second_config).await;
let second_snapshots = manager
.plugin_skill_snapshots_for_config(&second_config)
.expect("second configuration snapshots");
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);
assert_eq!(
manager.plugin_skill_snapshots_for_config(&first_config),
Some(first_snapshots),
);
assert_eq!(
manager
.plugins_for_config(&config(r#"model = "second""#))
.await,
first,
);
assert_eq!(
manager.plugin_skill_snapshots_for_config(&second_config),
Some(second_snapshots),
);
assert_eq!(manager.plugins_for_config(&second_config).await, second);
manager.clear_cache();
assert_eq!(
[first_config, second_config]
.iter()
.map(|config| manager.plugin_skill_snapshots_for_config(config))
.collect::<Vec<_>>(),
vec![None, None],
);
}
#[tokio::test]
@@ -3056,6 +3084,57 @@ enabled = true
}
}
#[test]
fn loaded_plugins_cache_evicts_least_recently_used_configuration() {
let codex_home = TempDir::new().unwrap();
let manager = test_plugins_manager(codex_home.path().to_path_buf());
let keys = (0..=LOADED_PLUGINS_CACHE_CAPACITY)
.map(|index| PluginLoadCacheKey {
configured_plugins: HashMap::from([(
format!("plugin-{index}@test"),
PluginConfig {
enabled: true,
mcp_servers: HashMap::new(),
},
)]),
skill_config_rules: SkillConfigRules::default(),
remote_global_catalog_active: false,
auth_identity: None,
})
.collect::<Vec<_>>();
let generation = manager.loaded_plugins_cache_generation();
for key in keys.iter().take(LOADED_PLUGINS_CACHE_CAPACITY) {
manager.cache_loaded_plugins_if_current(
generation,
key.clone(),
Vec::new(),
crate::skill_snapshots::new_plugin_skill_snapshots(),
);
}
manager.cache_loaded_plugins_if_current(
generation,
keys[LOADED_PLUGINS_CACHE_CAPACITY - 1].clone(),
Vec::new(),
crate::skill_snapshots::new_plugin_skill_snapshots(),
);
assert_eq!(manager.cached_loaded_plugins(&keys[0]), Some(Vec::new()));
manager.cache_loaded_plugins_if_current(
generation,
keys[LOADED_PLUGINS_CACHE_CAPACITY].clone(),
Vec::new(),
crate::skill_snapshots::new_plugin_skill_snapshots(),
);
assert_eq!(
keys.iter()
.map(|key| manager.cached_loaded_plugins(key).is_some())
.collect::<Vec<_>>(),
(0..=LOADED_PLUGINS_CACHE_CAPACITY)
.map(|index| index != 1)
.collect::<Vec<_>>(),
);
}
#[test]
fn loaded_plugins_cache_invalidation_rejects_stale_load_completion() {
let codex_home = TempDir::new().unwrap();
@@ -6671,7 +6750,7 @@ fn refresh_non_curated_plugin_cache_continues_after_plugin_error() {
}
#[tokio::test]
async fn load_plugins_ignores_project_config_files() {
async fn load_plugins_uses_project_config_files() {
let codex_home = TempDir::new().unwrap();
let project_root = codex_home.path().join("project");
let plugin_root = codex_home
@@ -6714,7 +6793,13 @@ async fn load_plugins_ignores_project_config_files() {
)
.await;
assert_eq!(plugins, Vec::new());
assert_eq!(
plugins
.iter()
.map(|plugin| (plugin.config_name.as_str(), plugin.enabled))
.collect::<Vec<_>>(),
vec![("sample@test", true)]
);
}
#[tokio::test]
+19 -21
View File
@@ -121,12 +121,8 @@ impl MarketplacePolicy {
return Ok(());
}
let user_config = config_layer_stack.effective_user_config().ok_or_else(|| {
format!(
"marketplace `{marketplace_name}` must be added to config before plugins can be installed while marketplace source restrictions are enabled"
)
})?;
let marketplace = user_config
let effective_config = config_layer_stack.effective_config();
let marketplace = effective_config
.get("marketplaces")
.and_then(toml::Value::as_table)
.and_then(|marketplaces| marketplaces.get(marketplace_name))
@@ -207,28 +203,31 @@ impl AllowedMarketplaceSource {
}
}
pub(crate) fn project_effective_user_config(
pub(crate) fn policy_filtered_plugin_config(
config_layer_stack: &ConfigLayerStack,
codex_home: &Path,
) -> Option<toml::Value> {
let mut user_config = config_layer_stack.effective_user_config()?;
let mut effective_config = config_layer_stack.effective_config();
if effective_config.get("plugins").is_none() && effective_config.get("marketplaces").is_none() {
return None;
}
let policy = MarketplacePolicy::from_requirements(config_layer_stack.requirements());
let allowed_marketplace_names =
allowed_configured_marketplace_names_with_policy(&user_config, &policy, codex_home);
let configured_marketplace_names = user_config
allowed_configured_marketplace_names_with_policy(&effective_config, &policy, codex_home);
let configured_marketplace_names = effective_config
.get("marketplaces")
.and_then(toml::Value::as_table)
.map(|marketplaces| marketplaces.keys().cloned().collect::<HashSet<_>>())
.unwrap_or_default();
if let Some(marketplaces) = user_config
if let Some(marketplaces) = effective_config
.get_mut("marketplaces")
.and_then(toml::Value::as_table_mut)
{
marketplaces
.retain(|marketplace_name, _| allowed_marketplace_names.contains(marketplace_name));
}
if let Some(plugins) = user_config
if let Some(plugins) = effective_config
.get_mut("plugins")
.and_then(toml::Value::as_table_mut)
{
@@ -243,26 +242,24 @@ pub(crate) fn project_effective_user_config(
is_openai_curated_marketplace_name(marketplace_name) || !policy.is_restricted()
});
}
Some(user_config)
Some(effective_config)
}
pub fn allowed_configured_marketplace_names(
config_layer_stack: &ConfigLayerStack,
codex_home: &Path,
) -> HashSet<String> {
let Some(user_config) = config_layer_stack.effective_user_config() else {
return HashSet::new();
};
let effective_config = config_layer_stack.effective_config();
let policy = MarketplacePolicy::from_requirements(config_layer_stack.requirements());
allowed_configured_marketplace_names_with_policy(&user_config, &policy, codex_home)
allowed_configured_marketplace_names_with_policy(&effective_config, &policy, codex_home)
}
fn allowed_configured_marketplace_names_with_policy(
user_config: &toml::Value,
effective_config: &toml::Value,
policy: &MarketplacePolicy,
codex_home: &Path,
) -> HashSet<String> {
let Some(marketplaces) = user_config
let Some(marketplaces) = effective_config
.get("marketplaces")
.and_then(toml::Value::as_table)
else {
@@ -288,10 +285,11 @@ pub(crate) fn configured_plugins_from_stack(
config_layer_stack: &ConfigLayerStack,
codex_home: &Path,
) -> HashMap<String, PluginConfig> {
let Some(user_config) = project_effective_user_config(config_layer_stack, codex_home) else {
let Some(effective_config) = policy_filtered_plugin_config(config_layer_stack, codex_home)
else {
return HashMap::new();
};
let Some(plugins_value) = user_config.get("plugins") else {
let Some(plugins_value) = effective_config.get("plugins") else {
return HashMap::new();
};
match plugins_value.clone().try_into() {
@@ -220,7 +220,7 @@ restrict_to_allowed_sources = {restricted}
}
#[test]
fn strict_install_validates_configured_name_source_and_root() {
fn system_marketplace_discovery_and_install_validate_source_and_root() {
let codex_home = TempDir::new().expect("create Codex home");
let configured_root = TempDir::new().expect("create configured marketplace");
let other_root = TempDir::new().expect("create other marketplace");
@@ -256,6 +256,18 @@ source = {configured_root:?}
config_file,
)),
);
let layer = stack.layers_low_to_high().next().expect("configured layer");
let stack = ConfigLayerStack::new(
vec![ConfigLayerEntry::new(
ConfigLayerSource::System {
file: AbsolutePathBuf::try_from(codex_home.path().join("system.toml")).unwrap(),
},
layer.config.clone(),
)],
stack.requirements().clone(),
stack.requirements_toml().clone(),
)
.expect("system config stack");
let policy = MarketplacePolicy::from_requirements(stack.requirements());
let configured_path =
AbsolutePathBuf::try_from(configured_root.join(".agents/plugins/marketplace.json"))
@@ -263,6 +275,15 @@ source = {configured_root:?}
let other_path = AbsolutePathBuf::try_from(other_root.join(".agents/plugins/marketplace.json"))
.expect("other marketplace path");
fs::create_dir_all(configured_path.as_path().parent().unwrap()).unwrap();
fs::write(&configured_path, r#"{"name":"company","plugins":[]}"#).unwrap();
assert_eq!(
crate::installed_marketplaces::installed_marketplace_roots_from_layer_stack(
&stack,
codex_home.path()
),
vec![AbsolutePathBuf::try_from(configured_root).unwrap()],
);
assert_eq!(
policy.validate_install(&stack, codex_home.path(), &configured_path, "company"),
Ok(())
@@ -508,7 +529,7 @@ enabled = true
);
let projected =
project_effective_user_config(&stack, codex_home.path()).expect("project user config");
policy_filtered_plugin_config(&stack, codex_home.path()).expect("project plugin config");
assert_eq!(
projected["marketplaces"]
.as_table()
@@ -574,7 +595,7 @@ enabled = true
);
let projected =
project_effective_user_config(&stack, codex_home.path()).expect("project user config");
policy_filtered_plugin_config(&stack, codex_home.path()).expect("project plugin config");
assert_eq!(
projected["marketplaces"]
@@ -640,7 +661,7 @@ enabled = true
);
let projected =
project_effective_user_config(&stack, codex_home.path()).expect("project user config");
policy_filtered_plugin_config(&stack, codex_home.path()).expect("project plugin config");
assert_eq!(
projected["marketplaces"]
.as_table()
+119
View File
@@ -5,8 +5,11 @@ use std::time::Duration;
use std::time::Instant;
use anyhow::Result;
use codex_config::LoaderOverrides;
use codex_core::TurnInputRequest;
use codex_core::config::Config;
use codex_core::config::ConfigBuilder;
use codex_core::config::set_project_trust_level;
use codex_core_plugins::store::PluginStore;
use codex_extension_api::ExtensionRegistry;
use codex_extension_api::ExtensionRegistryBuilder;
@@ -20,6 +23,7 @@ use codex_protocol::auth::AuthMode;
use codex_protocol::config_types::CollaborationMode;
use codex_protocol::config_types::ModeKind;
use codex_protocol::config_types::Settings;
use codex_protocol::config_types::TrustLevel;
use codex_protocol::models::PermissionProfile;
use codex_protocol::protocol::AskForApproval;
use codex_protocol::protocol::EventMsg;
@@ -1244,6 +1248,121 @@ async fn explicit_plugin_mentions_keep_non_conflicting_mcp_for_chatgpt_auth() ->
Ok(())
}
#[test_case(TrustLevel::Trusted, true, true, false, &[]; "trusted project disables the plugin")]
#[test_case(TrustLevel::Untrusted, true, true, false, &["echo_tool"]; "untrusted project cannot disable the plugin")]
#[test_case(TrustLevel::Trusted, false, true, true, &["echo"]; "trusted project enables system-disabled server and overrides user tool policy")]
#[test_case(TrustLevel::Untrusted, false, true, true, &[]; "untrusted project cannot enable system-disabled server")]
#[test_case(TrustLevel::Trusted, true, false, true, &[]; "trusted project disables system-enabled server")]
#[test_case(TrustLevel::Untrusted, true, false, true, &["echo_tool"]; "untrusted project preserves system startup and user tool policy")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn system_marketplace_plugin_honors_layered_activation_and_mcp_policy(
trust_level: TrustLevel,
system_enabled: bool,
project_enabled: bool,
plugin_enabled: bool,
expected_tools: &[&str],
) -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let mock = mount_plugin_tool_search_turn(&server).await;
let codex_home = Arc::new(TempDir::new()?);
let project = TempDir::new()?;
write_plugin_mcp_plugin(codex_home.as_ref(), &stdio_server_bin()?);
let user_config_path = codex_home.path().join("config.toml");
let user_config = std::fs::read_to_string(&user_config_path)?;
std::fs::write(
&user_config_path,
format!(
"{user_config}\n[plugins.\"{SAMPLE_PLUGIN_CONFIG_NAME}\".mcp_servers.sample]\ndisabled_tools = [\"echo\"]\n"
),
)?;
let system_config_path = codex_home.path().join("system.toml");
let marketplace = TempDir::new()?;
std::fs::create_dir_all(marketplace.path().join(".agents/plugins"))?;
std::fs::write(
marketplace.path().join(".agents/plugins/marketplace.json"),
r#"{"name":"test","plugins":[{"name":"sample","source":{"source":"local","path":"./sample"}}]}"#,
)?;
let marketplace_source = toml::Value::String(marketplace.path().to_string_lossy().into_owned());
std::fs::write(
&system_config_path,
format!(
"[marketplaces.test]\nsource_type = \"local\"\nsource = {marketplace_source}\n[plugins.\"{SAMPLE_PLUGIN_CONFIG_NAME}\".mcp_servers.sample]\nenabled = {system_enabled}\nenabled_tools = [\"echo\", \"echo-tool\"]\n"
),
)?;
// The cached plugin may activate only through this system-defined marketplace.
// Without the definition, source restrictions exclude it from the real turn.
let requirements_path = codex_home.path().join("requirements.toml");
std::fs::write(
&requirements_path,
format!(
"[marketplaces]\nrestrict_to_allowed_sources = true\n[marketplaces.allowed_sources.test]\nsource = \"local\"\npath = {marketplace_source}\n"
),
)?;
std::fs::create_dir_all(project.path().join(".git"))?;
std::fs::create_dir_all(project.path().join(".codex"))?;
std::fs::write(
project.path().join(".codex/config.toml"),
format!(
"[plugins.\"{SAMPLE_PLUGIN_CONFIG_NAME}\"]\nenabled = {plugin_enabled}\n[plugins.\"{SAMPLE_PLUGIN_CONFIG_NAME}\".mcp_servers.sample]\nenabled = {project_enabled}\ndisabled_tools = [\"echo-tool\"]\n"
),
)?;
set_project_trust_level(codex_home.path(), project.path(), trust_level)?;
// Exercise the real layer loader and trust checks while keeping the test harness's
// mock model provider and automatically selected executor environment.
let layered_config = ConfigBuilder::default()
.codex_home(codex_home.path().to_path_buf())
.fallback_cwd(Some(project.path().to_path_buf()))
.loader_overrides(LoaderOverrides {
system_config_path: Some(system_config_path),
system_requirements_path: Some(requirements_path),
..LoaderOverrides::without_managed_config_for_tests()
})
.build()
.await?;
let mut builder = test_codex()
.with_home(codex_home)
.with_config(move |config| config.config_layer_stack = layered_config.config_layer_stack);
let test = builder.build_with_remote_and_local_env(&server).await?;
let startup = wait_for_event_match(&test.codex, |event| match event {
EventMsg::McpStartupComplete(summary) => Some(summary.clone()),
_ => None,
})
.await;
let expected_ready = if expected_tools.is_empty() {
vec![]
} else {
vec!["sample"]
};
assert_eq!(
serde_json::to_value(startup)?,
serde_json::json!({"ready": expected_ready, "failed": [], "cancelled": []}),
);
test.codex
.start_or_steer_turn(TurnInputRequest::user_input(vec![UserInput::Mention {
name: "sample".into(),
path: format!("plugin://{SAMPLE_PLUGIN_CONFIG_NAME}"),
}]))
.await?;
wait_for_event(&test.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
let requests = mock.requests();
let output = requests[1].tool_search_output(PLUGIN_MCP_SEARCH_CALL_ID);
let mut visible_tools = output["tools"]
.as_array()
.into_iter()
.flatten()
.filter(|namespace| namespace["name"] == SAMPLE_PLUGIN_MCP_NAMESPACE)
.flat_map(|namespace| namespace["tools"].as_array().into_iter().flatten())
.map(|tool| tool["name"].as_str().expect("tool name"))
.collect::<Vec<_>>();
visible_tools.sort_unstable();
assert_eq!(visible_tools, expected_tools);
Ok(())
}
#[derive(Clone, Copy)]
enum ExplicitMcpRequest {
Plugin,