diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index ba27209d9..fb542f045 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -2,6 +2,7 @@ use crate::app_mcp_routing::apply_app_mcp_routing_policy; use crate::app_mcp_routing::apps_route_available; use crate::is_openai_curated_marketplace_name; use crate::manifest::PluginManifestHooks; +use crate::manifest::PluginManifestMcpServers; use crate::manifest::PluginManifestPaths; use crate::manifest::load_plugin_manifest; use crate::marketplace::MarketplacePluginSource; @@ -724,25 +725,12 @@ async fn load_plugin( let has_enabled_skills = resolved_skills.has_enabled_skills(); loaded_plugin.disabled_skill_paths = resolved_skills.disabled_skill_paths; loaded_plugin.has_enabled_skills = has_enabled_skills; - let mut mcp_servers = HashMap::new(); - for mcp_config_path in plugin_mcp_config_paths(plugin_root.as_path(), manifest_paths) { - let plugin_mcp = - load_mcp_servers_from_file(plugin_root.as_path(), &mcp_config_path).await; - for (name, mut config) in plugin_mcp.mcp_servers { - if let Some(policy) = plugin.mcp_servers.get(&name) { - apply_plugin_mcp_server_policy(&mut config, policy); - } - if mcp_servers.insert(name.clone(), config).is_some() { - warn!( - plugin = %plugin_root.display(), - path = %mcp_config_path.display(), - server = name, - "plugin MCP file overwrote an earlier server definition" - ); - } - } - } - loaded_plugin.mcp_servers = mcp_servers; + loaded_plugin.mcp_servers = load_plugin_mcp_servers_from_manifest( + plugin_root.as_path(), + manifest_paths, + Some(&plugin.mcp_servers), + ) + .await; loaded_plugin.apps = load_plugin_apps(plugin_root.as_path()).await; } PluginLoadScope::HooksOnly => {} @@ -853,7 +841,7 @@ fn plugin_mcp_config_paths( plugin_root: &Path, manifest_paths: &PluginManifestPaths, ) -> Vec { - if let Some(path) = &manifest_paths.mcp_servers { + if let Some(PluginManifestMcpServers::Path(path)) = &manifest_paths.mcp_servers { return vec![path.clone()]; } default_mcp_config_paths(plugin_root) @@ -1094,15 +1082,14 @@ pub async fn plugin_telemetry_metadata_from_root( let manifest_paths = &manifest.paths; let has_skills = !plugin_skill_roots(plugin_root, manifest_paths).is_empty(); - let mut mcp_server_names = Vec::new(); - for path in plugin_mcp_config_paths(plugin_root.as_path(), manifest_paths) { - mcp_server_names.extend( - load_mcp_servers_from_file(plugin_root.as_path(), &path) - .await - .mcp_servers - .into_keys(), - ); - } + let mut mcp_server_names = load_plugin_mcp_servers_from_manifest( + plugin_root.as_path(), + manifest_paths, + /*plugin_policy*/ None, + ) + .await + .into_keys() + .collect::>(); mcp_server_names.sort_unstable(); mcp_server_names.dedup(); @@ -1151,11 +1138,49 @@ async fn load_declared_plugin_mcp_servers(plugin_root: &Path) -> HashMap>, +) -> HashMap { let mut mcp_servers = HashMap::new(); - for mcp_config_path in plugin_mcp_config_paths(plugin_root, &manifest.paths) { - let plugin_mcp = load_mcp_servers_from_file(plugin_root, &mcp_config_path).await; - for (name, config) in plugin_mcp.mcp_servers { - mcp_servers.entry(name).or_insert(config); + match &manifest_paths.mcp_servers { + Some(PluginManifestMcpServers::Object(object_servers)) => { + let plugin_mcp = load_mcp_servers_from_manifest_object(plugin_root, object_servers); + for (name, mut config) in plugin_mcp.mcp_servers { + if let Some(policy) = plugin_policy.and_then(|policy| policy.get(&name)) { + apply_plugin_mcp_server_policy(&mut config, policy); + } + if mcp_servers.insert(name.clone(), config).is_some() { + warn!( + plugin = %plugin_root.display(), + server = name, + "plugin manifest MCP object overwrote an earlier server definition" + ); + } + } + } + Some(PluginManifestMcpServers::Path(_)) | None => { + for mcp_config_path in plugin_mcp_config_paths(plugin_root, manifest_paths) { + let plugin_mcp = load_mcp_servers_from_file(plugin_root, &mcp_config_path).await; + for (name, mut config) in plugin_mcp.mcp_servers { + if let Some(policy) = plugin_policy.and_then(|policy| policy.get(&name)) { + apply_plugin_mcp_server_policy(&mut config, policy); + } + if mcp_servers.insert(name.clone(), config).is_some() { + warn!( + plugin = %plugin_root.display(), + path = %mcp_config_path.display(), + server = name, + "plugin MCP file overwrote an earlier server definition" + ); + } + } + } } } @@ -1212,6 +1237,37 @@ async fn load_mcp_servers_from_file( } } +fn load_mcp_servers_from_manifest_object( + plugin_root: &Path, + object_config: &str, +) -> PluginMcpDiscovery { + let parsed = match parse_plugin_mcp_config( + plugin_root, + object_config, + PluginMcpServerPlacement::Declared, + ) { + Ok(parsed) => parsed, + Err(err) => { + warn!( + plugin = %plugin_root.display(), + "failed to parse plugin manifest MCP object: {err}" + ); + return PluginMcpDiscovery::default(); + } + }; + for error in parsed.errors { + warn!( + plugin = %plugin_root.display(), + server = error.name, + error = error.message, + "failed to parse plugin manifest MCP object server" + ); + } + PluginMcpDiscovery { + mcp_servers: parsed.servers.into_iter().collect(), + } +} + #[derive(Debug, Default)] struct PluginMcpDiscovery { mcp_servers: HashMap, diff --git a/codex-rs/core-plugins/src/manager_tests.rs b/codex-rs/core-plugins/src/manager_tests.rs index 7c88d114a..c2917211d 100644 --- a/codex-rs/core-plugins/src/manager_tests.rs +++ b/codex-rs/core-plugins/src/manager_tests.rs @@ -682,6 +682,70 @@ async fn load_plugins_loads_default_skills_and_mcp_servers() { ); } +#[tokio::test] +async fn load_plugins_loads_manifest_mcp_server_objects() { + let codex_home = TempDir::new().unwrap(); + let plugin_root = codex_home + .path() + .join("plugins/cache") + .join("test/counter-sample/local"); + + write_file( + &plugin_root.join(".codex-plugin/plugin.json"), + r#"{ + "name": "counter-sample", + "version": "1.1.1", + "description": "Plugin that declares MCP servers in the manifest", + "mcpServers": { + "counter": { + "type": "http", + "url": "https://sample.example/counter/mcp" + } + } +}"#, + ); + + let config_toml = r#" +[features] +plugins = true + +[plugins."counter-sample@test"] +enabled = true +"#; + let outcome = + load_plugins_from_config(config_toml, codex_home.path(), /*auth_mode*/ None).await; + + assert_eq!(outcome.plugins()[0].error, None); + assert_eq!( + outcome.plugins()[0].mcp_servers, + HashMap::from([( + "counter".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::StreamableHttp { + url: "https://sample.example/counter/mcp".to_string(), + bearer_token_env_var: None, + http_headers: None, + env_http_headers: None, + }, + environment_id: "local".to_string(), + enabled: true, + required: false, + supports_parallel_tool_calls: false, + disabled_reason: None, + startup_timeout_sec: None, + tool_timeout_sec: None, + default_tools_approval_mode: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + oauth: None, + oauth_resource: None, + tools: HashMap::new(), + }, + )]) + ); +} + #[tokio::test] async fn load_plugins_applies_plugin_mcp_server_policy() { let codex_home = TempDir::new().unwrap(); @@ -1098,6 +1162,47 @@ async fn plugin_telemetry_metadata_uses_default_mcp_config_path() { ); } +#[tokio::test] +async fn plugin_telemetry_metadata_uses_manifest_mcp_server_objects() { + let codex_home = TempDir::new().unwrap(); + let plugin_root = codex_home + .path() + .join("plugins/cache") + .join("test/counter-sample/local"); + + write_file( + &plugin_root.join(".codex-plugin/plugin.json"), + r#"{ + "name": "counter-sample", + "version": "1.1.1", + "mcpServers": { + "counter": { + "type": "http", + "url": "https://sample.example/counter/mcp" + } + } +}"#, + ); + + let metadata = plugin_telemetry_metadata_from_root( + &PluginId::parse("counter-sample@test").expect("plugin id should parse"), + &plugin_root.abs(), + ) + .await; + + assert_eq!( + metadata.capability_summary, + Some(PluginCapabilitySummary { + config_name: "counter-sample@test".to_string(), + display_name: "counter-sample".to_string(), + description: None, + has_skills: false, + mcp_server_names: vec!["counter".to_string()], + app_connector_ids: Vec::new(), + }) + ); +} + #[tokio::test] async fn capability_summary_sanitizes_plugin_descriptions_to_one_line() { 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 28a200ab8..fe0bb2564 100644 --- a/codex-rs/core-plugins/src/manifest.rs +++ b/codex-rs/core-plugins/src/manifest.rs @@ -12,6 +12,8 @@ const MAX_DEFAULT_PROMPT_LEN: usize = 128; pub type PluginManifest = codex_plugin::manifest::PluginManifest; pub type PluginManifestHooks = codex_plugin::manifest::PluginManifestHooks; pub type PluginManifestInterface = codex_plugin::manifest::PluginManifestInterface; +pub type PluginManifestMcpServers = + codex_plugin::manifest::PluginManifestMcpServers; pub type PluginManifestPaths = codex_plugin::manifest::PluginManifestPaths; #[derive(Debug, Default, Deserialize)] @@ -30,7 +32,7 @@ struct RawPluginManifest { #[serde(default)] skills: Option, #[serde(default)] - mcp_servers: Option, + mcp_servers: Option, #[serde(default)] apps: Option, #[serde(default)] @@ -97,6 +99,14 @@ enum RawPluginManifestPath { Invalid(JsonValue), } +#[derive(Debug, Deserialize)] +#[serde(untagged)] +enum RawPluginManifestMcpServers { + Path(String), + Object(std::collections::BTreeMap), + Invalid(JsonValue), +} + #[derive(Debug, Deserialize)] #[serde(untagged)] enum RawPluginManifestHooks { @@ -221,7 +231,7 @@ pub(crate) fn parse_plugin_manifest( keywords, paths: PluginManifestPaths { skills: resolve_manifest_path_value(plugin_root, "skills", skills.as_ref()), - mcp_servers: resolve_manifest_path(plugin_root, "mcpServers", mcp_servers.as_deref()), + mcp_servers: resolve_manifest_mcp_servers(plugin_root, mcp_servers), apps: resolve_manifest_path(plugin_root, "apps", apps.as_deref()), hooks: resolve_manifest_hooks(plugin_root, hooks), }, @@ -259,6 +269,32 @@ fn resolve_manifest_hooks( } } +fn resolve_manifest_mcp_servers( + plugin_root: &Path, + mcp_servers: Option, +) -> Option { + match mcp_servers? { + RawPluginManifestMcpServers::Path(path) => { + resolve_manifest_path(plugin_root, "mcpServers", Some(&path)) + .map(PluginManifestMcpServers::Path) + } + RawPluginManifestMcpServers::Object(servers) => match serde_json::to_string(&servers) { + Ok(servers) => Some(PluginManifestMcpServers::Object(servers)), + Err(err) => { + tracing::warn!("ignoring mcpServers: failed to serialize object: {err}"); + None + } + }, + RawPluginManifestMcpServers::Invalid(value) => { + tracing::warn!( + "ignoring mcpServers: expected a string or object; found {}", + json_value_type(&value) + ); + None + } + } +} + fn resolve_interface_asset_path( plugin_root: &Path, field: &'static str, diff --git a/codex-rs/core-plugins/src/store_tests.rs b/codex-rs/core-plugins/src/store_tests.rs index 200055fe6..6bf2b9368 100644 --- a/codex-rs/core-plugins/src/store_tests.rs +++ b/codex-rs/core-plugins/src/store_tests.rs @@ -71,6 +71,46 @@ fn install_copies_plugin_into_default_marketplace() { assert!(installed_path.join("skills/SKILL.md").is_file()); } +#[test] +fn install_accepts_manifest_mcp_server_objects() { + let tmp = tempdir().unwrap(); + let plugin_root = tmp.path().join("counter-sample"); + fs::create_dir_all(plugin_root.join(".codex-plugin")).unwrap(); + fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{ + "name": "counter-sample", + "version": "1.1.1", + "mcpServers": { + "counter": { + "type": "http", + "url": "https://sample.example/counter/mcp" + } + } +}"#, + ) + .unwrap(); + let plugin_id = PluginId::new("counter-sample".to_string(), "debug".to_string()).unwrap(); + + let result = PluginStore::new(tmp.path().to_path_buf()) + .install( + AbsolutePathBuf::try_from(plugin_root).unwrap(), + plugin_id.clone(), + ) + .unwrap(); + + let installed_path = tmp.path().join("plugins/cache/debug/counter-sample/1.1.1"); + assert_eq!( + result, + PluginInstallResult { + plugin_id, + plugin_version: "1.1.1".to_string(), + installed_path: AbsolutePathBuf::try_from(installed_path.clone()).unwrap(), + } + ); + assert!(installed_path.join(".codex-plugin/plugin.json").is_file()); +} + #[test] fn install_uses_manifest_name_for_destination_and_key() { let tmp = tempdir().unwrap(); diff --git a/codex-rs/ext/mcp/src/executor_plugin/provider.rs b/codex-rs/ext/mcp/src/executor_plugin/provider.rs index bbba63748..e9d5f803d 100644 --- a/codex-rs/ext/mcp/src/executor_plugin/provider.rs +++ b/codex-rs/ext/mcp/src/executor_plugin/provider.rs @@ -7,6 +7,7 @@ use codex_mcp::parse_plugin_mcp_config; use codex_plugin::PluginResourceLocator; use codex_plugin::ResolvedPlugin; use codex_plugin::ResolvedPluginLocation; +use codex_plugin::manifest::PluginManifestMcpServers; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use std::io; @@ -56,26 +57,47 @@ async fn load_from_file_system( ) -> Result, ExecutorPluginMcpProviderError> { let ResolvedPluginLocation::Environment { environment_id, .. } = plugin.location(); let plugin_id = plugin.selected_root_id(); - let (config_path, is_default) = match plugin.manifest().paths.mcp_servers.as_ref() { - Some(PluginResourceLocator::Environment { path, .. }) => (path.clone(), false), - None => (plugin_root.join(DEFAULT_MCP_CONFIG_FILE), true), - }; - let config_uri = PathUri::from_abs_path(&config_path); - - let contents = match file_system - .read_file_text(&config_uri, /*sandbox*/ None) - .await - { - Ok(contents) => contents, - Err(source) if is_default && source.kind() == io::ErrorKind::NotFound => { - return Ok(Vec::new()); + let (contents, config_path) = match plugin.manifest().paths.mcp_servers.as_ref() { + Some(PluginManifestMcpServers::Path(PluginResourceLocator::Environment { + path, .. + })) => { + let config_uri = PathUri::from_abs_path(path); + ( + file_system + .read_file_text(&config_uri, /*sandbox*/ None) + .await + .map_err(|source| ExecutorPluginMcpProviderError::ReadConfig { + plugin_id: plugin_id.to_string(), + path: path.clone(), + source, + })?, + path.clone(), + ) } - Err(source) => { - return Err(ExecutorPluginMcpProviderError::ReadConfig { - plugin_id: plugin_id.to_string(), - path: config_path.clone(), - source, - }); + Some(PluginManifestMcpServers::Object(object_config)) => ( + object_config.clone(), + plugin_root.join(".codex-plugin/plugin.json"), + ), + None => { + let config_path = plugin_root.join(DEFAULT_MCP_CONFIG_FILE); + let config_uri = PathUri::from_abs_path(&config_path); + let contents = match file_system + .read_file_text(&config_uri, /*sandbox*/ None) + .await + { + Ok(contents) => contents, + Err(source) if source.kind() == io::ErrorKind::NotFound => { + return Ok(Vec::new()); + } + Err(source) => { + return Err(ExecutorPluginMcpProviderError::ReadConfig { + plugin_id: plugin_id.to_string(), + path: config_path.clone(), + source, + }); + } + }; + (contents, config_path) } }; let parsed = parse_plugin_mcp_config( diff --git a/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs b/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs index 3bc9ef579..dc6a0403d 100644 --- a/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs +++ b/codex-rs/ext/mcp/src/executor_plugin/provider_tests.rs @@ -15,6 +15,7 @@ use codex_exec_server::ReadDirectoryEntry; use codex_exec_server::RemoveOptions; use codex_plugin::ResolvedPlugin; use codex_plugin::manifest::PluginManifest; +use codex_plugin::manifest::PluginManifestMcpServers; use codex_plugin::manifest::PluginManifestPaths; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; @@ -144,7 +145,10 @@ async fn reads_declared_config_only_through_executor_file_system() { .expect("absolute plugin root"); assert!(!plugin_root.as_path().exists()); let config_path = plugin_root.join("config/mcp.json"); - let plugin = resolved_plugin(&plugin_root, Some(config_path.clone())); + let plugin = resolved_plugin( + &plugin_root, + Some(PluginManifestMcpServers::Path(config_path.clone())), + ); let file_system = SyntheticExecutorFileSystem { config_path: config_path.clone(), config_contents: Some(MCP_CONFIG_CONTENTS), @@ -187,6 +191,60 @@ async fn reads_declared_config_only_through_executor_file_system() { assert_eq!(reads(&file_system), vec![config_path]); } +#[tokio::test] +async fn reads_manifest_object_config_without_executor_file_system_access() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let plugin_root = AbsolutePathBuf::from_absolute_path_checked(temp_dir.path().join("plugin")) + .expect("absolute plugin root"); + let config_path = plugin_root.join(DEFAULT_MCP_CONFIG_FILE); + let plugin = resolved_plugin( + &plugin_root, + Some(PluginManifestMcpServers::Object( + r#"{"counter":{"command":"counter-mcp","environment_id":"local"}}"#.to_string(), + )), + ); + let file_system = SyntheticExecutorFileSystem { + config_path, + config_contents: None, + reads: Mutex::new(Vec::new()), + }; + + let servers = load_from_file_system(&plugin, &plugin_root, &file_system) + .await + .expect("load manifest object executor MCP config"); + + assert_eq!( + servers, + vec![( + "counter".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::Stdio { + command: "counter-mcp".to_string(), + args: Vec::new(), + env: None, + env_vars: Vec::new(), + cwd: Some(plugin_root.to_path_buf()), + }, + environment_id: "executor-test".to_string(), + enabled: true, + required: false, + supports_parallel_tool_calls: false, + disabled_reason: None, + startup_timeout_sec: None, + tool_timeout_sec: None, + default_tools_approval_mode: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + oauth: None, + oauth_resource: None, + tools: HashMap::new(), + }, + )] + ); + assert_eq!(reads(&file_system), Vec::new()); +} + #[tokio::test] async fn missing_default_config_is_empty() { let temp_dir = tempfile::tempdir().expect("tempdir"); @@ -214,7 +272,10 @@ async fn malformed_declared_config_is_an_error() { let plugin_root = AbsolutePathBuf::from_absolute_path_checked(temp_dir.path().join("plugin")) .expect("absolute plugin root"); let config_path = plugin_root.join("mcp.json"); - let plugin = resolved_plugin(&plugin_root, Some(config_path.clone())); + let plugin = resolved_plugin( + &plugin_root, + Some(PluginManifestMcpServers::Path(config_path.clone())), + ); let file_system = SyntheticExecutorFileSystem { config_path: config_path.clone(), config_contents: Some("{not-json"), @@ -242,7 +303,7 @@ async fn malformed_declared_config_is_an_error() { fn resolved_plugin( plugin_root: &AbsolutePathBuf, - mcp_servers: Option, + mcp_servers: Option>, ) -> ResolvedPlugin { ResolvedPlugin::from_environment( "selected-root".to_string(), diff --git a/codex-rs/plugin/src/manifest.rs b/codex-rs/plugin/src/manifest.rs index b1f04c4c1..4aeec49be 100644 --- a/codex-rs/plugin/src/manifest.rs +++ b/codex-rs/plugin/src/manifest.rs @@ -18,11 +18,18 @@ pub struct PluginManifest { #[derive(Debug, Clone, PartialEq, Eq)] pub struct PluginManifestPaths { pub skills: Option, - pub mcp_servers: Option, + pub mcp_servers: Option>, pub apps: Option, pub hooks: Option>, } +/// MCP server declarations embedded in or referenced by a plugin manifest. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum PluginManifestMcpServers { + Path(Resource), + Object(String), +} + /// Hook declarations embedded in or referenced by a plugin manifest. #[derive(Debug, Clone, PartialEq, Eq)] pub enum PluginManifestHooks { @@ -109,6 +116,15 @@ impl PluginManifest { Some(PluginManifestHooks::Inline(hooks)) => Some(PluginManifestHooks::Inline(hooks)), None => None, }; + let mcp_servers = match mcp_servers { + Some(PluginManifestMcpServers::Path(path)) => { + Some(PluginManifestMcpServers::Path(map(path)?)) + } + Some(PluginManifestMcpServers::Object(servers)) => { + Some(PluginManifestMcpServers::Object(servers)) + } + None => None, + }; let interface = match interface { Some(interface) => { let PluginManifestInterface { @@ -157,7 +173,7 @@ impl PluginManifest { keywords, paths: PluginManifestPaths { skills: skills.map(&mut map).transpose()?, - mcp_servers: mcp_servers.map(&mut map).transpose()?, + mcp_servers, apps: apps.map(&mut map).transpose()?, hooks, }, diff --git a/codex-rs/plugin/src/provider_tests.rs b/codex-rs/plugin/src/provider_tests.rs index 44993a640..776e63b1c 100644 --- a/codex-rs/plugin/src/provider_tests.rs +++ b/codex-rs/plugin/src/provider_tests.rs @@ -4,6 +4,7 @@ use super::ResolvedPluginError; use crate::manifest::PluginManifest; use crate::manifest::PluginManifestHooks; use crate::manifest::PluginManifestInterface; +use crate::manifest::PluginManifestMcpServers; use crate::manifest::PluginManifestPaths; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; @@ -37,7 +38,7 @@ fn environment_descriptor_binds_every_manifest_resource() { keywords: Vec::new(), paths: PluginManifestPaths { skills: Some(skills.clone()), - mcp_servers: Some(mcp_servers.clone()), + mcp_servers: Some(PluginManifestMcpServers::Path(mcp_servers.clone())), apps: Some(apps.clone()), hooks: Some(PluginManifestHooks::Paths(vec![hooks.clone()])), }, @@ -71,7 +72,10 @@ fn environment_descriptor_binds_every_manifest_resource() { keywords: Vec::new(), paths: PluginManifestPaths { skills: Some(resource("executor-1", skills)), - mcp_servers: Some(resource("executor-1", mcp_servers)), + mcp_servers: Some(PluginManifestMcpServers::Path(resource( + "executor-1", + mcp_servers, + ))), apps: Some(resource("executor-1", apps)), hooks: Some(PluginManifestHooks::Paths(vec![resource( "executor-1", @@ -100,7 +104,7 @@ fn environment_descriptor_rejects_resources_outside_package_root() { keywords: Vec::new(), paths: PluginManifestPaths { skills: None, - mcp_servers: Some(outside.clone()), + mcp_servers: Some(PluginManifestMcpServers::Path(outside.clone())), apps: None, hooks: None, }, diff --git a/codex-rs/skills/src/assets/samples/plugin-creator/references/plugin-json-spec.md b/codex-rs/skills/src/assets/samples/plugin-creator/references/plugin-json-spec.md index 5eb2251a0..ec5476ae1 100644 --- a/codex-rs/skills/src/assets/samples/plugin-creator/references/plugin-json-spec.md +++ b/codex-rs/skills/src/assets/samples/plugin-creator/references/plugin-json-spec.md @@ -62,10 +62,31 @@ - `keywords` (`array` of `string`): Search/discovery tags. - `skills` (`string`): Relative path to skill directories/files. - `hooks` (`string`): Hook config path. -- `mcpServers` (`string`): MCP config path. +- `mcpServers` (`string` or `object`): MCP config path, or an object whose keys are MCP server names and whose values are MCP server config objects. - `apps` (`string`): App manifest path for plugin integrations. - `interface` (`object`): Interface/UX metadata block for plugin presentation. +`mcpServers` may be declared as a companion file path: + +```json +{ + "mcpServers": "./.mcp.json" +} +``` + +Or as an object directly in `plugin.json`: + +```json +{ + "mcpServers": { + "counter": { + "type": "http", + "url": "https://sample.example/counter/mcp" + } + } +} +``` + ### `interface` fields - `displayName` (`string`): User-facing title shown for the plugin. @@ -91,7 +112,7 @@ ### Path conventions and defaults - Path values should be relative and begin with `./`. -- `skills`, `hooks`, and `mcpServers` are supplemented on top of default component discovery; they do not replace defaults. +- `skills`, `hooks`, and string-valued `mcpServers` are supplemented on top of default component discovery; they do not replace defaults. - Custom path values must follow the plugin root convention and naming/namespacing rules. - This repo’s scaffold writes `.codex-plugin/plugin.json`; treat that as the manifest location this skill generates. @@ -186,8 +207,9 @@ personal marketplace unless the caller explicitly requests a repo-local destinat present. - `composerIcon`, `logo`, and `screenshots` must point to real files inside the plugin archive when present. -- `apps` and `mcpServers` should appear in `plugin.json` only when `.app.json` and `.mcp.json` - actually exist. +- `apps` should appear in `plugin.json` only when `.app.json` actually exists. +- `mcpServers` may point to `.mcp.json` or contain the MCP server object directly in + `plugin.json`. - Validation rejects unsupported manifest fields such as `hooks`, so the scaffold keeps them out of generated manifests. - Run `scripts/validate_plugin.py ` before handing back a generated plugin. It adds one diff --git a/codex-rs/skills/src/assets/samples/plugin-creator/scripts/validate_plugin.py b/codex-rs/skills/src/assets/samples/plugin-creator/scripts/validate_plugin.py index 6f49cb0fe..5c6fae8e6 100644 --- a/codex-rs/skills/src/assets/samples/plugin-creator/scripts/validate_plugin.py +++ b/codex-rs/skills/src/assets/samples/plugin-creator/scripts/validate_plugin.py @@ -126,18 +126,13 @@ def validate_manifest_shape( validate_optional_contract_path(manifest, "skills", "skills", errors) validate_optional_contract_path(manifest, "apps", ".app.json", errors) - validate_optional_contract_path(manifest, "mcpServers", ".mcp.json", errors) + validate_manifest_mcp_servers(plugin_root, manifest, errors) if manifest.get("apps") is not None: validate_app_manifest( plugin_root / ".app.json", errors, ) - if manifest.get("mcpServers") is not None: - validate_mcp_manifest( - plugin_root / ".mcp.json", - errors, - ) validate_skill_manifests(plugin_root, errors) interface = require_object(manifest, "interface", errors) @@ -286,6 +281,32 @@ def validate_optional_contract_path( errors.append(f"plugin.json field `{key}` must resolve to `{expected}`") +def validate_manifest_mcp_servers( + plugin_root: Path, + manifest: dict[str, Any], + errors: list[str], +) -> None: + value = manifest.get("mcpServers") + if value is None: + return + if isinstance(value, str): + validate_optional_contract_path(manifest, "mcpServers", ".mcp.json", errors) + validate_mcp_manifest( + plugin_root / ".mcp.json", + errors, + ) + return + if isinstance(value, dict): + validate_mcp_server_entries( + value, + "plugin.json field `mcpServers`", + "plugin.json field `mcpServers`", + errors, + ) + return + errors.append("plugin.json field `mcpServers` must be a string path or object") + + def normalize_contract_path(raw_path: str) -> str | None: path = Path(raw_path) if path.is_absolute(): @@ -326,14 +347,28 @@ def validate_mcp_manifest(path: Path, errors: list[str]) -> None: return reject_companion_unknown_fields(payload, {"mcpServers"}, "`.mcp.json`", errors) servers = payload.get("mcpServers") + validate_mcp_server_entries( + servers, + "`.mcp.json`", + "`.mcp.json` field `mcpServers`", + errors, + ) + + +def validate_mcp_server_entries( + servers: Any, + source_label: str, + field_label: str, + errors: list[str], +) -> None: if not isinstance(servers, dict): - errors.append("`.mcp.json` field `mcpServers` must be an object") + errors.append(f"{field_label} must be an object") return for key, value in servers.items(): if not isinstance(key, str) or not key.strip(): - errors.append("`.mcp.json` server names must be non-empty strings") + errors.append(f"{source_label} server names must be non-empty strings") if not isinstance(value, dict): - errors.append(f"`.mcp.json` server `{key}` must be an object") + errors.append(f"{source_label} server `{key}` must be an object") def load_companion_json_object(