fix: stdio extensions silently skipped when name missing or env: used in config (#10773)
This commit is contained in:
@@ -181,7 +181,7 @@ pub enum ExtensionConfig {
|
||||
description: String,
|
||||
cmd: String,
|
||||
args: Vec<String>,
|
||||
#[serde(default)]
|
||||
#[serde(default, alias = "env")]
|
||||
envs: Envs,
|
||||
#[serde(default)]
|
||||
env_keys: Vec<String>,
|
||||
|
||||
@@ -40,6 +40,18 @@ pub(crate) fn is_extension_available(config: &ExtensionConfig) -> bool {
|
||||
}
|
||||
}
|
||||
|
||||
fn inject_name_if_missing(key: &str, value: serde_yaml::Value) -> serde_yaml::Value {
|
||||
let name_key = serde_yaml::Value::String("name".to_string());
|
||||
if let serde_yaml::Value::Mapping(mut map) = value {
|
||||
if !map.contains_key(&name_key) {
|
||||
map.insert(name_key, serde_yaml::Value::String(key.to_string()));
|
||||
}
|
||||
serde_yaml::Value::Mapping(map)
|
||||
} else {
|
||||
value
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_extensions_map(raw: &Mapping) -> IndexMap<String, ExtensionEntry> {
|
||||
let mut extensions_map = IndexMap::with_capacity(raw.len());
|
||||
for (k, v) in raw {
|
||||
@@ -48,7 +60,8 @@ fn parse_extensions_map(raw: &Mapping) -> IndexMap<String, ExtensionEntry> {
|
||||
continue;
|
||||
};
|
||||
|
||||
match serde_yaml::from_value::<ExtensionEntry>(v.clone()) {
|
||||
let v = inject_name_if_missing(key, v.clone());
|
||||
match serde_yaml::from_value::<ExtensionEntry>(v) {
|
||||
Ok(entry) => {
|
||||
if !is_extension_available(&entry.config) {
|
||||
continue;
|
||||
@@ -600,6 +613,91 @@ extensions:
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_stdio_without_name_uses_map_key() {
|
||||
let (config, _config_file, _secrets_file) = test_config(
|
||||
r#"
|
||||
extensions:
|
||||
firecrawl:
|
||||
enabled: true
|
||||
type: stdio
|
||||
cmd: npx
|
||||
args: ["-y", "firecrawl-mcp"]
|
||||
envs:
|
||||
FIRECRAWL_API_KEY: test-key
|
||||
"#,
|
||||
);
|
||||
|
||||
let extensions = get_extensions_map_with_config(&config);
|
||||
let entry = extensions
|
||||
.get("firecrawl")
|
||||
.expect("firecrawl extension should parse");
|
||||
assert_eq!(entry.config.name(), "firecrawl");
|
||||
assert!(entry.enabled);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_stdio_env_alias_accepted() {
|
||||
let (config, _config_file, _secrets_file) = test_config(
|
||||
r#"
|
||||
extensions:
|
||||
brave-search:
|
||||
enabled: true
|
||||
type: stdio
|
||||
name: brave-search
|
||||
cmd: npx
|
||||
args: ["-y", "@modelcontextprotocol/server-brave-search"]
|
||||
env:
|
||||
BRAVE_API_KEY: test-key
|
||||
"#,
|
||||
);
|
||||
|
||||
let extensions = get_extensions_map_with_config(&config);
|
||||
let entry = extensions
|
||||
.get("brave-search")
|
||||
.expect("brave-search extension should parse");
|
||||
match &entry.config {
|
||||
ExtensionConfig::Stdio { envs, .. } => {
|
||||
assert_eq!(
|
||||
envs.get_env().get("BRAVE_API_KEY"),
|
||||
Some(&"test-key".to_string())
|
||||
);
|
||||
}
|
||||
other => panic!("expected Stdio, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_stdio_env_alias_without_name_uses_map_key() {
|
||||
let (config, _config_file, _secrets_file) = test_config(
|
||||
r#"
|
||||
extensions:
|
||||
brave-search:
|
||||
enabled: true
|
||||
type: stdio
|
||||
cmd: npx
|
||||
args: ["-y", "@modelcontextprotocol/server-brave-search"]
|
||||
env:
|
||||
BRAVE_API_KEY: test-key
|
||||
"#,
|
||||
);
|
||||
|
||||
let extensions = get_extensions_map_with_config(&config);
|
||||
let entry = extensions
|
||||
.get("brave-search")
|
||||
.expect("brave-search extension should parse when name is missing and env: is used");
|
||||
assert_eq!(entry.config.name(), "brave-search");
|
||||
match &entry.config {
|
||||
ExtensionConfig::Stdio { envs, .. } => {
|
||||
assert_eq!(
|
||||
envs.get_env().get("BRAVE_API_KEY"),
|
||||
Some(&"test-key".to_string())
|
||||
);
|
||||
}
|
||||
other => panic!("expected Stdio, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_deserialization_failure_logs_offending_key() {
|
||||
let (config, _config_file, _secrets_file) = test_config(
|
||||
|
||||
Reference in New Issue
Block a user