diff --git a/src/modules/bmb/workflows/module/steps-c/step-03-config.md b/src/modules/bmb/workflows/module/steps-c/step-03-config.md index 5f219503..2eaa59b2 100644 --- a/src/modules/bmb/workflows/module/steps-c/step-03-config.md +++ b/src/modules/bmb/workflows/module/steps-c/step-03-config.md @@ -42,19 +42,15 @@ Load `{moduleYamlConventionsFile}` for reference. Create `{targetLocation}/module.yaml` with: -**⚠️ CRITICAL: All required fields MUST be populated with actual values, not placeholders:** - -**Required fields:** +**Required fields (replace with actual values):** ```yaml -code: {module_code} # ⚠️ REQUIRED: Must be the actual kebab-case module code (e.g., "my-module") -name: "{module_display_name}" # ⚠️ REQUIRED: Human-readable name (e.g., "My Module: Description") -header: "{brief_header}" # ⚠️ REQUIRED: One-line summary -subheader: "{additional_context}" # ⚠️ REQUIRED: Additional context -default_selected: false # ⚠️ REQUIRED: Boolean, typically false for new modules +code: "my-module" # kebab-case, 2-20 chars, starts with letter +name: "My Module: Description" # human-readable name +header: "One-line summary" # one-line summary +subheader: "Additional context" # additional context +default_selected: false # typically false for new modules ``` -**Validation:** The module will fail installation if `code` is missing or contains placeholder text like `{module_code}`. - **Note for Extension modules:** `code:` matches base module ### 3. Add Custom Variables diff --git a/src/modules/bmb/workflows/module/steps-v/step-03-module-yaml.md b/src/modules/bmb/workflows/module/steps-v/step-03-module-yaml.md index 8cd4d56c..367edc84 100644 --- a/src/modules/bmb/workflows/module/steps-v/step-03-module-yaml.md +++ b/src/modules/bmb/workflows/module/steps-v/step-03-module-yaml.md @@ -38,13 +38,12 @@ Read `{targetPath}/module.yaml` ### 2. Validate Required Fields -**⚠️ CRITICAL:** Check for required frontmatter (these MUST be actual values, not placeholders): -- [ ] `code:` present and valid (kebab-case, 2-20 chars, starts with letter, e.g., `my-module`) - - ❌ FAIL if missing, empty, or contains placeholder text like `{module_code}` +Check required fields (must have actual values, not placeholders): +- [ ] `code:` present (kebab-case, 2-20 chars, starts with letter) - [ ] `name:` present (non-empty string) - [ ] `header:` present (non-empty string) - [ ] `subheader:` present (non-empty string) -- [ ] `default_selected:` present (boolean - typically `false` for new modules) +- [ ] `default_selected:` present (boolean) ### 3. Validate Custom Variables 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 711519ec..547525b7 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);