Support MCP extensions in open plugins (#9471)

Signed-off-by: Douwe M Osinga <douwe@sidewalklabs.com>
Co-authored-by: Douwe M Osinga <douwe@sidewalklabs.com>
This commit is contained in:
Jack Amadeo
2026-06-17 14:52:54 -04:00
committed by GitHub
parent 1701f5279a
commit b0f5db2a07
24 changed files with 623 additions and 27 deletions
+146 -8
View File
@@ -1,10 +1,20 @@
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use serde::Deserialize;
use serde::{Deserialize, Serialize};
use crate::config::Config;
use crate::plugins::plugin_install_dir;
const PLUGINS_CONFIG_KEY: &str = "plugins";
/// Per-plugin entry stored under the `plugins` map in `config.yaml`, keyed by
/// the plugin's filesystem path.
#[derive(Debug, Clone, Serialize, Deserialize)]
struct PluginConfigEntry {
enabled: bool,
}
/// A plugin found on disk and not disabled by any settings file.
#[derive(Debug, Clone)]
pub struct DiscoveredPlugin {
@@ -40,6 +50,13 @@ enum SettingsScope {
/// `project_root`, when supplied, enables project + local scope settings and
/// project-scope `.agents/plugins/` lookups.
pub fn discover_enabled_plugins(project_root: Option<&Path>) -> Vec<DiscoveredPlugin> {
discover_enabled_plugins_with_config(project_root, Config::global())
}
fn discover_enabled_plugins_with_config(
project_root: Option<&Path>,
config: &Config,
) -> Vec<DiscoveredPlugin> {
let scoped_settings = load_all_settings(project_root);
let mut found: HashMap<String, DiscoveredPlugin> = HashMap::new();
@@ -60,10 +77,46 @@ pub fn discover_enabled_plugins(project_root: Option<&Path>) -> Vec<DiscoveredPl
});
}
found
let enabled_by_settings: Vec<DiscoveredPlugin> = found
.into_values()
.filter(|plugin| is_enabled(&plugin.name, &scoped_settings))
.collect()
.collect();
filter_by_config(enabled_by_settings, config)
}
/// Apply the `plugins` map in `config.yaml`. Newly discovered plugins are added
/// to the map with `enabled: true`; plugins explicitly set to `enabled: false`
/// are dropped.
fn filter_by_config(plugins: Vec<DiscoveredPlugin>, config: &Config) -> Vec<DiscoveredPlugin> {
let mut entries: HashMap<String, PluginConfigEntry> =
config.get_param(PLUGINS_CONFIG_KEY).unwrap_or_default();
let mut dirty = false;
let mut enabled = Vec::new();
for plugin in plugins {
let key = plugin.root.to_string_lossy().to_string();
match entries.get(&key) {
Some(entry) => {
if entry.enabled {
enabled.push(plugin);
}
}
None => {
entries.insert(key, PluginConfigEntry { enabled: true });
dirty = true;
enabled.push(plugin);
}
}
}
if dirty {
if let Err(e) = config.set_param(PLUGINS_CONFIG_KEY, entries) {
tracing::warn!(error = %e, "Failed to persist plugin config entries");
}
}
enabled
}
fn is_enabled(plugin_name: &str, scoped_settings: &[(SettingsScope, PluginSettings)]) -> bool {
@@ -197,13 +250,22 @@ mod tests {
std::fs::write(dir.join("settings.local.json"), contents).unwrap();
}
fn test_config(dir: &Path) -> Config {
Config::new(dir.join("config.yaml"), "goose-discovery-test").unwrap()
}
fn discover(project: &Path) -> Vec<DiscoveredPlugin> {
let cfg_dir = tempfile::tempdir().unwrap();
discover_enabled_plugins_with_config(Some(project), &test_config(cfg_dir.path()))
}
#[test]
fn finds_project_scope_plugin() {
let tmp = tempfile::tempdir().unwrap();
let project = tmp.path();
write_plugin_dir(&project.join(".agents").join("plugins"), "demo");
let found = discover_enabled_plugins(Some(project));
let found = discover(project);
let names: Vec<_> = found.iter().map(|p| p.name.as_str()).collect();
assert!(names.contains(&"demo"), "got: {names:?}");
let demo = found.iter().find(|p| p.name == "demo").unwrap();
@@ -221,7 +283,7 @@ mod tests {
r#"{"disabledPlugins":["demo"]}"#,
);
let found = discover_enabled_plugins(Some(project));
let found = discover(project);
assert!(found.iter().all(|p| p.name != "demo"));
}
@@ -237,7 +299,7 @@ mod tests {
r#"{"enabledPlugins":["demo"]}"#,
);
let found = discover_enabled_plugins(Some(project));
let found = discover(project);
let names: Vec<_> = found.iter().map(|p| p.name.as_str()).collect();
assert!(names.contains(&"demo"), "got: {names:?}");
assert!(names.contains(&"other"), "got: {names:?}");
@@ -258,7 +320,7 @@ mod tests {
r#"{"enabledPlugins":["demo"]}"#,
);
let found = discover_enabled_plugins(Some(project));
let found = discover(project);
assert!(
found.iter().any(|p| p.name == "demo"),
"local scope should win; got: {:?}",
@@ -285,7 +347,7 @@ mod tests {
let prev = std::env::var("GOOSE_PATH_ROOT").ok();
unsafe { std::env::set_var("GOOSE_PATH_ROOT", fake_home.path()) };
let found = discover_enabled_plugins(Some(project));
let found = discover(project);
match prev {
Some(v) => unsafe { std::env::set_var("GOOSE_PATH_ROOT", v) },
None => unsafe { std::env::remove_var("GOOSE_PATH_ROOT") },
@@ -297,4 +359,80 @@ mod tests {
found.iter().map(|p| &p.name).collect::<Vec<_>>()
);
}
#[test]
fn newly_discovered_plugin_is_added_to_config_as_enabled() {
let tmp = tempfile::tempdir().unwrap();
let project = tmp.path();
write_plugin_dir(&project.join(".agents").join("plugins"), "demo");
let cfg_dir = tempfile::tempdir().unwrap();
let config = test_config(cfg_dir.path());
let found = discover_enabled_plugins_with_config(Some(project), &config);
assert!(found.iter().any(|p| p.name == "demo"));
let entries: HashMap<String, PluginConfigEntry> =
config.get_param(PLUGINS_CONFIG_KEY).unwrap();
let key = project
.join(".agents")
.join("plugins")
.join("demo")
.to_string_lossy()
.to_string();
assert!(
entries.get(&key).is_some_and(|e| e.enabled),
"got: {entries:?}"
);
}
#[test]
fn disabled_in_config_drops_plugin() {
let tmp = tempfile::tempdir().unwrap();
let project = tmp.path();
write_plugin_dir(&project.join(".agents").join("plugins"), "demo");
let cfg_dir = tempfile::tempdir().unwrap();
let config = test_config(cfg_dir.path());
let key = project
.join(".agents")
.join("plugins")
.join("demo")
.to_string_lossy()
.to_string();
let entries = HashMap::from([(key, PluginConfigEntry { enabled: false })]);
config.set_param(PLUGINS_CONFIG_KEY, entries).unwrap();
let found = discover_enabled_plugins_with_config(Some(project), &config);
assert!(found.iter().all(|p| p.name != "demo"));
}
#[test]
fn enabled_in_config_keeps_plugin_without_modifying_config() {
let tmp = tempfile::tempdir().unwrap();
let project = tmp.path();
write_plugin_dir(&project.join(".agents").join("plugins"), "demo");
let cfg_dir = tempfile::tempdir().unwrap();
let config = test_config(cfg_dir.path());
let key = project
.join(".agents")
.join("plugins")
.join("demo")
.to_string_lossy()
.to_string();
config
.set_param(
PLUGINS_CONFIG_KEY,
HashMap::from([(key.clone(), PluginConfigEntry { enabled: true })]),
)
.unwrap();
let found = discover_enabled_plugins_with_config(Some(project), &config);
assert!(found.iter().any(|p| p.name == "demo"));
let entries: HashMap<String, PluginConfigEntry> =
config.get_param(PLUGINS_CONFIG_KEY).unwrap();
assert!(entries.get(&key).is_some_and(|e| e.enabled));
}
}
@@ -18,16 +18,18 @@ const MANIFESTS: [&str; 3] = [
];
const FORMAT: &str = "open-plugins";
const COMPONENT_MARKERS: &[&str] = &["hooks/hooks.json", "commands", "agents"];
const COMPONENT_MARKERS: &[&str] = &["hooks/hooks.json", "commands", "agents", ".mcp.json"];
#[derive(Debug, Deserialize)]
struct OpenPluginsManifest {
pub struct OpenPluginsManifest {
#[serde(default)]
name: Option<String>,
pub name: Option<String>,
#[serde(default)]
version: Option<String>,
pub version: Option<String>,
#[serde(default)]
skills: Option<serde_json::Value>,
pub skills: Option<serde_json::Value>,
#[serde(default, rename = "mcpServers")]
pub mcp_servers: Option<serde_json::Value>,
}
#[derive(Debug)]
@@ -88,6 +90,7 @@ fn install_from_manifest(
}
let skills = find_agent_skills(checkout_dir, manifest.skills.as_ref())?;
validate_mcp_servers(checkout_dir, manifest.mcp_servers.as_ref())?;
copy_dir_all(checkout_dir, &destination)?;
@@ -146,7 +149,10 @@ pub(in crate::plugins) fn installed_skill_dirs(plugin_dir: &Path) -> Vec<PathBuf
dedupe_paths(dirs)
}
fn read_manifest(plugin_dir: &Path, source: &str) -> Result<OpenPluginsManifest> {
pub(in crate::plugins) fn read_manifest(
plugin_dir: &Path,
source: &str,
) -> Result<OpenPluginsManifest> {
let mut manifest = match manifest_path(plugin_dir) {
Some(manifest_path) => {
serde_json::from_str::<OpenPluginsManifest>(&fs::read_to_string(&manifest_path)?)
@@ -156,6 +162,7 @@ fn read_manifest(plugin_dir: &Path, source: &str) -> Result<OpenPluginsManifest>
name: None,
version: None,
skills: None,
mcp_servers: None,
},
};
@@ -240,6 +247,50 @@ fn namespaced_component_name(plugin_name: &str, component_name: &str) -> String
format!("{plugin_name}:{component_name}")
}
fn validate_mcp_servers(
plugin_dir: &Path,
mcp_servers_config: Option<&serde_json::Value>,
) -> Result<()> {
if let Some(value) = mcp_servers_config {
crate::plugins::mcp_servers::validate_mcp_servers_manifest_value(value)?;
}
for path in mcp_config_paths_for_validation(plugin_dir, mcp_servers_config)? {
if !path.is_file() {
continue;
}
let value = serde_json::from_str::<serde_json::Value>(&fs::read_to_string(&path)?)
.with_context(|| format!("Failed to parse {}", path.display()))?;
crate::plugins::mcp_servers::validate_mcp_server_document(&value)?;
}
Ok(())
}
fn mcp_config_paths_for_validation(
plugin_dir: &Path,
config: Option<&serde_json::Value>,
) -> Result<Vec<PathBuf>> {
let custom_paths = config
.filter(|value| {
!value
.as_object()
.is_some_and(|object| object.contains_key("mcpServers"))
})
.map(parse_component_paths)
.transpose()?
.unwrap_or_default();
let mut paths = Vec::new();
if !custom_paths.exclusive {
paths.push(plugin_dir.join(".mcp.json"));
}
for path in custom_paths.paths {
paths.push(plugin_dir.join(validate_relative_plugin_path(&path)?));
}
Ok(dedupe_paths(paths))
}
fn find_agent_skills(
plugin_dir: &Path,
skills_config: Option<&serde_json::Value>,
@@ -283,12 +334,14 @@ fn skill_root_directories(
}
#[derive(Default)]
struct ComponentPaths {
paths: Vec<String>,
exclusive: bool,
pub(in crate::plugins) struct ComponentPaths {
pub paths: Vec<String>,
pub exclusive: bool,
}
fn parse_component_paths(value: &serde_json::Value) -> Result<ComponentPaths> {
pub(in crate::plugins) fn parse_component_paths(
value: &serde_json::Value,
) -> Result<ComponentPaths> {
match value {
serde_json::Value::String(path) => Ok(ComponentPaths {
paths: vec![path.clone()],
@@ -332,7 +385,7 @@ fn parse_component_paths(value: &serde_json::Value) -> Result<ComponentPaths> {
}
}
fn validate_relative_plugin_path(path: &str) -> Result<PathBuf> {
pub(in crate::plugins) fn validate_relative_plugin_path(path: &str) -> Result<PathBuf> {
if !path.starts_with("./") {
bail!(
"Open Plugins component paths must start with './': {}",
@@ -460,7 +513,7 @@ fn build_skill_md(name: &str, body: &str) -> String {
)
}
fn dedupe_paths(paths: Vec<PathBuf>) -> Vec<PathBuf> {
pub(in crate::plugins) fn dedupe_paths(paths: Vec<PathBuf>) -> Vec<PathBuf> {
let mut seen = HashSet::new();
paths
.into_iter()
+335
View File
@@ -0,0 +1,335 @@
use crate::agents::extension::{Envs, ExtensionConfig};
use crate::config::{DEFAULT_EXTENSION_DESCRIPTION, DEFAULT_EXTENSION_TIMEOUT};
use crate::plugins::discovery::discover_enabled_plugins;
use crate::plugins::formats::open_plugins;
use anyhow::{bail, Context, Result};
use fs_err as fs;
use serde::Deserialize;
use std::collections::{HashMap, HashSet};
use std::path::{Path, PathBuf};
use tracing::warn;
const DEFAULT_MCP_CONFIG: &str = ".mcp.json";
const PLUGIN_ROOT: &str = "${PLUGIN_ROOT}";
#[derive(Debug, Deserialize)]
struct McpServersDocument {
#[serde(default, rename = "mcpServers")]
mcp_servers: HashMap<String, McpServerConfig>,
}
#[derive(Debug, Deserialize)]
struct McpServerConfig {
command: String,
#[serde(default)]
args: Vec<String>,
#[serde(default)]
env: HashMap<String, String>,
#[serde(default)]
cwd: Option<String>,
}
pub fn enabled_plugin_mcp_servers(project_root: Option<&Path>) -> Vec<ExtensionConfig> {
let mut configs = Vec::new();
for plugin in discover_enabled_plugins(project_root) {
match plugin_mcp_servers(&plugin.name, &plugin.root) {
Ok(plugin_configs) => configs.extend(plugin_configs),
Err(err) => warn!(
plugin = %plugin.name,
root = %plugin.root.display(),
error = %err,
"Failed to load plugin MCP servers; skipping",
),
}
}
configs
}
pub fn plugin_mcp_servers(plugin_name: &str, plugin_root: &Path) -> Result<Vec<ExtensionConfig>> {
let manifest = open_plugins::read_manifest(plugin_root, "")?;
let mut configs = Vec::new();
let mut seen = HashSet::new();
for path in mcp_config_paths(plugin_root, manifest.mcp_servers.as_ref())? {
if !seen.insert(path.clone()) || !path.is_file() {
continue;
}
let document = serde_json::from_str::<McpServersDocument>(&fs::read_to_string(&path)?)
.with_context(|| format!("Failed to parse {}", path.display()))?;
configs.extend(document_to_extension_configs(
plugin_name,
plugin_root,
document.mcp_servers,
));
}
if let Some(value) = manifest
.mcp_servers
.as_ref()
.filter(|value| is_inline_config(value))
{
let servers = parse_inline_servers(value)?;
configs.extend(document_to_extension_configs(
plugin_name,
plugin_root,
servers,
));
}
Ok(configs)
}
fn mcp_config_paths(
plugin_root: &Path,
config: Option<&serde_json::Value>,
) -> Result<Vec<PathBuf>> {
let custom_paths = config
.filter(|value| !is_inline_config(value))
.map(open_plugins::parse_component_paths)
.transpose()?
.unwrap_or_default();
let mut paths = Vec::new();
if !custom_paths.exclusive {
paths.push(plugin_root.join(DEFAULT_MCP_CONFIG));
}
for path in custom_paths.paths {
paths.push(plugin_root.join(open_plugins::validate_relative_plugin_path(&path)?));
}
Ok(open_plugins::dedupe_paths(paths))
}
fn is_inline_config(value: &serde_json::Value) -> bool {
value.as_object().is_some_and(|object| {
!object
.keys()
.all(|key| matches!(key.as_str(), "paths" | "exclusive"))
})
}
fn parse_inline_servers(value: &serde_json::Value) -> Result<HashMap<String, McpServerConfig>> {
serde_json::from_value(value.clone())
.with_context(|| "Failed to parse inline Open Plugins MCP servers")
}
fn document_to_extension_configs(
plugin_name: &str,
plugin_root: &Path,
servers: HashMap<String, McpServerConfig>,
) -> Vec<ExtensionConfig> {
let mut entries: Vec<_> = servers.into_iter().collect();
entries.sort_by(|a, b| a.0.cmp(&b.0));
entries
.into_iter()
.map(|(server_name, server)| {
server_to_extension_config(plugin_name, plugin_root, server_name, server)
})
.collect()
}
fn server_to_extension_config(
plugin_name: &str,
plugin_root: &Path,
server_name: String,
server: McpServerConfig,
) -> ExtensionConfig {
let root = plugin_root.to_string_lossy();
let mut env = HashMap::from([("PLUGIN_ROOT".to_string(), root.to_string())]);
env.extend(
server
.env
.into_iter()
.map(|(key, value)| (key, expand_plugin_root(&value, &root))),
);
ExtensionConfig::Stdio {
name: format!("{plugin_name}:{server_name}"),
description: DEFAULT_EXTENSION_DESCRIPTION.to_string(),
cmd: expand_plugin_root(&server.command, &root),
args: server
.args
.into_iter()
.map(|arg| expand_plugin_root(&arg, &root))
.collect(),
envs: Envs::new(env),
env_keys: Vec::new(),
timeout: Some(DEFAULT_EXTENSION_TIMEOUT),
cwd: server.cwd.map(|cwd| expand_plugin_root(&cwd, &root)),
bundled: Some(false),
available_tools: Vec::new(),
}
}
fn expand_plugin_root(value: &str, plugin_root: &str) -> String {
value.replace(PLUGIN_ROOT, plugin_root)
}
pub fn validate_mcp_servers_manifest_value(value: &serde_json::Value) -> Result<()> {
if is_inline_config(value) {
validate_servers(parse_inline_servers(value)?)?;
return Ok(());
}
open_plugins::parse_component_paths(value)?;
Ok(())
}
pub fn validate_mcp_server_document(value: &serde_json::Value) -> Result<()> {
let document = serde_json::from_value::<McpServersDocument>(value.clone())?;
validate_servers(document.mcp_servers)
}
fn validate_servers(servers: HashMap<String, McpServerConfig>) -> Result<()> {
for (name, server) in servers {
if server.command.trim().is_empty() {
bail!(
"Open Plugins MCP server '{}' command must not be empty",
name
);
}
}
Ok(())
}
#[cfg(test)]
mod tests {
use super::*;
use crate::agents::extension::ExtensionConfig;
#[test]
fn loads_default_mcp_json_with_plugin_root_expansion() {
let plugin = tempfile::tempdir().unwrap();
fs::write(
plugin.path().join(DEFAULT_MCP_CONFIG),
r#"{
"mcpServers": {
"database": {
"command": "${PLUGIN_ROOT}/servers/db-server",
"args": ["--config", "${PLUGIN_ROOT}/config.json"],
"env": {"DB_PATH": "${PLUGIN_ROOT}/data"},
"cwd": "${PLUGIN_ROOT}"
}
}
}"#,
)
.unwrap();
let configs = plugin_mcp_servers("test-plugin", plugin.path()).unwrap();
assert_eq!(configs.len(), 1);
let ExtensionConfig::Stdio {
name,
cmd,
args,
envs,
cwd,
..
} = &configs[0]
else {
panic!("expected stdio config");
};
assert_eq!(name, "test-plugin:database");
assert_eq!(
cmd,
plugin
.path()
.join("servers/db-server")
.to_string_lossy()
.as_ref()
);
assert_eq!(
args,
&vec![
"--config".to_string(),
plugin
.path()
.join("config.json")
.to_string_lossy()
.to_string()
]
);
assert_eq!(
envs.get_env().get("DB_PATH"),
Some(&plugin.path().join("data").to_string_lossy().to_string())
);
assert_eq!(
cwd.as_deref(),
Some(plugin.path().to_string_lossy().as_ref())
);
}
#[test]
fn loads_inline_manifest_mcp_servers() {
let plugin = tempfile::tempdir().unwrap();
fs::create_dir_all(plugin.path().join(".plugin")).unwrap();
fs::write(
plugin.path().join(".plugin/plugin.json"),
r#"{
"name": "test-plugin",
"mcpServers": {
"api": {"command": "npx", "args": ["@company/mcp-server"]}
}
}"#,
)
.unwrap();
let configs = plugin_mcp_servers("test-plugin", plugin.path()).unwrap();
assert_eq!(configs.len(), 1);
assert_eq!(configs[0].name(), "test-plugin:api");
}
#[test]
fn manifest_paths_supplement_default_config() {
let plugin = tempfile::tempdir().unwrap();
fs::create_dir_all(plugin.path().join(".plugin")).unwrap();
fs::write(
plugin.path().join(".plugin/plugin.json"),
r#"{"name":"test-plugin","mcpServers":"./custom-mcp.json"}"#,
)
.unwrap();
fs::write(
plugin.path().join(DEFAULT_MCP_CONFIG),
r#"{"mcpServers":{"default":{"command":"default-server"}}}"#,
)
.unwrap();
fs::write(
plugin.path().join("custom-mcp.json"),
r#"{"mcpServers":{"custom":{"command":"custom-server"}}}"#,
)
.unwrap();
let names: Vec<_> = plugin_mcp_servers("test-plugin", plugin.path())
.unwrap()
.into_iter()
.map(|config| config.name())
.collect();
assert_eq!(names, vec!["test-plugin:default", "test-plugin:custom"]);
}
#[test]
fn validates_manifest_mcp_servers_value() {
validate_mcp_servers_manifest_value(&serde_json::json!({
"api": {"command": "npx"}
}))
.unwrap();
validate_mcp_servers_manifest_value(&serde_json::json!({
"paths": ["./mcp.json"],
"exclusive": true
}))
.unwrap();
}
#[test]
fn rejects_inline_mcp_server_with_empty_command() {
let error = validate_mcp_servers_manifest_value(&serde_json::json!({
"api": {"command": ""}
}))
.unwrap_err();
assert!(error.to_string().contains("command must not be empty"));
}
}
+1
View File
@@ -1,5 +1,6 @@
pub mod discovery;
pub mod formats;
pub mod mcp_servers;
use crate::config::paths::Paths;
use crate::subprocess::SubprocessExt;