Commit Graph

5 Commits

Author SHA1 Message Date
Brian Madison 7ad054f0e9 fix(installer): address fourth-round PR #2353 review comments
(1) Use process.exitCode instead of process.exit() after --list-options
write (CodeRabbit major). process.exit() forces immediate termination
even with pending I/O, which can truncate buffered writes when stdout
is piped or captured by CI. Await the write callback, set exitCode,
and return so the event loop drains naturally.

(2) Thread setOverrides through Config → OfficialModules.build for
headless callers (CodeRabbit major). Non-UI entry points (direct
installer.install({...}) without going through ui.collectModuleConfigs)
previously got an empty override map. Config now carries setOverrides
and the headless branch of OfficialModules.build also runs
loadExistingConfig + applyOverridesAfterSeeding('core') to mirror the
UI path's semantics. The UI path is unaffected because it takes the
moduleConfigs early-return.

(3) Evaluate function defaults under skipPrompts and accept-defaults
paths (CodeRabbit major). Both branches were dropping function defaults
silently, so any same-module dynamic default (`{other_key}` placeholder
in default:) disappeared under --yes. Two-pass: write non-function
defaults first so the answer bag is populated, then call function
defaults with that bag. Try/catch around the call surfaces resolution
failures as warnings instead of crashing the install.

(4) Track result-only schema keys as declared (Augment medium). A
schema entry with `result:` and no `prompt:` was being classified as
"unknown" when targeted by --set, producing a wrong warning and
overwriting the computed template output with the raw value. Added
declaredResultKeys parallel to declaredPromptKeys; an override on
either is now seeded as the answer so the result template still
renders ({value} substitution preserved). Carry-forward block
refactored to consume the same set.

(5) Diagnose non-object module.yaml under --list-options (Augment low).
The non-object branch silently flipped moduleScopedFailure with no
output. Now emits "module.yaml is not a valid object (got <type>)"
mirroring the catch branch, and the type guard also catches arrays
which typeof reports as 'object'.

(6) Reword --list-options doc cache scope (CodeRabbit minor).
"Installed at least once on this machine" → "currently cached official
modules" with a note that cache can be cleared or absent on ephemeral
CI workers — accurately reflects what the command can discover.

Tests: +4 cases — Config.build setOverrides threading and default,
formatOptionsList non-object yaml diagnostic and ok:false. Total 347
passing.
2026-04-28 18:59:17 -05:00
Brian Madison fb57c81176 fix(installer): address third-round PR #2353 review comments
(J) Prototype pollution guard (CodeRabbit major).
`--set __proto__.x=1` previously mutated Object.prototype because
`overrides.__proto__` returned Object.prototype on a plain object,
and assigning `[key]=value` polluted every plain object in the process.
Verified the attack reproduces on f1c9e12 and is now blocked: parser
rejects __proto__/prototype/constructor segments, and the maps are
Object.create(null) for defense-in-depth.

(I) Non-zero exit when --list-options <module>'s yaml is unparseable
(CodeRabbit major). formatOptionsList tracks moduleScopedFailure and
returns ok:false in that case; install.js exits 1.

(F) Dynamic defaults can now see --set sibling values (Augment medium).
buildQuestion's function default falls back to
`this.collectedConfig[mod][otherKey]`, but overrides were only in
`allAnswers` (local) at default-evaluation time. Pre-write override
raw values to collectedConfig before the prompt batch so the
fallback resolves. Post-prompt template processing overwrites with
the rendered version.

(E) applyOverridesAfterSeeding no longer bypasses carry-forward when
the schema can't be loaded (Augment low). Restructured: schema-load
is now best-effort; without schema, declaredKeys is an empty Set, so
all overrides are flagged as "unknown" and carry-forward runs against
every prior key. Comment now matches behavior.

(G) Flag placeholder --set <spec> instead of <module.key=value>
(Augment low) — angle brackets in the placeholder were misleading;
the description spells out the spec format.

(H) README wording: "every available key" → "locally-known official
keys (built-in modules plus any external officials cached on this
machine)" (CodeRabbit minor) — accurately reflects scope.

Tests: +2 cases for prototype-pollution rejection. Total 343 passing.
2026-04-28 11:42:07 -05:00
Brian Madison ce12cc1a7f fix(installer): address second-round PR #2353 review comments
(A) Carry forward unknown core keys in applyOverridesAfterSeeding
(CodeRabbit major). Mirrors collectModuleConfig's carry-forward so
the skip-collection path used by core (when seeded by --yes / legacy
shortcuts) doesn't drop unknown keys on subsequent installs. Without
this, `--set core.future=x` on run #1 would silently disappear on
the next install.

(B) --list-options now exits non-zero on a single-module miss
(CodeRabbit major). formatOptionsList returns { text, ok }; install.js
exits 1 with text on stderr when ok=false, 0 with text on stdout
otherwise. CI scripts catch typos like `--list-options bmn`.

(C) Hermetic Suite 44 discovery tests (CodeRabbit minor). Point
BMAD_EXTERNAL_MODULES_CACHE at a temp dir and restore in a finally
block so test results don't depend on the developer / CI cache state.

(D) Case-insensitive --list-options filter (Augment). Discovery
already dedupes case-insensitively; the filter now matches the same
way, so `--list-options BMM` and `--list-options bmm` both find the
bmm built-in.

Tests: +7 cases (uppercase listing, ok flag, core carry-forward).
Total 340 passing.
2026-04-28 11:24:51 -05:00
Brian Madison f1c9e12618 fix(installer): address PR #2353 review comments
Carry forward unknown --set keys across upgrades (CodeRabbit major).
Without this, an unknown key like --set bmm.future_thing=hello landed in
config.toml on run #1 but was silently dropped on the next install
because collectModuleConfig rebuilds collectedConfig from prompt answers
only. collectModuleConfig now copies any non-declared keys from
_existingConfig into collectedConfig and tracks them in setOverrideKeys
so the manifest writer's schema-strict partition keeps them.

Guard single-select rendering with Array.isArray (CodeRabbit major):
a malformed truthy non-array would have aborted --list-options.

Unify core override handling: move the inline post-collection block
from ui.js into OfficialModules.applyOverridesAfterSeeding so core and
non-core take a single validated path. Removes duplicated schema-load
logic and inline requires from ui.js.

Remove dead code: findOfficialModuleYaml and readDeclaredKeys in
set-overrides.js were exported but never imported. Drop them and
their path/fs/yaml/project-root imports — the module is now pure
string-parsing with zero deps.

Doc fix: change "silently ignored" to "ignored with a warning" for
the --action quick-update note (Augment + CodeRabbit).

Polish: clearer flag placeholder (--set <module.key=value> instead of
the misleading <key=value>), trim-asymmetry rationale comment in
parseSetEntry, dedupe rationale in list-options.

Tests: +6 cases — collectModuleConfig --set application end-to-end
(prompt-skip with template rendering), and carry-forward of unknown
keys from _existingConfig. Total 333 passing.
2026-04-28 10:27:37 -05:00
Brian Madison f33d251790 feat(installer): add --set and --list-options for non-interactive config (#1663)
`--set <module>.<key>=<value>` (repeatable) sets any module config option
non-interactively. Scales to every module without growing the CLI surface
per option, and persists into _bmad/config.toml so values survive upgrades.

`--list-options [module]` prints every available --set key for built-in
and locally-cached official modules (community/custom users read their own
module.yaml). Pass a module code to scope the listing.

Validation rules, all non-fatal:
- Module not in --modules → warn and drop the value.
- Key not declared in module.yaml → warn but persist (forward-compat).
  The manifest writer's schema-strict partition exempts these so they
  survive into config.toml even though the schema doesn't know them.
- Malformed --set syntax → exit non-zero up front.

The legacy core shortcuts (--user-name, --output-folder, etc.) remain
supported as aliases for `--set core.<key>=<value>`. --set with
--action quick-update is ignored with a warning since quick-update
preserves the existing answers by design.

Files:
- tools/installer/set-overrides.js (new): parser
- tools/installer/list-options.js (new): discovery + formatter
- tools/installer/commands/install.js: flags + early validation
- tools/installer/ui.js: parse, warn-on-unselected, thread to OfficialModules
- tools/installer/modules/official-modules.js: pre-fill answers, persist unknowns
- tools/installer/core/config.js + installer.js: carry setOverrideKeys through
- tools/installer/core/manifest-generator.js: partition exempts override keys
- test/test-installation-components.js: +15 cases (Suite 44)
- docs/how-to/install-bmad.md, README.md: --set as preferred non-interactive path

Closes #1663
2026-04-28 09:54:34 -05:00