diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index d3eb1df79..4bb91bae9 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -4842,6 +4842,7 @@ impl CodexMessageProcessor { MarketplaceError::InvalidMarketplaceFile { .. } | MarketplaceError::PluginNotFound { .. } | MarketplaceError::PluginNotAvailable { .. } + | MarketplaceError::PluginsDisabled | MarketplaceError::InvalidPlugin(_) => { self.send_invalid_request_error(request_id, err.to_string()) .await; @@ -5363,6 +5364,13 @@ impl CodexMessageProcessor { .extend(valid_extra_roots); } + let config = match self.load_latest_config(/*fallback_cwd*/ None).await { + Ok(config) => config, + Err(error) => { + self.outgoing.send_error(request_id, error).await; + return; + } + }; let skills_manager = self.thread_manager.skills_manager(); let mut data = Vec::new(); for cwd in cwds { @@ -5370,7 +5378,7 @@ impl CodexMessageProcessor { .get(&cwd) .map_or(&[][..], std::vec::Vec::as_slice); let outcome = skills_manager - .skills_for_cwd_with_extra_user_roots(&cwd, force_reload, extra_roots) + .skills_for_cwd_with_extra_user_roots(&cwd, &config, force_reload, extra_roots) .await; let errors = errors_to_info(&outcome.errors); let skills = skills_to_info(&outcome.skills, &outcome.disabled_paths); diff --git a/codex-rs/app-server/tests/suite/v2/plugin_list.rs b/codex-rs/app-server/tests/suite/v2/plugin_list.rs index c409fdeb3..73f4602af 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_list.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_list.rs @@ -28,12 +28,22 @@ use wiremock::matchers::path; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10); const TEST_CURATED_PLUGIN_SHA: &str = "0123456789abcdef0123456789abcdef01234567"; +fn write_plugins_enabled_config(codex_home: &std::path::Path) -> std::io::Result<()> { + std::fs::write( + codex_home.join("config.toml"), + r#"[features] +plugins = true +"#, + ) +} + #[tokio::test] -async fn plugin_list_returns_invalid_request_for_invalid_marketplace_file() -> Result<()> { +async fn plugin_list_skips_invalid_marketplace_file() -> Result<()> { let codex_home = TempDir::new()?; let repo_root = TempDir::new()?; std::fs::create_dir_all(repo_root.path().join(".git"))?; std::fs::create_dir_all(repo_root.path().join(".agents/plugins"))?; + write_plugins_enabled_config(codex_home.path())?; std::fs::write( repo_root.path().join(".agents/plugins/marketplace.json"), "{not json", @@ -57,14 +67,23 @@ async fn plugin_list_returns_invalid_request_for_invalid_marketplace_file() -> R }) .await?; - let err = timeout( + let response: JSONRPCResponse = timeout( DEFAULT_TIMEOUT, - mcp.read_stream_until_error_message(RequestId::Integer(request_id)), + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) .await??; + let response: PluginListResponse = to_response(response)?; - assert_eq!(err.error.code, -32600); - assert!(err.error.message.contains("invalid marketplace file")); + assert!( + response.marketplaces.iter().all(|marketplace| { + marketplace.path + != AbsolutePathBuf::try_from( + repo_root.path().join(".agents/plugins/marketplace.json"), + ) + .expect("absolute marketplace path") + }), + "invalid marketplace should be skipped" + ); Ok(()) } @@ -98,6 +117,7 @@ async fn plugin_list_rejects_relative_cwds() -> Result<()> { async fn plugin_list_accepts_omitted_cwds() -> Result<()> { let codex_home = TempDir::new()?; std::fs::create_dir_all(codex_home.path().join(".agents/plugins"))?; + write_plugins_enabled_config(codex_home.path())?; std::fs::write( codex_home.path().join(".agents/plugins/marketplace.json"), r#"{ @@ -385,6 +405,7 @@ async fn plugin_list_returns_plugin_interface_with_absolute_asset_paths() -> Res std::fs::create_dir_all(repo_root.path().join(".git"))?; std::fs::create_dir_all(repo_root.path().join(".agents/plugins"))?; std::fs::create_dir_all(plugin_root.join(".codex-plugin"))?; + write_plugins_enabled_config(codex_home.path())?; std::fs::write( repo_root.path().join(".agents/plugins/marketplace.json"), r#"{ @@ -518,6 +539,7 @@ async fn plugin_list_accepts_legacy_string_default_prompt() -> Result<()> { std::fs::create_dir_all(repo_root.path().join(".git"))?; std::fs::create_dir_all(repo_root.path().join(".agents/plugins"))?; std::fs::create_dir_all(plugin_root.join(".codex-plugin"))?; + write_plugins_enabled_config(codex_home.path())?; std::fs::write( repo_root.path().join(".agents/plugins/marketplace.json"), r#"{ diff --git a/codex-rs/app-server/tests/suite/v2/plugin_read.rs b/codex-rs/app-server/tests/suite/v2/plugin_read.rs index cbd36c37a..98d0fa8f3 100644 --- a/codex-rs/app-server/tests/suite/v2/plugin_read.rs +++ b/codex-rs/app-server/tests/suite/v2/plugin_read.rs @@ -232,6 +232,7 @@ async fn plugin_read_accepts_legacy_string_default_prompt() -> Result<()> { } }"##, )?; + write_plugins_enabled_config(&codex_home)?; let mut mcp = McpProcess::new(codex_home.path()).await?; timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; @@ -285,6 +286,7 @@ async fn plugin_read_returns_invalid_request_when_plugin_is_missing() -> Result< ] }"#, )?; + write_plugins_enabled_config(&codex_home)?; let mut mcp = McpProcess::new(codex_home.path()).await?; timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; @@ -336,6 +338,7 @@ async fn plugin_read_returns_invalid_request_when_plugin_manifest_is_missing() - ] }"#, )?; + write_plugins_enabled_config(&codex_home)?; let mut mcp = McpProcess::new(codex_home.path()).await?; timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; @@ -382,3 +385,13 @@ fn write_installed_plugin( )?; Ok(()) } + +fn write_plugins_enabled_config(codex_home: &TempDir) -> Result<()> { + std::fs::write( + codex_home.path().join("config.toml"), + r#"[features] +plugins = true +"#, + )?; + Ok(()) +} diff --git a/codex-rs/config/src/state.rs b/codex-rs/config/src/state.rs index cb9b1a590..f6899651b 100644 --- a/codex-rs/config/src/state.rs +++ b/codex-rs/config/src/state.rs @@ -148,7 +148,9 @@ impl ConfigLayerStack { }) } - /// Returns the user config layer, if any. + /// Returns the raw user config layer, if any. + /// + /// This does not merge other config layers or apply any requirements. pub fn get_user_layer(&self) -> Option<&ConfigLayerEntry> { self.user_layer_index .and_then(|index| self.layers.get(index)) @@ -209,6 +211,10 @@ impl ConfigLayerStack { } } + /// Returns the merged config-layer view. + /// + /// This only merges ordinary config layers and does not apply requirements + /// such as cloud requirements. pub fn effective_config(&self) -> TomlValue { let mut merged = TomlValue::Table(toml::map::Map::new()); for layer in self.get_layers( @@ -220,6 +226,9 @@ impl ConfigLayerStack { merged } + /// Returns field origins for the merged config-layer view. + /// + /// Requirement sources are tracked separately and are not included here. pub fn origins(&self) -> HashMap { let mut origins = HashMap::new(); let mut path = Vec::new(); @@ -234,8 +243,9 @@ impl ConfigLayerStack { origins } - /// Returns the highest-precedence to lowest-precedence layers, so - /// `ConfigLayerSource::SessionFlags` would be first, if present. + /// Returns config layers from highest precedence to lowest precedence. + /// + /// Requirement sources are tracked separately and are not included here. pub fn layers_high_to_low(&self) -> Vec<&ConfigLayerEntry> { self.get_layers( ConfigLayerStackOrdering::HighestPrecedenceFirst, @@ -243,8 +253,9 @@ impl ConfigLayerStack { ) } - /// Returns the highest-precedence to lowest-precedence layers, so - /// `ConfigLayerSource::SessionFlags` would be first, if present. + /// Returns config layers in the requested precedence order. + /// + /// Requirement sources are tracked separately and are not included here. pub fn get_layers( &self, ordering: ConfigLayerStackOrdering, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index efde7d848..a916f3311 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -4781,9 +4781,12 @@ mod handlers { }; let skills_manager = &sess.services.skills_manager; + let config = sess.get_config().await; let mut skills = Vec::new(); for cwd in cwds { - let outcome = skills_manager.skills_for_cwd(&cwd, force_reload).await; + let outcome = skills_manager + .skills_for_cwd(&cwd, config.as_ref(), force_reload) + .await; let errors = super::errors_to_info(&outcome.errors); let skills_metadata = super::skills_to_info(&outcome.skills, &outcome.disabled_paths); skills.push(SkillsListEntry { diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index a7f5b72ea..89d14e37e 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -2187,7 +2187,7 @@ async fn new_default_turn_uses_config_aware_skills_for_role_overrides() { let parent_outcome = session .services .skills_manager - .skills_for_cwd(&parent_config.cwd, true) + .skills_for_cwd(&parent_config.cwd, &parent_config, true) .await; let parent_skill = parent_outcome .skills diff --git a/codex-rs/core/src/plugins/manager.rs b/codex-rs/core/src/plugins/manager.rs index 22c536f21..5c65f1024 100644 --- a/codex-rs/core/src/plugins/manager.rs +++ b/codex-rs/core/src/plugins/manager.rs @@ -29,16 +29,12 @@ use crate::auth::CodexAuth; use crate::config::Config; use crate::config::ConfigService; use crate::config::ConfigServiceError; -use crate::config::ConfigToml; use crate::config::edit::ConfigEdit; use crate::config::edit::ConfigEditsBuilder; -use crate::config::profile::ConfigProfile; use crate::config::types::McpServerConfig; use crate::config::types::PluginConfig; use crate::config_loader::ConfigLayerStack; use crate::features::Feature; -use crate::features::FeatureOverrides; -use crate::features::Features; use crate::skills::SkillMetadata; use crate::skills::loader::SkillRoot; use crate::skills::loader::load_skills_from_roots; @@ -421,7 +417,7 @@ impl From for PluginRemoteSyncError { pub struct PluginsManager { codex_home: PathBuf, store: PluginStore, - cache_by_cwd: RwLock>, + cached_enabled_outcome: RwLock>, analytics_events_client: RwLock>, } @@ -430,7 +426,7 @@ impl PluginsManager { Self { codex_home: codex_home.clone(), store: PluginStore::new(codex_home), - cache_by_cwd: RwLock::new(HashMap::new()), + cached_enabled_outcome: RwLock::new(None), analytics_events_client: RwLock::new(None), } } @@ -444,49 +440,44 @@ impl PluginsManager { } pub fn plugins_for_config(&self, config: &Config) -> PluginLoadOutcome { - self.plugins_for_layer_stack( - &config.cwd, - &config.config_layer_stack, - /*force_reload*/ false, - ) + self.plugins_for_config_with_force_reload(config, /*force_reload*/ false) } - pub fn plugins_for_layer_stack( + pub(crate) fn plugins_for_config_with_force_reload( &self, - cwd: &Path, - config_layer_stack: &ConfigLayerStack, + config: &Config, force_reload: bool, ) -> PluginLoadOutcome { - if !plugins_feature_enabled_from_stack(config_layer_stack) { + if !config.features.enabled(Feature::Plugins) { return PluginLoadOutcome::default(); } - if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(cwd) { + if !force_reload && let Some(outcome) = self.cached_enabled_outcome() { return outcome; } - let outcome = load_plugins_from_layer_stack(config_layer_stack, &self.store); + let outcome = load_plugins_from_layer_stack(&config.config_layer_stack, &self.store); log_plugin_load_errors(&outcome); - let mut cache = match self.cache_by_cwd.write() { + let mut cache = match self.cached_enabled_outcome.write() { Ok(cache) => cache, Err(err) => err.into_inner(), }; - cache.insert(cwd.to_path_buf(), outcome.clone()); + *cache = Some(outcome.clone()); outcome } pub fn clear_cache(&self) { - let mut cache_by_cwd = match self.cache_by_cwd.write() { + let mut cached_enabled_outcome = match self.cached_enabled_outcome.write() { Ok(cache) => cache, Err(err) => err.into_inner(), }; - cache_by_cwd.clear(); + *cached_enabled_outcome = None; } - fn cached_outcome_for_cwd(&self, cwd: &Path) -> Option { - match self.cache_by_cwd.read() { - Ok(cache) => cache.get(cwd).cloned(), - Err(err) => err.into_inner().get(cwd).cloned(), + fn cached_enabled_outcome(&self) -> Option { + match self.cached_enabled_outcome.read() { + Ok(cache) => cache.clone(), + Err(err) => err.into_inner().clone(), } } @@ -634,6 +625,10 @@ impl PluginsManager { config: &Config, auth: Option<&CodexAuth>, ) -> Result { + if !config.features.enabled(Feature::Plugins) { + return Ok(RemotePluginSyncResult::default()); + } + info!("starting remote plugin sync"); let remote_plugins = fetch_remote_plugin_status(config, auth) .await @@ -817,7 +812,11 @@ impl PluginsManager { config: &Config, additional_roots: &[AbsolutePathBuf], ) -> Result, MarketplaceError> { - let (installed_plugins, configured_plugins) = self.configured_plugin_states(config); + if !config.features.enabled(Feature::Plugins) { + return Ok(Vec::new()); + } + + let (installed_plugins, enabled_plugins) = self.configured_plugin_states(config); let marketplaces = list_marketplaces(&self.marketplace_roots(additional_roots))?; let mut seen_plugin_keys = HashSet::new(); @@ -840,10 +839,7 @@ impl PluginsManager { // resolve to the first discovered source. id: plugin_key.clone(), installed: installed_plugins.contains(&plugin_key), - enabled: configured_plugins - .get(&plugin_key) - .copied() - .unwrap_or(false), + enabled: enabled_plugins.contains(&plugin_key), name: plugin.name, source: plugin.source, policy: plugin.policy, @@ -867,6 +863,10 @@ impl PluginsManager { config: &Config, request: &PluginReadRequest, ) -> Result { + if !config.features.enabled(Feature::Plugins) { + return Err(MarketplaceError::PluginsDisabled); + } + let marketplace = load_marketplace(&request.marketplace_path)?; let marketplace_name = marketplace.name.clone(); let plugin = marketplace @@ -886,7 +886,7 @@ impl PluginsManager { }, )?; let plugin_key = plugin_id.as_key(); - let (installed_plugins, configured_plugins) = self.configured_plugin_states(config); + let (installed_plugins, enabled_plugins) = self.configured_plugin_states(config); let source_path = match &plugin.source { MarketplacePluginSource::Local { path } => path.clone(), }; @@ -927,10 +927,7 @@ impl PluginsManager { policy: plugin.policy, interface: plugin.interface, installed: installed_plugins.contains(&plugin_key), - enabled: configured_plugins - .get(&plugin_key) - .copied() - .unwrap_or(false), + enabled: enabled_plugins.contains(&plugin_key), skills, apps, mcp_server_names, @@ -939,7 +936,7 @@ impl PluginsManager { } pub fn maybe_start_curated_repo_sync_for_config(self: &Arc, config: &Config) { - if plugins_feature_enabled_from_stack(&config.config_layer_stack) { + if config.features.enabled(Feature::Plugins) { let mut configured_curated_plugin_ids = configured_plugins_from_stack(&config.config_layer_stack) .into_keys() @@ -1005,25 +1002,22 @@ impl PluginsManager { } } - fn configured_plugin_states( - &self, - config: &Config, - ) -> (HashSet, HashMap) { - let installed_plugins = configured_plugins_from_stack(&config.config_layer_stack) - .into_keys() + fn configured_plugin_states(&self, config: &Config) -> (HashSet, HashSet) { + let configured_plugins = configured_plugins_from_stack(&config.config_layer_stack); + let installed_plugins = configured_plugins + .keys() .filter(|plugin_key| { PluginId::parse(plugin_key) .ok() .is_some_and(|plugin_id| self.store.is_installed(&plugin_id)) }) + .cloned() .collect::>(); - let configured_plugins = self - .plugins_for_config(config) - .plugins() - .iter() - .map(|plugin| (plugin.config_name.clone(), plugin.enabled)) - .collect::>(); - (installed_plugins, configured_plugins) + let enabled_plugins = configured_plugins + .into_iter() + .filter_map(|(plugin_key, plugin)| plugin.enabled.then_some(plugin_key)) + .collect::>(); + (installed_plugins, enabled_plugins) } fn marketplace_roots(&self, additional_roots: &[AbsolutePathBuf]) -> Vec { @@ -1107,24 +1101,6 @@ impl PluginUninstallError { } } -fn plugins_feature_enabled_from_stack(config_layer_stack: &ConfigLayerStack) -> bool { - // Plugins are intentionally opt-in from the persisted user config only. Project config - // layers should not be able to enable plugin loading for a checkout. - let Some(user_layer) = config_layer_stack.get_user_layer() else { - return false; - }; - let Ok(config_toml) = user_layer.config.clone().try_into::() else { - warn!("failed to deserialize config when checking plugin feature flag"); - return false; - }; - let config_profile = config_toml - .get_config_profile(config_toml.profile.clone()) - .unwrap_or_else(|_| ConfigProfile::default()); - let features = - Features::from_config(&config_toml, &config_profile, FeatureOverrides::default()); - features.enabled(Feature::Plugins) -} - fn log_plugin_load_errors(outcome: &PluginLoadOutcome) { for plugin in outcome .plugins @@ -1263,7 +1239,7 @@ fn refresh_curated_plugin_cache( fn configured_plugins_from_stack( config_layer_stack: &ConfigLayerStack, ) -> HashMap { - // Keep plugin entries aligned with the same user-layer-only semantics as the feature gate. + // Plugin entries remain persisted user config only. let Some(user_layer) = config_layer_stack.get_user_layer() else { return HashMap::new(); }; diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index d113d56a9..49a63eee4 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -59,18 +59,8 @@ fn plugin_config_toml(enabled: bool, plugins_feature_enabled: bool) -> String { fn load_plugins_from_config(config_toml: &str, codex_home: &Path) -> PluginLoadOutcome { write_file(&codex_home.join(CONFIG_TOML_FILE), config_toml); - let stack = ConfigLayerStack::new( - vec![ConfigLayerEntry::new( - ConfigLayerSource::User { - file: AbsolutePathBuf::try_from(codex_home.join(CONFIG_TOML_FILE)).unwrap(), - }, - toml::from_str(config_toml).expect("plugin test config should parse"), - )], - ConfigRequirements::default(), - ConfigRequirementsToml::default(), - ) - .expect("config layer stack should build"); - PluginsManager::new(codex_home.to_path_buf()).plugins_for_layer_stack(codex_home, &stack, false) + let config = load_config_blocking(codex_home, codex_home); + PluginsManager::new(codex_home.to_path_buf()).plugins_for_config(&config) } async fn load_config(codex_home: &Path, cwd: &Path) -> crate::config::Config { @@ -82,6 +72,14 @@ async fn load_config(codex_home: &Path, cwd: &Path) -> crate::config::Config { .expect("config should load") } +fn load_config_blocking(codex_home: &Path, cwd: &Path) -> crate::config::Config { + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("tokio runtime should build") + .block_on(load_config(codex_home, cwd)) +} + #[test] fn load_plugins_loads_default_skills_and_mcp_servers() { let codex_home = TempDir::new().unwrap(); @@ -749,8 +747,13 @@ fn load_plugins_returns_empty_when_feature_disabled() { &plugin_root.join("skills/sample-search/SKILL.md"), "---\nname: sample-search\ndescription: search sample data\n---\n", ); + write_file( + &codex_home.path().join(CONFIG_TOML_FILE), + &plugin_config_toml(true, false), + ); - let outcome = load_plugins_from_config(&plugin_config_toml(true, false), codex_home.path()); + let config = load_config_blocking(codex_home.path(), codex_home.path()); + let outcome = PluginsManager::new(codex_home.path().to_path_buf()).plugins_for_config(&config); assert_eq!(outcome, PluginLoadOutcome::default()); } @@ -1000,12 +1003,125 @@ enabled = false ); } +#[tokio::test] +async fn list_marketplaces_returns_empty_when_feature_disabled() { + let tmp = tempfile::tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + fs::create_dir_all(repo_root.join(".git")).unwrap(); + fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap(); + fs::write( + repo_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "debug", + "plugins": [ + { + "name": "enabled-plugin", + "source": { + "source": "local", + "path": "./enabled-plugin" + } + } + ] +}"#, + ) + .unwrap(); + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = false + +[plugins."enabled-plugin@debug"] +enabled = true +"#, + ); + + let config = load_config(tmp.path(), &repo_root).await; + let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + .list_marketplaces_for_config(&config, &[AbsolutePathBuf::try_from(repo_root).unwrap()]) + .unwrap(); + + assert_eq!(marketplaces, Vec::new()); +} + +#[tokio::test] +async fn read_plugin_for_config_returns_plugins_disabled_when_feature_disabled() { + let tmp = tempfile::tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + fs::create_dir_all(repo_root.join(".git")).unwrap(); + fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap(); + let marketplace_path = + AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(); + fs::write( + marketplace_path.as_path(), + r#"{ + "name": "debug", + "plugins": [ + { + "name": "enabled-plugin", + "source": { + "source": "local", + "path": "./enabled-plugin" + } + } + ] +}"#, + ) + .unwrap(); + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = false + +[plugins."enabled-plugin@debug"] +enabled = true +"#, + ); + + let config = load_config(tmp.path(), &repo_root).await; + let err = PluginsManager::new(tmp.path().to_path_buf()) + .read_plugin_for_config( + &config, + &PluginReadRequest { + plugin_name: "enabled-plugin".to_string(), + marketplace_path, + }, + ) + .unwrap_err(); + + assert!(matches!(err, MarketplaceError::PluginsDisabled)); +} + +#[tokio::test] +async fn sync_plugins_from_remote_returns_default_when_feature_disabled() { + let tmp = tempfile::tempdir().unwrap(); + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = false +"#, + ); + + let config = load_config(tmp.path(), tmp.path()).await; + let outcome = PluginsManager::new(tmp.path().to_path_buf()) + .sync_plugins_from_remote(&config, None) + .await + .unwrap(); + + assert_eq!(outcome, RemotePluginSyncResult::default()); +} + #[tokio::test] async fn list_marketplaces_includes_curated_repo_marketplace() { let tmp = tempfile::tempdir().unwrap(); let curated_root = curated_plugins_repo_path(tmp.path()); let plugin_root = curated_root.join("plugins/linear"); + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +"#, + ); fs::create_dir_all(curated_root.join(".agents/plugins")).unwrap(); fs::create_dir_all(plugin_root.join(".codex-plugin")).unwrap(); fs::write( @@ -1723,11 +1839,8 @@ fn load_plugins_ignores_project_config_files() { ) .expect("config layer stack should build"); - let outcome = PluginsManager::new(codex_home.path().to_path_buf()).plugins_for_layer_stack( - &project_root, - &stack, - false, - ); + let outcome = + load_plugins_from_layer_stack(&stack, &PluginStore::new(codex_home.path().to_path_buf())); assert_eq!(outcome, PluginLoadOutcome::default()); } diff --git a/codex-rs/core/src/plugins/marketplace.rs b/codex-rs/core/src/plugins/marketplace.rs index ff6bdecc8..aee612d71 100644 --- a/codex-rs/core/src/plugins/marketplace.rs +++ b/codex-rs/core/src/plugins/marketplace.rs @@ -14,6 +14,7 @@ use std::io; use std::path::Component; use std::path::Path; use std::path::PathBuf; +use tracing::warn; const MARKETPLACE_RELATIVE_PATH: &str = ".agents/plugins/marketplace.json"; @@ -127,6 +128,9 @@ pub enum MarketplaceError { marketplace_name: String, }, + #[error("plugins feature is disabled")] + PluginsDisabled, + #[error("{0}")] InvalidPlugin(String), } @@ -238,7 +242,16 @@ fn list_marketplaces_with_home( let mut marketplaces = Vec::new(); for marketplace_path in discover_marketplace_paths_from_roots(additional_roots, home_dir) { - marketplaces.push(load_marketplace(&marketplace_path)?); + match load_marketplace(&marketplace_path) { + Ok(marketplace) => marketplaces.push(marketplace), + Err(err) => { + warn!( + path = %marketplace_path.display(), + error = %err, + "skipping marketplace that failed to load" + ); + } + } } Ok(marketplaces) diff --git a/codex-rs/core/src/plugins/marketplace_tests.rs b/codex-rs/core/src/plugins/marketplace_tests.rs index 2516419ed..bfdce5e18 100644 --- a/codex-rs/core/src/plugins/marketplace_tests.rs +++ b/codex-rs/core/src/plugins/marketplace_tests.rs @@ -403,6 +403,62 @@ fn list_marketplaces_reads_marketplace_display_name() { ); } +#[test] +fn list_marketplaces_skips_marketplaces_that_fail_to_load() { + let tmp = tempdir().unwrap(); + let valid_repo_root = tmp.path().join("valid-repo"); + let invalid_repo_root = tmp.path().join("invalid-repo"); + + fs::create_dir_all(valid_repo_root.join(".git")).unwrap(); + fs::create_dir_all(valid_repo_root.join(".agents/plugins")).unwrap(); + fs::create_dir_all(invalid_repo_root.join(".git")).unwrap(); + fs::create_dir_all(invalid_repo_root.join(".agents/plugins")).unwrap(); + fs::write( + valid_repo_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "valid-marketplace", + "plugins": [ + { + "name": "valid-plugin", + "source": { + "source": "local", + "path": "./plugin" + } + } + ] +}"#, + ) + .unwrap(); + fs::write( + invalid_repo_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "invalid-marketplace", + "plugins": [ + { + "name": "broken-plugin", + "source": { + "source": "local", + "path": "plugin-without-dot-slash" + } + } + ] +}"#, + ) + .unwrap(); + + let marketplaces = list_marketplaces_with_home( + &[ + AbsolutePathBuf::try_from(valid_repo_root).unwrap(), + AbsolutePathBuf::try_from(invalid_repo_root).unwrap(), + ], + None, + ) + .unwrap(); + + assert_eq!(marketplaces.len(), 1); + assert_eq!(marketplaces[0].name, "valid-marketplace"); +} + #[test] fn list_marketplaces_resolves_plugin_interface_paths_to_absolute() { let tmp = tempdir().unwrap(); diff --git a/codex-rs/core/src/skills/manager.rs b/codex-rs/core/src/skills/manager.rs index c8354fed3..7aa3e6d7a 100644 --- a/codex-rs/core/src/skills/manager.rs +++ b/codex-rs/core/src/skills/manager.rs @@ -92,18 +92,24 @@ impl SkillsManager { roots } - pub async fn skills_for_cwd(&self, cwd: &Path, force_reload: bool) -> SkillLoadOutcome { + pub async fn skills_for_cwd( + &self, + cwd: &Path, + config: &Config, + force_reload: bool, + ) -> SkillLoadOutcome { if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(cwd) { return outcome; } - self.skills_for_cwd_with_extra_user_roots(cwd, force_reload, &[]) + self.skills_for_cwd_with_extra_user_roots(cwd, config, force_reload, &[]) .await } pub async fn skills_for_cwd_with_extra_user_roots( &self, cwd: &Path, + config: &Config, force_reload: bool, extra_user_roots: &[PathBuf], ) -> SkillLoadOutcome { @@ -147,9 +153,9 @@ impl SkillsManager { } }; - let loaded_plugins = - self.plugins_manager - .plugins_for_layer_stack(cwd, &config_layer_stack, force_reload); + let loaded_plugins = self + .plugins_manager + .plugins_for_config_with_force_reload(config, force_reload); let mut roots = skill_roots( &config_layer_stack, cwd, diff --git a/codex-rs/core/src/skills/manager_tests.rs b/codex-rs/core/src/skills/manager_tests.rs index 98ad9627b..cb3c48ed7 100644 --- a/codex-rs/core/src/skills/manager_tests.rs +++ b/codex-rs/core/src/skills/manager_tests.rs @@ -93,6 +93,7 @@ async fn skills_for_cwd_reuses_cached_entry_even_when_entry_has_extra_roots() { let outcome_with_extra = skills_manager .skills_for_cwd_with_extra_user_roots( cwd.path(), + &config, true, std::slice::from_ref(&extra_root_path), ) @@ -112,7 +113,9 @@ async fn skills_for_cwd_reuses_cached_entry_even_when_entry_has_extra_roots() { // The cwd-only API returns the current cached entry for this cwd, even when that entry // was produced with extra roots. - let outcome_without_extra = skills_manager.skills_for_cwd(cwd.path(), false).await; + let outcome_without_extra = skills_manager + .skills_for_cwd(cwd.path(), &config, false) + .await; assert_eq!(outcome_without_extra.skills, outcome_with_extra.skills); assert_eq!(outcome_without_extra.errors, outcome_with_extra.errors); } @@ -204,6 +207,7 @@ async fn skills_for_cwd_with_extra_roots_only_refreshes_on_force_reload() { let outcome_a = skills_manager .skills_for_cwd_with_extra_user_roots( cwd.path(), + &config, true, std::slice::from_ref(&extra_root_a_path), ) @@ -225,6 +229,7 @@ async fn skills_for_cwd_with_extra_roots_only_refreshes_on_force_reload() { let outcome_b = skills_manager .skills_for_cwd_with_extra_user_roots( cwd.path(), + &config, false, std::slice::from_ref(&extra_root_b_path), ) @@ -245,6 +250,7 @@ async fn skills_for_cwd_with_extra_roots_only_refreshes_on_force_reload() { let outcome_reloaded = skills_manager .skills_for_cwd_with_extra_user_roots( cwd.path(), + &config, true, std::slice::from_ref(&extra_root_b_path), ) @@ -417,7 +423,9 @@ enabled = true let plugins_manager = Arc::new(PluginsManager::new(codex_home.path().to_path_buf())); let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), plugins_manager, true); - let parent_outcome = skills_manager.skills_for_cwd(cwd.path(), true).await; + let parent_outcome = skills_manager + .skills_for_cwd(cwd.path(), &parent_config, true) + .await; let parent_skill = parent_outcome .skills .iter() diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs index 89bf4d424..afad1da2a 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -489,7 +489,7 @@ impl CoreShellActionProvider { .session .services .skills_manager - .skills_for_cwd(&self.turn.cwd, force_reload) + .skills_for_cwd(&self.turn.cwd, self.turn.config.as_ref(), force_reload) .await; let program_path = program.as_path();