From 05c1b204585b7834fbc63f7df58f806a2b490cb2 Mon Sep 17 00:00:00 2001 From: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> Date: Sat, 20 Jun 2026 11:38:33 +0200 Subject: [PATCH 1/2] fix(validate-skills): exempt deprecated skills from SKILL-06 trigger SKILL-06 flagged deprecated compatibility shims for missing a "Use when" trigger phrase. Deprecated skills omit it on purpose so users are steered to their replacement, so exempt them from that specific check. - detect deprecation via description starting with "DEPRECATED" - keep the other SKILL-06 check (length) and all other rules applying - add test/test-validate-skills.js + fixtures, wired into npm test --- package.json | 3 +- .../validate-skills/deprecated-shim/SKILL.md | 9 ++ .../validate-skills/missing-trigger/SKILL.md | 9 ++ .../validate-skills/with-trigger/SKILL.md | 8 ++ test/test-validate-skills.js | 92 +++++++++++++++++++ tools/validate-skills.js | 7 +- 6 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/validate-skills/deprecated-shim/SKILL.md create mode 100644 test/fixtures/validate-skills/missing-trigger/SKILL.md create mode 100644 test/fixtures/validate-skills/with-trigger/SKILL.md create mode 100644 test/test-validate-skills.js diff --git a/package.json b/package.json index 505c6e8e0..2ed9678ca 100644 --- a/package.json +++ b/package.json @@ -42,10 +42,11 @@ "prepare": "command -v husky >/dev/null 2>&1 && husky || exit 0", "quality": "npm run format:check && npm run lint && npm run lint:md && npm run docs:build && npm run test:install && npm run test:urls && npm run validate:refs && npm run validate:skills && npm run docs:validate-sidebar", "rebundle": "node tools/installer/bundlers/bundle-web.js rebundle", - "test": "npm run test:refs && npm run test:install && npm run test:urls && npm run test:channels && npm run lint && npm run lint:md && npm run format:check", + "test": "npm run test:refs && npm run test:install && npm run test:urls && npm run test:channels && npm run test:skills && npm run lint && npm run lint:md && npm run format:check", "test:channels": "node test/test-installer-channels.js", "test:install": "node test/test-installation-components.js", "test:refs": "node test/test-file-refs-csv.js", + "test:skills": "node test/test-validate-skills.js", "test:urls": "node test/test-parse-source-urls.js", "validate:refs": "node tools/validate-file-refs.js --strict", "validate:skills": "node tools/validate-skills.js --strict" diff --git a/test/fixtures/validate-skills/deprecated-shim/SKILL.md b/test/fixtures/validate-skills/deprecated-shim/SKILL.md new file mode 100644 index 000000000..522889e0a --- /dev/null +++ b/test/fixtures/validate-skills/deprecated-shim/SKILL.md @@ -0,0 +1,9 @@ +--- +name: deprecated-shim +description: 'DEPRECATED — consolidated into bmad-foo; this skill will be removed in v7 in favor of `bmad-foo`.' +--- + +# DEPRECATED — forwards to bmad-foo + +This skill was consolidated into `bmad-foo` and is retained as a thin compatibility +shim so existing invocations keep working. New work should invoke `bmad-foo` directly. diff --git a/test/fixtures/validate-skills/missing-trigger/SKILL.md b/test/fixtures/validate-skills/missing-trigger/SKILL.md new file mode 100644 index 000000000..1d098ea82 --- /dev/null +++ b/test/fixtures/validate-skills/missing-trigger/SKILL.md @@ -0,0 +1,9 @@ +--- +name: missing-trigger +description: 'Generates a thing and writes it to disk for the user.' +--- + +# Missing Trigger + +An active (non-deprecated) skill whose description omits a "Use when" trigger phrase. +This fixture guards against regressions: SKILL-06 must still flag it. diff --git a/test/fixtures/validate-skills/with-trigger/SKILL.md b/test/fixtures/validate-skills/with-trigger/SKILL.md new file mode 100644 index 000000000..1ae6f994b --- /dev/null +++ b/test/fixtures/validate-skills/with-trigger/SKILL.md @@ -0,0 +1,8 @@ +--- +name: with-trigger +description: 'Generates a thing and writes it to disk. Use when the user asks to scaffold a thing.' +--- + +# With Trigger + +An active skill whose description includes a "Use when" trigger phrase. diff --git a/test/test-validate-skills.js b/test/test-validate-skills.js new file mode 100644 index 000000000..64bb3a858 --- /dev/null +++ b/test/test-validate-skills.js @@ -0,0 +1,92 @@ +/** + * Skill Validation Test Runner + * + * Tests validateSkill() from validate-skills.js against fixtures, focused on + * SKILL-06 (description quality) and its deprecated-skill exemption. + * + * Usage: node test/test-validate-skills.js + * Exit codes: 0 = all tests pass, 1 = test failures + */ + +const path = require('node:path'); +const { validateSkill } = require('../tools/validate-skills.js'); + +// ANSI color codes +const colors = { + reset: '\u001B[0m', + green: '\u001B[32m', + red: '\u001B[31m', + cyan: '\u001B[36m', + dim: '\u001B[2m', +}; + +const FIXTURES = path.join(__dirname, 'fixtures/validate-skills'); + +let totalTests = 0; +let passedTests = 0; +const failures = []; + +function test(name, fn) { + totalTests++; + try { + fn(); + passedTests++; + console.log(` ${colors.green}✓${colors.reset} ${name}`); + } catch (error) { + console.log(` ${colors.red}✗${colors.reset} ${name} ${colors.red}${error.message}${colors.reset}`); + failures.push({ name, message: error.message }); + } +} + +function assert(condition, message) { + if (!condition) throw new Error(message); +} + +// True when validateSkill emitted the SKILL-06 "Use when/Use if" trigger finding. +function hasTriggerFinding(skillName) { + const findings = validateSkill(path.join(FIXTURES, skillName)); + return findings.some((f) => f.rule === 'SKILL-06' && /trigger phrase/i.test(f.detail)); +} + +console.log(`\n${colors.cyan}Skill Validation — SKILL-06 trigger phrase${colors.reset}\n`); + +test('deprecated skill is exempt from the trigger-phrase requirement', () => { + assert( + hasTriggerFinding('deprecated-shim') === false, + 'Expected no SKILL-06 trigger finding for a DEPRECATED skill', + ); +}); + +test('active skill missing a trigger phrase is still flagged', () => { + assert( + hasTriggerFinding('missing-trigger') === true, + 'Expected a SKILL-06 trigger finding for a non-deprecated skill without "Use when"', + ); +}); + +test('active skill with a "Use when" trigger is not flagged', () => { + assert( + hasTriggerFinding('with-trigger') === false, + 'Expected no SKILL-06 trigger finding when description contains "Use when"', + ); +}); + +// --- Summary --- +console.log(`\n${colors.cyan}${'═'.repeat(55)}${colors.reset}`); +console.log(`${colors.cyan}Test Results:${colors.reset}`); +console.log(` Total: ${totalTests}`); +console.log(` Passed: ${colors.green}${passedTests}${colors.reset}`); +console.log(` Failed: ${passedTests === totalTests ? colors.green : colors.red}${totalTests - passedTests}${colors.reset}`); +console.log(`${colors.cyan}${'═'.repeat(55)}${colors.reset}\n`); + +if (failures.length > 0) { + console.log(`${colors.red}FAILED TESTS:${colors.reset}\n`); + for (const failure of failures) { + console.log(`${colors.red}✗${colors.reset} ${failure.name}`); + console.log(` ${failure.message}\n`); + } + process.exit(1); +} + +console.log(`${colors.green}All tests passed!${colors.reset}\n`); +process.exit(0); diff --git a/tools/validate-skills.js b/tools/validate-skills.js index 8ab5bc2ad..979a695e8 100644 --- a/tools/validate-skills.js +++ b/tools/validate-skills.js @@ -312,6 +312,11 @@ function validateSkill(skillDir) { const name = skillFm && skillFm.name; const description = skillFm && skillFm.description; + // Deprecated skills are thin compatibility shims that forward to a replacement. + // They intentionally omit a "Use when" trigger so users are steered to the new + // skill instead, so exempt them from the SKILL-06 trigger-phrase requirement. + const isDeprecated = typeof description === 'string' && /^\s*deprecated\b/i.test(description); + // --- SKILL-04: name format --- if (name && !NAME_REGEX.test(name)) { findings.push({ @@ -349,7 +354,7 @@ function validateSkill(skillDir) { }); } - if (!/use\s+when\b/i.test(description) && !/use\s+if\b/i.test(description)) { + if (!isDeprecated && !/use\s+when\b/i.test(description) && !/use\s+if\b/i.test(description)) { findings.push({ rule: 'SKILL-06', title: 'description Quality', From aba48f90f1763b649938133c18aab54fb05f1684 Mon Sep 17 00:00:00 2001 From: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> Date: Sat, 20 Jun 2026 14:38:38 +0200 Subject: [PATCH 2/2] test(validate-skills): document test helpers with JSDoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added JSDoc to the test/assert/hasTriggerFinding helpers so the new test file documents its own helpers. No behaviour change — the suite still passes 3/3. --- test/test-validate-skills.js | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/test/test-validate-skills.js b/test/test-validate-skills.js index 64bb3a858..ddc518c59 100644 --- a/test/test-validate-skills.js +++ b/test/test-validate-skills.js @@ -26,6 +26,11 @@ let totalTests = 0; let passedTests = 0; const failures = []; +/** + * Run a single named test case, recording the result and printing a status line. + * @param {string} name - Human-readable test description. + * @param {Function} fn - Test body; throw to signal failure. + */ function test(name, fn) { totalTests++; try { @@ -38,11 +43,21 @@ function test(name, fn) { } } +/** + * Throw an Error with `message` when `condition` is falsy. + * @param {boolean} condition - Expression that must hold. + * @param {string} message - Failure message. + */ function assert(condition, message) { if (!condition) throw new Error(message); } -// True when validateSkill emitted the SKILL-06 "Use when/Use if" trigger finding. +/** + * Whether validateSkill emitted the SKILL-06 "Use when/Use if" trigger finding + * for the given fixture skill directory. + * @param {string} skillName - Fixture subdirectory name under FIXTURES. + * @returns {boolean} True if the trigger-phrase finding was reported. + */ function hasTriggerFinding(skillName) { const findings = validateSkill(path.join(FIXTURES, skillName)); return findings.some((f) => f.rule === 'SKILL-06' && /trigger phrase/i.test(f.detail));