diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md new file mode 100644 index 00000000..67a50f90 --- /dev/null +++ b/.claude/skills/create-skill/SKILL.md @@ -0,0 +1,599 @@ +--- +name: create-skill +description: Scaffold, write, and validate a new Claude Code skill — enforces quality standards derived from 250+ review comments +argument-hint: " (kebab-case, e.g. deploy-check)" +allowed-tools: Bash, Read, Write, Edit, Glob, Grep, Agent +--- + +# /create-skill — Skill Factory + +Create a new Claude Code skill with correct structure, robust bash, and built-in quality gates. This skill encodes lessons from 250+ Greptile review comments to prevent the most common skill authoring mistakes. + +## Arguments + +- `$ARGUMENTS` must contain the skill name in kebab-case (e.g. `deploy-check`) +- If `$ARGUMENTS` is empty, ask the user for a skill name before proceeding + +Set `SKILL_NAME` to the provided name. Validate it is kebab-case (`^[a-z][a-z0-9]*(-[a-z0-9]+)*$`). Reject otherwise. + +--- + +## Phase 0 — Discovery & Pre-flight + +**Pre-flight:** Verify required tools and environment: + +```bash +for tool in git mktemp; do + # > /dev/null 2>&1: suppress command path on success and shell's "not found" on failure — the || clause provides the error message + command -v "$tool" > /dev/null 2>&1 || { echo "ERROR: required tool '$tool' not found"; exit 1; } +done +# > /dev/null 2>&1: suppress git's own "fatal: not a git repository" — our || message is more actionable +git rev-parse --show-toplevel > /dev/null 2>&1 || { echo "ERROR: not in a git repository — run /create-skill from the repo root"; exit 1; } +``` + +Parse `$ARGUMENTS` per the Arguments section above. If validation fails, abort with a clear error. + +**Discovery:** Before writing anything, gather requirements interactively. Ask the user these questions (all at once, not one-by-one): + +1. **Purpose** — What does this skill do? (one paragraph) +2. **Arguments** — What CLI arguments should it accept? (e.g. `--fix`, `--dry-run`, ``) +3. **Phases** — What are the major steps? (bullet list of 3-8 phases) +4. **Tools needed** — Which tools does it need? (Bash, Read, Write, Edit, Glob, Grep, Agent) +5. **Artifacts** — Does it produce output files? If so, where and what format? +6. **Dangerous operations** — Does it modify code, push to git, call external APIs, or delete files? +7. **Resume/skip support** — Should it support `--start-from` or `--skip-*` flags for long-running pipelines? + +**Wait for the user's answers before proceeding.** Do not guess or assume. + +**Exit condition:** Pre-flight passed (git repo confirmed, skill name validated). All 7 questions have answers. Purpose, arguments, phases, tools, artifacts, dangerous ops, and resume support are defined. + +--- + +## Phase 1 — Scaffold + +**Idempotency guard:** Before writing, check for an existing skill: + +```bash +if [ -f ".claude/skills/$SKILL_NAME/SKILL.md" ]; then + echo "WARN: .claude/skills/$SKILL_NAME/SKILL.md already exists." + echo "Proceeding will overwrite it. Confirm (y) or abort (n)." + # STOP — ask the user whether to overwrite before continuing. Exit 1 if they decline. +fi +``` + +Create the skill directory and SKILL.md with frontmatter: + +```bash +mkdir -p ".claude/skills/$SKILL_NAME" +``` + +Write the SKILL.md file starting with this structure: + +```markdown +--- +name: $SKILL_NAME +description: +argument-hint: "" +allowed-tools: +--- + +# /$SKILL_NAME — + +<Purpose paragraph from Phase 0> + +## Arguments + +- `$ARGUMENTS` parsing rules here +- Set state variables: `DRY_RUN`, `AUTO_FIX`, etc. + +## Phase 0 — Pre-flight + +```bash +for tool in git mktemp; do # replace with the actual shell commands your skill uses (e.g. jq, python, curl) + # > /dev/null 2>&1: suppress command path on success and shell's "not found" on failure — the || clause provides the error message + command -v "$tool" > /dev/null 2>&1 || { echo "ERROR: required tool '$tool' not found"; exit 1; } +done +# > /dev/null 2>&1: suppress git's own "fatal: not a git repository" — our || message is more actionable +git rev-parse --show-toplevel > /dev/null 2>&1 || { echo "ERROR: not in a git repository"; exit 1; } +``` + +1. Confirm environment (repo root, required runtime/toolchain version, required tools) +2. Parse `$ARGUMENTS` into state variables +3. Validate preconditions + +**Exit condition:** <What must be true before Phase 1 starts, e.g. "git repo confirmed, arguments validated, all required tools present"> + +## Phase N — <Name> + +<Steps> + +**Exit condition:** <What must be true before the next phase starts> + +## Examples + +- <Usage examples showing common invocations and expected behavior> + +## Rules + +- <Hard constraints> +``` + +### Structural requirements to include in every skill: + +1. **Phase 0 always exists** — pre-flight checks, argument parsing, environment validation +2. **Every phase has a clear exit condition** — what must be true before moving to the next phase +3. **Arguments section** — explicit parsing of `$ARGUMENTS` into named state variables +4. **Rules section** — hard constraints at the bottom, kept in sync with the procedure +5. **Examples section** — usage examples showing common invocations and expected behavior +6. **Artifact definitions** — if the skill produces files, specify path, format, and schema + +**Exit condition:** `.claude/skills/$SKILL_NAME/SKILL.md` exists with valid frontmatter, Phase 0, Arguments section, and Rules section. + +--- + +## Phase 2 — Write the Skill Body + +Write each phase following these **17 mandatory patterns** (derived from Greptile review findings across 250+ comments): + +### Pattern 1: No shell variables across code fences + +Each fenced code block is a **separate shell invocation**. Variables set in one block do not exist in the next. + +**Wrong:** +````markdown +```bash +WORK_DIR=$(mktemp -d) +``` +Later: +```bash +rm -rf $WORK_DIR # BUG: $WORK_DIR is empty here +``` +```` + +**Correct:** Persist state to a file (use your actual skill name, not a variable). +First ensure the directory exists: +````markdown +```bash +mkdir -p .codegraph/deploy-check +mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX" > .codegraph/deploy-check/.tmpdir +``` +Later: +```bash +rm -rf "$(cat .codegraph/deploy-check/.tmpdir)" +``` +```` + +Or keep everything in a single code block if the operations are sequential. + +### Pattern 2: No silent failures + +Never use `2>/dev/null` without documenting the skip path. Every command that can fail must either: +- Have explicit error handling (`|| { echo "ERROR: ..."; exit 1; }`) +- Be documented as intentionally tolerant ("this may fail on X, which is acceptable because Y") + +**Wrong:** +````markdown +```bash +git show HEAD:$FILE 2>/dev/null | codegraph where --file - +``` +```` + +**Correct:** +````markdown +```bash +PREV_FILE=$(mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX.js") # adjust extension to match the language of $FILE; template syntax is portable (macOS + Linux) +trap 'rm -f "$PREV_FILE"' EXIT +# $FILE is expected to be set by the surrounding loop, e.g. for FILE in $(git diff --name-only HEAD); do ... done +# 2>/dev/null: suppress git's "fatal: Path X does not exist in HEAD" — the else branch already warns the user +if git show HEAD:"$FILE" > "$PREV_FILE" 2>/dev/null; then + codegraph where --file "$PREV_FILE" +else + echo "WARN: $FILE is new or unreadable in HEAD — skipping before/after comparison" +fi +rm -f "$PREV_FILE" +trap - EXIT +``` +```` + +### Pattern 3: Temp files need extensions + +Codegraph's language detection is extension-based. Temp files passed to codegraph must have the correct extension: + +```bash +mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX.js" # NOT just mktemp — template syntax is cross-platform (macOS + Linux) +``` + +### Pattern 4: No hardcoded temp paths + +Use `mktemp` for temp files, never hardcoded paths like `/tmp/skill-output.json`. Concurrent sessions or re-runs will collide. + +### Pattern 5: Stable cross-references + +When referencing other steps, use **phase names**, not numbers. Numbers break when steps are inserted. + +**Wrong:** "Use the results from Step 2" +**Correct:** "Use the diff-impact results from Phase: Impact Analysis" + +### Pattern 6: No undefined placeholders + +Every variable or placeholder in pseudocode must have a preceding assignment. If a value requires detection, write the explicit detection script — do not use `<detected-value>` placeholders. + +**Wrong:** "Run `<detected-test-command>`" +**Correct:** +````markdown +Detect the test runner and run in a single block: +```bash +if [ -f "pnpm-lock.yaml" ]; then TEST_CMD="pnpm test" +elif [ -f "yarn.lock" ]; then TEST_CMD="yarn test" +elif [ -f "package.json" ]; then TEST_CMD="npm test" +else echo "WARN: No recognised test runner found — skipping tests"; TEST_CMD="true"; fi +$TEST_CMD +``` +```` + +### Pattern 7: No internal contradictions + +Each decision (pass/fail, rollback/continue, skip/run) must be defined in **exactly one place**. If two sections describe the same decision path, consolidate them and reference the single source. + +### Pattern 8: Rules ↔ Procedure sync + +Every codegraph command or tool invocation in the procedure must be permitted by the Rules section. Every exception in the Rules must correspond to an actual procedure step. After writing, cross-check both directions. + +### Pattern 9: No command redundancy + +If a phase runs a codegraph command and stores the result, later phases must reference that result — not re-run the command. Add a note like: "Using `impact_report` from Phase: Impact Analysis". + +### Pattern 10: Skip/resume flag validation + +If the skill supports `--start-from` or `--skip-*`: +- Skipping a phase must NOT skip its artifact validation +- Add a pre-validation table listing which artifacts are required for each entry point +- Each skip path must be explicitly tested in Phase: Self-Review + +### Pattern 11: Progress indicators + +For any phase that takes longer than ~10 seconds (file iteration, API calls, batch operations), emit progress: + +```bash +# $i, $total, and $FILE are loop variables, e.g. i=0; total=$(wc -l < filelist); while read FILE; do i=$((i+1)); ... +echo "Processing file $i/$total: $FILE" +``` + +Never leave the user staring at a silent terminal during long operations. + +### Pattern 12: Artifact reuse + +Before running expensive operations (codegraph build, embedding generation, batch analysis), check if usable output already exists (replace `deploy-check` with your actual skill name): + +````markdown +```bash +if [ -f ".codegraph/deploy-check/results.json" ]; then + echo "Using cached results from previous run" +else + # run expensive operation +fi +``` +```` + +This supports both idempotent re-runs and resume-after-failure. + +### Pattern 13: Platform portability + +Avoid shell constructs that behave differently across platforms: +- Use `find ... -name "*.ext" | grep -q .` instead of `find ... -quit` (non-portable) or glob expansion (`ls *.ext`) which differs between bash versions +- Use `mktemp` with template syntax (`mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX.ext"`) — GNU flags like `--suffix` and `-p` are not available on macOS BSD `mktemp` +- Use `sed -i.bak` instead of `sed -i ''` (GNU vs BSD incompatibility) +- Document any platform-specific behavior with a comment: `# NOTE: requires GNU coreutils` + +### Pattern 14: Trap-based cleanup + +Any phase that creates temp files or modifies repo state must set a cleanup trap. Without it, errors mid-phase leak temp files or leave dirty state: + +```bash +TMPFILE=$(mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX.json") +trap 'rm -f "$TMPFILE"' EXIT +# ... operations that might fail ... +# Reset when done if more work follows: +trap - EXIT +``` + +For phases that `cd` into a temp directory, clean up both the directory and the working directory change: + +```bash +WORK_DIR=$(mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX") +# > /dev/null 2>&1: suppress cd's directory-path output — cleanup should be silent +trap 'cd - > /dev/null 2>&1; rm -rf "$WORK_DIR"' EXIT +cd "$WORK_DIR" +# ... operations inside the temp directory ... +# > /dev/null 2>&1: suppress cd's directory-path output — cleanup should be silent +cd - > /dev/null 2>&1 +rm -rf "$WORK_DIR" +trap - EXIT +``` + +### Pattern 15: Git stash safety + +Never rely on `$?` or `stash@{0}` after `git stash push` — modern git (2.16+) returns 0 even when nothing was stashed, and other operations may push to the stash stack between your push and pop. + +**Wrong:** +````markdown +```bash +git stash push -- package.json +# ... operations ... +git stash pop # BUG: pops wrong entry if stash was no-op or stack changed +``` +```` + +**Correct:** Use a named stash with STASH_REF lookup — keep push and pop in one block or persist the ref to a file if other work must happen in between. + +Single-block approach (push and pop bracket the work): +````markdown +```bash +git stash push -m "deploy-check-backup" -- package.json package-lock.json +STASH_REF=$(git stash list --format='%gd %s' | grep 'deploy-check-backup' | head -1 | awk '{print $1}') +# STASH_REF is non-empty only if a stash entry was actually created. +# ... work here ... +[ -n "$STASH_REF" ] && git stash pop "$STASH_REF" +``` +```` + +If the pop must happen in a later code fence, persist the ref to a file (per Pattern 1): +````markdown +```bash +mkdir -p .codegraph/deploy-check +git stash push -m "deploy-check-backup" -- package.json package-lock.json +git stash list --format='%gd %s' | grep 'deploy-check-backup' | head -1 | awk '{print $1}' > .codegraph/deploy-check/.stash-ref +``` +Later: +```bash +STASH_REF=$(cat .codegraph/deploy-check/.stash-ref) +[ -n "$STASH_REF" ] && git stash pop "$STASH_REF" +rm -f .codegraph/deploy-check/.stash-ref +``` +```` + +### Pattern 16: Division-by-zero guards + +Every arithmetic division or percentage computation must guard against zero denominators. Common in benchmark comparisons, complexity deltas, and ratio calculations: + +**Wrong:** +````markdown +```bash +DELTA=$(( (CURRENT - BASELINE) * 100 / BASELINE )) +``` +```` + +**Correct:** +````markdown +```bash +# $BASELINE and $CURRENT are expected to be set by the surrounding context (e.g. from codegraph stats output) +if [ "$BASELINE" -gt 0 ]; then + DELTA=$(( (CURRENT - BASELINE) * 100 / BASELINE )) +else + DELTA=0 # no baseline — treat as no change +fi +``` +```` + +### Pattern 17: DRY_RUN consistency + +If the skill supports `--dry-run`, every destructive operation must check the flag **at the point of action** — not just at phase entry. A single phase often mixes reads (always run) and writes (skip in dry-run): + +**Wrong:** +````markdown +```bash +# Phase skips entirely in dry-run — but the analysis part is useful +if [ "$DRY_RUN" = "true" ]; then exit 0; fi +# ... 50 lines of analysis ... +rm -rf "$OUTPUT_DIR" +``` +```` + +**Correct:** +````markdown +```bash +# $DRY_RUN is expected to be parsed from $ARGUMENTS in Phase 0; $OUTPUT_DIR is set earlier in this phase +# Analysis always runs +codegraph audit --quick src/ +# Only the destructive part checks DRY_RUN +if [ "$DRY_RUN" = "true" ]; then + echo "[DRY RUN] Would remove $OUTPUT_DIR" +else + rm -rf "$OUTPUT_DIR" +fi +``` +```` + +**Exit condition:** Every phase body in the SKILL.md follows all 17 patterns. No wrong/correct examples remain as actual instructions — only the correct versions. + +--- + +## Phase 3 — Dangerous Operation Guards + +If the skill performs dangerous operations (from Phase 0 discovery), add explicit guards: + +### For git modifications: +- Stage only named files (never `git add .` or `git add -A`) +- Include rollback instructions: "To undo: `git reset HEAD~1`" or similar +- For destructive operations (force-push, reset, clean): require explicit confirmation + +### For file deletions: +- List what will be deleted before deleting +- Use `rm -i` or prompt for confirmation in non-`--force` mode + +### For external API calls: +- Handle network failures gracefully (don't crash the pipeline) +- Add timeout limits + +### For code modifications: +- Run tests after changes: detect test runner per Phase: Write the Skill Body, Pattern 6 +- Run lint after changes: detect lint runner: + ```bash + if [ -f "biome.json" ]; then LINT_CMD="npx biome check" + elif find . -maxdepth 1 -name "eslint.config.*" | grep -q .; then LINT_CMD="npx eslint ." + elif [ -f "package.json" ]; then LINT_CMD="npm run lint" + else echo "WARN: No recognised lint runner found — skipping lint"; LINT_CMD="true"; fi + $LINT_CMD + ``` + +**Exit condition:** Every dangerous operation identified in Phase: Discovery has a corresponding guard in the SKILL.md. + +--- + +## Phase 4 — Self-Review Checklist + +Before finalizing, audit the SKILL.md against every item below. **Do not skip any item.** Fix violations before proceeding. + +### Structure checks: +- [ ] Frontmatter has all four fields: `name`, `description`, `argument-hint`, `allowed-tools` +- [ ] `name` matches the directory name +- [ ] Phase 0 exists and validates the environment +- [ ] Arguments section explicitly parses `$ARGUMENTS` into named variables +- [ ] Rules section exists at the bottom +- [ ] Every phase has a clear name (not just a number) + +### Anti-pattern checks (all 17 patterns): +- [ ] **Shell variables**: No variable is set in one code fence and used in another. State that must persist is written to a file +- [ ] **Silent failures**: No `2>/dev/null` without a documented skip rationale. No commands that swallow errors +- [ ] **Temp file extensions**: Every temp file passed to codegraph has the correct language extension +- [ ] **Temp file uniqueness**: Every temp path uses `mktemp`, never hardcoded paths +- [ ] **Cross-references**: All step references use phase names, not bare numbers +- [ ] **Placeholders**: Every `<placeholder>` has a preceding detection/assignment script +- [ ] **Contradictions**: No two sections describe contradictory behavior for the same condition +- [ ] **Rules sync**: Every command/tool in the procedure is covered by Rules. Every Rules exception maps to a real step +- [ ] **Redundancy**: No codegraph command is run twice with the same arguments. Later phases reference earlier results +- [ ] **Skip validation**: If `--start-from`/`--skip-*` is supported, every skip path validates required artifacts +- [ ] **Progress indicators**: Phases that iterate over files or run batch operations emit progress (`Processing $i/$total`) +- [ ] **Artifact reuse**: Expensive operations (codegraph build, embedding generation, batch analysis) check for existing output before re-running +- [ ] **Platform portability**: No `sed -i ''`, no unquoted globs, no GNU-only flags without fallback or documentation +- [ ] **Cleanup traps**: Phases that create temp files or modify repo state use `trap ... EXIT` for cleanup on error paths +- [ ] **Git stash safety**: Every `git stash push` has a named STASH_REF lookup; every `pop`/`drop` is guarded by `[ -n "$STASH_REF" ]` +- [ ] **Division-by-zero**: Every arithmetic division or percentage computation guards against zero denominators +- [ ] **DRY_RUN consistency**: If `--dry-run` is supported, every destructive operation is gated on the flag at the point of action, not just at phase entry + +### Robustness checks: +- [ ] **Rollback paths**: Every destructive operation has documented undo instructions +- [ ] **Error messages**: Every failure path produces a specific, actionable error message (not just "failed") +- [ ] **Concurrency safety**: No shared global state that would break under parallel invocation +- [ ] **Determinism**: No non-deterministic algorithm output used for before/after comparisons (e.g., Louvain community IDs) + +### Completeness checks: +- [ ] **Artifact schema**: If the skill produces files, path/format/schema are documented +- [ ] **Exit conditions**: Each phase states what must be true before the next phase starts +- [ ] **Scope boundary**: The skill's purpose is clear — it does one thing, not five +- [ ] **Examples section**: At least 2-3 realistic usage examples showing common invocations are included + +### Safety checks: +- [ ] **Idempotency**: Re-running the skill on the same state is safe. Existing output files are handled (skip, overwrite with warning, or merge) +- [ ] **Dependency validation**: Phase 0 verifies all shell commands used in bash blocks are available before starting work (e.g. `command -v git mktemp jq`). "Command not found" is caught before Phase 2, not during Phase 3 +- [ ] **Exit codes**: Every error path uses explicit `exit 1`. No silent early returns that leave the pipeline in an ambiguous state +- [ ] **State cleanup**: If the skill creates `.codegraph/$SKILL_NAME/*` files, the skill documents when they're cleaned up or how users remove them (e.g., `rm -rf .codegraph/$SKILL_NAME` in a cleanup section) +- [ ] **Git commit safety**: All `git add` calls use explicit file paths (never `.` or `-A`); `git diff --cached --quiet` is checked before committing to avoid empty commits + +Read through the entire SKILL.md one more time after checking all items. Fix anything found. + +**Exit condition:** All checklist items pass. The SKILL.md has no unresolved structure, anti-pattern, robustness, completeness, or safety violations. + +--- + +## Phase 5 — Smoke Test + +The self-review is purely theoretical — most real issues (wrong paths, shell syntax, missing tools, argument parsing bugs) only surface when you actually try to run the code. Before finalizing, execute these validation steps: + +### Automated validation + +Run both validation scripts against the generated SKILL.md: + +```bash +LINT_RC=0 +bash .claude/skills/create-skill/scripts/lint-skill.sh ".claude/skills/$SKILL_NAME/SKILL.md" || LINT_RC=$? +SMOKE_RC=0 +bash .claude/skills/create-skill/scripts/smoke-test-skill.sh ".claude/skills/$SKILL_NAME/SKILL.md" || SMOKE_RC=$? +if [ "$LINT_RC" -ne 0 ] || [ "$SMOKE_RC" -ne 0 ]; then + echo "Validation failed — fix ERRORs before proceeding"; exit 1 +fi +``` + +- **`lint-skill.sh`** checks for cross-fence variable bugs, bare `2>/dev/null`, hardcoded `npm test` / `npm run test` / `npm run lint` / `yarn test` / `yarn run test` / `yarn run lint` / `pnpm test` / `pnpm run test` / `pnpm run lint`, `git add .` / `git add -- .`, missing frontmatter, missing Phase 0 / Rules, missing exit conditions, GNU-only `find -quit`, hardcoded `/tmp/` paths, and `sed -i` portability issues. +- **`smoke-test-skill.sh`** extracts every `bash` code block (skipping example regions inside quadruple backticks) and runs `bash -n` syntax checking on each. + +Fix all ERROR findings. Review WARN findings — fix or annotate with justification. + +### Phase 0 dry-run + +Run the skill's Phase 0 (pre-flight) logic in a temporary test directory to verify: +- Argument parsing works for valid inputs and rejects invalid ones +- Tool availability checks actually detect missing tools (temporarily rename one to confirm) +- Environment validation produces clear error messages on failure + +```bash +TEST_DIR=$(mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX") +# > /dev/null 2>&1: suppress cd's directory-path output — cleanup should be silent +trap 'cd - > /dev/null 2>&1; rm -rf "$TEST_DIR"' EXIT +cd "$TEST_DIR" +git init --quiet + +# --- Happy path: valid skill name and all tools present --- +# Simulate: ARGUMENTS="deploy-check"; run the Phase 0 pre-flight checks + +# --- Failure path 1: not in a git repo --- +# (run checks from a non-git directory — expect ERROR message + non-zero exit) + +# --- Failure path 2: invalid skill name --- +# ARGUMENTS="BadName!"; run argument validation — expect rejection + +# --- Failure path 3: missing required tool --- +# (temporarily hide a required tool — expect clear error message + non-zero exit) + +# > /dev/null 2>&1: suppress cd's directory-path output — returning to original directory +cd - > /dev/null 2>&1 +rm -rf "$TEST_DIR" +trap - EXIT +``` + +### Idempotency check + +Mentally trace a second execution of the skill on the same state: +- Does Phase 0 handle pre-existing artifacts (skip, warn, overwrite)? +- Do file-creation steps fail if the file already exists? +- Are `mktemp` paths unique across runs (they should be by default)? + +Document any idempotency fix applied. + +**Exit condition:** All bash blocks pass `bash -n` syntax check. Phase 0 logic runs without errors in a test directory. Idempotency is confirmed or fixed. + +--- + +## Phase 6 — Finalize + +1. Read the final SKILL.md end-to-end and confirm it passes all Phase: Self-Review Checklist checks and Phase: Smoke Test validations +2. Show the user the complete skill for review +3. Ask: "Ready to commit, or want changes?" + +If the user approves: +- Stage only `.claude/skills/$SKILL_NAME/SKILL.md` (and any scripts in `.claude/skills/$SKILL_NAME/scripts/` if created) +- Commit: `feat(skill): add /$SKILL_NAME skill` + +**Exit condition:** User has approved the skill. `.claude/skills/$SKILL_NAME/SKILL.md` is committed to the repo. + +--- + +## Examples + +- `/create-skill deploy-check` — scaffold a deployment validation skill that runs preflight checks before deploying +- `/create-skill review-pr` — scaffold a PR review skill with API calls and diff analysis +- `/create-skill db-migrate` — scaffold a database migration skill with dangerous-operation guards and rollback paths + +--- + +## Rules + +- **Never write the skill without Phase 0 discovery answers.** The user must describe what they want before you write anything. +- **Never skip the self-review checklist or smoke test.** Phase: Self-Review Checklist and Phase: Smoke Test are both mandatory, not optional. +- **Phase names over step numbers.** All cross-references in the generated skill must use phase names. +- **One skill = one concern.** If the user's requirements span multiple unrelated workflows, suggest splitting into separate skills. +- **No co-author lines** in commit messages. +- **No Claude Code references** in commit messages or the skill body. +- **Persist temp state to files, not shell variables** when it must survive across code fences. +- **Test commands must be detected, not assumed.** Never hardcode `npm test`, `npm run test`, `npm run lint`, `yarn test`, `yarn run test`, `yarn run lint`, `pnpm test`, `pnpm run test`, or `pnpm run lint` — detect the package manager first. +- **Every `2>/dev/null` needs a justification comment** in the generated skill. diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh new file mode 100644 index 00000000..3e3f172e --- /dev/null +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -0,0 +1,309 @@ +#!/usr/bin/env bash +# lint-skill.sh — Static analysis for SKILL.md files +# Catches the most common issues found in 250+ Greptile review comments. +# Exit 0 = warnings only, Exit 1 = errors found. + +set -euo pipefail + +if [ "${BASH_VERSINFO[0]}" -lt 4 ]; then + echo "lint-skill.sh requires bash 4+ (for associative arrays). On macOS: brew install bash" >&2 + exit 1 +fi + +SKILL_FILE="${1:?Usage: lint-skill.sh <path-to-SKILL.md>}" + +if [ ! -f "$SKILL_FILE" ]; then + echo "ERROR: File not found: $SKILL_FILE" + exit 1 +fi + +ERRORS=0 +WARNINGS=0 + +error() { echo "ERROR: $1"; ERRORS=$((ERRORS + 1)); } +warn() { echo "WARN: $1"; WARNINGS=$((WARNINGS + 1)); } + +# ── Check 1: Cross-fence variable usage ────────────────────────────── +# Extract bash blocks (skip quadruple-backtick example regions) and check +# if UPPER_CASE variables assigned in one block are referenced in a later +# block without file-based persistence. +BLOCKS_FILE=$(mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX.blocks") +trap 'rm -f "$BLOCKS_FILE"' EXIT + +# Extract bash blocks with block index, skipping those inside ```` regions. +# Patterns use ^[[:space:]]* to match indented blocks (e.g. inside Markdown list items). +awk ' + /^[[:space:]]*````/ { quad = !quad; next } + quad { next } + /^[[:space:]]*```bash/ { inblock = 1; blocknum++; next } + /^[[:space:]]*```/ && inblock { inblock = 0; next } + inblock { print blocknum "\t" $0 } +' "$SKILL_FILE" > "$BLOCKS_FILE" + +# Collect variable assignments per block and build reassignment lookup (O(1) per check) +declare -A VAR_BLOCK +declare -A REASSIGNED +while IFS=$'\t' read -r bnum line; do + # Skip comment lines — they document context but don't register variable assignments + echo "$line" | grep -qE '^\s*#' && continue + # Match UPPER_CASE_VAR= assignments (skip lowercase/mixed to reduce false positives) + # Use while-read instead of for-in-$() to avoid empty-string iteration when grep matches nothing + while IFS= read -r var; do + [ -z "$var" ] && continue + if [ -z "${VAR_BLOCK[$var]+x}" ]; then + VAR_BLOCK["$var"]="$bnum" + else + # Track re-assignments in later blocks for O(1) lookup + REASSIGNED["${var}:${bnum}"]=1 + fi + done < <(echo "$line" | grep -oE '\b[A-Z][A-Z0-9_]+\+?=' | sed -E 's/\+?=$//') +done < "$BLOCKS_FILE" + +# Check for references in later blocks without file persistence +while IFS=$'\t' read -r bnum line; do + # Skip comment lines — they document context but don't execute variables at runtime + echo "$line" | grep -qE '^\s*#' && continue + for var in "${!VAR_BLOCK[@]}"; do + assigned_in="${VAR_BLOCK[$var]}" + if [ "$bnum" -gt "$assigned_in" ]; then + # Check if this line references the variable ($VAR or ${VAR}) + if echo "$line" | grep -qE '\$'"${var}"'([^A-Za-z0-9_]|$)' \ + || echo "$line" | grep -qF "\${${var}}"; then + # Check if the same block also assigns it (re-assignment is fine) — O(1) lookup + if [ -z "${REASSIGNED[${var}:${bnum}]+x}" ]; then + # Check it's not read from a file (cat, $(...) with cat/read) + if ! echo "$line" | grep -qE 'cat |read |< |<"|\$\(<'; then + error "Cross-fence variable: \$$var assigned in bash block $assigned_in, referenced in block $bnum without file persistence (Pattern 1)" + fi + fi + fi + fi + done +done < "$BLOCKS_FILE" + +# ── Check 2: Bare 2>/dev/null without justification ───────────────── +line_num=0 +in_quad=false +in_block=false +prev_line="" +while IFS= read -r line; do + line_num=$((line_num + 1)) + stripped="${line#"${line%%[! ]*}"}" + case "$stripped" in + '````'*) if $in_quad; then in_quad=false; else in_quad=true; fi; prev_line="$line"; continue ;; + esac + $in_quad && { prev_line="$line"; continue; } + case "$stripped" in + '```bash'*) in_block=true; prev_line="$line"; continue ;; + '```'*) in_block=false; prev_line="$line"; continue ;; + esac + if $in_block && echo "$line" | grep -qE '2>/dev/null|>[ ]*/dev/null 2>&1|&>/dev/null'; then + # Check same line or previous line for justification comment + justification_re='#.*intentional|#.*tolera|#.*acceptable|#.*expected|#.*safe to ignore|#.*may fail|#.*optional|#.*fallback|#.*portable|#.*suppress|#.*provid' + if ! echo "${prev_line}${line}" | grep -qiE "$justification_re"; then + warn "Line $line_num: '2>/dev/null' without justification comment (Pattern 2)" + fi + fi + prev_line="$line" +done < "$SKILL_FILE" + +# ── Check 3: git add . or git add -A (inside bash blocks only) ─────── +while IFS=$'\t' read -r bnum line; do + if echo "$line" | grep -qE '^\s*git add (--\s+\.([ \t;#]|$)|\.([ \t;#]|$)|(-A|--all)([ \t;#]|$))'; then + error "bash block $bnum: 'git add .' or 'git add -A' — stage named files only" + fi +done < "$BLOCKS_FILE" + +# ── Check 4: Hardcoded npm test / npm run lint ─────────────────────── +# Only flag if not inside an if/elif detection block +line_num=0 +in_quad=false +in_block=false +in_detect=false +detect_depth=0 +while IFS= read -r line; do + line_num=$((line_num + 1)) + stripped="${line#"${line%%[! ]*}"}" + case "$stripped" in + '````'*) if $in_quad; then in_quad=false; else in_quad=true; fi; continue ;; + esac + $in_quad && continue + case "$stripped" in + '```bash'*) in_block=true; in_detect=false; detect_depth=0; continue ;; + '```'*) in_block=false; in_detect=false; detect_depth=0; continue ;; + esac + if $in_block; then + # Track if we're inside an if/elif chain (detection block) with depth. + # Only `if` increments depth; `elif` is a sibling branch of the same if-statement, + # not a new nesting level, so it sets in_detect but does NOT increment depth. + # Save in_detect before fi-processing so inline commands on the same line + # (e.g. "else npm test; fi") are evaluated in the correct detection context. + was_in_detect=$in_detect + if echo "$line" | grep -qE '^\s*if\s.*(-f\s|-d\s|\block\b|\bpackage\b|command -v|which\s|find\s)'; then + in_detect=true + was_in_detect=true + # Only increment depth if fi does NOT also close on this line (one-liner guard) + if ! echo "$line" | grep -qE '\bfi\b'; then + detect_depth=$((detect_depth + 1)) + else + # One-liner: detection block is self-contained — reset so subsequent lines are checked normally + in_detect=false + fi + elif echo "$line" | grep -qE '^\s*elif\s.*(-f\s|-d\s|\block\b|\bpackage\b|command -v|which\s|find\s)'; then + # elif is a sibling branch — set in_detect but do NOT increment depth + in_detect=true + was_in_detect=true + # Handle inline fi on this same elif line (e.g. "elif [ -f yarn.lock ]; then CMD=yarn; fi") + if echo "$line" | grep -qE '\bfi\b'; then + if [ "$detect_depth" -gt 0 ]; then + detect_depth=$((detect_depth - 1)) + [ "$detect_depth" -eq 0 ] && in_detect=false + else + in_detect=false + fi + fi + elif echo "$line" | grep -qE '^\s*if\b'; then + # nested if (not a detection block) — track depth only when inside detection + [ "$in_detect" = true ] && detect_depth=$((detect_depth + 1)) + elif echo "$line" | grep -qE '^\s*fi\b'; then + if [ "$detect_depth" -gt 0 ]; then + detect_depth=$((detect_depth - 1)) + [ "$detect_depth" -eq 0 ] && in_detect=false + else + # Safety reset: in_detect was set by an elif without a preceding detection if + in_detect=false + fi + elif $in_detect && echo "$line" | grep -qE '\bfi\b'; then + # fi appears inline (e.g. "else ...; fi") — still closes the outermost detection block + if [ "$detect_depth" -gt 0 ]; then + detect_depth=$((detect_depth - 1)) + [ "$detect_depth" -eq 0 ] && in_detect=false + else + in_detect=false + fi + fi + # Use was_in_detect so commands on the same line as an inline fi + # (e.g. "else npm test; fi") are not falsely flagged — the command + # was part of the detection block, not after it. + if ! $was_in_detect; then + if echo "$line" | grep -qE '^\s*((npm|yarn|pnpm) test|(npm|yarn|pnpm) run (test|lint))([^:A-Za-z0-9_]|$)'; then + warn "Line $line_num: Hardcoded '$(echo "$line" | sed 's/^[[:space:]]*//')' — detect package manager first (Pattern 6)" + fi + fi + fi +done < "$SKILL_FILE" + +# ── Check 5: sed -i without .bak (inside bash blocks only) ────────── +while IFS=$'\t' read -r bnum line; do + if echo "$line" | grep -qE "sed\s+-i\s*(''|\"|[^.])"; then + warn "bash block $bnum: 'sed -i' without .bak extension — GNU/BSD incompatibility (Pattern 13)" + fi +done < "$BLOCKS_FILE" + +# ── Check 6: Missing frontmatter fields ───────────────────────────── +for field in name description argument-hint allowed-tools; do + if ! head -20 "$SKILL_FILE" | grep -qE "^${field}:"; then + error "Missing frontmatter field: '$field'" + fi +done + +# ── Check 7: Missing Phase 0 ──────────────────────────────────────── +if ! grep -qE '^## Phase 0' "$SKILL_FILE"; then + error "Missing '## Phase 0' heading — every skill needs pre-flight checks" +fi + +# ── Check 8: Missing Rules section ─────────────────────────────────── +if ! grep -qE '^## Rules' "$SKILL_FILE"; then + error "Missing '## Rules' section" +fi + +# ── Check 8b: Missing Examples section ────────────────────────────── +if ! grep -qE '^## Examples' "$SKILL_FILE"; then + error "Missing '## Examples' section — every skill needs 2-3 usage examples" +fi + +# ── Check 6b: name field matches directory name ───────────────────── +expected_name=$(basename "$(dirname "$SKILL_FILE")") +actual_name=$(head -20 "$SKILL_FILE" | grep -m1 '^name:' | sed 's/^name:[[:space:]]*//') +if [ -n "$actual_name" ] && [ "$actual_name" != "$expected_name" ]; then + error "Frontmatter 'name: $actual_name' does not match directory name '$expected_name' (Phase 4 checklist item 2)" +fi + +# ── Check 9: Missing exit conditions between phases ────────────────── +prev_phase="" +phase_has_exit=true +in_quad=false +in_block=false +while IFS= read -r line; do + stripped="${line#"${line%%[! ]*}"}" + # Skip content inside quadruple-backtick example regions + case "$stripped" in + '````'*) if $in_quad; then in_quad=false; else in_quad=true; fi; continue ;; + esac + $in_quad && continue + # Skip content inside triple-backtick code blocks. + # Limitation: nested fences inside ```markdown blocks (e.g. scaffold templates + # containing ```bash examples) will toggle in_block incorrectly. Wrap such + # regions in quadruple-backtick ```` fences to avoid false positives. + case "$stripped" in + '```'*) if $in_block; then in_block=false; else in_block=true; fi; continue ;; + esac + $in_block && continue + + if echo "$line" | grep -qE '^## Phase [0-9]+'; then + if [ -n "$prev_phase" ] && [ "$phase_has_exit" = false ]; then + warn "Phase '$prev_phase' has no 'Exit condition' before the next phase" + fi + prev_phase="$line" + phase_has_exit=false + fi + if echo "$line" | grep -qiE '\*\*Exit condition'; then + phase_has_exit=true + fi +done < "$SKILL_FILE" +# Check last phase +if [ -n "$prev_phase" ] && [ "$phase_has_exit" = false ]; then + warn "Phase '$prev_phase' has no 'Exit condition'" +fi + +# ── Check 10: find with -quit (inside bash blocks only) ────────────── +while IFS=$'\t' read -r bnum line; do + if echo "$line" | grep -qE 'find\s+.*-quit'; then + warn "bash block $bnum: 'find -quit' is GNU-only — use 'head -1' or 'grep -q' instead (Pattern 13)" + fi +done < "$BLOCKS_FILE" + +# ── Check 11: Hardcoded /tmp/ paths ───────────────────────────────── +line_num=0 +in_quad=false +in_block=false +while IFS= read -r line; do + line_num=$((line_num + 1)) + lstripped="${line#"${line%%[! ]*}"}" + case "$lstripped" in + '````'*) if $in_quad; then in_quad=false; else in_quad=true; fi; continue ;; + esac + $in_quad && continue + case "$lstripped" in + '```bash'*) in_block=true; continue ;; + '```'*) in_block=false; continue ;; + esac + if $in_block; then + # Skip comments and mktemp template syntax + stripped=$(echo "$line" | sed 's/#.*//') + if echo "$stripped" | grep -qE '"/tmp/[a-zA-Z]|/tmp/[a-zA-Z]'; then + # Allow ${TMPDIR:-/tmp} pattern + if ! echo "$stripped" | grep -qE '\$\{TMPDIR:-/tmp\}'; then + warn "Line $line_num: Hardcoded '/tmp/' path — use mktemp instead (Pattern 4)" + fi + fi + fi +done < "$SKILL_FILE" + +# ── Summary ────────────────────────────────────────────────────────── +echo "" +echo "lint-skill: $ERRORS error(s), $WARNINGS warning(s)" +if [ "$ERRORS" -gt 0 ]; then + exit 1 +fi +exit 0 diff --git a/.claude/skills/create-skill/scripts/smoke-test-skill.sh b/.claude/skills/create-skill/scripts/smoke-test-skill.sh new file mode 100644 index 00000000..54e4d3ea --- /dev/null +++ b/.claude/skills/create-skill/scripts/smoke-test-skill.sh @@ -0,0 +1,108 @@ +#!/usr/bin/env bash +# smoke-test-skill.sh — Extract bash blocks from a SKILL.md and run bash -n +# Skips blocks inside quadruple-backtick example regions (Wrong/Correct pairs). +# Exit 0 = all blocks pass, Exit 1 = syntax errors found. + +set -euo pipefail + +if [ "${BASH_VERSINFO[0]}" -lt 4 ]; then + echo "smoke-test-skill.sh requires bash 4+ (for set -o pipefail reliability). On macOS: brew install bash" >&2 + exit 1 +fi + +SKILL_FILE="${1:?Usage: smoke-test-skill.sh <path-to-SKILL.md>}" + +if [ ! -f "$SKILL_FILE" ]; then + echo "ERROR: File not found: $SKILL_FILE" + exit 1 +fi + +TMPBLOCK=$(mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX.sh") +trap 'rm -f "$TMPBLOCK"' EXIT + +PASS=0 +FAIL=0 +SKIP=0 + +line_num=0 +in_quad=false +in_block=false +block_start=0 + +while IFS= read -r line; do + line_num=$((line_num + 1)) + + # Strip leading whitespace for fence detection (indented blocks inside list items) + stripped="${line#"${line%%[! ]*}"}" + + # Track quadruple-backtick regions (example pairs) + case "$stripped" in + '````'*) + if $in_quad; then + in_quad=false + else + in_quad=true + fi + continue + ;; + esac + + # Skip blocks inside example regions + if $in_quad; then + continue + fi + + # Track triple-backtick bash blocks + case "$stripped" in + '```bash'*) + in_block=true + block_start=$line_num + > "$TMPBLOCK" + continue + ;; + '```'*) + if $in_block; then + in_block=false + # Run syntax check on the collected block + if [ -s "$TMPBLOCK" ]; then + if output=$("$BASH" -n "$TMPBLOCK" 2>&1); then + echo " OK block at line $block_start" + PASS=$((PASS + 1)) + else + echo " FAIL block at line $block_start:" + echo "$output" | sed 's/^/ /' + FAIL=$((FAIL + 1)) + fi + else + echo " SKIP empty block at line $block_start" + SKIP=$((SKIP + 1)) + fi + fi + continue + ;; + esac + + # Accumulate block contents + if $in_block; then + echo "$line" >> "$TMPBLOCK" + fi + +done < "$SKILL_FILE" + +# Warn about unclosed bash block (closing fence was never reached) +if $in_block; then + echo " FAIL unclosed bash block starting at line $block_start (missing closing fence)" + FAIL=$((FAIL + 1)) +fi + +# Warn when no bash blocks were found — likely wrong path or prose-only file +if [ "$PASS" -eq 0 ] && [ "$FAIL" -eq 0 ] && [ "$SKIP" -eq 0 ]; then + echo " WARN no bash blocks found — is this a valid SKILL.md?" +fi + +echo "" +echo "smoke-test-skill: $PASS passed, $FAIL failed, $SKIP skipped" +if [ "$FAIL" -gt 0 ]; then + exit 1 +fi +exit 0 diff --git a/CLAUDE.md b/CLAUDE.md index 03c4d2a9..9aa806c2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,6 +4,8 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co > **Hooks enforce code quality.** This project uses Claude Code hooks (`.claude/hooks/`) to automatically inject file-level dependency context on reads, rebuild the graph after edits, block commits with cycles or dead exports, run lint on staged files, and show diff-impact before commits. If codegraph reports an error or produces wrong results when analyzing itself, **fix the bug in the codebase**. +> **Never fabricate facts.** Do not state licenses, version numbers, feature claims, or any factual information without first verifying it (read the file, run the command, check the source). If you don't know, say so — do not guess. + > **Never document bugs as expected behavior.** If two engines (native vs WASM) produce different results, that is a bug in the less-accurate engine — not an acceptable "parity gap." Adding comments or tests that frame wrong output as "expected" blocks future agents from ever fixing it. Instead: identify the root cause, file an issue, and fix the extraction/resolution layer that produces incorrect results. The correct response to "engine A reports 8 cycles, engine B reports 11" is to fix the 3 false cycles in engine B, not to document why the difference is okay. ## Codegraph Workflow