ldy26098
/
BMAD-METHOD
mirror of https://github.com/bmad-code-org/BMAD-METHOD.git
1
0
Fork 0
Code Packages Projects Releases Wiki Activity

fix(installer): address PR #2313 review findings 

Three issues raised by augmentcode and coderabbit bot reviewers:

1. _removeDeselectedIdes silently swallowed cleanup failures after the
   refactor to cleanupByList. The old per-IDE try/catch logged a warning;
   the new path discarded the result array. Now logs a warning per failed
   ide so failures stay visible.

2. The legacy-dir cleanup hint printed `rm -rf "<path>"/bmad*` which both
   matched bmad-os-* utility skills the user should keep AND missed the
   custom-module skills (e.g. fred-cool-skill) that the new canonical-id
   detection now finds. Findings now carry the exact entry names from the
   scan, and the warning prints one precise rm line per entry.

3. warnPreNativeSkillsLegacy did unguarded fs reads at install start. A
   permission/IO error would have aborted the whole install. Wrapped the
   call site in try/catch so legacy-scan failures only emit a warning.
This commit is contained in:
Brian Madison 2026-04-25 21:11:06 -05:00
parent bcf86f2330
commit ff5c5b257a
2 changed files with 24 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
tools/installer/core/installer.js
View File

@ -42,10 +42,15 @@ class Installer {
const officialModules = await OfficialModules.build(config, paths);
const existingInstall = await ExistingInstall.detect(paths.bmadDir);
await warnPreNativeSkillsLegacy({
projectRoot: paths.projectRoot,
existingVersion: existingInstall.installed ? existingInstall.version : null,
});
try {
await warnPreNativeSkillsLegacy({
projectRoot: paths.projectRoot,
existingVersion: existingInstall.installed ? existingInstall.version : null,
});
} catch (error) {
// Legacy-dir scan is informational; never let it abort install.
await prompts.log.warn(`Warning: Could not check for legacy BMAD entries: ${error.message}`);
}
if (existingInstall.installed) {
await this._removeDeselectedModules(existingInstall, config, paths);
@ -192,9 +197,15 @@ class Installer {
// Pass the newly-selected list as remainingIdes so cleanupByList skips
// target_dir wipes for IDEs whose directory is still owned by a peer
// (e.g. removing 'cursor' while 'gemini' remains — both share .agents/skills).
await this.ideManager.cleanupByList(paths.projectRoot, toRemove, {
const results = await this.ideManager.cleanupByList(paths.projectRoot, toRemove, {
remainingIdes: [...newlySelected],
});
for (const result of results || []) {
if (result && result.success === false) {
await prompts.log.warn(`Warning: Failed to remove ${result.ide}: ${result.error || 'unknown error'}`);
}
}
}
/**

10
tools/installer/core/legacy-warnings.js
View File

@ -92,7 +92,7 @@ async function findStaleLegacyDirs(projectRoot) {
const entries = await fs.readdir(resolved);
const bmadEntries = entries.filter((e) => isBmadOwnedEntry(e, canonicalIds));
if (bmadEntries.length > 0) {
findings.push({ path: resolved, displayPath: legacyPath, count: bmadEntries.length });
findings.push({ path: resolved, displayPath: legacyPath, count: bmadEntries.length, entries: bmadEntries });
}
} catch {
// Unreadable dir — skip
@ -127,7 +127,13 @@ async function warnPreNativeSkillsLegacy({ projectRoot, existingVersion } = {})
`Your AI tool may load these alongside the new skills, causing duplicates. Remove them manually:`,
);
for (const finding of staleDirs) {
await prompts.log.message(` rm -rf "${finding.path}"/bmad* # ${finding.count} stale entr${finding.count === 1 ? 'y' : 'ies'}`);
// Print each entry by exact name. A `bmad*` glob would (a) miss
// custom-module skills the canonicalId scan now picks up, and
// (b) match bmad-os-* utility skills the user should keep.
const entries = finding.entries || [];
for (const entry of entries) {
await prompts.log.message(` rm -rf "${path.join(finding.path, entry)}"`);
}
}
} else if (versionTriggered) {
await prompts.log.message(