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..a8bf77756 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,122 @@ 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', + ); + + // Malformed config.yaml (parses to a scalar) must not crash loadExistingConfig + // or the hoist pass — they should treat it as "no config for that module" + // and continue. Regression for augment review on PR #2348. + const fixtureRoot43c = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-fixture-43c-')); + const bmadDir43c = path.join(fixtureRoot43c, '_bmad'); + await fs.ensureDir(path.join(bmadDir43c, '_config')); + await fs.writeFile(path.join(bmadDir43c, '_config', 'manifest.yaml'), 'modules: []\n', 'utf8'); + await fs.ensureDir(path.join(bmadDir43c, 'core')); + await fs.ensureDir(path.join(bmadDir43c, 'bmm')); + // Scalar YAML — yaml.parse returns the literal 42 (truthy non-object). + // Pre-fix this crashed _hoistCoreKeysFromLegacyModuleConfigs with + // "Cannot use 'in' operator to search for 'project_name' in 42". + await fs.writeFile(path.join(bmadDir43c, 'core', 'config.yaml'), '42\n', 'utf8'); + await fs.writeFile(path.join(bmadDir43c, 'bmm', 'config.yaml'), 'project_name: rescued\n', 'utf8'); + + const officialModules43c = new OfficialModules(); + let crashErr; + try { + await officialModules43c.loadExistingConfig(fixtureRoot43c); + } catch (error) { + crashErr = error; + } + assert(!crashErr, 'loadExistingConfig does not crash on a scalar core/config.yaml', crashErr?.stack); + + assert( + officialModules43c.existingConfig.core?.project_name === 'rescued', + 'scalar core gets replaced with {} and bmm.project_name still hoists in', + ); + + await fs.remove(fixtureRoot43).catch(() => {}); + await fs.remove(fixtureRoot43b).catch(() => {}); + await fs.remove(fixtureRoot43c).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..615daba86 100644 --- a/tools/installer/modules/official-modules.js +++ b/tools/installer/modules/official-modules.js @@ -903,7 +903,10 @@ class OfficialModules { try { const content = await fs.readFile(moduleConfigPath, 'utf8'); const moduleConfig = yaml.parse(content); - if (moduleConfig) { + // Only keep plain object parses. A corrupt config.yaml that parses + // to a scalar or array would crash later code that does `key in cfg` + // / `Object.keys(cfg)`; treat it the same as a parse error. + if (moduleConfig && typeof moduleConfig === 'object' && !Array.isArray(moduleConfig)) { this._existingConfig[entry.name] = moduleConfig; foundAny = true; } @@ -914,9 +917,58 @@ 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; + + // Belt-and-suspenders: loadExistingConfig already filters non-object parses, + // but anyone calling _hoistCoreKeysFromLegacyModuleConfigs in isolation (or + // future code paths populating _existingConfig directly) shouldn't be able + // to crash this with a scalar / array. + const existingCore = this._existingConfig.core; + this._existingConfig.core = existingCore && typeof existingCore === 'object' && !Array.isArray(existingCore) ? existingCore : {}; + + for (const [moduleName, cfg] of Object.entries(this._existingConfig)) { + if (moduleName === 'core' || !cfg || typeof cfg !== 'object' || Array.isArray(cfg)) 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',