From bbc7d58273a1f19f4d94b8be2aace56244072bc2 Mon Sep 17 00:00:00 2001 From: Brian Madison Date: Tue, 21 Apr 2026 22:46:38 -0500 Subject: [PATCH] fix(installer): resolve external-module agents from cache during manifest write External official modules (bmb, cis, gds, tea, wds) are cloned to ~/.bmad/cache/external-modules// and never copied into src/modules/, so collectAgentsFromModuleYaml silently skipped them and their agents never reached config.toml. Swap the hardcoded src/modules lookup for a resolveInstalledModuleYaml() helper that also searches the external cache (handling src/, skills/, nested, and root layouts) and warns instead of silently skipping when a module.yaml can't be found. --- test/test-installation-components.js | 99 ++++++++++++++++++++++ tools/installer/core/manifest-generator.js | 29 +++++-- tools/installer/project-root.js | 54 ++++++++++++ 3 files changed, 176 insertions(+), 6 deletions(-) diff --git a/test/test-installation-components.js b/test/test-installation-components.js index 7a5aefd6c..1e66e35bc 100644 --- a/test/test-installation-components.js +++ b/test/test-installation-components.js @@ -2256,6 +2256,105 @@ async function runTests() { console.log(''); + // ============================================================ + // Test Suite 38: External-Module Agent Resolution + // ============================================================ + console.log(`${colors.yellow}Test Suite 38: External-Module Agent Resolution${colors.reset}\n`); + + { + // Scenario: external official modules (bmb, cis, gds, ...) are cloned into + // ~/.bmad/cache/external-modules// โ€” NOT copied into src/modules/. + // collectAgentsFromModuleYaml must resolve them from the cache or their + // agent roster silently vanishes from config.toml. + const tempCacheDir38 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-ext-cache-')); + const tempBmadDir38 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-ext-install-')); + const priorCacheEnv = process.env.BMAD_EXTERNAL_MODULES_CACHE; + process.env.BMAD_EXTERNAL_MODULES_CACHE = tempCacheDir38; + + try { + // Seed a fake external module with agents at cache//src/module.yaml โ€” + // matches the real CIS layout. + const extSrcDir = path.join(tempCacheDir38, 'fake-ext', 'src'); + await fs.ensureDir(extSrcDir); + await fs.writeFile( + path.join(extSrcDir, 'module.yaml'), + [ + 'code: fake-ext', + 'name: "Fake External Module"', + 'agents:', + ' - code: bmad-fake-ext-agent-one', + ' name: Ext-One', + ' title: External Agent One', + ' icon: "๐Ÿงช"', + ' team: fake', + ' description: "First fake external agent."', + ' - code: bmad-fake-ext-agent-two', + ' name: Ext-Two', + ' title: External Agent Two', + ' icon: "๐Ÿงฌ"', + ' team: fake', + ' description: "Second fake external agent."', + '', + ].join('\n'), + ); + + // Second fake module at cache//skills/module.yaml โ€” matches bmb layout. + const extSkillsDir = path.join(tempCacheDir38, 'fake-skills', 'skills'); + await fs.ensureDir(extSkillsDir); + await fs.writeFile( + path.join(extSkillsDir, 'module.yaml'), + [ + 'code: fake-skills', + 'name: "Fake Skills-Layout Module"', + 'agents:', + ' - code: bmad-fake-skills-agent', + ' name: SkillsHero', + ' title: Skills Layout Agent', + ' icon: "๐Ÿ› ๏ธ"', + ' team: fake-skills', + ' description: "Lives under skills/ not src/."', + '', + ].join('\n'), + ); + + const generator38 = new ManifestGenerator(); + generator38.bmadDir = tempBmadDir38; + generator38.bmadFolderName = path.basename(tempBmadDir38); + generator38.updatedModules = ['core', 'bmm', 'fake-ext', 'fake-skills']; + + await generator38.collectAgentsFromModuleYaml(); + + const byCode = new Map(generator38.agents.map((a) => [a.code, a])); + assert(byCode.has('bmad-fake-ext-agent-one'), 'external module at cache//src resolves and contributes agent one'); + assert(byCode.has('bmad-fake-ext-agent-two'), 'external module at cache//src resolves and contributes agent two'); + assert(byCode.has('bmad-fake-skills-agent'), 'external module at cache//skills layout also resolves'); + assert(byCode.get('bmad-fake-ext-agent-one').module === 'fake-ext', 'agent.module matches the owning external module name'); + assert(byCode.get('bmad-fake-ext-agent-one').team === 'fake', 'explicit team from module.yaml is preserved'); + + await generator38.writeCentralConfig(tempBmadDir38, { + core: {}, + bmm: {}, + 'fake-ext': {}, + 'fake-skills': {}, + }); + + const teamContent = await fs.readFile(path.join(tempBmadDir38, 'config.toml'), 'utf8'); + assert(teamContent.includes('[agents.bmad-fake-ext-agent-one]'), 'external-module agents land in config.toml [agents.*] section'); + assert(teamContent.includes('[agents.bmad-fake-skills-agent]'), 'skills-layout external module agents also land in config.toml'); + assert(teamContent.includes('First fake external agent.'), 'agent description from external module.yaml is written'); + } finally { + if (priorCacheEnv === undefined) { + delete process.env.BMAD_EXTERNAL_MODULES_CACHE; + } else { + process.env.BMAD_EXTERNAL_MODULES_CACHE = priorCacheEnv; + } + await fs.remove(tempCacheDir38).catch(() => {}); + await fs.remove(tempBmadDir38).catch(() => {}); + } + } + + console.log(''); + // ============================================================ // Summary // ============================================================ diff --git a/tools/installer/core/manifest-generator.js b/tools/installer/core/manifest-generator.js index 0977b9e6b..206325638 100644 --- a/tools/installer/core/manifest-generator.js +++ b/tools/installer/core/manifest-generator.js @@ -2,7 +2,7 @@ const path = require('node:path'); const fs = require('../fs-native'); const yaml = require('yaml'); const crypto = require('node:crypto'); -const { getModulePath } = require('../project-root'); +const { resolveInstalledModuleYaml } = require('../project-root'); const prompts = require('../prompts'); // Load package.json for version info @@ -244,8 +244,17 @@ class ManifestGenerator { const debug = process.env.BMAD_DEBUG_MANIFEST === 'true'; for (const moduleName of this.updatedModules) { - const moduleYamlPath = path.join(getModulePath(moduleName), 'module.yaml'); - if (!(await fs.pathExists(moduleYamlPath))) continue; + const moduleYamlPath = await resolveInstalledModuleYaml(moduleName); + if (!moduleYamlPath) { + // External modules live in ~/.bmad/cache/external-modules, not src/modules. + // Warn rather than silently skip so missing agent rosters don't vanish + // from config.toml without notice. + console.warn( + `[warn] collectAgentsFromModuleYaml: could not locate module.yaml for '${moduleName}'. ` + + `Agents declared by this module will not be written to config.toml.`, + ); + continue; + } let moduleDef; try { @@ -271,7 +280,9 @@ class ManifestGenerator { } if (debug) { - console.log(`[DEBUG] collectAgentsFromModuleYaml: ${moduleName} contributed ${moduleDef.agents.length} agents`); + console.log( + `[DEBUG] collectAgentsFromModuleYaml: ${moduleName} contributed ${moduleDef.agents.length} agents from ${moduleYamlPath}`, + ); } } @@ -410,8 +421,14 @@ class ManifestGenerator { // team config, so the operator should notice. const scopeByModuleKey = {}; for (const moduleName of this.updatedModules) { - const moduleYamlPath = path.join(getModulePath(moduleName), 'module.yaml'); - if (!(await fs.pathExists(moduleYamlPath))) continue; + const moduleYamlPath = await resolveInstalledModuleYaml(moduleName); + if (!moduleYamlPath) { + console.warn( + `[warn] writeCentralConfig: could not locate module.yaml for '${moduleName}'. ` + + `Answers from this module will default to team scope โ€” user-scoped keys may mis-file into config.toml.`, + ); + continue; + } try { const parsed = yaml.parse(await fs.readFile(moduleYamlPath, 'utf8')); if (!parsed || typeof parsed !== 'object') continue; diff --git a/tools/installer/project-root.js b/tools/installer/project-root.js index 037f1a430..1cdc30566 100644 --- a/tools/installer/project-root.js +++ b/tools/installer/project-root.js @@ -1,4 +1,5 @@ const path = require('node:path'); +const os = require('node:os'); const fs = require('./fs-native'); /** @@ -69,9 +70,62 @@ function getModulePath(moduleName, ...segments) { return getSourcePath('modules', moduleName, ...segments); } +/** + * Path to the local external-module clone cache. + * External official modules (bmb, cis, gds, tea, wds, etc.) are cloned here + * by ExternalModuleManager during install and are not copied into /modules/. + */ +function getExternalModuleCachePath(moduleName, ...segments) { + const base = process.env.BMAD_EXTERNAL_MODULES_CACHE || path.join(os.homedir(), '.bmad', 'cache', 'external-modules'); + return path.join(base, moduleName, ...segments); +} + +/** + * Locate an installed module's `module.yaml` by filesystem lookup only. + * + * Built-in modules (core, bmm) live under . External official modules are + * cloned into ~/.bmad/cache/external-modules// with varying internal + * layouts (some at src/module.yaml, some at skills/module.yaml, some nested). + * This mirrors the candidate-path search in + * ExternalModuleManager.findExternalModuleSource but performs no git/network + * work, which keeps it safe to call during manifest writing. + * + * @param {string} moduleName + * @returns {Promise} Absolute path to module.yaml, or null if not found. + */ +async function resolveInstalledModuleYaml(moduleName) { + const builtIn = path.join(getModulePath(moduleName), 'module.yaml'); + if (await fs.pathExists(builtIn)) return builtIn; + + const cacheRoot = getExternalModuleCachePath(moduleName); + if (!(await fs.pathExists(cacheRoot))) return null; + + for (const dir of ['skills', 'src']) { + const direct = path.join(cacheRoot, dir, 'module.yaml'); + if (await fs.pathExists(direct)) return direct; + + const dirPath = path.join(cacheRoot, dir); + if (await fs.pathExists(dirPath)) { + const entries = await fs.readdir(dirPath, { withFileTypes: true }); + for (const entry of entries) { + if (!entry.isDirectory()) continue; + const nested = path.join(dirPath, entry.name, 'module.yaml'); + if (await fs.pathExists(nested)) return nested; + } + } + } + + const atRoot = path.join(cacheRoot, 'module.yaml'); + if (await fs.pathExists(atRoot)) return atRoot; + + return null; +} + module.exports = { getProjectRoot, getSourcePath, getModulePath, + getExternalModuleCachePath, + resolveInstalledModuleYaml, findProjectRoot, };