mirror of
https://github.com/openai/codex.git
synced 2026-09-05 15:18:41 +00:00
core-skills: key skill identity by source path
This commit is contained in:
@@ -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<AbsolutePathBuf>,
|
||||
outcome: &codex_core::skills::SkillLoadOutcome,
|
||||
) -> Vec<codex_app_server_protocol::SkillMetadata> {
|
||||
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 {
|
||||
|
||||
@@ -665,7 +665,11 @@ pub async fn load_plugin_skills(
|
||||
.into_iter()
|
||||
.filter(|skill| skill.matches_product_restriction_for_product(restriction_product))
|
||||
.collect::<Vec<_>>();
|
||||
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,
|
||||
|
||||
@@ -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<AbsolutePathBuf> {
|
||||
) -> HashSet<EnvironmentPathRef> {
|
||||
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);
|
||||
|
||||
@@ -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<AbsolutePathBuf>,
|
||||
connector_slug_counts: &HashMap<String, usize>,
|
||||
) -> Vec<SkillMetadata> {
|
||||
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,
|
||||
|
||||
@@ -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<Item = SkillRoot>,
|
||||
{
|
||||
let mut outcome = SkillLoadOutcome::default();
|
||||
let mut skill_roots: Vec<AbsolutePathBuf> = Vec::new();
|
||||
let mut skill_root_by_path: HashMap<AbsolutePathBuf, AbsolutePathBuf> = HashMap::new();
|
||||
let mut file_systems_by_skill_path: HashMap<AbsolutePathBuf, Arc<dyn ExecutorFileSystem>> =
|
||||
HashMap::new();
|
||||
let mut skill_roots: Vec<EnvironmentPathRef> = Vec::new();
|
||||
let mut skill_root_by_path: HashMap<EnvironmentPathRef, EnvironmentPathRef> = 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<AbsolutePathBuf> = HashSet::new();
|
||||
let mut seen: HashSet<EnvironmentPathRef> = HashSet::new();
|
||||
outcome
|
||||
.skills
|
||||
.retain(|skill| seen.insert(skill.path_to_skills_md.clone()));
|
||||
let retained_skill_paths: HashSet<AbsolutePathBuf> = outcome
|
||||
.retain(|skill| seen.insert(skill.source_path.clone()));
|
||||
let retained_skill_paths: HashSet<EnvironmentPathRef> = 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<AbsolutePathBuf> = skill_root_by_path.values().cloned().collect();
|
||||
let used_roots: HashSet<EnvironmentPathRef> = 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<SkillRoot>) {
|
||||
let mut seen: HashSet<AbsolutePathBuf> = HashSet::new();
|
||||
roots.retain(|root| seen.insert(root.path.clone()));
|
||||
let mut seen: HashSet<EnvironmentPathRef> = 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 {
|
||||
|
||||
@@ -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<Product>,
|
||||
extra_roots: RwLock<Vec<AbsolutePathBuf>>,
|
||||
cache_by_cwd: RwLock<HashMap<AbsolutePathBuf, SkillLoadOutcome>>,
|
||||
cache_by_cwd: RwLock<HashMap<EnvironmentPathRef, SkillLoadOutcome>>,
|
||||
cache_by_config: RwLock<HashMap<ConfigSkillsCacheKey, SkillLoadOutcome>>,
|
||||
}
|
||||
|
||||
@@ -147,9 +149,10 @@ impl SkillsManager {
|
||||
fs: Option<Arc<dyn ExecutorFileSystem>>,
|
||||
) -> 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<SkillLoadOutcome> {
|
||||
fn cached_outcome_for_cwd(&self, cwd: &EnvironmentPathRef) -> Option<SkillLoadOutcome> {
|
||||
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<String>)>,
|
||||
roots: Vec<(EnvironmentPathRef, u8, Option<String>)>,
|
||||
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<AbsolutePathBuf>,
|
||||
disabled_paths: HashSet<EnvironmentPathRef>,
|
||||
) -> 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<dyn ExecutorFileSystem>>,
|
||||
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;
|
||||
|
||||
@@ -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"),
|
||||
)])
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -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<EnvironmentPathRef>,
|
||||
) -> (HashMap<String, usize>, HashMap<String, usize>) {
|
||||
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<AbsolutePathBuf>,
|
||||
) -> (HashMap<String, usize>, HashMap<String, usize>) {
|
||||
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<String, usize>, HashMap<String, usize>) {
|
||||
let mut exact_counts: HashMap<String, usize> = HashMap::new();
|
||||
let mut lower_counts: HashMap<String, usize> = 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;
|
||||
|
||||
@@ -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<SkillMetadata>,
|
||||
pub errors: Vec<SkillError>,
|
||||
pub disabled_paths: HashSet<AbsolutePathBuf>,
|
||||
pub(crate) skill_roots: Vec<AbsolutePathBuf>,
|
||||
pub(crate) skill_root_by_path: Arc<HashMap<AbsolutePathBuf, AbsolutePathBuf>>,
|
||||
pub(crate) file_systems_by_skill_path: SkillFileSystemsByPath,
|
||||
pub disabled_paths: HashSet<EnvironmentPathRef>,
|
||||
pub(crate) skill_roots: Vec<EnvironmentPathRef>,
|
||||
pub(crate) skill_root_by_path: Arc<HashMap<EnvironmentPathRef, EnvironmentPathRef>>,
|
||||
pub(crate) implicit_skills_by_scripts_dir: Arc<HashMap<AbsolutePathBuf, SkillMetadata>>,
|
||||
pub(crate) implicit_skills_by_doc_path: Arc<HashMap<AbsolutePathBuf, SkillMetadata>>,
|
||||
}
|
||||
|
||||
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<Arc<dyn ExecutorFileSystem>> {
|
||||
self.file_systems_by_skill_path
|
||||
.get(&skill.path_to_skills_md)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Default)]
|
||||
pub(crate) struct SkillFileSystemsByPath {
|
||||
values: Arc<HashMap<AbsolutePathBuf, Arc<dyn ExecutorFileSystem>>>,
|
||||
}
|
||||
|
||||
impl SkillFileSystemsByPath {
|
||||
pub(crate) fn new(values: HashMap<AbsolutePathBuf, Arc<dyn ExecutorFileSystem>>) -> Self {
|
||||
Self {
|
||||
values: Arc::new(values),
|
||||
}
|
||||
}
|
||||
|
||||
fn get(&self, path: &AbsolutePathBuf) -> Option<Arc<dyn ExecutorFileSystem>> {
|
||||
self.values.get(path).map(Arc::clone)
|
||||
}
|
||||
|
||||
fn retain_paths(&mut self, paths: &HashSet<AbsolutePathBuf>) {
|
||||
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<AbsolutePathBuf> {
|
||||
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<AbsolutePathBuf> = outcome
|
||||
let retained_paths: HashSet<EnvironmentPathRef> = 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<AbsolutePathBuf> =
|
||||
let retained_roots: HashSet<EnvironmentPathRef> =
|
||||
outcome.skill_root_by_path.values().cloned().collect();
|
||||
outcome
|
||||
.skill_roots
|
||||
|
||||
@@ -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<AbsolutePathBuf, String>,
|
||||
alias_root_by_path: HashMap<AbsolutePathBuf, AbsolutePathBuf>,
|
||||
root_aliases: HashMap<EnvironmentPathRef, String>,
|
||||
alias_root_by_path: HashMap<EnvironmentPathRef, EnvironmentPathRef>,
|
||||
table_cost: usize,
|
||||
}
|
||||
|
||||
@@ -677,7 +678,7 @@ fn build_alias_plan(
|
||||
) -> Option<AliasPlan> {
|
||||
let skill_paths = skills
|
||||
.iter()
|
||||
.map(|skill| skill.path_to_skills_md.clone())
|
||||
.map(|skill| skill.source_path.clone())
|
||||
.collect::<HashSet<_>>();
|
||||
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<AbsolutePathBuf, AbsolutePathBuf>,
|
||||
) -> Option<Vec<AbsolutePathBuf>> {
|
||||
used_roots: &[EnvironmentPathRef],
|
||||
alias_root_by_skill_root: &HashMap<EnvironmentPathRef, EnvironmentPathRef>,
|
||||
) -> Option<Vec<EnvironmentPathRef>> {
|
||||
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, usize>,
|
||||
) -> 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<Item = &'a AbsolutePathBuf>,
|
||||
skill_roots: impl Iterator<Item = &'a EnvironmentPathRef>,
|
||||
) -> HashMap<AbsolutePathBuf, usize> {
|
||||
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<String> {
|
||||
fn build_skill_root_lines(roots: &[EnvironmentPathRef]) -> Vec<String> {
|
||||
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<String> {
|
||||
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<SkillMetadata>,
|
||||
roots: Vec<AbsolutePathBuf>,
|
||||
) -> SkillLoadOutcome {
|
||||
let roots = roots
|
||||
.into_iter()
|
||||
.map(EnvironmentPathRef::local)
|
||||
.collect::<Vec<_>>();
|
||||
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::<HashMap<_, _>>();
|
||||
SkillLoadOutcome {
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user