fix: remove empty skill-group dirs left in _bmad after install (#2461)
* fix: remove empty skill-group dirs left in _bmad after install Skill cleanup removed each skill's own directory but never pruned the now-empty grouping folders above it (e.g. _bmad/bmm/1-analysis), leaving empty dirs behind after every install. Walk up from each removed skill dir and drop empty parents, stopping at the bmad root. * fix: harden empty-parent pruning boundary and cleanup Use a path-boundary check instead of a string prefix so sibling dirs (e.g. _bmad2) can't match the bmad root, and make the walk best-effort so a dir that vanishes or fills in mid-walk never aborts the install. Move the test fixture cleanup into finally so failures don't leak temp dirs. --------- Co-authored-by: Brian <bmadcode@gmail.com>
This commit is contained in:
parent
b9431d6d99
commit
fbb48ed711
|
|
@ -3273,6 +3273,51 @@ async function runTests() {
|
||||||
|
|
||||||
console.log('');
|
console.log('');
|
||||||
|
|
||||||
|
// ============================================================
|
||||||
|
// Test Suite 45: _cleanupSkillDirs prunes empty parent dirs (#empty-bmm-folders)
|
||||||
|
// ============================================================
|
||||||
|
console.log(`${colors.yellow}Test Suite 45: cleanup prunes empty skill-group dirs${colors.reset}\n`);
|
||||||
|
|
||||||
|
let root45;
|
||||||
|
try {
|
||||||
|
root45 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-cleanup-test-'));
|
||||||
|
const bmadDir45 = path.join(root45, '_bmad');
|
||||||
|
await fs.ensureDir(path.join(bmadDir45, '_config'));
|
||||||
|
|
||||||
|
// Two skills nested under the same grouping dir (1-analysis), plus a
|
||||||
|
// module-level file that must survive the cleanup.
|
||||||
|
await fs.writeFile(
|
||||||
|
path.join(bmadDir45, '_config', 'skill-manifest.csv'),
|
||||||
|
[
|
||||||
|
'canonicalId,name,description,module,path',
|
||||||
|
'"bmad-agent-analyst","bmad-agent-analyst","fixture","bmm","_bmad/bmm/1-analysis/bmad-agent-analyst/SKILL.md"',
|
||||||
|
'"bmad-research","bmad-research","fixture","bmm","_bmad/bmm/1-analysis/research/bmad-research/SKILL.md"',
|
||||||
|
'',
|
||||||
|
].join('\n'),
|
||||||
|
);
|
||||||
|
await fs.ensureDir(path.join(bmadDir45, 'bmm', '1-analysis', 'bmad-agent-analyst'));
|
||||||
|
await fs.writeFile(path.join(bmadDir45, 'bmm', '1-analysis', 'bmad-agent-analyst', 'SKILL.md'), 'x');
|
||||||
|
await fs.ensureDir(path.join(bmadDir45, 'bmm', '1-analysis', 'research', 'bmad-research'));
|
||||||
|
await fs.writeFile(path.join(bmadDir45, 'bmm', '1-analysis', 'research', 'bmad-research', 'SKILL.md'), 'x');
|
||||||
|
await fs.writeFile(path.join(bmadDir45, 'bmm', 'config.yaml'), 'module: bmm\n');
|
||||||
|
|
||||||
|
const installer45 = new Installer();
|
||||||
|
await installer45._cleanupSkillDirs(bmadDir45);
|
||||||
|
|
||||||
|
assert(!(await fs.pathExists(path.join(bmadDir45, 'bmm', '1-analysis'))), 'empty skill-group dir is pruned after cleanup');
|
||||||
|
assert(!(await fs.pathExists(path.join(bmadDir45, 'bmm', '1-analysis', 'research'))), 'empty nested skill-group dir is pruned');
|
||||||
|
assert(await fs.pathExists(path.join(bmadDir45, 'bmm', 'config.yaml')), 'module-level files are preserved');
|
||||||
|
assert(await fs.pathExists(bmadDir45), 'bmad root is never removed');
|
||||||
|
} catch (error) {
|
||||||
|
console.log(`${colors.red}Test Suite 45 setup failed: ${error.message}${colors.reset}`);
|
||||||
|
console.log(error.stack);
|
||||||
|
failed++;
|
||||||
|
} finally {
|
||||||
|
if (root45) await fs.remove(root45).catch(() => {});
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log('');
|
||||||
|
|
||||||
// ============================================================
|
// ============================================================
|
||||||
// Summary
|
// Summary
|
||||||
// ============================================================
|
// ============================================================
|
||||||
|
|
|
||||||
|
|
@ -419,10 +419,35 @@ class Installer {
|
||||||
const sourceDir = path.dirname(path.join(bmadDir, relativePath));
|
const sourceDir = path.dirname(path.join(bmadDir, relativePath));
|
||||||
if (await fs.pathExists(sourceDir)) {
|
if (await fs.pathExists(sourceDir)) {
|
||||||
await fs.remove(sourceDir);
|
await fs.remove(sourceDir);
|
||||||
|
await this._removeEmptyParents(path.dirname(sourceDir), bmadDir);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Remove now-empty parent directories left behind after skill dir cleanup.
|
||||||
|
* Walks up from dir, stopping at (and never removing) bmadDir. Best-effort:
|
||||||
|
* a directory that vanishes or fills in mid-walk just ends the walk.
|
||||||
|
* @param {string} dir - Directory to start walking up from
|
||||||
|
* @param {string} bmadDir - BMAD installation directory (boundary)
|
||||||
|
*/
|
||||||
|
async _removeEmptyParents(dir, bmadDir) {
|
||||||
|
let current = dir;
|
||||||
|
while (true) {
|
||||||
|
// Path-boundary check (not a string prefix, so siblings like _bmad2 don't match).
|
||||||
|
const rel = path.relative(bmadDir, current);
|
||||||
|
if (rel === '' || rel.startsWith('..') || path.isAbsolute(rel)) break;
|
||||||
|
try {
|
||||||
|
const entries = await fs.readdir(current);
|
||||||
|
if (entries.length > 0) break;
|
||||||
|
await fs.rmdir(current);
|
||||||
|
} catch {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
current = path.dirname(current);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async _readSkillManifestRows(bmadDir) {
|
async _readSkillManifestRows(bmadDir) {
|
||||||
const csvPath = path.join(bmadDir, '_config', 'skill-manifest.csv');
|
const csvPath = path.join(bmadDir, '_config', 'skill-manifest.csv');
|
||||||
if (!(await fs.pathExists(csvPath))) return [];
|
if (!(await fs.pathExists(csvPath))) return [];
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue