fix: skill support also requires function calling to be enabled
This commit is contained in:
@@ -2519,6 +2519,10 @@ impl RequestContext {
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub async fn load_skill_repl(&mut self, name: &str, abort_signal: AbortSignal) -> Result<()> {
|
pub async fn load_skill_repl(&mut self, name: &str, abort_signal: AbortSignal) -> Result<()> {
|
||||||
|
if !self.app.config.function_calling_support {
|
||||||
|
bail!("Skills require function calling, which is disabled. Enable function calling in your config then try again.");
|
||||||
|
}
|
||||||
|
|
||||||
if !paths::has_skill(name) {
|
if !paths::has_skill(name) {
|
||||||
bail!(
|
bail!(
|
||||||
"Skill '{name}' is not installed (expected at {})",
|
"Skill '{name}' is not installed (expected at {})",
|
||||||
@@ -2542,22 +2546,12 @@ impl RequestContext {
|
|||||||
}
|
}
|
||||||
|
|
||||||
let skill = Skill::load(name)?;
|
let skill = Skill::load(name)?;
|
||||||
let fn_on = self.app.config.function_calling_support;
|
|
||||||
let mcp_on = self.app.config.mcp_server_support;
|
|
||||||
let needs_tools = skill
|
|
||||||
.enabled_tools()
|
|
||||||
.map(|s| !s.trim().is_empty())
|
|
||||||
.unwrap_or(false);
|
|
||||||
let needs_mcps = skill
|
let needs_mcps = skill
|
||||||
.enabled_mcp_servers()
|
.enabled_mcp_servers()
|
||||||
.map(|s| !s.trim().is_empty())
|
.map(|s| !s.trim().is_empty())
|
||||||
.unwrap_or(false);
|
.unwrap_or(false);
|
||||||
|
|
||||||
if needs_tools && !fn_on {
|
if needs_mcps && !self.app.config.mcp_server_support {
|
||||||
bail!("Skill '{name}' requires function calling, which is disabled");
|
|
||||||
}
|
|
||||||
|
|
||||||
if needs_mcps && !mcp_on {
|
|
||||||
bail!("Skill '{name}' requires MCP servers, which are disabled");
|
bail!("Skill '{name}' requires MCP servers, which are disabled");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+15
-33
@@ -146,11 +146,7 @@ impl Skill {
|
|||||||
self.auto_unload.unwrap_or(false)
|
self.auto_unload.unwrap_or(false)
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn is_compatible(&self, function_calling_enabled: bool, mcp_enabled: bool) -> bool {
|
pub fn is_compatible(&self, mcp_enabled: bool) -> bool {
|
||||||
if self.declares_tools() && !function_calling_enabled {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
|
|
||||||
if self.declares_mcp_servers() && !mcp_enabled {
|
if self.declares_mcp_servers() && !mcp_enabled {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
@@ -158,13 +154,6 @@ impl Skill {
|
|||||||
true
|
true
|
||||||
}
|
}
|
||||||
|
|
||||||
fn declares_tools(&self) -> bool {
|
|
||||||
self.enabled_tools
|
|
||||||
.as_deref()
|
|
||||||
.map(|s| !s.trim().is_empty())
|
|
||||||
.unwrap_or(false)
|
|
||||||
}
|
|
||||||
|
|
||||||
fn declares_mcp_servers(&self) -> bool {
|
fn declares_mcp_servers(&self) -> bool {
|
||||||
self.enabled_mcp_servers
|
self.enabled_mcp_servers
|
||||||
.as_deref()
|
.as_deref()
|
||||||
@@ -271,25 +260,21 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn is_compatible_knowledge_only_passes_all_combinations() {
|
fn is_compatible_knowledge_only_passes_both_mcp_states() {
|
||||||
let skill = Skill::new("test", "Just knowledge");
|
let skill = Skill::new("test", "Just knowledge");
|
||||||
|
|
||||||
assert!(skill.is_compatible(false, false));
|
assert!(skill.is_compatible(false));
|
||||||
assert!(skill.is_compatible(true, false));
|
assert!(skill.is_compatible(true));
|
||||||
assert!(skill.is_compatible(false, true));
|
|
||||||
assert!(skill.is_compatible(true, true));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn is_compatible_with_tools_requires_function_calling() {
|
fn is_compatible_with_tools_only_passes_both_mcp_states() {
|
||||||
let content = "---\nenabled_tools: shell\n---\nbody";
|
let content = "---\nenabled_tools: shell\n---\nbody";
|
||||||
|
|
||||||
let skill = Skill::new("test", content);
|
let skill = Skill::new("test", content);
|
||||||
|
|
||||||
assert!(!skill.is_compatible(false, true));
|
assert!(skill.is_compatible(false));
|
||||||
assert!(!skill.is_compatible(false, false));
|
assert!(skill.is_compatible(true));
|
||||||
assert!(skill.is_compatible(true, true));
|
|
||||||
assert!(skill.is_compatible(true, false));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@@ -298,29 +283,26 @@ mod tests {
|
|||||||
|
|
||||||
let skill = Skill::new("test", content);
|
let skill = Skill::new("test", content);
|
||||||
|
|
||||||
assert!(!skill.is_compatible(true, false));
|
assert!(!skill.is_compatible(false));
|
||||||
assert!(!skill.is_compatible(false, false));
|
assert!(skill.is_compatible(true));
|
||||||
assert!(skill.is_compatible(true, true));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn is_compatible_requires_both_when_both_declared() {
|
fn is_compatible_with_both_requires_mcp_enabled() {
|
||||||
let content = "---\nenabled_tools: shell\nenabled_mcp_servers: github\n---\nbody";
|
let content = "---\nenabled_tools: shell\nenabled_mcp_servers: github\n---\nbody";
|
||||||
|
|
||||||
let skill = Skill::new("test", content);
|
let skill = Skill::new("test", content);
|
||||||
|
|
||||||
assert!(!skill.is_compatible(true, false));
|
assert!(!skill.is_compatible(false));
|
||||||
assert!(!skill.is_compatible(false, true));
|
assert!(skill.is_compatible(true));
|
||||||
assert!(!skill.is_compatible(false, false));
|
|
||||||
assert!(skill.is_compatible(true, true));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn is_compatible_empty_string_tools_is_knowledge_only() {
|
fn is_compatible_empty_string_mcps_is_knowledge_only() {
|
||||||
let content = "---\nenabled_tools: \"\"\n---\nbody";
|
let content = "---\nenabled_mcp_servers: \"\"\n---\nbody";
|
||||||
|
|
||||||
let skill = Skill::new("test", content);
|
let skill = Skill::new("test", content);
|
||||||
|
|
||||||
assert!(skill.is_compatible(false, false));
|
assert!(skill.is_compatible(false));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -102,7 +102,6 @@ pub async fn handle_skill_tool(
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn handle_list(ctx: &RequestContext, policy: &SkillPolicy) -> Result<Value> {
|
fn handle_list(ctx: &RequestContext, policy: &SkillPolicy) -> Result<Value> {
|
||||||
let function_calling_on = ctx.app.config.function_calling_support;
|
|
||||||
let mcp_on = ctx.app.config.mcp_server_support;
|
let mcp_on = ctx.app.config.mcp_server_support;
|
||||||
|
|
||||||
let mut entries = Vec::new();
|
let mut entries = Vec::new();
|
||||||
@@ -118,9 +117,9 @@ fn handle_list(ctx: &RequestContext, policy: &SkillPolicy) -> Result<Value> {
|
|||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
if !skill.is_compatible(function_calling_on, mcp_on) {
|
if !skill.is_compatible(mcp_on) {
|
||||||
warn!(
|
warn!(
|
||||||
"Skill '{name}' filtered from list: declares tools or MCP servers but those features are disabled"
|
"Skill '{name}' filtered from list: declares MCP servers but MCP support is disabled"
|
||||||
);
|
);
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user