Loosen and fix skill validation

pkg/skills/skills.go

@@ -5,23 +5,14 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strings"
 
 	"github.com/goccy/go-yaml"
 
 	"github.com/docker/cagent/pkg/paths"
 )
 
-const (
-	skillFile = "SKILL.md"
-
-	maxNameLength        = 64
-	maxDescriptionLength = 1024
-	maxCompatLength      = 500
-)
-
-var namePattern = regexp.MustCompile(`^[a-z0-9]+(-[a-z0-9]+)*$`)
+const skillFile = "SKILL.md"
 
 // Skill represents a loaded skill with its metadata and content location.
 type Skill struct {
@@ -171,7 +162,7 @@ func loadSkillFile(path, dirName string) (Skill, bool) {
 	}
 
 	skill, ok := parseFrontmatter(string(content))
-	if !ok || !isValidSkill(skill, dirName) {
+	if !ok || !isValidSkill(skill) {
 		return Skill{}, false
 	}
 
@@ -208,32 +199,15 @@ func parseFrontmatter(content string) (Skill, bool) {
 }
 
 // isValidSkill validates skill constraints.
-func isValidSkill(skill Skill, dirName string) bool {
-	// Description is required and has a max length
-	if skill.Description == "" || len(skill.Description) > maxDescriptionLength {
-		return false
-	}
-
-	// Compatibility has a max length
-	if len(skill.Compatibility) > maxCompatLength {
+func isValidSkill(skill Skill) bool {
+	// Description and name is required
+	if skill.Description == "" || skill.Name == "" {
 		return false
 	}
 
-	// If name is set, it must be valid and match the directory name
-	if skill.Name != "" {
-		if !isValidName(skill.Name) || skill.Name != dirName {
-			return false
-		}
-	}
-
 	return true
 }
 
-// isValidName checks if a skill name follows the naming convention.
-func isValidName(name string) bool {
-	return name != "" && len(name) <= maxNameLength && namePattern.MatchString(name)
-}
-
 // isHiddenOrSymlink returns true for hidden files/dirs or symlinks.
 func isHiddenOrSymlink(entry fs.DirEntry) bool {
 	return strings.HasPrefix(entry.Name(), ".") || entry.Type()&os.ModeSymlink != 0

pkg/skills/skills_test.go

@@ -3,7 +3,6 @@ package skills
 import (
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -112,45 +111,14 @@ Body`,
 	}
 }
 
-func TestIsValidName(t *testing.T) {
-	tests := []struct {
-		name  string
-		valid bool
-	}{
-		{"pdf-processing", true},
-		{"data-analysis", true},
-		{"code-review", true},
-		{"simple", true},
-		{"a1b2c3", true},
-		{"skill123", true},
-		{"my-skill-name", true},
-		{"", false},
-		{"PDF-Processing", false},
-		{"-pdf", false},
-		{"pdf-", false},
-		{"pdf--processing", false},
-		{"pdf_processing", false},
-		{"pdf processing", false},
-		{"pdf.processing", false},
-		{strings.Repeat("a", 64), true},
-		{strings.Repeat("a", 65), false},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got := isValidName(tt.name)
-			assert.Equal(t, tt.valid, got, "isValidName(%q) = %v, want %v", tt.name, got, tt.valid)
-		})
-	}
-}
-
 func TestLoadSkillsFromDir_Flat(t *testing.T) {
 	tmpDir := t.TempDir()
 
 	skillDir := filepath.Join(tmpDir, "pdf-extractor")
 	require.NoError(t, os.MkdirAll(skillDir, 0o755))
 
 	skillContent := `---
+name: pdf-extractor
 description: Extract text from PDF files
 ---
 
@@ -226,72 +194,6 @@ This skill has no description field.
 	assert.Empty(t, skills)
 }
 
-func TestLoadSkillsFromDir_SkipNameMismatch(t *testing.T) {
-	tmpDir := t.TempDir()
-
-	skillDir := filepath.Join(tmpDir, "actual-dir-name")
-	require.NoError(t, os.MkdirAll(skillDir, 0o755))
-
-	skillContent := `---
-name: different-name
-description: A skill with mismatched name
----
-
-# Mismatched
-`
-	require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(skillContent), 0o644))
-
-	skills := loadSkillsFromDir(tmpDir, false)
-	assert.Empty(t, skills, "skill with name not matching directory should be skipped")
-}
-
-func TestLoadSkillsFromDir_SkipInvalidName(t *testing.T) {
-	tmpDir := t.TempDir()
-
-	skillDir := filepath.Join(tmpDir, "Invalid-Name")
-	require.NoError(t, os.MkdirAll(skillDir, 0o755))
-
-	skillContent := `---
-name: Invalid-Name
-description: A skill with invalid name format
----
-
-# Invalid
-`
-	require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(skillContent), 0o644))
-
-	skills := loadSkillsFromDir(tmpDir, false)
-	assert.Empty(t, skills, "skill with invalid name format should be skipped")
-}
-
-func TestLoadSkillsFromDir_SkipDescriptionTooLong(t *testing.T) {
-	tmpDir := t.TempDir()
-
-	skillDir := filepath.Join(tmpDir, "long-desc")
-	require.NoError(t, os.MkdirAll(skillDir, 0o755))
-
-	longDesc := strings.Repeat("a", 1025)
-	skillContent := "---\nname: long-desc\ndescription: " + longDesc + "\n---\n\n# Long\n"
-	require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(skillContent), 0o644))
-
-	skills := loadSkillsFromDir(tmpDir, false)
-	assert.Empty(t, skills, "skill with description > 1024 chars should be skipped")
-}
-
-func TestLoadSkillsFromDir_SkipCompatibilityTooLong(t *testing.T) {
-	tmpDir := t.TempDir()
-
-	skillDir := filepath.Join(tmpDir, "long-compat")
-	require.NoError(t, os.MkdirAll(skillDir, 0o755))
-
-	longCompat := strings.Repeat("a", 501)
-	skillContent := "---\nname: long-compat\ndescription: A skill\ncompatibility: " + longCompat + "\n---\n\n# Long\n"
-	require.NoError(t, os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(skillContent), 0o644))
-
-	skills := loadSkillsFromDir(tmpDir, false)
-	assert.Empty(t, skills, "skill with compatibility > 500 chars should be skipped")
-}
-
 func TestLoadSkillsFromDir_AllOptionalFields(t *testing.T) {
 	tmpDir := t.TempDir()
 
@@ -383,6 +285,7 @@ func TestLoad_Integration(t *testing.T) {
 	require.NoError(t, os.MkdirAll(claudeProjectDir, 0o755))
 
 	skillContent := `---
+name: test-skill
 description: Test project skill
 ---