fix(installer): address Augment review findings

- Fix plugins[0] fragility: extract highest version across all plugins
  in marketplace.json instead of assuming first entry (ui.js, installer.js,
  manifest.js)
- Fix _readMarketplaceVersion ignoring moduleSourcePath: custom modules
  can now source their own marketplace.json by walking up from source path
- Hard-exclude bmad-os-* utility skills in both surgical and legacy cleanup
  modes, preventing accidental deletion if tracked in manifests
- Distinguish missing file vs parse error in skill-manifest.csv reading:
  warn on corrupt CSV instead of silently skipping cleanup
This commit is contained in:
Brian Madison 2026-04-07 01:49:33 -05:00
parent 2333ee9488
commit 3c155e3cbc
4 changed files with 66 additions and 11 deletions

View File

@ -39,7 +39,7 @@ class Installer {
if (await fs.pathExists(marketplacePath)) {
try {
const data = JSON.parse(await fs.readFile(marketplacePath, 'utf8'));
return data.plugins?.[0]?.version || '';
return this._extractMarketplaceVersion(data);
} catch {
return '';
}
@ -51,6 +51,19 @@ class Installer {
return '';
}
/**
* Extract the highest version from marketplace.json plugins array
*/
_extractMarketplaceVersion(data) {
const plugins = data?.plugins;
if (!Array.isArray(plugins) || plugins.length === 0) return '';
let best = '';
for (const p of plugins) {
if (p.version && (!best || p.version > best)) best = p.version;
}
return best;
}
/**
* Main installation method
* @param {Object} config - Installation configuration
@ -95,17 +108,17 @@ class Installer {
// Capture previously installed skill IDs before they get overwritten
const previousSkillIds = new Set();
const prevCsvPath = path.join(paths.bmadDir, '_config', 'skill-manifest.csv');
try {
if (await fs.pathExists(prevCsvPath)) {
if (await fs.pathExists(prevCsvPath)) {
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}`);
}
} catch {
// No previous manifest - fresh install
}
await this._cacheCustomModules(paths, addResult);

View File

@ -841,7 +841,7 @@ class Manifest {
const yaml = require('yaml');
// All module versions come from .claude-plugin/marketplace.json
const version = await this._readMarketplaceVersion(moduleName);
const version = await this._readMarketplaceVersion(moduleName, moduleSourcePath);
// Determine source type
if (['core', 'bmm'].includes(moduleName)) {
@ -900,13 +900,29 @@ class Manifest {
* @param {string} moduleName - Module code
* @returns {string|null} Version or null
*/
async _readMarketplaceVersion(moduleName) {
async _readMarketplaceVersion(moduleName, moduleSourcePath = null) {
const os = require('node:os');
let marketplacePath;
if (['core', 'bmm'].includes(moduleName)) {
marketplacePath = path.join(getProjectRoot(), '.claude-plugin', 'marketplace.json');
} else {
} else if (moduleSourcePath) {
// Walk up from source path to find marketplace.json
let dir = moduleSourcePath;
for (let i = 0; i < 5; i++) {
const candidate = path.join(dir, '.claude-plugin', 'marketplace.json');
if (await fs.pathExists(candidate)) {
marketplacePath = candidate;
break;
}
const parent = path.dirname(dir);
if (parent === dir) break;
dir = parent;
}
}
// Fallback to external module cache
if (!marketplacePath) {
const cacheDir = path.join(os.homedir(), '.bmad', 'cache', 'external-modules', moduleName);
marketplacePath = path.join(cacheDir, '.claude-plugin', 'marketplace.json');
}
@ -914,7 +930,13 @@ class Manifest {
try {
if (await fs.pathExists(marketplacePath)) {
const data = JSON.parse(await fs.readFile(marketplacePath, 'utf8'));
return data.plugins?.[0]?.version || null;
const plugins = data?.plugins;
if (!Array.isArray(plugins) || plugins.length === 0) return null;
let best = null;
for (const p of plugins) {
if (p.version && (!best || p.version > best)) best = p.version;
}
return best;
}
} catch {
// ignore

View File

@ -428,8 +428,11 @@ class ConfigDrivenIdeSetup {
for (const entry of entries) {
if (!entry || typeof entry !== 'string') continue;
// Always preserve bmad-os-* utility skills regardless of cleanup mode
if (entry.startsWith('bmad-os-')) continue;
// Surgical removal from set, or legacy prefix matching when set is null
const shouldRemove = removalSet ? removalSet.has(entry) : entry.startsWith('bmad') && !entry.startsWith('bmad-os-');
const shouldRemove = removalSet ? removalSet.has(entry) : entry.startsWith('bmad');
if (shouldRemove) {
try {

View File

@ -23,7 +23,7 @@ async function getMarketplaceVersion(moduleCode) {
try {
if (await fs.pathExists(marketplacePath)) {
const data = JSON.parse(await fs.readFile(marketplacePath, 'utf8'));
return data.plugins?.[0]?.version || '';
return _extractMarketplaceVersion(data);
}
} catch {
// ignore
@ -31,6 +31,23 @@ async function getMarketplaceVersion(moduleCode) {
return '';
}
/**
* Extract the highest version from marketplace.json plugins array.
* Handles multiple plugins per file safely.
* @param {Object} data - Parsed marketplace.json
* @returns {string} Version string or empty string
*/
function _extractMarketplaceVersion(data) {
const plugins = data?.plugins;
if (!Array.isArray(plugins) || plugins.length === 0) return '';
// Use the highest version across all plugins in the file
let best = '';
for (const p of plugins) {
if (p.version && (!best || p.version > best)) best = p.version;
}
return best;
}
// Separator class for visual grouping in select/multiselect prompts
// Note: @clack/prompts doesn't support separators natively, they are filtered out
class Separator {