diff --git a/skill/frontmatter.go b/skill/frontmatter.go index 34d4cb1..2650b06 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,72 @@ 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 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) { + 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.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 cc9b137..c144b0c 100644 --- a/skill/skill_test.go +++ b/skill/skill_test.go @@ -106,6 +106,67 @@ 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) + } + }) + } +} + +// 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)