Skip to content

Remove load from SKILL.toml fallback #10007

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alexsong-oai merged 2 commits into from
Jan 27, 2026
+84 −159
Merged

Remove load from SKILL.toml fallback #10007

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5eafecf
Remove load from SKILL.toml fallback
alexsong-oai Jan 27, 2026
28253fa
minor
alexsong-oai Jan 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
243 changes: 84 additions & 159 deletions codex-rs/core/src/skills/loader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ struct Interface {

const SKILLS_FILENAME: &str = "SKILL.md";
const SKILLS_JSON_FILENAME: &str = "SKILL.json";
const SKILLS_TOML_FILENAME: &str = "SKILL.toml";
const SKILLS_DIR_NAME: &str = "skills";
const MAX_NAME_LEN: usize = 64;
const MAX_DESCRIPTION_LEN: usize = 1024;
Expand Down Expand Up @@ -373,92 +372,60 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result<SkillMetadata, Ski
fn load_skill_interface(skill_path: &Path) -> Option<SkillInterface> {
// Fail open: optional interface metadata should not block loading SKILL.md.
let skill_dir = skill_path.parent()?;
let interface_paths = [
(skill_dir.join(SKILLS_JSON_FILENAME), InterfaceFormat::Json),
(skill_dir.join(SKILLS_TOML_FILENAME), InterfaceFormat::Toml),
];

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 };
let interface_path = skill_dir.join(SKILLS_JSON_FILENAME);
if !interface_path.exists() {
return None;
Comment on lines +375 to +377
Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve legacy SKILL.toml interface metadata

Because load_skill_interface now returns None whenever SKILL.json is missing, any skill that still publishes interface metadata only in SKILL.toml (the legacy format still referenced in the protocol docs) will silently lose display_name, icons, and default_prompt, so clients/UI regress to bare SKILL.md metadata. If TOML-based skills are still in circulation, this change is a backward‑compat break unless you migrate them or keep a fallback reader.

Useful? React with 👍 / 👎.

}

None
}

#[derive(Clone, Copy)]
enum InterfaceFormat {
Json,
Toml,
}

impl InterfaceFormat {
fn label(self) -> &'static str {
match self {
InterfaceFormat::Json => "SKILL.json",
InterfaceFormat::Toml => "SKILL.toml",
let contents = match fs::read_to_string(&interface_path) {
Ok(contents) => contents,
Err(error) => {
tracing::warn!(
"ignoring {path}: failed to read SKILL.json: {error}",
path = interface_path.display()
);
return None;
}
}

fn parse(self, contents: &str) -> Result<SkillMetadataFile, String> {
match self {
InterfaceFormat::Json => serde_json::from_str(contents).map_err(|err| err.to_string()),
InterfaceFormat::Toml => toml::from_str(contents).map_err(|err| err.to_string()),
};
let parsed: SkillMetadataFile = match serde_json::from_str(&contents) {
Ok(parsed) => parsed,
Err(error) => {
tracing::warn!(
"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(
Expand Down Expand Up @@ -789,73 +756,19 @@ mod tests {
}

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);
fs::write(&path, contents).unwrap();
path
}

#[tokio::test]
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 skill_path = write_skill(&codex_home, "demo", "ui-skill", "from json");
let skill_dir = skill_path.parent().expect("skill dir");
let normalized_skill_dir = normalized(skill_dir);

write_skill_interface_json_at(
write_skill_interface_at(
skill_dir,
r##"
{
Expand Down Expand Up @@ -902,17 +815,20 @@ default_prompt = " default prompt "
#[tokio::test]
async fn accepts_icon_paths_under_assets_dir() {
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 normalized_skill_dir = normalized(skill_dir);

write_skill_interface_at(
skill_dir,
r#"
[interface]
display_name = "UI Skill"
icon_small = "assets/icon.png"
icon_large = "./assets/logo.svg"
{
"interface": {
"display_name": "UI Skill",
"icon_small": "assets/icon.png",
"icon_large": "./assets/logo.svg"
}
}
"#,
);

Expand All @@ -928,7 +844,7 @@ icon_large = "./assets/logo.svg"
outcome.skills,
vec![SkillMetadata {
name: "ui-skill".to_string(),
description: "from toml".to_string(),
description: "from json".to_string(),
short_description: None,
interface: Some(SkillInterface {
display_name: Some("UI Skill".to_string()),
Expand All @@ -947,14 +863,17 @@ icon_large = "./assets/logo.svg"
#[tokio::test]
async fn ignores_invalid_brand_color() {
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");

write_skill_interface_at(
skill_dir,
r#"
[interface]
brand_color = "blue"
{
"interface": {
"brand_color": "blue"
}
}
"#,
);

Expand All @@ -970,7 +889,7 @@ brand_color = "blue"
outcome.skills,
vec![SkillMetadata {
name: "ui-skill".to_string(),
description: "from toml".to_string(),
description: "from json".to_string(),
short_description: None,
interface: None,
path: normalized(&skill_path),
Expand All @@ -982,7 +901,7 @@ brand_color = "blue"
#[tokio::test]
async fn ignores_default_prompt_over_max_length() {
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 normalized_skill_dir = normalized(skill_dir);
let too_long = "x".repeat(MAX_DEFAULT_PROMPT_LEN + 1);
Expand All @@ -991,10 +910,13 @@ brand_color = "blue"
skill_dir,
&format!(
r##"
[interface]
display_name = "UI Skill"
icon_small = "./assets/small-400px.png"
default_prompt = "{too_long}"
{{
"interface": {{
"display_name": "UI Skill",
"icon_small": "./assets/small-400px.png",
"default_prompt": "{too_long}"
}}
}}
"##
),
);
Expand All @@ -1011,7 +933,7 @@ default_prompt = "{too_long}"
outcome.skills,
vec![SkillMetadata {
name: "ui-skill".to_string(),
description: "from toml".to_string(),
description: "from json".to_string(),
short_description: None,
interface: Some(SkillInterface {
display_name: Some("UI Skill".to_string()),
Expand All @@ -1030,15 +952,18 @@ default_prompt = "{too_long}"
#[tokio::test]
async fn drops_interface_when_icons_are_invalid() {
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");

write_skill_interface_at(
skill_dir,
r#"
[interface]
icon_small = "icon.png"
icon_large = "./assets/../logo.svg"
{
"interface": {
"icon_small": "icon.png",
"icon_large": "./assets/../logo.svg"
}
}
"#,
);

Expand All @@ -1054,7 +979,7 @@ icon_large = "./assets/../logo.svg"
outcome.skills,
vec![SkillMetadata {
name: "ui-skill".to_string(),
description: "from toml".to_string(),
description: "from json".to_string(),
short_description: None,
interface: None,
path: normalized(&skill_path),
Expand Down
Loading