refactor: tighten SKILL-04 regex, broaden WF-01/WF-02, remove forbidden 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>
This commit is contained in:
Alex Verkhovsky
parent
7a214cc7d8
commit
4f1894908c
|
|
@ -10,7 +10,7 @@ Before running inference-based validation, run the deterministic validator:
|
|||
node tools/validate-skills.js --json path/to/skill-dir
|
||||
```
|
||||
|
||||
This checks 13 rules deterministically: SKILL-01, SKILL-02, SKILL-03, SKILL-04, SKILL-05, SKILL-06, WF-01, WF-02, PATH-02, STEP-01, STEP-06, STEP-07, SEQ-02.
|
||||
This checks 14 rules deterministically: SKILL-01, SKILL-02, SKILL-03, SKILL-04, SKILL-05, SKILL-06, SKILL-07, WF-01, WF-02, PATH-02, STEP-01, STEP-06, STEP-07, SEQ-02.
|
||||
|
||||
Review its JSON output. For any rule that produced **zero findings** in the first pass, **skip it** during inference-based validation below — it has already been verified. If a rule produced any findings, the inference validator should still review that rule (some rules like SKILL-04 and SKILL-06 have sub-checks that benefit from judgment). Focus your inference effort on the remaining rules that require judgment (PATH-01, PATH-03, PATH-04, PATH-05, WF-03, STEP-02, STEP-03, STEP-04, STEP-05, SEQ-01, REF-01, REF-02, REF-03).
|
||||
|
||||
|
|
@ -68,9 +68,9 @@ If no findings are generated (from either pass), the skill passes validation.
|
|||
|
||||
- **Severity:** HIGH
|
||||
- **Applies to:** `SKILL.md`
|
||||
- **Rule:** The `name` value must use only lowercase letters, numbers, and hyphens. Max 64 characters. Must not contain "anthropic" or "claude".
|
||||
- **Detection:** Regex test: `^[a-z0-9][a-z0-9-]{0,62}[a-z0-9]$`. String search for forbidden substrings.
|
||||
- **Fix:** Rename to comply with the format.
|
||||
- **Rule:** The `name` value must start with `bmad-`, use only lowercase letters, numbers, and single hyphens between segments.
|
||||
- **Detection:** Regex test: `^bmad-[a-z0-9]+(-[a-z0-9]+)*$`.
|
||||
- **Fix:** Rename to comply with the format (e.g., `bmad-my-skill`).
|
||||
|
||||
### SKILL-05 — `name` Must Match Directory Name
|
||||
|
||||
|
|
@ -88,23 +88,33 @@ If no findings are generated (from either pass), the skill passes validation.
|
|||
- **Detection:** Check length. Look for trigger phrases like "Use when" or "Use if" — their absence suggests the description only says _what_ but not _when_.
|
||||
- **Fix:** Append a "Use when..." clause to the description.
|
||||
|
||||
### SKILL-07 — SKILL.md Must Have Body Content
|
||||
|
||||
- **Severity:** HIGH
|
||||
- **Applies to:** `SKILL.md`
|
||||
- **Rule:** SKILL.md must have non-empty markdown body content after the frontmatter. The body provides L2 instructions — a SKILL.md with only frontmatter is incomplete.
|
||||
- **Detection:** Extract content after the closing `---` frontmatter delimiter and check it is non-empty after trimming whitespace.
|
||||
- **Fix:** Add markdown body with skill instructions after the closing `---`.
|
||||
|
||||
---
|
||||
|
||||
### WF-01 — workflow.md Must NOT Have `name` in Frontmatter
|
||||
### WF-01 — Only SKILL.md May Have `name` in Frontmatter
|
||||
|
||||
- **Severity:** HIGH
|
||||
- **Applies to:** `workflow.md` (if it exists)
|
||||
- **Rule:** The `name` field belongs only in `SKILL.md`. If `workflow.md` has YAML frontmatter, it must not contain `name:`.
|
||||
- **Detection:** Parse frontmatter and check for `name:` key.
|
||||
- **Fix:** Remove the `name:` line from workflow.md frontmatter.
|
||||
- **Applies to:** all `.md` files except `SKILL.md`
|
||||
- **Rule:** The `name` field belongs only in `SKILL.md`. No other markdown file in the skill directory may have `name:` in its frontmatter.
|
||||
- **Detection:** Parse frontmatter of every non-SKILL.md markdown file and check for `name:` key.
|
||||
- **Fix:** Remove the `name:` line from the file's frontmatter.
|
||||
- **Exception:** `bmad-agent-tech-writer` — has sub-skill files with intentional `name` fields (to be revisited).
|
||||
|
||||
### WF-02 — workflow.md Must NOT Have `description` in Frontmatter
|
||||
### WF-02 — Only SKILL.md May Have `description` in Frontmatter
|
||||
|
||||
- **Severity:** HIGH
|
||||
- **Applies to:** `workflow.md` (if it exists)
|
||||
- **Rule:** The `description` field belongs only in `SKILL.md`. If `workflow.md` has YAML frontmatter, it must not contain `description:`.
|
||||
- **Detection:** Parse frontmatter and check for `description:` key.
|
||||
- **Fix:** Remove the `description:` line from workflow.md frontmatter.
|
||||
- **Applies to:** all `.md` files except `SKILL.md`
|
||||
- **Rule:** The `description` field belongs only in `SKILL.md`. No other markdown file in the skill directory may have `description:` in its frontmatter.
|
||||
- **Detection:** Parse frontmatter of every non-SKILL.md markdown file and check for `description:` key.
|
||||
- **Fix:** Remove the `description:` line from the file's frontmatter.
|
||||
- **Exception:** `bmad-agent-tech-writer` — has sub-skill files with intentional `description` fields (to be revisited).
|
||||
|
||||
### WF-03 — workflow.md Frontmatter Variables Must Be Config or Runtime Only
|
||||
|
||||
|
|
|
|||
|
|
@ -42,9 +42,8 @@ const positionalArgs = args.filter((a) => !a.startsWith('--'));
|
|||
|
||||
// --- Constants ---
|
||||
|
||||
const NAME_REGEX = /^[a-z0-9][a-z0-9-]{0,62}[a-z0-9]$/;
|
||||
const NAME_REGEX = /^bmad-[a-z0-9]+(-[a-z0-9]+)*$/;
|
||||
const STEP_FILENAME_REGEX = /^step-\d{2}[a-z]?-[a-z0-9-]+\.md$/;
|
||||
const FORBIDDEN_NAME_SUBSTRINGS = ['anthropic', 'claude'];
|
||||
const TIME_ESTIMATE_PATTERNS = [/takes?\s+\d+\s*min/i, /~\s*\d+\s*min/i, /estimated\s+time/i, /\bETA\b/];
|
||||
|
||||
const SEVERITY_ORDER = { CRITICAL: 0, HIGH: 1, MEDIUM: 2, LOW: 3 };
|
||||
|
|
@ -314,8 +313,7 @@ function validateSkill(skillDir) {
|
|||
const description = skillFm && skillFm.description;
|
||||
|
||||
// --- SKILL-04: name format ---
|
||||
if (name) {
|
||||
if (!NAME_REGEX.test(name)) {
|
||||
if (name && !NAME_REGEX.test(name)) {
|
||||
findings.push({
|
||||
rule: 'SKILL-04',
|
||||
title: 'name Format',
|
||||
|
|
@ -326,20 +324,6 @@ function validateSkill(skillDir) {
|
|||
});
|
||||
}
|
||||
|
||||
for (const forbidden of FORBIDDEN_NAME_SUBSTRINGS) {
|
||||
if (name.toLowerCase().includes(forbidden)) {
|
||||
findings.push({
|
||||
rule: 'SKILL-04',
|
||||
title: 'name Format',
|
||||
severity: 'HIGH',
|
||||
file: 'SKILL.md',
|
||||
detail: `name "${name}" contains forbidden substring "${forbidden}".`,
|
||||
fix: `Remove "${forbidden}" from the name.`,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// --- SKILL-05: name matches directory ---
|
||||
if (name && name !== dirName) {
|
||||
findings.push({
|
||||
|
|
@ -404,30 +388,39 @@ function validateSkill(skillDir) {
|
|||
}
|
||||
}
|
||||
|
||||
// --- WF-01 / WF-02: workflow.md must NOT have name/description ---
|
||||
if (fs.existsSync(workflowMdPath)) {
|
||||
const wfContent = safeReadFile(workflowMdPath, findings, 'workflow.md');
|
||||
const wfFm = wfContent ? parseFrontmatter(wfContent) : null;
|
||||
// --- WF-01 / WF-02: non-SKILL.md files must NOT have name/description ---
|
||||
// TODO: bmad-agent-tech-writer has sub-skill files with intentional name/description
|
||||
const WF_SKIP_SKILLS = new Set(['bmad-agent-tech-writer']);
|
||||
for (const filePath of allFiles) {
|
||||
if (path.extname(filePath) !== '.md') continue;
|
||||
if (path.basename(filePath) === 'SKILL.md') continue;
|
||||
if (WF_SKIP_SKILLS.has(dirName)) continue;
|
||||
|
||||
if (wfFm && 'name' in wfFm) {
|
||||
const relFile = path.relative(skillDir, filePath);
|
||||
const content = safeReadFile(filePath, findings, relFile);
|
||||
if (content === null) continue;
|
||||
const fm = parseFrontmatter(content);
|
||||
if (!fm) continue;
|
||||
|
||||
if ('name' in fm) {
|
||||
findings.push({
|
||||
rule: 'WF-01',
|
||||
title: 'workflow.md Must NOT Have name in Frontmatter',
|
||||
title: 'Only SKILL.md May Have name in Frontmatter',
|
||||
severity: 'HIGH',
|
||||
file: 'workflow.md',
|
||||
detail: 'workflow.md frontmatter contains `name` — this belongs only in SKILL.md.',
|
||||
fix: 'Remove the `name:` line from workflow.md frontmatter.',
|
||||
file: relFile,
|
||||
detail: `${relFile} frontmatter contains \`name\` — this belongs only in SKILL.md.`,
|
||||
fix: "Remove the `name:` line from this file's frontmatter.",
|
||||
});
|
||||
}
|
||||
|
||||
if (wfFm && 'description' in wfFm) {
|
||||
if ('description' in fm) {
|
||||
findings.push({
|
||||
rule: 'WF-02',
|
||||
title: 'workflow.md Must NOT Have description in Frontmatter',
|
||||
title: 'Only SKILL.md May Have description in Frontmatter',
|
||||
severity: 'HIGH',
|
||||
file: 'workflow.md',
|
||||
detail: 'workflow.md frontmatter contains `description` — this belongs only in SKILL.md.',
|
||||
fix: 'Remove the `description:` line from workflow.md frontmatter.',
|
||||
file: relFile,
|
||||
detail: `${relFile} frontmatter contains \`description\` — this belongs only in SKILL.md.`,
|
||||
fix: "Remove the `description:` line from this file's frontmatter.",
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue