From b37ca3b3142fe716f3485506c2f24b50d913e129 Mon Sep 17 00:00:00 2001 From: Brian Madison Date: Mon, 27 Apr 2026 23:20:52 -0500 Subject: [PATCH] fix(config): promote project_name to core, fixes #2279 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit project_name was a bmm-specific prompt despite being a universal project-level concept used by every module — including core skills like bmad-brainstorming, which loads from _bmad/core/config.yaml and was silently broken because project_name lived under bmm. Users without bmm installed could not run brainstorming at all. Move: - src/core-skills/module.yaml: declare project_name with prompt "What is your project called?" and default {directory_name}, matching what bmm previously had. - src/bmm-skills/module.yaml: remove the bmm definition; add project_name to the "Variables from Core Config inserted" header comment so contributors can see what's inherited. Migration for existing installs: - tools/installer/modules/official-modules.js: after loadExistingConfig reads each per-module config.yaml, hoist any keys that are now declared in core but appear under non-core modules. Without this, the partition logic in writeCentralConfig (which strips core keys from non-core buckets) would silently drop the user's prior project_name on the next quick-update. Generic — handles project_name today and any future module→core promotions. - The hoist preserves precedence: an existing core value beats a stale module-side copy. --yes seed: - tools/installer/ui.js: add project_name to the hardcoded core seed (using path.basename(directory) to match the {directory_name} default) so non-interactive fresh installs populate it. Without this the seed silently omits project_name and core skills fall back to literals. Tests: - test/test-installation-components.js Suite 43 (9 assertions) covers the schema move, the loadExistingConfig hoist, and the precedence rule. - Suite 35 fixture updated: project_name moved from bmm bucket to core, with a stale bmm copy left in place to verify it gets stripped. Verified manually: - Fresh install -y: project_name lands in [core] of config.toml. - Existing install with project_name in bmm/config.yaml: quick-update hoists it to [core] and strips it from [modules.bmm]. --- src/bmm-skills/module.yaml | 6 +- src/core-skills/module.yaml | 5 ++ test/test-installation-components.js | 96 ++++++++++++++++++++- tools/installer/modules/official-modules.js | 43 +++++++++ tools/installer/ui.js | 3 + 5 files changed, 146 insertions(+), 7 deletions(-) diff --git a/src/bmm-skills/module.yaml b/src/bmm-skills/module.yaml index cf3232614..490de183c 100644 --- a/src/bmm-skills/module.yaml +++ b/src/bmm-skills/module.yaml @@ -5,15 +5,11 @@ default_selected: true # This module will be selected by default for new install # Variables from Core Config inserted: ## user_name +## project_name ## communication_language ## document_output_language ## output_folder -project_name: - prompt: "What is your project called?" - default: "{directory_name}" - result: "{value}" - user_skill_level: prompt: - "What is your development experience level?" diff --git a/src/core-skills/module.yaml b/src/core-skills/module.yaml index 0ccc68a78..b2b2650fb 100644 --- a/src/core-skills/module.yaml +++ b/src/core-skills/module.yaml @@ -11,6 +11,11 @@ user_name: default: "BMad" result: "{value}" +project_name: + prompt: "What is your project called?" + default: "{directory_name}" + result: "{value}" + communication_language: prompt: "What language should agents use when chatting with you?" scope: user diff --git a/test/test-installation-components.js b/test/test-installation-components.js index f63f1b446..b7beec9ff 100644 --- a/test/test-installation-components.js +++ b/test/test-installation-components.js @@ -1813,12 +1813,12 @@ async function runTests() { const moduleConfigs = { core: { user_name: 'TestUser', + project_name: 'demo-project', communication_language: 'Spanish', document_output_language: 'English', output_folder: '_bmad-output', }, bmm: { - project_name: 'demo-project', user_skill_level: 'expert', planning_artifacts: '{project-root}/_bmad-output/planning-artifacts', implementation_artifacts: '{project-root}/_bmad-output/implementation-artifacts', @@ -1826,7 +1826,10 @@ async function runTests() { // Spread-from-core pollution: legacy per-module config.yaml merges // core values into every module; writeCentralConfig must strip these // from [modules.bmm] so core values only live in [core]. + // project_name is now a core key (#2279), so it joins user_name etc. + // as a spread-from-core key that must be stripped. user_name: 'TestUser', + project_name: 'stale-bmm-copy', communication_language: 'Spanish', document_output_language: 'English', output_folder: '_bmad-output', @@ -1874,6 +1877,7 @@ async function runTests() { assert(teamContent.includes('[core]'), 'config.toml has [core] section'); assert(teamContent.includes('document_output_language = "English"'), 'Team-scope core key lands in config.toml'); assert(teamContent.includes('output_folder = "_bmad-output"'), 'Team-scope output_folder lands in config.toml'); + assert(teamContent.includes('project_name = "demo-project"'), 'project_name lands in [core] (core key as of #2279)'); assert(!teamContent.includes('user_name'), 'user_name (scope: user) is absent from config.toml'); assert(!teamContent.includes('communication_language'), 'communication_language (scope: user) is absent from config.toml'); @@ -1888,7 +1892,9 @@ async function runTests() { assert(bmmTeamMatch !== null, 'config.toml has [modules.bmm] section'); if (bmmTeamMatch) { const bmmTeamBlock = bmmTeamMatch[0]; - assert(bmmTeamBlock.includes('project_name = "demo-project"'), 'bmm team-scope key lands under [modules.bmm]'); + assert(bmmTeamBlock.includes('planning_artifacts'), 'bmm-owned team-scope key (planning_artifacts) lands under [modules.bmm]'); + assert(!bmmTeamBlock.includes('project_name'), 'project_name stripped from [modules.bmm] (now a core key, #2279)'); + assert(!bmmTeamBlock.includes('stale-bmm-copy'), 'stale bmm-copy of project_name not leaked into config.toml'); assert(!bmmTeamBlock.includes('user_name'), 'user_name stripped from [modules.bmm] (core-key pollution)'); assert(!bmmTeamBlock.includes('communication_language'), 'communication_language stripped from [modules.bmm]'); assert(!bmmTeamBlock.includes('user_skill_level'), 'user_skill_level (scope: user) absent from [modules.bmm] in config.toml'); @@ -2861,6 +2867,92 @@ async function runTests() { console.log(''); + // ============================================================ + // Test Suite 43: project_name promoted to core + hoist migration (#2279) + // ============================================================ + console.log(`${colors.yellow}Test Suite 43: project_name in core + hoist migration${colors.reset}\n`); + try { + const yamlLib = require('yaml'); + const coreSchemaPath = path.join(__dirname, '..', 'src', 'core-skills', 'module.yaml'); + const bmmSchemaPath = path.join(__dirname, '..', 'src', 'bmm-skills', 'module.yaml'); + const coreSchema = yamlLib.parse(await fs.readFile(coreSchemaPath, 'utf8')); + const bmmSchema = yamlLib.parse(await fs.readFile(bmmSchemaPath, 'utf8')); + + assert( + coreSchema.project_name && coreSchema.project_name.prompt && coreSchema.project_name.default === '{directory_name}', + 'core/module.yaml declares project_name with {directory_name} default', + ); + + assert(coreSchema.project_name.scope === undefined, 'project_name has no user scope (project-scoped, not user-scoped)'); + + assert(bmmSchema.project_name === undefined, 'bmm/module.yaml no longer declares project_name (now inherited from core)'); + + // Set up a mock existing install: bmm directory has project_name (legacy), + // core has user_name but not project_name. After hoist, project_name should + // move to core, leaving bmm with only its own keys. + const fixtureRoot43 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-fixture-43-')); + const bmadDir43 = path.join(fixtureRoot43, '_bmad'); + await fs.ensureDir(path.join(bmadDir43, '_config')); + await fs.writeFile(path.join(bmadDir43, '_config', 'manifest.yaml'), 'modules: []\n', 'utf8'); + await fs.ensureDir(path.join(bmadDir43, 'core')); + await fs.ensureDir(path.join(bmadDir43, 'bmm')); + await fs.writeFile(path.join(bmadDir43, 'core', 'config.yaml'), 'user_name: alice\n', 'utf8'); + await fs.writeFile( + path.join(bmadDir43, 'bmm', 'config.yaml'), + 'project_name: legacy-from-bmm\nuser_skill_level: intermediate\n', + 'utf8', + ); + + const officialModules43 = new OfficialModules(); + await officialModules43.loadExistingConfig(fixtureRoot43); + + assert( + officialModules43.existingConfig.core?.project_name === 'legacy-from-bmm', + 'loadExistingConfig hoists bmm.project_name to core on existing-install upgrade', + ); + + assert( + !('project_name' in (officialModules43.existingConfig.bmm || {})), + 'loadExistingConfig removes project_name from bmm after hoisting', + ); + + assert( + officialModules43.existingConfig.bmm?.user_skill_level === 'intermediate', + 'loadExistingConfig leaves non-core bmm keys (user_skill_level) untouched', + ); + + assert(officialModules43.existingConfig.core?.user_name === 'alice', 'loadExistingConfig preserves pre-existing core values'); + + // Precedence: if core already has the key, hoist must NOT overwrite it. + const fixtureRoot43b = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-fixture-43b-')); + const bmadDir43b = path.join(fixtureRoot43b, '_bmad'); + await fs.ensureDir(path.join(bmadDir43b, '_config')); + await fs.writeFile(path.join(bmadDir43b, '_config', 'manifest.yaml'), 'modules: []\n', 'utf8'); + await fs.ensureDir(path.join(bmadDir43b, 'core')); + await fs.ensureDir(path.join(bmadDir43b, 'bmm')); + await fs.writeFile(path.join(bmadDir43b, 'core', 'config.yaml'), 'project_name: from-core\n', 'utf8'); + await fs.writeFile(path.join(bmadDir43b, 'bmm', 'config.yaml'), 'project_name: stale-from-bmm\n', 'utf8'); + + const officialModules43b = new OfficialModules(); + await officialModules43b.loadExistingConfig(fixtureRoot43b); + + assert(officialModules43b.existingConfig.core?.project_name === 'from-core', 'hoist does not overwrite an existing core value'); + + assert( + !('project_name' in (officialModules43b.existingConfig.bmm || {})), + 'hoist still strips the duplicate from bmm so writeCentralConfig partition stays clean', + ); + + await fs.remove(fixtureRoot43).catch(() => {}); + await fs.remove(fixtureRoot43b).catch(() => {}); + } catch (error) { + console.log(`${colors.red}Test Suite 43 setup failed: ${error.message}${colors.reset}`); + console.log(error.stack); + failed++; + } + + console.log(''); + // ============================================================ // Summary // ============================================================ diff --git a/tools/installer/modules/official-modules.js b/tools/installer/modules/official-modules.js index 4bd1e56b3..40c885360 100644 --- a/tools/installer/modules/official-modules.js +++ b/tools/installer/modules/official-modules.js @@ -914,9 +914,52 @@ class OfficialModules { } } + if (foundAny) { + await this._hoistCoreKeysFromLegacyModuleConfigs(); + } + return foundAny; } + /** + * Migrate prior answers when a key has moved from a non-core module to core + * (e.g. project_name moving from bmm to core in #2279). Without this, the + * partition logic in writeCentralConfig drops the value from the bmm bucket + * (because it's now a core key) without re-homing it under [core], so the + * user's prior answer silently disappears on the next install/quick-update. + */ + async _hoistCoreKeysFromLegacyModuleConfigs() { + const coreSchemaPath = path.join(getSourcePath(), 'core-skills', 'module.yaml'); + if (!(await fs.pathExists(coreSchemaPath))) return; + + let coreSchema; + try { + coreSchema = yaml.parse(await fs.readFile(coreSchemaPath, 'utf8')); + } catch { + return; + } + if (!coreSchema || typeof coreSchema !== 'object') return; + + const coreKeys = new Set( + Object.entries(coreSchema) + .filter(([, v]) => v && typeof v === 'object' && 'prompt' in v) + .map(([k]) => k), + ); + if (coreKeys.size === 0) return; + + this._existingConfig.core = this._existingConfig.core || {}; + for (const [moduleName, cfg] of Object.entries(this._existingConfig)) { + if (moduleName === 'core' || !cfg || typeof cfg !== 'object') continue; + for (const key of Object.keys(cfg)) { + if (!coreKeys.has(key)) continue; + if (!(key in this._existingConfig.core)) { + this._existingConfig.core[key] = cfg[key]; + } + delete cfg[key]; + } + } + } + /** * Pre-scan module schemas to gather metadata for the configuration gateway prompt. * Returns info about which modules have configurable options. diff --git a/tools/installer/ui.js b/tools/installer/ui.js index 1200c37ea..12501b3f2 100644 --- a/tools/installer/ui.js +++ b/tools/installer/ui.js @@ -758,6 +758,9 @@ class UI { const defaultUsername = safeUsername.charAt(0).toUpperCase() + safeUsername.slice(1); configCollector.collectedConfig.core = { user_name: defaultUsername, + // {directory_name} default per src/core-skills/module.yaml — matches what the + // interactive flow resolves via buildQuestion()'s {directory_name} placeholder. + project_name: path.basename(directory), communication_language: 'English', document_output_language: 'English', output_folder: '_bmad-output',