Migrate skills usage guidance to model instructions

This commit is contained in:
ani
2026-06-18 09:23:21 -07:00
parent 07298a948c
commit 8e315ac027
5 changed files with 67 additions and 15 deletions

View File

@@ -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<String> = 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();

View File

@@ -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<String>,
skill_lines: Vec<String>,
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<AvailableSkills> 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)
}
}
}

View File

@@ -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<CodexSpawnOk> {
@@ -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(),

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!(

View File

@@ -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>"