Skip to content

Commit f7fe99a

Browse files
committed
refactor(skills): parse frontmatter with yaml library
Replaces the homegrown parsing code.
1 parent 4f44fa6 commit f7fe99a

5 files changed

Lines changed: 27 additions & 60 deletions

File tree

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ require (
1212
github.com/hashicorp/go-retryablehttp v0.7.8
1313
github.com/pelletier/go-toml/v2 v2.4.3
1414
github.com/zalando/go-keyring v0.2.8
15+
go.yaml.in/yaml/v4 v4.0.0-rc.6
1516
)
1617

1718
require (

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ github.com/yuin/goldmark-emoji v1.0.6 h1:QWfF2FYaXwL74tfGOW5izeiZepUDroDJfWubQI9
9898
github.com/yuin/goldmark-emoji v1.0.6/go.mod h1:ukxJDKFpdFb5x0a5HqbdlcKtebh086iJpI31LTKmWuA=
9999
github.com/zalando/go-keyring v0.2.8 h1:6sD/Ucpl7jNq10rM2pgqTs0sZ9V3qMrqfIIy5YPccHs=
100100
github.com/zalando/go-keyring v0.2.8/go.mod h1:tsMo+VpRq5NGyKfxoBVjCuMrG47yj8cmakZDO5QGii0=
101+
go.yaml.in/yaml/v4 v4.0.0-rc.6 h1:1h7H1ohdUh93/FyE4YaDa1Zh64K6VVbjF4K6WUxMtH4=
102+
go.yaml.in/yaml/v4 v4.0.0-rc.6/go.mod h1:aZqd9kCMsGL7AuUv/m/PvWLdg5sjJsZ4oHDEnfPPfY0=
101103
golang.org/x/exp v0.0.0-20260410095643-746e56fc9e2f h1:W3F4c+6OLc6H2lb//N1q4WpJkhzJCK5J6kUi1NTVXfM=
102104
golang.org/x/exp v0.0.0-20260410095643-746e56fc9e2f/go.mod h1:J1xhfL/vlindoeF/aINzNzt2Bket5bjo9sdOYzOsU80=
103105
golang.org/x/net v0.58.0 h1:ynWG7rqYi4ccpTEuPZ2QGWHktVEM9DMCj9yzDE0Q7To=

internal/agent/agent_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ func newSkillReg(t *testing.T, dir, content string) *skills.Registry {
420420
}
421421

422422
func TestActivateSkillNudgesModel(t *testing.T) {
423-
content := "---\nname: git\n description: follow repo conventions\n---\nbody\n"
423+
content := "---\nname: git\ndescription: follow repo conventions\n---\nbody\n"
424424
reg := newSkillReg(t, "git", content)
425425
skill, _ := reg.Get("git")
426426

internal/skills/skills.go

Lines changed: 15 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import (
1313
"path/filepath"
1414
"sort"
1515
"strings"
16+
17+
"go.yaml.in/yaml/v4"
1618
)
1719

1820
// Skill is a discovered skill's catalog metadata and the path to its SKILL.md.
@@ -89,9 +91,6 @@ func (r *Registry) load(md string) error {
8991
if dir := filepath.Base(filepath.Dir(md)); dir != name {
9092
log.Printf("skills: %s: name %q does not match directory %q", md, name, dir)
9193
}
92-
if strings.Contains(desc, ":") {
93-
log.Printf("skills: %s: description for %q may contain an unquoted colon (subtly malformed YAML); loading anyway", md, name)
94-
}
9594
r.byName[name] = Skill{
9695
Name: name,
9796
Description: desc,
@@ -139,15 +138,19 @@ func (r *Registry) catalog(keep func(Skill) bool) []Skill {
139138
return out
140139
}
141140

142-
// parseFrontmatter extracts the name, description and disable-model-invocation fields
143-
// from a SKILL.md's YAML frontmatter block. It is deliberately lenient: it only needs
144-
// the scalar keys and tolerates unquoted values containing colons, which strict YAML
145-
// would reject.
141+
// frontmatter is the metadata block at the start of a SKILL.md file.
142+
type frontmatter struct {
143+
Name string `yaml:"name"`
144+
Description string `yaml:"description"`
145+
DisableModelInvocation bool `yaml:"disable-model-invocation"`
146+
}
147+
148+
// parseFrontmatter extracts the metadata from a SKILL.md's YAML frontmatter block.
146149
func parseFrontmatter(content string) (name, desc string, disableMD bool, ok bool) {
147-
// Must start with a standalone `---` line.
148150
if content != "---" && !strings.HasPrefix(content, "---\n") {
149151
return "", "", false, false
150152
}
153+
151154
rest := content[3:]
152155
nl := strings.IndexByte(rest, '\n')
153156
if nl < 0 {
@@ -159,38 +162,12 @@ func parseFrontmatter(content string) (name, desc string, disableMD bool, ok boo
159162
if end < 0 {
160163
return "", "", false, false
161164
}
162-
for _, line := range strings.Split(rest[:end], "\n") {
163-
line = strings.TrimSpace(line)
164-
if line == "" || strings.HasPrefix(line, "#") {
165-
continue
166-
}
167-
colon := strings.IndexByte(line, ':')
168-
if colon < 0 {
169-
continue
170-
}
171-
key := strings.TrimSpace(line[:colon])
172-
val := strings.TrimSpace(line[colon+1:])
173-
switch key {
174-
case "name":
175-
name = stripQuotes(val)
176-
case "description":
177-
desc = stripQuotes(val)
178-
case "disable-model-invocation":
179-
disableMD = parseBool(val)
180-
}
181-
}
182-
return name, desc, disableMD, name != ""
183-
}
184165

185-
// parseBool leniently parses a YAML boolean. It returns false for the default/empty
186-
// value, so an absent disable-model-invocation key never flips the flag.
187-
func parseBool(v string) bool {
188-
switch strings.ToLower(strings.TrimSpace(v)) {
189-
case "true", "yes", "y", "on", "1":
190-
return true
191-
default:
192-
return false
166+
var metadata frontmatter
167+
if err := yaml.Unmarshal([]byte(rest[:end]), &metadata); err != nil {
168+
return "", "", false, false
193169
}
170+
return metadata.Name, metadata.Description, metadata.DisableModelInvocation, metadata.Name != ""
194171
}
195172

196173
// closingDelim returns the index of the first line exactly equal to `---`, or -1. It
@@ -208,14 +185,3 @@ func closingDelim(s string) int {
208185
s = s[i+3:]
209186
}
210187
}
211-
212-
// stripQuotes removes a single matching surrounding quote pair, leaving a value with
213-
// stray or unbalanced quotes (e.g. an apostrophe) untouched.
214-
func stripQuotes(v string) string {
215-
if len(v) >= 2 {
216-
if (v[0] == '"' && v[len(v)-1] == '"') || (v[0] == '\'' && v[len(v)-1] == '\'') {
217-
return v[1 : len(v)-1]
218-
}
219-
}
220-
return v
221-
}

internal/skills/skills_test.go

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,13 @@ func TestNew_ProjectOverridesUser(t *testing.T) {
7676
}
7777
}
7878

79-
func TestNew_LenientParsing(t *testing.T) {
79+
func TestNew_YAMLFrontmatter(t *testing.T) {
8080
root := t.TempDir()
8181

8282
// Name/directory mismatch warns but still loads.
8383
writeSkill(t, root, "mismatch-dir", "---\nname: actual-name\ndescription: a skill\n---\nbody\n")
84-
// Unquoted colon in the value must not break parsing.
85-
writeSkill(t, root, "pdf", "---\nname: pdf\ndescription: use when: the user shares a PDF\n---\nbody\n")
84+
// YAML supports quoted values containing colons.
85+
writeSkill(t, root, "pdf", "---\nname: pdf\ndescription: \"use when: the user shares a PDF\"\n---\nbody\n")
8686
// A horizontal rule (`---`) in the body must not truncate the frontmatter.
8787
writeSkill(t, root, "body-hr", "---\nname: body-hr\ndescription: has a rule below\n---\n\n---\n\nmarkdown body\n")
8888
// A lone apostrophe must not be mangled; a matching quote pair is stripped.
@@ -115,7 +115,7 @@ func TestNew_LenientParsing(t *testing.T) {
115115
t.Error("mismatched name/dir skill was not loaded (should warn and load)")
116116
}
117117
if s, ok := got["pdf"]; !ok || s.Description != "use when: the user shares a PDF" {
118-
t.Errorf("lenient colon description not preserved: %+v", got["pdf"])
118+
t.Errorf("quoted colon description not preserved: %+v", got["pdf"])
119119
}
120120
if s, ok := got["body-hr"]; !ok || !strings.Contains(s.Description, "rule below") {
121121
t.Errorf("body horizontal rule truncated the frontmatter: %+v", got["body-hr"])
@@ -173,7 +173,7 @@ func TestModelCatalogExcludesDisabledSkills(t *testing.T) {
173173
}
174174
}
175175

176-
func TestParseDisabledModelInvocationVariants(t *testing.T) {
176+
func TestParseDisabledModelInvocation(t *testing.T) {
177177
cases := []struct {
178178
val string
179179
want bool
@@ -182,15 +182,13 @@ func TestParseDisabledModelInvocationVariants(t *testing.T) {
182182
{val: "True", want: true},
183183
{val: "yes", want: true},
184184
{val: "on", want: true},
185-
{val: "1", want: true},
186185
{val: "false", want: false},
187186
{val: "", want: false},
188-
{val: "banana", want: false},
189187
}
190188
for _, c := range cases {
191-
_, _, got, _ := parseFrontmatter("---\nname: x\ndescription: d\ndisable-model-invocation: " + c.val + "\n---\n")
192-
if got != c.want {
193-
t.Errorf("parseFrontmatter(disable-model-invocation: %q) = %v, want %v", c.val, got, c.want)
189+
_, _, got, ok := parseFrontmatter("---\nname: x\ndescription: d\ndisable-model-invocation: " + c.val + "\n---\n")
190+
if !ok || got != c.want {
191+
t.Errorf("parseFrontmatter(disable-model-invocation: %q) = %v, %v; want %v, true", c.val, got, ok, c.want)
194192
}
195193
}
196194
}

0 commit comments

Comments
 (0)