feat: Compress skill paths with root aliases (#19098)

Add skill root tracking so model-visible skill lists can use short path
aliases when absolute paths would exceed the metadata budget.
This commit is contained in:
xl-openai
2026-04-24 15:49:07 -07:00
committed by GitHub
parent 588f7a9fc4
commit 1e560f33e1
8 changed files with 935 additions and 103 deletions

View File

@@ -23,7 +23,12 @@ pub use model::SkillMetadata;
pub use model::SkillPolicy;
pub use model::filter_skill_load_outcome_for_product;
pub use render::AvailableSkills;
pub use render::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS;
pub use render::SKILLS_HOW_TO_USE_WITH_ALIASES;
pub use render::SKILLS_INTRO_WITH_ABSOLUTE_PATHS;
pub use render::SKILLS_INTRO_WITH_ALIASES;
pub use render::SkillMetadataBudget;
pub use render::SkillRenderReport;
pub use render::build_available_skills;
pub use render::default_skill_metadata_budget;
pub use render::render_available_skills_body;

View File

@@ -159,13 +159,22 @@ 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();
for root in roots {
let root_path = canonicalize_for_skill_identity(&root.path);
let fs = root.file_system;
let skills_before_root = outcome.skills.len();
discover_skills_under_root(fs.as_ref(), &root.path, root.scope, &mut outcome).await;
discover_skills_under_root(fs.as_ref(), &root_path, root.scope, &mut outcome).await;
for skill in &outcome.skills[skills_before_root..] {
if !skill_roots.contains(&root_path) {
skill_roots.push(root_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));
@@ -181,7 +190,12 @@ where
.iter()
.map(|skill| skill.path_to_skills_md.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();
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 {

View File

@@ -89,6 +89,8 @@ 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(crate) implicit_skills_by_scripts_dir: Arc<HashMap<AbsolutePathBuf, SkillMetadata>>,
pub(crate) implicit_skills_by_doc_path: Arc<HashMap<AbsolutePathBuf, SkillMetadata>>,
@@ -176,6 +178,19 @@ pub fn filter_skill_load_outcome_for_product(
outcome
.file_systems_by_skill_path
.retain_paths(&retained_paths);
outcome.skill_root_by_path = Arc::new(
outcome
.skill_root_by_path
.iter()
.filter(|(path, _)| retained_paths.contains(*path))
.map(|(path, root)| (path.clone(), root.clone()))
.collect(),
);
let retained_roots: HashSet<AbsolutePathBuf> =
outcome.skill_root_by_path.values().cloned().collect();
outcome
.skill_roots
.retain(|root| retained_roots.contains(root));
outcome.implicit_skills_by_scripts_dir = Arc::new(
outcome
.implicit_skills_by_scripts_dir

View File

@@ -1,3 +1,9 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::path::Component;
use std::path::Path;
use crate::model::SkillLoadOutcome;
use crate::model::SkillMetadata;
use codex_otel::SessionTelemetry;
use codex_otel::THREAD_SKILLS_DESCRIPTION_TRUNCATED_CHARS_METRIC;
@@ -5,6 +11,7 @@ use codex_otel::THREAD_SKILLS_ENABLED_TOTAL_METRIC;
use codex_otel::THREAD_SKILLS_KEPT_TOTAL_METRIC;
use codex_otel::THREAD_SKILLS_TRUNCATED_METRIC;
use codex_protocol::protocol::SkillScope;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_utils_output_truncation::approx_token_count;
const DEFAULT_SKILL_METADATA_CHAR_BUDGET: usize = 8_000;
@@ -14,6 +21,66 @@ const APPROX_BYTES_PER_TOKEN: usize = 4;
pub const SKILL_DESCRIPTION_TRUNCATED_WARNING_PREFIX: &str = "Warning: Exceeded skills context budget. Loaded skill descriptions were truncated by an average of";
pub const SKILL_DESCRIPTIONS_REMOVED_WARNING_PREFIX: &str =
"Warning: Exceeded skills context budget. All skill descriptions were removed and";
pub const SKILLS_INTRO_WITH_ABSOLUTE_PATHS: &str = "A skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list of skills that can be used. Each entry includes a name, description, and file path so you can open the source for full instructions when using a specific skill.";
pub const SKILLS_INTRO_WITH_ALIASES: &str = "A skill is a set of local instructions to follow that is stored in a `SKILL.md` file. Below is the list of skills that can be used. Each entry includes a name, description, and a short path that can be expanded into an absolute path using the skill roots table.";
pub const SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS: &str = r###"- Discovery: The list above is the skills available in this session (name + description + file path). Skill bodies live on disk at the listed paths.
- Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned.
- Missing/blocked: If a named skill isn't in the list or the path can't be read, say so briefly and continue with the best fallback.
- How to use a skill (progressive disclosure):
1) After deciding to use a skill, open its `SKILL.md`. Read only enough to follow the workflow.
2) When `SKILL.md` references relative paths (e.g., `scripts/foo.py`), resolve them relative to the skill directory listed above first, and only consider other paths if needed.
3) If `SKILL.md` points to extra folders such as `references/`, load only the specific files needed for the request; don't bulk-load everything.
4) If `scripts/` exist, prefer running or patching them instead of retyping large code blocks.
5) If `assets/` or templates exist, reuse them instead of recreating from scratch.
- Coordination and sequencing:
- If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them.
- Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why.
- Context hygiene:
- Keep context small: summarize long sections instead of pasting them; only load extra files when needed.
- Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked.
- When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice.
- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue."###;
pub const SKILLS_HOW_TO_USE_WITH_ALIASES: &str = r###"- Discovery: The list above is the skills available in this session (name + description + short path). Skill bodies live on disk at the listed paths after expanding the matching alias from `### Skill roots`.
- Trigger rules: If the user names a skill (with `$SkillName` or plain text) OR the task clearly matches a skill's description shown above, you must use that skill for that turn. Multiple mentions mean use them all. Do not carry skills across turns unless re-mentioned.
- Missing/blocked: If a named skill isn't in the list or the path can't be read, say so briefly and continue with the best fallback.
- How to use a skill (progressive disclosure):
1) After deciding to use a skill, expand the listed short `path` with the matching alias from `### Skill roots`, then open its `SKILL.md`. Read only enough to follow the workflow.
2) When `SKILL.md` references relative paths (e.g., `scripts/foo.py`), resolve them relative to the directory containing that expanded `SKILL.md` first, and only consider other paths if needed.
3) If `SKILL.md` points to extra folders such as `references/`, load only the specific files needed for the request; don't bulk-load everything.
4) If `scripts/` exist, prefer running or patching them instead of retyping large code blocks.
5) If `assets/` or templates exist, reuse them instead of recreating from scratch.
- Coordination and sequencing:
- If multiple skills apply, choose the minimal set that covers the request and state the order you'll use them.
- Announce which skill(s) you're using and why (one short line). If you skip an obvious skill, say why.
- Context hygiene:
- Keep context small: summarize long sections instead of pasting them; only load extra files when needed.
- Avoid deep reference-chasing: prefer opening only files directly linked from `SKILL.md` unless you're blocked.
- When variants exist (frameworks, providers, domains), pick only the relevant reference file(s) and note that choice.
- Safety and fallback: If a skill can't be applied cleanly (missing files, unclear instructions), state the issue, pick the next-best approach, and continue."###;
pub fn render_available_skills_body(skill_root_lines: &[String], skill_lines: &[String]) -> String {
let mut lines: Vec<String> = Vec::new();
lines.push("## Skills".to_string());
if skill_root_lines.is_empty() {
lines.push(SKILLS_INTRO_WITH_ABSOLUTE_PATHS.to_string());
} else {
lines.push(SKILLS_INTRO_WITH_ALIASES.to_string());
lines.push("### Skill roots".to_string());
lines.extend(skill_root_lines.iter().cloned());
}
lines.push("### Available skills".to_string());
lines.extend(skill_lines.iter().cloned());
lines.push("### How to use skills".to_string());
let how_to_use = if skill_root_lines.is_empty() {
SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS
} else {
SKILLS_HOW_TO_USE_WITH_ALIASES
};
lines.push(how_to_use.to_string());
format!("\n{}\n", lines.join("\n"))
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum SkillMetadataBudget {
@@ -66,6 +133,7 @@ pub enum SkillRenderSideEffects<'a> {
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct AvailableSkills {
pub skill_root_lines: Vec<String>,
pub skill_lines: Vec<String>,
pub report: SkillRenderReport,
pub warning_message: Option<String>,
@@ -89,10 +157,11 @@ pub fn default_skill_metadata_budget(context_window: Option<i64>) -> SkillMetada
}
pub fn build_available_skills(
skills: &[SkillMetadata],
outcome: &SkillLoadOutcome,
budget: SkillMetadataBudget,
side_effects: SkillRenderSideEffects<'_>,
) -> Option<AvailableSkills> {
let skills = outcome.allowed_skills_for_implicit_invocation();
if skills.is_empty() {
record_skill_render_side_effects(
side_effects,
@@ -104,7 +173,42 @@ pub fn build_available_skills(
return None;
}
let (skill_lines, report) = render_skill_lines(skills, budget);
let absolute_lines = ordered_absolute_skill_lines(&skills);
let absolute = build_available_skills_from_lines(
absolute_lines,
skills.len(),
budget,
SkillPathAliases::default(),
)?;
let selected =
if absolute.report.omitted_count == 0 && absolute.report.truncated_description_chars == 0 {
absolute
} else if let Some(aliased) = build_aliased_available_skills(outcome, &skills, budget) {
if aliased_render_is_better(&aliased, &absolute, budget) {
aliased
} else {
absolute
}
} else {
absolute
};
record_available_skills_side_effects(&selected, budget, side_effects);
Some(selected)
}
fn build_available_skills_from_lines(
skill_lines: Vec<SkillLine<'_>>,
total_count: usize,
budget: SkillMetadataBudget,
path_aliases: SkillPathAliases,
) -> Option<AvailableSkills> {
if total_count == 0 {
return None;
}
let (skill_lines, report) = render_skill_lines_from_lines(skill_lines, total_count, budget);
let warning_message = if report.omitted_count > 0 {
let skill_word = if report.omitted_count == 1 {
"skill"
@@ -134,29 +238,39 @@ pub fn build_available_skills(
} else {
None
};
record_skill_render_side_effects(
side_effects,
report.total_count,
report.included_count,
report.omitted_count,
report.truncated_description_chars,
);
if report.omitted_count > 0 || report.truncated_description_chars > 0 {
tracing::info!(
budget_limit = budget.limit(),
total_skills = report.total_count,
included_skills = report.included_count,
omitted_skills = report.omitted_count,
truncated_description_chars_per_skill = report.average_truncated_description_chars(),
truncated_skill_descriptions = report.truncated_description_count,
"truncated skill metadata to fit skills context budget"
);
}
Some(AvailableSkills {
let available = AvailableSkills {
skill_root_lines: path_aliases.skill_root_lines,
skill_lines,
report,
warning_message,
})
};
Some(available)
}
fn record_available_skills_side_effects(
available: &AvailableSkills,
budget: SkillMetadataBudget,
side_effects: SkillRenderSideEffects<'_>,
) {
record_skill_render_side_effects(
side_effects,
available.report.total_count,
available.report.included_count,
available.report.omitted_count,
available.report.truncated_description_chars,
);
if available.report.omitted_count > 0 || available.report.truncated_description_chars > 0 {
tracing::info!(
budget_limit = budget.limit(),
total_skills = available.report.total_count,
included_skills = available.report.included_count,
omitted_skills = available.report.omitted_count,
truncated_description_chars_per_skill =
available.report.average_truncated_description_chars(),
truncated_skill_descriptions = available.report.truncated_description_count,
"truncated skill metadata to fit skills context budget"
);
}
}
fn budget_warning_prefix(budget: SkillMetadataBudget, prefix: &str) -> String {
@@ -204,16 +318,11 @@ fn record_skill_render_side_effects(
}
}
fn render_skill_lines(
skills: &[SkillMetadata],
fn render_skill_lines_from_lines(
skill_lines: Vec<SkillLine<'_>>,
total_count: usize,
budget: SkillMetadataBudget,
) -> (Vec<String>, SkillRenderReport) {
let ordered_skills = ordered_skills_for_budget(skills);
let skill_lines = ordered_skills
.into_iter()
.map(SkillLine::new)
.collect::<Vec<_>>();
let full_cost = skill_lines.iter().fold(0usize, |used, line| {
used.saturating_add(line.full_cost(budget))
});
@@ -226,7 +335,7 @@ fn render_skill_lines(
return (
included,
skill_render_report(
/*total_count*/ skills.len(),
total_count,
/*included_count*/ skill_lines.len(),
/*omitted_count*/ 0,
/*truncated_description_chars*/ 0,
@@ -254,7 +363,7 @@ fn render_skill_lines(
return (
included,
skill_render_report(
/*total_count*/ skills.len(),
total_count,
/*included_count*/ skill_lines.len(),
/*omitted_count*/ 0,
truncated_description_chars,
@@ -263,7 +372,7 @@ fn render_skill_lines(
);
}
render_minimum_skill_lines_until_budget(budget, skill_lines, skills.len())
render_minimum_skill_lines_until_budget(budget, skill_lines, total_count)
}
fn render_minimum_skill_lines_until_budget(
@@ -366,10 +475,17 @@ fn sum_description_truncation(rendered: &[RenderedSkillLine]) -> (usize, usize)
impl<'a> SkillLine<'a> {
fn new(skill: &'a SkillMetadata) -> Self {
Self::with_path(
skill,
skill.path_to_skills_md.to_string_lossy().replace('\\', "/"),
)
}
fn with_path(skill: &'a SkillMetadata, path: String) -> Self {
Self {
name: skill.name.as_str(),
description: skill.description.as_str(),
path: skill.path_to_skills_md.to_string_lossy().replace('\\', "/"),
path,
}
}
@@ -455,6 +571,12 @@ fn line_cost(budget: SkillMetadataBudget, line: &str) -> usize {
budget.cost(&format!("{line}\n"))
}
fn lines_cost(budget: SkillMetadataBudget, lines: &[String]) -> usize {
lines.iter().fold(0usize, |used, line| {
used.saturating_add(line_cost(budget, line))
})
}
fn render_lines_with_description_budget(
budget: SkillMetadataBudget,
skill_lines: &[SkillLine<'_>],
@@ -510,6 +632,253 @@ fn render_lines_with_description_budget(
.collect()
}
fn build_aliased_available_skills(
outcome: &SkillLoadOutcome,
skills: &[SkillMetadata],
budget: SkillMetadataBudget,
) -> Option<AvailableSkills> {
let plan = build_alias_plan(outcome, skills, budget)?;
if plan.table_cost >= budget.limit() {
return None;
}
let adjusted_limit = budget.limit().saturating_sub(plan.table_cost);
let adjusted_budget = match budget {
SkillMetadataBudget::Tokens(_) => SkillMetadataBudget::Tokens(adjusted_limit),
SkillMetadataBudget::Characters(_) => SkillMetadataBudget::Characters(adjusted_limit),
};
let ordered_skills = ordered_skills_for_budget(skills);
let skill_lines = ordered_skills
.into_iter()
.map(|skill| SkillLine::with_path(skill, render_skill_path_with_aliases(skill, &plan)))
.collect::<Vec<_>>();
build_available_skills_from_lines(skill_lines, skills.len(), adjusted_budget, plan.aliases)
}
#[derive(Debug, Clone, Default, PartialEq, Eq)]
struct SkillPathAliases {
skill_root_lines: Vec<String>,
}
struct AliasPlan {
aliases: SkillPathAliases,
root_aliases: HashMap<AbsolutePathBuf, String>,
alias_root_by_path: HashMap<AbsolutePathBuf, AbsolutePathBuf>,
table_cost: usize,
}
fn build_alias_plan(
outcome: &SkillLoadOutcome,
skills: &[SkillMetadata],
budget: SkillMetadataBudget,
) -> Option<AliasPlan> {
let skill_paths = skills
.iter()
.map(|skill| skill.path_to_skills_md.clone())
.collect::<HashSet<_>>();
let skill_root_by_path = outcome
.skill_root_by_path
.iter()
.filter(|(path, _)| skill_paths.contains(*path))
.map(|(path, root)| (path.clone(), root.clone()))
.collect::<HashMap<_, _>>();
let used_roots = outcome
.skill_roots
.iter()
.filter(|root| {
skill_root_by_path
.values()
.any(|skill_root| skill_root == *root)
})
.cloned()
.collect::<Vec<_>>();
if used_roots.is_empty() {
return None;
}
let plugin_version_skill_counts =
plugin_version_skill_counts_for_skill_roots(skill_root_by_path.values());
let alias_root_by_skill_root = used_roots
.iter()
.map(|root| {
(
root.clone(),
alias_root_for_skill_root(root, &plugin_version_skill_counts),
)
})
.collect::<HashMap<_, _>>();
let alias_roots = ordered_alias_roots(&used_roots, &alias_root_by_skill_root)?;
let root_aliases = alias_roots
.iter()
.enumerate()
.map(|(index, alias_root)| (alias_root.clone(), format!("r{index}")))
.collect::<HashMap<_, _>>();
let alias_root_by_path = skill_root_by_path
.iter()
.filter_map(|(path, skill_root)| {
alias_root_by_skill_root
.get(skill_root)
.map(|alias_root| (path.clone(), alias_root.clone()))
})
.collect::<HashMap<_, _>>();
let skill_root_lines = build_skill_root_lines(&alias_roots);
let table_cost = aliased_metadata_overhead_cost(budget, &skill_root_lines);
Some(AliasPlan {
aliases: SkillPathAliases { skill_root_lines },
root_aliases,
alias_root_by_path,
table_cost,
})
}
fn ordered_alias_roots(
used_roots: &[AbsolutePathBuf],
alias_root_by_skill_root: &HashMap<AbsolutePathBuf, AbsolutePathBuf>,
) -> Option<Vec<AbsolutePathBuf>> {
let mut seen = HashSet::new();
let mut alias_roots = Vec::new();
for root in used_roots {
let alias_root = alias_root_by_skill_root.get(root)?.clone();
if seen.insert(alias_root.clone()) {
alias_roots.push(alias_root);
}
}
Some(alias_roots)
}
fn alias_root_for_skill_root(
root: &AbsolutePathBuf,
plugin_version_skill_counts: &HashMap<AbsolutePathBuf, usize>,
) -> AbsolutePathBuf {
let Some(plugin_version_base) = plugin_version_base(root.as_path()) else {
return root.clone();
};
let skill_count = plugin_version_skill_counts
.get(&plugin_version_base)
.copied()
.unwrap_or_default();
if skill_count > 1 {
root.clone()
} else {
plugin_marketplace_base(root.as_path()).unwrap_or_else(|| root.clone())
}
}
fn plugin_version_skill_counts_for_skill_roots<'a>(
skill_roots: impl Iterator<Item = &'a AbsolutePathBuf>,
) -> 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()) {
let count = counts.entry(plugin_version_base).or_insert(0usize);
*count = count.saturating_add(1);
}
}
counts
}
fn aliased_metadata_overhead_cost(
budget: SkillMetadataBudget,
skill_root_lines: &[String],
) -> usize {
let empty_skill_lines: &[String] = &[];
let absolute_body = render_available_skills_body(&[], empty_skill_lines);
let aliased_body = render_available_skills_body(skill_root_lines, empty_skill_lines);
budget
.cost(&aliased_body)
.saturating_sub(budget.cost(&absolute_body))
}
fn build_skill_root_lines(roots: &[AbsolutePathBuf]) -> Vec<String> {
roots
.iter()
.enumerate()
.map(|(index, root)| {
let root_str = root.to_string_lossy().replace('\\', "/");
format!("- `r{index}` = `{root_str}`")
})
.collect()
}
fn plugin_marketplace_base(path: &Path) -> Option<AbsolutePathBuf> {
let mut candidate = path;
while let Some(parent) = candidate.parent() {
if parent.file_name()?.to_str()? == "cache"
&& parent.parent()?.file_name()?.to_str()? == "plugins"
{
return AbsolutePathBuf::from_absolute_path(candidate).ok();
}
candidate = parent;
}
None
}
fn plugin_version_base(path: &Path) -> Option<AbsolutePathBuf> {
let marketplace_base = plugin_marketplace_base(path)?;
let mut relative_components = path
.strip_prefix(marketplace_base.as_path())
.ok()?
.components();
let plugin = match relative_components.next()? {
Component::Normal(plugin) => plugin,
_ => return None,
};
let version = match relative_components.next()? {
Component::Normal(version) => version,
_ => return None,
};
AbsolutePathBuf::from_absolute_path(marketplace_base.join(plugin).join(version)).ok()
}
fn render_skill_path_with_aliases(skill: &SkillMetadata, plan: &AliasPlan) -> String {
outcome_relative_skill_path(skill, plan)
.unwrap_or_else(|| skill.path_to_skills_md.to_string_lossy().replace('\\', "/"))
}
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 = plan.root_aliases.get(alias_root)?;
let relative_path = skill
.path_to_skills_md
.as_path()
.strip_prefix(alias_root.as_path())
.ok()?;
let relative_path = relative_path.to_string_lossy().replace('\\', "/");
Some(format!("{alias}/{relative_path}"))
}
fn aliased_render_is_better(
aliased: &AvailableSkills,
absolute: &AvailableSkills,
budget: SkillMetadataBudget,
) -> bool {
if aliased.report.included_count != absolute.report.included_count {
return aliased.report.included_count > absolute.report.included_count;
}
if aliased.report.truncated_description_chars != absolute.report.truncated_description_chars {
return aliased.report.truncated_description_chars
< absolute.report.truncated_description_chars;
}
available_skills_cost(budget, aliased) < available_skills_cost(budget, absolute)
}
fn available_skills_cost(budget: SkillMetadataBudget, available: &AvailableSkills) -> usize {
let metadata_cost = if available.skill_root_lines.is_empty() {
0
} else {
aliased_metadata_overhead_cost(budget, &available.skill_root_lines)
};
metadata_cost.saturating_add(lines_cost(budget, &available.skill_lines))
}
fn ordered_absolute_skill_lines(skills: &[SkillMetadata]) -> Vec<SkillLine<'_>> {
ordered_skills_for_budget(skills)
.into_iter()
.map(SkillLine::new)
.collect()
}
fn ordered_skills_for_budget(skills: &[SkillMetadata]) -> Vec<&SkillMetadata> {
let mut ordered = skills.iter().collect::<Vec<_>>();
ordered.sort_by(|a, b| {
@@ -533,6 +902,9 @@ fn prompt_scope_rank(scope: SkillScope) -> u8 {
#[cfg(test)]
mod tests {
use super::*;
use std::collections::HashMap;
use std::sync::Arc;
use codex_utils_absolute_path::test_support::PathBufExt;
use codex_utils_absolute_path::test_support::test_path_buf;
use pretty_assertions::assert_eq;
@@ -564,6 +936,48 @@ mod tests {
SkillLine::new(skill).render_with_description(description)
}
fn normalized_path(path: &AbsolutePathBuf) -> String {
path.to_string_lossy().replace('\\', "/")
}
fn outcome_with_roots(
skills: Vec<SkillMetadata>,
roots: Vec<AbsolutePathBuf>,
) -> SkillLoadOutcome {
let skill_root_by_path = skills
.iter()
.filter_map(|skill| {
roots
.iter()
.find(|root| {
skill
.path_to_skills_md
.as_path()
.starts_with(root.as_path())
})
.map(|root| (skill.path_to_skills_md.clone(), root.clone()))
})
.collect::<HashMap<_, _>>();
SkillLoadOutcome {
skills,
skill_roots: roots,
skill_root_by_path: Arc::new(skill_root_by_path),
..Default::default()
}
}
fn build_available_skills_from_metadata(
skills: &[SkillMetadata],
budget: SkillMetadataBudget,
) -> Option<AvailableSkills> {
build_available_skills_from_lines(
ordered_absolute_skill_lines(skills),
skills.len(),
budget,
SkillPathAliases::default(),
)
}
#[test]
fn default_budget_uses_two_percent_of_full_context_window() {
assert_eq!(
@@ -597,12 +1011,8 @@ mod tests {
+ SkillLine::new(&beta).minimum_cost(SkillMetadataBudget::Characters(usize::MAX));
let budget = SkillMetadataBudget::Characters(minimum_cost + 6);
let rendered = build_available_skills(
&[beta.clone(), alpha.clone()],
budget,
SkillRenderSideEffects::None,
)
.expect("skills should render");
let rendered = build_available_skills_from_metadata(&[beta.clone(), alpha.clone()], budget)
.expect("skills should render");
assert_eq!(rendered.report.included_count, 2);
assert_eq!(rendered.report.omitted_count, 0);
@@ -626,7 +1036,7 @@ mod tests {
+ SkillLine::new(&beta).minimum_cost(SkillMetadataBudget::Characters(usize::MAX));
let budget = SkillMetadataBudget::Characters(minimum_cost + 6);
let rendered = build_available_skills(&[alpha, beta], budget, SkillRenderSideEffects::None)
let rendered = build_available_skills_from_metadata(&[alpha, beta], budget)
.expect("skills should render");
assert_eq!(rendered.report.included_count, 2);
@@ -646,7 +1056,7 @@ mod tests {
+ SkillLine::new(&beta).minimum_cost(SkillMetadataBudget::Characters(usize::MAX));
let budget = SkillMetadataBudget::Characters(minimum_cost + 6);
let rendered = build_available_skills(&[alpha, beta], budget, SkillRenderSideEffects::None)
let rendered = build_available_skills_from_metadata(&[alpha, beta], budget)
.expect("skills should render");
assert_eq!(rendered.report.included_count, 2);
@@ -671,12 +1081,8 @@ mod tests {
+ SkillLine::new(&long).minimum_cost(SkillMetadataBudget::Characters(usize::MAX));
let budget = SkillMetadataBudget::Characters(minimum_cost + 11);
let rendered = build_available_skills(
&[short.clone(), long.clone()],
budget,
SkillRenderSideEffects::None,
)
.expect("skills should render");
let rendered = build_available_skills_from_metadata(&[short.clone(), long.clone()], budget)
.expect("skills should render");
assert_eq!(rendered.report.included_count, 2);
assert_eq!(rendered.report.omitted_count, 0);
@@ -702,12 +1108,8 @@ mod tests {
.cost(&format!("{}\n", SkillLine::new(&admin).render_minimum()));
let budget = SkillMetadataBudget::Characters(system_cost + admin_cost);
let rendered = build_available_skills(
&[system, user, repo, admin],
budget,
SkillRenderSideEffects::None,
)
.expect("skills should render");
let rendered = build_available_skills_from_metadata(&[system, user, repo, admin], budget)
.expect("skills should render");
assert_eq!(rendered.report.included_count, 2);
assert_eq!(rendered.report.omitted_count, 2);
@@ -735,9 +1137,8 @@ mod tests {
.cost(&format!("{}\n", SkillLine::new(&repo).render_full()));
let budget = SkillMetadataBudget::Characters(repo_cost);
let rendered =
build_available_skills(&[oversized, repo], budget, SkillRenderSideEffects::None)
.expect("skills render");
let rendered = build_available_skills_from_metadata(&[oversized, repo], budget)
.expect("skills render");
assert_eq!(rendered.report.included_count, 1);
assert_eq!(rendered.report.omitted_count, 1);
@@ -752,4 +1153,335 @@ mod tests {
assert!(!rendered_text.contains("- oversized-system-skill:"));
assert!(rendered_text.contains("- repo-skill:"));
}
#[test]
fn outcome_rendering_omits_aliases_when_absolute_plan_has_no_budget_pressure() {
let root = test_path_buf("/tmp/skills").abs();
let alpha_path = root.join("alpha/SKILL.md");
let beta_path = root.join("beta/SKILL.md");
let outcome = outcome_with_roots(
vec![
skill_with_path("alpha-skill", &alpha_path),
skill_with_path("beta-skill", &beta_path),
],
vec![root],
);
let rendered = build_available_skills(
&outcome,
SkillMetadataBudget::Characters(usize::MAX),
SkillRenderSideEffects::None,
)
.expect("skills should render");
assert!(rendered.skill_root_lines.is_empty());
assert_eq!(rendered.report.included_count, 2);
}
#[test]
fn outcome_rendering_uses_aliases_when_they_allow_more_skills_to_fit() {
let root = test_path_buf(
"/Users/xl/.codex/plugins/cache/openai-curated/example/hash1234567890/skills-with-a-very-long-shared-prefix",
)
.abs();
let skills = (0..12)
.map(|index| {
let name = format!("shared-root-skill-{index}");
skill_with_path(&name, &root.join(format!("skill-{index}/SKILL.md")))
})
.collect::<Vec<_>>();
let outcome = outcome_with_roots(skills.clone(), vec![root]);
let absolute_minimum = skills.iter().fold(0usize, |cost, skill| {
cost.saturating_add(
SkillLine::new(skill).minimum_cost(SkillMetadataBudget::Characters(usize::MAX)),
)
});
let plan = build_alias_plan(
&outcome,
&skills,
SkillMetadataBudget::Characters(usize::MAX),
)
.expect("alias plan should build");
let alias_minimum = skills.iter().fold(plan.table_cost, |cost, skill| {
cost.saturating_add(
SkillLine::with_path(skill, render_skill_path_with_aliases(skill, &plan))
.minimum_cost(SkillMetadataBudget::Characters(usize::MAX)),
)
});
assert!(
alias_minimum < absolute_minimum,
"test fixture should make aliases cheaper"
);
let rendered = build_available_skills(
&outcome,
SkillMetadataBudget::Characters(alias_minimum),
SkillRenderSideEffects::None,
)
.expect("skills should render");
assert_eq!(rendered.report.included_count, skills.len());
assert_eq!(rendered.report.omitted_count, 0);
assert_eq!(
rendered.skill_root_lines,
vec![format!(
"- `r0` = `{}`",
normalized_path(
&test_path_buf(
"/Users/xl/.codex/plugins/cache/openai-curated/example/hash1234567890/skills-with-a-very-long-shared-prefix"
)
.abs()
)
)]
);
let rendered_text = rendered.skill_lines.join("\n");
assert!(rendered_text.contains("r0/skill-0/SKILL.md"));
assert!(rendered_text.contains("r0/skill-11/SKILL.md"));
}
#[test]
fn outcome_rendering_uses_marketplace_root_for_single_skill_plugin_versions() {
let github_root =
test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated/github/hash123/skills")
.abs();
let marketplace_root = test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated").abs();
let github = skill_with_path("github:gh-fix-ci", &github_root.join("gh-fix-ci/SKILL.md"));
let outcome = outcome_with_roots(vec![github.clone()], vec![github_root.clone()]);
let plan = build_alias_plan(
&outcome,
&[github],
SkillMetadataBudget::Characters(usize::MAX),
)
.expect("alias plan should build");
assert_eq!(
plan.aliases.skill_root_lines,
vec![format!("- `r0` = `{}`", normalized_path(&marketplace_root))]
);
assert_eq!(
render_skill_path_with_aliases(
&skill_with_path("github:gh-fix-ci", &github_root.join("gh-fix-ci/SKILL.md")),
&plan
),
"r0/github/hash123/skills/gh-fix-ci/SKILL.md"
);
}
#[test]
fn outcome_rendering_uses_skill_root_for_multiple_skills_in_one_plugin_version() {
let github_root =
test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated/github/hash123/skills")
.abs();
let fix_ci = skill_with_path("github:gh-fix-ci", &github_root.join("gh-fix-ci/SKILL.md"));
let yeet = skill_with_path("github:yeet", &github_root.join("yeet/SKILL.md"));
let outcome = outcome_with_roots(
vec![fix_ci.clone(), yeet.clone()],
vec![github_root.clone()],
);
let plan = build_alias_plan(
&outcome,
&[fix_ci, yeet],
SkillMetadataBudget::Characters(usize::MAX),
)
.expect("alias plan should build");
assert_eq!(
plan.aliases.skill_root_lines,
vec![format!("- `r0` = `{}`", normalized_path(&github_root))]
);
assert_eq!(
render_skill_path_with_aliases(
&skill_with_path("github:gh-fix-ci", &github_root.join("gh-fix-ci/SKILL.md")),
&plan
),
"r0/gh-fix-ci/SKILL.md"
);
assert_eq!(
render_skill_path_with_aliases(
&skill_with_path("github:yeet", &github_root.join("yeet/SKILL.md")),
&plan
),
"r0/yeet/SKILL.md"
);
}
#[test]
fn outcome_rendering_counts_plugin_version_skills_before_budget_omission() {
let root = test_path_buf(
"/Users/xl/.codex/plugins/cache/openai-curated/example/hash1234567890/skills-with-a-very-long-shared-prefix",
)
.abs();
let alpha = skill_with_path("alpha-skill", &root.join("alpha/SKILL.md"));
let beta = skill_with_path("beta-skill", &root.join("beta/SKILL.md"));
let outcome = outcome_with_roots(vec![alpha.clone(), beta.clone()], vec![root.clone()]);
let plan = build_alias_plan(
&outcome,
&[alpha.clone(), beta.clone()],
SkillMetadataBudget::Characters(usize::MAX),
)
.expect("alias plan should build");
let alpha_cost = SkillMetadataBudget::Characters(usize::MAX).cost(&format!(
"{}\n",
SkillLine::with_path(&alpha, render_skill_path_with_aliases(&alpha, &plan))
.render_minimum()
));
let rendered = build_aliased_available_skills(
&outcome,
&[alpha, beta],
SkillMetadataBudget::Characters(plan.table_cost + alpha_cost),
)
.expect("skills should render");
assert_eq!(rendered.report.included_count, 1);
assert_eq!(
rendered.skill_root_lines,
vec![format!("- `r0` = `{}`", normalized_path(&root))]
);
assert_eq!(
rendered.skill_lines,
vec!["- alpha-skill: (file: r0/alpha/SKILL.md)"]
);
}
#[test]
fn outcome_rendering_uses_each_skill_root_for_multiple_roots_in_one_plugin_version() {
let skills_root =
test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated/github/hash123/skills")
.abs();
let extra_root = test_path_buf(
"/Users/xl/.codex/plugins/cache/openai-curated/github/hash123/extra-skills",
)
.abs();
let fix_ci = skill_with_path("github:gh-fix-ci", &skills_root.join("gh-fix-ci/SKILL.md"));
let yeet = skill_with_path("github:yeet", &extra_root.join("yeet/SKILL.md"));
let outcome = outcome_with_roots(
vec![fix_ci.clone(), yeet.clone()],
vec![skills_root.clone(), extra_root.clone()],
);
let plan = build_alias_plan(
&outcome,
&[fix_ci, yeet],
SkillMetadataBudget::Characters(usize::MAX),
)
.expect("alias plan should build");
assert_eq!(
plan.aliases.skill_root_lines,
vec![
format!("- `r0` = `{}`", normalized_path(&skills_root)),
format!("- `r1` = `{}`", normalized_path(&extra_root)),
]
);
assert_eq!(
render_skill_path_with_aliases(
&skill_with_path("github:gh-fix-ci", &skills_root.join("gh-fix-ci/SKILL.md")),
&plan
),
"r0/gh-fix-ci/SKILL.md"
);
assert_eq!(
render_skill_path_with_aliases(
&skill_with_path("github:yeet", &extra_root.join("yeet/SKILL.md")),
&plan
),
"r1/yeet/SKILL.md"
);
}
#[test]
fn outcome_rendering_extracts_plugin_marketplace_root_for_multiple_plugins() {
let github_root =
test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated/github/hash123/skills")
.abs();
let slack_root =
test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated/slack/hash456/skills")
.abs();
let marketplace_root = test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated").abs();
let github = skill_with_path("github:gh-fix-ci", &github_root.join("gh-fix-ci/SKILL.md"));
let slack = skill_with_path(
"slack:daily-digest",
&slack_root.join("daily-digest/SKILL.md"),
);
let outcome = outcome_with_roots(
vec![github.clone(), slack.clone()],
vec![github_root.clone(), slack_root.clone()],
);
let plan = build_alias_plan(
&outcome,
&[github, slack],
SkillMetadataBudget::Characters(usize::MAX),
)
.expect("alias plan should build");
assert_eq!(
plan.aliases.skill_root_lines,
vec![format!("- `r0` = `{}`", normalized_path(&marketplace_root))]
);
assert_eq!(
render_skill_path_with_aliases(
&skill_with_path("github:gh-fix-ci", &github_root.join("gh-fix-ci/SKILL.md")),
&plan
),
"r0/github/hash123/skills/gh-fix-ci/SKILL.md"
);
assert_eq!(
render_skill_path_with_aliases(
&skill_with_path(
"slack:daily-digest",
&slack_root.join("daily-digest/SKILL.md")
),
&plan
),
"r0/slack/hash456/skills/daily-digest/SKILL.md"
);
}
#[test]
fn outcome_rendering_uses_one_marketplace_root_for_multiple_plugin_versions() {
let skills_root =
test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated/github/hash123/skills")
.abs();
let extra_root = test_path_buf(
"/Users/xl/.codex/plugins/cache/openai-curated/github/hash456/extra-skills",
)
.abs();
let marketplace_root = test_path_buf("/Users/xl/.codex/plugins/cache/openai-curated").abs();
let fix_ci = skill_with_path("github:gh-fix-ci", &skills_root.join("gh-fix-ci/SKILL.md"));
let yeet = skill_with_path("github:yeet", &extra_root.join("yeet/SKILL.md"));
let outcome = outcome_with_roots(
vec![fix_ci.clone(), yeet.clone()],
vec![skills_root.clone(), extra_root.clone()],
);
let plan = build_alias_plan(
&outcome,
&[fix_ci, yeet],
SkillMetadataBudget::Characters(usize::MAX),
)
.expect("alias plan should build");
assert_eq!(
plan.aliases.skill_root_lines,
vec![format!("- `r0` = `{}`", normalized_path(&marketplace_root))]
);
assert_eq!(
render_skill_path_with_aliases(
&skill_with_path("github:gh-fix-ci", &skills_root.join("gh-fix-ci/SKILL.md")),
&plan
),
"r0/github/hash123/skills/gh-fix-ci/SKILL.md"
);
assert_eq!(
render_skill_path_with_aliases(
&skill_with_path("github:yeet", &extra_root.join("yeet/SKILL.md")),
&plan
),
"r0/github/hash456/extra-skills/yeet/SKILL.md"
);
}
fn skill_with_path(name: &str, path: &AbsolutePathBuf) -> SkillMetadata {
let mut skill = make_skill(name, SkillScope::User);
skill.path_to_skills_md = path.clone();
skill
}
}