feat(skills): add validation for skill info and test cases (#231)

Add validation logic for SkillInfo to ensure name and description meet requirements
Include test cases covering various validation scenarios
Add testify dependency for testing assertions

pkg/skills/loader.go

@@ -2,13 +2,22 @@ package skills
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
+	"log/slog"
 	"os"
 	"path/filepath"
 	"regexp"
 	"strings"
 )
 
+var namePattern = regexp.MustCompile(`^[a-zA-Z0-9]+(-[a-zA-Z0-9]+)*$`)
+
+const (
+	MaxNameLength        = 64
+	MaxDescriptionLength = 1024
+)
+
 type SkillMetadata struct {
 	Name        string `json:"name"`
 	Description string `json:"description"`
@@ -21,6 +30,27 @@ type SkillInfo struct {
 	Description string `json:"description"`
 }
 
+func (info SkillInfo) validate() error {
+	var errs error
+	if info.Name == "" {
+		errs = errors.Join(errs, errors.New("name is required"))
+	} else {
+		if len(info.Name) > MaxNameLength {
+			errs = errors.Join(errs, fmt.Errorf("name exceeds %d characters", MaxNameLength))
+		}
+		if !namePattern.MatchString(info.Name) {
+			errs = errors.Join(errs, errors.New("name must be alphanumeric with hyphens"))
+		}
+	}
+
+	if info.Description == "" {
+		errs = errors.Join(errs, errors.New("description is required"))
+	} else if len(info.Description) > MaxDescriptionLength {
+		errs = errors.Join(errs, fmt.Errorf("description exceeds %d character", MaxDescriptionLength))
+	}
+	return errs
+}
+
 type SkillsLoader struct {
 	workspace       string
 	workspaceSkills string // workspace skills (项目级别)
@@ -54,6 +84,11 @@ func (sl *SkillsLoader) ListSkills() []SkillInfo {
 						metadata := sl.getSkillMetadata(skillFile)
 						if metadata != nil {
 							info.Description = metadata.Description
+							info.Name = metadata.Name
+						}
+						if err := info.validate(); err != nil {
+							slog.Warn("invalid skill from workspace", "name", info.Name, "error", err)
+							continue
 						}
 						skills = append(skills, info)
 					}
@@ -89,6 +124,11 @@ func (sl *SkillsLoader) ListSkills() []SkillInfo {
 						metadata := sl.getSkillMetadata(skillFile)
 						if metadata != nil {
 							info.Description = metadata.Description
+							info.Name = metadata.Name
+						}
+						if err := info.validate(); err != nil {
+							slog.Warn("invalid skill from global", "name", info.Name, "error", err)
+							continue
 						}
 						skills = append(skills, info)
 					}
@@ -123,6 +163,11 @@ func (sl *SkillsLoader) ListSkills() []SkillInfo {
 						metadata := sl.getSkillMetadata(skillFile)
 						if metadata != nil {
 							info.Description = metadata.Description
+							info.Name = metadata.Name
+						}
+						if err := info.validate(); err != nil {
+							slog.Warn("invalid skill from builtin", "name", info.Name, "error", err)
+							continue
 						}
 						skills = append(skills, info)
 					}

pkg/skills/loader_test.go

@@ -0,0 +1,77 @@
+package skills
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestSkillsInfoValidate(t *testing.T) {
+	testcases := []struct {
+		name        string
+		skillName   string
+		description string
+		wantErr     bool
+		errContains []string
+	}{
+		{
+			name:        "valid-skill",
+			skillName:   "valid-skill",
+			description: "a valid skill description",
+			wantErr:     false,
+		},
+		{
+			name:        "empty-name",
+			skillName:   "",
+			description: "description without name",
+			wantErr:     true,
+			errContains: []string{"name is required"},
+		},
+		{
+			name:        "empty-description",
+			skillName:   "skill-without-description",
+			description: "",
+			wantErr:     true,
+			errContains: []string{"description is required"},
+		},
+		{
+			name:        "empty-both",
+			skillName:   "",
+			description: "",
+			wantErr:     true,
+			errContains: []string{"name is required", "description is required"},
+		},
+		{
+			name:        "name-with-spaces",
+			skillName:   "skill with spaces",
+			description: "invalid name with spaces",
+			wantErr:     true,
+			errContains: []string{"name must be alphanumeric with hyphens"},
+		},
+		{
+			name:        "name-with-underscore",
+			skillName:   "skill_underscore",
+			description: "invalid name with underscore",
+			wantErr:     true,
+			errContains: []string{"name must be alphanumeric with hyphens"},
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			info := SkillInfo{
+				Name:        tc.skillName,
+				Description: tc.description,
+			}
+			err := info.validate()
+			if tc.wantErr {
+				assert.Error(t, err)
+				for _, msg := range tc.errContains {
+					assert.ErrorContains(t, err, msg)
+				}
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}