Compare commits

..

No commits in common. "84bade9a9590af8ea57c51524deaab43bba08391" and "22035ef01579103349ea7c0c629abf4c95bafb2b" have entirely different histories.

4 changed files with 70 additions and 109 deletions

View File

@ -26,7 +26,7 @@ deferred_work_file: '{implementation_artifacts}/deferred-work.md'
Present summary. If token count exceeded 1600 and user chose [K], include the token count and explain why it may be a problem. HALT and ask human: `[A] Approve` | `[E] Edit` Present summary. If token count exceeded 1600 and user chose [K], include the token count and explain why it may be a problem. HALT and ask human: `[A] Approve` | `[E] Edit`
- **A**: Rename `{wipFile}` to `{spec_file}`, set status `ready-for-dev`. Everything inside `<frozen-after-approval>` is now locked — only the human can change it. Display the finalized spec path to the user as a CWD-relative path (no leading `/`) so it is clickable in the terminal. → Step 3. - **A**: Rename `{wipFile}` to `{spec_file}`, set status `ready-for-dev`. Everything inside `<frozen-after-approval>` is now locked — only the human can change it. → Step 3.
- **E**: Apply changes, then return to CHECKPOINT 1. - **E**: Apply changes, then return to CHECKPOINT 1.

View File

@ -22,7 +22,7 @@ Build the trail as an ordered sequence of **stops** — clickable `path:line` re
2. **Lead with the entry point** — the single highest-leverage file:line a reviewer should look at first to grasp the design intent. 2. **Lead with the entry point** — the single highest-leverage file:line a reviewer should look at first to grasp the design intent.
3. **Inside each concern**, order stops from most important / architecturally interesting to supporting. Lightly bias toward higher-risk or boundary-crossing stops. 3. **Inside each concern**, order stops from most important / architecturally interesting to supporting. Lightly bias toward higher-risk or boundary-crossing stops.
4. **End with peripherals** — tests, config, types, and other supporting changes come last. 4. **End with peripherals** — tests, config, types, and other supporting changes come last.
5. **Every code reference is a clickable workspace-relative link** (project-root-relative for clickability in the editor). Format each stop as a markdown link: `[short-name:line](/project-root-relative/path/to/file.ts#L42)`. The link target uses a leading `/` (workspace root) with a `#L` line anchor. Use the file's basename (or shortest unambiguous suffix) plus line number as the link text. 5. **Every code reference is a clickable workspace-relative link.** Format each stop as a markdown link: `[short-name:line](/project-root-relative/path/to/file.ts#L42)`. The link target uses a leading `/` (workspace root) with a `#L` line anchor. Use the file's basename (or shortest unambiguous suffix) plus line number as the link text.
6. **Each stop gets one ultra-concise line of framing** (≤15 words) — why this approach was chosen here and what it achieves in the context of the change. No paragraphs. 6. **Each stop gets one ultra-concise line of framing** (≤15 words) — why this approach was chosen here and what it achieves in the context of the change. No paragraphs.
Format each stop as framing first, link on the next indented line: Format each stop as framing first, link on the next indented line:
@ -53,7 +53,7 @@ When there is only one concern, omit the bold label — just list the stops dire
3. Open the spec in the user's editor so they can click through the Suggested Review Order: 3. Open the spec in the user's editor so they can click through the Suggested Review Order:
- Run `code -r "{spec_file}"` to open the spec in the current VS Code window (reuses the window where the project or worktree is open). Always double-quote the path to handle spaces and special characters. - Run `code -r "{spec_file}"` to open the spec in the current VS Code window (reuses the window where the project or worktree is open). Always double-quote the path to handle spaces and special characters.
- If `code` is not available (command fails), skip gracefully and tell the user the spec file path instead. - If `code` is not available (command fails), skip gracefully and tell the user the spec file path instead.
4. Display summary of your work to the user, including the commit hash if one was created. Any file paths shown in conversation/terminal output must use CWD-relative format (no leading `/`) for terminal clickability — this differs from spec-file links which use project-root-relative paths. Include: 4. Display summary of your work to the user, including the commit hash if one was created. Include:
- A note that the spec is open in their editor (or the file path if it couldn't be opened). Mention that `{spec_file}` now contains a Suggested Review Order. - A note that the spec is open in their editor (or the file path if it couldn't be opened). Mention that `{spec_file}` now contains a Suggested Review Order.
- **Navigation tip:** "Ctrl+click (Cmd+click on macOS) the links in the Suggested Review Order to jump to each stop." - **Navigation tip:** "Ctrl+click (Cmd+click on macOS) the links in the Suggested Review Order to jump to each stop."
- Offer to push and/or create a pull request. - Offer to push and/or create a pull request.

View File

@ -10,7 +10,7 @@ Before running inference-based validation, run the deterministic validator:
node tools/validate-skills.js --json path/to/skill-dir node tools/validate-skills.js --json path/to/skill-dir
``` ```
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. 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.
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). 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 - **Severity:** HIGH
- **Applies to:** `SKILL.md` - **Applies to:** `SKILL.md`
- **Rule:** The `name` value must start with `bmad-`, use only lowercase letters, numbers, and single hyphens between segments. - **Rule:** The `name` value must use only lowercase letters, numbers, and hyphens. Max 64 characters. Must not contain "anthropic" or "claude".
- **Detection:** Regex test: `^bmad-[a-z0-9]+(-[a-z0-9]+)*$`. - **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 (e.g., `bmad-my-skill`). - **Fix:** Rename to comply with the format.
### SKILL-05 — `name` Must Match Directory Name ### SKILL-05 — `name` Must Match Directory Name
@ -88,33 +88,23 @@ 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_. - **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. - **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 — Only SKILL.md May Have `name` in Frontmatter ### WF-01 — workflow.md Must NOT Have `name` in Frontmatter
- **Severity:** HIGH - **Severity:** HIGH
- **Applies to:** all `.md` files except `SKILL.md` - **Applies to:** `workflow.md` (if it exists)
- **Rule:** The `name` field belongs only in `SKILL.md`. No other markdown file in the skill directory may have `name:` in its frontmatter. - **Rule:** The `name` field belongs only in `SKILL.md`. If `workflow.md` has YAML frontmatter, it must not contain `name:`.
- **Detection:** Parse frontmatter of every non-SKILL.md markdown file and check for `name:` key. - **Detection:** Parse frontmatter and check for `name:` key.
- **Fix:** Remove the `name:` line from the file's frontmatter. - **Fix:** Remove the `name:` line from workflow.md frontmatter.
- **Exception:** `bmad-agent-tech-writer` — has sub-skill files with intentional `name` fields (to be revisited).
### WF-02 — Only SKILL.md May Have `description` in Frontmatter ### WF-02 — workflow.md Must NOT Have `description` in Frontmatter
- **Severity:** HIGH - **Severity:** HIGH
- **Applies to:** all `.md` files except `SKILL.md` - **Applies to:** `workflow.md` (if it exists)
- **Rule:** The `description` field belongs only in `SKILL.md`. No other markdown file in the skill directory may have `description:` in its frontmatter. - **Rule:** The `description` field belongs only in `SKILL.md`. If `workflow.md` has YAML frontmatter, it must not contain `description:`.
- **Detection:** Parse frontmatter of every non-SKILL.md markdown file and check for `description:` key. - **Detection:** Parse frontmatter and check for `description:` key.
- **Fix:** Remove the `description:` line from the file's frontmatter. - **Fix:** Remove the `description:` line from workflow.md 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 ### WF-03 — workflow.md Frontmatter Variables Must Be Config or Runtime Only

View File

@ -1,7 +1,7 @@
/** /**
* Deterministic Skill Validator * Deterministic Skill Validator
* *
* Validates 14 deterministic rules across all skill directories. * Validates 13 deterministic rules across all skill directories.
* Acts as a fast first-pass complement to the inference-based skill validator. * Acts as a fast first-pass complement to the inference-based skill validator.
* *
* What it checks: * What it checks:
@ -11,7 +11,6 @@
* - SKILL-04: name format (lowercase, hyphens, no forbidden substrings) * - SKILL-04: name format (lowercase, hyphens, no forbidden substrings)
* - SKILL-05: name matches directory basename * - SKILL-05: name matches directory basename
* - SKILL-06: description quality (length, "Use when"/"Use if") * - SKILL-06: description quality (length, "Use when"/"Use if")
* - SKILL-07: SKILL.md has body content after frontmatter
* - WF-01: workflow.md frontmatter has no name * - WF-01: workflow.md frontmatter has no name
* - WF-02: workflow.md frontmatter has no description * - WF-02: workflow.md frontmatter has no description
* - PATH-02: no installed_path variable * - PATH-02: no installed_path variable
@ -42,8 +41,14 @@ const positionalArgs = args.filter((a) => !a.startsWith('--'));
// --- Constants --- // --- Constants ---
const NAME_REGEX = /^bmad-[a-z0-9]+(-[a-z0-9]+)*$/; const SKILL_LOCATIONS = [path.join(SRC_DIR, 'core', 'skills'), path.join(SRC_DIR, 'core', 'tasks'), path.join(SRC_DIR, 'bmm', 'workflows')];
// Agent skills live separately
const AGENT_LOCATION = path.join(SRC_DIR, 'bmm', 'agents');
const NAME_REGEX = /^[a-z0-9][a-z0-9-]{0,62}[a-z0-9]$/;
const STEP_FILENAME_REGEX = /^step-\d{2}[a-z]?-[a-z0-9-]+\.md$/; 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 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 }; const SEVERITY_ORDER = { CRITICAL: 0, HIGH: 1, MEDIUM: 2, LOW: 3 };
@ -68,15 +73,8 @@ function parseFrontmatter(content) {
const trimmed = content.trimStart(); const trimmed = content.trimStart();
if (!trimmed.startsWith('---')) return null; if (!trimmed.startsWith('---')) return null;
let endIndex = trimmed.indexOf('\n---\n', 3); const endIndex = trimmed.indexOf('\n---', 3);
if (endIndex === -1) { if (endIndex === -1) return null;
// Handle file ending with \n---
if (trimmed.endsWith('\n---')) {
endIndex = trimmed.length - 4;
} else {
return null;
}
}
const fmBlock = trimmed.slice(3, endIndex).trim(); const fmBlock = trimmed.slice(3, endIndex).trim();
if (fmBlock === '') return {}; if (fmBlock === '') return {};
@ -107,15 +105,8 @@ function parseFrontmatterMultiline(content) {
const trimmed = content.trimStart(); const trimmed = content.trimStart();
if (!trimmed.startsWith('---')) return null; if (!trimmed.startsWith('---')) return null;
let endIndex = trimmed.indexOf('\n---\n', 3); const endIndex = trimmed.indexOf('\n---', 3);
if (endIndex === -1) { if (endIndex === -1) return null;
// Handle file ending with \n---
if (trimmed.endsWith('\n---')) {
endIndex = trimmed.length - 4;
} else {
return null;
}
}
const fmBlock = trimmed.slice(3, endIndex).trim(); const fmBlock = trimmed.slice(3, endIndex).trim();
if (fmBlock === '') return {}; if (fmBlock === '') return {};
@ -259,7 +250,7 @@ function validateSkill(skillDir) {
detail: 'SKILL.md not found in skill directory.', detail: 'SKILL.md not found in skill directory.',
fix: 'Create SKILL.md as the skill entrypoint.', fix: 'Create SKILL.md as the skill entrypoint.',
}); });
// Cannot check SKILL-02 through SKILL-07 without SKILL.md // Cannot check SKILL-02 through SKILL-06 without SKILL.md
return findings; return findings;
} }
@ -313,7 +304,8 @@ function validateSkill(skillDir) {
const description = skillFm && skillFm.description; const description = skillFm && skillFm.description;
// --- SKILL-04: name format --- // --- SKILL-04: name format ---
if (name && !NAME_REGEX.test(name)) { if (name) {
if (!NAME_REGEX.test(name)) {
findings.push({ findings.push({
rule: 'SKILL-04', rule: 'SKILL-04',
title: 'name Format', title: 'name Format',
@ -324,6 +316,20 @@ 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 --- // --- SKILL-05: name matches directory ---
if (name && name !== dirName) { if (name && name !== dirName) {
findings.push({ findings.push({
@ -361,66 +367,30 @@ function validateSkill(skillDir) {
} }
} }
// --- SKILL-07: SKILL.md must have body content after frontmatter --- // --- WF-01 / WF-02: workflow.md must NOT have name/description ---
{ if (fs.existsSync(workflowMdPath)) {
const trimmed = skillContent.trimStart(); const wfContent = safeReadFile(workflowMdPath, findings, 'workflow.md');
let bodyStart = -1; const wfFm = wfContent ? parseFrontmatter(wfContent) : null;
if (trimmed.startsWith('---')) {
let endIdx = trimmed.indexOf('\n---\n', 3);
if (endIdx !== -1) {
bodyStart = endIdx + 4;
} else if (trimmed.endsWith('\n---')) {
bodyStart = trimmed.length; // no body at all
}
} else {
bodyStart = 0; // no frontmatter, entire file is body
}
const body = bodyStart >= 0 ? trimmed.slice(bodyStart).trim() : '';
if (body === '') {
findings.push({
rule: 'SKILL-07',
title: 'SKILL.md Must Have Body Content',
severity: 'HIGH',
file: 'SKILL.md',
detail: 'SKILL.md has no content after frontmatter. L2 instructions are required.',
fix: 'Add markdown body with skill instructions after the closing ---.',
});
}
}
// --- WF-01 / WF-02: non-SKILL.md files must NOT have name/description --- if (wfFm && 'name' in wfFm) {
// 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;
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({ findings.push({
rule: 'WF-01', rule: 'WF-01',
title: 'Only SKILL.md May Have name in Frontmatter', title: 'workflow.md Must NOT Have name in Frontmatter',
severity: 'HIGH', severity: 'HIGH',
file: relFile, file: 'workflow.md',
detail: `${relFile} frontmatter contains \`name\` — this belongs only in SKILL.md.`, detail: 'workflow.md frontmatter contains `name` — this belongs only in SKILL.md.',
fix: "Remove the `name:` line from this file's frontmatter.", fix: 'Remove the `name:` line from workflow.md frontmatter.',
}); });
} }
if ('description' in fm) { if (wfFm && 'description' in wfFm) {
findings.push({ findings.push({
rule: 'WF-02', rule: 'WF-02',
title: 'Only SKILL.md May Have description in Frontmatter', title: 'workflow.md Must NOT Have description in Frontmatter',
severity: 'HIGH', severity: 'HIGH',
file: relFile, file: 'workflow.md',
detail: `${relFile} frontmatter contains \`description\` — this belongs only in SKILL.md.`, detail: 'workflow.md frontmatter contains `description` — this belongs only in SKILL.md.',
fix: "Remove the `description:` line from this file's frontmatter.", fix: 'Remove the `description:` line from workflow.md frontmatter.',
}); });
} }
} }
@ -705,7 +675,8 @@ if (require.main === module) {
skillDirs = [target]; skillDirs = [target];
} else { } else {
// Discover all skills // Discover all skills
skillDirs = discoverSkillDirs([SRC_DIR]); const allLocations = [...SKILL_LOCATIONS, AGENT_LOCATION];
skillDirs = discoverSkillDirs(allLocations);
} }
if (skillDirs.length === 0) { if (skillDirs.length === 0) {