From 1575b199165d4c51935fd9c413aa0706657d18a3 Mon Sep 17 00:00:00 2001 From: Sai Karthik Date: Mon, 9 Nov 2026 15:53:06 +0530 Subject: [PATCH] fix(skills): follow symlinks when enumerating skill resources Skill.Resources() previously checked e.IsDir() on os.DirEntry when walking through scripts/, references/, and assets/. Because Go's DirEntry.IsDir() returns false for symlinks, any symlink pointing to a subdirectory was incorrectly treated as a resource file rather than being skipped. Inspect the target via os.Stat when an entry is a symlink to verify whether it points to a directory before adding it to resources. Skip unresolvable or broken symlinks, and enforce maxResources immediately after appending to avoid unnecessary traversal. Add TestSkillResources_Symlinks to verify symlinked files are included and symlinked directories are skipped. --- internal/skills/skills.go | 11 +++++++++- internal/skills/skills_test.go | 37 ++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/internal/skills/skills.go b/internal/skills/skills.go index 3b62a675..c38d1e26 100644 --- a/internal/skills/skills.go +++ b/internal/skills/skills.go @@ -254,7 +254,16 @@ func (s *Skill) Resources() []string { continue } for _, e := range entries { - if e.IsDir() { + isDir := e.IsDir() + if !isDir && e.Type()&os.ModeSymlink != 0 { + info, err := os.Stat(filepath.Join(dir, e.Name())) + if err != nil { + // Skip unresolvable symlinks (e.g. broken or inaccessible target) + continue + } + isDir = info.IsDir() + } + if isDir { continue } out = append(out, sub+"/"+e.Name()) diff --git a/internal/skills/skills_test.go b/internal/skills/skills_test.go index e81dee52..c37ef087 100644 --- a/internal/skills/skills_test.go +++ b/internal/skills/skills_test.go @@ -617,6 +617,43 @@ func TestSkillResources(t *testing.T) { } } +func TestSkillResources_Symlinks(t *testing.T) { + dir := t.TempDir() + skillDir := filepath.Join(dir, "my-skill") + if err := os.MkdirAll(filepath.Join(skillDir, "scripts"), 0o755); err != nil { + t.Fatal(err) + } + + extDir := t.TempDir() + extScript := filepath.Join(extDir, "external.sh") + if err := os.WriteFile(extScript, []byte("echo hi"), 0o755); err != nil { + t.Fatal(err) + } + extSubdir := filepath.Join(extDir, "subfolder") + if err := os.MkdirAll(extSubdir, 0o755); err != nil { + t.Fatal(err) + } + + // Symlink a file into scripts + if err := os.Symlink(extScript, filepath.Join(skillDir, "scripts", "linked.sh")); err != nil { + t.Skipf("symlinks unsupported: %v", err) + } + // Symlink a directory into scripts (should be skipped, not treated as a file) + if err := os.Symlink(extSubdir, filepath.Join(skillDir, "scripts", "linked-dir")); err != nil { + t.Skipf("symlinks unsupported: %v", err) + } + // Broken/dangling symlink into scripts (should be skipped) + if err := os.Symlink(filepath.Join(extDir, "nonexistent.sh"), filepath.Join(skillDir, "scripts", "dangling.sh")); err != nil { + t.Skipf("symlinks unsupported: %v", err) + } + + s := &Skill{Name: "my-skill", Path: filepath.Join(skillDir, "SKILL.md")} + res := s.Resources() + if len(res) != 1 || res[0] != "scripts/linked.sh" { + t.Fatalf("expected [scripts/linked.sh], got %v", res) + } +} + // TestValidate_SpecWarnings verifies the lenient spec checks: deviations in // name format, length limits, directory mismatch and body length are // reported as warnings (the skill still loads), never as errors.