fix(installer): apply --set core overrides before config collection - #2671
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
alexeyv has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
📝 WalkthroughWalkthroughThe installer now applies ChangesCore override collection
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/installer/ui.js`:
- Around line 817-829: The collection logic around coreSetToOption must apply
every supported setOverrides.core key, including project_name, while preserving
--set precedence over legacy options; update tools/installer/ui.js lines 817-829
accordingly. Add a test assertion in test/test-installation-components.js lines
3820-3849 confirming collectedConfig.core.project_name contains the supplied
override.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 18d4a239-ce06-4b32-83a8-4dae3c3df04d
📒 Files selected for processing (2)
test/test-installation-components.jstools/installer/ui.js
Comment on lines
+817
to
+829
| const coreSetToOption = { | ||
| user_name: 'userName', | ||
| communication_language: 'communicationLanguage', | ||
| document_output_language: 'documentOutputLanguage', | ||
| output_folder: 'outputFolder', | ||
| }; | ||
| const fromSet = {}; | ||
| for (const [key, optionField] of Object.entries(coreSetToOption)) { | ||
| if (setOverrides.core[key] !== undefined) fromSet[optionField] = setOverrides.core[key]; | ||
| } | ||
| // --set is the preferred form, so it wins if a legacy shortcut sets the | ||
| // same key. Rebind rather than mutate — `options` belongs to the caller. | ||
| options = { ...options, ...fromSet }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Apply every supported core override during collection.
project_name is a core key, but coreSetToOption omits it. Therefore --set core.project_name=... stays at the post-install patch stage and collectedConfig.core.project_name retains the default or prompted value.
tools/installer/ui.js#L817-L829: Merge all supportedsetOverrides.corekeys into the collection seed. Preserve--setprecedence over legacy options.test/test-installation-components.js#L3820-L3849: Add acore.project_nameassertion that verifies the collected core configuration contains the override.
📍 Affects 2 files
tools/installer/ui.js#L817-L829(this comment)test/test-installation-components.js#L3820-L3849
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/installer/ui.js` around lines 817 - 829, The collection logic around
coreSetToOption must apply every supported setOverrides.core key, including
project_name, while preserving --set precedence over legacy options; update
tools/installer/ui.js lines 817-829 accordingly. Add a test assertion in
test/test-installation-components.js lines 3820-3849 confirming
collectedConfig.core.project_name contains the supplied override.
alexeyv
force-pushed
the
fix/set-core-output-folder
branch
from
August 2, 2026 11:44
70cd78d to
27d05a0
Compare
There was a problem hiding this comment.
alexeyv has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
`--set core.<key>` was applied only as a post-install TOML patch, but core values are dependency-bearing: module artifact paths are built from output_folder during config collection, the output directory is created from those paths, and each module's config.yaml snapshots the core values at generate time. A patch that lands after all of that leaves the sources disagreeing. `--set core.output_folder=generated` produced output_folder: generated in core config, BMM paths under _bmad-output, and a _bmad-output/ directory on disk. `--set core.project_name=Foo` left BMM's copy on the default. The docs present --set core.<key> and the legacy shortcuts as equivalent and label --set the preferred form, so both were reachable by following the documented advice. Seed core config from setOverrides.core alongside the legacy shortcut flags, so every core key takes effect during collection rather than only the four that have a dedicated flag. Non-core overrides keep the existing post-install patch path.
alexeyv
force-pushed
the
fix/set-core-output-folder
branch
from
August 2, 2026 12:00
27d05a0 to
b29fe3d
Compare
There was a problem hiding this comment.
alexeyv has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
--set core.<key>now takes effect during config collection instead of only as a post-install TOML patch.Why
Core values are dependency-bearing: module artifact paths are built from
core.output_folderduring collection, and the output directory is created from those paths. Patching the key in afterwards left three sources of truth disagreeing.On
main,--set core.output_folder=generated:_bmad/core/config.yamlgeneratedgenerated_bmad/bmm/config.yamlpaths{project-root}/_bmad-output/…{project-root}/generated/…_bmad-output/generated/Exit code 0, no warning — BMAD then wrote artifacts into the folder the user had explicitly overridden. The docs present
--set core.<key>and the legacy shortcuts as equivalent, and label--setthe preferred form, so this was reachable by following the documented advice.How
Fold
--set core.<key>into the corresponding option fields (--user-name,--output-folder, …) before anything reads them, so both spellings feed the same collection step. Non-core overrides keep the existing post-install patch path unchanged.Testing
--setstill reaches the patch step. Revertingui.jsalone fails 4 of the 6 and passes the 2 guard assertions.--yes, legacy flag,--set core.output_folder,--set core.user_name,--set bmm.user_skill_level, and a combined core+bmm run — non-core paths unchanged.--set 'core.output_folder={project-root}/out'is written unrendered and resolves correctly.HUSKY=0 npm ci && npm run qualitygreen on the pushed HEAD (428 assertions, 0 failures).