From 14d145f35baa4da3a86947ea0f6a7e6025b10600 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henry=20=C3=81vila?= Date: Tue, 17 Mar 2026 14:40:02 -0300 Subject: [PATCH] fix(cli): support parent directories in --custom-content path When --custom-content points to a parent directory (no module.yaml at root), scan immediate subdirectories (1 level) for module.yaml files. Previously the path was silently rejected and the installer fell back to cached modules, reporting "updated" without actually copying new files. Adds resolveCustomContentPaths() method and applies it to all three custom content processing flows (quick-update, update, fresh install). Closes #2040 Co-Authored-By: Claude Opus 4.6 (1M context) --- package.json | 1 + test/test-custom-content-parent-dir.js | 210 ++++++++++++++++++++++++ tools/cli/lib/ui.js | 214 +++++++++++++++++-------- 3 files changed, 356 insertions(+), 69 deletions(-) create mode 100644 test/test-custom-content-parent-dir.js diff --git a/package.json b/package.json index 4340a2fe0..a1d3c5caf 100644 --- a/package.json +++ b/package.json @@ -43,6 +43,7 @@ "rebundle": "node tools/cli/bundlers/bundle-web.js rebundle", "test": "npm run test:schemas && npm run test:refs && npm run test:install && npm run validate:schemas && npm run lint && npm run lint:md && npm run format:check", "test:coverage": "c8 --reporter=text --reporter=html npm run test:schemas", + "test:custom-content": "node test/test-custom-content-parent-dir.js", "test:install": "node test/test-installation-components.js", "test:refs": "node test/test-file-refs-csv.js", "test:schemas": "node test/test-agent-schema.js", diff --git a/test/test-custom-content-parent-dir.js b/test/test-custom-content-parent-dir.js new file mode 100644 index 000000000..03d4bf397 --- /dev/null +++ b/test/test-custom-content-parent-dir.js @@ -0,0 +1,210 @@ +/** + * Custom Content Parent Directory Tests + * + * Tests that --custom-content works with parent directories containing + * multiple module subdirectories (1-level recursive scan). + * + * Related: https://github.com/bmad-code-org/BMAD-METHOD/issues/2040 + * + * Usage: node test/test-custom-content-parent-dir.js + */ + +const path = require('node:path'); +const os = require('node:os'); +const fs = require('fs-extra'); +const { UI } = require('../tools/cli/lib/ui'); + +// ANSI colors +const colors = { + reset: '\u001B[0m', + green: '\u001B[32m', + red: '\u001B[31m', + yellow: '\u001B[33m', + cyan: '\u001B[36m', + dim: '\u001B[2m', +}; + +let passed = 0; +let failed = 0; + +function assert(condition, testName, errorMessage = '') { + if (condition) { + console.log(`${colors.green}✓${colors.reset} ${testName}`); + passed++; + } else { + console.log(`${colors.red}✗${colors.reset} ${testName}`); + if (errorMessage) { + console.log(` ${colors.dim}${errorMessage}${colors.reset}`); + } + failed++; + } +} + +/** + * Create a temp directory simulating a parent dir with multiple custom modules + * + * Structure: + * parentDir/ + * module-a/ + * module.yaml (code: "module-a") + * module-b/ + * module.yaml (code: "module-b") + * not-a-module/ + * readme.md + */ +async function createParentDirFixture() { + const parentDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-parent-')); + + // Module A + await fs.ensureDir(path.join(parentDir, 'module-a')); + await fs.writeFile(path.join(parentDir, 'module-a', 'module.yaml'), 'code: module-a\nname: Module A\n'); + + // Module B + await fs.ensureDir(path.join(parentDir, 'module-b')); + await fs.writeFile(path.join(parentDir, 'module-b', 'module.yaml'), 'code: module-b\nname: Module B\n'); + + // Not a module (no module.yaml) + await fs.ensureDir(path.join(parentDir, 'not-a-module')); + await fs.writeFile(path.join(parentDir, 'not-a-module', 'readme.md'), '# Not a module\n'); + + return parentDir; +} + +/** + * Create a temp directory simulating a direct module path + */ +async function createDirectModuleFixture() { + const moduleDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-module-')); + await fs.writeFile(path.join(moduleDir, 'module.yaml'), 'code: direct-module\nname: Direct Module\n'); + return moduleDir; +} + +/** + * Create a temp directory with no modules at any level + */ +async function createEmptyDirFixture() { + const emptyDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-empty-')); + await fs.writeFile(path.join(emptyDir, 'readme.md'), '# Empty\n'); + return emptyDir; +} + +async function runTests() { + const ui = new UI(); + + // ============================================================ + // Test Suite 1: validateCustomContentPathSync with parent dirs + // ============================================================ + console.log(`\n${colors.yellow}Test Suite 1: validateCustomContentPathSync${colors.reset}\n`); + + let parentDir; + let directModuleDir; + let emptyDir; + + try { + parentDir = await createParentDirFixture(); + directModuleDir = await createDirectModuleFixture(); + emptyDir = await createEmptyDirFixture(); + + // Direct module path should pass (existing behavior) + const directResult = ui.validateCustomContentPathSync(directModuleDir); + assert(directResult === undefined, 'Direct module path validates successfully'); + + // Parent dir with module subdirs should pass (new behavior) + const parentResult = ui.validateCustomContentPathSync(parentDir); + assert(parentResult === undefined, 'Parent directory with module subdirectories validates successfully', `Got error: ${parentResult}`); + + // Empty dir (no modules at any level) should fail + const emptyResult = ui.validateCustomContentPathSync(emptyDir); + assert( + typeof emptyResult === 'string' && emptyResult.length > 0, + 'Directory with no modules at any level fails validation', + `Expected error string, got: ${emptyResult}`, + ); + + // Non-existent path should fail + const nonExistResult = ui.validateCustomContentPathSync('/tmp/does-not-exist-bmad-xyz'); + assert(typeof nonExistResult === 'string', 'Non-existent path fails validation'); + + // Empty input should pass (allows cancel) + const emptyInputResult = ui.validateCustomContentPathSync(''); + assert(emptyInputResult === undefined, 'Empty input passes (allows cancel)'); + } finally { + if (parentDir) await fs.remove(parentDir).catch(() => {}); + if (directModuleDir) await fs.remove(directModuleDir).catch(() => {}); + if (emptyDir) await fs.remove(emptyDir).catch(() => {}); + } + + // ============================================================ + // Test Suite 2: resolveCustomContentPaths + // ============================================================ + console.log(`\n${colors.yellow}Test Suite 2: resolveCustomContentPaths${colors.reset}\n`); + + try { + parentDir = await createParentDirFixture(); + directModuleDir = await createDirectModuleFixture(); + emptyDir = await createEmptyDirFixture(); + + // Direct module path returns itself + const directPaths = ui.resolveCustomContentPaths(directModuleDir); + assert(Array.isArray(directPaths), 'resolveCustomContentPaths returns an array for direct module'); + assert(directPaths.length === 1, 'Direct module path resolves to exactly 1 path'); + assert( + directPaths[0] === directModuleDir, + 'Direct module path resolves to itself', + `Expected ${directModuleDir}, got ${directPaths[0]}`, + ); + + // Parent dir expands to individual module subdirs + const parentPaths = ui.resolveCustomContentPaths(parentDir); + assert(Array.isArray(parentPaths), 'resolveCustomContentPaths returns an array for parent dir'); + assert( + parentPaths.length === 2, + 'Parent dir resolves to 2 module paths (skips not-a-module)', + `Expected 2, got ${parentPaths.length}: ${JSON.stringify(parentPaths)}`, + ); + + const resolvedNames = parentPaths.map((p) => path.basename(p)).sort(); + assert( + resolvedNames[0] === 'module-a' && resolvedNames[1] === 'module-b', + 'Parent dir resolves to module-a and module-b', + `Got: ${JSON.stringify(resolvedNames)}`, + ); + + // Empty dir returns empty array + const emptyPaths = ui.resolveCustomContentPaths(emptyDir); + assert(Array.isArray(emptyPaths), 'resolveCustomContentPaths returns an array for empty dir'); + assert(emptyPaths.length === 0, 'Empty dir resolves to 0 paths', `Expected 0, got ${emptyPaths.length}`); + + // Non-existent path returns empty array + const nonExistPaths = ui.resolveCustomContentPaths('/tmp/does-not-exist-bmad-xyz'); + assert(Array.isArray(nonExistPaths), 'resolveCustomContentPaths returns array for non-existent path'); + assert(nonExistPaths.length === 0, 'Non-existent path resolves to 0 paths'); + } finally { + if (parentDir) await fs.remove(parentDir).catch(() => {}); + if (directModuleDir) await fs.remove(directModuleDir).catch(() => {}); + if (emptyDir) await fs.remove(emptyDir).catch(() => {}); + } + + // ============================================================ + // Summary + // ============================================================ + console.log(`\n${colors.cyan}========================================`); + console.log('Test Results:'); + console.log(` Passed: ${colors.green}${passed}${colors.reset}`); + console.log(` Failed: ${colors.red}${failed}${colors.reset}`); + console.log(`========================================${colors.reset}\n`); + + if (failed === 0) { + console.log(`${colors.green}✨ All custom content parent dir tests passed!${colors.reset}\n`); + process.exit(0); + } else { + console.log(`${colors.red}❌ Some custom content parent dir tests failed${colors.reset}\n`); + process.exit(1); + } +} + +runTests().catch((error) => { + console.error(`${colors.red}Test runner failed:${colors.reset}`, error.message); + console.error(error.stack); + process.exit(1); +}); diff --git a/tools/cli/lib/ui.js b/tools/cli/lib/ui.js index 1338c1f17..b45f9a06d 100644 --- a/tools/cli/lib/ui.js +++ b/tools/cli/lib/ui.js @@ -258,19 +258,20 @@ class UI { const sources = []; for (const customPath of paths) { const expandedPath = this.expandUserPath(customPath); - const validation = this.validateCustomContentPathSync(expandedPath); - if (validation) continue; - let moduleMeta; - try { - const moduleYamlPath = path.join(expandedPath, 'module.yaml'); - moduleMeta = require('yaml').parse(await fs.readFile(moduleYamlPath, 'utf-8')); - } catch { - continue; + const resolvedPaths = this.resolveCustomContentPaths(expandedPath); + for (const modulePath of resolvedPaths) { + let moduleMeta; + try { + const moduleYamlPath = path.join(modulePath, 'module.yaml'); + moduleMeta = require('yaml').parse(await fs.readFile(moduleYamlPath, 'utf-8')); + } catch { + continue; + } + if (!moduleMeta?.code) continue; + customPaths.push(modulePath); + selectedModuleIds.push(moduleMeta.code); + sources.push({ path: modulePath, id: moduleMeta.code, name: moduleMeta.name || moduleMeta.code }); } - if (!moduleMeta?.code) continue; - customPaths.push(expandedPath); - selectedModuleIds.push(moduleMeta.code); - sources.push({ path: expandedPath, id: moduleMeta.code, name: moduleMeta.name || moduleMeta.code }); } if (customPaths.length > 0) { customContentForQuickUpdate = { @@ -346,41 +347,52 @@ class UI { for (const customPath of paths) { const expandedPath = this.expandUserPath(customPath); - const validation = this.validateCustomContentPathSync(expandedPath); - if (validation) { - await prompts.log.warn(`Skipping invalid custom content path: ${customPath} - ${validation}`); + + // Resolve path to module directories (supports parent dirs with subdirectories) + const resolvedPaths = this.resolveCustomContentPaths(expandedPath); + + if (resolvedPaths.length === 0) { + await prompts.log.warn( + `Skipping custom content path: ${customPath} - no module.yaml found (checked root and immediate subdirectories)`, + ); continue; } - // Read module metadata - let moduleMeta; - try { - const moduleYamlPath = path.join(expandedPath, 'module.yaml'); - const moduleYaml = await fs.readFile(moduleYamlPath, 'utf-8'); - const yaml = require('yaml'); - moduleMeta = yaml.parse(moduleYaml); - } catch (error) { - await prompts.log.warn(`Skipping custom content path: ${customPath} - failed to read module.yaml: ${error.message}`); - continue; + if (resolvedPaths.length > 1) { + await prompts.log.info(`Found ${resolvedPaths.length} modules in ${customPath}`); } - if (!moduleMeta) { - await prompts.log.warn(`Skipping custom content path: ${customPath} - module.yaml is empty`); - continue; - } + for (const modulePath of resolvedPaths) { + // Read module metadata + let moduleMeta; + try { + const moduleYamlPath = path.join(modulePath, 'module.yaml'); + const moduleYaml = await fs.readFile(moduleYamlPath, 'utf-8'); + const yaml = require('yaml'); + moduleMeta = yaml.parse(moduleYaml); + } catch (error) { + await prompts.log.warn(`Skipping custom content path: ${modulePath} - failed to read module.yaml: ${error.message}`); + continue; + } - if (!moduleMeta.code) { - await prompts.log.warn(`Skipping custom content path: ${customPath} - module.yaml missing 'code' field`); - continue; - } + if (!moduleMeta) { + await prompts.log.warn(`Skipping custom content path: ${modulePath} - module.yaml is empty`); + continue; + } - customPaths.push(expandedPath); - selectedModuleIds.push(moduleMeta.code); - sources.push({ - path: expandedPath, - id: moduleMeta.code, - name: moduleMeta.name || moduleMeta.code, - }); + if (!moduleMeta.code) { + await prompts.log.warn(`Skipping custom content path: ${modulePath} - module.yaml missing 'code' field`); + continue; + } + + customPaths.push(modulePath); + selectedModuleIds.push(moduleMeta.code); + sources.push({ + path: modulePath, + id: moduleMeta.code, + name: moduleMeta.name || moduleMeta.code, + }); + } } if (customPaths.length > 0) { @@ -500,41 +512,52 @@ class UI { for (const customPath of paths) { const expandedPath = this.expandUserPath(customPath); - const validation = this.validateCustomContentPathSync(expandedPath); - if (validation) { - await prompts.log.warn(`Skipping invalid custom content path: ${customPath} - ${validation}`); + + // Resolve path to module directories (supports parent dirs with subdirectories) + const resolvedPaths = this.resolveCustomContentPaths(expandedPath); + + if (resolvedPaths.length === 0) { + await prompts.log.warn( + `Skipping custom content path: ${customPath} - no module.yaml found (checked root and immediate subdirectories)`, + ); continue; } - // Read module metadata - let moduleMeta; - try { - const moduleYamlPath = path.join(expandedPath, 'module.yaml'); - const moduleYaml = await fs.readFile(moduleYamlPath, 'utf-8'); - const yaml = require('yaml'); - moduleMeta = yaml.parse(moduleYaml); - } catch (error) { - await prompts.log.warn(`Skipping custom content path: ${customPath} - failed to read module.yaml: ${error.message}`); - continue; + if (resolvedPaths.length > 1) { + await prompts.log.info(`Found ${resolvedPaths.length} modules in ${customPath}`); } - if (!moduleMeta) { - await prompts.log.warn(`Skipping custom content path: ${customPath} - module.yaml is empty`); - continue; - } + for (const modulePath of resolvedPaths) { + // Read module metadata + let moduleMeta; + try { + const moduleYamlPath = path.join(modulePath, 'module.yaml'); + const moduleYaml = await fs.readFile(moduleYamlPath, 'utf-8'); + const yaml = require('yaml'); + moduleMeta = yaml.parse(moduleYaml); + } catch (error) { + await prompts.log.warn(`Skipping custom content path: ${modulePath} - failed to read module.yaml: ${error.message}`); + continue; + } - if (!moduleMeta.code) { - await prompts.log.warn(`Skipping custom content path: ${customPath} - module.yaml missing 'code' field`); - continue; - } + if (!moduleMeta) { + await prompts.log.warn(`Skipping custom content path: ${modulePath} - module.yaml is empty`); + continue; + } - customPaths.push(expandedPath); - selectedModuleIds.push(moduleMeta.code); - sources.push({ - path: expandedPath, - id: moduleMeta.code, - name: moduleMeta.name || moduleMeta.code, - }); + if (!moduleMeta.code) { + await prompts.log.warn(`Skipping custom content path: ${modulePath} - module.yaml missing 'code' field`); + continue; + } + + customPaths.push(modulePath); + selectedModuleIds.push(moduleMeta.code); + sources.push({ + path: modulePath, + id: moduleMeta.code, + name: moduleMeta.name || moduleMeta.code, + }); + } } if (customPaths.length > 0) { @@ -1450,6 +1473,54 @@ class UI { return existingInstall.ides || []; } + /** + * Resolve a custom content path to an array of module paths. + * If the path contains module.yaml at root, returns [path]. + * If not, scans immediate subdirectories (1 level) for module.yaml. + * Returns an empty array if no modules are found. + * @param {string} inputPath - Absolute path to resolve + * @returns {string[]} Array of resolved module directory paths + */ + resolveCustomContentPaths(inputPath) { + // Direct module path + const moduleYamlPath = path.join(inputPath, 'module.yaml'); + if (fs.pathExistsSync(moduleYamlPath)) { + return [inputPath]; + } + + // Scan immediate subdirectories (1 level only) + if (!fs.pathExistsSync(inputPath)) { + return []; + } + + const resolved = []; + try { + const entries = fs.readdirSync(inputPath, { withFileTypes: true }); + for (const entry of entries) { + if (entry.isDirectory()) { + const subModuleYaml = path.join(inputPath, entry.name, 'module.yaml'); + if (fs.pathExistsSync(subModuleYaml)) { + // Validate the module.yaml has a code field + try { + const yaml = require('yaml'); + const content = fs.readFileSync(subModuleYaml, 'utf8'); + const moduleData = yaml.parse(content); + if (moduleData && moduleData.code) { + resolved.push(path.join(inputPath, entry.name)); + } + } catch { + // Skip invalid module.yaml files + } + } + } + } + } catch { + // Directory read failed + } + + return resolved; + } + /** * Validate custom content path synchronously * @param {string} input - User input path @@ -1479,7 +1550,12 @@ class UI { // Check for module.yaml in the root const moduleYamlPath = path.join(expandedPath, 'module.yaml'); if (!fs.pathExistsSync(moduleYamlPath)) { - return 'Directory must contain a module.yaml file in the root'; + // No module.yaml at root — scan immediate subdirectories (1 level) + const resolved = this.resolveCustomContentPaths(expandedPath); + if (resolved.length > 0) { + return; // Valid — parent directory contains module subdirectories + } + return 'Directory must contain a module.yaml file (in root or in immediate subdirectories)'; } // Try to parse the module.yaml to get the module ID