feat(installer): prompt to pick module.yaml when a plugin has several
When the ancestor walk added in the previous commit finds more than one module.yaml + module-help.csv pair between the skills' common parent and the repo root, the deepest one was chosen silently. That could surprise a user whose repo legitimately carries module definitions at multiple levels. PluginResolver.resolve now accepts an optional chooseModuleDefinition callback. When >1 candidate is found it is invoked with each candidate enriched with its relativePath + module.yaml code/name/description, and its selection wins. Headless callers (the --custom-source CLI flow, tests, re-resolution lookups) omit the callback and keep the deterministic deepest-first default, so nothing blocks. The interactive custom-module flow supplies a prompt, pausing/resuming its spinner around the choice. Threads options through CustomModuleManager.resolvePlugin and wires the prompt into both resolve sites of _addCustomUrlModules. Adds tests for the deepest-first default, candidate enrichment, and chooser override. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
acbae8e107
commit
be1235fac6
|
|
@ -3348,6 +3348,47 @@ async function runTests() {
|
||||||
await fs.remove(tmp).catch(() => {});
|
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-'));
|
||||||
|
|
|
||||||
|
|
@ -572,12 +572,14 @@ 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) {
|
async resolvePlugin(repoPath, plugin, sourceUrl, localPath, options = {}) {
|
||||||
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);
|
const resolved = await resolver.resolve(repoPath, plugin, options);
|
||||||
|
|
||||||
// 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.
|
||||||
|
|
|
||||||
|
|
@ -32,9 +32,15 @@ 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) {
|
async resolve(repoPath, plugin, options = {}) {
|
||||||
// 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
|
||||||
|
|
@ -70,7 +76,7 @@ class PluginResolver {
|
||||||
|
|
||||||
// Try each strategy in order
|
// Try each strategy in order
|
||||||
const result =
|
const result =
|
||||||
(await this._tryRootModuleFiles(repoPath, plugin, skillPaths)) ||
|
(await this._tryRootModuleFiles(repoPath, plugin, skillPaths, options)) ||
|
||||||
(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)) ||
|
||||||
|
|
@ -170,13 +176,38 @@ class PluginResolver {
|
||||||
* directory that has both files. This catches the common case where, e.g.,
|
* 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/.
|
* 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 commonParent = this._computeCommonParent(skillPaths);
|
||||||
const found = await this._findModuleFilesUpward(commonParent, repoPath);
|
const candidates = await this._findModuleFileCandidatesUpward(commonParent, repoPath);
|
||||||
if (!found) {
|
if (candidates.length === 0) {
|
||||||
return null;
|
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);
|
const moduleData = await this._readModuleYaml(moduleYamlPath);
|
||||||
if (!moduleData) return null;
|
if (!moduleData) return null;
|
||||||
|
|
@ -376,14 +407,15 @@ class PluginResolver {
|
||||||
// ─── Helpers ────────────────────────────────────────────────────────────────
|
// ─── 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
|
* 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} startDir - Directory to start searching from (inclusive)
|
||||||
* @param {string} repoPath - Repository root (upper bound, inclusive)
|
* @param {string} repoPath - Repository root (upper bound, inclusive)
|
||||||
* @returns {Promise<{moduleYamlPath: string, moduleHelpPath: string}|null>}
|
* @returns {Promise<Array<{moduleYamlPath: string, moduleHelpPath: string}>>}
|
||||||
*/
|
*/
|
||||||
async _findModuleFilesUpward(startDir, repoPath) {
|
async _findModuleFileCandidatesUpward(startDir, repoPath) {
|
||||||
const repoRoot = path.resolve(repoPath);
|
const repoRoot = path.resolve(repoPath);
|
||||||
let dir = path.resolve(startDir);
|
let dir = path.resolve(startDir);
|
||||||
|
|
||||||
|
|
@ -392,11 +424,12 @@ class PluginResolver {
|
||||||
dir = repoRoot;
|
dir = repoRoot;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const candidates = [];
|
||||||
while (true) {
|
while (true) {
|
||||||
const moduleYamlPath = path.join(dir, 'module.yaml');
|
const moduleYamlPath = path.join(dir, 'module.yaml');
|
||||||
const moduleHelpPath = path.join(dir, 'module-help.csv');
|
const moduleHelpPath = path.join(dir, 'module-help.csv');
|
||||||
if ((await fs.pathExists(moduleYamlPath)) && (await fs.pathExists(moduleHelpPath))) {
|
if ((await fs.pathExists(moduleYamlPath)) && (await fs.pathExists(moduleHelpPath))) {
|
||||||
return { moduleYamlPath, moduleHelpPath };
|
candidates.push({ moduleYamlPath, moduleHelpPath });
|
||||||
}
|
}
|
||||||
if (dir === repoRoot) break;
|
if (dir === repoRoot) break;
|
||||||
const parent = path.dirname(dir);
|
const parent = path.dirname(dir);
|
||||||
|
|
@ -404,7 +437,7 @@ class PluginResolver {
|
||||||
dir = parent;
|
dir = parent;
|
||||||
}
|
}
|
||||||
|
|
||||||
return null;
|
return candidates;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
||||||
|
|
@ -1008,6 +1008,25 @@ 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({
|
||||||
|
|
@ -1021,6 +1040,7 @@ 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;
|
||||||
|
|
@ -1062,7 +1082,9 @@ 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 {
|
||||||
|
|
@ -1117,7 +1139,9 @@ 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}`);
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue