mirror of
https://github.com/openai/codex.git
synced 2026-11-10 16:44:38 +00:00
Remove skill model delegation support (#39068)
## What changed - Stop parsing and exposing the `model` field from skill frontmatter. - Remove the skill model delegation types and instruction generation. GitOrigin-RevId: 01198c68c095da5062e8abec417cc3539099d9d5
This commit is contained in:
1 parent
51a9edc083
commit
d24507a59b
17 files changed
+1
-360
No files matched your search
@@ -187,7 +187,6 @@ fn load_test_skill_root(root: &PluginSkillRoot) -> LoadedSkillRoot {
|
|||||||
name: format!("{}:{}", root.plugin_namespace, parsed.name),
|
name: format!("{}:{}", root.plugin_namespace, parsed.name),
|
||||||
description: parsed.description,
|
description: parsed.description,
|
||||||
short_description: parsed.short_description,
|
short_description: parsed.short_description,
|
||||||
model: parsed.model,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
|
|||||||
@@ -24,7 +24,6 @@ fn host_interface_metadata_makes_unlisted_workflow_discoverable() {
|
|||||||
name: "content-tools".to_string(),
|
name: "content-tools".to_string(),
|
||||||
description: "Prepare visual content.".to_string(),
|
description: "Prepare visual content.".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: Some(SkillInterface {
|
interface: Some(SkillInterface {
|
||||||
display_name: Some("Visual Assets".to_string()),
|
display_name: Some("Visual Assets".to_string()),
|
||||||
short_description: Some("Create illustrated assets.".to_string()),
|
short_description: Some("Create illustrated assets.".to_string()),
|
||||||
|
|||||||
@@ -225,7 +225,6 @@ fn expected_skill(path: AbsolutePathBuf, name: &str, scope: SkillScope) -> Skill
|
|||||||
name: name.to_string(),
|
name: name.to_string(),
|
||||||
description: format!("{name} description"),
|
description: format!("{name} description"),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
|
|||||||
@@ -324,7 +324,6 @@ async fn snapshot_for_config_preserves_host_precedence_for_symlinked_plugin_root
|
|||||||
name: "sample:search".to_string(),
|
name: "sample:search".to_string(),
|
||||||
description: "shared skill".to_string(),
|
description: "shared skill".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
|
|||||||
@@ -72,7 +72,6 @@ impl ParsedEnvironmentSkill {
|
|||||||
name: base_name,
|
name: base_name,
|
||||||
description,
|
description,
|
||||||
short_description,
|
short_description,
|
||||||
model: _,
|
|
||||||
} = parse_skill_frontmatter_metadata(&contents, || default_skill_name(&skill.path))
|
} = parse_skill_frontmatter_metadata(&contents, || default_skill_name(&skill.path))
|
||||||
.map_err(|err| err.to_string())?;
|
.map_err(|err| err.to_string())?;
|
||||||
let (dependencies, policy) = match &skill.metadata {
|
let (dependencies, policy) = match &skill.metadata {
|
||||||
@@ -241,7 +240,6 @@ pub fn load_environment_skills_from_discovery(
|
|||||||
name: base_name,
|
name: base_name,
|
||||||
description,
|
description,
|
||||||
short_description,
|
short_description,
|
||||||
model: _,
|
|
||||||
} = match parse_skill_frontmatter_metadata(&skill.instructions.contents, || {
|
} = match parse_skill_frontmatter_metadata(&skill.instructions.contents, || {
|
||||||
default_skill_name(&skill.instructions.path)
|
default_skill_name(&skill.instructions.path)
|
||||||
}) {
|
}) {
|
||||||
|
|||||||
@@ -356,7 +356,6 @@ async fn parse_skill_file(
|
|||||||
name,
|
name,
|
||||||
description,
|
description,
|
||||||
short_description,
|
short_description,
|
||||||
model,
|
|
||||||
} = parse_skill_frontmatter_metadata(&contents, || default_skill_name(path))
|
} = parse_skill_frontmatter_metadata(&contents, || default_skill_name(path))
|
||||||
.map_err(|error| error.to_string())?;
|
.map_err(|error| error.to_string())?;
|
||||||
let LoadedSkillMetadata {
|
let LoadedSkillMetadata {
|
||||||
@@ -369,7 +368,6 @@ async fn parse_skill_file(
|
|||||||
name,
|
name,
|
||||||
description,
|
description,
|
||||||
short_description,
|
short_description,
|
||||||
model,
|
|
||||||
interface,
|
interface,
|
||||||
dependencies,
|
dependencies,
|
||||||
policy,
|
policy,
|
||||||
|
|||||||
@@ -84,7 +84,6 @@ async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() {
|
|||||||
name: "linked:first".to_string(),
|
name: "linked:first".to_string(),
|
||||||
description: "first skill.".to_string(),
|
description: "first skill.".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: Some(SkillPolicy {
|
policy: Some(SkillPolicy {
|
||||||
@@ -100,7 +99,6 @@ async fn host_loading_reuses_walk_inventory_for_symlinked_skill_pack() {
|
|||||||
name: "linked:second".to_string(),
|
name: "linked:second".to_string(),
|
||||||
description: "second skill.".to_string(),
|
description: "second skill.".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
|
|||||||
@@ -7,7 +7,6 @@ use codex_protocol::protocol::SkillScope;
|
|||||||
use codex_skills::SkillDependencies;
|
use codex_skills::SkillDependencies;
|
||||||
use codex_skills::SkillInterface;
|
use codex_skills::SkillInterface;
|
||||||
use codex_skills::SkillMetadata;
|
use codex_skills::SkillMetadata;
|
||||||
use codex_skills::SkillModel;
|
|
||||||
use codex_skills::SkillPolicy;
|
use codex_skills::SkillPolicy;
|
||||||
use codex_skills::SkillToolDependency;
|
use codex_skills::SkillToolDependency;
|
||||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||||
@@ -108,7 +107,7 @@ async fn loads_host_frontmatter_dependencies_and_policy() {
|
|||||||
let skill_path = write_skill(
|
let skill_path = write_skill(
|
||||||
&root,
|
&root,
|
||||||
"demo",
|
"demo",
|
||||||
"name: demo\ndescription: Demo skill\nmodel: luna\nmetadata:\n short-description: Short demo",
|
"name: demo\ndescription: Demo skill\nmetadata:\n short-description: Short demo",
|
||||||
);
|
);
|
||||||
write_metadata(
|
write_metadata(
|
||||||
&root,
|
&root,
|
||||||
@@ -135,7 +134,6 @@ policy:
|
|||||||
name: "demo".to_string(),
|
name: "demo".to_string(),
|
||||||
description: "Demo skill".to_string(),
|
description: "Demo skill".to_string(),
|
||||||
short_description: Some("Short demo".to_string()),
|
short_description: Some("Short demo".to_string()),
|
||||||
model: Some(SkillModel::Luna),
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: Some(SkillDependencies {
|
dependencies: Some(SkillDependencies {
|
||||||
tools: vec![SkillToolDependency {
|
tools: vec![SkillToolDependency {
|
||||||
@@ -175,7 +173,6 @@ async fn invalid_optional_metadata_fails_open() {
|
|||||||
name: "demo".to_string(),
|
name: "demo".to_string(),
|
||||||
description: "Demo skill".to_string(),
|
description: "Demo skill".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
@@ -215,7 +212,6 @@ async fn loads_host_interface_metadata_and_local_asset_paths() {
|
|||||||
name: "demo".to_string(),
|
name: "demo".to_string(),
|
||||||
description: "Demo skill".to_string(),
|
description: "Demo skill".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: Some(SkillInterface {
|
interface: Some(SkillInterface {
|
||||||
display_name: Some("Demo".to_string()),
|
display_name: Some("Demo".to_string()),
|
||||||
short_description: Some("Interface summary".to_string()),
|
short_description: Some("Interface summary".to_string()),
|
||||||
@@ -255,7 +251,6 @@ async fn loads_plugin_skill_interface_icons_from_local_and_shared_assets() {
|
|||||||
name: "plugin:send-message".to_string(),
|
name: "plugin:send-message".to_string(),
|
||||||
description: "Send messages".to_string(),
|
description: "Send messages".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: Some(SkillInterface {
|
interface: Some(SkillInterface {
|
||||||
display_name: None,
|
display_name: None,
|
||||||
short_description: None,
|
short_description: None,
|
||||||
@@ -298,7 +293,6 @@ async fn rejects_plugin_skill_interface_icons_outside_shared_assets() {
|
|||||||
name: "plugin:send-message".to_string(),
|
name: "plugin:send-message".to_string(),
|
||||||
description: "Send messages".to_string(),
|
description: "Send messages".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: Some(SkillInterface {
|
interface: Some(SkillInterface {
|
||||||
display_name: Some("Send Message".to_string()),
|
display_name: Some("Send Message".to_string()),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
@@ -336,7 +330,6 @@ async fn rejects_interface_fields_that_escape_or_fail_validation() {
|
|||||||
name: "demo".to_string(),
|
name: "demo".to_string(),
|
||||||
description: "Demo skill".to_string(),
|
description: "Demo skill".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
@@ -387,7 +380,6 @@ async fn discovers_nested_plugin_namespace_without_plugin_identity() {
|
|||||||
name: "plugin-name:demo".to_string(),
|
name: "plugin-name:demo".to_string(),
|
||||||
description: "Demo skill".to_string(),
|
description: "Demo skill".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
@@ -433,7 +425,6 @@ async fn plugin_root_accepts_maximum_length_qualified_skill_name() {
|
|||||||
name: format!("{plugin_namespace}:{skill_name}"),
|
name: format!("{plugin_namespace}:{skill_name}"),
|
||||||
description: "Search skill".to_string(),
|
description: "Search skill".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
@@ -528,7 +519,6 @@ async fn recursive_plugin_root_preserves_owner_namespace_and_shared_asset_policy
|
|||||||
name: "plugin:demo".to_string(),
|
name: "plugin:demo".to_string(),
|
||||||
description: "Demo skill".to_string(),
|
description: "Demo skill".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: Some(SkillInterface {
|
interface: Some(SkillInterface {
|
||||||
display_name: None,
|
display_name: None,
|
||||||
short_description: None,
|
short_description: None,
|
||||||
@@ -588,7 +578,6 @@ async fn direct_child_plugin_root_ignores_nested_skills() {
|
|||||||
name: "plugin:direct".to_string(),
|
name: "plugin:direct".to_string(),
|
||||||
description: "Direct skill".to_string(),
|
description: "Direct skill".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
@@ -670,7 +659,6 @@ async fn recursive_plugin_root_preserves_symlinked_skill_discovery_path() {
|
|||||||
name: "plugin:demo".to_string(),
|
name: "plugin:demo".to_string(),
|
||||||
description: "Symlinked skill".to_string(),
|
description: "Symlinked skill".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
|
|||||||
@@ -175,7 +175,6 @@ async fn installed_extension_uses_host_service_snapshot() -> TestResult {
|
|||||||
name: "demo".to_string(),
|
name: "demo".to_string(),
|
||||||
description: "Demo skill.".to_string(),
|
description: "Demo skill.".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
@@ -257,7 +256,6 @@ async fn host_world_state_records_catalog_metrics_on_publish_and_change() -> Tes
|
|||||||
name: "demo".to_string(),
|
name: "demo".to_string(),
|
||||||
description: "Demo skill.".to_string(),
|
description: "Demo skill.".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
@@ -322,7 +320,6 @@ async fn host_world_state_records_catalog_metrics_on_publish_and_change() -> Tes
|
|||||||
name: "other".to_string(),
|
name: "other".to_string(),
|
||||||
description: "Other skill.".to_string(),
|
description: "Other skill.".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
@@ -375,7 +372,6 @@ async fn persisted_host_snapshot_deduplicates_warning_after_reinitialization() -
|
|||||||
name: "demo".to_string(),
|
name: "demo".to_string(),
|
||||||
description: "Demo skill.".to_string(),
|
description: "Demo skill.".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
|
|||||||
@@ -28,7 +28,6 @@ fn test_skill_metadata(skill_doc_path: AbsolutePathBuf) -> SkillMetadata {
|
|||||||
name: "test-skill".to_string(),
|
name: "test-skill".to_string(),
|
||||||
description: "test".to_string(),
|
description: "test".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
|
|||||||
@@ -3,7 +3,6 @@ mod invocation;
|
|||||||
mod loading;
|
mod loading;
|
||||||
mod mentions;
|
mod mentions;
|
||||||
mod model;
|
mod model;
|
||||||
mod model_delegation;
|
|
||||||
mod name_counts;
|
mod name_counts;
|
||||||
mod parser;
|
mod parser;
|
||||||
mod selection;
|
mod selection;
|
||||||
@@ -37,8 +36,6 @@ pub use model::SkillInterface;
|
|||||||
pub use model::SkillMetadata;
|
pub use model::SkillMetadata;
|
||||||
pub use model::SkillPolicy;
|
pub use model::SkillPolicy;
|
||||||
pub use model::SkillToolDependency;
|
pub use model::SkillToolDependency;
|
||||||
pub use model_delegation::SkillModel;
|
|
||||||
pub use model_delegation::SkillModelDelegationInstruction;
|
|
||||||
pub use name_counts::build_skill_name_counts;
|
pub use name_counts::build_skill_name_counts;
|
||||||
pub use parser::ParsedSkillFrontmatter;
|
pub use parser::ParsedSkillFrontmatter;
|
||||||
pub use parser::SkillParseError;
|
pub use parser::SkillParseError;
|
||||||
|
|||||||
@@ -1,4 +1,3 @@
|
|||||||
use crate::SkillModel;
|
|
||||||
use codex_protocol::protocol::Product;
|
use codex_protocol::protocol::Product;
|
||||||
use codex_protocol::protocol::SkillScope;
|
use codex_protocol::protocol::SkillScope;
|
||||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||||
@@ -10,8 +9,6 @@ pub struct SkillMetadata {
|
|||||||
pub name: String,
|
pub name: String,
|
||||||
pub description: String,
|
pub description: String,
|
||||||
pub short_description: Option<String>,
|
pub short_description: Option<String>,
|
||||||
/// Optional model requested by this skill's frontmatter.
|
|
||||||
pub model: Option<SkillModel>,
|
|
||||||
pub interface: Option<SkillInterface>,
|
pub interface: Option<SkillInterface>,
|
||||||
pub dependencies: Option<SkillDependencies>,
|
pub dependencies: Option<SkillDependencies>,
|
||||||
pub policy: Option<SkillPolicy>,
|
pub policy: Option<SkillPolicy>,
|
||||||
|
|||||||
@@ -1,84 +0,0 @@
|
|||||||
use serde::Deserialize;
|
|
||||||
|
|
||||||
const LUNA_MODEL: &str = "gpt-5.6-luna";
|
|
||||||
const TERRA_MODEL: &str = "gpt-5.6-terra";
|
|
||||||
const SOL_MODEL: &str = "gpt-5.6-sol";
|
|
||||||
const MAX_TARGET_MODEL_BYTES: usize = 128;
|
|
||||||
const MAX_DELEGATION_INSTRUCTION_BYTES: usize = 2_048;
|
|
||||||
|
|
||||||
/// Model requested for work governed by a skill.
|
|
||||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Deserialize)]
|
|
||||||
#[serde(rename_all = "lowercase")]
|
|
||||||
pub enum SkillModel {
|
|
||||||
Luna,
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Bounded instructions for delegating skill-governed work to a cheaper model.
|
|
||||||
#[derive(Clone, Debug, Eq, PartialEq)]
|
|
||||||
pub struct SkillModelDelegationInstruction(String);
|
|
||||||
|
|
||||||
impl SkillModelDelegationInstruction {
|
|
||||||
/// Builds bounded instructions only when the indexed skill requests an available lower-tier model.
|
|
||||||
pub fn from_skill_model(
|
|
||||||
skill_model: SkillModel,
|
|
||||||
skill_name: &str,
|
|
||||||
current_model: &str,
|
|
||||||
available_models: &[String],
|
|
||||||
) -> Option<Self> {
|
|
||||||
if skill_name
|
|
||||||
.chars()
|
|
||||||
.any(|character| matches!(character, '`' | '<' | '>'))
|
|
||||||
{
|
|
||||||
return None;
|
|
||||||
}
|
|
||||||
|
|
||||||
let provider_prefix = [SOL_MODEL, TERRA_MODEL]
|
|
||||||
.into_iter()
|
|
||||||
.find_map(|parent_model| {
|
|
||||||
current_model.strip_suffix(parent_model).filter(|prefix| {
|
|
||||||
prefix.is_empty() || prefix.ends_with('.') || prefix.ends_with('/')
|
|
||||||
})
|
|
||||||
})?;
|
|
||||||
let target_model = format!("{provider_prefix}{LUNA_MODEL}");
|
|
||||||
let target_model = available_models
|
|
||||||
.iter()
|
|
||||||
.find(|model| model.as_str() == target_model && is_safe_model_identifier(model))?;
|
|
||||||
let skill_model = match skill_model {
|
|
||||||
SkillModel::Luna => "luna",
|
|
||||||
};
|
|
||||||
let instruction = format!(
|
|
||||||
"<skill_model_delegation>\n\
|
|
||||||
For this invocation only, skill `{skill_name}` requests `model: {skill_model}`. \
|
|
||||||
If the user prohibits delegation or subagents, or the work depends on an image or audio \
|
|
||||||
attachment, work locally. Otherwise, delegate only self-contained text-based skill work \
|
|
||||||
and use `spawn_agent` \
|
|
||||||
exactly once. Set `model` to `{target_model}`, set `fork_turns` to `\"none\"`, and choose a \
|
|
||||||
unique `task_name`. Omit `agent_type` and `reasoning_effort`.\n\
|
|
||||||
Give the child only this skill's work and necessary context; never include this block and tell \
|
|
||||||
it not to spawn agents. Delegate the whole request only if the skill fully covers it; otherwise, \
|
|
||||||
retain ownership of the remaining work and final answer. Wait for the result. If spawning \
|
|
||||||
fails, continue locally. If waiting fails, retry waiting without duplicating the \
|
|
||||||
child's work.\n\
|
|
||||||
Ignore this instruction in child agents and on later turns.\n\
|
|
||||||
</skill_model_delegation>"
|
|
||||||
);
|
|
||||||
|
|
||||||
(instruction.len() <= MAX_DELEGATION_INSTRUCTION_BYTES).then_some(Self(instruction))
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Returns the bounded model-visible delegation instructions.
|
|
||||||
pub fn as_str(&self) -> &str {
|
|
||||||
&self.0
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
fn is_safe_model_identifier(model: &str) -> bool {
|
|
||||||
model.len() <= MAX_TARGET_MODEL_BYTES
|
|
||||||
&& model.chars().all(|character| {
|
|
||||||
character.is_ascii_alphanumeric() || matches!(character, '.' | '/' | '_' | '-')
|
|
||||||
})
|
|
||||||
}
|
|
||||||
|
|
||||||
#[cfg(test)]
|
|
||||||
#[path = "model_delegation_tests.rs"]
|
|
||||||
mod tests;
|
|
||||||
@@ -1,224 +0,0 @@
|
|||||||
use pretty_assertions::assert_eq;
|
|
||||||
|
|
||||||
use super::MAX_DELEGATION_INSTRUCTION_BYTES;
|
|
||||||
use super::SkillModel;
|
|
||||||
use super::SkillModelDelegationInstruction;
|
|
||||||
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)
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn delegates_only_from_supported_parent_models() {
|
|
||||||
let available_models = available_models();
|
|
||||||
for (current_model, should_delegate) in [
|
|
||||||
("gpt-5.6-sol", true),
|
|
||||||
("gpt-5.6-terra", true),
|
|
||||||
("gpt-5.6-luna", false),
|
|
||||||
("gpt-5.5-codex", false),
|
|
||||||
] {
|
|
||||||
let instruction = SkillModelDelegationInstruction::from_skill_model(
|
|
||||||
SkillModel::Luna,
|
|
||||||
"demo",
|
|
||||||
current_model,
|
|
||||||
&available_models,
|
|
||||||
);
|
|
||||||
assert_eq!(
|
|
||||||
instruction.is_some(),
|
|
||||||
should_delegate,
|
|
||||||
"current={current_model}"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn resolves_luna_within_the_current_provider_namespace() {
|
|
||||||
for (current_model, target_model, unrelated_model) in [
|
|
||||||
("gpt-5.6-sol", "gpt-5.6-luna", "openai.gpt-5.6-luna"),
|
|
||||||
(
|
|
||||||
"tenant-a/gpt-5.6-sol",
|
|
||||||
"tenant-a/gpt-5.6-luna",
|
|
||||||
"tenant-b/gpt-5.6-luna",
|
|
||||||
),
|
|
||||||
(
|
|
||||||
"openai.gpt-5.6-terra",
|
|
||||||
"openai.gpt-5.6-luna",
|
|
||||||
"other.gpt-5.6-luna",
|
|
||||||
),
|
|
||||||
] {
|
|
||||||
let instruction = SkillModelDelegationInstruction::from_skill_model(
|
|
||||||
SkillModel::Luna,
|
|
||||||
"demo",
|
|
||||||
current_model,
|
|
||||||
&[unrelated_model.to_string(), target_model.to_string()],
|
|
||||||
)
|
|
||||||
.expect("lower-tier model in the same provider namespace should be resolved");
|
|
||||||
|
|
||||||
assert!(
|
|
||||||
instruction
|
|
||||||
.as_str()
|
|
||||||
.contains(&format!("Set `model` to `{target_model}`"))
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn rejects_targets_outside_the_current_provider_namespace() {
|
|
||||||
for (current_model, available_models) in [
|
|
||||||
(
|
|
||||||
"tenant-a/gpt-5.6-sol",
|
|
||||||
vec![
|
|
||||||
"tenant-b/gpt-5.6-luna".to_string(),
|
|
||||||
"gpt-5.6-luna".to_string(),
|
|
||||||
],
|
|
||||||
),
|
|
||||||
("gpt-5.6-sol", vec!["tenant-a/gpt-5.6-luna".to_string()]),
|
|
||||||
(
|
|
||||||
"tenant-a/gpt-5.6-sol",
|
|
||||||
vec!["tenant-a.gpt-5.6-luna".to_string()],
|
|
||||||
),
|
|
||||||
] {
|
|
||||||
assert_eq!(
|
|
||||||
SkillModelDelegationInstruction::from_skill_model(
|
|
||||||
SkillModel::Luna,
|
|
||||||
"demo",
|
|
||||||
current_model,
|
|
||||||
&available_models,
|
|
||||||
),
|
|
||||||
None,
|
|
||||||
"current={current_model}, available={available_models:?}"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn rejects_unavailable_or_unsafe_models() {
|
|
||||||
let invalid_identifier = format!("{}/gpt-5.6-luna", "<unsafe>".repeat(32));
|
|
||||||
let invalid_current_model = format!("{}/gpt-5.6-sol", "<unsafe>".repeat(32));
|
|
||||||
let overlong_identifier = format!("{}.gpt-5.6-luna", "a".repeat(128));
|
|
||||||
let overlong_current_model = format!("{}.gpt-5.6-sol", "a".repeat(128));
|
|
||||||
for (current_model, available_models) in [
|
|
||||||
(
|
|
||||||
"custom/gpt-5.6-luna",
|
|
||||||
vec!["custom/gpt-5.6-luna".to_string()],
|
|
||||||
),
|
|
||||||
("gpt-5.6-sol", vec!["gpt-5.6-terra".to_string()]),
|
|
||||||
(invalid_current_model.as_str(), vec![invalid_identifier]),
|
|
||||||
(overlong_current_model.as_str(), vec![overlong_identifier]),
|
|
||||||
] {
|
|
||||||
assert_eq!(
|
|
||||||
SkillModelDelegationInstruction::from_skill_model(
|
|
||||||
SkillModel::Luna,
|
|
||||||
"demo",
|
|
||||||
current_model,
|
|
||||||
&available_models,
|
|
||||||
),
|
|
||||||
None,
|
|
||||||
"current={current_model}, available={available_models:?}"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn renders_bounded_instruction_for_selected_skill_and_model() {
|
|
||||||
let instruction = SkillModelDelegationInstruction::from_skill_model(
|
|
||||||
SkillModel::Luna,
|
|
||||||
"demo",
|
|
||||||
"gpt-5.6-sol",
|
|
||||||
&available_models(),
|
|
||||||
)
|
|
||||||
.expect("available lower tier should delegate");
|
|
||||||
let rendered = instruction.as_str();
|
|
||||||
|
|
||||||
assert!(rendered.contains("skill `demo`"));
|
|
||||||
assert!(rendered.contains("Set `model` to `gpt-5.6-luna`"));
|
|
||||||
assert!(rendered.contains("image or audio attachment, work locally"));
|
|
||||||
assert!(rendered.len() <= MAX_DELEGATION_INSTRUCTION_BYTES);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn rejects_skill_names_that_escape_instruction_framing() {
|
|
||||||
for skill_name in ["unsafe`name", "</skill_model_delegation>", "<unsafe>"] {
|
|
||||||
assert_eq!(
|
|
||||||
SkillModelDelegationInstruction::from_skill_model(
|
|
||||||
SkillModel::Luna,
|
|
||||||
skill_name,
|
|
||||||
"gpt-5.6-sol",
|
|
||||||
&available_models(),
|
|
||||||
),
|
|
||||||
None,
|
|
||||||
"skill_name={skill_name:?}"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn rejects_instruction_exceeding_context_bound() {
|
|
||||||
let skill_name = "x".repeat(MAX_DELEGATION_INSTRUCTION_BYTES);
|
|
||||||
|
|
||||||
assert_eq!(
|
|
||||||
SkillModelDelegationInstruction::from_skill_model(
|
|
||||||
SkillModel::Luna,
|
|
||||||
&skill_name,
|
|
||||||
"gpt-5.6-sol",
|
|
||||||
&available_models(),
|
|
||||||
),
|
|
||||||
None
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
fn available_models() -> Vec<String> {
|
|
||||||
vec!["gpt-5.6-luna".to_string()]
|
|
||||||
}
|
|
||||||
@@ -1,4 +1,3 @@
|
|||||||
use crate::SkillModel;
|
|
||||||
use serde::Deserialize;
|
use serde::Deserialize;
|
||||||
use thiserror::Error;
|
use thiserror::Error;
|
||||||
|
|
||||||
@@ -11,8 +10,6 @@ struct SkillFrontmatter {
|
|||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
description: Option<String>,
|
description: Option<String>,
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
model: Option<serde_yaml::Value>,
|
|
||||||
#[serde(default)]
|
|
||||||
metadata: SkillFrontmatterMetadata,
|
metadata: SkillFrontmatterMetadata,
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -28,7 +25,6 @@ pub struct ParsedSkillFrontmatter {
|
|||||||
pub name: String,
|
pub name: String,
|
||||||
pub description: String,
|
pub description: String,
|
||||||
pub short_description: Option<String>,
|
pub short_description: Option<String>,
|
||||||
pub model: Option<SkillModel>,
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Error produced while parsing or validating `SKILL.md` metadata.
|
/// Error produced while parsing or validating `SKILL.md` metadata.
|
||||||
@@ -92,13 +88,6 @@ pub fn parse_skill_frontmatter_metadata(
|
|||||||
name,
|
name,
|
||||||
description,
|
description,
|
||||||
short_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,7 +17,6 @@ fn parses_repairs_and_sanitizes_frontmatter() {
|
|||||||
name: "deploy service".to_string(),
|
name: "deploy service".to_string(),
|
||||||
description: "Build for AWS: ECS".to_string(),
|
description: "Build for AWS: ECS".to_string(),
|
||||||
short_description: Some("Deploy safely".to_string()),
|
short_description: Some("Deploy safely".to_string()),
|
||||||
model: None,
|
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -34,7 +33,6 @@ fn uses_default_name_and_requires_description() {
|
|||||||
name: "demo".to_string(),
|
name: "demo".to_string(),
|
||||||
description: "Demo skill".to_string(),
|
description: "Demo skill".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -58,7 +56,6 @@ fn repairs_short_descriptions_containing_colons_and_apostrophes() {
|
|||||||
name: "short".to_string(),
|
name: "short".to_string(),
|
||||||
description: "Short skill".to_string(),
|
description: "Short skill".to_string(),
|
||||||
short_description: Some("What's included: builds and tests".to_string()),
|
short_description: Some("What's included: builds and tests".to_string()),
|
||||||
model: None,
|
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -77,7 +74,6 @@ fn repairs_unrecognized_frontmatter_fields_that_need_quotes() {
|
|||||||
name: "unknown".to_string(),
|
name: "unknown".to_string(),
|
||||||
description: "Unknown fields".to_string(),
|
description: "Unknown fields".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -96,7 +92,6 @@ fn preserves_block_scalar_bodies_while_repairing_other_fields() {
|
|||||||
name: "block".to_string(),
|
name: "block".to_string(),
|
||||||
description: "Build for AWS: ECS".to_string(),
|
description: "Build for AWS: ECS".to_string(),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -119,7 +114,6 @@ fn preserves_overlong_descriptions_and_short_descriptions() {
|
|||||||
name: "long".to_string(),
|
name: "long".to_string(),
|
||||||
description,
|
description,
|
||||||
short_description: Some(short_description),
|
short_description: Some(short_description),
|
||||||
model: None,
|
|
||||||
}
|
}
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -32,7 +32,6 @@ fn make_skill(name: &str, path: &str) -> SkillMetadata {
|
|||||||
name: name.to_string(),
|
name: name.to_string(),
|
||||||
description: format!("{name} skill"),
|
description: format!("{name} skill"),
|
||||||
short_description: None,
|
short_description: None,
|
||||||
model: None,
|
|
||||||
interface: None,
|
interface: None,
|
||||||
dependencies: None,
|
dependencies: None,
|
||||||
policy: None,
|
policy: None,
|
||||||
|
|||||||
Reference in new issue
Block a user