From 7a0aff117ef5f3067e8fa0b49ec325c95f502bfc Mon Sep 17 00:00:00 2001 From: Jean du Plessis Date: Fri, 9 Jan 2026 17:54:18 +0200 Subject: [PATCH] feat: add extended frontmatter support and validation enhancements Add support for extended skill frontmatter fields: model, context, agent, hooks, and user-invocable. Update validation scripts to handle these properties including hooks structure and forked context agents. Changes: - SKILL.md: Updated to v4.1.0 with expanded frontmatter table (10 properties) - scripts/_constants.py: NEW - shared validation constants - scripts/quick_validate.py: Extended property validation - scripts/validate-skill.py: Hooks and agent validation - scripts/discover_skills.py: Fixed version extraction from metadata.version - references/script-integration-framework.md: Added Hooks Integration section - references/synthesis-protocol.md: Added Forked Context documentation - assets/templates/skill-md-template.md: Updated with modern best practices --- SKILL.md | 155 ++++++++- assets/templates/skill-md-template.md | 14 +- references/script-integration-framework.md | 105 ++++++ references/synthesis-protocol.md | 113 +++++++ scripts/_constants.py | 74 +++++ scripts/discover_skills.py | 87 ++++- scripts/package_skill.py | 38 ++- scripts/quick_validate.py | 135 ++++++-- scripts/validate-skill.py | 370 +++++++++++++++++++-- 9 files changed, 987 insertions(+), 104 deletions(-) create mode 100644 scripts/_constants.py diff --git a/SKILL.md b/SKILL.md index 41ec608..73ff56d 100644 --- a/SKILL.md +++ b/SKILL.md @@ -2,9 +2,20 @@ name: skillforge description: "Intelligent skill router and creator. Analyzes ANY input to recommend existing skills, improve them, or create new ones. Uses deep iterative analysis with 11 thinking models, regression questioning, evolution lens, and multi-agent synthesis panel. Phase 0 triage ensures you never duplicate existing functionality." license: MIT +model: claude-opus-4-5-20251101 +user-invocable: true +allowed-tools: + - Read + - Glob + - Grep + - Write + - Edit + - Bash + - Task + - WebFetch + - WebSearch metadata: - version: 4.0.0 - model: claude-opus-4-5-20251101 + version: 4.1.0 subagent_model: claude-opus-4-5-20251101 domains: [meta-skill, automation, skill-creation, orchestration, agentic, routing] type: orchestrator @@ -12,7 +23,7 @@ metadata: outputs: [SKILL.md, references/, scripts/, SKILL_SPEC.md, recommendations] --- -# SkillForge 4.0 - Intelligent Skill Router & Creator +# SkillForge 4.1 - Intelligent Skill Router & Creator Analyzes ANY input to find, improve, or create the right skill. @@ -257,20 +268,62 @@ Skills must use only these allowed frontmatter properties: | `name` | Yes | Hyphen-case, max 64 chars | | `description` | Yes | Max 1024 chars, no angle brackets | | `license` | No | MIT, Apache-2.0, etc. | -| `allowed-tools` | No | Restrict tool access | -| `metadata` | No | Custom fields (version, model, etc.) | - +| `allowed-tools` | No | Restrict tool access (comma-separated or YAML list) | +| `model` | No | Specific Claude model (e.g., `claude-sonnet-4-20250514`) | +| `context` | No | Set to `fork` for isolated sub-agent context | +| `agent` | No | Agent type when `context: fork` (`Explore`, `Plan`, `general-purpose`) | +| `hooks` | No | Lifecycle hooks (`PreToolUse`, `PostToolUse`, `Stop`) | +| `user-invocable` | No | Show in slash menu (default: `true`) | +| `metadata` | No | Custom fields (version, author, domains, etc.) | + +**Basic Example:** ```yaml --- name: my-skill -description: What this skill does +description: What this skill does and when to use it license: MIT +model: claude-opus-4-5-20251101 +user-invocable: true metadata: version: 1.0.0 - model: claude-opus-4-5-20251101 + author: your-name --- ``` +**Advanced Example (with forked context and hooks):** +```yaml +--- +name: isolated-analyzer +description: Runs analysis in isolated context with validation hooks +license: MIT +model: claude-opus-4-5-20251101 +context: fork +agent: Explore +user-invocable: true +allowed-tools: + - Read + - Glob + - Grep +hooks: + PreToolUse: + - matcher: "Bash" + hooks: + - type: command + command: "./scripts/validate.sh $TOOL_INPUT" +metadata: + version: 1.0.0 +--- +``` + +**Field Details:** + +| Field | Values | Notes | +|-------|--------|-------| +| `context` | `fork` | Creates isolated sub-agent with separate conversation history | +| `agent` | `Explore`, `Plan`, `general-purpose` | Only valid when `context: fork` | +| `user-invocable` | `true`, `false` | `false` hides from slash menu but Claude can still auto-invoke | +| `hooks` | Object | See [Hooks Integration](#hooks-integration-claude-code-v210) section | + --- ## Skill Output Structure @@ -310,6 +363,72 @@ Scripts enable skills to be **agentic** - capable of autonomous operation with s See: [references/script-integration-framework.md](references/script-integration-framework.md) +### Hooks Integration + +Skills can define lifecycle hooks for validation, logging, and safety: + +```yaml +--- +name: secure-skill +hooks: + PreToolUse: + - matcher: "Bash" + hooks: + - type: command + command: "./scripts/validate-input.sh $TOOL_INPUT" + PostToolUse: + - matcher: "Write" + hooks: + - type: command + command: "./scripts/log-output.sh $TOOL_OUTPUT" + once: true + Stop: + - hooks: + - type: command + command: "./scripts/cleanup.sh" +--- +``` + +**Hook Types:** + +| Hook | When Triggered | Use Case | +|------|----------------|----------| +| `PreToolUse` | Before tool execution | Input validation, security checks | +| `PostToolUse` | After tool execution | Output logging, verification | +| `Stop` | When skill completes | Cleanup, state persistence | + +**Hook Configuration:** + +| Field | Description | +|-------|-------------| +| `matcher` | Tool name pattern to match (e.g., "Bash", "Write", "Bash(python:*)") | +| `type` | Hook type: `command` (shell) or `prompt` (Claude evaluation) | +| `command` | Shell command to execute (for `type: command`) | +| `once` | If `true`, run only once per session (default: `false`) | + +**When to Use Hooks:** + +| Scenario | Hook Type | Example | +|----------|-----------|---------| +| Validate script inputs | PreToolUse | Check parameters before `python scripts/*.py` | +| Log generated artifacts | PostToolUse | Record files created by Write tool | +| Security gate | PreToolUse | Block dangerous bash commands | +| Cleanup temp files | Stop | Remove intermediate artifacts | + +**Example: Script Validation Hook** + +For skills with scripts, add input validation: + +```yaml +hooks: + PreToolUse: + - matcher: "Bash(python:scripts/*)" + hooks: + - type: command + command: "python scripts/quick_validate.py . 2>/dev/null || true" + once: true +``` + --- ## Anti-Patterns @@ -838,7 +957,25 @@ SKILLCREATOR_CONFIG: ## Changelog -### v3.2.0 (Current) +### v4.1.0 (Current) +- **Extended frontmatter support** - Full support for `model`, `context`, `agent`, `hooks`, `user-invocable` +- Created `scripts/_constants.py` for shared validation constants +- Updated `scripts/quick_validate.py` with extended property validation +- Updated `scripts/validate-skill.py` with hooks and agent validation +- Fixed `scripts/discover_skills.py` to extract version from `metadata.version` +- Added Hooks Integration section to SKILL.md and script-integration-framework.md +- Added Forked Context documentation to synthesis-protocol.md +- Updated skill template with modern best practices +- Expanded frontmatter requirements table with 10 properties + +### v4.0.0 +- **Phase 0 Skill Triage** - Intelligent routing before creation +- Universal input handling - any prompt works +- Skill ecosystem scanning with 250+ skills indexed +- Decision matrix: USE | IMPROVE | CREATE | COMPOSE +- Renamed from SkillCreator to SkillForge + +### v3.2.0 - Added Script Integration Framework for agentic skills - Added 4th Script Agent to synthesis panel (conditional) - Added Phase 1D: Automation Analysis diff --git a/assets/templates/skill-md-template.md b/assets/templates/skill-md-template.md index a812804..e7d176f 100644 --- a/assets/templates/skill-md-template.md +++ b/assets/templates/skill-md-template.md @@ -1,10 +1,22 @@ --- name: {{SKILL_NAME}} -version: 1.0.0 description: > {{DESCRIPTION}} license: MIT model: claude-opus-4-5-20251101 +user-invocable: true +# Uncomment to restrict available tools: +# allowed-tools: +# - Read +# - Glob +# - Grep +# - Bash +# Uncomment for isolated sub-agent execution: +# context: fork +# agent: general-purpose +metadata: + version: 1.0.0 + author: {{AUTHOR}} --- # {{SKILL_TITLE}} diff --git a/references/script-integration-framework.md b/references/script-integration-framework.md index 268f93c..11a8340 100644 --- a/references/script-integration-framework.md +++ b/references/script-integration-framework.md @@ -431,6 +431,111 @@ Before finalizing a skill with scripts, verify: --- +## Hooks Integration + +Skills can leverage hooks for automatic script invocation during tool use. + +### When to Use Hooks with Scripts + +| Scenario | Hook | Script Pattern | +|----------|------|----------------| +| Validate before generation | PreToolUse on Write | `validation/validate_input.py` | +| Verify after generation | PostToolUse on Write | `validation/verify_output.py` | +| Log all tool activity | PostToolUse (all) | `logging/activity_log.py` | +| Cleanup on completion | Stop | `state/cleanup.py` | + +### Hook + Script Integration Pattern + +**Skill frontmatter:** +```yaml +--- +name: validated-generator +hooks: + PreToolUse: + - matcher: "Bash(python:scripts/generate*)" + hooks: + - type: command + command: "python scripts/validate_params.py $TOOL_INPUT" + PostToolUse: + - matcher: "Write" + hooks: + - type: command + command: "python scripts/verify_artifact.py $TOOL_OUTPUT" +--- +``` + +**Script requirements for hook integration:** +1. Accept input via `$TOOL_INPUT` or `$TOOL_OUTPUT` environment variables +2. Exit code 0 allows tool execution to proceed +3. Exit code non-0 blocks tool execution (PreToolUse) or flags error (PostToolUse) +4. Output to stderr for error messages (stdout may be captured) + +### Hook Script Template + +```python +#!/usr/bin/env python3 +""" +hook_validator.py - Validate tool input before execution + +Called by PreToolUse hook with $TOOL_INPUT containing the tool parameters. +Exit 0 to allow, exit 1 to block. +""" + +import os +import sys +import json + +def validate_input(tool_input: str) -> tuple[bool, str]: + """Validate the tool input. Returns (is_valid, reason).""" + try: + params = json.loads(tool_input) + # Add validation logic here + return True, "Input valid" + except json.JSONDecodeError: + return False, "Invalid JSON input" + +def main(): + tool_input = os.environ.get("TOOL_INPUT", "") + + if not tool_input: + print("Warning: No TOOL_INPUT provided", file=sys.stderr) + sys.exit(0) # Allow by default if no input + + is_valid, reason = validate_input(tool_input) + + if not is_valid: + print(f"Blocked: {reason}", file=sys.stderr) + sys.exit(1) + + sys.exit(0) + +if __name__ == "__main__": + main() +``` + +### Agentic Capability Enhancement + +Hooks enable fully autonomous skill execution: + +``` +WITHOUT HOOKS: + Claude runs script → Script fails → Claude notices → Claude retries + (Multiple tool calls, potential for missed errors) + +WITH HOOKS: + PreToolUse validates → Only valid calls proceed → PostToolUse verifies + (Single tool call, guaranteed validation) +``` + +| Capability | Without Hooks | With Hooks | +|------------|---------------|------------| +| Input validation | Manual check in script | Automatic gate | +| Output verification | Separate tool call | Inline verification | +| Error handling | After-the-fact | Preventive | +| Audit trail | Custom logging | Built-in hook logging | + +--- + ## Anti-Patterns | Avoid | Why | Instead | diff --git a/references/synthesis-protocol.md b/references/synthesis-protocol.md index c38d72c..64f92c2 100644 --- a/references/synthesis-protocol.md +++ b/references/synthesis-protocol.md @@ -422,3 +422,116 @@ Phase 4: Synthesis Panel ──────────┐ │ Registry │ └─────────────┘ ``` + +--- + +## Forked Context for Panel Agents + +Skills can run in isolated forked contexts using `context: fork`. This has implications for the synthesis panel. + +### What is Forked Context? + +```yaml +--- +name: isolated-skill +context: fork +agent: general-purpose +--- +``` + +When `context: fork` is set: +- Skill executes in a separate sub-agent process +- Sub-agent has its own conversation history (starts fresh) +- Parent conversation context is NOT inherited +- Results are returned to parent when sub-agent completes + +### Implications for Multi-Agent Synthesis + +**Current Approach (Shared Context):** +``` +Main Context + │ + ├── Phase 1: Analysis (in main context) + ├── Phase 2: Specification (in main context) + ├── Phase 3: Generation (in main context) + └── Phase 4: Synthesis Panel + ├── Design Agent (Task, shares context) + ├── Audience Agent (Task, shares context) + └── Evolution Agent (Task, shares context) +``` + +Agents share context but run via Task tool with `run_in_background: true`. + +**Alternative Approach (Forked Contexts):** + +For skills that NEED context isolation (e.g., security-sensitive reviews): + +``` +Main Context + │ + └── Phase 4: Synthesis Panel + ├── Design Agent (context: fork) ─── Clean slate + ├── Audience Agent (context: fork) ─── Clean slate + └── Evolution Agent (context: fork) ─── Clean slate +``` + +### When to Use Forked Contexts + +| Scenario | Recommendation | Rationale | +|----------|----------------|-----------| +| Standard skill creation | Shared context | Agents need spec + generated skill | +| Security-sensitive review | Forked context | Prevent context pollution | +| Independent evaluations | Forked context | Each agent judges without bias | +| Iterative refinement | Shared context | Agents need previous feedback | + +### Skill Configuration for Forked Panel Agents + +If creating a skill that spawns forked review agents: + +```markdown +## Phase 4: Synthesis Panel + +Launch review agents in forked contexts: + +\`\`\`yaml +# Agent configuration for forked execution +- name: design-reviewer + context: fork + agent: general-purpose + prompt: | + You are the Design Agent. Review this skill for: + - Architecture pattern appropriateness + - Phase ordering and dependencies + - Verification concreteness + + Skill to review: + {{SKILL_CONTENT}} + + Specification: + {{SPEC_CONTENT}} +\`\`\` + +Note: When using forked context, you MUST include all necessary context +in the prompt since the agent cannot access parent conversation history. +``` + +### Trade-offs + +| Aspect | Shared Context | Forked Context | +|--------|----------------|----------------| +| Context access | Full history | Prompt only | +| Independence | May be biased by prior discussion | Clean evaluation | +| Efficiency | Lower token usage | Higher (must include context) | +| Coordination | Easier | Requires explicit data passing | +| Setup complexity | Minimal | Must include all context in prompt | +| Use case | Iterative workflows | Independent evaluations | + +### SkillForge's Choice + +SkillForge uses **shared context** (no `context: fork`) because: +1. Phase 0 triage results must inform later phases +2. Specification from Phase 2 must be available in Phase 3 +3. Panel agents need both specification AND generated skill +4. Iteration requires feedback from previous rounds + +Skills created BY SkillForge may choose either approach based on their needs. diff --git a/scripts/_constants.py b/scripts/_constants.py new file mode 100644 index 0000000..4313965 --- /dev/null +++ b/scripts/_constants.py @@ -0,0 +1,74 @@ +#!/usr/bin/env python3 +""" +_constants.py - Shared constants for skill validation scripts + +These constants are used by: +- quick_validate.py (packaging validation) +- validate-skill.py (full structural validation) +- discover_skills.py (skill indexing) +""" + +# =========================================================================== +# FRONTMATTER PROPERTIES +# =========================================================================== + +# Required fields +REQUIRED_PROPERTIES = { + 'name', # Skill identifier (hyphen-case, max 64 chars) + 'description', # Discovery text (max 1024 chars, no angle brackets) +} + +# Optional fields +OPTIONAL_PROPERTIES = { + 'license', # Distribution license (MIT, Apache-2.0, etc.) + 'allowed-tools', # Tool restrictions (comma-separated or YAML list) + 'metadata', # Custom fields (version, author, domains, etc.) + 'model', # Specific Claude model (e.g., claude-sonnet-4-20250514) + 'context', # Execution context ('fork' for isolated sub-agent) + 'agent', # Agent type when context: fork + 'hooks', # Lifecycle hooks (PreToolUse, PostToolUse, Stop) + 'user-invocable', # Slash menu visibility (default: true) +} + +# All allowed properties +ALLOWED_PROPERTIES = REQUIRED_PROPERTIES | OPTIONAL_PROPERTIES + +# Recommended but optional fields +RECOMMENDED_PROPERTIES = {'license'} + +# =========================================================================== +# VALIDATION CONSTANTS +# =========================================================================== + +# Valid agent types for context: fork +VALID_AGENT_TYPES = {'Explore', 'Plan', 'general-purpose'} + +# Valid hook event names +VALID_HOOK_EVENTS = {'PreToolUse', 'PostToolUse', 'Stop'} + +# Valid hook types +VALID_HOOK_TYPES = {'command', 'prompt'} + +# Known tool names for allowed-tools validation (warning only, not error) +KNOWN_TOOLS = { + 'Read', 'Glob', 'Grep', 'Write', 'Edit', + 'Bash', 'Task', 'WebFetch', 'WebSearch', + 'TodoWrite', 'NotebookEdit', 'AskUserQuestion' +} + +# Field constraints +NAME_MAX_LENGTH = 64 +DESCRIPTION_MAX_LENGTH = 1024 + +# Skill name regex (unified across all validators) +# - Must start with lowercase letter +# - Can contain lowercase letters, digits, and hyphens +# - Cannot have consecutive hyphens (checked separately) +# - Cannot start or end with hyphen (enforced by regex requiring letter start) +NAME_REGEX = r'^[a-z][a-z0-9-]*[a-z0-9]$|^[a-z]$' # Single char or multi-char ending in alnum + +# Semver regex (supports pre-release versions like 1.0.0-beta.1) +SEMVER_REGEX = r'^\d+\.\d+\.\d+(-[a-zA-Z0-9.]+)?(\+[a-zA-Z0-9.]+)?$' + +# Frontmatter regex (handles both LF and CRLF line endings) +FRONTMATTER_REGEX = r'^---\r?\n(.*?)\r?\n---' diff --git a/scripts/discover_skills.py b/scripts/discover_skills.py index 860920c..dc1997d 100644 --- a/scripts/discover_skills.py +++ b/scripts/discover_skills.py @@ -29,6 +29,15 @@ from pathlib import Path from typing import List, Dict, Optional, Any +# Import shared constants (optional - used for validation if available) +try: + from _constants import SEMVER_REGEX, VALID_AGENT_TYPES + HAS_CONSTANTS = True +except ImportError: + HAS_CONSTANTS = False + SEMVER_REGEX = r'^\d+\.\d+\.\d+(-[a-zA-Z0-9.]+)?(\+[a-zA-Z0-9.]+)?$' + VALID_AGENT_TYPES = {'Explore', 'Plan', 'general-purpose'} + # =========================================================================== # RESULT TYPES @@ -62,7 +71,7 @@ def to_dict(self) -> dict: { "name": "custom", "path": Path.home() / ".claude" / "skills", - "pattern": "*/skill.md", + "pattern": "*/[Ss][Kk][Ii][Ll][Ll].[Mm][Dd]", # Case-insensitive: SKILL.md or skill.md "priority": 1 }, { @@ -140,14 +149,56 @@ def extract_frontmatter(content: str) -> Dict[str, Any]: parts = content.split("---", 2) if len(parts) >= 3: yaml_content = parts[1].strip() - for line in yaml_content.split("\n"): - if ":" in line: - key, value = line.split(":", 1) - frontmatter[key.strip()] = value.strip() + + # Try proper YAML parsing first + try: + import yaml + parsed = yaml.safe_load(yaml_content) + if isinstance(parsed, dict): + frontmatter = parsed + # else: non-dict YAML (e.g., scalar) - keep empty dict + except ImportError: + # PyYAML not installed - use fallback parser below + pass + except Exception: + # Malformed YAML - use fallback parser below + pass + + # Fallback to basic parsing if YAML parsing failed or returned non-dict + if not frontmatter: + current_key = None + for line in yaml_content.split("\n"): + if ":" in line and not line.startswith(" "): + key, value = line.split(":", 1) + current_key = key.strip() + frontmatter[current_key] = value.strip() + elif line.startswith(" ") and current_key == "metadata": + # Basic nested parsing for metadata + if "metadata" not in frontmatter or not isinstance(frontmatter["metadata"], dict): + frontmatter["metadata"] = {} + if ":" in line: + key, value = line.strip().split(":", 1) + frontmatter["metadata"][key.strip()] = value.strip() return frontmatter +def get_version(frontmatter: Dict[str, Any]) -> str: + """Extract version from frontmatter, checking both root and metadata.""" + # First check root level (legacy/deprecated) + if "version" in frontmatter: + return str(frontmatter["version"]) + + # Then check metadata.version (preferred location) + if "metadata" in frontmatter and isinstance(frontmatter["metadata"], dict): + version = frontmatter["metadata"].get("version") + if version: + return str(version) + + # Default fallback + return "1.0.0" + + def extract_triggers(content: str) -> List[str]: """Extract trigger phrases from skill content.""" triggers = [] @@ -166,8 +217,13 @@ def extract_triggers(content: str) -> List[str]: # Also extract from trigger tables table_pattern = r'\|\s*`([^`]+)`\s*\|' - if "Trigger" in content: - table_section = content[content.find("Trigger"):content.find("---", content.find("Trigger"))] + trigger_start = content.find("Trigger") + if trigger_start != -1: + # Guard against find() returning -1 + end_marker = content.find("---", trigger_start) + if end_marker == -1: + end_marker = len(content) # Use end of content if no delimiter found + table_section = content[trigger_start:end_marker] matches = re.findall(table_pattern, table_section) triggers.extend(matches) @@ -252,7 +308,7 @@ def parse_skill_file(path: Path, source_name: str, priority: int) -> Optional[Di "triggers": triggers, "keywords": keywords, "domains": domains, - "version": frontmatter.get("version", "1.0.0") + "version": get_version(frontmatter) } @@ -280,12 +336,12 @@ def discover_skills(verbose: bool = False) -> Result: pattern_parts = source["pattern"].split("/") if len(pattern_parts) == 2: - # Simple pattern like */skill.md + # Simple pattern like */skill.md or */[Ss][Kk][Ii][Ll][Ll].[Mm][Dd] skill_files = list(source_path.glob(source["pattern"])) else: - # Complex pattern - use recursive glob + # Complex pattern - use recursive glob (case-insensitive for skill files) skill_files = list(source_path.glob("**/*.md")) - skill_files = [f for f in skill_files if "skill" in f.name.lower()] + skill_files = [f for f in skill_files if f.name.lower() in ("skill.md", "skills.md")] for skill_file in skill_files: skill_data = parse_skill_file(skill_file, source["name"], source["priority"]) @@ -398,7 +454,14 @@ def main(): for warning in result.warnings: print(f" - {warning}") - sys.exit(0 if result.success else 1) + # Exit codes: 0 = success, 1 = general failure, 3 = no sources found + if not result.success: + sys.exit(1) + elif result.data['total_count'] == 0 and len(result.warnings) == len(SKILL_SOURCES): + # All sources were missing - this is exit code 3 per docstring + sys.exit(3) + else: + sys.exit(0) if __name__ == "__main__": diff --git a/scripts/package_skill.py b/scripts/package_skill.py index dba46b6..5c945bb 100644 --- a/scripts/package_skill.py +++ b/scripts/package_skill.py @@ -15,7 +15,17 @@ import sys import zipfile +from dataclasses import dataclass from pathlib import Path +from typing import Optional + + +@dataclass +class PackageResult: + """Result of packaging a skill.""" + success: bool + message: str + output_path: Optional[Path] = None # Import validation from quick_validate try: @@ -27,7 +37,7 @@ from quick_validate import validate_skill -def package_skill(skill_path, output_dir=None): +def package_skill(skill_path, output_dir=None) -> PackageResult: """ Package a skill folder into a .skill file. @@ -36,32 +46,27 @@ def package_skill(skill_path, output_dir=None): output_dir: Optional output directory for the .skill file (defaults to current directory) Returns: - Path to the created .skill file, or None if error + PackageResult with success status, message, and output path """ skill_path = Path(skill_path).resolve() # Validate skill folder exists if not skill_path.exists(): - print(f"❌ Error: Skill folder not found: {skill_path}") - return None + return PackageResult(False, f"Skill folder not found: {skill_path}") if not skill_path.is_dir(): - print(f"❌ Error: Path is not a directory: {skill_path}") - return None + return PackageResult(False, f"Path is not a directory: {skill_path}") # Validate SKILL.md exists skill_md = skill_path / "SKILL.md" if not skill_md.exists(): - print(f"❌ Error: SKILL.md not found in {skill_path}") - return None + return PackageResult(False, f"SKILL.md not found in {skill_path}") # Run validation before packaging print("🔍 Validating skill...") valid, message = validate_skill(skill_path) if not valid: - print(f"❌ Validation failed: {message}") - print(" Please fix the validation errors before packaging.") - return None + return PackageResult(False, f"Validation failed: {message}") print(f"✅ {message}\n") # Determine output location @@ -88,12 +93,10 @@ def package_skill(skill_path, output_dir=None): zipf.write(file_path, arcname) print(f" Added: {arcname}") - print(f"\n✅ Successfully packaged skill to: {skill_filename}") - return skill_filename + return PackageResult(True, f"Successfully packaged skill to: {skill_filename}", skill_filename) except Exception as e: - print(f"❌ Error creating .skill file: {e}") - return None + return PackageResult(False, f"Error creating .skill file: {e}") def main(): @@ -114,9 +117,12 @@ def main(): result = package_skill(skill_path, output_dir) - if result: + if result.success: + print(f"\n✅ {result.message}") sys.exit(0) else: + print(f"❌ {result.message}") + print(" Please fix the errors before packaging.") sys.exit(1) diff --git a/scripts/quick_validate.py b/scripts/quick_validate.py index db32522..7e6327a 100644 --- a/scripts/quick_validate.py +++ b/scripts/quick_validate.py @@ -11,7 +11,6 @@ """ import sys -import os import re from pathlib import Path @@ -21,6 +20,80 @@ except ImportError: HAS_YAML = False +# Import shared constants +try: + from _constants import ( + ALLOWED_PROPERTIES, REQUIRED_PROPERTIES, + NAME_MAX_LENGTH, DESCRIPTION_MAX_LENGTH, NAME_REGEX + ) +except ImportError: + # Fallback if _constants.py not available + ALLOWED_PROPERTIES = { + 'name', 'description', 'license', 'allowed-tools', 'metadata', + 'model', 'context', 'agent', 'hooks', 'user-invocable' + } + REQUIRED_PROPERTIES = {'name', 'description'} + NAME_MAX_LENGTH = 64 + DESCRIPTION_MAX_LENGTH = 1024 + NAME_REGEX = r'^[a-z][a-z0-9-]*[a-z0-9]$|^[a-z]$' + + +def _parse_frontmatter_fallback(frontmatter_text: str) -> dict: + """Fallback YAML parser for when PyYAML is not available. + + Handles folded (>) and literal (|) scalars for multi-line descriptions. + """ + frontmatter = {} + lines = frontmatter_text.split('\n') + current_key = None + current_value_lines = [] + is_folded = False # Track folded scalar (>) + is_literal = False # Track literal scalar (|) + + for line in lines: + # Check for top-level key + if ':' in line and not line.startswith(' ') and not line.startswith('\t'): + # Save previous key if exists + if current_key and (is_folded or is_literal): + frontmatter[current_key] = ' '.join(current_value_lines).strip() + + key, value = line.split(':', 1) + current_key = key.strip() + value = value.strip() + + # Check for folded (>) or literal (|) scalar + if value == '>' or value == '>-': + is_folded = True + is_literal = False + current_value_lines = [] + elif value == '|' or value == '|-': + is_literal = True + is_folded = False + current_value_lines = [] + else: + is_folded = False + is_literal = False + frontmatter[current_key] = value + current_value_lines = [] + + elif (is_folded or is_literal) and (line.startswith(' ') or line.startswith('\t')): + # Continuation of folded/literal scalar + current_value_lines.append(line.strip()) + + elif line.startswith(' ') and current_key == 'metadata': + # Basic nested parsing for metadata + if 'metadata' not in frontmatter or not isinstance(frontmatter['metadata'], dict): + frontmatter['metadata'] = {} + if ':' in line: + nested_key, nested_value = line.strip().split(':', 1) + frontmatter['metadata'][nested_key.strip()] = nested_value.strip() + + # Save final key if it was a folded/literal scalar + if current_key and (is_folded or is_literal) and current_value_lines: + frontmatter[current_key] = ' '.join(current_value_lines).strip() + + return frontmatter + def validate_skill(skill_path): """ @@ -44,13 +117,13 @@ def validate_skill(skill_path): if not skill_md.exists(): return False, "SKILL.md not found" - # Read and validate frontmatter - content = skill_md.read_text() + # Read and validate frontmatter (explicit UTF-8 encoding) + content = skill_md.read_text(encoding='utf-8') if not content.startswith('---'): return False, "No YAML frontmatter found" - # Extract frontmatter - match = re.match(r'^---\n(.*?)\n---', content, re.DOTALL) + # Extract frontmatter (handles both LF and CRLF line endings) + match = re.match(r'^---\r?\n(.*?)\r?\n---', content, re.DOTALL) if not match: return False, "Invalid frontmatter format" @@ -60,22 +133,17 @@ def validate_skill(skill_path): if HAS_YAML: try: frontmatter = yaml.safe_load(frontmatter_text) - if not isinstance(frontmatter, dict): + if frontmatter is None: + frontmatter = {} + elif not isinstance(frontmatter, dict): return False, "Frontmatter must be a YAML dictionary" except yaml.YAMLError as e: return False, f"Invalid YAML in frontmatter: {e}" else: - # Basic parsing without yaml library - frontmatter = {} - for line in frontmatter_text.split('\n'): - if ':' in line and not line.startswith(' '): - key, value = line.split(':', 1) - frontmatter[key.strip()] = value.strip() - - # Define allowed properties - ALLOWED_PROPERTIES = {'name', 'description', 'license', 'allowed-tools', 'metadata'} + # Basic parsing without yaml library (handles folded scalars) + frontmatter = _parse_frontmatter_fallback(frontmatter_text) - # Check for unexpected properties (excluding nested keys under metadata) + # Check for unexpected properties unexpected_keys = set(frontmatter.keys()) - ALLOWED_PROPERTIES if unexpected_keys: return False, ( @@ -84,27 +152,26 @@ def validate_skill(skill_path): ) # Check required fields - if 'name' not in frontmatter: - return False, "Missing 'name' in frontmatter" - if 'description' not in frontmatter: - return False, "Missing 'description' in frontmatter" + for field in REQUIRED_PROPERTIES: + if field not in frontmatter: + return False, f"Missing '{field}' in frontmatter" - # Extract name for validation + # Validate name field name = frontmatter.get('name', '') if not isinstance(name, str): return False, f"Name must be a string, got {type(name).__name__}" name = name.strip() if name: - # Check naming convention (hyphen-case: lowercase with hyphens) - if not re.match(r'^[a-z0-9-]+$', name): - return False, f"Name '{name}' should be hyphen-case (lowercase letters, digits, and hyphens only)" - if name.startswith('-') or name.endswith('-') or '--' in name: - return False, f"Name '{name}' cannot start/end with hyphen or contain consecutive hyphens" - # Check name length (max 64 characters per spec) - if len(name) > 64: - return False, f"Name is too long ({len(name)} characters). Maximum is 64 characters." - - # Extract and validate description + # Check naming convention (hyphen-case: starts with letter, lowercase with hyphens) + if not re.match(NAME_REGEX, name): + return False, f"Name '{name}' should be hyphen-case (start with letter, lowercase letters, digits, and hyphens only)" + if '--' in name: + return False, f"Name '{name}' cannot contain consecutive hyphens" + # Check name length + if len(name) > NAME_MAX_LENGTH: + return False, f"Name is too long ({len(name)} characters). Maximum is {NAME_MAX_LENGTH} characters." + + # Validate description field description = frontmatter.get('description', '') if not isinstance(description, str): return False, f"Description must be a string, got {type(description).__name__}" @@ -113,9 +180,9 @@ def validate_skill(skill_path): # Check for angle brackets if '<' in description or '>' in description: return False, "Description cannot contain angle brackets (< or >)" - # Check description length (max 1024 characters per spec) - if len(description) > 1024: - return False, f"Description is too long ({len(description)} characters). Maximum is 1024 characters." + # Check description length + if len(description) > DESCRIPTION_MAX_LENGTH: + return False, f"Description is too long ({len(description)} characters). Maximum is {DESCRIPTION_MAX_LENGTH} characters." return True, "Skill is valid!" diff --git a/scripts/validate-skill.py b/scripts/validate-skill.py index 0f8feed..d1d3823 100644 --- a/scripts/validate-skill.py +++ b/scripts/validate-skill.py @@ -3,7 +3,7 @@ validate-skill.py - Structural validation for Claude Code skills Validates that a SKILL.md file meets the requirements defined in -SkillForge 4.0's quality standards. +SkillForge 4.1's quality standards. Usage: python validate-skill.py @@ -12,13 +12,12 @@ import sys import re -import os from pathlib import Path from typing import List, Tuple, Dict, Any class SkillValidator: - """Validates skill files against SkillForge 4.0 standards.""" + """Validates skill files against SkillForge 4.1 standards.""" def __init__(self, skill_path: str): self.skill_path = Path(skill_path) @@ -53,27 +52,84 @@ def load_skill(self) -> bool: def parse_frontmatter(self) -> bool: """Parse YAML frontmatter from skill file.""" - match = re.match(r'^---\n(.*?)\n---', self.content, re.DOTALL) + # Handle both LF and CRLF line endings + match = re.match(r'^---\r?\n(.*?)\r?\n---', self.content, re.DOTALL) if not match: self.errors.append("Missing YAML frontmatter") return False + frontmatter_text = match.group(1) + try: import yaml - self.frontmatter = yaml.safe_load(match.group(1)) + parsed = yaml.safe_load(frontmatter_text) + # Guard against None or non-dict returns from yaml.safe_load + if parsed is None: + self.frontmatter = {} + elif not isinstance(parsed, dict): + self.errors.append(f"Frontmatter must be a YAML dictionary, got {type(parsed).__name__}") + return False + else: + self.frontmatter = parsed return True except ImportError: - # Parse basic fields without yaml library - frontmatter_text = match.group(1) - for line in frontmatter_text.split('\n'): - if ':' in line: - key, value = line.split(':', 1) - self.frontmatter[key.strip()] = value.strip() + # Parse basic fields without yaml library (handles folded scalars) + self._parse_frontmatter_fallback(frontmatter_text) return True except Exception as e: self.errors.append(f"Failed to parse frontmatter: {e}") return False + def _parse_frontmatter_fallback(self, frontmatter_text: str) -> None: + """Fallback YAML parser for when PyYAML is not available.""" + lines = frontmatter_text.split('\n') + current_key = None + current_value_lines = [] + is_folded = False # Track folded scalar (>) + is_literal = False # Track literal scalar (|) + + for line in lines: + # Check for top-level key + if ':' in line and not line.startswith(' ') and not line.startswith('\t'): + # Save previous key if exists + if current_key and (is_folded or is_literal): + self.frontmatter[current_key] = ' '.join(current_value_lines).strip() + + key, value = line.split(':', 1) + current_key = key.strip() + value = value.strip() + + # Check for folded (>) or literal (|) scalar + if value == '>' or value == '>-': + is_folded = True + is_literal = False + current_value_lines = [] + elif value == '|' or value == '|-': + is_literal = True + is_folded = False + current_value_lines = [] + else: + is_folded = False + is_literal = False + self.frontmatter[current_key] = value + current_value_lines = [] + + elif (is_folded or is_literal) and (line.startswith(' ') or line.startswith('\t')): + # Continuation of folded/literal scalar + current_value_lines.append(line.strip()) + + elif line.startswith(' ') and current_key == 'metadata': + # Basic nested parsing for metadata + if 'metadata' not in self.frontmatter or not isinstance(self.frontmatter['metadata'], dict): + self.frontmatter['metadata'] = {} + if ':' in line: + nested_key, nested_value = line.strip().split(':', 1) + self.frontmatter['metadata'][nested_key.strip()] = nested_value.strip() + + # Save final key if it was a folded/literal scalar + if current_key and (is_folded or is_literal) and current_value_lines: + self.frontmatter[current_key] = ' '.join(current_value_lines).strip() + def check(self, name: str, condition: bool, error_msg: str = None, warning: bool = False): """Run a check and record result.""" self.checks_total += 1 @@ -89,44 +145,286 @@ def check(self, name: str, condition: bool, error_msg: str = None, warning: bool def validate_frontmatter(self): """Validate frontmatter fields.""" - required_fields = ["name", "version", "description", "license", "model"] - - for field in required_fields: + # Import or define constants + try: + from _constants import ( + ALLOWED_PROPERTIES, REQUIRED_PROPERTIES, RECOMMENDED_PROPERTIES, + VALID_AGENT_TYPES, NAME_MAX_LENGTH, DESCRIPTION_MAX_LENGTH, + SEMVER_REGEX, NAME_REGEX + ) + except ImportError: + ALLOWED_PROPERTIES = { + 'name', 'description', 'license', 'allowed-tools', 'metadata', + 'model', 'context', 'agent', 'hooks', 'user-invocable' + } + REQUIRED_PROPERTIES = {'name', 'description'} + RECOMMENDED_PROPERTIES = {'license'} + VALID_AGENT_TYPES = {'Explore', 'Plan', 'general-purpose'} + NAME_MAX_LENGTH = 64 + DESCRIPTION_MAX_LENGTH = 1024 + SEMVER_REGEX = r'^\d+\.\d+\.\d+(-[a-zA-Z0-9.]+)?(\+[a-zA-Z0-9.]+)?$' + NAME_REGEX = r'^[a-z][a-z0-9-]*[a-z0-9]$|^[a-z]$' + + # Check required fields + for field in REQUIRED_PROPERTIES: self.check( f"frontmatter.{field}", field in self.frontmatter and self.frontmatter[field], f"Missing required frontmatter field: {field}" ) - # Check name format (kebab-case) + # Warn about recommended fields (not required) + for field in RECOMMENDED_PROPERTIES: + self.check( + f"frontmatter.{field}", + field in self.frontmatter, + f"Recommended frontmatter field missing: {field}", + warning=True + ) + + # Validate allowed properties + unexpected_keys = set(self.frontmatter.keys()) - ALLOWED_PROPERTIES + if unexpected_keys: + self.check( + "frontmatter.allowed_properties", + False, + f"Unexpected frontmatter properties: {', '.join(sorted(unexpected_keys))}. " + f"Allowed: {', '.join(sorted(ALLOWED_PROPERTIES))}" + ) + + # Validate name field (format and length) if "name" in self.frontmatter: - name = self.frontmatter["name"] + name = str(self.frontmatter["name"]) self.check( "frontmatter.name.format", - re.match(r'^[a-z][a-z0-9-]*$', str(name)), - f"Skill name should be kebab-case: {name}" + re.match(NAME_REGEX, name) and '--' not in name, + f"Skill name should be hyphen-case (start with letter, no consecutive hyphens): {name}" ) - - # Check version format (semver) - if "version" in self.frontmatter: - version = self.frontmatter["version"] self.check( - "frontmatter.version.format", - re.match(r'^\d+\.\d+\.\d+', str(version)), - f"Version should be semver format: {version}" + "frontmatter.name.length", + len(name) <= NAME_MAX_LENGTH, + f"Skill name too long ({len(name)} chars, max {NAME_MAX_LENGTH})" ) - # Check description length + # Validate description field (no angle brackets, length limit) if "description" in self.frontmatter: desc = str(self.frontmatter["description"]) - word_count = len(desc.split()) + self.check( + "frontmatter.description.characters", + '<' not in desc and '>' not in desc, + "Description cannot contain angle brackets (< or >)" + ) self.check( "frontmatter.description.length", - word_count >= 10, - f"Description too short ({word_count} words, minimum 10)", + len(desc) <= DESCRIPTION_MAX_LENGTH, + f"Description too long ({len(desc)} chars, max {DESCRIPTION_MAX_LENGTH})", + warning=True + ) + + # Check version location (should be in metadata, not root) + if "version" in self.frontmatter: + self.check( + "frontmatter.version.location", + False, + "'version' should be under 'metadata', not at root level. " + "Move to metadata.version for better organization.", + warning=True + ) + + # Validate version format if in metadata + if "metadata" in self.frontmatter and isinstance(self.frontmatter["metadata"], dict): + version = self.frontmatter["metadata"].get("version") + if version: + self.check( + "metadata.version.format", + re.match(SEMVER_REGEX, str(version)), + f"Version should be semver format (e.g., 1.0.0 or 1.0.0-beta.1): {version}", + warning=True + ) + + # Validate context field - warning not error for future-proofing + if "context" in self.frontmatter: + context = self.frontmatter["context"] + self.check( + "frontmatter.context.value", + context == "fork", + f"context should be 'fork' (got '{context}'). Other values may be added in future.", warning=True ) + # Validate agent field + if "agent" in self.frontmatter: + agent = self.frontmatter["agent"] + self.check( + "frontmatter.agent.value", + agent in VALID_AGENT_TYPES, + f"agent should be one of {VALID_AGENT_TYPES} (got '{agent}')", + warning=True + ) + # Agent requires context: fork + if "context" not in self.frontmatter or self.frontmatter.get("context") != "fork": + self.check( + "frontmatter.agent.requires_context", + False, + "'agent' field requires 'context: fork' to be set", + warning=True + ) + + # Validate user-invocable field + if "user-invocable" in self.frontmatter: + value = self.frontmatter["user-invocable"] + self.check( + "frontmatter.user-invocable.type", + isinstance(value, bool), + f"user-invocable must be a boolean (got {type(value).__name__})" + ) + + # Validate allowed-tools field + self.validate_allowed_tools() + + # Validate hooks field + self.validate_hooks() + + def validate_allowed_tools(self): + """Validate allowed-tools field if present.""" + if "allowed-tools" not in self.frontmatter: + return + + tools_value = self.frontmatter["allowed-tools"] + + # Parse as list or comma-separated string + if isinstance(tools_value, str): + tools = [t.strip() for t in tools_value.split(",")] + elif isinstance(tools_value, list): + tools = [str(t) for t in tools_value] + else: + self.check( + "frontmatter.allowed-tools.type", + False, + f"allowed-tools should be string or list (got {type(tools_value).__name__})" + ) + return + + # Import known tools + try: + from _constants import KNOWN_TOOLS + except ImportError: + KNOWN_TOOLS = { + 'Read', 'Glob', 'Grep', 'Write', 'Edit', + 'Bash', 'Task', 'WebFetch', 'WebSearch', + 'TodoWrite', 'NotebookEdit', 'AskUserQuestion' + } + + # Check for unknown tools (warning only - custom tools may exist) + unknown_tools = [t for t in tools if t not in KNOWN_TOOLS] + if unknown_tools: + self.check( + "frontmatter.allowed-tools.values", + False, + f"Unknown tool(s): {unknown_tools}. Known tools: {sorted(KNOWN_TOOLS)}. " + "This may be intentional for custom tools.", + warning=True + ) + + def validate_hooks(self): + """Validate hooks object structure if present.""" + if "hooks" not in self.frontmatter: + return + + hooks = self.frontmatter["hooks"] + + # Hooks must be a dictionary + if not isinstance(hooks, dict): + self.check( + "frontmatter.hooks.type", + False, + f"hooks must be an object/dictionary (got {type(hooks).__name__})" + ) + return + + # Import valid hook events + try: + from _constants import VALID_HOOK_EVENTS, VALID_HOOK_TYPES + except ImportError: + VALID_HOOK_EVENTS = {'PreToolUse', 'PostToolUse', 'Stop'} + VALID_HOOK_TYPES = {'command', 'prompt'} + + # Validate each hook event + for hook_name, hook_config in hooks.items(): + # Check hook name is valid + self.check( + f"frontmatter.hooks.{hook_name}.name", + hook_name in VALID_HOOK_EVENTS, + f"Unknown hook event: '{hook_name}'. Valid events: {VALID_HOOK_EVENTS}" + ) + + # Hook config should be a list + if not isinstance(hook_config, list): + self.check( + f"frontmatter.hooks.{hook_name}.type", + False, + f"Hook '{hook_name}' config should be a list of matchers" + ) + continue + + # Validate each matcher in the hook + for i, matcher_config in enumerate(hook_config): + if not isinstance(matcher_config, dict): + self.check( + f"frontmatter.hooks.{hook_name}[{i}].type", + False, + f"Hook matcher should be an object with 'matcher' and 'hooks' keys" + ) + continue + + # PreToolUse and PostToolUse require matcher field + if hook_name in {'PreToolUse', 'PostToolUse'}: + self.check( + f"frontmatter.hooks.{hook_name}[{i}].matcher", + "matcher" in matcher_config, + f"'{hook_name}' hook requires 'matcher' field", + warning=True + ) + + # Validate inner hooks array + inner_hooks = matcher_config.get("hooks", []) + if not isinstance(inner_hooks, list): + self.check( + f"frontmatter.hooks.{hook_name}[{i}].hooks.type", + False, + "Inner 'hooks' should be a list" + ) + continue + + for j, inner_hook in enumerate(inner_hooks): + if not isinstance(inner_hook, dict): + continue + + # Validate hook type + hook_type = inner_hook.get("type") + if hook_type: + self.check( + f"frontmatter.hooks.{hook_name}[{i}].hooks[{j}].type", + hook_type in VALID_HOOK_TYPES, + f"Hook type should be one of {VALID_HOOK_TYPES} (got '{hook_type}')" + ) + + # command type requires command field + if hook_type == "command": + self.check( + f"frontmatter.hooks.{hook_name}[{i}].hooks[{j}].command", + "command" in inner_hook and inner_hook["command"], + "Hook with type 'command' requires non-empty 'command' field" + ) + + # Validate 'once' field if present + if "once" in inner_hook: + self.check( + f"frontmatter.hooks.{hook_name}[{i}].hooks[{j}].once", + isinstance(inner_hook["once"], bool), + "'once' field must be a boolean" + ) + def validate_triggers(self): """Validate trigger phrases section.""" # Find triggers section @@ -306,7 +604,11 @@ def _validate_script(self, script_path: Path): script_name = script_path.name - # Check for shebang and docstring + # Skip private modules (starting with _) for CLI-related checks + # They are helper/config modules, not runnable scripts + is_private_module = script_name.startswith('_') + + # Check for shebang and docstring (applies to all Python files) has_shebang = content.strip().startswith('#!/usr/bin/env python3') has_docstring = '"""' in content[:500] or "'''" in content[:500] self.check( @@ -316,6 +618,10 @@ def _validate_script(self, script_path: Path): warning=True ) + # Skip CLI-related checks for private modules + if is_private_module: + return + # Check for argparse usage (if main function exists) has_main = "def main():" in content or 'if __name__' in content has_argparse = "argparse" in content or "sys.argv" in content @@ -346,11 +652,11 @@ def _validate_script(self, script_path: Path): ) # Check for result class or validation result pattern + # Matches: Result, ValidationResult, return (True/False, or return True/False, has_result_pattern = ( "Result" in content or "ValidationResult" in content or - "return (True" in content or - "return (False" in content + re.search(r'return\s*\(?\s*(True|False)\s*,', content) is not None ) self.check( f"script.{script_name}.result_pattern",