refactor: Support both CSV and list formats for enabled_mcp_servers

This commit is contained in:
2026-06-03 13:23:13 -06:00
parent 7637a4e46b
commit 1263ada775
15 changed files with 174 additions and 97 deletions
+8 -1
View File
@@ -118,7 +118,14 @@ visible_tools: # Which tools are visible to be compiled (and a
mcp_server_support: true # Enables or disables MCP servers (globally). mcp_server_support: true # Enables or disables MCP servers (globally).
mapping_mcp_servers: # Alias for an MCP server or set of servers mapping_mcp_servers: # Alias for an MCP server or set of servers
git: github,gitmcp git: github,gitmcp
enabled_mcp_servers: null # Which MCP servers to enable by default (e.g. 'github,slack,ddg-search') enabled_mcp_servers: null # Which MCP servers to enable by default.
# Accepts either a YAML list or a comma-separated string. Use 'all' to enable everything.
# Example (list form):
# enabled_mcp_servers:
# - github
# - slack
# Example (comma-separated form):
# enabled_mcp_servers: github,slack,ddg-search
# ---- Skills ---- # ---- Skills ----
# Skills are modular knowledge or capability packs the LLM can load and unload mid-conversation. # Skills are modular knowledge or capability packs the LLM can load and unload mid-conversation.
+3 -1
View File
@@ -9,7 +9,9 @@ model: openai:gpt-4o # The model to use for this role
temperature: 0.2 # The temperature to use for this role when querying the model temperature: 0.2 # The temperature to use for this role when querying the model
top_p: 0 # The top_p to use for this role when querying the model top_p: 0 # The top_p to use for this role when querying the model
enabled_tools: fs_ls,fs_cat # A comma-separated list of tools to enable for this role enabled_tools: fs_ls,fs_cat # A comma-separated list of tools to enable for this role
enabled_mcp_servers: github,gitmcp # A comma-separated list of MCP servers to enable for this role enabled_mcp_servers: # MCP servers to enable for this role. Accepts a YAML list (preferred)
- github # or a comma-separated string (e.g. `enabled_mcp_servers: github,gitmcp`).
- gitmcp # Use `all` to enable every configured MCP server.
skills_enabled: true # Master switch for skills in this role (default: inherit from global). skills_enabled: true # Master switch for skills in this role (default: inherit from global).
# Skills also require `function_calling_support: true` in the global config. # Skills also require `function_calling_support: true` in the global config.
enabled_skills: # Skills available when this role is active. Accepts a YAML list (preferred) enabled_skills: # Skills available when this role is active. Accepts a YAML list (preferred)
+5 -6
View File
@@ -552,8 +552,8 @@ impl RoleLike for Agent {
None None
} }
fn enabled_mcp_servers(&self) -> Option<String> { fn enabled_mcp_servers(&self) -> Option<Vec<String>> {
self.config.mcp_servers.clone().join(",").into() Some(self.config.mcp_servers.clone())
} }
fn set_model(&mut self, model: Model) { fn set_model(&mut self, model: Model) {
@@ -585,15 +585,14 @@ impl RoleLike for Agent {
} }
} }
fn set_enabled_mcp_servers(&mut self, value: Option<String>) { fn set_enabled_mcp_servers(&mut self, value: Option<Vec<String>>) {
match value { match value {
Some(servers) => { Some(servers) => {
let servers = servers self.config.mcp_servers = servers
.split(',') .into_iter()
.map(|v| v.trim().to_string()) .map(|v| v.trim().to_string())
.filter(|v| !v.is_empty()) .filter(|v| !v.is_empty())
.collect::<Vec<_>>(); .collect::<Vec<_>>();
self.config.mcp_servers = servers;
} }
None => { None => {
self.config.mcp_servers.clear(); self.config.mcp_servers.clear();
+4 -3
View File
@@ -44,7 +44,8 @@ pub struct AppConfig {
pub mcp_server_support: bool, pub mcp_server_support: bool,
pub mapping_mcp_servers: IndexMap<String, String>, pub mapping_mcp_servers: IndexMap<String, String>,
pub enabled_mcp_servers: Option<String>, #[serde(default, deserialize_with = "super::deserialize_csv_or_vec")]
pub enabled_mcp_servers: Option<Vec<String>>,
pub auto_continue: bool, pub auto_continue: bool,
pub max_auto_continues: usize, pub max_auto_continues: usize,
@@ -413,7 +414,7 @@ impl AppConfig {
self.mapping_mcp_servers = v; self.mapping_mcp_servers = v;
} }
if let Some(v) = super::read_env_value::<String>(&get_env_name("enabled_mcp_servers")) { if let Some(v) = super::read_env_value::<String>(&get_env_name("enabled_mcp_servers")) {
self.enabled_mcp_servers = v; self.enabled_mcp_servers = v.map(|raw| super::csv_to_vec(&raw));
} }
if let Some(v) = super::read_env_value::<String>(&get_env_name("repl_prelude")) { if let Some(v) = super::read_env_value::<String>(&get_env_name("repl_prelude")) {
@@ -520,7 +521,7 @@ impl AppConfig {
} }
#[allow(dead_code)] #[allow(dead_code)]
pub fn set_enabled_mcp_servers_default(&mut self, value: Option<String>) { pub fn set_enabled_mcp_servers_default(&mut self, value: Option<Vec<String>>) {
self.enabled_mcp_servers = value; self.enabled_mcp_servers = value;
} }
+1 -1
View File
@@ -34,7 +34,7 @@ pub async fn macro_execute(
app_config.temperature = role.temperature(); app_config.temperature = role.temperature();
app_config.top_p = role.top_p(); app_config.top_p = role.top_p();
app_config.enabled_tools = role.enabled_tools().clone(); app_config.enabled_tools = role.enabled_tools().clone();
app_config.enabled_mcp_servers = role.enabled_mcp_servers().clone(); app_config.enabled_mcp_servers = role.enabled_mcp_servers();
let mut app_state = (*ctx.app).clone(); let mut app_state = (*ctx.app).clone();
app_state.config = Arc::new(app_config); app_state.config = Arc::new(app_config);
+2 -1
View File
@@ -206,7 +206,8 @@ pub struct Config {
pub mcp_server_support: bool, pub mcp_server_support: bool,
pub mapping_mcp_servers: IndexMap<String, String>, pub mapping_mcp_servers: IndexMap<String, String>,
pub enabled_mcp_servers: Option<String>, #[serde(default, deserialize_with = "deserialize_csv_or_vec")]
pub enabled_mcp_servers: Option<Vec<String>>,
pub auto_continue: bool, pub auto_continue: bool,
pub max_auto_continues: usize, pub max_auto_continues: usize,
+37 -34
View File
@@ -710,7 +710,7 @@ impl RequestContext {
} }
} }
pub fn set_enabled_mcp_servers_on_role_like(&mut self, value: Option<String>) -> bool { pub fn set_enabled_mcp_servers_on_role_like(&mut self, value: Option<Vec<String>>) -> bool {
match self.role_like_mut() { match self.role_like_mut() {
Some(role_like) => { Some(role_like) => {
role_like.set_enabled_mcp_servers(value); role_like.set_enabled_mcp_servers(value);
@@ -858,7 +858,7 @@ impl RequestContext {
), ),
( (
"enabled_mcp_servers", "enabled_mcp_servers",
super::format_option_value(&role.enabled_mcp_servers()), super::format_option_value(&role.enabled_mcp_servers().map(|v| v.join(","))),
), ),
( (
"max_output_tokens", "max_output_tokens",
@@ -1279,10 +1279,10 @@ impl RequestContext {
} }
let mut server_names: HashSet<String> = Default::default(); let mut server_names: HashSet<String> = Default::default();
if enabled_mcp_servers == "all" { if enabled_mcp_servers.iter().any(|s| s.trim() == "all") {
server_names.extend(mcp_declaration_names); server_names.extend(mcp_declaration_names);
} else { } else {
for item in enabled_mcp_servers.split(',') { for item in enabled_mcp_servers.iter() {
let item = item.trim(); let item = item.trim();
if item.is_empty() { if item.is_empty() {
continue; continue;
@@ -1733,8 +1733,9 @@ impl RequestContext {
} }
} }
"enabled_mcp_servers" => { "enabled_mcp_servers" => {
let value: Option<String> = super::parse_value(value)?; let raw: Option<String> = super::parse_value(value)?;
if let Some(servers) = value.as_ref() { let parsed: Option<Vec<String>> = raw.map(|s| super::csv_to_vec(&s));
if let Some(servers) = parsed.as_ref() {
let Some(mcp_config) = &self.app.mcp_config else { let Some(mcp_config) = &self.app.mcp_config else {
bail!( bail!(
"No MCP servers are configured. Please configure MCP servers first before setting 'enabled_mcp_servers'." "No MCP servers are configured. Please configure MCP servers first before setting 'enabled_mcp_servers'."
@@ -1746,7 +1747,7 @@ impl RequestContext {
); );
} }
if !servers.split(',').all(|s| { if !servers.iter().all(|s| {
let server = s.trim(); let server = s.trim();
server == "all" || mcp_config.mcp_servers.contains_key(server) server == "all" || mcp_config.mcp_servers.contains_key(server)
}) { }) {
@@ -1755,8 +1756,8 @@ impl RequestContext {
); );
} }
} }
if !self.set_enabled_mcp_servers_on_role_like(value.clone()) { if !self.set_enabled_mcp_servers_on_role_like(parsed.clone()) {
self.update_app_config(|app| app.enabled_mcp_servers = value.clone()); self.update_app_config(|app| app.enabled_mcp_servers = parsed.clone());
} }
if self.app.config.mcp_server_support { if self.app.config.mcp_server_support {
let app = Arc::clone(&self.app.config); let app = Arc::clone(&self.app.config);
@@ -2163,7 +2164,7 @@ impl RequestContext {
async fn rebuild_tool_scope( async fn rebuild_tool_scope(
&mut self, &mut self,
app: &AppConfig, app: &AppConfig,
enabled_mcp_servers: Option<String>, enabled_mcp_servers: Option<Vec<String>>,
abort_signal: AbortSignal, abort_signal: AbortSignal,
) -> Result<()> { ) -> Result<()> {
let policy = SkillPolicy::effective( let policy = SkillPolicy::effective(
@@ -2175,21 +2176,23 @@ impl RequestContext {
let enabled_mcp_servers = if policy.skills_enabled && app.mcp_server_support { let enabled_mcp_servers = if policy.skills_enabled && app.mcp_server_support {
let skill_mcps = self.skill_registry.loaded_mcp_servers(); let skill_mcps = self.skill_registry.loaded_mcp_servers();
match (enabled_mcp_servers.as_deref(), skill_mcps.is_empty()) { let has_all = enabled_mcp_servers
(Some("all"), _) | (_, true) => enabled_mcp_servers, .as_ref()
(base, false) => { .map(|v| v.iter().any(|s| s.trim() == "all"))
let mut merged: BTreeSet<String> = skill_mcps; .unwrap_or(false);
if let Some(s) = base { if has_all || skill_mcps.is_empty() {
for token in s.split(',') { enabled_mcp_servers
let t = token.trim(); } else {
if !t.is_empty() { let mut merged: BTreeSet<String> = skill_mcps;
merged.insert(t.to_string()); if let Some(servers) = &enabled_mcp_servers {
} for token in servers {
let t = token.trim();
if !t.is_empty() {
merged.insert(t.to_string());
} }
} }
Some(merged.into_iter().collect::<Vec<_>>().join(","))
} }
Some(merged.into_iter().collect())
} }
} else { } else {
enabled_mcp_servers enabled_mcp_servers
@@ -2201,12 +2204,12 @@ impl RequestContext {
&& let Some(mcp_config) = &self.app.mcp_config && let Some(mcp_config) = &self.app.mcp_config
{ {
let server_ids: Vec<String> = match &enabled_mcp_servers { let server_ids: Vec<String> = match &enabled_mcp_servers {
Some(servers) if servers == "all" => { Some(servers) if servers.iter().any(|s| s.trim() == "all") => {
mcp_config.mcp_servers.keys().cloned().collect() mcp_config.mcp_servers.keys().cloned().collect()
} }
Some(servers) => { Some(servers) => {
let mut ids = Vec::new(); let mut ids = Vec::new();
for item in servers.split(',').map(|s| s.trim()) { for item in servers.iter().map(|s| s.trim()) {
if mcp_config.mcp_servers.contains_key(item) { if mcp_config.mcp_servers.contains_key(item) {
ids.push(item.to_string()); ids.push(item.to_string());
} else if let Some(mapped) = app.mapping_mcp_servers.get(item) { } else if let Some(mapped) = app.mapping_mcp_servers.get(item) {
@@ -2285,7 +2288,7 @@ impl RequestContext {
if names.is_empty() { if names.is_empty() {
None None
} else { } else {
Some(names.join(",")) Some(names.to_vec())
} }
} else if let Some(role) = &self.role { } else if let Some(role) = &self.role {
role.enabled_mcp_servers() role.enabled_mcp_servers()
@@ -2445,7 +2448,7 @@ impl RequestContext {
} }
let mcp_servers = if app.mcp_server_support { let mcp_servers = if app.mcp_server_support {
(!agent.mcp_server_names().is_empty()).then(|| agent.mcp_server_names().join(",")) (!agent.mcp_server_names().is_empty()).then(|| agent.mcp_server_names().to_vec())
} else { } else {
if !agent.mcp_server_names().is_empty() { if !agent.mcp_server_names().is_empty() {
bail!( bail!(
@@ -2621,7 +2624,7 @@ impl RequestContext {
let skill = Skill::load(name)?; let skill = Skill::load(name)?;
let needs_mcps = skill let needs_mcps = skill
.enabled_mcp_servers() .enabled_mcp_servers()
.map(|s| !s.trim().is_empty()) .map(|v| !v.is_empty())
.unwrap_or(false); .unwrap_or(false);
if needs_mcps && !self.app.config.mcp_server_support { if needs_mcps && !self.app.config.mcp_server_support {
@@ -2728,13 +2731,13 @@ impl RequestContext {
&self, &self,
app: &AppConfig, app: &AppConfig,
start_mcp_servers: bool, start_mcp_servers: bool,
) -> Option<String> { ) -> Option<Vec<String>> {
if !start_mcp_servers || !app.mcp_server_support { if !start_mcp_servers || !app.mcp_server_support {
return None; return None;
} }
if let Some(agent) = self.agent.as_ref() { if let Some(agent) = self.agent.as_ref() {
return (!agent.mcp_server_names().is_empty()) return (!agent.mcp_server_names().is_empty())
.then(|| agent.mcp_server_names().join(",")); .then(|| agent.mcp_server_names().to_vec());
} }
if let Some(session) = self.session.as_ref() { if let Some(session) = self.session.as_ref() {
return session.enabled_mcp_servers(); return session.enabled_mcp_servers();
@@ -3227,7 +3230,7 @@ mod tests {
let app = ctx.app.config.clone(); let app = ctx.app.config.clone();
let abort = utils::create_abort_signal(); let abort = utils::create_abort_signal();
run_async(ctx.rebuild_tool_scope(&app, Some("all".to_string()), abort)).unwrap(); run_async(ctx.rebuild_tool_scope(&app, Some(vec!["all".to_string()]), abort)).unwrap();
assert!(ctx.tool_scope.mcp_runtime.is_empty()); assert!(ctx.tool_scope.mcp_runtime.is_empty());
} }
@@ -3255,7 +3258,7 @@ mod tests {
let app = ctx.app.config.clone(); let app = ctx.app.config.clone();
let abort = utils::create_abort_signal(); let abort = utils::create_abort_signal();
run_async(ctx.rebuild_tool_scope(&app, Some("all".to_string()), abort)).unwrap(); run_async(ctx.rebuild_tool_scope(&app, Some(vec!["all".to_string()]), abort)).unwrap();
assert!(ctx.tool_scope.mcp_runtime.is_empty()); assert!(ctx.tool_scope.mcp_runtime.is_empty());
} }
@@ -3417,7 +3420,7 @@ mod tests {
}; };
let ctx = RequestContext::new(app_state, WorkingMode::Cmd); let ctx = RequestContext::new(app_state, WorkingMode::Cmd);
let mut role = Role::new("r", "p"); let mut role = Role::new("r", "p");
role.set_enabled_mcp_servers(Some("all".to_string())); role.set_enabled_mcp_servers(Some(vec!["all".to_string()]));
let result = ctx.select_enabled_mcp_servers(&role); let result = ctx.select_enabled_mcp_servers(&role);
assert!(result.is_empty()); assert!(result.is_empty());
} }
@@ -3430,7 +3433,7 @@ mod tests {
.append_mcp_meta_functions(vec!["github".into(), "slack".into()]); .append_mcp_meta_functions(vec!["github".into(), "slack".into()]);
let mut role = Role::new("r", "p"); let mut role = Role::new("r", "p");
role.set_enabled_mcp_servers(Some("all".to_string())); role.set_enabled_mcp_servers(Some(vec!["all".to_string()]));
let fns = ctx.select_enabled_mcp_servers(&role); let fns = ctx.select_enabled_mcp_servers(&role);
let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect(); let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect();
@@ -3447,7 +3450,7 @@ mod tests {
.append_mcp_meta_functions(vec!["github".into(), "slack".into()]); .append_mcp_meta_functions(vec!["github".into(), "slack".into()]);
let mut role = Role::new("r", "p"); let mut role = Role::new("r", "p");
role.set_enabled_mcp_servers(Some("github".to_string())); role.set_enabled_mcp_servers(Some(vec!["github".to_string()]));
let fns = ctx.select_enabled_mcp_servers(&role); let fns = ctx.select_enabled_mcp_servers(&role);
let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect(); let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect();
+22 -13
View File
@@ -29,12 +29,12 @@ pub trait RoleLike {
fn temperature(&self) -> Option<f64>; fn temperature(&self) -> Option<f64>;
fn top_p(&self) -> Option<f64>; fn top_p(&self) -> Option<f64>;
fn enabled_tools(&self) -> Option<String>; fn enabled_tools(&self) -> Option<String>;
fn enabled_mcp_servers(&self) -> Option<String>; fn enabled_mcp_servers(&self) -> Option<Vec<String>>;
fn set_model(&mut self, model: Model); fn set_model(&mut self, model: Model);
fn set_temperature(&mut self, value: Option<f64>); fn set_temperature(&mut self, value: Option<f64>);
fn set_top_p(&mut self, value: Option<f64>); fn set_top_p(&mut self, value: Option<f64>);
fn set_enabled_tools(&mut self, value: Option<String>); fn set_enabled_tools(&mut self, value: Option<String>);
fn set_enabled_mcp_servers(&mut self, value: Option<String>); fn set_enabled_mcp_servers(&mut self, value: Option<Vec<String>>);
} }
#[derive(Debug, Clone, Default, Deserialize, Serialize)] #[derive(Debug, Clone, Default, Deserialize, Serialize)]
@@ -53,8 +53,12 @@ pub struct Role {
top_p: Option<f64>, top_p: Option<f64>,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
enabled_tools: Option<String>, enabled_tools: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(
enabled_mcp_servers: Option<String>, default,
skip_serializing_if = "Option::is_none",
deserialize_with = "super::deserialize_csv_or_vec"
)]
enabled_mcp_servers: Option<Vec<String>>,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
skills_enabled: Option<bool>, skills_enabled: Option<bool>,
#[serde( #[serde(
@@ -104,10 +108,10 @@ impl Role {
"top_p" => role.top_p = value.as_f64(), "top_p" => role.top_p = value.as_f64(),
"enabled_tools" => role.enabled_tools = value.as_str().map(|v| v.to_string()), "enabled_tools" => role.enabled_tools = value.as_str().map(|v| v.to_string()),
"enabled_mcp_servers" => { "enabled_mcp_servers" => {
role.enabled_mcp_servers = value.as_str().map(|v| v.to_string()) role.enabled_mcp_servers = parse_string_or_array(value)
} }
"skills_enabled" => role.skills_enabled = value.as_bool(), "skills_enabled" => role.skills_enabled = value.as_bool(),
"enabled_skills" => role.enabled_skills = parse_enabled_skills_value(value), "enabled_skills" => role.enabled_skills = parse_string_or_array(value),
"auto_continue" => role.auto_continue = value.as_bool(), "auto_continue" => role.auto_continue = value.as_bool(),
"max_auto_continues" => { "max_auto_continues" => {
role.max_auto_continues = value.as_u64().map(|v| v as usize) role.max_auto_continues = value.as_u64().map(|v| v as usize)
@@ -154,8 +158,10 @@ impl Role {
if let Some(enabled_tools) = self.enabled_tools() { if let Some(enabled_tools) = self.enabled_tools() {
metadata.push(format!("enabled_tools: {enabled_tools}")); metadata.push(format!("enabled_tools: {enabled_tools}"));
} }
if let Some(enabled_mcp_servers) = self.enabled_mcp_servers() { if let Some(enabled_mcp_servers) = &self.enabled_mcp_servers {
metadata.push(format!("enabled_mcp_servers: {enabled_mcp_servers}")); let inline =
serde_json::to_string(enabled_mcp_servers).unwrap_or_else(|_| "[]".to_string());
metadata.push(format!("enabled_mcp_servers: {inline}"));
} }
if let Some(skills_enabled) = self.skills_enabled { if let Some(skills_enabled) = self.skills_enabled {
metadata.push(format!("skills_enabled: {skills_enabled}")); metadata.push(format!("skills_enabled: {skills_enabled}"));
@@ -231,7 +237,7 @@ impl Role {
temperature: Option<f64>, temperature: Option<f64>,
top_p: Option<f64>, top_p: Option<f64>,
enabled_tools: Option<String>, enabled_tools: Option<String>,
enabled_mcp_servers: Option<String>, enabled_mcp_servers: Option<Vec<String>>,
) { ) {
self.set_model(model.clone()); self.set_model(model.clone());
if temperature.is_some() { if temperature.is_some() {
@@ -369,7 +375,7 @@ impl RoleLike for Role {
self.enabled_tools.clone() self.enabled_tools.clone()
} }
fn enabled_mcp_servers(&self) -> Option<String> { fn enabled_mcp_servers(&self) -> Option<Vec<String>> {
self.enabled_mcp_servers.clone() self.enabled_mcp_servers.clone()
} }
@@ -392,12 +398,12 @@ impl RoleLike for Role {
self.enabled_tools = value; self.enabled_tools = value;
} }
fn set_enabled_mcp_servers(&mut self, value: Option<String>) { fn set_enabled_mcp_servers(&mut self, value: Option<Vec<String>>) {
self.enabled_mcp_servers = value; self.enabled_mcp_servers = value;
} }
} }
fn parse_enabled_skills_value(value: &Value) -> Option<Vec<String>> { fn parse_string_or_array(value: &Value) -> Option<Vec<String>> {
if value.is_null() { if value.is_null() {
return None; return None;
} }
@@ -500,7 +506,10 @@ mod tests {
fn role_new_parses_enabled_mcp_servers() { fn role_new_parses_enabled_mcp_servers() {
let content = "---\nenabled_mcp_servers: github,jira\n---\nPrompt"; let content = "---\nenabled_mcp_servers: github,jira\n---\nPrompt";
let role = Role::new("test", content); let role = Role::new("test", content);
assert_eq!(role.enabled_mcp_servers(), Some("github,jira".to_string())); assert_eq!(
role.enabled_mcp_servers(),
Some(vec!["github".to_string(), "jira".to_string()])
);
} }
#[test] #[test]
+24 -6
View File
@@ -26,8 +26,12 @@ pub struct Session {
top_p: Option<f64>, top_p: Option<f64>,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
enabled_tools: Option<String>, enabled_tools: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(
enabled_mcp_servers: Option<String>, default,
skip_serializing_if = "Option::is_none",
deserialize_with = "super::deserialize_csv_or_vec"
)]
enabled_mcp_servers: Option<Vec<String>>,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
skills_enabled: Option<bool>, skills_enabled: Option<bool>,
#[serde( #[serde(
@@ -196,7 +200,13 @@ impl Session {
data["enabled_tools"] = enabled_tools.into(); data["enabled_tools"] = enabled_tools.into();
} }
if let Some(enabled_mcp_servers) = self.enabled_mcp_servers() { if let Some(enabled_mcp_servers) = self.enabled_mcp_servers() {
data["enabled_mcp_servers"] = enabled_mcp_servers.into(); data["enabled_mcp_servers"] = json!(enabled_mcp_servers);
}
if let Some(skills_enabled) = self.skills_enabled() {
data["skills_enabled"] = skills_enabled.into();
}
if let Some(enabled_skills) = self.enabled_skills() {
data["enabled_skills"] = json!(enabled_skills);
} }
if let Some(save_session) = self.save_session() { if let Some(save_session) = self.save_session() {
data["save_session"] = save_session.into(); data["save_session"] = save_session.into();
@@ -257,7 +267,15 @@ impl Session {
} }
if let Some(enabled_mcp_servers) = self.enabled_mcp_servers() { if let Some(enabled_mcp_servers) = self.enabled_mcp_servers() {
items.push(("enabled_mcp_servers", enabled_mcp_servers)); items.push(("enabled_mcp_servers", enabled_mcp_servers.join(",")));
}
if let Some(skills_enabled) = self.skills_enabled() {
items.push(("skills_enabled", skills_enabled.to_string()));
}
if let Some(enabled_skills) = self.enabled_skills() {
items.push(("enabled_skills", enabled_skills.join(",")));
} }
if let Some(save_session) = self.save_session() { if let Some(save_session) = self.save_session() {
@@ -697,7 +715,7 @@ impl RoleLike for Session {
self.enabled_tools.clone() self.enabled_tools.clone()
} }
fn enabled_mcp_servers(&self) -> Option<String> { fn enabled_mcp_servers(&self) -> Option<Vec<String>> {
self.enabled_mcp_servers.clone() self.enabled_mcp_servers.clone()
} }
@@ -731,7 +749,7 @@ impl RoleLike for Session {
} }
} }
fn set_enabled_mcp_servers(&mut self, value: Option<String>) { fn set_enabled_mcp_servers(&mut self, value: Option<Vec<String>>) {
if self.enabled_mcp_servers != value { if self.enabled_mcp_servers != value {
self.enabled_mcp_servers = value; self.enabled_mcp_servers = value;
self.dirty = true; self.dirty = true;
+26 -5
View File
@@ -35,7 +35,7 @@ pub struct Skill {
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
enabled_tools: Option<String>, enabled_tools: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
enabled_mcp_servers: Option<String>, enabled_mcp_servers: Option<Vec<String>>,
#[serde(skip_serializing_if = "Option::is_none")] #[serde(skip_serializing_if = "Option::is_none")]
auto_unload: Option<bool>, auto_unload: Option<bool>,
} }
@@ -72,7 +72,7 @@ impl Skill {
skill.enabled_tools = value.as_str().map(|v| v.to_string()); skill.enabled_tools = value.as_str().map(|v| v.to_string());
} }
"enabled_mcp_servers" => { "enabled_mcp_servers" => {
skill.enabled_mcp_servers = value.as_str().map(|v| v.to_string()); skill.enabled_mcp_servers = parse_skill_string_or_array(value);
} }
"auto_unload" => { "auto_unload" => {
skill.auto_unload = value.as_bool(); skill.auto_unload = value.as_bool();
@@ -138,7 +138,7 @@ impl Skill {
self.enabled_tools.as_deref() self.enabled_tools.as_deref()
} }
pub fn enabled_mcp_servers(&self) -> Option<&str> { pub fn enabled_mcp_servers(&self) -> Option<&[String]> {
self.enabled_mcp_servers.as_deref() self.enabled_mcp_servers.as_deref()
} }
@@ -157,11 +157,29 @@ impl Skill {
fn declares_mcp_servers(&self) -> bool { fn declares_mcp_servers(&self) -> bool {
self.enabled_mcp_servers self.enabled_mcp_servers
.as_deref() .as_deref()
.map(|s| !s.trim().is_empty()) .map(|servers| !servers.is_empty())
.unwrap_or(false) .unwrap_or(false)
} }
} }
fn parse_skill_string_or_array(value: &Value) -> Option<Vec<String>> {
if value.is_null() {
return None;
}
if let Some(s) = value.as_str() {
return Some(csv_to_vec(s));
}
if let Some(arr) = value.as_array() {
let items: Vec<String> = arr
.iter()
.filter_map(|v| v.as_str().map(|s| s.trim().to_string()))
.filter(|s| !s.is_empty())
.collect();
return Some(items);
}
None
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
@@ -190,7 +208,10 @@ mod tests {
assert_eq!(skill.name(), "git-master"); assert_eq!(skill.name(), "git-master");
assert_eq!(skill.description(), "Atomic commits, rebase surgery"); assert_eq!(skill.description(), "Atomic commits, rebase surgery");
assert_eq!(skill.enabled_tools(), Some("shell,fs")); assert_eq!(skill.enabled_tools(), Some("shell,fs"));
assert_eq!(skill.enabled_mcp_servers(), Some("github")); assert_eq!(
skill.enabled_mcp_servers(),
Some(["github".to_string()].as_slice())
);
assert!(skill.auto_unload()); assert!(skill.auto_unload());
assert_eq!(skill.body(), "You are a git expert"); assert_eq!(skill.body(), "You are a git expert");
} }
+12 -7
View File
@@ -38,8 +38,8 @@ impl SkillRegistry {
pub fn loaded_mcp_servers(&self) -> BTreeSet<String> { pub fn loaded_mcp_servers(&self) -> BTreeSet<String> {
let mut out = BTreeSet::new(); let mut out = BTreeSet::new();
for skill in self.loaded.values() { for skill in self.loaded.values() {
if let Some(csv) = skill.enabled_mcp_servers() { if let Some(servers) = skill.enabled_mcp_servers() {
for token in csv.split(',') { for token in servers {
let t = token.trim(); let t = token.trim();
if !t.is_empty() { if !t.is_empty() {
out.insert(t.to_string()); out.insert(t.to_string());
@@ -70,11 +70,16 @@ impl SkillRegistry {
let base_mcps_set = effective.enabled_mcp_servers().is_some(); let base_mcps_set = effective.enabled_mcp_servers().is_some();
let mut tools = parse_csv(effective.enabled_tools().as_deref()); let mut tools = parse_csv(effective.enabled_tools().as_deref());
let mut mcps = parse_csv(effective.enabled_mcp_servers().as_deref()); let mut mcps: BTreeSet<String> = effective
.enabled_mcp_servers()
.map(|v| v.into_iter().collect())
.unwrap_or_default();
for (_, skill) in &self.loaded { for (_, skill) in &self.loaded {
tools.extend(parse_csv(skill.enabled_tools())); tools.extend(parse_csv(skill.enabled_tools()));
mcps.extend(parse_csv(skill.enabled_mcp_servers())); if let Some(servers) = skill.enabled_mcp_servers() {
mcps.extend(servers.iter().cloned());
}
if !skip_body && !skill.body().is_empty() { if !skip_body && !skill.body().is_empty() {
let separator = if effective.is_empty_prompt() { let separator = if effective.is_empty_prompt() {
"" ""
@@ -91,7 +96,7 @@ impl SkillRegistry {
} }
if base_mcps_set || !mcps.is_empty() { if base_mcps_set || !mcps.is_empty() {
effective.set_enabled_mcp_servers(Some(join_csv(&mcps))); effective.set_enabled_mcp_servers(Some(mcps.into_iter().collect()));
} }
effective effective
@@ -231,8 +236,8 @@ mod tests {
let tools: BTreeSet<&str> = tools_str.split(',').collect(); let tools: BTreeSet<&str> = tools_str.split(',').collect();
assert_eq!(tools, BTreeSet::from(["fs", "git", "shell", "web_search"])); assert_eq!(tools, BTreeSet::from(["fs", "git", "shell", "web_search"]));
let mcps_str = effective.enabled_mcp_servers().unwrap(); let mcps_vec = effective.enabled_mcp_servers().unwrap();
let mcps: BTreeSet<&str> = mcps_str.split(',').collect(); let mcps: BTreeSet<&str> = mcps_vec.iter().map(|s| s.as_str()).collect();
assert_eq!(mcps, BTreeSet::from(["github", "jira"])); assert_eq!(mcps, BTreeSet::from(["github", "jira"]));
} }
+2 -2
View File
@@ -128,7 +128,7 @@ fn handle_list(ctx: &RequestContext, policy: &SkillPolicy) -> Result<Value> {
"name": skill.name(), "name": skill.name(),
"description": skill.description(), "description": skill.description(),
"grants_tools": csv_to_vec(skill.enabled_tools()), "grants_tools": csv_to_vec(skill.enabled_tools()),
"grants_mcp_servers": csv_to_vec(skill.enabled_mcp_servers()), "grants_mcp_servers": skill.enabled_mcp_servers().unwrap_or_default(),
"loaded": ctx.skill_registry.is_loaded(skill.name()), "loaded": ctx.skill_registry.is_loaded(skill.name()),
})); }));
} }
@@ -170,7 +170,7 @@ async fn handle_load(
.unwrap_or(false); .unwrap_or(false);
let mcps_declared = skill let mcps_declared = skill
.enabled_mcp_servers() .enabled_mcp_servers()
.map(|s| !s.trim().is_empty()) .map(|v| !v.is_empty())
.unwrap_or(false); .unwrap_or(false);
if tools_declared && !function_calling_on { if tools_declared && !function_calling_on {
+3 -3
View File
@@ -257,7 +257,7 @@ fn build_inline_role(
if node.tools.as_deref().unwrap_or_default().is_empty() { if node.tools.as_deref().unwrap_or_default().is_empty() {
role.set_enabled_tools(Some(String::new())); role.set_enabled_tools(Some(String::new()));
role.set_enabled_mcp_servers(Some(String::new())); role.set_enabled_mcp_servers(Some(Vec::new()));
} else { } else {
if !regular_tools.is_empty() { if !regular_tools.is_empty() {
role.set_enabled_tools(Some(regular_tools.join(","))); role.set_enabled_tools(Some(regular_tools.join(",")));
@@ -265,9 +265,9 @@ fn build_inline_role(
role.set_enabled_tools(Some(String::new())); role.set_enabled_tools(Some(String::new()));
} }
if !mcp_servers.is_empty() { if !mcp_servers.is_empty() {
role.set_enabled_mcp_servers(Some(mcp_servers.join(","))); role.set_enabled_mcp_servers(Some(mcp_servers.to_vec()));
} else { } else {
role.set_enabled_mcp_servers(Some(String::new())); role.set_enabled_mcp_servers(Some(Vec::new()));
} }
} }
+5 -2
View File
@@ -56,7 +56,7 @@ async fn extract_via_extractor(
fn build_extractor_role() -> Result<Role> { fn build_extractor_role() -> Result<Role> {
let mut role = Role::new(EXTRACTOR_ROLE_NAME, EXTRACTOR_ROLE_PROMPT); let mut role = Role::new(EXTRACTOR_ROLE_NAME, EXTRACTOR_ROLE_PROMPT);
role.set_enabled_tools(Some(String::new())); role.set_enabled_tools(Some(String::new()));
role.set_enabled_mcp_servers(Some(String::new())); role.set_enabled_mcp_servers(Some(Vec::new()));
Ok(role) Ok(role)
} }
@@ -184,6 +184,9 @@ mod tests {
let role = build_extractor_role().expect("builtin role must exist"); let role = build_extractor_role().expect("builtin role must exist");
assert_eq!(role.enabled_tools().as_deref(), Some("")); assert_eq!(role.enabled_tools().as_deref(), Some(""));
assert_eq!(role.enabled_mcp_servers().as_deref(), Some("")); assert_eq!(
role.enabled_mcp_servers().as_deref(),
Some([].as_slice())
);
} }
} }
+20 -12
View File
@@ -146,7 +146,7 @@ impl McpRegistry {
pub async fn init( pub async fn init(
log_path: Option<PathBuf>, log_path: Option<PathBuf>,
start_mcp_servers: bool, start_mcp_servers: bool,
enabled_mcp_servers: Option<String>, enabled_mcp_servers: Option<Vec<String>>,
abort_signal: AbortSignal, abort_signal: AbortSignal,
app_config: &AppConfig, app_config: &AppConfig,
vault: &Vault, vault: &Vault,
@@ -216,7 +216,7 @@ impl McpRegistry {
async fn start_select_mcp_servers( async fn start_select_mcp_servers(
&mut self, &mut self,
enabled_mcp_servers: Option<String>, enabled_mcp_servers: Option<Vec<String>>,
) -> Result<()> { ) -> Result<()> {
if self.config.is_none() { if self.config.is_none() {
debug!( debug!(
@@ -292,15 +292,15 @@ impl McpRegistry {
Ok((id.to_string(), service, catalog)) Ok((id.to_string(), service, catalog))
} }
fn resolve_server_ids(&self, enabled_mcp_servers: Option<String>) -> Vec<String> { fn resolve_server_ids(&self, enabled_mcp_servers: Option<Vec<String>>) -> Vec<String> {
if let Some(config) = &self.config if let Some(config) = &self.config
&& let Some(servers) = enabled_mcp_servers && let Some(servers) = enabled_mcp_servers
{ {
if servers == "all" { if servers.iter().any(|s| s.trim() == "all") {
config.mcp_servers.keys().cloned().collect() config.mcp_servers.keys().cloned().collect()
} else { } else {
let enabled_servers: HashSet<String> = let enabled_servers: HashSet<String> =
servers.split(',').map(|s| s.trim().to_string()).collect(); servers.into_iter().map(|s| s.trim().to_string()).collect();
config config
.mcp_servers .mcp_servers
.keys() .keys()
@@ -754,7 +754,7 @@ mod tests {
#[test] #[test]
fn resolve_all_returns_all_configured_servers() { fn resolve_all_returns_all_configured_servers() {
let registry = make_registry_with_config(&["github", "slack", "jira"]); let registry = make_registry_with_config(&["github", "slack", "jira"]);
let mut ids = registry.resolve_server_ids(Some("all".to_string())); let mut ids = registry.resolve_server_ids(Some(vec!["all".to_string()]));
ids.sort(); ids.sort();
assert_eq!(ids, vec!["github", "jira", "slack"]); assert_eq!(ids, vec!["github", "jira", "slack"]);
} }
@@ -762,7 +762,8 @@ mod tests {
#[test] #[test]
fn resolve_comma_separated_returns_matching_servers() { fn resolve_comma_separated_returns_matching_servers() {
let registry = make_registry_with_config(&["github", "slack", "jira"]); let registry = make_registry_with_config(&["github", "slack", "jira"]);
let mut ids = registry.resolve_server_ids(Some("github, jira".to_string())); let mut ids = registry
.resolve_server_ids(Some(vec!["github".to_string(), "jira".to_string()]));
ids.sort(); ids.sort();
assert_eq!(ids, vec!["github", "jira"]); assert_eq!(ids, vec!["github", "jira"]);
} }
@@ -770,7 +771,7 @@ mod tests {
#[test] #[test]
fn resolve_single_server_name() { fn resolve_single_server_name() {
let registry = make_registry_with_config(&["github", "slack"]); let registry = make_registry_with_config(&["github", "slack"]);
let ids = registry.resolve_server_ids(Some("slack".to_string())); let ids = registry.resolve_server_ids(Some(vec!["slack".to_string()]));
assert_eq!(ids, vec!["slack"]); assert_eq!(ids, vec!["slack"]);
} }
@@ -784,28 +785,35 @@ mod tests {
#[test] #[test]
fn resolve_no_config_returns_empty() { fn resolve_no_config_returns_empty() {
let registry = McpRegistry::default(); let registry = McpRegistry::default();
let ids = registry.resolve_server_ids(Some("all".to_string())); let ids = registry.resolve_server_ids(Some(vec!["all".to_string()]));
assert!(ids.is_empty()); assert!(ids.is_empty());
} }
#[test] #[test]
fn resolve_nonexistent_server_filtered_out() { fn resolve_nonexistent_server_filtered_out() {
let registry = make_registry_with_config(&["github"]); let registry = make_registry_with_config(&["github"]);
let ids = registry.resolve_server_ids(Some("github, nonexistent".to_string())); let ids = registry.resolve_server_ids(Some(vec![
"github".to_string(),
"nonexistent".to_string(),
]));
assert_eq!(ids, vec!["github"]); assert_eq!(ids, vec!["github"]);
} }
#[test] #[test]
fn resolve_all_nonexistent_returns_empty() { fn resolve_all_nonexistent_returns_empty() {
let registry = make_registry_with_config(&["github"]); let registry = make_registry_with_config(&["github"]);
let ids = registry.resolve_server_ids(Some("foo, bar".to_string())); let ids =
registry.resolve_server_ids(Some(vec!["foo".to_string(), "bar".to_string()]));
assert!(ids.is_empty()); assert!(ids.is_empty());
} }
#[test] #[test]
fn resolve_trims_whitespace() { fn resolve_trims_whitespace() {
let registry = make_registry_with_config(&["github", "slack"]); let registry = make_registry_with_config(&["github", "slack"]);
let mut ids = registry.resolve_server_ids(Some(" github , slack ".to_string())); let mut ids = registry.resolve_server_ids(Some(vec![
" github ".to_string(),
" slack ".to_string(),
]));
ids.sort(); ids.sort();
assert_eq!(ids, vec!["github", "slack"]); assert_eq!(ids, vec!["github", "slack"]);
} }