feat: add pr rule & skill
This commit is contained in:
62
.claude/skills/pr-review/SKILL.md
Normal file
62
.claude/skills/pr-review/SKILL.md
Normal file
@@ -0,0 +1,62 @@
|
||||
---
|
||||
name: pr-review
|
||||
description: >
|
||||
Review pull requests for the MiniMax Skills repository. Use when reviewing PRs,
|
||||
validating new skill submissions, or checking existing skills for compliance.
|
||||
Run the validation script first for hard checks, then apply quality guidelines
|
||||
for content review. Triggers: PR review, pull request, validate skill, check skill.
|
||||
license: MIT
|
||||
metadata:
|
||||
version: "1.0"
|
||||
category: tooling
|
||||
---
|
||||
|
||||
# PR Review Skill
|
||||
|
||||
Review pull requests against repository standards. Two-phase process: automated validation, then manual content review.
|
||||
|
||||
## Phase 1: Automated Validation (Hard Rules)
|
||||
|
||||
Run the validation script to check structural requirements:
|
||||
|
||||
```bash
|
||||
python .claude/skills/pr-review/scripts/validate_skills.py
|
||||
```
|
||||
|
||||
The script checks:
|
||||
- `SKILL.md` exists in every skill directory
|
||||
- YAML frontmatter is parseable
|
||||
- Required fields present: `name`, `description`
|
||||
- `name` matches directory name
|
||||
- No hardcoded secrets detected
|
||||
|
||||
All ERROR-level checks must pass. WARNING-level items (missing `license`, `metadata`) should be flagged but are not blockers.
|
||||
|
||||
See [references/structure-rules.md](references/structure-rules.md) for the complete hard rules specification.
|
||||
|
||||
## Phase 2: Content Review (Soft Guidelines)
|
||||
|
||||
After automated checks pass, review the PR against quality guidelines:
|
||||
|
||||
1. **Skill scope** — Does it overlap with existing skills? Is the boundary clear?
|
||||
2. **Description quality** — Does the `description` include clear trigger conditions?
|
||||
3. **File size** — Are reference docs reasonably sized for context window consumption?
|
||||
4. **API key handling** — If external APIs are used, are credentials read from environment variables?
|
||||
5. **Script quality** — Do scripts have shebang, requirements.txt, and error handling?
|
||||
6. **README sync** — Are `README.md` and `README_zh.md` updated for new skills?
|
||||
|
||||
See [references/quality-guidelines.md](references/quality-guidelines.md) for soft guidelines details.
|
||||
|
||||
## Review Checklist Summary
|
||||
|
||||
### Must Pass (Blockers)
|
||||
- [ ] `validate_skills.py` exits with code 0
|
||||
- [ ] PR title follows conventional commit format
|
||||
- [ ] One PR, one purpose
|
||||
|
||||
### Should Pass (Flagged in Review)
|
||||
- [ ] No functional overlap with existing skills
|
||||
- [ ] Description includes trigger conditions
|
||||
- [ ] Files are reasonably sized
|
||||
- [ ] API keys via environment variables
|
||||
- [ ] README tables updated for new skills (Source column set to `Community`)
|
||||
54
.claude/skills/pr-review/references/quality-guidelines.md
Normal file
54
.claude/skills/pr-review/references/quality-guidelines.md
Normal file
@@ -0,0 +1,54 @@
|
||||
# Quality Guidelines (Soft Review)
|
||||
|
||||
These guidelines are not enforced by automated tooling. Reviewers should check these during manual PR review and flag violations as suggestions.
|
||||
|
||||
## 1. Skill Scope — Avoid Overlap
|
||||
|
||||
Before approving a new skill, check existing skills for functional overlap.
|
||||
|
||||
- If the new skill's capability is a subset of an existing skill, suggest extending the existing one instead
|
||||
- If there is partial overlap, the PR description must clearly explain the boundary
|
||||
- Example: a voice synthesis skill should clarify how it differs from `frontend-dev`'s TTS capabilities
|
||||
|
||||
## 2. Description Quality
|
||||
|
||||
The `description` field in SKILL.md is what the agent uses to decide whether to activate the skill. A good description must include:
|
||||
|
||||
- What the skill does
|
||||
- When to use it (trigger conditions)
|
||||
- Keywords or phrases that should activate it
|
||||
|
||||
Bad: `"A skill for making PDFs"`
|
||||
Good: `"Generate, fill, and reformat PDF documents. Use when the user asks to create, modify, or design any PDF file. Triggers: PDF, .pdf, document generation."`
|
||||
|
||||
## 3. File Size Awareness
|
||||
|
||||
Skills are loaded into the agent's context window. Every token counts.
|
||||
|
||||
- Individual `.md` files should stay focused and concise
|
||||
- If a reference document exceeds ~500 lines, consider splitting it into parts
|
||||
- Do not embed large data blobs (base64 images, full API responses) in Markdown
|
||||
- Prefer linking to external resources over inlining lengthy content
|
||||
|
||||
## 4. Credential Handling
|
||||
|
||||
The validation script only blocks high-confidence secret patterns (OpenAI keys, AWS keys, JWT tokens). Reviewers should additionally check for:
|
||||
|
||||
- API keys or passwords assigned directly in code (e.g., `api_key = "abc123..."`)
|
||||
- Credentials passed as plain string arguments instead of environment variable reads
|
||||
- Example keys that look realistic enough to be mistaken for real ones
|
||||
- Scripts that lack a clear error message when a required env var is missing
|
||||
|
||||
If a skill involves external APIs, verify that SKILL.md documents the required environment variables.
|
||||
|
||||
## 5. Script Quality
|
||||
|
||||
If the skill includes helper scripts in `scripts/`:
|
||||
|
||||
- Scripts should have a shebang line (`#!/usr/bin/env python3`)
|
||||
- A `requirements.txt` should be present listing all dependencies if external libraries are needed.
|
||||
- Errors should produce clear messages, not raw tracebacks
|
||||
|
||||
## 6. README Sync
|
||||
|
||||
When a new skill is added, both `README.md` and `README_zh.md` should be updated with the new skill in the table. Community-submitted skills should set the Source column to `Community`.
|
||||
53
.claude/skills/pr-review/references/structure-rules.md
Normal file
53
.claude/skills/pr-review/references/structure-rules.md
Normal file
@@ -0,0 +1,53 @@
|
||||
# Structure Rules (Hard Validation)
|
||||
|
||||
These rules are enforced by `scripts/validate_skills.py`. PRs that violate ERROR-level rules will not be merged.
|
||||
|
||||
## Directory Structure
|
||||
|
||||
Every skill must follow this layout:
|
||||
|
||||
```
|
||||
skills/<skill-name>/
|
||||
├── SKILL.md # Required
|
||||
├── references/ # Optional
|
||||
│ └── *.md
|
||||
└── scripts/ # Optional
|
||||
├── *.py
|
||||
└── requirements.txt # Required if scripts/ exists
|
||||
```
|
||||
|
||||
- Directory name must be lowercase `kebab-case` (e.g., `gif-sticker-maker`)
|
||||
- `SKILL.md` is the only required file
|
||||
|
||||
## SKILL.md Frontmatter
|
||||
|
||||
The file must begin with a valid YAML frontmatter block enclosed by `---` markers.
|
||||
|
||||
### Required Fields (ERROR if missing)
|
||||
|
||||
| Field | Rule |
|
||||
|-------|------|
|
||||
| `name` | Must exist and exactly match the directory name |
|
||||
| `description` | Must exist and be non-empty |
|
||||
|
||||
### Recommended Fields (WARNING if missing)
|
||||
|
||||
| Field | Rule |
|
||||
|-------|------|
|
||||
| `license` | Should be `MIT` or a license declaration |
|
||||
| `metadata` | Should include `version`, `category`, and optionally `sources` |
|
||||
|
||||
## Secret Scanning
|
||||
|
||||
No file in the skill directory may contain hardcoded secrets. The following high-confidence patterns are scanned:
|
||||
|
||||
- OpenAI-style API keys: `sk-` followed by 20+ alphanumeric characters
|
||||
- AWS Access Key IDs: `AKIA` followed by 16 uppercase alphanumeric characters
|
||||
- Hardcoded Bearer tokens: `Bearer` followed by 50+ characters (typical JWT length)
|
||||
|
||||
Other forms of hardcoded credentials (API key assignments, passwords, etc.) are not automatically blocked but should be flagged during manual review.
|
||||
|
||||
## Validation Severity Levels
|
||||
|
||||
- **ERROR** — PR must not be merged. Must be fixed before approval.
|
||||
- **WARN** — Reviewer should flag. Not a merge blocker but should be addressed.
|
||||
214
.claude/skills/pr-review/scripts/validate_skills.py
Normal file
214
.claude/skills/pr-review/scripts/validate_skills.py
Normal file
@@ -0,0 +1,214 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Validate skill directory structure and SKILL.md frontmatter.
|
||||
|
||||
Zero external dependencies — uses only Python standard library.
|
||||
Exit code 0: all checks passed (warnings are OK).
|
||||
Exit code 1: at least one ERROR found.
|
||||
|
||||
Usage:
|
||||
python validate_skills.py # scan default path (skills/)
|
||||
python validate_skills.py --path some/dir # scan specific directory
|
||||
"""
|
||||
|
||||
import argparse
|
||||
import os
|
||||
import re
|
||||
import sys
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Minimal frontmatter parser
|
||||
# ---------------------------------------------------------------------------
|
||||
def extract_frontmatter(text):
|
||||
"""Extract YAML frontmatter string between --- markers. Returns None if not found."""
|
||||
stripped = text.lstrip("\ufeff")
|
||||
if not stripped.startswith("---"):
|
||||
return None
|
||||
end = stripped.find("---", 3)
|
||||
if end == -1:
|
||||
return None
|
||||
return stripped[3:end]
|
||||
|
||||
|
||||
def parse_frontmatter_fields(fm_text):
|
||||
"""Parse top-level scalar fields from frontmatter text.
|
||||
|
||||
Returns dict of {field_name: value_string}. Nested keys under a mapping
|
||||
are ignored — we only need top-level presence checks.
|
||||
"""
|
||||
fields = {}
|
||||
lines = fm_text.splitlines()
|
||||
i = 0
|
||||
while i < len(lines):
|
||||
line = lines[i]
|
||||
if not line.strip() or line.strip().startswith("#"):
|
||||
i += 1
|
||||
continue
|
||||
m = re.match(r"^([a-zA-Z_][a-zA-Z0-9_]*)\s*:\s*(.*)", line)
|
||||
if m:
|
||||
key = m.group(1)
|
||||
rest = m.group(2).strip()
|
||||
if rest in ("|", ">", "|+", "|-", ">+", ">-"):
|
||||
block_lines = []
|
||||
i += 1
|
||||
while i < len(lines) and (lines[i].startswith(" ") or lines[i].startswith("\t") or lines[i].strip() == ""):
|
||||
block_lines.append(lines[i])
|
||||
i += 1
|
||||
fields[key] = "\n".join(block_lines).strip()
|
||||
continue
|
||||
elif rest == "":
|
||||
block_lines = []
|
||||
i += 1
|
||||
while i < len(lines) and (lines[i].startswith(" ") or lines[i].startswith("\t")):
|
||||
block_lines.append(lines[i])
|
||||
i += 1
|
||||
fields[key] = "\n".join(block_lines).strip() if block_lines else ""
|
||||
continue
|
||||
else:
|
||||
fields[key] = rest.strip("\"'")
|
||||
i += 1
|
||||
return fields
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Secret scanning
|
||||
# ---------------------------------------------------------------------------
|
||||
SECRET_PATTERNS = [
|
||||
(r"sk-[a-zA-Z0-9]{20,}", "OpenAI-style API key"),
|
||||
(r"AKIA[0-9A-Z]{16}", "AWS access key"),
|
||||
(r"Bearer\s+[a-zA-Z0-9_\-\.]{50,}", "Hardcoded bearer token"),
|
||||
]
|
||||
|
||||
SCAN_EXTENSIONS = {".md", ".py", ".sh", ".js", ".ts", ".json", ".yaml", ".yml", ".txt", ".toml", ".cfg", ".ini"}
|
||||
|
||||
def scan_secrets(filepath):
|
||||
"""Scan a file for hardcoded secrets. Returns list of (line_no, pattern_desc, matched_text)."""
|
||||
try:
|
||||
with open(filepath, "r", encoding="utf-8", errors="ignore") as f:
|
||||
content = f.read()
|
||||
except Exception:
|
||||
return []
|
||||
|
||||
findings = []
|
||||
for line_no, line in enumerate(content.splitlines(), 1):
|
||||
for pattern, desc in SECRET_PATTERNS:
|
||||
for match in re.finditer(pattern, line):
|
||||
findings.append((line_no, desc, match.group(0)[:60]))
|
||||
return findings
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Skill discovery and validation
|
||||
# ---------------------------------------------------------------------------
|
||||
def find_skill_dirs(base_path):
|
||||
"""Find directories that contain a SKILL.md."""
|
||||
skill_dirs = []
|
||||
for root, dirs, files in os.walk(base_path):
|
||||
dirs[:] = [d for d in dirs if not d.startswith(".")]
|
||||
if "SKILL.md" in files:
|
||||
skill_dirs.append(root)
|
||||
return sorted(skill_dirs)
|
||||
|
||||
|
||||
def validate_skill(skill_dir):
|
||||
"""Validate a single skill directory. Returns (errors, warnings) lists."""
|
||||
errors = []
|
||||
warnings = []
|
||||
dir_name = os.path.basename(skill_dir)
|
||||
skill_md = os.path.join(skill_dir, "SKILL.md")
|
||||
|
||||
if not os.path.isfile(skill_md):
|
||||
errors.append("SKILL.md not found")
|
||||
return errors, warnings
|
||||
|
||||
with open(skill_md, "r", encoding="utf-8", errors="ignore") as f:
|
||||
content = f.read()
|
||||
|
||||
fm_text = extract_frontmatter(content)
|
||||
if fm_text is None:
|
||||
errors.append("SKILL.md has no valid YAML frontmatter (missing --- markers)")
|
||||
return errors, warnings
|
||||
|
||||
fields = parse_frontmatter_fields(fm_text)
|
||||
|
||||
name = fields.get("name", "").strip()
|
||||
if not name:
|
||||
errors.append("Missing required field: name")
|
||||
elif name != dir_name:
|
||||
errors.append(f"name '{name}' does not match directory name '{dir_name}'")
|
||||
|
||||
desc = fields.get("description", "").strip()
|
||||
if not desc:
|
||||
errors.append("Missing required field: description")
|
||||
|
||||
if "license" not in fields or not fields["license"].strip():
|
||||
warnings.append("Missing recommended field: license")
|
||||
|
||||
if "metadata" not in fields or not fields["metadata"].strip():
|
||||
warnings.append("Missing recommended field: metadata")
|
||||
|
||||
for root, dirs, files in os.walk(skill_dir):
|
||||
dirs[:] = [d for d in dirs if not d.startswith(".")]
|
||||
for fname in files:
|
||||
_, ext = os.path.splitext(fname)
|
||||
if ext not in SCAN_EXTENSIONS:
|
||||
continue
|
||||
fpath = os.path.join(root, fname)
|
||||
for line_no, sdesc, matched in scan_secrets(fpath):
|
||||
rel = os.path.relpath(fpath, skill_dir)
|
||||
errors.append(f"Potential secret in {rel}:{line_no} ({sdesc}): {matched}...")
|
||||
|
||||
return errors, warnings
|
||||
|
||||
|
||||
def main():
|
||||
parser = argparse.ArgumentParser(description="Validate MiniMax Skills structure")
|
||||
parser.add_argument("--path", default="skills", help="Directory to scan (default: skills/)")
|
||||
args = parser.parse_args()
|
||||
|
||||
scan_path = os.path.abspath(args.path)
|
||||
|
||||
skill_dirs = find_skill_dirs(scan_path)
|
||||
if not skill_dirs:
|
||||
print("No skill directories found.")
|
||||
sys.exit(0)
|
||||
|
||||
print(f"\nValidating {len(skill_dirs)} skill(s)...\n")
|
||||
|
||||
total_errors = 0
|
||||
total_warnings = 0
|
||||
|
||||
for sd in skill_dirs:
|
||||
rel = os.path.relpath(sd)
|
||||
errors, warnings = validate_skill(sd)
|
||||
|
||||
if errors:
|
||||
status = "FAIL"
|
||||
elif warnings:
|
||||
status = "WARN"
|
||||
else:
|
||||
status = "PASS"
|
||||
|
||||
print(f" [{status}] {rel}")
|
||||
for msg in errors:
|
||||
print(f" ERROR {msg}")
|
||||
for msg in warnings:
|
||||
print(f" WARN {msg}")
|
||||
|
||||
total_errors += len(errors)
|
||||
total_warnings += len(warnings)
|
||||
|
||||
print()
|
||||
if total_errors:
|
||||
print(f" {total_errors} error(s), {total_warnings} warning(s)")
|
||||
print(" Validation FAILED.\n")
|
||||
sys.exit(1)
|
||||
elif total_warnings:
|
||||
print(f" 0 errors, {total_warnings} warning(s)")
|
||||
print(" Validation PASSED.\n")
|
||||
else:
|
||||
print(" All checks passed.\n")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
Reference in New Issue
Block a user