From 6dc292c850a957eac12cc96ac21fb452e7b7ef01 Mon Sep 17 00:00:00 2001 From: cx-noam-brendel <139764378+cx-noam-brendel@users.noreply.github.com> Date: Wed, 14 Jan 2026 13:28:41 +0200 Subject: [PATCH 1/4] fix(bmb): add module.yaml schema validation and fix #1288 - Add Zod schema validation for module.yaml files - Create test suite for module schema with 17 test fixtures - Fix create-module workflow to make 'code' field requirements clearer Fixes #1288 - The create-module workflow could produce module.yaml files without the required 'code' field or with placeholder text like '{module_code}'. 1. Created tools/schema/module.js with Zod validation for: - Required fields: code, name, header, subheader - Code format: kebab-case, 2-20 chars, starts with letter - Variable definitions with prompt/inherit patterns 2. Created tools/validate-module-schema.js CLI validator 3. Added npm scripts: - validate:modules - validates all module.yaml files - test:modules - runs module schema test suite 4. Updated BMB workflow instructions with explicit warnings about required fields in step-03-config.md and step-03-module-yaml.md 5. Added 17 test fixtures covering: - Missing required fields (code, name, header, subheader) - Invalid code formats (placeholder, uppercase, underscore, etc.) - Variable definitions (prompt, inherit, single-select, multi-select) - npm run test:modules - 17/17 tests pass - npm run validate:modules - All 5 existing modules pass - npm test - Full test suite passes --- package.json | 4 +- .../code-format/number-start-code.module.yaml | 11 + .../code-format/placeholder-code.module.yaml | 11 + .../code-format/too-short-code.module.yaml | 11 + .../code-format/underscore-code.module.yaml | 11 + .../code-format/uppercase-code.module.yaml | 11 + .../required-fields/missing-code.module.yaml | 11 + .../missing-header.module.yaml | 11 + .../required-fields/missing-name.module.yaml | 11 + .../missing-subheader.module.yaml | 11 + .../variables/empty-prompt.module.yaml | 15 + .../variables/missing-prompt.module.yaml | 16 + .../valid/minimal/minimal-module.module.yaml | 9 + .../minimal/no-default-selected.module.yaml | 8 + .../valid/minimal/short-code.module.yaml | 9 + .../valid/variables/with-inherit.module.yaml | 12 + .../valid/variables/with-prompt.module.yaml | 14 + .../valid/variables/with-select.module.yaml | 31 ++ test/test-module-schema.js | 339 ++++++++++++++++++ tools/schema/module.js | 188 ++++++++++ tools/validate-module-schema.js | 110 ++++++ 21 files changed, 853 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/module-schema/invalid/code-format/number-start-code.module.yaml create mode 100644 test/fixtures/module-schema/invalid/code-format/placeholder-code.module.yaml create mode 100644 test/fixtures/module-schema/invalid/code-format/too-short-code.module.yaml create mode 100644 test/fixtures/module-schema/invalid/code-format/underscore-code.module.yaml create mode 100644 test/fixtures/module-schema/invalid/code-format/uppercase-code.module.yaml create mode 100644 test/fixtures/module-schema/invalid/required-fields/missing-code.module.yaml create mode 100644 test/fixtures/module-schema/invalid/required-fields/missing-header.module.yaml create mode 100644 test/fixtures/module-schema/invalid/required-fields/missing-name.module.yaml create mode 100644 test/fixtures/module-schema/invalid/required-fields/missing-subheader.module.yaml create mode 100644 test/fixtures/module-schema/invalid/variables/empty-prompt.module.yaml create mode 100644 test/fixtures/module-schema/invalid/variables/missing-prompt.module.yaml create mode 100644 test/fixtures/module-schema/valid/minimal/minimal-module.module.yaml create mode 100644 test/fixtures/module-schema/valid/minimal/no-default-selected.module.yaml create mode 100644 test/fixtures/module-schema/valid/minimal/short-code.module.yaml create mode 100644 test/fixtures/module-schema/valid/variables/with-inherit.module.yaml create mode 100644 test/fixtures/module-schema/valid/variables/with-prompt.module.yaml create mode 100644 test/fixtures/module-schema/valid/variables/with-select.module.yaml create mode 100644 test/test-module-schema.js create mode 100644 tools/schema/module.js create mode 100644 tools/validate-module-schema.js diff --git a/package.json b/package.json index ed427489..1e926720 100644 --- a/package.json +++ b/package.json @@ -45,10 +45,12 @@ "release:minor": "gh workflow run \"Manual Release\" -f version_bump=minor", "release:patch": "gh workflow run \"Manual Release\" -f version_bump=patch", "release:watch": "gh run watch", - "test": "npm run test:schemas && npm run test:install && npm run validate:schemas && npm run lint && npm run lint:md && npm run format:check", + "test": "npm run test:schemas && npm run test:modules && npm run test:install && npm run validate:schemas && npm run validate:modules && npm run lint && npm run lint:md && npm run format:check", "test:coverage": "c8 --reporter=text --reporter=html npm run test:schemas", "test:install": "node test/test-installation-components.js", + "test:modules": "node test/test-module-schema.js", "test:schemas": "node test/test-agent-schema.js", + "validate:modules": "node tools/validate-module-schema.js", "validate:schemas": "node tools/validate-agent-schema.js" }, "lint-staged": { diff --git a/test/fixtures/module-schema/invalid/code-format/number-start-code.module.yaml b/test/fixtures/module-schema/invalid/code-format/number-start-code.module.yaml new file mode 100644 index 00000000..622e24e6 --- /dev/null +++ b/test/fixtures/module-schema/invalid/code-format/number-start-code.module.yaml @@ -0,0 +1,11 @@ +# Test: Code field starts with a number +# Expected: FAIL +# Error code: invalid_string +# Error path: code + +code: 123-module +name: Test Module +header: Test Header +subheader: Test Subheader +default_selected: false + diff --git a/test/fixtures/module-schema/invalid/code-format/placeholder-code.module.yaml b/test/fixtures/module-schema/invalid/code-format/placeholder-code.module.yaml new file mode 100644 index 00000000..73cd56cb --- /dev/null +++ b/test/fixtures/module-schema/invalid/code-format/placeholder-code.module.yaml @@ -0,0 +1,11 @@ +# Test: Code field contains placeholder text (the main bug we're fixing) +# Expected: FAIL +# Error code: invalid_string +# Error path: code + +code: "{module_code}" +name: Test Module +header: Test Header +subheader: Test Subheader +default_selected: false + diff --git a/test/fixtures/module-schema/invalid/code-format/too-short-code.module.yaml b/test/fixtures/module-schema/invalid/code-format/too-short-code.module.yaml new file mode 100644 index 00000000..c40a7551 --- /dev/null +++ b/test/fixtures/module-schema/invalid/code-format/too-short-code.module.yaml @@ -0,0 +1,11 @@ +# Test: Code field too short (minimum 2 characters) +# Expected: FAIL +# Error code: invalid_string +# Error path: code + +code: x +name: Test Module +header: Test Header +subheader: Test Subheader +default_selected: false + diff --git a/test/fixtures/module-schema/invalid/code-format/underscore-code.module.yaml b/test/fixtures/module-schema/invalid/code-format/underscore-code.module.yaml new file mode 100644 index 00000000..0938e074 --- /dev/null +++ b/test/fixtures/module-schema/invalid/code-format/underscore-code.module.yaml @@ -0,0 +1,11 @@ +# Test: Code field with underscores (should be kebab-case) +# Expected: FAIL +# Error code: invalid_string +# Error path: code + +code: test_module +name: Test Module +header: Test Header +subheader: Test Subheader +default_selected: false + diff --git a/test/fixtures/module-schema/invalid/code-format/uppercase-code.module.yaml b/test/fixtures/module-schema/invalid/code-format/uppercase-code.module.yaml new file mode 100644 index 00000000..23cafab3 --- /dev/null +++ b/test/fixtures/module-schema/invalid/code-format/uppercase-code.module.yaml @@ -0,0 +1,11 @@ +# Test: Code field with uppercase letters +# Expected: FAIL +# Error code: invalid_string +# Error path: code + +code: TestModule +name: Test Module +header: Test Header +subheader: Test Subheader +default_selected: false + diff --git a/test/fixtures/module-schema/invalid/required-fields/missing-code.module.yaml b/test/fixtures/module-schema/invalid/required-fields/missing-code.module.yaml new file mode 100644 index 00000000..f0fabfe3 --- /dev/null +++ b/test/fixtures/module-schema/invalid/required-fields/missing-code.module.yaml @@ -0,0 +1,11 @@ +# Test: Missing required code field +# Expected: FAIL +# Error code: invalid_type +# Error path: code +# Error message: Required + +name: Test Module +header: Test Header +subheader: Test Subheader +default_selected: false + diff --git a/test/fixtures/module-schema/invalid/required-fields/missing-header.module.yaml b/test/fixtures/module-schema/invalid/required-fields/missing-header.module.yaml new file mode 100644 index 00000000..2e712371 --- /dev/null +++ b/test/fixtures/module-schema/invalid/required-fields/missing-header.module.yaml @@ -0,0 +1,11 @@ +# Test: Missing required header field +# Expected: FAIL +# Error code: invalid_type +# Error path: header +# Error message: Required + +code: test-module +name: Test Module +subheader: Test Subheader +default_selected: false + diff --git a/test/fixtures/module-schema/invalid/required-fields/missing-name.module.yaml b/test/fixtures/module-schema/invalid/required-fields/missing-name.module.yaml new file mode 100644 index 00000000..3b36b5fa --- /dev/null +++ b/test/fixtures/module-schema/invalid/required-fields/missing-name.module.yaml @@ -0,0 +1,11 @@ +# Test: Missing required name field +# Expected: FAIL +# Error code: invalid_type +# Error path: name +# Error message: Required + +code: test-module +header: Test Header +subheader: Test Subheader +default_selected: false + diff --git a/test/fixtures/module-schema/invalid/required-fields/missing-subheader.module.yaml b/test/fixtures/module-schema/invalid/required-fields/missing-subheader.module.yaml new file mode 100644 index 00000000..6ad37d21 --- /dev/null +++ b/test/fixtures/module-schema/invalid/required-fields/missing-subheader.module.yaml @@ -0,0 +1,11 @@ +# Test: Missing required subheader field +# Expected: FAIL +# Error code: invalid_type +# Error path: subheader +# Error message: Required + +code: test-module +name: Test Module +header: Test Header +default_selected: false + diff --git a/test/fixtures/module-schema/invalid/variables/empty-prompt.module.yaml b/test/fixtures/module-schema/invalid/variables/empty-prompt.module.yaml new file mode 100644 index 00000000..97472867 --- /dev/null +++ b/test/fixtures/module-schema/invalid/variables/empty-prompt.module.yaml @@ -0,0 +1,15 @@ +# Test: Variable with empty prompt +# Expected: FAIL +# Error code: custom +# Error path: my_variable +# Error message: my_variable.prompt must be a non-empty string + +code: test-module +name: Test Module +header: Test Header +subheader: Test Subheader +default_selected: false + +my_variable: + prompt: " " + diff --git a/test/fixtures/module-schema/invalid/variables/missing-prompt.module.yaml b/test/fixtures/module-schema/invalid/variables/missing-prompt.module.yaml new file mode 100644 index 00000000..fd4e6411 --- /dev/null +++ b/test/fixtures/module-schema/invalid/variables/missing-prompt.module.yaml @@ -0,0 +1,16 @@ +# Test: Variable without prompt or inherit +# Expected: FAIL +# Error code: custom +# Error path: my_variable +# Error message: my_variable must have a 'prompt' or 'inherit' field + +code: test-module +name: Test Module +header: Test Header +subheader: Test Subheader +default_selected: false + +my_variable: + required: true + result: some result + diff --git a/test/fixtures/module-schema/valid/minimal/minimal-module.module.yaml b/test/fixtures/module-schema/valid/minimal/minimal-module.module.yaml new file mode 100644 index 00000000..0115c7ae --- /dev/null +++ b/test/fixtures/module-schema/valid/minimal/minimal-module.module.yaml @@ -0,0 +1,9 @@ +# Test: Valid module with only required fields +# Expected: PASS + +code: my-module +name: My Module Name +header: Module Header +subheader: Short description of what this module does +default_selected: false + diff --git a/test/fixtures/module-schema/valid/minimal/no-default-selected.module.yaml b/test/fixtures/module-schema/valid/minimal/no-default-selected.module.yaml new file mode 100644 index 00000000..eaddbe03 --- /dev/null +++ b/test/fixtures/module-schema/valid/minimal/no-default-selected.module.yaml @@ -0,0 +1,8 @@ +# Test: Valid module without default_selected (optional for core module) +# Expected: PASS + +code: core-like +name: Core Module +header: Core Header +subheader: Core modules don't need default_selected + diff --git a/test/fixtures/module-schema/valid/minimal/short-code.module.yaml b/test/fixtures/module-schema/valid/minimal/short-code.module.yaml new file mode 100644 index 00000000..bbc321f9 --- /dev/null +++ b/test/fixtures/module-schema/valid/minimal/short-code.module.yaml @@ -0,0 +1,9 @@ +# Test: Valid module with minimum 2-character code +# Expected: PASS + +code: ab +name: Two Letter Module +header: Short Code Header +subheader: The shortest valid code is 2 characters +default_selected: true + diff --git a/test/fixtures/module-schema/valid/variables/with-inherit.module.yaml b/test/fixtures/module-schema/valid/variables/with-inherit.module.yaml new file mode 100644 index 00000000..0fc91e7b --- /dev/null +++ b/test/fixtures/module-schema/valid/variables/with-inherit.module.yaml @@ -0,0 +1,12 @@ +# Test: Valid module with inherit variable +# Expected: PASS + +code: inherit-mod +name: Inherit Module +header: Module With Inherited Variables +subheader: Uses inherit instead of prompt +default_selected: false + +inherited_var: + inherit: other-module.some_var + diff --git a/test/fixtures/module-schema/valid/variables/with-prompt.module.yaml b/test/fixtures/module-schema/valid/variables/with-prompt.module.yaml new file mode 100644 index 00000000..ba4ffc78 --- /dev/null +++ b/test/fixtures/module-schema/valid/variables/with-prompt.module.yaml @@ -0,0 +1,14 @@ +# Test: Valid module with variable definition (prompt style) +# Expected: PASS + +code: var-module +name: Variable Module +header: Module With Variables +subheader: Demonstrates variable definitions +default_selected: false + +project_name: + prompt: What is your project name? + required: true + result: The project name is {project_name} + diff --git a/test/fixtures/module-schema/valid/variables/with-select.module.yaml b/test/fixtures/module-schema/valid/variables/with-select.module.yaml new file mode 100644 index 00000000..81b5ff6e --- /dev/null +++ b/test/fixtures/module-schema/valid/variables/with-select.module.yaml @@ -0,0 +1,31 @@ +# Test: Valid module with single-select and multi-select +# Expected: PASS + +code: select-mod +name: Select Module +header: Module With Selects +subheader: Demonstrates single-select and multi-select +default_selected: false + +environment: + prompt: Select your target environment + single-select: + - value: dev + label: Development + - value: staging + label: Staging + - value: prod + label: Production + +features: + prompt: + - What features do you want? + - You can select multiple options. + multi-select: + - value: auth + label: Authentication + - value: api + label: REST API + - value: db + label: Database + diff --git a/test/test-module-schema.js b/test/test-module-schema.js new file mode 100644 index 00000000..17402968 --- /dev/null +++ b/test/test-module-schema.js @@ -0,0 +1,339 @@ +/** + * Module Schema Validation Test Runner + * + * Runs all test fixtures and verifies expected outcomes. + * Reports pass/fail for each test and overall coverage statistics. + * + * Usage: node test/test-module-schema.js + * Exit codes: 0 = all tests pass, 1 = test failures + */ + +const fs = require('node:fs'); +const path = require('node:path'); +const yaml = require('yaml'); +const { validateModuleFile } = require('../tools/schema/module.js'); +const { glob } = require('glob'); + +// ANSI color codes +const colors = { + reset: '\u001B[0m', + green: '\u001B[32m', + red: '\u001B[31m', + yellow: '\u001B[33m', + blue: '\u001B[34m', + cyan: '\u001B[36m', + dim: '\u001B[2m', +}; + +/** + * Parse test metadata from YAML comments + * @param {string} filePath + * @returns {{shouldPass: boolean, errorExpectation?: object}} + */ +function parseTestMetadata(filePath) { + const content = fs.readFileSync(filePath, 'utf8'); + const lines = content.split('\n'); + + let shouldPass = true; + const errorExpectation = {}; + + for (const line of lines) { + if (line.includes('Expected: PASS')) { + shouldPass = true; + } else if (line.includes('Expected: FAIL')) { + shouldPass = false; + } + + // Parse error metadata + const codeMatch = line.match(/^# Error code: (.+)$/); + if (codeMatch) { + errorExpectation.code = codeMatch[1].trim(); + } + + const pathMatch = line.match(/^# Error path: (.+)$/); + if (pathMatch) { + errorExpectation.path = pathMatch[1].trim(); + } + + const messageMatch = line.match(/^# Error message: (.+)$/); + if (messageMatch) { + errorExpectation.message = messageMatch[1].trim(); + } + + const minimumMatch = line.match(/^# Error minimum: (\d+)$/); + if (minimumMatch) { + errorExpectation.minimum = parseInt(minimumMatch[1], 10); + } + + const expectedMatch = line.match(/^# Error expected: (.+)$/); + if (expectedMatch) { + errorExpectation.expected = expectedMatch[1].trim(); + } + + const receivedMatch = line.match(/^# Error received: (.+)$/); + if (receivedMatch) { + errorExpectation.received = receivedMatch[1].trim(); + } + + const keysMatch = line.match(/^# Error keys: \[(.+)\]$/); + if (keysMatch) { + errorExpectation.keys = keysMatch[1].split(',').map((k) => k.trim().replaceAll(/['"]/g, '')); + } + } + + return { + shouldPass, + errorExpectation: Object.keys(errorExpectation).length > 0 ? errorExpectation : null, + }; +} + +/** + * Convert dot-notation path string to array (handles array indices) + * e.g., "module.dependencies[0]" => ["module", "dependencies", 0] + */ +function parsePathString(pathString) { + return pathString + .replaceAll(/\[(\d+)\]/g, '.$1') + .split('.') + .map((part) => { + const num = parseInt(part, 10); + return isNaN(num) ? part : num; + }); +} + +/** + * Validate error against expectations + * @param {object} error - Zod error issue + * @param {object} expectation - Expected error structure + * @returns {{valid: boolean, reason?: string}} + */ +function validateError(error, expectation) { + if (expectation.code && error.code !== expectation.code) { + return { valid: false, reason: `Expected code "${expectation.code}", got "${error.code}"` }; + } + + if (expectation.path) { + const expectedPath = parsePathString(expectation.path); + const actualPath = error.path; + + if (JSON.stringify(expectedPath) !== JSON.stringify(actualPath)) { + return { + valid: false, + reason: `Expected path ${JSON.stringify(expectedPath)}, got ${JSON.stringify(actualPath)}`, + }; + } + } + + if (expectation.code === 'custom' && expectation.message && error.message !== expectation.message) { + return { + valid: false, + reason: `Expected message "${expectation.message}", got "${error.message}"`, + }; + } + + if (expectation.minimum !== undefined && error.minimum !== expectation.minimum) { + return { valid: false, reason: `Expected minimum ${expectation.minimum}, got ${error.minimum}` }; + } + + if (expectation.expected && error.expected !== expectation.expected) { + return { valid: false, reason: `Expected type "${expectation.expected}", got "${error.expected}"` }; + } + + if (expectation.received && error.received !== expectation.received) { + return { valid: false, reason: `Expected received "${expectation.received}", got "${error.received}"` }; + } + + if (expectation.keys) { + const expectedKeys = expectation.keys.sort(); + const actualKeys = (error.keys || []).sort(); + if (JSON.stringify(expectedKeys) !== JSON.stringify(actualKeys)) { + return { + valid: false, + reason: `Expected keys ${JSON.stringify(expectedKeys)}, got ${JSON.stringify(actualKeys)}`, + }; + } + } + + return { valid: true }; +} + +/** + * Run a single test case + * @param {string} filePath + * @returns {{passed: boolean, message: string}} + */ +function runTest(filePath) { + const metadata = parseTestMetadata(filePath); + const { shouldPass, errorExpectation } = metadata; + + try { + const fileContent = fs.readFileSync(filePath, 'utf8'); + let moduleData; + + try { + moduleData = yaml.parse(fileContent); + } catch (parseError) { + if (shouldPass) { + return { + passed: false, + message: `Expected PASS but got YAML parse error: ${parseError.message}`, + }; + } + return { + passed: true, + message: 'Got expected YAML parse error', + }; + } + + const result = validateModuleFile(filePath, moduleData); + + if (result.success && shouldPass) { + return { + passed: true, + message: 'Validation passed as expected', + }; + } + + if (!result.success && !shouldPass) { + const actualError = result.error.issues[0]; + + if (errorExpectation) { + const validation = validateError(actualError, errorExpectation); + + if (!validation.valid) { + return { + passed: false, + message: `Error validation failed: ${validation.reason}`, + }; + } + + return { + passed: true, + message: `Got expected error (${errorExpectation.code}): ${actualError.message}`, + }; + } + + return { + passed: true, + message: `Got expected validation error: ${actualError?.message}`, + }; + } + + if (result.success && !shouldPass) { + return { + passed: false, + message: 'Expected validation to FAIL but it PASSED', + }; + } + + if (!result.success && shouldPass) { + return { + passed: false, + message: `Expected validation to PASS but it FAILED: ${result.error.issues[0]?.message}`, + }; + } + + return { + passed: false, + message: 'Unexpected test state', + }; + } catch (error) { + return { + passed: false, + message: `Test execution error: ${error.message}`, + }; + } +} + +/** + * Main test runner + */ +async function main() { + console.log(`${colors.cyan}╔═══════════════════════════════════════════════════════════╗${colors.reset}`); + console.log(`${colors.cyan}║ Module Schema Validation Test Suite ║${colors.reset}`); + console.log(`${colors.cyan}╚═══════════════════════════════════════════════════════════╝${colors.reset}\n`); + + const testFiles = await glob('test/fixtures/module-schema/**/*.module.yaml', { + cwd: path.join(__dirname, '..'), + absolute: true, + }); + + if (testFiles.length === 0) { + console.log(`${colors.yellow}⚠️ No test fixtures found${colors.reset}`); + process.exit(0); + } + + console.log(`Found ${colors.cyan}${testFiles.length}${colors.reset} test fixture(s)\n`); + + // Group tests by category + const categories = {}; + for (const testFile of testFiles) { + const relativePath = path.relative(path.join(__dirname, 'fixtures/module-schema'), testFile); + const parts = relativePath.split(path.sep); + const validInvalid = parts[0]; // 'valid' or 'invalid' + const category = parts[1] || 'general'; + + const categoryKey = `${validInvalid}/${category}`; + if (!categories[categoryKey]) { + categories[categoryKey] = []; + } + categories[categoryKey].push(testFile); + } + + // Run tests by category + let totalTests = 0; + let passedTests = 0; + const failures = []; + + for (const [categoryKey, files] of Object.entries(categories).sort()) { + const [validInvalid, category] = categoryKey.split('/'); + const categoryLabel = category.replaceAll('-', ' ').toUpperCase(); + const validLabel = validInvalid === 'valid' ? '✅' : '❌'; + + console.log(`${colors.blue}${validLabel} ${categoryLabel} (${validInvalid})${colors.reset}`); + + for (const testFile of files) { + totalTests++; + const testName = path.basename(testFile, '.module.yaml'); + const result = runTest(testFile); + + if (result.passed) { + passedTests++; + console.log(` ${colors.green}✓${colors.reset} ${testName} ${colors.dim}${result.message}${colors.reset}`); + } else { + console.log(` ${colors.red}✗${colors.reset} ${testName} ${colors.red}${result.message}${colors.reset}`); + failures.push({ + file: path.relative(process.cwd(), testFile), + message: result.message, + }); + } + } + console.log(''); + } + + // Summary + console.log(`${colors.cyan}═══════════════════════════════════════════════════════════${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}═══════════════════════════════════════════════════════════${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.file}`); + console.log(` ${failure.message}\n`); + } + process.exit(1); + } + + console.log(`${colors.green}✨ All tests passed!${colors.reset}\n`); + process.exit(0); +} + +// Run +main().catch((error) => { + console.error(`${colors.red}Fatal error:${colors.reset}`, error); + process.exit(1); +}); diff --git a/tools/schema/module.js b/tools/schema/module.js new file mode 100644 index 00000000..5810c417 --- /dev/null +++ b/tools/schema/module.js @@ -0,0 +1,188 @@ +// Zod schema definition for module.yaml files +const { z } = require('zod'); + +// Pattern for module code: kebab-case, 2-20 characters, starts with letter +const MODULE_CODE_PATTERN = /^[a-z][a-z0-9-]{1,19}$/; + +// Public API --------------------------------------------------------------- + +/** + * Validate a module YAML payload against the schema. + * + * @param {string} filePath Path to the module file (for consistency with other validators). + * @param {unknown} moduleYaml Parsed YAML content. + * @returns {import('zod').SafeParseReturnType} SafeParse result. + */ +function validateModuleFile(filePath, moduleYaml) { + const schema = moduleSchema(); + return schema.safeParse(moduleYaml); +} + +module.exports = { validateModuleFile }; + +// Internal helpers --------------------------------------------------------- + +/** + * Build the Zod schema for validating a module.yaml file. + * @returns {import('zod').ZodSchema} Configured Zod schema instance. + */ +function moduleSchema() { + return z + .object({ + // Required fields + code: z.string().regex(MODULE_CODE_PATTERN, { + message: 'module.code must be kebab-case, 2-20 characters, starting with a letter', + }), + name: createNonEmptyString('module.name'), + header: createNonEmptyString('module.header'), + subheader: createNonEmptyString('module.subheader'), + // default_selected is optional for core module, required for others + // Core module doesn't need this as it's always included + default_selected: z.boolean().optional(), + + // Optional fields + type: createNonEmptyString('module.type').optional(), + global: z.boolean().optional(), + }) + .strict() + .passthrough() + .superRefine((value, ctx) => { + // Validate any additional keys as variable definitions + const reservedKeys = new Set(['code', 'name', 'header', 'subheader', 'default_selected', 'type', 'global']); + + for (const key of Object.keys(value)) { + if (reservedKeys.has(key)) { + continue; + } + + const variableValue = value[key]; + + // Skip if it's a comment (starts with #) or null/undefined + if (variableValue === null || variableValue === undefined) { + continue; + } + + // Validate variable definition + const variableResult = validateVariableDefinition(key, variableValue); + if (!variableResult.valid) { + ctx.addIssue({ + code: 'custom', + path: [key], + message: variableResult.error, + }); + } + } + }); +} + +/** + * Validate a variable definition object. + * @param {string} variableName The name of the variable. + * @param {unknown} variableValue The variable definition value. + * @returns {{ valid: boolean, error?: string }} + */ +function validateVariableDefinition(variableName, variableValue) { + // If it's not an object, it's invalid + if (typeof variableValue !== 'object' || variableValue === null) { + return { valid: false, error: `${variableName} must be an object with variable definition properties` }; + } + + // Check for inherit alias - if present, it's the only required field + if ('inherit' in variableValue) { + if (typeof variableValue.inherit !== 'string' || variableValue.inherit.trim().length === 0) { + return { valid: false, error: `${variableName}.inherit must be a non-empty string` }; + } + return { valid: true }; + } + + // Otherwise, prompt is required + if (!('prompt' in variableValue)) { + return { valid: false, error: `${variableName} must have a 'prompt' or 'inherit' field` }; + } + + // Validate prompt: string or array of strings + const prompt = variableValue.prompt; + if (typeof prompt === 'string') { + if (prompt.trim().length === 0) { + return { valid: false, error: `${variableName}.prompt must be a non-empty string` }; + } + } else if (Array.isArray(prompt)) { + if (prompt.length === 0) { + return { valid: false, error: `${variableName}.prompt array must not be empty` }; + } + for (const [index, promptItem] of prompt.entries()) { + if (typeof promptItem !== 'string' || promptItem.trim().length === 0) { + return { valid: false, error: `${variableName}.prompt[${index}] must be a non-empty string` }; + } + } + } else { + return { valid: false, error: `${variableName}.prompt must be a string or array of strings` }; + } + + // Validate optional single-select + if ('single-select' in variableValue) { + const selectResult = validateSelectOptions(variableName, 'single-select', variableValue['single-select']); + if (!selectResult.valid) { + return selectResult; + } + } + + // Validate optional multi-select + if ('multi-select' in variableValue) { + const selectResult = validateSelectOptions(variableName, 'multi-select', variableValue['multi-select']); + if (!selectResult.valid) { + return selectResult; + } + } + + // Validate optional required field + if ('required' in variableValue && typeof variableValue.required !== 'boolean') { + return { valid: false, error: `${variableName}.required must be a boolean` }; + } + + // Validate optional result field + if ('result' in variableValue && (typeof variableValue.result !== 'string' || variableValue.result.trim().length === 0)) { + return { valid: false, error: `${variableName}.result must be a non-empty string` }; + } + + return { valid: true }; +} + +/** + * Validate single-select or multi-select options array. + * @param {string} variableName The variable name for error messages. + * @param {string} selectType Either 'single-select' or 'multi-select'. + * @param {unknown} options The options array to validate. + * @returns {{ valid: boolean, error?: string }} + */ +function validateSelectOptions(variableName, selectType, options) { + if (!Array.isArray(options)) { + return { valid: false, error: `${variableName}.${selectType} must be an array` }; + } + + if (options.length === 0) { + return { valid: false, error: `${variableName}.${selectType} must not be empty` }; + } + + for (const [index, option] of options.entries()) { + if (typeof option !== 'object' || option === null) { + return { valid: false, error: `${variableName}.${selectType}[${index}] must be an object` }; + } + if (!('value' in option) || typeof option.value !== 'string') { + return { valid: false, error: `${variableName}.${selectType}[${index}].value must be a string` }; + } + if (!('label' in option) || typeof option.label !== 'string') { + return { valid: false, error: `${variableName}.${selectType}[${index}].label must be a string` }; + } + } + + return { valid: true }; +} + +// Primitive validators ----------------------------------------------------- + +function createNonEmptyString(label) { + return z.string().refine((value) => value.trim().length > 0, { + message: `${label} must be a non-empty string`, + }); +} diff --git a/tools/validate-module-schema.js b/tools/validate-module-schema.js new file mode 100644 index 00000000..17dd7b40 --- /dev/null +++ b/tools/validate-module-schema.js @@ -0,0 +1,110 @@ +/** + * Module Schema Validator CLI + * + * Scans all module.yaml files in src/core/ and src/modules/ + * and validates them against the Zod schema. + * + * Usage: node tools/validate-module-schema.js [project_root] + * Exit codes: 0 = success, 1 = validation failures + * + * Optional argument: + * project_root - Directory to scan (defaults to BMAD repo root) + */ + +const { glob } = require('glob'); +const yaml = require('yaml'); +const fs = require('node:fs'); +const path = require('node:path'); +const { validateModuleFile } = require('./schema/module.js'); + +/** + * Main validation routine + * @param {string} [customProjectRoot] - Optional project root to scan (for testing) + */ +async function main(customProjectRoot) { + console.log('🔍 Scanning for module files...\n'); + + // Determine project root: use custom path if provided, otherwise default to repo root + const project_root = customProjectRoot || path.join(__dirname, '..'); + + // Find all module files: core/module.yaml and bmm/module.yaml (and any other top-level modules) + const moduleFiles = await glob('src/{core,bmm}/module.yaml', { + cwd: project_root, + absolute: true, + }); + + if (moduleFiles.length === 0) { + console.log('❌ No module files found. This likely indicates a configuration error.'); + console.log(' Expected to find module.yaml files in src/core/ and src/modules/*/'); + process.exit(1); + } + + console.log(`Found ${moduleFiles.length} module file(s)\n`); + + const errors = []; + + // Validate each file + for (const filePath of moduleFiles) { + const relativePath = path.relative(process.cwd(), filePath); + + try { + const fileContent = fs.readFileSync(filePath, 'utf8'); + const moduleData = yaml.parse(fileContent); + + // Convert absolute path to relative src/ path for context + const srcRelativePath = relativePath.startsWith('src/') ? relativePath : path.relative(project_root, filePath).replaceAll('\\', '/'); + + const result = validateModuleFile(srcRelativePath, moduleData); + + if (result.success) { + console.log(`✅ ${relativePath}`); + } else { + errors.push({ + file: relativePath, + issues: result.error.issues, + }); + } + } catch (error) { + errors.push({ + file: relativePath, + issues: [ + { + code: 'parse_error', + message: `Failed to parse YAML: ${error.message}`, + path: [], + }, + ], + }); + } + } + + // Report errors + if (errors.length > 0) { + console.log('\n❌ Validation failed for the following files:\n'); + + for (const { file, issues } of errors) { + console.log(`\n📄 ${file}`); + for (const issue of issues) { + const pathString = issue.path.length > 0 ? issue.path.join('.') : '(root)'; + console.log(` Path: ${pathString}`); + console.log(` Error: ${issue.message}`); + if (issue.code) { + console.log(` Code: ${issue.code}`); + } + } + } + + console.log(`\n\n💥 ${errors.length} file(s) failed validation`); + process.exit(1); + } + + console.log(`\n✨ All ${moduleFiles.length} module file(s) passed validation!\n`); + process.exit(0); +} + +// Run with optional command-line argument for project root +const customProjectRoot = process.argv[2]; +main(customProjectRoot).catch((error) => { + console.error('Fatal error:', error); + process.exit(1); +}); From 6bf6a2b6d2b5b9d6d1c7922997bf2b484b3b6f04 Mon Sep 17 00:00:00 2001 From: cx-noam-brendel <139764378+cx-noam-brendel@users.noreply.github.com> Date: Thu, 15 Jan 2026 07:23:52 +0200 Subject: [PATCH 2/4] fix: address PR review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes per reviewer feedback: - Remove ⚠️ CRITICAL warnings from documentation (per @alexeyv) - Replace placeholder YAML examples with concrete values - Simplify validation checklist text Technical fixes per CodeRabbit: - Remove .strict() that contradicted .passthrough() - Add isCoreModule parameter for conditional default_selected validation - Enforce mutual exclusivity: inherit/prompt and single-select/multi-select - Use project_root for consistent path reporting - Harden test runner with guarded access for error.issues - Move parseTestMetadata inside try/catch block All tests pass (52 agent + 17 module + 13 installation) --- .../minimal/default-selected-true.module.yaml | 9 ++++ .../minimal/no-default-selected.module.yaml | 8 ---- test/test-module-schema.js | 18 ++++--- tools/schema/module.js | 47 ++++++++++++++----- tools/validate-module-schema.js | 6 +-- 5 files changed, 60 insertions(+), 28 deletions(-) create mode 100644 test/fixtures/module-schema/valid/minimal/default-selected-true.module.yaml delete mode 100644 test/fixtures/module-schema/valid/minimal/no-default-selected.module.yaml diff --git a/test/fixtures/module-schema/valid/minimal/default-selected-true.module.yaml b/test/fixtures/module-schema/valid/minimal/default-selected-true.module.yaml new file mode 100644 index 00000000..44ca1be5 --- /dev/null +++ b/test/fixtures/module-schema/valid/minimal/default-selected-true.module.yaml @@ -0,0 +1,9 @@ +# Test: Valid module with default_selected set to true +# Expected: PASS + +code: core-like +name: Core Module +header: Core Header +subheader: Module with default_selected true +default_selected: true + diff --git a/test/fixtures/module-schema/valid/minimal/no-default-selected.module.yaml b/test/fixtures/module-schema/valid/minimal/no-default-selected.module.yaml deleted file mode 100644 index eaddbe03..00000000 --- a/test/fixtures/module-schema/valid/minimal/no-default-selected.module.yaml +++ /dev/null @@ -1,8 +0,0 @@ -# Test: Valid module without default_selected (optional for core module) -# Expected: PASS - -code: core-like -name: Core Module -header: Core Header -subheader: Core modules don't need default_selected - diff --git a/test/test-module-schema.js b/test/test-module-schema.js index 17402968..34713b62 100644 --- a/test/test-module-schema.js +++ b/test/test-module-schema.js @@ -163,10 +163,10 @@ function validateError(error, expectation) { * @returns {{passed: boolean, message: string}} */ function runTest(filePath) { - const metadata = parseTestMetadata(filePath); - const { shouldPass, errorExpectation } = metadata; - try { + const metadata = parseTestMetadata(filePath); + const { shouldPass, errorExpectation } = metadata; + const fileContent = fs.readFileSync(filePath, 'utf8'); let moduleData; @@ -195,7 +195,13 @@ function runTest(filePath) { } if (!result.success && !shouldPass) { - const actualError = result.error.issues[0]; + const actualError = result.error?.issues?.[0]; + if (!actualError) { + return { + passed: false, + message: 'Expected validation error issues, but validator returned none', + }; + } if (errorExpectation) { const validation = validateError(actualError, errorExpectation); @@ -215,7 +221,7 @@ function runTest(filePath) { return { passed: true, - message: `Got expected validation error: ${actualError?.message}`, + message: `Got expected validation error: ${actualError.message}`, }; } @@ -229,7 +235,7 @@ function runTest(filePath) { if (!result.success && shouldPass) { return { passed: false, - message: `Expected validation to PASS but it FAILED: ${result.error.issues[0]?.message}`, + message: `Expected validation to PASS but it FAILED: ${result.error?.issues?.[0]?.message ?? 'Unknown error'}`, }; } diff --git a/tools/schema/module.js b/tools/schema/module.js index 5810c417..7aa98ffc 100644 --- a/tools/schema/module.js +++ b/tools/schema/module.js @@ -9,12 +9,13 @@ const MODULE_CODE_PATTERN = /^[a-z][a-z0-9-]{1,19}$/; /** * Validate a module YAML payload against the schema. * - * @param {string} filePath Path to the module file (for consistency with other validators). + * @param {string} filePath Path to the module file (used to detect core vs non-core modules). * @param {unknown} moduleYaml Parsed YAML content. * @returns {import('zod').SafeParseReturnType} SafeParse result. */ function validateModuleFile(filePath, moduleYaml) { - const schema = moduleSchema(); + const isCoreModule = typeof filePath === 'string' && filePath.replaceAll('\\', '/').includes('src/core/'); + const schema = moduleSchema({ isCoreModule }); return schema.safeParse(moduleYaml); } @@ -24,9 +25,11 @@ module.exports = { validateModuleFile }; /** * Build the Zod schema for validating a module.yaml file. + * @param {{isCoreModule?: boolean}} options - Options for schema validation. * @returns {import('zod').ZodSchema} Configured Zod schema instance. */ -function moduleSchema() { +function moduleSchema(options) { + const { isCoreModule = false } = options ?? {}; return z .object({ // Required fields @@ -36,17 +39,24 @@ function moduleSchema() { name: createNonEmptyString('module.name'), header: createNonEmptyString('module.header'), subheader: createNonEmptyString('module.subheader'), - // default_selected is optional for core module, required for others - // Core module doesn't need this as it's always included + // default_selected is optional for core module, required for non-core modules default_selected: z.boolean().optional(), // Optional fields type: createNonEmptyString('module.type').optional(), global: z.boolean().optional(), }) - .strict() .passthrough() .superRefine((value, ctx) => { + // Enforce default_selected for non-core modules + if (!isCoreModule && !('default_selected' in value)) { + ctx.addIssue({ + code: 'custom', + path: ['default_selected'], + message: 'module.default_selected is required for non-core modules', + }); + } + // Validate any additional keys as variable definitions const reservedKeys = new Set(['code', 'name', 'header', 'subheader', 'default_selected', 'type', 'global']); @@ -57,7 +67,7 @@ function moduleSchema() { const variableValue = value[key]; - // Skip if it's a comment (starts with #) or null/undefined + // Skip if null/undefined if (variableValue === null || variableValue === undefined) { continue; } @@ -87,8 +97,16 @@ function validateVariableDefinition(variableName, variableValue) { return { valid: false, error: `${variableName} must be an object with variable definition properties` }; } + const hasInherit = 'inherit' in variableValue; + const hasPrompt = 'prompt' in variableValue; + + // Enforce mutual exclusivity: inherit and prompt cannot coexist + if (hasInherit && hasPrompt) { + return { valid: false, error: `${variableName} must not define both 'inherit' and 'prompt'` }; + } + // Check for inherit alias - if present, it's the only required field - if ('inherit' in variableValue) { + if (hasInherit) { if (typeof variableValue.inherit !== 'string' || variableValue.inherit.trim().length === 0) { return { valid: false, error: `${variableName}.inherit must be a non-empty string` }; } @@ -96,7 +114,7 @@ function validateVariableDefinition(variableName, variableValue) { } // Otherwise, prompt is required - if (!('prompt' in variableValue)) { + if (!hasPrompt) { return { valid: false, error: `${variableName} must have a 'prompt' or 'inherit' field` }; } @@ -119,8 +137,15 @@ function validateVariableDefinition(variableName, variableValue) { return { valid: false, error: `${variableName}.prompt must be a string or array of strings` }; } + // Enforce mutual exclusivity: single-select and multi-select cannot coexist + const hasSingle = 'single-select' in variableValue; + const hasMulti = 'multi-select' in variableValue; + if (hasSingle && hasMulti) { + return { valid: false, error: `${variableName} must not define both 'single-select' and 'multi-select'` }; + } + // Validate optional single-select - if ('single-select' in variableValue) { + if (hasSingle) { const selectResult = validateSelectOptions(variableName, 'single-select', variableValue['single-select']); if (!selectResult.valid) { return selectResult; @@ -128,7 +153,7 @@ function validateVariableDefinition(variableName, variableValue) { } // Validate optional multi-select - if ('multi-select' in variableValue) { + if (hasMulti) { const selectResult = validateSelectOptions(variableName, 'multi-select', variableValue['multi-select']); if (!selectResult.valid) { return selectResult; diff --git a/tools/validate-module-schema.js b/tools/validate-module-schema.js index 17dd7b40..7e65fd9c 100644 --- a/tools/validate-module-schema.js +++ b/tools/validate-module-schema.js @@ -45,14 +45,14 @@ async function main(customProjectRoot) { // Validate each file for (const filePath of moduleFiles) { - const relativePath = path.relative(process.cwd(), filePath); + const relativePath = path.relative(project_root, filePath).replaceAll('\\', '/'); try { const fileContent = fs.readFileSync(filePath, 'utf8'); const moduleData = yaml.parse(fileContent); - // Convert absolute path to relative src/ path for context - const srcRelativePath = relativePath.startsWith('src/') ? relativePath : path.relative(project_root, filePath).replaceAll('\\', '/'); + // Ensure path starts with src/ for core module detection + const srcRelativePath = relativePath.startsWith('src/') ? relativePath : `src/${relativePath}`; const result = validateModuleFile(srcRelativePath, moduleData); From b46e20dc3a2d2f8926c8540ae1f22903adceb50d Mon Sep 17 00:00:00 2001 From: cx-noam-brendel <139764378+cx-noam-brendel@users.noreply.github.com> Date: Thu, 15 Jan 2026 08:49:06 +0200 Subject: [PATCH 3/4] fix: exit with code 1 when no test fixtures found Per CodeRabbit review - quality gate scripts should fail on unexpected states --- test/test-module-schema.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test-module-schema.js b/test/test-module-schema.js index 34713b62..9edf619e 100644 --- a/test/test-module-schema.js +++ b/test/test-module-schema.js @@ -266,7 +266,7 @@ async function main() { if (testFiles.length === 0) { console.log(`${colors.yellow}⚠️ No test fixtures found${colors.reset}`); - process.exit(0); + process.exit(1); } console.log(`Found ${colors.cyan}${testFiles.length}${colors.reset} test fixture(s)\n`); From 0c660aa0e9dd613ab3696dad7b4c938e3ad0ddbc Mon Sep 17 00:00:00 2001 From: Noam Brendel <139764378+cx-noam-brendel@users.noreply.github.com> Date: Mon, 26 Jan 2026 15:36:24 +0200 Subject: [PATCH 4/4] fix: add missing header and subheader fields to module.yaml files The new module schema validation requires header and subheader fields. Updated bmm and bmb modules to comply with the schema by: - Renaming 'description' to 'header' - Adding 'subheader' field with configuration context --- src/bmm/module.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/bmm/module.yaml b/src/bmm/module.yaml index a9884e58..b818c7d1 100644 --- a/src/bmm/module.yaml +++ b/src/bmm/module.yaml @@ -1,6 +1,7 @@ code: bmm name: "BMad Method Agile-AI Driven-Development" -description: "AI-driven agile development framework" +header: "AI-driven agile development framework" +subheader: "Configure the BMad Method module for AI-powered agile project development" default_selected: true # This module will be selected by default for new installations # Variables from Core Config inserted: