fix: installer fixes and non-interactive mode improvements #1612
Conversation
…nts output The spinner.stop() in compileAgents() already displays this message, so the additional log.success() call produced a redundant line. Co-Authored-By: Claude Opus 4.6 <[email protected]>
When updating an existing installation, IDEs that were previously configured but unchecked in the new selection are now detected and cleaned up after user confirmation, mirroring the existing module removal flow. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Remove 4 dead `return;` statements that immediately follow `process.exit(0)` calls in install.js. Since process.exit() terminates the process, the subsequent return statements are unreachable. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Collect all created directory names during module directory setup and emit them as a single log message instead of one per directory. This eliminates the excessive blank-line spacing that @clack/prompts adds between individual log.message() calls. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Move directory creation logging from ModuleManager.createModuleDirectories into the installer caller. The method now returns created directory info instead of logging directly, allowing the installer to batch all module directories into a single log message under one spinner. Also adds spacing before the final "Installation complete" status line. Co-Authored-By: Claude Opus 4.6 <[email protected]>
The --yes flag (skipPrompts) was not being passed through to IDE handler collectConfiguration() calls, causing the Codex installer to hang on its interactive prompt in non-interactive mode (CI/CD, --yes flag). - Add skipPrompts parameter to collectToolConfigurations() and forward it to handler.collectConfiguration() options - Add early return in CodexSetup.collectConfiguration() when skipPrompts is true, defaulting to global install location - Guard IDE removal confirmation prompt with skipPrompts check, defaulting to preserve existing configs (consistent with prompt default: false) Fixes bmad-code-org#1610 Co-Authored-By: Claude Opus 4.6 <[email protected]>
🤖 Augment PR SummarySummary: Improves installer update and non-interactive (
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThis pull request implements non-interactive/headless installation support by propagating a Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI Command
participant Installer as Installer
participant IDEs as IDE Configurators
participant Codex as Codex IDE
participant ModMgr as Module Manager
rect rgba(100, 150, 200, 0.5)
Note over CLI,ModMgr: With --yes flag (skipPrompts = true)
end
CLI->>Installer: collectToolConfigurations(..., skipPrompts=true)
Installer->>IDEs: collectConfiguration(..., skipPrompts=true)
IDEs->>Codex: collectConfiguration({skipPrompts: true})
alt skipPrompts enabled
Codex-->>IDEs: {installLocation: 'global'} (no prompt)
else skipPrompts disabled
Codex->>Codex: prompts.select() (interactive)
Codex-->>IDEs: user selection
end
IDEs-->>Installer: IDE configs
Installer->>ModMgr: createModuleDirectories(...)
ModMgr-->>Installer: {createdDirs: [...], createdWdsFolders: [...]}
Installer-->>CLI: completion with batch-logged results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@tools/cli/installers/lib/core/installer.js`:
- Around line 745-751: When cancelled removal pushes ides back into config.ides,
their saved configuration in ideConfigurations is not restored so re-added IDEs
get preCollectedConfig:null; after the loop that pushes ides from idesToRemove
back into config.ides, iterate those re-added IDE names and populate
ideConfigurations[name] from the existing savedIdeConfigs (the same source used
when building ideConfigurations originally) so setup uses the restored
preCollectedConfig (which fixes Codex using
options.preCollectedConfig?.installLocation). Update references around
config.ides, idesToRemove, ideConfigurations and savedIdeConfigs to ensure
re-added IDEs receive their saved config objects.
- Around line 705-755: The new IDE-removal block can throw if config.ides is
null and also produces noisy logs when skipPrompts/--yes is true; update the
block so that before pushing back canceled IDEs you ensure config.ides is
initialized (e.g., if (!config.ides) config.ides = []), and short-circuit the
entire removal flow when config.skipPrompts is true by silently restoring
idesToRemove into config.ides (no prompts/logs/spinner changes) instead of
running the warnings/confirm/cancel path; keep the existing removal logic
(this.ideManager.ensureInitialized, handler.cleanup,
this.ideConfigManager.deleteIdeConfig, prompts.log.success/warn) only when
skipPrompts is false and confirmRemoval is true.
🧹 Nitpick comments (2)
tools/cli/installers/lib/modules/manager.js (1)
1257-1288: Early returns yieldundefinedwhile the happy path returns an object — fragile contract.Lines 1268, 1275, 1283, and 1287 all
return;(i.e.,undefined), while line 1346 returns{ createdDirs, createdWdsFolders }. The caller ininstaller.jsguards withif (result), so it works today, but any future caller that destructures directly will get aTypeError.Consider returning the empty-arrays object from the early exits for a uniform contract:
Proposed fix
+ const emptyResult = { createdDirs: [], createdWdsFolders: [] }; + // Special handling for core module - it's in src/core not src/modules let sourcePath; if (moduleName === 'core') { sourcePath = getSourcePath('core'); } else { sourcePath = await this.findModuleSource(moduleName, { silent: true }); if (!sourcePath) { - return; // No source found, skip + return emptyResult; // No source found, skip } } // Read module.yaml to find the `directories` key const moduleYamlPath = path.join(sourcePath, 'module.yaml'); if (!(await fs.pathExists(moduleYamlPath))) { - return; // No module.yaml, skip + return emptyResult; // No module.yaml, skip } let moduleYaml; try { const yamlContent = await fs.readFile(moduleYamlPath, 'utf8'); moduleYaml = yaml.parse(yamlContent); } catch { - return; // Invalid YAML, skip + return emptyResult; // Invalid YAML, skip } if (!moduleYaml || !moduleYaml.directories) { - return; // No directories declared, skip + return emptyResult; // No directories declared, skip }tools/cli/installers/lib/core/installer.js (1)
1238-1239: Rawconsole.log()for spacing is inconsistent with the rest of the logging approach.The file uses
prompts.log.*everywhere else for user-facing output. A bareconsole.log()can produce unexpected results when console.log is temporarily suppressed (see line 1077:console.log = () => {};). Here it's fine because the suppression is restored by line 1119, but it's a latent risk if the suppression block is ever restructured.Consider
await prompts.log.message('');or confirming that the@clack/promptsspinner handles trailing spacing onspinner.stop().
- Restore saved IDE configurations when removal is cancelled or skipped in non-interactive mode, preventing silent config downgrade (e.g., Codex 'project' install reverting to 'global') - Short-circuit IDE removal block when skipPrompts is true to eliminate unnecessary warning/cancelled log output in --yes mode - Add null guard on config.ides before pushing re-added IDEs - Return empty result object from createModuleDirectories early exits instead of undefined for a uniform return contract Co-Authored-By: Claude Opus 4.6 <[email protected]>
…active mode Add skipPrompts guards to 4 remaining unguarded interactive prompts in installer.js that caused hangs in non-interactive mode (--yes flag): - Module removal confirmation: preserves modules by default - Update action selection: defaults to 'update' - Custom module missing sources: keeps all modules - Custom module delete confirmation: unreachable via early return Additional robustness fixes: - Defensive type-check before .trim() on prompt result (symbol guard) - Console.log suppression scoped per-IDE instead of global try/finally - process.exit flush via setImmediate for legacy v4 exit path - JSDoc updated for new skipPrompts parameter Follows established pattern from commit f967fdd (IDE skipPrompts guards). Co-Authored-By: Claude Opus 4.6 <[email protected]>
Replace all user-facing console.warn(), console.error(), and console.log()
calls with their prompts.log.* equivalents for consistent @clack/prompts
styled output across the installer codebase.
- Migrate 13 console.warn/error calls across 5 files to prompts.log.*
- Convert moduleLogger callbacks to async prompts.log.* in installer.js
- Replace blank-line console.log() with prompts.log.message('')
- Add prompts import to 5 files that lacked it
- Remove redundant "Warning:" prefixes (prompts.log.warn adds styling)
- Remove redundant local prompts require in installer.js L454
- Add missing await on logger.log call in manager.js
- Debug-gated console.log calls in manifest-generator.js left as-is
Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
Update: Two additional commits 1701788 — fix: guard remaining unguarded prompts with skipPrompts for non-interactive mode Ensures all interactive prompt calls are guarded by skipPrompts so bmad install --yes never hangs waiting for user input. Non-interactive mode now silently preserves existing modules/IDEs instead of prompting for removal confirmation. da37069 — fix: migrate installer console.warn/error calls to prompts.log API Replaces all remaining raw console.warn() / console.error() / console.log() calls in the installer with the unified prompts.log.* API so all user-facing output goes through @clack/prompts styling. Changes across 7 files:
Not changed (intentional):
|
… summary
Replace the serial spinner during non-interactive install phases with
@clack/prompts tasks() component for clearer progress visibility. The
install flow now uses two tasks() blocks (pre-IDE and post-IDE) with
the IDE setup retaining its own spinner since it may prompt.
- Refactor install phases into tasks() callbacks with message() updates
- Merge next-steps content into the "BMAD is ready to use!" summary note
- Fix spinner.stop() tense: "Reviewing..." → past tense ("reviewed")
- Move directory creation output after tasks() to avoid breaking progress
- Remove dead showInstallSummary() from ui.js
- Harden error handling: try/finally on IDE spinner, safe catch block
Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
Changes in 65b5afb The installer uses a single spinner cycling through ~36 messages across 15 install phases — users have no sense of progress, and no guidance on what to do after installation completes. Both are identified as critical CLI UX gaps by Evil Martians CLI UX best practices and Atlassian CLI Design Principles. |
Combine start and end marketing messages into one banner shown before installation begins. Remove the post-install end message and its displayEndMessage() calls — the install summary note now serves as the final output. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Co-Authored-By: Claude Opus 4.6 <[email protected]>
The old script-based module installer pattern was replaced with declarative directory creation, but the task title and summary label were never updated to reflect that change. Co-Authored-By: Claude Opus 4.6 <[email protected]>
…odes
Replace N individual "Accept Defaults?" confirm prompts with a single
select gateway ("Express Setup" vs "Customize"). When customizing, a
multiselect shows only modules with configurable options. All others
silently receive defaults via spinner progress.
- Add scanModuleSchemas() to pre-scan module metadata and display names
- Add select/multiselect gateway in collectAllConfigurations()
- Replace per-module confirm with modulesToCustomize Set check
- Suppress UI output during silent default config via _silentConfig flag
- Reorder installer tasks: module dirs before config generation
- Add resolution null guards for edge-case safety
- Cache ModuleManager instance via _getModuleManager() for reuse
Co-Authored-By: Claude Opus 4.6 <[email protected]>
When users modify a module's directory path during installer update/modify, the old directory is now moved to the new location instead of creating an empty directory while leaving the old one (with its documents) behind. Includes: cross-device fs.move, error handling with fallback, path normalization, parent/child path guard, and warning when both dirs exist. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
Update: 2f28ef1 feat: add batch module configuration gateway with express/customize modes Adds a configuration gateway for non-core modules during install. Users now choose between "Express Setup" (accept all defaults) and "Customize" (pick which modules to configure individually). Includes module schema pre-scanning, spinner-based silent config for default modules, and cached ModuleManager instance. Also moves directory creation task earlier in the install pipeline and adds null-safety guards for resolution data. 3287073 fix: move module directories instead of creating new ones on path change Fixes a bug where modifying a module's directory path during update/modify would create a new empty directory while leaving the old one (with user documents) behind — making those documents invisible to agents reading the new config. Changes:
|
…path Guard 5 interactive prompts in ui.js that caused the installer to hang when --yes flag was used with existing installations. Add skipPrompts field to 3 return objects that were missing it, ensuring installer.js downstream guards work correctly for all update paths. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
On e67d541 The BMAD CLI installer was hanging on interactive prompts when --yes flag was used with existing installations. The fresh install path already had options.yes guards on all prompts, but the update/modify path in ui.js had 5 unguarded prompts and 3 return objects missing the skipPrompts field. This fix adds guards to all 5 prompts (action menu, module selection, custom module confirm, tool selection upgrade, legacy version warning) using the established "safest non-destructive default" pattern, and adds skipPrompts: options.yes || false to the quick-update, compile-agents, and update return objects so downstream installer.js guards work correctly. |
|
Thanks @dracic ! |
2 participants
Summary
A collection of installer bug fixes addressing non-interactive mode (
--yes) hanging, output noise, and missing IDE cleanup during updates.Changes
1. fix: remove duplicate 'recompilation complete' message in compile-agents output
The
spinner.stop()incompileAgents()already displays this message, so the additionallog.success()call produced a redundant line.2. fix: remove deselected IDE configurations during installer update
When updating an existing installation, IDEs that were previously configured but unchecked in the new selection are now detected and cleaned up after user confirmation, mirroring the existing module removal flow.
3. chore: remove unreachable return statements after process.exit calls
Remove 4 dead
return;statements that immediately followprocess.exit(0)calls ininstall.js. Sinceprocess.exit()terminates the process, the subsequent return statements are unreachable.4. fix: batch directory creation log messages for cleaner installer output
Collect all created directory names during module directory setup and emit them as a single log message instead of one per directory. This eliminates the excessive blank-line spacing that
@clack/promptsadds between individuallog.message()calls.5. fix: consolidate directory creation output across all modules
Move directory creation logging from
ModuleManager.createModuleDirectoriesinto the installer caller. The method now returns created directory info instead of logging directly, allowing the installer to batch all module directories into a single log message under one spinner. Also adds spacing before the final "Installation complete" status line.6. fix: propagate skipPrompts flag to IDE collectConfiguration calls
The
--yesflag (skipPrompts) was not being passed through to IDE handlercollectConfiguration()calls, causing the Codex installer to hang on its interactive prompt in non-interactive mode (CI/CD,--yesflag).skipPromptsparameter tocollectToolConfigurations()and forward it tohandler.collectConfiguration()optionsCodexSetup.collectConfiguration()whenskipPromptsis true, defaulting to global install locationskipPromptscheck, defaulting to preserve existing configs (consistent with promptdefault: false)Files changed
tools/cli/commands/install.jstools/cli/installers/lib/core/installer.jstools/cli/installers/lib/ide/codex.jstools/cli/installers/lib/modules/manager.jsTest plan
npx bmad-method install --directory . --modules bmm --tools codex --yescompletes without hanging, defaults to global installnpx bmad-method install --tools codex(no--yes) still prompts for global vs. project-specificnpx bmad-method install --yes --tools codexon a project with other IDEs previously installed preserves those IDE configs (no silent deletion)npx bmad-method install --tools codexon a project with other IDEs shows removal confirmation promptnpx bmad-method install --tools codexthen update deselecting codex removes its config after confirmation