Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit bdf2698

Browse filesBrowse files
ibetitsmikeibetitsmike
authored
fix: parse skill frontmatter as YAML (#25610)
1 parent 15ada66 commit bdf2698
Copy full SHA for bdf2698

5 files changed

+223-87Lines changed: 223 additions & 87 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎coderd/x/skills/skills_test.go‎

Copy file name to clipboardExpand all lines: coderd/x/skills/skills_test.go
+37Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,33 @@ func TestParsePersonalSkillMarkdown(t *testing.T) {
2626
require.Equal(t, "Use this skill.", content.Body)
2727
})
2828

29+
t.Run("ValidWithFoldedDescription", func(t *testing.T) {
30+
t.Parallel()
31+
32+
content, err := skills.ParsePersonalSkillMarkdown([]byte(strings.Join([]string{
33+
"---",
34+
"name: brainstorming",
35+
"description: >",
36+
" Use before any creative work: features, components, functionality changes,",
37+
" or behavior modifications. Turns ideas into approved designs through",
38+
" collaborative dialog. Hard gate: no implementation action until the",
39+
" design is presented and approved.",
40+
"---",
41+
"Use this skill.",
42+
}, "\n")))
43+
44+
require.NoError(t, err)
45+
require.Equal(t, "brainstorming", content.Name)
46+
require.Equal(t, strings.Join([]string{
47+
"Use before any creative work: features, components, functionality changes,",
48+
"or behavior modifications. Turns ideas into approved designs through",
49+
"collaborative dialog. Hard gate: no implementation action until the",
50+
"design is presented and approved.",
51+
}, " "), content.Description)
52+
require.Equal(t, skills.SourcePersonal, content.Source)
53+
require.Equal(t, "Use this skill.", content.Body)
54+
})
55+
2956
t.Run("ValidWithoutDescription", func(t *testing.T) {
3057
t.Parallel()
3158

@@ -67,6 +94,16 @@ func TestParsePersonalSkillMarkdown(t *testing.T) {
6794
require.ErrorContains(t, err, "frontmatter must contain a 'name' field")
6895
})
6996

97+
t.Run("NonStringName", func(t *testing.T) {
98+
t.Parallel()
99+
100+
_, err := skills.ParsePersonalSkillMarkdown([]byte(
101+
"---\nname: null\n---\nBody.\n",
102+
))
103+
104+
require.ErrorIs(t, err, skills.ErrInvalidSkillName)
105+
})
106+
70107
t.Run("NonKebabCaseName", func(t *testing.T) {
71108
t.Parallel()
72109

Collapse file

‎codersdk/workspacesdk/frontmatter.go‎

Copy file name to clipboardExpand all lines: codersdk/workspacesdk/frontmatter.go
+23-35Lines changed: 23 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"strings"
66

77
"golang.org/x/xerrors"
8+
"gopkg.in/yaml.v3"
89
)
910

1011
// SkillNameRegex is the regular expression used to validate kebab-case skill names.
@@ -24,32 +25,21 @@ var markdownCommentRe = regexp.MustCompile(`<!--[\s\S]*?-->`)
2425
// the frontmatter is missing a required name field.
2526
var ErrFrontmatterNameRequired = xerrors.New("frontmatter missing required 'name' field")
2627

27-
func unquoteFrontmatterScalar(value string) string {
28-
if len(value) < 2 {
29-
return value
28+
func frontmatterStringField(frontmatter map[string]any, key string) (string, bool, error) {
29+
value, ok := frontmatter[key]
30+
if !ok {
31+
return "", false, nil
3032
}
31-
32-
quote := value[0]
33-
if quote != value[len(value)-1] {
34-
return value
35-
}
36-
37-
inner := value[1 : len(value)-1]
38-
switch quote {
39-
case '"':
40-
// This parser supports a small SKILL.md scalar subset, not full
41-
// YAML. Double quotes only unescape quoted text and Windows paths.
42-
return strings.NewReplacer(`\"`, `"`, `\\`, `\`).Replace(inner)
43-
case '\'':
44-
return inner
45-
default:
46-
return value
33+
stringValue, ok := value.(string)
34+
if !ok {
35+
return "", true, xerrors.Errorf("frontmatter field %q must be a string", key)
4736
}
37+
return strings.TrimRight(stringValue, "\r\n"), true, nil
4838
}
4939

5040
// ParseSkillFrontmatter extracts name, description, and the
5141
// remaining body from a skill meta file. The expected format is
52-
// YAML-ish frontmatter delimited by "---" lines:
42+
// YAML frontmatter delimited by "---" lines:
5343
//
5444
// ---
5545
// name: my-skill
@@ -78,25 +68,23 @@ func ParseSkillFrontmatter(content string) (name, description, body string, err
7868
)
7969
}
8070

81-
for _, line := range lines[1:closingIdx] {
82-
key, value, ok := strings.Cut(line, ":")
83-
if !ok {
84-
continue
85-
}
86-
key = strings.TrimSpace(key)
87-
value = strings.TrimSpace(value)
88-
value = unquoteFrontmatterScalar(value)
89-
switch strings.ToLower(key) {
90-
case "name":
91-
name = value
92-
case "description":
93-
description = value
94-
}
71+
frontmatterContent := strings.Join(lines[1:closingIdx], "\n")
72+
var frontmatter map[string]any
73+
if err := yaml.Unmarshal([]byte(frontmatterContent), &frontmatter); err != nil {
74+
return "", "", "", xerrors.Errorf("parse frontmatter YAML: %w", err)
9575
}
9676

97-
if name == "" {
77+
name, ok, err := frontmatterStringField(frontmatter, "name")
78+
if err != nil {
79+
return "", "", "", xerrors.Errorf("%w: %v", ErrFrontmatterNameRequired, err)
80+
}
81+
if !ok || name == "" {
9882
return "", "", "", xerrors.Errorf("%w", ErrFrontmatterNameRequired)
9983
}
84+
description, _, err = frontmatterStringField(frontmatter, "description")
85+
if err != nil {
86+
return "", "", "", err
87+
}
10088

10189
// Everything after the closing delimiter is the body.
10290
body = strings.Join(lines[closingIdx+1:], "\n")
Collapse file

‎codersdk/workspacesdk/frontmatter_test.go‎

Copy file name to clipboardExpand all lines: codersdk/workspacesdk/frontmatter_test.go
+50-7Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package workspacesdk_test
22

33
import (
4+
"strings"
45
"testing"
56

67
"github.com/stretchr/testify/require"
@@ -41,13 +42,48 @@ func TestParseSkillFrontmatter(t *testing.T) {
4142
require.Equal(t, "Review \"critical\" C:\\paths.", desc)
4243
})
4344

44-
t.Run("PlainHashValue", func(t *testing.T) {
45+
t.Run("FoldedDescription", func(t *testing.T) {
46+
t.Parallel()
47+
name, desc, body, err := workspacesdk.ParseSkillFrontmatter(
48+
strings.Join([]string{
49+
"---",
50+
"name: brainstorming",
51+
"description: >",
52+
" Use before any creative work: features, components, functionality changes,",
53+
" or behavior modifications. Turns ideas into approved designs through",
54+
" collaborative dialog. Hard gate: no implementation action until the",
55+
" design is presented and approved.",
56+
"",
57+
"---",
58+
"Use this skill.",
59+
}, "\n"),
60+
)
61+
require.NoError(t, err)
62+
require.Equal(t, "brainstorming", name)
63+
require.Equal(t, strings.Join([]string{
64+
"Use before any creative work: features, components, functionality changes,",
65+
"or behavior modifications. Turns ideas into approved designs through",
66+
"collaborative dialog. Hard gate: no implementation action until the",
67+
"design is presented and approved.",
68+
}, " "), desc)
69+
require.Equal(t, "Use this skill.", body)
70+
})
71+
72+
t.Run("YAMLComments", func(t *testing.T) {
4573
t.Parallel()
4674
_, desc, _, err := workspacesdk.ParseSkillFrontmatter(
4775
"---\nname: plain-hash\ndescription: Build # test\n---\nBody\n",
4876
)
4977
require.NoError(t, err)
50-
require.Equal(t, "Build # test", desc)
78+
require.Equal(t, "Build", desc)
79+
})
80+
81+
t.Run("ErrorNullDescription", func(t *testing.T) {
82+
t.Parallel()
83+
_, _, _, err := workspacesdk.ParseSkillFrontmatter(
84+
"---\nname: null-description\ndescription: null\n---\nBody\n",
85+
)
86+
require.ErrorContains(t, err, `frontmatter field "description" must be a string`)
5187
})
5288

5389
t.Run("NoDescription", func(t *testing.T) {
@@ -99,14 +135,12 @@ func TestParseSkillFrontmatter(t *testing.T) {
99135
require.Empty(t, body)
100136
})
101137

102-
t.Run("CaseInsensitiveKeys", func(t *testing.T) {
138+
t.Run("YAMLKeysAreCaseSensitive", func(t *testing.T) {
103139
t.Parallel()
104-
name, desc, _, err := workspacesdk.ParseSkillFrontmatter(
140+
_, _, _, err := workspacesdk.ParseSkillFrontmatter(
105141
"---\nName: upper\nDescription: Also upper\n---\n",
106142
)
107-
require.NoError(t, err)
108-
require.Equal(t, "upper", name)
109-
require.Equal(t, "Also upper", desc)
143+
require.ErrorIs(t, err, workspacesdk.ErrFrontmatterNameRequired)
110144
})
111145

112146
t.Run("UnknownKeysIgnored", func(t *testing.T) {
@@ -139,6 +173,15 @@ func TestParseSkillFrontmatter(t *testing.T) {
139173
require.ErrorContains(t, err, "frontmatter missing required 'name' field")
140174
})
141175

176+
t.Run("ErrorNullName", func(t *testing.T) {
177+
t.Parallel()
178+
_, _, _, err := workspacesdk.ParseSkillFrontmatter(
179+
"---\nname: null\n---\nBody\n",
180+
)
181+
require.ErrorIs(t, err, workspacesdk.ErrFrontmatterNameRequired)
182+
require.ErrorContains(t, err, `frontmatter field "name" must be a string`)
183+
})
184+
142185
t.Run("WhitespaceAroundDelimiters", func(t *testing.T) {
143186
t.Parallel()
144187
name, _, _, err := workspacesdk.ParseSkillFrontmatter(
Collapse file

‎site/src/pages/AgentsPage/utils/personalSkills.test.ts‎

Copy file name to clipboardExpand all lines: site/src/pages/AgentsPage/utils/personalSkills.test.ts
+74-3Lines changed: 74 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ describe("parsePersonalSkillMarkdown", () => {
9898
it("parses SKILL.md frontmatter and body", () => {
9999
expect(
100100
parsePersonalSkillMarkdown(
101-
'---\nname: test-skill\ndescription: "Does a thing"\n---\n\nUse this skill.',
101+
'---\nname: "test-skill"\ndescription: "Does a thing"\n---\n\nUse this skill.',
102102
),
103103
).toEqual({
104104
name: "test-skill",
@@ -107,14 +107,62 @@ describe("parsePersonalSkillMarkdown", () => {
107107
});
108108
});
109109

110-
it("uses backend-compatible parsing for YAML-comment-sensitive values", () => {
110+
it("parses folded YAML description values", () => {
111+
expect(
112+
parsePersonalSkillMarkdown(
113+
[
114+
"---",
115+
"name: brainstorming",
116+
"description: >",
117+
" Use before any creative work: features, components, functionality changes,",
118+
" or behavior modifications. Turns ideas into approved designs through",
119+
" collaborative dialog. Hard gate: no implementation action until the",
120+
" design is presented and approved.",
121+
"---",
122+
"Use this skill.",
123+
].join("\n"),
124+
),
125+
).toEqual({
126+
name: "brainstorming",
127+
description: [
128+
"Use before any creative work: features, components, functionality changes,",
129+
"or behavior modifications. Turns ideas into approved designs through",
130+
"collaborative dialog. Hard gate: no implementation action until the",
131+
"design is presented and approved.",
132+
].join(" "),
133+
body: "Use this skill.",
134+
});
135+
});
136+
137+
it("uses YAML comment semantics in frontmatter", () => {
111138
expect(
112139
parsePersonalSkillMarkdown(
113140
"---\nname: test-skill\ndescription: Build # test\n---\nBody",
114141
),
115142
).toEqual({
116143
name: "test-skill",
117-
description: "Build # test",
144+
description: "Build",
145+
body: "Body",
146+
});
147+
});
148+
149+
it("rejects non-string frontmatter fields", () => {
150+
expect(() =>
151+
parsePersonalSkillMarkdown("---\nname: null\n---\nBody"),
152+
).toThrow("Skill name must be a string.");
153+
expect(() =>
154+
parsePersonalSkillMarkdown(
155+
"---\nname: test-skill\ndescription: null\n---\nBody",
156+
),
157+
).toThrow("Skill description must be a string.");
158+
});
159+
160+
it("allows whitespace around frontmatter delimiters", () => {
161+
expect(
162+
parsePersonalSkillMarkdown(" --- \nname: test-skill\n --- \nBody"),
163+
).toEqual({
164+
name: "test-skill",
165+
description: "",
118166
body: "Body",
119167
});
120168
});
@@ -207,6 +255,29 @@ describe("buildPersonalSkillMarkdown", () => {
207255
).toBe("---\nname: test-skill\n---\nUse this skill.\n");
208256
});
209257

258+
it("quotes skill names that YAML would otherwise coerce", () => {
259+
const content = buildPersonalSkillMarkdown({
260+
name: "true",
261+
description: "",
262+
body: "Use this skill.",
263+
});
264+
265+
expect(content).toContain('name: "true"');
266+
expect(parsePersonalSkillMarkdown(content)).toMatchObject({
267+
name: "true",
268+
});
269+
270+
const numericNameContent = buildPersonalSkillMarkdown({
271+
name: "123",
272+
description: "",
273+
body: "Use this skill.",
274+
});
275+
expect(numericNameContent).toContain('name: "123"');
276+
expect(parsePersonalSkillMarkdown(numericNameContent)).toMatchObject({
277+
name: "123",
278+
});
279+
});
280+
210281
it("escapes quoted description values", () => {
211282
const content = buildPersonalSkillMarkdown({
212283
name: "test-skill",

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.