diff --git a/server/system_prompt.go b/server/system_prompt.go index c9f3fffb7c54e983bb60fb3e10b72dcc066b2bdc..3113f91c65a5d07bac90039d616d527c448dafdf 100644 --- a/server/system_prompt.go +++ b/server/system_prompt.go @@ -298,12 +298,16 @@ func isExeDev() bool { return err == nil } -// collectSkills discovers skills from default directories and project tree. +// collectSkills discovers skills from default directories, project .skills dirs, +// and the project tree. func collectSkills(workingDir, gitRoot string) string { // Start with default directories (user-level skills) dirs := skills.DefaultDirs() - // Discover user-level skills from configured directories + // Add .skills directories found in the project tree + dirs = append(dirs, skills.ProjectSkillsDirs(workingDir, gitRoot)...) + + // Discover skills from all directories foundSkills := skills.Discover(dirs) // Also discover skills anywhere in the project tree diff --git a/server/system_prompt_test.go b/server/system_prompt_test.go index 3c60d3065f3b660cd931539c5470018a326e5c4e..62d8a568db795ef43f1e16104f962338fbdc9414 100644 --- a/server/system_prompt_test.go +++ b/server/system_prompt_test.go @@ -112,3 +112,37 @@ func min(a, b int) int { } return b } + +// TestSystemPromptIncludesSkillsFromAnyWorkingDir verifies that user-level +// skills (e.g. from ~/.config/agents/skills) appear in the system prompt +// regardless of the conversation's working directory. +// Regression test for https://github.com/boldsoftware/shelley/issues/83 +func TestSystemPromptIncludesSkillsFromAnyWorkingDir(t *testing.T) { + // Create a fake home with a skill + tmpHome := t.TempDir() + skillDir := filepath.Join(tmpHome, ".config", "agents", "skills", "test-skill") + if err := os.MkdirAll(skillDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("---\nname: test-skill\ndescription: A test skill for issue 83.\n---\nInstructions.\n"), 0o644); err != nil { + t.Fatal(err) + } + + oldHome := os.Getenv("HOME") + os.Setenv("HOME", tmpHome) + t.Cleanup(func() { os.Setenv("HOME", oldHome) }) + + // Generate system prompt from a directory completely unrelated to home + unrelatedDir := t.TempDir() + prompt, err := GenerateSystemPrompt(unrelatedDir) + if err != nil { + t.Fatalf("GenerateSystemPrompt failed: %v", err) + } + + if !strings.Contains(prompt, "test-skill") { + t.Error("system prompt should contain skill 'test-skill' even when working dir is unrelated to home") + } + if !strings.Contains(prompt, "A test skill for issue 83.") { + t.Error("system prompt should contain the skill description") + } +} diff --git a/skills/skills.go b/skills/skills.go index 8b4694c4798ed3b761bd35554a4c41cfc1175875..6fb922d31ee85bc857c3f15944aa406f549c88e3 100644 --- a/skills/skills.go +++ b/skills/skills.go @@ -303,8 +303,9 @@ func ToPromptXML(skills []Skill) string { } // DefaultDirs returns the default skill directories to search. +// These are always returned if they exist, regardless of the current working directory. func DefaultDirs() []string { - dirs := []string{} + var dirs []string home, err := os.UserHomeDir() if err != nil { @@ -321,48 +322,15 @@ func DefaultDirs() []string { filepath.Join(home, ".shelley"), } - for _, candidateDir := range candidateDirs { - entries, err := os.ReadDir(candidateDir) - if err != nil { - continue - } - for _, entry := range entries { - if entry.IsDir() { - subdir := filepath.Join(candidateDir, entry.Name()) - // Check if this directory contains skills (has subdirs with SKILL.md) - // or is itself a skill directory - if findSkillMD(subdir) != "" { - // This is a skill directory itself, add parent - dirs = append(dirs, candidateDir) - break - } - // Otherwise check if it's a container of skills - if hasSkillSubdirs(subdir) { - dirs = append(dirs, subdir) - } - } + for _, dir := range candidateDirs { + if info, err := os.Stat(dir); err == nil && info.IsDir() { + dirs = append(dirs, dir) } } return dirs } -// hasSkillSubdirs checks if a directory contains any skill subdirectories. -func hasSkillSubdirs(dir string) bool { - entries, err := os.ReadDir(dir) - if err != nil { - return false - } - for _, entry := range entries { - if entry.IsDir() { - if findSkillMD(filepath.Join(dir, entry.Name())) != "" { - return true - } - } - } - return false -} - // expandPath expands ~ to the user's home directory. func expandPath(path string) string { if strings.HasPrefix(path, "~/") { diff --git a/skills/skills_test.go b/skills/skills_test.go index 306d817b64263d2ff59295721a607a3eb07abdfc..bcc65e7986e21b936e6b0569fbd5b4b60b78f85f 100644 --- a/skills/skills_test.go +++ b/skills/skills_test.go @@ -445,3 +445,118 @@ func TestProjectSkillsDirs(t *testing.T) { t.Errorf("second dir = %q, want %q", dirs[1], expectedSecond) } } + +func TestDefaultDirsReturnsExistingCandidates(t *testing.T) { + // Create a fake home directory with skill directories + tmpHome := t.TempDir() + + // Create all three candidate directories + configShelley := filepath.Join(tmpHome, ".config", "shelley") + configAgents := filepath.Join(tmpHome, ".config", "agents", "skills") + dotShelley := filepath.Join(tmpHome, ".shelley") + + for _, dir := range []string{configShelley, configAgents, dotShelley} { + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + } + + // Override HOME + oldHome := os.Getenv("HOME") + os.Setenv("HOME", tmpHome) + t.Cleanup(func() { os.Setenv("HOME", oldHome) }) + + dirs := DefaultDirs() + + if len(dirs) != 3 { + t.Fatalf("expected 3 dirs, got %d: %v", len(dirs), dirs) + } + + // Verify all three candidates are returned + want := map[string]bool{ + configShelley: true, + configAgents: true, + dotShelley: true, + } + for _, d := range dirs { + if !want[d] { + t.Errorf("unexpected dir in result: %s", d) + } + } +} + +func TestDefaultDirsSkipsMissingDirs(t *testing.T) { + tmpHome := t.TempDir() + + // Only create one of the candidate directories + configAgents := filepath.Join(tmpHome, ".config", "agents", "skills") + if err := os.MkdirAll(configAgents, 0o755); err != nil { + t.Fatal(err) + } + + oldHome := os.Getenv("HOME") + os.Setenv("HOME", tmpHome) + t.Cleanup(func() { os.Setenv("HOME", oldHome) }) + + dirs := DefaultDirs() + + if len(dirs) != 1 { + t.Fatalf("expected 1 dir, got %d: %v", len(dirs), dirs) + } + if dirs[0] != configAgents { + t.Errorf("expected %s, got %s", configAgents, dirs[0]) + } +} + +func TestSkillsFoundRegardlessOfWorkingDir(t *testing.T) { + // This is a regression test for: + // https://github.com/boldsoftware/shelley/issues/83 + // + // Skills from ~/.config/agents/skills should be discovered + // regardless of the current working directory. + + tmpHome := t.TempDir() + + // Create a skill in ~/.config/agents/skills/ + skillDir := filepath.Join(tmpHome, ".config", "agents", "skills", "my-skill") + if err := os.MkdirAll(skillDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("---\nname: my-skill\ndescription: A test skill.\n---\nContent\n"), 0o644); err != nil { + t.Fatal(err) + } + + oldHome := os.Getenv("HOME") + os.Setenv("HOME", tmpHome) + t.Cleanup(func() { os.Setenv("HOME", oldHome) }) + + // Create a project directory far from home + projectDir := t.TempDir() + + // Simulate what collectSkills does: + // DefaultDirs + Discover should find the skill regardless of project dir + dirs := DefaultDirs() + found := Discover(dirs) + + if len(found) != 1 { + t.Fatalf("expected 1 skill, got %d (dirs=%v)", len(found), dirs) + } + if found[0].Name != "my-skill" { + t.Errorf("expected my-skill, got %s", found[0].Name) + } + + // DiscoverInTree from the project dir should NOT find user-level skills + // (they're in hidden directories which are skipped) + treeSkills := DiscoverInTree(projectDir, projectDir) + if len(treeSkills) != 0 { + t.Errorf("expected 0 tree skills from unrelated project, got %d", len(treeSkills)) + } + + // But the combined result should still have the skill + all := append(found, treeSkills...) + if len(all) != 1 { + t.Fatalf("expected 1 total skill, got %d", len(all)) + } + + _ = projectDir // used above +}