From 20b4ff2f35f2193608385e4caa8da8ceee45a3cf Mon Sep 17 00:00:00 2001 From: Anthony Tafoya Date: Tue, 13 Oct 2026 20:34:24 +0000 Subject: [PATCH] Enrich V2 capability discovery with resolved plugin and skill metadata (#53763) ## What changed - Return qualified skill names such as `team:deploy` and parsed `dependencies` and `policy` instead of a separate namespace and raw metadata file. Share `agents/openai.yaml` parsing and validation between skill loaders and executor discovery. - Include each standalone skill's search-root ID and path in `discoveryRoot`, while accepting responses from older executors that omit it. - Add plugin `displayName`, falling back to the manifest name or installation directory. - Materialize inline MCP declarations in `mcpConfig` with the manifest as their source path, and report incomplete discovery when declared MCP or Apps configuration files have no returned contents. ## Testing Add coverage for JSON round trips, omitted `discoveryRoot`, optional and invalid skill metadata, qualified names, display-name fallbacks, and inline MCP configuration. Verify standalone search roots survive cached discovery and sandboxed RPC responses. GitOrigin-RevId: 4124972444ad7c2fb8c6418acbf9a589d797fad2 --- codex-rs/Cargo.lock | 2 +- .../core/tests/suite/executor_discovery_v2.rs | 6 +- codex-rs/exec-server-protocol/Cargo.toml | 1 + .../exec-server-protocol/src/capabilities.rs | 28 ++- .../src/capabilities_tests.rs | 55 ++++- codex-rs/exec-server-protocol/src/protocol.rs | 7 +- .../src/discoverV2/capability_discoveries.rs | 97 +++++++- .../src/discoverV2/capability_locations.rs | 13 +- .../capability_locations/candidates.rs | 14 +- .../tests/capability_discoveries_tests.rs | 227 ++++++++++++++++-- .../tests/capability_locations_tests.rs | 8 +- codex-rs/exec-server/src/lib.rs | 1 + codex-rs/exec-server/tests/capabilities_v2.rs | 35 ++- codex-rs/ext/skills/Cargo.toml | 1 - codex-rs/ext/skills/src/loader/environment.rs | 20 +- codex-rs/ext/skills/src/loader/metadata.rs | 156 +----------- codex-rs/ext/skills/src/loader/mod.rs | 7 - codex-rs/skills/src/lib.rs | 3 + codex-rs/skills/src/metadata.rs | 164 +++++++++++++ codex-rs/skills/src/model.rs | 9 +- 20 files changed, 621 insertions(+), 233 deletions(-) create mode 100644 codex-rs/skills/src/metadata.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index ad3060d741..d2d52d2268 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -3303,6 +3303,7 @@ dependencies = [ "codex-network-proxy", "codex-protocol", "codex-shell-command", + "codex-skills", "codex-utils-path-uri", "pretty_assertions", "serde", @@ -4639,7 +4640,6 @@ dependencies = [ "schemars 0.8.22", "serde", "serde_json", - "serde_yaml", "tempfile", "tokio", "toml 0.9.11+spec-1.1.0", diff --git a/codex-rs/core/tests/suite/executor_discovery_v2.rs b/codex-rs/core/tests/suite/executor_discovery_v2.rs index f8e278ec56..55393cb72d 100644 --- a/codex-rs/core/tests/suite/executor_discovery_v2.rs +++ b/codex-rs/core/tests/suite/executor_discovery_v2.rs @@ -147,11 +147,13 @@ impl Executor { .iter().map(|root| json!({"id":root["id"],"path":root["path"]})).collect::>() })), Some("capabilities/discoverV2") => Some(json!({ - "plugins":[{"id":"inert-sentinel@market","remotePluginId":null, + "plugins":[{"id":"inert-sentinel@market","displayName":"inert-sentinel","remotePluginId":null, "version":"1","root":"file:///inert-sentinel", "files":{"manifest":{"path":"file:///inert-sentinel/.codex-plugin/plugin.json", "contents":"{\"name\":\"inert-sentinel\",\"mcpServers\":{\"inert-sentinel\":{\"command\":\"never-run\"}}}"}, - "mcpConfig":null,"appsConfig":null},"skills":[]}], + "mcpConfig":{"path":"file:///inert-sentinel/.codex-plugin/plugin.json", + "contents":"{\"inert-sentinel\":{\"command\":\"never-run\"}}"}, + "appsConfig":null},"skills":[]}], "skills":[{"name":"inert-sentinel","description":"must not enter the prompt", "shortDescription":null,"path":"file:///inert-sentinel/SKILL.md","metadata":null}], "warnings":[] diff --git a/codex-rs/exec-server-protocol/Cargo.toml b/codex-rs/exec-server-protocol/Cargo.toml index eda1add272..a203df0535 100644 --- a/codex-rs/exec-server-protocol/Cargo.toml +++ b/codex-rs/exec-server-protocol/Cargo.toml @@ -19,6 +19,7 @@ codex-install-context = { workspace = true } codex-network-proxy = { workspace = true } codex-protocol = { workspace = true } codex-shell-command = { workspace = true } +codex-skills = { workspace = true } codex-utils-path-uri = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["arbitrary_precision", "raw_value"] } diff --git a/codex-rs/exec-server-protocol/src/capabilities.rs b/codex-rs/exec-server-protocol/src/capabilities.rs index 3adb4538f8..3e952f318b 100644 --- a/codex-rs/exec-server-protocol/src/capabilities.rs +++ b/codex-rs/exec-server-protocol/src/capabilities.rs @@ -1,13 +1,14 @@ //! Executor capability inventory; callers apply identity and enablement. use codex_file_system::FileSystemSandboxContext; +use codex_skills::SkillDependencies; +use codex_skills::SkillPolicy; use codex_utils_path_uri::PathUri; use serde::Deserialize; use serde::Serialize; use std::fmt; use std::sync::Arc; -use crate::CapabilityTextFile; use crate::DiscoveredPluginFiles; pub const CAPABILITIES_DISCOVER_V2_METHOD: &str = "capabilities/discoverV2"; @@ -44,6 +45,8 @@ pub struct DiscoverV2CapabilitiesResponse { pub struct ExecutorPlugin { /// Environment-local ID: `@`. pub id: String, + /// Manifest display name, falling back to its name or the installation directory. + pub display_name: String, pub remote_plugin_id: Option, pub version: String, pub root: PathUri, @@ -51,20 +54,30 @@ pub struct ExecutorPlugin { pub skills: Vec, } +/// Executor-owned search root for a standalone skill. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct SkillDiscoveryRoot { + pub id: String, + pub path: PathUri, +} + /// Skill metadata, excluding instructions and resources. #[derive(Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct ExecutorSkill { + /// Final skill name, qualified by the nearest enclosing plugin namespace when present. pub name: String, - /// Nearest enclosing legacy plugin's name, following V1 precedence. - #[serde(default)] - pub namespace: Option, pub description: String, #[serde(default)] pub short_description: Option, /// Path to SKILL.md, not its containing directory. pub path: PathUri, - pub metadata: Option, + /// Absent for plugin skills and responses from older executors. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub discovery_root: Option, + pub dependencies: Option, + pub policy: Option, } impl fmt::Debug for ExecutorPlugin { @@ -95,12 +108,7 @@ impl fmt::Debug for ExecutorSkill { formatter .debug_struct("ExecutorSkill") .field("name", &self.name) - .field("namespace", &self.namespace) .field("path", &self.path) - .field( - "metadata_path", - &self.metadata.as_ref().map(|file| &file.path), - ) .finish() } } diff --git a/codex-rs/exec-server-protocol/src/capabilities_tests.rs b/codex-rs/exec-server-protocol/src/capabilities_tests.rs index d2cdab71b1..eda7263af5 100644 --- a/codex-rs/exec-server-protocol/src/capabilities_tests.rs +++ b/codex-rs/exec-server-protocol/src/capabilities_tests.rs @@ -1,6 +1,7 @@ //! V2 discovery requests and responses round-trip through JSON. use super::*; +use crate::CapabilityTextFile; use pretty_assertions::assert_eq; #[test] @@ -19,16 +20,37 @@ fn discovery_contract_round_trips_json() -> Result<(), Box Result<(), Box(wire.clone())?, + response + ); + // Older executors omit the root metadata. + wire["skills"][0] + .as_object_mut() + .expect("skill object") + .remove("discoveryRoot"); + let mut legacy_response = response; + legacy_response.skills[0].discovery_root = None; assert_eq!( serde_json::from_value::(wire)?, - response + legacy_response ); Ok(()) } diff --git a/codex-rs/exec-server-protocol/src/protocol.rs b/codex-rs/exec-server-protocol/src/protocol.rs index 26614350c3..c0bb0966ee 100644 --- a/codex-rs/exec-server-protocol/src/protocol.rs +++ b/codex-rs/exec-server-protocol/src/protocol.rs @@ -966,7 +966,8 @@ pub struct CapabilityRootsDiscoverResponse { pub roots: Vec, } -/// Recognized UTF-8 capability file materialized by the exec-server. +/// UTF-8 capability contents materialized by the exec-server. +/// `path` identifies the source file; V2 inline MCP contents are extracted from its manifest. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct CapabilityTextFile { @@ -979,7 +980,9 @@ pub struct CapabilityTextFile { #[serde(rename_all = "camelCase")] pub struct DiscoveredPluginFiles { pub manifest: CapabilityTextFile, - /// File-backed MCP declarations, including the conventional `.mcp.json` fallback. + /// MCP declarations from a selected file, including the conventional `.mcp.json` fallback. + /// V2 also materializes inline declarations, using the manifest as the source path. + /// V1 leaves inline declarations only in `manifest`. #[serde(default)] pub mcp_config: Option, /// File-backed connector declarations. diff --git a/codex-rs/exec-server/src/discoverV2/capability_discoveries.rs b/codex-rs/exec-server/src/discoverV2/capability_discoveries.rs index b681a417c0..5a8e51b024 100644 --- a/codex-rs/exec-server/src/discoverV2/capability_discoveries.rs +++ b/codex-rs/exec-server/src/discoverV2/capability_discoveries.rs @@ -6,16 +6,19 @@ use std::sync::Arc; use codex_file_system::ExecutorFileSystem; use codex_skills::parse_skill_frontmatter_metadata; +use codex_skills::parse_skill_metadata; use futures::StreamExt; use serde::Deserialize; use serde::Serialize; use crate::CapabilityRootDiscoverRequest; use crate::CapabilityRootDiscovery; +use crate::CapabilityTextFile; use crate::DiscoverV2CapabilitiesResponse; use crate::ExecutorPlugin; use crate::ExecutorSkill; use crate::LocalFileSystem; +use crate::SkillDiscoveryRoot; use crate::capability_discovery::MAX_CONCURRENT_ROOTS; use crate::capability_discovery::discover_root; use crate::discover_v2::capability_locations::CapabilityLocation; @@ -39,6 +42,23 @@ struct CapabilityDiscovery { warnings: Vec, } +// Only the metadata V2 returns or needs to materialize discovered configuration. +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct PluginManifestMetadata { + #[serde(default)] + name: String, + interface: Option, + mcp_servers: Option, + apps: Option, +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase")] +struct PluginManifestInterface { + display_name: Option, +} + impl Default for CapabilityDiscovery { fn default() -> Self { Self { @@ -237,18 +257,87 @@ fn parse_root( } ancestor = root.parent(); } + let metadata = skill.metadata.as_ref().and_then(|metadata| { + parse_skill_metadata(&metadata.contents).map_err(|error| { + tracing::warn!(path = %metadata.path, %error, "ignoring invalid discovered skill metadata"); + }).ok() + }).unwrap_or_default(); skills.push(ExecutorSkill { - name: parsed.name, - namespace, + name: namespace + .map(|namespace| format!("{namespace}:{}", parsed.name)) + .unwrap_or(parsed.name), description: parsed.description, short_description: parsed.short_description, path, - metadata: skill.metadata, + discovery_root: location + .standalone_root_id + .as_ref() + .map(|id| SkillDiscoveryRoot { + id: id.clone(), + path: location.root.clone(), + }), + dependencies: metadata.dependencies, + policy: metadata.policy, }); } - let content = if let (Some(plugin), Some(files)) = (location.plugin, discovery.plugin) { + let content = if let (Some(plugin), Some(mut files)) = (location.plugin, discovery.plugin) { + let manifest = + match serde_json::from_str::(&files.manifest.contents) { + Ok(manifest) => manifest, + Err(_) => { + warnings.push(format!( + "invalid plugin manifest at {}", + files.manifest.path + )); + return CapabilityDiscovery { + warnings, + ..Default::default() + }; + } + }; + let name = if manifest.name.trim().is_empty() { + location.root.basename().unwrap_or(manifest.name) + } else { + manifest.name + }; + let display_name = manifest + .interface + .and_then(|interface| interface.display_name) + .map(|display_name| display_name.trim().to_string()) + .filter(|display_name| !display_name.is_empty()) + .unwrap_or(name); + // V2 materializes inline declarations; the path identifies their source manifest. + if files.mcp_config.is_none() { + if let Some(contents) = manifest + .mcp_servers + .as_ref() + .filter(|value| value.is_object()) + { + files.mcp_config = Some(CapabilityTextFile { + path: files.manifest.path.clone(), + contents: contents.to_string(), + }); + } else if manifest + .mcp_servers + .as_ref() + .is_some_and(serde_json::Value::is_string) + { + warnings.push(format!( + "plugin `{}` at {} declares an MCP configuration file, but discovery returned no contents", + plugin.id, location.root + )); + } + } + + if files.apps_config.is_none() && manifest.apps.is_some() { + warnings.push(format!( + "plugin `{}` at {} declares an Apps configuration file, but discovery returned no contents", + plugin.id, location.root + )); + } DiscoveredSource::Plugin(Box::new(ExecutorPlugin { id: plugin.id, + display_name, remote_plugin_id: plugin.remote_plugin_id, version: plugin.version, root: location.root, diff --git a/codex-rs/exec-server/src/discoverV2/capability_locations.rs b/codex-rs/exec-server/src/discoverV2/capability_locations.rs index 12c443af94..e32d460a88 100644 --- a/codex-rs/exec-server/src/discoverV2/capability_locations.rs +++ b/codex-rs/exec-server/src/discoverV2/capability_locations.rs @@ -39,6 +39,7 @@ const AGENTS_SKILLS_SUFFIX: &str = ".agents/skills"; #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub(crate) struct CapabilityLocation { pub root: PathUri, + pub standalone_root_id: Option, pub plugin: Option, } @@ -69,19 +70,22 @@ pub(crate) async fn prewarm_locations( .map(PathUri::to_abs_path) .transpose()?; // Fixed standalone roots extend Flora's V1 root; project skill discovery is deferred. - let mut skill_roots = vec![codex_home.join("skills")]; + let mut skill_roots = vec![("codex-home-skills", codex_home.join("skills"))]; if let Some(home) = user_home { - skill_roots.push(home.join(AGENTS_SKILLS_SUFFIX)); + skill_roots.push(("user-agent-skills", home.join(AGENTS_SKILLS_SUFFIX))); } if let Some(system_root) = &request.system_skills_root { - skill_roots.push(system_root.to_abs_path()?); + skill_roots.push(("system-skills", system_root.to_abs_path()?)); } CandidateCapabilityLocations::resolve( fs, Some(PathUri::from_abs_path(&codex_home.join(PLUGINS_CACHE_DIR))), skill_roots .into_iter() - .map(|root| PathUri::from_abs_path(&root)) + .map(|(id, root)| crate::SkillDiscoveryRoot { + id: id.to_string(), + path: PathUri::from_abs_path(&root), + }) .collect(), ) .await @@ -171,6 +175,7 @@ async fn discover_cached_plugins( }; locations.locations.push(CapabilityLocation { root: PathUri::from_abs_path(&package_root.join(&version)), + standalone_root_id: None, plugin: Some(InstalledPlugin { id, version, diff --git a/codex-rs/exec-server/src/discoverV2/capability_locations/candidates.rs b/codex-rs/exec-server/src/discoverV2/capability_locations/candidates.rs index 332baeb3a2..0036a85c5b 100644 --- a/codex-rs/exec-server/src/discoverV2/capability_locations/candidates.rs +++ b/codex-rs/exec-server/src/discoverV2/capability_locations/candidates.rs @@ -5,6 +5,7 @@ use super::CapabilityLocations; use super::MAX_LOCATIONS; use super::discover_cached_plugins; use crate::LocalFileSystem; +use crate::SkillDiscoveryRoot; use codex_file_system::ExecutorFileSystem; use codex_file_watcher::WatchPath; use codex_utils_path_uri::PathUri; @@ -22,7 +23,7 @@ pub(super) struct CandidateCapabilityLocations { #[derive(Clone, Debug, PartialEq, Eq)] enum CandidateCapabilityLocation { Cache(PathUri), - Standalone(PathUri), + Standalone(SkillDiscoveryRoot), } impl CandidateCapabilityLocations { @@ -30,7 +31,7 @@ impl CandidateCapabilityLocations { pub(super) async fn resolve( fs: &LocalFileSystem, cache_root: Option, - skill_roots: Vec, + skill_roots: Vec, ) -> io::Result { let mut candidates = cache_root .into_iter() @@ -38,7 +39,7 @@ impl CandidateCapabilityLocations { .collect::>(); let mut standalone = skill_roots; let mut seen = std::collections::HashSet::new(); - standalone.retain(|root| seen.insert(root.clone())); + standalone.retain(|root| seen.insert(root.path.clone())); candidates.extend( standalone .into_iter() @@ -49,8 +50,8 @@ impl CandidateCapabilityLocations { let mut retained_candidates = Vec::new(); for candidate in candidates { let root = match &candidate { - CandidateCapabilityLocation::Cache(root) - | CandidateCapabilityLocation::Standalone(root) => root, + CandidateCapabilityLocation::Cache(root) => root, + CandidateCapabilityLocation::Standalone(root) => &root.path, }; let exists = match fs .get_metadata(root, Default::default(), /*sandbox*/ None) @@ -90,7 +91,8 @@ impl CandidateCapabilityLocations { // Standalone roots need no manifest or version selection. CandidateCapabilityLocation::Standalone(root) => Ok(CapabilityLocations { locations: vec![CapabilityLocation { - root: root.clone(), + root: root.path.clone(), + standalone_root_id: Some(root.id.clone()), plugin: None, }], ..Default::default() diff --git a/codex-rs/exec-server/src/discoverV2/tests/capability_discoveries_tests.rs b/codex-rs/exec-server/src/discoverV2/tests/capability_discoveries_tests.rs index dd144081ec..2975953969 100644 --- a/codex-rs/exec-server/src/discoverV2/tests/capability_discoveries_tests.rs +++ b/codex-rs/exec-server/src/discoverV2/tests/capability_discoveries_tests.rs @@ -9,6 +9,7 @@ use crate::CapabilityRootDiscoverRequest; use crate::DiscoverV2CapabilitiesRequest; use crate::ExecutorSkill; use crate::LocalFileSystem; +use crate::SkillDiscoveryRoot; use crate::capability_discovery::discover_root; use crate::discover_v2::capability_locations::tests::MANIFEST; use crate::discover_v2::capability_locations::tests::request; @@ -36,6 +37,24 @@ async fn discovery_preserves_inventory_and_v1_metadata() -> anyhow::Result<()> { MANIFEST, )?; } + let inline_root = "codex/plugins/cache/test/unconfigured/local"; + write( + temp.path(), + "codex/plugins/cache/test/disabled/local/.codex-plugin/plugin.json", + r#"{"name":" ","interface":{"displayName":" "},"mcpServers":"./missing-mcp.json","apps":"./missing-apps.json"}"#, + )?; + let inline_manifest = r#"{"name":"inline","interface":{"displayName":" Inline Plugin "},"mcpServers":{"inline":{"command":"node"}}}"#; + write( + temp.path(), + &format!("{inline_root}/.codex-plugin/plugin.json"), + inline_manifest, + )?; + // Inline declarations take precedence over a conventional MCP file. + write( + temp.path(), + &format!("{inline_root}/.mcp.json"), + r#"{"wrong":{"command":"wrong"}}"#, + )?; let root = "codex/plugins/cache/test/demo/local"; write( temp.path(), @@ -86,6 +105,17 @@ async fn discovery_preserves_inventory_and_v1_metadata() -> anyhow::Result<()> { .await; // V1 keeps undeclared/migrated skills and excludes the skill beyond its depth limit. assert_eq!(v1.skills.len(), 4); + let inline_v1 = discover_root( + &file_system, + CapabilityRootDiscoverRequest { + id: "unconfigured@test".to_string(), + path: PathUri::from_host_native_path(temp.path().join(inline_root))?, + sandbox: None, + }, + ) + .await; + let inline_v1_files = inline_v1.plugin.expect("inline plugin"); + assert_eq!(inline_v1_files.mcp_config, None); let manager = CapabilityManager::new(file_system); manager.seed_prewarmed_locations(request.clone()).await?; let response = manager @@ -102,6 +132,14 @@ async fn discovery_preserves_inventory_and_v1_metadata() -> anyhow::Result<()> { .collect::>(), vec!["demo@test", "disabled@test", "unconfigured@test"], ); + assert_eq!( + response + .plugins + .iter() + .map(|plugin| plugin.display_name.as_str()) + .collect::>(), + vec!["demo", "local", "Inline Plugin"], + ); let plugin = response .plugins .first() @@ -122,38 +160,60 @@ async fn discovery_preserves_inventory_and_v1_metadata() -> anyhow::Result<()> { v1.skills .into_iter() .map(|skill| { - let namespace = if skill + let name = if skill .instructions .path .starts_with(&plugin.root.join("nested")?) { - "team" + "team:deploy" } else { - "demo" + "demo:deploy" }; - Ok(expected_skill(skill.instructions.path, Some(namespace))) + Ok(expected_skill(skill.instructions.path, name)) }) .collect::>>()? ); assert_eq!( response.skills, vec![ - ExecutorSkill { - name: "deploy skill".to_string(), - ..expected_skill( - request.codex_home.join("skills/deploy skill/SKILL.md")?, - /*namespace*/ None, - ) - }, + expected_skill( + request.codex_home.join("skills/deploy skill/SKILL.md")?, + "deploy skill", + ), expected_skill( request .codex_home .join("skills/nested/skills/deploy/SKILL.md")?, - Some("team") + "team:deploy" ), - ], + ] + .into_iter() + .map(|mut skill| { + skill.discovery_root = Some(SkillDiscoveryRoot { + id: "codex-home-skills".to_string(), + path: request.codex_home.join("skills").expect("skills root"), + }); + skill + }) + .collect::>(), + ); + let inline_plugin = response + .plugins + .iter() + .find(|plugin| plugin.id == "unconfigured@test") + .expect("inline plugin"); + assert_eq!(inline_plugin.files.manifest, inline_v1_files.manifest); + assert_eq!( + inline_plugin.files.mcp_config, + Some(crate::CapabilityTextFile { + path: inline_v1_files.manifest.path, + contents: serde_json::json!({"inline": {"command": "node"}}).to_string(), + }) + ); + assert_eq!( + response.warnings, + vec!["some capability metadata could not be loaded; discovery may be incomplete"], ); - assert_eq!(response.warnings, v1.warnings); let json = serde_json::to_string(&response)?; assert!( response.plugins.len() + plugin.skills.len() + response.skills.len() <= MAX_CAPABILITIES @@ -164,14 +224,15 @@ async fn discovery_preserves_inventory_and_v1_metadata() -> anyhow::Result<()> { Ok(()) } -fn expected_skill(path: PathUri, namespace: Option<&str>) -> ExecutorSkill { +fn expected_skill(path: PathUri, name: &str) -> ExecutorSkill { ExecutorSkill { - name: "deploy".to_string(), - namespace: namespace.map(str::to_string), + name: name.to_string(), description: "Deploy the service.".to_string(), short_description: None, path, - metadata: None, + discovery_root: None, + dependencies: None, + policy: None, } } @@ -214,3 +275,133 @@ async fn discovery_reports_location_failures_without_paths() -> anyhow::Result<( } Ok(()) } + +#[tokio::test] +async fn discovery_resolves_optional_skill_metadata() -> anyhow::Result<()> { + use codex_protocol::protocol::Product; + use codex_skills::SkillDependencies; + use codex_skills::SkillPolicy; + use codex_skills::SkillToolDependency; + + let dependencies = Some(SkillDependencies { + tools: vec![SkillToolDependency { + r#type: "mcp".into(), + value: "deploy".into(), + description: None, + transport: Some("stdio".into()), + command: Some("deploy-mcp".into()), + url: None, + oauth_callback_port: Some(3118), + }], + }); + let policy = Some(SkillPolicy { + allow_implicit_invocation: Some(false), + products: vec![Product::Codex, Product::Chatgpt], + }); + for (yaml, dependencies, policy) in [ + (None, None, None), + (Some("interface: [invalid]"), None, None), + (Some("dependencies:\n tools: [{type: mcp}]"), None, None), + (Some("dependencies: {tools: []}"), None, None), + ( + Some( + r#"dependencies: + tools: + - type: " mcp " + value: " deploy " + transport: stdio + command: deploy-mcp + oauth: {callback_port: 3118} + - type: mcp + value: " " +policy: + allow_implicit_invocation: false + products: [codex, CHATGPT] +"#, + ), + dependencies, + policy, + ), + ] { + let temp = tempfile::tempdir()?; + let request = request(temp.path())?; + let plugin_root = "codex/plugins/cache/test/demo/local"; + write( + temp.path(), + &format!("{plugin_root}/.codex-plugin/plugin.json"), + MANIFEST, + )?; + for root in [ + "codex/skills/deploy".to_string(), + format!("{plugin_root}/skills/deploy"), + ] { + write(temp.path(), &format!("{root}/SKILL.md"), SKILL)?; + if let Some(yaml) = yaml { + write(temp.path(), &format!("{root}/agents/openai.yaml"), yaml)?; + } + } + let manager = CapabilityManager::new(LocalFileSystem::unsandboxed()); + manager.seed_prewarmed_locations(request.clone()).await?; + let response = manager + .get_or_refresh_discovery(DiscoverV2CapabilitiesRequest { + cwd: PathUri::from_host_native_path(temp.path().join("project"))?, + sandbox: None, + }) + .await?; + let mut expected = + expected_skill(request.codex_home.join("skills/deploy/SKILL.md")?, "deploy"); + expected.dependencies = dependencies; + expected.policy = policy; + expected.discovery_root = Some(SkillDiscoveryRoot { + id: "codex-home-skills".to_string(), + path: request.codex_home.join("skills")?, + }); + assert_eq!(response.skills, vec![expected.clone()]); + expected.path = request + .codex_home + .join("plugins/cache/test/demo/local/skills/deploy/SKILL.md")?; + expected.name = "demo:deploy".into(); + expected.discovery_root = None; + assert_eq!(response.plugins[0].skills, vec![expected]); + assert_eq!(response.warnings, Vec::::new()); + } + Ok(()) +} + +#[tokio::test] +async fn standalone_skills_retain_their_search_roots() -> anyhow::Result<()> { + let temp = tempfile::tempdir()?; + let request = request(temp.path())?; + let roots = [ + ("codex-home-skills", "codex/skills"), + ("user-agent-skills", "home/.agents/skills"), + ("system-skills", "etc/codex/skills"), + ]; + let mut expected = Vec::new(); + for (id, root) in roots { + for relative in ["deploy/SKILL.md", "nested/deploy/SKILL.md"] { + let path = format!("{root}/{relative}"); + write(temp.path(), &path, SKILL)?; + let mut skill = expected_skill( + PathUri::from_host_native_path(temp.path().join(path))?, + "deploy", + ); + skill.discovery_root = Some(SkillDiscoveryRoot { + id: id.to_string(), + path: PathUri::from_host_native_path(temp.path().join(root))?, + }); + expected.push(skill); + } + } + let manager = CapabilityManager::new(LocalFileSystem::unsandboxed()); + manager.seed_prewarmed_locations(request.clone()).await?; + let query = DiscoverV2CapabilitiesRequest { + cwd: PathUri::from_host_native_path(temp.path().join("project"))?, + sandbox: None, + }; + let response = manager.get_or_refresh_discovery(query.clone()).await?; + assert_eq!(response.skills, expected); + // Cached discovery must retain the same root metadata. + assert_eq!(manager.get_or_refresh_discovery(query).await?, response); + Ok(()) +} diff --git a/codex-rs/exec-server/src/discoverV2/tests/capability_locations_tests.rs b/codex-rs/exec-server/src/discoverV2/tests/capability_locations_tests.rs index 280b1b60ba..5b62a2abb9 100644 --- a/codex-rs/exec-server/src/discoverV2/tests/capability_locations_tests.rs +++ b/codex-rs/exec-server/src/discoverV2/tests/capability_locations_tests.rs @@ -67,15 +67,21 @@ async fn prewarming_resolves_global_locations_without_cwd_or_config() -> anyhow: actual.sort_unstable_by_key(|location| location.root.to_string()); let mut expected = vec![CapabilityLocation { root: request.codex_home.join("plugins/cache/team/demo/1.10.0")?, + standalone_root_id: None, plugin: Some(InstalledPlugin { id: "demo@team".to_string(), version: "1.10.0".to_string(), remote_plugin_id: Some("plugins~demo".to_string()), }), }]; - for root in ["codex/skills", "home/.agents/skills", "etc/codex/skills"] { + for (id, root) in [ + ("codex-home-skills", "codex/skills"), + ("user-agent-skills", "home/.agents/skills"), + ("system-skills", "etc/codex/skills"), + ] { expected.push(CapabilityLocation { root: PathUri::from_host_native_path(temp.path().join(root))?, + standalone_root_id: Some(id.to_string()), plugin: None, }); } diff --git a/codex-rs/exec-server/src/lib.rs b/codex-rs/exec-server/src/lib.rs index 93d7c98fbd..cd928c0fed 100644 --- a/codex-rs/exec-server/src/lib.rs +++ b/codex-rs/exec-server/src/lib.rs @@ -228,6 +228,7 @@ pub use protocol::ShellInfo; pub use protocol::ShellSnapshotRequest; pub use protocol::SignalParams; pub use protocol::SignalResponse; +pub use protocol::SkillDiscoveryRoot; pub use protocol::TerminateParams; pub use protocol::TerminateResponse; pub use protocol::WriteParams; diff --git a/codex-rs/exec-server/tests/capabilities_v2.rs b/codex-rs/exec-server/tests/capabilities_v2.rs index 3eacf46f85..2cc49bac28 100644 --- a/codex-rs/exec-server/tests/capabilities_v2.rs +++ b/codex-rs/exec-server/tests/capabilities_v2.rs @@ -6,10 +6,12 @@ use codex_exec_server::CAPABILITIES_DISCOVER_V2_METHOD; use codex_exec_server::DiscoverV2CapabilitiesRequest; use codex_exec_server::DiscoverV2CapabilitiesResponse; use codex_exec_server::InitializeResponse; +use codex_exec_server::SkillDiscoveryRoot; use codex_exec_server_protocol::JSONRPCMessage; use codex_utils_path_uri::PathUri; use common::exec_server::ExecServerHarness; use common::exec_server::exec_server_with_env; +use pretty_assertions::assert_eq; #[cfg(any(target_os = "linux", target_os = "macos", windows))] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -71,6 +73,19 @@ async fn v2_rpc_refreshes_metadata_and_enforces_sandbox() -> anyhow::Result<()> .iter() .any(|skill| skill.name == "demo" && skill.description == "original") ); + let expected_root = Some(SkillDiscoveryRoot { + id: "codex-home-skills".to_string(), + path: PathUri::from_host_native_path(codex_home_path.join("skills"))?, + }); + assert_eq!( + first + .skills + .iter() + .find(|skill| skill.name == "demo") + .context("demo skill")? + .discovery_root, + expected_root, + ); assert!(!serde_json::to_string(&first)?.contains("PRIVATE INSTRUCTION BODY")); assert!( @@ -131,7 +146,15 @@ async fn v2_rpc_refreshes_metadata_and_enforces_sandbox() -> anyhow::Result<()> { let response = discover(&mut server, &restricted).await?; assert!(response.plugins.is_empty()); - assert!(response.skills.iter().any(|skill| skill.name == "demo")); + assert_eq!( + response + .skills + .iter() + .find(|skill| skill.name == "demo") + .context("sandboxed demo skill")? + .discovery_root, + expected_root, + ); assert!(!serde_json::to_string(&response)?.contains("mail@market")); } // Windows restricted tokens reject split read access; a supported read-only policy succeeds. @@ -163,7 +186,15 @@ async fn v2_rpc_refreshes_metadata_and_enforces_sandbox() -> anyhow::Result<()> .iter() .any(|plugin| plugin.id == "mail@market") ); - assert!(response.skills.iter().any(|skill| skill.name == "demo")); + assert_eq!( + response + .skills + .iter() + .find(|skill| skill.name == "demo") + .context("sandboxed demo skill")? + .discovery_root, + expected_root, + ); } } diff --git a/codex-rs/ext/skills/Cargo.toml b/codex-rs/ext/skills/Cargo.toml index 80bcfba8cb..96019ce3f9 100644 --- a/codex-rs/ext/skills/Cargo.toml +++ b/codex-rs/ext/skills/Cargo.toml @@ -32,7 +32,6 @@ futures = { workspace = true } schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } -serde_yaml = { workspace = true } tokio = { workspace = true, features = ["rt", "sync", "time"] } toml = { workspace = true } tracing = { workspace = true } diff --git a/codex-rs/ext/skills/src/loader/environment.rs b/codex-rs/ext/skills/src/loader/environment.rs index c887ef41c0..b674291d67 100644 --- a/codex-rs/ext/skills/src/loader/environment.rs +++ b/codex-rs/ext/skills/src/loader/environment.rs @@ -24,12 +24,10 @@ use super::discovery::MAX_CONCURRENT_SKILL_LOADS; use super::discovery::SkillDiscoveryOptions; use super::discovery::SkillMetadataDiscovery; use super::discovery::discover_skills; -use super::metadata::SkillMetadataFile; -use super::metadata::resolve_dependencies; -use super::metadata::resolve_policy; use super::metadata::sanitize_single_line; use super::metadata::validate_len; use super::namespace::SkillNamespaceResolver; +use codex_skills::parse_skill_metadata; struct ParsedEnvironmentSkill { path_to_skills_md: PathUri, @@ -269,7 +267,7 @@ pub fn load_environment_skills_from_discovery( .metadata .as_ref() .and_then(|metadata| { - serde_yaml::from_str::(&metadata.contents) + parse_skill_metadata(&metadata.contents) .map_err(|error| { tracing::warn!( path = %metadata.path, @@ -278,12 +276,7 @@ pub fn load_environment_skills_from_discovery( }) .ok() }) - .map(|metadata| { - ( - resolve_dependencies(metadata.dependencies), - resolve_policy(metadata.policy), - ) - }) + .map(|metadata| (metadata.dependencies, metadata.policy)) .unwrap_or((None, None)); let metadata = EnvironmentSkillMetadata { path_to_skills_md: skill.instructions.path.clone(), @@ -367,7 +360,7 @@ async fn read_skill_metadata( return (None, None); } }; - let parsed: SkillMetadataFile = match serde_yaml::from_str(&contents) { + let parsed = match parse_skill_metadata(&contents) { Ok(parsed) => parsed, Err(error) => { tracing::warn!("ignoring {metadata_path}: invalid metadata: {error}"); @@ -375,10 +368,7 @@ async fn read_skill_metadata( } }; - ( - resolve_dependencies(parsed.dependencies), - resolve_policy(parsed.policy), - ) + (parsed.dependencies, parsed.policy) } fn default_skill_name(path: &PathUri) -> String { diff --git a/codex-rs/ext/skills/src/loader/metadata.rs b/codex-rs/ext/skills/src/loader/metadata.rs index 0527b3c881..7c0299d443 100644 --- a/codex-rs/ext/skills/src/loader/metadata.rs +++ b/codex-rs/ext/skills/src/loader/metadata.rs @@ -3,37 +3,19 @@ use std::io; use codex_exec_server::ExecutorFileSystem; use codex_exec_server::GetMetadataOptions; use codex_exec_server::ReadFileOptions; -use codex_protocol::protocol::Product; +use codex_skills::ParsedSkillMetadata; use codex_skills::SkillDependencies; use codex_skills::SkillInterface; use codex_skills::SkillInterfaceAssetPolicy; -use codex_skills::SkillInterfaceFile; use codex_skills::SkillParseError; use codex_skills::SkillPolicy; -use codex_skills::SkillToolDependency; +use codex_skills::parse_skill_metadata; use codex_skills::resolve_skill_interface; use codex_utils_absolute_path::AbsolutePathBuf; -use serde::Deserialize; -use super::MAX_DEPENDENCY_COMMAND_LEN; -use super::MAX_DEPENDENCY_DESCRIPTION_LEN; -use super::MAX_DEPENDENCY_TRANSPORT_LEN; -use super::MAX_DEPENDENCY_TYPE_LEN; -use super::MAX_DEPENDENCY_URL_LEN; -use super::MAX_DEPENDENCY_VALUE_LEN; use super::SKILLS_METADATA_FILENAME; use super::discovery::SkillMetadataDiscovery; -#[derive(Debug, Default, Deserialize)] -pub(super) struct SkillMetadataFile { - #[serde(default)] - interface: Option, - #[serde(default)] - pub(super) dependencies: Option, - #[serde(default)] - pub(super) policy: Option, -} - #[derive(Default)] pub(super) struct LoadedSkillMetadata { pub(super) interface: Option, @@ -41,39 +23,6 @@ pub(super) struct LoadedSkillMetadata { pub(super) policy: Option, } -#[derive(Debug, Default, Deserialize)] -pub(super) struct Dependencies { - #[serde(default)] - tools: Vec, -} - -#[derive(Debug, Deserialize)] -pub(super) struct Policy { - #[serde(default)] - allow_implicit_invocation: Option, - #[serde(default)] - products: Vec, -} - -#[derive(Debug, Default, Deserialize)] -struct DependencyTool { - #[serde(rename = "type")] - kind: Option, - value: Option, - description: Option, - transport: Option, - command: Option, - url: Option, - oauth: Option, -} - -#[derive(Debug, Default, Deserialize)] -#[serde(rename_all = "camelCase")] -struct DependencyOAuth { - #[serde(alias = "callback_port")] - callback_port: Option, -} - pub(super) async fn load_host_skill_metadata( file_system: &dyn ExecutorFileSystem, skill_path: &AbsolutePathBuf, @@ -127,7 +76,7 @@ pub(super) async fn load_host_skill_metadata( } }; - let parsed: SkillMetadataFile = match serde_yaml::from_str(&contents) { + let parsed = match parse_skill_metadata(&contents) { Ok(parsed) => parsed, Err(error) => { tracing::warn!( @@ -138,7 +87,7 @@ pub(super) async fn load_host_skill_metadata( } }; - let SkillMetadataFile { + let ParsedSkillMetadata { interface, dependencies, policy, @@ -149,73 +98,10 @@ pub(super) async fn load_host_skill_metadata( }; LoadedSkillMetadata { interface: resolve_skill_interface(interface, &skill_dir, asset_policy), - dependencies: resolve_dependencies(dependencies), - policy: resolve_policy(policy), + dependencies, + policy, } } -pub(super) fn resolve_dependencies( - dependencies: Option, -) -> Option { - let dependencies = dependencies?; - let tools = dependencies - .tools - .into_iter() - .filter_map(resolve_dependency_tool) - .collect::>(); - if tools.is_empty() { - None - } else { - Some(SkillDependencies { tools }) - } -} - -pub(super) fn resolve_policy(policy: Option) -> Option { - policy.map(|policy| SkillPolicy { - allow_implicit_invocation: policy.allow_implicit_invocation, - products: policy.products, - }) -} - -fn resolve_dependency_tool(tool: DependencyTool) -> Option { - let r#type = resolve_required_str( - tool.kind, - MAX_DEPENDENCY_TYPE_LEN, - "dependencies.tools.type", - )?; - let value = resolve_required_str( - tool.value, - MAX_DEPENDENCY_VALUE_LEN, - "dependencies.tools.value", - )?; - let description = resolve_str( - tool.description, - MAX_DEPENDENCY_DESCRIPTION_LEN, - "dependencies.tools.description", - ); - let transport = resolve_str( - tool.transport, - MAX_DEPENDENCY_TRANSPORT_LEN, - "dependencies.tools.transport", - ); - let command = resolve_str( - tool.command, - MAX_DEPENDENCY_COMMAND_LEN, - "dependencies.tools.command", - ); - let url = resolve_str(tool.url, MAX_DEPENDENCY_URL_LEN, "dependencies.tools.url"); - let oauth_callback_port = tool.oauth.and_then(|oauth| oauth.callback_port); - - Some(SkillToolDependency { - r#type, - value, - description, - transport, - command, - url, - oauth_callback_port, - }) -} - pub(super) fn sanitize_single_line(raw: &str) -> String { raw.split_whitespace().collect::>().join(" ") } @@ -236,33 +122,3 @@ pub(super) fn validate_len( } Ok(()) } - -pub(super) fn resolve_str( - value: Option, - max_len: usize, - field: &'static str, -) -> Option { - let value = value?; - let value = sanitize_single_line(&value); - if value.is_empty() { - tracing::warn!("ignoring {field}: value is empty"); - return None; - } - if value.chars().count() > max_len { - tracing::warn!("ignoring {field}: exceeds maximum length of {max_len} characters"); - return None; - } - Some(value) -} - -fn resolve_required_str( - value: Option, - max_len: usize, - field: &'static str, -) -> Option { - let Some(value) = value else { - tracing::warn!("ignoring {field}: value is missing"); - return None; - }; - resolve_str(Some(value), max_len, field) -} diff --git a/codex-rs/ext/skills/src/loader/mod.rs b/codex-rs/ext/skills/src/loader/mod.rs index 0802dc670c..55df37677b 100644 --- a/codex-rs/ext/skills/src/loader/mod.rs +++ b/codex-rs/ext/skills/src/loader/mod.rs @@ -21,12 +21,5 @@ pub(super) const SKILLS_METADATA_DIR: &str = "agents"; pub(super) const SKILLS_METADATA_FILENAME: &str = "openai.yaml"; pub(super) const MAX_NAME_LEN: usize = 64; pub(super) const MAX_QUALIFIED_NAME_LEN: usize = MAX_NAME_LEN * 2 + 1; -pub(super) const MAX_DESCRIPTION_LEN: usize = 1024; -pub(super) const MAX_DEPENDENCY_TYPE_LEN: usize = MAX_NAME_LEN; -pub(super) const MAX_DEPENDENCY_TRANSPORT_LEN: usize = MAX_NAME_LEN; -pub(super) const MAX_DEPENDENCY_VALUE_LEN: usize = MAX_DESCRIPTION_LEN; -pub(super) const MAX_DEPENDENCY_DESCRIPTION_LEN: usize = MAX_DESCRIPTION_LEN; -pub(super) const MAX_DEPENDENCY_COMMAND_LEN: usize = MAX_DESCRIPTION_LEN; -pub(super) const MAX_DEPENDENCY_URL_LEN: usize = MAX_DESCRIPTION_LEN; pub(super) const MAX_SCAN_DEPTH: usize = 6; pub(super) const MAX_SKILLS_DIRS_PER_ROOT: usize = 2000; diff --git a/codex-rs/skills/src/lib.rs b/codex-rs/skills/src/lib.rs index 7b19d32788..7ac553c8d1 100644 --- a/codex-rs/skills/src/lib.rs +++ b/codex-rs/skills/src/lib.rs @@ -2,6 +2,7 @@ mod interface; mod invocation; mod loading; mod mentions; +mod metadata; mod model; mod name_counts; mod parser; @@ -30,6 +31,8 @@ pub use mentions::extract_tool_mentions_with_sigil; pub use mentions::normalize_skill_path; pub use mentions::plugin_config_name_from_path; pub use mentions::tool_kind_for_path; +pub use metadata::ParsedSkillMetadata; +pub use metadata::parse_skill_metadata; pub use model::EnvironmentSkillMetadata; pub use model::SkillDependencies; pub use model::SkillInterface; diff --git a/codex-rs/skills/src/metadata.rs b/codex-rs/skills/src/metadata.rs new file mode 100644 index 0000000000..bb377e1773 --- /dev/null +++ b/codex-rs/skills/src/metadata.rs @@ -0,0 +1,164 @@ +//! Parses optional skill YAML metadata with shared host and executor validation. + +use codex_protocol::protocol::Product; +use serde::Deserialize; + +use crate::SkillDependencies; +use crate::SkillInterfaceFile; +use crate::SkillPolicy; +use crate::SkillToolDependency; + +const MAX_DEPENDENCY_TYPE_LEN: usize = 64; +const MAX_DEPENDENCY_TRANSPORT_LEN: usize = 64; +const MAX_DEPENDENCY_VALUE_LEN: usize = 1024; +const MAX_DEPENDENCY_DESCRIPTION_LEN: usize = 1024; +const MAX_DEPENDENCY_COMMAND_LEN: usize = 1024; +const MAX_DEPENDENCY_URL_LEN: usize = 1024; + +/// Parsed optional metadata; callers resolve interface assets for their filesystem. +#[derive(Debug, Default)] +pub struct ParsedSkillMetadata { + pub interface: Option, + pub dependencies: Option, + pub policy: Option, +} + +/// Parses `agents/openai.yaml`, dropping invalid individual dependency declarations. +pub fn parse_skill_metadata(contents: &str) -> Result { + let parsed: SkillMetadataFile = serde_yaml::from_str(contents)?; + Ok(ParsedSkillMetadata { + interface: parsed.interface, + dependencies: resolve_dependencies(parsed.dependencies), + policy: resolve_policy(parsed.policy), + }) +} + +#[derive(Debug, Default, Deserialize)] +struct SkillMetadataFile { + #[serde(default)] + interface: Option, + #[serde(default)] + dependencies: Option, + #[serde(default)] + policy: Option, +} + +#[derive(Debug, Default, Deserialize)] +struct Dependencies { + #[serde(default)] + tools: Vec, +} + +#[derive(Debug, Deserialize)] +struct Policy { + #[serde(default)] + allow_implicit_invocation: Option, + #[serde(default)] + products: Vec, +} + +#[derive(Debug, Default, Deserialize)] +struct DependencyTool { + #[serde(rename = "type")] + kind: Option, + value: Option, + description: Option, + transport: Option, + command: Option, + url: Option, + oauth: Option, +} + +#[derive(Debug, Default, Deserialize)] +#[serde(rename_all = "camelCase")] +struct DependencyOAuth { + #[serde(alias = "callback_port")] + callback_port: Option, +} + +fn resolve_dependencies(dependencies: Option) -> Option { + let dependencies = dependencies?; + let tools = dependencies + .tools + .into_iter() + .filter_map(resolve_dependency_tool) + .collect::>(); + if tools.is_empty() { + None + } else { + Some(SkillDependencies { tools }) + } +} + +fn resolve_policy(policy: Option) -> Option { + policy.map(|policy| SkillPolicy { + allow_implicit_invocation: policy.allow_implicit_invocation, + products: policy.products, + }) +} + +fn resolve_dependency_tool(tool: DependencyTool) -> Option { + let r#type = resolve_required_str( + tool.kind, + MAX_DEPENDENCY_TYPE_LEN, + "dependencies.tools.type", + )?; + let value = resolve_required_str( + tool.value, + MAX_DEPENDENCY_VALUE_LEN, + "dependencies.tools.value", + )?; + let description = resolve_str( + tool.description, + MAX_DEPENDENCY_DESCRIPTION_LEN, + "dependencies.tools.description", + ); + let transport = resolve_str( + tool.transport, + MAX_DEPENDENCY_TRANSPORT_LEN, + "dependencies.tools.transport", + ); + let command = resolve_str( + tool.command, + MAX_DEPENDENCY_COMMAND_LEN, + "dependencies.tools.command", + ); + let url = resolve_str(tool.url, MAX_DEPENDENCY_URL_LEN, "dependencies.tools.url"); + let oauth_callback_port = tool.oauth.and_then(|oauth| oauth.callback_port); + + Some(SkillToolDependency { + r#type, + value, + description, + transport, + command, + url, + oauth_callback_port, + }) +} + +fn resolve_str(value: Option, max_len: usize, field: &'static str) -> Option { + let value = value?; + let value = value.split_whitespace().collect::>().join(" "); + if value.is_empty() { + tracing::warn!("ignoring {field}: value is empty"); + return None; + } + if value.chars().count() > max_len { + tracing::warn!("ignoring {field}: exceeds maximum length of {max_len} characters"); + return None; + } + Some(value) +} + +fn resolve_required_str( + value: Option, + max_len: usize, + field: &'static str, +) -> Option { + let Some(value) = value else { + tracing::warn!("ignoring {field}: value is missing"); + return None; + }; + resolve_str(Some(value), max_len, field) +} diff --git a/codex-rs/skills/src/model.rs b/codex-rs/skills/src/model.rs index 13a4764e84..c2d0e57376 100644 --- a/codex-rs/skills/src/model.rs +++ b/codex-rs/skills/src/model.rs @@ -59,7 +59,8 @@ impl EnvironmentSkillMetadata { } } -#[derive(Debug, Clone, PartialEq, Eq, Default)] +#[derive(Debug, Clone, PartialEq, Eq, Default, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] pub struct SkillPolicy { pub allow_implicit_invocation: Option, // TODO: Enforce product gating in Codex skill selection/injection instead of only parsing and @@ -77,12 +78,14 @@ pub struct SkillInterface { pub default_prompt: Option, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] pub struct SkillDependencies { pub tools: Vec, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] pub struct SkillToolDependency { pub r#type: String, pub value: String,