From 1cdc7f4a48bcf081676089e9545446315f08c672 Mon Sep 17 00:00:00 2001 From: Jonah Schulte Date: Fri, 13 Mar 2026 16:27:26 -0400 Subject: [PATCH 1/3] fix(cli): resolve non-interactive install defaults for directory and output_folder When using --yes with partial CLI options (e.g. --user-name without --output-folder), the installer had two bugs: 1. Still prompted for directory instead of defaulting to cwd 2. Left {output_folder} unresolved in module configs, creating a literal "{output_folder}" directory instead of "_bmad-output" Extract getDefaultCoreConfig() that reads defaults from src/core/module.yaml (single source of truth) and use it to backfill missing fields when --yes skips interactive prompts. --- tools/cli/lib/ui.js | 63 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 13 deletions(-) diff --git a/tools/cli/lib/ui.js b/tools/cli/lib/ui.js index 1338c1f17..4d0777be8 100644 --- a/tools/cli/lib/ui.js +++ b/tools/cli/lib/ui.js @@ -47,6 +47,10 @@ class UI { } confirmedDirectory = expandedDir; await prompts.log.info(`Using directory from command-line: ${confirmedDirectory}`); + } else if (options.yes) { + // Default to current directory when --yes flag is set + confirmedDirectory = process.cwd(); + await prompts.log.info(`Using current directory (--yes flag): ${confirmedDirectory}`); } else { confirmedDirectory = await this.getConfirmedDirectory(); } @@ -848,6 +852,43 @@ class UI { * @param {Object} options - Command-line options * @returns {Object} Core configuration */ + /** + * Get default core config values by reading from src/core/module.yaml + * @returns {Object} Default core config with user_name, communication_language, document_output_language, output_folder + */ + getDefaultCoreConfig() { + const { getModulePath } = require('./project-root'); + const yaml = require('yaml'); + + let safeUsername; + try { + safeUsername = os.userInfo().username; + } catch { + safeUsername = process.env.USER || process.env.USERNAME || 'User'; + } + const defaultUsername = safeUsername.charAt(0).toUpperCase() + safeUsername.slice(1); + + // Read defaults from core module.yaml (single source of truth) + try { + const moduleYamlPath = path.join(getModulePath('core'), 'module.yaml'); + const moduleConfig = yaml.parse(fs.readFileSync(moduleYamlPath, 'utf8')); + return { + user_name: defaultUsername, + communication_language: moduleConfig.communication_language?.default || 'English', + document_output_language: moduleConfig.document_output_language?.default || 'English', + output_folder: moduleConfig.output_folder?.default || '_bmad-output', + }; + } catch { + // Fallback if module.yaml is unreadable + return { + user_name: defaultUsername, + communication_language: 'English', + document_output_language: 'English', + output_folder: '_bmad-output', + }; + } + } + async collectCoreConfig(directory, options = {}) { const { ConfigCollector } = require('../installers/lib/core/config-collector'); const configCollector = new ConfigCollector(); @@ -885,6 +926,14 @@ class UI { (!options.userName || !options.communicationLanguage || !options.documentOutputLanguage || !options.outputFolder) ) { await configCollector.collectModuleConfig('core', directory, false, true); + } else if (options.yes) { + // Fill in defaults for any fields not provided via command-line or existing config + const defaults = this.getDefaultCoreConfig(); + for (const [key, value] of Object.entries(defaults)) { + if (!configCollector.collectedConfig.core[key]) { + configCollector.collectedConfig.core[key] = value; + } + } } } else if (options.yes) { // Use all defaults when --yes flag is set @@ -893,19 +942,7 @@ class UI { // If no existing config, use defaults if (Object.keys(existingConfig).length === 0) { - let safeUsername; - try { - safeUsername = os.userInfo().username; - } catch { - safeUsername = process.env.USER || process.env.USERNAME || 'User'; - } - const defaultUsername = safeUsername.charAt(0).toUpperCase() + safeUsername.slice(1); - configCollector.collectedConfig.core = { - user_name: defaultUsername, - communication_language: 'English', - document_output_language: 'English', - output_folder: '_bmad-output', - }; + configCollector.collectedConfig.core = this.getDefaultCoreConfig(); await prompts.log.info('Using default configuration (--yes flag)'); } } else { From 7422d3a0a3825c436168f6865632a7a1cccc9177 Mon Sep 17 00:00:00 2001 From: Jonah Schulte Date: Fri, 13 Mar 2026 16:53:45 -0400 Subject: [PATCH 2/3] fix(cli): address PR review feedback for non-interactive install defaults - Validate cwd with validateDirectorySync in --yes path - Fix JSDoc placement: move collectCoreConfig docs above its method - Read user_name default from module.yaml, fall back to OS username - Harden YAML defaults with type/empty-string normalization - Log warning on module.yaml load failure instead of silent catch - Fix backfill predicate to detect unresolved placeholders like {output_folder} - Always merge defaults into existing config in --yes mode --- tools/cli/lib/ui.js | 55 +++++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/tools/cli/lib/ui.js b/tools/cli/lib/ui.js index 4d0777be8..6aff08f6f 100644 --- a/tools/cli/lib/ui.js +++ b/tools/cli/lib/ui.js @@ -49,7 +49,12 @@ class UI { await prompts.log.info(`Using directory from command-line: ${confirmedDirectory}`); } else if (options.yes) { // Default to current directory when --yes flag is set - confirmedDirectory = process.cwd(); + const cwd = process.cwd(); + const validation = this.validateDirectorySync(cwd); + if (validation) { + throw new Error(`Invalid current directory: ${validation}`); + } + confirmedDirectory = cwd; await prompts.log.info(`Using current directory (--yes flag): ${confirmedDirectory}`); } else { confirmedDirectory = await this.getConfirmedDirectory(); @@ -846,12 +851,6 @@ class UI { return { existingInstall, installedModuleIds, bmadDir }; } - /** - * Collect core configuration - * @param {string} directory - Installation directory - * @param {Object} options - Command-line options - * @returns {Object} Core configuration - */ /** * Get default core config values by reading from src/core/module.yaml * @returns {Object} Default core config with user_name, communication_language, document_output_language, output_folder @@ -866,22 +865,24 @@ class UI { } catch { safeUsername = process.env.USER || process.env.USERNAME || 'User'; } - const defaultUsername = safeUsername.charAt(0).toUpperCase() + safeUsername.slice(1); + const osUsername = safeUsername.charAt(0).toUpperCase() + safeUsername.slice(1); + + const norm = (value, fallback) => (typeof value === 'string' && value.trim() !== '' ? value.trim() : fallback); // Read defaults from core module.yaml (single source of truth) try { const moduleYamlPath = path.join(getModulePath('core'), 'module.yaml'); const moduleConfig = yaml.parse(fs.readFileSync(moduleYamlPath, 'utf8')); return { - user_name: defaultUsername, - communication_language: moduleConfig.communication_language?.default || 'English', - document_output_language: moduleConfig.document_output_language?.default || 'English', - output_folder: moduleConfig.output_folder?.default || '_bmad-output', + user_name: norm(moduleConfig.user_name?.default, osUsername), + communication_language: norm(moduleConfig.communication_language?.default, 'English'), + document_output_language: norm(moduleConfig.document_output_language?.default, 'English'), + output_folder: norm(moduleConfig.output_folder?.default, '_bmad-output'), }; - } catch { - // Fallback if module.yaml is unreadable + } catch (error) { + console.warn(`Failed to load module.yaml, falling back to defaults: ${error.message}`); return { - user_name: defaultUsername, + user_name: osUsername, communication_language: 'English', document_output_language: 'English', output_folder: '_bmad-output', @@ -889,6 +890,12 @@ class UI { } } + /** + * Collect core configuration + * @param {string} directory - Installation directory + * @param {Object} options - Command-line options + * @returns {Object} Core configuration + */ async collectCoreConfig(directory, options = {}) { const { ConfigCollector } = require('../installers/lib/core/config-collector'); const configCollector = new ConfigCollector(); @@ -928,21 +935,31 @@ class UI { await configCollector.collectModuleConfig('core', directory, false, true); } else if (options.yes) { // Fill in defaults for any fields not provided via command-line or existing config + const isMissingOrUnresolved = (v) => v == null || (typeof v === 'string' && (v.trim() === '' || /^\{[^}]+\}$/.test(v.trim()))); + const defaults = this.getDefaultCoreConfig(); for (const [key, value] of Object.entries(defaults)) { - if (!configCollector.collectedConfig.core[key]) { + if (isMissingOrUnresolved(configCollector.collectedConfig.core[key])) { configCollector.collectedConfig.core[key] = value; } } } } else if (options.yes) { - // Use all defaults when --yes flag is set + // Use all defaults when --yes flag is set, merging with any existing config await configCollector.loadExistingConfig(directory); const existingConfig = configCollector.collectedConfig.core || {}; + const defaults = this.getDefaultCoreConfig(); + configCollector.collectedConfig.core = { ...defaults, ...existingConfig }; + + // Clean up any unresolved placeholder tokens from existing config + const isMissingOrUnresolved = (v) => v == null || (typeof v === 'string' && (v.trim() === '' || /^\{[^}]+\}$/.test(v.trim()))); + for (const [key, value] of Object.entries(configCollector.collectedConfig.core)) { + if (isMissingOrUnresolved(value)) { + configCollector.collectedConfig.core[key] = defaults[key]; + } + } - // If no existing config, use defaults if (Object.keys(existingConfig).length === 0) { - configCollector.collectedConfig.core = this.getDefaultCoreConfig(); await prompts.log.info('Using default configuration (--yes flag)'); } } else { From 8927e207e84a0c57a70e9318b602b3663a421c8f Mon Sep 17 00:00:00 2001 From: Jonah Schulte Date: Wed, 25 Mar 2026 00:31:01 -0400 Subject: [PATCH 3/3] fix(cli): extract core-config-defaults module and centralize fallback logic - Extract getDefaultCoreConfig, applyDefaultCoreConfig, and isMissingOrUnresolvedCoreConfigValue into core-config-defaults.js - Memoize YAML parsing with clearable cache for testing - Use prompts.log.warn instead of console.warn for error logging - Guard process.cwd() with try/catch in --yes path - Centralize handler.js placeholder replacement via shared defaults - Add tests for backfill logic and custom handler shared defaults --- test/test-installation-components.js | 33 ++++++++++ tools/cli/lib/core-config-defaults.js | 89 +++++++++++++++++++++++++++ tools/cli/lib/ui.js | 74 ++++++---------------- 3 files changed, 141 insertions(+), 55 deletions(-) create mode 100644 tools/cli/lib/core-config-defaults.js diff --git a/test/test-installation-components.js b/test/test-installation-components.js index 8b6f505de..2d840f713 100644 --- a/test/test-installation-components.js +++ b/test/test-installation-components.js @@ -15,6 +15,7 @@ const path = require('node:path'); const os = require('node:os'); const fs = require('fs-extra'); const { ConfigCollector } = require('../tools/cli/installers/lib/core/config-collector'); +const { applyDefaultCoreConfig, clearCoreConfigDefaultsCache, getDefaultCoreConfig } = require('../tools/cli/lib/core-config-defaults'); const { ManifestGenerator } = require('../tools/cli/installers/lib/core/manifest-generator'); const { IdeManager } = require('../tools/cli/installers/lib/ide/manager'); const { clearCache, loadPlatformCodes } = require('../tools/cli/installers/lib/ide/platform-codes'); @@ -2028,6 +2029,38 @@ async function runTests() { console.log(''); + // ============================================================ + // Test Suite 34: Core Config Default Backfill + // ============================================================ + console.log(`${colors.yellow}Test Suite 34: Core Config Default Backfill${colors.reset}\n`); + + try { + clearCoreConfigDefaultsCache(); + const defaults = await getDefaultCoreConfig(); + const normalized = applyDefaultCoreConfig( + { + user_name: '{user_name}', + communication_language: 'Spanish', + document_output_language: '', + output_folder: '{output_folder}', + }, + defaults, + ); + + assert(normalized.appliedDefaults === true, 'Core config backfill reports when defaults were applied'); + assert(normalized.coreConfig.user_name === defaults.user_name, 'Core config backfill replaces unresolved user_name placeholder'); + assert(normalized.coreConfig.communication_language === 'Spanish', 'Core config backfill preserves existing valid values'); + assert( + normalized.coreConfig.document_output_language === defaults.document_output_language, + 'Core config backfill replaces blank document output language', + ); + assert(normalized.coreConfig.output_folder === defaults.output_folder, 'Core config backfill replaces unresolved output_folder'); + } catch (error) { + assert(false, 'Core config default backfill test succeeds', error.message); + } + + console.log(''); + // ============================================================ // Summary // ============================================================ diff --git a/tools/cli/lib/core-config-defaults.js b/tools/cli/lib/core-config-defaults.js new file mode 100644 index 000000000..931722b3f --- /dev/null +++ b/tools/cli/lib/core-config-defaults.js @@ -0,0 +1,89 @@ +const os = require('node:os'); +const fs = require('fs-extra'); +const yaml = require('yaml'); +const prompts = require('./prompts'); +const { getModulePath } = require('./project-root'); + +let cachedCoreConfigDefaults = null; + +function getFallbackUsername() { + let safeUsername; + try { + safeUsername = os.userInfo().username; + } catch { + safeUsername = process.env.USER || process.env.USERNAME || 'User'; + } + + if (typeof safeUsername !== 'string' || safeUsername.trim() === '') { + return 'User'; + } + + const normalizedUsername = safeUsername.trim(); + return normalizedUsername.charAt(0).toUpperCase() + normalizedUsername.slice(1); +} + +function normalizeDefaultString(value, fallback) { + return typeof value === 'string' && value.trim() !== '' ? value.trim() : fallback; +} + +function isMissingOrUnresolvedCoreConfigValue(value) { + return value == null || (typeof value === 'string' && (value.trim() === '' || /^\{[^}]+\}$/.test(value.trim()))); +} + +function applyDefaultCoreConfig(coreConfig = {}, defaults = {}) { + const normalizedConfig = { ...coreConfig }; + let appliedDefaults = false; + + for (const [key, value] of Object.entries(defaults)) { + if (isMissingOrUnresolvedCoreConfigValue(normalizedConfig[key])) { + normalizedConfig[key] = value; + appliedDefaults = true; + } + } + + return { coreConfig: normalizedConfig, appliedDefaults }; +} + +async function getDefaultCoreConfig() { + if (cachedCoreConfigDefaults) { + return { ...cachedCoreConfigDefaults }; + } + + const fallbackDefaults = { + user_name: getFallbackUsername(), + communication_language: 'English', + document_output_language: 'English', + output_folder: '_bmad-output', + }; + + try { + const moduleYamlPath = getModulePath('core', 'module.yaml'); + const moduleConfig = yaml.parse(await fs.readFile(moduleYamlPath, 'utf8')) || {}; + + cachedCoreConfigDefaults = { + user_name: normalizeDefaultString(moduleConfig.user_name?.default, fallbackDefaults.user_name), + communication_language: normalizeDefaultString(moduleConfig.communication_language?.default, fallbackDefaults.communication_language), + document_output_language: normalizeDefaultString( + moduleConfig.document_output_language?.default, + fallbackDefaults.document_output_language, + ), + output_folder: normalizeDefaultString(moduleConfig.output_folder?.default, fallbackDefaults.output_folder), + }; + } catch (error) { + await prompts.log.warn(`Failed to load module.yaml, falling back to defaults: ${error.message}`); + cachedCoreConfigDefaults = fallbackDefaults; + } + + return { ...cachedCoreConfigDefaults }; +} + +function clearCoreConfigDefaultsCache() { + cachedCoreConfigDefaults = null; +} + +module.exports = { + applyDefaultCoreConfig, + clearCoreConfigDefaultsCache, + getDefaultCoreConfig, + isMissingOrUnresolvedCoreConfigValue, +}; diff --git a/tools/cli/lib/ui.js b/tools/cli/lib/ui.js index b0324fb47..d4805670a 100644 --- a/tools/cli/lib/ui.js +++ b/tools/cli/lib/ui.js @@ -2,6 +2,7 @@ const path = require('node:path'); const os = require('node:os'); const fs = require('fs-extra'); const { CLIUtils } = require('./cli-utils'); +const { applyDefaultCoreConfig, getDefaultCoreConfig: loadDefaultCoreConfig } = require('./core-config-defaults'); const { CustomHandler } = require('../installers/lib/custom/handler'); const { ExternalModuleManager } = require('../installers/lib/modules/external-manager'); const prompts = require('./prompts'); @@ -49,7 +50,13 @@ class UI { await prompts.log.info(`Using directory from command-line: ${confirmedDirectory}`); } else if (options.yes) { // Default to current directory when --yes flag is set - const cwd = process.cwd(); + let cwd; + try { + cwd = process.cwd(); + } catch (error) { + await prompts.log.error(`Failed to resolve current directory (--yes flag): ${error.message}`); + throw new Error(`Unable to determine current directory: ${error.message}`); + } const validation = this.validateDirectorySync(cwd); if (validation) { throw new Error(`Invalid current directory: ${validation}`); @@ -836,39 +843,8 @@ class UI { * Get default core config values by reading from src/core/module.yaml * @returns {Object} Default core config with user_name, communication_language, document_output_language, output_folder */ - getDefaultCoreConfig() { - const { getModulePath } = require('./project-root'); - const yaml = require('yaml'); - - let safeUsername; - try { - safeUsername = os.userInfo().username; - } catch { - safeUsername = process.env.USER || process.env.USERNAME || 'User'; - } - const osUsername = safeUsername.charAt(0).toUpperCase() + safeUsername.slice(1); - - const norm = (value, fallback) => (typeof value === 'string' && value.trim() !== '' ? value.trim() : fallback); - - // Read defaults from core module.yaml (single source of truth) - try { - const moduleYamlPath = path.join(getModulePath('core'), 'module.yaml'); - const moduleConfig = yaml.parse(fs.readFileSync(moduleYamlPath, 'utf8')); - return { - user_name: norm(moduleConfig.user_name?.default, osUsername), - communication_language: norm(moduleConfig.communication_language?.default, 'English'), - document_output_language: norm(moduleConfig.document_output_language?.default, 'English'), - output_folder: norm(moduleConfig.output_folder?.default, '_bmad-output'), - }; - } catch (error) { - console.warn(`Failed to load module.yaml, falling back to defaults: ${error.message}`); - return { - user_name: osUsername, - communication_language: 'English', - document_output_language: 'English', - output_folder: '_bmad-output', - }; - } + async getDefaultCoreConfig() { + return loadDefaultCoreConfig(); } /** @@ -915,32 +891,20 @@ class UI { ) { await configCollector.collectModuleConfig('core', directory, false, true); } else if (options.yes) { - // Fill in defaults for any fields not provided via command-line or existing config - const isMissingOrUnresolved = (v) => v == null || (typeof v === 'string' && (v.trim() === '' || /^\{[^}]+\}$/.test(v.trim()))); - - const defaults = this.getDefaultCoreConfig(); - for (const [key, value] of Object.entries(defaults)) { - if (isMissingOrUnresolved(configCollector.collectedConfig.core[key])) { - configCollector.collectedConfig.core[key] = value; - } + const defaults = await this.getDefaultCoreConfig(); + const normalizedConfig = applyDefaultCoreConfig(configCollector.collectedConfig.core, defaults); + configCollector.collectedConfig.core = normalizedConfig.coreConfig; + if (normalizedConfig.appliedDefaults) { + await prompts.log.info('Using default configuration (--yes flag)'); } } } else if (options.yes) { - // Use all defaults when --yes flag is set, merging with any existing config await configCollector.loadExistingConfig(directory); const existingConfig = configCollector.collectedConfig.core || {}; - const defaults = this.getDefaultCoreConfig(); - configCollector.collectedConfig.core = { ...defaults, ...existingConfig }; - - // Clean up any unresolved placeholder tokens from existing config - const isMissingOrUnresolved = (v) => v == null || (typeof v === 'string' && (v.trim() === '' || /^\{[^}]+\}$/.test(v.trim()))); - for (const [key, value] of Object.entries(configCollector.collectedConfig.core)) { - if (isMissingOrUnresolved(value)) { - configCollector.collectedConfig.core[key] = defaults[key]; - } - } - - if (Object.keys(existingConfig).length === 0) { + const defaults = await this.getDefaultCoreConfig(); + const normalizedConfig = applyDefaultCoreConfig(existingConfig, defaults); + configCollector.collectedConfig.core = normalizedConfig.coreConfig; + if (normalizedConfig.appliedDefaults) { await prompts.log.info('Using default configuration (--yes flag)'); } } else {