From 303e7ae290f96552fe24f28a3d8c673b31cadc29 Mon Sep 17 00:00:00 2001 From: Murat K Ozcan <34237651+muratkeremozcan@users.noreply.github.com> Date: Mon, 23 Mar 2026 15:55:19 -0500 Subject: [PATCH] fix: issue 55 config paths (#2113) * fix: issue 55 config paths * Fix: ci test failure --- test/test-installation-components.js | 88 +++++++++++ .../installers/lib/core/config-collector.js | 139 ++++++++++++++---- 2 files changed, 201 insertions(+), 26 deletions(-) diff --git a/test/test-installation-components.js b/test/test-installation-components.js index c5b04a1ee..d75ec9871 100644 --- a/test/test-installation-components.js +++ b/test/test-installation-components.js @@ -14,6 +14,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 { 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'); @@ -1853,6 +1854,93 @@ async function runTests() { console.log(''); + // ============================================================ + // Test Suite 33: ConfigCollector Prompt Normalization + // ============================================================ + console.log(`${colors.yellow}Test Suite 33: ConfigCollector Prompt Normalization${colors.reset}\n`); + + try { + const teaModuleConfig33 = { + test_artifacts: { + default: '_bmad-output/test-artifacts', + }, + test_design_output: { + prompt: 'Where should test design documents be stored?', + default: 'test-design', + result: '{test_artifacts}/{value}', + }, + test_review_output: { + prompt: 'Where should test review reports be stored?', + default: 'test-reviews', + result: '{test_artifacts}/{value}', + }, + trace_output: { + prompt: 'Where should traceability reports be stored?', + default: 'traceability', + result: '{test_artifacts}/{value}', + }, + }; + + const collector33 = new ConfigCollector(); + collector33.currentProjectDir = path.join(os.tmpdir(), 'bmad-config-normalization'); + collector33.allAnswers = {}; + collector33.collectedConfig = { + tea: { + test_artifacts: '_bmad-output/test-artifacts', + }, + }; + collector33.existingConfig = { + tea: { + test_artifacts: '_bmad-output/test-artifacts', + test_design_output: '_bmad-output/test-artifacts/test-design', + test_review_output: '_bmad-output/test-artifacts/test-reviews', + trace_output: '_bmad-output/test-artifacts/traceability', + }, + }; + + const testDesignQuestion33 = await collector33.buildQuestion( + 'tea', + 'test_design_output', + teaModuleConfig33.test_design_output, + teaModuleConfig33, + ); + const testReviewQuestion33 = await collector33.buildQuestion( + 'tea', + 'test_review_output', + teaModuleConfig33.test_review_output, + teaModuleConfig33, + ); + const traceQuestion33 = await collector33.buildQuestion('tea', 'trace_output', teaModuleConfig33.trace_output, teaModuleConfig33); + + assert(testDesignQuestion33.default === 'test-design', 'ConfigCollector normalizes existing test_design_output prompt default'); + assert(testReviewQuestion33.default === 'test-reviews', 'ConfigCollector normalizes existing test_review_output prompt default'); + assert(traceQuestion33.default === 'traceability', 'ConfigCollector normalizes existing trace_output prompt default'); + + collector33.allAnswers = { + tea_test_artifacts: '_bmad-output/test-artifacts', + }; + + assert( + collector33.processResultTemplate(teaModuleConfig33.test_design_output.result, testDesignQuestion33.default) === + '_bmad-output/test-artifacts/test-design', + 'ConfigCollector re-applies test_design_output template without duplicating prefix', + ); + assert( + collector33.processResultTemplate(teaModuleConfig33.test_review_output.result, testReviewQuestion33.default) === + '_bmad-output/test-artifacts/test-reviews', + 'ConfigCollector re-applies test_review_output template without duplicating prefix', + ); + assert( + collector33.processResultTemplate(teaModuleConfig33.trace_output.result, traceQuestion33.default) === + '_bmad-output/test-artifacts/traceability', + 'ConfigCollector re-applies trace_output template without duplicating prefix', + ); + } catch (error) { + assert(false, 'ConfigCollector prompt normalization test succeeds', error.message); + } + + console.log(''); + // ============================================================ // Summary // ============================================================ diff --git a/tools/cli/installers/lib/core/config-collector.js b/tools/cli/installers/lib/core/config-collector.js index e8569cd0f..665c7957a 100644 --- a/tools/cli/installers/lib/core/config-collector.js +++ b/tools/cli/installers/lib/core/config-collector.js @@ -954,31 +954,123 @@ class ConfigCollector { return match; } - // Look for the config value in allAnswers (already answered questions) - let configValue = this.allAnswers[configKey] || this.allAnswers[`core_${configKey}`]; - - // Check in already collected config - if (!configValue) { - for (const mod of Object.keys(this.collectedConfig)) { - if (mod !== '_meta' && this.collectedConfig[mod] && this.collectedConfig[mod][configKey]) { - configValue = this.collectedConfig[mod][configKey]; - break; - } - } - } - - // If still not found and we're in the same module, use the default from the config schema - if (!configValue && currentModule && moduleConfig && moduleConfig[configKey]) { - const referencedItem = moduleConfig[configKey]; - if (referencedItem && referencedItem.default !== undefined) { - configValue = referencedItem.default; - } - } + const configValue = this.resolveConfigValue(configKey, currentModule, moduleConfig); return configValue || match; }); } + /** + * Clean a stored path-like value for prompt display/input reuse. + * @param {*} value - Stored value + * @returns {*} Cleaned value + */ + cleanPromptValue(value) { + if (typeof value === 'string' && value.startsWith('{project-root}/')) { + return value.replace('{project-root}/', ''); + } + + return value; + } + + /** + * Resolve a config key from answers, collected config, existing config, or schema defaults. + * @param {string} configKey - Config key to resolve + * @param {string} currentModule - Current module name + * @param {Object} moduleConfig - Current module config schema + * @returns {*} Resolved value + */ + resolveConfigValue(configKey, currentModule = null, moduleConfig = null) { + // Look for the config value in allAnswers (already answered questions) + let configValue = this.allAnswers?.[configKey] || this.allAnswers?.[`core_${configKey}`]; + + if (!configValue && this.allAnswers) { + for (const [answerKey, answerValue] of Object.entries(this.allAnswers)) { + if (answerKey.endsWith(`_${configKey}`)) { + configValue = answerValue; + break; + } + } + } + + // Prefer the current module's persisted value when re-prompting an existing install + if (!configValue && currentModule && this.existingConfig?.[currentModule]?.[configKey] !== undefined) { + configValue = this.existingConfig[currentModule][configKey]; + } + + // Check in already collected config + if (!configValue) { + for (const mod of Object.keys(this.collectedConfig)) { + if (mod !== '_meta' && this.collectedConfig[mod] && this.collectedConfig[mod][configKey]) { + configValue = this.collectedConfig[mod][configKey]; + break; + } + } + } + + // Fall back to other existing module config values + if (!configValue && this.existingConfig) { + for (const mod of Object.keys(this.existingConfig)) { + if (mod !== '_meta' && this.existingConfig[mod] && this.existingConfig[mod][configKey]) { + configValue = this.existingConfig[mod][configKey]; + break; + } + } + } + + // If still not found and we're in the same module, use the default from the config schema + if (!configValue && currentModule && moduleConfig && moduleConfig[configKey]) { + const referencedItem = moduleConfig[configKey]; + if (referencedItem && referencedItem.default !== undefined) { + configValue = referencedItem.default; + } + } + + return this.cleanPromptValue(configValue); + } + + /** + * Convert an existing stored value back into the prompt-facing value for templated fields. + * For example, "{test_artifacts}/{value}" + "_bmad-output/test-artifacts/test-design" + * becomes "test-design" so the template is not applied twice on modify. + * @param {*} existingValue - Stored config value + * @param {string} moduleName - Module name + * @param {Object} item - Config item definition + * @param {Object} moduleConfig - Current module config schema + * @returns {*} Prompt-facing default value + */ + normalizeExistingValueForPrompt(existingValue, moduleName, item, moduleConfig = null) { + const cleanedValue = this.cleanPromptValue(existingValue); + + if (typeof cleanedValue !== 'string' || typeof item?.result !== 'string' || !item.result.includes('{value}')) { + return cleanedValue; + } + + const [prefixTemplate = '', suffixTemplate = ''] = item.result.split('{value}'); + const prefix = this.cleanPromptValue(this.replacePlaceholders(prefixTemplate, moduleName, moduleConfig)); + const suffix = this.cleanPromptValue(this.replacePlaceholders(suffixTemplate, moduleName, moduleConfig)); + + if ((prefix && !cleanedValue.startsWith(prefix)) || (suffix && !cleanedValue.endsWith(suffix))) { + return cleanedValue; + } + + const startIndex = prefix.length; + const endIndex = suffix ? cleanedValue.length - suffix.length : cleanedValue.length; + if (endIndex < startIndex) { + return cleanedValue; + } + + let promptValue = cleanedValue.slice(startIndex, endIndex); + if (promptValue.startsWith('/')) { + promptValue = promptValue.slice(1); + } + if (promptValue.endsWith('/')) { + promptValue = promptValue.slice(0, -1); + } + + return promptValue || cleanedValue; + } + /** * Build a prompt question from a config item * @param {string} moduleName - Module name @@ -993,12 +1085,7 @@ class ConfigCollector { let existingValue = null; if (this.existingConfig && this.existingConfig[moduleName]) { existingValue = this.existingConfig[moduleName][key]; - - // Clean up existing value - remove {project-root}/ prefix if present - // This prevents duplication when the result template adds it back - if (typeof existingValue === 'string' && existingValue.startsWith('{project-root}/')) { - existingValue = existingValue.replace('{project-root}/', ''); - } + existingValue = this.normalizeExistingValueForPrompt(existingValue, moduleName, item, moduleConfig); } // Special handling for user_name: default to system user