diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 65f3a64328..0fe4c255ae 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2863,7 +2863,6 @@ dependencies = [ "codex-app-server-protocol", "codex-config", "codex-connectors", - "codex-core-skills", "codex-exec-server", "codex-exec-server-test-support", "codex-git-utils", @@ -2912,7 +2911,6 @@ name = "codex-core-skills" version = "0.0.0" dependencies = [ "codex-analytics", - "codex-config", "codex-context-fragments", "codex-exec-server", "codex-otel", @@ -2921,8 +2919,6 @@ dependencies = [ "codex-utils-path-uri", "codex-utils-string", "pretty_assertions", - "tempfile", - "toml 0.9.11+spec-1.1.0", "tracing", ] diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index dec1829056..395764de8d 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -153,7 +153,11 @@ pub use requirements_layers::compose_requirements; pub use shell_environment_policy::validate_shell_environment_policy_filter_config; pub use skills_config::BundledSkillsConfig; pub use skills_config::SkillConfig; +pub use skills_config::SkillConfigRule; +pub use skills_config::SkillConfigRuleSelector; +pub use skills_config::SkillConfigRules; pub use skills_config::SkillsConfig; +pub use skills_config::skill_config_rules_from_stack; pub use state::ConfigLayerEntry; pub use state::ConfigLayerStack; pub use state::ConfigLoadOptions; diff --git a/codex-rs/config/src/skills_config.rs b/codex-rs/config/src/skills_config.rs index 948b9060c4..09f482df02 100644 --- a/codex-rs/config/src/skills_config.rs +++ b/codex-rs/config/src/skills_config.rs @@ -1,9 +1,14 @@ //! Skill-related configuration types shared across crates. +use std::collections::HashSet; + +use crate::ConfigLayerSource; +use crate::ConfigLayerStack; use codex_utils_absolute_path::AbsolutePathBuf; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; +use tracing::warn; const fn default_enabled() -> bool { true @@ -55,3 +60,129 @@ impl TryFrom for SkillsConfig { SkillsConfig::deserialize(value) } } + +/// Selects configured skills by their name or canonical document path. +#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum SkillConfigRuleSelector { + Name(String), + Path(AbsolutePathBuf), +} + +/// Enables or disables every skill matched by its selector. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct SkillConfigRule { + pub selector: SkillConfigRuleSelector, + pub enabled: bool, +} + +/// Ordered effective skill enablement rules from configuration layers. +#[derive(Debug, Clone, Default, PartialEq, Eq, Hash)] +pub struct SkillConfigRules { + pub entries: Vec, +} + +impl SkillConfigRules { + /// Applies rules in order; later rules override earlier rules for matching skills. + /// + /// Explicit path selectors remain effective even when no current skill matches. + pub fn resolve_disabled_paths<'a>( + &self, + skills: impl IntoIterator + Clone, + ) -> HashSet { + let mut disabled_paths = HashSet::new(); + + for entry in &self.entries { + match &entry.selector { + SkillConfigRuleSelector::Path(path) => { + if entry.enabled { + disabled_paths.remove(path); + } else { + disabled_paths.insert(path.clone()); + } + } + SkillConfigRuleSelector::Name(name) => { + for (skill_name, path) in skills.clone() { + if skill_name != name { + continue; + } + if entry.enabled { + disabled_paths.remove(path); + } else { + disabled_paths.insert(path.clone()); + } + } + } + } + } + + disabled_paths + } +} + +/// Resolves skill enablement rules from user and session configuration layers. +pub fn skill_config_rules_from_stack(config_layer_stack: &ConfigLayerStack) -> SkillConfigRules { + let mut entries = Vec::new(); + for layer in config_layer_stack.all_layers_low_to_high() { + if !matches!( + layer.name, + ConfigLayerSource::User { .. } | ConfigLayerSource::SessionFlags + ) { + continue; + } + + let Some(skills_value) = layer.config.get("skills") else { + continue; + }; + let skills: SkillsConfig = match skills_value.clone().try_into() { + Ok(skills) => skills, + Err(err) => { + warn!("invalid skills config: {err}"); + continue; + } + }; + + for entry in skills.config { + let Some(selector) = skill_config_rule_selector(&entry) else { + continue; + }; + // Preserve layer order so a later name selector can override an earlier path selector + // for the same loaded skill. + entries.retain(|entry: &SkillConfigRule| entry.selector != selector); + entries.push(SkillConfigRule { + selector, + enabled: entry.enabled, + }); + } + } + + SkillConfigRules { entries } +} + +fn skill_config_rule_selector(entry: &SkillConfig) -> Option { + match (entry.path.as_ref(), entry.name.as_deref()) { + (Some(path), None) => Some(SkillConfigRuleSelector::Path( + path.canonicalize().unwrap_or_else(|_| path.clone()), + )), + (None, Some(name)) => { + let name = name.trim(); + if name.is_empty() { + warn!("ignoring empty skills.config name override"); + None + } else { + Some(SkillConfigRuleSelector::Name(name.to_string())) + } + } + (Some(_), Some(_)) => { + warn!("ignoring skills.config entry with both path and name selectors"); + None + } + (None, None) => { + warn!("ignoring skills.config entry without a path or name selector"); + None + } + } +} + +#[cfg(test)] +#[path = "skills_config_tests.rs"] +mod tests; diff --git a/codex-rs/core-skills/src/config_rules_tests.rs b/codex-rs/config/src/skills_config_tests.rs similarity index 65% rename from codex-rs/core-skills/src/config_rules_tests.rs rename to codex-rs/config/src/skills_config_tests.rs index 82fcb2cc32..6615f62c85 100644 --- a/codex-rs/core-skills/src/config_rules_tests.rs +++ b/codex-rs/config/src/skills_config_tests.rs @@ -1,8 +1,8 @@ -use codex_config::CONFIG_TOML_FILE; -use codex_config::ConfigLayerEntry; -use codex_config::ConfigLayerSource; -use codex_config::ConfigLayerStack; -use codex_config::ConfigRequirementsToml; +use crate::CONFIG_TOML_FILE; +use crate::ConfigLayerEntry; +use crate::ConfigLayerSource; +use crate::ConfigLayerStack; +use crate::ConfigRequirementsToml; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::test_support::PathBufExt; use pretty_assertions::assert_eq; @@ -145,3 +145,68 @@ enabled = true } ); } + +#[test] +fn path_rule_disables_selected_path() { + let codex_home = TempDir::new().expect("temp dir"); + let path = codex_home.path().join("disable-by-path/SKILL.md").abs(); + let rules = SkillConfigRules { + entries: vec![SkillConfigRule { + selector: SkillConfigRuleSelector::Path(path.clone()), + enabled: false, + }], + }; + + assert_eq!( + rules.resolve_disabled_paths(std::iter::empty()), + [path].into_iter().collect() + ); +} + +#[test] +fn later_name_rule_reenables_path_disabled_skill() { + let codex_home = TempDir::new().expect("temp dir"); + let path = codex_home.path().join("reenable-by-name/SKILL.md").abs(); + let rules = SkillConfigRules { + entries: vec![ + SkillConfigRule { + selector: SkillConfigRuleSelector::Path(path.clone()), + enabled: false, + }, + SkillConfigRule { + selector: SkillConfigRuleSelector::Name("demo".to_string()), + enabled: true, + }, + ], + }; + + assert_eq!( + rules.resolve_disabled_paths([("demo", &path)]), + Default::default() + ); +} + +#[test] +fn later_path_rule_reenables_one_skill_disabled_by_name() { + let codex_home = TempDir::new().expect("temp dir"); + let root = codex_home.path().join("reenable-by-path"); + let first_path = root.join("first/SKILL.md").abs(); + let second_path = root.join("second/SKILL.md").abs(); + let rules = SkillConfigRules { + entries: vec![ + SkillConfigRule { + selector: SkillConfigRuleSelector::Name("demo".to_string()), + enabled: false, + }, + SkillConfigRule { + selector: SkillConfigRuleSelector::Path(first_path.clone()), + enabled: true, + }, + ], + }; + + assert_eq!( + rules.resolve_disabled_paths([("demo", &first_path), ("demo", &second_path)]), + [second_path].into_iter().collect() + ); +} diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml index 3c3526f061..56cfd12a92 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -18,7 +18,6 @@ codex-analytics = { workspace = true } codex-app-server-protocol = { workspace = true } codex-config = { workspace = true } codex-connectors = { workspace = true } -codex-core-skills = { workspace = true } codex-exec-server = { workspace = true } codex-git-utils = { workspace = true } codex-hooks = { workspace = true } diff --git a/codex-rs/core-plugins/src/discoverable.rs b/codex-rs/core-plugins/src/discoverable.rs index eb1df6ff52..baad28b53f 100644 --- a/codex-rs/core-plugins/src/discoverable.rs +++ b/codex-rs/core-plugins/src/discoverable.rs @@ -1,7 +1,7 @@ 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_config::skill_config_rules_from_stack; use codex_login::CodexAuth; use codex_plugin::PluginId; use std::collections::HashSet; diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 4c399c32f6..51c61123c1 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -22,14 +22,14 @@ use crate::store::plugin_version_for_source; use crate::store::plugin_version_for_source_with_fallback_manifest; use codex_config::ConfigLayerStack; use codex_config::HooksFile; +use codex_config::SkillConfigRules; +use codex_config::skill_config_rules_from_stack; use codex_config::types::McpServerConfig; use codex_config::types::McpServerTransportConfig; use codex_config::types::PluginConfig; use codex_config::types::PluginMcpServerConfig; use codex_connectors::parse_plugin_app_config; use codex_connectors::parse_plugin_app_config_value; -use codex_core_skills::config_rules::resolve_disabled_skill_paths; -use codex_core_skills::config_rules::skill_config_rules_from_stack; use codex_mcp::parse_agent_plugin_mcp_config; use codex_mcp::parse_plugin_mcp_config; use codex_plugin::AppDeclaration; @@ -41,7 +41,6 @@ use codex_plugin::PluginIdError; use codex_plugin::app_connector_ids_from_declarations; use codex_protocol::auth::AuthMode; use codex_protocol::protocol::Product; -use codex_skills::SkillConfigRules; use codex_skills::SkillMetadata; use codex_skills::SkillRootLoadRequest; use codex_skills::SkillRootLoader; @@ -976,12 +975,20 @@ 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), + &skill_config_rules.resolve_disabled_paths( + self.skills + .iter() + .map(|skill| (skill.name.as_str(), &skill.path_to_skills_md)), + ), ) } pub(crate) fn resolve(self, skill_config_rules: &SkillConfigRules) -> ResolvedPluginSkills { - let disabled_skill_paths = resolve_disabled_skill_paths(&self.skills, skill_config_rules); + let disabled_skill_paths = skill_config_rules.resolve_disabled_paths( + self.skills + .iter() + .map(|skill| (skill.name.as_str(), &skill.path_to_skills_md)), + ); ResolvedPluginSkills { skills: self.skills, disabled_skill_paths, diff --git a/codex-rs/core-plugins/src/manager.rs b/codex-rs/core-plugins/src/manager.rs index 087b846e90..1519f6a0c2 100644 --- a/codex-rs/core-plugins/src/manager.rs +++ b/codex-rs/core-plugins/src/manager.rs @@ -70,12 +70,13 @@ use crate::tool_suggest_metadata::ToolSuggestMetadataCache; use codex_analytics::AnalyticsEventsClient; use codex_analytics::PluginInstallSource; use codex_config::ConfigLayerStack; +use codex_config::SkillConfigRules; use codex_config::clear_user_plugin; use codex_config::set_user_plugin_enabled; +use codex_config::skill_config_rules_from_stack; use codex_config::types::PluginConfig; use codex_config::types::ToolSuggestDisabledTool; use codex_config::types::ToolSuggestDiscoverableType; -use codex_core_skills::config_rules::skill_config_rules_from_stack; use codex_hooks::plugin_hook_declarations; use codex_http_client::HttpClientFactory; use codex_login::AuthManager; @@ -91,7 +92,6 @@ use codex_plugin::prompt_safe_plugin_description; use codex_protocol::auth::AuthMode; use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::Product; -use codex_skills::SkillConfigRules; use codex_skills::SkillMetadata; use codex_skills::SkillRootLoader; use codex_skills::SkillRootSnapshots; @@ -2055,9 +2055,7 @@ impl PluginsManager { plugin_id: plugin_id.as_key(), remote_plugin_id: self.remote_plugin_id_for(&plugin_id), }; - let skill_config_rules = codex_core_skills::config_rules::skill_config_rules_from_stack( - &config.config_layer_stack, - ); + let skill_config_rules = skill_config_rules_from_stack(&config.config_layer_stack); let resolved_skills = load_plugin_skill_inventory( &source_path, &plugin_identity, diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 0c28a217a7..8e98d767ae 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -41,6 +41,7 @@ use codex_config::McpServerOAuthConfig; use codex_config::McpServerToolConfig; use codex_config::RequirementSource; use codex_config::RequirementsLayerEntry; +use codex_config::SkillConfigRules; use codex_config::compose_requirements; use codex_config::types::McpServerTransportConfig; use codex_login::CodexAuth; @@ -49,7 +50,6 @@ use codex_plugin::PluginId; use codex_protocol::auth::AuthMode; use codex_protocol::protocol::HookEventName; use codex_protocol::protocol::Product; -use codex_skills::SkillConfigRules; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::test_support::PathBufExt; use codex_utils_plugins::SkillDiscoveryMode; diff --git a/codex-rs/core-plugins/src/tool_suggest_metadata.rs b/codex-rs/core-plugins/src/tool_suggest_metadata.rs index 28671050a5..d0b367f96c 100644 --- a/codex-rs/core-plugins/src/tool_suggest_metadata.rs +++ b/codex-rs/core-plugins/src/tool_suggest_metadata.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use std::sync::Arc; use std::sync::RwLock; +use codex_config::SkillConfigRules; use codex_plugin::AppDeclaration; use codex_plugin::PluginCapabilitySummary; use codex_plugin::PluginId; @@ -10,7 +11,6 @@ use codex_plugin::app_connector_ids_from_declarations; use codex_plugin::prompt_safe_plugin_description; use codex_protocol::auth::AuthMode; use codex_protocol::protocol::Product; -use codex_skills::SkillConfigRules; use codex_skills::SkillRootLoader; use codex_utils_plugins::PluginIdentity; use codex_utils_plugins::PluginSkillRoot; diff --git a/codex-rs/core-skills/Cargo.toml b/codex-rs/core-skills/Cargo.toml index 16e98853f1..4bc5fd5b09 100644 --- a/codex-rs/core-skills/Cargo.toml +++ b/codex-rs/core-skills/Cargo.toml @@ -14,7 +14,6 @@ workspace = true [dependencies] codex-analytics = { workspace = true } -codex-config = { workspace = true } codex-context-fragments = { workspace = true } codex-exec-server = { workspace = true } codex-otel = { workspace = true } @@ -26,5 +25,3 @@ tracing = { workspace = true } [dev-dependencies] pretty_assertions = { workspace = true } -tempfile = { workspace = true } -toml = { workspace = true } diff --git a/codex-rs/core-skills/src/config_rules.rs b/codex-rs/core-skills/src/config_rules.rs deleted file mode 100644 index 064dd0f654..0000000000 --- a/codex-rs/core-skills/src/config_rules.rs +++ /dev/null @@ -1,76 +0,0 @@ -use codex_config::ConfigLayerSource; -use codex_config::ConfigLayerStack; -use codex_config::SkillConfig; -use codex_config::SkillsConfig; -pub use codex_skills::SkillConfigRule; -pub use codex_skills::SkillConfigRuleSelector; -pub use codex_skills::SkillConfigRules; -pub use codex_skills::resolve_disabled_skill_paths; -use tracing::warn; - -pub fn skill_config_rules_from_stack(config_layer_stack: &ConfigLayerStack) -> SkillConfigRules { - let mut entries = Vec::new(); - for layer in config_layer_stack.all_layers_low_to_high() { - if !matches!( - layer.name, - ConfigLayerSource::User { .. } | ConfigLayerSource::SessionFlags - ) { - continue; - } - - let Some(skills_value) = layer.config.get("skills") else { - continue; - }; - let skills: SkillsConfig = match skills_value.clone().try_into() { - Ok(skills) => skills, - Err(err) => { - warn!("invalid skills config: {err}"); - continue; - } - }; - - for entry in skills.config { - let Some(selector) = skill_config_rule_selector(&entry) else { - continue; - }; - // Preserve layer order so a later name selector can override an earlier path selector - // for the same loaded skill. - entries.retain(|entry: &SkillConfigRule| entry.selector != selector); - entries.push(SkillConfigRule { - selector, - enabled: entry.enabled, - }); - } - } - - SkillConfigRules { entries } -} - -fn skill_config_rule_selector(entry: &SkillConfig) -> Option { - match (entry.path.as_ref(), entry.name.as_deref()) { - (Some(path), None) => Some(SkillConfigRuleSelector::Path( - path.canonicalize().unwrap_or_else(|_| path.clone()), - )), - (None, Some(name)) => { - let name = name.trim(); - if name.is_empty() { - warn!("ignoring empty skills.config name override"); - None - } else { - Some(SkillConfigRuleSelector::Name(name.to_string())) - } - } - (Some(_), Some(_)) => { - warn!("ignoring skills.config entry with both path and name selectors"); - None - } - (None, None) => { - warn!("ignoring skills.config entry without a path or name selector"); - None - } - } -} - -#[cfg(test)] -#[path = "config_rules_tests.rs"] -mod tests; diff --git a/codex-rs/core-skills/src/lib.rs b/codex-rs/core-skills/src/lib.rs index b373c28d51..2e3a377405 100644 --- a/codex-rs/core-skills/src/lib.rs +++ b/codex-rs/core-skills/src/lib.rs @@ -1,4 +1,3 @@ -pub mod config_rules; pub mod injection; pub(crate) mod invocation_utils; pub mod model; diff --git a/codex-rs/core/src/skills.rs b/codex-rs/core/src/skills.rs index 726bf93e89..f6638fb002 100644 --- a/codex-rs/core/src/skills.rs +++ b/codex-rs/core/src/skills.rs @@ -16,7 +16,6 @@ use tokio::sync::Mutex; pub use codex_core_skills::SkillError; pub use codex_core_skills::SkillLoadOutcome; pub use codex_core_skills::build_skill_name_counts; -pub use codex_core_skills::config_rules; pub use codex_core_skills::detect_implicit_skill_invocation_for_command; pub use codex_core_skills::injection; pub use codex_core_skills::injection::SkillInjections; diff --git a/codex-rs/ext/skills/src/host_service.rs b/codex-rs/ext/skills/src/host_service.rs index 0a33736bc1..a3c6d54e65 100644 --- a/codex-rs/ext/skills/src/host_service.rs +++ b/codex-rs/ext/skills/src/host_service.rs @@ -6,6 +6,8 @@ use std::sync::RwLock; use std::sync::Weak; use codex_config::ConfigLayerStack; +use codex_config::SkillConfigRules; +use codex_config::skill_config_rules_from_stack; use codex_exec_server::ExecutorFileSystem; use codex_exec_server::LOCAL_FS; use codex_protocol::protocol::Product; @@ -21,9 +23,6 @@ use tracing::warn; use codex_config::SkillsConfig; use codex_core_skills::SkillLoadOutcome; -use codex_core_skills::config_rules::SkillConfigRules; -use codex_core_skills::config_rules::resolve_disabled_skill_paths; -use codex_core_skills::config_rules::skill_config_rules_from_stack; use codex_skills::LoadedSkills; use codex_skills::SkillLoadFuture; use codex_skills::SkillRootLoadRequest; @@ -294,7 +293,12 @@ impl HostSkillsService { input.plugin_skill_snapshots.as_ref(), ) .await; - let disabled_paths = resolve_disabled_skill_paths(&outcome.skills, skill_config_rules); + let disabled_paths = skill_config_rules.resolve_disabled_paths( + outcome + .skills + .iter() + .map(|skill| (skill.name.as_str(), &skill.path_to_skills_md)), + ); outcome.with_disabled_paths(disabled_paths) } diff --git a/codex-rs/skills/src/lib.rs b/codex-rs/skills/src/lib.rs index a33a552902..df739fa3dd 100644 --- a/codex-rs/skills/src/lib.rs +++ b/codex-rs/skills/src/lib.rs @@ -5,7 +5,6 @@ mod mentions; mod model; mod name_counts; mod parser; -mod policy; mod selection; pub use interface::SkillInterfaceAssetPolicy; @@ -30,9 +29,6 @@ pub use mentions::normalize_skill_path; pub use mentions::plugin_config_name_from_path; pub use mentions::tool_kind_for_path; pub use model::EnvironmentSkillMetadata; -pub use model::SkillConfigRule; -pub use model::SkillConfigRuleSelector; -pub use model::SkillConfigRules; pub use model::SkillDependencies; pub use model::SkillInterface; pub use model::SkillMetadata; @@ -42,7 +38,6 @@ pub use name_counts::build_skill_name_counts; pub use parser::ParsedSkillFrontmatter; pub use parser::SkillParseError; pub use parser::parse_skill_frontmatter_metadata; -pub use policy::resolve_disabled_skill_paths; pub use selection::ExplicitSkillLookup; pub use selection::collect_explicit_skill_mentions; diff --git a/codex-rs/skills/src/model.rs b/codex-rs/skills/src/model.rs index 977decdf75..e656c4267e 100644 --- a/codex-rs/skills/src/model.rs +++ b/codex-rs/skills/src/model.rs @@ -92,23 +92,6 @@ pub struct SkillToolDependency { pub url: Option, } -#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] -pub enum SkillConfigRuleSelector { - Name(String), - Path(AbsolutePathBuf), -} - -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct SkillConfigRule { - pub selector: SkillConfigRuleSelector, - pub enabled: bool, -} - -#[derive(Debug, Clone, Default, PartialEq, Eq, Hash)] -pub struct SkillConfigRules { - pub entries: Vec, -} - fn matches_product_restriction( policy: Option<&SkillPolicy>, restriction_product: Option, diff --git a/codex-rs/skills/src/policy.rs b/codex-rs/skills/src/policy.rs deleted file mode 100644 index e7972a4588..0000000000 --- a/codex-rs/skills/src/policy.rs +++ /dev/null @@ -1,48 +0,0 @@ -use std::collections::HashSet; - -use codex_utils_absolute_path::AbsolutePathBuf; - -use crate::SkillConfigRuleSelector; -use crate::SkillConfigRules; -use crate::SkillMetadata; - -/// Resolves disabled `SKILL.md` paths by applying configuration rules in order. -/// -/// Later rules override earlier rules for every skill path they select. -pub fn resolve_disabled_skill_paths( - skills: &[SkillMetadata], - rules: &SkillConfigRules, -) -> HashSet { - let mut disabled_paths = HashSet::new(); - - for entry in &rules.entries { - match &entry.selector { - SkillConfigRuleSelector::Path(path) => { - if entry.enabled { - disabled_paths.remove(path); - } else { - disabled_paths.insert(path.clone()); - } - } - SkillConfigRuleSelector::Name(name) => { - for path in skills - .iter() - .filter(|skill| skill.name == *name) - .map(|skill| skill.path_to_skills_md.clone()) - { - if entry.enabled { - disabled_paths.remove(&path); - } else { - disabled_paths.insert(path); - } - } - } - } - } - - disabled_paths -} - -#[cfg(test)] -#[path = "policy_tests.rs"] -mod tests; diff --git a/codex-rs/skills/src/policy_tests.rs b/codex-rs/skills/src/policy_tests.rs deleted file mode 100644 index da120210cd..0000000000 --- a/codex-rs/skills/src/policy_tests.rs +++ /dev/null @@ -1,96 +0,0 @@ -use codex_protocol::protocol::SkillScope; -use codex_utils_absolute_path::AbsolutePathBuf; -use pretty_assertions::assert_eq; - -use super::resolve_disabled_skill_paths; -use crate::SkillConfigRule; -use crate::SkillConfigRuleSelector; -use crate::SkillConfigRules; -use crate::SkillMetadata; - -fn skill(name: &str, path: AbsolutePathBuf) -> SkillMetadata { - SkillMetadata { - name: name.to_string(), - description: "test".to_string(), - short_description: None, - interface: None, - dependencies: None, - policy: None, - path_to_skills_md: path, - scope: SkillScope::User, - plugin_id: None, - remote_plugin_id: None, - } -} - -fn test_root() -> AbsolutePathBuf { - AbsolutePathBuf::try_from(std::env::current_dir().expect("current directory")) - .expect("absolute current directory") -} - -#[test] -fn path_rule_disables_selected_path() { - let path = test_root().join("disable-by-path/SKILL.md"); - let rules = SkillConfigRules { - entries: vec![SkillConfigRule { - selector: SkillConfigRuleSelector::Path(path.clone()), - enabled: false, - }], - }; - - assert_eq!( - resolve_disabled_skill_paths(&[], &rules), - [path].into_iter().collect() - ); -} - -#[test] -fn later_name_rule_reenables_path_disabled_skill() { - let path = test_root().join("reenable-by-name/SKILL.md"); - let skills = vec![skill("demo", path.clone())]; - let rules = SkillConfigRules { - entries: vec![ - SkillConfigRule { - selector: SkillConfigRuleSelector::Path(path), - enabled: false, - }, - SkillConfigRule { - selector: SkillConfigRuleSelector::Name("demo".to_string()), - enabled: true, - }, - ], - }; - - assert_eq!( - resolve_disabled_skill_paths(&skills, &rules), - Default::default() - ); -} - -#[test] -fn later_path_rule_reenables_one_skill_disabled_by_name() { - let root = test_root().join("reenable-by-path"); - let first_path = root.join("first/SKILL.md"); - let second_path = root.join("second/SKILL.md"); - let skills = vec![ - skill("demo", first_path.clone()), - skill("demo", second_path.clone()), - ]; - let rules = SkillConfigRules { - entries: vec![ - SkillConfigRule { - selector: SkillConfigRuleSelector::Name("demo".to_string()), - enabled: false, - }, - SkillConfigRule { - selector: SkillConfigRuleSelector::Path(first_path), - enabled: true, - }, - ], - }; - - assert_eq!( - resolve_disabled_skill_paths(&skills, &rules), - [second_path].into_iter().collect() - ); -}