shelley: fix skills not loaded from non-home working directories

Philip Zeyliger and Shelley created

Prompt: Recent boldsoftware/shelley gh issue talks about skills not being read depending on working dir. Fix that.

DefaultDirs() had overly complex pre-checking logic that scanned
candidate directories for skill subdirectories before returning them.
This was unnecessary since Discover() already handles scanning, and
the complexity could cause edge cases where valid skill directories
were missed.

Simplified DefaultDirs() to always return candidate directories that
exist (~/.config/shelley, ~/.config/agents/skills, ~/.shelley),
letting Discover() do the actual skill scanning.

Also wired up ProjectSkillsDirs() in collectSkills() so that .skills
directories in the project tree are now discovered (they were defined
but never called).

Fixes https://github.com/boldsoftware/shelley/issues/83

Co-authored-by: Shelley <shelley@exe.dev>

Change summary

server/system_prompt.go      |   8 +
server/system_prompt_test.go |  34 +++++++++++
skills/skills.go             |  42 +------------
skills/skills_test.go        | 115 ++++++++++++++++++++++++++++++++++++++
4 files changed, 160 insertions(+), 39 deletions(-)

Detailed changes

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

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")
+	}
+}

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, "~/") {

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
+}