test: added a few additional tests to the request_context surrounding the skills system

assets/roles/create-prompt.md

@@ -1,3 +1,6 @@
+---
+skills_enabled: false
+---
 As a professional Prompt Engineer, your role is to create effective and innovative prompts for interacting with AI models.
 
 Your core skills include:

assets/roles/create-title.md

@@ -1,3 +1,6 @@
+---
+skills_enabled: false
+---
 Create a concise, 3-6 word title.
 
 **Notes**:

assets/roles/explain-shell.md

@@ -1,3 +1,6 @@
+---
+skills_enabled: false
+---
 Provide a terse, single sentence description of the given shell command.
 Describe each argument and option of the command.
 Provide short responses in about 80 words.

assets/roles/shell.md

@@ -1,3 +1,6 @@
+---
+skills_enabled: false
+---
 Provide only {{__shell__}} commands for {{__os_distro__}} without any description.
 Ensure the output is a valid {{__shell__}} command.
 If there is a lack of details, provide most logical solution.

src/config/request_context.rs

@@ -652,10 +652,10 @@ impl RequestContext {
 
         if should_inject_skill_instructions(app, &policy) {
             let config = self.skill_instructions_config();
-            
+
             if config.inject {
                 let separator = if role.is_empty_prompt() { "" } else { "\n\n" };
-                
+
                 role.append_to_prompt(separator);
                 role.append_to_prompt(
                     config
@@ -1275,7 +1275,8 @@ impl RequestContext {
                     .iter()
                     .filter(|v| {
                         (v.name.starts_with(USER_FUNCTION_PREFIX)
-                            || v.name.starts_with(SKILL_FUNCTION_PREFIX))
+                            || (!matches!(role.skills_enabled(), Some(false))
+                                && v.name.starts_with(SKILL_FUNCTION_PREFIX)))
                             && !existing.contains(&v.name)
                     })
                     .cloned()
@@ -3072,11 +3073,12 @@ mod tests {
     use super::super::mcp_factory::McpFactory;
     use super::*;
     use crate::config::AppState;
-    use crate::function::ToolCall;
+    use crate::function::{ToolCall, skill};
     use crate::mcp::{McpServer, McpServersConfig, McpTransportType};
     use crate::utils;
     use crate::utils::get_env_name;
     use crate::vault::Vault;
+    use serde_json::json;
     use serial_test::serial;
     use std::env;
     use std::fs::{create_dir_all, remove_dir_all, write};
@@ -3641,6 +3643,182 @@ mod tests {
         assert!(!names.contains(&"todo__done"));
     }
 
+    #[test]
+    fn select_functions_re_adds_skill_tools_when_role_skills_enabled_unset() {
+        let mut ctx = create_test_ctx();
+        ctx.tool_scope.functions.append_skill_functions();
+
+        let mut role = Role::new("r", "p");
+        role.set_enabled_tools(Some(vec!["foo".to_string()]));
+
+        let fns = ctx.select_functions(&role).unwrap();
+        let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect();
+        assert!(names.contains(&"skill__list"));
+        assert!(names.contains(&"skill__load"));
+        assert!(names.contains(&"skill__unload"));
+    }
+
+    #[test]
+    fn select_functions_suppresses_skill_tools_when_role_skills_enabled_false() {
+        let mut ctx = create_test_ctx();
+        ctx.tool_scope.functions.append_skill_functions();
+        ctx.tool_scope.functions.append_todo_functions();
+
+        let mut role = Role::new("r", "---\nskills_enabled: false\n---\np");
+        role.set_enabled_tools(Some(vec!["todo__init".to_string()]));
+
+        let fns = ctx.select_functions(&role).unwrap();
+        let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect();
+        assert!(names.contains(&"todo__init"));
+        assert!(!names.contains(&"skill__list"));
+        assert!(!names.contains(&"skill__load"));
+        assert!(!names.contains(&"skill__unload"));
+    }
+
+    #[test]
+    fn select_functions_still_re_adds_user_tools_when_role_skills_enabled_false() {
+        let mut ctx = create_test_ctx();
+        ctx.tool_scope.functions.append_user_interaction_functions();
+        ctx.tool_scope.functions.append_skill_functions();
+
+        let mut role = Role::new("r", "---\nskills_enabled: false\n---\np");
+        role.set_enabled_tools(Some(vec!["foo".to_string()]));
+
+        let fns = ctx.select_functions(&role).unwrap();
+        let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect();
+        assert!(names.contains(&"user__ask"));
+        assert!(!names.contains(&"skill__list"));
+    }
+
+    #[test]
+    #[serial]
+    fn select_functions_re_adds_skill_tools_when_agent_skills_enabled_not_false() {
+        let _guard = TestConfigDirGuard::new();
+        let mut ctx = create_test_ctx();
+        let app = ctx.app.config.clone();
+        let agent_name = format!(
+            "test_skill_agent_{}",
+            SystemTime::now()
+                .duration_since(UNIX_EPOCH)
+                .unwrap()
+                .as_nanos()
+        );
+        let agent_dir = paths::agent_data_dir(&agent_name);
+        create_dir_all(&agent_dir).unwrap();
+        write(
+            agent_dir.join("graph.yaml"),
+            format!(
+                "name: {agent_name}\nversion: \"1.0\"\nstart: done\nnodes:\n  done:\n    type: end\n    output: ok\n"
+            ),
+        )
+        .unwrap();
+
+        let abort = utils::create_abort_signal();
+        run_async(ctx.use_agent(&app, &agent_name, None, abort)).unwrap();
+        ctx.tool_scope.functions.append_skill_functions();
+
+        let mut role = Role::new("r", "p");
+        role.set_enabled_tools(Some(vec!["foo".to_string()]));
+
+        let fns = ctx.select_functions(&role).unwrap();
+        let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect();
+        assert!(names.contains(&"skill__list"));
+        assert!(names.contains(&"skill__load"));
+        assert!(names.contains(&"skill__unload"));
+    }
+
+    #[test]
+    fn fork_for_branch_clones_skill_registry() {
+        let mut ctx = create_test_ctx();
+        let skill = Skill::new("shared", "---\nauto_unload: false\n---\nbody");
+        ctx.skill_registry.insert(skill).unwrap();
+
+        let fork = ctx.fork_for_branch();
+
+        assert!(
+            fork.skill_registry.is_loaded("shared"),
+            "Parallel branches must share loaded skills with parent"
+        );
+        assert!(ctx.skill_registry.is_loaded("shared"));
+    }
+
+    #[test]
+    fn handle_skill_tool_returns_error_when_skills_disabled() {
+        let mut ctx = create_test_ctx();
+        let role = Role::new("r", "---\nskills_enabled: false\n---\np");
+        ctx.use_role_obj(role).unwrap();
+
+        let result = run_async(skill::handle_skill_tool(
+            &mut ctx,
+            "skill__list",
+            &json!({}),
+        ))
+        .unwrap();
+
+        assert!(
+            result.get("error").is_some(),
+            "Expected error when skills are disabled, got: {result:?}"
+        );
+    }
+
+    #[test]
+    fn handle_unload_returns_error_when_skill_not_loaded() {
+        let mut ctx = create_test_ctx();
+
+        let result = run_async(skill::handle_skill_tool(
+            &mut ctx,
+            "skill__unload",
+            &json!({"name": "ghost"}),
+        ))
+        .unwrap();
+
+        assert!(
+            result.get("error").is_some(),
+            "Expected error when unloading unloaded skill, got: {result:?}"
+        );
+    }
+
+    #[test]
+    #[serial]
+    fn select_functions_suppresses_skill_tools_when_agent_skills_enabled_false() {
+        let _guard = TestConfigDirGuard::new();
+        let mut ctx = create_test_ctx();
+        let app = ctx.app.config.clone();
+        let agent_name = format!(
+            "test_skill_agent_off_{}",
+            SystemTime::now()
+                .duration_since(UNIX_EPOCH)
+                .unwrap()
+                .as_nanos()
+        );
+        let agent_dir = paths::agent_data_dir(&agent_name);
+        create_dir_all(&agent_dir).unwrap();
+        write(
+            agent_dir.join("graph.yaml"),
+            format!(
+                "name: {agent_name}\nversion: \"1.0\"\nstart: done\nnodes:\n  done:\n    type: end\n    output: ok\n"
+            ),
+        )
+        .unwrap();
+
+        let abort = utils::create_abort_signal();
+        run_async(ctx.use_agent(&app, &agent_name, None, abort)).unwrap();
+        ctx.agent
+            .as_mut()
+            .expect("agent loaded")
+            .set_skills_enabled(Some(false));
+        ctx.tool_scope.functions.append_skill_functions();
+
+        let mut role = Role::new("r", "p");
+        role.set_enabled_tools(Some(vec!["foo".to_string()]));
+
+        let fns = ctx.select_functions(&role).unwrap();
+        let names: Vec<&str> = fns.iter().map(|f| f.name.as_str()).collect();
+        assert!(!names.contains(&"skill__list"));
+        assert!(!names.contains(&"skill__load"));
+        assert!(!names.contains(&"skill__unload"));
+    }
+
     #[test]
     fn select_enabled_mcp_servers_returns_empty_when_mcp_disabled() {
         let app_state = {
@@ -3870,8 +4048,7 @@ mod tests {
 
         let input = Input::from_str(&ctx, "hello", None).unwrap();
         let app = Arc::clone(&ctx.app.config);
-        let tool_result =
-            ToolResult::new(crate::function::ToolCall::default(), serde_json::json!({}));
+        let tool_result = ToolResult::new(crate::function::ToolCall::default(), json!({}));
         ctx.after_chat_completion(app.as_ref(), &input, "", &[tool_result])
             .unwrap();