mirror of
https://github.com/openai/codex.git
synced 2026-10-21 04:26:54 +00:00
Compact host skill paths under metadata pressure (#35172)
## Why Repeated absolute path prefixes consume the bounded skill catalog and can leave less room for descriptions or later skills. ## What changed - Track the discovery root for host skills and render shared roots as `r0`, `r1`, and similar aliases when doing so improves the bounded catalog. - Include a skill-roots table and alias-aware usage instructions whenever compact paths are selected. - Keep absolute, authority-aware locators when aliases provide no benefit or the catalog contains non-host skills. ## Testing Added unit and extension coverage for unpressured catalogs, constrained shared paths, mixed-source catalogs, and plugin-cache root selection. GitOrigin-RevId: 4196df2c52be27338796065cb2e6434efcfac6d5
This commit is contained in:
@@ -56,6 +56,11 @@ impl SkillLoadOutcome {
|
||||
.map(|skill| (skill, self.is_skill_enabled(skill)))
|
||||
}
|
||||
|
||||
/// Returns the discovery root that supplied a loaded skill path.
|
||||
pub fn skill_root_for_path(&self, path: &AbsolutePathBuf) -> Option<&AbsolutePathBuf> {
|
||||
self.skill_root_by_path.get(path)
|
||||
}
|
||||
|
||||
pub(crate) fn file_system_for_skill(
|
||||
&self,
|
||||
skill: &SkillMetadata,
|
||||
|
||||
@@ -140,6 +140,7 @@ pub struct SkillCatalogEntry {
|
||||
pub short_description: Option<String>,
|
||||
pub main_prompt: SkillResourceId,
|
||||
pub display_path: Option<String>,
|
||||
display_path_root: Option<String>,
|
||||
prompt_scope: Option<SkillScope>,
|
||||
pub dependencies: Option<SkillDependencies>,
|
||||
pub enabled: bool,
|
||||
@@ -162,6 +163,7 @@ impl SkillCatalogEntry {
|
||||
short_description: None,
|
||||
main_prompt,
|
||||
display_path: None,
|
||||
display_path_root: None,
|
||||
prompt_scope: None,
|
||||
dependencies: None,
|
||||
enabled: true,
|
||||
@@ -179,6 +181,12 @@ impl SkillCatalogEntry {
|
||||
self
|
||||
}
|
||||
|
||||
/// Sets the shared filesystem prefix that may be compacted in model-visible paths.
|
||||
pub fn with_display_path_root(mut self, display_path_root: impl Into<String>) -> Self {
|
||||
self.display_path_root = Some(display_path_root.into());
|
||||
self
|
||||
}
|
||||
|
||||
pub(crate) fn with_prompt_scope(mut self, prompt_scope: SkillScope) -> Self {
|
||||
self.prompt_scope = Some(prompt_scope);
|
||||
self
|
||||
@@ -205,6 +213,10 @@ impl SkillCatalogEntry {
|
||||
.unwrap_or_else(|| self.main_prompt.as_str())
|
||||
}
|
||||
|
||||
pub(crate) fn display_path_root(&self) -> Option<&str> {
|
||||
self.display_path_root.as_deref()
|
||||
}
|
||||
|
||||
pub(crate) fn prompt_scope(&self) -> Option<SkillScope> {
|
||||
self.prompt_scope
|
||||
}
|
||||
|
||||
@@ -14,15 +14,21 @@ pub(crate) struct AvailableSkillsInstructions {
|
||||
|
||||
impl AvailableSkillsInstructions {
|
||||
pub(crate) fn from_skill_lines(
|
||||
skill_root_lines: Vec<String>,
|
||||
mut skill_lines: Vec<String>,
|
||||
include_skills_usage_instructions: bool,
|
||||
) -> Self {
|
||||
if include_skills_usage_instructions {
|
||||
skill_lines.push("### How to use skills".to_string());
|
||||
skill_lines.push(SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS.to_string());
|
||||
let instructions = if skill_root_lines.is_empty() {
|
||||
SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS
|
||||
} else {
|
||||
SKILLS_HOW_TO_USE_WITH_ALIASES
|
||||
};
|
||||
skill_lines.push(instructions.to_string());
|
||||
}
|
||||
Self {
|
||||
skill_root_lines: Vec::new(),
|
||||
skill_root_lines,
|
||||
skill_lines,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -98,7 +98,11 @@ fn catalog_from_outcome(outcome: &SkillLoadOutcome) -> SkillCatalog {
|
||||
};
|
||||
|
||||
for (skill, enabled) in outcome.skills_with_enabled() {
|
||||
catalog.push_entry(catalog_entry_from_skill(skill, enabled));
|
||||
let mut entry = catalog_entry_from_skill(skill, enabled);
|
||||
if let Some(root) = outcome.skill_root_for_path(&skill.path_to_skills_md) {
|
||||
entry = entry.with_display_path_root(root.to_string_lossy().replace('\\', "/"));
|
||||
}
|
||||
catalog.push_entry(entry);
|
||||
}
|
||||
|
||||
catalog
|
||||
|
||||
@@ -13,7 +13,8 @@ use tokio::sync::Semaphore;
|
||||
use super::catalog_from_outcome;
|
||||
|
||||
#[tokio::test]
|
||||
async fn host_catalog_entries_carry_their_prompt_scope() -> Result<(), Box<dyn std::error::Error>> {
|
||||
async fn host_catalog_entries_carry_their_render_metadata() -> Result<(), Box<dyn std::error::Error>>
|
||||
{
|
||||
let unique = SystemTime::now().duration_since(UNIX_EPOCH)?.as_nanos();
|
||||
let root = std::env::temp_dir().join(format!(
|
||||
"codex-skills-extension-host-provider-{}-{unique}",
|
||||
@@ -48,7 +49,16 @@ async fn host_catalog_entries_carry_their_prompt_scope() -> Result<(), Box<dyn s
|
||||
let catalog = catalog_from_outcome(&outcome);
|
||||
|
||||
assert_eq!(catalog.entries.len(), 1);
|
||||
assert_eq!(catalog.entries[0].prompt_scope(), Some(SkillScope::User));
|
||||
assert_eq!(
|
||||
(
|
||||
catalog.entries[0].display_path_root(),
|
||||
catalog.entries[0].prompt_scope(),
|
||||
),
|
||||
(
|
||||
Some(root.to_string_lossy().replace('\\', "/").as_str()),
|
||||
Some(SkillScope::User),
|
||||
)
|
||||
);
|
||||
|
||||
std::fs::remove_dir_all(root.as_path())?;
|
||||
Ok(())
|
||||
|
||||
@@ -1,5 +1,11 @@
|
||||
use std::borrow::Cow;
|
||||
use std::collections::HashMap;
|
||||
use std::collections::HashSet;
|
||||
use std::path::Component;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use codex_core_skills::render_available_skills_body;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_utils_string::approx_token_count;
|
||||
use codex_utils_string::take_bytes_at_char_boundary;
|
||||
@@ -167,6 +173,13 @@ impl SkillMetadataBudget {
|
||||
Self::Characters(_) => chars,
|
||||
}
|
||||
}
|
||||
|
||||
fn cost(self, text: &str) -> usize {
|
||||
match self {
|
||||
Self::Tokens(_) => approx_token_count(text),
|
||||
Self::Characters(_) => text.chars().count(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
struct SkillLine<'a> {
|
||||
@@ -178,11 +191,19 @@ struct SkillLine<'a> {
|
||||
|
||||
impl<'a> SkillLine<'a> {
|
||||
fn new(entry: &'a SkillCatalogEntry, policy: SkillCatalogRenderPolicy) -> Self {
|
||||
Self::with_locator(entry, policy, entry.rendered_path().to_string())
|
||||
}
|
||||
|
||||
fn with_locator(
|
||||
entry: &'a SkillCatalogEntry,
|
||||
policy: SkillCatalogRenderPolicy,
|
||||
locator: String,
|
||||
) -> Self {
|
||||
let description = policy.description(entry);
|
||||
Self {
|
||||
name: entry.name.as_str(),
|
||||
description: truncate_catalog_skill_description(description),
|
||||
locator: entry.rendered_path().to_string(),
|
||||
locator,
|
||||
locator_kind: match &entry.authority.kind {
|
||||
SkillSourceKind::Host => "file",
|
||||
SkillSourceKind::Executor => "environment resource",
|
||||
@@ -422,7 +443,14 @@ fn sum_description_truncation(rendered: &[RenderedSkillLine]) -> (usize, usize)
|
||||
})
|
||||
}
|
||||
|
||||
struct RenderedCatalog {
|
||||
skill_root_lines: Vec<String>,
|
||||
skill_lines: Vec<String>,
|
||||
report: SkillRenderReport,
|
||||
}
|
||||
|
||||
pub(crate) struct AvailableSkillsRender {
|
||||
skill_root_lines: Vec<String>,
|
||||
skill_lines: Vec<String>,
|
||||
pub(crate) report: SkillRenderReport,
|
||||
}
|
||||
@@ -434,6 +462,7 @@ impl AvailableSkillsRender {
|
||||
) -> Option<AvailableSkillsInstructions> {
|
||||
(!self.skill_lines.is_empty()).then(|| {
|
||||
AvailableSkillsInstructions::from_skill_lines(
|
||||
self.skill_root_lines,
|
||||
self.skill_lines,
|
||||
include_skills_usage_instructions,
|
||||
)
|
||||
@@ -457,14 +486,43 @@ pub(crate) fn render_available_skills(
|
||||
.filter(|entry| entry.enabled && entry.prompt_visible)
|
||||
.collect::<Vec<_>>();
|
||||
policy.order_entries(&mut entries);
|
||||
let skill_lines = entries
|
||||
.iter()
|
||||
.map(|entry| SkillLine::new(entry, policy))
|
||||
.collect::<Vec<_>>();
|
||||
if skill_lines.is_empty() {
|
||||
if entries.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let absolute = render_catalog(
|
||||
entries
|
||||
.iter()
|
||||
.map(|entry| SkillLine::new(entry, policy))
|
||||
.collect(),
|
||||
budget,
|
||||
Vec::new(),
|
||||
policy,
|
||||
);
|
||||
let selected =
|
||||
if absolute.report.omitted_count == 0 && absolute.report.truncated_description_chars == 0 {
|
||||
absolute
|
||||
} else if let Some(aliased) = build_aliased_catalog(&entries, policy, budget)
|
||||
&& aliased_render_is_better(&aliased, &absolute, budget)
|
||||
{
|
||||
aliased
|
||||
} else {
|
||||
absolute
|
||||
};
|
||||
|
||||
Some(AvailableSkillsRender {
|
||||
skill_root_lines: selected.skill_root_lines,
|
||||
skill_lines: selected.skill_lines,
|
||||
report: selected.report,
|
||||
})
|
||||
}
|
||||
|
||||
fn render_catalog(
|
||||
skill_lines: Vec<SkillLine<'_>>,
|
||||
budget: SkillMetadataBudget,
|
||||
skill_root_lines: Vec<String>,
|
||||
policy: SkillCatalogRenderPolicy,
|
||||
) -> RenderedCatalog {
|
||||
let total_count = skill_lines.len();
|
||||
let RenderedSkillLines {
|
||||
lines: mut rendered_lines,
|
||||
@@ -494,7 +552,8 @@ pub(crate) fn render_available_skills(
|
||||
}
|
||||
}
|
||||
|
||||
Some(AvailableSkillsRender {
|
||||
RenderedCatalog {
|
||||
skill_root_lines,
|
||||
skill_lines: rendered_lines
|
||||
.into_iter()
|
||||
.map(|rendered| rendered.line)
|
||||
@@ -506,7 +565,7 @@ pub(crate) fn render_available_skills(
|
||||
truncated_description_chars,
|
||||
truncated_description_count,
|
||||
},
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -520,6 +579,220 @@ fn available_skills_fragment(
|
||||
.into_fragment(include_skills_usage_instructions)
|
||||
}
|
||||
|
||||
struct AliasPlan {
|
||||
skill_root_lines: Vec<String>,
|
||||
alias_root_by_display_root: HashMap<String, String>,
|
||||
root_aliases: HashMap<String, String>,
|
||||
table_cost: usize,
|
||||
}
|
||||
|
||||
fn build_aliased_catalog(
|
||||
entries: &[&SkillCatalogEntry],
|
||||
policy: SkillCatalogRenderPolicy,
|
||||
budget: SkillMetadataBudget,
|
||||
) -> Option<RenderedCatalog> {
|
||||
let plan = build_alias_plan(entries, 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 skill_lines = entries
|
||||
.iter()
|
||||
.map(|entry| {
|
||||
SkillLine::with_locator(entry, policy, render_skill_path_with_aliases(entry, &plan))
|
||||
})
|
||||
.collect();
|
||||
Some(render_catalog(
|
||||
skill_lines,
|
||||
adjusted_budget,
|
||||
plan.skill_root_lines,
|
||||
policy,
|
||||
))
|
||||
}
|
||||
|
||||
fn build_alias_plan(
|
||||
entries: &[&SkillCatalogEntry],
|
||||
budget: SkillMetadataBudget,
|
||||
) -> Option<AliasPlan> {
|
||||
// The shared alias prompt only describes host filesystem skills.
|
||||
if entries
|
||||
.iter()
|
||||
.any(|entry| entry.authority.kind != SkillSourceKind::Host)
|
||||
{
|
||||
return None;
|
||||
}
|
||||
|
||||
let plugin_version_skill_counts = plugin_version_skill_counts_for_entries(entries);
|
||||
let mut alias_root_by_display_root = HashMap::new();
|
||||
let mut alias_roots = Vec::new();
|
||||
let mut seen = HashSet::new();
|
||||
for entry in entries {
|
||||
if entry.authority.kind != SkillSourceKind::Host {
|
||||
continue;
|
||||
}
|
||||
let Some(display_root) = entry.display_path_root() else {
|
||||
continue;
|
||||
};
|
||||
let alias_root =
|
||||
alias_root_for_display_root(Path::new(display_root), &plugin_version_skill_counts)
|
||||
.to_string_lossy()
|
||||
.replace('\\', "/");
|
||||
alias_root_by_display_root.insert(display_root.to_string(), alias_root.clone());
|
||||
if seen.insert(alias_root.clone()) {
|
||||
alias_roots.push(alias_root);
|
||||
}
|
||||
}
|
||||
if alias_roots.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let root_aliases = alias_roots
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(index, root)| (root.clone(), format!("r{index}")))
|
||||
.collect();
|
||||
let skill_root_lines = alias_roots
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(index, root)| format!("- `r{index}` = `{root}`"))
|
||||
.collect::<Vec<_>>();
|
||||
let table_cost = aliased_metadata_overhead_cost(budget, &skill_root_lines);
|
||||
Some(AliasPlan {
|
||||
skill_root_lines,
|
||||
alias_root_by_display_root,
|
||||
root_aliases,
|
||||
table_cost,
|
||||
})
|
||||
}
|
||||
|
||||
fn plugin_version_skill_counts_for_entries(
|
||||
entries: &[&SkillCatalogEntry],
|
||||
) -> HashMap<PathBuf, usize> {
|
||||
let mut counts = HashMap::new();
|
||||
for root in entries.iter().filter_map(|entry| {
|
||||
(entry.authority.kind == SkillSourceKind::Host)
|
||||
.then(|| entry.display_path_root())
|
||||
.flatten()
|
||||
}) {
|
||||
if let Some(plugin_version_base) = plugin_version_base(Path::new(root)) {
|
||||
let count = counts.entry(plugin_version_base).or_insert(0usize);
|
||||
*count = count.saturating_add(1);
|
||||
}
|
||||
}
|
||||
counts
|
||||
}
|
||||
|
||||
fn alias_root_for_display_root(
|
||||
root: &Path,
|
||||
plugin_version_skill_counts: &HashMap<PathBuf, usize>,
|
||||
) -> PathBuf {
|
||||
let Some(plugin_version_base) = plugin_version_base(root) else {
|
||||
return root.to_path_buf();
|
||||
};
|
||||
let skill_count = plugin_version_skill_counts
|
||||
.get(&plugin_version_base)
|
||||
.copied()
|
||||
.unwrap_or_default();
|
||||
if skill_count > 1 {
|
||||
root.to_path_buf()
|
||||
} else {
|
||||
plugin_marketplace_base(root).unwrap_or_else(|| root.to_path_buf())
|
||||
}
|
||||
}
|
||||
|
||||
fn plugin_marketplace_base(path: &Path) -> Option<PathBuf> {
|
||||
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 Some(candidate.to_path_buf());
|
||||
}
|
||||
candidate = parent;
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
fn plugin_version_base(path: &Path) -> Option<PathBuf> {
|
||||
let marketplace_base = plugin_marketplace_base(path)?;
|
||||
let mut relative_components = path.strip_prefix(&marketplace_base).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,
|
||||
};
|
||||
Some(marketplace_base.join(plugin).join(version))
|
||||
}
|
||||
|
||||
fn render_skill_path_with_aliases(entry: &SkillCatalogEntry, plan: &AliasPlan) -> String {
|
||||
if entry.authority.kind != SkillSourceKind::Host {
|
||||
return entry.rendered_path().to_string();
|
||||
}
|
||||
let Some(display_root) = entry.display_path_root() else {
|
||||
return entry.rendered_path().to_string();
|
||||
};
|
||||
let Some(alias_root) = plan.alias_root_by_display_root.get(display_root) else {
|
||||
return entry.rendered_path().to_string();
|
||||
};
|
||||
let Some(alias) = plan.root_aliases.get(alias_root) else {
|
||||
return entry.rendered_path().to_string();
|
||||
};
|
||||
let Ok(relative_path) = Path::new(entry.rendered_path()).strip_prefix(alias_root) else {
|
||||
return entry.rendered_path().to_string();
|
||||
};
|
||||
let relative_path = relative_path.to_string_lossy().replace('\\', "/");
|
||||
format!("{alias}/{relative_path}")
|
||||
}
|
||||
|
||||
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 aliased_render_is_better(
|
||||
aliased: &RenderedCatalog,
|
||||
absolute: &RenderedCatalog,
|
||||
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;
|
||||
}
|
||||
rendered_catalog_cost(budget, aliased) < rendered_catalog_cost(budget, absolute)
|
||||
}
|
||||
|
||||
fn rendered_catalog_cost(budget: SkillMetadataBudget, rendered: &RenderedCatalog) -> usize {
|
||||
let metadata_cost = if rendered.skill_root_lines.is_empty() {
|
||||
0
|
||||
} else {
|
||||
aliased_metadata_overhead_cost(budget, &rendered.skill_root_lines)
|
||||
};
|
||||
rendered
|
||||
.skill_lines
|
||||
.iter()
|
||||
.fold(metadata_cost, |used, line| {
|
||||
used.saturating_add(metadata_line_cost(budget, line))
|
||||
})
|
||||
}
|
||||
|
||||
fn omission_marker(omitted: usize) -> String {
|
||||
let skill_word = if omitted == 1 { "skill" } else { "skills" };
|
||||
format!("- {omitted} additional {skill_word} omitted from this bounded skills list.")
|
||||
|
||||
@@ -180,6 +180,183 @@ fn catalog_budget_uses_capped_context_percentage_or_character_fallback() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn path_aliases_are_not_used_without_budget_pressure() {
|
||||
let root = "/Users/test/.codex/plugins/cache/openai-curated/example/hash/skills";
|
||||
let catalog = SkillCatalog {
|
||||
entries: vec![
|
||||
entry("alpha", "Alpha skill.", /*short_description*/ None)
|
||||
.with_display_path(format!("{root}/alpha/SKILL.md"))
|
||||
.with_display_path_root(root),
|
||||
entry("beta", "Beta skill.", /*short_description*/ None)
|
||||
.with_display_path(format!("{root}/beta/SKILL.md"))
|
||||
.with_display_path_root(root),
|
||||
],
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
|
||||
let fragment = available_skills_fragment(
|
||||
&catalog,
|
||||
/*include_skills_usage_instructions*/ false,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
SkillMetadataBudget::Characters(usize::MAX),
|
||||
)
|
||||
.expect("catalog should render");
|
||||
|
||||
assert!(!fragment.body().contains("### Skill roots"));
|
||||
assert!(
|
||||
fragment
|
||||
.body()
|
||||
.contains(&format!("(file: {root}/alpha/SKILL.md)"))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn path_aliases_retain_every_skill_under_budget_pressure() {
|
||||
let root = "/Users/test/.codex/plugins/cache/openai-curated/example/hash1234567890/skills-with-a-very-long-shared-prefix";
|
||||
let entries = (0..12)
|
||||
.map(|index| {
|
||||
let name = format!("shared-root-skill-{index}");
|
||||
entry(&name, "Description.", /*short_description*/ None)
|
||||
.with_display_path(format!("{root}/skill-{index}/SKILL.md"))
|
||||
.with_display_path_root(root)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
let catalog = SkillCatalog {
|
||||
entries,
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
let visible_entries = catalog.entries.iter().collect::<Vec<_>>();
|
||||
let plan = build_alias_plan(
|
||||
&visible_entries,
|
||||
SkillMetadataBudget::Characters(usize::MAX),
|
||||
)
|
||||
.expect("alias plan should build");
|
||||
let alias_minimum = visible_entries.iter().fold(plan.table_cost, |cost, entry| {
|
||||
cost.saturating_add(
|
||||
SkillLine::with_locator(
|
||||
entry,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
render_skill_path_with_aliases(entry, &plan),
|
||||
)
|
||||
.minimum_cost(SkillMetadataBudget::Characters(usize::MAX)),
|
||||
)
|
||||
});
|
||||
let absolute_minimum = visible_entries.iter().fold(0usize, |cost, entry| {
|
||||
cost.saturating_add(
|
||||
SkillLine::new(entry, SkillCatalogRenderPolicy::ExtensionCompatible)
|
||||
.minimum_cost(SkillMetadataBudget::Characters(usize::MAX)),
|
||||
)
|
||||
});
|
||||
assert!(alias_minimum < absolute_minimum);
|
||||
|
||||
let fragment = available_skills_fragment(
|
||||
&catalog,
|
||||
/*include_skills_usage_instructions*/ true,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
SkillMetadataBudget::Characters(alias_minimum),
|
||||
)
|
||||
.expect("catalog should render");
|
||||
let body = fragment.body();
|
||||
|
||||
assert!(body.contains(&format!("- `r0` = `{root}`")));
|
||||
assert!(body.contains("(file: r0/skill-0/SKILL.md)"));
|
||||
assert!(body.contains("(file: r0/skill-11/SKILL.md)"));
|
||||
assert!(body.contains("Skill bodies live on disk at the listed paths after expanding"));
|
||||
assert!(!body.contains("additional skills omitted"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mixed_catalogs_keep_absolute_authority_aware_rendering_under_budget_pressure() {
|
||||
let root = "/Users/test/.codex/plugins/cache/openai-curated/example/hash1234567890/skills-with-a-very-long-shared-prefix";
|
||||
let mut entries = (0..12)
|
||||
.map(|index| {
|
||||
let name = format!("host-skill-{index}");
|
||||
entry(&name, "Description.", /*short_description*/ None)
|
||||
.with_display_path(format!("{root}/skill-{index}/SKILL.md"))
|
||||
.with_display_path_root(root)
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
entries.push(
|
||||
SkillCatalogEntry::new(
|
||||
SkillPackageId("executor-skill".to_string()),
|
||||
SkillAuthority::new(SkillSourceKind::Executor, "env-1"),
|
||||
"executor-skill",
|
||||
"Description.",
|
||||
SkillResourceId::new("skill://executor/demo/SKILL.md"),
|
||||
)
|
||||
.with_display_path("skill://executor/demo/SKILL.md"),
|
||||
);
|
||||
let catalog = SkillCatalog {
|
||||
entries,
|
||||
warnings: Vec::new(),
|
||||
};
|
||||
let visible_entries = catalog.entries.iter().collect::<Vec<_>>();
|
||||
let absolute_minimum = visible_entries.iter().fold(0usize, |cost, entry| {
|
||||
cost.saturating_add(
|
||||
SkillLine::new(entry, SkillCatalogRenderPolicy::ExtensionCompatible)
|
||||
.minimum_cost(SkillMetadataBudget::Characters(usize::MAX)),
|
||||
)
|
||||
});
|
||||
|
||||
assert!(
|
||||
build_alias_plan(
|
||||
&visible_entries,
|
||||
SkillMetadataBudget::Characters(usize::MAX),
|
||||
)
|
||||
.is_none()
|
||||
);
|
||||
|
||||
let fragment = available_skills_fragment(
|
||||
&catalog,
|
||||
/*include_skills_usage_instructions*/ true,
|
||||
SkillCatalogRenderPolicy::ExtensionCompatible,
|
||||
SkillMetadataBudget::Characters(absolute_minimum),
|
||||
)
|
||||
.expect("catalog should render");
|
||||
let body = fragment.body();
|
||||
|
||||
assert!(!body.contains("### Skill roots"));
|
||||
assert!(body.contains(&format!("(file: {root}/skill-0/SKILL.md)")));
|
||||
assert!(body.contains("(environment resource: skill://executor/demo/SKILL.md)"));
|
||||
assert!(body.contains("For a `file` entry, open the listed path."));
|
||||
assert!(!body.contains("additional skills omitted"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn singleton_plugin_versions_share_the_marketplace_alias_root() {
|
||||
let github_root = "/Users/test/.codex/plugins/cache/openai-curated/github/hash123/skills";
|
||||
let slack_root = "/Users/test/.codex/plugins/cache/openai-curated/slack/hash456/skills";
|
||||
let entries = [
|
||||
entry("github", "GitHub skill.", /*short_description*/ None)
|
||||
.with_display_path(format!("{github_root}/github/SKILL.md"))
|
||||
.with_display_path_root(github_root),
|
||||
entry("slack", "Slack skill.", /*short_description*/ None)
|
||||
.with_display_path(format!("{slack_root}/slack/SKILL.md"))
|
||||
.with_display_path_root(slack_root),
|
||||
];
|
||||
let visible_entries = entries.iter().collect::<Vec<_>>();
|
||||
|
||||
let plan = build_alias_plan(
|
||||
&visible_entries,
|
||||
SkillMetadataBudget::Characters(usize::MAX),
|
||||
)
|
||||
.expect("alias plan should build");
|
||||
|
||||
assert_eq!(
|
||||
plan.skill_root_lines,
|
||||
vec!["- `r0` = `/Users/test/.codex/plugins/cache/openai-curated`".to_string()]
|
||||
);
|
||||
assert_eq!(
|
||||
render_skill_path_with_aliases(&entries[0], &plan),
|
||||
"r0/github/hash123/skills/github/SKILL.md"
|
||||
);
|
||||
assert_eq!(
|
||||
render_skill_path_with_aliases(&entries[1], &plan),
|
||||
"r0/slack/hash456/skills/slack/SKILL.md"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn omission_notice_follows_render_policy_and_is_charged_to_catalog_budget() {
|
||||
let catalog = SkillCatalog {
|
||||
|
||||
@@ -8,6 +8,10 @@ use codex_core_skills::HostSkillsSnapshot;
|
||||
use codex_core_skills::SKILLS_INTRO_WITH_ABSOLUTE_PATHS;
|
||||
use codex_core_skills::SkillLoadOutcome;
|
||||
use codex_core_skills::injection::InjectedHostSkillPrompts;
|
||||
use codex_core_skills::loader::MAX_CONCURRENT_ROOT_SCANS;
|
||||
use codex_core_skills::loader::SkillRoot;
|
||||
use codex_core_skills::loader::load_skills_from_roots;
|
||||
use codex_exec_server::LOCAL_FS;
|
||||
use codex_extension_api::ConversationHistory;
|
||||
use codex_extension_api::ExtensionData;
|
||||
use codex_extension_api::ExtensionEventSink;
|
||||
@@ -53,6 +57,7 @@ use codex_skills_extension::provider::SkillSearchRequest;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use codex_utils_path_uri::PathUri;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
type TestResult = Result<(), Box<dyn std::error::Error>>;
|
||||
|
||||
@@ -1028,6 +1033,98 @@ async fn executor_catalog_emits_at_most_four_warnings() -> TestResult {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn host_catalog_compacts_shared_paths_under_budget_pressure() -> TestResult {
|
||||
let test_root = test_codex_home();
|
||||
let root = test_root.join(
|
||||
"plugins/cache/openai-curated/example/hash1234567890/skills-with-a-very-long-shared-prefix",
|
||||
);
|
||||
for index in 0..12 {
|
||||
let skill_dir = root.join(format!("skill-{index:02}"));
|
||||
std::fs::create_dir_all(&skill_dir)?;
|
||||
std::fs::write(
|
||||
skill_dir.join("SKILL.md"),
|
||||
format!(
|
||||
"---\nname: skill-{index:02}\ndescription: Fix lint errors.\n---\n# Skill {index:02}\n"
|
||||
),
|
||||
)?;
|
||||
}
|
||||
let root = AbsolutePathBuf::try_from(std::fs::canonicalize(root)?)?;
|
||||
let rendered_root = root.to_string_lossy().replace('\\', "/");
|
||||
let outcome = load_skills_from_roots(
|
||||
[SkillRoot {
|
||||
path: root,
|
||||
scope: SkillScope::User,
|
||||
file_system: Arc::clone(&LOCAL_FS),
|
||||
plugin_id: None,
|
||||
plugin_namespace: None,
|
||||
plugin_root: None,
|
||||
discovery_mode: Default::default(),
|
||||
}],
|
||||
/*plugin_skill_snapshots*/ None,
|
||||
Arc::new(Semaphore::new(MAX_CONCURRENT_ROOT_SCANS)),
|
||||
)
|
||||
.await;
|
||||
assert_eq!(outcome.errors, Vec::new());
|
||||
assert_eq!(outcome.skills.len(), 12);
|
||||
|
||||
let mut builder = ExtensionRegistryBuilder::new();
|
||||
install(&mut builder, skills_extension_config);
|
||||
let registry = builder.build();
|
||||
let session_store = ExtensionData::new("session");
|
||||
let thread_store = ExtensionData::new("thread");
|
||||
let mut config = default_config();
|
||||
config.bundled_skills_enabled = false;
|
||||
registry.thread_lifecycle_contributors()[0]
|
||||
.on_thread_start(ThreadStartInput {
|
||||
config: &config,
|
||||
session_source: &SessionSource::Cli,
|
||||
persistent_thread_state_available: true,
|
||||
environments: &[],
|
||||
mcp_resource_client: None,
|
||||
session_store: &session_store,
|
||||
thread_store: &thread_store,
|
||||
})
|
||||
.await;
|
||||
let mut model_info = model_info_from_slug("test-model");
|
||||
model_info.context_window = Some(10_000);
|
||||
thread_store.insert(model_info);
|
||||
let turn_store = ExtensionData::new("turn-1");
|
||||
turn_store.insert(HostSkillsSnapshot::new(Arc::new(outcome)));
|
||||
|
||||
let fragments = registry.turn_input_contributors()[0]
|
||||
.contribute(
|
||||
TurnInputContext {
|
||||
turn_id: "turn-1".to_string(),
|
||||
user_input: vec![UserInput::Text {
|
||||
text: "hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
environments: Vec::new(),
|
||||
},
|
||||
&session_store,
|
||||
&thread_store,
|
||||
&turn_store,
|
||||
)
|
||||
.await;
|
||||
let catalog = fragments
|
||||
.iter()
|
||||
.find(|fragment| fragment.role() == "developer")
|
||||
.ok_or("host catalog should render")?
|
||||
.render();
|
||||
|
||||
assert!(
|
||||
catalog.contains(&format!("- `r0` = `{rendered_root}`")),
|
||||
"{catalog}"
|
||||
);
|
||||
assert!(catalog.contains("(file: r0/skill-00/SKILL.md)"));
|
||||
assert!(catalog.contains("(file: r0/skill-11/SKILL.md)"));
|
||||
assert!(!catalog.contains("additional skills omitted"));
|
||||
|
||||
std::fs::remove_dir_all(test_root)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn prompt_hidden_skill_can_still_be_invoked() -> TestResult {
|
||||
let read_requests = Arc::new(Mutex::new(Vec::new()));
|
||||
|
||||
Reference in New Issue
Block a user