From 189d87e9e9992a51b419b87e7dbc4061b51068fe Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 22 Sep 2026 02:48:36 +0000 Subject: [PATCH 1/2] [dotnet-port-fixes] Refine skill frontmatter parsing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- agent/skills/fsskills/source.go | 125 +++++++++++++++---- agent/skills/fsskills/source_test.go | 176 +++++++++++++++++++++++++++ 2 files changed, 277 insertions(+), 24 deletions(-) diff --git a/agent/skills/fsskills/source.go b/agent/skills/fsskills/source.go index 5bf6ed6e..01884d62 100644 --- a/agent/skills/fsskills/source.go +++ b/agent/skills/fsskills/source.go @@ -35,10 +35,18 @@ var ( ) var ( + knownFrontmatterFields = map[string]string{ + "name": "name", + "description": "description", + "license": "license", + "compatibility": "compatibility", + "metadata": "metadata", + "allowed-tools": "allowed-tools", + } frontmatterRegex = regexp.MustCompile(`(?ms)\A^---\s*$(.+?)^---\s*$`) - yamlKeyValueRegex = regexp.MustCompile(`(?m)^([\w-]+)\s*:\s*(?:["'](.+?)["']|(.+?))\s*$`) - yamlMetadataBlockRegex = regexp.MustCompile(`(?m)^metadata\s*:\s*$\n((?:[ \t]+\S.*\n?|[ \t]*\r?\n)+)`) - yamlIndentedKeyValueRegex = regexp.MustCompile(`(?m)^\s+([\w-]+)\s*:\s*(?:["'](.+?)["']|(.+?))\s*$`) + yamlKeyValueRegex = regexp.MustCompile(`(?m)^([\w-]+|["'][\w-]+["'])[ \t]*:[ \t]*(?:["'](.*?)["']|([^\r\n]*?))[ \t]*\r?$`) + yamlMetadataBlockRegex = regexp.MustCompile(`(?m)^(?:metadata|"metadata"|'metadata')\s*:\s*$\r?\n((?:[ \t]+\S.*\n?|[ \t]*\r?\n)+)`) + yamlIndentedKeyValueRegex = regexp.MustCompile(`(?m)^[ \t]+([\w-]+)[ \t]*:[ \t]*(?:["'](.+?)["']|(.+?))[ \t]*\r?$`) ) // FilterContext provides contextual information about a discovered file to the @@ -315,16 +323,30 @@ func (s *Source) tryParseFrontmatter(content, skillFilePath string) (skills.Fron yamlContent := strings.TrimSpace(contentForParsing[match[2]:match[3]]) frontmatter := skills.Frontmatter{} + seenFields := make(map[string]struct{}, len(knownFrontmatterFields)) for _, kv := range yamlKeyValueRegex.FindAllStringSubmatchIndex(yamlContent, -1) { - key := yamlContent[kv[2]:kv[3]] - value := "" - if kv[4] >= 0 { - value = yamlContent[kv[4]:kv[5]] - } else if kv[6] >= 0 { - value = parseYamlScalarValue(yamlContent, kv) - } - switch strings.ToLower(key) { + key := normalizeFrontmatterKey(yamlContent[kv[2]:kv[3]]) + canonicalKey, recognized := knownFrontmatterFields[strings.ToLower(key)] + if !recognized { + continue + } + if key != canonicalKey { + s.logger.Error("SKILL.md uses incorrectly cased frontmatter field", "skillFilePath", skillFilePath, "fieldName", key, "expectedFieldName", canonicalKey) + return skills.Frontmatter{}, false + } + if _, duplicated := seenFields[canonicalKey]; duplicated { + s.logger.Error("SKILL.md contains duplicate frontmatter field", "skillFilePath", skillFilePath, "fieldName", canonicalKey) + return skills.Frontmatter{}, false + } + seenFields[canonicalKey] = struct{}{} + + value, hasValue := parseYamlValue(yamlContent, kv) + if !hasValue && canonicalKey != "name" && canonicalKey != "description" { + continue + } + + switch canonicalKey { case "name": frontmatter.Name = value case "description": @@ -340,11 +362,18 @@ func (s *Source) tryParseFrontmatter(content, skillFilePath string) (skills.Fron if metadataMatch := yamlMetadataBlockRegex.FindStringSubmatch(yamlContent); len(metadataMatch) == 2 { metadata := make(map[string]any) + seenMetadataKeys := make(map[string]struct{}) for _, kv := range yamlIndentedKeyValueRegex.FindAllStringSubmatch(metadataMatch[1], -1) { value := kv[2] if value == "" { value = kv[3] } + lowerKey := strings.ToLower(kv[1]) + if _, duplicated := seenMetadataKeys[lowerKey]; duplicated { + s.logger.Warn("SKILL.md contains duplicate metadata key; keeping the first value", "skillFilePath", skillFilePath, "key", kv[1]) + continue + } + seenMetadataKeys[lowerKey] = struct{}{} metadata[kv[1]] = value } if len(metadata) > 0 { @@ -368,6 +397,25 @@ func (s *Source) tryParseFrontmatter(content, skillFilePath string) (skills.Fron return frontmatter, true } +func normalizeFrontmatterKey(key string) string { + if len(key) >= 2 && (key[0] == '"' || key[0] == '\'') && key[len(key)-1] == key[0] { + return key[1 : len(key)-1] + } + return key +} + +func parseYamlValue(yamlContent string, kv []int) (string, bool) { + if kv[4] >= 0 { + return yamlContent[kv[4]:kv[5]], true + } + + if kv[6] < kv[7] { + return parseYamlScalarValue(yamlContent, kv), true + } + + return parseYamlIndentedValue(yamlContent, kv) +} + func parseYamlScalarValue(yamlContent string, kv []int) string { value := yamlContent[kv[6]:kv[7]] if value == "" || (value[0] != '|' && value[0] != '>') { @@ -376,9 +424,44 @@ func parseYamlScalarValue(yamlContent string, kv []int) string { scalarStyle := value[0] keepTrailingNewline := len(value) > 1 && value[1] == '+' + blockLines, ok := collectIndentedBlockLines(yamlContent, kv) + if !ok { + return "" + } + + normalizedLines := normalizeIndentedLines(blockLines) + + var parsedValue string + if scalarStyle == '|' { + parsedValue = strings.Join(normalizedLines, "\n") + } else { + parsedValue = foldYamlLines(normalizedLines) + } + + if keepTrailingNewline { + return parsedValue + "\n" + } + return parsedValue +} + +func parseYamlIndentedValue(yamlContent string, kv []int) (string, bool) { + blockLines, ok := collectIndentedBlockLines(yamlContent, kv) + if !ok { + return "", false + } + + value := strings.TrimSpace(strings.Join(normalizeIndentedLines(blockLines), "\n")) + if value == "" { + return "", false + } + + return normalizeFrontmatterKey(value), true +} + +func collectIndentedBlockLines(yamlContent string, kv []int) ([]string, bool) { lineBreak := strings.IndexByte(yamlContent[kv[1]:], '\n') if lineBreak < 0 { - return value + return nil, false } remaining := yamlContent[kv[1]+lineBreak+1:] @@ -400,9 +483,13 @@ func parseYamlScalarValue(yamlContent string, kv []int) string { } if len(blockLines) == 0 { - return "" + return nil, false } + return blockLines, true +} + +func normalizeIndentedLines(blockLines []string) []string { commonIndent := -1 for _, line := range blockLines { if line == "" { @@ -427,17 +514,7 @@ func parseYamlScalarValue(yamlContent string, kv []int) string { } } - var parsedValue string - if scalarStyle == '|' { - parsedValue = strings.Join(normalizedLines, "\n") - } else { - parsedValue = foldYamlLines(normalizedLines) - } - - if keepTrailingNewline { - return parsedValue + "\n" - } - return parsedValue + return normalizedLines } func foldYamlLines(lines []string) string { diff --git a/agent/skills/fsskills/source_test.go b/agent/skills/fsskills/source_test.go index 1509274d..8f706491 100644 --- a/agent/skills/fsskills/source_test.go +++ b/agent/skills/fsskills/source_test.go @@ -564,6 +564,182 @@ func TestFileSource_NoOptionalFields_DefaultZeroValues(t *testing.T) { } } +func TestFileSource_QuotedFrontmatterPropertyNames_AreParsed(t *testing.T) { + root := t.TempDir() + createSkillDirRaw(t, root, "quoted-root-keys", strings.Join([]string{ + "---", + `"name": quoted-root-keys`, + `'description': "A quoted root property skill"`, + `"metadata":`, + " author: contoso", + "---", + "Body.", + }, "\n")) + source := fsskills.NewSource(os.DirFS(root)) + + loaded, err := source.Skills(t.Context()) + if err != nil { + t.Fatal(err) + } + if len(loaded) != 1 { + t.Fatalf("expected 1 skill, got %d", len(loaded)) + } + fm := loaded[0].Frontmatter + if fm.Name != "quoted-root-keys" || fm.Description != "A quoted root property skill" { + t.Fatalf("unexpected frontmatter: %#v", fm) + } + if fm.Metadata["author"] != "contoso" { + t.Fatalf("expected metadata author contoso, got %#v", fm.Metadata["author"]) + } +} + +func TestFileSource_AmbiguousFrontmatter_IsRejected(t *testing.T) { + tests := []struct { + name string + fields []string + }{ + { + name: "duplicate recognized field", + fields: []string{ + "description: first", + "description: second", + }, + }, + { + name: "incorrectly cased recognized field", + fields: []string{ + "Description: invalid casing", + }, + }, + { + name: "duplicate quoted recognized field", + fields: []string{ + "allowed-tools: read", + `"allowed-tools": write`, + }, + }, + { + name: "duplicate metadata root", + fields: []string{ + "metadata:", + " author: first", + `"metadata":`, + " author: second", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := t.TempDir() + lines := []string{"---", "name: ambiguous-skill"} + lines = append(lines, tt.fields...) + lines = append(lines, "---", "Body.") + createSkillDirRaw(t, root, "ambiguous-skill", strings.Join(lines, "\n")) + + source := fsskills.NewSource(os.DirFS(root)) + loaded, err := source.Skills(t.Context()) + if err != nil { + t.Fatal(err) + } + if len(loaded) != 0 { + t.Fatalf("expected ambiguous frontmatter to be rejected, got %d skill(s)", len(loaded)) + } + }) + } +} + +func TestFileSource_IndentedValueOnNextLine_IsParsed(t *testing.T) { + root := t.TempDir() + createSkillDirRaw(t, root, "indented-next-line", strings.Join([]string{ + "---", + "name: indented-next-line", + "description:", + " 'Read files'", + "license: MIT", + "---", + "Body.", + }, "\n")) + source := fsskills.NewSource(os.DirFS(root)) + + loaded, err := source.Skills(t.Context()) + if err != nil { + t.Fatal(err) + } + if len(loaded) != 1 { + t.Fatalf("expected 1 skill, got %d", len(loaded)) + } + fm := loaded[0].Frontmatter + if fm.Description != "Read files" || fm.License != "MIT" { + t.Fatalf("unexpected frontmatter: %#v", fm) + } +} + +func TestFileSource_EmptyOptionalScalar_RemainsZeroValue(t *testing.T) { + root := t.TempDir() + createSkillDirRaw(t, root, "empty-optionals", strings.Join([]string{ + "---", + "name: empty-optionals", + "description: Read files", + "license: ", + "compatibility:\t", + "allowed-tools: ", + "---", + "Body.", + }, "\n")) + source := fsskills.NewSource(os.DirFS(root)) + + loaded, err := source.Skills(t.Context()) + if err != nil { + t.Fatal(err) + } + if len(loaded) != 1 { + t.Fatalf("expected 1 skill, got %d", len(loaded)) + } + fm := loaded[0].Frontmatter + if fm.License != "" || fm.Compatibility != "" || fm.AllowedTools != "" { + t.Fatalf("expected zero-value optional fields, got %#v", fm) + } +} + +func TestFileSource_DuplicateMetadata_KeepsFirstValue(t *testing.T) { + root := t.TempDir() + createSkillDirRaw(t, root, "duplicate-metadata", strings.Join([]string{ + "---", + "name: duplicate-metadata", + "description: Read files", + "metadata:", + " author: First", + " Author: Second", + " author: Third", + " version: 1.0", + "---", + "Body.", + }, "\n")) + source := fsskills.NewSource(os.DirFS(root)) + + loaded, err := source.Skills(t.Context()) + if err != nil { + t.Fatal(err) + } + if len(loaded) != 1 { + t.Fatalf("expected 1 skill, got %d", len(loaded)) + } + fm := loaded[0].Frontmatter + if len(fm.Metadata) != 2 { + t.Fatalf("expected 2 metadata entries, got %#v", fm.Metadata) + } + if fm.Metadata["author"] != "First" { + t.Fatalf("expected first metadata value to win, got %#v", fm.Metadata["author"]) + } + if _, exists := fm.Metadata["Author"]; exists { + t.Fatalf("expected first metadata key spelling to be preserved, got %#v", fm.Metadata) + } + if fm.Metadata["version"] != "1.0" { + t.Fatalf("expected version metadata to be preserved, got %#v", fm.Metadata["version"]) + } +} + func TestFileSource_ResourcesInSubdirectory_DiscoveredWithDefaultDepth(t *testing.T) { root := t.TempDir() createSkillDir(t, root, "sub-res-skill", "Subdirectory resources", "Body.") From 46ebe5d47b5fbc14a83acd8b60ee9f6999b6ce4f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 24 Sep 2026 20:13:50 +0000 Subject: [PATCH 2/2] Fold next-line block scalar indicators in indented values Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com> --- agent/skills/fsskills/source.go | 42 +++++++++------ agent/skills/fsskills/source_test.go | 76 ++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 15 deletions(-) diff --git a/agent/skills/fsskills/source.go b/agent/skills/fsskills/source.go index 01884d62..68c8830d 100644 --- a/agent/skills/fsskills/source.go +++ b/agent/skills/fsskills/source.go @@ -422,26 +422,12 @@ func parseYamlScalarValue(yamlContent string, kv []int) string { return value } - scalarStyle := value[0] - keepTrailingNewline := len(value) > 1 && value[1] == '+' blockLines, ok := collectIndentedBlockLines(yamlContent, kv) if !ok { return "" } - normalizedLines := normalizeIndentedLines(blockLines) - - var parsedValue string - if scalarStyle == '|' { - parsedValue = strings.Join(normalizedLines, "\n") - } else { - parsedValue = foldYamlLines(normalizedLines) - } - - if keepTrailingNewline { - return parsedValue + "\n" - } - return parsedValue + return foldYamlBlockScalar(value, blockLines) } func parseYamlIndentedValue(yamlContent string, kv []int) (string, bool) { @@ -450,6 +436,12 @@ func parseYamlIndentedValue(yamlContent string, kv []int) (string, bool) { return "", false } + if len(blockLines) > 0 { + if indicator := strings.TrimSpace(blockLines[0]); indicator != "" && (indicator[0] == '|' || indicator[0] == '>') { + return foldYamlBlockScalar(indicator, blockLines[1:]), true + } + } + value := strings.TrimSpace(strings.Join(normalizeIndentedLines(blockLines), "\n")) if value == "" { return "", false @@ -458,6 +450,26 @@ func parseYamlIndentedValue(yamlContent string, kv []int) (string, bool) { return normalizeFrontmatterKey(value), true } +// foldYamlBlockScalar folds blockLines according to the YAML block scalar +// indicator (e.g. "|", "|-", "|+", ">", ">-", ">+") given in indicator. +func foldYamlBlockScalar(indicator string, blockLines []string) string { + scalarStyle := indicator[0] + keepTrailingNewline := len(indicator) > 1 && indicator[1] == '+' + normalizedLines := normalizeIndentedLines(blockLines) + + var parsedValue string + if scalarStyle == '|' { + parsedValue = strings.Join(normalizedLines, "\n") + } else { + parsedValue = foldYamlLines(normalizedLines) + } + + if keepTrailingNewline { + return parsedValue + "\n" + } + return parsedValue +} + func collectIndentedBlockLines(yamlContent string, kv []int) ([]string, bool) { lineBreak := strings.IndexByte(yamlContent[kv[1]:], '\n') if lineBreak < 0 { diff --git a/agent/skills/fsskills/source_test.go b/agent/skills/fsskills/source_test.go index 8f706491..db32cc4e 100644 --- a/agent/skills/fsskills/source_test.go +++ b/agent/skills/fsskills/source_test.go @@ -675,6 +675,82 @@ func TestFileSource_IndentedValueOnNextLine_IsParsed(t *testing.T) { } } +func TestFileSource_IndentedBlockScalarOnNextLine_IsFolded(t *testing.T) { + tests := []struct { + name string + newline string + fields []string + expected string + }{ + { + name: "folded scalar LF", + newline: "\n", + fields: []string{ + "description:", + " >-", + " Read", + " files", + }, + expected: "Read files", + }, + { + name: "literal scalar LF", + newline: "\n", + fields: []string{ + "description:", + " |-", + " Read", + " files", + }, + expected: "Read\nfiles", + }, + { + name: "folded scalar CRLF", + newline: "\r\n", + fields: []string{ + "description:", + " >-", + " Read", + " files", + }, + expected: "Read files", + }, + { + name: "literal scalar CRLF", + newline: "\r\n", + fields: []string{ + "description:", + " |-", + " Read", + " files", + }, + expected: "Read\nfiles", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root := t.TempDir() + skillName := "indented-block-scalar" + lines := append([]string{"---", "name: " + skillName}, tt.fields...) + lines = append(lines, "---", "Body.") + createSkillDirRaw(t, root, skillName, strings.Join(lines, tt.newline)) + + source := fsskills.NewSource(os.DirFS(root)) + loaded, err := source.Skills(t.Context()) + if err != nil { + t.Fatal(err) + } + if len(loaded) != 1 { + t.Fatalf("expected 1 skill, got %d", len(loaded)) + } + if loaded[0].Frontmatter.Description != tt.expected { + t.Fatalf("unexpected description: %q", loaded[0].Frontmatter.Description) + } + }) + } +} + func TestFileSource_EmptyOptionalScalar_RemainsZeroValue(t *testing.T) { root := t.TempDir() createSkillDirRaw(t, root, "empty-optionals", strings.Join([]string{