From fbb48ed711c3381aede1497ca107b97dd2172210 Mon Sep 17 00:00:00 2001
From: Dov Benyomin Sohacheski <b@kloud.email>
Date: Thu, 11 Jun 2026 16:29:02 +0300
Subject: [PATCH] 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>
---
 test/test-installation-components.js | 45 ++++++++++++++++++++++++++++
 tools/installer/core/installer.js    | 25 ++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/test/test-installation-components.js b/test/test-installation-components.js
index d5d1a6e62..6e015322a 100644
--- a/test/test-installation-components.js
+++ b/test/test-installation-components.js
@@ -3273,6 +3273,51 @@ async function runTests() {
 
   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
   // ============================================================
diff --git a/tools/installer/core/installer.js b/tools/installer/core/installer.js
index 85d47c26e..9c6c6cb6c 100644
--- a/tools/installer/core/installer.js
+++ b/tools/installer/core/installer.js
@@ -419,10 +419,35 @@ class Installer {
       const sourceDir = path.dirname(path.join(bmadDir, relativePath));
       if (await fs.pathExists(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) {
     const csvPath = path.join(bmadDir, '_config', 'skill-manifest.csv');
     if (!(await fs.pathExists(csvPath))) return [];