fix(installer): preserve stale installed modules during update (#2391)

* fix(installer): preserve stale installed modules on update

* test: drop stale baut regression case

* fix(installer): preserve source-backed modules and configs

* fix(installer): retain preserved module config in quick update

* fix(installer): preserve module config blocks for retained modules

* fix(installer): preserve user-scope blocks for retained modules

* fix(installer): retain stale modules during updates
This commit is contained in:
Dicky Moore 2026-05-18 14:39:11 +01:00 committed by GitHub
parent 0eae7c4352
commit a08522631b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 153 additions and 25 deletions

View File

@ -54,7 +54,7 @@ class Installer {
} }
if (existingInstall.installed) { if (existingInstall.installed) {
await this._removeDeselectedModules(existingInstall, config, paths); await this._removeDeselectedModules(existingInstall, config, paths, originalConfig._preserveModules || []);
updateState = await this._prepareUpdateState(paths, config, existingInstall, officialModules); updateState = await this._prepareUpdateState(paths, config, existingInstall, officialModules);
await this._removeDeselectedIdes(existingInstall, config, paths); await this._removeDeselectedIdes(existingInstall, config, paths);
} }
@ -76,25 +76,23 @@ class Installer {
const results = []; const results = [];
const addResult = (step, status, detail = '', meta = {}) => results.push({ step, status, detail, ...meta }); 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 || []; 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); await this._setupIdes(config, allModules, paths, addResult, previousSkillIds);
@ -144,10 +142,11 @@ class Installer {
* Remove modules that were previously installed but are no longer selected. * Remove modules that were previously installed but are no longer selected.
* No confirmation the user's module selection is the decision. * No confirmation the user's module selection is the decision.
*/ */
async _removeDeselectedModules(existingInstall, config, paths) { async _removeDeselectedModules(existingInstall, config, paths, preservedModules = []) {
const previouslyInstalled = new Set(existingInstall.moduleIds); const previouslyInstalled = new Set(existingInstall.moduleIds);
const newlySelected = new Set(config.modules || []); const newlySelected = new Set(config.modules || []);
const toRemove = [...previouslyInstalled].filter((m) => !newlySelected.has(m) && m !== 'core'); const preserved = new Set(preservedModules);
const toRemove = [...previouslyInstalled].filter((m) => !newlySelected.has(m) && m !== 'core' && !preserved.has(m));
for (const moduleId of toRemove) { for (const moduleId of toRemove) {
const modulePath = paths.moduleDir(moduleId); const modulePath = paths.moduleDir(moduleId);
@ -212,7 +211,16 @@ class Installer {
/** /**
* Install modules, create directories, generate configs and manifests. * 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 isQuickUpdate = config.isQuickUpdate();
const moduleConfigs = officialModules.moduleConfigs; const moduleConfigs = officialModules.moduleConfigs;
@ -291,25 +299,29 @@ class Installer {
message('Generating manifests...'); message('Generating manifests...');
const manifestGen = new ManifestGenerator(); const manifestGen = new ManifestGenerator();
const preservedModules = originalConfig._preserveModules || [];
const allModulesForManifest = config.isQuickUpdate() const allModulesForManifest = config.isQuickUpdate()
? originalConfig._existingModules || allModules || [] ? originalConfig._existingModules || allModules || []
: originalConfig._preserveModules : preservedModules.length > 0
? [...allModules, ...originalConfig._preserveModules] ? [...allModules, ...preservedModules]
: allModules || []; : allModules || [];
let modulesForCsvPreserve; let modulesForCsvPreserve;
if (config.isQuickUpdate()) { if (config.isQuickUpdate()) {
modulesForCsvPreserve = originalConfig._existingModules || allModules || []; modulesForCsvPreserve = originalConfig._existingModules || allModules || [];
} else { } 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], { await manifestGen.generateManifests(paths.bmadDir, allModulesForManifest, [...this.installedFiles], {
ides: config.ides || [], ides: config.ides || [],
preservedModules: modulesForCsvPreserve, preservedModules: modulesForCsvPreserve,
moduleConfigs, moduleConfigs,
}); });
await this._appendPreservedSkillManifestRows(paths.bmadDir, previousSkillManifestRows, preservedModules);
// Apply post-install --set TOML patches. Runs after writeCentralConfig // Apply post-install --set TOML patches. Runs after writeCentralConfig
// (inside generateManifests above) so the patch operates on the // (inside generateManifests above) so the patch operates on the
@ -411,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. * Restore custom and modified files that were backed up before the update.
* No-op for fresh installs (updateState is null). * No-op for fresh installs (updateState is null).
@ -597,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. * Install official (non-custom) modules.
* @param {Object} config - Installation configuration * @param {Object} config - Installation configuration

View File

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

View File

@ -110,6 +110,44 @@ async function getModuleVersion(moduleCode, { repoUrl = null, registryDefault =
* UI utilities for the installer * UI utilities for the installer
*/ */
class UI { class UI {
async _retainUnavailableInstalledModules(selectedModules, installedModuleIds, bmadDir, options = {}) {
const { OfficialModules } = require('./modules/official-modules');
const officialCodes = new Set(['core']);
const builtInModules = (await new OfficialModules().listAvailable()).modules || [];
for (const mod of builtInModules) {
officialCodes.add(mod.id);
}
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,
};
}
/** /**
* Prompt for installation configuration * Prompt for installation configuration
* @param {Object} options - Command-line options from install command * @param {Object} options - Command-line options from install command
@ -273,6 +311,18 @@ class UI {
selectedModules.unshift('core'); selectedModules.unshift('core');
} }
const retainedModuleResult = await this._retainUnavailableInstalledModules(selectedModules, installedModuleIds, bmadDir, {
preserveUnselected: options.yes && !options.modules,
});
selectedModules = retainedModuleResult.selectedModules;
const preservedModules = retainedModuleResult.preserveModules;
if (preservedModules.length > 0) {
await prompts.log.warn(
`Retaining ${preservedModules.length} installed module(s) with no available source: ${preservedModules.join(', ')}`,
);
}
// For existing installs, resolve per-module update decisions BEFORE // For existing installs, resolve per-module update decisions BEFORE
// we clone anything. Reads the existing manifest's recorded channel // we clone anything. Reads the existing manifest's recorded channel
// per module and prompts the user on available upgrades (patch/minor // per module and prompts the user on available upgrades (patch/minor
@ -317,6 +367,7 @@ class UI {
setOverrides, setOverrides,
skipPrompts: options.yes || false, skipPrompts: options.yes || false,
channelOptions, channelOptions,
_preserveModules: preservedModules,
}; };
} }
} }