ldy26098
/
BMAD-METHOD
mirror of https://github.com/bmad-code-org/BMAD-METHOD.git
1
0
Fork 0
Code Packages Projects Releases Wiki Activity

fix(cli): resolve code review findings from @clack/prompts migration 

Address 31 issues across 14 CLI files found during PR #1586 review
(Augment Code + CodeRabbit):

- Fix bmadDir ReferenceError by hoisting declaration before try block
- Wrap console.log monkey-patch in try/finally for safe restoration
- Fix transformWorkflowPath dead code and undefined return path
- Fix broken symlink crash in _config-driven.js and codex.js cleanup
- Pass installer instance through update() for agent recompilation
- Fix @clack/prompts API: defaultValue→default, initialValue→default
- Use nullish coalescing (??) instead of logical OR for falsy values
- Forward options in recursive promptToolSelection calls
- Remove no-op replaceAll('_bmad','_bmad') in manager and generator
- Remove unused confirm prompt in config-collector hasNoConfig branch
- Guard spinner.message() when spinner is not running
- Add missing methods to silent spinner stub (cancel, clear, isSpinning)
- Wrap install.js error handler with inner try/catch + console fallback
- Gate codex per-entry error log with silent flag
- Add return statements to all stream wrapper methods
- Remove dead variables (availableNames, hasCustomContentItems)
- Filter core module from update flow selection
- Replace borderColor ternary chain with object map
- Fix Kilo "agents" label to "modes" in IDE manager
- Normalize error return shape for unsupported IDEs
- Fix spinner message timing before dependency resolution
- Guard undefined moduleDir in dependency-resolver
- Fix workflowsInstalled counter inflation in custom handler
- Migrate console.warn calls to prompts.log.warn
- Replace console.log() with prompts.log.message('')
- Fix legacyBmadPath hardcoded to .bmad instead of entry.name
- Fix focusedValue never assigned breaking SPACE toggle and hints

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Davor Racić 2026-02-07 22:54:24 +01:00
parent 04edbf6ee3
commit 78090f6f4a
14 changed files with 118 additions and 125 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
tools/cli/commands/install.js
View File

@ -85,16 +85,17 @@ module.exports = {
process.exit(0);
}
} catch (error) {
// Check if error has a complete formatted message
if (error.fullMessage) {
console.error(error.fullMessage);
try {
if (error.fullMessage) {
await prompts.log.error(error.fullMessage);
} else {
await prompts.log.error(`Installation failed: ${error.message}`);
}
if (error.stack) {
await prompts.log.message(error.stack);
}
} else {
// Generic error handling for all other errors
await prompts.log.error(`Installation failed: ${error.message}`);
await prompts.log.message(error.stack);
} catch {
console.error(error.fullMessage || error.message || error);
}
process.exit(1);
}

15
tools/cli/installers/lib/core/config-collector.js
View File

@ -344,7 +344,7 @@ class ConfigCollector {
if (questions.length > 0) {
// Only show header if we actually have questions
await CLIUtils.displayModuleConfigHeader(moduleName, moduleConfig.header, moduleConfig.subheader);
console.log(); // Line break before questions
await prompts.log.message('');
const promptedAnswers = await prompts.prompt(questions);
// Merge prompted answers with static answers
@ -738,20 +738,7 @@ class ConfigCollector {
const hasNoConfig = actualConfigKeys.length === 0;
if (hasNoConfig && (moduleConfig.subheader || moduleConfig.header)) {
// Module explicitly has no configuration - show with special styling
await prompts.log.step(moduleDisplayName);
// Ask user if they want to accept defaults or customize on the next line
const { customize } = await prompts.prompt([
{
type: 'confirm',
name: 'customize',
message: 'Accept Defaults (no to customize)?',
default: true,
},
]);
// Show the subheader if available, otherwise show a default message
if (moduleConfig.subheader) {
await prompts.log.message(` \u2713 ${moduleConfig.subheader}`);
} else {

4
tools/cli/installers/lib/core/dependency-resolver.js
View File

@ -90,6 +90,10 @@ class DependencyResolver {
}
}
if (!moduleDir) {
continue;
}
if (!(await fs.pathExists(moduleDir))) {
await prompts.log.warn('Module directory not found: ' + moduleDir);
continue;

88
tools/cli/installers/lib/core/installer.js
View File

@ -237,6 +237,7 @@ class Installer {
// before any config collection, so we don't need to check again here
const projectDir = path.resolve(config.directory);
const bmadDir = path.join(projectDir, BMAD_FOLDER_NAME);
// If core config was pre-collected (from interactive mode), use it
if (config.coreConfig && Object.keys(config.coreConfig).length > 0) {
@ -374,12 +375,6 @@ class Installer {
spinner.start('Preparing installation...');
try {
// Resolve target directory (path.resolve handles platform differences)
const projectDir = path.resolve(config.directory);
// Always use the standard _bmad folder name
const bmadDir = path.join(projectDir, BMAD_FOLDER_NAME);
// Create a project directory if it doesn't exist (user already confirmed)
if (!(await fs.pathExists(projectDir))) {
spinner.message('Creating installation directory...');
@ -807,13 +802,13 @@ class Installer {
bmadDir: bmadDir, // Pass bmadDir so we can check cache
});
spinner.message('Resolving dependencies...');
const resolution = await this.dependencyResolver.resolve(projectRoot, regularModulesForResolution, {
verbose: config.verbose,
moduleManager: tempModuleManager,
});
spinner.message('Resolving dependencies...');
// Install modules with their dependencies
if (allModules && allModules.length > 0) {
const installedModuleNames = new Set();
@ -1020,46 +1015,47 @@ class Installer {
console.log = () => {};
}
for (const ide of validIdes) {
if (!needsPrompting || ideConfigurations[ide]) {
// All IDEs pre-configured, or this specific IDE has config: keep spinner running
spinner.message(`Configuring ${ide}...`);
} else {
// This IDE needs prompting: stop spinner to allow user interaction
if (spinner.isSpinning) {
spinner.stop('Ready for IDE configuration');
try {
for (const ide of validIdes) {
if (!needsPrompting || ideConfigurations[ide]) {
// All IDEs pre-configured, or this specific IDE has config: keep spinner running
spinner.message(`Configuring ${ide}...`);
} else {
// This IDE needs prompting: stop spinner to allow user interaction
if (spinner.isSpinning) {
spinner.stop('Ready for IDE configuration');
}
}
// Silent when this IDE has pre-collected config (no prompts for THIS IDE)
const ideHasConfig = Boolean(ideConfigurations[ide]);
const setupResult = await this.ideManager.setup(ide, projectDir, bmadDir, {
selectedModules: allModules || [],
preCollectedConfig: ideConfigurations[ide] || null,
verbose: config.verbose,
silent: ideHasConfig,
});
// Save IDE configuration for future updates
if (ideConfigurations[ide] && !ideConfigurations[ide]._alreadyConfigured) {
await this.ideConfigManager.saveIdeConfig(bmadDir, ide, ideConfigurations[ide]);
}
// Collect result for summary
if (setupResult.success) {
addResult(ide, 'ok', setupResult.detail || '');
} else {
addResult(ide, 'error', setupResult.error || 'failed');
}
// Restart spinner if we stopped it for prompting
if (needsPrompting && !spinner.isSpinning) {
spinner.start('Configuring IDEs...');
}
}
// Silent when this IDE has pre-collected config (no prompts for THIS IDE)
const ideHasConfig = Boolean(ideConfigurations[ide]);
const setupResult = await this.ideManager.setup(ide, projectDir, bmadDir, {
selectedModules: allModules || [],
preCollectedConfig: ideConfigurations[ide] || null,
verbose: config.verbose,
silent: ideHasConfig,
});
// Save IDE configuration for future updates
if (ideConfigurations[ide] && !ideConfigurations[ide]._alreadyConfigured) {
await this.ideConfigManager.saveIdeConfig(bmadDir, ide, ideConfigurations[ide]);
}
// Collect result for summary
if (setupResult.success) {
addResult(ide, 'ok', setupResult.detail || '');
} else {
addResult(ide, 'error', setupResult.error || 'failed');
}
// Restart spinner if we stopped it for prompting
if (needsPrompting && !spinner.isSpinning) {
spinner.start('Configuring IDEs...');
}
} finally {
console.log = originalLog;
}
// Restore console.log
console.log = originalLog;
}
}
@ -1338,7 +1334,7 @@ class Installer {
for (const module of existingInstall.modules) {
spinner.message(`Updating module: ${module.id}...`);
await this.moduleManager.update(module.id, bmadDir, config.force);
await this.moduleManager.update(module.id, bmadDir, config.force, { installer: this });
}
// Update manifest

7
tools/cli/installers/lib/custom/handler.js
View File

@ -268,14 +268,13 @@ class CustomHandler {
}
results.filesCopied++;
if (entry.name.endsWith('.md')) {
results.workflowsInstalled++;
}
if (fileTrackingCallback) {
fileTrackingCallback(targetPath);
}
}
if (entry.name.endsWith('.md')) {
results.workflowsInstalled++;
}
} catch (error) {
results.errors.push(`Failed to copy ${entry.name}: ${error.message}`);
}

9
tools/cli/installers/lib/ide/_config-driven.js
View File

@ -492,19 +492,16 @@ LOAD and execute from: {project-root}/{{bmadFolderName}}/{{path}}
let removedCount = 0;
for (const entry of entries) {
// Skip non-strings or undefined entries
if (!entry || typeof entry !== 'string') {
continue;
}
if (entry.startsWith('bmad')) {
const entryPath = path.join(targetPath, entry);
const stat = await fs.stat(entryPath);
if (stat.isFile()) {
await fs.remove(entryPath);
removedCount++;
} else if (stat.isDirectory()) {
try {
await fs.remove(entryPath);
removedCount++;
} catch {
// Skip entries that can't be removed (broken symlinks, permission errors)
}
}
}

12
tools/cli/installers/lib/ide/codex.js
View File

@ -284,15 +284,11 @@ class CodexSetup extends BaseIdeSetup {
const entryPath = path.join(destDir, entry);
try {
const stat = await fs.stat(entryPath);
if (stat.isFile()) {
await fs.remove(entryPath);
} else if (stat.isDirectory()) {
await fs.remove(entryPath);
}
await fs.remove(entryPath);
} catch (error) {
// Skip files that can't be processed
await prompts.log.message(` Skipping ${entry}: ${error.message}`);
if (!options.silent) {
await prompts.log.message(` Skipping ${entry}: ${error.message}`);
}
}
}
}

4
tools/cli/installers/lib/ide/manager.js
View File

@ -173,7 +173,7 @@ class IdeManager {
if (!handler) {
await prompts.log.warn(`IDE '${ideName}' is not yet supported`);
await prompts.log.message(`Supported IDEs: ${[...this.handlers.keys()].join(', ')}`);
return { success: false, reason: 'unsupported' };
return { success: false, ide: ideName, error: 'unsupported IDE' };
}
try {
@ -200,7 +200,7 @@ class IdeManager {
} else if (handlerResult && handlerResult.modes !== undefined) {
// Kilo handler returns { success, modes, workflows, tasks, tools }
const parts = [];
if (handlerResult.modes > 0) parts.push(`${handlerResult.modes} agents`);
if (handlerResult.modes > 0) parts.push(`${handlerResult.modes} modes`);
if (handlerResult.workflows > 0) parts.push(`${handlerResult.workflows} workflows`);
if (handlerResult.tasks > 0) parts.push(`${handlerResult.tasks} tasks`);
if (handlerResult.tools > 0) parts.push(`${handlerResult.tools} tools`);

17
tools/cli/installers/lib/ide/shared/workflow-command-generator.js
View File

@ -157,8 +157,7 @@ class WorkflowCommandGenerator {
.replaceAll('{{module}}', workflow.module)
.replaceAll('{{description}}', workflow.description)
.replaceAll('{{workflow_path}}', workflowPath)
.replaceAll('_bmad', this.bmadFolderName)
.replaceAll('_bmad', '_bmad');
.replaceAll('_bmad', this.bmadFolderName);
}
/**
@ -238,15 +237,15 @@ When running any workflow:
const match = workflowPath.match(/\/src\/bmm\/(.+)/);
if (match) {
transformed = `{project-root}/${this.bmadFolderName}/bmm/${match[1]}`;
} else if (workflowPath.includes('/src/core/')) {
const match = workflowPath.match(/\/src\/core\/(.+)/);
if (match) {
transformed = `{project-root}/${this.bmadFolderName}/core/${match[1]}`;
}
}
return transformed;
} else if (workflowPath.includes('/src/core/')) {
const match = workflowPath.match(/\/src\/core\/(.+)/);
if (match) {
transformed = `{project-root}/${this.bmadFolderName}/core/${match[1]}`;
}
}
return transformed;
}
async loadWorkflowManifest(bmadDir) {

28
tools/cli/installers/lib/modules/manager.js
View File

@ -287,7 +287,7 @@ class ModuleManager {
moduleInfo.dependencies = config.dependencies || [];
moduleInfo.defaultSelected = config.default_selected === undefined ? false : config.default_selected;
} catch (error) {
console.warn(`Failed to read config for ${defaultName}:`, error.message);
await prompts.log.warn(`Failed to read config for ${defaultName}: ${error.message}`);
}
return moduleInfo;
@ -365,7 +365,20 @@ class ModuleManager {
// Helper to create a spinner or a no-op when silent
const createSpinner = async () => {
if (silent) {
return { start() {}, stop() {}, error() {}, message() {} };
return {
start() {},
stop() {},
error() {},
message() {},
cancel() {},
clear() {},
get isSpinning() {
return false;
},
get isCancelled() {
return false;
},
};
}
return await prompts.spinner();
};
@ -603,7 +616,7 @@ class ModuleManager {
* @param {string} bmadDir - Target bmad directory
* @param {boolean} force - Force update (overwrite modifications)
*/
async update(moduleName, bmadDir, force = false) {
async update(moduleName, bmadDir, force = false, options = {}) {
const sourcePath = await this.findModuleSource(moduleName);
const targetPath = path.join(bmadDir, moduleName);
@ -620,13 +633,13 @@ class ModuleManager {
if (force) {
// Force update - remove and reinstall
await fs.remove(targetPath);
return await this.install(moduleName, bmadDir);
return await this.install(moduleName, bmadDir, null, { installer: options.installer });
} else {
// Selective update - preserve user modifications
await this.syncModule(sourcePath, targetPath);
// Recompile agents (#1133)
await this.compileModuleAgents(sourcePath, targetPath, moduleName, bmadDir);
await this.compileModuleAgents(sourcePath, targetPath, moduleName, bmadDir, options.installer);
await this.processAgentFiles(targetPath, moduleName);
}
@ -694,7 +707,7 @@ class ModuleManager {
const config = yaml.parse(configContent);
Object.assign(moduleInfo, config);
} catch (error) {
console.warn(`Failed to read installed module config:`, error.message);
await prompts.log.warn(`Failed to read installed module config: ${error.message}`);
}
}
@ -789,7 +802,6 @@ class ModuleManager {
// IMPORTANT: Replace escape sequence and placeholder BEFORE parsing YAML
// Otherwise parsing will fail on the placeholder
yamlContent = yamlContent.replaceAll('_bmad', '_bmad');
yamlContent = yamlContent.replaceAll('_bmad', this.bmadFolderName);
try {
@ -1323,7 +1335,7 @@ class ModuleManager {
await fs.writeFile(configPath, configContent, 'utf8');
} catch (error) {
console.warn(`Failed to process module config:`, error.message);
await prompts.log.warn(`Failed to process module config: ${error.message}`);
}
}
}

6
tools/cli/lib/agent/installer.js
View File

@ -164,15 +164,15 @@ async function promptInstallQuestions(installConfig, defaults, presetAnswers = {
case 'text': {
const response = await prompts.text({
message: q.prompt,
defaultValue: q.default || '',
default: q.default ?? '',
});
answers[q.var] = response || q.default || '';
answers[q.var] = response ?? q.default ?? '';
break;
}
case 'boolean': {
const response = await prompts.confirm({
message: q.prompt,
initialValue: q.default,
default: q.default,
});
answers[q.var] = response;
break;

4
tools/cli/lib/cli-utils.js
View File

@ -66,8 +66,8 @@ const CLIUtils = {
const color = await prompts.getColor();
const borderColor = options.borderColor || 'cyan';
const formatBorder =
borderColor === 'green' ? color.green : borderColor === 'red' ? color.red : borderColor === 'yellow' ? color.yellow : color.cyan;
const colorMap = { green: color.green, red: color.red, yellow: color.yellow, cyan: color.cyan, blue: color.blue };
const formatBorder = colorMap[borderColor] || color.cyan;
await prompts.box(text, options.title, {
rounded: options.borderStyle === 'round' || options.borderStyle === undefined,

23
tools/cli/lib/prompts.js
View File

@ -112,7 +112,9 @@ async function spinner() {
s.stop(msg);
}
},
message: (msg) => s.message(msg),
message: (msg) => {
if (spinning) s.message(msg);
},
error: (msg) => {
spinning = false;
s.error(msg);
@ -297,7 +299,7 @@ async function autocompleteMultiselect(options) {
const isSelected = this.selectedValues.includes(opt.value);
const isLocked = lockedSet.has(opt.value);
const label = opt.label ?? String(opt.value ?? '');
const hintText = opt.hint && opt.value === this.focusedValue ? color.dim(` (${opt.hint})`) : '';
const hintText = opt.hint && isHighlighted ? color.dim(` (${opt.hint})`) : '';
let checkbox;
if (isLocked) {
@ -370,8 +372,9 @@ async function autocompleteMultiselect(options) {
// Handle SPACE toggle when NOT navigating (internal code only handles it when isNavigating=true)
prompt.on('key', (char, key) => {
if (key && key.name === 'space' && !prompt.isNavigating && prompt.focusedValue !== undefined) {
prompt.toggleSelected(prompt.focusedValue);
if (key && key.name === 'space' && !prompt.isNavigating) {
const focused = prompt.filteredOptions[prompt.cursor];
if (focused) prompt.toggleSelected(focused.value);
}
});
// === END FIX ===
@ -648,27 +651,27 @@ async function selectKey(options) {
const stream = {
async info(generator) {
const clack = await getClack();
clack.stream.info(generator);
return clack.stream.info(generator);
},
async success(generator) {
const clack = await getClack();
clack.stream.success(generator);
return clack.stream.success(generator);
},
async step(generator) {
const clack = await getClack();
clack.stream.step(generator);
return clack.stream.step(generator);
},
async warn(generator) {
const clack = await getClack();
clack.stream.warn(generator);
return clack.stream.warn(generator);
},
async error(generator) {
const clack = await getClack();
clack.stream.error(generator);
return clack.stream.error(generator);
},
async message(generator, options) {
const clack = await getClack();
clack.stream.message(generator, options);
return clack.stream.message(generator, options);
},
};

11
tools/cli/lib/ui.js
View File

@ -74,7 +74,7 @@ class UI {
for (const entry of entries) {
if (entry.isDirectory() && (entry.name === '.bmad' || entry.name === 'bmad')) {
hasLegacyBmadFolder = true;
legacyBmadPath = path.join(confirmedDirectory, '.bmad');
legacyBmadPath = path.join(confirmedDirectory, entry.name);
bmadDir = legacyBmadPath;
// Check if it has _cfg folder
@ -151,7 +151,7 @@ class UI {
const newBmadPath = path.join(confirmedDirectory, '_bmad');
await fs.move(legacyBmadPath, newBmadPath);
bmadDir = newBmadPath;
s.stop('Renamed ".bmad" to "_bmad"');
s.stop(`Renamed "${path.basename(legacyBmadPath)}" to "_bmad"`);
}
// Handle _cfg folder (either from .bmad or standalone)
@ -274,6 +274,7 @@ class UI {
await prompts.log.info(`Using modules from command-line: ${selectedModules.join(', ')}`);
} else {
selectedModules = await this.selectAllModules(installedModuleIds);
selectedModules = selectedModules.filter((m) => m !== 'core');
}
// After module selection, ask about custom modules
@ -561,7 +562,7 @@ class UI {
});
if (!confirmNoTools) {
return this.promptToolSelection(projectDir);
return this.promptToolSelection(projectDir, options);
}
return { ides: [], skipIde: true };
@ -639,7 +640,7 @@ class UI {
if (!confirmNoTools) {
// User wants to select tools - recurse
return this.promptToolSelection(projectDir);
return this.promptToolSelection(projectDir, options);
}
return {
@ -825,7 +826,6 @@ class UI {
const isNewInstallation = installedModuleIds.size === 0;
const customContentItems = [];
const hasCustomContentItems = false;
// Add custom content items
if (customContentConfig && customContentConfig.hasCustomContent && customContentConfig.customPath) {
@ -962,7 +962,6 @@ class UI {
*/
async selectExternalModules(externalModuleChoices, defaultSelections = []) {
// Build a message showing available modules
const availableNames = externalModuleChoices.map((c) => c.name).join(', ');
const message = 'Select official BMad modules to install (use arrow keys, space to toggle):';
// Mark choices as checked based on defaultSelections