From d05f6ee420bbf7e40d9e7d5a9cc7d9ad6750034a Mon Sep 17 00:00:00 2001 From: Wojciech Tyziniec <10180694+wtznc@users.noreply.github.com> Date: Sat, 15 Nov 2025 17:13:41 +0100 Subject: [PATCH 1/3] fix: prevent spinner from hiding confirmation prompt during upgrade Fixes #907 When quickUpdate() called install(), two spinners ran simultaneously. The quickUpdate spinner continued running while install() stopped its own spinner to show prompts, causing the confirmation prompt to be hidden behind the spinner animation. Now quickUpdate() always stops its spinner before calling install(), ensuring only one spinner is active and all prompts are visible. --- tools/cli/installers/lib/core/installer.js | 24 +++++++++++++--------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/tools/cli/installers/lib/core/installer.js b/tools/cli/installers/lib/core/installer.js index 95a33bd2..94fe67e6 100644 --- a/tools/cli/installers/lib/core/installer.js +++ b/tools/cli/installers/lib/core/installer.js @@ -1763,10 +1763,11 @@ class Installer { const newBmadFolderName = this.configCollector.collectedConfig.core?.bmad_folder || existingBmadFolderName; if (existingBmadFolderName === newBmadFolderName) { - // Normal quick update - start the spinner - spinner.start('Updating BMAD installation...'); + // Normal quick update - stop spinner before calling install() + // install() will manage its own spinner + spinner.stop(); } else { - // Folder name has changed - stop spinner and let install() handle it + // Folder name has changed - notify user that install() will handle migration spinner.stop(); console.log(chalk.yellow(`\n⚠️ Folder name will change: ${existingBmadFolderName} → ${newBmadFolderName}`)); console.log(chalk.yellow('The installer will handle the folder migration.\n')); @@ -1786,14 +1787,12 @@ class Installer { _savedIdeConfigs: savedIdeConfigs, // Pass saved IDE configs to installer }; - // Call the standard install method + // Call the standard install method (it will manage its own spinner) const result = await this.install(installConfig); - // Only succeed the spinner if it's still spinning - // (install method might have stopped it if folder name changed) - if (spinner.isSpinning) { - spinner.succeed('Quick update complete!'); - } + // install() handles its own spinner, so no need to check if our spinner is spinning + // Just show completion message + console.log(chalk.green('✓ Quick update complete!')); return { success: true, @@ -1804,7 +1803,12 @@ class Installer { ides: configuredIdes, }; } catch (error) { - spinner.fail('Quick update failed'); + // Spinner is already stopped, just show error + if (spinner.isSpinning) { + spinner.fail('Quick update failed'); + } else { + console.error(chalk.red('✗ Quick update failed')); + } throw error; } } From 59ed774154b85bbd1718fe87338e04f87f94b78f Mon Sep 17 00:00:00 2001 From: Wojciech Tyziniec <10180694+wtznc@users.noreply.github.com> Date: Sat, 15 Nov 2025 17:22:55 +0100 Subject: [PATCH 2/3] refactor: simplify spinner handling and remove duplicate messages - Move spinner.stop() before conditional to eliminate code duplication - Remove duplicate success message (caller already shows detailed message) - Simplify error handling (caller handles error messages) Addresses Copilot feedback on PR #921 --- tools/cli/installers/lib/core/installer.js | 23 +++++++++------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/tools/cli/installers/lib/core/installer.js b/tools/cli/installers/lib/core/installer.js index 94fe67e6..de52b1e0 100644 --- a/tools/cli/installers/lib/core/installer.js +++ b/tools/cli/installers/lib/core/installer.js @@ -1762,13 +1762,11 @@ class Installer { const existingBmadFolderName = path.basename(bmadDir); const newBmadFolderName = this.configCollector.collectedConfig.core?.bmad_folder || existingBmadFolderName; - if (existingBmadFolderName === newBmadFolderName) { - // Normal quick update - stop spinner before calling install() - // install() will manage its own spinner - spinner.stop(); - } else { - // Folder name has changed - notify user that install() will handle migration - spinner.stop(); + // Stop spinner before calling install() since install() manages its own spinner + spinner.stop(); + + // Notify user if folder name will change + if (existingBmadFolderName !== newBmadFolderName) { console.log(chalk.yellow(`\n⚠️ Folder name will change: ${existingBmadFolderName} → ${newBmadFolderName}`)); console.log(chalk.yellow('The installer will handle the folder migration.\n')); } @@ -1790,9 +1788,8 @@ class Installer { // Call the standard install method (it will manage its own spinner) const result = await this.install(installConfig); - // install() handles its own spinner, so no need to check if our spinner is spinning - // Just show completion message - console.log(chalk.green('✓ Quick update complete!')); + // Note: Completion message is shown by the caller in commands/install.js + // to avoid duplication and provide more detailed success information return { success: true, @@ -1803,11 +1800,9 @@ class Installer { ides: configuredIdes, }; } catch (error) { - // Spinner is already stopped, just show error + // Ensure spinner is stopped, then let caller handle error message if (spinner.isSpinning) { - spinner.fail('Quick update failed'); - } else { - console.error(chalk.red('✗ Quick update failed')); + spinner.stop(); } throw error; } From f056ec7c96d07a6a43ee4b0451e82e0735f46ebc Mon Sep 17 00:00:00 2001 From: Wojciech Tyziniec <10180694+wtznc@users.noreply.github.com> Date: Sat, 15 Nov 2025 17:35:09 +0100 Subject: [PATCH 3/3] fix: add operation context to error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restore error message in quickUpdate to provide operation context. Users now see: 1. '✗ Quick update failed' (what operation failed) 2. 'Installation failed: [details]' (why it failed from caller) This provides better UX than generic 'Installation failed' alone. --- tools/cli/installers/lib/core/installer.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/cli/installers/lib/core/installer.js b/tools/cli/installers/lib/core/installer.js index de52b1e0..715d04d9 100644 --- a/tools/cli/installers/lib/core/installer.js +++ b/tools/cli/installers/lib/core/installer.js @@ -1800,10 +1800,12 @@ class Installer { ides: configuredIdes, }; } catch (error) { - // Ensure spinner is stopped, then let caller handle error message + // Ensure spinner is stopped if (spinner.isSpinning) { spinner.stop(); } + // Provide operation context before detailed error from caller + console.error(chalk.red('✗ Quick update failed')); throw error; } }