diff --git a/test/test-installation-components.js b/test/test-installation-components.js index b7beec9ff..a8bf77756 100644 --- a/test/test-installation-components.js +++ b/test/test-installation-components.js @@ -2943,8 +2943,38 @@ async function runTests() { '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); diff --git a/tools/installer/modules/official-modules.js b/tools/installer/modules/official-modules.js index 40c885360..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; } @@ -947,9 +950,15 @@ class OfficialModules { ); if (coreKeys.size === 0) return; - this._existingConfig.core = this._existingConfig.core || {}; + // 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') continue; + 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)) {