diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 187fcca79..5e3619137 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1850,6 +1850,7 @@ dependencies = [ "futures", "multimap", "pretty_assertions", + "schemars 0.8.22", "serde", "serde_json", "serde_path_to_error", @@ -1899,6 +1900,7 @@ dependencies = [ "codex-code-mode", "codex-config", "codex-connectors", + "codex-core-skills", "codex-exec-server", "codex-execpolicy", "codex-features", @@ -1916,7 +1918,6 @@ dependencies = [ "codex-secrets", "codex-shell-command", "codex-shell-escalation", - "codex-skills", "codex-state", "codex-terminal-detection", "codex-test-macros", @@ -1966,7 +1967,6 @@ dependencies = [ "seccompiler", "serde", "serde_json", - "serde_yaml", "serial_test", "sha1", "shlex", @@ -1995,6 +1995,35 @@ dependencies = [ "zstd", ] +[[package]] +name = "codex-core-skills" +version = "0.0.0" +dependencies = [ + "anyhow", + "codex-analytics", + "codex-app-server-protocol", + "codex-config", + "codex-instructions", + "codex-login", + "codex-otel", + "codex-protocol", + "codex-skills", + "codex-utils-absolute-path", + "codex-utils-plugins", + "dirs", + "dunce", + "pretty_assertions", + "serde", + "serde_json", + "serde_yaml", + "shlex", + "tempfile", + "tokio", + "toml 0.9.11+spec-1.1.0", + "tracing", + "zip", +] + [[package]] name = "codex-debug-client" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 350a98e45..f19fb650f 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -25,6 +25,7 @@ members = [ "shell-escalation", "skills", "core", + "core-skills", "hooks", "instructions", "secrets", @@ -119,6 +120,7 @@ codex-cloud-requirements = { path = "cloud-requirements" } codex-connectors = { path = "connectors" } codex-config = { path = "config" } codex-core = { path = "core" } +codex-core-skills = { path = "core-skills" } codex-exec = { path = "exec" } codex-exec-server = { path = "exec-server" } codex-execpolicy = { path = "execpolicy" } diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 6e42f18e4..22122a799 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -203,6 +203,8 @@ use codex_core::config::types::McpServerTransportConfig; use codex_core::config_loader::CloudRequirementsLoadError; use codex_core::config_loader::CloudRequirementsLoadErrorCode; use codex_core::config_loader::CloudRequirementsLoader; +use codex_core::config_loader::LoaderOverrides; +use codex_core::config_loader::load_config_layers_state; use codex_core::default_client::set_default_client_residency_requirement; use codex_core::error::CodexErr; use codex_core::error::Result as CodexResult; @@ -282,6 +284,7 @@ use codex_state::StateRuntime; use codex_state::ThreadMetadata; use codex_state::ThreadMetadataBuilder; use codex_state::log_db::LogDbLayer; +use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_json_to_toml::json_to_toml; use codex_utils_pty::DEFAULT_OUTPUT_BYTES_CAP; use std::collections::BTreeMap; @@ -5509,13 +5512,63 @@ impl CodexMessageProcessor { } }; let skills_manager = self.thread_manager.skills_manager(); + let plugins_manager = self.thread_manager.plugins_manager(); + let cli_overrides = self.current_cli_overrides(); let mut data = Vec::new(); for cwd in cwds { let extra_roots = extra_roots_by_cwd .get(&cwd) .map_or(&[][..], std::vec::Vec::as_slice); + let cwd_abs = match AbsolutePathBuf::try_from(cwd.as_path()) { + Ok(path) => path, + Err(err) => { + let error_path = cwd.clone(); + data.push(codex_app_server_protocol::SkillsListEntry { + cwd, + skills: Vec::new(), + errors: errors_to_info(&[codex_core::skills::SkillError { + path: error_path, + message: err.to_string(), + }]), + }); + continue; + } + }; + let config_layer_stack = match load_config_layers_state( + &self.config.codex_home, + Some(cwd_abs), + &cli_overrides, + LoaderOverrides::default(), + CloudRequirementsLoader::default(), + ) + .await + { + Ok(config_layer_stack) => config_layer_stack, + Err(err) => { + let error_path = cwd.clone(); + data.push(codex_app_server_protocol::SkillsListEntry { + cwd, + skills: Vec::new(), + errors: errors_to_info(&[codex_core::skills::SkillError { + path: error_path, + message: err.to_string(), + }]), + }); + continue; + } + }; + let effective_skill_roots = plugins_manager.effective_skill_roots_for_layer_stack( + &config_layer_stack, + config.features.enabled(Feature::Plugins), + ); + let skills_input = codex_core::skills::SkillsLoadInput::new( + cwd.clone(), + effective_skill_roots, + config_layer_stack, + config.bundled_skills_enabled(), + ); let outcome = skills_manager - .skills_for_cwd_with_extra_user_roots(&cwd, &config, force_reload, extra_roots) + .skills_for_cwd_with_extra_user_roots(&skills_input, 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/config/Cargo.toml b/codex-rs/config/Cargo.toml index 02446ed6f..f1bd2dba1 100644 --- a/codex-rs/config/Cargo.toml +++ b/codex-rs/config/Cargo.toml @@ -14,6 +14,7 @@ codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } futures = { workspace = true, features = ["alloc", "std"] } multimap = { workspace = true } +schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } serde_path_to_error = { workspace = true } diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index 995a5b8db..7bd572fea 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -5,7 +5,9 @@ mod diagnostics; mod fingerprint; mod merge; mod overrides; +mod project_root_markers; mod requirements_exec_policy; +mod skills_config; mod state; pub const CONFIG_TOML_FILE: &str = "config.toml"; @@ -46,12 +48,17 @@ pub use diagnostics::io_error_from_config_error; pub use fingerprint::version_for_toml; pub use merge::merge_toml_values; pub use overrides::build_cli_overrides_layer; +pub use project_root_markers::default_project_root_markers; +pub use project_root_markers::project_root_markers_from_config; pub use requirements_exec_policy::RequirementsExecPolicy; pub use requirements_exec_policy::RequirementsExecPolicyDecisionToml; pub use requirements_exec_policy::RequirementsExecPolicyParseError; pub use requirements_exec_policy::RequirementsExecPolicyPatternTokenToml; pub use requirements_exec_policy::RequirementsExecPolicyPrefixRuleToml; pub use requirements_exec_policy::RequirementsExecPolicyToml; +pub use skills_config::BundledSkillsConfig; +pub use skills_config::SkillConfig; +pub use skills_config::SkillsConfig; pub use state::ConfigLayerEntry; pub use state::ConfigLayerStack; pub use state::ConfigLayerStackOrdering; diff --git a/codex-rs/config/src/project_root_markers.rs b/codex-rs/config/src/project_root_markers.rs new file mode 100644 index 000000000..3061dacc6 --- /dev/null +++ b/codex-rs/config/src/project_root_markers.rs @@ -0,0 +1,50 @@ +use std::io; + +use toml::Value as TomlValue; + +const DEFAULT_PROJECT_ROOT_MARKERS: &[&str] = &[".git"]; + +/// Reads `project_root_markers` from a merged `config.toml` [toml::Value]. +/// +/// Invariants: +/// - If `project_root_markers` is not specified, returns `Ok(None)`. +/// - If `project_root_markers` is specified, returns `Ok(Some(markers))` where +/// `markers` is a `Vec` (including `Ok(Some(Vec::new()))` for an +/// empty array, which indicates that root detection should be disabled). +/// - Returns an error if `project_root_markers` is specified but is not an +/// array of strings. +pub fn project_root_markers_from_config(config: &TomlValue) -> io::Result>> { + let Some(table) = config.as_table() else { + return Ok(None); + }; + let Some(markers_value) = table.get("project_root_markers") else { + return Ok(None); + }; + let TomlValue::Array(entries) = markers_value else { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "project_root_markers must be an array of strings", + )); + }; + if entries.is_empty() { + return Ok(Some(Vec::new())); + } + let mut markers = Vec::new(); + for entry in entries { + let Some(marker) = entry.as_str() else { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "project_root_markers must be an array of strings", + )); + }; + markers.push(marker.to_string()); + } + Ok(Some(markers)) +} + +pub fn default_project_root_markers() -> Vec { + DEFAULT_PROJECT_ROOT_MARKERS + .iter() + .map(ToString::to_string) + .collect() +} diff --git a/codex-rs/config/src/skills_config.rs b/codex-rs/config/src/skills_config.rs new file mode 100644 index 000000000..3dbe65a24 --- /dev/null +++ b/codex-rs/config/src/skills_config.rs @@ -0,0 +1,53 @@ +//! Skill-related configuration types shared across crates. + +use codex_utils_absolute_path::AbsolutePathBuf; +use schemars::JsonSchema; +use serde::Deserialize; +use serde::Serialize; + +const fn default_enabled() -> bool { + true +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct SkillConfig { + /// Path-based selector. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub path: Option, + /// Name-based selector. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub name: Option, + pub enabled: bool, +} + +#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct SkillsConfig { + #[serde(default, skip_serializing_if = "Option::is_none")] + pub bundled: Option, + + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub config: Vec, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct BundledSkillsConfig { + #[serde(default = "default_enabled")] + pub enabled: bool, +} + +impl Default for BundledSkillsConfig { + fn default() -> Self { + Self { enabled: true } + } +} + +impl TryFrom for SkillsConfig { + type Error = toml::de::Error; + + fn try_from(value: toml::Value) -> Result { + SkillsConfig::deserialize(value) + } +} diff --git a/codex-rs/core-skills/BUILD.bazel b/codex-rs/core-skills/BUILD.bazel new file mode 100644 index 000000000..e80412a55 --- /dev/null +++ b/codex-rs/core-skills/BUILD.bazel @@ -0,0 +1,15 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "core-skills", + crate_name = "codex_core_skills", + compile_data = glob( + include = ["**"], + exclude = [ + "**/* *", + "BUILD.bazel", + "Cargo.toml", + ], + allow_empty = True, + ), +) diff --git a/codex-rs/core-skills/Cargo.toml b/codex-rs/core-skills/Cargo.toml new file mode 100644 index 000000000..35c9ed0a5 --- /dev/null +++ b/codex-rs/core-skills/Cargo.toml @@ -0,0 +1,40 @@ +[package] +edition.workspace = true +license.workspace = true +name = "codex-core-skills" +version.workspace = true + +[lib] +doctest = false +name = "codex_core_skills" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +anyhow = { workspace = true } +codex-analytics = { workspace = true } +codex-app-server-protocol = { workspace = true } +codex-config = { workspace = true } +codex-instructions = { workspace = true } +codex-login = { workspace = true } +codex-otel = { workspace = true } +codex-protocol = { workspace = true } +codex-skills = { workspace = true } +codex-utils-absolute-path = { workspace = true } +codex-utils-plugins = { workspace = true } +dirs = { workspace = true } +dunce = { workspace = true } +serde = { workspace = true, features = ["derive"] } +serde_json = { workspace = true } +serde_yaml = { workspace = true } +shlex = { workspace = true } +tokio = { workspace = true, features = ["fs", "macros", "rt"] } +toml = { workspace = true } +tracing = { workspace = true } +zip = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } +tempfile = { workspace = true } diff --git a/codex-rs/core/src/skills/config_rules.rs b/codex-rs/core-skills/src/config_rules.rs similarity index 88% rename from codex-rs/core/src/skills/config_rules.rs rename to codex-rs/core-skills/src/config_rules.rs index fa1be878b..f613d494a 100644 --- a/codex-rs/core/src/skills/config_rules.rs +++ b/codex-rs/core-skills/src/config_rules.rs @@ -3,34 +3,32 @@ use std::path::Path; use std::path::PathBuf; use codex_app_server_protocol::ConfigLayerSource; +use codex_config::ConfigLayerStack; +use codex_config::ConfigLayerStackOrdering; +use codex_config::SkillConfig; +use codex_config::SkillsConfig; use tracing::warn; -use crate::config::types::SkillConfig; -use crate::config::types::SkillsConfig; -use crate::config_loader::ConfigLayerStack; -use crate::config_loader::ConfigLayerStackOrdering; -use crate::skills::SkillMetadata; +use crate::SkillMetadata; #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] -pub(crate) enum SkillConfigRuleSelector { +pub enum SkillConfigRuleSelector { Name(String), Path(PathBuf), } #[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub(crate) struct SkillConfigRule { +pub struct SkillConfigRule { pub selector: SkillConfigRuleSelector, pub enabled: bool, } #[derive(Debug, Clone, Default, PartialEq, Eq, Hash)] -pub(crate) struct SkillConfigRules { +pub struct SkillConfigRules { pub entries: Vec, } -pub(crate) fn skill_config_rules_from_stack( - config_layer_stack: &ConfigLayerStack, -) -> SkillConfigRules { +pub fn skill_config_rules_from_stack(config_layer_stack: &ConfigLayerStack) -> SkillConfigRules { let mut entries = Vec::new(); for layer in config_layer_stack.get_layers( ConfigLayerStackOrdering::LowestPrecedenceFirst, @@ -71,7 +69,7 @@ pub(crate) fn skill_config_rules_from_stack( SkillConfigRules { entries } } -pub(crate) fn resolve_disabled_skill_paths( +pub fn resolve_disabled_skill_paths( skills: &[SkillMetadata], rules: &SkillConfigRules, ) -> HashSet { diff --git a/codex-rs/core-skills/src/env_var_dependencies.rs b/codex-rs/core-skills/src/env_var_dependencies.rs new file mode 100644 index 000000000..4d8e57164 --- /dev/null +++ b/codex-rs/core-skills/src/env_var_dependencies.rs @@ -0,0 +1,30 @@ +use crate::SkillMetadata; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SkillDependencyInfo { + pub skill_name: String, + pub name: String, + pub description: Option, +} + +pub fn collect_env_var_dependencies( + mentioned_skills: &[SkillMetadata], +) -> Vec { + let mut dependencies = Vec::new(); + for skill in mentioned_skills { + let Some(skill_dependencies) = &skill.dependencies else { + continue; + }; + for tool in &skill_dependencies.tools { + if tool.r#type != "env_var" || tool.value.is_empty() { + continue; + } + dependencies.push(SkillDependencyInfo { + skill_name: skill.name.clone(), + name: tool.value.clone(), + description: tool.description.clone(), + }); + } + } + dependencies +} diff --git a/codex-rs/core/src/skills/injection.rs b/codex-rs/core-skills/src/injection.rs similarity index 93% rename from codex-rs/core/src/skills/injection.rs rename to codex-rs/core-skills/src/injection.rs index dac05f836..b31885b8c 100644 --- a/codex-rs/core/src/skills/injection.rs +++ b/codex-rs/core-skills/src/injection.rs @@ -2,26 +2,26 @@ use std::collections::HashMap; use std::collections::HashSet; use std::path::PathBuf; -use crate::instructions::SkillInstructions; -use crate::mention_syntax::TOOL_MENTION_SIGIL; -use crate::mentions::build_skill_name_counts; -use crate::skills::SkillMetadata; +use crate::SkillMetadata; +use crate::build_skill_name_counts; use codex_analytics::AnalyticsEventsClient; use codex_analytics::InvocationType; use codex_analytics::SkillInvocation; use codex_analytics::TrackEventsContext; +use codex_instructions::SkillInstructions; use codex_otel::SessionTelemetry; use codex_protocol::models::ResponseItem; use codex_protocol::user_input::UserInput; +use codex_utils_plugins::mention_syntax::TOOL_MENTION_SIGIL; use tokio::fs; #[derive(Debug, Default)] -pub(crate) struct SkillInjections { - pub(crate) items: Vec, - pub(crate) warnings: Vec, +pub struct SkillInjections { + pub items: Vec, + pub warnings: Vec, } -pub(crate) async fn build_skill_injections( +pub async fn build_skill_injections( mentioned_skills: &[SkillMetadata], otel: Option<&SessionTelemetry>, analytics_client: &AnalyticsEventsClient, @@ -97,7 +97,7 @@ fn emit_skill_injected_metric( /// Complexity: `O(T + (N_s + N_t) * S)` time, `O(S + M)` space, where: /// `S` = number of skills, `T` = total text length, `N_s` = number of structured skill inputs, /// `N_t` = number of text inputs, `M` = max mentions parsed from a single text input. -pub(crate) fn collect_explicit_skill_mentions( +pub fn collect_explicit_skill_mentions( inputs: &[UserInput], skills: &[SkillMetadata], disabled_paths: &HashSet, @@ -159,7 +159,7 @@ struct SkillSelectionContext<'a> { connector_slug_counts: &'a HashMap, } -pub(crate) struct ToolMentions<'a> { +pub struct ToolMentions<'a> { names: HashSet<&'a str>, paths: HashSet<&'a str>, plain_names: HashSet<&'a str>, @@ -170,17 +170,17 @@ impl<'a> ToolMentions<'a> { self.names.is_empty() && self.paths.is_empty() } - pub(crate) fn plain_names(&self) -> impl Iterator + '_ { + pub fn plain_names(&self) -> impl Iterator + '_ { self.plain_names.iter().copied() } - pub(crate) fn paths(&self) -> impl Iterator + '_ { + pub fn paths(&self) -> impl Iterator + '_ { self.paths.iter().copied() } } #[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub(crate) enum ToolMentionKind { +pub enum ToolMentionKind { App, Mcp, Plugin, @@ -194,7 +194,7 @@ const PLUGIN_PATH_PREFIX: &str = "plugin://"; const SKILL_PATH_PREFIX: &str = "skill://"; const SKILL_FILENAME: &str = "SKILL.md"; -pub(crate) fn tool_kind_for_path(path: &str) -> ToolMentionKind { +pub fn tool_kind_for_path(path: &str) -> ToolMentionKind { if path.starts_with(APP_PATH_PREFIX) { ToolMentionKind::App } else if path.starts_with(MCP_PATH_PREFIX) { @@ -213,12 +213,12 @@ fn is_skill_filename(path: &str) -> bool { file_name.eq_ignore_ascii_case(SKILL_FILENAME) } -pub(crate) fn app_id_from_path(path: &str) -> Option<&str> { +pub fn app_id_from_path(path: &str) -> Option<&str> { path.strip_prefix(APP_PATH_PREFIX) .filter(|value| !value.is_empty()) } -pub(crate) fn plugin_config_name_from_path(path: &str) -> Option<&str> { +pub fn plugin_config_name_from_path(path: &str) -> Option<&str> { path.strip_prefix(PLUGIN_PATH_PREFIX) .filter(|value| !value.is_empty()) } @@ -232,11 +232,11 @@ pub(crate) fn normalize_skill_path(path: &str) -> &str { /// Supports explicit resource links in the form `[$tool-name](resource path)`. When a /// resource path is present, it is captured for exact path matching while also tracking /// the name for fallback matching. -pub(crate) fn extract_tool_mentions(text: &str) -> ToolMentions<'_> { +pub fn extract_tool_mentions(text: &str) -> ToolMentions<'_> { extract_tool_mentions_with_sigil(text, TOOL_MENTION_SIGIL) } -pub(crate) fn extract_tool_mentions_with_sigil(text: &str, sigil: char) -> ToolMentions<'_> { +pub fn extract_tool_mentions_with_sigil(text: &str, sigil: char) -> ToolMentions<'_> { let text_bytes = text.as_bytes(); let mut mentioned_names: HashSet<&str> = HashSet::new(); let mut mentioned_paths: HashSet<&str> = HashSet::new(); diff --git a/codex-rs/core/src/skills/injection_tests.rs b/codex-rs/core-skills/src/injection_tests.rs similarity index 100% rename from codex-rs/core/src/skills/injection_tests.rs rename to codex-rs/core-skills/src/injection_tests.rs diff --git a/codex-rs/core/src/skills/invocation_utils.rs b/codex-rs/core-skills/src/invocation_utils.rs similarity index 58% rename from codex-rs/core/src/skills/invocation_utils.rs rename to codex-rs/core-skills/src/invocation_utils.rs index 084670fa8..1936bbe63 100644 --- a/codex-rs/core/src/skills/invocation_utils.rs +++ b/codex-rs/core-skills/src/invocation_utils.rs @@ -2,13 +2,8 @@ use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; -use crate::codex::Session; -use crate::codex::TurnContext; -use crate::skills::SkillLoadOutcome; -use crate::skills::SkillMetadata; -use codex_analytics::InvocationType; -use codex_analytics::SkillInvocation; -use codex_analytics::build_track_events_context; +use crate::SkillLoadOutcome; +use crate::SkillMetadata; pub(crate) fn build_implicit_skill_path_indexes( skills: Vec, @@ -31,14 +26,12 @@ pub(crate) fn build_implicit_skill_path_indexes( (by_scripts_dir, by_skill_doc_path) } -fn detect_implicit_skill_invocation_for_command( +pub fn detect_implicit_skill_invocation_for_command( outcome: &SkillLoadOutcome, - turn_context: &TurnContext, command: &str, - workdir: Option<&str>, + workdir: &Path, ) -> Option { - let workdir = turn_context.resolve_path(workdir.map(str::to_owned)); - let workdir = normalize_path(workdir.as_path()); + let workdir = normalize_path(workdir); let tokens = tokenize_command(command); if let Some(candidate) = detect_skill_script_run(outcome, tokens.as_slice(), workdir.as_path()) @@ -46,82 +39,12 @@ fn detect_implicit_skill_invocation_for_command( return Some(candidate); } - if let Some(candidate) = detect_skill_doc_read(outcome, tokens.as_slice(), workdir.as_path()) { - return Some(candidate); - } - - None -} - -pub(crate) async fn maybe_emit_implicit_skill_invocation( - sess: &Session, - turn_context: &TurnContext, - command: &str, - workdir: Option<&str>, -) { - let Some(candidate) = detect_implicit_skill_invocation_for_command( - &turn_context.turn_skills.outcome, - turn_context, - command, - workdir, - ) else { - return; - }; - let invocation = SkillInvocation { - skill_name: candidate.name, - skill_scope: candidate.scope, - skill_path: candidate.path_to_skills_md, - invocation_type: InvocationType::Implicit, - }; - let skill_scope = match invocation.skill_scope { - codex_protocol::protocol::SkillScope::User => "user", - codex_protocol::protocol::SkillScope::Repo => "repo", - codex_protocol::protocol::SkillScope::System => "system", - codex_protocol::protocol::SkillScope::Admin => "admin", - }; - let skill_path = invocation.skill_path.to_string_lossy(); - let skill_name = invocation.skill_name.clone(); - let seen_key = format!("{skill_scope}:{skill_path}:{skill_name}"); - let inserted = { - let mut seen_skills = turn_context - .turn_skills - .implicit_invocation_seen_skills - .lock() - .await; - seen_skills.insert(seen_key) - }; - if !inserted { - return; - } - - turn_context.session_telemetry.counter( - "codex.skill.injected", - /*inc*/ 1, - &[ - ("status", "ok"), - ("skill", skill_name.as_str()), - ("invoke_type", "implicit"), - ], - ); - sess.services - .analytics_events_client - .track_skill_invocations( - build_track_events_context( - turn_context.model_info.slug.clone(), - sess.conversation_id.to_string(), - turn_context.sub_id.clone(), - ), - vec![invocation], - ); + detect_skill_doc_read(outcome, tokens.as_slice(), workdir.as_path()) } fn tokenize_command(command: &str) -> Vec { - shlex::split(command).unwrap_or_else(|| { - command - .split_whitespace() - .map(std::string::ToString::to_string) - .collect() - }) + shlex::split(command) + .unwrap_or_else(|| command.split_whitespace().map(str::to_string).collect()) } fn script_run_token(tokens: &[String]) -> Option<&str> { @@ -137,12 +60,9 @@ fn script_run_token(tokens: &[String]) -> Option<&str> { return None; } - let mut script_token: Option<&str> = None; + let mut script_token = None; for token in tokens.iter().skip(1) { - if token == "--" { - continue; - } - if token.starts_with('-') { + if token == "--" || token.starts_with('-') { continue; } script_token = Some(token.as_str()); diff --git a/codex-rs/core/src/skills/invocation_utils_tests.rs b/codex-rs/core-skills/src/invocation_utils_tests.rs similarity index 100% rename from codex-rs/core/src/skills/invocation_utils_tests.rs rename to codex-rs/core-skills/src/invocation_utils_tests.rs diff --git a/codex-rs/core/src/skills/mod.rs b/codex-rs/core-skills/src/lib.rs similarity index 55% rename from codex-rs/core/src/skills/mod.rs rename to codex-rs/core-skills/src/lib.rs index ad76cd9da..b967e4ddd 100644 --- a/codex-rs/core/src/skills/mod.rs +++ b/codex-rs/core-skills/src/lib.rs @@ -1,22 +1,22 @@ -pub(crate) mod config_rules; +pub mod config_rules; mod env_var_dependencies; pub mod injection; pub(crate) mod invocation_utils; pub mod loader; pub mod manager; +mod mention_counts; pub mod model; pub mod remote; pub mod render; pub mod system; -pub(crate) use env_var_dependencies::collect_env_var_dependencies; -pub(crate) use env_var_dependencies::resolve_skill_dependencies_for_turn; -pub(crate) use injection::SkillInjections; -pub(crate) use injection::build_skill_injections; -pub(crate) use injection::collect_explicit_skill_mentions; +pub use env_var_dependencies::SkillDependencyInfo; +pub use env_var_dependencies::collect_env_var_dependencies; pub(crate) use invocation_utils::build_implicit_skill_path_indexes; -pub(crate) use invocation_utils::maybe_emit_implicit_skill_invocation; +pub use invocation_utils::detect_implicit_skill_invocation_for_command; +pub use manager::SkillsLoadInput; pub use manager::SkillsManager; +pub use mention_counts::build_skill_name_counts; pub use model::SkillError; pub use model::SkillLoadOutcome; pub use model::SkillMetadata; diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core-skills/src/loader.rs similarity index 96% rename from codex-rs/core/src/skills/loader.rs rename to codex-rs/core-skills/src/loader.rs index 5672bdb0a..b6ef00084 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -1,19 +1,18 @@ -use crate::config_loader::ConfigLayerStack; -use crate::config_loader::ConfigLayerStackOrdering; -use crate::config_loader::default_project_root_markers; -use crate::config_loader::merge_toml_values; -use crate::config_loader::project_root_markers_from_config; -use crate::plugins::plugin_namespace_for_skill_path; -use crate::skills::model::SkillDependencies; -use crate::skills::model::SkillError; -use crate::skills::model::SkillInterface; -use crate::skills::model::SkillLoadOutcome; -use crate::skills::model::SkillManagedNetworkOverride; -use crate::skills::model::SkillMetadata; -use crate::skills::model::SkillPolicy; -use crate::skills::model::SkillToolDependency; -use crate::skills::system::system_cache_root_dir; +use crate::model::SkillDependencies; +use crate::model::SkillError; +use crate::model::SkillInterface; +use crate::model::SkillLoadOutcome; +use crate::model::SkillManagedNetworkOverride; +use crate::model::SkillMetadata; +use crate::model::SkillPolicy; +use crate::model::SkillToolDependency; +use crate::system::system_cache_root_dir; use codex_app_server_protocol::ConfigLayerSource; +use codex_config::ConfigLayerStack; +use codex_config::ConfigLayerStackOrdering; +use codex_config::default_project_root_markers; +use codex_config::merge_toml_values; +use codex_config::project_root_markers_from_config; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::MacOsSeatbeltProfileExtensions; use codex_protocol::models::NetworkPermissions; @@ -21,6 +20,7 @@ use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBufGuard; +use codex_utils_plugins::plugin_namespace_for_skill_path; use dirs::home_dir; use dunce::canonicalize as canonicalize_path; use serde::Deserialize; @@ -35,9 +35,6 @@ use std::path::PathBuf; use toml::Value as TomlValue; use tracing::error; -#[cfg(test)] -use crate::config::Config; - #[derive(Debug, Deserialize)] struct SkillFrontmatter { #[serde(default)] @@ -176,12 +173,12 @@ impl fmt::Display for SkillParseError { impl Error for SkillParseError {} -pub(crate) struct SkillRoot { - pub(crate) path: PathBuf, - pub(crate) scope: SkillScope, +pub struct SkillRoot { + pub path: PathBuf, + pub scope: SkillScope, } -pub(crate) fn load_skills_from_roots(roots: I) -> SkillLoadOutcome +pub fn load_skills_from_roots(roots: I) -> SkillLoadOutcome where I: IntoIterator, { diff --git a/codex-rs/core/src/skills/loader_tests.rs b/codex-rs/core-skills/src/loader_tests.rs similarity index 95% rename from codex-rs/core/src/skills/loader_tests.rs rename to codex-rs/core-skills/src/loader_tests.rs index 7f758735c..5875b1a04 100644 --- a/codex-rs/core/src/skills/loader_tests.rs +++ b/codex-rs/core-skills/src/loader_tests.rs @@ -1,14 +1,9 @@ use super::*; -use crate::config::ConfigBuilder; -use crate::config::ConfigOverrides; -use crate::config::ConfigToml; -use crate::config::ProjectConfig; -use crate::config_loader::ConfigLayerEntry; -use crate::config_loader::ConfigLayerStack; -use crate::config_loader::ConfigRequirements; -use crate::config_loader::ConfigRequirementsToml; use codex_config::CONFIG_TOML_FILE; -use codex_protocol::config_types::TrustLevel; +use codex_config::ConfigLayerEntry; +use codex_config::ConfigLayerStack; +use codex_config::ConfigRequirements; +use codex_config::ConfigRequirementsToml; use codex_protocol::models::FileSystemPermissions; use codex_protocol::models::MacOsAutomationPermission; use codex_protocol::models::MacOsContactsPermission; @@ -19,53 +14,109 @@ use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; -use std::collections::HashMap; use std::path::Path; use tempfile::TempDir; use toml::Value as TomlValue; const REPO_ROOT_CONFIG_DIR_NAME: &str = ".codex"; -async fn make_config(codex_home: &TempDir) -> Config { +struct TestConfig { + cwd: PathBuf, + config_layer_stack: ConfigLayerStack, +} + +async fn make_config(codex_home: &TempDir) -> TestConfig { make_config_for_cwd(codex_home, codex_home.path().to_path_buf()).await } -async fn make_config_for_cwd(codex_home: &TempDir, cwd: PathBuf) -> Config { - let trust_root = cwd - .ancestors() - .find(|ancestor| ancestor.join(".git").exists()) - .map(Path::to_path_buf) - .unwrap_or_else(|| cwd.clone()); - - fs::write( - codex_home.path().join(CONFIG_TOML_FILE), - toml::to_string(&ConfigToml { - projects: Some(HashMap::from([( - trust_root.to_string_lossy().to_string(), - ProjectConfig { - trust_level: Some(TrustLevel::Trusted), - }, - )])), - ..Default::default() - }) - .expect("serialize config"), - ) - .unwrap(); - - let harness_overrides = ConfigOverrides { - cwd: Some(cwd), - ..Default::default() - }; - - ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .harness_overrides(harness_overrides) - .build() - .await - .expect("defaults for test should always succeed") +fn config_file(path: PathBuf) -> AbsolutePathBuf { + AbsolutePathBuf::from_absolute_path(path).expect("config file path should be absolute") } -fn load_skills_for_test(config: &Config) -> SkillLoadOutcome { +fn project_layers_for_cwd(cwd: &Path) -> Vec { + let cwd_dir = if cwd.is_dir() { + cwd.to_path_buf() + } else { + cwd.parent() + .expect("file cwd should have a parent directory") + .to_path_buf() + }; + let project_root = cwd_dir + .ancestors() + .find(|ancestor| ancestor.join(".git").exists()) + .unwrap_or(cwd_dir.as_path()) + .to_path_buf(); + + let mut layers = cwd_dir + .ancestors() + .scan(false, |done, dir| { + if *done { + None + } else { + if dir == project_root { + *done = true; + } + Some(dir.to_path_buf()) + } + }) + .collect::>(); + layers.reverse(); + + layers + .into_iter() + .filter_map(|dir| { + let dot_codex = dir.join(REPO_ROOT_CONFIG_DIR_NAME); + dot_codex.is_dir().then(|| { + ConfigLayerEntry::new( + ConfigLayerSource::Project { + dot_codex_folder: AbsolutePathBuf::from_absolute_path(dot_codex) + .expect("project .codex path should be absolute"), + }, + TomlValue::Table(toml::map::Map::new()), + ) + }) + }) + .collect() +} + +async fn make_config_for_cwd(codex_home: &TempDir, cwd: PathBuf) -> TestConfig { + let user_config_path = codex_home.path().join(CONFIG_TOML_FILE); + let system_config_path = codex_home.path().join("etc/codex/config.toml"); + fs::create_dir_all( + system_config_path + .parent() + .expect("system config path should have a parent"), + ) + .expect("create fake system config dir"); + + let mut layers = vec![ + ConfigLayerEntry::new( + ConfigLayerSource::System { + file: config_file(system_config_path), + }, + TomlValue::Table(toml::map::Map::new()), + ), + ConfigLayerEntry::new( + ConfigLayerSource::User { + file: config_file(user_config_path), + }, + TomlValue::Table(toml::map::Map::new()), + ), + ]; + layers.extend(project_layers_for_cwd(&cwd)); + + TestConfig { + cwd, + config_layer_stack: ConfigLayerStack::new( + layers, + ConfigRequirements::default(), + ConfigRequirementsToml::default(), + ) + .expect("valid config layer stack"), + } +} + +fn load_skills_for_test(config: &TestConfig) -> SkillLoadOutcome { // Keep unit tests hermetic by never scanning the real `$HOME/.agents/skills`. super::load_skills_from_roots(super::skill_roots_with_home_dir( &config.config_layer_stack, diff --git a/codex-rs/core/src/skills/manager.rs b/codex-rs/core-skills/src/manager.rs similarity index 67% rename from codex-rs/core/src/skills/manager.rs rename to codex-rs/core-skills/src/manager.rs index 446580b6c..cd3b42771 100644 --- a/codex-rs/core/src/skills/manager.rs +++ b/codex-rs/core-skills/src/manager.rs @@ -5,61 +5,67 @@ use std::path::PathBuf; use std::sync::Arc; use std::sync::RwLock; +use codex_config::ConfigLayerStack; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; -use codex_utils_absolute_path::AbsolutePathBuf; -use toml::Value as TomlValue; use tracing::info; use tracing::warn; -use crate::config::Config; -use crate::config::types::SkillsConfig; -use crate::config_loader::CloudRequirementsLoader; -use crate::config_loader::LoaderOverrides; -use crate::config_loader::load_config_layers_state; -use crate::plugins::PluginsManager; -use crate::skills::SkillLoadOutcome; -use crate::skills::build_implicit_skill_path_indexes; -use crate::skills::config_rules::SkillConfigRules; -use crate::skills::config_rules::resolve_disabled_skill_paths; -use crate::skills::config_rules::skill_config_rules_from_stack; -use crate::skills::loader::SkillRoot; -use crate::skills::loader::load_skills_from_roots; -use crate::skills::loader::skill_roots; -use crate::skills::system::install_system_skills; -use crate::skills::system::uninstall_system_skills; +use crate::SkillLoadOutcome; +use crate::build_implicit_skill_path_indexes; +use crate::config_rules::SkillConfigRules; +use crate::config_rules::resolve_disabled_skill_paths; +use crate::config_rules::skill_config_rules_from_stack; +use crate::loader::SkillRoot; +use crate::loader::load_skills_from_roots; +use crate::loader::skill_roots; +use crate::system::install_system_skills; +use crate::system::uninstall_system_skills; +use codex_config::SkillsConfig; + +#[derive(Debug, Clone)] +pub struct SkillsLoadInput { + pub cwd: PathBuf, + pub effective_skill_roots: Vec, + pub config_layer_stack: ConfigLayerStack, + pub bundled_skills_enabled: bool, +} + +impl SkillsLoadInput { + pub fn new( + cwd: PathBuf, + effective_skill_roots: Vec, + config_layer_stack: ConfigLayerStack, + bundled_skills_enabled: bool, + ) -> Self { + Self { + cwd, + effective_skill_roots, + config_layer_stack, + bundled_skills_enabled, + } + } +} pub struct SkillsManager { codex_home: PathBuf, - plugins_manager: Arc, restriction_product: Option, cache_by_cwd: RwLock>, cache_by_config: RwLock>, } impl SkillsManager { - pub fn new( - codex_home: PathBuf, - plugins_manager: Arc, - bundled_skills_enabled: bool, - ) -> Self { - Self::new_with_restriction_product( - codex_home, - plugins_manager, - bundled_skills_enabled, - Some(Product::Codex), - ) + pub fn new(codex_home: PathBuf, bundled_skills_enabled: bool) -> Self { + Self::new_with_restriction_product(codex_home, bundled_skills_enabled, Some(Product::Codex)) } pub fn new_with_restriction_product( codex_home: PathBuf, - plugins_manager: Arc, bundled_skills_enabled: bool, restriction_product: Option, ) -> Self { let manager = Self { codex_home, - plugins_manager, restriction_product, cache_by_cwd: RwLock::new(HashMap::new()), cache_by_config: RwLock::new(HashMap::new()), @@ -80,9 +86,9 @@ impl SkillsManager { /// This path uses a cache keyed by the effective skill-relevant config state rather than just /// cwd so role-local and session-local skill overrides cannot bleed across sessions that happen /// to share a directory. - pub fn skills_for_config(&self, config: &Config) -> SkillLoadOutcome { - let roots = self.skill_roots_for_config(config); - let skill_config_rules = skill_config_rules_from_stack(&config.config_layer_stack); + pub fn skills_for_config(&self, input: &SkillsLoadInput) -> SkillLoadOutcome { + let roots = self.skill_roots_for_config(input); + let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack); let cache_key = config_skills_cache_key(&roots, &skill_config_rules); if let Some(outcome) = self.cached_outcome_for_config(&cache_key) { return outcome; @@ -97,14 +103,13 @@ impl SkillsManager { outcome } - pub(crate) fn skill_roots_for_config(&self, config: &Config) -> Vec { - let loaded_plugins = self.plugins_manager.plugins_for_config(config); + pub fn skill_roots_for_config(&self, input: &SkillsLoadInput) -> Vec { let mut roots = skill_roots( - &config.config_layer_stack, - &config.cwd, - loaded_plugins.effective_skill_roots(), + &input.config_layer_stack, + input.cwd.as_path(), + input.effective_skill_roots.clone(), ); - if !config.bundled_skills_enabled() { + if !input.bundled_skills_enabled { roots.retain(|root| root.scope != SkillScope::System); } roots @@ -112,74 +117,34 @@ impl SkillsManager { pub async fn skills_for_cwd( &self, - cwd: &Path, - config: &Config, + input: &SkillsLoadInput, force_reload: bool, ) -> SkillLoadOutcome { - if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(cwd) { + if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(input.cwd.as_path()) { return outcome; } - self.skills_for_cwd_with_extra_user_roots(cwd, config, force_reload, &[]) + self.skills_for_cwd_with_extra_user_roots(input, force_reload, &[]) .await } pub async fn skills_for_cwd_with_extra_user_roots( &self, - cwd: &Path, - config: &Config, + input: &SkillsLoadInput, force_reload: bool, extra_user_roots: &[PathBuf], ) -> SkillLoadOutcome { - if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(cwd) { + if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(input.cwd.as_path()) { return outcome; } let normalized_extra_user_roots = normalize_extra_user_roots(extra_user_roots); - let cwd_abs = match AbsolutePathBuf::try_from(cwd) { - Ok(cwd_abs) => cwd_abs, - Err(err) => { - return SkillLoadOutcome { - errors: vec![crate::skills::model::SkillError { - path: cwd.to_path_buf(), - message: err.to_string(), - }], - ..Default::default() - }; - } - }; - - let cli_overrides: Vec<(String, TomlValue)> = Vec::new(); - let config_layer_stack = match load_config_layers_state( - &self.codex_home, - Some(cwd_abs), - &cli_overrides, - LoaderOverrides::default(), - CloudRequirementsLoader::default(), - ) - .await - { - Ok(config_layer_stack) => config_layer_stack, - Err(err) => { - return SkillLoadOutcome { - errors: vec![crate::skills::model::SkillError { - path: cwd.to_path_buf(), - message: err.to_string(), - }], - ..Default::default() - }; - } - }; - - let loaded_plugins = self - .plugins_manager - .plugins_for_config_with_force_reload(config, force_reload); let mut roots = skill_roots( - &config_layer_stack, - cwd, - loaded_plugins.effective_skill_roots(), + &input.config_layer_stack, + input.cwd.as_path(), + input.effective_skill_roots.clone(), ); - if !bundled_skills_enabled_from_stack(&config_layer_stack) { + if !bundled_skills_enabled_from_stack(&input.config_layer_stack) { roots.retain(|root| root.scope != SkillScope::System); } roots.extend( @@ -191,13 +156,13 @@ impl SkillsManager { scope: SkillScope::User, }), ); - let skill_config_rules = skill_config_rules_from_stack(&config_layer_stack); + let skill_config_rules = skill_config_rules_from_stack(&input.config_layer_stack); let outcome = self.build_skill_outcome(roots, &skill_config_rules); let mut cache = self .cache_by_cwd .write() .unwrap_or_else(std::sync::PoisonError::into_inner); - cache.insert(cwd.to_path_buf(), outcome.clone()); + cache.insert(input.cwd.clone(), outcome.clone()); outcome } @@ -206,7 +171,7 @@ impl SkillsManager { roots: Vec, skill_config_rules: &SkillConfigRules, ) -> SkillLoadOutcome { - let outcome = crate::skills::filter_skill_load_outcome_for_product( + let outcome = crate::filter_skill_load_outcome_for_product( load_skills_from_roots(roots), self.restriction_product, ); @@ -261,8 +226,8 @@ struct ConfigSkillsCacheKey { skill_config_rules: SkillConfigRules, } -pub(crate) fn bundled_skills_enabled_from_stack( - config_layer_stack: &crate::config_loader::ConfigLayerStack, +pub fn bundled_skills_enabled_from_stack( + config_layer_stack: &codex_config::ConfigLayerStack, ) -> bool { let effective_config = config_layer_stack.effective_config(); let Some(skills_value) = effective_config diff --git a/codex-rs/core/src/skills/manager_tests.rs b/codex-rs/core-skills/src/manager_tests.rs similarity index 67% rename from codex-rs/core/src/skills/manager_tests.rs rename to codex-rs/core-skills/src/manager_tests.rs index d9e5859f4..2507fc982 100644 --- a/codex-rs/core/src/skills/manager_tests.rs +++ b/codex-rs/core-skills/src/manager_tests.rs @@ -1,15 +1,15 @@ use super::*; -use crate::config::ConfigBuilder; -use crate::config::ConfigOverrides; -use crate::config_loader::ConfigLayerEntry; -use crate::config_loader::ConfigLayerStack; -use crate::config_loader::ConfigRequirementsToml; -use crate::plugins::PluginsManager; -use crate::skills::SkillMetadata; -use crate::skills::config_rules::resolve_disabled_skill_paths; -use crate::skills::config_rules::skill_config_rules_from_stack; +use crate::SkillMetadata; +use crate::config_rules::resolve_disabled_skill_paths; +use crate::config_rules::skill_config_rules_from_stack; use codex_app_server_protocol::ConfigLayerSource; +use codex_config::CONFIG_TOML_FILE; +use codex_config::ConfigLayerEntry; +use codex_config::ConfigLayerStack; +use codex_config::ConfigRequirementsToml; +use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; +use std::collections::HashSet; use std::fs; use std::path::PathBuf; use tempfile::TempDir; @@ -64,6 +64,77 @@ fn test_skill(name: &str, path: PathBuf) -> SkillMetadata { } } +fn user_config_layer(codex_home: &TempDir, config_toml: &str) -> ConfigLayerEntry { + let config_path = AbsolutePathBuf::try_from(codex_home.path().join(CONFIG_TOML_FILE)) + .expect("user config path should be absolute"); + ConfigLayerEntry::new( + ConfigLayerSource::User { file: config_path }, + toml::from_str(config_toml).expect("user layer toml"), + ) +} + +fn config_stack(codex_home: &TempDir, user_config_toml: &str) -> ConfigLayerStack { + ConfigLayerStack::new( + vec![user_config_layer(codex_home, user_config_toml)], + Default::default(), + ConfigRequirementsToml::default(), + ) + .expect("valid config layer stack") +} + +fn config_stack_with_session_flags( + codex_home: &TempDir, + user_config_toml: &str, + session_flags_toml: &str, +) -> ConfigLayerStack { + ConfigLayerStack::new( + vec![ + user_config_layer(codex_home, user_config_toml), + ConfigLayerEntry::new( + ConfigLayerSource::SessionFlags, + toml::from_str(session_flags_toml).expect("session layer toml"), + ), + ], + Default::default(), + ConfigRequirementsToml::default(), + ) + .expect("valid config layer stack") +} + +fn path_toggle_config(path: &std::path::Path, enabled: bool) -> String { + format!( + r#"[[skills.config]] +path = "{}" +enabled = {enabled} +"#, + path.display() + ) +} + +fn name_toggle_config(name: &str, enabled: bool) -> String { + format!( + r#"[[skills.config]] +name = "{name}" +enabled = {enabled} +"# + ) +} + +fn skills_for_config_with_stack( + skills_manager: &SkillsManager, + cwd: &TempDir, + config_layer_stack: &ConfigLayerStack, + effective_skill_roots: &[PathBuf], +) -> SkillLoadOutcome { + let skills_input = SkillsLoadInput::new( + cwd.path().to_path_buf(), + effective_skill_roots.to_vec(), + config_layer_stack.clone(), + bundled_skills_enabled_from_stack(config_layer_stack), + ); + skills_manager.skills_for_config(&skills_input) +} + #[test] fn new_with_disabled_bundled_skills_removes_stale_cached_system_skills() { let codex_home = tempfile::tempdir().expect("tempdir"); @@ -72,9 +143,7 @@ fn new_with_disabled_bundled_skills_removes_stale_cached_system_skills() { fs::write(stale_system_skill_dir.join("SKILL.md"), "# stale\n") .expect("write stale system skill"); - 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, false); + let _skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), false); assert!( !codex_home.path().join("skills/.system").exists(), @@ -86,22 +155,11 @@ fn new_with_disabled_bundled_skills_removes_stale_cached_system_skills() { async fn skills_for_config_reuses_cache_for_same_effective_config() { let codex_home = tempfile::tempdir().expect("tempdir"); let cwd = tempfile::tempdir().expect("tempdir"); - - let cfg = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .harness_overrides(ConfigOverrides { - cwd: Some(cwd.path().to_path_buf()), - ..Default::default() - }) - .build() - .await - .expect("defaults for test should always succeed"); - - 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 config_layer_stack = config_stack(&codex_home, ""); + let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), true); write_user_skill(&codex_home, "a", "skill-a", "from a"); - let outcome1 = skills_manager.skills_for_config(&cfg); + let outcome1 = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]); assert!( outcome1.skills.iter().any(|s| s.name == "skill-a"), "expected skill-a to be discovered" @@ -110,7 +168,7 @@ async fn skills_for_config_reuses_cache_for_same_effective_config() { // Write a new skill after the first call; the second call should reuse the config-aware cache // entry because the effective skill config is unchanged. write_user_skill(&codex_home, "b", "skill-b", "from b"); - let outcome2 = skills_manager.skills_for_config(&cfg); + let outcome2 = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]); assert_eq!(outcome2.errors, outcome1.errors); assert_eq!(outcome2.skills, outcome1.skills); } @@ -127,39 +185,23 @@ async fn skills_for_config_disables_plugin_skills_by_name() { "sample-search", "search sample data", ); - fs::write( - codex_home.path().join(crate::config::CONFIG_TOML_FILE), - r#"[features] -plugins = true - -[[skills.config]] -name = "sample:sample-search" -enabled = false - -[plugins."sample@test"] -enabled = true -"#, - ) - .expect("write config"); - - let config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .harness_overrides(ConfigOverrides { - cwd: Some(cwd.path().to_path_buf()), - ..Default::default() - }) - .build() - .await - .expect("load config"); - - 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, - config.bundled_skills_enabled(), + let config_layer_stack = config_stack( + &codex_home, + &name_toggle_config("sample:sample-search", false), ); + let plugin_skill_root = skill_path + .parent() + .and_then(std::path::Path::parent) + .expect("plugin skill should live under a skills root") + .to_path_buf(); + let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), true); - let outcome = skills_manager.skills_for_config(&config); + let outcome = skills_for_config_with_stack( + &skills_manager, + &cwd, + &config_layer_stack, + &[plugin_skill_root], + ); let skill = outcome .skills .iter() @@ -182,27 +224,21 @@ async fn skills_for_cwd_reuses_cached_entry_even_when_entry_has_extra_roots() { let codex_home = tempfile::tempdir().expect("tempdir"); let cwd = tempfile::tempdir().expect("tempdir"); let extra_root = tempfile::tempdir().expect("tempdir"); - - let config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .harness_overrides(ConfigOverrides { - cwd: Some(cwd.path().to_path_buf()), - ..Default::default() - }) - .build() - .await - .expect("defaults for test should always succeed"); - - 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 _ = skills_manager.skills_for_config(&config); + let config_layer_stack = config_stack(&codex_home, ""); + let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), true); + let _ = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]); write_user_skill(&extra_root, "x", "extra-skill", "from extra root"); let extra_root_path = extra_root.path().to_path_buf(); + let base_input = SkillsLoadInput::new( + cwd.path().to_path_buf(), + Vec::new(), + config_layer_stack.clone(), + bundled_skills_enabled_from_stack(&config_layer_stack), + ); let outcome_with_extra = skills_manager .skills_for_cwd_with_extra_user_roots( - cwd.path(), - &config, + &base_input, true, std::slice::from_ref(&extra_root_path), ) @@ -222,9 +258,13 @@ 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(), &config, false) - .await; + let base_input = SkillsLoadInput::new( + cwd.path().to_path_buf(), + Vec::new(), + config_layer_stack.clone(), + bundled_skills_enabled_from_stack(&config_layer_stack), + ); + let outcome_without_extra = skills_manager.skills_for_cwd(&base_input, false).await; assert_eq!(outcome_without_extra.skills, outcome_with_extra.skills); assert_eq!(outcome_without_extra.errors, outcome_with_extra.errors); } @@ -240,29 +280,8 @@ async fn skills_for_config_excludes_bundled_skills_when_disabled_in_config() { "---\nname: bundled-skill\ndescription: from bundled root\n---\n\n# Body\n", ) .expect("write bundled skill"); - - fs::write( - codex_home.path().join(crate::config::CONFIG_TOML_FILE), - "[skills.bundled]\nenabled = false\n", - ) - .expect("write config"); - - let config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .harness_overrides(ConfigOverrides { - cwd: Some(cwd.path().to_path_buf()), - ..Default::default() - }) - .build() - .await - .expect("load config"); - - 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, - config.bundled_skills_enabled(), - ); + let config_layer_stack = config_stack(&codex_home, "[skills.bundled]\nenabled = false\n"); + let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), false); // Recreate the cached bundled skill after startup cleanup so this assertion exercises // root selection rather than relying on directory removal succeeding. @@ -273,7 +292,7 @@ async fn skills_for_config_excludes_bundled_skills_when_disabled_in_config() { ) .expect("rewrite bundled skill"); - let outcome = skills_manager.skills_for_config(&config); + let outcome = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]); assert!( outcome .skills @@ -294,29 +313,23 @@ async fn skills_for_cwd_with_extra_roots_only_refreshes_on_force_reload() { let cwd = tempfile::tempdir().expect("tempdir"); let extra_root_a = tempfile::tempdir().expect("tempdir"); let extra_root_b = tempfile::tempdir().expect("tempdir"); - - let config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .harness_overrides(ConfigOverrides { - cwd: Some(cwd.path().to_path_buf()), - ..Default::default() - }) - .build() - .await - .expect("defaults for test should always succeed"); - - 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 _ = skills_manager.skills_for_config(&config); + let config_layer_stack = config_stack(&codex_home, ""); + let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), true); + let _ = skills_for_config_with_stack(&skills_manager, &cwd, &config_layer_stack, &[]); write_user_skill(&extra_root_a, "x", "extra-skill-a", "from extra root a"); write_user_skill(&extra_root_b, "x", "extra-skill-b", "from extra root b"); let extra_root_a_path = extra_root_a.path().to_path_buf(); + let base_input = SkillsLoadInput::new( + cwd.path().to_path_buf(), + Vec::new(), + config_layer_stack.clone(), + bundled_skills_enabled_from_stack(&config_layer_stack), + ); let outcome_a = skills_manager .skills_for_cwd_with_extra_user_roots( - cwd.path(), - &config, + &base_input, true, std::slice::from_ref(&extra_root_a_path), ) @@ -337,8 +350,7 @@ async fn skills_for_cwd_with_extra_roots_only_refreshes_on_force_reload() { let extra_root_b_path = extra_root_b.path().to_path_buf(); let outcome_b = skills_manager .skills_for_cwd_with_extra_user_roots( - cwd.path(), - &config, + &base_input, false, std::slice::from_ref(&extra_root_b_path), ) @@ -358,8 +370,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, + &base_input, true, std::slice::from_ref(&extra_root_b_path), ) @@ -399,25 +410,11 @@ fn disabled_paths_for_skills_allows_session_flags_to_override_user_layer() { .expect("user config path should be absolute"); let user_layer = ConfigLayerEntry::new( ConfigLayerSource::User { file: user_file }, - toml::from_str(&format!( - r#"[[skills.config]] -path = "{}" -enabled = false -"#, - skill_path.display() - )) - .expect("user layer toml"), + toml::from_str(&path_toggle_config(&skill_path, false)).expect("user layer toml"), ); let session_layer = ConfigLayerEntry::new( ConfigLayerSource::SessionFlags, - toml::from_str(&format!( - r#"[[skills.config]] -path = "{}" -enabled = true -"#, - skill_path.display() - )) - .expect("session layer toml"), + toml::from_str(&path_toggle_config(&skill_path, true)).expect("session layer toml"), ); let stack = ConfigLayerStack::new( vec![user_layer, session_layer], @@ -443,25 +440,11 @@ fn disabled_paths_for_skills_allows_session_flags_to_disable_user_enabled_skill( .expect("user config path should be absolute"); let user_layer = ConfigLayerEntry::new( ConfigLayerSource::User { file: user_file }, - toml::from_str(&format!( - r#"[[skills.config]] -path = "{}" -enabled = true -"#, - skill_path.display() - )) - .expect("user layer toml"), + toml::from_str(&path_toggle_config(&skill_path, true)).expect("user layer toml"), ); let session_layer = ConfigLayerEntry::new( ConfigLayerSource::SessionFlags, - toml::from_str(&format!( - r#"[[skills.config]] -path = "{}" -enabled = false -"#, - skill_path.display() - )) - .expect("session layer toml"), + toml::from_str(&path_toggle_config(&skill_path, false)).expect("session layer toml"), ); let stack = ConfigLayerStack::new( vec![user_layer, session_layer], @@ -487,13 +470,7 @@ fn disabled_paths_for_skills_disables_matching_name_selectors() { .expect("user config path should be absolute"); let user_layer = ConfigLayerEntry::new( ConfigLayerSource::User { file: user_file }, - toml::from_str( - r#"[[skills.config]] -name = "github:yeet" -enabled = false -"#, - ) - .expect("user layer toml"), + toml::from_str(&name_toggle_config("github:yeet", false)).expect("user layer toml"), ); let stack = ConfigLayerStack::new( vec![user_layer], @@ -519,24 +496,11 @@ fn disabled_paths_for_skills_allows_name_selector_to_override_path_selector() { .expect("user config path should be absolute"); let user_layer = ConfigLayerEntry::new( ConfigLayerSource::User { file: user_file }, - toml::from_str(&format!( - r#"[[skills.config]] -path = "{}" -enabled = false -"#, - skill_path.display() - )) - .expect("user layer toml"), + toml::from_str(&path_toggle_config(&skill_path, false)).expect("user layer toml"), ); let session_layer = ConfigLayerEntry::new( ConfigLayerSource::SessionFlags, - toml::from_str( - r#"[[skills.config]] -name = "github:yeet" -enabled = true -"#, - ) - .expect("session layer toml"), + toml::from_str(&name_toggle_config("github:yeet", true)).expect("session layer toml"), ); let stack = ConfigLayerStack::new( vec![user_layer, session_layer], @@ -565,58 +529,20 @@ async fn skills_for_config_ignores_cwd_cache_when_session_flags_reenable_skill() "---\nname: demo-skill\ndescription: demo description\n---\n\n# Body\n", ) .expect("write skill"); - fs::write( - codex_home.path().join(crate::config::CONFIG_TOML_FILE), - format!( - r#"[[skills.config]] -path = "{}" -enabled = false -"#, - skill_path.display() - ), - ) - .expect("write config"); - - let parent_config = ConfigBuilder::default() - .codex_home(codex_home.path().to_path_buf()) - .harness_overrides(ConfigOverrides { - cwd: Some(cwd.path().to_path_buf()), - ..Default::default() - }) - .build() - .await - .expect("load parent config"); - let role_path = codex_home.path().join("enable-role.toml"); - fs::write( - &role_path, - format!( - r#"[[skills.config]] -path = "{}" -enabled = true -"#, - skill_path.display() - ), - ) - .expect("write role config"); - let mut child_config = parent_config.clone(); - child_config.agent_roles.insert( - "custom".to_string(), - crate::config::AgentRoleConfig { - description: None, - config_file: Some(role_path), - nickname_candidates: None, - }, + let disabled_skill_config = path_toggle_config(&skill_path, false); + let enabled_skill_config = path_toggle_config(&skill_path, true); + let parent_stack = config_stack(&codex_home, &disabled_skill_config); + let child_stack = + config_stack_with_session_flags(&codex_home, &disabled_skill_config, &enabled_skill_config); + let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), true); + let parent_input = SkillsLoadInput::new( + cwd.path().to_path_buf(), + Vec::new(), + parent_stack.clone(), + bundled_skills_enabled_from_stack(&parent_stack), ); - crate::agent::role::apply_role_to_config(&mut child_config, Some("custom")) - .await - .expect("custom role should apply"); - 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(), &parent_config, true) - .await; + let parent_outcome = skills_manager.skills_for_cwd(&parent_input, true).await; let parent_skill = parent_outcome .skills .iter() @@ -624,7 +550,7 @@ enabled = true .expect("demo skill should be discovered"); assert_eq!(parent_outcome.is_skill_enabled(parent_skill), false); - let child_outcome = skills_manager.skills_for_config(&child_config); + let child_outcome = skills_for_config_with_stack(&skills_manager, &cwd, &child_stack, &[]); let child_skill = child_outcome .skills .iter() diff --git a/codex-rs/core-skills/src/mention_counts.rs b/codex-rs/core-skills/src/mention_counts.rs new file mode 100644 index 000000000..a9b3da9d3 --- /dev/null +++ b/codex-rs/core-skills/src/mention_counts.rs @@ -0,0 +1,24 @@ +use std::collections::HashMap; +use std::collections::HashSet; +use std::path::PathBuf; + +use super::SkillMetadata; + +/// Counts how often each skill name appears (exact and ASCII-lowercase), excluding disabled paths. +pub fn build_skill_name_counts( + skills: &[SkillMetadata], + disabled_paths: &HashSet, +) -> (HashMap, HashMap) { + let mut exact_counts: HashMap = HashMap::new(); + let mut lower_counts: HashMap = HashMap::new(); + for skill in skills { + if disabled_paths.contains(&skill.path_to_skills_md) { + continue; + } + *exact_counts.entry(skill.name.clone()).or_insert(0) += 1; + *lower_counts + .entry(skill.name.to_ascii_lowercase()) + .or_insert(0) += 1; + } + (exact_counts, lower_counts) +} diff --git a/codex-rs/core/src/skills/model.rs b/codex-rs/core-skills/src/model.rs similarity index 100% rename from codex-rs/core/src/skills/model.rs rename to codex-rs/core-skills/src/model.rs diff --git a/codex-rs/core/src/skills/remote.rs b/codex-rs/core-skills/src/remote.rs similarity index 95% rename from codex-rs/core/src/skills/remote.rs rename to codex-rs/core-skills/src/remote.rs index 165c45063..2dc620b86 100644 --- a/codex-rs/core/src/skills/remote.rs +++ b/codex-rs/core-skills/src/remote.rs @@ -6,9 +6,8 @@ use std::path::Path; use std::path::PathBuf; use std::time::Duration; -use crate::auth::CodexAuth; -use crate::config::Config; -use crate::default_client::build_reqwest_client; +use codex_login::CodexAuth; +use codex_login::default_client::build_reqwest_client; const REMOTE_SKILLS_API_TIMEOUT: Duration = Duration::from_secs(30); @@ -88,13 +87,13 @@ struct RemoteSkill { } pub async fn list_remote_skills( - config: &Config, + chatgpt_base_url: String, auth: Option<&CodexAuth>, scope: RemoteSkillScope, product_surface: RemoteSkillProductSurface, enabled: Option, ) -> Result> { - let base_url = config.chatgpt_base_url.trim_end_matches('/'); + let base_url = chatgpt_base_url.trim_end_matches('/'); let auth = ensure_chatgpt_auth(auth)?; let url = format!("{base_url}/hazelnuts"); @@ -146,14 +145,15 @@ pub async fn list_remote_skills( } pub async fn export_remote_skill( - config: &Config, + chatgpt_base_url: String, + codex_home: PathBuf, auth: Option<&CodexAuth>, skill_id: &str, ) -> Result { let auth = ensure_chatgpt_auth(auth)?; let client = build_reqwest_client(); - let base_url = config.chatgpt_base_url.trim_end_matches('/'); + let base_url = chatgpt_base_url.trim_end_matches('/'); let url = format!("{base_url}/hazelnuts/{skill_id}/export"); let mut request = client.get(&url).timeout(REMOTE_SKILLS_API_TIMEOUT); @@ -181,7 +181,7 @@ pub async fn export_remote_skill( anyhow::bail!("Downloaded remote skill payload is not a zip archive"); } - let output_dir = config.codex_home.join("skills").join(skill_id); + let output_dir = codex_home.join("skills").join(skill_id); tokio::fs::create_dir_all(&output_dir) .await .context("Failed to create downloaded skills directory")?; diff --git a/codex-rs/core/src/skills/render.rs b/codex-rs/core-skills/src/render.rs similarity index 98% rename from codex-rs/core/src/skills/render.rs rename to codex-rs/core-skills/src/render.rs index 797d53db2..9089a0543 100644 --- a/codex-rs/core/src/skills/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -1,4 +1,4 @@ -use crate::skills::model::SkillMetadata; +use crate::model::SkillMetadata; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; diff --git a/codex-rs/core/src/skills/system.rs b/codex-rs/core-skills/src/system.rs similarity index 100% rename from codex-rs/core/src/skills/system.rs rename to codex-rs/core-skills/src/system.rs diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 42ca739fa..25e44d763 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -35,11 +35,11 @@ codex-async-utils = { workspace = true } codex-code-mode = { workspace = true } codex-connectors = { workspace = true } codex-config = { workspace = true } +codex-core-skills = { workspace = true } codex-exec-server = { workspace = true } codex-features = { workspace = true } codex-login = { workspace = true } codex-shell-command = { workspace = true } -codex-skills = { workspace = true } codex-execpolicy = { workspace = true } codex-git-utils = { workspace = true } codex-hooks = { workspace = true } @@ -93,7 +93,6 @@ rmcp = { workspace = true, default-features = false, features = [ schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } -serde_yaml = { workspace = true } sha1 = { workspace = true } shlex = { workspace = true } similar = { workspace = true } diff --git a/codex-rs/core/src/agent/role_tests.rs b/codex-rs/core/src/agent/role_tests.rs index 7726ce795..c96bd2acf 100644 --- a/codex-rs/core/src/agent/role_tests.rs +++ b/codex-rs/core/src/agent/role_tests.rs @@ -1,9 +1,10 @@ use super::*; +use crate::SkillsManager; use crate::config::CONFIG_TOML_FILE; use crate::config::ConfigBuilder; use crate::config_loader::ConfigLayerStackOrdering; use crate::plugins::PluginsManager; -use crate::skills::SkillsManager; +use crate::skills_load_input_from_config; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::config_types::Verbosity; use codex_protocol::openai_models::ReasoningEffort; @@ -629,8 +630,11 @@ enabled = false .expect("custom role should apply"); let plugins_manager = Arc::new(PluginsManager::new(home.path().to_path_buf())); - let skills_manager = SkillsManager::new(home.path().to_path_buf(), plugins_manager, true); - let outcome = skills_manager.skills_for_config(&config); + let skills_manager = SkillsManager::new(home.path().to_path_buf(), true); + let plugin_outcome = plugins_manager.plugins_for_config(&config); + let effective_skill_roots = plugin_outcome.effective_skill_roots(); + let skills_input = skills_load_input_from_config(&config, effective_skill_roots); + let outcome = skills_manager.skills_for_config(&skills_input); let skill = outcome .skills .iter() diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 7da6799d6..ecc9d3dad 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -35,8 +35,9 @@ use crate::realtime_conversation::handle_audio as handle_realtime_conversation_a use crate::realtime_conversation::handle_close as handle_realtime_conversation_close; use crate::realtime_conversation::handle_start as handle_realtime_conversation_start; use crate::realtime_conversation::handle_text as handle_realtime_conversation_text; +use crate::render_skills_section; use crate::rollout::session_index; -use crate::skills::render_skills_section; +use crate::skills_load_input_from_config; use crate::stream_events_utils::HandleOutputCtx; use crate::stream_events_utils::handle_non_tool_response_item; use crate::stream_events_utils::handle_output_item_done; @@ -49,6 +50,10 @@ use async_channel::Receiver; use async_channel::Sender; use chrono::Local; use chrono::Utc; +use codex_analytics::AnalyticsEventsClient; +use codex_analytics::AppInvocation; +use codex_analytics::InvocationType; +use codex_analytics::build_track_events_context; use codex_app_server_protocol::McpServerElicitationRequest; use codex_app_server_protocol::McpServerElicitationRequestParams; use codex_exec_server::Environment; @@ -230,6 +235,14 @@ pub(crate) struct PreviousTurnSettings { pub(crate) realtime_active: Option, } +use crate::SkillError; +use crate::SkillInjections; +use crate::SkillLoadOutcome; +use crate::SkillMetadata; +use crate::SkillsManager; +use crate::build_skill_injections; +use crate::collect_env_var_dependencies; +use crate::collect_explicit_skill_mentions; use crate::exec_policy::ExecPolicyUpdateError; use crate::feedback_tags; use crate::guardian::GuardianReviewSessionManager; @@ -239,6 +252,9 @@ use crate::hook_runtime::record_additional_contexts; use crate::hook_runtime::record_pending_input; use crate::hook_runtime::run_pending_session_start_hooks; use crate::hook_runtime::run_user_prompt_submit_hooks; +use crate::injection::ToolMentionKind; +use crate::injection::app_id_from_path; +use crate::injection::tool_kind_for_path; use crate::instructions::UserInstructions; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::mcp::McpManager; @@ -296,6 +312,7 @@ use crate::protocol::TokenUsage; use crate::protocol::TokenUsageInfo; use crate::protocol::TurnDiffEvent; use crate::protocol::WarningEvent; +use crate::resolve_skill_dependencies_for_turn; use crate::rollout::RolloutRecorder; use crate::rollout::RolloutRecorderParams; use crate::rollout::map_session_init_error; @@ -304,18 +321,6 @@ use crate::rollout::policy::EventPersistenceMode; use crate::session_startup_prewarm::SessionStartupPrewarmHandle; use crate::shell; use crate::shell_snapshot::ShellSnapshot; -use crate::skills::SkillError; -use crate::skills::SkillInjections; -use crate::skills::SkillLoadOutcome; -use crate::skills::SkillMetadata; -use crate::skills::SkillsManager; -use crate::skills::build_skill_injections; -use crate::skills::collect_env_var_dependencies; -use crate::skills::collect_explicit_skill_mentions; -use crate::skills::injection::ToolMentionKind; -use crate::skills::injection::app_id_from_path; -use crate::skills::injection::tool_kind_for_path; -use crate::skills::resolve_skill_dependencies_for_turn; use crate::skills_watcher::SkillsWatcher; use crate::skills_watcher::SkillsWatcherEvent; use crate::state::ActiveTurn; @@ -345,10 +350,6 @@ use crate::turn_timing::record_turn_ttft_metric; use crate::unified_exec::UnifiedExecProcessManager; use crate::util::backoff; use crate::windows_sandbox::WindowsSandboxLevelExt; -use codex_analytics::AnalyticsEventsClient; -use codex_analytics::AppInvocation; -use codex_analytics::InvocationType; -use codex_analytics::build_track_events_context; use codex_async_utils::OrCancelExt; use codex_git_utils::get_git_repo_root; use codex_otel::SessionTelemetry; @@ -472,7 +473,10 @@ impl Codex { let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY); let (tx_event, rx_event) = async_channel::unbounded(); - let loaded_skills = skills_manager.skills_for_config(&config); + let plugin_outcome = plugins_manager.plugins_for_config(&config); + let effective_skill_roots = plugin_outcome.effective_skill_roots(); + let skills_input = skills_load_input_from_config(&config, effective_skill_roots); + let loaded_skills = skills_manager.skills_for_config(&skills_input); for err in &loaded_skills.errors { error!( @@ -2434,10 +2438,16 @@ impl Session { &per_turn_config, ) .await; + let plugin_outcome = self + .services + .plugins_manager + .plugins_for_config(&per_turn_config); + let effective_skill_roots = plugin_outcome.effective_skill_roots(); + let skills_input = skills_load_input_from_config(&per_turn_config, effective_skill_roots); let skills_outcome = Arc::new( self.services .skills_manager - .skills_for_config(&per_turn_config), + .skills_for_config(&skills_input), ); let mut turn_context: TurnContext = Self::make_turn_context( self.conversation_id, @@ -4492,8 +4502,14 @@ mod handlers { use crate::codex::SessionSettingsUpdate; use crate::codex::SteerInputError; + use crate::SkillError; use crate::codex::spawn_review_thread; use crate::config::Config; + use crate::config_loader::CloudRequirementsLoader; + use crate::config_loader::LoaderOverrides; + use crate::config_loader::load_config_layers_state; + use codex_features::Feature; + use codex_utils_absolute_path::AbsolutePathBuf; use crate::mcp::auth::compute_auth_statuses; use crate::mcp::collect_mcp_snapshot_from_manager; @@ -4929,11 +4945,64 @@ mod handlers { }; let skills_manager = &sess.services.skills_manager; + let plugins_manager = &sess.services.plugins_manager; let config = sess.get_config().await; + let codex_home = sess.codex_home().await; let mut skills = Vec::new(); + let empty_cli_overrides: &[(String, toml::Value)] = &[]; for cwd in cwds { + let cwd_abs = match AbsolutePathBuf::try_from(cwd.as_path()) { + Ok(path) => path, + Err(err) => { + let message = err.to_string(); + let cwd_for_entry = cwd.clone(); + skills.push(SkillsListEntry { + cwd: cwd_for_entry.clone(), + skills: Vec::new(), + errors: super::errors_to_info(&[SkillError { + path: cwd_for_entry, + message, + }]), + }); + continue; + } + }; + let config_layer_stack = match load_config_layers_state( + &codex_home, + Some(cwd_abs), + empty_cli_overrides, + LoaderOverrides::default(), + CloudRequirementsLoader::default(), + ) + .await + { + Ok(config_layer_stack) => config_layer_stack, + Err(err) => { + let message = err.to_string(); + let cwd_for_entry = cwd.clone(); + skills.push(SkillsListEntry { + cwd: cwd_for_entry.clone(), + skills: Vec::new(), + errors: super::errors_to_info(&[SkillError { + path: cwd_for_entry, + message, + }]), + }); + continue; + } + }; + let effective_skill_roots = plugins_manager.effective_skill_roots_for_layer_stack( + &config_layer_stack, + config.features.enabled(Feature::Plugins), + ); + let skills_input = crate::SkillsLoadInput::new( + cwd.clone(), + effective_skill_roots, + config_layer_stack, + config.bundled_skills_enabled(), + ); let outcome = skills_manager - .skills_for_cwd(&cwd, config.as_ref(), force_reload) + .skills_for_cwd(&skills_input, force_reload) .await; let errors = super::errors_to_info(&outcome.errors); let skills_metadata = super::skills_to_info(&outcome.skills, &outcome.disabled_paths); diff --git a/codex-rs/core/src/codex_tests.rs b/codex-rs/core/src/codex_tests.rs index 82371f5be..74aea1296 100644 --- a/codex-rs/core/src/codex_tests.rs +++ b/codex-rs/core/src/codex_tests.rs @@ -2341,7 +2341,10 @@ 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, &parent_config, true) + .skills_for_cwd( + &crate::skills_load_input_from_config(&parent_config, Vec::new()), + true, + ) .await; let parent_skill = parent_outcome .skills @@ -2537,11 +2540,7 @@ async fn session_new_fails_when_zsh_fork_enabled_without_zsh_path() { let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.clone())); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); - let skills_manager = Arc::new(SkillsManager::new( - config.codex_home.clone(), - Arc::clone(&plugins_manager), - true, - )); + let skills_manager = Arc::new(SkillsManager::new(config.codex_home.clone(), true)); let result = Session::new( session_configuration, Arc::clone(&config), @@ -2642,11 +2641,7 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { let state = SessionState::new(session_configuration.clone()); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.clone())); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); - let skills_manager = Arc::new(SkillsManager::new( - config.codex_home.clone(), - Arc::clone(&plugins_manager), - true, - )); + let skills_manager = Arc::new(SkillsManager::new(config.codex_home.clone(), true)); let network_approval = Arc::new(NetworkApprovalService::default()); let environment = Arc::new( codex_exec_server::Environment::create(None) @@ -2714,7 +2709,13 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { config.js_repl_node_module_dirs.clone(), )); - let skills_outcome = Arc::new(services.skills_manager.skills_for_config(&per_turn_config)); + let plugin_outcome = services + .plugins_manager + .plugins_for_config(&per_turn_config); + let effective_skill_roots = plugin_outcome.effective_skill_roots(); + let skills_input = + crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots); + let skills_outcome = Arc::new(services.skills_manager.skills_for_config(&skills_input)); let turn_context = Session::make_turn_context( conversation_id, Some(Arc::clone(&auth_manager)), @@ -3478,11 +3479,7 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( let state = SessionState::new(session_configuration.clone()); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.clone())); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); - let skills_manager = Arc::new(SkillsManager::new( - config.codex_home.clone(), - Arc::clone(&plugins_manager), - true, - )); + let skills_manager = Arc::new(SkillsManager::new(config.codex_home.clone(), true)); let network_approval = Arc::new(NetworkApprovalService::default()); let environment = Arc::new( codex_exec_server::Environment::create(None) @@ -3550,7 +3547,13 @@ pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx( config.js_repl_node_module_dirs.clone(), )); - let skills_outcome = Arc::new(services.skills_manager.skills_for_config(&per_turn_config)); + let plugin_outcome = services + .plugins_manager + .plugins_for_config(&per_turn_config); + let effective_skill_roots = plugin_outcome.effective_skill_roots(); + let skills_input = + crate::skills_load_input_from_config(&per_turn_config, effective_skill_roots); + let skills_outcome = Arc::new(services.skills_manager.skills_for_config(&skills_input)); let turn_context = Arc::new(Session::make_turn_context( conversation_id, Some(Arc::clone(&auth_manager)), diff --git a/codex-rs/core/src/codex_tests_guardian.rs b/codex-rs/core/src/codex_tests_guardian.rs index b576ff3eb..54f7405be 100644 --- a/codex-rs/core/src/codex_tests_guardian.rs +++ b/codex-rs/core/src/codex_tests_guardian.rs @@ -429,11 +429,7 @@ async fn guardian_subagent_does_not_inherit_parent_exec_policy_rules() { CollaborationModesConfig::default(), )); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.clone())); - let skills_manager = Arc::new(SkillsManager::new( - config.codex_home.clone(), - Arc::clone(&plugins_manager), - true, - )); + let skills_manager = Arc::new(SkillsManager::new(config.codex_home.clone(), true)); let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_watcher = Arc::new(SkillsWatcher::noop()); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index c785b0461..e799f1b9b 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -2849,7 +2849,7 @@ impl Config { } pub fn bundled_skills_enabled(&self) -> bool { - crate::skills::manager::bundled_skills_enabled_from_stack(&self.config_layer_stack) + crate::manager::bundled_skills_enabled_from_stack(&self.config_layer_stack) } } diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index 7a94700ce..c9f83482a 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -801,17 +801,9 @@ impl Notice { pub(crate) const TABLE_KEY: &'static str = "notice"; } -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)] -#[schemars(deny_unknown_fields)] -pub struct SkillConfig { - /// Path-based selector. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub path: Option, - /// Name-based selector. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub name: Option, - pub enabled: bool, -} +pub use codex_config::BundledSkillsConfig; +pub use codex_config::SkillConfig; +pub use codex_config::SkillsConfig; #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)] #[schemars(deny_unknown_fields)] @@ -820,29 +812,6 @@ pub struct PluginConfig { pub enabled: bool, } -#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] -#[schemars(deny_unknown_fields)] -pub struct SkillsConfig { - #[serde(default, skip_serializing_if = "Option::is_none")] - pub bundled: Option, - - #[serde(default, skip_serializing_if = "Vec::is_empty")] - pub config: Vec, -} - -#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)] -#[schemars(deny_unknown_fields)] -pub struct BundledSkillsConfig { - #[serde(default = "default_enabled")] - pub enabled: bool, -} - -impl Default for BundledSkillsConfig { - fn default() -> Self { - Self { enabled: true } - } -} - #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct SandboxWorkspaceWrite { diff --git a/codex-rs/core/src/config_loader/mod.rs b/codex-rs/core/src/config_loader/mod.rs index dab130dad..e4c503958 100644 --- a/codex-rs/core/src/config_loader/mod.rs +++ b/codex-rs/core/src/config_loader/mod.rs @@ -52,10 +52,12 @@ pub use codex_config::TextRange; pub use codex_config::WebSearchModeRequirement; pub(crate) use codex_config::build_cli_overrides_layer; pub(crate) use codex_config::config_error_from_toml; +pub use codex_config::default_project_root_markers; pub use codex_config::format_config_error; pub use codex_config::format_config_error_with_source; pub(crate) use codex_config::io_error_from_config_error; pub use codex_config::merge_toml_values; +pub use codex_config::project_root_markers_from_config; #[cfg(test)] pub(crate) use codex_config::version_for_toml; @@ -67,8 +69,6 @@ pub const SYSTEM_CONFIG_TOML_FILE_UNIX: &str = "/etc/codex/config.toml"; #[cfg(windows)] const DEFAULT_PROGRAM_DATA_DIR_WINDOWS: &str = r"C:\ProgramData"; -const DEFAULT_PROJECT_ROOT_MARKERS: &[&str] = &[".git"]; - pub(crate) async fn first_layer_config_error(layers: &ConfigLayerStack) -> Option { codex_config::first_layer_config_error::(layers, CONFIG_TOML_FILE).await } @@ -529,55 +529,6 @@ async fn load_requirements_from_legacy_scheme( Ok(()) } -/// Reads `project_root_markers` from the [toml::Value] produced by merging -/// `config.toml` from the config layers in the stack preceding -/// [ConfigLayerSource::Project]. -/// -/// Invariants: -/// - If `project_root_markers` is not specified, returns `Ok(None)`. -/// - If `project_root_markers` is specified, returns `Ok(Some(markers))` where -/// `markers` is a `Vec` (including `Ok(Some(Vec::new()))` for an -/// empty array, which indicates that root detection should be disabled). -/// - Returns an error if `project_root_markers` is specified but is not an -/// array of strings. -pub(crate) fn project_root_markers_from_config( - config: &TomlValue, -) -> io::Result>> { - let Some(table) = config.as_table() else { - return Ok(None); - }; - let Some(markers_value) = table.get("project_root_markers") else { - return Ok(None); - }; - let TomlValue::Array(entries) = markers_value else { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "project_root_markers must be an array of strings", - )); - }; - if entries.is_empty() { - return Ok(Some(Vec::new())); - } - let mut markers = Vec::new(); - for entry in entries { - let Some(marker) = entry.as_str() else { - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "project_root_markers must be an array of strings", - )); - }; - markers.push(marker.to_string()); - } - Ok(Some(markers)) -} - -pub(crate) fn default_project_root_markers() -> Vec { - DEFAULT_PROJECT_ROOT_MARKERS - .iter() - .map(ToString::to_string) - .collect() -} - struct ProjectTrustContext { project_root: AbsolutePathBuf, project_root_key: String, diff --git a/codex-rs/core/src/instructions/mod.rs b/codex-rs/core/src/instructions/mod.rs index 0ef48ff62..5af9a6e89 100644 --- a/codex-rs/core/src/instructions/mod.rs +++ b/codex-rs/core/src/instructions/mod.rs @@ -1,3 +1,2 @@ -pub(crate) use codex_instructions::SkillInstructions; pub use codex_instructions::USER_INSTRUCTIONS_PREFIX; pub(crate) use codex_instructions::UserInstructions; diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index b4426c9cd..446a1a27e 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -61,18 +61,44 @@ pub use text_encoding::bytes_to_string_smart; mod mcp_tool_call; mod memories; pub mod mention_syntax; -mod mentions; pub mod message_history; mod model_provider_info; pub mod utils; pub use utils::path_utils; pub mod personality_migration; pub mod plugins; +pub(crate) mod mentions { + pub(crate) use crate::plugins::build_connector_slug_counts; + pub(crate) use crate::plugins::build_skill_name_counts; + pub(crate) use crate::plugins::collect_explicit_app_ids; + pub(crate) use crate::plugins::collect_explicit_plugin_mentions; + pub(crate) use crate::plugins::collect_tool_mentions_from_messages; +} mod sandbox_tags; pub mod sandboxing; mod session_prefix; mod session_startup_prewarm; mod shell_detect; +pub mod skills; +pub(crate) use skills::SkillError; +pub(crate) use skills::SkillInjections; +pub(crate) use skills::SkillLoadOutcome; +pub(crate) use skills::SkillMetadata; +pub(crate) use skills::SkillsLoadInput; +pub(crate) use skills::SkillsManager; +pub(crate) use skills::build_skill_injections; +pub(crate) use skills::build_skill_name_counts; +pub(crate) use skills::collect_env_var_dependencies; +pub(crate) use skills::collect_explicit_skill_mentions; +pub(crate) use skills::config_rules; +pub(crate) use skills::injection; +pub(crate) use skills::loader; +pub(crate) use skills::manager; +pub(crate) use skills::maybe_emit_implicit_skill_invocation; +pub(crate) use skills::model; +pub(crate) use skills::render_skills_section; +pub(crate) use skills::resolve_skill_dependencies_for_turn; +pub(crate) use skills::skills_load_input_from_config; mod skills_watcher; mod stream_events_utils; pub mod test_support; @@ -126,7 +152,6 @@ pub mod seatbelt; mod session_rollout_init_error; pub mod shell; pub mod shell_snapshot; -pub mod skills; pub mod spawn; pub mod state_db_bridge; pub use codex_rollout::state_db; diff --git a/codex-rs/core/src/mcp/skill_dependencies.rs b/codex-rs/core/src/mcp/skill_dependencies.rs index dc2dc360e..489dc2e43 100644 --- a/codex-rs/core/src/mcp/skill_dependencies.rs +++ b/codex-rs/core/src/mcp/skill_dependencies.rs @@ -15,6 +15,7 @@ use super::auth::McpOAuthLoginSupport; use super::auth::oauth_login_support; use super::auth::resolve_oauth_scopes; use super::auth::should_retry_without_scopes; +use crate::SkillMetadata; use crate::codex::Session; use crate::codex::TurnContext; use crate::config::Config; @@ -24,8 +25,7 @@ use crate::config::types::McpServerConfig; use crate::config::types::McpServerTransportConfig; use crate::default_client::is_first_party_originator; use crate::default_client::originator; -use crate::skills::SkillMetadata; -use crate::skills::model::SkillToolDependency; +use crate::model::SkillToolDependency; use codex_features::Feature; const SKILL_MCP_DEPENDENCY_PROMPT_ID: &str = "skill_mcp_dependency_install"; diff --git a/codex-rs/core/src/mcp/skill_dependencies_tests.rs b/codex-rs/core/src/mcp/skill_dependencies_tests.rs index 49ba4e9f7..ebabe16f8 100644 --- a/codex-rs/core/src/mcp/skill_dependencies_tests.rs +++ b/codex-rs/core/src/mcp/skill_dependencies_tests.rs @@ -1,5 +1,5 @@ use super::*; -use crate::skills::model::SkillDependencies; +use crate::model::SkillDependencies; use codex_protocol::protocol::SkillScope; use pretty_assertions::assert_eq; use std::path::PathBuf; diff --git a/codex-rs/core/src/plugins/manager.rs b/codex-rs/core/src/plugins/manager.rs index 261cdf88d..3f3097d5e 100644 --- a/codex-rs/core/src/plugins/manager.rs +++ b/codex-rs/core/src/plugins/manager.rs @@ -27,6 +27,7 @@ use super::store::PluginStore; use super::store::PluginStoreError; use super::sync_openai_plugins_repo; use crate::AuthManager; +use crate::SkillMetadata; use crate::auth::CodexAuth; use crate::config::Config; use crate::config::ConfigService; @@ -36,12 +37,12 @@ use crate::config::edit::ConfigEditsBuilder; use crate::config::types::McpServerConfig; use crate::config::types::PluginConfig; use crate::config_loader::ConfigLayerStack; -use crate::skills::SkillMetadata; -use crate::skills::config_rules::SkillConfigRules; -use crate::skills::config_rules::resolve_disabled_skill_paths; -use crate::skills::config_rules::skill_config_rules_from_stack; -use crate::skills::loader::SkillRoot; -use crate::skills::loader::load_skills_from_roots; +use crate::config_rules::SkillConfigRules; +use crate::config_rules::resolve_disabled_skill_paths; +use crate::config_rules::skill_config_rules_from_stack; +use crate::loader::SkillRoot; +use crate::loader::load_skills_from_roots; +use codex_analytics::AnalyticsEventsClient; use codex_app_server_protocol::ConfigValueWriteParams; use codex_app_server_protocol::MergeStrategy; use codex_features::Feature; @@ -73,8 +74,6 @@ use toml_edit::value; use tracing::info; use tracing::warn; -use crate::AnalyticsEventsClient; - const DEFAULT_SKILLS_DIR_NAME: &str = "skills"; const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json"; const DEFAULT_APP_CONFIG_FILE: &str = ".app.json"; diff --git a/codex-rs/core/src/plugins/manifest.rs b/codex-rs/core/src/plugins/manifest.rs index d1b2c648e..8fbacb456 100644 --- a/codex-rs/core/src/plugins/manifest.rs +++ b/codex-rs/core/src/plugins/manifest.rs @@ -1,5 +1,5 @@ use codex_utils_absolute_path::AbsolutePathBuf; -pub(crate) use codex_utils_plugins::PLUGIN_MANIFEST_PATH; +use codex_utils_plugins::PLUGIN_MANIFEST_PATH; use serde::Deserialize; use serde_json::Value as JsonValue; use std::fs; diff --git a/codex-rs/core/src/mentions.rs b/codex-rs/core/src/plugins/mentions.rs similarity index 77% rename from codex-rs/core/src/mentions.rs rename to codex-rs/core/src/plugins/mentions.rs index fec7c40e4..5e11b9b91 100644 --- a/codex-rs/core/src/mentions.rs +++ b/codex-rs/core/src/plugins/mentions.rs @@ -1,19 +1,18 @@ use std::collections::HashMap; use std::collections::HashSet; -use std::path::PathBuf; use codex_protocol::user_input::UserInput; use crate::connectors; +use crate::injection::ToolMentionKind; +use crate::injection::app_id_from_path; +use crate::injection::extract_tool_mentions_with_sigil; +use crate::injection::plugin_config_name_from_path; +use crate::injection::tool_kind_for_path; use crate::mention_syntax::PLUGIN_TEXT_MENTION_SIGIL; use crate::mention_syntax::TOOL_MENTION_SIGIL; -use crate::plugins::PluginCapabilitySummary; -use crate::skills::SkillMetadata; -use crate::skills::injection::ToolMentionKind; -use crate::skills::injection::app_id_from_path; -use crate::skills::injection::extract_tool_mentions_with_sigil; -use crate::skills::injection::plugin_config_name_from_path; -use crate::skills::injection::tool_kind_for_path; + +use super::PluginCapabilitySummary; pub(crate) struct CollectedToolMentions { pub(crate) plain_names: HashSet, @@ -102,23 +101,7 @@ pub(crate) fn collect_explicit_plugin_mentions( .collect() } -pub(crate) fn build_skill_name_counts( - skills: &[SkillMetadata], - disabled_paths: &HashSet, -) -> (HashMap, HashMap) { - let mut exact_counts: HashMap = HashMap::new(); - let mut lower_counts: HashMap = HashMap::new(); - for skill in skills { - if disabled_paths.contains(&skill.path_to_skills_md) { - continue; - } - *exact_counts.entry(skill.name.clone()).or_insert(0) += 1; - *lower_counts - .entry(skill.name.to_ascii_lowercase()) - .or_insert(0) += 1; - } - (exact_counts, lower_counts) -} +pub(crate) use crate::build_skill_name_counts; pub(crate) fn build_connector_slug_counts( connectors: &[connectors::AppInfo], diff --git a/codex-rs/core/src/mentions_tests.rs b/codex-rs/core/src/plugins/mentions_tests.rs similarity index 100% rename from codex-rs/core/src/mentions_tests.rs rename to codex-rs/core/src/plugins/mentions_tests.rs diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index c16ebfcc5..3e4902214 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -5,6 +5,7 @@ mod injection; mod manager; mod manifest; mod marketplace; +mod mentions; mod remote; mod render; mod startup_sync; @@ -23,7 +24,6 @@ pub use codex_plugin::PluginTelemetryMetadata; pub type LoadedPlugin = codex_plugin::LoadedPlugin; pub type PluginLoadOutcome = codex_plugin::PluginLoadOutcome; -pub(crate) use codex_plugin::plugin_namespace_for_skill_path; pub(crate) use discoverable::list_tool_suggest_discoverable_plugins; pub(crate) use injection::build_plugin_injections; pub use manager::ConfiguredMarketplace; @@ -61,3 +61,9 @@ pub(crate) use startup_sync::curated_plugins_repo_path; pub(crate) use startup_sync::read_curated_plugins_sha; pub(crate) use startup_sync::sync_openai_plugins_repo; pub use toggles::collect_plugin_enabled_candidates; + +pub(crate) use mentions::build_connector_slug_counts; +pub(crate) use mentions::build_skill_name_counts; +pub(crate) use mentions::collect_explicit_app_ids; +pub(crate) use mentions::collect_explicit_plugin_mentions; +pub(crate) use mentions::collect_tool_mentions_from_messages; diff --git a/codex-rs/core/src/skills.rs b/codex-rs/core/src/skills.rs new file mode 100644 index 000000000..8d6886222 --- /dev/null +++ b/codex-rs/core/src/skills.rs @@ -0,0 +1,230 @@ +use std::collections::HashMap; +use std::collections::HashSet; +use std::env; +use std::path::Path; +use std::path::PathBuf; +use std::sync::Arc; + +use crate::codex::Session; +use crate::codex::TurnContext; +use crate::config::Config; +use codex_analytics::InvocationType; +use codex_analytics::SkillInvocation; +use codex_analytics::build_track_events_context; +use codex_protocol::protocol::SkillScope; +use codex_protocol::request_user_input::RequestUserInputArgs; +use codex_protocol::request_user_input::RequestUserInputQuestion; +use codex_protocol::request_user_input::RequestUserInputResponse; +use tracing::warn; + +pub use codex_core_skills::SkillDependencyInfo; +pub use codex_core_skills::SkillError; +pub use codex_core_skills::SkillLoadOutcome; +pub use codex_core_skills::SkillMetadata; +pub use codex_core_skills::SkillPolicy; +pub use codex_core_skills::SkillsLoadInput; +pub use codex_core_skills::SkillsManager; +pub use codex_core_skills::build_skill_name_counts; +pub use codex_core_skills::collect_env_var_dependencies; +pub use codex_core_skills::config_rules; +pub use codex_core_skills::detect_implicit_skill_invocation_for_command; +pub use codex_core_skills::filter_skill_load_outcome_for_product; +pub use codex_core_skills::injection; +pub use codex_core_skills::injection::SkillInjections; +pub use codex_core_skills::injection::build_skill_injections; +pub use codex_core_skills::injection::collect_explicit_skill_mentions; +pub use codex_core_skills::loader; +pub use codex_core_skills::manager; +pub use codex_core_skills::model; +pub use codex_core_skills::remote; +pub use codex_core_skills::render; +pub use codex_core_skills::render_skills_section; +pub use codex_core_skills::system; + +pub(crate) fn skills_load_input_from_config( + config: &Config, + effective_skill_roots: Vec, +) -> SkillsLoadInput { + SkillsLoadInput::new( + config.cwd.clone().to_path_buf(), + effective_skill_roots, + config.config_layer_stack.clone(), + config.bundled_skills_enabled(), + ) +} + +pub(crate) async fn resolve_skill_dependencies_for_turn( + sess: &Arc, + turn_context: &Arc, + dependencies: &[SkillDependencyInfo], +) { + if dependencies.is_empty() { + return; + } + + let existing_env = sess.dependency_env().await; + let mut loaded_values = HashMap::new(); + let mut missing = Vec::new(); + let mut seen_names = HashSet::new(); + + for dependency in dependencies { + let name = dependency.name.clone(); + if !seen_names.insert(name.clone()) || existing_env.contains_key(&name) { + continue; + } + match env::var(&name) { + Ok(value) => { + loaded_values.insert(name.clone(), value); + } + Err(env::VarError::NotPresent) => { + missing.push(dependency.clone()); + } + Err(err) => { + warn!("failed to read env var {name}: {err}"); + missing.push(dependency.clone()); + } + } + } + + if !loaded_values.is_empty() { + sess.set_dependency_env(loaded_values).await; + } + + if !missing.is_empty() { + request_skill_dependencies(sess, turn_context, &missing).await; + } +} + +async fn request_skill_dependencies( + sess: &Arc, + turn_context: &Arc, + dependencies: &[SkillDependencyInfo], +) { + let questions = dependencies + .iter() + .map(|dependency| { + let requirement = dependency.description.as_ref().map_or_else( + || { + format!( + "The skill \"{}\" requires \"{}\" to be set.", + dependency.skill_name, dependency.name + ) + }, + |description| { + format!( + "The skill \"{}\" requires \"{}\" to be set ({}).", + dependency.skill_name, dependency.name, description + ) + }, + ); + RequestUserInputQuestion { + id: dependency.name.clone(), + header: "Skill requires environment variable".to_string(), + question: format!( + "{requirement} This is an experimental internal feature. The value is stored in memory for this session only." + ), + is_other: false, + is_secret: true, + options: None, + } + }) + .collect::>(); + if questions.is_empty() { + return; + } + + let response = sess + .request_user_input( + turn_context, + format!("skill-deps-{}", turn_context.sub_id), + RequestUserInputArgs { questions }, + ) + .await + .unwrap_or_else(|| RequestUserInputResponse { + answers: HashMap::new(), + }); + if response.answers.is_empty() { + return; + } + + let mut values = HashMap::new(); + for (name, answer) in response.answers { + let mut user_note = None; + for entry in &answer.answers { + if let Some(note) = entry.strip_prefix("user_note: ") + && !note.trim().is_empty() + { + user_note = Some(note.trim().to_string()); + } + } + if let Some(value) = user_note { + values.insert(name, value); + } + } + if values.is_empty() { + return; + } + + sess.set_dependency_env(values).await; +} + +pub(crate) async fn maybe_emit_implicit_skill_invocation( + sess: &Session, + turn_context: &TurnContext, + command: &str, + workdir: &Path, +) { + let Some(candidate) = detect_implicit_skill_invocation_for_command( + turn_context.turn_skills.outcome.as_ref(), + command, + workdir, + ) else { + return; + }; + let invocation = SkillInvocation { + skill_name: candidate.name, + skill_scope: candidate.scope, + skill_path: candidate.path_to_skills_md, + invocation_type: InvocationType::Implicit, + }; + let skill_scope = match invocation.skill_scope { + SkillScope::User => "user", + SkillScope::Repo => "repo", + SkillScope::System => "system", + SkillScope::Admin => "admin", + }; + let skill_path = invocation.skill_path.to_string_lossy(); + let skill_name = invocation.skill_name.clone(); + let seen_key = format!("{skill_scope}:{skill_path}:{skill_name}"); + let inserted = { + let mut seen_skills = turn_context + .turn_skills + .implicit_invocation_seen_skills + .lock() + .await; + seen_skills.insert(seen_key) + }; + if !inserted { + return; + } + + turn_context.session_telemetry.counter( + "codex.skill.injected", + /*inc*/ 1, + &[ + ("status", "ok"), + ("skill", skill_name.as_str()), + ("invoke_type", "implicit"), + ], + ); + sess.services + .analytics_events_client + .track_skill_invocations( + build_track_events_context( + turn_context.model_info.slug.clone(), + sess.conversation_id.to_string(), + turn_context.sub_id.clone(), + ), + vec![invocation], + ); +} diff --git a/codex-rs/core/src/skills/env_var_dependencies.rs b/codex-rs/core/src/skills/env_var_dependencies.rs deleted file mode 100644 index 00f5bad8c..000000000 --- a/codex-rs/core/src/skills/env_var_dependencies.rs +++ /dev/null @@ -1,162 +0,0 @@ -use std::collections::HashMap; -use std::collections::HashSet; -use std::env; -use std::sync::Arc; - -use codex_protocol::request_user_input::RequestUserInputArgs; -use codex_protocol::request_user_input::RequestUserInputQuestion; -use codex_protocol::request_user_input::RequestUserInputResponse; -use tracing::warn; - -use crate::codex::Session; -use crate::codex::TurnContext; -use crate::skills::SkillMetadata; - -#[derive(Debug, Clone, PartialEq, Eq)] -pub(crate) struct SkillDependencyInfo { - pub(crate) skill_name: String, - pub(crate) name: String, - pub(crate) description: Option, -} - -/// Resolve required dependency values (session cache, then env vars), -/// and prompt the UI for any missing ones. -pub(crate) async fn resolve_skill_dependencies_for_turn( - sess: &Arc, - turn_context: &Arc, - dependencies: &[SkillDependencyInfo], -) { - if dependencies.is_empty() { - return; - } - - let existing_env = sess.dependency_env().await; - let mut loaded_values = HashMap::new(); - let mut missing = Vec::new(); - let mut seen_names = HashSet::new(); - - for dependency in dependencies { - let name = dependency.name.clone(); - if !seen_names.insert(name.clone()) { - continue; - } - if existing_env.contains_key(&name) { - continue; - } - match env::var(&name) { - Ok(value) => { - loaded_values.insert(name.clone(), value); - continue; - } - Err(env::VarError::NotPresent) => {} - Err(err) => { - warn!("failed to read env var {name}: {err}"); - } - } - missing.push(dependency.clone()); - } - - if !loaded_values.is_empty() { - sess.set_dependency_env(loaded_values).await; - } - - if !missing.is_empty() { - request_skill_dependencies(sess, turn_context, &missing).await; - } -} - -pub(crate) fn collect_env_var_dependencies( - mentioned_skills: &[SkillMetadata], -) -> Vec { - let mut dependencies = Vec::new(); - for skill in mentioned_skills { - let Some(skill_dependencies) = &skill.dependencies else { - continue; - }; - for tool in &skill_dependencies.tools { - if tool.r#type != "env_var" { - continue; - } - if tool.value.is_empty() { - continue; - } - dependencies.push(SkillDependencyInfo { - skill_name: skill.name.clone(), - name: tool.value.clone(), - description: tool.description.clone(), - }); - } - } - dependencies -} - -/// Prompt via request_user_input to gather missing env vars. -pub(crate) async fn request_skill_dependencies( - sess: &Arc, - turn_context: &Arc, - dependencies: &[SkillDependencyInfo], -) { - let questions = dependencies - .iter() - .map(|dep| { - let requirement = dep.description.as_ref().map_or_else( - || format!("The skill \"{}\" requires \"{}\" to be set.", dep.skill_name, dep.name), - |description| { - format!( - "The skill \"{}\" requires \"{}\" to be set ({}).", - dep.skill_name, dep.name, description - ) - }, - ); - let question = format!( - "{requirement} This is an experimental internal feature. The value is stored in memory for this session only.", - ); - RequestUserInputQuestion { - id: dep.name.clone(), - header: "Skill requires environment variable".to_string(), - question, - is_other: false, - is_secret: true, - options: None, - } - }) - .collect::>(); - - if questions.is_empty() { - return; - } - - let args = RequestUserInputArgs { questions }; - let call_id = format!("skill-deps-{}", turn_context.sub_id); - let response = sess - .request_user_input(turn_context, call_id, args) - .await - .unwrap_or_else(|| RequestUserInputResponse { - answers: HashMap::new(), - }); - - if response.answers.is_empty() { - return; - } - - let mut values = HashMap::new(); - for (name, answer) in response.answers { - let mut user_note = None; - for entry in &answer.answers { - if let Some(note) = entry.strip_prefix("user_note: ") - && !note.trim().is_empty() - { - user_note = Some(note.trim().to_string()); - } - } - if let Some(value) = user_note { - values.insert(name, value); - } - } - - if values.is_empty() { - return; - } - - sess.set_dependency_env(values).await; -} diff --git a/codex-rs/core/src/skills_watcher.rs b/codex-rs/core/src/skills_watcher.rs index fc0b86d90..947137353 100644 --- a/codex-rs/core/src/skills_watcher.rs +++ b/codex-rs/core/src/skills_watcher.rs @@ -8,6 +8,7 @@ use tokio::runtime::Handle; use tokio::sync::broadcast; use tracing::warn; +use crate::SkillsManager; use crate::config::Config; use crate::file_watcher::FileWatcher; use crate::file_watcher::FileWatcherSubscriber; @@ -15,7 +16,8 @@ use crate::file_watcher::Receiver; use crate::file_watcher::ThrottledWatchReceiver; use crate::file_watcher::WatchPath; use crate::file_watcher::WatchRegistration; -use crate::skills::SkillsManager; +use crate::plugins::PluginsManager; +use crate::skills_load_input_from_config; #[cfg(not(test))] const WATCHER_THROTTLE_INTERVAL: Duration = Duration::from_secs(10); @@ -56,9 +58,13 @@ impl SkillsWatcher { &self, config: &Config, skills_manager: &SkillsManager, + plugins_manager: &PluginsManager, ) -> WatchRegistration { + let plugin_outcome = plugins_manager.plugins_for_config(config); + let effective_skill_roots = plugin_outcome.effective_skill_roots(); + let skills_input = skills_load_input_from_config(config, effective_skill_roots); let roots = skills_manager - .skill_roots_for_config(config) + .skill_roots_for_config(&skills_input) .into_iter() .map(|root| WatchPath { path: root.path, diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index ad980db1a..d44c5eea7 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use crate::AuthManager; use crate::RolloutRecorder; +use crate::SkillsManager; use crate::agent::AgentControl; use crate::client::ModelClient; use crate::config::StartedNetworkProxy; @@ -11,7 +12,6 @@ use crate::mcp::McpManager; use crate::mcp_connection_manager::McpConnectionManager; use crate::models_manager::manager::ModelsManager; use crate::plugins::PluginsManager; -use crate::skills::SkillsManager; use crate::skills_watcher::SkillsWatcher; use crate::state_db::StateDbHandle; use crate::tools::code_mode::CodeModeService; diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 6ce55c622..511a6fc97 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -2,6 +2,7 @@ use crate::AuthManager; use crate::CodexAuth; use crate::ModelProviderInfo; use crate::OPENAI_PROVIDER_ID; +use crate::SkillsManager; use crate::agent::AgentControl; use crate::codex::Codex; use crate::codex::CodexSpawnArgs; @@ -22,7 +23,6 @@ use crate::protocol::SessionConfiguredEvent; use crate::rollout::RolloutRecorder; use crate::rollout::truncation; use crate::shell_snapshot::ShellSnapshot; -use crate::skills::SkillsManager; use crate::skills_watcher::SkillsWatcher; use crate::skills_watcher::SkillsWatcherEvent; use crate::tasks::interrupted_turn_history_marker; @@ -231,7 +231,6 @@ impl ThreadManager { let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_manager = Arc::new(SkillsManager::new_with_restriction_product( codex_home.clone(), - Arc::clone(&plugins_manager), config.bundled_skills_enabled(), restriction_product, )); @@ -297,7 +296,6 @@ impl ThreadManager { let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_manager = Arc::new(SkillsManager::new_with_restriction_product( codex_home.clone(), - Arc::clone(&plugins_manager), /*bundled_skills_enabled*/ true, restriction_product, )); @@ -835,9 +833,11 @@ impl ThreadManagerState { parent_trace: Option, user_shell_override: Option, ) -> CodexResult { - let watch_registration = self - .skills_watcher - .register_config(&config, self.skills_manager.as_ref()); + let watch_registration = self.skills_watcher.register_config( + &config, + self.skills_manager.as_ref(), + self.plugins_manager.as_ref(), + ); let CodexSpawnOk { codex, thread_id, .. } = Codex::spawn(CodexSpawnArgs { diff --git a/codex-rs/core/src/tools/handlers/shell.rs b/codex-rs/core/src/tools/handlers/shell.rs index 446ca100b..3c8bbe132 100644 --- a/codex-rs/core/src/tools/handlers/shell.rs +++ b/codex-rs/core/src/tools/handlers/shell.rs @@ -11,9 +11,9 @@ use crate::exec_env::create_env; use crate::exec_policy::ExecApprovalRequest; use crate::function_tool::FunctionCallError; use crate::is_safe_command::is_known_safe_command; +use crate::maybe_emit_implicit_skill_invocation; use crate::protocol::ExecCommandSource; use crate::shell::Shell; -use crate::skills::maybe_emit_implicit_skill_invocation; use crate::tools::context::FunctionToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; @@ -288,11 +288,12 @@ impl ToolHandler for ShellCommandHandler { let cwd = resolve_workdir_base_path(&arguments, turn.cwd.as_path())?; let params: ShellCommandToolCallParams = parse_arguments_with_base_path(&arguments, cwd.as_path())?; + let workdir = turn.resolve_path(params.workdir.clone()); maybe_emit_implicit_skill_invocation( session.as_ref(), turn.as_ref(), ¶ms.command, - params.workdir.as_deref(), + &workdir, ) .await; let prefix_rule = params.prefix_rule.clone(); diff --git a/codex-rs/core/src/tools/handlers/unified_exec.rs b/codex-rs/core/src/tools/handlers/unified_exec.rs index 54543277f..463168808 100644 --- a/codex-rs/core/src/tools/handlers/unified_exec.rs +++ b/codex-rs/core/src/tools/handlers/unified_exec.rs @@ -1,11 +1,11 @@ use crate::function_tool::FunctionCallError; use crate::is_safe_command::is_known_safe_command; +use crate::maybe_emit_implicit_skill_invocation; use crate::protocol::EventMsg; use crate::protocol::TerminalInteractionEvent; use crate::sandboxing::SandboxPermissions; use crate::shell::Shell; use crate::shell::get_shell_by_model_provided_path; -use crate::skills::maybe_emit_implicit_skill_invocation; use crate::tools::context::ExecCommandToolOutput; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolPayload; @@ -147,11 +147,12 @@ impl ToolHandler for UnifiedExecHandler { let cwd = resolve_workdir_base_path(&arguments, context.turn.cwd.as_path())?; let args: ExecCommandArgs = parse_arguments_with_base_path(&arguments, cwd.as_path())?; + let workdir = context.turn.resolve_path(args.workdir.clone()); maybe_emit_implicit_skill_invocation( session.as_ref(), - turn.as_ref(), + context.turn.as_ref(), &args.cmd, - args.workdir.as_deref(), + &workdir, ) .await; let process_id = manager.allocate_process_id().await; diff --git a/codex-rs/core/src/tools/runtimes/mod.rs b/codex-rs/core/src/tools/runtimes/mod.rs index bc2f56a43..aea23163f 100644 --- a/codex-rs/core/src/tools/runtimes/mod.rs +++ b/codex-rs/core/src/tools/runtimes/mod.rs @@ -4,9 +4,9 @@ Module: runtimes Concrete ToolRuntime implementations for specific tools. Each runtime stays small and focused and reuses the orchestrator for approvals + sandbox + retry. */ +use crate::SkillMetadata; use crate::path_utils; use crate::shell::Shell; -use crate::skills::SkillMetadata; use crate::tools::sandboxing::ToolError; use codex_protocol::models::PermissionProfile; use codex_sandboxing::SandboxCommand; 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 9df94e828..f40d09daf 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation.rs @@ -1,4 +1,5 @@ use super::ShellRequest; +use crate::SkillMetadata; use crate::error::CodexErr; use crate::error::SandboxErr; use crate::exec::ExecCapturePolicy; @@ -12,7 +13,7 @@ use crate::sandboxing::ExecOptions; use crate::sandboxing::ExecRequest; use crate::sandboxing::SandboxPermissions; use crate::shell::ShellType; -use crate::skills::SkillMetadata; +use crate::skills_load_input_from_config; use crate::tools::runtimes::ExecveSessionApproval; use crate::tools::runtimes::build_sandbox_command; use crate::tools::sandboxing::SandboxAttempt; @@ -487,11 +488,19 @@ impl CoreShellActionProvider { /// any skills. async fn find_skill(&self, program: &AbsolutePathBuf) -> Option { let force_reload = false; + let turn_config = self.turn.config.as_ref(); + let plugin_outcome = self + .session + .services + .plugins_manager + .plugins_for_config(turn_config); + let effective_skill_roots = plugin_outcome.effective_skill_roots(); + let skills_input = skills_load_input_from_config(turn_config, effective_skill_roots); let skills_outcome = self .session .services .skills_manager - .skills_for_cwd(&self.turn.cwd, self.turn.config.as_ref(), force_reload) + .skills_for_cwd(&skills_input, force_reload) .await; let program_path = program.as_path(); diff --git a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs index 25cbb71c0..844e9dcad 100644 --- a/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs +++ b/codex-rs/core/src/tools/runtimes/shell/unix_escalation_tests.rs @@ -8,6 +8,7 @@ use super::evaluate_intercepted_exec_policy; use super::extract_shell_script; use super::join_program_and_argv; use super::map_exec_result; +use crate::SkillMetadata; #[cfg(target_os = "macos")] use crate::config::Constrained; #[cfg(target_os = "macos")] @@ -19,7 +20,6 @@ use crate::protocol::GranularApprovalConfig; use crate::protocol::ReadOnlyAccess; use crate::protocol::SandboxPolicy; use crate::sandboxing::SandboxPermissions; -use crate::skills::SkillMetadata; use codex_execpolicy::Decision; use codex_execpolicy::Evaluation; use codex_execpolicy::PolicyParser; diff --git a/codex-rs/plugin/src/plugin_namespace.rs b/codex-rs/plugin/src/plugin_namespace.rs new file mode 100644 index 000000000..6688ae046 --- /dev/null +++ b/codex-rs/plugin/src/plugin_namespace.rs @@ -0,0 +1,70 @@ +//! Resolve plugin namespace from skill file paths by walking ancestors for `plugin.json`. + +use std::fs; +use std::path::Path; + +/// Relative path from a plugin root to its manifest file. +pub const PLUGIN_MANIFEST_PATH: &str = ".codex-plugin/plugin.json"; + +#[derive(serde::Deserialize)] +#[serde(rename_all = "camelCase")] +struct RawPluginManifestName { + #[serde(default)] + name: String, +} + +fn plugin_manifest_name(plugin_root: &Path) -> Option { + let manifest_path = plugin_root.join(PLUGIN_MANIFEST_PATH); + if !manifest_path.is_file() { + return None; + } + let contents = fs::read_to_string(&manifest_path).ok()?; + let RawPluginManifestName { name: raw_name } = serde_json::from_str(&contents).ok()?; + Some( + plugin_root + .file_name() + .and_then(|entry| entry.to_str()) + .filter(|_| raw_name.trim().is_empty()) + .unwrap_or(raw_name.as_str()) + .to_string(), + ) +} + +/// Returns the plugin manifest `name` for the nearest ancestor of `path` that contains a valid +/// plugin manifest (same `name` rules as full manifest loading in codex-core). +pub fn plugin_namespace_for_skill_path(path: &Path) -> Option { + for ancestor in path.ancestors() { + if let Some(name) = plugin_manifest_name(ancestor) { + return Some(name); + } + } + None +} + +#[cfg(test)] +mod tests { + use super::plugin_namespace_for_skill_path; + use std::fs; + use tempfile::tempdir; + + #[test] + fn uses_manifest_name() { + let tmp = tempdir().expect("tempdir"); + let plugin_root = tmp.path().join("plugins/sample"); + let skill_path = plugin_root.join("skills/search/SKILL.md"); + + fs::create_dir_all(skill_path.parent().expect("parent")).expect("mkdir"); + fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("mkdir manifest"); + fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ) + .expect("write manifest"); + fs::write(&skill_path, "---\ndescription: search\n---\n").expect("write skill"); + + assert_eq!( + plugin_namespace_for_skill_path(&skill_path), + Some("sample".to_string()) + ); + } +}