Minor installer fixes (#1590)

* fix: remove redundant "None" skip option from module selection

The "None - Skip module installation" option was unnecessary since
core is always locked/selected, satisfying the required constraint.
Users can simply press Enter with only core selected to skip modules.
Also removes dead code: selectModules(), getExternalModuleChoices(),
and selectExternalModules() methods that were never called.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: support ESM and .cjs module installers in ModuleManager

Module installer loading now handles three cases:
- .cjs files loaded via require() (always CommonJS regardless of package type)
- .js files loaded via dynamic import() (works for both CJS and ESM)
- CJS default export unwrapped automatically for consistent API

This fixes errors when external modules set "type":"module" in their
package.json. Those modules must still rename installer.js to
installer.cjs if it uses require() internally.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address code review findings from PR #1590

- Filter 'core' from CLI --modules in update path for consistency
- Update selectAllModules() JSDoc to reflect core exclusion
- Fix ESM default-export unwrap to handle function/class exports

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: clarify module post-install script errors as non-fatal warnings

Change error display from log.error to log.warn and explain that the
module was installed successfully — only the optional post-install
script could not run. Prevents users from thinking the module
installation itself failed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: suppress non-fatal module post-install script errors

Post-install scripts fail due to CJS/ESM incompatibility but module
files are already copied successfully. Silently catch the error instead
of showing a warning that alarms users into thinking installation failed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: remove redundant modules and tools lines from install summary

The checkmark list already shows each installed module and IDE tool.
Keep only the install path and file-warning lines in the summary footer.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Brian <bmadcode@gmail.com>

tools/cli/installers/lib/core/installer.js

@@ -1201,19 +1201,11 @@ class Installer {
       lines.push(`  ${icon}  ${r.step}${detail}`);
     }
 
-    // Add context info
+    // Context and warnings
     lines.push('');
     if (context.bmadDir) {
       lines.push(`  Installed to: ${color.dim(context.bmadDir)}`);
     }
-    if (context.modules && context.modules.length > 0) {
-      lines.push(`  Modules: ${color.dim(context.modules.join(', '))}`);
-    }
-    if (context.ides && context.ides.length > 0) {
-      lines.push(`  Tools: ${color.dim(context.ides.join(', '))}`);
-    }
-
-    // Custom/modified file warnings
     if (context.customFiles && context.customFiles.length > 0) {
       lines.push(`  ${color.cyan(`Custom files preserved: ${context.customFiles.length}`)}`);
     }

tools/cli/installers/lib/modules/manager.js

@@ -1277,16 +1277,31 @@ class ModuleManager {
       }
     }
 
-    const installerPath = path.join(sourcePath, '_module-installer', 'installer.js');
+    const installerDir = path.join(sourcePath, '_module-installer');
+    // Prefer .cjs (always CommonJS) then fall back to .js
+    const cjsPath = path.join(installerDir, 'installer.cjs');
+    const jsPath = path.join(installerDir, 'installer.js');
+    const hasCjs = await fs.pathExists(cjsPath);
+    const installerPath = hasCjs ? cjsPath : jsPath;
 
     // Check if module has a custom installer
-    if (!(await fs.pathExists(installerPath))) {
+    if (!hasCjs && !(await fs.pathExists(jsPath))) {
       return; // No custom installer
     }
 
     try {
-      // Load the module installer
+      // .cjs files are always CommonJS and safe to require().
-      const moduleInstaller = require(installerPath);
+      // .js files may be ESM (when the package sets "type":"module"),
+      // so use dynamic import() which handles both CJS and ESM.
+      let moduleInstaller;
+      if (hasCjs) {
+        moduleInstaller = require(installerPath);
+      } else {
+        const { pathToFileURL } = require('node:url');
+        const imported = await import(pathToFileURL(installerPath).href);
+        // CJS module.exports lands on .default; ESM default can be object, function, or class
+        moduleInstaller = imported.default == null ? imported : imported.default;
+      }
 
       if (typeof moduleInstaller.install === 'function') {
         // Get project root (parent of bmad directory)
@@ -1312,8 +1327,12 @@ class ModuleManager {
           await prompts.log.warn(`Module installer for ${moduleName} returned false`);
         }
       }
-    } catch (error) {
+    } catch {
-      await prompts.log.error(`Error running module installer for ${moduleName}: ${error.message}`);
+      // Post-install scripts are optional; module files are already installed.
+      // TODO: Eliminate post-install scripts entirely by adding a `directories` key
+      // to module.yaml that declares which config keys are paths to auto-create.
+      // The main installer can then handle directory creation centrally, removing
+      // the need for per-module installer.js scripts and their CJS/ESM issues.
     }
   }
 

tools/cli/lib/ui.js

@@ -274,7 +274,6 @@ class UI {
           await prompts.log.info(`Using modules from command-line: ${selectedModules.join(', ')}`);
         } else {
           selectedModules = await this.selectAllModules(installedModuleIds);
-          selectedModules = selectedModules.filter((m) => m !== 'core');
         }
 
         // After module selection, ask about custom modules
@@ -362,6 +361,9 @@ class UI {
           selectedModules.push(...customModuleResult.selectedCustomModules);
         }
 
+        // Filter out core - it's always installed via installCore flag
+        selectedModules = selectedModules.filter((m) => m !== 'core');
+
         // Get tool selection
         const toolSelection = await this.promptToolSelection(confirmedDirectory, options);
 
@@ -899,107 +901,10 @@ class UI {
   }
 
   /**
-   * Prompt for module selection
+   * Select all modules (official + community) using grouped multiselect.
-   * @param {Array} moduleChoices - Available module choices
+   * Core is shown as locked but filtered from the result since it's always installed separately.
-   * @returns {Array} Selected module IDs
-   */
-  async selectModules(moduleChoices, defaultSelections = null) {
-    // If defaultSelections is provided, use it to override checked state
-    // Otherwise preserve the checked state from moduleChoices (set by getModuleChoices)
-    const choicesWithDefaults = moduleChoices.map((choice) => ({
-      ...choice,
-      ...(defaultSelections === null ? {} : { checked: defaultSelections.includes(choice.value) }),
-    }));
-
-    // Add a "None" option at the end for users who changed their mind
-    const choicesWithSkipOption = [
-      ...choicesWithDefaults,
-      {
-        value: '__NONE__',
-        label: '\u26A0 None / I changed my mind - skip module installation',
-        checked: false,
-      },
-    ];
-
-    const selected = await prompts.multiselect({
-      message: 'Select modules to install (use arrow keys, space to toggle):',
-      choices: choicesWithSkipOption,
-      required: true,
-    });
-
-    // If user selected both "__NONE__" and other items, honor the "None" choice
-    if (selected && selected.includes('__NONE__') && selected.length > 1) {
-      await prompts.log.warn('"None / I changed my mind" was selected, so no modules will be installed.');
-      return [];
-    }
-
-    // Filter out the special '__NONE__' value
-    return selected ? selected.filter((m) => m !== '__NONE__') : [];
-  }
-
-  /**
-   * Get external module choices for selection
-   * @returns {Array} External module choices for prompt
-   */
-  async getExternalModuleChoices() {
-    const externalManager = new ExternalModuleManager();
-    const modules = await externalManager.listAvailable();
-
-    return modules.map((mod) => ({
-      name: mod.name,
-      value: mod.code, // Use the code (e.g., 'cis') as the value
-      checked: mod.defaultSelected || false,
-      hint: mod.description || undefined, // Show description as hint
-      module: mod, // Store full module info for later use
-    }));
-  }
-
-  /**
-   * Prompt for external module selection
-   * @param {Array} externalModuleChoices - Available external module choices
-   * @param {Array} defaultSelections - Module codes to pre-select
-   * @returns {Array} Selected external module codes
-   */
-  async selectExternalModules(externalModuleChoices, defaultSelections = []) {
-    // Build a message showing available modules
-    const message = 'Select official BMad modules to install (use arrow keys, space to toggle):';
-
-    // Mark choices as checked based on defaultSelections
-    const choicesWithDefaults = externalModuleChoices.map((choice) => ({
-      ...choice,
-      checked: defaultSelections.includes(choice.value),
-    }));
-
-    // Add a "None" option at the end for users who changed their mind
-    const choicesWithSkipOption = [
-      ...choicesWithDefaults,
-      {
-        name: '⚠ None / I changed my mind - skip external module installation',
-        value: '__NONE__',
-        checked: false,
-      },
-    ];
-
-    const selected = await prompts.multiselect({
-      message,
-      choices: choicesWithSkipOption,
-      required: true,
-    });
-
-    // If user selected both "__NONE__" and other items, honor the "None" choice
-    if (selected && selected.includes('__NONE__') && selected.length > 1) {
-      await prompts.log.warn('"None / I changed my mind" was selected, so no external modules will be installed.');
-      return [];
-    }
-
-    // Filter out the special '__NONE__' value
-    return selected ? selected.filter((m) => m !== '__NONE__') : [];
-  }
-
-  /**
-   * Select all modules (core + official + community) using grouped multiselect
    * @param {Set} installedModuleIds - Currently installed module IDs
-   * @returns {Array} Selected module codes
+   * @returns {Array} Selected module codes (excluding core)
    */
   async selectAllModules(installedModuleIds = new Set()) {
     const { ModuleManager } = require('../installers/lib/modules/manager');
@@ -1068,11 +973,7 @@ class UI {
         }
       }
     }
-    allOptions.push(...communityModules.map(({ label, value, hint }) => ({ label, value, hint })), {
+    allOptions.push(...communityModules.map(({ label, value, hint }) => ({ label, value, hint })));
-      // "None" option at the end
-      label: '\u26A0 None - Skip module installation',
-      value: '__NONE__',
-    });
 
     const selected = await prompts.autocompleteMultiselect({
       message: 'Select modules to install:',
@@ -1083,14 +984,7 @@ class UI {
       maxItems: allOptions.length,
     });
 
-    // If user selected both "__NONE__" and other items, honor the "None" choice
+    const result = selected ? selected.filter((m) => m !== 'core') : [];
-    if (selected && selected.includes('__NONE__') && selected.length > 1) {
-      await prompts.log.warn('"None" was selected, so no modules will be installed.');
-      return [];
-    }
-
-    // Filter out the special '__NONE__' value
-    const result = selected ? selected.filter((m) => m !== '__NONE__') : [];
 
     // Display selected modules as bulleted list
     if (result.length > 0) {