From beeb747028317b17c69d596b487ee690aba5f834 Mon Sep 17 00:00:00 2001 From: starr-openai Date: Mon, 1 Jun 2026 20:17:47 +0000 Subject: [PATCH] core-skills: key skill identity by source path --- .../request_processors/catalog_processor.rs | 12 ++-- codex-rs/core-plugins/src/loader.rs | 6 +- codex-rs/core-skills/src/config_rules.rs | 19 ++++-- codex-rs/core-skills/src/injection.rs | 15 ++--- codex-rs/core-skills/src/loader.rs | 40 ++++++----- codex-rs/core-skills/src/manager.rs | 31 +++++++-- codex-rs/core-skills/src/manager_tests.rs | 22 ++++--- codex-rs/core-skills/src/mention_counts.rs | 21 +++++- codex-rs/core-skills/src/model.rs | 66 ++++--------------- codex-rs/core-skills/src/render.rs | 46 +++++++------ codex-rs/core/src/session/turn.rs | 3 +- 11 files changed, 146 insertions(+), 135 deletions(-) diff --git a/codex-rs/app-server/src/request_processors/catalog_processor.rs b/codex-rs/app-server/src/request_processors/catalog_processor.rs index 6f17e4614f..c55c4e8e80 100644 --- a/codex-rs/app-server/src/request_processors/catalog_processor.rs +++ b/codex-rs/app-server/src/request_processors/catalog_processor.rs @@ -16,13 +16,11 @@ pub(crate) struct CatalogRequestProcessor { const SKILLS_LIST_CWD_CONCURRENCY: usize = 5; fn skills_to_info( - skills: &[codex_core::skills::SkillMetadata], - disabled_paths: &HashSet, + outcome: &codex_core::skills::SkillLoadOutcome, ) -> Vec { - skills - .iter() - .map(|skill| { - let enabled = !disabled_paths.contains(&skill.path_to_skills_md); + outcome + .skills_with_enabled() + .map(|(skill, enabled)| { codex_app_server_protocol::SkillMetadata { name: skill.name.clone(), description: skill.description.clone(), @@ -563,7 +561,7 @@ impl CatalogRequestProcessor { .skills_for_cwd(&skills_input, force_reload, fs) .await; let errors = errors_to_info(&outcome.errors); - let skills = skills_to_info(&outcome.skills, &outcome.disabled_paths); + let skills = skills_to_info(&outcome); ( index, codex_app_server_protocol::SkillsListEntry { diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 9e0ef7c471..27e56b1811 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -665,7 +665,11 @@ pub async fn load_plugin_skills( .into_iter() .filter(|skill| skill.matches_product_restriction_for_product(restriction_product)) .collect::>(); - let disabled_skill_paths = resolve_disabled_skill_paths(&skills, skill_config_rules); + // Local plugin summaries still expose raw disabled paths outside core-skills. + let disabled_skill_paths = resolve_disabled_skill_paths(&skills, skill_config_rules) + .into_iter() + .map(|path| path.path().clone()) + .collect(); ResolvedPluginSkills { skills, diff --git a/codex-rs/core-skills/src/config_rules.rs b/codex-rs/core-skills/src/config_rules.rs index 92ad2ab1a6..cb6acdbba5 100644 --- a/codex-rs/core-skills/src/config_rules.rs +++ b/codex-rs/core-skills/src/config_rules.rs @@ -5,6 +5,7 @@ use codex_config::ConfigLayerStack; use codex_config::ConfigLayerStackOrdering; use codex_config::SkillConfig; use codex_config::SkillsConfig; +use codex_exec_server::EnvironmentPathRef; use codex_utils_absolute_path::AbsolutePathBuf; use tracing::warn; @@ -71,23 +72,29 @@ pub fn skill_config_rules_from_stack(config_layer_stack: &ConfigLayerStack) -> S pub fn resolve_disabled_skill_paths( skills: &[SkillMetadata], rules: &SkillConfigRules, -) -> HashSet { +) -> HashSet { let mut disabled_paths = HashSet::new(); for entry in &rules.entries { match &entry.selector { SkillConfigRuleSelector::Path(path) => { - if entry.enabled { - disabled_paths.remove(path); - } else { - disabled_paths.insert(path.clone()); + for path in skills + .iter() + .filter(|skill| skill.path_to_skills_md == *path) + .map(|skill| skill.source_path.clone()) + { + if entry.enabled { + disabled_paths.remove(&path); + } else { + disabled_paths.insert(path); + } } } SkillConfigRuleSelector::Name(name) => { for path in skills .iter() .filter(|skill| skill.name == *name) - .map(|skill| skill.path_to_skills_md.clone()) + .map(|skill| skill.source_path.clone()) { if entry.enabled { disabled_paths.remove(&path); diff --git a/codex-rs/core-skills/src/injection.rs b/codex-rs/core-skills/src/injection.rs index 45979fa5c4..5dd704fec6 100644 --- a/codex-rs/core-skills/src/injection.rs +++ b/codex-rs/core-skills/src/injection.rs @@ -1,15 +1,13 @@ use std::collections::HashMap; use std::collections::HashSet; -use std::sync::Arc; use crate::SkillLoadOutcome; use crate::SkillMetadata; -use crate::build_skill_name_counts; +use crate::mention_counts::build_skill_name_counts_for_raw_paths; use codex_analytics::AnalyticsEventsClient; use codex_analytics::InvocationType; use codex_analytics::SkillInvocation; use codex_analytics::TrackEventsContext; -use codex_exec_server::LOCAL_FS; use codex_otel::SessionTelemetry; use codex_protocol::user_input::UserInput; use codex_utils_absolute_path::AbsolutePathBuf; @@ -30,7 +28,7 @@ pub struct SkillInjection { pub async fn build_skill_injections( mentioned_skills: &[SkillMetadata], - loaded_skills: Option<&SkillLoadOutcome>, + _loaded_skills: Option<&SkillLoadOutcome>, otel: Option<&SessionTelemetry>, analytics_client: &AnalyticsEventsClient, tracking: TrackEventsContext, @@ -46,10 +44,9 @@ pub async fn build_skill_injections( let mut invocations = Vec::new(); for skill in mentioned_skills { - let fs = loaded_skills - .and_then(|outcome| outcome.file_system_for_skill(skill)) - .unwrap_or_else(|| Arc::clone(&LOCAL_FS)); - match fs + match skill + .source_path + .file_system() .read_file_text(&skill.path_to_skills_md, /*sandbox*/ None) .await { @@ -118,7 +115,7 @@ pub fn collect_explicit_skill_mentions( disabled_paths: &HashSet, connector_slug_counts: &HashMap, ) -> Vec { - let skill_name_counts = build_skill_name_counts(skills, disabled_paths).0; + let skill_name_counts = build_skill_name_counts_for_raw_paths(skills, disabled_paths).0; let selection_context = SkillSelectionContext { skills, diff --git a/codex-rs/core-skills/src/loader.rs b/codex-rs/core-skills/src/loader.rs index 37f6e610b4..49dbcc754e 100644 --- a/codex-rs/core-skills/src/loader.rs +++ b/codex-rs/core-skills/src/loader.rs @@ -1,6 +1,5 @@ use crate::model::SkillDependencies; use crate::model::SkillError; -use crate::model::SkillFileSystemsByPath; use crate::model::SkillInterface; use crate::model::SkillLoadOutcome; use crate::model::SkillMetadata; @@ -164,13 +163,12 @@ where I: IntoIterator, { let mut outcome = SkillLoadOutcome::default(); - let mut skill_roots: Vec = Vec::new(); - let mut skill_root_by_path: HashMap = HashMap::new(); - let mut file_systems_by_skill_path: HashMap> = - HashMap::new(); + let mut skill_roots: Vec = Vec::new(); + let mut skill_root_by_path: HashMap = HashMap::new(); for root in roots { let root_path = canonicalize_for_skill_identity(&root.path); let fs = root.file_system; + let root_source_path = EnvironmentPathRef::new(Arc::clone(&fs), root_path.clone()); let skills_before_root = outcome.skills.len(); discover_skills_under_root( Arc::clone(&fs), @@ -182,34 +180,29 @@ where ) .await; for skill in &outcome.skills[skills_before_root..] { - if !skill_roots.contains(&root_path) { - skill_roots.push(root_path.clone()); + if !skill_roots.contains(&root_source_path) { + skill_roots.push(root_source_path.clone()); } skill_root_by_path - .entry(skill.path_to_skills_md.clone()) - .or_insert_with(|| root_path.clone()); - file_systems_by_skill_path - .entry(skill.path_to_skills_md.clone()) - .or_insert_with(|| Arc::clone(&fs)); + .entry(skill.source_path.clone()) + .or_insert_with(|| root_source_path.clone()); } } - let mut seen: HashSet = HashSet::new(); + let mut seen: HashSet = HashSet::new(); outcome .skills - .retain(|skill| seen.insert(skill.path_to_skills_md.clone())); - let retained_skill_paths: HashSet = outcome + .retain(|skill| seen.insert(skill.source_path.clone())); + let retained_skill_paths: HashSet = outcome .skills .iter() - .map(|skill| skill.path_to_skills_md.clone()) + .map(|skill| skill.source_path.clone()) .collect(); skill_root_by_path.retain(|path, _| retained_skill_paths.contains(path)); - let used_roots: HashSet = skill_root_by_path.values().cloned().collect(); + let used_roots: HashSet = skill_root_by_path.values().cloned().collect(); skill_roots.retain(|root| used_roots.contains(root)); - file_systems_by_skill_path.retain(|path, _| retained_skill_paths.contains(path)); outcome.skill_roots = skill_roots; outcome.skill_root_by_path = Arc::new(skill_root_by_path); - outcome.file_systems_by_skill_path = SkillFileSystemsByPath::new(file_systems_by_skill_path); fn scope_rank(scope: SkillScope) -> u8 { // Higher-priority scopes first (matches root scan order for dedupe). @@ -467,8 +460,13 @@ fn dirs_between_project_root_and_cwd( } fn dedupe_skill_roots_by_path(roots: &mut Vec) { - let mut seen: HashSet = HashSet::new(); - roots.retain(|root| seen.insert(root.path.clone())); + let mut seen: HashSet = HashSet::new(); + roots.retain(|root| { + seen.insert(EnvironmentPathRef::new( + Arc::clone(&root.file_system), + root.path.clone(), + )) + }); } fn canonicalize_for_skill_identity(path: &AbsolutePathBuf) -> AbsolutePathBuf { diff --git a/codex-rs/core-skills/src/manager.rs b/codex-rs/core-skills/src/manager.rs index 185eaa9320..0e5856d00f 100644 --- a/codex-rs/core-skills/src/manager.rs +++ b/codex-rs/core-skills/src/manager.rs @@ -4,7 +4,9 @@ use std::sync::Arc; use std::sync::RwLock; use codex_config::ConfigLayerStack; +use codex_exec_server::EnvironmentPathRef; use codex_exec_server::ExecutorFileSystem; +use codex_exec_server::LOCAL_FS; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; @@ -52,7 +54,7 @@ pub struct SkillsManager { codex_home: AbsolutePathBuf, restriction_product: Option, extra_roots: RwLock>, - cache_by_cwd: RwLock>, + cache_by_cwd: RwLock>, cache_by_config: RwLock>, } @@ -147,9 +149,10 @@ impl SkillsManager { fs: Option>, ) -> SkillLoadOutcome { let use_cwd_cache = fs.is_some(); + let cwd = environment_path_ref(fs.as_ref(), &input.cwd); if use_cwd_cache && !force_reload - && let Some(outcome) = self.cached_outcome_for_cwd(&input.cwd) + && let Some(outcome) = self.cached_outcome_for_cwd(&cwd) { return outcome; } @@ -172,7 +175,7 @@ impl SkillsManager { .cache_by_cwd .write() .unwrap_or_else(std::sync::PoisonError::into_inner); - cache.insert(input.cwd.clone(), outcome.clone()); + cache.insert(cwd, outcome.clone()); } outcome } @@ -213,7 +216,7 @@ impl SkillsManager { info!("skills cache cleared ({cleared} entries)"); } - fn cached_outcome_for_cwd(&self, cwd: &AbsolutePathBuf) -> Option { + fn cached_outcome_for_cwd(&self, cwd: &EnvironmentPathRef) -> Option { match self.cache_by_cwd.read() { Ok(cache) => cache.get(cwd).cloned(), Err(err) => err.into_inner().get(cwd).cloned(), @@ -240,7 +243,7 @@ impl SkillsManager { #[derive(Debug, Clone, PartialEq, Eq, Hash)] struct ConfigSkillsCacheKey { - roots: Vec<(AbsolutePathBuf, u8, Option)>, + roots: Vec<(EnvironmentPathRef, u8, Option)>, skill_config_rules: SkillConfigRules, } @@ -280,7 +283,11 @@ fn config_skills_cache_key( SkillScope::System => 2, SkillScope::Admin => 3, }; - (root.path.clone(), scope_rank, root.plugin_id.clone()) + ( + EnvironmentPathRef::new(Arc::clone(&root.file_system), root.path.clone()), + scope_rank, + root.plugin_id.clone(), + ) }) .collect(), skill_config_rules: skill_config_rules.clone(), @@ -289,7 +296,7 @@ fn config_skills_cache_key( fn finalize_skill_outcome( mut outcome: SkillLoadOutcome, - disabled_paths: HashSet, + disabled_paths: HashSet, ) -> SkillLoadOutcome { outcome.disabled_paths = disabled_paths; let (by_scripts_dir, by_doc_path) = @@ -299,6 +306,16 @@ fn finalize_skill_outcome( outcome } +fn environment_path_ref( + fs: Option<&Arc>, + path: &AbsolutePathBuf, +) -> EnvironmentPathRef { + EnvironmentPathRef::new( + fs.map_or_else(|| Arc::clone(&LOCAL_FS), Arc::clone), + path.clone(), + ) +} + #[cfg(test)] #[path = "manager_tests.rs"] mod tests; diff --git a/codex-rs/core-skills/src/manager_tests.rs b/codex-rs/core-skills/src/manager_tests.rs index bdf1294ddc..a5df2acdd6 100644 --- a/codex-rs/core-skills/src/manager_tests.rs +++ b/codex-rs/core-skills/src/manager_tests.rs @@ -386,7 +386,7 @@ async fn skills_for_config_disables_plugin_skills_by_name() { .abs(); assert_eq!(skill.path_to_skills_md, skill_path); - assert!(outcome.disabled_paths.contains(&skill.path_to_skills_md)); + assert!(outcome.disabled_paths.contains(&skill.source_path)); assert!( !outcome .allowed_skills_for_implicit_invocation() @@ -686,10 +686,12 @@ fn disabled_paths_for_skills_allows_session_flags_to_disable_user_enabled_skill( let skill_config_rules = skill_config_rules_from_stack(&stack); assert_eq!( resolve_disabled_skill_paths(&[skill], &skill_config_rules), - HashSet::from([skill_path - .abs() - .canonicalize() - .expect("skill path should canonicalize")]) + HashSet::from([EnvironmentPathRef::local( + skill_path + .abs() + .canonicalize() + .expect("skill path should canonicalize"), + )]) ); } @@ -719,10 +721,12 @@ fn disabled_paths_for_skills_disables_matching_name_selectors() { let skill_config_rules = skill_config_rules_from_stack(&stack); assert_eq!( resolve_disabled_skill_paths(&[skill], &skill_config_rules), - HashSet::from([skill_path - .abs() - .canonicalize() - .expect("skill path should canonicalize")]) + HashSet::from([EnvironmentPathRef::local( + skill_path + .abs() + .canonicalize() + .expect("skill path should canonicalize"), + )]) ); } diff --git a/codex-rs/core-skills/src/mention_counts.rs b/codex-rs/core-skills/src/mention_counts.rs index b7482ca36e..daa79fed5e 100644 --- a/codex-rs/core-skills/src/mention_counts.rs +++ b/codex-rs/core-skills/src/mention_counts.rs @@ -2,17 +2,36 @@ use std::collections::HashMap; use std::collections::HashSet; use super::SkillMetadata; +use codex_exec_server::EnvironmentPathRef; use codex_utils_absolute_path::AbsolutePathBuf; /// 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) { + build_skill_name_counts_with_disabled(skills, |skill| { + disabled_paths.contains(&skill.source_path) + }) +} + +pub(crate) fn build_skill_name_counts_for_raw_paths( skills: &[SkillMetadata], disabled_paths: &HashSet, +) -> (HashMap, HashMap) { + build_skill_name_counts_with_disabled(skills, |skill| { + disabled_paths.contains(&skill.path_to_skills_md) + }) +} + +fn build_skill_name_counts_with_disabled( + skills: &[SkillMetadata], + mut is_disabled: impl FnMut(&SkillMetadata) -> bool, ) -> (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) { + if is_disabled(skill) { continue; } *exact_counts.entry(skill.name.clone()).or_insert(0) += 1; diff --git a/codex-rs/core-skills/src/model.rs b/codex-rs/core-skills/src/model.rs index 9de985803d..d5e950e16d 100644 --- a/codex-rs/core-skills/src/model.rs +++ b/codex-rs/core-skills/src/model.rs @@ -1,10 +1,8 @@ use std::collections::HashMap; use std::collections::HashSet; -use std::fmt; use std::sync::Arc; use codex_exec_server::EnvironmentPathRef; -use codex_exec_server::ExecutorFileSystem; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; @@ -92,17 +90,16 @@ pub struct SkillError { pub struct SkillLoadOutcome { pub skills: Vec, pub errors: Vec, - pub disabled_paths: HashSet, - pub(crate) skill_roots: Vec, - pub(crate) skill_root_by_path: Arc>, - pub(crate) file_systems_by_skill_path: SkillFileSystemsByPath, + pub disabled_paths: HashSet, + pub(crate) skill_roots: Vec, + pub(crate) skill_root_by_path: Arc>, pub(crate) implicit_skills_by_scripts_dir: Arc>, pub(crate) implicit_skills_by_doc_path: Arc>, } impl SkillLoadOutcome { pub fn is_skill_enabled(&self, skill: &SkillMetadata) -> bool { - !self.disabled_paths.contains(&skill.path_to_skills_md) + !self.disabled_paths.contains(&skill.source_path) } pub fn is_skill_allowed_for_implicit_invocation(&self, skill: &SkillMetadata) -> bool { @@ -123,47 +120,11 @@ impl SkillLoadOutcome { .map(|skill| (skill, self.is_skill_enabled(skill))) } - pub(crate) fn file_system_for_skill( - &self, - skill: &SkillMetadata, - ) -> Option> { - self.file_systems_by_skill_path - .get(&skill.path_to_skills_md) - } -} - -#[derive(Clone, Default)] -pub(crate) struct SkillFileSystemsByPath { - values: Arc>>, -} - -impl SkillFileSystemsByPath { - pub(crate) fn new(values: HashMap>) -> Self { - Self { - values: Arc::new(values), - } - } - - fn get(&self, path: &AbsolutePathBuf) -> Option> { - self.values.get(path).map(Arc::clone) - } - - fn retain_paths(&mut self, paths: &HashSet) { - self.values = Arc::new( - self.values - .iter() - .filter(|(path, _)| paths.contains(*path)) - .map(|(path, fs)| (path.clone(), Arc::clone(fs))) - .collect(), - ); - } -} - -impl fmt::Debug for SkillFileSystemsByPath { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("SkillFileSystemsByPath") - .field("len", &self.values.len()) - .finish() + pub fn disabled_path_values(&self) -> HashSet { + self.disabled_paths + .iter() + .map(|path| path.path().clone()) + .collect() } } @@ -174,14 +135,11 @@ pub fn filter_skill_load_outcome_for_product( outcome .skills .retain(|skill| skill.matches_product_restriction_for_product(restriction_product)); - let retained_paths: HashSet = outcome + let retained_paths: HashSet = outcome .skills .iter() - .map(|skill| skill.path_to_skills_md.clone()) + .map(|skill| skill.source_path.clone()) .collect(); - outcome - .file_systems_by_skill_path - .retain_paths(&retained_paths); outcome.skill_root_by_path = Arc::new( outcome .skill_root_by_path @@ -190,7 +148,7 @@ pub fn filter_skill_load_outcome_for_product( .map(|(path, root)| (path.clone(), root.clone())) .collect(), ); - let retained_roots: HashSet = + let retained_roots: HashSet = outcome.skill_root_by_path.values().cloned().collect(); outcome .skill_roots diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index 0d8e64ebbd..4abbca5700 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -5,6 +5,7 @@ use std::path::Path; use crate::model::SkillLoadOutcome; use crate::model::SkillMetadata; +use codex_exec_server::EnvironmentPathRef; use codex_otel::SessionTelemetry; use codex_otel::THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC; use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC; @@ -665,8 +666,8 @@ struct SkillPathAliases { struct AliasPlan { aliases: SkillPathAliases, - root_aliases: HashMap, - alias_root_by_path: HashMap, + root_aliases: HashMap, + alias_root_by_path: HashMap, table_cost: usize, } @@ -677,7 +678,7 @@ fn build_alias_plan( ) -> Option { let skill_paths = skills .iter() - .map(|skill| skill.path_to_skills_md.clone()) + .map(|skill| skill.source_path.clone()) .collect::>(); let skill_root_by_path = outcome .skill_root_by_path @@ -736,9 +737,9 @@ fn build_alias_plan( } fn ordered_alias_roots( - used_roots: &[AbsolutePathBuf], - alias_root_by_skill_root: &HashMap, -) -> Option> { + used_roots: &[EnvironmentPathRef], + alias_root_by_skill_root: &HashMap, +) -> Option> { let mut seen = HashSet::new(); let mut alias_roots = Vec::new(); for root in used_roots { @@ -751,10 +752,10 @@ fn ordered_alias_roots( } fn alias_root_for_skill_root( - root: &AbsolutePathBuf, + root: &EnvironmentPathRef, plugin_version_skill_counts: &HashMap, -) -> AbsolutePathBuf { - let Some(plugin_version_base) = plugin_version_base(root.as_path()) else { +) -> EnvironmentPathRef { + let Some(plugin_version_base) = plugin_version_base(root.path().as_path()) else { return root.clone(); }; let skill_count = plugin_version_skill_counts @@ -764,16 +765,18 @@ fn alias_root_for_skill_root( if skill_count > 1 { root.clone() } else { - plugin_marketplace_base(root.as_path()).unwrap_or_else(|| root.clone()) + root.with_path( + plugin_marketplace_base(root.path().as_path()).unwrap_or_else(|| root.path().clone()), + ) } } fn plugin_version_skill_counts_for_skill_roots<'a>( - skill_roots: impl Iterator, + skill_roots: impl Iterator, ) -> HashMap { let mut counts = HashMap::new(); for root in skill_roots { - if let Some(plugin_version_base) = plugin_version_base(root.as_path()) { + if let Some(plugin_version_base) = plugin_version_base(root.path().as_path()) { let count = counts.entry(plugin_version_base).or_insert(0usize); *count = count.saturating_add(1); } @@ -793,12 +796,12 @@ fn aliased_metadata_overhead_cost( .saturating_sub(budget.cost(&absolute_body)) } -fn build_skill_root_lines(roots: &[AbsolutePathBuf]) -> Vec { +fn build_skill_root_lines(roots: &[EnvironmentPathRef]) -> Vec { roots .iter() .enumerate() .map(|(index, root)| { - let root_str = root.to_string_lossy().replace('\\', "/"); + let root_str = root.path().to_string_lossy().replace('\\', "/"); format!("- `r{index}` = `{root_str}`") }) .collect() @@ -840,12 +843,13 @@ fn render_skill_path_with_aliases(skill: &SkillMetadata, plan: &AliasPlan) -> St } fn outcome_relative_skill_path(skill: &SkillMetadata, plan: &AliasPlan) -> Option { - let alias_root = plan.alias_root_by_path.get(&skill.path_to_skills_md)?; + let alias_root = plan.alias_root_by_path.get(&skill.source_path)?; let alias = plan.root_aliases.get(alias_root)?; let relative_path = skill - .path_to_skills_md + .source_path + .path() .as_path() - .strip_prefix(alias_root.as_path()) + .strip_prefix(alias_root.path().as_path()) .ok()?; let relative_path = relative_path.to_string_lossy().replace('\\', "/"); Some(format!("{alias}/{relative_path}")) @@ -950,6 +954,10 @@ mod tests { skills: Vec, roots: Vec, ) -> SkillLoadOutcome { + let roots = roots + .into_iter() + .map(EnvironmentPathRef::local) + .collect::>(); let skill_root_by_path = skills .iter() .filter_map(|skill| { @@ -959,9 +967,9 @@ mod tests { skill .path_to_skills_md .as_path() - .starts_with(root.as_path()) + .starts_with(root.path().as_path()) }) - .map(|root| (skill.path_to_skills_md.clone(), root.clone())) + .map(|root| (skill.source_path.clone(), root.clone())) }) .collect::>(); SkillLoadOutcome { diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index f808134f99..40d6d50050 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -513,10 +513,11 @@ async fn build_skills_and_plugins( let connector_slug_counts = build_connector_slug_counts(&available_connectors); let skill_name_counts_lower = build_skill_name_counts(&skills_outcome.skills, &skills_outcome.disabled_paths).1; + let disabled_skill_paths = skills_outcome.disabled_path_values(); let mentioned_skills = collect_explicit_skill_mentions( &user_input, &skills_outcome.skills, - &skills_outcome.disabled_paths, + &disabled_skill_paths, &connector_slug_counts, ); maybe_prompt_and_install_mcp_dependencies(