os.DirEntry.IsDir() does not follow symlinks, so scan() silently skipped a skill directory reached via a symlink (e.g. from a dotfiles repo). The existing os.Stat on <dir>/SKILL.md already follows symlinks, so it now doubles as the "is this a skill directory?" test; a symlink to a non-directory still fails that stat harmlessly. Fix round 1 of 5, addressing an Important review finding on Task 1. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Showing 2 changed files with 40 additions and 5 deletions.
| @@ -90,14 +90,15 @@ func scan(root, source string) ([]Skill, []error) { | ||
| 90 | 90 | var found []Skill |
| 91 | 91 | var problems []error |
| 92 | 92 | for _, e := range entries { |
| 93 | - if !e.IsDir() { | |
| 94 | - continue | |
| 95 | - } | |
| 96 | 93 | dir := filepath.Join(root, e.Name()) |
| 97 | 94 | path := filepath.Join(dir, filename) |
| 95 | + // os.Stat follows symlinks, so this both decides "is this a skill | |
| 96 | + // directory?" and tolerates a symlinked-in skill directory (e.g. from | |
| 97 | + // a dotfiles repo) in the same check. A non-directory entry, or a | |
| 98 | + // symlink to a non-directory, fails this stat (ENOTDIR) and is | |
| 99 | + // skipped exactly like a directory with no SKILL.md: not a skill, | |
| 100 | + // and not a complaint either, since the entry may be anything at all. | |
| 98 | 101 | if _, err := os.Stat(path); err != nil { |
| 99 | - // A directory without a SKILL.md is not a skill, and not a | |
| 100 | - // complaint either: it may be anything at all. | |
| 101 | 102 | continue |
| 102 | 103 | } |
| 103 | 104 | s, err := load(dir, e.Name(), source) |
| @@ -161,6 +161,40 @@ func TestNestedSkillIsNotDiscovered(t *testing.T) { | ||
| 161 | 161 | } |
| 162 | 162 | } |
| 163 | 163 | |
| 164 | + func TestSymlinkedSkillIsDiscovered(t *testing.T) { | |
| 165 | + home, work := t.TempDir(), t.TempDir() | |
| 166 | + elsewhere := t.TempDir() | |
| 167 | + realDir := write(t, elsewhere, "linked-in", `--- | |
| 168 | + name: linked-in | |
| 169 | + description: reached through a symlink | |
| 170 | + --- | |
| 171 | + symlinked body | |
| 172 | + `) | |
| 173 | + | |
| 174 | + root := filepath.Join(home, ".agents", "skills") | |
| 175 | + if err := os.MkdirAll(root, 0o755); err != nil { | |
| 176 | + t.Fatal(err) | |
| 177 | + } | |
| 178 | + link := filepath.Join(root, "linked-in") | |
| 179 | + if err := os.Symlink(realDir, link); err != nil { | |
| 180 | + t.Skipf("symlinks unavailable: %v", err) | |
| 181 | + } | |
| 182 | + | |
| 183 | + found, problems := Discover(home, work) | |
| 184 | + if len(problems) != 0 { | |
| 185 | + t.Fatalf("problems: %v", problems) | |
| 186 | + } | |
| 187 | + if len(found) != 1 { | |
| 188 | + t.Fatalf("got %d skills, want 1", len(found)) | |
| 189 | + } | |
| 190 | + if found[0].Name != "linked-in" { | |
| 191 | + t.Errorf("Name = %q", found[0].Name) | |
| 192 | + } | |
| 193 | + if found[0].Description != "reached through a symlink" { | |
| 194 | + t.Errorf("Description = %q", found[0].Description) | |
| 195 | + } | |
| 196 | + } | |
| 197 | + | |
| 164 | 198 | func TestSortedByName(t *testing.T) { |
| 165 | 199 | home, work := t.TempDir(), t.TempDir() |
| 166 | 200 | root := filepath.Join(home, ".agents", "skills") |