diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index 1477a299b6..e46ee3f026 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -60,6 +60,26 @@ pub const SKILLS_HOW_TO_USE_WITH_ALIASES: &str = r###"- Discovery: The list abov - 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 { + render_skills_body(skill_root_lines, skill_lines, None) +} + +pub fn render_legacy_available_skills_body( + skill_root_lines: &[String], + skill_lines: &[String], +) -> 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 + }; + render_skills_body(skill_root_lines, skill_lines, Some(how_to_use)) +} + +fn render_skills_body( + skill_root_lines: &[String], + skill_lines: &[String], + how_to_use: Option<&str>, +) -> String { let mut lines: Vec = Vec::new(); lines.push("## Skills".to_string()); if skill_root_lines.is_empty() { @@ -72,13 +92,10 @@ pub fn render_available_skills_body(skill_root_lines: &[String], skill_lines: &[ 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()); + if let Some(how_to_use) = how_to_use { + lines.push("### How to use skills".to_string()); + lines.push(how_to_use.to_string()); + } format!("\n{}\n", lines.join("\n")) } @@ -904,10 +921,10 @@ fn prompt_scope_rank(scope: SkillScope) -> u8 { #[cfg(test)] mod tests { - use super::*; use std::collections::HashMap; use std::sync::Arc; + use super::*; use codex_utils_absolute_path::test_support::PathBufExt; use codex_utils_absolute_path::test_support::test_path_buf; use pretty_assertions::assert_eq; @@ -1203,6 +1220,19 @@ mod tests { assert!(rendered_text.contains("- repo-skill:")); } + #[test] + fn alias_overhead_uses_catalog_only_prompt() { + let roots = vec![format!("- `r0` = `/very/long/{}`", "x".repeat(600))]; + let budget = SkillMetadataBudget::Tokens(usize::MAX); + let empty: &[String] = &[]; + let expected = budget + .cost(&render_available_skills_body(&roots, empty)) + .saturating_sub(budget.cost(&render_available_skills_body(&[], empty))); + + assert!(expected > 0); + assert_eq!(aliased_metadata_overhead_cost(budget, &roots), expected); + } + #[test] fn outcome_rendering_omits_aliases_when_absolute_plan_has_no_budget_pressure() { let root = test_path_buf("/tmp/skills").abs(); diff --git a/codex-rs/core/src/context/available_skills_instructions.rs b/codex-rs/core/src/context/available_skills_instructions.rs index 4e166d436c..db8175b4de 100644 --- a/codex-rs/core/src/context/available_skills_instructions.rs +++ b/codex-rs/core/src/context/available_skills_instructions.rs @@ -1,5 +1,6 @@ use codex_core_skills::AvailableSkills; use codex_core_skills::render_available_skills_body; +use codex_core_skills::render::render_legacy_available_skills_body; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_CLOSE_TAG; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; @@ -10,6 +11,7 @@ use super::ContextualUserFragment; pub struct AvailableSkillsInstructions { skill_root_lines: Vec, skill_lines: Vec, + include_legacy_usage_instructions: bool, } impl AvailableSkillsInstructions { @@ -18,15 +20,18 @@ impl AvailableSkillsInstructions { Self { skill_root_lines: Vec::new(), skill_lines, + include_legacy_usage_instructions: false, } } -} -impl From for AvailableSkillsInstructions { - fn from(available_skills: AvailableSkills) -> Self { + pub fn from_available_skills( + available_skills: AvailableSkills, + include_legacy_usage_instructions: bool, + ) -> Self { Self { skill_root_lines: available_skills.skill_root_lines, skill_lines: available_skills.skill_lines, + include_legacy_usage_instructions, } } } @@ -45,6 +50,10 @@ impl ContextualUserFragment for AvailableSkillsInstructions { } fn body(&self) -> String { - render_available_skills_body(&self.skill_root_lines, &self.skill_lines) + if self.include_legacy_usage_instructions { + render_legacy_available_skills_body(&self.skill_root_lines, &self.skill_lines) + } else { + render_available_skills_body(&self.skill_root_lines, &self.skill_lines) + } } } diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 0ecc706818..754eb7324c 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -461,6 +461,13 @@ pub(crate) const SUBMISSION_CHANNEL_CAPACITY: usize = 512; const CYBER_VERIFY_URL: &str = "https://chatgpt.com/cyber"; const CYBER_SAFETY_URL: &str = "https://developers.openai.com/codex/concepts/cyber-safety"; +fn model_requires_legacy_skills_instructions(model: &str) -> bool { + matches!( + model, + "gpt-5.5" | "gpt-5.4" | "gpt-5.4-mini" | "gpt-5.3-codex" | "gpt-5.2" | "codex-auto-review" + ) +} + impl Codex { /// Spawn a new [`Codex`] and initialize the session. pub(crate) async fn spawn(args: CodexSpawnArgs) -> CodexResult { @@ -2979,7 +2986,10 @@ impl Session { ); if let Some(available_skills) = available_skills { let warning_message = available_skills.warning_message.clone(); - let skills_instructions = AvailableSkillsInstructions::from(available_skills); + let skills_instructions = AvailableSkillsInstructions::from_available_skills( + available_skills, + model_requires_legacy_skills_instructions(&turn_context.model_info.slug), + ); if let Some(warning_message) = warning_message { self.send_event_raw(Event { id: String::new(), diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 54e24d49cb..264a1b6aa6 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1640,6 +1640,10 @@ async fn skills_append_to_developer_message() { developer_text.contains("demo: build charts"), "expected skill summary: {developer_messages:?}" ); + assert!( + developer_text.contains("### How to use skills"), + "expected legacy skills guidance for models without model-owned instructions: {developer_messages:?}" + ); let expected_path = normalize_path(skill_dir.join("SKILL.md")).unwrap(); let expected_path_str = expected_path.to_string_lossy().replace('\\', "/"); assert!( diff --git a/codex-rs/ext/skills/tests/skills_extension.rs b/codex-rs/ext/skills/tests/skills_extension.rs index cd6f46d9c9..b3f948368d 100644 --- a/codex-rs/ext/skills/tests/skills_extension.rs +++ b/codex-rs/ext/skills/tests/skills_extension.rs @@ -5,7 +5,6 @@ use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; use codex_core_skills::HostSkillsSnapshot; -use codex_core_skills::SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS; use codex_core_skills::SKILLS_INTRO_WITH_ABSOLUTE_PATHS; use codex_core_skills::SkillLoadOutcome; use codex_core_skills::SkillMetadata; @@ -116,7 +115,7 @@ async fn installed_extension_uses_host_service_snapshot() -> TestResult { .await; let expected_catalog = format!( - "{SKILLS_INSTRUCTIONS_OPEN_TAG}\n## Skills\n{SKILLS_INTRO_WITH_ABSOLUTE_PATHS}\n### Available skills\n- demo: Demo skill. (file: {skill_prompt_path})\n### How to use skills\n{SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS}\n{SKILLS_INSTRUCTIONS_CLOSE_TAG}" + "{SKILLS_INSTRUCTIONS_OPEN_TAG}\n## Skills\n{SKILLS_INTRO_WITH_ABSOLUTE_PATHS}\n### Available skills\n- demo: Demo skill. (file: {skill_prompt_path})\n{SKILLS_INSTRUCTIONS_CLOSE_TAG}" ); let expected_skill = format!( "\ndemo\n{skill_prompt_path}\n{DEMO_SKILL_CONTENTS}\n"