diff --git a/codex-rs/core-skills/src/lib.rs b/codex-rs/core-skills/src/lib.rs index 31e5a7c531..af87c5e59d 100644 --- a/codex-rs/core-skills/src/lib.rs +++ b/codex-rs/core-skills/src/lib.rs @@ -29,6 +29,7 @@ pub use render::SkillRenderReport; pub use render::build_available_skills; pub use render::default_skill_metadata_budget; pub use render::render_available_skills_body; +pub use render::render_legacy_available_skills_body; pub use service::SkillsLoadInput; pub use service::SkillsService; pub use skill_instructions::SkillInstructions; diff --git a/codex-rs/core-skills/src/render.rs b/codex-rs/core-skills/src/render.rs index 1477a299b6..0effea1b7a 100644 --- a/codex-rs/core-skills/src/render.rs +++ b/codex-rs/core-skills/src/render.rs @@ -60,6 +60,27 @@ 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 { + format_available_skills_body(skill_root_lines, skill_lines, None) +} + +/// Renders the compatibility prompt used by models that do not own their skills guidance yet. +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 + }; + format_available_skills_body(skill_root_lines, skill_lines, Some(how_to_use)) +} + +fn format_available_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 +93,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")) } @@ -786,8 +804,10 @@ fn aliased_metadata_overhead_cost( 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); + // Preserve the existing alias-selection accounting while legacy models still + // receive path-format-specific usage guidance. + let absolute_body = render_legacy_available_skills_body(&[], empty_skill_lines); + let aliased_body = render_legacy_available_skills_body(skill_root_lines, empty_skill_lines); budget .cost(&aliased_body) .saturating_sub(budget.cost(&absolute_body)) @@ -1006,6 +1026,29 @@ mod tests { } } + #[test] + fn available_skills_body_omits_client_owned_usage_guidance() { + let body = render_available_skills_body(&[], &["- demo: desc".to_string()]); + + assert!(body.contains("### Available skills")); + assert!(body.contains("- demo: desc")); + assert!(!body.contains("### How to use skills")); + } + + #[test] + fn legacy_available_skills_body_includes_matching_usage_guidance() { + let absolute = render_legacy_available_skills_body(&[], &["- demo: desc".to_string()]); + assert!(absolute.contains("### How to use skills")); + assert!(absolute.contains(SKILLS_HOW_TO_USE_WITH_ABSOLUTE_PATHS)); + + let aliased = render_legacy_available_skills_body( + &["- `r0` = `/tmp/skills`".to_string()], + &["- demo: desc".to_string()], + ); + assert!(aliased.contains("### How to use skills")); + assert!(aliased.contains(SKILLS_HOW_TO_USE_WITH_ALIASES)); + } + #[test] fn default_budget_uses_two_percent_of_full_context_window() { assert_eq!( diff --git a/codex-rs/core/src/context/available_skills_instructions.rs b/codex-rs/core/src/context/available_skills_instructions.rs index 4e166d436c..ac2bbbbd1b 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_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,8 +20,16 @@ impl AvailableSkillsInstructions { Self { skill_root_lines: Vec::new(), skill_lines, + include_legacy_usage_instructions: false, } } + + /// Retains the client-owned guidance for models that have not migrated it + /// into their own instructions yet. + pub fn with_legacy_usage_instructions(mut self) -> Self { + self.include_legacy_usage_instructions = true; + self + } } impl From for AvailableSkillsInstructions { @@ -27,6 +37,7 @@ impl From for AvailableSkillsInstructions { Self { skill_root_lines: available_skills.skill_root_lines, skill_lines: available_skills.skill_lines, + include_legacy_usage_instructions: false, } } } @@ -45,6 +56,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..28782e6298 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -460,6 +460,19 @@ pub(crate) const INITIAL_SUBMIT_ID: &str = ""; 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"; +// Compatibility list for models through GPT-5.5. Newer models own this guidance. +const MODELS_REQUIRING_LEGACY_SKILLS_INSTRUCTIONS: &[&str] = &[ + "gpt-5.5", + "gpt-5.4", + "gpt-5.4-mini", + "gpt-5.3-codex", + "gpt-5.2", + "codex-auto-review", +]; + +fn model_requires_legacy_skills_instructions(model: &str) -> bool { + MODELS_REQUIRING_LEGACY_SKILLS_INSTRUCTIONS.contains(&model) +} impl Codex { /// Spawn a new [`Codex`] and initialize the session. @@ -2980,6 +2993,13 @@ 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 = if model_requires_legacy_skills_instructions( + turn_context.model_info.slug.as_str(), + ) { + skills_instructions.with_legacy_usage_instructions() + } else { + skills_instructions + }; 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..cf000f541c 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!( @@ -1649,6 +1653,73 @@ async fn skills_append_to_developer_message() { let _codex_home_guard = codex_home; } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn models_outside_legacy_list_omit_legacy_guidance() { + skip_if_no_network!(); + let server = MockServer::start().await; + + let resp_mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp1"), ev_completed("resp1")]), + ) + .await; + + let codex_home = Arc::new(TempDir::new().unwrap()); + let skill_dir = codex_home.path().join("skills/demo"); + std::fs::create_dir_all(&skill_dir).expect("create skill dir"); + std::fs::write( + skill_dir.join("SKILL.md"), + "---\nname: demo\ndescription: build charts\n---\n\n# body\n", + ) + .expect("write skill"); + + let codex_home_path = codex_home.path().to_path_buf(); + let mut builder = test_codex() + .with_home(codex_home.clone()) + .with_auth(CodexAuth::from_api_key("Test API Key")) + .with_model_info_override("gpt-5.4", |model_info| { + model_info.slug = "future-model".to_string(); + }) + .with_model("future-model") + .with_config(move |config| { + config.cwd = codex_home_path.abs(); + }); + let codex = builder + .build(&server) + .await + .expect("create new conversation") + .codex; + + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "hello".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + responsesapi_client_metadata: None, + additional_context: Default::default(), + thread_settings: Default::default(), + }) + .await + .unwrap(); + + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + let request = resp_mock.single_request(); + let developer_messages = request.message_input_texts("developer"); + let developer_text = developer_messages.join("\n\n"); + assert!( + developer_text.contains("demo: build charts"), + "expected dynamic skills catalog: {developer_messages:?}" + ); + assert!( + !developer_text.contains("### How to use skills"), + "did not expect legacy skills guidance when the model owns it: {developer_messages:?}" + ); + let _codex_home_guard = codex_home; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn skills_use_aliases_in_developer_message_under_budget_pressure() { skip_if_no_network!(); 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"