From 5a1c54fc2110b1299c80e949425044dff478da25 Mon Sep 17 00:00:00 2001 From: felixxia-oai Date: Fri, 24 Jul 2026 12:53:33 +0000 Subject: [PATCH] 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 --- codex-rs/core-skills/src/model.rs | 5 + codex-rs/ext/skills/src/catalog.rs | 12 + codex-rs/ext/skills/src/fragments.rs | 10 +- codex-rs/ext/skills/src/provider/host.rs | 6 +- .../ext/skills/src/provider/host_tests.rs | 14 +- codex-rs/ext/skills/src/render.rs | 289 +++++++++++++++++- codex-rs/ext/skills/src/render_tests.rs | 177 +++++++++++ codex-rs/ext/skills/tests/skills_extension.rs | 97 ++++++ 8 files changed, 597 insertions(+), 13 deletions(-) diff --git a/codex-rs/core-skills/src/model.rs b/codex-rs/core-skills/src/model.rs index fd6d9a3b36..e146b79866 100644 --- a/codex-rs/core-skills/src/model.rs +++ b/codex-rs/core-skills/src/model.rs @@ -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, diff --git a/codex-rs/ext/skills/src/catalog.rs b/codex-rs/ext/skills/src/catalog.rs index 23bf112767..15850878f7 100644 --- a/codex-rs/ext/skills/src/catalog.rs +++ b/codex-rs/ext/skills/src/catalog.rs @@ -140,6 +140,7 @@ pub struct SkillCatalogEntry { pub short_description: Option, pub main_prompt: SkillResourceId, pub display_path: Option, + display_path_root: Option, prompt_scope: Option, pub dependencies: Option, 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) -> 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 { self.prompt_scope } diff --git a/codex-rs/ext/skills/src/fragments.rs b/codex-rs/ext/skills/src/fragments.rs index 7f0e063da3..21e9366982 100644 --- a/codex-rs/ext/skills/src/fragments.rs +++ b/codex-rs/ext/skills/src/fragments.rs @@ -14,15 +14,21 @@ pub(crate) struct AvailableSkillsInstructions { impl AvailableSkillsInstructions { pub(crate) fn from_skill_lines( + skill_root_lines: Vec, mut skill_lines: Vec, 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, } } diff --git a/codex-rs/ext/skills/src/provider/host.rs b/codex-rs/ext/skills/src/provider/host.rs index 7b8f8b4291..e2c47b6a1a 100644 --- a/codex-rs/ext/skills/src/provider/host.rs +++ b/codex-rs/ext/skills/src/provider/host.rs @@ -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 diff --git a/codex-rs/ext/skills/src/provider/host_tests.rs b/codex-rs/ext/skills/src/provider/host_tests.rs index 83f795d4b2..39243cbf60 100644 --- a/codex-rs/ext/skills/src/provider/host_tests.rs +++ b/codex-rs/ext/skills/src/provider/host_tests.rs @@ -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> { +async fn host_catalog_entries_carry_their_render_metadata() -> Result<(), Box> +{ 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 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, + skill_lines: Vec, + report: SkillRenderReport, +} + pub(crate) struct AvailableSkillsRender { + skill_root_lines: Vec, skill_lines: Vec, pub(crate) report: SkillRenderReport, } @@ -434,6 +462,7 @@ impl AvailableSkillsRender { ) -> Option { (!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::>(); policy.order_entries(&mut entries); - let skill_lines = entries - .iter() - .map(|entry| SkillLine::new(entry, policy)) - .collect::>(); - 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>, + budget: SkillMetadataBudget, + skill_root_lines: Vec, + 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, + alias_root_by_display_root: HashMap, + root_aliases: HashMap, + table_cost: usize, +} + +fn build_aliased_catalog( + entries: &[&SkillCatalogEntry], + policy: SkillCatalogRenderPolicy, + budget: SkillMetadataBudget, +) -> Option { + 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 { + // 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::>(); + 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 { + 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 { + 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 { + 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 { + 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.") diff --git a/codex-rs/ext/skills/src/render_tests.rs b/codex-rs/ext/skills/src/render_tests.rs index 9dffad3456..2f5318a635 100644 --- a/codex-rs/ext/skills/src/render_tests.rs +++ b/codex-rs/ext/skills/src/render_tests.rs @@ -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::>(); + let catalog = SkillCatalog { + entries, + warnings: Vec::new(), + }; + let visible_entries = catalog.entries.iter().collect::>(); + 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::>(); + 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::>(); + 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::>(); + + 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 { diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index d0a0afd61d..3616eb3809 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -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>; @@ -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()));