feat: poka-yoke validate action with actionable feedback #626

Adds skill_manage(action='validate', name='...') that checks an
existing skill and provides specific remediation steps for each issue.

13 checks with specific fix suggestions:
1. Skill exists (with fuzzy-match suggestions)
2. SKILL.md readable
3. Content non-empty
4. Frontmatter delimiter (---)
5. Frontmatter closing
6. YAML valid (with common error hints)
7. Frontmatter name field
8. Frontmatter description field
9. Body content after frontmatter
10. Content size limits
11. Linked files (references/, templates/, scripts/)
12. Naming convention
13. File organization (orphaned files)

Each issue includes: check name, FAIL/WARN status, message, and
a specific fix instruction (often with exact command to run).

Closes #626

tools/skill_manager_tool.py

@@ -245,6 +245,269 @@ def _validate_file_path(file_path: str) -> Optional[str]:
     return None
 
 
+def _validate_skill(name: str) -> Dict[str, Any]:
+    """
+    Validate an existing skill and provide actionable feedback.
+
+    Checks:
+    1. Skill exists
+    2. SKILL.md frontmatter (name, description, valid YAML)
+    3. Content structure (body after frontmatter)
+    4. Content size limits
+    5. Linked files (references/, templates/, scripts/) exist
+    6. Naming conventions
+
+    Returns dict with success, issues (list of {check, status, message, fix}),
+    and summary.
+    """
+    issues = []
+    warnings = []
+
+    # Check 1: Does the skill exist?
+    skill_info = _find_skill(name)
+    if not skill_info:
+        # Try to find similar names for the suggestion
+        from agent.skill_utils import get_all_skills_dirs
+        all_names = []
+        for skills_dir in get_all_skills_dirs():
+            if skills_dir.exists():
+                for md in skills_dir.rglob("SKILL.md"):
+                    all_names.append(md.parent.name)
+        suggestion = ""
+        if all_names:
+            import difflib
+            close = difflib.get_close_matches(name, all_names, n=3, cutoff=0.6)
+            if close:
+                suggestion = f" Did you mean: {', '.join(close)}?"
+
+        return {
+            "success": False,
+            "valid": False,
+            "issues": [{"check": "existence", "status": "FAIL",
+                        "message": f"Skill '{name}' not found.{suggestion}",
+                        "fix": f"Create it with: skill_manage(action='create', name='{name}', content='...')"}],
+            "summary": f"Skill '{name}' does not exist."
+        }
+
+    skill_dir = skill_info["path"]
+    skill_md = skill_dir / "SKILL.md"
+
+    # Check 2: SKILL.md exists
+    if not skill_md.exists():
+        issues.append({
+            "check": "SKILL.md exists",
+            "status": "FAIL",
+            "message": f"No SKILL.md found in {skill_dir}",
+            "fix": f"Create SKILL.md with: skill_manage(action='create', name='{name}', content='---\\nname: {name}\\ndescription: ...\\n---\\n# Instructions\\n...')"
+        })
+        return {"success": True, "valid": False, "issues": issues, "summary": f"Skill '{name}' is missing SKILL.md."}
+
+    # Read content
+    try:
+        content = skill_md.read_text(encoding="utf-8")
+    except Exception as e:
+        issues.append({
+            "check": "SKILL.md readable",
+            "status": "FAIL",
+            "message": f"Cannot read SKILL.md: {e}",
+            "fix": "Check file permissions: chmod 644 SKILL.md"
+        })
+        return {"success": True, "valid": False, "issues": issues, "summary": f"Cannot read SKILL.md."}
+
+    # Check 3: Content not empty
+    if not content.strip():
+        issues.append({
+            "check": "content non-empty",
+            "status": "FAIL",
+            "message": "SKILL.md is empty.",
+            "fix": f"Add content with: skill_manage(action='edit', name='{name}', content='---\\nname: {name}\\ndescription: ...\\n---\\n# Instructions\\n...')"
+        })
+        return {"success": True, "valid": False, "issues": issues, "summary": "SKILL.md is empty."}
+
+    # Check 4: Frontmatter starts with ---
+    if not content.startswith("---"):
+        issues.append({
+            "check": "frontmatter delimiter",
+            "status": "FAIL",
+            "message": "SKILL.md must start with YAML frontmatter (---).",
+            "fix": "Add '---' as the first line, then YAML metadata, then '---' to close.\n"
+                   "Example:\n---\nname: my-skill\ndescription: What this skill does\n---\n# Instructions\n..."
+        })
+    else:
+        # Check 5: Frontmatter closes
+        end_match = re.search(r'\n---\s*\n', content[3:])
+        if not end_match:
+            issues.append({
+                "check": "frontmatter closing",
+                "status": "FAIL",
+                "message": "Frontmatter is not closed with '---'.",
+                "fix": "Add a line with just '---' after your YAML metadata to close the frontmatter block."
+            })
+        else:
+            # Check 6: Valid YAML
+            yaml_content = content[3:end_match.start() + 3]
+            try:
+                parsed = yaml.safe_load(yaml_content)
+            except yaml.YAMLError as e:
+                issues.append({
+                    "check": "YAML valid",
+                    "status": "FAIL",
+                    "message": f"YAML parse error: {e}",
+                    "fix": "Fix the YAML syntax in your frontmatter. Common issues:\n"
+                           "  - Missing quotes around values with special chars (:, {, }, [, ])\n"
+                           "  - Inconsistent indentation (use spaces, not tabs)\n"
+                           "  - Unescaped colons in descriptions"
+                })
+                parsed = None
+
+            if parsed and isinstance(parsed, dict):
+                # Check 7: name field
+                if "name" not in parsed:
+                    issues.append({
+                        "check": "frontmatter name",
+                        "status": "FAIL",
+                        "message": "Frontmatter missing 'name' field.",
+                        "fix": f"Add 'name: {name}' to your frontmatter YAML."
+                    })
+                elif parsed["name"] != name:
+                    warnings.append({
+                        "check": "frontmatter name match",
+                        "status": "WARN",
+                        "message": f"Frontmatter name '{parsed['name']}' doesn't match directory name '{name}'.",
+                        "fix": "Change 'name: " + str(parsed.get("name", "")) + "' to 'name: " + name + "' in frontmatter, or rename the directory to match."
+                    })
+
+                # Check 8: description field
+                if "description" not in parsed:
+                    issues.append({
+                        "check": "frontmatter description",
+                        "status": "FAIL",
+                        "message": "Frontmatter missing 'description' field.",
+                        "fix": "Add 'description: A brief description of what this skill does' to frontmatter. "
+                               f"Max {MAX_DESCRIPTION_LENGTH} characters."
+                    })
+                elif len(str(parsed["description"])) > MAX_DESCRIPTION_LENGTH:
+                    issues.append({
+                        "check": "description length",
+                        "status": "FAIL",
+                        "message": f"Description is {len(str(parsed['description']))} chars (max {MAX_DESCRIPTION_LENGTH}).",
+                        "fix": f"Shorten the description to under {MAX_DESCRIPTION_LENGTH} characters. "
+                               "Put detailed instructions in the body, not the description."
+                    })
+
+            elif parsed and not isinstance(parsed, dict):
+                issues.append({
+                    "check": "frontmatter structure",
+                    "status": "FAIL",
+                    "message": "Frontmatter must be a YAML mapping (key: value pairs).",
+                    "fix": "Ensure frontmatter contains key-value pairs like:\nname: my-skill\ndescription: What it does"
+                })
+
+            # Check 9: Body content after frontmatter
+            if end_match:
+                body = content[end_match.end() + 3:].strip()
+                if not body:
+                    issues.append({
+                        "check": "body content",
+                        "status": "FAIL",
+                        "message": "No content after frontmatter.",
+                        "fix": "Add instructions, steps, or reference content after the closing '---'. "
+                               "Skills need a body to be useful — at minimum a description of when to use the skill."
+                    })
+                elif len(body) < 20:
+                    warnings.append({
+                        "check": "body content size",
+                        "status": "WARN",
+                        "message": f"Body content is very short ({len(body)} chars).",
+                        "fix": "Add more detail: numbered steps, examples, pitfalls to avoid, "
+                               "or reference files in references/ or templates/."
+                    })
+
+    # Check 10: Content size
+    if len(content) > MAX_SKILL_CONTENT_CHARS:
+        issues.append({
+            "check": "content size",
+            "status": "FAIL",
+            "message": f"SKILL.md is {len(content):,} chars (max {MAX_SKILL_CONTENT_CHARS:,}).",
+            "fix": f"Split into a shorter SKILL.md (core instructions) with detailed content in:\n"
+                   f"  - references/detailed-guide.md\n"
+                   f"  - templates/example.yaml\n"
+                   f"  - scripts/validate.py\n"
+                   f"Use skill_manage(action='write_file') to add linked files."
+        })
+    elif len(content) > MAX_SKILL_CONTENT_CHARS * 0.8:
+        warnings.append({
+            "check": "content size warning",
+            "status": "WARN",
+            "message": f"SKILL.md is {len(content):,} chars ({len(content) * 100 // MAX_SKILL_CONTENT_CHARS}% of limit).",
+            "fix": "Consider moving detailed content to references/ or templates/ files."
+        })
+
+    # Check 11: Linked files exist
+    for subdir in ["references", "templates", "scripts"]:
+        subdir_path = skill_dir / subdir
+        if subdir_path.exists():
+            for linked_file in subdir_path.rglob("*"):
+                if linked_file.is_file():
+                    try:
+                        linked_file.read_text(encoding="utf-8")
+                    except Exception as e:
+                        warnings.append({
+                            "check": f"linked file {subdir}/{linked_file.name}",
+                            "status": "WARN",
+                            "message": f"Cannot read {linked_file.relative_to(skill_dir)}: {e}",
+                            "fix": f"Check file exists and has read permissions."
+                        })
+
+    # Check 12: Naming convention
+    if not VALID_NAME_RE.match(name):
+        warnings.append({
+            "check": "naming convention",
+            "status": "WARN",
+            "message": f"Skill name '{name}' doesn't follow convention (lowercase, hyphens, underscores).",
+            "fix": "Rename to use lowercase letters, numbers, hyphens, dots, and underscores only. "
+                   "Must start with a letter or digit."
+        })
+
+    # Check 13: Orphaned files (files not in allowed subdirs)
+    if skill_dir.exists():
+        for item in skill_dir.iterdir():
+            if item.name == "SKILL.md":
+                continue
+            if item.name.startswith("."):
+                continue
+            if item.is_dir() and item.name in ALLOWED_SUBDIRS:
+                continue
+            warnings.append({
+                "check": "file organization",
+                "status": "WARN",
+                "message": f"'{item.name}' is in the skill root, not in an allowed subdirectory.",
+                "fix": f"Move to references/, templates/, or scripts/. Allowed subdirs: {', '.join(sorted(ALLOWED_SUBDIRS))}"
+            })
+
+    # Build summary
+    fail_count = sum(1 for i in issues if i["status"] == "FAIL")
+    warn_count = len(warnings)
+    valid = fail_count == 0
+
+    if valid and warn_count == 0:
+        summary = f"Skill '{name}' is valid. No issues found."
+    elif valid:
+        summary = f"Skill '{name}' is valid with {warn_count} warning(s)."
+    else:
+        summary = f"Skill '{name}' has {fail_count} issue(s) and {warn_count} warning(s)."
+
+    return {
+        "success": True,
+        "valid": valid,
+        "issues": issues,
+        "warnings": warnings,
+        "summary": summary,
+        "skill_path": str(skill_dir),
+        "skill_md_size": len(content),
+    }
+
+
 def _atomic_write_text(file_path: Path, content: str, encoding: str = "utf-8") -> None:
     """
     Atomically write text content to a file.
@@ -899,7 +1162,7 @@ SKILL_MANAGE_SCHEMA = {
         "Actions: create (full SKILL.md + optional category), validate (check skill with actionable feedback), "
         "patch (old_string/new_string — preferred for fixes), "
         "edit (full SKILL.md rewrite — major overhauls only), "
-        "delete, write_file, remove_file.\n\n"
+        "delete, write_file, remove_file, validate (check skill with actionable feedback).\n\n"
         "Create when: complex task succeeded (5+ calls), errors overcome, "
         "user-corrected approach worked, non-trivial workflow discovered, "
         "or user asks you to remember a procedure.\n"