diff --git a/test/test-installation-components.js b/test/test-installation-components.js index f6f08af8b..8ccb05eb1 100644 --- a/test/test-installation-components.js +++ b/test/test-installation-components.js @@ -3348,6 +3348,47 @@ async function runTests() { await fs.remove(tmp).catch(() => {}); } + // ---- Multiple module definitions: deepest-first default + chooser ---- + // Both src/ and src/skills/ carry module.yaml + module-help.csv. Headless + // resolution must take the deepest (src/skills); an interactive caller can + // override via chooseModuleDefinition, which receives enriched metadata. + { + const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-multimod-')); + const csv = + 'module,skill,display-name,menu-code,description,action,args,phase,preceded-by,followed-by,required,output-location,outputs\n'; + const srcDir = path.join(tmp, 'src'); + const skillsDir = path.join(srcDir, 'skills'); + await fs.ensureDir(skillsDir); + await fs.writeFile(path.join(srcDir, 'module.yaml'), 'code: outer\nname: Outer\ndescription: at src\n', 'utf8'); + await fs.writeFile(path.join(srcDir, 'module-help.csv'), csv, 'utf8'); + await fs.writeFile(path.join(skillsDir, 'module.yaml'), 'code: inner\nname: Inner\ndescription: at src/skills\n', 'utf8'); + await fs.writeFile(path.join(skillsDir, 'module-help.csv'), csv, 'utf8'); + for (const name of ['skill-a', 'skill-b']) { + const skill = path.join(skillsDir, name); + await fs.ensureDir(skill); + await fs.writeFile(path.join(skill, 'SKILL.md'), `---\nname: ${name}\ndescription: x\n---\n`, 'utf8'); + } + const plugin = { name: 'multi', source: '.', skills: ['./src/skills/skill-a', './src/skills/skill-b'] }; + + const def = await new PluginResolver().resolve(tmp, plugin); + assert(def[0].code === 'inner', 'with no chooser, the deepest module definition (src/skills) is used by default'); + + let seen = null; + const picked = await new PluginResolver().resolve(tmp, plugin, { + chooseModuleDefinition: async (candidates) => { + seen = candidates; + return candidates.find((c) => c.code === 'outer'); + }, + }); + assert(Array.isArray(seen) && seen.length === 2, 'chooser receives all candidates when more than one is found'); + assert( + seen.every((c) => typeof c.relativePath === 'string' && 'name' in c && 'description' in c), + 'chooser candidates are enriched with relativePath + module.yaml metadata', + ); + assert(picked[0].code === 'outer', "chooser's selection (src) overrides the deepest default"); + await fs.remove(tmp).catch(() => {}); + } + // ---- End-to-end install of a new-system module via OfficialModules ---- { const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-pj-install-')); diff --git a/tools/installer/modules/custom-module-manager.js b/tools/installer/modules/custom-module-manager.js index 8a5ea8863..50dedb16e 100644 --- a/tools/installer/modules/custom-module-manager.js +++ b/tools/installer/modules/custom-module-manager.js @@ -572,12 +572,14 @@ class CustomModuleManager { * @param {Object} plugin - Raw plugin object from marketplace.json * @param {string} [sourceUrl] - Original URL for manifest tracking (null for local) * @param {string} [localPath] - Local source path for manifest tracking (null for URLs) + * @param {Object} [options] - Forwarded to PluginResolver.resolve (e.g. + * chooseModuleDefinition for interactive disambiguation). * @returns {Promise>} Array of ResolvedModule objects */ - async resolvePlugin(repoPath, plugin, sourceUrl, localPath) { + async resolvePlugin(repoPath, plugin, sourceUrl, localPath, options = {}) { const { PluginResolver } = require('./plugin-resolver'); const resolver = new PluginResolver(); - const resolved = await resolver.resolve(repoPath, plugin); + const resolved = await resolver.resolve(repoPath, plugin, options); // Read clone metadata (written by cloneRepo) so we can pick up the // resolved git ref + SHA for manifest recording. diff --git a/tools/installer/modules/plugin-resolver.js b/tools/installer/modules/plugin-resolver.js index 7f2a6e06c..25ab6c99e 100644 --- a/tools/installer/modules/plugin-resolver.js +++ b/tools/installer/modules/plugin-resolver.js @@ -32,9 +32,15 @@ class PluginResolver { * @param {string} [plugin.version] - Semantic version * @param {string} [plugin.description] - Plugin description * @param {string[]} [plugin.skills] - Relative paths to skill directories + * @param {Object} [options] - Resolution options + * @param {function} [options.chooseModuleDefinition] - Async selector invoked + * when more than one module.yaml + module-help.csv pair is found between the + * skills' common parent and the repo root. Receives (candidates, context) + * and returns the chosen candidate. When omitted (headless / non-interactive + * / CLI), the deepest candidate is used so resolution never blocks. * @returns {Promise} Array of resolved module definitions */ - async resolve(repoPath, plugin) { + async resolve(repoPath, plugin, options = {}) { // Strategy 0: new module system. Tried before everything else — and before // the no-skills early return below — because new-system modules declare // their skills inside plugin.json rather than via marketplace.json's @@ -70,7 +76,7 @@ class PluginResolver { // Try each strategy in order const result = - (await this._tryRootModuleFiles(repoPath, plugin, skillPaths)) || + (await this._tryRootModuleFiles(repoPath, plugin, skillPaths, options)) || (await this._trySetupSkill(repoPath, plugin, skillPaths)) || (await this._trySingleStandalone(repoPath, plugin, skillPaths)) || (await this._tryMultipleStandalone(repoPath, plugin, skillPaths)) || @@ -170,13 +176,38 @@ class PluginResolver { * directory that has both files. This catches the common case where, e.g., * module.yaml sits at src/module.yaml but skills are in src/skills/. */ - async _tryRootModuleFiles(repoPath, plugin, skillPaths) { + async _tryRootModuleFiles(repoPath, plugin, skillPaths, options = {}) { const commonParent = this._computeCommonParent(skillPaths); - const found = await this._findModuleFilesUpward(commonParent, repoPath); - if (!found) { + const candidates = await this._findModuleFileCandidatesUpward(commonParent, repoPath); + if (candidates.length === 0) { return null; } - const { moduleYamlPath, moduleHelpPath } = found; + + // Deepest candidate (closest to the skills) is the safe default. When more + // than one directory in the chain carries both files, give an interactive + // caller the chance to pick — enriching each option with its module.yaml + // metadata so the choice is meaningful. Headless callers fall through to + // the deepest candidate without prompting. + let chosen = candidates[0]; + if (candidates.length > 1 && typeof options.chooseModuleDefinition === 'function') { + const enriched = []; + for (const candidate of candidates) { + const data = await this._readModuleYaml(candidate.moduleYamlPath); + enriched.push({ + ...candidate, + relativePath: path.relative(path.resolve(repoPath), candidate.moduleYamlPath), + code: data?.code || null, + name: data?.name || null, + description: data?.description || null, + }); + } + const picked = await options.chooseModuleDefinition(enriched, { plugin }); + if (picked && picked.moduleYamlPath && picked.moduleHelpPath) { + chosen = picked; + } + } + + const { moduleYamlPath, moduleHelpPath } = chosen; const moduleData = await this._readModuleYaml(moduleYamlPath); if (!moduleData) return null; @@ -376,14 +407,15 @@ class PluginResolver { // ─── Helpers ──────────────────────────────────────────────────────────────── /** - * Walk up from startDir to the repo root, returning the first directory that + * Walk up from startDir to the repo root, collecting every directory that * contains BOTH module.yaml and module-help.csv. Bounded by repoRoot so we - * never escape the cloned repository. Returns null if neither pair is found. + * never escape the cloned repository. Results are ordered deepest-first + * (closest to startDir), so candidates[0] is the safe default. * @param {string} startDir - Directory to start searching from (inclusive) * @param {string} repoPath - Repository root (upper bound, inclusive) - * @returns {Promise<{moduleYamlPath: string, moduleHelpPath: string}|null>} + * @returns {Promise>} */ - async _findModuleFilesUpward(startDir, repoPath) { + async _findModuleFileCandidatesUpward(startDir, repoPath) { const repoRoot = path.resolve(repoPath); let dir = path.resolve(startDir); @@ -392,11 +424,12 @@ class PluginResolver { dir = repoRoot; } + const candidates = []; while (true) { const moduleYamlPath = path.join(dir, 'module.yaml'); const moduleHelpPath = path.join(dir, 'module-help.csv'); if ((await fs.pathExists(moduleYamlPath)) && (await fs.pathExists(moduleHelpPath))) { - return { moduleYamlPath, moduleHelpPath }; + candidates.push({ moduleYamlPath, moduleHelpPath }); } if (dir === repoRoot) break; const parent = path.dirname(dir); @@ -404,7 +437,7 @@ class PluginResolver { dir = parent; } - return null; + return candidates; } /** diff --git a/tools/installer/ui.js b/tools/installer/ui.js index 2a520144f..810f6be90 100644 --- a/tools/installer/ui.js +++ b/tools/installer/ui.js @@ -1008,6 +1008,25 @@ class UI { const customMgr = new CustomModuleManager(); const selectedModules = []; + // Interactive disambiguation: when a plugin has more than one module.yaml + + // module-help.csv pair between its skills and the repo root, let the user + // pick which one defines the module. `activeSpinner` is paused around the + // prompt so the choice renders cleanly, then resumed by the caller. + let activeSpinner = null; + const chooseModuleDefinition = async (candidates, { plugin }) => { + activeSpinner?.stop('Multiple module definitions found'); + const choice = await prompts.select({ + message: `"${plugin.name}" declares multiple module definitions — choose which to install:`, + choices: candidates.map((c) => ({ + name: `${c.code || '(no code)'}${c.name ? ` — ${c.name}` : ''} [${c.relativePath}]`, + value: c, + })), + default: candidates[0], + }); + activeSpinner?.start('Analyzing plugin structure...'); + return choice; + }; + let addMore = true; while (addMore) { const sourceInput = await prompts.text({ @@ -1021,6 +1040,7 @@ class UI { }); const s = await prompts.spinner(); + activeSpinner = s; s.start('Resolving source...'); let sourceResult; @@ -1062,7 +1082,9 @@ class UI { const effectiveRepoPath = sourceResult.repoPath || sourceResult.rootDir; for (const plugin of plugins) { try { - const resolved = await customMgr.resolvePlugin(effectiveRepoPath, plugin.rawPlugin, sourceResult.sourceUrl, localPath); + const resolved = await customMgr.resolvePlugin(effectiveRepoPath, plugin.rawPlugin, sourceResult.sourceUrl, localPath, { + chooseModuleDefinition, + }); if (resolved.length > 0) { allResolved.push(...resolved); } else { @@ -1117,7 +1139,9 @@ class UI { // so an empty skills[] here is expected); legacy modules need ≥1 skill. if (rootManifest || directPlugin.skills.length > 0) { try { - const resolved = await customMgr.resolvePlugin(sourceResult.rootDir, directPlugin, sourceResult.sourceUrl, localPath); + const resolved = await customMgr.resolvePlugin(sourceResult.rootDir, directPlugin, sourceResult.sourceUrl, localPath, { + chooseModuleDefinition, + }); allResolved.push(...resolved); } catch (resolveError) { await prompts.log.warn(` Could not resolve: ${resolveError.message}`);