Compare commits

..

No commits in common. "be1235fac62aca07df0027a269aabe0be4b51339" and "bca9388750e1f51753d4845c0762989662f5832d" have entirely different histories.

5 changed files with 13 additions and 203 deletions

View File

@ -3302,93 +3302,6 @@ async function runTests() {
await fs.remove(tmp).catch(() => {}); await fs.remove(tmp).catch(() => {});
} }
// ---- Strategy 1: module files ABOVE the skills' common parent ----
// Mirrors the bmad-creative-intelligence-suite layout: module.yaml +
// module-help.csv at src/, skills nested under src/skills/<name>/. The
// skills' common parent is src/skills (no module files), so the resolver
// must walk up to src/ to find them — otherwise it synthesizes a degenerate
// module named after the plugin and loses the real code/agents roster.
{
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-srcmod-'));
const srcDir = path.join(tmp, 'src');
await fs.ensureDir(path.join(srcDir, 'skills'));
await fs.writeFile(
path.join(srcDir, 'module.yaml'),
'code: cis\nname: "CIS: Creative Innovation Suite"\ndescription: legacy at src\n',
'utf8',
);
await fs.writeFile(
path.join(srcDir, 'module-help.csv'),
'module,skill,display-name,menu-code,description,action,args,phase,preceded-by,followed-by,required,output-location,outputs\n',
'utf8',
);
for (const name of ['bmad-cis-storytelling', 'bmad-cis-design-thinking']) {
const skill = path.join(srcDir, 'skills', name);
await fs.ensureDir(skill);
await fs.writeFile(path.join(skill, 'SKILL.md'), `---\nname: ${name}\ndescription: x\n---\n`, 'utf8');
}
const resolved = await new PluginResolver().resolve(tmp, {
name: 'bmad-creative-intelligence-suite',
source: '.',
skills: ['./src/skills/bmad-cis-storytelling', './src/skills/bmad-cis-design-thinking'],
});
assert(
resolved.length === 1 && resolved[0].strategy === 1,
'module files above the skills common parent resolve via strategy 1 (not synthesized strategy 5)',
);
assert(
resolved[0].code === 'cis' && resolved[0].name === 'CIS: Creative Innovation Suite',
'code/name come from src/module.yaml, not the marketplace plugin name',
);
assert(
resolved[0].moduleYamlPath && resolved[0].moduleYamlPath.endsWith(path.join('src', 'module.yaml')),
'moduleYamlPath points at src/module.yaml',
);
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 ---- // ---- End-to-end install of a new-system module via OfficialModules ----
{ {
const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-pj-install-')); const tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-pj-install-'));

View File

@ -572,14 +572,12 @@ class CustomModuleManager {
* @param {Object} plugin - Raw plugin object from marketplace.json * @param {Object} plugin - Raw plugin object from marketplace.json
* @param {string} [sourceUrl] - Original URL for manifest tracking (null for local) * @param {string} [sourceUrl] - Original URL for manifest tracking (null for local)
* @param {string} [localPath] - Local source path for manifest tracking (null for URLs) * @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<Object>>} Array of ResolvedModule objects * @returns {Promise<Array<Object>>} Array of ResolvedModule objects
*/ */
async resolvePlugin(repoPath, plugin, sourceUrl, localPath, options = {}) { async resolvePlugin(repoPath, plugin, sourceUrl, localPath) {
const { PluginResolver } = require('./plugin-resolver'); const { PluginResolver } = require('./plugin-resolver');
const resolver = new PluginResolver(); const resolver = new PluginResolver();
const resolved = await resolver.resolve(repoPath, plugin, options); const resolved = await resolver.resolve(repoPath, plugin);
// Read clone metadata (written by cloneRepo) so we can pick up the // Read clone metadata (written by cloneRepo) so we can pick up the
// resolved git ref + SHA for manifest recording. // resolved git ref + SHA for manifest recording.

View File

@ -32,15 +32,9 @@ class PluginResolver {
* @param {string} [plugin.version] - Semantic version * @param {string} [plugin.version] - Semantic version
* @param {string} [plugin.description] - Plugin description * @param {string} [plugin.description] - Plugin description
* @param {string[]} [plugin.skills] - Relative paths to skill directories * @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<ResolvedModule[]>} Array of resolved module definitions * @returns {Promise<ResolvedModule[]>} Array of resolved module definitions
*/ */
async resolve(repoPath, plugin, options = {}) { async resolve(repoPath, plugin) {
// Strategy 0: new module system. Tried before everything else — and before // Strategy 0: new module system. Tried before everything else — and before
// the no-skills early return below — because new-system modules declare // the no-skills early return below — because new-system modules declare
// their skills inside plugin.json rather than via marketplace.json's // their skills inside plugin.json rather than via marketplace.json's
@ -76,7 +70,7 @@ class PluginResolver {
// Try each strategy in order // Try each strategy in order
const result = const result =
(await this._tryRootModuleFiles(repoPath, plugin, skillPaths, options)) || (await this._tryRootModuleFiles(repoPath, plugin, skillPaths)) ||
(await this._trySetupSkill(repoPath, plugin, skillPaths)) || (await this._trySetupSkill(repoPath, plugin, skillPaths)) ||
(await this._trySingleStandalone(repoPath, plugin, skillPaths)) || (await this._trySingleStandalone(repoPath, plugin, skillPaths)) ||
(await this._tryMultipleStandalone(repoPath, plugin, skillPaths)) || (await this._tryMultipleStandalone(repoPath, plugin, skillPaths)) ||
@ -166,49 +160,17 @@ class PluginResolver {
// ─── Strategy 1: Root Module Files ────────────────────────────────────────── // ─── Strategy 1: Root Module Files ──────────────────────────────────────────
/** /**
* Check if module.yaml + module-help.csv exist at the common parent of all * Check if module.yaml + module-help.csv exist at the common parent of all skills.
* skills, or in any directory between there and the repo root.
*
* The canonical BMad layout puts module.yaml + module-help.csv at the repo
* root or under src/, while skills live in src/skills/<name>/ i.e. one or
* more levels ABOVE the skills' common parent. We therefore start at the
* common parent and walk up to the repo root, using the first (deepest)
* 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, options = {}) { async _tryRootModuleFiles(repoPath, plugin, skillPaths) {
const commonParent = this._computeCommonParent(skillPaths); const commonParent = this._computeCommonParent(skillPaths);
const candidates = await this._findModuleFileCandidatesUpward(commonParent, repoPath); const moduleYamlPath = path.join(commonParent, 'module.yaml');
if (candidates.length === 0) { const moduleHelpPath = path.join(commonParent, 'module-help.csv');
if (!(await fs.pathExists(moduleYamlPath)) || !(await fs.pathExists(moduleHelpPath))) {
return null; return null;
} }
// 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); const moduleData = await this._readModuleYaml(moduleYamlPath);
if (!moduleData) return null; if (!moduleData) return null;
@ -406,40 +368,6 @@ class PluginResolver {
// ─── Helpers ──────────────────────────────────────────────────────────────── // ─── Helpers ────────────────────────────────────────────────────────────────
/**
* 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. 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<Array<{moduleYamlPath: string, moduleHelpPath: string}>>}
*/
async _findModuleFileCandidatesUpward(startDir, repoPath) {
const repoRoot = path.resolve(repoPath);
let dir = path.resolve(startDir);
// If startDir somehow falls outside the repo, only consider the repo root.
if (dir !== repoRoot && !dir.startsWith(repoRoot + path.sep)) {
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))) {
candidates.push({ moduleYamlPath, moduleHelpPath });
}
if (dir === repoRoot) break;
const parent = path.dirname(dir);
if (parent === dir) break; // filesystem root — stop defensively
dir = parent;
}
return candidates;
}
/** /**
* Compute the deepest common ancestor directory of an array of absolute paths. * Compute the deepest common ancestor directory of an array of absolute paths.
* @param {string[]} absPaths - Absolute directory paths * @param {string[]} absPaths - Absolute directory paths

View File

@ -171,12 +171,7 @@ async function resolveInstalledModuleYaml(moduleName) {
try { try {
const { CustomModuleManager } = require('./modules/custom-module-manager'); const { CustomModuleManager } = require('./modules/custom-module-manager');
for (const [, mod] of CustomModuleManager._resolutionCache) { for (const [, mod] of CustomModuleManager._resolutionCache) {
// Match on code, display name, OR the marketplace plugin name. A legacy if ((mod.code === moduleName || mod.name === moduleName) && mod.localPath) {
// module whose module.yaml `code`/`name` (e.g. cis / "CIS: …") diverges
// from its marketplace plugin name (e.g. bmad-creative-intelligence-suite)
// can be tracked downstream under any of the three — match all of them.
const matches = mod.code === moduleName || mod.name === moduleName || mod.pluginName === moduleName;
if (matches && mod.localPath) {
const found = await searchRoot(mod.localPath); const found = await searchRoot(mod.localPath);
if (found) return found; if (found) return found;
} }

View File

@ -1008,25 +1008,6 @@ class UI {
const customMgr = new CustomModuleManager(); const customMgr = new CustomModuleManager();
const selectedModules = []; 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; let addMore = true;
while (addMore) { while (addMore) {
const sourceInput = await prompts.text({ const sourceInput = await prompts.text({
@ -1040,7 +1021,6 @@ class UI {
}); });
const s = await prompts.spinner(); const s = await prompts.spinner();
activeSpinner = s;
s.start('Resolving source...'); s.start('Resolving source...');
let sourceResult; let sourceResult;
@ -1082,9 +1062,7 @@ class UI {
const effectiveRepoPath = sourceResult.repoPath || sourceResult.rootDir; const effectiveRepoPath = sourceResult.repoPath || sourceResult.rootDir;
for (const plugin of plugins) { for (const plugin of plugins) {
try { 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) { if (resolved.length > 0) {
allResolved.push(...resolved); allResolved.push(...resolved);
} else { } else {
@ -1139,9 +1117,7 @@ class UI {
// so an empty skills[] here is expected); legacy modules need ≥1 skill. // so an empty skills[] here is expected); legacy modules need ≥1 skill.
if (rootManifest || directPlugin.skills.length > 0) { if (rootManifest || directPlugin.skills.length > 0) {
try { 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); allResolved.push(...resolved);
} catch (resolveError) { } catch (resolveError) {
await prompts.log.warn(` Could not resolve: ${resolveError.message}`); await prompts.log.warn(` Could not resolve: ${resolveError.message}`);