Skip to content

Commit 115f3db

Browse files
committed
feat(skills): add built-in skill support to UseSkill handler
- Add SkillSource::Builtin variant to distinguish embedded skills - Add load_builtin() method to SkillLoader for loading built-in skills - Update load() method to check built-ins before filesystem search - Update list() method to include built-in skills in listings - Add comprehensive tests for built-in skill loading
1 parent 4c03d5d commit 115f3db

1 file changed

Lines changed: 201 additions & 10 deletions

File tree

  • src/cortex-engine/src/tools/handlers

src/cortex-engine/src/tools/handlers/skill.rs

Lines changed: 201 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,8 @@ pub enum SkillSource {
164164
Project,
165165
/// Current directory SKILL.md
166166
Local,
167+
/// Built-in skill embedded in the agent
168+
Builtin,
167169
}
168170

169171
impl std::fmt::Display for SkillSource {
@@ -172,6 +174,7 @@ impl std::fmt::Display for SkillSource {
172174
Self::Personal => write!(f, "personal"),
173175
Self::Project => write!(f, "project"),
174176
Self::Local => write!(f, "local"),
177+
Self::Builtin => write!(f, "builtin"),
175178
}
176179
}
177180
}
@@ -191,11 +194,12 @@ impl SkillLoader {
191194
/// Create a new skill loader.
192195
///
193196
/// Skill search paths (in priority order):
194-
/// 1. `./SKILL.md` (local file)
195-
/// 2. `.agents/` (project, https://agent.md/ compatible)
196-
/// 3. `.agent/` (project, https://agent.md/ compatible)
197-
/// 4. `.cortex/skills/` (project, traditional format)
198-
/// 5. `~/.cortex/skills/` (personal)
197+
/// 1. Built-in skills (embedded in the agent)
198+
/// 2. `./SKILL.md` (local file)
199+
/// 3. `.agents/` (project, https://agent.md/ compatible)
200+
/// 4. `.agent/` (project, https://agent.md/ compatible)
201+
/// 5. `.cortex/skills/` (project, traditional format)
202+
/// 6. `~/.cortex/skills/` (personal)
199203
pub fn new(cwd: PathBuf) -> Self {
200204
let personal_dir = dirs::home_dir()
201205
.map(|h| h.join(".cortex").join("skills"))
@@ -215,9 +219,39 @@ impl SkillLoader {
215219
}
216220
}
217221

222+
/// Load a built-in skill by name.
223+
///
224+
/// Returns `Ok(Some(skill))` if the skill is a built-in and was loaded successfully,
225+
/// `Ok(None)` if the skill is not a built-in, or `Err` if parsing failed.
226+
fn load_builtin(&self, name: &str) -> Result<Option<LoadedSkill>> {
227+
use cortex_prompt_harness::prompts::builtin_skills::{get_builtin_skill, is_builtin_skill};
228+
229+
if !is_builtin_skill(name) {
230+
return Ok(None);
231+
}
232+
233+
let content = get_builtin_skill(name).ok_or_else(|| {
234+
CortexError::NotFound(format!("Built-in skill not found: {}", name))
235+
})?;
236+
237+
let (definition, markdown) = parse_skill_md(content)?;
238+
239+
Ok(Some(LoadedSkill {
240+
definition,
241+
content: markdown,
242+
path: PathBuf::from("<builtin>"),
243+
source: SkillSource::Builtin,
244+
}))
245+
}
246+
218247
/// Load a skill by name.
219248
pub async fn load(&self, name: &str) -> Result<LoadedSkill> {
220-
// Search order: local SKILL.md, project skills, personal skills
249+
// Search order: built-in, local SKILL.md, project skills, personal skills
250+
251+
// 0. Check built-in skills first
252+
if let Some(skill) = self.load_builtin(name)? {
253+
return Ok(skill);
254+
}
221255

222256
// 1. Check for SKILL.md in current directory
223257
let local_skill = self.cwd.join("SKILL.md");
@@ -256,7 +290,7 @@ impl SkillLoader {
256290
.collect();
257291

258292
Err(CortexError::NotFound(format!(
259-
"Skill not found: {}. Searched in:\n - {}/SKILL.md (local)\n - {} (project)\n - {} (personal)",
293+
"Skill not found: {}. Searched in:\n - built-in skills\n - {}/SKILL.md (local)\n - {} (project)\n - {} (personal)",
260294
name,
261295
self.cwd.display(),
262296
project_paths.join(", "),
@@ -325,15 +359,28 @@ impl SkillLoader {
325359

326360
/// List all available skills.
327361
pub async fn list(&self) -> Result<Vec<LoadedSkill>> {
362+
use cortex_prompt_harness::prompts::builtin_skills::BUILTIN_SKILL_NAMES;
363+
328364
let mut skills = Vec::new();
329365
let mut seen_names = std::collections::HashSet::new();
330366

367+
// Load built-in skills first
368+
for name in BUILTIN_SKILL_NAMES {
369+
if let Ok(Some(skill)) = self.load_builtin(name) {
370+
seen_names.insert(skill.definition.name.clone());
371+
skills.push(skill);
372+
}
373+
}
374+
331375
// Check local SKILL.md
332376
let local_skill = self.cwd.join("SKILL.md");
333377
if local_skill.exists() {
334378
if let Ok(skill) = self.load_skill_file(&local_skill, SkillSource::Local).await {
335-
seen_names.insert(skill.definition.name.clone());
336-
skills.push(skill);
379+
// Skip if a built-in has the same name
380+
if !seen_names.contains(&skill.definition.name) {
381+
seen_names.insert(skill.definition.name.clone());
382+
skills.push(skill);
383+
}
337384
}
338385
}
339386

@@ -542,7 +589,8 @@ impl SkillHandler {
542589

543590
if skills.is_empty() {
544591
return Ok(ToolResult::success(
545-
"No skills found. Create skills in:\n \
592+
"No skills found. Built-in skills should always be available.\n\
593+
You can also create custom skills in:\n \
546594
- ./SKILL.md (local)\n \
547595
- .agents/<skill-name>/SKILL.md (project, agent.md format)\n \
548596
- .agent/<skill-name>/SKILL.md (project, agent.md format)\n \
@@ -835,4 +883,147 @@ Analyze {{target}} with verbose={{verbose}}.
835883
assert!(unrestricted.allows_tool("Execute"));
836884
assert!(unrestricted.allows_tool("Write"));
837885
}
886+
887+
#[test]
888+
fn test_skill_source_display() {
889+
assert_eq!(format!("{}", SkillSource::Personal), "personal");
890+
assert_eq!(format!("{}", SkillSource::Project), "project");
891+
assert_eq!(format!("{}", SkillSource::Local), "local");
892+
assert_eq!(format!("{}", SkillSource::Builtin), "builtin");
893+
}
894+
895+
#[test]
896+
fn test_load_builtin_git_skill() {
897+
let loader = SkillLoader::new(PathBuf::from("/tmp"));
898+
let skill = loader.load_builtin("git").unwrap();
899+
900+
assert!(skill.is_some());
901+
let skill = skill.unwrap();
902+
assert_eq!(skill.definition.name, "git");
903+
assert_eq!(skill.source, SkillSource::Builtin);
904+
assert_eq!(skill.path, PathBuf::from("<builtin>"));
905+
assert!(skill.content.contains("Git Operations Skill"));
906+
}
907+
908+
#[test]
909+
fn test_load_builtin_code_quality_skill() {
910+
let loader = SkillLoader::new(PathBuf::from("/tmp"));
911+
let skill = loader.load_builtin("code-quality").unwrap();
912+
913+
assert!(skill.is_some());
914+
let skill = skill.unwrap();
915+
assert_eq!(skill.definition.name, "code-quality");
916+
assert_eq!(skill.source, SkillSource::Builtin);
917+
assert!(skill.content.contains("Code Quality Skill"));
918+
}
919+
920+
#[test]
921+
fn test_load_builtin_debugging_skill() {
922+
let loader = SkillLoader::new(PathBuf::from("/tmp"));
923+
let skill = loader.load_builtin("debugging").unwrap();
924+
925+
assert!(skill.is_some());
926+
let skill = skill.unwrap();
927+
assert_eq!(skill.definition.name, "debugging");
928+
assert_eq!(skill.source, SkillSource::Builtin);
929+
assert!(skill.content.contains("Debugging Skill"));
930+
}
931+
932+
#[test]
933+
fn test_load_builtin_security_skill() {
934+
let loader = SkillLoader::new(PathBuf::from("/tmp"));
935+
let skill = loader.load_builtin("security").unwrap();
936+
937+
assert!(skill.is_some());
938+
let skill = skill.unwrap();
939+
assert_eq!(skill.definition.name, "security");
940+
assert_eq!(skill.source, SkillSource::Builtin);
941+
assert!(skill.content.contains("Security Skill"));
942+
}
943+
944+
#[test]
945+
fn test_load_builtin_planning_skill() {
946+
let loader = SkillLoader::new(PathBuf::from("/tmp"));
947+
let skill = loader.load_builtin("planning").unwrap();
948+
949+
assert!(skill.is_some());
950+
let skill = skill.unwrap();
951+
assert_eq!(skill.definition.name, "planning");
952+
assert_eq!(skill.source, SkillSource::Builtin);
953+
assert!(skill.content.contains("Planning Skill"));
954+
}
955+
956+
#[test]
957+
fn test_load_builtin_file_operations_skill() {
958+
let loader = SkillLoader::new(PathBuf::from("/tmp"));
959+
let skill = loader.load_builtin("file-operations").unwrap();
960+
961+
assert!(skill.is_some());
962+
let skill = skill.unwrap();
963+
assert_eq!(skill.definition.name, "file-operations");
964+
assert_eq!(skill.source, SkillSource::Builtin);
965+
assert!(skill.content.contains("File Operations Skill"));
966+
}
967+
968+
#[test]
969+
fn test_load_builtin_nonexistent_returns_none() {
970+
let loader = SkillLoader::new(PathBuf::from("/tmp"));
971+
let skill = loader.load_builtin("nonexistent-skill").unwrap();
972+
assert!(skill.is_none());
973+
}
974+
975+
#[test]
976+
fn test_load_builtin_case_insensitive() {
977+
let loader = SkillLoader::new(PathBuf::from("/tmp"));
978+
979+
// Test various case combinations
980+
let skill_lower = loader.load_builtin("git").unwrap();
981+
let skill_upper = loader.load_builtin("GIT").unwrap();
982+
let skill_mixed = loader.load_builtin("Git").unwrap();
983+
984+
assert!(skill_lower.is_some());
985+
assert!(skill_upper.is_some());
986+
assert!(skill_mixed.is_some());
987+
988+
// All should return the same skill
989+
assert_eq!(
990+
skill_lower.unwrap().definition.name,
991+
skill_upper.unwrap().definition.name
992+
);
993+
}
994+
995+
#[tokio::test]
996+
async fn test_skill_loader_load_builtin_first() {
997+
// The loader should check built-in skills before filesystem
998+
let loader = SkillLoader::new(PathBuf::from("/nonexistent/path"));
999+
1000+
// Should still be able to load built-in skills
1001+
let result = loader.load("git").await;
1002+
assert!(result.is_ok());
1003+
1004+
let skill = result.unwrap();
1005+
assert_eq!(skill.definition.name, "git");
1006+
assert_eq!(skill.source, SkillSource::Builtin);
1007+
}
1008+
1009+
#[tokio::test]
1010+
async fn test_skill_loader_list_includes_builtins() {
1011+
let loader = SkillLoader::new(PathBuf::from("/nonexistent/path"));
1012+
1013+
let skills = loader.list().await.unwrap();
1014+
1015+
// Should include all built-in skills
1016+
let builtin_names: Vec<&str> = skills
1017+
.iter()
1018+
.filter(|s| s.source == SkillSource::Builtin)
1019+
.map(|s| s.definition.name.as_str())
1020+
.collect();
1021+
1022+
assert!(builtin_names.contains(&"git"));
1023+
assert!(builtin_names.contains(&"code-quality"));
1024+
assert!(builtin_names.contains(&"file-operations"));
1025+
assert!(builtin_names.contains(&"debugging"));
1026+
assert!(builtin_names.contains(&"security"));
1027+
assert!(builtin_names.contains(&"planning"));
1028+
}
8381029
}

0 commit comments

Comments
 (0)