mirror of
https://github.com/openai/codex.git
synced 2026-10-19 17:28:46 +00:00
Parse model annotations from skill frontmatter (#38467)
## What changed - Add an optional `model` field to parsed and loaded skill metadata. - Recognize `model: luna` while ignoring unsupported model values without preventing the rest of the skill metadata from loading. ## Testing - Cover supported, absent, unsupported, and repaired frontmatter model values. GitOrigin-RevId: 376b65555e81d29a5eae93cc4c2633aacf7cd7c8
This commit is contained in:
@@ -187,6 +187,7 @@ fn load_test_skill_root(root: &PluginSkillRoot) -> LoadedSkillRoot {
|
||||
name: format!("{}:{}", root.plugin_namespace, parsed.name),
|
||||
description: parsed.description,
|
||||
short_description: parsed.short_description,
|
||||
model: parsed.model,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
|
||||
@@ -24,6 +24,7 @@ fn host_interface_metadata_makes_unlisted_workflow_discoverable() {
|
||||
name: "content-tools".to_string(),
|
||||
description: "Prepare visual content.".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: Some(SkillInterface {
|
||||
display_name: Some("Visual Assets".to_string()),
|
||||
short_description: Some("Create illustrated assets.".to_string()),
|
||||
|
||||
@@ -225,6 +225,7 @@ fn expected_skill(path: AbsolutePathBuf, name: &str, scope: SkillScope) -> Skill
|
||||
name: name.to_string(),
|
||||
description: format!("{name} description"),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
|
||||
@@ -324,6 +324,7 @@ async fn snapshot_for_config_preserves_host_precedence_for_symlinked_plugin_root
|
||||
name: "sample:search".to_string(),
|
||||
description: "shared skill".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
|
||||
@@ -72,6 +72,7 @@ impl ParsedEnvironmentSkill {
|
||||
name: base_name,
|
||||
description,
|
||||
short_description,
|
||||
model: _,
|
||||
} = parse_skill_frontmatter_metadata(&contents, || default_skill_name(&skill.path))
|
||||
.map_err(|err| err.to_string())?;
|
||||
let (dependencies, policy) = match &skill.metadata {
|
||||
@@ -240,6 +241,7 @@ pub fn load_environment_skills_from_discovery(
|
||||
name: base_name,
|
||||
description,
|
||||
short_description,
|
||||
model: _,
|
||||
} = match parse_skill_frontmatter_metadata(&skill.instructions.contents, || {
|
||||
default_skill_name(&skill.instructions.path)
|
||||
}) {
|
||||
|
||||
@@ -356,6 +356,7 @@ async fn parse_skill_file(
|
||||
name,
|
||||
description,
|
||||
short_description,
|
||||
model,
|
||||
} = parse_skill_frontmatter_metadata(&contents, || default_skill_name(path))
|
||||
.map_err(|error| error.to_string())?;
|
||||
let LoadedSkillMetadata {
|
||||
@@ -368,6 +369,7 @@ async fn parse_skill_file(
|
||||
name,
|
||||
description,
|
||||
short_description,
|
||||
model,
|
||||
interface,
|
||||
dependencies,
|
||||
policy,
|
||||
|
||||
@@ -84,6 +84,7 @@ async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() {
|
||||
name: "linked:first".to_string(),
|
||||
description: "first skill.".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: Some(SkillPolicy {
|
||||
@@ -99,6 +100,7 @@ async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() {
|
||||
name: "linked:second".to_string(),
|
||||
description: "second skill.".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
|
||||
@@ -7,6 +7,7 @@ use codex_protocol::protocol::SkillScope;
|
||||
use codex_skills::SkillDependencies;
|
||||
use codex_skills::SkillInterface;
|
||||
use codex_skills::SkillMetadata;
|
||||
use codex_skills::SkillModel;
|
||||
use codex_skills::SkillPolicy;
|
||||
use codex_skills::SkillToolDependency;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -107,7 +108,7 @@ async fn loads_host_frontmatter_dependencies_and_policy() {
|
||||
let skill_path = write_skill(
|
||||
&root,
|
||||
"demo",
|
||||
"name: demo\ndescription: Demo skill\nmetadata:\n short-description: Short demo",
|
||||
"name: demo\ndescription: Demo skill\nmodel: luna\nmetadata:\n short-description: Short demo",
|
||||
);
|
||||
write_metadata(
|
||||
&root,
|
||||
@@ -134,6 +135,7 @@ policy:
|
||||
name: "demo".to_string(),
|
||||
description: "Demo skill".to_string(),
|
||||
short_description: Some("Short demo".to_string()),
|
||||
model: Some(SkillModel::Luna),
|
||||
interface: None,
|
||||
dependencies: Some(SkillDependencies {
|
||||
tools: vec![SkillToolDependency {
|
||||
@@ -173,6 +175,7 @@ async fn invalid_optional_metadata_fails_open() {
|
||||
name: "demo".to_string(),
|
||||
description: "Demo skill".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
@@ -212,6 +215,7 @@ async fn loads_host_interface_metadata_and_local_asset_paths() {
|
||||
name: "demo".to_string(),
|
||||
description: "Demo skill".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: Some(SkillInterface {
|
||||
display_name: Some("Demo".to_string()),
|
||||
short_description: Some("Interface summary".to_string()),
|
||||
@@ -251,6 +255,7 @@ async fn loads_plugin_skill_interface_icons_from_local_and_shared_assets() {
|
||||
name: "plugin:send-message".to_string(),
|
||||
description: "Send messages".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: Some(SkillInterface {
|
||||
display_name: None,
|
||||
short_description: None,
|
||||
@@ -293,6 +298,7 @@ async fn rejects_plugin_skill_interface_icons_outside_shared_assets() {
|
||||
name: "plugin:send-message".to_string(),
|
||||
description: "Send messages".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: Some(SkillInterface {
|
||||
display_name: Some("Send Message".to_string()),
|
||||
short_description: None,
|
||||
@@ -330,6 +336,7 @@ async fn rejects_interface_fields_that_escape_or_fail_validation() {
|
||||
name: "demo".to_string(),
|
||||
description: "Demo skill".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
@@ -380,6 +387,7 @@ async fn discovers_nested_plugin_namespace_without_plugin_identity() {
|
||||
name: "plugin-name:demo".to_string(),
|
||||
description: "Demo skill".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
@@ -425,6 +433,7 @@ async fn plugin_root_accepts_maximum_length_qualified_skill_name() {
|
||||
name: format!("{plugin_namespace}:{skill_name}"),
|
||||
description: "Search skill".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
@@ -519,6 +528,7 @@ async fn recursive_plugin_root_preserves_owner_namespace_and_shared_asset_policy
|
||||
name: "plugin:demo".to_string(),
|
||||
description: "Demo skill".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: Some(SkillInterface {
|
||||
display_name: None,
|
||||
short_description: None,
|
||||
@@ -578,6 +588,7 @@ async fn direct_child_plugin_root_ignores_nested_skills() {
|
||||
name: "plugin:direct".to_string(),
|
||||
description: "Direct skill".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
@@ -659,6 +670,7 @@ async fn recursive_plugin_root_preserves_symlinked_skill_discovery_path() {
|
||||
name: "plugin:demo".to_string(),
|
||||
description: "Symlinked skill".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
|
||||
@@ -171,6 +171,7 @@ async fn installed_extension_uses_host_service_snapshot() -> TestResult {
|
||||
name: "demo".to_string(),
|
||||
description: "Demo skill.".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
@@ -252,6 +253,7 @@ async fn host_world_state_records_catalog_metrics_on_publish_and_change() -> Tes
|
||||
name: "demo".to_string(),
|
||||
description: "Demo skill.".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
@@ -316,6 +318,7 @@ async fn host_world_state_records_catalog_metrics_on_publish_and_change() -> Tes
|
||||
name: "other".to_string(),
|
||||
description: "Other skill.".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
@@ -368,6 +371,7 @@ async fn persisted_host_snapshot_deduplicates_warning_after_reinitialization() -
|
||||
name: "demo".to_string(),
|
||||
description: "Demo skill.".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
|
||||
@@ -28,6 +28,7 @@ fn test_skill_metadata(skill_doc_path: AbsolutePathBuf) -> SkillMetadata {
|
||||
name: "test-skill".to_string(),
|
||||
description: "test".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
|
||||
@@ -3,6 +3,7 @@ mod invocation;
|
||||
mod loading;
|
||||
mod mentions;
|
||||
mod model;
|
||||
mod model_delegation;
|
||||
mod name_counts;
|
||||
mod parser;
|
||||
mod selection;
|
||||
@@ -36,6 +37,7 @@ pub use model::SkillInterface;
|
||||
pub use model::SkillMetadata;
|
||||
pub use model::SkillPolicy;
|
||||
pub use model::SkillToolDependency;
|
||||
pub use model_delegation::SkillModel;
|
||||
pub use name_counts::build_skill_name_counts;
|
||||
pub use parser::ParsedSkillFrontmatter;
|
||||
pub use parser::SkillParseError;
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
use crate::SkillModel;
|
||||
use codex_protocol::protocol::Product;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
@@ -9,6 +10,8 @@ pub struct SkillMetadata {
|
||||
pub name: String,
|
||||
pub description: String,
|
||||
pub short_description: Option<String>,
|
||||
/// Optional model requested by this skill's frontmatter.
|
||||
pub model: Option<SkillModel>,
|
||||
pub interface: Option<SkillInterface>,
|
||||
pub dependencies: Option<SkillDependencies>,
|
||||
pub policy: Option<SkillPolicy>,
|
||||
|
||||
@@ -0,0 +1,12 @@
|
||||
use serde::Deserialize;
|
||||
|
||||
/// Model requested for work governed by a skill.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
pub enum SkillModel {
|
||||
Luna,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "model_delegation_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,59 @@
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::SkillModel;
|
||||
use crate::ParsedSkillFrontmatter;
|
||||
use crate::parse_skill_frontmatter_metadata;
|
||||
|
||||
#[test]
|
||||
fn parses_supported_luna_skill_model() {
|
||||
let contents = "---\nname: demo\ndescription: Demo skill\nmodel: luna\n---\n";
|
||||
|
||||
assert_eq!(
|
||||
parse_skill_frontmatter_metadata(contents, || "fallback".to_string())
|
||||
.unwrap()
|
||||
.model,
|
||||
Some(SkillModel::Luna)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn permits_skills_without_a_model_annotation() {
|
||||
let contents = "---\nname: demo\ndescription: Demo skill\n---\n";
|
||||
|
||||
assert_eq!(
|
||||
parse_skill_frontmatter_metadata(contents, || "fallback".to_string())
|
||||
.unwrap()
|
||||
.model,
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_unsupported_model_without_disabling_ordinary_skill_loading() {
|
||||
for unsupported_model in ["terra", "sol", "unknown"] {
|
||||
let contents =
|
||||
format!("---\nname: demo\ndescription: Demo skill\nmodel: {unsupported_model}\n---\n");
|
||||
|
||||
assert_eq!(
|
||||
parse_skill_frontmatter_metadata(&contents, || "fallback".to_string()).unwrap(),
|
||||
ParsedSkillFrontmatter {
|
||||
name: "demo".to_string(),
|
||||
description: "Demo skill".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reuses_existing_frontmatter_scalar_repair() {
|
||||
let contents = "---\nname: demo\ndescription: Build for AWS: ECS\nmodel: luna\n---\n";
|
||||
|
||||
assert_eq!(
|
||||
parse_skill_frontmatter_metadata(contents, || "fallback".to_string())
|
||||
.unwrap()
|
||||
.model,
|
||||
Some(SkillModel::Luna)
|
||||
);
|
||||
}
|
||||
@@ -1,3 +1,4 @@
|
||||
use crate::SkillModel;
|
||||
use serde::Deserialize;
|
||||
use thiserror::Error;
|
||||
|
||||
@@ -10,6 +11,8 @@ struct SkillFrontmatter {
|
||||
#[serde(default)]
|
||||
description: Option<String>,
|
||||
#[serde(default)]
|
||||
model: Option<serde_yaml::Value>,
|
||||
#[serde(default)]
|
||||
metadata: SkillFrontmatterMetadata,
|
||||
}
|
||||
|
||||
@@ -25,6 +28,7 @@ pub struct ParsedSkillFrontmatter {
|
||||
pub name: String,
|
||||
pub description: String,
|
||||
pub short_description: Option<String>,
|
||||
pub model: Option<SkillModel>,
|
||||
}
|
||||
|
||||
/// Error produced while parsing or validating `SKILL.md` metadata.
|
||||
@@ -88,6 +92,13 @@ pub fn parse_skill_frontmatter_metadata(
|
||||
name,
|
||||
description,
|
||||
short_description,
|
||||
model: parsed.model.and_then(|model| {
|
||||
serde_yaml::from_value(model)
|
||||
.inspect_err(|error| {
|
||||
tracing::warn!(%error, "ignoring invalid skill model annotation");
|
||||
})
|
||||
.ok()
|
||||
}),
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -17,6 +17,7 @@ fn parses_repairs_and_sanitizes_frontmatter() {
|
||||
name: "deploy service".to_string(),
|
||||
description: "Build for AWS: ECS".to_string(),
|
||||
short_description: Some("Deploy safely".to_string()),
|
||||
model: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -33,6 +34,7 @@ fn uses_default_name_and_requires_description() {
|
||||
name: "demo".to_string(),
|
||||
description: "Demo skill".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
}
|
||||
);
|
||||
|
||||
@@ -56,6 +58,7 @@ fn repairs_short_descriptions_containing_colons_and_apostrophes() {
|
||||
name: "short".to_string(),
|
||||
description: "Short skill".to_string(),
|
||||
short_description: Some("What's included: builds and tests".to_string()),
|
||||
model: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -74,6 +77,7 @@ fn repairs_unrecognized_frontmatter_fields_that_need_quotes() {
|
||||
name: "unknown".to_string(),
|
||||
description: "Unknown fields".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -92,6 +96,7 @@ fn preserves_block_scalar_bodies_while_repairing_other_fields() {
|
||||
name: "block".to_string(),
|
||||
description: "Build for AWS: ECS".to_string(),
|
||||
short_description: None,
|
||||
model: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
@@ -114,6 +119,7 @@ fn preserves_overlong_descriptions_and_short_descriptions() {
|
||||
name: "long".to_string(),
|
||||
description,
|
||||
short_description: Some(short_description),
|
||||
model: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
@@ -32,6 +32,7 @@ fn make_skill(name: &str, path: &str) -> SkillMetadata {
|
||||
name: name.to_string(),
|
||||
description: format!("{name} skill"),
|
||||
short_description: None,
|
||||
model: None,
|
||||
interface: None,
|
||||
dependencies: None,
|
||||
policy: None,
|
||||
|
||||
Reference in New Issue
Block a user