fix(installer): address code-review findings on config refactor
Nine fixes covering bugs caught by my code-review, Augment, and CodeRabbit:
quickUpdate identity overwrite (official-modules.js): collectModuleConfigQuick
now lazy-loads globalConfig and falls back through ~/.bmad/config.user.toml
before getDefaultUsername(), so `bmad update` no longer silently overwrites
the user's global identity with the OS username.
computeProcessedDefault placeholder asymmetry (manifest-generator.js): build
a shipped-defaults crossKeyDefaults map and feed BOTH writeCentralConfig and
writeModuleTomls through it. Previously stripDefaults resolved {output_folder}
against user answers while module.toml resolved it against shipped defaults —
overriding output_folder silently reverted every derived path on read.
parseTomlScalar escape order (global-config.js): single-pass regex replacer
so `\\n` round-trips as backslash+n instead of collapsing to a newline.
Windows paths and any TOML string with literal `\n`/`\t`/`\r` survive.
writeGlobalUserCore nested-table corruption (manifest-generator.js): emit
nested objects as proper dotted sub-tables (`[modules.bmm]`) instead of
serializing them as `"[object Object]"` strings inside the parent section.
extractAgentBlocks dead code (manifest-generator.js): deleted. The agent
preservation invariant was intentionally retired by Task F (agents live in
module.toml floor); the function had no callers.
applySetOverrides 3-tier routing (set-overrides.js): consults
~/.bmad/config.user.toml first, then project config.user.toml, then routes
known core scope:user keys (user_name, communication_language per core
schema) to global when neither file owns them. Prevents
`bmad install --set core.user_name=Alice` from polluting project team config.
BMAD_HOME tilde expansion (global-config.js): JS resolver now matches
Python's Path.expanduser() so installer and resolver agree on the global
location when BMAD_HOME is set in non-shell contexts (.env, Docker, Windows).
resolve_config.py fail-fast (resolve_config.py): when --project-root is
explicitly passed but _bmad/ doesn't exist, exit 1 with a clear error
instead of silently returning {}. Global-only mode (no --project-root)
remains permissive.
_MODULE_SKILLS_RE hyphen support (resolve_customization.py): allow hyphens
in module slugs (e.g. `foo-bar-skills/`) so qualified-name skill matching
works for hyphenated modules.
Tests: 383 JS tests + 21 Python tests pass; lint, markdownlint, prettier all
green. Suite 44 now isolates BMAD_HOME and asserts the new 3-tier routing
contract for `--set` overrides.
This commit is contained in:
parent
bec2c04a6d
commit
579c78d2aa
|
|
@ -227,6 +227,17 @@ def main():
|
|||
project_root = Path(args.project_root).resolve() if args.project_root else None
|
||||
global_dir = resolve_global_dir()
|
||||
|
||||
# If the caller explicitly named a project root, that's a promise it exists
|
||||
# and has been installed. Fail loudly on a missing _bmad/ rather than
|
||||
# silently returning {} — that masked broken installs in the old required=
|
||||
# True behavior. Global-only mode (no --project-root) stays permissive.
|
||||
if project_root is not None and not (project_root / "_bmad").is_dir():
|
||||
sys.stderr.write(
|
||||
f"error: --project-root {project_root} has no _bmad/ directory "
|
||||
f"(install not present, or wrong path)\n"
|
||||
)
|
||||
sys.exit(1)
|
||||
|
||||
merged: dict = {}
|
||||
# Floor: per-module shipped defaults (lowest priority).
|
||||
for module_toml in collect_module_layers(project_root):
|
||||
|
|
|
|||
|
|
@ -197,7 +197,7 @@ def deep_merge(base, override):
|
|||
return override
|
||||
|
||||
|
||||
_MODULE_SKILLS_RE = re.compile(r"^(?P<module>[A-Za-z0-9_]+)-skills$")
|
||||
_MODULE_SKILLS_RE = re.compile(r"^(?P<module>[A-Za-z0-9_-]+)-skills$")
|
||||
|
||||
|
||||
def detect_skill_module(skill_dir: Path) -> str | None:
|
||||
|
|
|
|||
|
|
@ -10,7 +10,9 @@ from pathlib import Path
|
|||
SCRIPT = Path(__file__).resolve().parents[1] / "resolve_config.py"
|
||||
|
||||
|
||||
def run(args, env_overrides=None):
|
||||
def run_raw(args, env_overrides=None):
|
||||
"""Run resolver, return CompletedProcess-like object with decoded streams.
|
||||
Use this when the test expects a non-zero exit (e.g. fail-fast checks)."""
|
||||
env = os.environ.copy()
|
||||
env["BMAD_HOME"] = env.get("BMAD_HOME", "/nonexistent-bmad-home-default")
|
||||
if env_overrides:
|
||||
|
|
@ -22,10 +24,16 @@ def run(args, env_overrides=None):
|
|||
env=env,
|
||||
check=False,
|
||||
)
|
||||
stderr = result.stderr.decode("utf-8", errors="replace")
|
||||
result.stdout = result.stdout.decode("utf-8", errors="replace")
|
||||
result.stderr = result.stderr.decode("utf-8", errors="replace")
|
||||
return result
|
||||
|
||||
|
||||
def run(args, env_overrides=None):
|
||||
result = run_raw(args, env_overrides=env_overrides)
|
||||
if result.returncode != 0:
|
||||
raise AssertionError(f"resolve_config failed ({result.returncode}): {stderr}")
|
||||
return json.loads(result.stdout.decode("utf-8"))
|
||||
raise AssertionError(f"resolve_config failed ({result.returncode}): {result.stderr}")
|
||||
return json.loads(result.stdout)
|
||||
|
||||
|
||||
class ResolveConfigTests(unittest.TestCase):
|
||||
|
|
@ -79,11 +87,23 @@ class ResolveConfigTests(unittest.TestCase):
|
|||
self.assertEqual(data["core"]["user_name"], "Pinned")
|
||||
|
||||
def test_project_config_optional(self):
|
||||
# No _bmad/config.toml; should not error
|
||||
# _bmad/ exists (installed project) but no config.toml inside — that's
|
||||
# the lean / global-only case and must not error.
|
||||
with tempfile.TemporaryDirectory() as proj:
|
||||
(Path(proj) / "_bmad").mkdir()
|
||||
data = run(["--project-root", proj])
|
||||
self.assertEqual(data, {})
|
||||
|
||||
def test_project_root_without_bmad_dir_errors(self):
|
||||
# --project-root pointing at a directory with no _bmad/ is treated
|
||||
# as a broken install (typo, wiped install) — resolver exits non-zero
|
||||
# rather than silently returning {}. Global-only mode (no
|
||||
# --project-root) keeps the permissive behavior.
|
||||
with tempfile.TemporaryDirectory() as proj:
|
||||
result = run_raw(["--project-root", proj])
|
||||
self.assertNotEqual(result.returncode, 0)
|
||||
self.assertIn("no _bmad/ directory", result.stderr)
|
||||
|
||||
def test_module_floor_contributes_when_no_overrides(self):
|
||||
# Module-shipped defaults from _bmad/{module}/module.toml should
|
||||
# appear when nothing else specifies them.
|
||||
|
|
|
|||
|
|
@ -3027,6 +3027,12 @@ async function runTests() {
|
|||
// Test Suite 44: --set <module>.<key>=<value> CLI overrides (#1663)
|
||||
// ============================================================
|
||||
console.log(`${colors.yellow}Test Suite 44: --set CLI overrides${colors.reset}\n`);
|
||||
// Isolate $BMAD_HOME for the whole suite — applySetOverrides now consults
|
||||
// ~/.bmad/config.user.toml for routing decisions, and we don't want tests
|
||||
// touching the developer's actual global identity.
|
||||
const tempGlobalDir44 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-applyset-global-'));
|
||||
const priorBmadHome44 = process.env.BMAD_HOME;
|
||||
process.env.BMAD_HOME = tempGlobalDir44;
|
||||
try {
|
||||
const { parseSetEntry, parseSetEntries, applySetOverrides, upsertTomlKey, tomlString } = require('../tools/installer/set-overrides');
|
||||
const { discoverOfficialModuleYamls, formatOptionsList } = require('../tools/installer/list-options');
|
||||
|
|
@ -3191,15 +3197,24 @@ async function runTests() {
|
|||
await fs.writeFile(path.join(bmadDir, 'config.toml'), '[core]\nuser_name = "Brian"\n', 'utf8');
|
||||
await fs.ensureDir(path.join(bmadDir, 'core'));
|
||||
await fs.writeFile(path.join(bmadDir, 'core', 'config.yaml'), 'user_name: Brian\n', 'utf8');
|
||||
// Override targets a key only in team config; routes to team. user.toml
|
||||
// never gets created in this case (correct — no user-scope writes).
|
||||
// Override targets a core scope:user key. Per Task D's 3-tier routing,
|
||||
// core.user_name lands in ~/.bmad/config.user.toml regardless of whether
|
||||
// project user.toml exists — that's where the resolver picks it up.
|
||||
// Project files (team and user) are left untouched.
|
||||
await applySetOverrides({ core: { user_name: 'Updated' } }, bmadDir);
|
||||
const team = await fs.readFile(path.join(bmadDir, 'config.toml'), 'utf8');
|
||||
assert(team.includes('user_name = "Updated"'), 'applySetOverrides updates team key when user.toml is absent');
|
||||
const globalContent = await fs.readFile(path.join(tempGlobalDir44, 'config.user.toml'), 'utf8');
|
||||
assert(
|
||||
globalContent.includes('user_name = "Updated"'),
|
||||
'applySetOverrides routes core.user_name to global when project user.toml is absent',
|
||||
);
|
||||
assert(!team.includes('user_name = "Updated"'), 'applySetOverrides does not write core scope:user keys to project team config');
|
||||
assert(
|
||||
!(await fs.pathExists(path.join(bmadDir, 'config.user.toml'))),
|
||||
'applySetOverrides does not create config.user.toml unnecessarily',
|
||||
'applySetOverrides does not create project config.user.toml for scope:user core keys',
|
||||
);
|
||||
// Reset global file for the next test block.
|
||||
await fs.remove(path.join(tempGlobalDir44, 'config.user.toml')).catch(() => {});
|
||||
await fs.remove(tmp).catch(() => {});
|
||||
}
|
||||
|
||||
|
|
@ -3213,11 +3228,17 @@ async function runTests() {
|
|||
await fs.writeFile(path.join(bmadDir, 'core', 'config.yaml'), 'user_name: Brian\n', 'utf8');
|
||||
// bmm is not installed (no `_bmad/bmm/config.yaml`). The override for
|
||||
// bmm should be silently skipped, no `[modules.bmm]` section created.
|
||||
// core.user_name is scope:user → routed to global (~/.bmad).
|
||||
const applied = await applySetOverrides({ bmm: { foo: 'bar' }, core: { user_name: 'Updated' } }, bmadDir);
|
||||
const team = await fs.readFile(path.join(bmadDir, 'config.toml'), 'utf8');
|
||||
const globalContent = await fs.readFile(path.join(tempGlobalDir44, 'config.user.toml'), 'utf8');
|
||||
assert(!team.includes('[modules.bmm]'), 'applySetOverrides does NOT create section for uninstalled module');
|
||||
assert(team.includes('user_name = "Updated"'), 'applySetOverrides still applies overrides for installed modules');
|
||||
assert(
|
||||
globalContent.includes('user_name = "Updated"'),
|
||||
'applySetOverrides still applies overrides for installed modules (routed to global for scope:user core)',
|
||||
);
|
||||
assert(applied.length === 1 && applied[0].module === 'core', 'applySetOverrides reports only the installed-module entries');
|
||||
await fs.remove(path.join(tempGlobalDir44, 'config.user.toml')).catch(() => {});
|
||||
await fs.remove(tmp).catch(() => {});
|
||||
}
|
||||
|
||||
|
|
@ -3273,6 +3294,13 @@ async function runTests() {
|
|||
console.log(`${colors.red}Test Suite 44 setup failed: ${error.message}${colors.reset}`);
|
||||
console.log(error.stack);
|
||||
failed++;
|
||||
} finally {
|
||||
if (priorBmadHome44 === undefined) {
|
||||
delete process.env.BMAD_HOME;
|
||||
} else {
|
||||
process.env.BMAD_HOME = priorBmadHome44;
|
||||
}
|
||||
await fs.remove(tempGlobalDir44).catch(() => {});
|
||||
}
|
||||
|
||||
console.log('');
|
||||
|
|
|
|||
|
|
@ -459,9 +459,17 @@ class ManifestGenerator {
|
|||
// 1. scope per prompt key (team vs user)
|
||||
// 2. the canonical module code (for [modules.{code}] section names)
|
||||
// 3. processed defaults per key (for delta detection)
|
||||
//
|
||||
// Pass 1: parse every module.yaml and capture its raw shipped defaults.
|
||||
// We use those — NOT the user's answered moduleConfigs — to resolve
|
||||
// cross-key placeholders like `{output_folder}`. Otherwise a user override
|
||||
// of output_folder would make every derived default (e.g. planning_artifacts)
|
||||
// match the user's value and get stripped from config.toml as "default",
|
||||
// even though module.toml's floor still carries the shipped path.
|
||||
const scopeByModuleKey = {};
|
||||
const codeByModuleName = {};
|
||||
const defaultsByModuleKey = {};
|
||||
const parsedByModule = {};
|
||||
for (const moduleName of this.updatedModules) {
|
||||
const moduleYamlPath = await resolveInstalledModuleYaml(moduleName);
|
||||
if (!moduleYamlPath) {
|
||||
|
|
@ -474,17 +482,8 @@ class ManifestGenerator {
|
|||
try {
|
||||
const parsed = yaml.parse(await fs.readFile(moduleYamlPath, 'utf8'));
|
||||
if (!parsed || typeof parsed !== 'object') continue;
|
||||
parsedByModule[moduleName] = parsed;
|
||||
if (parsed.code) codeByModuleName[moduleName] = parsed.code;
|
||||
scopeByModuleKey[moduleName] = {};
|
||||
defaultsByModuleKey[moduleName] = {};
|
||||
for (const [key, value] of Object.entries(parsed)) {
|
||||
if (!value || typeof value !== 'object' || !('prompt' in value)) continue;
|
||||
scopeByModuleKey[moduleName][key] = value.scope === 'user' ? 'user' : 'team';
|
||||
const processedDefault = computeProcessedDefault(value, moduleConfigs);
|
||||
if (processedDefault !== undefined) {
|
||||
defaultsByModuleKey[moduleName][key] = processedDefault;
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
console.warn(
|
||||
`[warn] writeCentralConfig: could not parse module.yaml for '${moduleName}' (${error.message}). ` +
|
||||
|
|
@ -493,6 +492,35 @@ class ManifestGenerator {
|
|||
}
|
||||
}
|
||||
|
||||
// Build the cross-key defaults map (same logic writeModuleTomls uses).
|
||||
// Shipped defaults only — never user answers.
|
||||
const crossKeyDefaults = {};
|
||||
for (const parsed of Object.values(parsedByModule)) {
|
||||
const raw = extractModuleDefaults(parsed);
|
||||
for (const [key, value] of Object.entries(raw)) {
|
||||
if (crossKeyDefaults[key] !== undefined) continue;
|
||||
let stripped = value;
|
||||
if (typeof stripped === 'string' && stripped.startsWith('{project-root}/')) {
|
||||
stripped = stripped.slice('{project-root}/'.length);
|
||||
}
|
||||
crossKeyDefaults[key] = stripped;
|
||||
}
|
||||
}
|
||||
|
||||
// Pass 2: compute scopes and processed defaults using the symmetric map.
|
||||
for (const [moduleName, parsed] of Object.entries(parsedByModule)) {
|
||||
scopeByModuleKey[moduleName] = {};
|
||||
defaultsByModuleKey[moduleName] = {};
|
||||
for (const [key, value] of Object.entries(parsed)) {
|
||||
if (!value || typeof value !== 'object' || !('prompt' in value)) continue;
|
||||
scopeByModuleKey[moduleName][key] = value.scope === 'user' ? 'user' : 'team';
|
||||
const processedDefault = computeProcessedDefault(value, crossKeyDefaults);
|
||||
if (processedDefault !== undefined) {
|
||||
defaultsByModuleKey[moduleName][key] = processedDefault;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Load the global config snapshot for [core] delta detection. If a key's
|
||||
// current value equals the global value, no need to duplicate it into the
|
||||
// project file — the resolver finds it globally.
|
||||
|
|
@ -702,15 +730,10 @@ class ManifestGenerator {
|
|||
'# ─────────────────────────────────────────────────────────────────',
|
||||
'',
|
||||
];
|
||||
for (const [section, table] of Object.entries(mergedConfig)) {
|
||||
if (!table || typeof table !== 'object' || Array.isArray(table)) continue;
|
||||
lines.push(`[${section}]`);
|
||||
for (const [key, value] of Object.entries(table)) {
|
||||
if (value === undefined || value === null || value === '') continue;
|
||||
lines.push(`${key} = ${formatTomlValue(value)}`);
|
||||
}
|
||||
lines.push('');
|
||||
}
|
||||
// Emit sections recursively so nested tables (e.g. [modules.bmm] reachable
|
||||
// via dotted parseSimpleToml output) round-trip as proper sub-tables
|
||||
// instead of being stringified as "[object Object]" inside their parent.
|
||||
emitTomlSections(mergedConfig, [], lines);
|
||||
const content = lines.join('\n').replace(/\n+$/, '\n');
|
||||
await fs.writeFile(globalPath, content);
|
||||
return globalPath;
|
||||
|
|
@ -1019,25 +1042,27 @@ class ManifestGenerator {
|
|||
* Objects are not expected at this emit path.
|
||||
*/
|
||||
/**
|
||||
* Compute the processed default value for a module.yaml question item, using
|
||||
* the already-resolved moduleConfigs map for cross-key references like
|
||||
* `{output_folder}`. Used by writeCentralConfig to detect default-equal
|
||||
* values that should NOT be re-emitted into the lean config.toml.
|
||||
* Compute the processed default value for a module.yaml question item.
|
||||
* Resolves `{key}` cross-references against the flat `crossKeyDefaults` lookup
|
||||
* (shipped defaults, never user answers — see writeCentralConfig comment).
|
||||
* Used by writeCentralConfig to detect default-equal values that should NOT
|
||||
* be re-emitted into the lean config.toml. Matches the lookup table that
|
||||
* writeModuleTomls uses, so module.toml's floor and config.toml's delta
|
||||
* detection agree on what "default" means.
|
||||
*
|
||||
* Steps:
|
||||
* 1. Substitute {key} references against any module's already-collected
|
||||
* value (with leading "{project-root}/" stripped, matching the
|
||||
* installer's processResultTemplate behavior).
|
||||
* 1. Substitute {key} references against crossKeyDefaults (with leading
|
||||
* "{project-root}/" stripped, matching the installer's
|
||||
* processResultTemplate behavior).
|
||||
* 2. Apply the result: template with {value} substituted.
|
||||
*
|
||||
* Returns undefined for items without a default, leaving the caller's delta
|
||||
* check to fall through unchanged.
|
||||
* Returns undefined for items without a default.
|
||||
*
|
||||
* @param {object} item - one module.yaml question schema
|
||||
* @param {object} moduleConfigs - already-resolved per-module configs
|
||||
* @param {Record<string, *>} crossKeyDefaults - flat shipped-defaults lookup
|
||||
* @returns {*} processed default value (string/scalar) or undefined
|
||||
*/
|
||||
function computeProcessedDefault(item, moduleConfigs) {
|
||||
function computeProcessedDefault(item, crossKeyDefaults) {
|
||||
if (!item || item.default === undefined || item.default === null) return;
|
||||
let value = item.default;
|
||||
if (typeof value === 'string') {
|
||||
|
|
@ -1045,16 +1070,8 @@ function computeProcessedDefault(item, moduleConfigs) {
|
|||
if (refKey === 'project-root' || refKey === 'value' || refKey === 'directory_name') {
|
||||
return match;
|
||||
}
|
||||
for (const mod of Object.values(moduleConfigs || {})) {
|
||||
if (mod && typeof mod === 'object' && mod[refKey] !== undefined) {
|
||||
let resolved = mod[refKey];
|
||||
if (typeof resolved === 'string' && resolved.startsWith('{project-root}/')) {
|
||||
resolved = resolved.slice('{project-root}/'.length);
|
||||
}
|
||||
return resolved;
|
||||
}
|
||||
}
|
||||
return match;
|
||||
const replacement = (crossKeyDefaults || {})[refKey];
|
||||
return replacement === undefined ? match : String(replacement);
|
||||
});
|
||||
}
|
||||
if (typeof item.result === 'string' && value !== undefined) {
|
||||
|
|
@ -1163,39 +1180,26 @@ function formatTomlValue(value) {
|
|||
return `"${escaped}"`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract [agents.<code>] blocks from a previously-emitted config.toml.
|
||||
* We only need this for roster preservation — the file is our own controlled
|
||||
* output, so a simple line scanner is safer than adding a TOML parser
|
||||
* dependency. Each block runs from its `[agents.<code>]` header until the
|
||||
* next `[` heading or EOF; the `module = "..."` line inside drives which
|
||||
* entries we keep on the next write.
|
||||
* @returns {Array<{code: string, module: string | null, body: string}>}
|
||||
*/
|
||||
function extractAgentBlocks(tomlContent) {
|
||||
const blocks = [];
|
||||
const lines = tomlContent.split('\n');
|
||||
let i = 0;
|
||||
while (i < lines.length) {
|
||||
const header = lines[i].match(/^\[agents\.([^\]]+)]\s*$/);
|
||||
if (!header) {
|
||||
i++;
|
||||
continue;
|
||||
// Recursively serialize a parsed TOML object back to text. Scalar entries
|
||||
// are written under the current section header; nested objects are emitted
|
||||
// as dotted sub-tables. Skips null/undefined/'' values.
|
||||
function emitTomlSections(node, sectionPath, lines) {
|
||||
const scalars = [];
|
||||
const nested = [];
|
||||
for (const [key, value] of Object.entries(node)) {
|
||||
if (value === null || value === undefined || value === '') continue;
|
||||
if (value && typeof value === 'object' && !Array.isArray(value)) {
|
||||
nested.push([key, value]);
|
||||
} else {
|
||||
scalars.push([key, value]);
|
||||
}
|
||||
const code = header[1];
|
||||
const blockLines = [lines[i]];
|
||||
let moduleName = null;
|
||||
i++;
|
||||
while (i < lines.length && !lines[i].startsWith('[')) {
|
||||
blockLines.push(lines[i]);
|
||||
const m = lines[i].match(/^module\s*=\s*"((?:[^"\\]|\\.)*)"\s*$/);
|
||||
if (m) moduleName = m[1];
|
||||
i++;
|
||||
}
|
||||
while (blockLines.length > 1 && blockLines.at(-1) === '') blockLines.pop();
|
||||
blocks.push({ code, module: moduleName, body: blockLines.join('\n') });
|
||||
}
|
||||
return blocks;
|
||||
if (scalars.length > 0) {
|
||||
if (sectionPath.length > 0) lines.push(`[${sectionPath.join('.')}]`);
|
||||
for (const [key, value] of scalars) lines.push(`${key} = ${formatTomlValue(value)}`);
|
||||
lines.push('');
|
||||
}
|
||||
for (const [key, value] of nested) emitTomlSections(value, [...sectionPath, key], lines);
|
||||
}
|
||||
|
||||
module.exports = { ManifestGenerator };
|
||||
|
|
|
|||
|
|
@ -22,11 +22,22 @@ const fs = require('./fs-native');
|
|||
function resolveGlobalDir() {
|
||||
const override = process.env.BMAD_HOME;
|
||||
if (override && override.trim()) {
|
||||
return path.resolve(override.trim());
|
||||
return path.resolve(expandTilde(override.trim()));
|
||||
}
|
||||
return path.join(os.homedir(), '.bmad');
|
||||
}
|
||||
|
||||
// JS counterpart to Python's Path.expanduser() — keeps installer/resolver
|
||||
// agreement when BMAD_HOME is set in non-shell contexts (Docker, .env files,
|
||||
// Windows env var GUI) where the shell never expands `~`.
|
||||
function expandTilde(input) {
|
||||
if (input === '~') return os.homedir();
|
||||
if (input.startsWith('~/') || input.startsWith('~\\')) {
|
||||
return path.join(os.homedir(), input.slice(2));
|
||||
}
|
||||
return input;
|
||||
}
|
||||
|
||||
function globalTeamConfigPath() {
|
||||
return path.join(resolveGlobalDir(), 'config.toml');
|
||||
}
|
||||
|
|
@ -111,13 +122,11 @@ function stripInlineComment(line) {
|
|||
|
||||
function parseTomlScalar(raw) {
|
||||
if (raw.startsWith('"') && raw.endsWith('"') && raw.length >= 2) {
|
||||
return raw
|
||||
.slice(1, -1)
|
||||
.replaceAll(String.raw`\"`, '"')
|
||||
.replaceAll(String.raw`\\`, '\\')
|
||||
.replaceAll(String.raw`\n`, '\n')
|
||||
.replaceAll(String.raw`\r`, '\r')
|
||||
.replaceAll(String.raw`\t`, '\t');
|
||||
// Single-pass unescape — sequential replaceAll lets `\\n` (backslash + n)
|
||||
// collapse into a newline because the second pass sees the just-produced
|
||||
// `\n` and treats it as the escape sequence. One regex avoids that.
|
||||
const escapes = { '\\\\': '\\', '\\"': '"', '\\n': '\n', '\\r': '\r', '\\t': '\t' };
|
||||
return raw.slice(1, -1).replaceAll(/\\["\\nrt]/g, (m) => escapes[m] ?? m);
|
||||
}
|
||||
if (raw === 'true') return true;
|
||||
if (raw === 'false') return false;
|
||||
|
|
|
|||
|
|
@ -1144,6 +1144,18 @@ class OfficialModules {
|
|||
if (!this._existingConfig) {
|
||||
await this.loadExistingConfig(projectDir);
|
||||
}
|
||||
// Lazy-load global config so identity fallbacks below can consult
|
||||
// ~/.bmad/config.user.toml. quickUpdate doesn't go through
|
||||
// collectAllConfigurations, so this.globalConfig would otherwise be unset
|
||||
// and user_name would silently default to the OS username — overwriting
|
||||
// the value the user previously committed to global.
|
||||
if (!this.globalConfig) {
|
||||
try {
|
||||
this.globalConfig = await loadGlobalConfig();
|
||||
} catch {
|
||||
this.globalConfig = { merged: {} };
|
||||
}
|
||||
}
|
||||
|
||||
// Initialize allAnswers if not already initialized
|
||||
if (!this.allAnswers) {
|
||||
|
|
@ -1220,12 +1232,14 @@ class OfficialModules {
|
|||
}
|
||||
this.collectedConfig[moduleName] = { ...this._existingConfig[moduleName] };
|
||||
|
||||
// Special handling for user_name: ensure it has a value
|
||||
// Special handling for user_name: ensure it has a value. Prefer the
|
||||
// global value (~/.bmad/config.user.toml) before the OS username, or
|
||||
// we'll silently overwrite the user's prior global identity.
|
||||
if (
|
||||
moduleName === 'core' &&
|
||||
(!this.collectedConfig[moduleName].user_name || this.collectedConfig[moduleName].user_name === '[USER_NAME]')
|
||||
) {
|
||||
this.collectedConfig[moduleName].user_name = this.getDefaultUsername();
|
||||
this.collectedConfig[moduleName].user_name = this._identityFallback('user_name');
|
||||
}
|
||||
|
||||
// Also populate allAnswers for cross-referencing
|
||||
|
|
@ -1233,18 +1247,20 @@ class OfficialModules {
|
|||
// Ensure user_name is properly set in allAnswers too
|
||||
let finalValue = value;
|
||||
if (moduleName === 'core' && key === 'user_name' && (!value || value === '[USER_NAME]')) {
|
||||
finalValue = this.getDefaultUsername();
|
||||
finalValue = this._identityFallback('user_name');
|
||||
}
|
||||
this.allAnswers[`${moduleName}_${key}`] = finalValue;
|
||||
}
|
||||
} else if (moduleName === 'core') {
|
||||
// No existing core config - ensure we at least have user_name
|
||||
// No existing core config - ensure we at least have user_name.
|
||||
// Same global-first preference as above.
|
||||
if (!this.collectedConfig[moduleName]) {
|
||||
this.collectedConfig[moduleName] = {};
|
||||
}
|
||||
if (!this.collectedConfig[moduleName].user_name) {
|
||||
this.collectedConfig[moduleName].user_name = this.getDefaultUsername();
|
||||
this.allAnswers[`${moduleName}_user_name`] = this.getDefaultUsername();
|
||||
const fallback = this._identityFallback('user_name');
|
||||
this.collectedConfig[moduleName].user_name = fallback;
|
||||
this.allAnswers[`${moduleName}_user_name`] = fallback;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1422,6 +1438,22 @@ class OfficialModules {
|
|||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fall back through identity sources for a core scope:user key. Prefers the
|
||||
* global value (~/.bmad/config.user.toml) so quickUpdate / re-install never
|
||||
* silently overwrites a previously-set identity with the OS username.
|
||||
* Only user_name has an OS-derived ultimate fallback; other keys return
|
||||
* undefined so the caller can decide.
|
||||
*/
|
||||
_identityFallback(key) {
|
||||
const globalCore = (this.globalConfig && this.globalConfig.merged && this.globalConfig.merged.core) || {};
|
||||
if (globalCore[key] !== undefined && globalCore[key] !== '' && globalCore[key] !== null) {
|
||||
return globalCore[key];
|
||||
}
|
||||
if (key === 'user_name') return this.getDefaultUsername();
|
||||
return;
|
||||
}
|
||||
|
||||
/**
|
||||
* Collect configuration for a single module
|
||||
* @param {string} moduleName - Module name
|
||||
|
|
@ -1932,9 +1964,10 @@ class OfficialModules {
|
|||
existingValue = this.normalizeExistingValueForPrompt(existingValue, moduleName, item, moduleConfig);
|
||||
}
|
||||
|
||||
// Special handling for user_name: default to system user
|
||||
// Special handling for user_name: prefer global identity (~/.bmad) over
|
||||
// OS username so the prompt's default reflects what the user already chose.
|
||||
if (moduleName === 'core' && key === 'user_name' && !existingValue) {
|
||||
item.default = this.getDefaultUsername();
|
||||
item.default = this._identityFallback('user_name');
|
||||
}
|
||||
|
||||
// Determine question type and default value
|
||||
|
|
|
|||
|
|
@ -25,6 +25,8 @@ const PROTOTYPE_POLLUTING_NAMES = new Set(['__proto__', 'prototype', 'constructo
|
|||
const path = require('node:path');
|
||||
const fs = require('./fs-native');
|
||||
const yaml = require('yaml');
|
||||
const { globalUserConfigPath } = require('./global-config');
|
||||
const { resolveInstalledModuleYaml } = require('./project-root');
|
||||
|
||||
/**
|
||||
* Parse a single `--set <module>.<key>=<value>` entry.
|
||||
|
|
@ -215,19 +217,49 @@ async function tomlHasKey(filePath, section, key) {
|
|||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Look up which prompt keys in `core/module.yaml` are declared `scope: user`.
|
||||
* Used so `--set` routes core scope:user keys (user_name, communication_language)
|
||||
* to the global identity file the installer's writeGlobalUserCore writes to,
|
||||
* rather than polluting project config.toml as a team-scope key.
|
||||
* Returns an empty set if core isn't installed or the schema can't be parsed.
|
||||
*/
|
||||
async function loadCoreUserScopeKeys() {
|
||||
const result = new Set();
|
||||
try {
|
||||
const corePath = await resolveInstalledModuleYaml('core');
|
||||
if (!corePath) return result;
|
||||
const parsed = yaml.parse(await fs.readFile(corePath, 'utf8'));
|
||||
if (!parsed || typeof parsed !== 'object') return result;
|
||||
for (const [key, value] of Object.entries(parsed)) {
|
||||
if (value && typeof value === 'object' && 'prompt' in value && value.scope === 'user') {
|
||||
result.add(key);
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Schema unavailable — fall back to two-tier routing.
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply parsed `--set` overrides to the central TOML files written by the
|
||||
* installer. Called at the end of an install / quick-update.
|
||||
*
|
||||
* Routing per (module, key):
|
||||
* 1. If `_bmad/config.user.toml` already has `[section] key`, update there
|
||||
* (user-scope key like `core.user_name`, `bmm.user_skill_level`).
|
||||
* 2. Otherwise update `_bmad/config.toml` (team scope, the default).
|
||||
* 1. If `~/.bmad/config.user.toml` already has `[section] key`, update there
|
||||
* (global identity store — same place writeGlobalUserCore writes to).
|
||||
* 2. Else if `_bmad/config.user.toml` already has `[section] key`, update
|
||||
* there (project-scoped personal override).
|
||||
* 3. Else if the key is a known core scope:user key (user_name,
|
||||
* communication_language per core/module.yaml), route to global. Otherwise
|
||||
* writeCentralConfig's next-install partition would strip the value out
|
||||
* of project files.
|
||||
* 4. Otherwise update `_bmad/config.toml` (team scope, the default).
|
||||
*
|
||||
* The schema-correct user/team partition lives in `manifest-generator`. We
|
||||
* intentionally don't re-read module schemas here — the only goal is to
|
||||
* match the file the installer just wrote the key to. For brand-new keys
|
||||
* (not in either file yet), team scope is the safe default.
|
||||
* The schema-correct partition lives in `manifest-generator`. We only reach
|
||||
* for the core schema (small, always present) so the first-run case for
|
||||
* `--set core.user_name=...` doesn't land in the wrong file.
|
||||
*
|
||||
* @param {Object<string, Object<string, string>>} overrides
|
||||
* @param {string} bmadDir absolute path to `_bmad/`
|
||||
|
|
@ -240,6 +272,8 @@ async function applySetOverrides(overrides, bmadDir) {
|
|||
|
||||
const teamPath = path.join(bmadDir, 'config.toml');
|
||||
const userPath = path.join(bmadDir, 'config.user.toml');
|
||||
const globalPath = globalUserConfigPath();
|
||||
const coreUserKeys = await loadCoreUserScopeKeys();
|
||||
|
||||
for (const moduleCode of Object.keys(overrides)) {
|
||||
// Skip overrides for modules not actually installed. The installer writes
|
||||
|
|
@ -258,16 +292,35 @@ async function applySetOverrides(overrides, bmadDir) {
|
|||
const value = moduleOverrides[key];
|
||||
const valueToml = tomlString(value);
|
||||
|
||||
const userOwnsIt = await tomlHasKey(userPath, section, key);
|
||||
const targetPath = userOwnsIt ? userPath : teamPath;
|
||||
// 3-tier routing: prefer the file that already owns the key; otherwise
|
||||
// honor core's user-scope partition (so `--set core.user_name` lands in
|
||||
// ~/.bmad on a fresh install, not in project team config).
|
||||
const globalOwnsIt = moduleCode === 'core' && (await tomlHasKey(globalPath, section, key));
|
||||
const userOwnsIt = !globalOwnsIt && (await tomlHasKey(userPath, section, key));
|
||||
const isCoreUserScope = moduleCode === 'core' && coreUserKeys.has(key) && !userOwnsIt;
|
||||
let targetPath;
|
||||
let scope;
|
||||
if (globalOwnsIt || isCoreUserScope) {
|
||||
targetPath = globalPath;
|
||||
scope = 'user';
|
||||
} else if (userOwnsIt) {
|
||||
targetPath = userPath;
|
||||
scope = 'user';
|
||||
} else {
|
||||
targetPath = teamPath;
|
||||
scope = 'team';
|
||||
}
|
||||
|
||||
// The team file always exists post-install; the user file only exists
|
||||
// if the install wrote at least one user-scope key. If we're routing to
|
||||
// it but it doesn't exist yet, create it with a minimal header so it
|
||||
// has the same shape as installer-written user toml.
|
||||
// The team file always exists post-install; the user/global files only
|
||||
// exist once the installer has reason to write to them. If we're routing
|
||||
// to one that doesn't exist yet, create it with a minimal header so it
|
||||
// has the same shape as installer-written files.
|
||||
let content = '';
|
||||
if (await fs.pathExists(targetPath)) {
|
||||
content = await fs.readFile(targetPath, 'utf8');
|
||||
} else if (targetPath === globalPath) {
|
||||
await fs.ensureDir(path.dirname(globalPath));
|
||||
content = '# Global personal BMad config (see ~/.bmad).\n';
|
||||
} else {
|
||||
content = '# Personal overrides for _bmad/config.toml.\n';
|
||||
}
|
||||
|
|
@ -277,8 +330,8 @@ async function applySetOverrides(overrides, bmadDir) {
|
|||
applied.push({
|
||||
module: moduleCode,
|
||||
key,
|
||||
scope: userOwnsIt ? 'user' : 'team',
|
||||
file: path.basename(targetPath),
|
||||
scope,
|
||||
file: targetPath === globalPath ? '~/.bmad/config.user.toml' : path.basename(targetPath),
|
||||
});
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue