feat(mcp): add mcp_tools allowlist config surfaces across roles, sessions, agents, graphs, and skills
This commit is contained in:
9 files changed
+480
No files matched your search
@@ -506,6 +506,7 @@ mod tests {
|
||||
instructions: Some("sys".into()),
|
||||
prompt: "user".into(),
|
||||
tools: None,
|
||||
mcp_tools: None,
|
||||
model: None,
|
||||
temperature: None,
|
||||
top_p: None,
|
||||
|
||||
@@ -37,6 +37,9 @@ pub struct Graph {
|
||||
#[serde(default)]
|
||||
pub mcp_servers: Vec<String>,
|
||||
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub mcp_tools: Option<IndexMap<String, Vec<String>>>,
|
||||
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub skills_enabled: Option<bool>,
|
||||
|
||||
@@ -285,6 +288,9 @@ pub struct LlmNode {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub tools: Option<Vec<String>>,
|
||||
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub mcp_tools: Option<IndexMap<String, Vec<String>>>,
|
||||
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub model: Option<String>,
|
||||
|
||||
@@ -530,6 +536,47 @@ nodes:
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserializes_mcp_tools_at_graph_and_node_level() {
|
||||
let yaml = r#"
|
||||
name: g
|
||||
mcp_servers: [github]
|
||||
mcp_tools:
|
||||
github:
|
||||
- get_*
|
||||
- list_*
|
||||
start: l
|
||||
nodes:
|
||||
l:
|
||||
id: l
|
||||
type: llm
|
||||
prompt: hi
|
||||
mcp_tools:
|
||||
github:
|
||||
- search_code
|
||||
next: e
|
||||
e:
|
||||
id: e
|
||||
type: end
|
||||
output: done
|
||||
"#;
|
||||
let graph: Graph = serde_yaml::from_str(yaml).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
graph.mcp_tools.as_ref().unwrap().get("github"),
|
||||
Some(&vec!["get_*".to_string(), "list_*".to_string()])
|
||||
);
|
||||
match &graph.get_node("l").unwrap().node_type {
|
||||
NodeType::Llm(llm) => {
|
||||
assert_eq!(
|
||||
llm.mcp_tools.as_ref().unwrap().get("github"),
|
||||
Some(&vec!["search_code".to_string()])
|
||||
);
|
||||
}
|
||||
_ => panic!("expected Llm variant"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserializes_every_node_type() {
|
||||
let yaml = r#"
|
||||
|
||||
@@ -214,6 +214,19 @@ impl GraphValidator {
|
||||
return;
|
||||
};
|
||||
|
||||
let expand_alias = |name: &str| {
|
||||
ctx.app_config
|
||||
.mapping_mcp_servers
|
||||
.get(name)
|
||||
.into_iter()
|
||||
.flat_map(|mapped| mapped.split(',').map(|s| s.trim().to_string()))
|
||||
};
|
||||
let mut enabled_servers: HashSet<String> = ctx.mcp_servers.clone();
|
||||
for server in &ctx.mcp_servers {
|
||||
enabled_servers.extend(expand_alias(server));
|
||||
}
|
||||
let all_servers_enabled = ctx.mcp_servers.iter().any(|s| s.trim() == "all");
|
||||
|
||||
for (node_id, node) in &graph.nodes {
|
||||
let NodeType::Llm(llm) = &node.node_type else {
|
||||
continue;
|
||||
@@ -237,6 +250,23 @@ impl GraphValidator {
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(mcp_tools) = &llm.mcp_tools
|
||||
&& !all_servers_enabled
|
||||
{
|
||||
for key in mcp_tools.keys() {
|
||||
let enabled = enabled_servers.contains(key)
|
||||
|| expand_alias(key).any(|id| enabled_servers.contains(&id));
|
||||
if !enabled {
|
||||
result.error(ValidationError::with_node(
|
||||
node_id,
|
||||
format!(
|
||||
"llm node 'mcp_tools' references MCP server '{key}' not enabled by this graph"
|
||||
),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(model_id) = &llm.model
|
||||
&& Model::retrieve_model(ctx.app_config.as_ref(), model_id, ModelType::Chat)
|
||||
.is_err()
|
||||
@@ -1001,6 +1031,7 @@ mod tests {
|
||||
max_concurrent_jobs: None,
|
||||
global_tools: Vec::new(),
|
||||
mcp_servers: Vec::new(),
|
||||
mcp_tools: None,
|
||||
skills_enabled: None,
|
||||
enabled_skills: None,
|
||||
inject_skill_instructions: None,
|
||||
@@ -1099,6 +1130,7 @@ mod tests {
|
||||
instructions: None,
|
||||
prompt: "p".into(),
|
||||
tools: None,
|
||||
mcp_tools: None,
|
||||
model: None,
|
||||
temperature: None,
|
||||
top_p: None,
|
||||
@@ -1257,6 +1289,19 @@ mod tests {
|
||||
node
|
||||
}
|
||||
|
||||
fn llm_node_with_mcp_tools(id: &str, servers: &[&str]) -> Node {
|
||||
let mut node = llm_node(id, None, Some("end"));
|
||||
if let NodeType::Llm(ref mut n) = node.node_type {
|
||||
let mut mcp_tools = IndexMap::new();
|
||||
for server in servers {
|
||||
mcp_tools.insert(server.to_string(), vec!["get_*".to_string()]);
|
||||
}
|
||||
n.mcp_tools = Some(mcp_tools);
|
||||
}
|
||||
|
||||
node
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn llm_node_unknown_tool_is_an_error() {
|
||||
let graph = graph_with(
|
||||
@@ -1340,6 +1385,103 @@ mod tests {
|
||||
assert!(result.is_valid());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn llm_node_mcp_tools_enabled_server_passes() {
|
||||
let graph = graph_with(
|
||||
vec![
|
||||
("l", llm_node_with_mcp_tools("l", &["github"])),
|
||||
("end", end_node("end")),
|
||||
],
|
||||
"l",
|
||||
);
|
||||
|
||||
let result = validator()
|
||||
.with_agent_context(agent_ctx(&[], &["github"]))
|
||||
.validate(&graph);
|
||||
|
||||
assert!(result.is_valid());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn llm_node_mcp_tools_unknown_server_is_an_error() {
|
||||
let graph = graph_with(
|
||||
vec![
|
||||
("l", llm_node_with_mcp_tools("l", &["slack"])),
|
||||
("end", end_node("end")),
|
||||
],
|
||||
"l",
|
||||
);
|
||||
|
||||
let result = validator()
|
||||
.with_agent_context(agent_ctx(&[], &["github"]))
|
||||
.validate(&graph);
|
||||
|
||||
assert!(!result.is_valid());
|
||||
assert!(
|
||||
result
|
||||
.errors
|
||||
.iter()
|
||||
.any(|e| e.message.contains("'slack' not enabled"))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn llm_node_mcp_tools_alias_key_passes() {
|
||||
let graph = graph_with(
|
||||
vec![
|
||||
("l", llm_node_with_mcp_tools("l", &["gh"])),
|
||||
("end", end_node("end")),
|
||||
],
|
||||
"l",
|
||||
);
|
||||
let mut ctx = agent_ctx(&[], &["github-mcp"]);
|
||||
let mut app = AppConfig::default();
|
||||
app.mapping_mcp_servers
|
||||
.insert("gh".to_string(), "github-mcp".to_string());
|
||||
ctx.app_config = Arc::new(app);
|
||||
|
||||
let result = validator().with_agent_context(ctx).validate(&graph);
|
||||
|
||||
assert!(result.is_valid(), "errors: {:?}", result.errors);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn llm_node_mcp_tools_key_matching_alias_expansion_passes() {
|
||||
let graph = graph_with(
|
||||
vec![
|
||||
("l", llm_node_with_mcp_tools("l", &["github-mcp"])),
|
||||
("end", end_node("end")),
|
||||
],
|
||||
"l",
|
||||
);
|
||||
let mut ctx = agent_ctx(&[], &["gh"]);
|
||||
let mut app = AppConfig::default();
|
||||
app.mapping_mcp_servers
|
||||
.insert("gh".to_string(), "github-mcp".to_string());
|
||||
ctx.app_config = Arc::new(app);
|
||||
|
||||
let result = validator().with_agent_context(ctx).validate(&graph);
|
||||
|
||||
assert!(result.is_valid(), "errors: {:?}", result.errors);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn llm_node_mcp_tools_with_all_sentinel_passes() {
|
||||
let graph = graph_with(
|
||||
vec![
|
||||
("l", llm_node_with_mcp_tools("l", &["github"])),
|
||||
("end", end_node("end")),
|
||||
],
|
||||
"l",
|
||||
);
|
||||
|
||||
let result = validator()
|
||||
.with_agent_context(agent_ctx(&[], &["all"]))
|
||||
.validate(&graph);
|
||||
|
||||
assert!(result.is_valid(), "errors: {:?}", result.errors);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn llm_node_unknown_model_is_an_error() {
|
||||
let graph = graph_with(
|
||||
|
||||
Reference in new issue
Block a user