Skip to content

Commit 2395b0e

Browse files
fix: bmad tea instal version (#2298)
* fix: bmad tea instal version * fix: addressed review comments
1 parent 914c4ed commit 2395b0e

5 files changed

Lines changed: 642 additions & 143 deletions

File tree

test/test-installation-components.js

Lines changed: 269 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2355,6 +2355,275 @@ async function runTests() {
23552355

23562356
console.log('');
23572357

2358+
// ============================================================
2359+
// Test Suite 39: Module Version Resolution
2360+
// ============================================================
2361+
console.log(`${colors.yellow}Test Suite 39: Module Version Resolution${colors.reset}\n`);
2362+
2363+
// --- package.json beats module.yaml and marketplace.json for cached external modules ---
2364+
{
2365+
const { resolveModuleVersion } = require('../tools/installer/modules/version-resolver');
2366+
const tempCacheDir39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-version-cache-'));
2367+
const priorCacheEnv39 = process.env.BMAD_EXTERNAL_MODULES_CACHE;
2368+
process.env.BMAD_EXTERNAL_MODULES_CACHE = tempCacheDir39;
2369+
2370+
try {
2371+
const moduleRoot = path.join(tempCacheDir39, 'tea');
2372+
const moduleSrc = path.join(moduleRoot, 'src');
2373+
await fs.ensureDir(path.join(moduleRoot, '.claude-plugin'));
2374+
await fs.ensureDir(moduleSrc);
2375+
2376+
await fs.writeFile(
2377+
path.join(moduleRoot, 'package.json'),
2378+
JSON.stringify({ name: 'bmad-method-test-architecture-enterprise', version: '1.12.3' }, null, 2) + '\n',
2379+
);
2380+
await fs.writeFile(
2381+
path.join(moduleSrc, 'module.yaml'),
2382+
['code: tea', 'name: Test Architect', 'module_version: 1.11.0', ''].join('\n'),
2383+
);
2384+
await fs.writeFile(
2385+
path.join(moduleRoot, '.claude-plugin', 'marketplace.json'),
2386+
JSON.stringify({ plugins: [{ name: 'tea', version: '1.7.2' }] }, null, 2) + '\n',
2387+
);
2388+
2389+
const versionInfo = await resolveModuleVersion('tea');
2390+
assert(versionInfo.version === '1.12.3', 'resolver prefers cached package.json over stale marketplace metadata for external modules');
2391+
assert(versionInfo.source === 'package.json', 'resolver reports package.json as the winning metadata source');
2392+
} finally {
2393+
if (priorCacheEnv39 === undefined) {
2394+
delete process.env.BMAD_EXTERNAL_MODULES_CACHE;
2395+
} else {
2396+
process.env.BMAD_EXTERNAL_MODULES_CACHE = priorCacheEnv39;
2397+
}
2398+
await fs.remove(tempCacheDir39).catch(() => {});
2399+
}
2400+
}
2401+
2402+
// --- module.yaml is used when package.json is absent ---
2403+
{
2404+
const { resolveModuleVersion } = require('../tools/installer/modules/version-resolver');
2405+
const tempRepo39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-version-module-yaml-'));
2406+
const tempCacheDir39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-version-module-yaml-cache-'));
2407+
const priorCacheEnv39 = process.env.BMAD_EXTERNAL_MODULES_CACHE;
2408+
process.env.BMAD_EXTERNAL_MODULES_CACHE = tempCacheDir39;
2409+
2410+
try {
2411+
const moduleDir = path.join(tempRepo39, 'src');
2412+
await fs.ensureDir(path.join(tempRepo39, '.claude-plugin'));
2413+
await fs.ensureDir(moduleDir);
2414+
2415+
await fs.writeFile(path.join(moduleDir, 'module.yaml'), ['code: sample-mod', 'module_version: 2.4.0', ''].join('\n'));
2416+
await fs.writeFile(
2417+
path.join(tempRepo39, '.claude-plugin', 'marketplace.json'),
2418+
JSON.stringify({ plugins: [{ name: 'sample-mod', version: '1.7.2' }] }, null, 2) + '\n',
2419+
);
2420+
2421+
const versionInfo = await resolveModuleVersion('sample-mod', { moduleSourcePath: moduleDir });
2422+
assert(versionInfo.version === '2.4.0', 'resolver falls back to module.yaml when package.json is missing');
2423+
assert(versionInfo.source === 'module.yaml', 'resolver reports module.yaml when it provides the selected version');
2424+
} finally {
2425+
if (priorCacheEnv39 === undefined) {
2426+
delete process.env.BMAD_EXTERNAL_MODULES_CACHE;
2427+
} else {
2428+
process.env.BMAD_EXTERNAL_MODULES_CACHE = priorCacheEnv39;
2429+
}
2430+
await fs.remove(tempRepo39).catch(() => {});
2431+
await fs.remove(tempCacheDir39).catch(() => {});
2432+
}
2433+
}
2434+
2435+
// --- marketplace fallback uses semver-aware comparison ---
2436+
{
2437+
const { resolveModuleVersion } = require('../tools/installer/modules/version-resolver');
2438+
const tempRepo39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-version-marketplace-'));
2439+
const tempCacheDir39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-version-marketplace-cache-'));
2440+
const priorCacheEnv39 = process.env.BMAD_EXTERNAL_MODULES_CACHE;
2441+
process.env.BMAD_EXTERNAL_MODULES_CACHE = tempCacheDir39;
2442+
2443+
try {
2444+
const moduleDir = path.join(tempRepo39, 'src');
2445+
await fs.ensureDir(path.join(tempRepo39, '.claude-plugin'));
2446+
await fs.ensureDir(moduleDir);
2447+
2448+
await fs.writeFile(
2449+
path.join(tempRepo39, '.claude-plugin', 'marketplace.json'),
2450+
JSON.stringify(
2451+
{
2452+
plugins: [
2453+
{ name: 'older-plugin', version: '1.7.2' },
2454+
{ name: 'newer-plugin', version: '1.12.3' },
2455+
],
2456+
},
2457+
null,
2458+
2,
2459+
) + '\n',
2460+
);
2461+
2462+
const versionInfo = await resolveModuleVersion('missing-plugin', { moduleSourcePath: moduleDir });
2463+
assert(
2464+
versionInfo.version === '1.12.3',
2465+
'resolver picks the highest marketplace fallback version using semver instead of string comparison',
2466+
);
2467+
assert(versionInfo.source === 'marketplace.json', 'resolver reports marketplace.json when it is the only usable metadata source');
2468+
} finally {
2469+
if (priorCacheEnv39 === undefined) {
2470+
delete process.env.BMAD_EXTERNAL_MODULES_CACHE;
2471+
} else {
2472+
process.env.BMAD_EXTERNAL_MODULES_CACHE = priorCacheEnv39;
2473+
}
2474+
await fs.remove(tempRepo39).catch(() => {});
2475+
await fs.remove(tempCacheDir39).catch(() => {});
2476+
}
2477+
}
2478+
2479+
// --- package.json lookup must not escape the module repo boundary ---
2480+
{
2481+
const { resolveModuleVersion } = require('../tools/installer/modules/version-resolver');
2482+
const tempHost39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-version-boundary-host-'));
2483+
const tempCacheDir39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-version-boundary-cache-'));
2484+
const priorCacheEnv39 = process.env.BMAD_EXTERNAL_MODULES_CACHE;
2485+
process.env.BMAD_EXTERNAL_MODULES_CACHE = tempCacheDir39;
2486+
2487+
try {
2488+
const moduleRoot = path.join(tempHost39, 'nested-module');
2489+
const moduleDir = path.join(moduleRoot, 'src');
2490+
await fs.ensureDir(path.join(moduleRoot, '.claude-plugin'));
2491+
await fs.ensureDir(moduleDir);
2492+
2493+
await fs.writeFile(path.join(tempHost39, 'package.json'), JSON.stringify({ name: 'host-project', version: '9.9.9' }, null, 2) + '\n');
2494+
await fs.writeFile(path.join(moduleDir, 'module.yaml'), ['code: sample-mod', 'module_version: 2.4.0', ''].join('\n'));
2495+
await fs.writeFile(
2496+
path.join(moduleRoot, '.claude-plugin', 'marketplace.json'),
2497+
JSON.stringify({ plugins: [{ name: 'sample-mod', version: '1.7.2' }] }, null, 2) + '\n',
2498+
);
2499+
2500+
const versionInfo = await resolveModuleVersion('sample-mod', { moduleSourcePath: moduleDir });
2501+
assert(versionInfo.version === '2.4.0', 'resolver does not read a host project package.json outside the module repo boundary');
2502+
assert(versionInfo.source === 'module.yaml', 'resolver stops at the module repo boundary before climbing into host project metadata');
2503+
} finally {
2504+
if (priorCacheEnv39 === undefined) {
2505+
delete process.env.BMAD_EXTERNAL_MODULES_CACHE;
2506+
} else {
2507+
process.env.BMAD_EXTERNAL_MODULES_CACHE = priorCacheEnv39;
2508+
}
2509+
await fs.remove(tempHost39).catch(() => {});
2510+
await fs.remove(tempCacheDir39).catch(() => {});
2511+
}
2512+
}
2513+
2514+
// --- Manifest uses the shared resolver for external modules ---
2515+
{
2516+
const { Manifest } = require('../tools/installer/core/manifest');
2517+
const { ExternalModuleManager } = require('../tools/installer/modules/external-manager');
2518+
const tempCacheDir39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-manifest-version-cache-'));
2519+
const tempBmadDir39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-manifest-version-install-'));
2520+
const priorCacheEnv39 = process.env.BMAD_EXTERNAL_MODULES_CACHE;
2521+
const originalLoadConfig39 = ExternalModuleManager.prototype.loadExternalModulesConfig;
2522+
process.env.BMAD_EXTERNAL_MODULES_CACHE = tempCacheDir39;
2523+
2524+
ExternalModuleManager.prototype.loadExternalModulesConfig = async function () {
2525+
return {
2526+
modules: [
2527+
{
2528+
code: 'tea',
2529+
name: 'Test Architect',
2530+
repository: 'https://example.com/tea.git',
2531+
module_definition: 'src/module.yaml',
2532+
npm_package: 'bmad-method-test-architecture-enterprise',
2533+
},
2534+
],
2535+
};
2536+
};
2537+
2538+
try {
2539+
const moduleRoot = path.join(tempCacheDir39, 'tea');
2540+
const moduleSrc = path.join(moduleRoot, 'src');
2541+
await fs.ensureDir(path.join(moduleRoot, '.claude-plugin'));
2542+
await fs.ensureDir(moduleSrc);
2543+
2544+
await fs.writeFile(
2545+
path.join(moduleRoot, 'package.json'),
2546+
JSON.stringify({ name: 'bmad-method-test-architecture-enterprise', version: '1.12.3' }, null, 2) + '\n',
2547+
);
2548+
await fs.writeFile(path.join(moduleSrc, 'module.yaml'), ['code: tea', 'module_version: 1.11.0', ''].join('\n'));
2549+
await fs.writeFile(
2550+
path.join(moduleRoot, '.claude-plugin', 'marketplace.json'),
2551+
JSON.stringify({ plugins: [{ name: 'tea', version: '1.7.2' }] }, null, 2) + '\n',
2552+
);
2553+
2554+
const manifest39 = new Manifest();
2555+
const versionInfo = await manifest39.getModuleVersionInfo('tea', tempBmadDir39, moduleSrc);
2556+
2557+
assert(versionInfo.version === '1.12.3', 'manifest version info prefers external package.json over stale marketplace metadata');
2558+
assert(versionInfo.source === 'external', 'manifest preserves external source classification while using the shared resolver');
2559+
assert(
2560+
versionInfo.npmPackage === 'bmad-method-test-architecture-enterprise',
2561+
'manifest preserves npm package metadata for external modules',
2562+
);
2563+
} finally {
2564+
ExternalModuleManager.prototype.loadExternalModulesConfig = originalLoadConfig39;
2565+
if (priorCacheEnv39 === undefined) {
2566+
delete process.env.BMAD_EXTERNAL_MODULES_CACHE;
2567+
} else {
2568+
process.env.BMAD_EXTERNAL_MODULES_CACHE = priorCacheEnv39;
2569+
}
2570+
await fs.remove(tempCacheDir39).catch(() => {});
2571+
await fs.remove(tempBmadDir39).catch(() => {});
2572+
}
2573+
}
2574+
2575+
// --- Update checks should not advertise npm downgrades when source installs are newer ---
2576+
{
2577+
const { Manifest } = require('../tools/installer/core/manifest');
2578+
const manifest39 = new Manifest();
2579+
const originalGetAllModuleVersions39 = manifest39.getAllModuleVersions.bind(manifest39);
2580+
const originalFetchNpmVersion39 = manifest39.fetchNpmVersion.bind(manifest39);
2581+
2582+
manifest39.getAllModuleVersions = async () => [
2583+
{
2584+
name: 'tea',
2585+
version: '1.12.3',
2586+
npmPackage: 'bmad-method-test-architecture-enterprise',
2587+
},
2588+
];
2589+
manifest39.fetchNpmVersion = async () => '1.7.2';
2590+
2591+
try {
2592+
const updates = await manifest39.checkForUpdates('/unused');
2593+
assert(updates.length === 0, 'update check ignores older npm versions when installed source metadata is newer');
2594+
} finally {
2595+
manifest39.getAllModuleVersions = originalGetAllModuleVersions39;
2596+
manifest39.fetchNpmVersion = originalFetchNpmVersion39;
2597+
}
2598+
}
2599+
2600+
// --- Update checks ignore non-semver version strings instead of flagging false positives ---
2601+
{
2602+
const { Manifest } = require('../tools/installer/core/manifest');
2603+
const manifest39 = new Manifest();
2604+
const originalGetAllModuleVersions39 = manifest39.getAllModuleVersions.bind(manifest39);
2605+
const originalFetchNpmVersion39 = manifest39.fetchNpmVersion.bind(manifest39);
2606+
2607+
manifest39.getAllModuleVersions = async () => [
2608+
{
2609+
name: 'tea',
2610+
version: 'workspace-build',
2611+
npmPackage: 'bmad-method-test-architecture-enterprise',
2612+
},
2613+
];
2614+
manifest39.fetchNpmVersion = async () => 'latest-build';
2615+
2616+
try {
2617+
const updates = await manifest39.checkForUpdates('/unused');
2618+
assert(updates.length === 0, 'update check ignores non-semver version strings instead of reporting misleading updates');
2619+
} finally {
2620+
manifest39.getAllModuleVersions = originalGetAllModuleVersions39;
2621+
manifest39.fetchNpmVersion = originalFetchNpmVersion39;
2622+
}
2623+
}
2624+
2625+
console.log('');
2626+
23582627
// ============================================================
23592628
// Summary
23602629
// ============================================================

tools/installer/core/installer.js

Lines changed: 8 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const prompts = require('../prompts');
1111
const { BMAD_FOLDER_NAME } = require('../ide/shared/path-utils');
1212
const { InstallPaths } = require('./install-paths');
1313
const { ExternalModuleManager } = require('../modules/external-manager');
14+
const { resolveModuleVersion } = require('../modules/version-resolver');
1415

1516
const { ExistingInstall } = require('./existing-install');
1617

@@ -24,44 +25,6 @@ class Installer {
2425
this.bmadFolderName = BMAD_FOLDER_NAME;
2526
}
2627

27-
/**
28-
* Read the module version from .claude-plugin/marketplace.json
29-
* Walks up from sourcePath looking for .claude-plugin/marketplace.json
30-
* @param {string} sourcePath - Module source directory
31-
* @returns {string} Version string or empty string
32-
*/
33-
async _getMarketplaceVersion(sourcePath) {
34-
let dir = sourcePath;
35-
for (let i = 0; i < 5; i++) {
36-
const marketplacePath = path.join(dir, '.claude-plugin', 'marketplace.json');
37-
if (await fs.pathExists(marketplacePath)) {
38-
try {
39-
const data = JSON.parse(await fs.readFile(marketplacePath, 'utf8'));
40-
return this._extractMarketplaceVersion(data);
41-
} catch {
42-
return '';
43-
}
44-
}
45-
const parent = path.dirname(dir);
46-
if (parent === dir) break;
47-
dir = parent;
48-
}
49-
return '';
50-
}
51-
52-
/**
53-
* Extract the highest version from marketplace.json plugins array
54-
*/
55-
_extractMarketplaceVersion(data) {
56-
const plugins = data?.plugins;
57-
if (!Array.isArray(plugins) || plugins.length === 0) return '';
58-
let best = '';
59-
for (const p of plugins) {
60-
if (p.version && (!best || p.version > best)) best = p.version;
61-
}
62-
return best;
63-
}
64-
6528
/**
6629
* Main installation method
6730
* @param {Object} config - Installation configuration
@@ -641,15 +604,18 @@ class Installer {
641604
},
642605
);
643606

644-
// Get display name from source module.yaml; version from resolution cache or marketplace.json
607+
// Get display name from source module.yaml and resolve the freshest version metadata we can find locally.
645608
const sourcePath = await officialModules.findModuleSource(moduleName, { silent: true });
646609
const moduleInfo = sourcePath ? await officialModules.getModuleInfo(sourcePath, moduleName, '') : null;
647610
const displayName = moduleInfo?.name || moduleName;
648611

649-
// Prefer version from resolution cache (accurate for custom/local modules),
650-
// fall back to marketplace.json walk-up for official modules
651612
const cachedResolution = CustomModuleManager._resolutionCache.get(moduleName);
652-
const version = cachedResolution?.version || (sourcePath ? await this._getMarketplaceVersion(sourcePath) : '');
613+
const versionInfo = await resolveModuleVersion(moduleName, {
614+
moduleSourcePath: sourcePath,
615+
fallbackVersion: cachedResolution?.version,
616+
marketplacePluginNames: cachedResolution?.pluginName ? [cachedResolution.pluginName] : [],
617+
});
618+
const version = versionInfo.version || '';
653619
addResult(displayName, 'ok', '', { moduleCode: moduleName, newVersion: version });
654620
}
655621
}

0 commit comments

Comments
 (0)