From 41224bc983f586fac23a0c35800030179ec2783e Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 15:15:51 -0600 Subject: [PATCH 01/61] =?UTF-8?q?feat(skill):=20add=20/create-skill=20?= =?UTF-8?q?=E2=80=94=20skill=20factory=20with=20quality=20gates?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Encodes lessons from 200+ Greptile review comments across the titan skill PRs into a reusable skill-creation workflow. Includes 10 mandatory patterns (no shell vars across fences, no silent failures, stable cross-references, etc.) and a 17-item self-review checklist that must pass before any new skill can be committed. --- .claude/skills/create-skill/SKILL.md | 295 +++++++++++++++++++++++++++ 1 file changed, 295 insertions(+) create mode 100644 .claude/skills/create-skill/SKILL.md diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md new file mode 100644 index 00000000..fdd1cd98 --- /dev/null +++ b/.claude/skills/create-skill/SKILL.md @@ -0,0 +1,295 @@ +--- +name: create-skill +description: Scaffold, write, and validate a new Claude Code skill — enforces quality standards derived from 200+ 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 200+ 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 + +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:** All 7 questions have answers. Purpose, arguments, phases, tools, artifacts, dangerous ops, and resume support are defined. + +--- + +## Phase 1 — Scaffold + +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 + +1. Confirm environment (repo root, node version, required tools) +2. Parse `$ARGUMENTS` into state variables +3. Validate preconditions + +## Phase N — <Name> + +<Steps> + +## 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. **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 **mandatory patterns** (derived from the top 10 Greptile review findings): + +### 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 +TMPDIR=$(mktemp -d) +``` +Later: +```bash +rm -rf $TMPDIR # BUG: $TMPDIR is empty here +``` +```` + +**Correct:** Persist state to a file: +````markdown +```bash +mktemp -d > .codegraph/$SKILL_NAME/.tmpdir +``` +Later: +```bash +rm -rf "$(cat .codegraph/$SKILL_NAME/.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 --suffix=.js) +if git show HEAD:$FILE > "$PREV_FILE" 2>&1; then + codegraph where --file "$PREV_FILE" +else + echo "WARN: $FILE is new (not in HEAD) — skipping before/after comparison" +fi +rm -f "$PREV_FILE" +``` +```` + +### 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 --suffix=.js # NOT just mktemp +``` + +### 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: +```bash +if [ -f "pnpm-lock.yaml" ]; then TEST_CMD="pnpm test" +elif [ -f "yarn.lock" ]; then TEST_CMD="yarn test" +else TEST_CMD="npm test"; fi +``` +Then run: `$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: "Using <result> from Phase: <Name>". + +### 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 + +**Exit condition:** Every phase body in the SKILL.md follows all 10 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 + +**Exit condition:** Every dangerous operation identified in Phase: Discovery has a corresponding guard in the SKILL.md. + +### 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 the same way (check for `biome.json` → `npx biome check`, `eslint.config.*` → `npx eslint`, fallback → `npm run lint`) + +--- + +## 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 (the top 10): +- [ ] **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 + +### 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 + +Read through the entire SKILL.md one more time after checking all items. Fix anything found. + +--- + +## Phase 5 — Finalize + +1. Read the final SKILL.md end-to-end and confirm it passes all Phase 4 checks +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` +- Commit: `feat(skill): add /$SKILL_NAME skill` + +--- + +## 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.** Phase 4 is 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` — detect the package manager first. +- **Every `2>/dev/null` needs a justification comment** in the generated skill. From 923b51b99252f591ecb99125bf022689afbf529b Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 15:25:42 -0600 Subject: [PATCH 02/61] fix(skill): address review feedback on /create-skill SKILL.md (#587) - Pattern 6 example: keep TEST_CMD assignment and usage in same block to avoid violating Pattern 1 (no cross-fence variables) - Phase 3: move "For code modifications" section above exit condition so the gate covers all Phase 3 content - Phase 3: replace prose lint detection with explicit bash script to follow Pattern 6's own guidance (no placeholders) --- .claude/skills/create-skill/SKILL.md | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index fdd1cd98..a033a918 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -174,13 +174,13 @@ Every variable or placeholder in pseudocode must have a preceding assignment. If **Wrong:** "Run `<detected-test-command>`" **Correct:** ````markdown -Detect the test runner: +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" else TEST_CMD="npm test"; fi +$TEST_CMD ``` -Then run: `$TEST_CMD` ```` ### Pattern 7: No internal contradictions @@ -223,11 +223,17 @@ If the skill performs dangerous operations (from Phase 0 discovery), add explici - Handle network failures gracefully (don't crash the pipeline) - Add timeout limits -**Exit condition:** Every dangerous operation identified in Phase: Discovery has a corresponding guard in the SKILL.md. - ### 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 the same way (check for `biome.json` → `npx biome check`, `eslint.config.*` → `npx eslint`, fallback → `npm run lint`) +- Run lint after changes: detect lint runner: + ```bash + if [ -f "biome.json" ]; then LINT_CMD="npx biome check" + elif ls eslint.config.* 2>/dev/null | grep -q .; then LINT_CMD="npx eslint ." + else LINT_CMD="npm run lint"; fi + $LINT_CMD + ``` + +**Exit condition:** Every dangerous operation identified in Phase: Discovery has a corresponding guard in the SKILL.md. --- From cd5729c55d352d9ea9528be0a68d2f06b796a7e3 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 15:40:37 -0600 Subject: [PATCH 03/61] feat(skill): add smoke test phase, patterns 11-13, and safety checklist items to /create-skill --- .claude/skills/create-skill/SKILL.md | 97 ++++++++++++++++++++++++++-- 1 file changed, 93 insertions(+), 4 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index a033a918..40e7430a 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -202,7 +202,41 @@ If the skill supports `--start-from` or `--skip-*`: - 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 -**Exit condition:** Every phase body in the SKILL.md follows all 10 patterns. No wrong/correct examples remain as actual instructions — only the correct versions. +### Pattern 11: Progress indicators + +For any phase that takes longer than ~10 seconds (file iteration, API calls, batch operations), emit progress: + +```bash +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: + +````markdown +```bash +if [ -f ".codegraph/$SKILL_NAME/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"` instead of glob expansion (`ls *.ext`) which differs between bash versions +- Use `mktemp` without `-p` (macOS `mktemp` requires a template argument differently than GNU) +- Use `sed -i.bak` instead of `sed -i ''` (GNU vs BSD incompatibility) +- Document any platform-specific behavior with a comment: `# NOTE: requires GNU coreutils` + +**Exit condition:** Every phase body in the SKILL.md follows all 13 patterns. No wrong/correct examples remain as actual instructions — only the correct versions. --- @@ -271,14 +305,69 @@ Before finalizing, audit the SKILL.md against every item below. **Do not skip an - [ ] **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 tools listed in `allowed-tools` are available before starting work. "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) +- [ ] **Progress indicators**: Phases that iterate over files or run batch operations emit progress (`Processing $i/$total`) +- [ ] **Platform portability**: No `sed -i ''`, no unquoted globs, no GNU-only flags without fallback or documentation Read through the entire SKILL.md one more time after checking all items. Fix anything found. --- -## Phase 5 — Finalize +## 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: + +### Shell syntax validation + +Extract every bash code block from the SKILL.md and check for syntax errors: + +```bash +# For each ```bash block in the skill: +bash -n <<'BLOCK' + # paste the block contents here +BLOCK +``` + +Fix any syntax errors found before proceeding. + +### 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) +cd "$TEST_DIR" +git init +# Simulate the Phase 0 checks from the skill here +cd - +rm -rf "$TEST_DIR" +``` + +### 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 4 checks +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?" @@ -291,7 +380,7 @@ If the user approves: ## 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.** Phase 4 is mandatory, not optional. +- **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. From f356b68c6ca45263bdd89a12659e4c96e26bf64b Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 15:47:17 -0600 Subject: [PATCH 04/61] fix(skill): address round-2 Greptile review on /create-skill (#587) - Add 2>/dev/null justification comment to lint detection template (Pattern 2 compliance) - Replace $SKILL_NAME with concrete example in Pattern 1 to avoid cross-fence ambiguity - Add extension-adjustment comment to Pattern 2 temp file example --- .claude/skills/create-skill/SKILL.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 40e7430a..ab24cf5d 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -109,14 +109,14 @@ rm -rf $TMPDIR # BUG: $TMPDIR is empty here ``` ```` -**Correct:** Persist state to a file: +**Correct:** Persist state to a file (use your actual skill name, not a variable): ````markdown ```bash -mktemp -d > .codegraph/$SKILL_NAME/.tmpdir +mktemp -d > .codegraph/deploy-check/.tmpdir ``` Later: ```bash -rm -rf "$(cat .codegraph/$SKILL_NAME/.tmpdir)" +rm -rf "$(cat .codegraph/deploy-check/.tmpdir)" ``` ```` @@ -138,7 +138,7 @@ git show HEAD:$FILE 2>/dev/null | codegraph where --file - **Correct:** ````markdown ```bash -PREV_FILE=$(mktemp --suffix=.js) +PREV_FILE=$(mktemp --suffix=.js) # adjust extension to match the language of $FILE if git show HEAD:$FILE > "$PREV_FILE" 2>&1; then codegraph where --file "$PREV_FILE" else @@ -262,7 +262,7 @@ If the skill performs dangerous operations (from Phase 0 discovery), add explici - Run lint after changes: detect lint runner: ```bash if [ -f "biome.json" ]; then LINT_CMD="npx biome check" - elif ls eslint.config.* 2>/dev/null | grep -q .; then LINT_CMD="npx eslint ." + elif ls eslint.config.* 2>/dev/null | grep -q .; then LINT_CMD="npx eslint ." # 2>/dev/null: ls exits non-zero when glob matches nothing — intentionally tolerant else LINT_CMD="npm run lint"; fi $LINT_CMD ``` From e912c672757f5607291b1b24b3fe5c4bd8823067 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 16:09:14 -0600 Subject: [PATCH 05/61] docs: add never-fabricate-facts rule to CLAUDE.md Adds a top-level callout instructing Claude to always verify factual claims (licenses, versions, features) before stating them. --- CLAUDE.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 6d441c78..9e2fb314 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. + ## Codegraph Workflow Hooks handle: file-level deps on reads, graph rebuild after edits, commit-time checks (cycles, dead exports, diff-impact, lint). **You must actively run these for function-level understanding:** From fb08cbda168271d42656bb82592cd58aaefce25e Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 16:41:23 -0600 Subject: [PATCH 06/61] fix(skill): address round-3 Greptile review feedback (#587) - Replace "top 10" with accurate "all 13 patterns" in Phase 2 intro and checklist header since 13 patterns are defined - Replace ambiguous $SKILL_NAME with concrete deploy-check in Pattern 12 example (same fix as Pattern 1 in previous round) - Add Patterns 11-13 (progress, artifact reuse, portability) to Phase 4 anti-pattern checklist and remove duplicates from Safety checks - Add missing Examples section with 3 realistic usage examples (required by own Phase 4 completeness checklist) --- .claude/skills/create-skill/SKILL.md | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index ab24cf5d..5e3551b8 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -92,7 +92,7 @@ allowed-tools: <from user's tool list> ## Phase 2 — Write the Skill Body -Write each phase following these **mandatory patterns** (derived from the top 10 Greptile review findings): +Write each phase following these **mandatory patterns** (derived from Greptile review findings across 200+ comments): ### Pattern 1: No shell variables across code fences @@ -214,11 +214,11 @@ 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: +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/$SKILL_NAME/results.json" ]; then +if [ -f ".codegraph/deploy-check/results.json" ]; then echo "Using cached results from previous run" else # run expensive operation @@ -283,7 +283,7 @@ Before finalizing, audit the SKILL.md against every item below. **Do not skip an - [ ] Rules section exists at the bottom - [ ] Every phase has a clear name (not just a number) -### Anti-pattern checks (the top 10): +### Anti-pattern checks (all 13 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 @@ -294,6 +294,9 @@ Before finalizing, audit the SKILL.md against every item below. **Do not skip an - [ ] **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 ### Robustness checks: - [ ] **Rollback paths**: Every destructive operation has documented undo instructions @@ -312,8 +315,6 @@ Before finalizing, audit the SKILL.md against every item below. **Do not skip an - [ ] **Dependency validation**: Phase 0 verifies all tools listed in `allowed-tools` are available before starting work. "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) -- [ ] **Progress indicators**: Phases that iterate over files or run batch operations emit progress (`Processing $i/$total`) -- [ ] **Platform portability**: No `sed -i ''`, no unquoted globs, no GNU-only flags without fallback or documentation Read through the entire SKILL.md one more time after checking all items. Fix anything found. @@ -377,6 +378,14 @@ If the user approves: --- +## 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. From 98abfb2b49ec003565d74b5831de5af3b892e663 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 17:07:44 -0600 Subject: [PATCH 07/61] fix(skill): replace glob expansion with find and add cleanup trap (#587) - Phase 3 lint template: replace `ls eslint.config.*` with `find . -maxdepth 1 -name "eslint.config.*"` to comply with Pattern 13 (platform portability) - Phase 5 smoke test: add `trap` guard for cleanup so early exits don't leave the shell in a foreign directory or temp files on disk --- .claude/skills/create-skill/SKILL.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 5e3551b8..4da8d4de 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -262,7 +262,7 @@ If the skill performs dangerous operations (from Phase 0 discovery), add explici - Run lint after changes: detect lint runner: ```bash if [ -f "biome.json" ]; then LINT_CMD="npx biome check" - elif ls eslint.config.* 2>/dev/null | grep -q .; then LINT_CMD="npx eslint ." # 2>/dev/null: ls exits non-zero when glob matches nothing — intentionally tolerant + elif find . -maxdepth 1 -name "eslint.config.*" -print -quit 2>/dev/null | grep -q .; then LINT_CMD="npx eslint ." # 2>/dev/null: find exits non-zero when path is unreadable — intentionally tolerant else LINT_CMD="npm run lint"; fi $LINT_CMD ``` @@ -346,11 +346,13 @@ Run the skill's Phase 0 (pre-flight) logic in a temporary test directory to veri ```bash TEST_DIR=$(mktemp -d) +trap 'cd - > /dev/null 2>&1; rm -rf "$TEST_DIR"' EXIT cd "$TEST_DIR" git init # Simulate the Phase 0 checks from the skill here cd - rm -rf "$TEST_DIR" +trap - EXIT ``` ### Idempotency check From 5abff3848bce3fb6eb9e41799b6b0b05b10e0158 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 17:43:39 -0600 Subject: [PATCH 08/61] fix(skill): add Phase 0 pre-flight checks and fix Pattern 9 placeholders (#587) --- .claude/skills/create-skill/SKILL.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 4da8d4de..426d6ea9 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -18,9 +18,11 @@ Set `SKILL_NAME` to the provided name. Validate it is kebab-case (`^[a-z][a-z0-9 --- -## Phase 0 — Discovery +## Phase 0 — Discovery & Pre-flight -Before writing anything, gather requirements interactively. Ask the user these questions (all at once, not one-by-one): +**Pre-flight:** Confirm you are in a git repository root (`git rev-parse --show-toplevel` should succeed). Validate `$ARGUMENTS` is set and matches kebab-case (`^[a-z][a-z0-9]*(-[a-z0-9]+)*$`). If not, 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`, `<path>`) @@ -32,7 +34,7 @@ Before writing anything, gather requirements interactively. Ask the user these q **Wait for the user's answers before proceeding.** Do not guess or assume. -**Exit condition:** All 7 questions have answers. Purpose, arguments, phases, tools, artifacts, dangerous ops, and resume support are defined. +**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. --- @@ -193,7 +195,7 @@ Every codegraph command or tool invocation in the procedure must be permitted by ### 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: "Using <result> from Phase: <Name>". +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 From 88955d05ff98d07ae867f2fefa9dd46269a54cf9 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 18:15:30 -0600 Subject: [PATCH 09/61] fix(skill): address round-6 Greptile review on /create-skill (#587) - Remove unnecessary 2>/dev/null from find command in lint template - Deduplicate kebab-case validation (Phase 0 now references Arguments section) - Suppress cd - stdout in smoke test template --- .claude/skills/create-skill/SKILL.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 426d6ea9..2a03d5d6 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -20,7 +20,7 @@ Set `SKILL_NAME` to the provided name. Validate it is kebab-case (`^[a-z][a-z0-9 ## Phase 0 — Discovery & Pre-flight -**Pre-flight:** Confirm you are in a git repository root (`git rev-parse --show-toplevel` should succeed). Validate `$ARGUMENTS` is set and matches kebab-case (`^[a-z][a-z0-9]*(-[a-z0-9]+)*$`). If not, abort with a clear error. +**Pre-flight:** Confirm you are in a git repository root (`git rev-parse --show-toplevel` should succeed). 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): @@ -264,7 +264,7 @@ If the skill performs dangerous operations (from Phase 0 discovery), add explici - 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.*" -print -quit 2>/dev/null | grep -q .; then LINT_CMD="npx eslint ." # 2>/dev/null: find exits non-zero when path is unreadable — intentionally tolerant + elif find . -maxdepth 1 -name "eslint.config.*" -print -quit | grep -q .; then LINT_CMD="npx eslint ." else LINT_CMD="npm run lint"; fi $LINT_CMD ``` @@ -352,7 +352,7 @@ trap 'cd - > /dev/null 2>&1; rm -rf "$TEST_DIR"' EXIT cd "$TEST_DIR" git init # Simulate the Phase 0 checks from the skill here -cd - +cd - > /dev/null rm -rf "$TEST_DIR" trap - EXIT ``` From 659e60d3a39ed1148b42f83a215fd03697c94772 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 18:32:49 -0600 Subject: [PATCH 10/61] fix(skill): address round-7 Greptile review on /create-skill (#587) - Add idempotency guard to Phase 1: check for existing SKILL.md before overwriting, with warning and confirmation prompt - Add mkdir -p to Pattern 1 "Correct" example so parent directory exists before redirect (prevents silent failure) - Fix inconsistent stderr suppression: explicit cleanup path now uses cd - > /dev/null 2>&1 matching the trap line --- .claude/skills/create-skill/SKILL.md | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 2a03d5d6..0db31649 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -40,6 +40,16 @@ Set `SKILL_NAME` to the provided name. Validate it is kebab-case (`^[a-z][a-z0-9 ## 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 or abort." + # Prompt user for confirmation before continuing +fi +``` + Create the skill directory and SKILL.md with frontmatter: ```bash @@ -111,9 +121,11 @@ rm -rf $TMPDIR # BUG: $TMPDIR is empty here ``` ```` -**Correct:** Persist state to a file (use your actual skill name, not a variable): +**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 > .codegraph/deploy-check/.tmpdir ``` Later: @@ -352,7 +364,7 @@ trap 'cd - > /dev/null 2>&1; rm -rf "$TEST_DIR"' EXIT cd "$TEST_DIR" git init # Simulate the Phase 0 checks from the skill here -cd - > /dev/null +cd - > /dev/null 2>&1 rm -rf "$TEST_DIR" trap - EXIT ``` From e0945ed43fe22072bcd0b2e0ff28edc344dd3f3e Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 18:40:14 -0600 Subject: [PATCH 11/61] fix(skill): address round-8 Greptile review on /create-skill (#587) - Replace GNU-only mktemp --suffix with POSIX template syntax in Pattern 2 and Pattern 3 examples (portable: macOS + Linux) - Update Pattern 13 to document --suffix as GNU-only alongside -p - Add tool dependency validation (git, mktemp, bash) to Phase 0 pre-flight, satisfying Phase 4 "Dependency validation" checklist item --- .claude/skills/create-skill/SKILL.md | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 0db31649..62a3f2a5 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -20,7 +20,15 @@ Set `SKILL_NAME` to the provided name. Validate it is kebab-case (`^[a-z][a-z0-9 ## Phase 0 — Discovery & Pre-flight -**Pre-flight:** Confirm you are in a git repository root (`git rev-parse --show-toplevel` should succeed). Parse `$ARGUMENTS` per the Arguments section above. If validation fails, abort with a clear error. +**Pre-flight:** Verify required tools and environment: + +```bash +for tool in git mktemp bash; do + command -v "$tool" > /dev/null 2>&1 || { echo "ERROR: required tool '$tool' not found"; exit 1; } +done +``` + +Confirm you are in a git repository root (`git rev-parse --show-toplevel` should succeed). 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): @@ -152,7 +160,7 @@ git show HEAD:$FILE 2>/dev/null | codegraph where --file - **Correct:** ````markdown ```bash -PREV_FILE=$(mktemp --suffix=.js) # adjust extension to match the language of $FILE +PREV_FILE=$(mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX.js") # adjust extension to match the language of $FILE; template syntax is portable (macOS + Linux) if git show HEAD:$FILE > "$PREV_FILE" 2>&1; then codegraph where --file "$PREV_FILE" else @@ -167,7 +175,7 @@ rm -f "$PREV_FILE" Codegraph's language detection is extension-based. Temp files passed to codegraph must have the correct extension: ```bash -mktemp --suffix=.js # NOT just mktemp +mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX.js" # NOT just mktemp — template syntax is cross-platform (macOS + Linux) ``` ### Pattern 4: No hardcoded temp paths @@ -246,7 +254,7 @@ This supports both idempotent re-runs and resume-after-failure. Avoid shell constructs that behave differently across platforms: - Use `find ... -name "*.ext"` instead of glob expansion (`ls *.ext`) which differs between bash versions -- Use `mktemp` without `-p` (macOS `mktemp` requires a template argument differently than GNU) +- 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` From ec44ec85ef9feddbaddfbcb631cbe140f8050978 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 18:48:48 -0600 Subject: [PATCH 12/61] fix(skill): address round-9 Greptile review on /create-skill (#587) - Remove tautological 'command -v bash' from Phase 0 pre-flight - Add explicit abort instruction to idempotency guard (STOP + exit 1) - Fix dependency validation checklist: check shell commands used in bash blocks, not Claude Code allowed-tools --- .claude/skills/create-skill/SKILL.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 62a3f2a5..5fcb620f 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -23,7 +23,7 @@ Set `SKILL_NAME` to the provided name. Validate it is kebab-case (`^[a-z][a-z0-9 **Pre-flight:** Verify required tools and environment: ```bash -for tool in git mktemp bash; do +for tool in git mktemp; do command -v "$tool" > /dev/null 2>&1 || { echo "ERROR: required tool '$tool' not found"; exit 1; } done ``` @@ -53,8 +53,8 @@ Confirm you are in a git repository root (`git rev-parse --show-toplevel` should ```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 or abort." - # Prompt user for confirmation before continuing + 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 ``` @@ -334,7 +334,7 @@ Before finalizing, audit the SKILL.md against every item below. **Do not skip an ### 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 tools listed in `allowed-tools` are available before starting work. "Command not found" is caught before Phase 2, not during Phase 3 +- [ ] **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) From 07144c7ef1c20f9c5b23da767ec472066a660d62 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 22:17:23 -0600 Subject: [PATCH 13/61] fix(skill): address round-10 Greptile review feedback (#587) - Add package.json guard to lint fallback so non-Node.js projects degrade gracefully instead of failing with npm errors - Use mktemp template syntax in Phase 5 smoke test for Pattern 13 consistency (portable macOS + Linux) - Replace Node.js-specific "node version" with language-agnostic "required runtime/toolchain version" in scaffolded Phase 0 template --- .claude/skills/create-skill/SKILL.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 5fcb620f..e169a16a 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -85,7 +85,7 @@ allowed-tools: <from user's tool list> ## Phase 0 — Pre-flight -1. Confirm environment (repo root, node version, required tools) +1. Confirm environment (repo root, required runtime/toolchain version, required tools) 2. Parse `$ARGUMENTS` into state variables 3. Validate preconditions @@ -285,7 +285,8 @@ If the skill performs dangerous operations (from Phase 0 discovery), add explici ```bash if [ -f "biome.json" ]; then LINT_CMD="npx biome check" elif find . -maxdepth 1 -name "eslint.config.*" -print -quit | grep -q .; then LINT_CMD="npx eslint ." - else LINT_CMD="npm run lint"; fi + 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 ``` @@ -367,7 +368,7 @@ Run the skill's Phase 0 (pre-flight) logic in a temporary test directory to veri - Environment validation produces clear error messages on failure ```bash -TEST_DIR=$(mktemp -d) +TEST_DIR=$(mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX") trap 'cd - > /dev/null 2>&1; rm -rf "$TEST_DIR"' EXIT cd "$TEST_DIR" git init From d577f38992a02a64f84d8a279814b672b87057ab Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 22:28:09 -0600 Subject: [PATCH 14/61] fix(skill): add context comments for undefined loop variables in examples (#587) - Pattern 2 example: add comment noting $FILE is set by surrounding loop - Pattern 11 example: add comment noting $i, $total, $FILE are loop vars --- .claude/skills/create-skill/SKILL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index e169a16a..45de86ce 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -161,6 +161,7 @@ git show HEAD:$FILE 2>/dev/null | codegraph where --file - ````markdown ```bash PREV_FILE=$(mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX.js") # adjust extension to match the language of $FILE; template syntax is portable (macOS + Linux) +# $FILE is expected to be set by the surrounding loop, e.g. for FILE in $(git diff --name-only HEAD); do ... done if git show HEAD:$FILE > "$PREV_FILE" 2>&1; then codegraph where --file "$PREV_FILE" else @@ -229,6 +230,7 @@ If the skill supports `--start-from` or `--skip-*`: 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" ``` From 2d976c040fc6a3d8dc799faa6f81a8ccd2f4e26c Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 22:32:54 -0600 Subject: [PATCH 15/61] fix(skill): add package.json guard to test runner detection in Pattern 6 (#587) Mirror the lint runner fix: add package.json guard before npm test fallback so non-Node.js projects get a warning and graceful skip. Resolves Pattern 7/8 self-contradiction with Rules section. --- .claude/skills/create-skill/SKILL.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 45de86ce..4ad83e82 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -201,7 +201,8 @@ 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" -else TEST_CMD="npm test"; fi +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 ``` ```` From 39493f50afeb61a6d382e76aa165082f76dd135a Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Tue, 24 Mar 2026 23:23:56 -0600 Subject: [PATCH 16/61] fix(skill): add git repo validation to pre-flight bash block and quiet git init (#587) Move git rev-parse check from prose into the executable pre-flight bash block with explicit error handling. Add --quiet flag to git init in the Phase 5 smoke test to suppress noisy output. --- .claude/skills/create-skill/SKILL.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 4ad83e82..9672c5e4 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -26,9 +26,10 @@ Set `SKILL_NAME` to the provided name. Validate it is kebab-case (`^[a-z][a-z0-9 for tool in git mktemp; do command -v "$tool" > /dev/null 2>&1 || { echo "ERROR: required tool '$tool' not found"; exit 1; } done +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; } ``` -Confirm you are in a git repository root (`git rev-parse --show-toplevel` should succeed). Parse `$ARGUMENTS` per the Arguments section above. If validation fails, abort with a clear error. +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): @@ -374,7 +375,7 @@ Run the skill's Phase 0 (pre-flight) logic in a temporary test directory to veri TEST_DIR=$(mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX") trap 'cd - > /dev/null 2>&1; rm -rf "$TEST_DIR"' EXIT cd "$TEST_DIR" -git init +git init --quiet # Simulate the Phase 0 checks from the skill here cd - > /dev/null 2>&1 rm -rf "$TEST_DIR" From 815758de218c13a9b3107767894b6be777758a01 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 00:19:57 -0600 Subject: [PATCH 17/61] fix(skill): remove GNU-only -quit from find in lint template (#587) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop `-print -quit` from the lint detection `find` command — `-quit` is a GNU extension undocumented in Pattern 13, and `grep -q` already short-circuits on the first match, making `-quit` redundant. --- .claude/skills/create-skill/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 9672c5e4..380495cf 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -288,7 +288,7 @@ If the skill performs dangerous operations (from Phase 0 discovery), add explici - 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.*" -print -quit | grep -q .; then LINT_CMD="npx eslint ." + 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 From 90d10b5ad0c6fd74c38bd26b7867bfda8ecc27c0 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 00:38:33 -0600 Subject: [PATCH 18/61] fix(skill): add exit-condition placeholders to scaffold template and align mktemp syntax (#587) --- .claude/skills/create-skill/SKILL.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 380495cf..cacb94a6 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -90,10 +90,14 @@ allowed-tools: <from user's tool list> 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> + ## Rules - <Hard constraints> @@ -135,7 +139,7 @@ First ensure the directory exists: ````markdown ```bash mkdir -p .codegraph/deploy-check -mktemp -d > .codegraph/deploy-check/.tmpdir +mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX" > .codegraph/deploy-check/.tmpdir ``` Later: ```bash From 60156252a9f54c1e097b29357f4380fdb7544468 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 01:14:58 -0600 Subject: [PATCH 19/61] feat(skill): add lint and smoke-test scripts for SKILL.md validation (#587) Two automated validation scripts that catch the most common issues found across 250+ Greptile review comments: - lint-skill.sh: checks cross-fence variables, bare 2>/dev/null, git add ., hardcoded npm test, sed -i portability, missing frontmatter/Phase 0/Rules, missing exit conditions, find -quit, and hardcoded /tmp paths - smoke-test-skill.sh: extracts bash blocks (skipping example regions) and runs bash -n syntax checking on each --- .../skills/create-skill/scripts/lint-skill.sh | 225 ++++++++++++++++++ .../create-skill/scripts/smoke-test-skill.sh | 89 +++++++ 2 files changed, 314 insertions(+) create mode 100644 .claude/skills/create-skill/scripts/lint-skill.sh create mode 100644 .claude/skills/create-skill/scripts/smoke-test-skill.sh 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..9ba2bd2f --- /dev/null +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -0,0 +1,225 @@ +#!/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 + +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 +awk ' + /^````/ { quad = !quad; next } + quad { next } + /^```bash/ { inblock = 1; blocknum++; next } + /^```/ && inblock { inblock = 0; next } + inblock { print blocknum "\t" $0 } +' "$SKILL_FILE" > "$BLOCKS_FILE" + +# Collect variable assignments per block +declare -A VAR_BLOCK +while IFS=$'\t' read -r bnum line; do + # Match UPPER_CASE_VAR= assignments (skip lowercase/mixed to reduce false positives) + for var in $(echo "$line" | grep -oE '\b[A-Z][A-Z0-9_]+=' | sed 's/=$//'); do + VAR_BLOCK["$var"]="$bnum" + done +done < "$BLOCKS_FILE" + +# Check for references in later blocks without file persistence +while IFS=$'\t' read -r bnum line; do + 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 -qF "\$${var}" || echo "$line" | grep -qF "\${${var}}"; then + # Check if the same block also assigns it (re-assignment is fine) + reassigned=false + while IFS=$'\t' read -r bn2 ln2; do + if [ "$bn2" = "$bnum" ] && echo "$ln2" | grep -qF "${var}="; then + reassigned=true + break + fi + done < "$BLOCKS_FILE" + if [ "$reassigned" = false ]; 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 +while IFS= read -r line; do + line_num=$((line_num + 1)) + case "$line" in + '````'*) if $in_quad; then in_quad=false; else in_quad=true; fi; continue ;; + esac + $in_quad && continue + case "$line" in + '```bash'*) in_block=true; continue ;; + '```'*) in_block=false; continue ;; + esac + if $in_block && echo "$line" | grep -qE '2>/dev/null'; then + # Check same line or previous line for justification comment + if ! echo "$line" | grep -qiE '#.*intentional|#.*tolera|#.*acceptable|#.*expected|#.*safe to ignore|#.*may fail|#.*optional|#.*fallback|#.*portable'; then + warn "Line $line_num: '2>/dev/null' without justification comment (Pattern 2)" + fi + fi +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 (\.|(-A|--all))'; 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 +while IFS= read -r line; do + line_num=$((line_num + 1)) + case "$line" in + '````'*) if $in_quad; then in_quad=false; else in_quad=true; fi; continue ;; + esac + $in_quad && continue + case "$line" in + '```bash'*) in_block=true; in_detect=false; continue ;; + '```'*) in_block=false; in_detect=false; continue ;; + esac + if $in_block; then + # Track if we're inside an if/elif chain (detection block) + if echo "$line" | grep -qE '^\s*(if|elif)\s.*(-f\s|lock|package)'; then + in_detect=true + elif echo "$line" | grep -qE '^\s*fi\b'; then + in_detect=false + fi + if ! $in_detect; then + if echo "$line" | grep -qE '^\s*(npm test|npm run lint)\b'; 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 9: Missing exit conditions between phases ────────────────── +prev_phase="" +phase_has_exit=true +in_fence=false +while IFS= read -r line; do + # Skip content inside any code fence (``` or ````) + case "$line" in + '````'*|'```'*) + if $in_fence; then in_fence=false; else in_fence=true; fi + continue + ;; + esac + $in_fence && 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)) + case "$line" in + '````'*) if $in_quad; then in_quad=false; else in_quad=true; fi; continue ;; + esac + $in_quad && continue + case "$line" 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][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..5e9a2437 --- /dev/null +++ b/.claude/skills/create-skill/scripts/smoke-test-skill.sh @@ -0,0 +1,89 @@ +#!/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 + +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)) + + # Track quadruple-backtick regions (example pairs) + case "$line" 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 "$line" 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 bash -n "$TMPBLOCK" 2>/dev/null; then + echo " OK block at line $block_start" + PASS=$((PASS + 1)) + else + echo " FAIL block at line $block_start:" + bash -n "$TMPBLOCK" 2>&1 | 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" + +echo "" +echo "smoke-test-skill: $PASS passed, $FAIL failed, $SKIP skipped" +if [ "$FAIL" -gt 0 ]; then + exit 1 +fi +exit 0 From ad503b3b183a2a242bb5f0a768371a69477a96c9 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 01:15:17 -0600 Subject: [PATCH 20/61] feat(skill): add 4 patterns and 5 checklist items to create-skill (#587) Analysis of 274 Greptile comments across PRs #565, #557, #587 revealed 4 recurring issue categories not covered by the existing 13 patterns: - Pattern 14: Trap-based cleanup (temp files leaked on error) - Pattern 15: Git stash safety (named STASH_REF, not $? or stash@{0}) - Pattern 16: Division-by-zero guards (ratio/percentage computations) - Pattern 17: DRY_RUN consistency (check at point of action, not phase entry) Also adds 5 matching checklist items in Phase 4 (Self-Review) and integrates lint-skill.sh + smoke-test-skill.sh into Phase 5 (Smoke Test) for automated validation of generated skills. --- .claude/skills/create-skill/SKILL.md | 128 ++++++++++++++++++++++++--- 1 file changed, 115 insertions(+), 13 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index cacb94a6..4e62b1f3 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -1,13 +1,13 @@ --- name: create-skill -description: Scaffold, write, and validate a new Claude Code skill — enforces quality standards derived from 200+ review comments +description: Scaffold, write, and validate a new Claude Code skill — enforces quality standards derived from 250+ review comments argument-hint: "<skill-name> (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 200+ Greptile review comments to prevent the most common skill authoring mistakes. +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 @@ -117,7 +117,7 @@ allowed-tools: <from user's tool list> ## Phase 2 — Write the Skill Body -Write each phase following these **mandatory patterns** (derived from Greptile review findings across 200+ comments): +Write each phase following these **17 mandatory patterns** (derived from Greptile review findings across 250+ comments): ### Pattern 1: No shell variables across code fences @@ -266,7 +266,103 @@ Avoid shell constructs that behave differently across platforms: - Use `sed -i.bak` instead of `sed -i ''` (GNU vs BSD incompatibility) - Document any platform-specific behavior with a comment: `# NOTE: requires GNU coreutils` -**Exit condition:** Every phase body in the SKILL.md follows all 13 patterns. No wrong/correct examples remain as actual instructions — only the correct versions. +### 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") +trap 'cd - > /dev/null 2>&1; rm -rf "$WORK_DIR"' 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: +````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. +# Use [ -n "$STASH_REF" ] for all branching. Use $STASH_REF (not stash@{0}) in pop/drop. +``` +Later: +```bash +[ -n "$STASH_REF" ] && git stash pop "$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 +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 +# 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. --- @@ -314,7 +410,7 @@ Before finalizing, audit the SKILL.md against every item below. **Do not skip an - [ ] Rules section exists at the bottom - [ ] Every phase has a clear name (not just a number) -### Anti-pattern checks (all 13 patterns): +### 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 @@ -328,6 +424,10 @@ Before finalizing, audit the SKILL.md against every item below. **Do not skip an - [ ] **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 @@ -346,6 +446,7 @@ Before finalizing, audit the SKILL.md against every item below. **Do not skip an - [ ] **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. @@ -355,18 +456,19 @@ Read through the entire SKILL.md one more time after checking all items. Fix any 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: -### Shell syntax validation +### Automated validation -Extract every bash code block from the SKILL.md and check for syntax errors: +Run both validation scripts against the generated SKILL.md: ```bash -# For each ```bash block in the skill: -bash -n <<'BLOCK' - # paste the block contents here -BLOCK +bash .claude/skills/create-skill/scripts/lint-skill.sh .claude/skills/$SKILL_NAME/SKILL.md +bash .claude/skills/create-skill/scripts/smoke-test-skill.sh .claude/skills/$SKILL_NAME/SKILL.md ``` -Fix any syntax errors found before proceeding. +- **`lint-skill.sh`** checks for cross-fence variable bugs, bare `2>/dev/null`, hardcoded `npm test`, `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 @@ -406,7 +508,7 @@ Document any idempotency fix applied. 3. Ask: "Ready to commit, or want changes?" If the user approves: -- Stage only `.claude/skills/$SKILL_NAME/SKILL.md` +- 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` --- From a73d0650ca6e7e6e26b60defabaab972f542ff23 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 10:58:37 -0600 Subject: [PATCH 21/61] fix(skill): address Greptile review feedback on lint and smoke-test scripts - Fix Check 9 fence tracking: use separate in_quad/in_block flags instead of single in_fence boolean that corrupted state on nested fences (P1) - Store earliest variable assignment in VAR_BLOCK to catch intermediate cross-fence references (P2) - Implement previous-line justification check for 2>/dev/null (P2) - Add bash 4+ version guard for declare -A compatibility (P2) - Capture bash -n stderr on first invocation to avoid double execution (P2) --- .../skills/create-skill/scripts/lint-skill.sh | 40 +++++++++++++------ .../create-skill/scripts/smoke-test-skill.sh | 4 +- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 9ba2bd2f..d0486ed7 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -5,6 +5,11 @@ 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 @@ -38,8 +43,11 @@ awk ' declare -A VAR_BLOCK while IFS=$'\t' read -r bnum line; do # Match UPPER_CASE_VAR= assignments (skip lowercase/mixed to reduce false positives) + # Store the earliest block that assigns each variable so intermediate references are caught for var in $(echo "$line" | grep -oE '\b[A-Z][A-Z0-9_]+=' | sed 's/=$//'); do - VAR_BLOCK["$var"]="$bnum" + if [ -z "${VAR_BLOCK[$var]+x}" ]; then + VAR_BLOCK["$var"]="$bnum" + fi done done < "$BLOCKS_FILE" @@ -73,22 +81,25 @@ done < "$BLOCKS_FILE" line_num=0 in_quad=false in_block=false +prev_line="" while IFS= read -r line; do line_num=$((line_num + 1)) case "$line" in - '````'*) if $in_quad; then in_quad=false; else in_quad=true; fi; continue ;; + '````'*) if $in_quad; then in_quad=false; else in_quad=true; fi; prev_line="$line"; continue ;; esac - $in_quad && continue + $in_quad && { prev_line="$line"; continue; } case "$line" in - '```bash'*) in_block=true; continue ;; - '```'*) in_block=false; continue ;; + '```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'; then # Check same line or previous line for justification comment - if ! echo "$line" | grep -qiE '#.*intentional|#.*tolera|#.*acceptable|#.*expected|#.*safe to ignore|#.*may fail|#.*optional|#.*fallback|#.*portable'; then + justification_re='#.*intentional|#.*tolera|#.*acceptable|#.*expected|#.*safe to ignore|#.*may fail|#.*optional|#.*fallback|#.*portable' + 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) ─────── @@ -156,16 +167,19 @@ fi # ── Check 9: Missing exit conditions between phases ────────────────── prev_phase="" phase_has_exit=true -in_fence=false +in_quad=false +in_block=false while IFS= read -r line; do - # Skip content inside any code fence (``` or ````) + # Skip content inside quadruple-backtick example regions + case "$line" 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 case "$line" in - '````'*|'```'*) - if $in_fence; then in_fence=false; else in_fence=true; fi - continue - ;; + '```'*) if $in_block; then in_block=false; else in_block=true; fi; continue ;; esac - $in_fence && continue + $in_block && continue if echo "$line" | grep -qE '^## Phase [0-9]'; then if [ -n "$prev_phase" ] && [ "$phase_has_exit" = false ]; then diff --git a/.claude/skills/create-skill/scripts/smoke-test-skill.sh b/.claude/skills/create-skill/scripts/smoke-test-skill.sh index 5e9a2437..e0467a1a 100644 --- a/.claude/skills/create-skill/scripts/smoke-test-skill.sh +++ b/.claude/skills/create-skill/scripts/smoke-test-skill.sh @@ -57,12 +57,12 @@ while IFS= read -r line; do in_block=false # Run syntax check on the collected block if [ -s "$TMPBLOCK" ]; then - if bash -n "$TMPBLOCK" 2>/dev/null; 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:" - bash -n "$TMPBLOCK" 2>&1 | sed 's/^/ /' + echo "$output" | sed 's/^/ /' FAIL=$((FAIL + 1)) fi else From f1da3347157a38ba79ddfefb47ea2a47c5428371 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 10:59:40 -0600 Subject: [PATCH 22/61] fix(skill): add Pattern 2 justification comments to pre-flight suppressions (#587) --- .claude/skills/create-skill/SKILL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index cacb94a6..4f89c8d5 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -25,8 +25,10 @@ Set `SKILL_NAME` to the provided name. Validate it is kebab-case (`^[a-z][a-z0-9 ```bash for tool in git mktemp; do command -v "$tool" > /dev/null 2>&1 || { echo "ERROR: required tool '$tool' not found"; exit 1; } + # > /dev/null 2>&1: suppress command path on success and shell's "not found" on failure — the || clause provides the error message done 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; } +# > /dev/null 2>&1: suppress git's own "fatal: not a git repository" — our || message is more actionable ``` Parse `$ARGUMENTS` per the Arguments section above. If validation fails, abort with a clear error. From 5e06130d5180e1e24b38cf4fbd58e2bb39e40e6e Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 11:59:41 -0600 Subject: [PATCH 23/61] fix(skill): replace 2>&1 with justified 2>/dev/null in Pattern 2 example (#587) Pattern 2's "Correct" example used 2>&1 to redirect git stderr into a temp file that was subsequently deleted, silently swallowing git diagnostics. Replace with 2>/dev/null plus a justification comment per Pattern 2's own rules, and update the error message to cover both missing-file and other error cases. --- .claude/skills/create-skill/SKILL.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index ed1fbf83..1ec746d2 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -169,10 +169,11 @@ git show HEAD:$FILE 2>/dev/null | codegraph where --file - ```bash PREV_FILE=$(mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX.js") # adjust extension to match the language of $FILE; template syntax is portable (macOS + Linux) # $FILE is expected to be set by the surrounding loop, e.g. for FILE in $(git diff --name-only HEAD); do ... done -if git show HEAD:$FILE > "$PREV_FILE" 2>&1; then +# 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 (not in HEAD) — skipping before/after comparison" + echo "WARN: $FILE is new or unreadable in HEAD — skipping before/after comparison" fi rm -f "$PREV_FILE" ``` From ab4d17b4408a50b5ece65f9c50a31aa94b9b3ff6 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 13:02:27 -0600 Subject: [PATCH 24/61] fix(skill): fix Pattern 15 cross-fence violation and broaden lint-skill Check 2 (#587) - Pattern 15: combine stash push+pop in single block to avoid cross-fence STASH_REF usage; add file-persistence alternative per Pattern 1 - lint-skill Check 2: add "suppress"/"provid" to justification keywords so Pattern 2's own taught comment style is recognized - lint-skill Check 2: detect `> /dev/null 2>&1` in addition to `2>/dev/null` - Add Pattern 2 justification comments to all `> /dev/null 2>&1` usages in pre-flight, trap cleanup, and smoke test blocks --- .claude/skills/create-skill/SKILL.md | 24 +++++++++++++++---- .../skills/create-skill/scripts/lint-skill.sh | 4 ++-- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 1ec746d2..e8ca1c57 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -24,11 +24,11 @@ Set `SKILL_NAME` to the provided name. Validate it is kebab-case (`^[a-z][a-z0-9 ```bash for tool in git mktemp; do - command -v "$tool" > /dev/null 2>&1 || { echo "ERROR: required tool '$tool' not found"; exit 1; } # > /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 -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; } # > /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. @@ -285,6 +285,7 @@ For phases that `cd` into a temp directory, clean up both the directory and the ```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 ``` @@ -301,17 +302,30 @@ git stash pop # BUG: pops wrong entry if stash was no-op or stack changed ``` ```` -**Correct:** Use a named stash with STASH_REF lookup: +**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. -# Use [ -n "$STASH_REF" ] for all branching. Use $STASH_REF (not stash@{0}) in pop/drop. +# ... 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 +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 ``` ```` @@ -482,10 +496,12 @@ Run the skill's Phase 0 (pre-flight) logic in a temporary test directory to veri ```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 # Simulate the Phase 0 checks from the skill here +# > /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 diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index d0486ed7..1ca09e36 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -92,9 +92,9 @@ while IFS= read -r line; do '```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'; then + if $in_block && echo "$line" | grep -qE '2>/dev/null|> /dev/null 2>&1'; then # Check same line or previous line for justification comment - justification_re='#.*intentional|#.*tolera|#.*acceptable|#.*expected|#.*safe to ignore|#.*may fail|#.*optional|#.*fallback|#.*portable' + 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 From daa4c88bf8ba2b362ef2a7c778da6626cac7d885 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 14:54:47 -0600 Subject: [PATCH 25/61] fix(skill): word-boundary guard for cross-fence check, quote $FILE (#587) - lint-skill.sh Check 1: replace grep -qF substring match with regex word-boundary anchor so $FILE does not falsely match $FILENAME, $FILE_PATH, etc. - SKILL.md Pattern 2: quote $FILE in git show HEAD:"$FILE" to handle filenames containing spaces. --- .claude/skills/create-skill/SKILL.md | 2 +- .claude/skills/create-skill/scripts/lint-skill.sh | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index e8ca1c57..88162ddd 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -170,7 +170,7 @@ git show HEAD:$FILE 2>/dev/null | codegraph where --file - PREV_FILE=$(mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX.js") # adjust extension to match the language of $FILE; template syntax is portable (macOS + Linux) # $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 +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" diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 1ca09e36..d7f9d191 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -57,7 +57,8 @@ while IFS=$'\t' read -r bnum line; 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 -qF "\$${var}" || echo "$line" | grep -qF "\${${var}}"; then + 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) reassigned=false while IFS=$'\t' read -r bn2 ln2; do From 07c84507cd88867264f0a07c9d8700c31b2c7abe Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 15:18:59 -0600 Subject: [PATCH 26/61] fix(skill): broaden lint-skill.sh detection heuristics and optimize Check 1 (#587) - Check 4: extend in_detect to recognize command -v, which, and -d patterns alongside file-test conditions - Check 1: add $(<file) suppression to file-persistence false-positive guard - Check 1: replace O(V*L) inner reassignment loop with O(1) associative-array lookup built during first pass --- .../skills/create-skill/scripts/lint-skill.sh | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index d7f9d191..feae2895 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -39,14 +39,17 @@ awk ' inblock { print blocknum "\t" $0 } ' "$SKILL_FILE" > "$BLOCKS_FILE" -# Collect variable assignments per block +# 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 # Match UPPER_CASE_VAR= assignments (skip lowercase/mixed to reduce false positives) - # Store the earliest block that assigns each variable so intermediate references are caught for var in $(echo "$line" | grep -oE '\b[A-Z][A-Z0-9_]+=' | sed 's/=$//'); do 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 done < "$BLOCKS_FILE" @@ -59,17 +62,10 @@ while IFS=$'\t' read -r bnum line; do # 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) - reassigned=false - while IFS=$'\t' read -r bn2 ln2; do - if [ "$bn2" = "$bnum" ] && echo "$ln2" | grep -qF "${var}="; then - reassigned=true - break - fi - done < "$BLOCKS_FILE" - if [ "$reassigned" = false ]; 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 + 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 @@ -128,7 +124,7 @@ while IFS= read -r line; do esac if $in_block; then # Track if we're inside an if/elif chain (detection block) - if echo "$line" | grep -qE '^\s*(if|elif)\s.*(-f\s|lock|package)'; then + if echo "$line" | grep -qE '^\s*(if|elif)\s.*(-f\s|-d\s|lock|package|command -v|which\s)'; then in_detect=true elif echo "$line" | grep -qE '^\s*fi\b'; then in_detect=false From bae2a0aff69ab70cd0e2b92e4b729f1a475c1495 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 15:19:13 -0600 Subject: [PATCH 27/61] fix(skill): quote $SKILL_NAME in scaffold mkdir (#587) --- .claude/skills/create-skill/SKILL.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 88162ddd..b48a2a23 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -64,7 +64,7 @@ fi Create the skill directory and SKILL.md with frontmatter: ```bash -mkdir -p .claude/skills/$SKILL_NAME +mkdir -p ".claude/skills/$SKILL_NAME" ``` Write the SKILL.md file starting with this structure: From 2b43afa8a492dfe0df052e586a90ba7044be3752 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:21:01 -0600 Subject: [PATCH 28/61] fix(skill): bash 4+ guard, Check 2 regex gap, Pattern 14 cd step (#587) - Add bash 4+ version guard to smoke-test-skill.sh (matching lint-skill.sh) - Broaden Check 2 regex to match >/dev/null 2>&1 (no space after >) - Add missing cd "$WORK_DIR" to Pattern 14 directory-cleanup example --- .claude/skills/create-skill/SKILL.md | 2 ++ .claude/skills/create-skill/scripts/lint-skill.sh | 2 +- .claude/skills/create-skill/scripts/smoke-test-skill.sh | 5 +++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index b48a2a23..3a7b1d9d 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -287,6 +287,8 @@ For phases that `cd` into a temp directory, clean up both the directory and the 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 ... ``` ### Pattern 15: Git stash safety diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index feae2895..0d8fee4c 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -89,7 +89,7 @@ while IFS= read -r line; do '```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'; then + if $in_block && echo "$line" | grep -qE '2>/dev/null|>[ ]?/dev/null 2>&1'; 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 diff --git a/.claude/skills/create-skill/scripts/smoke-test-skill.sh b/.claude/skills/create-skill/scripts/smoke-test-skill.sh index e0467a1a..e7f04f41 100644 --- a/.claude/skills/create-skill/scripts/smoke-test-skill.sh +++ b/.claude/skills/create-skill/scripts/smoke-test-skill.sh @@ -5,6 +5,11 @@ 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 From d956789cfad814c206e9f148723e309f85f15a1c Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:54:23 -0600 Subject: [PATCH 29/61] fix(skill): rename TMPDIR to WORK_DIR in Pattern 1 wrong example (#587) TMPDIR is a reserved POSIX env var used by mktemp. Using it in the "Wrong" example introduces secondary confusion beyond the cross-fence scope issue being demonstrated. --- .claude/skills/create-skill/SKILL.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 3a7b1d9d..77f97c47 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -128,11 +128,11 @@ Each fenced code block is a **separate shell invocation**. Variables set in one **Wrong:** ````markdown ```bash -TMPDIR=$(mktemp -d) +WORK_DIR=$(mktemp -d) ``` Later: ```bash -rm -rf $TMPDIR # BUG: $TMPDIR is empty here +rm -rf $WORK_DIR # BUG: $WORK_DIR is empty here ``` ```` From 6d803bfe0596f5564e3a0705b19b960733cb2d23 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:54:40 -0600 Subject: [PATCH 30/61] fix(skill): add depth counter for nested fi in lint-skill.sh (#587) Any fi token was resetting in_detect=false, including inner fi from nested if statements inside a detection block. This caused false positive warnings for npm fallback lines in the outer else branch. Use a depth counter so only the outermost fi resets the flag. --- .../skills/create-skill/scripts/lint-skill.sh | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 0d8fee4c..12d087f3 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -112,6 +112,7 @@ 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)) case "$line" in @@ -119,15 +120,22 @@ while IFS= read -r line; do esac $in_quad && continue case "$line" in - '```bash'*) in_block=true; in_detect=false; continue ;; - '```'*) in_block=false; in_detect=false; continue ;; + '```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) + # Track if we're inside an if/elif chain (detection block) with depth if echo "$line" | grep -qE '^\s*(if|elif)\s.*(-f\s|-d\s|lock|package|command -v|which\s)'; then in_detect=true + detect_depth=$((detect_depth + 1)) + 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 - in_detect=false + if [ "$detect_depth" -gt 0 ]; then + detect_depth=$((detect_depth - 1)) + [ "$detect_depth" -eq 0 ] && in_detect=false + fi fi if ! $in_detect; then if echo "$line" | grep -qE '^\s*(npm test|npm run lint)\b'; then From d0c3d7b0fda0d11459fdb7df043e85a6c9ba9feb Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 17:49:34 -0600 Subject: [PATCH 31/61] fix(lint-skill): separate elif from if in detect_depth counter (#587) elif branches are siblings of the same if-statement, not new nesting levels. Incrementing detect_depth for both if and elif caused the counter to exceed 1 after a multi-branch detection block, leaving in_detect=true after fi and silently suppressing Check 4 warnings for bare npm test calls that follow the block. --- .claude/skills/create-skill/scripts/lint-skill.sh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 12d087f3..c2025111 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -124,10 +124,15 @@ while IFS= read -r line; do '```'*) 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 - if echo "$line" | grep -qE '^\s*(if|elif)\s.*(-f\s|-d\s|lock|package|command -v|which\s)'; 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. + if echo "$line" | grep -qE '^\s*if\s.*(-f\s|-d\s|lock|package|command -v|which\s)'; then in_detect=true detect_depth=$((detect_depth + 1)) + elif echo "$line" | grep -qE '^\s*elif\s.*(-f\s|-d\s|lock|package|command -v|which\s)'; then + # elif is a sibling branch — set in_detect but do NOT increment depth + in_detect=true 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)) From c7c632b259b4ff97d99ee179c7c5e3bf090dc2ad Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 20:11:38 -0600 Subject: [PATCH 32/61] fix(lint-skill): support double-digit phases and quote $SKILL_NAME (#587) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Check 9 regex: [0-9] → [0-9]+ so Phase 10+ are validated for exit conditions - Phase 5 script invocations: quote $SKILL_NAME expansion for space-safety --- .claude/skills/create-skill/SKILL.md | 4 ++-- .claude/skills/create-skill/scripts/lint-skill.sh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 77f97c47..3e24a343 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -480,8 +480,8 @@ The self-review is purely theoretical — most real issues (wrong paths, shell s Run both validation scripts against the generated SKILL.md: ```bash -bash .claude/skills/create-skill/scripts/lint-skill.sh .claude/skills/$SKILL_NAME/SKILL.md -bash .claude/skills/create-skill/scripts/smoke-test-skill.sh .claude/skills/$SKILL_NAME/SKILL.md +bash .claude/skills/create-skill/scripts/lint-skill.sh ".claude/skills/$SKILL_NAME/SKILL.md" +bash .claude/skills/create-skill/scripts/smoke-test-skill.sh ".claude/skills/$SKILL_NAME/SKILL.md" ``` - **`lint-skill.sh`** checks for cross-fence variable bugs, bare `2>/dev/null`, hardcoded `npm test`, `git add .`, missing frontmatter, missing Phase 0 / Rules, missing exit conditions, GNU-only `find -quit`, hardcoded `/tmp/` paths, and `sed -i` portability issues. diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index c2025111..1848c48e 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -191,7 +191,7 @@ while IFS= read -r line; do esac $in_block && continue - if echo "$line" | grep -qE '^## Phase [0-9]'; then + 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 From f713225cdf766de6de6da3e1681157abfc41810d Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 21:10:00 -0600 Subject: [PATCH 33/61] fix(skill): add phase 6 exit condition and elif detect_depth safety reset (#587) --- .claude/skills/create-skill/SKILL.md | 2 ++ .claude/skills/create-skill/scripts/lint-skill.sh | 3 +++ 2 files changed, 5 insertions(+) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 3e24a343..e83f0db6 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -532,6 +532,8 @@ 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 diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 1848c48e..6af1f9ef 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -140,6 +140,9 @@ while IFS= read -r line; do 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 fi if ! $in_detect; then From ffe077cbdf861bc36000ba96b359ca17d7d2d571 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 21:53:15 -0600 Subject: [PATCH 34/61] fix(lint-skill): detect npm run test as hardcoded command (#587) --- .claude/skills/create-skill/scripts/lint-skill.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 6af1f9ef..f2443778 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -146,7 +146,7 @@ while IFS= read -r line; do fi fi if ! $in_detect; then - if echo "$line" | grep -qE '^\s*(npm test|npm run lint)\b'; then + if echo "$line" | grep -qE '^\s*(npm test|npm run test|npm run lint)\b'; then warn "Line $line_num: Hardcoded '$(echo "$line" | sed 's/^[[:space:]]*//')' — detect package manager first (Pattern 6)" fi fi From 457881aea62c64be79b17324d95d3cb8adf6c92e Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Wed, 25 Mar 2026 23:21:31 -0600 Subject: [PATCH 35/61] fix(skill): detect indented bash blocks in lint and smoke-test scripts Both lint-skill.sh and smoke-test-skill.sh used column-0 anchored patterns to detect code fences, silently skipping any ```bash block indented inside a Markdown list item. Strip leading whitespace before case matching and update the awk extractor to use ^\s* anchors. Also update the lint-skill.sh description in SKILL.md to list all hardcoded command variants. --- .claude/skills/create-skill/SKILL.md | 2 +- .../skills/create-skill/scripts/lint-skill.sh | 33 +++++++++++-------- .../create-skill/scripts/smoke-test-skill.sh | 7 ++-- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index e83f0db6..a6509e26 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -484,7 +484,7 @@ bash .claude/skills/create-skill/scripts/lint-skill.sh ".claude/skills/$SKILL_NA bash .claude/skills/create-skill/scripts/smoke-test-skill.sh ".claude/skills/$SKILL_NAME/SKILL.md" ``` -- **`lint-skill.sh`** checks for cross-fence variable bugs, bare `2>/dev/null`, hardcoded `npm test`, `git add .`, missing frontmatter, missing Phase 0 / Rules, missing exit conditions, GNU-only `find -quit`, hardcoded `/tmp/` paths, and `sed -i` portability issues. +- **`lint-skill.sh`** checks for cross-fence variable bugs, bare `2>/dev/null`, hardcoded `npm test` / `npm run test` / `npm run lint`, `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. diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index f2443778..7f2b6c59 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -30,13 +30,14 @@ warn() { echo "WARN: $1"; WARNINGS=$((WARNINGS + 1)); } BLOCKS_FILE=$(mktemp "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX.blocks") trap 'rm -f "$BLOCKS_FILE"' EXIT -# Extract bash blocks with block index, skipping those inside ```` regions +# Extract bash blocks with block index, skipping those inside ```` regions. +# Patterns use ^\s* to match indented blocks (e.g. inside Markdown list items). awk ' - /^````/ { quad = !quad; next } - quad { next } - /^```bash/ { inblock = 1; blocknum++; next } - /^```/ && inblock { inblock = 0; next } - inblock { print blocknum "\t" $0 } + /^\s*````/ { quad = !quad; next } + quad { next } + /^\s*```bash/ { inblock = 1; blocknum++; next } + /^\s*```/ && 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) @@ -81,11 +82,12 @@ in_block=false prev_line="" while IFS= read -r line; do line_num=$((line_num + 1)) - case "$line" in + 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 "$line" in + case "$stripped" in '```bash'*) in_block=true; prev_line="$line"; continue ;; '```'*) in_block=false; prev_line="$line"; continue ;; esac @@ -115,11 +117,12 @@ in_detect=false detect_depth=0 while IFS= read -r line; do line_num=$((line_num + 1)) - case "$line" in + stripped="${line#"${line%%[! ]*}"}" + case "$stripped" in '````'*) if $in_quad; then in_quad=false; else in_quad=true; fi; continue ;; esac $in_quad && continue - case "$line" in + 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 @@ -183,13 +186,14 @@ 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 "$line" in + 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 - case "$line" in + case "$stripped" in '```'*) if $in_block; then in_block=false; else in_block=true; fi; continue ;; esac $in_block && continue @@ -223,11 +227,12 @@ in_quad=false in_block=false while IFS= read -r line; do line_num=$((line_num + 1)) - case "$line" in + lstripped="${line#"${line%%[! ]*}"}" + case "$lstripped" in '````'*) if $in_quad; then in_quad=false; else in_quad=true; fi; continue ;; esac $in_quad && continue - case "$line" in + case "$lstripped" in '```bash'*) in_block=true; continue ;; '```'*) in_block=false; continue ;; esac diff --git a/.claude/skills/create-skill/scripts/smoke-test-skill.sh b/.claude/skills/create-skill/scripts/smoke-test-skill.sh index e7f04f41..30c9b886 100644 --- a/.claude/skills/create-skill/scripts/smoke-test-skill.sh +++ b/.claude/skills/create-skill/scripts/smoke-test-skill.sh @@ -32,8 +32,11 @@ 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 "$line" in + case "$stripped" in '````'*) if $in_quad; then in_quad=false @@ -50,7 +53,7 @@ while IFS= read -r line; do fi # Track triple-backtick bash blocks - case "$line" in + case "$stripped" in '```bash'*) in_block=true block_start=$line_num From fadab53825d90a6ee1dd592af1446adc834c6f6f Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 02:28:24 -0600 Subject: [PATCH 36/61] fix(lint-skill): catch git add -- . in Check 3 regex (#587) --- .claude/skills/create-skill/scripts/lint-skill.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 7f2b6c59..4b4f66f3 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -103,7 +103,7 @@ 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 (\.|(-A|--all))'; then + if echo "$line" | grep -qE '^\s*git add (--\s+\.|\.|(-A|--all))'; then error "bash block $bnum: 'git add .' or 'git add -A' — stage named files only" fi done < "$BLOCKS_FILE" From 23fe2328cdddc231e3706d3a9a1a17283f1eaca8 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 14:04:00 -0600 Subject: [PATCH 37/61] fix(skill): add trap to Pattern 2 example, document git add -- . in lint description (#587) --- .claude/skills/create-skill/SKILL.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index a6509e26..0ed782af 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -168,6 +168,7 @@ git show HEAD:$FILE 2>/dev/null | codegraph where --file - ````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 @@ -176,6 +177,7 @@ else echo "WARN: $FILE is new or unreadable in HEAD — skipping before/after comparison" fi rm -f "$PREV_FILE" +trap - EXIT ``` ```` @@ -484,7 +486,7 @@ bash .claude/skills/create-skill/scripts/lint-skill.sh ".claude/skills/$SKILL_NA bash .claude/skills/create-skill/scripts/smoke-test-skill.sh ".claude/skills/$SKILL_NAME/SKILL.md" ``` -- **`lint-skill.sh`** checks for cross-fence variable bugs, bare `2>/dev/null`, hardcoded `npm test` / `npm run test` / `npm run lint`, `git add .`, missing frontmatter, missing Phase 0 / Rules, missing exit conditions, GNU-only `find -quit`, hardcoded `/tmp/` paths, and `sed -i` portability issues. +- **`lint-skill.sh`** checks for cross-fence variable bugs, bare `2>/dev/null`, hardcoded `npm test` / `npm run test` / `npm 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. From 8ab4f907079902810eedd178ef8e76732b504c18 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 14:36:02 -0600 Subject: [PATCH 38/61] fix(skill): address final 3 Greptile review findings (#587) - Pattern 14 directory variant: add explicit cd/rm/trap cleanup after operations - Pattern 13: document `grep -q .` as portable alternative to `find -quit` - Check 1 (lint-skill.sh): detect `+=` append assignments alongside `=` --- .claude/skills/create-skill/SKILL.md | 6 +++++- .claude/skills/create-skill/scripts/lint-skill.sh | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 0ed782af..7ef038e5 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -266,7 +266,7 @@ 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"` instead of glob expansion (`ls *.ext`) which differs between bash versions +- 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` @@ -291,6 +291,10 @@ WORK_DIR=$(mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX") 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 diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 4b4f66f3..7caf264b 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -45,7 +45,7 @@ declare -A VAR_BLOCK declare -A REASSIGNED while IFS=$'\t' read -r bnum line; do # Match UPPER_CASE_VAR= assignments (skip lowercase/mixed to reduce false positives) - for var in $(echo "$line" | grep -oE '\b[A-Z][A-Z0-9_]+=' | sed 's/=$//'); do + for var in $(echo "$line" | grep -oE '\b[A-Z][A-Z0-9_]+\+?=' | sed -E 's/\+?=$//'); do if [ -z "${VAR_BLOCK[$var]+x}" ]; then VAR_BLOCK["$var"]="$bnum" else From b52d19ef74b6ab231944234d0ddef75dca1b46e9 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 15:07:05 -0600 Subject: [PATCH 39/61] fix(skill): add context comments to Pattern 16/17 examples (#587) Add variable-origin comments to Pattern 16 ($BASELINE, $CURRENT) and Pattern 17 ($DRY_RUN, $OUTPUT_DIR) Correct examples, consistent with the convention established in Pattern 11. --- .claude/skills/create-skill/SKILL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 7ef038e5..9efcec07 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -351,6 +351,7 @@ 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 @@ -376,6 +377,7 @@ 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 From 91b23c0bf5590510622cb4905d5dcc3c82bf811b Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 15:56:41 -0600 Subject: [PATCH 40/61] fix(lint-skill): awk portability and Check 4 false positive (#587) - Replace GNU awk \s* with POSIX [[:space:]]* in block extractor so indented-fence detection works on macOS BSD awk - Replace \b word-boundary in Check 4 with ([^:A-Za-z0-9_]|$) so colon-namespaced scripts like npm run test:unit are not flagged --- .claude/skills/create-skill/scripts/lint-skill.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 7caf264b..ff6ed779 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -31,13 +31,13 @@ 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 ^\s* to match indented blocks (e.g. inside Markdown list items). +# Patterns use ^[[:space:]]* to match indented blocks (e.g. inside Markdown list items). awk ' - /^\s*````/ { quad = !quad; next } - quad { next } - /^\s*```bash/ { inblock = 1; blocknum++; next } - /^\s*```/ && inblock { inblock = 0; next } - inblock { print blocknum "\t" $0 } + /^[[: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) @@ -149,7 +149,7 @@ while IFS= read -r line; do fi fi if ! $in_detect; then - if echo "$line" | grep -qE '^\s*(npm test|npm run test|npm run lint)\b'; then + if echo "$line" | grep -qE '^\s*(npm test|npm 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 From 396bde5feb2f4810c7126303de24302cf3917691 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 19:48:09 -0600 Subject: [PATCH 41/61] fix(skill): address 3 Greptile findings in create-skill quality gates 1. smoke-test-skill.sh: add post-loop guard that detects unclosed bash blocks at EOF and emits FAIL instead of silently skipping them. 2. lint-skill.sh: replace bare for-in-$() with while-read loop to avoid empty-string iteration when grep matches no uppercase assignments. 3. SKILL.md Phase 5: collect lint and smoke-test exit codes independently so set -e cannot short-circuit the second script. --- .claude/skills/create-skill/SKILL.md | 9 +++++++-- .claude/skills/create-skill/scripts/lint-skill.sh | 6 ++++-- .claude/skills/create-skill/scripts/smoke-test-skill.sh | 6 ++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 9efcec07..879427d3 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -488,8 +488,13 @@ The self-review is purely theoretical — most real issues (wrong paths, shell s Run both validation scripts against the generated SKILL.md: ```bash -bash .claude/skills/create-skill/scripts/lint-skill.sh ".claude/skills/$SKILL_NAME/SKILL.md" -bash .claude/skills/create-skill/scripts/smoke-test-skill.sh ".claude/skills/$SKILL_NAME/SKILL.md" +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`, `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. diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index ff6ed779..50060c64 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -45,14 +45,16 @@ declare -A VAR_BLOCK declare -A REASSIGNED while IFS=$'\t' read -r bnum line; do # Match UPPER_CASE_VAR= assignments (skip lowercase/mixed to reduce false positives) - for var in $(echo "$line" | grep -oE '\b[A-Z][A-Z0-9_]+\+?=' | sed -E 's/\+?=$//'); do + # 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 + 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 diff --git a/.claude/skills/create-skill/scripts/smoke-test-skill.sh b/.claude/skills/create-skill/scripts/smoke-test-skill.sh index 30c9b886..ec247c1a 100644 --- a/.claude/skills/create-skill/scripts/smoke-test-skill.sh +++ b/.claude/skills/create-skill/scripts/smoke-test-skill.sh @@ -89,6 +89,12 @@ while IFS= read -r line; do 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 + echo "" echo "smoke-test-skill: $PASS passed, $FAIL failed, $SKIP skipped" if [ "$FAIL" -gt 0 ]; then From 1df69b3f15273e2756b54ed988c0a823d9bb428d Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 20:06:32 -0600 Subject: [PATCH 42/61] fix(skill): add mkdir -p guard to Pattern 15 two-fence stash example (#587) --- .claude/skills/create-skill/SKILL.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 879427d3..8192ceca 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -326,6 +326,7 @@ STASH_REF=$(git stash list --format='%gd %s' | grep 'deploy-check-backup' | head 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 ``` From 5717c21af7b75edb9f372d4180280a999b256709 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 20:34:01 -0600 Subject: [PATCH 43/61] fix(skill): handle inline fi and &>/dev/null in lint-skill.sh (#587) --- .claude/skills/create-skill/scripts/lint-skill.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 50060c64..5e269ef2 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -93,7 +93,7 @@ while IFS= read -r line; do '```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'; then + 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 @@ -149,6 +149,14 @@ while IFS= read -r line; do # 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 if ! $in_detect; then if echo "$line" | grep -qE '^\s*(npm test|npm run (test|lint))([^:A-Za-z0-9_]|$)'; then From 163b32802257f4bd356e54ac5e8e76aa645d2a9e Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 20:51:57 -0600 Subject: [PATCH 44/61] fix: prevent false-positive git-add violations on dotfiles and relative paths (#587) --- .claude/skills/create-skill/scripts/lint-skill.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 5e269ef2..b5f8317c 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -105,7 +105,7 @@ 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+\.|\.|(-A|--all))'; then + if echo "$line" | grep -qE '^\s*git add (--\s+\.([ \t;#]|$)|\.([ \t;#]|$)|(-A|--all))'; then error "bash block $bnum: 'git add .' or 'git add -A' — stage named files only" fi done < "$BLOCKS_FILE" From 00fb02b1b5cbd54d8c9ae51caa243e2bafa78ef7 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 21:10:29 -0600 Subject: [PATCH 45/61] fix(skill): extend Check 4 to catch yarn/pnpm hardcoding and align Rules wording (#587) --- .claude/skills/create-skill/SKILL.md | 2 +- .claude/skills/create-skill/scripts/lint-skill.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 8192ceca..c656ee17 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -567,5 +567,5 @@ If the user approves: - **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` — detect the package manager first. +- **Test commands must be detected, not assumed.** Never hardcode `npm test`, `npm run test`, `npm run lint`, `yarn test`, or `pnpm test` — 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 index b5f8317c..63adbac8 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -159,7 +159,7 @@ while IFS= read -r line; do fi fi if ! $in_detect; then - if echo "$line" | grep -qE '^\s*(npm test|npm run (test|lint))([^:A-Za-z0-9_]|$)'; 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 From 3f43d8b4c1efb78275d9f93e44b4dda784104e5d Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 21:43:29 -0600 Subject: [PATCH 46/61] fix(skill): align Phase 5 and Rules docs with Check 4 yarn/pnpm coverage (#587) --- .claude/skills/create-skill/SKILL.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index c656ee17..4ec468f4 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -498,7 +498,7 @@ if [ "$LINT_RC" -ne 0 ] || [ "$SMOKE_RC" -ne 0 ]; then fi ``` -- **`lint-skill.sh`** checks for cross-fence variable bugs, bare `2>/dev/null`, hardcoded `npm test` / `npm run test` / `npm 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. +- **`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. @@ -567,5 +567,5 @@ If the user approves: - **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`, or `pnpm test` — detect the package manager first. +- **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. From de8e6c512415ab4b6bc8eba4b8e42f2857c6f35c Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 22:19:57 -0600 Subject: [PATCH 47/61] fix(skill): widen Check 2 and Check 11 regexes in lint-skill.sh (#587) Check 2: change `>[ ]?/dev/null` to `>[ ]*/dev/null` so multi-space variants emitted by auto-formatters are caught. Check 11: change `/tmp/[a-zA-Z][a-zA-Z]` to `/tmp/[a-zA-Z]` so single-letter unquoted paths like `/tmp/f.json` are caught. --- .claude/skills/create-skill/scripts/lint-skill.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 63adbac8..d7627264 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -93,7 +93,7 @@ while IFS= read -r line; do '```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 + 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 @@ -249,7 +249,7 @@ while IFS= read -r line; do 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][a-zA-Z]'; then + 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)" From fc54517779fb7439f3c211fb7f94d5219fdde819 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 22:20:35 -0600 Subject: [PATCH 48/61] fix(skill): add failure-path stubs to Phase 5 smoke-test template (#587) The template previously only scaffolded the happy path. Now includes stub comments for three failure scenarios (non-git directory, invalid skill name, missing required tool) so generated skills exercise both success and error paths. --- .claude/skills/create-skill/SKILL.md | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 4ec468f4..bc9b420d 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -516,7 +516,19 @@ TEST_DIR=$(mktemp -d "${TMPDIR:-/tmp}/tmp.XXXXXXXXXX") trap 'cd - > /dev/null 2>&1; rm -rf "$TEST_DIR"' EXIT cd "$TEST_DIR" git init --quiet -# Simulate the Phase 0 checks from the skill here + +# --- 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" From ba179cde728bbc466cae7abfb2392ae62f634228 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 22:50:13 -0600 Subject: [PATCH 49/61] fix(skill): use $BASH instead of PATH bash for syntax checks (#587) On macOS, `bash -n` resolves to /bin/bash (3.2) even when the script is running under bash 4+. Using "$BASH" ensures the same interpreter that passed the version guard is used for syntax checking. --- .claude/skills/create-skill/scripts/smoke-test-skill.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/scripts/smoke-test-skill.sh b/.claude/skills/create-skill/scripts/smoke-test-skill.sh index ec247c1a..b72f50a5 100644 --- a/.claude/skills/create-skill/scripts/smoke-test-skill.sh +++ b/.claude/skills/create-skill/scripts/smoke-test-skill.sh @@ -65,7 +65,7 @@ while IFS= read -r line; do in_block=false # Run syntax check on the collected block if [ -s "$TMPBLOCK" ]; then - if output=$(bash -n "$TMPBLOCK" 2>&1); then + if output=$("$BASH" -n "$TMPBLOCK" 2>&1); then echo " OK block at line $block_start" PASS=$((PASS + 1)) else From e63d742c0ea382dfa2776bdd0ae16ad5cf6d72a7 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 22:50:38 -0600 Subject: [PATCH 50/61] fix(skill): add trailing boundary to Check 3 --all regex, fix inline-fi detection context (#587) - Check 3: added ([ \t;#]|$) after (-A|--all) to prevent false positives on hypothetical --all-prefixed options. - Check 4: use was_in_detect to evaluate hardcoded-command check in the detection context that was active at the start of the line, so compact forms like "else npm test; fi" are not falsely flagged after the inline fi resets in_detect. --- .claude/skills/create-skill/scripts/lint-skill.sh | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index d7627264..b7c6f428 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -105,7 +105,7 @@ 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))'; then + 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" @@ -132,12 +132,17 @@ while IFS= read -r line; do # 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|lock|package|command -v|which\s)'; then in_detect=true + was_in_detect=true detect_depth=$((detect_depth + 1)) elif echo "$line" | grep -qE '^\s*elif\s.*(-f\s|-d\s|lock|package|command -v|which\s)'; then # elif is a sibling branch — set in_detect but do NOT increment depth in_detect=true + was_in_detect=true 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)) @@ -158,7 +163,10 @@ while IFS= read -r line; do in_detect=false fi fi - if ! $in_detect; then + # 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 From e04545da40574af5caecb9c28a96deefedbe7e55 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Thu, 26 Mar 2026 23:25:29 -0600 Subject: [PATCH 51/61] fix(skill): add Phase 4 exit condition to pass its own Check 9 (#587) --- .claude/skills/create-skill/SKILL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index bc9b420d..27f63059 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -478,6 +478,8 @@ Before finalizing, audit the SKILL.md against every item below. **Do not skip an 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 From 5b5ff9eee690f97df7b9f227a03d0025fdfc8b73 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Fri, 27 Mar 2026 01:38:52 -0600 Subject: [PATCH 52/61] fix(skill): handle one-liner if/fi and find-based detection in Check 4 (#587) - Guard detect_depth increment when fi appears on same line as detection if - Add find\s to both if and elif detection keyword patterns --- .claude/skills/create-skill/scripts/lint-skill.sh | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index b7c6f428..41505f27 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -135,11 +135,14 @@ while IFS= read -r line; do # 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|lock|package|command -v|which\s)'; then + if echo "$line" | grep -qE '^\s*if\s.*(-f\s|-d\s|lock|package|command -v|which\s|find\s)'; then in_detect=true was_in_detect=true - detect_depth=$((detect_depth + 1)) - elif echo "$line" | grep -qE '^\s*elif\s.*(-f\s|-d\s|lock|package|command -v|which\s)'; then + # 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)) + fi + elif echo "$line" | grep -qE '^\s*elif\s.*(-f\s|-d\s|lock|package|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 From 2507820fda45f07cd01f8904e68659360ca2ed6a Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Fri, 27 Mar 2026 02:19:22 -0600 Subject: [PATCH 53/61] perf(hooks): narrow Bash hook matchers to git commands only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bash hooks (guard-git, pre-commit, lint-staged, check-readme, guard-pr-body, track-moves, post-git-ops) now use `Bash(git )` matcher instead of `Bash`, so they only fire on git commands rather than every Bash invocation — reduces token consumption. --- .claude/settings.json | 4 ++-- docs/examples/claude-code-hooks/settings.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 2f9b092f..7ac8c478 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -2,7 +2,7 @@ "hooks": { "PreToolUse": [ { - "matcher": "Bash", + "matcher": "Bash(git )", "hooks": [ { "type": "command", @@ -69,7 +69,7 @@ ] }, { - "matcher": "Bash", + "matcher": "Bash(git )", "hooks": [ { "type": "command", diff --git a/docs/examples/claude-code-hooks/settings.json b/docs/examples/claude-code-hooks/settings.json index 13c5afb0..31c087bb 100644 --- a/docs/examples/claude-code-hooks/settings.json +++ b/docs/examples/claude-code-hooks/settings.json @@ -12,7 +12,7 @@ ] }, { - "matcher": "Bash", + "matcher": "Bash(git )", "hooks": [ { "type": "command", @@ -49,7 +49,7 @@ ] }, { - "matcher": "Bash", + "matcher": "Bash(git )", "hooks": [ { "type": "command", From f365f8b029af2b5693314bf96f41e6c77723a426 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Fri, 27 Mar 2026 02:25:26 -0600 Subject: [PATCH 54/61] fix(skill): add Examples section placeholder to scaffold template (#587) --- .claude/skills/create-skill/SKILL.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 27f63059..7199a9df 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -103,6 +103,10 @@ allowed-tools: <from user's tool list> ## Rules - <Hard constraints> + +## Examples + +- <Usage examples showing common invocations and expected behavior> ``` ### Structural requirements to include in every skill: @@ -111,7 +115,8 @@ allowed-tools: <from user's tool list> 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. **Artifact definitions** — if the skill produces files, specify path, format, and schema +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. From 262a5a47b61907435625f5e0721ee0634cbede5e Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Fri, 27 Mar 2026 11:02:32 -0600 Subject: [PATCH 55/61] fix(skill): harden lint checks against false positives (#587) Skip comment lines in Check 1 cross-fence variable detection to avoid flagging context comments like `# $VAR is set above`. Add word-boundary anchors to `lock` and `package` keywords in Check 4 detection heuristic so substrings like `CLOCKWORK` or `repackage` do not falsely trigger detection mode. --- .claude/skills/create-skill/scripts/lint-skill.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 41505f27..2d71f0b7 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -59,6 +59,8 @@ 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 @@ -135,14 +137,14 @@ while IFS= read -r line; do # 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|lock|package|command -v|which\s|find\s)'; then + 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)) fi - elif echo "$line" | grep -qE '^\s*elif\s.*(-f\s|-d\s|lock|package|command -v|which\s|find\s)'; then + 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 From 927a3d125d8cc6e86e6f1dc49ad204bfce396106 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Fri, 27 Mar 2026 11:02:52 -0600 Subject: [PATCH 56/61] fix(skill): align scaffold template ordering and add pre-flight example (#587) Add a concrete bash pre-flight block to the scaffold template Phase 0 so generated skills follow the same Pattern 2 convention the actual SKILL.md demonstrates. Swap Rules/Examples ordering in the template to match the exemplar (Examples before Rules). --- .claude/skills/create-skill/SKILL.md | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index 7199a9df..df8e21e9 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -88,6 +88,15 @@ allowed-tools: <from user's tool list> ## Phase 0 — Pre-flight +```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"; exit 1; } +``` + 1. Confirm environment (repo root, required runtime/toolchain version, required tools) 2. Parse `$ARGUMENTS` into state variables 3. Validate preconditions @@ -100,13 +109,13 @@ allowed-tools: <from user's tool list> **Exit condition:** <What must be true before the next phase starts> -## Rules - -- <Hard constraints> - ## Examples - <Usage examples showing common invocations and expected behavior> + +## Rules + +- <Hard constraints> ``` ### Structural requirements to include in every skill: From 8bb49d39906ea25d32c1f0456c898f6ccd029d75 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Fri, 27 Mar 2026 13:36:24 -0600 Subject: [PATCH 57/61] fix(skill): handle inline fi on elif detection branch in Check 4 (#587) When an elif detection branch contains an inline fi (e.g. "elif [ -f yarn.lock ]; then CMD=yarn; fi"), the fi handler was never reached because it was a sibling elif in the same chain. This left in_detect permanently true, causing false negatives for hardcoded package commands after the detection block. --- .claude/skills/create-skill/scripts/lint-skill.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 2d71f0b7..6732daa9 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -148,6 +148,15 @@ while IFS= read -r line; do # 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)) From f7e8bd5af3faf2165aff85ab8f09b41bce02d54a Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Fri, 27 Mar 2026 13:36:46 -0600 Subject: [PATCH 58/61] fix(skill): revert unrelated hook matcher scope change (#587) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts the Bash→Bash(git ) matcher change in both settings.json files. This is a separate behavioral concern from the skill addition and should be in its own PR per the one-PR-one-concern rule. --- .claude/settings.json | 4 ++-- docs/examples/claude-code-hooks/settings.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index 7ac8c478..2f9b092f 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -2,7 +2,7 @@ "hooks": { "PreToolUse": [ { - "matcher": "Bash(git )", + "matcher": "Bash", "hooks": [ { "type": "command", @@ -69,7 +69,7 @@ ] }, { - "matcher": "Bash(git )", + "matcher": "Bash", "hooks": [ { "type": "command", diff --git a/docs/examples/claude-code-hooks/settings.json b/docs/examples/claude-code-hooks/settings.json index 31c087bb..13c5afb0 100644 --- a/docs/examples/claude-code-hooks/settings.json +++ b/docs/examples/claude-code-hooks/settings.json @@ -12,7 +12,7 @@ ] }, { - "matcher": "Bash(git )", + "matcher": "Bash", "hooks": [ { "type": "command", @@ -49,7 +49,7 @@ ] }, { - "matcher": "Bash(git )", + "matcher": "Bash", "hooks": [ { "type": "command", From 65f905ffd29285958adeafb82f30d7b01fde03d8 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Fri, 27 Mar 2026 15:20:43 -0600 Subject: [PATCH 59/61] fix(skill): add Examples check, name-directory consistency check, and zero-block warning (#587) - lint-skill.sh Check 8b: error when ## Examples section is missing - lint-skill.sh Check 6b: error when name: frontmatter doesn't match directory name - smoke-test-skill.sh: warn when no bash blocks are found (false-clean signal) --- .claude/skills/create-skill/scripts/lint-skill.sh | 12 ++++++++++++ .../skills/create-skill/scripts/smoke-test-skill.sh | 5 +++++ 2 files changed, 17 insertions(+) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 6732daa9..055f4532 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -212,6 +212,18 @@ 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=$(grep -m1 '^name:' "$SKILL_FILE" | 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 diff --git a/.claude/skills/create-skill/scripts/smoke-test-skill.sh b/.claude/skills/create-skill/scripts/smoke-test-skill.sh index b72f50a5..54e4d3ea 100644 --- a/.claude/skills/create-skill/scripts/smoke-test-skill.sh +++ b/.claude/skills/create-skill/scripts/smoke-test-skill.sh @@ -95,6 +95,11 @@ if $in_block; then 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 From c305c7caf29eee6331daca6e41b514862790687f Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Fri, 27 Mar 2026 16:14:10 -0600 Subject: [PATCH 60/61] fix(skill): address 3 Greptile findings in lint-skill.sh (#587) - P1: One-liner if detection blocks now reset in_detect=false when fi is on the same line, preventing false negatives on subsequent lines - P2: Check 6b scoped to head -20 (frontmatter only) instead of full file - P2: Check 9 documents nested-fence limitation with workaround guidance --- .claude/skills/create-skill/scripts/lint-skill.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index 055f4532..fa7722ad 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -143,6 +143,9 @@ while IFS= read -r line; do # 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 @@ -219,7 +222,7 @@ fi # ── Check 6b: name field matches directory name ───────────────────── expected_name=$(basename "$(dirname "$SKILL_FILE")") -actual_name=$(grep -m1 '^name:' "$SKILL_FILE" | sed 's/^name:[[:space:]]*//') +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 @@ -236,7 +239,10 @@ while IFS= read -r line; do '````'*) if $in_quad; then in_quad=false; else in_quad=true; fi; continue ;; esac $in_quad && continue - # Skip content inside triple-backtick code blocks + # 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 From d297c3e719e445f3a551e649ee963b2c650654e5 Mon Sep 17 00:00:00 2001 From: carlos-alm <127798846+carlos-alm@users.noreply.github.com> Date: Fri, 27 Mar 2026 16:50:57 -0600 Subject: [PATCH 61/61] fix(skill): skip comment lines in Check 1 assignment loop, add tool-list hint to scaffold (#587) --- .claude/skills/create-skill/SKILL.md | 2 +- .claude/skills/create-skill/scripts/lint-skill.sh | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.claude/skills/create-skill/SKILL.md b/.claude/skills/create-skill/SKILL.md index df8e21e9..67a50f90 100644 --- a/.claude/skills/create-skill/SKILL.md +++ b/.claude/skills/create-skill/SKILL.md @@ -89,7 +89,7 @@ allowed-tools: <from user's tool list> ## Phase 0 — Pre-flight ```bash -for tool in git mktemp; do +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 diff --git a/.claude/skills/create-skill/scripts/lint-skill.sh b/.claude/skills/create-skill/scripts/lint-skill.sh index fa7722ad..3e3f172e 100644 --- a/.claude/skills/create-skill/scripts/lint-skill.sh +++ b/.claude/skills/create-skill/scripts/lint-skill.sh @@ -44,6 +44,8 @@ awk ' 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