diff --git a/test/test-installation-components.js b/test/test-installation-components.js index 797b75b11..d25160cce 100644 --- a/test/test-installation-components.js +++ b/test/test-installation-components.js @@ -3295,6 +3295,56 @@ async function runTests() { console.log(error.stack); failed++; } + + // collectModuleConfigQuick (the quick-update path) must also track + // non-schema keys as overrides so the manifest writer keeps them through + // the partition. Before this fix, `--set bmm.future_thing=x` landed in + // config.toml on install #1 but was silently dropped on the next + // quick-update reinstall — the per-module config.yaml retained the value + // but the central manifest's schema-strict partition stripped it because + // setOverrideKeys was never populated for carried-forward keys. + const tmp44e = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-fixture-44e-')); + try { + const om = new OfficialModules(); + // Simulate prior install state: future_thing was --set on run #1, lives + // in _bmad/bmm/config.yaml, and is now loaded via loadExistingConfig. + om._existingConfig = { bmm: { future_thing: 'persists', user_skill_level: 'expert' } }; + om.allAnswers = {}; + // collectModuleConfigQuick reads moduleConfig from disk; bmm built-in + // schema is the same one used elsewhere in this suite. + await om.collectModuleConfigQuick('bmm', tmp44e, true); + + assert(om.collectedConfig.bmm?.future_thing === 'persists', 'collectModuleConfigQuick carries unknown bmm.future_thing forward'); + assert( + om.setOverrideKeys?.bmm?.has('future_thing'), + 'collectModuleConfigQuick tracks carried-forward unknown keys in setOverrideKeys (so writeCentralConfig keeps them)', + ); + assert( + !om.setOverrideKeys?.bmm?.has('user_skill_level'), + 'collectModuleConfigQuick does NOT flag declared schema keys (user_skill_level) as overrides', + ); + } catch (error) { + console.log(`${colors.red} collectModuleConfigQuick carry-forward failed: ${error.message}${colors.reset}`); + console.log(error.stack); + failed++; + } + await fs.remove(tmp44e).catch(() => {}); + + // _trackUnknownKeysAsOverrides with no schema treats every key as + // unknown — the fallback when a module has no module.yaml at all. + try { + const om = new OfficialModules(); + om.collectedConfig = { weirdmod: { foo: 1, bar: 2 } }; + om._trackUnknownKeysAsOverrides('weirdmod', null); + assert( + om.setOverrideKeys?.weirdmod?.has('foo') && om.setOverrideKeys.weirdmod.has('bar'), + '_trackUnknownKeysAsOverrides flags every key as unknown when no schema is provided', + ); + } catch (error) { + console.log(`${colors.red} _trackUnknownKeysAsOverrides no-schema failed: ${error.message}${colors.reset}`); + console.log(error.stack); + failed++; + } } catch (error) { console.log(`${colors.red}Test Suite 44 setup failed: ${error.message}${colors.reset}`); console.log(error.stack); diff --git a/tools/installer/core/installer.js b/tools/installer/core/installer.js index 748b1b6f5..ac1af0a58 100644 --- a/tools/installer/core/installer.js +++ b/tools/installer/core/installer.js @@ -1277,6 +1277,19 @@ class Installer { lastModified: new Date().toISOString(), }; + // Convert per-module override-key Sets to plain string arrays so the + // value round-trips cleanly through Config.build / Object.freeze. Mirrors + // ui.collectModuleConfigs's return shape so quick-update's central + // manifest writer applies the same schema-strict partition exemption, + // letting prior `--set .=` forward-compat keys + // survive subsequent quick-updates without re-passing the flag. + const setOverrideKeys = {}; + if (quickModules.setOverrideKeys) { + for (const [moduleCode, keys] of Object.entries(quickModules.setOverrideKeys)) { + setOverrideKeys[moduleCode] = [...keys]; + } + } + // Build config and delegate to install() const installConfig = { directory: projectDir, @@ -1284,6 +1297,7 @@ class Installer { ides: configuredIdes, coreConfig: quickModules.collectedConfig.core, moduleConfigs: quickModules.collectedConfig, + setOverrideKeys, actionType: 'install', _quickUpdate: true, _preserveModules: skippedModules, diff --git a/tools/installer/modules/official-modules.js b/tools/installer/modules/official-modules.js index 4a3a4817a..f400eec9e 100644 --- a/tools/installer/modules/official-modules.js +++ b/tools/installer/modules/official-modules.js @@ -1266,6 +1266,42 @@ class OfficialModules { * @param {boolean} silentMode - If true, only prompt for new/missing fields * @returns {boolean} True if new fields were prompted, false if all fields existed */ + /** + * Track keys carried over from `_existingConfig` (or merged in from elsewhere) + * that are NOT declared in the module's `module.yaml` schema. The manifest + * writer's schema-strict partition would otherwise drop these on the next + * write, silently undoing the user's prior `--set .=` + * forward-compat assertion. Used by both `collectModuleConfig` and + * `collectModuleConfigQuick` so the persistence contract holds across + * regular updates and quick-updates alike. + * + * Without a schema (`moduleSchema = null`), every key is treated as unknown + * since we can't tell what's declared. + */ + _trackUnknownKeysAsOverrides(moduleName, moduleSchema) { + const moduleData = this.collectedConfig?.[moduleName]; + if (!moduleData || typeof moduleData !== 'object') return; + + const declaredKeys = new Set(); + if (moduleSchema && typeof moduleSchema === 'object') { + for (const [key, decl] of Object.entries(moduleSchema)) { + if (key === 'prompt') continue; + if (decl && typeof decl === 'object' && (decl.prompt !== undefined || decl.result !== undefined)) { + declaredKeys.add(key); + } + } + } + + if (!this.setOverrideKeys) this.setOverrideKeys = {}; + if (!this.setOverrideKeys[moduleName]) this.setOverrideKeys[moduleName] = new Set(); + + for (const key of Object.keys(moduleData)) { + if (!declaredKeys.has(key)) { + this.setOverrideKeys[moduleName].add(key); + } + } + } + async collectModuleConfigQuick(moduleName, projectDir, silentMode = true) { this.currentProjectDir = projectDir; // Load existing config if not already loaded @@ -1299,6 +1335,10 @@ class OfficialModules { } this.collectedConfig[moduleName] = { ...this._existingConfig[moduleName] }; } + // Without a schema we can't tell declared from undeclared, so every + // carried-forward key gets the override exemption to survive + // writeCentralConfig's schema-strict partition. + this._trackUnknownKeysAsOverrides(moduleName, null); return false; } @@ -1306,6 +1346,8 @@ class OfficialModules { const moduleConfig = yaml.parse(configContent); if (!moduleConfig) { + // Schema unparseable — same fallback as missing schema. + this._trackUnknownKeysAsOverrides(moduleName, null); return false; } @@ -1324,6 +1366,7 @@ class OfficialModules { const moduleDisplayName = moduleConfig.header || `${moduleName.toUpperCase()} Module`; await prompts.log.step(moduleDisplayName); await prompts.log.message(` \u2713 ${moduleConfig.subheader}`); + this._trackUnknownKeysAsOverrides(moduleName, moduleConfig); return false; // No new fields } @@ -1378,6 +1421,9 @@ class OfficialModules { // Show "no config" message for modules with no new questions (that have config keys) await prompts.log.message(` \u2713 ${moduleName.toUpperCase()} module already up to date`); + // Track non-schema keys so the manifest writer keeps prior `--set` + // assertions through quick-update's schema-strict partition. + this._trackUnknownKeysAsOverrides(moduleName, moduleConfig); return false; // No new fields } @@ -1465,6 +1511,11 @@ class OfficialModules { } } + // Track non-schema keys carried over so writeCentralConfig keeps them + // through the partition (forward-compat contract: a `--set` from a prior + // run survives quick-update reinstalls without re-passing the flag). + this._trackUnknownKeysAsOverrides(moduleName, moduleConfig); + await this.displayModulePostConfigNotes(moduleName, moduleConfig); return newKeys.length > 0 || newStaticKeys.length > 0; // Return true if we had any new fields (interactive or static)