diff --git a/codex-rs/ext/skills/src/extension.rs b/codex-rs/ext/skills/src/extension.rs index e34cac46f9..e89b5923c8 100644 --- a/codex-rs/ext/skills/src/extension.rs +++ b/codex-rs/ext/skills/src/extension.rs @@ -43,6 +43,7 @@ use crate::provider::SkillListQuery; use crate::provider::SkillReadRequest; use crate::render::MAX_SKILL_NAME_BYTES; use crate::render::MAX_SKILL_PATH_BYTES; +use crate::render::SkillCatalogRenderPolicy; use crate::render::available_skills_fragment; use crate::render::truncate_main_prompt_contents; use crate::render::truncate_utf8_to_bytes; @@ -144,10 +145,14 @@ where let include_usage = thread_store .get::() .is_some_and(|model_info| model_info.include_skills_usage_instructions); - available_skills_fragment(&catalog, include_usage) - .map(|fragment| PromptFragment::developer_capability(fragment.render())) - .into_iter() - .collect() + available_skills_fragment( + &catalog, + include_usage, + SkillCatalogRenderPolicy::ExtensionCompatible, + ) + .map(|fragment| PromptFragment::developer_capability(fragment.render())) + .into_iter() + .collect() }) } @@ -329,7 +334,11 @@ where let include_usage = thread_store .get::() .is_some_and(|model_info| model_info.include_skills_usage_instructions); - if let Some(fragment) = available_skills_fragment(&turn_catalog, include_usage) { + if let Some(fragment) = available_skills_fragment( + &turn_catalog, + include_usage, + SkillCatalogRenderPolicy::ExtensionCompatible, + ) { fragments.push(Box::new(fragment)); } } diff --git a/codex-rs/ext/skills/src/render.rs b/codex-rs/ext/skills/src/render.rs index 8e9fea41a1..809d723f91 100644 --- a/codex-rs/ext/skills/src/render.rs +++ b/codex-rs/ext/skills/src/render.rs @@ -14,6 +14,31 @@ const TRUNCATED_SKILL_DESCRIPTION_SUFFIX: &str = "..."; pub(crate) const MAX_SKILL_NAME_BYTES: usize = 256; pub(crate) const MAX_SKILL_PATH_BYTES: usize = 1_024; +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum SkillCatalogRenderPolicy { + #[cfg_attr( + not(test), + expect( + dead_code, + reason = "used by the host renderer compatibility path in a follow-up" + ) + )] + CoreCompatible, + ExtensionCompatible, +} + +impl SkillCatalogRenderPolicy { + fn description(self, entry: &SkillCatalogEntry) -> &str { + match self { + Self::CoreCompatible => entry.description.as_str(), + Self::ExtensionCompatible => entry + .short_description + .as_deref() + .unwrap_or(entry.description.as_str()), + } + } +} + #[tracing::instrument( level = "trace", skip_all, @@ -22,6 +47,7 @@ pub(crate) const MAX_SKILL_PATH_BYTES: usize = 1_024; pub(crate) fn available_skills_fragment( catalog: &SkillCatalog, include_skills_usage_instructions: bool, + policy: SkillCatalogRenderPolicy, ) -> Option { let mut total_bytes = 0usize; let mut omitted = 0usize; @@ -32,10 +58,7 @@ pub(crate) fn available_skills_fragment( .iter() .filter(|entry| entry.enabled && entry.prompt_visible) { - let description = entry - .short_description - .as_deref() - .unwrap_or(entry.description.as_str()); + let description = policy.description(entry); let description = truncate_catalog_skill_description(description); let line = render_skill_line(entry, description.as_ref()); let next_bytes = total_bytes.saturating_add(line.len()); @@ -107,3 +130,7 @@ pub(crate) fn truncate_utf8_to_bytes(contents: &str, max_bytes: usize) -> (Strin let truncated = take_bytes_at_char_boundary(contents, max_bytes); (truncated.to_string(), truncated.len() < contents.len()) } + +#[cfg(test)] +#[path = "render_tests.rs"] +mod tests; diff --git a/codex-rs/ext/skills/src/render_tests.rs b/codex-rs/ext/skills/src/render_tests.rs new file mode 100644 index 0000000000..69d5df5442 --- /dev/null +++ b/codex-rs/ext/skills/src/render_tests.rs @@ -0,0 +1,67 @@ +use super::*; +use crate::catalog::SkillAuthority; +use crate::catalog::SkillPackageId; +use crate::catalog::SkillResourceId; +use codex_core_skills::render_available_skills_body; +use codex_extension_api::ContextualUserFragment; +use pretty_assertions::assert_eq; + +fn entry(name: &str, description: &str, short_description: Option<&str>) -> SkillCatalogEntry { + SkillCatalogEntry::new( + SkillPackageId(name.to_string()), + SkillAuthority::new(SkillSourceKind::Host, "host"), + name, + description, + SkillResourceId::new(format!("/skills/{name}/SKILL.md")), + ) + .with_short_description(short_description.map(str::to_string)) +} + +#[test] +fn description_selection_follows_render_policy() { + let catalog = SkillCatalog { + entries: vec![ + entry("shortened", "full description", Some("short description")), + entry( + "fallback", + "fallback description", + /*short_description*/ None, + ), + ], + warnings: Vec::new(), + }; + + let core = available_skills_fragment( + &catalog, + /*include_skills_usage_instructions*/ false, + SkillCatalogRenderPolicy::CoreCompatible, + ) + .expect("catalog should render"); + let extension = available_skills_fragment( + &catalog, + /*include_skills_usage_instructions*/ false, + SkillCatalogRenderPolicy::ExtensionCompatible, + ) + .expect("catalog should render"); + + assert_eq!( + core.body(), + render_available_skills_body( + &[], + &[ + "- shortened: full description (file: /skills/shortened/SKILL.md)".to_string(), + "- fallback: fallback description (file: /skills/fallback/SKILL.md)".to_string(), + ], + ) + ); + assert_eq!( + extension.body(), + render_available_skills_body( + &[], + &[ + "- shortened: short description (file: /skills/shortened/SKILL.md)".to_string(), + "- fallback: fallback description (file: /skills/fallback/SKILL.md)".to_string(), + ], + ) + ); +} diff --git a/codex-rs/ext/skills/src/world_state.rs b/codex-rs/ext/skills/src/world_state.rs index 80a1b00777..a404540264 100644 --- a/codex-rs/ext/skills/src/world_state.rs +++ b/codex-rs/ext/skills/src/world_state.rs @@ -12,6 +12,7 @@ use serde_json::json; use crate::catalog::SkillCatalog; use crate::fragments::AvailableSkillsInstructions; +use crate::render::SkillCatalogRenderPolicy; use crate::render::available_skills_fragment; pub(crate) const SKILLS_WORLD_STATE_ID: &str = "skills"; @@ -31,8 +32,12 @@ pub(crate) fn executor_skills_world_state_section( include_skills_usage_instructions: bool, ) -> WorldStateSectionContribution { let body = if include_instructions { - available_skills_fragment(catalog, include_skills_usage_instructions) - .map(|fragment| fragment.body()) + available_skills_fragment( + catalog, + include_skills_usage_instructions, + SkillCatalogRenderPolicy::ExtensionCompatible, + ) + .map(|fragment| fragment.body()) } else { None };