skills_test.go

  1package skills
  2
  3import (
  4	"os"
  5	"path/filepath"
  6	"testing"
  7)
  8
  9func TestParse(t *testing.T) {
 10	tests := []struct {
 11		name      string
 12		content   string
 13		wantName  string
 14		wantDesc  string
 15		wantError bool
 16	}{
 17		{
 18			name: "basic skill",
 19			content: `---
 20name: pdf-processing
 21description: Extract text and tables from PDF files.
 22---
 23
 24Instructions here.
 25`,
 26			wantName:  "pdf-processing",
 27			wantDesc:  "Extract text and tables from PDF files.",
 28			wantError: false,
 29		},
 30		{
 31			name: "with metadata",
 32			content: `---
 33name: data-analysis
 34description: Analyzes datasets and generates reports.
 35license: MIT
 36metadata:
 37  author: example-org
 38  version: "1.0"
 39---
 40
 41Body content.
 42`,
 43			wantName:  "data-analysis",
 44			wantDesc:  "Analyzes datasets and generates reports.",
 45			wantError: false,
 46		},
 47		{
 48			name:      "missing frontmatter",
 49			content:   "# Just markdown\n\nNo frontmatter here.",
 50			wantError: true,
 51		},
 52		{
 53			name: "missing name",
 54			content: `---
 55description: A skill without a name
 56---
 57`,
 58			wantError: true,
 59		},
 60		{
 61			name: "invalid name - uppercase",
 62			content: `---
 63name: PDF-Processing
 64description: A skill with uppercase name
 65---
 66`,
 67			wantError: true,
 68		},
 69		{
 70			name: "invalid name - starts with hyphen",
 71			content: `---
 72name: -pdf
 73description: A skill starting with hyphen
 74---
 75`,
 76			wantError: true,
 77		},
 78		{
 79			name: "invalid name - consecutive hyphens",
 80			content: `---
 81name: pdf--processing
 82description: A skill with consecutive hyphens
 83---
 84`,
 85			wantError: true,
 86		},
 87		{
 88			name: "quoted values",
 89			content: `---
 90name: "my-skill"
 91description: 'A skill with quoted values'
 92---
 93`,
 94			wantName:  "my-skill",
 95			wantDesc:  "A skill with quoted values",
 96			wantError: false,
 97		},
 98	}
 99
100	for _, tt := range tests {
101		t.Run(tt.name, func(t *testing.T) {
102			// Create temp file
103			tmpDir := t.TempDir()
104			path := filepath.Join(tmpDir, "SKILL.md")
105			if err := os.WriteFile(path, []byte(tt.content), 0o644); err != nil {
106				t.Fatalf("failed to write test file: %v", err)
107			}
108
109			skill, err := Parse(path)
110			if tt.wantError {
111				if err == nil {
112					t.Error("expected error, got nil")
113				}
114				return
115			}
116
117			if err != nil {
118				t.Fatalf("unexpected error: %v", err)
119			}
120
121			if skill.Name != tt.wantName {
122				t.Errorf("name = %q, want %q", skill.Name, tt.wantName)
123			}
124
125			if skill.Description != tt.wantDesc {
126				t.Errorf("description = %q, want %q", skill.Description, tt.wantDesc)
127			}
128		})
129	}
130}
131
132func TestDiscover(t *testing.T) {
133	// Create a temp directory structure
134	tmpDir := t.TempDir()
135
136	// Create a valid skill
137	skillDir := filepath.Join(tmpDir, "my-skill")
138	if err := os.MkdirAll(skillDir, 0o755); err != nil {
139		t.Fatal(err)
140	}
141	skillContent := `---
142name: my-skill
143description: A test skill for discovery.
144---
145
146Test instructions.
147`
148	if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(skillContent), 0o644); err != nil {
149		t.Fatal(err)
150	}
151
152	// Create an invalid skill (name doesn't match directory)
153	badSkillDir := filepath.Join(tmpDir, "bad-skill")
154	if err := os.MkdirAll(badSkillDir, 0o755); err != nil {
155		t.Fatal(err)
156	}
157	badSkillContent := `---
158name: different-name
159description: Name doesn't match directory.
160---
161`
162	if err := os.WriteFile(filepath.Join(badSkillDir, "SKILL.md"), []byte(badSkillContent), 0o644); err != nil {
163		t.Fatal(err)
164	}
165
166	// Create a directory without SKILL.md
167	emptyDir := filepath.Join(tmpDir, "empty-skill")
168	if err := os.MkdirAll(emptyDir, 0o755); err != nil {
169		t.Fatal(err)
170	}
171
172	skills := Discover([]string{tmpDir})
173
174	if len(skills) != 1 {
175		t.Fatalf("expected 1 skill, got %d", len(skills))
176	}
177
178	if skills[0].Name != "my-skill" {
179		t.Errorf("skill name = %q, want %q", skills[0].Name, "my-skill")
180	}
181}
182
183func TestToPromptXML(t *testing.T) {
184	skills := []Skill{
185		{
186			Name:        "pdf-processing",
187			Description: "Extract text & tables from PDF files.",
188			Path:        "/home/user/.shelley/skills/pdf-processing/SKILL.md",
189		},
190		{
191			Name:        "data-analysis",
192			Description: "Analyze datasets and generate reports.",
193			Path:        "/home/user/.shelley/skills/data-analysis/SKILL.md",
194		},
195	}
196
197	xml := ToPromptXML(skills)
198
199	// Check that it contains expected elements
200	expected := []string{
201		"<available_skills>",
202		"</available_skills>",
203		"<skill>",
204		"</skill>",
205		"<name>pdf-processing</name>",
206		"<description>Extract text &amp; tables from PDF files.</description>",
207		"<location>/home/user/.shelley/skills/pdf-processing/SKILL.md</location>",
208		"<name>data-analysis</name>",
209	}
210
211	for _, s := range expected {
212		if !contains(xml, s) {
213			t.Errorf("expected XML to contain %q", s)
214		}
215	}
216}
217
218func TestToPromptXMLEmpty(t *testing.T) {
219	xml := ToPromptXML(nil)
220	if xml != "" {
221		t.Errorf("expected empty string for nil skills, got %q", xml)
222	}
223
224	xml = ToPromptXML([]Skill{})
225	if xml != "" {
226		t.Errorf("expected empty string for empty skills, got %q", xml)
227	}
228}
229
230func TestValidateName(t *testing.T) {
231	validNames := []string{
232		"a",
233		"pdf-processing",
234		"data-analysis",
235		"code-review",
236		"my-skill-123",
237		"skill",
238	}
239
240	for _, name := range validNames {
241		if err := validateName(name); err != nil {
242			t.Errorf("validateName(%q) returned error: %v", name, err)
243		}
244	}
245
246	invalidNames := []string{
247		"",
248		"PDF-Processing",
249		"-pdf",
250		"pdf-",
251		"pdf--processing",
252		"pdf processing",
253		"pdf_processing",
254		"pdf.processing",
255	}
256
257	for _, name := range invalidNames {
258		if err := validateName(name); err == nil {
259			t.Errorf("validateName(%q) should return error", name)
260		}
261	}
262}
263
264func contains(s, substr string) bool {
265	return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsHelper(s, substr))
266}
267
268func containsHelper(s, substr string) bool {
269	for i := 0; i <= len(s)-len(substr); i++ {
270		if s[i:i+len(substr)] == substr {
271			return true
272		}
273	}
274	return false
275}
276
277func TestDiscoverInTree(t *testing.T) {
278	// Create a directory structure:
279	// tmpDir/
280	//   skill-root/
281	//     SKILL.md
282	//   subdir/
283	//     nested/
284	//       skill-nested/
285	//         SKILL.md
286	//   .hidden/
287	//     skill-hidden/
288	//       SKILL.md  (should be skipped)
289	//   node_modules/
290	//     skill-nm/
291	//       SKILL.md  (should be skipped)
292
293	tmpDir := t.TempDir()
294
295	// Create root-level skill
296	rootSkillDir := filepath.Join(tmpDir, "skill-root")
297	if err := os.MkdirAll(rootSkillDir, 0o755); err != nil {
298		t.Fatal(err)
299	}
300	if err := os.WriteFile(filepath.Join(rootSkillDir, "SKILL.md"), []byte("---\nname: skill-root\ndescription: Root skill\n---\n"), 0o644); err != nil {
301		t.Fatal(err)
302	}
303
304	// Create nested skill
305	nestedSkillDir := filepath.Join(tmpDir, "subdir", "nested", "skill-nested")
306	if err := os.MkdirAll(nestedSkillDir, 0o755); err != nil {
307		t.Fatal(err)
308	}
309	if err := os.WriteFile(filepath.Join(nestedSkillDir, "SKILL.md"), []byte("---\nname: skill-nested\ndescription: Nested skill\n---\n"), 0o644); err != nil {
310		t.Fatal(err)
311	}
312
313	// Create hidden directory skill (should be skipped)
314	hiddenSkillDir := filepath.Join(tmpDir, ".hidden", "skill-hidden")
315	if err := os.MkdirAll(hiddenSkillDir, 0o755); err != nil {
316		t.Fatal(err)
317	}
318	if err := os.WriteFile(filepath.Join(hiddenSkillDir, "SKILL.md"), []byte("---\nname: skill-hidden\ndescription: Hidden skill\n---\n"), 0o644); err != nil {
319		t.Fatal(err)
320	}
321
322	// Create node_modules skill (should be skipped)
323	nmSkillDir := filepath.Join(tmpDir, "node_modules", "skill-nm")
324	if err := os.MkdirAll(nmSkillDir, 0o755); err != nil {
325		t.Fatal(err)
326	}
327	if err := os.WriteFile(filepath.Join(nmSkillDir, "SKILL.md"), []byte("---\nname: skill-nm\ndescription: Node modules skill\n---\n"), 0o644); err != nil {
328		t.Fatal(err)
329	}
330
331	// Test with git root
332	skills := DiscoverInTree(tmpDir, tmpDir)
333
334	if len(skills) != 2 {
335		t.Fatalf("expected 2 skills, got %d: %v", len(skills), skillNames(skills))
336	}
337
338	// Check we found the right skills
339	names := make(map[string]bool)
340	for _, s := range skills {
341		names[s.Name] = true
342	}
343
344	if !names["skill-root"] {
345		t.Error("expected to find skill-root")
346	}
347	if !names["skill-nested"] {
348		t.Error("expected to find skill-nested")
349	}
350	if names["skill-hidden"] {
351		t.Error("should not find skill-hidden (in hidden directory)")
352	}
353	if names["skill-nm"] {
354		t.Error("should not find skill-nm (in node_modules)")
355	}
356}
357
358func TestDiscoverInTreeNoGitRoot(t *testing.T) {
359	// When no git root, should search from working dir
360	tmpDir := t.TempDir()
361
362	// Create a skill
363	skillDir := filepath.Join(tmpDir, "my-skill")
364	if err := os.MkdirAll(skillDir, 0o755); err != nil {
365		t.Fatal(err)
366	}
367	if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("---\nname: my-skill\ndescription: Test skill\n---\n"), 0o644); err != nil {
368		t.Fatal(err)
369	}
370
371	// Test with empty git root
372	skills := DiscoverInTree(tmpDir, "")
373
374	if len(skills) != 1 {
375		t.Fatalf("expected 1 skill, got %d", len(skills))
376	}
377	if skills[0].Name != "my-skill" {
378		t.Errorf("expected my-skill, got %s", skills[0].Name)
379	}
380}
381
382func skillNames(skills []Skill) []string {
383	names := make([]string, len(skills))
384	for i, s := range skills {
385		names[i] = s.Name
386	}
387	return names
388}
389
390func TestProjectSkillsDirs(t *testing.T) {
391	// Create a directory structure:
392	// tmpDir/
393	//   .skills/
394	//     skill-a/
395	//       SKILL.md
396	//   subdir/
397	//     .skills/
398	//       skill-b/
399	//         SKILL.md
400	//     deeper/
401	//       (working dir)
402
403	tmpDir := t.TempDir()
404
405	// Create root .skills
406	rootSkillsDir := filepath.Join(tmpDir, ".skills", "skill-a")
407	if err := os.MkdirAll(rootSkillsDir, 0o755); err != nil {
408		t.Fatal(err)
409	}
410	if err := os.WriteFile(filepath.Join(rootSkillsDir, "SKILL.md"), []byte("---\nname: skill-a\ndescription: Skill A\n---\n"), 0o644); err != nil {
411		t.Fatal(err)
412	}
413
414	// Create subdir .skills
415	subSkillsDir := filepath.Join(tmpDir, "subdir", ".skills", "skill-b")
416	if err := os.MkdirAll(subSkillsDir, 0o755); err != nil {
417		t.Fatal(err)
418	}
419	if err := os.WriteFile(filepath.Join(subSkillsDir, "SKILL.md"), []byte("---\nname: skill-b\ndescription: Skill B\n---\n"), 0o644); err != nil {
420		t.Fatal(err)
421	}
422
423	// Create deeper working directory
424	workingDir := filepath.Join(tmpDir, "subdir", "deeper")
425	if err := os.MkdirAll(workingDir, 0o755); err != nil {
426		t.Fatal(err)
427	}
428
429	// Test walking up from working dir to git root (tmpDir)
430	dirs := ProjectSkillsDirs(workingDir, tmpDir)
431
432	// Should find both .skills directories
433	if len(dirs) != 2 {
434		t.Fatalf("expected 2 .skills dirs, got %d: %v", len(dirs), dirs)
435	}
436
437	// subdir/.skills should come first (closer to working dir)
438	expectedFirst := filepath.Join(tmpDir, "subdir", ".skills")
439	expectedSecond := filepath.Join(tmpDir, ".skills")
440
441	if dirs[0] != expectedFirst {
442		t.Errorf("first dir = %q, want %q", dirs[0], expectedFirst)
443	}
444	if dirs[1] != expectedSecond {
445		t.Errorf("second dir = %q, want %q", dirs[1], expectedSecond)
446	}
447}
448
449func TestDefaultDirsReturnsExistingCandidates(t *testing.T) {
450	// Create a fake home directory with skill directories
451	tmpHome := t.TempDir()
452
453	// Create all three candidate directories
454	configShelley := filepath.Join(tmpHome, ".config", "shelley")
455	configAgents := filepath.Join(tmpHome, ".config", "agents", "skills")
456	dotShelley := filepath.Join(tmpHome, ".shelley")
457
458	for _, dir := range []string{configShelley, configAgents, dotShelley} {
459		if err := os.MkdirAll(dir, 0o755); err != nil {
460			t.Fatal(err)
461		}
462	}
463
464	// Override HOME
465	oldHome := os.Getenv("HOME")
466	os.Setenv("HOME", tmpHome)
467	t.Cleanup(func() { os.Setenv("HOME", oldHome) })
468
469	dirs := DefaultDirs()
470
471	if len(dirs) != 3 {
472		t.Fatalf("expected 3 dirs, got %d: %v", len(dirs), dirs)
473	}
474
475	// Verify all three candidates are returned
476	want := map[string]bool{
477		configShelley: true,
478		configAgents:  true,
479		dotShelley:    true,
480	}
481	for _, d := range dirs {
482		if !want[d] {
483			t.Errorf("unexpected dir in result: %s", d)
484		}
485	}
486}
487
488func TestDefaultDirsSkipsMissingDirs(t *testing.T) {
489	tmpHome := t.TempDir()
490
491	// Only create one of the candidate directories
492	configAgents := filepath.Join(tmpHome, ".config", "agents", "skills")
493	if err := os.MkdirAll(configAgents, 0o755); err != nil {
494		t.Fatal(err)
495	}
496
497	oldHome := os.Getenv("HOME")
498	os.Setenv("HOME", tmpHome)
499	t.Cleanup(func() { os.Setenv("HOME", oldHome) })
500
501	dirs := DefaultDirs()
502
503	if len(dirs) != 1 {
504		t.Fatalf("expected 1 dir, got %d: %v", len(dirs), dirs)
505	}
506	if dirs[0] != configAgents {
507		t.Errorf("expected %s, got %s", configAgents, dirs[0])
508	}
509}
510
511func TestSkillsFoundRegardlessOfWorkingDir(t *testing.T) {
512	// This is a regression test for:
513	// https://github.com/boldsoftware/shelley/issues/83
514	//
515	// Skills from ~/.config/agents/skills should be discovered
516	// regardless of the current working directory.
517
518	tmpHome := t.TempDir()
519
520	// Create a skill in ~/.config/agents/skills/
521	skillDir := filepath.Join(tmpHome, ".config", "agents", "skills", "my-skill")
522	if err := os.MkdirAll(skillDir, 0o755); err != nil {
523		t.Fatal(err)
524	}
525	if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("---\nname: my-skill\ndescription: A test skill.\n---\nContent\n"), 0o644); err != nil {
526		t.Fatal(err)
527	}
528
529	oldHome := os.Getenv("HOME")
530	os.Setenv("HOME", tmpHome)
531	t.Cleanup(func() { os.Setenv("HOME", oldHome) })
532
533	// Create a project directory far from home
534	projectDir := t.TempDir()
535
536	// Simulate what collectSkills does:
537	// DefaultDirs + Discover should find the skill regardless of project dir
538	dirs := DefaultDirs()
539	found := Discover(dirs)
540
541	if len(found) != 1 {
542		t.Fatalf("expected 1 skill, got %d (dirs=%v)", len(found), dirs)
543	}
544	if found[0].Name != "my-skill" {
545		t.Errorf("expected my-skill, got %s", found[0].Name)
546	}
547
548	// DiscoverInTree from the project dir should NOT find user-level skills
549	// (they're in hidden directories which are skipped)
550	treeSkills := DiscoverInTree(projectDir, projectDir)
551	if len(treeSkills) != 0 {
552		t.Errorf("expected 0 tree skills from unrelated project, got %d", len(treeSkills))
553	}
554
555	// But the combined result should still have the skill
556	all := append(found, treeSkills...)
557	if len(all) != 1 {
558		t.Fatalf("expected 1 total skill, got %d", len(all))
559	}
560
561	_ = projectDir // used above
562}
563
564func TestDiscoverFollowsSymlinks(t *testing.T) {
565	tmpDir := t.TempDir()
566
567	// Create a real skill directory (the symlink target)
568	realSkillDir := filepath.Join(tmpDir, "real-skills", "my-skill")
569	if err := os.MkdirAll(realSkillDir, 0o755); err != nil {
570		t.Fatal(err)
571	}
572	skillContent := `---
573name: my-skill
574description: A symlinked skill.
575---
576
577Test instructions.
578`
579	if err := os.WriteFile(filepath.Join(realSkillDir, "SKILL.md"), []byte(skillContent), 0o644); err != nil {
580		t.Fatal(err)
581	}
582
583	// Directory containing only a symlink to the skill
584	symlinkParent := filepath.Join(tmpDir, "symlinked-skills")
585	if err := os.MkdirAll(symlinkParent, 0o755); err != nil {
586		t.Fatal(err)
587	}
588	if err := os.Symlink(realSkillDir, filepath.Join(symlinkParent, "my-skill")); err != nil {
589		t.Fatal(err)
590	}
591
592	// A broken symlink should be silently skipped
593	if err := os.Symlink(filepath.Join(tmpDir, "nonexistent"), filepath.Join(symlinkParent, "broken-skill")); err != nil {
594		t.Fatal(err)
595	}
596
597	skills := Discover([]string{symlinkParent})
598
599	if len(skills) != 1 {
600		t.Fatalf("expected 1 skill via symlink, got %d", len(skills))
601	}
602	if skills[0].Name != "my-skill" {
603		t.Errorf("skill name = %q, want %q", skills[0].Name, "my-skill")
604	}
605}