fix: preserve local custom module sources during quick update (#2172)

* fix: preserve local custom module sources during quick update

Keep customModules in the generated main manifest so local custom
module source paths survive update runs. Load those preserved source
paths during stock quick update before falling back to the custom
cache directory.

This fixes the case where BMAD would drop customModules, lose the
original source path for a local module, and then skip the module or
try to re-cache from _bmad/_config/custom/<module>, which could fail
with ENOENT after the cache directory was removed.

Also adds an installation component regression test to verify
customModules and sourcePath are preserved in manifest generation.

Fixes #1582

* fix: ensure consistent formatting

* refactor: extract quick update custom source assembly

Move quick-update custom module source collection out of Installer and into
CustomModules as assembleQuickUpdateSources(). This keeps discoverPaths() focused on consuming prepared install inputs while
making the quick-update source assembly step explicit and easier to evolve.

Also:
- preserve customModules metadata in manifest regeneration for installed modules
- drop stale customModules entries when modules are no longer installed
- cover manifest preservation and manifest-backed quick-update sources in tests
This commit is contained in:
Taras Romaniv 2026-03-31 02:49:05 +02:00 committed by GitHub
parent 2302d9cdc5
commit 1f99eb0496
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 273 additions and 53 deletions

View File

@ -14,7 +14,9 @@
const path = require('node:path'); const path = require('node:path');
const os = require('node:os'); const os = require('node:os');
const fs = require('fs-extra'); const fs = require('fs-extra');
const { Installer } = require('../tools/installer/core/installer');
const { ManifestGenerator } = require('../tools/installer/core/manifest-generator'); const { ManifestGenerator } = require('../tools/installer/core/manifest-generator');
const { OfficialModules } = require('../tools/installer/modules/official-modules');
const { IdeManager } = require('../tools/installer/ide/manager'); const { IdeManager } = require('../tools/installer/ide/manager');
const { clearCache, loadPlatformCodes } = require('../tools/installer/ide/platform-codes'); const { clearCache, loadPlatformCodes } = require('../tools/installer/ide/platform-codes');
@ -126,6 +128,56 @@ async function createSkillCollisionFixture() {
return { root: fixtureRoot, bmadDir: fixtureDir }; return { root: fixtureRoot, bmadDir: fixtureDir };
} }
async function createCustomModuleManifestFixture() {
const fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-custom-manifest-'));
const bmadDir = path.join(fixtureRoot, '_bmad');
const configDir = path.join(bmadDir, '_config');
const moduleSourceDir = path.join(fixtureRoot, 'test-module-source');
await fs.ensureDir(configDir);
await fs.ensureDir(moduleSourceDir);
const minimalAgent = '<agent name="Test" title="T"><persona>p</persona></agent>';
await fs.ensureDir(path.join(bmadDir, 'core', 'agents'));
await fs.writeFile(path.join(bmadDir, 'core', 'agents', 'test.md'), minimalAgent);
await fs.ensureDir(path.join(bmadDir, 'test-module', 'agents'));
await fs.writeFile(path.join(bmadDir, 'test-module', 'agents', 'test.md'), minimalAgent);
await fs.writeFile(path.join(moduleSourceDir, 'module.yaml'), ['code: test-module', 'name: Test Module', ''].join('\n'));
await fs.writeFile(
path.join(configDir, 'manifest.yaml'),
[
'installation:',
' version: 6.2.2',
' installDate: 2026-03-30T00:00:00.000Z',
' lastUpdated: 2026-03-30T00:00:00.000Z',
'modules:',
' - name: core',
' version: 6.2.2',
' installDate: 2026-03-30T00:00:00.000Z',
' lastUpdated: 2026-03-30T00:00:00.000Z',
' source: built-in',
' npmPackage: null',
' repoUrl: null',
' - name: test-module',
' version: null',
' installDate: 2026-03-30T00:00:00.000Z',
' lastUpdated: 2026-03-30T00:00:00.000Z',
' source: custom',
' npmPackage: null',
' repoUrl: null',
'customModules:',
' - id: test-module',
' name: "Test Module"',
` sourcePath: ${JSON.stringify(moduleSourceDir)}`,
'ides:',
' - codex',
'',
].join('\n'),
);
return { root: fixtureRoot, bmadDir, manifestPath: path.join(configDir, 'manifest.yaml'), moduleSourceDir };
}
/** /**
* Test Suite * Test Suite
*/ */
@ -1713,6 +1765,107 @@ async function runTests() {
console.log(''); console.log('');
// ============================================================
// Suite 33: Main manifest preserves active customModules only
// ============================================================
console.log(`${colors.yellow}Test Suite 33: Preserve active customModules in main manifest${colors.reset}\n`);
let customManifestFixture = null;
try {
customManifestFixture = await createCustomModuleManifestFixture();
const yaml = require('yaml');
const originalManifest = yaml.parse(await fs.readFile(customManifestFixture.manifestPath, 'utf8'));
originalManifest.customModules.push({
id: 'removed-module',
name: 'Removed Module',
sourcePath: path.join(customManifestFixture.root, 'removed-module-source'),
});
await fs.writeFile(customManifestFixture.manifestPath, yaml.stringify(originalManifest), 'utf8');
const generator33 = new ManifestGenerator();
await generator33.generateManifests(customManifestFixture.bmadDir, ['core', 'test-module'], [], { ides: ['codex'] });
const updatedManifest = yaml.parse(await fs.readFile(customManifestFixture.manifestPath, 'utf8'));
const customModule = updatedManifest.customModules?.find((entry) => entry.id === 'test-module');
assert(Array.isArray(updatedManifest.customModules), 'Main manifest keeps customModules array');
assert(customModule !== undefined, 'Main manifest preserves existing custom module entry');
assert(
customModule && customModule.sourcePath === customManifestFixture.moduleSourceDir,
'Main manifest preserves custom module sourcePath',
);
assert(
!updatedManifest.customModules?.some((entry) => entry.id === 'removed-module'),
'Main manifest drops stale custom module entries',
);
} catch (error) {
assert(false, 'Main manifest preserves customModules test succeeds', error.message);
} finally {
if (customManifestFixture?.root) await fs.remove(customManifestFixture.root).catch(() => {});
}
console.log('');
// ============================================================
// Suite 34: Quick update uses manifest-backed custom sources
// ============================================================
console.log(`${colors.yellow}Test Suite 34: Quick update uses manifest-backed custom module sources${colors.reset}\n`);
let quickUpdateFixture = null;
const originalListAvailable34 = OfficialModules.prototype.listAvailable;
const originalLoadExistingConfig34 = OfficialModules.prototype.loadExistingConfig;
const originalCollectModuleConfigQuick34 = OfficialModules.prototype.collectModuleConfigQuick;
try {
quickUpdateFixture = await createCustomModuleManifestFixture();
const installer34 = new Installer();
installer34.externalModuleManager.hasModule = async () => false;
installer34.externalModuleManager.listAvailable = async () => [];
let capturedInstallConfig34 = null;
installer34.install = async (config) => {
capturedInstallConfig34 = config;
return { success: true };
};
OfficialModules.prototype.listAvailable = async function () {
return { modules: [], customModules: [] };
};
OfficialModules.prototype.loadExistingConfig = async function () {
this.collectedConfig = this.collectedConfig || {};
};
OfficialModules.prototype.collectModuleConfigQuick = async function (moduleName) {
this.collectedConfig = this.collectedConfig || {};
if (!this.collectedConfig[moduleName]) {
this.collectedConfig[moduleName] = {};
}
return false;
};
await installer34.quickUpdate({
directory: quickUpdateFixture.root,
skipPrompts: true,
});
const customModule34 = capturedInstallConfig34?._customModuleSources?.get('test-module');
assert(capturedInstallConfig34 !== null, 'Quick update forwards config to install');
assert(customModule34 !== undefined, 'Quick update keeps manifest-backed custom module updateable');
assert(customModule34 && customModule34.cached === false, 'Quick update uses manifest-backed source before cache');
assert(
customModule34 && customModule34.sourcePath === quickUpdateFixture.moduleSourceDir,
'Quick update uses preserved manifest sourcePath for custom modules',
);
} catch (error) {
assert(false, 'Quick update manifest-backed custom source test succeeds', error.message);
} finally {
OfficialModules.prototype.listAvailable = originalListAvailable34;
OfficialModules.prototype.loadExistingConfig = originalLoadExistingConfig34;
OfficialModules.prototype.collectModuleConfigQuick = originalCollectModuleConfigQuick34;
if (quickUpdateFixture?.root) await fs.remove(quickUpdateFixture.root).catch(() => {});
}
console.log('');
// ============================================================ // ============================================================
// Summary // Summary
// ============================================================ // ============================================================

View File

@ -1144,59 +1144,12 @@ class Installer {
const configuredIdes = existingInstall.ides; const configuredIdes = existingInstall.ides;
const projectRoot = path.dirname(bmadDir); const projectRoot = path.dirname(bmadDir);
// Get custom module sources: first from --custom-content (re-cache from source), then from cache const customModuleSources = await this.customModules.assembleQuickUpdateSources(
const customModuleSources = new Map(); config,
if (config.customContent?.sources?.length > 0) { existingInstall,
for (const source of config.customContent.sources) { bmadDir,
if (source.id && source.path && (await fs.pathExists(source.path))) { this.externalModuleManager,
customModuleSources.set(source.id, { );
id: source.id,
name: source.name || source.id,
sourcePath: source.path,
cached: false, // From CLI, will be re-cached
});
}
}
}
const cacheDir = path.join(bmadDir, '_config', 'custom');
if (await fs.pathExists(cacheDir)) {
const cachedModules = await fs.readdir(cacheDir, { withFileTypes: true });
for (const cachedModule of cachedModules) {
const moduleId = cachedModule.name;
const cachedPath = path.join(cacheDir, moduleId);
// Skip if path doesn't exist (broken symlink, deleted dir) - avoids lstat ENOENT
if (!(await fs.pathExists(cachedPath))) {
continue;
}
if (!cachedModule.isDirectory()) {
continue;
}
// Skip if we already have this module from manifest
if (customModuleSources.has(moduleId)) {
continue;
}
// Check if this is an external official module - skip cache for those
const isExternal = await this.externalModuleManager.hasModule(moduleId);
if (isExternal) {
continue;
}
// Check if this is actually a custom module (has module.yaml)
const moduleYamlPath = path.join(cachedPath, 'module.yaml');
if (await fs.pathExists(moduleYamlPath)) {
customModuleSources.set(moduleId, {
id: moduleId,
name: moduleId,
sourcePath: cachedPath,
cached: true,
});
}
}
}
// Get available modules (what we have source for) // Get available modules (what we have source for)
const availableModulesData = await new OfficialModules().listAvailable(); const availableModulesData = await new OfficialModules().listAvailable();

View File

@ -377,10 +377,12 @@ class ManifestGenerator {
*/ */
async writeMainManifest(cfgDir) { async writeMainManifest(cfgDir) {
const manifestPath = path.join(cfgDir, 'manifest.yaml'); const manifestPath = path.join(cfgDir, 'manifest.yaml');
const installedModuleSet = new Set(this.modules);
// Read existing manifest to preserve install date // Read existing manifest to preserve install date
let existingInstallDate = null; let existingInstallDate = null;
const existingModulesMap = new Map(); const existingModulesMap = new Map();
let existingCustomModules = [];
if (await fs.pathExists(manifestPath)) { if (await fs.pathExists(manifestPath)) {
try { try {
@ -402,6 +404,12 @@ class ManifestGenerator {
} }
} }
} }
if (existingManifest.customModules && Array.isArray(existingManifest.customModules)) {
// We filter here so manifest regeneration preserves source metadata only for custom modules that
// are still installed. Without that, customModules can retain stale entries for modules that were removed.
existingCustomModules = existingManifest.customModules.filter((customModule) => installedModuleSet.has(customModule?.id));
}
} catch { } catch {
// If we can't read existing manifest, continue with defaults // If we can't read existing manifest, continue with defaults
} }
@ -437,6 +445,7 @@ class ManifestGenerator {
lastUpdated: new Date().toISOString(), lastUpdated: new Date().toISOString(),
}, },
modules: updatedModules, modules: updatedModules,
customModules: existingCustomModules,
ides: this.selectedIdes, ides: this.selectedIdes,
}; };

View File

@ -192,6 +192,111 @@ class CustomModules {
return this.paths; return this.paths;
} }
/**
* Assemble quick-update source candidates before install() hands them to discoverPaths().
* This exists because discoverPaths() consumes already-prepared quick-update sources,
* while quickUpdate() still has to build that source map from manifest, explicit inputs,
* and cache conventions.
* Precedence: manifest-backed paths, explicit sources override them, then cached modules.
* @param {Object} config - Quick update configuration
* @param {Object} existingInstall - Existing installation snapshot
* @param {string} bmadDir - BMAD directory
* @param {Object} externalModuleManager - External module manager
* @returns {Promise<Map<string, Object>>} Map of custom module ID to source info
*/
async assembleQuickUpdateSources(config, existingInstall, bmadDir, externalModuleManager) {
const projectRoot = path.dirname(bmadDir);
const customModuleSources = new Map();
if (existingInstall.customModules) {
for (const customModule of existingInstall.customModules) {
// Skip if no ID - can't reliably track or re-cache without it
if (!customModule?.id) continue;
let sourcePath = customModule.sourcePath;
if (sourcePath && sourcePath.startsWith('_config')) {
// Paths are relative to BMAD dir, but we want absolute paths for install
sourcePath = path.join(bmadDir, sourcePath);
} else if (!sourcePath && customModule.relativePath) {
// Fall back to relativePath
sourcePath = path.resolve(projectRoot, customModule.relativePath);
} else if (sourcePath && !path.isAbsolute(sourcePath)) {
// If we have a sourcePath but it's not absolute, resolve it relative to project root
sourcePath = path.resolve(projectRoot, sourcePath);
}
// If we still don't have a valid source path, skip this module
if (!sourcePath || !(await fs.pathExists(sourcePath))) {
continue;
}
customModuleSources.set(customModule.id, {
id: customModule.id,
name: customModule.name || customModule.id,
sourcePath,
relativePath: customModule.relativePath,
cached: false,
});
}
}
if (config.customContent?.sources?.length > 0) {
for (const source of config.customContent.sources) {
if (source.id && source.path) {
customModuleSources.set(source.id, {
id: source.id,
name: source.name || source.id,
sourcePath: source.path,
cached: false, // From CLI, will be re-cached
});
}
}
}
const cacheDir = path.join(bmadDir, '_config', 'custom');
if (!(await fs.pathExists(cacheDir))) {
return customModuleSources;
}
const cachedModules = await fs.readdir(cacheDir, { withFileTypes: true });
for (const cachedModule of cachedModules) {
const moduleId = cachedModule.name;
const cachedPath = path.join(cacheDir, moduleId);
// Skip if path doesn't exist (broken symlink, deleted dir) - avoids lstat ENOENT
if (!(await fs.pathExists(cachedPath))) {
continue;
}
if (!cachedModule.isDirectory()) {
continue;
}
// Skip if we already have this module from manifest
if (customModuleSources.has(moduleId)) {
continue;
}
// Check if this is an external official module - skip cache for those
const isExternal = await externalModuleManager.hasModule(moduleId);
if (isExternal) {
continue;
}
// Check if this is actually a custom module (has module.yaml)
const moduleYamlPath = path.join(cachedPath, 'module.yaml');
if (await fs.pathExists(moduleYamlPath)) {
customModuleSources.set(moduleId, {
id: moduleId,
name: moduleId,
sourcePath: cachedPath,
cached: true,
});
}
}
return customModuleSources;
}
} }
module.exports = { CustomModules }; module.exports = { CustomModules };