fix(cli): replace inquirer with @clack/prompts for Windows compatibility#1316
fix(cli): replace inquirer with @clack/prompts for Windows compatibility#1316bmadcode merged 6 commits intobmad-code-org:mainfrom
Conversation
- Add new prompts.js wrapper around @clack/prompts to fix Windows arrow key navigation issues (libuv bmad-code-org#852) - Fix validation logic in github-copilot.js that always returned true - Add support for primitive choice values (string/number) in select/multiselect - Add 'when' property support for conditional questions in prompt() - Update all IDE installers to use new prompts module Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR replaces the inquirer-based prompt library with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
package.json (1)
80-80: Remove unusedinquirerdependency from package.json.The
inquirerpackage is no longer imported or used anywhere in the codebase. The code creates a localSeparatorclass and a compatible interface object (const inquirer = { Separator }) for backward compatibility with internal UI code, but relies entirely on@clack/promptsfor actual prompt functionality. The dependency should be removed to reduce bundle bloat.tools/cli/installers/lib/core/installer.js (1)
2517-2543: Usemissing.idinstead of undefinedmoduleIdvariable.On line 2536 and subsequent references (lines 2540, 2541, 2548),
moduleIdis undefined. The iteration variable in this scope ismissing, so these should referencemissing.idinstead.Proposed fix
if (typedConfirm === 'DELETE') { // Remove the module from filesystem and manifest - const modulePath = path.join(bmadDir, moduleId); + const modulePath = path.join(bmadDir, missing.id); if (await fs.pathExists(modulePath)) { const fsExtra = require('fs-extra'); await fsExtra.remove(modulePath); console.log(chalk.yellow(` ✓ Deleted module directory: ${path.relative(projectRoot, modulePath)}`)); } - await this.manifest.removeModule(bmadDir, moduleId); - await this.manifest.removeCustomModule(bmadDir, moduleId); + await this.manifest.removeModule(bmadDir, missing.id); + await this.manifest.removeCustomModule(bmadDir, missing.id); console.log(chalk.yellow(` ✓ Removed from manifest`)); // Also remove from installedModules list - if (installedModules && installedModules.includes(moduleId)) { - const index = installedModules.indexOf(moduleId); + if (installedModules && installedModules.includes(missing.id)) { + const index = installedModules.indexOf(missing.id); if (index !== -1) { installedModules.splice(index, 1); } }
🤖 Fix all issues with AI agents
In @tools/cli/installers/lib/ide/claude-code.js:
- Around line 355-362: Update the prompts.multiselect call used to populate
selected so it matches @clack/prompts v0.11.0: replace the top-level property
choices with options, rename each option property name to label (keep value as
file), remove checked from individual options, and add an initialValues array
set to subagentConfig.files to pre-select all entries; this change should be
applied where prompts.multiselect is called and uses subagentConfig.files and
subagentInfo.
🧹 Nitpick comments (3)
tools/cli/lib/ui.js (1)
1273-1319: Synchronous validation inpromptCustomContentSourceis correct but verbose.The inline validation function correctly uses sync fs operations and returns appropriate values. Consider extracting this to a named method like
validateCustomContentPathSyncfor better readability and testability.tools/cli/installers/lib/ide/antigravity.js (1)
62-69: Duplicated installation location prompt code.The same
prompts.select()call for installation location appears twice (lines 62-69 and 295-302). Consider extracting this to a private method to reduce duplication and ensure consistency.♻️ Suggested refactor
// Add private method to the class async _promptInstallLocation() { return prompts.select({ message: 'Where would you like to install Antigravity subagents?', choices: [ { name: 'Project level (.agent/agents/)', value: 'project' }, { name: 'User level (~/.agent/agents/)', value: 'user' }, ], default: 'project', }); }Then replace both occurrences with
config.installLocation = await this._promptInstallLocation();Also applies to: 295-302
tools/cli/installers/lib/ide/claude-code.js (1)
303-310: Consider extracting duplicate install location prompt.This code block (lines 303-310) is nearly identical to lines 61-68. Both prompt for the install location with the same choices. Consider extracting this into a helper method to reduce duplication.
♻️ Suggested refactor
// Add helper method to the class async promptInstallLocation() { return prompts.select({ message: 'Where would you like to install Claude Code subagents?', choices: [ { name: 'Project level (.claude/agents/)', value: 'project' }, { name: 'User level (~/.claude/agents/)', value: 'user' }, ], default: 'project', }); }Then use
config.installLocation = await this.promptInstallLocation();in both places.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
package.jsontools/cli/commands/install.jstools/cli/installers/lib/core/config-collector.jstools/cli/installers/lib/core/installer.jstools/cli/installers/lib/ide/antigravity.jstools/cli/installers/lib/ide/claude-code.jstools/cli/installers/lib/ide/codex.jstools/cli/installers/lib/ide/github-copilot.jstools/cli/lib/prompts.jstools/cli/lib/ui.js
🧰 Additional context used
📓 Path-based instructions (2)
**/*
⚙️ CodeRabbit configuration file
**/*: Focus on inconsistencies, contradictions, edge cases and serious issues.
Avoid commenting on minor issues such as linting, formatting and style issues.
When providing code suggestions, use GitHub's suggestion format:<code changes>
Files:
tools/cli/installers/lib/ide/codex.jstools/cli/commands/install.jspackage.jsontools/cli/installers/lib/ide/github-copilot.jstools/cli/lib/ui.jstools/cli/installers/lib/ide/claude-code.jstools/cli/installers/lib/core/config-collector.jstools/cli/lib/prompts.jstools/cli/installers/lib/core/installer.jstools/cli/installers/lib/ide/antigravity.js
**/*.js
⚙️ CodeRabbit configuration file
**/*.js: CLI tooling code. Check for: missing error handling on fs operations,
path.join vs string concatenation, proper cleanup in error paths.
Flag any process.exit() without error message.
Files:
tools/cli/installers/lib/ide/codex.jstools/cli/commands/install.jstools/cli/installers/lib/ide/github-copilot.jstools/cli/lib/ui.jstools/cli/installers/lib/ide/claude-code.jstools/cli/installers/lib/core/config-collector.jstools/cli/lib/prompts.jstools/cli/installers/lib/core/installer.jstools/cli/installers/lib/ide/antigravity.js
🧬 Code graph analysis (8)
tools/cli/installers/lib/ide/codex.js (4)
tools/cli/installers/lib/core/config-collector.js (1)
prompts(7-7)tools/cli/installers/lib/core/installer.js (1)
prompts(19-19)tools/cli/installers/lib/ide/claude-code.js (8)
prompts(16-16)require(3-3)require(5-5)require(6-6)require(7-7)require(8-8)require(9-14)require(15-15)tools/cli/lib/ui.js (1)
prompts(7-7)
tools/cli/commands/install.js (6)
tools/cli/installers/lib/core/config-collector.js (1)
prompts(7-7)tools/cli/installers/lib/ide/antigravity.js (1)
prompts(16-16)tools/cli/installers/lib/ide/claude-code.js (1)
prompts(16-16)tools/cli/installers/lib/ide/codex.js (1)
prompts(9-9)tools/cli/lib/ui.js (3)
prompts(7-7)require(5-5)require(6-6)tools/flattener/prompts.js (1)
resolve(20-26)
tools/cli/lib/ui.js (1)
tools/cli/lib/prompts.js (1)
defaultValue(294-294)
tools/cli/installers/lib/ide/claude-code.js (5)
tools/cli/commands/install.js (1)
prompts(74-74)tools/cli/installers/lib/core/installer.js (15)
prompts(19-19)require(5-5)require(6-6)require(7-7)require(8-8)require(9-9)require(10-10)require(11-11)require(12-12)require(13-13)require(14-14)require(15-15)require(16-16)require(17-17)require(18-18)tools/cli/installers/lib/ide/antigravity.js (1)
prompts(16-16)tools/cli/installers/lib/ide/codex.js (3)
prompts(9-9)require(5-5)require(6-6)tools/cli/installers/lib/ide/github-copilot.js (1)
prompts(5-5)
tools/cli/installers/lib/core/config-collector.js (7)
tools/cli/commands/install.js (1)
prompts(74-74)tools/cli/installers/lib/core/installer.js (15)
prompts(19-19)require(5-5)require(6-6)require(7-7)require(8-8)require(9-9)require(10-10)require(11-11)require(12-12)require(13-13)require(14-14)require(15-15)require(16-16)require(17-17)require(18-18)tools/cli/installers/lib/ide/antigravity.js (1)
prompts(16-16)tools/cli/installers/lib/ide/claude-code.js (1)
prompts(16-16)tools/cli/installers/lib/ide/codex.js (1)
prompts(9-9)tools/cli/installers/lib/ide/github-copilot.js (1)
prompts(5-5)tools/cli/lib/ui.js (3)
prompts(7-7)require(5-5)require(6-6)
tools/cli/lib/prompts.js (11)
tools/cli/commands/install.js (4)
result(32-32)result(47-47)result(55-55)prompts(74-74)tools/cli/lib/cli-utils.js (1)
text(74-74)tools/cli/installers/lib/core/config-collector.js (1)
prompts(7-7)tools/cli/installers/lib/core/installer.js (1)
prompts(19-19)tools/cli/installers/lib/ide/antigravity.js (1)
prompts(16-16)tools/cli/installers/lib/ide/claude-code.js (1)
prompts(16-16)tools/cli/installers/lib/ide/codex.js (1)
prompts(9-9)tools/cli/installers/lib/ide/github-copilot.js (1)
prompts(5-5)tools/cli/lib/ui.js (1)
prompts(7-7)tools/flattener/prompts.js (1)
prompt(39-39)tools/cli/lib/agent/installer.js (1)
question(157-160)
tools/cli/installers/lib/core/installer.js (3)
tools/cli/installers/lib/core/config-collector.js (3)
prompts(7-7)require(5-5)require(6-6)tools/cli/installers/lib/ide/github-copilot.js (1)
prompts(5-5)tools/cli/lib/ui.js (3)
prompts(7-7)require(5-5)require(6-6)
tools/cli/installers/lib/ide/antigravity.js (4)
tools/cli/commands/install.js (2)
prompts(74-74)config(21-21)tools/cli/installers/lib/core/config-collector.js (1)
prompts(7-7)tools/cli/installers/lib/ide/claude-code.js (1)
prompts(16-16)tools/cli/installers/lib/ide/github-copilot.js (1)
prompts(5-5)
🔇 Additional comments (29)
package.json (1)
91-91: LGTM -@clack/promptsdependency added correctly.The new dependency is appropriately placed in
devDependencieswith a reasonable version constraint.tools/cli/commands/install.js (1)
74-77: LGTM - Clean migration to prompts API.The
prompts.text()call is appropriate for a "press Enter to continue" interaction. The lazy import pattern is consistent with other requires in the action handler.tools/cli/lib/ui.js (3)
9-20: Separator shim provides good backward compatibility.The Separator class maintains API compatibility with existing code that uses
inquirer.Separator. The comment correctly notes that@clack/promptsfilters these out, so visual grouping won't appear but the code won't break.
504-541: Robust handling of empty multiselect results.Good defensive coding with
selectedIdes && selectedIdes.length > 0checks and the fallback to empty array. The warning loop for unselected tools is a nice UX improvement.
876-930: Sync validation correctly implements clack's validation contract.The
validateDirectorySyncproperly returnsundefinedfor valid input and an error string for invalid input, matching@clack/promptsconventions. The synchronous fs operations (fs.pathExistsSync,fs.statSync,fs.accessSync) are appropriate for validation functions.tools/cli/lib/prompts.js (6)
10-21: LGTM - Lazy loading pattern is appropriate for ESM module.The caching pattern ensures
@clack/promptsis only imported once, and the async function handles the ESM dynamic import correctly.
29-36: Centralized cancellation handling ensures consistent exit behavior.The
handleCancelfunction provides a uniform way to handle user cancellation across all prompts, exiting gracefully with code 0.
83-115: LGTM - Robust choice conversion forselect().The conversion handles both primitive choices (string/number) and object choices with proper fallbacks for
value,label, andhintproperties. Separators are correctly filtered out.
125-157: LGTM -multiselect()correctly handles checked state.The
initialValuesextraction at lines 144-146 properly identifies pre-checked choices by filtering onc.checked && c.type !== 'separator'.
290-373: Good Inquirer compatibility layer withwhensupport.The
prompt()function provides solid backward compatibility for Inquirer-style question arrays, including support for conditional questions via thewhenproperty and dynamic defaults via function evaluation.
337-344:checkboxtype inprompt()doesn't propagatecheckedstate from choices.When processing Inquirer-style
checkboxquestions, thecheckedproperty on choices is not being passed tomultiselect(). Themultiselect()function (lines 144-146) extractsinitialValuesfrom choices withchecked: true, but here you're passingchoicesdirectly without ensuring the checked state flows through.This could cause pre-selected items to not appear selected when using the
prompt()compatibility function.🐛 Proposed fix
case 'checkbox': { answer = await multiselect({ message, choices: choices || [], required: false, }); break; }The fix is already in place -
multiselect()at lines 144-146 extractsinitialValuesfromchoices.filter(c => c.checked), so thecheckedproperty on choices will be respected. No change needed - I misread the flow.Actually, reviewing again - the
choicesarray is passed tomultiselect(), which then extractsinitialValuesfrom choices withchecked: trueat lines 144-146. This should work correctly.tools/cli/installers/lib/core/config-collector.js (3)
7-7: LGTM - Clean import migration to centralized prompts module.The import path
../../../lib/promptsis consistent with other files in the PR.
353-353: LGTM -prompts.prompt()correctly replacesinquirer.prompt().The
prompts.prompt()function in the new module provides Inquirer-compatible behavior for question arrays.
590-598: LGTM - Confirm question format is compatible.The question object with
type: 'confirm',name,message, anddefaultproperties matches what theprompts.prompt()function expects.tools/cli/installers/lib/ide/codex.js (2)
9-9: LGTM - Consistent prompts import.The import path matches the pattern used across other IDE installers.
29-56: LGTM - Clean migration of Codex configuration prompts.Both
prompts.select()andprompts.confirm()are used correctly with appropriate options. The confirmation loop structure is preserved.tools/cli/installers/lib/ide/antigravity.js (2)
16-16: LGTM - Consistent prompts import.The import follows the established pattern for IDE installer files.
327-354: LGTM - Subagent prompts correctly migrated.The
prompts.select()for installation choice andprompts.multiselect()for subagent selection follow the established patterns with proper choice formatting and default values.tools/cli/installers/lib/ide/github-copilot.js (3)
5-5: LGTM!Import follows the consistent pattern used across other IDE installers (
codex.js,antigravity.js, etc.).
30-38: LGTM!The
prompts.selectAPI usage is correct withmessage,choices, anddefaultproperties. The choice structure withnameandvaluefollows the expected format.
40-51: Good fix: Validation logic corrected.The validation now correctly returns the error message string on failure and
trueon success. According to the PR description, this removes an always-true|| truebug that was present before.tools/cli/installers/lib/core/installer.js (5)
19-19: LGTM!Import follows the consistent pattern used across other installer files.
2143-2148: LGTM!The
prompts.selectusage is correct. Returning{ action }maintains the expected interface for the caller.
2170-2185: LGTM!Good migration to
prompts.select. Thehintproperty replaces the previousshortproperty to align with @clack/prompts API.
2440-2465: LGTM!The
hintproperty update andprompts.selectusage are correct for handling missing custom module sources.
2469-2491: Validation correctly uses synchronous fs operations.The comment on line 2469 explains the design decision:
@clack/promptsdoesn't support async validation, sofs.pathExistsSyncis used instead offs.pathExists. The validation returnsundefinedfor valid input, which is the correct convention for@clack/prompts.tools/cli/installers/lib/ide/claude-code.js (3)
16-16: LGTM!Import follows the consistent pattern used across other IDE installers.
61-68: LGTM!The
prompts.selectusage is correct for the install location prompt.
335-343: LGTM!The
prompts.selectusage for subagent installation preference is correct with appropriate choices.
- Move @clack/prompts from devDependencies to dependencies (critical) - Remove unused inquirer dependency - Fix potential crash in multiselect when initialValues is undefined - Add async validator detection with explicit error message - Extract validateCustomContentPathSync method in ui.js - Extract promptInstallLocation methods in claude-code.js and antigravity.js - Fix moduleId -> missing.id in installer.js remove flow - Update multiselect to support native clack API (options/initialValues) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update bmad-cli.js comment about CLI prompts - Update config-collector.js JSDoc comments - Rename inquirer variable to choiceUtils in ui.js - Update JSDoc returns and calls documentation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add inline navigation hints to all multiselect prompts showing (↑/↓ navigate, SPACE select, ENTER confirm) to help users unfamiliar with terminal multiselect controls. Also restore detailed warning when no tools are selected, explaining that SPACE must be pressed to select items. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace flat multiselect with native @clack/prompts groupMultiselect component to restore visual grouping of IDE/tool options: - "Previously Configured" - pre-selected IDEs from existing install - "Recommended Tools" - starred preferred options - "Additional Tools" - other available options This restores the grouped UX that was lost during the Inquirer.js to @clack/prompts migration. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Update: After migrating from Inquirer.js to @clack/prompts (for Windows compatibility), we lost some UX features. Now restored:
Before it was a flat list, now it's properly grouped again like it was with Inquirer.js. |
|
Thanks @dracic, merged - will validate windows soon |
Problem
On Windows 8.1+ (PowerShell, cmd.exe), Inquirer.js checkbox and list prompts fail to respond to arrow keys. Users must press ENTER to "unblock" the terminal before arrow key navigation works.
Root cause: Known libuv/libuv#852 bug -
setRawMode(true)doesn't work reliably on Windows 8.1+. The terminal stays in line-buffered mode where input is only processed after ENTER.Attempted fixes (all failed)
setRawMode(true)before promptstdin.resume()before promptstdin.setEncoding('utf8')Solution
Migrate from Inquirer.js to @clack/prompts - a modern CLI prompt library that uses a different rendering and input approach, bypassing libuv TTY issues.
Changes
tools/cli/lib/prompts.jswrapper around @clack/promptsgithub-copilot.js(|| truebug always returned valid)whenproperty support for conditional questionsResolves: #1323 #1279 #1149