Move explicit skill selection into the skills crate (#37177)

## What changed

- Add `ExplicitSkillLookup` to decouple explicit mention selection from the
  core skill-loading model.
- Export `collect_explicit_skill_mentions` from `codex-skills` and implement
  the lookup interface for `SkillLoadOutcome`.
- Move the selection tests into `codex-skills` while keeping the prompt-size
  boundary test with prompt injection.

GitOrigin-RevId: 21105af31c9b519c231c5ea02464da560705c293

Author: felixxia-oai

codex-rs/core-skills/src/injection.rs

@@ -1,18 +1,15 @@
-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 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_otel::sanitize_metric_tag_value;
-use codex_protocol::user_input::UserInput;
 pub use codex_skills::ToolMentionKind;
 pub use codex_skills::ToolMentions;
 pub use codex_skills::app_id_from_path;
@@ -21,7 +18,6 @@ pub use codex_skills::extract_tool_mentions_with_sigil;
 pub use codex_skills::normalize_skill_path;
 pub use codex_skills::plugin_config_name_from_path;
 pub use codex_skills::tool_kind_for_path;
-use codex_utils_absolute_path::AbsolutePathBuf;
 use codex_utils_path_uri::PathUri;
 use codex_utils_string::take_bytes_at_char_boundary;
 
@@ -177,169 +173,6 @@ fn emit_skill_injected_metric(
     );
 }
 
-/// Collect explicitly mentioned skills from structured and text mentions.
-///
-/// Structured `UserInput::Skill` selections are resolved first by path against
-/// enabled skills. Text inputs are then scanned to extract `$skill-name` tokens, and we
-/// iterate loaded skills in their existing order to preserve prior ordering semantics.
-/// Explicit paths match either a skill's canonical identity or its logical discovery
-/// path, and plain names are only used when the match is unambiguous.
-///
-/// 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 fn collect_explicit_skill_mentions(
-    inputs: &[UserInput],
-    loaded_skills: &SkillLoadOutcome,
-    connector_slug_counts: &HashMap<String, usize>,
-) -> Vec<SkillMetadata> {
-    let skill_name_counts =
-        build_skill_name_counts(&loaded_skills.skills, &loaded_skills.disabled_paths).0;
-
-    let selection_context = SkillSelectionContext {
-        loaded_skills,
-        skill_name_counts: &skill_name_counts,
-        connector_slug_counts,
-    };
-    let mut selected: Vec<SkillMetadata> = Vec::new();
-    let mut seen_names: HashSet<String> = HashSet::new();
-    let mut seen_paths: HashSet<AbsolutePathBuf> = HashSet::new();
-    let mut blocked_plain_names: HashSet<String> = HashSet::new();
-
-    for input in inputs {
-        if let UserInput::Skill { name, path, .. } = input {
-            blocked_plain_names.insert(name.clone());
-            let Ok(path) = AbsolutePathBuf::relative_to_current_dir(path) else {
-                continue;
-            };
-
-            let Some(skill) = selection_context.loaded_skills.skills.iter().find(|skill| {
-                skill.path_to_skills_md == path
-                    || selection_context
-                        .loaded_skills
-                        .skill_discovery_path_for_path(&skill.path_to_skills_md)
-                        .is_some_and(|discovery_path| discovery_path == &path)
-            }) else {
-                continue;
-            };
-
-            if !selection_context.loaded_skills.is_skill_enabled(skill)
-                || seen_paths.contains(&skill.path_to_skills_md)
-            {
-                continue;
-            }
-
-            seen_paths.insert(skill.path_to_skills_md.clone());
-            seen_names.insert(skill.name.clone());
-            selected.push(skill.clone());
-        }
-    }
-
-    for input in inputs {
-        if let UserInput::Text { text, .. } = input {
-            let mentioned_names = extract_tool_mentions(text);
-            select_skills_from_mentions(
-                &selection_context,
-                &blocked_plain_names,
-                &mentioned_names,
-                &mut seen_names,
-                &mut seen_paths,
-                &mut selected,
-            );
-        }
-    }
-
-    selected
-}
-
-struct SkillSelectionContext<'a> {
-    loaded_skills: &'a SkillLoadOutcome,
-    skill_name_counts: &'a HashMap<String, usize>,
-    connector_slug_counts: &'a HashMap<String, usize>,
-}
-
-/// Select mentioned skills while preserving the order of `skills`.
-fn select_skills_from_mentions(
-    selection_context: &SkillSelectionContext<'_>,
-    blocked_plain_names: &HashSet<String>,
-    mentions: &ToolMentions<'_>,
-    seen_names: &mut HashSet<String>,
-    seen_paths: &mut HashSet<AbsolutePathBuf>,
-    selected: &mut Vec<SkillMetadata>,
-) {
-    if mentions.is_empty() {
-        return;
-    }
-
-    let mention_skill_paths: HashSet<String> = mentions
-        .paths()
-        .filter(|path| {
-            !matches!(
-                tool_kind_for_path(path),
-                ToolMentionKind::App | ToolMentionKind::Mcp | ToolMentionKind::Plugin
-            )
-        })
-        .map(normalize_host_skill_path)
-        .collect();
-
-    for skill in &selection_context.loaded_skills.skills {
-        if !selection_context.loaded_skills.is_skill_enabled(skill)
-            || seen_paths.contains(&skill.path_to_skills_md)
-        {
-            continue;
-        }
-
-        let canonical_path = normalize_host_skill_path(&skill.path_to_skills_md.to_string_lossy());
-        let matches_discovery_path = selection_context
-            .loaded_skills
-            .skill_discovery_path_for_path(&skill.path_to_skills_md)
-            .is_some_and(|discovery_path| {
-                mention_skill_paths.contains(&normalize_host_skill_path(
-                    &discovery_path.to_string_lossy(),
-                ))
-            });
-        if mention_skill_paths.contains(&canonical_path) || matches_discovery_path {
-            seen_paths.insert(skill.path_to_skills_md.clone());
-            seen_names.insert(skill.name.clone());
-            selected.push(skill.clone());
-        }
-    }
-
-    for skill in &selection_context.loaded_skills.skills {
-        if !selection_context.loaded_skills.is_skill_enabled(skill)
-            || seen_paths.contains(&skill.path_to_skills_md)
-        {
-            continue;
-        }
-
-        if blocked_plain_names.contains(skill.name.as_str()) {
-            continue;
-        }
-        if !mentions.contains_plain_name(skill.name.as_str()) {
-            continue;
-        }
-
-        let skill_count = selection_context
-            .skill_name_counts
-            .get(skill.name.as_str())
-            .copied()
-            .unwrap_or(0);
-        let connector_count = selection_context
-            .connector_slug_counts
-            .get(&skill.name.to_ascii_lowercase())
-            .copied()
-            .unwrap_or(0);
-        if skill_count != 1 || connector_count != 0 {
-            continue;
-        }
-
-        if seen_names.insert(skill.name.clone()) {
-            seen_paths.insert(skill.path_to_skills_md.clone());
-            selected.push(skill.clone());
-        }
-    }
-}
-
 #[cfg(test)]
-#[path = "injection_tests.rs"]
+#[path = "prompt_injection_tests.rs"]
 mod tests;

codex-rs/core-skills/src/model.rs

@@ -131,6 +131,24 @@ impl codex_skills::ImplicitSkillLookup for SkillLoadOutcome {
     }
 }
 
+impl codex_skills::ExplicitSkillLookup for SkillLoadOutcome {
+    fn skills(&self) -> &[SkillMetadata] {
+        &self.skills
+    }
+
+    fn disabled_paths(&self) -> &HashSet<AbsolutePathBuf> {
+        &self.disabled_paths
+    }
+
+    fn skill_discovery_path_for_path(&self, path: &AbsolutePathBuf) -> Option<&AbsolutePathBuf> {
+        SkillLoadOutcome::skill_discovery_path_for_path(self, path)
+    }
+
+    fn is_skill_enabled(&self, skill: &SkillMetadata) -> bool {
+        SkillLoadOutcome::is_skill_enabled(self, skill)
+    }
+}
+
 #[derive(Clone, Default)]
 pub(crate) struct SkillFileSystemsByPath {
     values: Arc<HashMap<AbsolutePathBuf, Arc<dyn ExecutorFileSystem>>>,

codex-rs/core-skills/src/prompt_injection_tests.rs

@@ -0,0 +1,14 @@
+use pretty_assertions::assert_eq;
+
+use super::MAX_SKILL_PROMPT_BYTES;
+use super::bounded_skill_prompt_contents;
+
+#[test]
+fn skill_prompt_contents_are_bounded_at_utf8_boundaries() {
+    let contents = format!("{}é", "a".repeat(MAX_SKILL_PROMPT_BYTES - 1));
+
+    let (bounded, truncated) = bounded_skill_prompt_contents(&contents);
+
+    assert_eq!(bounded.len(), MAX_SKILL_PROMPT_BYTES - 1);
+    assert_eq!(truncated, true);
+}

codex-rs/core/src/skills.rs

@@ -22,12 +22,12 @@ 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::model;
 pub use codex_core_skills::remote;
 pub use codex_skills::SkillMetadata;
 pub use codex_skills::SkillPolicy;
+pub use codex_skills::collect_explicit_skill_mentions;
 pub use codex_skills_extension::HostSkillsLoadInput;
 pub use codex_skills_extension::HostSkillsService;
 pub use codex_skills_extension::bundled_skills_enabled_from_stack;

codex-rs/skills/src/lib.rs

@@ -5,6 +5,7 @@ mod model;
 mod name_counts;
 mod parser;
 mod policy;
+mod selection;
 
 pub use interface::SkillInterfaceAssetPolicy;
 pub use interface::SkillInterfaceFile;
@@ -33,6 +34,8 @@ pub use parser::ParsedSkillFrontmatter;
 pub use parser::SkillParseError;
 pub use parser::parse_skill_frontmatter_metadata;
 pub use policy::resolve_disabled_skill_paths;
+pub use selection::ExplicitSkillLookup;
+pub use selection::collect_explicit_skill_mentions;
 
 use codex_utils_absolute_path::AbsolutePathBuf;
 use include_dir::Dir;