Merge pull request #411 from harshbansal7/frontmatter_fix

Bug Fix: Fix parsing of SKILL.md file frontmatter - regex

pkg/skills/loader.go

@@ -9,6 +9,8 @@ import (
 	"path/filepath"
 	"regexp"
 	"strings"
+
+	"github.com/sipeed/picoclaw/pkg/logger"
 )
 
 var namePattern = regexp.MustCompile(`^[a-zA-Z0-9]+(-[a-zA-Z0-9]+)*$`)
@@ -251,6 +253,11 @@ func (sl *SkillsLoader) BuildSkillsSummary() string {
 func (sl *SkillsLoader) getSkillMetadata(skillPath string) *SkillMetadata {
 	content, err := os.ReadFile(skillPath)
 	if err != nil {
+		logger.WarnCF("skills", "Failed to read skill metadata",
+			map[string]interface{}{
+				"skill_path": skillPath,
+				"error":      err.Error(),
+			})
 		return nil
 	}
 
@@ -283,10 +290,15 @@ func (sl *SkillsLoader) getSkillMetadata(skillPath string) *SkillMetadata {
 
 // parseSimpleYAML parses simple key: value YAML format
 // Example: name: github\n description: "..."
+// Normalizes line endings to handle \n (Unix), \r\n (Windows), and \r (classic Mac)
 func (sl *SkillsLoader) parseSimpleYAML(content string) map[string]string {
 	result := make(map[string]string)
 
-	for _, line := range strings.Split(content, "\n") {
+	// Normalize line endings: convert \r\n and \r to \n
+	normalized := strings.ReplaceAll(content, "\r\n", "\n")
+	normalized = strings.ReplaceAll(normalized, "\r", "\n")
+
+	for _, line := range strings.Split(normalized, "\n") {
 		line = strings.TrimSpace(line)
 		if line == "" || strings.HasPrefix(line, "#") {
 			continue
@@ -306,9 +318,10 @@ func (sl *SkillsLoader) parseSimpleYAML(content string) map[string]string {
 }
 
 func (sl *SkillsLoader) extractFrontmatter(content string) string {
-	// (?s) enables DOTALL mode so . matches newlines
-	// Match first ---, capture everything until next --- on its own line
-	re := regexp.MustCompile(`(?s)^---\n(.*)\n---`)
+	// Support \n (Unix), \r\n (Windows), and \r (classic Mac) line endings for frontmatter blocks
+	// (?s) enables DOTALL so . matches newlines;
+	// ^--- at start, then ... --- at start of line, honoring all three line ending types
+	re := regexp.MustCompile(`(?s)^---(?:\r\n|\n|\r)(.*?)(?:\r\n|\n|\r)---`)
 	match := re.FindStringSubmatch(content)
 	if len(match) > 1 {
 		return match[1]
@@ -317,7 +330,11 @@ func (sl *SkillsLoader) extractFrontmatter(content string) string {
 }
 
 func (sl *SkillsLoader) stripFrontmatter(content string) string {
-	re := regexp.MustCompile(`^---\n.*?\n---\n`)
+	// Support \n (Unix), \r\n (Windows), and \r (classic Mac) line endings for frontmatter blocks
+	// (?s) enables DOTALL so . matches newlines;
+	// ^--- at start, then ... --- at start of line, honoring all three line ending types
+	// Match zero or more trailing line endings after closing --- (handles both with and without blank lines)
+	re := regexp.MustCompile(`(?s)^---(?:\r\n|\n|\r)(.*?)(?:\r\n|\n|\r)---(?:\r\n|\n|\r)*`)
 	return re.ReplaceAllString(content, "")
 }
 

pkg/skills/loader_test.go

@@ -75,3 +75,105 @@ func TestSkillsInfoValidate(t *testing.T) {
 		})
 	}
 }
+
+func TestExtractFrontmatter(t *testing.T) {
+	sl := &SkillsLoader{}
+
+	testcases := []struct {
+		name            string
+		content         string
+		expectedName    string
+		expectedDesc    string
+		lineEndingType  string
+	}{
+		{
+			name:           "unix-line-endings",
+			lineEndingType: "Unix (\\n)",
+			content:        "---\nname: test-skill\ndescription: A test skill\n---\n\n# Skill Content",
+			expectedName:   "test-skill",
+			expectedDesc:   "A test skill",
+		},
+		{
+			name:           "windows-line-endings",
+			lineEndingType: "Windows (\\r\\n)",
+			content:        "---\r\nname: test-skill\r\ndescription: A test skill\r\n---\r\n\r\n# Skill Content",
+			expectedName:   "test-skill",
+			expectedDesc:   "A test skill",
+		},
+		{
+			name:           "classic-mac-line-endings",
+			lineEndingType: "Classic Mac (\\r)",
+			content:        "---\rname: test-skill\rdescription: A test skill\r---\r\r# Skill Content",
+			expectedName:   "test-skill",
+			expectedDesc:   "A test skill",
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			// Extract frontmatter
+			frontmatter := sl.extractFrontmatter(tc.content)
+			assert.NotEmpty(t, frontmatter, "Frontmatter should be extracted for %s line endings", tc.lineEndingType)
+
+			// Parse YAML to get name and description (parseSimpleYAML now handles all line ending types)
+			yamlMeta := sl.parseSimpleYAML(frontmatter)
+			assert.Equal(t, tc.expectedName, yamlMeta["name"], "Name should be correctly parsed from frontmatter with %s line endings", tc.lineEndingType)
+			assert.Equal(t, tc.expectedDesc, yamlMeta["description"], "Description should be correctly parsed from frontmatter with %s line endings", tc.lineEndingType)
+		})
+	}
+}
+
+func TestStripFrontmatter(t *testing.T) {
+	sl := &SkillsLoader{}
+
+	testcases := []struct {
+		name            string
+		content         string
+		expectedContent string
+		lineEndingType  string
+	}{
+		{
+			name:            "unix-line-endings",
+			lineEndingType:  "Unix (\\n)",
+			content:         "---\nname: test-skill\ndescription: A test skill\n---\n\n# Skill Content",
+			expectedContent: "# Skill Content",
+		},
+		{
+			name:            "windows-line-endings",
+			lineEndingType:  "Windows (\\r\\n)",
+			content:         "---\r\nname: test-skill\r\ndescription: A test skill\r\n---\r\n\r\n# Skill Content",
+			expectedContent: "# Skill Content",
+		},
+		{
+			name:            "classic-mac-line-endings",
+			lineEndingType:  "Classic Mac (\\r)",
+			content:         "---\rname: test-skill\rdescription: A test skill\r---\r\r# Skill Content",
+			expectedContent: "# Skill Content",
+		},
+		{
+			name:            "unix-line-endings-without-trailing-newline",
+			lineEndingType:  "Unix (\\n) without trailing newline",
+			content:         "---\nname: test-skill\ndescription: A test skill\n---\n# Skill Content",
+			expectedContent: "# Skill Content",
+		},
+		{
+			name:            "windows-line-endings-without-trailing-newline",
+			lineEndingType:  "Windows (\\r\\n) without trailing newline",
+			content:         "---\r\nname: test-skill\r\ndescription: A test skill\r\n---\r\n# Skill Content",
+			expectedContent: "# Skill Content",
+		},
+		{
+			name:            "no-frontmatter",
+			lineEndingType:  "No frontmatter",
+			content:         "# Skill Content\n\nSome content here.",
+			expectedContent: "# Skill Content\n\nSome content here.",
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := sl.stripFrontmatter(tc.content)
+			assert.Equal(t, tc.expectedContent, result, "Frontmatter should be stripped correctly for %s", tc.lineEndingType)
+		})
+	}
+}