git.iko.soy/crush
on main help sign in
refactor: simplify skills parsing and improve discovery visibility (#2350)
7a6a179huaiyuWangh
internal/skills/skills.go+39−9
compare with main
unifiedsplit
@@ -9,6 +9,8 @@ import (
9 "os"
9 "os"
10 "path/filepath"
10 "path/filepath"
11 "regexp"
11 "regexp"
12+ "slices"
13+ "sort"
12 "strings"
14 "strings"
13 "sync"
15 "sync"
14
16
@@ -23,7 +25,10 @@ const (
23 MaxCompatibilityLength = 500
25 MaxCompatibilityLength = 500
24)
26)
25
27
28+var (
26−var namePattern = regexp.MustCompile(`^[a-zA-Z0-9]+(-[a-zA-Z0-9]+)*$`)
29+ namePattern = regexp.MustCompile(`^[a-zA-Z0-9]+(-[a-zA-Z0-9]+)*$`)
30+ promptReplacer = strings.NewReplacer("&", "&amp;", "<", "&lt;", ">", "&gt;", "\"", "&quot;", "'", "&apos;")
31+)
27
32
28// Skill represents a parsed SKILL.md file.
33// Skill represents a parsed SKILL.md file.
29type Skill struct {
34type Skill struct {
@@ -106,19 +111,31 @@ func ParseContent(content []byte) (*Skill, error) {
106
111
107// splitFrontmatter extracts YAML frontmatter and body from markdown content.
112// splitFrontmatter extracts YAML frontmatter and body from markdown content.
108func splitFrontmatter(content string) (frontmatter, body string, err error) {
113func splitFrontmatter(content string) (frontmatter, body string, err error) {
114+ // Strip UTF-8 BOM for compatibility with editors that include it.
115+ content = strings.TrimPrefix(content, "\uFEFF")
109 // Normalize line endings to \n for consistent parsing.
116 // Normalize line endings to \n for consistent parsing.
110 content = strings.ReplaceAll(content, "\r\n", "\n")
117 content = strings.ReplaceAll(content, "\r\n", "\n")
111− if !strings.HasPrefix(content, "---\n") {
118+ content = strings.ReplaceAll(content, "\r", "\n")
119+
120+ lines := strings.Split(content, "\n")
121+ start := slices.IndexFunc(lines, func(line string) bool {
122+ return strings.TrimSpace(line) != ""
123+ })
124+ if start == -1 || strings.TrimSpace(lines[start]) != "---" {
112 return "", "", errors.New("no YAML frontmatter found")
125 return "", "", errors.New("no YAML frontmatter found")
113 }
126 }
114
127
115− rest := strings.TrimPrefix(content, "---\n")
128+ endOffset := slices.IndexFunc(lines[start+1:], func(line string) bool {
116− before, after, ok := strings.Cut(rest, "\n---")
129+ return strings.TrimSpace(line) == "---"
117− if !ok {
130+ })
131+ if endOffset == -1 {
118 return "", "", errors.New("unclosed frontmatter")
132 return "", "", errors.New("unclosed frontmatter")
119 }
133 }
134+ end := start + 1 + endOffset
120
135
136+ frontmatter = strings.Join(lines[start+1:end], "\n")
137+ body = strings.Join(lines[end+1:], "\n")
121− return before, after, nil
138+ return frontmatter, body, nil
122}
139}
123
140
124// Discover finds all valid skills in the given paths.
141// Discover finds all valid skills in the given paths.
@@ -136,8 +153,9 @@ func Discover(paths []string) []*Skill {
136 Follow: true,
153 Follow: true,
137 ToSlash: fastwalk.DefaultToSlash(),
154 ToSlash: fastwalk.DefaultToSlash(),
138 }
155 }
139− fastwalk.Walk(&conf, base, func(path string, d os.DirEntry, err error) error {
156+ err := fastwalk.Walk(&conf, base, func(path string, d os.DirEntry, err error) error {
140 if err != nil {
157 if err != nil {
158+ slog.Warn("Failed to walk skills path entry", "base", base, "path", path, "error", err)
141 return nil
159 return nil
142 }
160 }
143 if d.IsDir() || d.Name() != SkillFileName {
161 if d.IsDir() || d.Name() != SkillFileName {
@@ -165,8 +183,21 @@ func Discover(paths []string) []*Skill {
165 mu.Unlock()
183 mu.Unlock()
166 return nil
184 return nil
167 })
185 })
186+ if err != nil {
187+ slog.Warn("Failed to walk skills path", "path", base, "error", err)
188+ }
168 }
189 }
169
190
191+ // fastwalk traversal order is non-deterministic, so sort for stable output.
192+ sort.SliceStable(skills, func(i, j int) bool {
193+ left := strings.ToLower(skills[i].SkillFilePath)
194+ right := strings.ToLower(skills[j].SkillFilePath)
195+ if left == right {
196+ return skills[i].SkillFilePath < skills[j].SkillFilePath
197+ }
198+ return left < right
199+ })
200+
170 return skills
201 return skills
171}
202}
172
203
@@ -193,8 +224,7 @@ func ToPromptXML(skills []*Skill) string {
193}
224}
194
225
195func escape(s string) string {
226func escape(s string) string {
196− r := strings.NewReplacer("&", "&amp;", "<", "&lt;", ">", "&gt;", "\"", "&quot;", "'", "&apos;")
197− return r.Replace(s)
227+ return promptReplacer.Replace(s)
198}
228}
199
229
200// Deduplicate removes duplicate skills by name. When duplicates exist, the
230// Deduplicate removes duplicate skills by name. When duplicates exist, the
internal/skills/skills_test.go+62
@@ -63,6 +63,39 @@ Instructions here.
63 wantDesc: "A simple skill for testing.",
63 wantDesc: "A simple skill for testing.",
64 wantInstr: "# My Skill\n\nInstructions here.",
64 wantInstr: "# My Skill\n\nInstructions here.",
65 },
65 },
66+ {
67+ name: "frontmatter with utf8 bom",
68+ content: "\uFEFF---\n" +
69+ "name: bom-skill\n" +
70+ "description: Skill with bom.\n" +
71+ "---\n\n" +
72+ "# BOM Skill\n",
73+ wantName: "bom-skill",
74+ wantDesc: "Skill with bom.",
75+ wantInstr: "# BOM Skill",
76+ },
77+ {
78+ name: "frontmatter with leading blank lines",
79+ content: "\n\n---\n" +
80+ "name: blank-prefix\n" +
81+ "description: Skill with leading blank lines.\n" +
82+ "---\n\n" +
83+ "# Blank Prefix\n",
84+ wantName: "blank-prefix",
85+ wantDesc: "Skill with leading blank lines.",
86+ wantInstr: "# Blank Prefix",
87+ },
88+ {
89+ name: "frontmatter delimiter with trailing spaces",
90+ content: "--- \n" +
91+ "name: spaced-delimiter\n" +
92+ "description: Delimiter has spaces.\n" +
93+ "--- \n\n" +
94+ "# Spaced Delimiter\n",
95+ wantName: "spaced-delimiter",
96+ wantDesc: "Delimiter has spaces.",
97+ wantInstr: "# Spaced Delimiter",
98+ },
66 {
99 {
67 name: "no frontmatter",
100 name: "no frontmatter",
68 content: "# Just Markdown\n\nNo frontmatter here.",
101 content: "# Just Markdown\n\nNo frontmatter here.",
@@ -217,6 +250,7 @@ description: Name doesn't match directory.
217
250
218 skills := Discover([]string{tmpDir})
251 skills := Discover([]string{tmpDir})
219 require.Len(t, skills, 2)
252 require.Len(t, skills, 2)
253+ require.Equal(t, []string{"skill-two", "skill-one"}, []string{skills[0].Name, skills[1].Name})
220
254
221 names := make(map[string]bool)
255 names := make(map[string]bool)
222 for _, s := range skills {
256 for _, s := range skills {
@@ -248,6 +282,34 @@ func TestToPromptXMLEmpty(t *testing.T) {
248 require.Empty(t, ToPromptXML([]*Skill{}))
282 require.Empty(t, ToPromptXML([]*Skill{}))
249}
283}
250
284
285+func TestEscape(t *testing.T) {
286+ t.Parallel()
287+
288+ tests := []struct {
289+ name string
290+ in string
291+ want string
292+ }{
293+ {
294+ name: "escape xml special chars",
295+ in: `<tag attr="x&y">'z'</tag>`,
296+ want: `&lt;tag attr=&quot;x&amp;y&quot;&gt;&apos;z&apos;&lt;/tag&gt;`,
297+ },
298+ {
299+ name: "plain text unchanged",
300+ in: "hello world",
301+ want: "hello world",
302+ },
303+ }
304+
305+ for _, tt := range tests {
306+ t.Run(tt.name, func(t *testing.T) {
307+ t.Parallel()
308+ require.Equal(t, tt.want, escape(tt.in))
309+ })
310+ }
311+}
312+
251func TestToPromptXMLBuiltinType(t *testing.T) {
313func TestToPromptXMLBuiltinType(t *testing.T) {
252 t.Parallel()
314 t.Parallel()
253
315