| 💾 Saved. d722711 k33g 7h ago | 1 | // Tests for the skills package. What matters here: the catalogue is a TOOL |
| 2 | // DESCRIPTION, so what the model knows about the available skills is exactly |
| 3 | // what these functions produce. One header read wrong, and the model can no |
| 4 | // longer name the skill it wants. |
| 5 | package skills |
| 6 | |
| 7 | import ( |
| 8 | "os" |
| 9 | "path/filepath" |
| 10 | "strings" |
| 11 | "testing" |
| 12 | ) |
| 13 | |
| 14 | // write creates a skill file in a temporary directory. |
| 15 | func write(t *testing.T, dir, name, content string) { |
| 16 | t.Helper() |
| 17 | if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil { |
| 18 | t.Fatal(err) |
| 19 | } |
| 20 | } |
| 21 | |
| 22 | func TestParseHeader(t *testing.T) { |
| 23 | cases := []struct { |
| 24 | name string |
| 25 | content string |
| 26 | wantName string |
| 27 | wantDesc string |
| 28 | }{{ |
| 29 | name: "complete header", |
| 30 | content: "---\nname: go-rename\ndescription: rename a symbol\n---\n# Title\n", |
| 31 | wantName: "go-rename", |
| 32 | wantDesc: "rename a symbol", |
| 33 | }, { |
| 34 | name: "no header: the name comes from the file", |
| 35 | content: "# Just some markdown\n", |
| 36 | wantName: "file", |
| 37 | wantDesc: "", |
| 38 | }, { |
| 39 | name: "empty name: the file name is kept", |
| 40 | content: "---\nname:\ndescription: something\n---\n", |
| 41 | wantName: "file", |
| 42 | wantDesc: "something", |
| 43 | }, { |
| 44 | // The case parseHeader's comment announces: a `description:` in the |
| 45 | // BODY is not metadata. |
| 46 | name: "description in the body: ignored", |
| 47 | content: "---\nname: go-test\n---\ndescription: this is just text\n", |
| 48 | wantName: "go-test", |
| 49 | wantDesc: "", |
| 50 | }, { |
| 51 | name: "spaces around the values", |
| 52 | content: "---\n name : go-fmt \n description : format code \n---\n", |
| 53 | wantName: "go-fmt", |
| 54 | wantDesc: "format code", |
| 55 | }, { |
| 56 | name: "a colon inside the description", |
| 57 | content: "---\nname: go-run\ndescription: run: execute a program\n---\n", |
| 58 | wantName: "go-run", |
| 59 | wantDesc: "run: execute a program", |
| 60 | }} |
| 61 | |
| 62 | for _, c := range cases { |
| 63 | t.Run(c.name, func(t *testing.T) { |
| 64 | s := parseHeader(c.content, "/tmp/file.md") |
| 65 | if s.Name != c.wantName { |
| 66 | t.Errorf("Name = %q, want %q", s.Name, c.wantName) |
| 67 | } |
| 68 | if s.Description != c.wantDesc { |
| 69 | t.Errorf("Description = %q, want %q", s.Description, c.wantDesc) |
| 70 | } |
| 71 | }) |
| 72 | } |
| 73 | } |
| 74 | |
| 75 | func TestListSortsAndReadsHeaders(t *testing.T) { |
| 76 | dir := t.TempDir() |
| 77 | write(t, dir, "zeta.md", "---\nname: zeta\ndescription: the last one\n---\n") |
| 78 | write(t, dir, "alpha.md", "---\nname: alpha\ndescription: the first one\n---\n") |
| 79 | write(t, dir, "notes.txt", "not a skill") // ignored: not *.md |
| 80 | |
| 81 | list := List(dir) |
| 82 | if len(list) != 2 { |
| 83 | t.Fatalf("%d skills, want 2: %+v", len(list), list) |
| 84 | } |
| 85 | if list[0].Name != "alpha" || list[1].Name != "zeta" { |
| 86 | t.Errorf("expected a sort by name, got %q then %q", list[0].Name, list[1].Name) |
| 87 | } |
| 88 | } |
| 89 | |
| 90 | // A missing directory is not an error: this agent simply has no skills, and |
| 91 | // main.go will not declare the tool. |
| 92 | func TestListMissingDirIsEmpty(t *testing.T) { |
| 93 | if list := List(filepath.Join(t.TempDir(), "doesnotexist")); len(list) != 0 { |
| 94 | t.Errorf("%d skills for a missing directory, want 0", len(list)) |
| 95 | } |
| 96 | } |
| 97 | |
| 98 | func TestRead(t *testing.T) { |
| 99 | dir := t.TempDir() |
| 100 | write(t, dir, "go-fmt.md", "---\nname: go-fmt\n---\n# Format\n") |
| 101 | |
| 102 | content, err := Read(dir, "go-fmt") |
| 103 | if err != nil { |
| 104 | t.Fatalf("Read: %v", err) |
| 105 | } |
| 106 | if !strings.Contains(content, "# Format") { |
| 107 | t.Errorf("unexpected content: %q", content) |
| 108 | } |
| 109 | |
| 110 | if _, err := Read(dir, "unknown"); err == nil { |
| 111 | t.Error("an unknown skill must return an error") |
| 112 | } |
| 113 | } |
| 114 | |
| 115 | // filepath.Base in Read is a barrier: the name comes from the MODEL, so it must |
| 116 | // not be able to escape the skills directory. |
| 117 | func TestReadCannotEscapeDir(t *testing.T) { |
| 118 | dir := t.TempDir() |
| 119 | parent := filepath.Dir(dir) |
| 120 | if err := os.WriteFile(filepath.Join(parent, "secret.md"), []byte("forbidden"), 0o644); err != nil { |
| 121 | t.Fatal(err) |
| 122 | } |
| 123 | |
| 124 | for _, name := range []string{"../secret", "../../secret", "/etc/passwd"} { |
| 125 | if content, err := Read(dir, name); err == nil { |
| 126 | t.Errorf("Read(%q) succeeded and returned %q — the barrier is gone", name, content) |
| 127 | } |
| 128 | } |
| 129 | } |
| 130 | |
| 131 | // The catalogue IS the tool description: every skill must appear in it with its |
| 132 | // name, otherwise the model cannot ask for it. |
| 133 | func TestCatalogue(t *testing.T) { |
| 134 | list := []Skill{ |
| 135 | {Name: "go-fmt", Description: "format code"}, |
| 136 | {Name: "go-test"}, // no description |
| 137 | } |
| 138 | cat := Catalogue(list) |
| 139 | |
| 140 | for _, want := range []string{"go-fmt", "format code", "go-test"} { |
| 141 | if !strings.Contains(cat, want) { |
| 142 | t.Errorf("the catalogue does not contain %q:\n%s", want, cat) |
| 143 | } |
| 144 | } |
| 145 | if !strings.Contains(cat, "go-fmt — format code") { |
| 146 | t.Errorf("name and description must be joined by a dash:\n%s", cat) |
| 147 | } |
| 148 | // A skill with no description must not leave an orphan dash. |
| 149 | if strings.Contains(cat, "go-test —") { |
| 150 | t.Errorf("orphan dash for a skill with no description:\n%s", cat) |
| 151 | } |
| 152 | } |
| 153 | |
| 154 | func TestNames(t *testing.T) { |
| 155 | got := Names([]Skill{{Name: "a"}, {Name: "b"}}) |
| 156 | if strings.Join(got, ",") != "a,b" { |
| 157 | t.Errorf("Names = %v", got) |
| 158 | } |
| 159 | } |
| 160 | |
| 161 | // The skills actually shipped in the repository's skills/ must all have a |
| 162 | // name AND a description: without a description, the model picks blindly. |
| 163 | // |
| 164 | // The path is the real layout — skills/ at the repository root, one directory |
| 165 | // per skill. A t.Skip() here would mean "I am testing nothing" without saying |
| 166 | // so, so a missing directory is an ERROR, not a skip. |
| 167 | func TestShippedSkillsHaveDescriptions(t *testing.T) { |
| 168 | dir := filepath.Join("..", "..", "skills") |
| 169 | list := List(dir) |
| 170 | if len(list) == 0 { |
| 171 | t.Fatalf("no skill found in %s — has the layout changed?", dir) |
| 172 | } |
| 173 | for _, s := range list { |
| 174 | if s.Description == "" { |
| 175 | t.Errorf("skill %q has no description", s.Name) |
| 176 | } |
| 177 | if strings.Contains(s.Name, " ") { |
| 178 | t.Errorf("name %q contains a space: the model will not be able to quote it", s.Name) |
| 179 | } |
| 180 | } |
| 181 | t.Logf("%d shipped skills, all described", len(list)) |
| 182 | } |
| 183 | |
| 184 | // writeNested stores a skill the Agent Skills way: <dir>/<name>/SKILL.md. |
| 185 | func writeNested(t *testing.T, dir, name, content string) { |
| 186 | t.Helper() |
| 187 | if err := os.MkdirAll(filepath.Join(dir, name), 0o755); err != nil { |
| 188 | t.Fatal(err) |
| 189 | } |
| 190 | if err := os.WriteFile(filepath.Join(dir, name, "SKILL.md"), []byte(content), 0o644); err != nil { |
| 191 | t.Fatal(err) |
| 192 | } |
| 193 | } |
| 194 | |
| 195 | // The two layouts are both found, may be mixed, and sort together. A nested |
| 196 | // skill without a name in its front matter is named after its directory, not |
| 197 | // "SKILL"; a stray markdown file inside a skill's directory is not a skill. |
| 198 | func TestListAcceptsFlatAndNestedLayouts(t *testing.T) { |
| 199 | dir := t.TempDir() |
| 200 | write(t, dir, "zeta.md", "---\nname: zeta\ndescription: flat, named\n---\n") |
| 201 | writeNested(t, dir, "alpha", "---\ndescription: nested, unnamed\n---\n# Alpha\n") |
| 202 | writeNested(t, dir, "mid", "---\nname: renamed\ndescription: nested, named\n---\n") |
| 203 | if err := os.WriteFile(filepath.Join(dir, "alpha", "notes.md"), []byte("not a skill"), 0o644); err != nil { |
| 204 | t.Fatal(err) |
| 205 | } |
| 206 | |
| 207 | list := List(dir) |
| 208 | got := make([]string, 0, len(list)) |
| 209 | for _, s := range list { |
| 210 | got = append(got, s.Name+"="+s.Description) |
| 211 | } |
| 212 | want := []string{"alpha=nested, unnamed", "renamed=nested, named", "zeta=flat, named"} |
| 213 | if strings.Join(got, "|") != strings.Join(want, "|") { |
| 214 | t.Errorf("List:\n got %v\nwant %v", got, want) |
| 215 | } |
| 216 | } |
| 217 | |
| 218 | // Read finds a skill whichever layout stores it, prefers the flat file when |
| 219 | // both exist (it is the more explicit of the two), and still cannot be walked |
| 220 | // out of the directory with a nested name: "../escape" is reduced to "escape", |
| 221 | // which does not exist inside dir. |
| 222 | func TestReadAcceptsFlatAndNestedLayouts(t *testing.T) { |
| 223 | parent := t.TempDir() |
| 224 | dir := filepath.Join(parent, "skills") |
| 225 | writeNested(t, parent, "escape", "# outside") |
| 226 | writeNested(t, dir, "nested-only", "# nested") |
| 227 | write(t, dir, "both.md", "# flat wins") |
| 228 | writeNested(t, dir, "both", "# nested loses") |
| 229 | |
| 230 | if got, err := Read(dir, "nested-only"); err != nil || got != "# nested" { |
| 231 | t.Errorf("Read(nested-only) = %q, %v", got, err) |
| 232 | } |
| 233 | if got, err := Read(dir, "both"); err != nil || got != "# flat wins" { |
| 234 | t.Errorf("Read(both) = %q, %v; want the flat file", got, err) |
| 235 | } |
| 236 | if _, err := Read(dir, "missing"); err == nil { |
| 237 | t.Error("Read(missing) returned no error") |
| 238 | } |
| 239 | if _, err := Read(dir, "../escape"); err == nil { |
| 240 | t.Error("Read(../escape) escaped the directory") |
| 241 | } |
| 242 | } |