fix(skills): follow symlinks in Discover()

Amolith and Shelley created 1 month ago

os.ReadDir returns DirEntry values whose IsDir() reports the symlink's
own type, not the target's. When skill directories are symlinks (e.g. in
~/.config/agents/skills/), Discover() skipped them all.

Use os.Stat on the resolved path to follow symlinks.

References: https://github.com/boldsoftware/shelley/issues/83
Co-authored-by: Shelley <[e-mail]>

Change summary

skills/skills.go      |  7 ++++---
skills/skills_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 47 insertions(+), 3 deletions(-)

Detailed changes

skills/skills.go

@@ -41,11 +41,12 @@ func Discover(dirs []string) []Skill {
 		}
 
 		for _, entry := range entries {
-			if !entry.IsDir() {
+			skillDir := filepath.Join(dir, entry.Name())
+			// entry.IsDir() returns false for symlinks, even to directories;
+			// os.Stat follows symlinks so we can detect symlinked skill dirs
+			if info, err := os.Stat(skillDir); err != nil || !info.IsDir() {
 				continue
 			}
-
-			skillDir := filepath.Join(dir, entry.Name())
 			skillMD := findSkillMD(skillDir)
 			if skillMD == "" {
 				continue

skills/skills_test.go

@@ -560,3 +560,46 @@ func TestSkillsFoundRegardlessOfWorkingDir(t *testing.T) {
 
 	_ = projectDir // used above
 }
+
+func TestDiscoverFollowsSymlinks(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	// Create a real skill directory (the symlink target)
+	realSkillDir := filepath.Join(tmpDir, "real-skills", "my-skill")
+	if err := os.MkdirAll(realSkillDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	skillContent := `---
+name: my-skill
+description: A symlinked skill.
+---
+
+Test instructions.
+`
+	if err := os.WriteFile(filepath.Join(realSkillDir, "SKILL.md"), []byte(skillContent), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	// Directory containing only a symlink to the skill
+	symlinkParent := filepath.Join(tmpDir, "symlinked-skills")
+	if err := os.MkdirAll(symlinkParent, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(realSkillDir, filepath.Join(symlinkParent, "my-skill")); err != nil {
+		t.Fatal(err)
+	}
+
+	// A broken symlink should be silently skipped
+	if err := os.Symlink(filepath.Join(tmpDir, "nonexistent"), filepath.Join(symlinkParent, "broken-skill")); err != nil {
+		t.Fatal(err)
+	}
+
+	skills := Discover([]string{symlinkParent})
+
+	if len(skills) != 1 {
+		t.Fatalf("expected 1 skill via symlink, got %d", len(skills))
+	}
+	if skills[0].Name != "my-skill" {
+		t.Errorf("skill name = %q, want %q", skills[0].Name, "my-skill")
+	}
+}