fix: addressed review comments

This commit is contained in:
murat 2026-04-22 09:18:26 -05:00
parent 3698c88e75
commit 80d4a4c978
3 changed files with 120 additions and 5 deletions

View File

@ -2403,6 +2403,9 @@ async function runTests() {
{
const { resolveModuleVersion } = require('../tools/installer/modules/version-resolver');
const tempRepo39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-version-module-yaml-'));
const tempCacheDir39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-version-module-yaml-cache-'));
const priorCacheEnv39 = process.env.BMAD_EXTERNAL_MODULES_CACHE;
process.env.BMAD_EXTERNAL_MODULES_CACHE = tempCacheDir39;
try {
const moduleDir = path.join(tempRepo39, 'src');
@ -2419,7 +2422,13 @@ async function runTests() {
assert(versionInfo.version === '2.4.0', 'resolver falls back to module.yaml when package.json is missing');
assert(versionInfo.source === 'module.yaml', 'resolver reports module.yaml when it provides the selected version');
} finally {
if (priorCacheEnv39 === undefined) {
delete process.env.BMAD_EXTERNAL_MODULES_CACHE;
} else {
process.env.BMAD_EXTERNAL_MODULES_CACHE = priorCacheEnv39;
}
await fs.remove(tempRepo39).catch(() => {});
await fs.remove(tempCacheDir39).catch(() => {});
}
}
@ -2427,6 +2436,9 @@ async function runTests() {
{
const { resolveModuleVersion } = require('../tools/installer/modules/version-resolver');
const tempRepo39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-version-marketplace-'));
const tempCacheDir39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-version-marketplace-cache-'));
const priorCacheEnv39 = process.env.BMAD_EXTERNAL_MODULES_CACHE;
process.env.BMAD_EXTERNAL_MODULES_CACHE = tempCacheDir39;
try {
const moduleDir = path.join(tempRepo39, 'src');
@ -2454,7 +2466,48 @@ async function runTests() {
);
assert(versionInfo.source === 'marketplace.json', 'resolver reports marketplace.json when it is the only usable metadata source');
} finally {
if (priorCacheEnv39 === undefined) {
delete process.env.BMAD_EXTERNAL_MODULES_CACHE;
} else {
process.env.BMAD_EXTERNAL_MODULES_CACHE = priorCacheEnv39;
}
await fs.remove(tempRepo39).catch(() => {});
await fs.remove(tempCacheDir39).catch(() => {});
}
}
// --- package.json lookup must not escape the module repo boundary ---
{
const { resolveModuleVersion } = require('../tools/installer/modules/version-resolver');
const tempHost39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-version-boundary-host-'));
const tempCacheDir39 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-version-boundary-cache-'));
const priorCacheEnv39 = process.env.BMAD_EXTERNAL_MODULES_CACHE;
process.env.BMAD_EXTERNAL_MODULES_CACHE = tempCacheDir39;
try {
const moduleRoot = path.join(tempHost39, 'nested-module');
const moduleDir = path.join(moduleRoot, 'src');
await fs.ensureDir(path.join(moduleRoot, '.claude-plugin'));
await fs.ensureDir(moduleDir);
await fs.writeFile(path.join(tempHost39, 'package.json'), JSON.stringify({ name: 'host-project', version: '9.9.9' }, null, 2) + '\n');
await fs.writeFile(path.join(moduleDir, 'module.yaml'), ['code: sample-mod', 'module_version: 2.4.0', ''].join('\n'));
await fs.writeFile(
path.join(moduleRoot, '.claude-plugin', 'marketplace.json'),
JSON.stringify({ plugins: [{ name: 'sample-mod', version: '1.7.2' }] }, null, 2) + '\n',
);
const versionInfo = await resolveModuleVersion('sample-mod', { moduleSourcePath: moduleDir });
assert(versionInfo.version === '2.4.0', 'resolver does not read a host project package.json outside the module repo boundary');
assert(versionInfo.source === 'module.yaml', 'resolver stops at the module repo boundary before climbing into host project metadata');
} finally {
if (priorCacheEnv39 === undefined) {
delete process.env.BMAD_EXTERNAL_MODULES_CACHE;
} else {
process.env.BMAD_EXTERNAL_MODULES_CACHE = priorCacheEnv39;
}
await fs.remove(tempHost39).catch(() => {});
await fs.remove(tempCacheDir39).catch(() => {});
}
}
@ -2544,6 +2597,31 @@ async function runTests() {
}
}
// --- Update checks ignore non-semver version strings instead of flagging false positives ---
{
const { Manifest } = require('../tools/installer/core/manifest');
const manifest39 = new Manifest();
const originalGetAllModuleVersions39 = manifest39.getAllModuleVersions.bind(manifest39);
const originalFetchNpmVersion39 = manifest39.fetchNpmVersion.bind(manifest39);
manifest39.getAllModuleVersions = async () => [
{
name: 'tea',
version: 'workspace-build',
npmPackage: 'bmad-method-test-architecture-enterprise',
},
];
manifest39.fetchNpmVersion = async () => 'latest-build';
try {
const updates = await manifest39.checkForUpdates('/unused');
assert(updates.length === 0, 'update check ignores non-semver version strings instead of reporting misleading updates');
} finally {
manifest39.getAllModuleVersions = originalGetAllModuleVersions39;
manifest39.fetchNpmVersion = originalFetchNpmVersion39;
}
}
console.log('');
// ============================================================

View File

@ -396,7 +396,7 @@ class Manifest {
const installedVersion = semver.valid(module.version) || semver.valid(semver.coerce(module.version || ''));
const availableVersion = semver.valid(latestVersion) || semver.valid(semver.coerce(latestVersion));
if (!installedVersion || !availableVersion || semver.gt(availableVersion, installedVersion)) {
if (installedVersion && availableVersion && semver.gt(availableVersion, installedVersion)) {
updates.push({
name: module.name,
installedVersion: module.version,

View File

@ -73,7 +73,7 @@ async function findPackageJsonPath(moduleName, moduleSourcePath) {
const roots = await buildSearchRoots(moduleName, moduleSourcePath);
for (const root of roots) {
const packageJsonPath = await findNearestUpwardFile(root, 'package.json');
const packageJsonPath = await findNearestUpwardFile(root.searchDir, 'package.json', { boundaryDir: root.boundaryDir });
if (packageJsonPath) {
return packageJsonPath;
}
@ -97,7 +97,9 @@ async function findMarketplaceVersion(moduleName, moduleSourcePath, marketplaceP
const roots = await buildSearchRoots(moduleName, moduleSourcePath);
for (const root of roots) {
const marketplacePath = await findNearestUpwardFile(root, path.join('.claude-plugin', 'marketplace.json'));
const marketplacePath = await findNearestUpwardFile(root.searchDir, path.join('.claude-plugin', 'marketplace.json'), {
boundaryDir: root.boundaryDir,
});
if (!marketplacePath) {
continue;
}
@ -131,7 +133,10 @@ async function buildSearchRoots(moduleName, moduleSourcePath) {
}
seen.add(normalized);
roots.push(normalized);
roots.push({
searchDir: normalized,
boundaryDir: await findSearchBoundary(normalized),
});
};
await addRoot(moduleSourcePath);
@ -145,12 +150,14 @@ async function buildSearchRoots(moduleName, moduleSourcePath) {
return roots;
}
async function findNearestUpwardFile(startDir, relativeFilePath, maxDepth = DEFAULT_PARENT_DEPTH) {
async function findNearestUpwardFile(startDir, relativeFilePath, options = {}) {
const normalizedStartDir = await normalizeExistingDirectory(startDir);
if (!normalizedStartDir) {
return null;
}
const maxDepth = options.maxDepth ?? DEFAULT_PARENT_DEPTH;
const normalizedBoundaryDir = await normalizeDirectoryPath(options.boundaryDir);
let currentDir = normalizedStartDir;
for (let depth = 0; depth <= maxDepth; depth++) {
const candidate = path.join(currentDir, relativeFilePath);
@ -158,6 +165,10 @@ async function findNearestUpwardFile(startDir, relativeFilePath, maxDepth = DEFA
return candidate;
}
if (normalizedBoundaryDir && currentDir === normalizedBoundaryDir) {
break;
}
const parentDir = path.dirname(currentDir);
if (parentDir === currentDir) {
break;
@ -168,6 +179,32 @@ async function findNearestUpwardFile(startDir, relativeFilePath, maxDepth = DEFA
return null;
}
async function findSearchBoundary(startDir) {
const normalizedStartDir = await normalizeExistingDirectory(startDir);
if (!normalizedStartDir) {
return null;
}
let currentDir = normalizedStartDir;
for (let depth = 0; depth <= DEFAULT_PARENT_DEPTH; depth++) {
if (
(await fs.pathExists(path.join(currentDir, 'package.json'))) ||
(await fs.pathExists(path.join(currentDir, '.claude-plugin', 'marketplace.json'))) ||
(await fs.pathExists(path.join(currentDir, '.git')))
) {
return currentDir;
}
const parentDir = path.dirname(currentDir);
if (parentDir === currentDir) {
break;
}
currentDir = parentDir;
}
return normalizedStartDir;
}
async function normalizeDirectoryPath(candidate) {
if (!candidate) {
return null;