fix(installer): require --tools for fresh --yes installs; remove --tools none (#2346)
* fix(installer): require --tools for fresh --yes installs; remove --tools none (closes #2326) Fresh non-interactive installs without --tools previously produced a config-only install (~35 files vs ~1400 in the manifest) with no warning and a "BMAD is ready to use" success card, leaving slash commands unreachable. --tools none was an explicit opt-in for the same broken state. Now: fresh install + -y without --tools throws a helpful error pointing at --list-tools. --tools none is rejected as an unknown ID. Empty and typo'd tool IDs are also rejected. Existing-install paths (--action update, quick-update, modify) are unchanged - they continue to reuse previously-configured tools when --tools is omitted. Adds --list-tools flag that prints all 42 supported tool IDs (id, name, target_dir, preferred star) sourced from platform-codes.yaml. English docs updated; localized docs (vi-vn, fr, cs, etc.) will sync via the normal translation pass. * fix(installer): address review for #2326 — single source of truth, drop dead code, add tests - Refactor formatPlatformList to use IdeManager so --list-tools and --tools validation see the same set of platforms. Eliminates the drift where suspended platforms appeared in --list-tools but were rejected at validation. - Drop unused getValidPlatformIds export. - Flatten redundant block scope around the throw in the --yes-without-tools branch (refactor leftover). - Drop dead String() defensive cast (Commander always passes a string). - Add Test Suite 42: 8 unit tests covering _parseToolsFlag empty/whitespace/ unknown/typo cases plus an integration check that --list-tools output and --tools validation agree on the ID set. * fix(installer): close --tools "" bypass and drop hardcoded tool count - Replace truthy `if (options.tools)` guard with `!== undefined` in both upgrade and fresh-install branches. Empty string now reaches _parseToolsFlag and produces the specific "passed empty" error instead of falling through to a generic message (fresh-install) or being silently ignored (existing-install). - Drop the hardcoded "42 supported tools" count from the prereqs in install-bmad.md so the doc doesn't drift as platform-codes.yaml changes. Addresses augment / coderabbit review on #2346.
This commit is contained in:
Brian
GitHub
parent
3e89b30b3c
commit
7ee5fa313b
|
|
@ -18,7 +18,7 @@ Use `npx bmad-method install` to set up BMad in your project. One command handle
|
|||
|
||||
- **Node.js** 20+ (the installer requires it)
|
||||
- **Git** (for cloning external modules)
|
||||
- **An AI tool** such as Claude Code or Cursor — or install without one using `--tools none`
|
||||
- **An AI tool** such as Claude Code or Cursor (run `npx bmad-method install --list-tools` to see all supported tools)
|
||||
|
||||
:::
|
||||
|
||||
|
|
@ -122,7 +122,8 @@ Under `--yes`, patch and minor upgrades apply automatically. Majors stay frozen
|
|||
| `--yes`, `-y` | Skip all prompts; accept flag values + defaults |
|
||||
| `--directory <path>` | Install into this directory (default: current working dir) |
|
||||
| `--modules <a,b,c>` | Exact module set. Core is auto-added. Not a delta — list everything you want kept. |
|
||||
| `--tools <a,b>` or `--tools none` | IDE/tool selection. `none` skips tool config entirely. |
|
||||
| `--tools <a,b>` | IDE/tool selection. Required for fresh `--yes` installs. Run `--list-tools` for valid IDs. |
|
||||
| `--list-tools` | Print all supported tool/IDE IDs (with target directories) and exit. |
|
||||
| `--action <type>` | `install`, `update`, or `quick-update`. Defaults based on existing install state. |
|
||||
| `--custom-source <urls>` | Install custom modules from Git URLs or local paths |
|
||||
| `--channel <stable\|next>` | Apply to all externals (aliased as `--all-stable` / `--all-next`) |
|
||||
|
|
@ -165,17 +166,17 @@ npx bmad-method install --yes --modules bmm,bmb --all-next --tools claude-code
|
|||
|
||||
```bash
|
||||
npx bmad-method install --yes --action update \
|
||||
--modules bmm,bmb,gds \
|
||||
--tools none
|
||||
--modules bmm,bmb,gds
|
||||
```
|
||||
|
||||
`--tools` is omitted intentionally — `--action update` reuses the tools configured during the first install.
|
||||
|
||||
**Mix channels — bmb on next, gds on stable:**
|
||||
|
||||
```bash
|
||||
npx bmad-method install --yes --action update \
|
||||
--modules bmm,bmb,cis,gds \
|
||||
--next=bmb \
|
||||
--tools none
|
||||
--next=bmb
|
||||
```
|
||||
|
||||
:::caution[Rate limit on shared IPs]
|
||||
|
|
@ -204,7 +205,7 @@ For cross-machine reproducibility, don't rely on rerunning the same `--modules`
|
|||
|
||||
```bash
|
||||
npx bmad-method install --yes --modules bmb,cis \
|
||||
--pin bmb=v1.7.0 --pin cis=v0.4.2 --tools none
|
||||
--pin bmb=v1.7.0 --pin cis=v0.4.2 --tools claude-code
|
||||
```
|
||||
|
||||
## Troubleshooting
|
||||
|
|
|
|||
|
|
@ -2773,6 +2773,94 @@ async function runTests() {
|
|||
|
||||
console.log('');
|
||||
|
||||
// ============================================================
|
||||
// Test Suite 42: --tools flag parsing & validation (#2326)
|
||||
// ============================================================
|
||||
console.log(`${colors.yellow}Test Suite 42: --tools flag parsing & validation${colors.reset}\n`);
|
||||
try {
|
||||
const { UI } = require('../tools/installer/ui');
|
||||
const ui = new UI();
|
||||
const known = new Set(['claude-code', 'cursor', 'windsurf']);
|
||||
|
||||
assert(
|
||||
JSON.stringify(ui._parseToolsFlag('claude-code', known)) === JSON.stringify(['claude-code']),
|
||||
'parseToolsFlag returns single ID',
|
||||
);
|
||||
|
||||
assert(
|
||||
JSON.stringify(ui._parseToolsFlag('claude-code,cursor', known)) === JSON.stringify(['claude-code', 'cursor']),
|
||||
'parseToolsFlag returns multiple IDs',
|
||||
);
|
||||
|
||||
assert(
|
||||
JSON.stringify(ui._parseToolsFlag(' claude-code , cursor ', known)) === JSON.stringify(['claude-code', 'cursor']),
|
||||
'parseToolsFlag trims whitespace',
|
||||
);
|
||||
|
||||
let emptyErr;
|
||||
try {
|
||||
ui._parseToolsFlag('', known);
|
||||
} catch (error) {
|
||||
emptyErr = error;
|
||||
}
|
||||
assert(
|
||||
emptyErr && emptyErr.expected === true && /empty/i.test(emptyErr.message),
|
||||
'parseToolsFlag rejects empty string with expected=true',
|
||||
);
|
||||
|
||||
let commasOnlyErr;
|
||||
try {
|
||||
ui._parseToolsFlag(' , , ', known);
|
||||
} catch (error) {
|
||||
commasOnlyErr = error;
|
||||
}
|
||||
assert(commasOnlyErr && commasOnlyErr.expected === true, 'parseToolsFlag rejects whitespace/comma-only input');
|
||||
|
||||
let noneErr;
|
||||
try {
|
||||
ui._parseToolsFlag('none', known);
|
||||
} catch (error) {
|
||||
noneErr = error;
|
||||
}
|
||||
assert(noneErr && noneErr.expected === true && /Unknown tool ID/.test(noneErr.message), 'parseToolsFlag rejects "none" as unknown ID');
|
||||
|
||||
let typoErr;
|
||||
try {
|
||||
ui._parseToolsFlag('claude-code,claude-cdoe', known);
|
||||
} catch (error) {
|
||||
typoErr = error;
|
||||
}
|
||||
const typoHeader = typoErr ? typoErr.message.split('\n')[0] : '';
|
||||
assert(
|
||||
typoErr && typoErr.expected === true && /claude-cdoe/.test(typoHeader) && !/claude-code/.test(typoHeader),
|
||||
'parseToolsFlag reports only the unknown ID in error header (valid ones not listed as unknown)',
|
||||
);
|
||||
|
||||
// --list-tools and --tools validation must agree on what counts as a valid ID.
|
||||
const { formatPlatformList } = require('../tools/installer/ide/platform-codes');
|
||||
const { IdeManager } = require('../tools/installer/ide/manager');
|
||||
const ideManager42 = new IdeManager();
|
||||
await ideManager42.ensureInitialized();
|
||||
const validIds = new Set(ideManager42.getAvailableIdes().map((i) => i.value));
|
||||
const listed = await formatPlatformList();
|
||||
// Each entry line starts with ' *' (preferred) or ' ' (other), followed by the ID, then padding.
|
||||
const entryLines = listed.split('\n').filter((l) => /^( \*| {2})[a-z]/.test(l));
|
||||
const listedIds = entryLines.map((l) => l.trim().replace(/^\*/, '').split(/\s+/)[0]);
|
||||
const missingFromList = [...validIds].filter((id) => !listedIds.includes(id));
|
||||
const extraInList = listedIds.filter((id) => !validIds.has(id));
|
||||
assert(
|
||||
missingFromList.length === 0 && extraInList.length === 0,
|
||||
'--list-tools output matches the IDs that --tools accepts',
|
||||
`Missing from list: ${missingFromList.join(',') || '(none)'}; Extra in list: ${extraInList.join(',') || '(none)'}`,
|
||||
);
|
||||
} catch (error) {
|
||||
console.log(`${colors.red}Test Suite 42 setup failed: ${error.message}${colors.reset}`);
|
||||
console.log(error.stack);
|
||||
failed++;
|
||||
}
|
||||
|
||||
console.log('');
|
||||
|
||||
// ============================================================
|
||||
// Summary
|
||||
// ============================================================
|
||||
|
|
|
|||
|
|
@ -15,8 +15,9 @@ module.exports = {
|
|||
['--modules <modules>', 'Comma-separated list of module IDs to install (e.g., "bmm,bmb")'],
|
||||
[
|
||||
'--tools <tools>',
|
||||
'Comma-separated list of tool/IDE IDs to configure (e.g., "claude-code,cursor"). Use "none" to skip tool configuration.',
|
||||
'Comma-separated list of tool/IDE IDs to configure (e.g., "claude-code,cursor"). Required for fresh non-interactive (--yes) installs. Run with --list-tools to see all valid IDs.',
|
||||
],
|
||||
['--list-tools', 'Print all supported tool/IDE IDs (with target directories) and exit.'],
|
||||
['--action <type>', 'Action type for existing installations: install, update, or quick-update'],
|
||||
['--user-name <name>', 'Name for agents to use (default: system username)'],
|
||||
['--communication-language <lang>', 'Language for agent communication (default: English)'],
|
||||
|
|
@ -40,6 +41,12 @@ module.exports = {
|
|||
],
|
||||
action: async (options) => {
|
||||
try {
|
||||
if (options.listTools) {
|
||||
const { formatPlatformList } = require('../ide/platform-codes');
|
||||
process.stdout.write((await formatPlatformList()) + '\n');
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
// Set debug flag as environment variable for all components
|
||||
if (options.debug) {
|
||||
process.env.BMAD_DEBUG_MANIFEST = 'true';
|
||||
|
|
@ -81,7 +88,7 @@ module.exports = {
|
|||
} else {
|
||||
await prompts.log.error(`Installation failed: ${error.message}`);
|
||||
}
|
||||
if (error.stack) {
|
||||
if (error.stack && !error.expected) {
|
||||
await prompts.log.message(error.stack);
|
||||
}
|
||||
} catch {
|
||||
|
|
|
|||
|
|
@ -31,7 +31,50 @@ function clearCache() {
|
|||
_cachedPlatformCodes = null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Format the installable platform list for human-readable output (used by --list-tools).
|
||||
* Sourced from IdeManager so this view matches what --tools accepts at install time
|
||||
* (suspended platforms excluded).
|
||||
* @returns {Promise<string>} Formatted multi-line string with id, name, target_dir, preferred flag.
|
||||
*/
|
||||
async function formatPlatformList() {
|
||||
const { IdeManager } = require('./manager');
|
||||
const ideManager = new IdeManager();
|
||||
await ideManager.ensureInitialized();
|
||||
|
||||
const entries = ideManager.getAvailableIdes().map((ide) => {
|
||||
const handler = ideManager.handlers.get(ide.value);
|
||||
return {
|
||||
id: ide.value,
|
||||
name: ide.name,
|
||||
targetDir: handler?.installerConfig?.target_dir || '',
|
||||
preferred: ide.preferred,
|
||||
};
|
||||
});
|
||||
|
||||
const idWidth = Math.max(...entries.map((e) => e.id.length), 'ID'.length);
|
||||
const nameWidth = Math.max(...entries.map((e) => e.name.length), 'Name'.length);
|
||||
|
||||
const pad = (s, w) => s + ' '.repeat(Math.max(0, w - s.length));
|
||||
const lines = [
|
||||
`Supported tool IDs (pass via --tools <id>[,<id>...]):`,
|
||||
'',
|
||||
` ${pad('ID', idWidth)} ${pad('Name', nameWidth)} Target dir`,
|
||||
` ${pad('-'.repeat(idWidth), idWidth)} ${pad('-'.repeat(nameWidth), nameWidth)} ${'-'.repeat(10)}`,
|
||||
];
|
||||
|
||||
for (const e of entries) {
|
||||
const star = e.preferred ? ' *' : ' ';
|
||||
lines.push(`${star}${pad(e.id, idWidth)} ${pad(e.name, nameWidth)} ${e.targetDir}`);
|
||||
}
|
||||
|
||||
lines.push('', '* = recommended / preferred', '', 'Example: bmad-method install --modules bmm --tools claude-code');
|
||||
|
||||
return lines.join('\n');
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
loadPlatformCodes,
|
||||
clearCache,
|
||||
formatPlatformList,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -404,6 +404,37 @@ class UI {
|
|||
* @param {Object} options - Command-line options
|
||||
* @returns {Object} Tool configuration
|
||||
*/
|
||||
_parseToolsFlag(toolsArg, allKnownValues) {
|
||||
const selectedIdes = toolsArg
|
||||
.split(',')
|
||||
.map((t) => t.trim())
|
||||
.filter(Boolean);
|
||||
|
||||
if (selectedIdes.length === 0) {
|
||||
const err = new Error(
|
||||
'--tools was passed empty. Provide at least one tool ID (e.g. --tools claude-code) or run with --list-tools to see valid IDs.',
|
||||
);
|
||||
err.expected = true;
|
||||
throw err;
|
||||
}
|
||||
|
||||
const unknown = selectedIdes.filter((id) => !allKnownValues.has(id));
|
||||
if (unknown.length > 0) {
|
||||
const err = new Error(
|
||||
[
|
||||
`Unknown tool ID${unknown.length === 1 ? '' : 's'}: ${unknown.join(', ')}`,
|
||||
'',
|
||||
'Run with --list-tools to see all valid IDs.',
|
||||
'Common: claude-code, cursor, copilot, windsurf, cline',
|
||||
].join('\n'),
|
||||
);
|
||||
err.expected = true;
|
||||
throw err;
|
||||
}
|
||||
|
||||
return selectedIdes;
|
||||
}
|
||||
|
||||
async promptToolSelection(projectDir, options = {}) {
|
||||
const { ExistingInstall } = require('./core/existing-install');
|
||||
const { Installer } = require('./core/installer');
|
||||
|
|
@ -438,15 +469,10 @@ class UI {
|
|||
const allTools = [...preferredIdes, ...otherIdes];
|
||||
|
||||
// Non-interactive: handle --tools and --yes flags before interactive prompt
|
||||
if (options.tools) {
|
||||
if (options.tools.toLowerCase() === 'none') {
|
||||
await prompts.log.info('Skipping tool configuration (--tools none)');
|
||||
return { ides: [], skipIde: true };
|
||||
}
|
||||
const selectedIdes = options.tools
|
||||
.split(',')
|
||||
.map((t) => t.trim())
|
||||
.filter(Boolean);
|
||||
// Use !== undefined so an explicit --tools "" falls through to _parseToolsFlag and
|
||||
// gets a specific "passed empty" error instead of being silently ignored.
|
||||
if (options.tools !== undefined) {
|
||||
const selectedIdes = this._parseToolsFlag(options.tools, allKnownValues);
|
||||
await prompts.log.info(`Using tools from command-line: ${selectedIdes.join(', ')}`);
|
||||
await this.displaySelectedTools(selectedIdes, preferredIdes, allTools);
|
||||
return { ides: selectedIdes, skipIde: false };
|
||||
|
|
@ -522,21 +548,13 @@ class UI {
|
|||
|
||||
let selectedIdes = [];
|
||||
|
||||
// Check if tools are provided via command-line
|
||||
if (options.tools) {
|
||||
// Check for explicit "none" value to skip tool installation
|
||||
if (options.tools.toLowerCase() === 'none') {
|
||||
await prompts.log.info('Skipping tool configuration (--tools none)');
|
||||
return { ides: [], skipIde: true };
|
||||
} else {
|
||||
selectedIdes = options.tools
|
||||
.split(',')
|
||||
.map((t) => t.trim())
|
||||
.filter(Boolean);
|
||||
await prompts.log.info(`Using tools from command-line: ${selectedIdes.join(', ')}`);
|
||||
await this.displaySelectedTools(selectedIdes, preferredIdes, allTools);
|
||||
return { ides: selectedIdes, skipIde: false };
|
||||
}
|
||||
// Check if tools are provided via command-line.
|
||||
// Use !== undefined so an explicit --tools "" still hits _parseToolsFlag's empty-value error.
|
||||
if (options.tools !== undefined) {
|
||||
selectedIdes = this._parseToolsFlag(options.tools, allKnownValues);
|
||||
await prompts.log.info(`Using tools from command-line: ${selectedIdes.join(', ')}`);
|
||||
await this.displaySelectedTools(selectedIdes, preferredIdes, allTools);
|
||||
return { ides: selectedIdes, skipIde: false };
|
||||
} else if (options.yes) {
|
||||
// If --yes flag is set, skip tool prompt and use previously configured tools or empty
|
||||
if (configuredIdes.length > 0) {
|
||||
|
|
@ -544,8 +562,18 @@ class UI {
|
|||
await this.displaySelectedTools(configuredIdes, preferredIdes, allTools);
|
||||
return { ides: configuredIdes, skipIde: false };
|
||||
} else {
|
||||
await prompts.log.info('Skipping tool configuration (--yes flag, no previous tools)');
|
||||
return { ides: [], skipIde: true };
|
||||
const err = new Error(
|
||||
[
|
||||
'--tools is required for non-interactive install (--yes / -y) when no tools are previously configured.',
|
||||
'',
|
||||
'Common: claude-code, cursor, copilot, windsurf, cline',
|
||||
'See all supported tools: bmad-method install --list-tools',
|
||||
'',
|
||||
'Example: bmad-method install --modules bmm --tools claude-code -y',
|
||||
].join('\n'),
|
||||
);
|
||||
err.expected = true;
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue