From 58c7d5837a393b6dbb80c7106bd54e8448b64a09 Mon Sep 17 00:00:00 2001 From: Andy Bonventre Date: Mon, 13 Jul 2026 12:20:32 -0400 Subject: [PATCH 1/2] fix(skill): accept YAML block scalars in SKILL.md frontmatter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frontmatter parser only handled single-line `key: value` scalars, so a `description: |` (literal) or `description: >` (folded) block — valid YAML, and the common way to write a long multi-line description — was rejected with "unexpected indented line in frontmatter", which fails skill discovery on the first Prompt and takes the whole session down. Parse block scalars: collect the following more-indented lines as the value, dedented, joined with newlines (literal) or spaces (folded), ending at the next non-indented key or end of frontmatter, with an optional +/- chomping indicator accepted. Description length validation still applies to the assembled value. The explicit indentation-indicator form ("|2") stays out of scope. TDD: literal, folded, chomped, and block-before-next-key cases. --- skill/frontmatter.go | 78 ++++++++++++++++++++++++++++++++++++++++++++ skill/skill_test.go | 47 ++++++++++++++++++++++++++ 2 files changed, 125 insertions(+) diff --git a/skill/frontmatter.go b/skill/frontmatter.go index 34d4cb1..4d71fef 100644 --- a/skill/frontmatter.go +++ b/skill/frontmatter.go @@ -84,6 +84,20 @@ func parseFrontmatter(fm string) (fields map[string]string, meta map[string]stri continue } + // A YAML block scalar (description: |, or : >) carries its value on the + // following more-indented lines rather than inline. Common for long, + // multi-line descriptions and valid YAML — collect the block instead of + // tripping the indented-line guard above. + if style, isBlock := blockScalarIndicator(value); isBlock { + blockVal, next := parseBlockScalar(lines, i+1, style) + if _, dup := fields[key]; dup { + return nil, nil, fmt.Errorf("duplicate frontmatter key: %q", key) + } + fields[key] = blockVal + i = next - 1 + continue + } + if _, dup := fields[key]; dup { return nil, nil, fmt.Errorf("duplicate frontmatter key: %q", key) } @@ -92,6 +106,70 @@ func parseFrontmatter(fm string) (fields map[string]string, meta map[string]stri return fields, meta, nil } +// blockScalarIndicator reports whether a frontmatter value is a YAML block +// scalar header — '|' (literal) or '>' (folded), with an optional '+'/'-' +// chomping indicator — and returns the style byte. The YAML explicit +// indentation-indicator form (e.g. "|2") is intentionally not part of this +// subset; skills use the plain forms. +func blockScalarIndicator(value string) (style byte, ok bool) { + if value == "" || (value[0] != '|' && value[0] != '>') { + return 0, false + } + switch value[1:] { + case "", "-", "+": + return value[0], true + } + return 0, false +} + +// parseBlockScalar collects the indented lines of a YAML block scalar starting +// at line index start. style '|' joins the dedented lines with newlines +// (literal); '>' joins them with spaces, blank lines becoming newlines +// (folded). The block ends at the first non-indented, non-blank line (the next +// key) or end of input; trailing blank lines are dropped. It returns the value +// and the index of the first line after the block. This covers the scalar +// string values skills carry; full YAML chomping/indentation semantics are not +// reproduced. +func parseBlockScalar(lines []string, start int, style byte) (value string, next int) { + indent := -1 + var content []string + i := start + for ; i < len(lines); i++ { + line := lines[i] + if strings.TrimSpace(line) == "" { + content = append(content, "") // preserve blank lines within the block + continue + } + leading := len(line) - len(strings.TrimLeft(line, " \t")) + if leading == 0 { + break // a non-indented line ends the block + } + if indent < 0 { + indent = leading + } + content = append(content, line[min(indent, leading):]) + } + for len(content) > 0 && content[len(content)-1] == "" { + content = content[:len(content)-1] // strip trailing blank lines + } + if style == '>' { + var b strings.Builder + for idx, c := range content { + switch { + case c == "": + b.WriteByte('\n') + case idx > 0 && content[idx-1] != "": + b.WriteByte(' ') + b.WriteString(c) + default: + b.WriteString(c) + } + } + return b.String(), i + } + return strings.Join(content, "\n"), i +} + // parseMetadataBlock reads the indented "key: value" entries following a // "metadata:" line, starting at line index start. It returns the parsed map, // the index of the first line that is not part of the block, and any error. diff --git a/skill/skill_test.go b/skill/skill_test.go index cc9b137..6589679 100644 --- a/skill/skill_test.go +++ b/skill/skill_test.go @@ -106,6 +106,53 @@ Everything. } } +// TestLoadBlockScalarDescription covers YAML block scalars for multi-line +// scalar values (description: |, description: >). These are valid YAML and +// commonly used for long descriptions; the parser must accept them rather +// than erroring on the indented continuation lines. Fixtures are synthetic. +func TestLoadBlockScalarDescription(t *testing.T) { + cases := []struct { + name string + fm string // frontmatter lines after "name: my-skill\n" + want string + }{ + { + name: "literal block joins with newlines", + fm: "description: |\n First line.\n Second line.\n", + want: "First line.\nSecond line.", + }, + { + name: "folded block joins with spaces", + fm: "description: >\n First part\n second part.\n", + want: "First part second part.", + }, + { + name: "literal with strip chomping", + fm: "description: |-\n Only line.\n", + want: "Only line.", + }, + { + name: "block value before another key", + fm: "description: |\n Line one.\n Line two.\nlicense: MIT\n", + want: "Line one.\nLine two.", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + root := t.TempDir() + body := "---\nname: my-skill\n" + tc.fm + "---\nbody\n" + d := writeSkill(t, root, "my-skill", body) + s, err := Load(d) + if err != nil { + t.Fatalf("block-scalar description should parse, got: %v", err) + } + if s.Description != tc.want { + t.Fatalf("Description = %q, want %q", s.Description, tc.want) + } + }) + } +} + func TestLoadNameRules(t *testing.T) { cases := []struct { name string // dir name (and, unless mismatch, frontmatter name) From 41c052df2a1bc6c2094d86e2122af884fcb551ff Mon Sep 17 00:00:00 2001 From: Andy Bonventre Date: Mon, 13 Jul 2026 12:35:59 -0400 Subject: [PATCH 2/2] docs(skill): document block-scalar support; clarify + chomping; length test Address PR #76 review: - The package doc listed block scalars and multi-line values as deliberately unsupported, which the parser now contradicts. Move them to the supported- constructs list with the exact semantics (dedent, literal/folded join, |2 out of scope, + normalized to clip); the package doc is this package's spec. - Note at blockScalarIndicator that the +/- chomping indicator is accepted for a valid header but + (keep) is normalized to clip, matching parseBlockScalar. - Add TestLoadBlockScalarDescriptionLengthLimit: a block scalar whose joined value exceeds 1024 runes is still rejected, locking the value through the same length guard as inline scalars. --- skill/frontmatter.go | 4 +++- skill/skill.go | 15 ++++++++++----- skill/skill_test.go | 14 ++++++++++++++ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/skill/frontmatter.go b/skill/frontmatter.go index 4d71fef..2650b06 100644 --- a/skill/frontmatter.go +++ b/skill/frontmatter.go @@ -108,7 +108,9 @@ func parseFrontmatter(fm string) (fields map[string]string, meta map[string]stri // blockScalarIndicator reports whether a frontmatter value is a YAML block // scalar header — '|' (literal) or '>' (folded), with an optional '+'/'-' -// chomping indicator — and returns the style byte. The YAML explicit +// chomping indicator — and returns the style byte. The chomping indicator is +// accepted so a valid header parses, but parseBlockScalar always clips +// trailing blank lines, so '+' (keep) is normalized to clip. The YAML explicit // indentation-indicator form (e.g. "|2") is intentionally not part of this // subset; skills use the plain forms. func blockScalarIndicator(value string) (style byte, ok bool) { diff --git a/skill/skill.go b/skill/skill.go index 5d9bc2c..f1308b5 100644 --- a/skill/skill.go +++ b/skill/skill.go @@ -22,15 +22,20 @@ // the outer quote pair. // - A single one-level-deep "metadata:" mapping block whose entries are // indented "key: value" scalar pairs. +// - Literal (|) and folded (>) block scalars for a multi-line scalar value, +// with an optional -/+ chomping indicator. The following more-indented +// lines are dedented and joined (newlines for |, spaces for >), ending at +// the next non-indented key. Explicit indentation indicators (e.g. |2) are +// out of scope, and + (keep) is accepted for validity but normalized to +// clip (trailing blank lines are always dropped). // - Unknown top-level keys are rejected with an error (spec-first // strictness: only fields named by the specification are permitted). // // Deliberately unsupported YAML constructs (any use is an error or is treated -// as a plain string, never interpreted): block scalars (| and >), flow -// collections ([a, b] / {k: v}), multi-line values, anchors/aliases, tags, -// nested mappings deeper than metadata's one level, and sequence (- item) -// syntax. allowed-tools is a single space-separated scalar string per the -// spec, not a YAML sequence. +// as a plain string, never interpreted): flow collections ([a, b] / {k: v}), +// anchors/aliases, tags, nested mappings deeper than metadata's one level, and +// sequence (- item) syntax. allowed-tools is a single space-separated scalar +// string per the spec, not a YAML sequence. package skill import ( diff --git a/skill/skill_test.go b/skill/skill_test.go index 6589679..c144b0c 100644 --- a/skill/skill_test.go +++ b/skill/skill_test.go @@ -153,6 +153,20 @@ func TestLoadBlockScalarDescription(t *testing.T) { } } +// TestLoadBlockScalarDescriptionLengthLimit locks in that a block scalar +// cannot smuggle an over-length description past the 1024-rune limit: the +// assembled value is validated like any inline scalar. +func TestLoadBlockScalarDescriptionLengthLimit(t *testing.T) { + root := t.TempDir() + line := strings.Repeat("x", 600) + // Two 600-rune lines joined with a newline = 1201 runes, over the limit. + body := "---\nname: my-skill\ndescription: |\n " + line + "\n " + line + "\n---\nbody\n" + d := writeSkill(t, root, "my-skill", body) + if _, err := Load(d); err == nil || !strings.Contains(err.Error(), "description") { + t.Fatalf("over-length block-scalar description should be rejected, got err=%v", err) + } +} + func TestLoadNameRules(t *testing.T) { cases := []struct { name string // dir name (and, unless mismatch, frontmatter name)