fix(installer): retain stale modules during updates

tools/installer/core/installer.js

@@ -76,25 +76,23 @@ class Installer {
       const results = [];
       const addResult = (step, status, detail = '', meta = {}) => results.push({ step, status, detail, ...meta });
 
-      // Capture previously installed skill IDs before they get overwritten
+      // Capture previously installed skill rows before they get overwritten
-      const previousSkillIds = new Set();
+      const preservedModules = originalConfig._preserveModules || [];
-      const prevCsvPath = path.join(paths.bmadDir, '_config', 'skill-manifest.csv');
+      const previousSkillManifestRows = await this._readSkillManifestRows(paths.bmadDir);
-      if (await fs.pathExists(prevCsvPath)) {
+      const previousSkillIds = this._getPreviousSkillIdsForCleanup(previousSkillManifestRows, preservedModules);
-        try {
-          const csvParse = require('csv-parse/sync');
-          const content = await fs.readFile(prevCsvPath, 'utf8');
-          const records = csvParse.parse(content, { columns: true, skip_empty_lines: true });
-          for (const r of records) {
-            if (r.canonicalId) previousSkillIds.add(r.canonicalId);
-          }
-        } catch (error) {
-          await prompts.log.warn(`Failed to parse skill-manifest.csv: ${error.message}`);
-        }
-      }
 
       const allModules = config.modules || [];
 
-      await this._installAndConfigure(config, originalConfig, paths, allModules, allModules, addResult, officialModules);
+      await this._installAndConfigure(
+        config,
+        originalConfig,
+        paths,
+        allModules,
+        allModules,
+        addResult,
+        officialModules,
+        previousSkillManifestRows,
+      );
 
       await this._setupIdes(config, allModules, paths, addResult, previousSkillIds);
 
@@ -213,7 +211,16 @@ class Installer {
   /**
    * Install modules, create directories, generate configs and manifests.
    */
-  async _installAndConfigure(config, originalConfig, paths, officialModuleIds, allModules, addResult, officialModules) {
+  async _installAndConfigure(
+    config,
+    originalConfig,
+    paths,
+    officialModuleIds,
+    allModules,
+    addResult,
+    officialModules,
+    previousSkillManifestRows = [],
+  ) {
     const isQuickUpdate = config.isQuickUpdate();
     const moduleConfigs = officialModules.moduleConfigs;
 
@@ -292,25 +299,29 @@ class Installer {
 
         message('Generating manifests...');
         const manifestGen = new ManifestGenerator();
+        const preservedModules = originalConfig._preserveModules || [];
 
         const allModulesForManifest = config.isQuickUpdate()
           ? originalConfig._existingModules || allModules || []
-          : originalConfig._preserveModules
+          : preservedModules.length > 0
-            ? [...allModules, ...originalConfig._preserveModules]
+            ? [...allModules, ...preservedModules]
             : allModules || [];
 
         let modulesForCsvPreserve;
         if (config.isQuickUpdate()) {
           modulesForCsvPreserve = originalConfig._existingModules || allModules || [];
         } else {
-          modulesForCsvPreserve = originalConfig._preserveModules ? [...allModules, ...originalConfig._preserveModules] : allModules;
+          modulesForCsvPreserve = preservedModules.length > 0 ? [...allModules, ...preservedModules] : allModules;
         }
 
+        await this._trackPreservedModuleFiles(paths.bmadDir, preservedModules);
+
         await manifestGen.generateManifests(paths.bmadDir, allModulesForManifest, [...this.installedFiles], {
           ides: config.ides || [],
           preservedModules: modulesForCsvPreserve,
           moduleConfigs,
         });
+        await this._appendPreservedSkillManifestRows(paths.bmadDir, previousSkillManifestRows, preservedModules);
 
         // Apply post-install --set TOML patches. Runs after writeCentralConfig
         // (inside generateManifests above) so the patch operates on the
@@ -412,6 +423,62 @@ class Installer {
     }
   }
 
+  async _readSkillManifestRows(bmadDir) {
+    const csvPath = path.join(bmadDir, '_config', 'skill-manifest.csv');
+    if (!(await fs.pathExists(csvPath))) return [];
+
+    try {
+      const csvParse = require('csv-parse/sync');
+      const content = await fs.readFile(csvPath, 'utf8');
+      return csvParse.parse(content, { columns: true, skip_empty_lines: true });
+    } catch (error) {
+      await prompts.log.warn(`Failed to parse skill-manifest.csv: ${error.message}`);
+      return [];
+    }
+  }
+
+  _getPreviousSkillIdsForCleanup(previousRows, preservedModules = []) {
+    const preservedModuleSet = new Set(preservedModules || []);
+    const ids = new Set();
+    for (const row of previousRows || []) {
+      if (row.canonicalId && !preservedModuleSet.has(row.module)) {
+        ids.add(row.canonicalId);
+      }
+    }
+    return ids;
+  }
+
+  async _appendPreservedSkillManifestRows(bmadDir, previousRows, preservedModules = []) {
+    if (!previousRows || previousRows.length === 0 || preservedModules.length === 0) return;
+
+    const preservedModuleSet = new Set(preservedModules);
+    const rowsToPreserve = previousRows.filter((row) => row.canonicalId && row.module && preservedModuleSet.has(row.module));
+    if (rowsToPreserve.length === 0) return;
+
+    const csvPath = path.join(bmadDir, '_config', 'skill-manifest.csv');
+    if (!(await fs.pathExists(csvPath))) return;
+
+    const currentRows = await this._readSkillManifestRows(bmadDir);
+    const activeIds = new Set(currentRows.map((row) => row.canonicalId).filter(Boolean));
+    const appendedRows = [];
+
+    for (const row of rowsToPreserve) {
+      if (activeIds.has(row.canonicalId)) continue;
+      activeIds.add(row.canonicalId);
+      appendedRows.push(
+        [row.canonicalId, row.name || row.canonicalId, row.description || '', row.module, row.path || '']
+          .map((field) => this.escapeCSVField(field))
+          .join(','),
+      );
+    }
+
+    if (appendedRows.length === 0) return;
+
+    const currentContent = await fs.readFile(csvPath, 'utf8');
+    const prefix = currentContent.endsWith('\n') ? currentContent : `${currentContent}\n`;
+    await fs.writeFile(csvPath, prefix + appendedRows.join('\n') + '\n', 'utf8');
+  }
+
   /**
    * Restore custom and modified files that were backed up before the update.
    * No-op for fresh installs (updateState is null).
@@ -598,6 +665,15 @@ class Installer {
     }
   }
 
+  async _trackPreservedModuleFiles(bmadDir, preservedModules = []) {
+    for (const moduleName of preservedModules) {
+      const modulePath = path.join(bmadDir, moduleName);
+      if (await fs.pathExists(modulePath)) {
+        await this._trackFilesRecursive(modulePath);
+      }
+    }
+  }
+
   /**
    * Install official (non-custom) modules.
    * @param {Object} config - Installation configuration
@@ -1270,11 +1346,6 @@ class Installer {
       }
     }
 
-    for (const moduleName of skippedModules) {
-      if (quickModules.collectedConfig[moduleName] || !quickModules.existingConfig?.[moduleName]) continue;
-      quickModules.collectedConfig[moduleName] = { ...quickModules.existingConfig[moduleName] };
-    }
-
     if (!promptedForNewFields) {
       await prompts.log.success('All configuration is up to date, no new options to configure');
     }

tools/installer/core/manifest-generator.js

@@ -435,14 +435,12 @@ class ManifestGenerator {
     // this means user-scoped keys (e.g. user_name) could mis-file into the
     // team config, so the operator should notice.
     const scopeByModuleKey = {};
-    const unresolvedModuleNames = new Set();
     // Maps installer moduleName (may be full display name) → module code field
     // from module.yaml, so TOML sections use [modules.<code>] not [modules.<name>].
     const codeByModuleName = {};
     for (const moduleName of this.updatedModules) {
       const moduleYamlPath = await resolveInstalledModuleYaml(moduleName);
       if (!moduleYamlPath) {
-        unresolvedModuleNames.add(moduleName);
         console.warn(
           `[warn] writeCentralConfig: could not locate module.yaml for '${moduleName}'. ` +
             `Answers from this module will default to team scope — user-scoped keys may mis-file into config.toml.`,
@@ -460,7 +458,6 @@ class ManifestGenerator {
           }
         }
       } catch (error) {
-        unresolvedModuleNames.add(moduleName);
         console.warn(
           `[warn] writeCentralConfig: could not parse module.yaml for '${moduleName}' (${error.message}). ` +
             `Answers from this module will default to team scope — user-scoped keys may mis-file into config.toml.`,
@@ -547,35 +544,9 @@ class ManifestGenerator {
       userLines.push('');
     }
 
-    const updatedModuleSet = new Set(this.updatedModules);
-    const preservedModuleBlocks = { team: [], user: [] };
-    const preservedModuleCodes = new Set();
-    if (updatedModuleSet.size > 0) {
-      const collectPreservedModuleBlocks = async (filePath, target) => {
-        if (!(await fs.pathExists(filePath))) return;
-        try {
-          const prev = await fs.readFile(filePath, 'utf8');
-          for (const block of extractModuleBlocks(prev)) {
-            if (!updatedModuleSet.has(block.code)) continue;
-            if (!unresolvedModuleNames.has(block.code) && moduleConfigs[block.code] && Object.keys(moduleConfigs[block.code]).length > 0)
-              continue;
-            preservedModuleCodes.add(block.code);
-            target.push(block.body);
-          }
-        } catch (error) {
-          console.warn(
-            `[warn] writeCentralConfig: could not read prior ${path.basename(filePath)} to preserve module config: ${error.message}`,
-          );
-        }
-      };
-      await collectPreservedModuleBlocks(teamPath, preservedModuleBlocks.team);
-      await collectPreservedModuleBlocks(userPath, preservedModuleBlocks.user);
-    }
-
     // [modules.<code>] — split per module
     for (const moduleName of this.updatedModules) {
       if (moduleName === 'core') continue;
-      if (unresolvedModuleNames.has(moduleName) && preservedModuleCodes.has(moduleName)) continue;
       const cfg = moduleConfigs[moduleName];
       if (!cfg || Object.keys(cfg).length === 0) continue;
       // Use the module's code field from module.yaml as the TOML key so the
@@ -603,13 +574,6 @@ class ManifestGenerator {
       }
     }
 
-    for (const body of preservedModuleBlocks.team) {
-      teamLines.push(body, '');
-    }
-    for (const body of preservedModuleBlocks.user) {
-      userLines.push(body, '');
-    }
-
     // [agents.<code>] — always team (agent roster is organizational).
     // Freshly collected agents come from module.yaml this run. If a module
     // was preserved (e.g. during quickUpdate when its source isn't available),
@@ -892,26 +856,4 @@ function extractAgentBlocks(tomlContent) {
   return blocks;
 }
 
-function extractModuleBlocks(tomlContent) {
-  const lines = tomlContent.split(/\r?\n/);
-  const blocks = [];
-  for (let i = 0; i < lines.length; i++) {
-    const startMatch = lines[i].match(/^\[modules\.([^\]]+)\]\s*$/);
-    if (!startMatch) continue;
-    const code = startMatch[1];
-    const bodyLines = [lines[i]];
-    i++;
-    while (i < lines.length && !/^\[[^\]]+\]\s*$/.test(lines[i])) {
-      bodyLines.push(lines[i]);
-      i++;
-    }
-    i--;
-    while (bodyLines.length > 0 && bodyLines.at(-1).trim() === '') {
-      bodyLines.pop();
-    }
-    blocks.push({ code, body: bodyLines.join('\n') });
-  }
-  return blocks;
-}
-
 module.exports = { ManifestGenerator };

tools/installer/ide/_config-driven.js

@@ -501,7 +501,7 @@ class ConfigDrivenIdeSetup {
 
     // Build removal set: previously installed skills + removals.txt entries
     let removalSet;
-    if (options.previousSkillIds && options.previousSkillIds.size > 0) {
+    if (options.previousSkillIds) {
       // Install/update flow: use pre-captured skill IDs (before manifest was overwritten)
       removalSet = new Set(options.previousSkillIds);
       if (resolvedBmadDir) {
@@ -547,7 +547,7 @@ class ConfigDrivenIdeSetup {
       // previousSkillIds — full uninstall or per-IDE removal via
       // cleanupByList), don't spare anything; the IDE itself is going away,
       // so its pointers should go with it.
-      const isInstallFlow = options.previousSkillIds && options.previousSkillIds.size > 0;
+      const isInstallFlow = !!options.previousSkillIds;
       const activeSkillIds = isInstallFlow ? await this._readActiveSkillIds(resolvedBmadDir) : new Set();
       const extension = this.installerConfig.commands_extension || '.md';
       await this.cleanupCommandPointers(

tools/installer/ui.js

@@ -7,7 +7,6 @@ const { CLIUtils } = require('./cli-utils');
 const { ExternalModuleManager } = require('./modules/external-manager');
 const { resolveModuleVersion } = require('./modules/version-resolver');
 const { Manifest } = require('./core/manifest');
-const { resolveInstalledModuleYaml } = require('./project-root');
 const {
   parseChannelOptions,
   buildPlan,
@@ -111,21 +110,42 @@ async function getModuleVersion(moduleCode, { repoUrl = null, registryDefault =
  * UI utilities for the installer
  */
 class UI {
-  async _findUnavailableInstalledModules(installedModuleIds = new Set(), bmadDir) {
+  async _retainUnavailableInstalledModules(selectedModules, installedModuleIds, bmadDir, options = {}) {
-    const unavailable = [];
+    const { OfficialModules } = require('./modules/official-modules');
-    for (const moduleId of installedModuleIds) {
+    const officialCodes = new Set(['core']);
-      if (moduleId === 'core' || moduleId === 'bmm') continue;
-      const moduleYamlPath = await resolveInstalledModuleYaml(moduleId);
-      if (moduleYamlPath) continue;
 
-      const { CustomModuleManager } = require('./modules/custom-module-manager');
+    const builtInModules = (await new OfficialModules().listAvailable()).modules || [];
-      const customMgr = new CustomModuleManager();
+    for (const mod of builtInModules) {
-      const localSource = await customMgr.findModuleSourceByCode(moduleId, { bmadDir });
+      officialCodes.add(mod.id);
-      if (localSource) continue;
-
-      unavailable.push(moduleId);
     }
-    return unavailable;
+
+    const externalManager = new ExternalModuleManager();
+    const registryModules = await externalManager.listAvailable();
+    for (const mod of registryModules) {
+      officialCodes.add(mod.code);
+    }
+
+    const { CustomModuleManager } = require('./modules/custom-module-manager');
+    const customMgr = new CustomModuleManager();
+    const selectedSet = new Set(selectedModules);
+    const preserveModules = [];
+
+    for (const moduleId of installedModuleIds) {
+      if (moduleId === 'core') continue;
+      if (!selectedSet.has(moduleId) && !options.preserveUnselected) continue;
+      if (officialCodes.has(moduleId)) continue;
+
+      const customSource = await customMgr.findModuleSourceByCode(moduleId, { bmadDir });
+      if (!customSource) {
+        preserveModules.push(moduleId);
+      }
+    }
+
+    const preservedSet = new Set(preserveModules);
+    return {
+      selectedModules: selectedModules.filter((moduleId) => !preservedSet.has(moduleId)),
+      preserveModules,
+    };
   }
 
   /**
@@ -291,12 +311,15 @@ class UI {
           selectedModules.unshift('core');
         }
 
-        const preservedModules = await this._findUnavailableInstalledModules(installedModuleIds, bmadDir);
+        const retainedModuleResult = await this._retainUnavailableInstalledModules(selectedModules, installedModuleIds, bmadDir, {
+          preserveUnselected: options.yes && !options.modules,
+        });
+        selectedModules = retainedModuleResult.selectedModules;
+        const preservedModules = retainedModuleResult.preserveModules;
+
         if (preservedModules.length > 0) {
-          const preservedSet = new Set(preservedModules);
-          selectedModules = selectedModules.filter((moduleName) => !preservedSet.has(moduleName));
           await prompts.log.warn(
-            `Retaining ${preservedModules.length} installed module(s) with no source available: ${preservedModules.join(', ')}`,
+            `Retaining ${preservedModules.length} installed module(s) with no available source: ${preservedModules.join(', ')}`,
           );
         }
 
@@ -319,14 +342,6 @@ class UI {
           ...options,
           channelOptions,
         });
-        if (preservedModules.length > 0) {
-          const preservedConfigLoader = new (require('./modules/official-modules').OfficialModules)({ channelOptions });
-          await preservedConfigLoader.loadExistingConfig(confirmedDirectory);
-          for (const moduleName of preservedModules) {
-            if (moduleConfigs[moduleName] || !preservedConfigLoader.existingConfig?.[moduleName]) continue;
-            moduleConfigs[moduleName] = { ...preservedConfigLoader.existingConfig[moduleName] };
-          }
-        }
 
         // Warn about --pin/--next flags that refer to modules the user didn't
         // select, or that target bundled modules (core/bmm) where channel