Skip to content

fix(cli): resolve non-interactive install defaults for directory and output_folder #1963

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jschulte wants to merge 8 commits into
base: main
Choose a base branch
from
+157 −17
Open

fix(cli): resolve non-interactive install defaults for directory and output_folder #1963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1cdc7f4
fix(cli): resolve non-interactive install defaults for directory and …
jschulte Mar 13, 2026
fe2cbe9
Merge branch 'main' into fix/non-interactive-install-defaults
jschulte Mar 13, 2026
ac04378
Merge remote-tracking branch 'upstream/main' into fix/non-interactive…
jschulte Mar 13, 2026
d2e7158
Merge branch 'fix/non-interactive-install-defaults' of github.com:jsc…
jschulte Mar 13, 2026
7422d3a
fix(cli): address PR review feedback for non-interactive install defa…
jschulte Mar 13, 2026
5c0dfd8
Merge branch 'main' into fix/non-interactive-install-defaults
jschulte Mar 14, 2026
1571a3c
Merge branch 'main' into fix/non-interactive-install-defaults
jschulte Mar 25, 2026
8927e20
fix(cli): extract core-config-defaults module and centralize fallback…
jschulte Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions test/test-installation-components.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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
// ============================================================
Expand Down
89 changes: 89 additions & 0 deletions tools/cli/lib/core-config-defaults.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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,
};
52 changes: 35 additions & 17 deletions tools/cli/lib/ui.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -47,6 +48,21 @@ 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
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}`);
}
confirmedDirectory = cwd;
await prompts.log.info(`Using current directory (--yes flag): ${confirmedDirectory}`);
Comment on lines +51 to +65
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Guard process.cwd() in non-interactive mode.

If the working directory was removed between process start and this call, process.cwd() can throw and crash this path without installer-level context.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tools/cli/lib/ui.js` around lines 50 - 53, The call to process.cwd() in the
branch handling options.yes can throw if the current working directory was
removed; wrap the process.cwd() access in a try/catch around the
confirmedDirectory assignment in the same block (the code that sets
confirmedDirectory when options.yes is true), and on error handle it gracefully
by logging an error via prompts.log.error (or returning a clear failure) and
using a safe fallback (e.g., fallback to '.' or abort the installer) instead of
letting the exception crash the process; ensure the log message includes the
caught error details.

⚠️ Potential issue | 🟠 Major

Validate the --yes default directory before proceeding.

This branch skips validateDirectorySync, so non-writable or invalid cwd fails later with less actionable errors.

Proposed fix 
-    } 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 if (options.yes) {
+      // Default to current directory when --yes flag is set
+      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 {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tools/cli/lib/ui.js` around lines 50 - 53, The branch handling options.yes
should validate the chosen default directory before proceeding: after setting
confirmedDirectory = process.cwd() (in the options.yes branch of the CLI flow),
call validateDirectorySync(confirmedDirectory) and handle a validation failure
the same way other branches do (log the error via prompts.log.error or
processLogger and abort/exit). Ensure you reuse the same validation function
(validateDirectorySync) and error handling path so non-writable or invalid cwd
is detected early, and keep the existing informational log
prompts.log.info(`Using current directory (--yes flag): ${confirmedDirectory}`)
only after validation succeeds.

} else {
confirmedDirectory = await this.getConfirmedDirectory();
}
Expand Down Expand Up @@ -823,6 +839,14 @@ class UI {
return { existingInstall, installedModuleIds, bmadDir };
}

/**
* 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
*/
async getDefaultCoreConfig() {
return loadDefaultCoreConfig();
}

/**
* Collect core configuration
* @param {string} directory - Installation directory
Expand Down Expand Up @@ -866,27 +890,21 @@ class UI {
(!options.userName || !options.communicationLanguage || !options.documentOutputLanguage || !options.outputFolder)
) {
await configCollector.collectModuleConfig('core', directory, false, true);
} else if (options.yes) {
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)');
}
Comment on lines +893 to +899
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Backfill predicate still lets unresolved placeholders survive.

if (!configCollector.collectedConfig.core[key]) treats "{output_folder}" as valid (truthy), preserving broken placeholder values in --yes runs.

Proposed fix 
-        for (const [key, value] of Object.entries(defaults)) {
-          if (!configCollector.collectedConfig.core[key]) {
-            configCollector.collectedConfig.core[key] = value;
-          }
-        }
+        const isMissingOrUnresolved = (v) =>
+          v == null ||
+          (typeof v === 'string' && (v.trim() === '' || /^\{[^}]+\}$/.test(v.trim())));
+
+        for (const [key, value] of Object.entries(defaults)) {
+          if (isMissingOrUnresolved(configCollector.collectedConfig.core[key])) {
+            configCollector.collectedConfig.core[key] = value;
+          }
+        }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tools/cli/lib/ui.js` around lines 929 - 936, The backfill loop inside the
options.yes branch (using getDefaultCoreConfig and
configCollector.collectedConfig.core) treats placeholder strings like
"{output_folder}" as present; change the condition so it also considers
unresolved placeholders as missing — e.g., detect placeholder patterns (strings
matching /^\{.*\}$/ or containing un-replaced tokens) or add an isPlaceholder
helper and use it in the check (if value is falsy OR isPlaceholder(value))
before assigning the default for that key.

}
} else if (options.yes) {
// Use all defaults when --yes flag is set
await configCollector.loadExistingConfig(directory);
const existingConfig = configCollector.collectedConfig.core || {};

// 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',
};
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)');
}
Comment on lines 901 to 909
} else {
Expand Down