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
parent
75295dd989
commit
928c585b04
|
|
@ -14,7 +14,9 @@
|
|||
const path = require('node:path');
|
||||
const os = require('node:os');
|
||||
const fs = require('fs-extra');
|
||||
const { Installer } = require('../tools/installer/core/installer');
|
||||
const { ManifestGenerator } = require('../tools/installer/core/manifest-generator');
|
||||
const { OfficialModules } = require('../tools/installer/modules/official-modules');
|
||||
const { IdeManager } = require('../tools/installer/ide/manager');
|
||||
const { clearCache, loadPlatformCodes } = require('../tools/installer/ide/platform-codes');
|
||||
|
||||
|
|
@ -130,13 +132,16 @@ 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'),
|
||||
|
|
@ -163,14 +168,14 @@ async function createCustomModuleManifestFixture() {
|
|||
'customModules:',
|
||||
' - id: test-module',
|
||||
' name: "Test Module"',
|
||||
' sourcePath: "/tmp/test-module-source"',
|
||||
` sourcePath: ${JSON.stringify(moduleSourceDir)}`,
|
||||
'ides:',
|
||||
' - codex',
|
||||
'',
|
||||
].join('\n'),
|
||||
);
|
||||
|
||||
return { root: fixtureRoot, bmadDir, manifestPath: path.join(configDir, 'manifest.yaml') };
|
||||
return { root: fixtureRoot, bmadDir, manifestPath: path.join(configDir, 'manifest.yaml'), moduleSourceDir };
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -1411,8 +1416,8 @@ async function runTests() {
|
|||
} catch (error) {
|
||||
assert(false, 'Pi native skills test succeeds', error.message);
|
||||
} finally {
|
||||
if (tempProjectDir28) await fs.remove(tempProjectDir28).catch(() => { });
|
||||
if (installedBmadDir28) await fs.remove(path.dirname(installedBmadDir28)).catch(() => { });
|
||||
if (tempProjectDir28) await fs.remove(tempProjectDir28).catch(() => {});
|
||||
if (installedBmadDir28) await fs.remove(path.dirname(installedBmadDir28)).catch(() => {});
|
||||
}
|
||||
|
||||
console.log('');
|
||||
|
|
@ -1559,7 +1564,7 @@ async function runTests() {
|
|||
} catch (error) {
|
||||
assert(false, 'Unified skill scanner test succeeds', error.message);
|
||||
} finally {
|
||||
if (tempFixture29) await fs.remove(tempFixture29).catch(() => { });
|
||||
if (tempFixture29) await fs.remove(tempFixture29).catch(() => {});
|
||||
}
|
||||
|
||||
console.log('');
|
||||
|
|
@ -1626,7 +1631,7 @@ async function runTests() {
|
|||
} catch (error) {
|
||||
assert(false, 'parseSkillMd validation test succeeds', error.message);
|
||||
} finally {
|
||||
if (tempFixture30) await fs.remove(tempFixture30).catch(() => { });
|
||||
if (tempFixture30) await fs.remove(tempFixture30).catch(() => {});
|
||||
}
|
||||
|
||||
console.log('');
|
||||
|
|
@ -1663,8 +1668,8 @@ async function runTests() {
|
|||
} catch (error) {
|
||||
assert(false, 'Skill-format unique count test succeeds', error.message);
|
||||
} finally {
|
||||
if (collisionProjectDir) await fs.remove(collisionProjectDir).catch(() => { });
|
||||
if (collisionFixtureRoot) await fs.remove(collisionFixtureRoot).catch(() => { });
|
||||
if (collisionProjectDir) await fs.remove(collisionProjectDir).catch(() => {});
|
||||
if (collisionFixtureRoot) await fs.remove(collisionFixtureRoot).catch(() => {});
|
||||
}
|
||||
|
||||
console.log('');
|
||||
|
|
@ -1754,37 +1759,109 @@ async function runTests() {
|
|||
} catch (error) {
|
||||
assert(false, 'Ona native skills test succeeds', error.message);
|
||||
} finally {
|
||||
if (tempProjectDir32) await fs.remove(tempProjectDir32).catch(() => { });
|
||||
if (installedBmadDir32) await fs.remove(path.dirname(installedBmadDir32)).catch(() => { });
|
||||
if (tempProjectDir32) await fs.remove(tempProjectDir32).catch(() => {});
|
||||
if (installedBmadDir32) await fs.remove(path.dirname(installedBmadDir32)).catch(() => {});
|
||||
}
|
||||
|
||||
console.log('');
|
||||
|
||||
// ============================================================
|
||||
// Suite 33: Main manifest preserves customModules
|
||||
// Suite 33: Main manifest preserves active customModules only
|
||||
// ============================================================
|
||||
console.log(`${colors.yellow}Test Suite 33: Preserve customModules in main manifest${colors.reset}\n`);
|
||||
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 yaml = require('yaml');
|
||||
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 === '/tmp/test-module-source',
|
||||
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(() => { });
|
||||
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('');
|
||||
|
|
|
|||
|
|
@ -1144,86 +1144,12 @@ class Installer {
|
|||
const configuredIdes = existingInstall.ides;
|
||||
const projectRoot = path.dirname(bmadDir);
|
||||
|
||||
// Get custom module sources: first from manifest, then from --custom-content, then from cache
|
||||
const customModuleSources = new Map();
|
||||
if (existingInstall.customModules) {
|
||||
for (const customModule of existingInstall.customModules) {
|
||||
if (!customModule?.id) continue;
|
||||
|
||||
let sourcePath = customModule.sourcePath;
|
||||
if (sourcePath && sourcePath.startsWith('_config')) {
|
||||
sourcePath = path.join(bmadDir, sourcePath);
|
||||
} else if (!sourcePath && customModule.relativePath) {
|
||||
sourcePath = path.resolve(projectRoot, customModule.relativePath);
|
||||
} else if (sourcePath && !path.isAbsolute(sourcePath)) {
|
||||
sourcePath = path.resolve(sourcePath);
|
||||
}
|
||||
|
||||
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 && (await fs.pathExists(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)) {
|
||||
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,
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
const customModuleSources = await this.customModules.assembleQuickUpdateSources(
|
||||
config,
|
||||
existingInstall,
|
||||
bmadDir,
|
||||
this.externalModuleManager,
|
||||
);
|
||||
|
||||
// Get available modules (what we have source for)
|
||||
const availableModulesData = await new OfficialModules().listAvailable();
|
||||
|
|
|
|||
|
|
@ -377,6 +377,7 @@ class ManifestGenerator {
|
|||
*/
|
||||
async writeMainManifest(cfgDir) {
|
||||
const manifestPath = path.join(cfgDir, 'manifest.yaml');
|
||||
const installedModuleSet = new Set(this.modules);
|
||||
|
||||
// Read existing manifest to preserve install date
|
||||
let existingInstallDate = null;
|
||||
|
|
@ -405,7 +406,9 @@ class ManifestGenerator {
|
|||
}
|
||||
|
||||
if (existingManifest.customModules && Array.isArray(existingManifest.customModules)) {
|
||||
existingCustomModules = 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 {
|
||||
// If we can't read existing manifest, continue with defaults
|
||||
|
|
|
|||
|
|
@ -192,6 +192,111 @@ class CustomModules {
|
|||
|
||||
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 };
|
||||
|
|
|
|||
Loading…
Reference in New Issue