From 91704c56724610dff2732435e7189fcc01a36f1c Mon Sep 17 00:00:00 2001 From: alexsong-oai Date: Mon, 9 Feb 2026 23:13:27 -0800 Subject: [PATCH] feat: add SkillPolicy to skill metadata and support allow_implicit_invocation (#11244) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tested by setting the policy in agents/openai.yaml to true, false, and leaving it unset (default). ``` policy: allow_implicit_invocation: false ``` Screenshot 2026-02-09 at 3 42 41 PM --- codex-rs/core/src/codex.rs | 6 +- codex-rs/core/src/mcp/skill_dependencies.rs | 1 + .../skill-creator/references/openai_yaml.md | 6 + codex-rs/core/src/skills/injection.rs | 1 + codex-rs/core/src/skills/loader.rs | 151 ++++++++++++++++-- codex-rs/core/src/skills/mod.rs | 1 + codex-rs/core/src/skills/model.rs | 23 ++- codex-rs/tui/src/bottom_pane/mod.rs | 1 + codex-rs/tui/src/chatwidget/skills.rs | 1 + codex-rs/tui/src/chatwidget/tests.rs | 2 + 10 files changed, 176 insertions(+), 17 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 5eed41e77..b9ef007e8 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -294,8 +294,10 @@ impl Codex { config.features.disable(Feature::Collab); } - let enabled_skills = loaded_skills.enabled_skills(); - let user_instructions = get_user_instructions(&config, Some(&enabled_skills)).await; + let allowed_skills_for_implicit_invocation = + loaded_skills.allowed_skills_for_implicit_invocation(); + let user_instructions = + get_user_instructions(&config, Some(&allowed_skills_for_implicit_invocation)).await; let exec_policy = ExecPolicyManager::load(&config.config_layer_stack) .await diff --git a/codex-rs/core/src/mcp/skill_dependencies.rs b/codex-rs/core/src/mcp/skill_dependencies.rs index fa755f5da..e2817d5a4 100644 --- a/codex-rs/core/src/mcp/skill_dependencies.rs +++ b/codex-rs/core/src/mcp/skill_dependencies.rs @@ -431,6 +431,7 @@ mod tests { short_description: None, interface: None, dependencies: Some(SkillDependencies { tools }), + policy: None, path: PathBuf::from("skill"), scope: SkillScope::User, } diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/references/openai_yaml.md b/codex-rs/core/src/skills/assets/samples/skill-creator/references/openai_yaml.md index da5629f8d..90f9e8e86 100644 --- a/codex-rs/core/src/skills/assets/samples/skill-creator/references/openai_yaml.md +++ b/codex-rs/core/src/skills/assets/samples/skill-creator/references/openai_yaml.md @@ -20,6 +20,9 @@ dependencies: description: "GitHub MCP server" transport: "streamable_http" url: "https://api.githubcopilot.com/mcp/" + +policy: + allow_implicit_invocation: true ``` ## Field descriptions and constraints @@ -41,3 +44,6 @@ Top-level constraints: - `dependencies.tools[].description`: Human-readable explanation of the dependency. - `dependencies.tools[].transport`: Connection type when `type` is `mcp`. - `dependencies.tools[].url`: MCP server URL when `type` is `mcp`. +- `policy.allow_implicit_invocation`: When false, the skill is not injected into + the model context by default, but can still be invoked explicitly via `$skill`. + Defaults to true. diff --git a/codex-rs/core/src/skills/injection.rs b/codex-rs/core/src/skills/injection.rs index 19ccdb207..8896a4aa7 100644 --- a/codex-rs/core/src/skills/injection.rs +++ b/codex-rs/core/src/skills/injection.rs @@ -473,6 +473,7 @@ mod tests { short_description: None, interface: None, dependencies: None, + policy: None, path: PathBuf::from(path), scope: codex_protocol::protocol::SkillScope::User, } diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index 44a503888..6821d1b8f 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -9,6 +9,7 @@ use crate::skills::model::SkillError; use crate::skills::model::SkillInterface; use crate::skills::model::SkillLoadOutcome; use crate::skills::model::SkillMetadata; +use crate::skills::model::SkillPolicy; use crate::skills::model::SkillToolDependency; use crate::skills::system::system_cache_root_dir; use codex_app_server_protocol::ConfigLayerSource; @@ -47,6 +48,8 @@ struct SkillMetadataFile { interface: Option, #[serde(default)] dependencies: Option, + #[serde(default)] + policy: Option, } #[derive(Debug, Default, Deserialize)] @@ -65,6 +68,12 @@ struct Dependencies { tools: Vec, } +#[derive(Debug, Deserialize)] +struct Policy { + #[serde(default)] + allow_implicit_invocation: Option, +} + #[derive(Debug, Default, Deserialize)] struct DependencyTool { #[serde(rename = "type")] @@ -481,7 +490,7 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result Result (Option, Option) { +fn load_skill_metadata( + skill_path: &Path, +) -> ( + Option, + Option, + Option, +) { // Fail open: optional metadata should not block loading SKILL.md. let Some(skill_dir) = skill_path.parent() else { - return (None, None); + return (None, None, None); }; let metadata_path = skill_dir .join(SKILLS_METADATA_DIR) .join(SKILLS_METADATA_FILENAME); if !metadata_path.exists() { - return (None, None); + return (None, None, None); } let contents = match fs::read_to_string(&metadata_path) { @@ -526,7 +542,7 @@ fn load_skill_metadata(skill_path: &Path) -> (Option, Option (Option, Option) -> Option) -> Option { + policy.map(|policy| SkillPolicy { + allow_implicit_invocation: policy.allow_implicit_invocation, + }) +} + fn resolve_dependency_tool(tool: DependencyTool) -> Option { let r#type = resolve_required_str( tool.kind, @@ -991,6 +1020,7 @@ mod tests { short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1137,6 +1167,7 @@ mod tests { }, ], }), + policy: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1191,12 +1222,79 @@ interface: default_prompt: Some("default prompt".to_string()), }), dependencies: None, + policy: None, path: normalized(skill_path.as_path()), scope: SkillScope::User, }] ); } + #[tokio::test] + async fn loads_skill_policy_from_yaml() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "policy-skill", "from json"); + let skill_dir = skill_path.parent().expect("skill dir"); + + write_skill_metadata_at( + skill_dir, + r#" +policy: + allow_implicit_invocation: false +"#, + ); + + 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.len(), 1); + assert_eq!( + outcome.skills[0].policy, + Some(SkillPolicy { + allow_implicit_invocation: Some(false), + }) + ); + assert!(outcome.allowed_skills_for_implicit_invocation().is_empty()); + } + + #[tokio::test] + async fn empty_skill_policy_defaults_to_allow_implicit_invocation() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_skill(&codex_home, "demo", "policy-empty", "from json"); + let skill_dir = skill_path.parent().expect("skill dir"); + + write_skill_metadata_at( + skill_dir, + r#" +policy: {} +"#, + ); + + 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.len(), 1); + assert_eq!( + outcome.skills[0].policy, + Some(SkillPolicy { + allow_implicit_invocation: None, + }) + ); + assert_eq!( + outcome.allowed_skills_for_implicit_invocation(), + outcome.skills + ); + } + #[tokio::test] async fn accepts_icon_paths_under_assets_dir() { let codex_home = tempfile::tempdir().expect("tempdir"); @@ -1240,6 +1338,7 @@ interface: default_prompt: None, }), dependencies: None, + policy: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1279,6 +1378,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1331,6 +1431,7 @@ interface: default_prompt: None, }), dependencies: None, + policy: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1371,6 +1472,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1414,6 +1516,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&shared_skill_path), scope: SkillScope::User, }] @@ -1473,6 +1576,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1508,6 +1612,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&shared_skill_path), scope: SkillScope::Admin, }] @@ -1547,6 +1652,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&linked_skill_path), scope: SkillScope::Repo, }] @@ -1565,9 +1671,10 @@ interface: fs::create_dir_all(&system_root).unwrap(); symlink_dir(shared.path(), &system_root.join("shared")); - let cfg = make_config(&codex_home).await; - let outcome = load_skills(&cfg); - + let outcome = load_skills_from_roots([SkillRoot { + path: system_root, + scope: SkillScope::System, + }]); assert!( outcome.errors.is_empty(), "unexpected errors: {:?}", @@ -1593,8 +1700,11 @@ interface: "should not load", ); - let cfg = make_config(&codex_home).await; - let outcome = load_skills(&cfg); + let skills_root = codex_home.path().join("skills"); + let outcome = load_skills_from_roots([SkillRoot { + path: skills_root, + scope: SkillScope::User, + }]); assert!( outcome.errors.is_empty(), @@ -1609,6 +1719,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&within_depth_path), scope: SkillScope::User, }] @@ -1635,6 +1746,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1665,6 +1777,7 @@ interface: short_description: Some("short summary".to_string()), interface: None, dependencies: None, + policy: None, path: normalized(&skill_path), scope: SkillScope::User, }] @@ -1776,6 +1889,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -1810,6 +1924,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -1862,6 +1977,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&nested_skill_path), scope: SkillScope::Repo, }, @@ -1871,6 +1987,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&root_skill_path), scope: SkillScope::Repo, }, @@ -1909,6 +2026,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -1945,6 +2063,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -1985,6 +2104,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&repo_skill_path), scope: SkillScope::Repo, }, @@ -1994,6 +2114,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&user_skill_path), scope: SkillScope::User, }, @@ -2057,6 +2178,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: first_path, scope: SkillScope::Repo, }, @@ -2066,6 +2188,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: second_path, scope: SkillScope::Repo, }, @@ -2136,6 +2259,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&skill_path), scope: SkillScope::Repo, }] @@ -2193,6 +2317,7 @@ interface: short_description: None, interface: None, dependencies: None, + policy: None, path: normalized(&skill_path), scope: SkillScope::System, }] diff --git a/codex-rs/core/src/skills/mod.rs b/codex-rs/core/src/skills/mod.rs index 6148092a1..17405129e 100644 --- a/codex-rs/core/src/skills/mod.rs +++ b/codex-rs/core/src/skills/mod.rs @@ -17,4 +17,5 @@ pub use manager::SkillsManager; pub use model::SkillError; pub use model::SkillLoadOutcome; pub use model::SkillMetadata; +pub use model::SkillPolicy; pub use render::render_skills_section; diff --git a/codex-rs/core/src/skills/model.rs b/codex-rs/core/src/skills/model.rs index 92ecbd84b..bc75b7e36 100644 --- a/codex-rs/core/src/skills/model.rs +++ b/codex-rs/core/src/skills/model.rs @@ -10,10 +10,25 @@ pub struct SkillMetadata { pub short_description: Option, pub interface: Option, pub dependencies: Option, + pub policy: Option, pub path: PathBuf, pub scope: SkillScope, } +impl SkillMetadata { + fn allow_implicit_invocation(&self) -> bool { + self.policy + .as_ref() + .and_then(|policy| policy.allow_implicit_invocation) + .unwrap_or(true) + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub struct SkillPolicy { + pub allow_implicit_invocation: Option, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct SkillInterface { pub display_name: Option, @@ -57,10 +72,14 @@ impl SkillLoadOutcome { !self.disabled_paths.contains(&skill.path) } - pub fn enabled_skills(&self) -> Vec { + pub fn is_skill_allowed_for_implicit_invocation(&self, skill: &SkillMetadata) -> bool { + self.is_skill_enabled(skill) && skill.allow_implicit_invocation() + } + + pub fn allowed_skills_for_implicit_invocation(&self) -> Vec { self.skills .iter() - .filter(|skill| self.is_skill_enabled(skill)) + .filter(|skill| self.is_skill_allowed_for_implicit_invocation(skill)) .cloned() .collect() } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index edf103bc4..32029996a 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -1332,6 +1332,7 @@ mod tests { short_description: None, interface: None, dependencies: None, + policy: None, path: PathBuf::from("test-skill"), scope: SkillScope::User, }]), diff --git a/codex-rs/tui/src/chatwidget/skills.rs b/codex-rs/tui/src/chatwidget/skills.rs index 0920c8837..ba8dab416 100644 --- a/codex-rs/tui/src/chatwidget/skills.rs +++ b/codex-rs/tui/src/chatwidget/skills.rs @@ -189,6 +189,7 @@ fn protocol_skill_to_core(skill: &ProtocolSkillMetadata) -> SkillMetadata { }) .collect(), }), + policy: None, path: skill.path.clone(), scope: skill.scope, } diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 57f918309..ad4c4fc62 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -434,6 +434,7 @@ async fn submission_prefers_selected_duplicate_skill_path() { short_description: None, interface: None, dependencies: None, + policy: None, path: repo_skill_path, scope: SkillScope::Repo, }, @@ -443,6 +444,7 @@ async fn submission_prefers_selected_duplicate_skill_path() { short_description: None, interface: None, dependencies: None, + policy: None, path: user_skill_path.clone(), scope: SkillScope::User, },