fix: Extension module with code - bmm replaces base BMM agents directory instead of merging
This commit is contained in:
Marcos Fadul
parent
9421f20b69
commit
bee45a6d58
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue