Migrate skills usage guidance to model instructions

This commit is contained in:
ani
2026-06-18 09:23:21 -07:00
parent 2c7802e7cf
commit ecb3c7c1d1
6 changed files with 161 additions and 12 deletions
+1
View File
@@ -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;
+52 -9
View File
@@ -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<String> = 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!(
@@ -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<String>,
skill_lines: Vec<String>,
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<AvailableSkills> for AvailableSkillsInstructions {
@@ -27,6 +37,7 @@ impl From<AvailableSkills> 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)
}
}
}
+20
View File
@@ -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(),
+71
View File
@@ -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!();
@@ -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!(
"<skill>\n<name>demo</name>\n<path>{skill_prompt_path}</path>\n{DEMO_SKILL_CONTENTS}\n</skill>"