From bee45a6d5803d52bc0d9e6efe626007cff432d4a Mon Sep 17 00:00:00 2001 From: Marcos Fadul Date: Tue, 10 Mar 2026 00:36:55 +0100 Subject: [PATCH] fix: Extension module with code - bmm replaces base BMM agents directory instead of merging --- .github/workflows/quality.yaml | 3 + package.json | 5 +- test/test-installation-extensions.js | 302 ++++++++++++++++++++ tools/cli/installers/lib/core/installer.js | 35 +++ tools/cli/installers/lib/modules/manager.js | 5 +- 5 files changed, 347 insertions(+), 3 deletions(-) create mode 100644 test/test-installation-extensions.js diff --git a/.github/workflows/quality.yaml b/.github/workflows/quality.yaml index 78023e466..b1c7beed4 100644 --- a/.github/workflows/quality.yaml +++ b/.github/workflows/quality.yaml @@ -112,5 +112,8 @@ jobs: - name: Test agent compilation components run: npm run test:install + - name: Test module extension installer + run: npm run test:install:extensions + - name: Validate file references run: npm run validate:refs diff --git a/package.json b/package.json index f3207f5fa..31090c08d 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/package.json", "name": "bmad-method", - "version": "6.0.4", + "version": "6.0.5", "description": "Breakthrough Method of Agile AI-driven Development", "keywords": [ "agile", @@ -40,9 +40,10 @@ "lint:md": "markdownlint-cli2 \"**/*.md\"", "prepare": "command -v husky >/dev/null 2>&1 && husky || exit 0", "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": "npm run test:schemas && npm run test:refs && npm run test:install && npm run test:install:extensions && 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:install": "node test/test-installation-components.js", + "test:install:extensions": "node test/test-installation-extensions.js", "test:refs": "node test/test-file-refs-csv.js", "test:schemas": "node test/test-agent-schema.js", "validate:refs": "node tools/validate-file-refs.js --strict", diff --git a/test/test-installation-extensions.js b/test/test-installation-extensions.js new file mode 100644 index 000000000..19319f15b --- /dev/null +++ b/test/test-installation-extensions.js @@ -0,0 +1,302 @@ +/** + * Extension Module Merge Tests — Issue #1667 + * + * Verifies that a custom extension module with `code: bmm` in its module.yaml + * merges its files into the base BMM installation instead of replacing the + * entire directory. + * + * Expected behavior (file-level merge): + * - Files with the same name → extension overrides base + * - Files with unique names → extension adds alongside base + * + * Usage: node test/test-installation-extensions.js + */ + +const path = require('node:path'); +const os = require('node:os'); +const fs = require('fs-extra'); +const { ModuleManager } = require('../tools/cli/installers/lib/modules/manager'); + +// ANSI colors (match existing test files) +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++; + } +} + +/** + * Build a minimal compiled agent .md file (no YAML compilation needed). + */ +function minimalAgentMd(name) { + return ['---', `name: "${name}"`, `description: "Test agent: ${name}"`, '---', '', `You are ${name}.`].join('\n'); +} + +/** + * Create a fake base module source directory that looks like src/bmm/. + * Only files relevant to the merge test are created (agents directory). + */ +async function createBaseModuleSource(tmpDir) { + const src = path.join(tmpDir, 'src-base'); + + // module.yaml + await fs.ensureDir(src); + await fs.writeFile(path.join(src, 'module.yaml'), 'name: BMM\ncode: bmm\nversion: 6.0.0\n'); + + // agents: analyst + pm (standard base agents) + await fs.ensureDir(path.join(src, 'agents')); + await fs.writeFile(path.join(src, 'agents', 'analyst.md'), minimalAgentMd('analyst')); + await fs.writeFile(path.join(src, 'agents', 'pm.md'), minimalAgentMd('pm')); + + return src; +} + +/** + * Create a fake extension module source directory (simulates a user's + * custom module with code: bmm in its module.yaml). + * + * Includes: + * - pm.md → should OVERRIDE the base pm.md + * - my-agent.md → unique name, should be ADDED alongside base agents + */ +async function createExtensionModuleSource(tmpDir) { + const src = path.join(tmpDir, 'src-extension'); + + await fs.ensureDir(src); + await fs.writeFile(path.join(src, 'module.yaml'), 'name: My Extension\ncode: bmm\nversion: 1.0.0\n'); + + await fs.ensureDir(path.join(src, 'agents')); + await fs.writeFile(path.join(src, 'agents', 'pm.md'), minimalAgentMd('pm-override')); + await fs.writeFile(path.join(src, 'agents', 'my-agent.md'), minimalAgentMd('my-agent')); + + return src; +} + +async function runTests() { + console.log(`${colors.cyan}========================================`); + console.log('Extension Module Merge — Issue #1667'); + console.log(`========================================${colors.reset}\n`); + + // ───────────────────────────────────────────────────────────────────────── + // Test 1: isExtension:false (default) — second install replaces directory + // (confirms the bug existed: without the fix, extension would wipe base) + // ───────────────────────────────────────────────────────────────────────── + console.log(`${colors.yellow}Scenario A: install without isExtension (replacement mode)${colors.reset}\n`); + + { + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-ext-test-')); + try { + const baseSrc = await createBaseModuleSource(tmpDir); + const extSrc = await createExtensionModuleSource(tmpDir); + const bmadDir = path.join(tmpDir, '_bmad'); + const targetPath = path.join(bmadDir, 'bmm'); + + const manager = new ModuleManager(); + // Register extension as the 'bmm' custom module path + manager.setCustomModulePaths(new Map([['bmm', extSrc]])); + + // Simulate base install (not custom) + await fs.ensureDir(path.join(targetPath, 'agents')); + await fs.copy(path.join(baseSrc, 'agents', 'analyst.md'), path.join(targetPath, 'agents', 'analyst.md')); + await fs.copy(path.join(baseSrc, 'agents', 'pm.md'), path.join(targetPath, 'agents', 'pm.md')); + + // Simulate extension install WITHOUT isExtension flag (old/broken behavior) + if (await fs.pathExists(targetPath)) { + await fs.remove(targetPath); // This is what the old code did unconditionally + } + await fs.ensureDir(path.join(targetPath, 'agents')); + await fs.copy(path.join(extSrc, 'agents', 'pm.md'), path.join(targetPath, 'agents', 'pm.md')); + await fs.copy(path.join(extSrc, 'agents', 'my-agent.md'), path.join(targetPath, 'agents', 'my-agent.md')); + + const analystExists = await fs.pathExists(path.join(targetPath, 'agents', 'analyst.md')); + const myAgentExists = await fs.pathExists(path.join(targetPath, 'agents', 'my-agent.md')); + + assert(!analystExists, 'Without fix: base analyst.md is GONE (replacement behavior confirmed)'); + assert(myAgentExists, 'Without fix: extension my-agent.md is present'); + } finally { + await fs.remove(tmpDir); + } + } + + console.log(''); + + // ───────────────────────────────────────────────────────────────────────── + // Test 2: isExtension:true (the fix) — extension merges on top of base + // ───────────────────────────────────────────────────────────────────────── + console.log(`${colors.yellow}Scenario B: install with isExtension:true (merge mode — the fix)${colors.reset}\n`); + + { + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-ext-test-')); + try { + const baseSrc = await createBaseModuleSource(tmpDir); + const extSrc = await createExtensionModuleSource(tmpDir); + const bmadDir = path.join(tmpDir, '_bmad'); + const targetPath = path.join(bmadDir, 'bmm'); + + const manager = new ModuleManager(); + manager.setCustomModulePaths(new Map([['bmm', extSrc]])); + + // Step 1: Install base module (normal, no isExtension) + await fs.ensureDir(path.join(targetPath, 'agents')); + await fs.copy(path.join(baseSrc, 'agents', 'analyst.md'), path.join(targetPath, 'agents', 'analyst.md')); + await fs.copy(path.join(baseSrc, 'agents', 'pm.md'), path.join(targetPath, 'agents', 'pm.md')); + + // Step 2: Install extension with isExtension:true → should MERGE + // (reproduce what manager.install does: skip removal, then copy) + const skipRemoval = true; // isExtension:true skips fs.remove + if ((await fs.pathExists(targetPath)) && !skipRemoval) { + await fs.remove(targetPath); + } + // Copy extension files over (overwrite same-named, add unique) + await fs.copy(path.join(extSrc, 'agents', 'pm.md'), path.join(targetPath, 'agents', 'pm.md'), { overwrite: true }); + await fs.copy(path.join(extSrc, 'agents', 'my-agent.md'), path.join(targetPath, 'agents', 'my-agent.md')); + + const analystExists = await fs.pathExists(path.join(targetPath, 'agents', 'analyst.md')); + const myAgentExists = await fs.pathExists(path.join(targetPath, 'agents', 'my-agent.md')); + const pmContent = await fs.readFile(path.join(targetPath, 'agents', 'pm.md'), 'utf8'); + + assert(analystExists, 'Base analyst.md is PRESERVED after extension install'); + assert(myAgentExists, 'Extension my-agent.md is ADDED alongside base agents'); + assert(pmContent.includes('pm-override'), 'Extension pm.md OVERRIDES base pm.md (same-name wins)'); + } finally { + await fs.remove(tmpDir); + } + } + + console.log(''); + + // ───────────────────────────────────────────────────────────────────────── + // Test 3: ModuleManager.install() with isExtension:true via options + // ───────────────────────────────────────────────────────────────────────── + console.log(`${colors.yellow}Scenario C: ModuleManager.install() respects isExtension option${colors.reset}\n`); + + { + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-ext-test-')); + try { + const bmadDir = path.join(tmpDir, '_bmad'); + const targetPath = path.join(bmadDir, 'bmm'); + + // Pre-populate target with base files (simulates base module already installed) + await fs.ensureDir(path.join(targetPath, 'agents')); + await fs.writeFile(path.join(targetPath, 'agents', 'analyst.md'), minimalAgentMd('analyst')); + await fs.writeFile(path.join(targetPath, 'agents', 'pm.md'), minimalAgentMd('pm-base')); + + // Verify pre-condition + assert( + await fs.pathExists(path.join(targetPath, 'agents', 'analyst.md')), + 'Pre-condition: base analyst.md exists before extension install', + ); + + // Now check that isExtension:true in options causes skip of fs.remove + // by reading the manager source and verifying the condition + const managerSrc = await fs.readFile(path.join(__dirname, '../tools/cli/installers/lib/modules/manager.js'), 'utf8'); + + const hasExtensionGuard = managerSrc.includes('!options.isExtension'); + assert( + hasExtensionGuard, + 'manager.js guards fs.remove with !options.isExtension', + 'Expected: if ((await fs.pathExists(targetPath)) && !options.isExtension)', + ); + } finally { + await fs.remove(tmpDir); + } + } + + console.log(''); + + // ───────────────────────────────────────────────────────────────────────── + // Test 4 (E2E): installModuleWithDependencies → base install, then + // moduleManager.install with isExtension:true → merge + // ───────────────────────────────────────────────────────────────────────── + console.log( + `${colors.yellow}Scenario D: E2E install — base via installModuleWithDependencies, extension via moduleManager.install${colors.reset}\n`, + ); + + { + const { Installer } = require('../tools/cli/installers/lib/core/installer'); + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-e2e-test-')); + try { + const baseSrc = await createBaseModuleSource(tmpDir); + const extSrc = await createExtensionModuleSource(tmpDir); + const bmadDir = path.join(tmpDir, '_bmad'); + + // Pre-create a minimal manifest so moduleManager.install can record the module + await fs.ensureDir(path.join(bmadDir, '_config')); + await fs.writeFile( + path.join(bmadDir, '_config', 'manifest.yaml'), + 'installation:\n version: "0.0.0"\n installDate: "2026-01-01T00:00:00.000Z"\n lastUpdated: "2026-01-01T00:00:00.000Z"\nmodules: []\nides: []\n', + ); + + const installer = new Installer(); + + // Stub the resolver: point 'bmm' at the fake base source + installer.moduleManager.setCustomModulePaths(new Map([['bmm', baseSrc]])); + + // Step 1: Install base module via the public installModuleWithDependencies + await installer.installModuleWithDependencies('bmm', bmadDir, {}); + + const targetPath = path.join(bmadDir, 'bmm'); + assert( + await fs.pathExists(path.join(targetPath, 'agents', 'analyst.md')), + 'E2E: after base install via installModuleWithDependencies — analyst.md is present', + ); + assert( + await fs.pathExists(path.join(targetPath, 'agents', 'pm.md')), + 'E2E: after base install via installModuleWithDependencies — pm.md is present', + ); + + // Step 2: Re-stub the resolver to point to the extension source, then install + // the extension with isExtension:true so it merges on top of the base + installer.moduleManager.setCustomModulePaths(new Map([['bmm', extSrc]])); + await installer.moduleManager.install('bmm', bmadDir, null, { + isExtension: true, + skipModuleInstaller: true, + silent: true, + }); + + // Assert merged output: base files preserved, extension files added/overridden + assert( + await fs.pathExists(path.join(targetPath, 'agents', 'analyst.md')), + 'E2E: base analyst.md is PRESERVED after extension install (merge, not replace)', + ); + assert( + await fs.pathExists(path.join(targetPath, 'agents', 'my-agent.md')), + 'E2E: extension my-agent.md is ADDED alongside base agents', + ); + const pmContent = await fs.readFile(path.join(targetPath, 'agents', 'pm.md'), 'utf8'); + assert(pmContent.includes('pm-override'), 'E2E: extension pm.md OVERRIDES base pm.md (same-name wins)'); + } finally { + await fs.remove(tmpDir); + } + } + + // ───────────────────────────────────────────────────────────────────────── + // Summary + // ───────────────────────────────────────────────────────────────────────── + console.log(`\n${colors.cyan}========================================${colors.reset}`); + console.log(`Results: ${colors.green}${passed} passed${colors.reset}, ${failed > 0 ? colors.red : ''}${failed} failed${colors.reset}`); + console.log(`${colors.cyan}========================================${colors.reset}\n`); + + if (failed > 0) process.exit(1); +} + +runTests().catch((error) => { + console.error(`${colors.red}Unexpected error:${colors.reset}`, error); + process.exit(1); +}); diff --git a/tools/cli/installers/lib/core/installer.js b/tools/cli/installers/lib/core/installer.js index 1d9868b60..c56e52b0b 100644 --- a/tools/cli/installers/lib/core/installer.js +++ b/tools/cli/installers/lib/core/installer.js @@ -999,6 +999,40 @@ class Installer { this.moduleManager.setCustomModulePaths(customModulePaths); } + // Check if this custom module extends a built-in/external base module. + // If a base source exists at a different path, install it first so the + // extension merges on top instead of replacing the entire directory (#1667). + const customSourcePath = customModulePaths.get(moduleName); + const baseSourcePath = getModulePath(moduleName); + const hasBaseModule = + customSourcePath && + baseSourcePath !== customSourcePath && + (await fs.pathExists(path.join(baseSourcePath, 'module.yaml'))); + + if (hasBaseModule) { + // Temporarily clear custom path so findModuleSource resolves to the base + customModulePaths.delete(moduleName); + this.moduleManager.setCustomModulePaths(customModulePaths); + + // Install the base module first (clean install, removes any prior version) + if (resolution && resolution.byModule && resolution.byModule[moduleName]) { + await this.installModuleWithDependencies(moduleName, bmadDir, resolution.byModule[moduleName]); + } else { + await this.moduleManager.install( + moduleName, + bmadDir, + (filePath) => { + this.installedFiles.add(filePath); + }, + { installer: this, silent: true }, + ); + } + + // Restore the custom path for the extension overlay + customModulePaths.set(moduleName, customSourcePath); + this.moduleManager.setCustomModulePaths(customModulePaths); + } + const collectedModuleConfig = moduleConfigs[moduleName] || {}; await this.moduleManager.install( moduleName, @@ -1008,6 +1042,7 @@ class Installer { }, { isCustom: true, + isExtension: hasBaseModule, // merge on top of base when extending moduleConfig: collectedModuleConfig, isQuickUpdate: isQuickUpdate, installer: this, diff --git a/tools/cli/installers/lib/modules/manager.js b/tools/cli/installers/lib/modules/manager.js index 7ac85678b..fa1b3dd52 100644 --- a/tools/cli/installers/lib/modules/manager.js +++ b/tools/cli/installers/lib/modules/manager.js @@ -554,7 +554,10 @@ class ModuleManager { } // Check if already installed - if (await fs.pathExists(targetPath)) { + // When isExtension is true, the caller is merging an extension module on top + // of an already-installed base module (same code). Skip removal so that base + // files are preserved and the extension's files overlay at the file level. + if ((await fs.pathExists(targetPath)) && !options.isExtension) { await fs.remove(targetPath); }