Remove load from SKILL.toml fallback (#10007)

This commit is contained in:
alexsong-oai
2026-01-27 12:06:40 -08:00
committed by GitHub
parent 30eb655ad1
commit 2f8a44baea

View File

@@ -52,7 +52,6 @@ struct Interface {
const SKILLS_FILENAME: &str = "SKILL.md"; const SKILLS_FILENAME: &str = "SKILL.md";
const SKILLS_JSON_FILENAME: &str = "SKILL.json"; const SKILLS_JSON_FILENAME: &str = "SKILL.json";
const SKILLS_TOML_FILENAME: &str = "SKILL.toml";
const SKILLS_DIR_NAME: &str = "skills"; const SKILLS_DIR_NAME: &str = "skills";
const MAX_NAME_LEN: usize = 64; const MAX_NAME_LEN: usize = 64;
const MAX_DESCRIPTION_LEN: usize = 1024; const MAX_DESCRIPTION_LEN: usize = 1024;
@@ -373,92 +372,60 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result<SkillMetadata, Ski
fn load_skill_interface(skill_path: &Path) -> Option<SkillInterface> { fn load_skill_interface(skill_path: &Path) -> Option<SkillInterface> {
// Fail open: optional interface metadata should not block loading SKILL.md. // Fail open: optional interface metadata should not block loading SKILL.md.
let skill_dir = skill_path.parent()?; let skill_dir = skill_path.parent()?;
let interface_paths = [ let interface_path = skill_dir.join(SKILLS_JSON_FILENAME);
(skill_dir.join(SKILLS_JSON_FILENAME), InterfaceFormat::Json), if !interface_path.exists() {
(skill_dir.join(SKILLS_TOML_FILENAME), InterfaceFormat::Toml), return None;
];
for (interface_path, format) in interface_paths {
if !interface_path.exists() {
continue;
}
let contents = match fs::read_to_string(&interface_path) {
Ok(contents) => contents,
Err(error) => {
tracing::warn!(
"ignoring {path}: failed to read {label}: {error}",
path = interface_path.display(),
label = format.label()
);
continue;
}
};
let parsed: SkillMetadataFile = match format.parse(&contents) {
Ok(parsed) => parsed,
Err(error) => {
tracing::warn!(
"ignoring {path}: invalid {label}: {error}",
path = interface_path.display(),
label = format.label()
);
continue;
}
};
let interface = parsed.interface?;
let interface = SkillInterface {
display_name: resolve_str(
interface.display_name,
MAX_NAME_LEN,
"interface.display_name",
),
short_description: resolve_str(
interface.short_description,
MAX_SHORT_DESCRIPTION_LEN,
"interface.short_description",
),
icon_small: resolve_asset_path(skill_dir, "interface.icon_small", interface.icon_small),
icon_large: resolve_asset_path(skill_dir, "interface.icon_large", interface.icon_large),
brand_color: resolve_color_str(interface.brand_color, "interface.brand_color"),
default_prompt: resolve_str(
interface.default_prompt,
MAX_DEFAULT_PROMPT_LEN,
"interface.default_prompt",
),
};
let has_fields = interface.display_name.is_some()
|| interface.short_description.is_some()
|| interface.icon_small.is_some()
|| interface.icon_large.is_some()
|| interface.brand_color.is_some()
|| interface.default_prompt.is_some();
return if has_fields { Some(interface) } else { None };
} }
None let contents = match fs::read_to_string(&interface_path) {
} Ok(contents) => contents,
Err(error) => {
#[derive(Clone, Copy)] tracing::warn!(
enum InterfaceFormat { "ignoring {path}: failed to read SKILL.json: {error}",
Json, path = interface_path.display()
Toml, );
} return None;
impl InterfaceFormat {
fn label(self) -> &'static str {
match self {
InterfaceFormat::Json => "SKILL.json",
InterfaceFormat::Toml => "SKILL.toml",
} }
} };
let parsed: SkillMetadataFile = match serde_json::from_str(&contents) {
fn parse(self, contents: &str) -> Result<SkillMetadataFile, String> { Ok(parsed) => parsed,
match self { Err(error) => {
InterfaceFormat::Json => serde_json::from_str(contents).map_err(|err| err.to_string()), tracing::warn!(
InterfaceFormat::Toml => toml::from_str(contents).map_err(|err| err.to_string()), "ignoring {path}: invalid JSON: {error}",
path = interface_path.display()
);
return None;
} }
} };
let interface = parsed.interface?;
let interface = SkillInterface {
display_name: resolve_str(
interface.display_name,
MAX_NAME_LEN,
"interface.display_name",
),
short_description: resolve_str(
interface.short_description,
MAX_SHORT_DESCRIPTION_LEN,
"interface.short_description",
),
icon_small: resolve_asset_path(skill_dir, "interface.icon_small", interface.icon_small),
icon_large: resolve_asset_path(skill_dir, "interface.icon_large", interface.icon_large),
brand_color: resolve_color_str(interface.brand_color, "interface.brand_color"),
default_prompt: resolve_str(
interface.default_prompt,
MAX_DEFAULT_PROMPT_LEN,
"interface.default_prompt",
),
};
let has_fields = interface.display_name.is_some()
|| interface.short_description.is_some()
|| interface.icon_small.is_some()
|| interface.icon_large.is_some()
|| interface.brand_color.is_some()
|| interface.default_prompt.is_some();
if has_fields { Some(interface) } else { None }
} }
fn resolve_asset_path( fn resolve_asset_path(
@@ -789,12 +756,6 @@ mod tests {
} }
fn write_skill_interface_at(skill_dir: &Path, contents: &str) -> PathBuf { fn write_skill_interface_at(skill_dir: &Path, contents: &str) -> PathBuf {
let path = skill_dir.join(SKILLS_TOML_FILENAME);
fs::write(&path, contents).unwrap();
path
}
fn write_skill_interface_json_at(skill_dir: &Path, contents: &str) -> PathBuf {
let path = skill_dir.join(SKILLS_JSON_FILENAME); let path = skill_dir.join(SKILLS_JSON_FILENAME);
fs::write(&path, contents).unwrap(); fs::write(&path, contents).unwrap();
path path
@@ -802,60 +763,12 @@ mod tests {
#[tokio::test] #[tokio::test]
async fn loads_skill_interface_metadata_happy_path() { async fn loads_skill_interface_metadata_happy_path() {
let codex_home = tempfile::tempdir().expect("tempdir");
let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml");
let skill_dir = skill_path.parent().expect("skill dir");
let normalized_skill_dir = normalized(skill_dir);
write_skill_interface_at(
skill_dir,
r##"
[interface]
display_name = "UI Skill"
short_description = " short desc "
icon_small = "./assets/small-400px.png"
icon_large = "./assets/large-logo.svg"
brand_color = "#3B82F6"
default_prompt = " default prompt "
"##,
);
let cfg = make_config(&codex_home).await;
let outcome = load_skills(&cfg);
assert!(
outcome.errors.is_empty(),
"unexpected errors: {:?}",
outcome.errors
);
assert_eq!(
outcome.skills,
vec![SkillMetadata {
name: "ui-skill".to_string(),
description: "from toml".to_string(),
short_description: None,
interface: Some(SkillInterface {
display_name: Some("UI Skill".to_string()),
short_description: Some("short desc".to_string()),
icon_small: Some(normalized_skill_dir.join("assets/small-400px.png")),
icon_large: Some(normalized_skill_dir.join("assets/large-logo.svg")),
brand_color: Some("#3B82F6".to_string()),
default_prompt: Some("default prompt".to_string()),
}),
path: normalized(&skill_path),
scope: SkillScope::User,
}]
);
}
#[tokio::test]
async fn loads_skill_interface_metadata_from_json() {
let codex_home = tempfile::tempdir().expect("tempdir"); let codex_home = tempfile::tempdir().expect("tempdir");
let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from json"); let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from json");
let skill_dir = skill_path.parent().expect("skill dir"); let skill_dir = skill_path.parent().expect("skill dir");
let normalized_skill_dir = normalized(skill_dir); let normalized_skill_dir = normalized(skill_dir);
write_skill_interface_json_at( write_skill_interface_at(
skill_dir, skill_dir,
r##" r##"
{ {
@@ -902,17 +815,20 @@ default_prompt = " default prompt "
#[tokio::test] #[tokio::test]
async fn accepts_icon_paths_under_assets_dir() { async fn accepts_icon_paths_under_assets_dir() {
let codex_home = tempfile::tempdir().expect("tempdir"); let codex_home = tempfile::tempdir().expect("tempdir");
let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml"); let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from json");
let skill_dir = skill_path.parent().expect("skill dir"); let skill_dir = skill_path.parent().expect("skill dir");
let normalized_skill_dir = normalized(skill_dir); let normalized_skill_dir = normalized(skill_dir);
write_skill_interface_at( write_skill_interface_at(
skill_dir, skill_dir,
r#" r#"
[interface] {
display_name = "UI Skill" "interface": {
icon_small = "assets/icon.png" "display_name": "UI Skill",
icon_large = "./assets/logo.svg" "icon_small": "assets/icon.png",
"icon_large": "./assets/logo.svg"
}
}
"#, "#,
); );
@@ -928,7 +844,7 @@ icon_large = "./assets/logo.svg"
outcome.skills, outcome.skills,
vec![SkillMetadata { vec![SkillMetadata {
name: "ui-skill".to_string(), name: "ui-skill".to_string(),
description: "from toml".to_string(), description: "from json".to_string(),
short_description: None, short_description: None,
interface: Some(SkillInterface { interface: Some(SkillInterface {
display_name: Some("UI Skill".to_string()), display_name: Some("UI Skill".to_string()),
@@ -947,14 +863,17 @@ icon_large = "./assets/logo.svg"
#[tokio::test] #[tokio::test]
async fn ignores_invalid_brand_color() { async fn ignores_invalid_brand_color() {
let codex_home = tempfile::tempdir().expect("tempdir"); let codex_home = tempfile::tempdir().expect("tempdir");
let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml"); let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from json");
let skill_dir = skill_path.parent().expect("skill dir"); let skill_dir = skill_path.parent().expect("skill dir");
write_skill_interface_at( write_skill_interface_at(
skill_dir, skill_dir,
r#" r#"
[interface] {
brand_color = "blue" "interface": {
"brand_color": "blue"
}
}
"#, "#,
); );
@@ -970,7 +889,7 @@ brand_color = "blue"
outcome.skills, outcome.skills,
vec![SkillMetadata { vec![SkillMetadata {
name: "ui-skill".to_string(), name: "ui-skill".to_string(),
description: "from toml".to_string(), description: "from json".to_string(),
short_description: None, short_description: None,
interface: None, interface: None,
path: normalized(&skill_path), path: normalized(&skill_path),
@@ -982,7 +901,7 @@ brand_color = "blue"
#[tokio::test] #[tokio::test]
async fn ignores_default_prompt_over_max_length() { async fn ignores_default_prompt_over_max_length() {
let codex_home = tempfile::tempdir().expect("tempdir"); let codex_home = tempfile::tempdir().expect("tempdir");
let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml"); let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from json");
let skill_dir = skill_path.parent().expect("skill dir"); let skill_dir = skill_path.parent().expect("skill dir");
let normalized_skill_dir = normalized(skill_dir); let normalized_skill_dir = normalized(skill_dir);
let too_long = "x".repeat(MAX_DEFAULT_PROMPT_LEN + 1); let too_long = "x".repeat(MAX_DEFAULT_PROMPT_LEN + 1);
@@ -991,10 +910,13 @@ brand_color = "blue"
skill_dir, skill_dir,
&format!( &format!(
r##" r##"
[interface] {{
display_name = "UI Skill" "interface": {{
icon_small = "./assets/small-400px.png" "display_name": "UI Skill",
default_prompt = "{too_long}" "icon_small": "./assets/small-400px.png",
"default_prompt": "{too_long}"
}}
}}
"## "##
), ),
); );
@@ -1011,7 +933,7 @@ default_prompt = "{too_long}"
outcome.skills, outcome.skills,
vec![SkillMetadata { vec![SkillMetadata {
name: "ui-skill".to_string(), name: "ui-skill".to_string(),
description: "from toml".to_string(), description: "from json".to_string(),
short_description: None, short_description: None,
interface: Some(SkillInterface { interface: Some(SkillInterface {
display_name: Some("UI Skill".to_string()), display_name: Some("UI Skill".to_string()),
@@ -1030,15 +952,18 @@ default_prompt = "{too_long}"
#[tokio::test] #[tokio::test]
async fn drops_interface_when_icons_are_invalid() { async fn drops_interface_when_icons_are_invalid() {
let codex_home = tempfile::tempdir().expect("tempdir"); let codex_home = tempfile::tempdir().expect("tempdir");
let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from toml"); let skill_path = write_skill(&codex_home, "demo", "ui-skill", "from json");
let skill_dir = skill_path.parent().expect("skill dir"); let skill_dir = skill_path.parent().expect("skill dir");
write_skill_interface_at( write_skill_interface_at(
skill_dir, skill_dir,
r#" r#"
[interface] {
icon_small = "icon.png" "interface": {
icon_large = "./assets/../logo.svg" "icon_small": "icon.png",
"icon_large": "./assets/../logo.svg"
}
}
"#, "#,
); );
@@ -1054,7 +979,7 @@ icon_large = "./assets/../logo.svg"
outcome.skills, outcome.skills,
vec![SkillMetadata { vec![SkillMetadata {
name: "ui-skill".to_string(), name: "ui-skill".to_string(),
description: "from toml".to_string(), description: "from json".to_string(),
short_description: None, short_description: None,
interface: None, interface: None,
path: normalized(&skill_path), path: normalized(&skill_path),