feat(tools): add deterministic skill validator for CI#2051
Conversation
Add tools/validate-skills.js — a Node CLI that checks 13 deterministic rules (SKILL-01–06, WF-01–02, PATH-02, STEP-01/06/07, SEQ-02) across all skill directories. Runs in under a second, exits non-zero on HIGH+ findings in strict mode, and outputs JSON for the inference validator. - Add validate:skills npm script to quality chain - Update skill-validator.md with first-pass integration instructions - Update AGENTS.md push gate documentation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR introduces a deterministic skill validator script ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tools/validate-skills.js (1)
211-229: Consider adding error handling for directory read failures.Unlike
discoverSkillDirs(which checksfs.existsSyncbefore reading),collectSkillFilesdoesn't guard againstreaddirSyncfailures. If a directory becomes inaccessible mid-walk (e.g., permission change, symlink target removed), this would throw an unhandled exception.🛡️ Optional: wrap readdirSync in try-catch
function collectSkillFiles(skillDir) { const files = []; function walk(dir) { + let entries; + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + } catch { + return; // Skip inaccessible directories + } - const entries = fs.readdirSync(dir, { withFileTypes: true }); for (const entry of entries) { if (entry.name === 'node_modules' || entry.name === '.git') continue;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/validate-skills.js` around lines 211 - 229, The collectSkillFiles function currently calls fs.readdirSync inside the nested walk function without handling errors, so a mid-walk permission or IO failure will throw; wrap the fs.readdirSync(dir, { withFileTypes: true }) call in a try-catch inside walk (or check fs.existsSync/permissions before reading) to catch and log or silently skip unreadable directories, and ensure the walk continues for other entries; update collectSkillFiles/walk to handle and optionally report errors (e.g., using console.warn or the existing logger) while still returning the collected files.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tools/validate-skills.js`:
- Around line 211-229: The collectSkillFiles function currently calls
fs.readdirSync inside the nested walk function without handling errors, so a
mid-walk permission or IO failure will throw; wrap the fs.readdirSync(dir, {
withFileTypes: true }) call in a try-catch inside walk (or check
fs.existsSync/permissions before reading) to catch and log or silently skip
unreadable directories, and ensure the walk continues for other entries; update
collectSkillFiles/walk to handle and optionally report errors (e.g., using
console.warn or the existing logger) while still returning the collected files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: adc86a9f-0e0a-4565-99ff-8f68de7fdbd3
📒 Files selected for processing (4)
AGENTS.mdpackage.jsontools/skill-validator.mdtools/validate-skills.js
🤖 Augment PR SummarySummary: This PR introduces a fast, deterministic skill validator to complement the existing inference-based validation and make CI enforcement more consistent. Changes:
Technical Notes: The validator discovers skill directories under 🤖 Was this summary useful? React with 👍 or 👎 |
Replace SKILL_LOCATIONS array and AGENT_LOCATION constant with a single walk from SRC_DIR. Any directory under src/ containing SKILL.md is a skill — no need to enumerate locations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Require \n---\n (not just \n---) for closing frontmatter delimiter in both parseFrontmatter and parseFrontmatterMultiline, with fallback for files ending in \n--- - Add SKILL-07: SKILL.md must have non-empty body content after frontmatter (L2 instructions are required) - Update rule count to 14 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…en names - SKILL-04: require bmad- prefix, enforce single dashes via regex ^bmad-[a-z0-9]+(-[a-z0-9]+)*$, drop FORBIDDEN_NAME_SUBSTRINGS - WF-01/WF-02: check all .md files (not just workflow.md) for stray name/description frontmatter, with tech-writer exception - Update skill-validator.md prompt to match all rule changes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Guard against YAML comment lines in parseFrontmatterMultiline - Broaden PATH-02 to detect any installed_path mention, not just variable refs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The deterministic skill validator was in the npm quality chain but missing from the GitHub Actions workflow. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
tools/validate-skills.js— Node CLI that checks 13 deterministic rules (SKILL-01–06, WF-01–02, PATH-02, STEP-01/06/07, SEQ-02) across all skill directories in under 1 secondvalidate:skillsnpm script to quality chain and push gatetools/skill-validator.mdwith first-pass integration instructions so the inference validator skips already-verified rulesTest plan
node tools/validate-skills.js --strictexits 0 (3 true-positive MEDIUM/LOW findings)node tools/validate-skills.js --jsonproduces valid JSONnpm run qualityincludes and passesvalidate:skills🤖 Generated with Claude Code