fix(installer): resolve legacy module.yaml above skills' common parent
PluginResolver strategy 1 only checked the skills' common parent for module.yaml + module-help.csv. For the canonical BMad layout (module files at src/, skills under src/skills/<name>/) the common parent is src/skills, so the files were missed and the resolver fell through to strategy 5 — synthesizing a degenerate module named after the marketplace plugin (e.g. bmad-creative-intelligence-suite) and discarding the real `code` (cis) and `agents:` roster. That mismatch then made resolveInstalledModuleYaml fail, emitting the collectAgentsFromModuleYaml and writeCentralConfig "could not locate module.yaml" warnings. Strategy 1 now walks up from the skills' common parent to the repo root (bounded, deepest-first) and uses the first directory with both files, so src/module.yaml resolves correctly. Also match on `pluginName` in resolveInstalledModuleYaml's resolution-cache fallback so a module tracked under its marketplace plugin name still resolves. Adds a regression test mirroring the bmad-creative-intelligence-suite layout. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
bca9388750
commit
acbae8e107
|
|
@ -3302,6 +3302,52 @@ 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(() => {});
|
||||||
|
}
|
||||||
|
|
||||||
// ---- 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-'));
|
||||||
|
|
|
||||||
|
|
@ -160,16 +160,23 @@ 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 skills.
|
* Check if module.yaml + module-help.csv exist at the common parent of all
|
||||||
|
* 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) {
|
async _tryRootModuleFiles(repoPath, plugin, skillPaths) {
|
||||||
const commonParent = this._computeCommonParent(skillPaths);
|
const commonParent = this._computeCommonParent(skillPaths);
|
||||||
const moduleYamlPath = path.join(commonParent, 'module.yaml');
|
const found = await this._findModuleFilesUpward(commonParent, repoPath);
|
||||||
const moduleHelpPath = path.join(commonParent, 'module-help.csv');
|
if (!found) {
|
||||||
|
|
||||||
if (!(await fs.pathExists(moduleYamlPath)) || !(await fs.pathExists(moduleHelpPath))) {
|
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
const { moduleYamlPath, moduleHelpPath } = found;
|
||||||
|
|
||||||
const moduleData = await this._readModuleYaml(moduleYamlPath);
|
const moduleData = await this._readModuleYaml(moduleYamlPath);
|
||||||
if (!moduleData) return null;
|
if (!moduleData) return null;
|
||||||
|
|
@ -368,6 +375,38 @@ class PluginResolver {
|
||||||
|
|
||||||
// ─── Helpers ────────────────────────────────────────────────────────────────
|
// ─── Helpers ────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Walk up from startDir to the repo root, returning the first 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.
|
||||||
|
* @param {string} startDir - Directory to start searching from (inclusive)
|
||||||
|
* @param {string} repoPath - Repository root (upper bound, inclusive)
|
||||||
|
* @returns {Promise<{moduleYamlPath: string, moduleHelpPath: string}|null>}
|
||||||
|
*/
|
||||||
|
async _findModuleFilesUpward(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;
|
||||||
|
}
|
||||||
|
|
||||||
|
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 };
|
||||||
|
}
|
||||||
|
if (dir === repoRoot) break;
|
||||||
|
const parent = path.dirname(dir);
|
||||||
|
if (parent === dir) break; // filesystem root — stop defensively
|
||||||
|
dir = parent;
|
||||||
|
}
|
||||||
|
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* 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
|
||||||
|
|
|
||||||
|
|
@ -171,7 +171,12 @@ 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) {
|
||||||
if ((mod.code === moduleName || mod.name === moduleName) && mod.localPath) {
|
// Match on code, display name, OR the marketplace plugin name. A legacy
|
||||||
|
// 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;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue