Add stylistic lint checks to skill validate (#42) (#62)

* Add stylistic lint checks to skill validate (#42)

- Add SeverityHint for non-blocking style suggestions
- Add lintStyle() with 3 checks: body too short, description too vague, body lacks step structure
- Track hints separately from errors/warnings in CLI output
- Add ~ prefix for hint display in text output

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Add CLI-level tests for hint output in skill validate

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: devrimcavusoglu

internal/cli/skill_test.go

@@ -323,6 +323,35 @@ func TestSkillValidate_Text(t *testing.T) {
 	assert.Contains(t, out, "valid")
 }
 
+func TestSkillValidate_HintsJSON(t *testing.T) {
+	cc := testRegistry(t)
+
+	// Default body is short (~8 words) — triggers body-too-short hint
+	_, err := runCmd(t, cc, "skill", "create", "hint-skill", "--description", "A test skill", "--author", "alice")
+	require.NoError(t, err)
+
+	out, err := runCmd(t, cc, "skill", "validate", "hint-skill", "--json")
+	require.NoError(t, err)
+
+	var result output.SkillValidateResult
+	require.NoError(t, json.Unmarshal([]byte(out), &result))
+	assert.True(t, result.Valid, "hints should not make skill invalid")
+	assert.Equal(t, 0, result.Errors)
+	assert.Greater(t, result.Hints, 0, "should have at least one hint")
+}
+
+func TestSkillValidate_HintsText(t *testing.T) {
+	cc := testRegistry(t)
+
+	_, err := runCmd(t, cc, "skill", "create", "hint-text", "--description", "A test skill", "--author", "alice")
+	require.NoError(t, err)
+
+	out, err := runCmd(t, cc, "skill", "validate", "hint-text")
+	require.NoError(t, err)
+	assert.Contains(t, out, "hint(s)")
+	assert.Contains(t, out, "~")
+}
+
 // --- skill create with overlap ---
 
 func TestSkillCreate_OverlapWarn(t *testing.T) {

internal/cli/skill_validate.go

@@ -52,17 +52,21 @@ func toValidateResult(name string, issues []skill.ValidationIssue) output.SkillV
 	var issueResults []output.ValidationIssueResult
 	errors := 0
 	warns := 0
+	hints := 0
 
 	for _, issue := range issues {
 		issueResults = append(issueResults, output.ValidationIssueResult{
 			Field:    issue.Field,
 			Severity: string(issue.Severity),
 			Message:  issue.Message,
 		})
-		if issue.Severity == skill.SeverityError {
+		switch issue.Severity {
+		case skill.SeverityError:
 			errors++
-		} else {
+		case skill.SeverityWarning:
 			warns++
+		case skill.SeverityHint:
+			hints++
 		}
 	}
 
@@ -72,6 +76,7 @@ func toValidateResult(name string, issues []skill.ValidationIssue) output.SkillV
 		Issues: issueResults,
 		Errors: errors,
 		Warns:  warns,
+		Hints:  hints,
 	}
 }
 
@@ -83,16 +88,21 @@ func formatValidateResult(r output.SkillValidateResult) string {
 		return b.String()
 	}
 
-	if r.Valid {
-		fmt.Fprintf(&b, "Skill %q is valid with %d warning(s):\n", r.Name, r.Warns)
+	if r.Valid && r.Warns == 0 && r.Hints > 0 {
+		fmt.Fprintf(&b, "Skill %q is valid with %d hint(s):\n", r.Name, r.Hints)
+	} else if r.Valid {
+		fmt.Fprintf(&b, "Skill %q is valid with %d warning(s), %d hint(s):\n", r.Name, r.Warns, r.Hints)
 	} else {
-		fmt.Fprintf(&b, "Skill %q has %d error(s) and %d warning(s):\n", r.Name, r.Errors, r.Warns)
+		fmt.Fprintf(&b, "Skill %q has %d error(s), %d warning(s), %d hint(s):\n", r.Name, r.Errors, r.Warns, r.Hints)
 	}
 
 	for _, issue := range r.Issues {
 		prefix := "  ✗"
-		if issue.Severity == "warning" {
+		switch issue.Severity {
+		case "warning":
 			prefix = "  !"
+		case "hint":
+			prefix = "  ~"
 		}
 		fmt.Fprintf(&b, "%s %s: %s\n", prefix, issue.Field, issue.Message)
 	}

internal/output/types_skill.go

@@ -96,4 +96,5 @@ type SkillValidateResult struct {
 	Issues []ValidationIssueResult `json:"issues"`
 	Errors int                     `json:"errors"`
 	Warns  int                     `json:"warnings"`
+	Hints  int                     `json:"hints"`
 }

internal/skill/validator.go

@@ -15,6 +15,7 @@ type Severity string
 const (
 	SeverityError   Severity = "error"
 	SeverityWarning Severity = "warning"
+	SeverityHint    Severity = "hint"
 )
 
 // ValidationIssue represents a single validation problem found in a skill.
@@ -37,6 +38,7 @@ func Validate(s *Skill) []ValidationIssue {
 	issues = append(issues, validateBody(s.Body)...)
 	issues = append(issues, validateAllowedTools(s.AllowedTools)...)
 	issues = append(issues, validateMetadata(s.Metadata)...)
+	issues = append(issues, lintStyle(s)...)
 
 	return issues
 }
@@ -131,6 +133,54 @@ func ValidateFolder(s *Skill, skillDir string) []ValidationIssue {
 	return issues
 }
 
+// Stylistic lint thresholds.
+const (
+	lintBodyMinWords = 20
+	lintDescMinWords = 3
+)
+
+// lintStyle performs stylistic quality checks on a skill.
+// Issues use SeverityHint to distinguish from structural errors/warnings.
+func lintStyle(s *Skill) []ValidationIssue {
+	var issues []ValidationIssue
+
+	// Body too short
+	bodyWords := len(strings.Fields(strings.TrimSpace(s.Body)))
+	if bodyWords > 0 && bodyWords < lintBodyMinWords {
+		issues = append(issues, ValidationIssue{
+			Field:    "body",
+			Severity: SeverityHint,
+			Message:  fmt.Sprintf("body has only %d words; consider adding more detailed instructions", bodyWords),
+		})
+	}
+
+	// Description too vague (very short)
+	descWords := len(strings.Fields(strings.TrimSpace(s.Description)))
+	if descWords > 0 && descWords < lintDescMinWords {
+		issues = append(issues, ValidationIssue{
+			Field:    "description",
+			Severity: SeverityHint,
+			Message:  fmt.Sprintf("description has only %d word(s); consider being more specific", descWords),
+		})
+	}
+
+	// Body lacks step-by-step guidance markers
+	bodyLower := strings.ToLower(s.Body)
+	hasSteps := strings.Contains(bodyLower, "step") ||
+		strings.Contains(bodyLower, "1.") ||
+		strings.Contains(bodyLower, "- ") ||
+		strings.Contains(bodyLower, "* ")
+	if bodyWords >= lintBodyMinWords && !hasSteps {
+		issues = append(issues, ValidationIssue{
+			Field:    "body",
+			Severity: SeverityHint,
+			Message:  "body lacks step-by-step structure; consider adding numbered steps or bullet points",
+		})
+	}
+
+	return issues
+}
+
 var semverRegex = regexp.MustCompile(`^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)$`)
 
 func validateMetadata(m Metadata) []ValidationIssue {

internal/skill/validator_test.go

@@ -12,7 +12,7 @@ func TestValidate_ValidSkill(t *testing.T) {
 	s := &Skill{
 		Name:        "my-skill",
 		Description: "A valid skill description",
-		Body:        "## Instructions\n\nDo something useful.",
+		Body:        "## Instructions\n\nThis skill provides step-by-step guidance for completing tasks:\n\n- First, analyze the input carefully and validate the data\n- Then, process the data accordingly and format the results\n- Finally, return the formatted output to the user",
 		Metadata: Metadata{
 			Author:  Author{Name: "alice", Type: "human"},
 			Version: "1.0.0",
@@ -100,8 +100,8 @@ func TestValidate_Description1024OK(t *testing.T) {
 
 	issues := Validate(s)
 	for _, i := range issues {
-		if i.Field == "description" {
-			t.Errorf("unexpected description issue: %s", i.Message)
+		if i.Field == "description" && i.Severity == SeverityError {
+			t.Errorf("unexpected description error: %s", i.Message)
 		}
 	}
 }
@@ -242,6 +242,7 @@ func TestHasErrors(t *testing.T) {
 	}{
 		{"no issues", nil, false},
 		{"warnings only", []ValidationIssue{{Severity: SeverityWarning}}, false},
+		{"hints only", []ValidationIssue{{Severity: SeverityHint}}, false},
 		{"has error", []ValidationIssue{{Severity: SeverityError}}, true},
 		{"mixed", []ValidationIssue{{Severity: SeverityWarning}, {Severity: SeverityError}}, true},
 	}
@@ -261,3 +262,58 @@ func TestValidationIssue_String(t *testing.T) {
 	}
 	assert.Equal(t, "[error] name: name is invalid", issue.String())
 }
+
+// Lint style tests
+
+func TestLintStyle_BodyTooShort(t *testing.T) {
+	s := &Skill{
+		Name:        "my-skill",
+		Description: "A valid skill description",
+		Body:        "## Instructions\n\nDo something useful.",
+	}
+
+	issues := lintStyle(s)
+	require.Len(t, issues, 1)
+	assert.Equal(t, "body", issues[0].Field)
+	assert.Equal(t, SeverityHint, issues[0].Severity)
+	assert.Contains(t, issues[0].Message, "words")
+}
+
+func TestLintStyle_DescriptionTooVague(t *testing.T) {
+	s := &Skill{
+		Name:        "my-skill",
+		Description: "Deploy",
+		Body:        "## Instructions\n\nThis skill provides step-by-step guidance for completing tasks:\n\n- First, analyze the input carefully and validate the data\n- Then, process the data accordingly and format the results\n- Finally, return the formatted output to the user",
+	}
+
+	issues := lintStyle(s)
+	require.Len(t, issues, 1)
+	assert.Equal(t, "description", issues[0].Field)
+	assert.Equal(t, SeverityHint, issues[0].Severity)
+	assert.Contains(t, issues[0].Message, "word")
+}
+
+func TestLintStyle_BodyLacksSteps(t *testing.T) {
+	s := &Skill{
+		Name:        "my-skill",
+		Description: "A valid skill description",
+		Body:        strings.Repeat("word ", 25),
+	}
+
+	issues := lintStyle(s)
+	require.Len(t, issues, 1)
+	assert.Equal(t, "body", issues[0].Field)
+	assert.Equal(t, SeverityHint, issues[0].Severity)
+	assert.Contains(t, issues[0].Message, "step-by-step")
+}
+
+func TestLintStyle_NoHints(t *testing.T) {
+	s := &Skill{
+		Name:        "my-skill",
+		Description: "A valid skill description",
+		Body:        "## Instructions\n\nThis skill provides step-by-step guidance for completing tasks:\n\n- First, analyze the input carefully and validate the data\n- Then, process the data accordingly and format the results\n- Finally, return the formatted output to the user",
+	}
+
+	issues := lintStyle(s)
+	assert.Empty(t, issues)
+}