From 1ad0d7aa4b92b8bfcae0272a29609b27fc334ad9 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Mon, 1 Jun 2026 17:19:34 -0700 Subject: [PATCH] Handle invalid plugin skills manifest field (#25717) ## Summary - Treat invalid `plugin.json` `skills` shapes as a field-level warning instead of rejecting the whole manifest - Keep valid string path behavior unchanged and continue falling back to the default `skills/` root - Add regression coverage for array-shaped `skills` ## Tests - `just fmt` - `cargo test -p codex-core-plugins` --- codex-rs/core-plugins/src/manager_tests.rs | 37 ++++++++++++++++++++++ codex-rs/core-plugins/src/manifest.rs | 28 ++++++++++++++-- 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index f4b6c49da..905d120a8 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -959,6 +959,43 @@ async fn load_plugins_ignores_manifest_component_paths_without_dot_slash() { ); } +#[tokio::test] +async fn load_plugins_ignores_invalid_manifest_skills_shape() { + let codex_home = TempDir::new().unwrap(); + let plugin_root = codex_home + .path() + .join("plugins/cache") + .join("test/sample/local"); + + write_file( + &plugin_root.join(".codex-plugin/plugin.json"), + r#"{ + "name": "sample", + "skills": ["./custom-skills/"] +}"#, + ); + write_file( + &plugin_root.join("skills/default-skill/SKILL.md"), + "---\nname: default-skill\ndescription: default skill\n---\n", + ); + write_file( + &plugin_root.join("custom-skills/custom-skill/SKILL.md"), + "---\nname: custom-skill\ndescription: custom skill\n---\n", + ); + + let outcome = load_plugins_from_config( + &plugin_config_toml(/*enabled*/ true, /*plugins_feature_enabled*/ true), + codex_home.path(), + ) + .await; + + assert_eq!(outcome.plugins()[0].error, None); + assert_eq!( + outcome.plugins()[0].skill_roots, + vec![plugin_root.join("skills").abs()] + ); +} + #[tokio::test] async fn load_plugins_preserves_disabled_plugins_without_effective_contributions() { let codex_home = TempDir::new().unwrap(); diff --git a/codex-rs/core-plugins/src/manifest.rs b/codex-rs/core-plugins/src/manifest.rs index 6de7f820b..606ca8147 100644 --- a/codex-rs/core-plugins/src/manifest.rs +++ b/codex-rs/core-plugins/src/manifest.rs @@ -23,7 +23,7 @@ struct RawPluginManifest { // Keep manifest paths as raw strings so we can validate the required `./...` syntax before // resolving them under the plugin root. #[serde(default)] - skills: Option, + skills: Option, #[serde(default)] mcp_servers: Option, #[serde(default)] @@ -127,6 +127,13 @@ enum RawPluginManifestDefaultPromptEntry { Invalid(JsonValue), } +#[derive(Debug, Deserialize)] +#[serde(untagged)] +enum RawPluginManifestPath { + Path(String), + Invalid(JsonValue), +} + #[derive(Debug, Deserialize)] #[serde(untagged)] enum RawPluginManifestHooks { @@ -238,7 +245,7 @@ pub fn load_plugin_manifest(plugin_root: &Path) -> Option { description, keywords, paths: PluginManifestPaths { - skills: resolve_manifest_path(plugin_root, "skills", skills.as_deref()), + skills: resolve_manifest_path_value(plugin_root, "skills", skills.as_ref()), mcp_servers: resolve_manifest_path( plugin_root, "mcpServers", @@ -394,6 +401,23 @@ fn json_value_type(value: &JsonValue) -> &'static str { } } +fn resolve_manifest_path_value( + plugin_root: &Path, + field: &'static str, + path: Option<&RawPluginManifestPath>, +) -> Option { + match path? { + RawPluginManifestPath::Path(path) => resolve_manifest_path(plugin_root, field, Some(path)), + RawPluginManifestPath::Invalid(value) => { + tracing::warn!( + "ignoring {field}: expected a string; found {}", + json_value_type(value) + ); + None + } + } +} + fn resolve_manifest_path( plugin_root: &Path, field: &'static str,