Address code review feedback from CodeRabbit and Augment

- Move "Setting up..." log after conflict check so it only shows when
  install will proceed
- Fix rm command: add -rf flags and correct quoting for glob outside quotes
- Improve error wording: "ancestor installation" instead of misleading
  "ancestor directory"
- Use case-insensitive startsWith for bmad file detection (macOS/Windows)
- Document ancestor_conflict_check in the installer config schema

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Alex Verkhovsky 2026-02-24 19:34:39 -07:00
parent db0c06f9dc
commit 886a070d2b
2 changed files with 8 additions and 5 deletions

View File

@ -34,17 +34,15 @@ class ConfigDrivenIdeSetup extends BaseIdeSetup {
* @returns {Promise<Object>} Setup result * @returns {Promise<Object>} Setup result
*/ */
async setup(projectDir, bmadDir, options = {}) { async setup(projectDir, bmadDir, options = {}) {
if (!options.silent) await prompts.log.info(`Setting up ${this.name}...`);
// Check for BMAD files in ancestor directories that would cause duplicates // Check for BMAD files in ancestor directories that would cause duplicates
if (this.installerConfig?.ancestor_conflict_check) { if (this.installerConfig?.ancestor_conflict_check) {
const conflict = await this.findAncestorConflict(projectDir); const conflict = await this.findAncestorConflict(projectDir);
if (conflict) { if (conflict) {
await prompts.log.error( await prompts.log.error(
`Found existing BMAD commands in ancestor directory: ${conflict}\n` + `Found existing BMAD commands in ancestor installation: ${conflict}\n` +
` ${this.name} inherits commands from parent directories, so this would cause duplicates.\n` + ` ${this.name} inherits commands from parent directories, so this would cause duplicates.\n` +
` Please remove the BMAD files from that directory first:\n` + ` Please remove the BMAD files from that directory first:\n` +
` rm "${conflict}/"bmad*`, ` rm -rf "${conflict}"/bmad*`,
); );
return { return {
success: false, success: false,
@ -55,6 +53,8 @@ class ConfigDrivenIdeSetup extends BaseIdeSetup {
} }
} }
if (!options.silent) await prompts.log.info(`Setting up ${this.name}...`);
// Clean up any old BMAD installation first // Clean up any old BMAD installation first
await this.cleanup(projectDir, options); await this.cleanup(projectDir, options);
@ -570,7 +570,7 @@ LOAD and execute from: {project-root}/{{bmadFolderName}}/{{path}}
try { try {
if (await fs.pathExists(candidatePath)) { if (await fs.pathExists(candidatePath)) {
const entries = await fs.readdir(candidatePath); const entries = await fs.readdir(candidatePath);
const hasBmad = entries.some((e) => typeof e === 'string' && e.startsWith('bmad')); const hasBmad = entries.some((e) => typeof e === 'string' && e.toLowerCase().startsWith('bmad'));
if (hasBmad) { if (hasBmad) {
return candidatePath; return candidatePath;
} }

View File

@ -198,6 +198,9 @@ platforms:
# artifact_types: [agents, workflows, tasks, tools] # artifact_types: [agents, workflows, tasks, tools]
# artifact_types: array (optional) # Filter which artifacts to install (default: all) # artifact_types: array (optional) # Filter which artifacts to install (default: all)
# skip_existing: boolean (optional) # Skip files that already exist (default: false) # skip_existing: boolean (optional) # Skip files that already exist (default: false)
# ancestor_conflict_check: boolean (optional) # Refuse install when ancestor dir has BMAD files
# # in the same target_dir (for IDEs that inherit
# # commands from parent directories)
# ============================================================================ # ============================================================================
# Platform Categories # Platform Categories