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<()> {
|
||||
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) {
|
||||
bail!(
|
||||
"Skill '{name}' is not installed (expected at {})",
|
||||
@@ -2542,22 +2546,12 @@ impl RequestContext {
|
||||
}
|
||||
|
||||
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
|
||||
.enabled_mcp_servers()
|
||||
.map(|s| !s.trim().is_empty())
|
||||
.unwrap_or(false);
|
||||
|
||||
if needs_tools && !fn_on {
|
||||
bail!("Skill '{name}' requires function calling, which is disabled");
|
||||
}
|
||||
|
||||
if needs_mcps && !mcp_on {
|
||||
if needs_mcps && !self.app.config.mcp_server_support {
|
||||
bail!("Skill '{name}' requires MCP servers, which are disabled");
|
||||
}
|
||||
|
||||
|
||||
+15
-33
@@ -146,11 +146,7 @@ impl Skill {
|
||||
self.auto_unload.unwrap_or(false)
|
||||
}
|
||||
|
||||
pub fn is_compatible(&self, function_calling_enabled: bool, mcp_enabled: bool) -> bool {
|
||||
if self.declares_tools() && !function_calling_enabled {
|
||||
return false;
|
||||
}
|
||||
|
||||
pub fn is_compatible(&self, mcp_enabled: bool) -> bool {
|
||||
if self.declares_mcp_servers() && !mcp_enabled {
|
||||
return false;
|
||||
}
|
||||
@@ -158,13 +154,6 @@ impl Skill {
|
||||
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 {
|
||||
self.enabled_mcp_servers
|
||||
.as_deref()
|
||||
@@ -271,25 +260,21 @@ mod tests {
|
||||
}
|
||||
|
||||
#[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");
|
||||
|
||||
assert!(skill.is_compatible(false, false));
|
||||
assert!(skill.is_compatible(true, false));
|
||||
assert!(skill.is_compatible(false, true));
|
||||
assert!(skill.is_compatible(true, true));
|
||||
assert!(skill.is_compatible(false));
|
||||
assert!(skill.is_compatible(true));
|
||||
}
|
||||
|
||||
#[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 skill = Skill::new("test", content);
|
||||
|
||||
assert!(!skill.is_compatible(false, true));
|
||||
assert!(!skill.is_compatible(false, false));
|
||||
assert!(skill.is_compatible(true, true));
|
||||
assert!(skill.is_compatible(true, false));
|
||||
assert!(skill.is_compatible(false));
|
||||
assert!(skill.is_compatible(true));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -298,29 +283,26 @@ mod tests {
|
||||
|
||||
let skill = Skill::new("test", content);
|
||||
|
||||
assert!(!skill.is_compatible(true, false));
|
||||
assert!(!skill.is_compatible(false, false));
|
||||
assert!(skill.is_compatible(true, true));
|
||||
assert!(!skill.is_compatible(false));
|
||||
assert!(skill.is_compatible(true));
|
||||
}
|
||||
|
||||
#[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 skill = Skill::new("test", content);
|
||||
|
||||
assert!(!skill.is_compatible(true, false));
|
||||
assert!(!skill.is_compatible(false, true));
|
||||
assert!(!skill.is_compatible(false, false));
|
||||
assert!(skill.is_compatible(true, true));
|
||||
assert!(!skill.is_compatible(false));
|
||||
assert!(skill.is_compatible(true));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn is_compatible_empty_string_tools_is_knowledge_only() {
|
||||
let content = "---\nenabled_tools: \"\"\n---\nbody";
|
||||
fn is_compatible_empty_string_mcps_is_knowledge_only() {
|
||||
let content = "---\nenabled_mcp_servers: \"\"\n---\nbody";
|
||||
|
||||
let skill = Skill::new("test", content);
|
||||
|
||||
assert!(skill.is_compatible(false, false));
|
||||
assert!(skill.is_compatible(false));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user