fix: add missing sources/selectedFiles to CLI --custom-content config#1624
fix: add missing sources/selectedFiles to CLI --custom-content config#1624jschulte wants to merge 4 commits intobmad-code-org:mainfrom
Conversation
The non-interactive --custom-content CLI flag builds a customContentConfig object missing the `sources`, `selected`, and `selectedFiles` properties that the installer expects. This causes findModuleSource() to fail with "Source for module X is not available" because customModulePaths is never populated. Align both CLI code paths (modify and fresh install flows) with the config shape produced by the interactive promptCustomContentSource() method. Fixes bmad-code-org#1623
There was a problem hiding this comment.
Pull request overview
Fixes the non-interactive --custom-content CLI flow so the installer receives the expected customContent config shape and can resolve local custom module sources (matching the interactive promptCustomContentSource() contract).
Changes:
- Add
selected: truewhen--custom-contentis provided and valid paths are found. - Populate
sourcesandselectedFilesin both “modify existing install” and “fresh install” CLI code paths. - Derive
selectedModuleIdsfrom each custom module’smodule.yamlcodefield.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tools/cli/lib/ui.js
Outdated
| const sources = customPaths.map((p, i) => ({ path: p, id: selectedModuleIds[i], name: selectedModuleIds[i] })); | ||
| customContentConfig = { | ||
| hasCustomContent: true, | ||
| selected: true, | ||
| paths: customPaths, | ||
| sources: sources, | ||
| selectedFiles: customPaths.map((p) => path.join(p, 'module.yaml')), |
There was a problem hiding this comment.
Same as earlier block: sources currently sets name to the module id, but the interactive flow uses module.yaml's name when present. Consider populating sources during the module.yaml parse loop (including name) to fully match promptCustomContentSource() output and avoid depending on positional indexing.
tools/cli/lib/ui.js
Outdated
| const sources = customPaths.map((p, i) => ({ path: p, id: selectedModuleIds[i], name: selectedModuleIds[i] })); | ||
| customModuleResult = { | ||
| selectedCustomModules: selectedModuleIds, | ||
| customContentConfig: { | ||
| hasCustomContent: true, | ||
| selected: true, | ||
| paths: customPaths, | ||
| sources: sources, | ||
| selectedFiles: customPaths.map((p) => path.join(p, 'module.yaml')), |
There was a problem hiding this comment.
sources is built with name set to the module id, but the interactive promptCustomContentSource() uses module.yaml's name (fallback to code). Since you already parse module.yaml earlier in this block, consider building sources during that loop so name matches the interactive config shape and avoids relying on index alignment between customPaths and selectedModuleIds.
| if (customPaths.length > 0) { | ||
| const sources = customPaths.map((p, i) => ({ path: p, id: selectedModuleIds[i], name: selectedModuleIds[i] })); | ||
| customModuleResult = { | ||
| selectedCustomModules: selectedModuleIds, | ||
| customContentConfig: { | ||
| hasCustomContent: true, | ||
| selected: true, | ||
| paths: customPaths, | ||
| sources: sources, | ||
| selectedFiles: customPaths.map((p) => path.join(p, 'module.yaml')), | ||
| selectedModuleIds: selectedModuleIds, | ||
| }, |
There was a problem hiding this comment.
This PR changes the non-interactive --custom-content behavior in both modify and fresh-install flows, but there doesn’t appear to be automated coverage for the flag (no tests reference custom-content). Consider adding an integration/unit test that runs an install with --custom-content and asserts the generated config includes selected: true, sources, and selectedFiles so this regression doesn’t reappear.
📝 WalkthroughWalkthroughThe change augments the customContentConfig object returned by CLI argument handling to include additional metadata fields ( Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tools/cli/lib/ui.js (3)
328-332:⚠️ Potential issue | 🟡 Minor
moduleMeta.codeaccess afteryaml.parsecan throw if YAML is empty.
yaml.parse('')returnsnull. The try-catch on lines 319–327 (and 464–472) only guardsreadFileandyaml.parse— but line 329 (and 474) accessesmoduleMeta.codeoutside that catch. If the file is empty or contains only comments,moduleMetaisnulland!moduleMeta.codethrows an uncaughtTypeError.Proposed fix (apply to both occurrences)
+ if (!moduleMeta) { + await prompts.log.warn(`Skipping custom content path: ${customPath} - module.yaml is empty`); + continue; + } if (!moduleMeta.code) {Also applies to: 473-477
306-336:⚠️ Potential issue | 🟡 MinorNo deduplication of custom content paths.
If a user passes the same path twice via
--custom-content /foo,/foo, both iterations will pass validation, and the same module will appear twice incustomPaths,selectedModuleIds,sources, andselectedFiles. The interactive flow avoids this by design (one-at-a-time with a "continue" prompt). Consider deduplicating onexpandedPathormoduleMeta.code.Also applies to: 451-481
352-367:⚠️ Potential issue | 🔴 Critical
--yespath for modify flow preserves existing custom modules but doesn't populate the installer's custom module metadata.When
options.yesis truthy andoptions.customContentis falsy (lines 352–367),customModuleResult.customContentConfigremains{ hasCustomContent: false }even though custom module names are preserved inselectedCustomModules. The installer (line 306–340) builds itscustomModulePathsmap only fromcustomContent.selectedFiles,customContent.sources, orcustomContent.cachedModules—none of which are populated by this code path. As a result, whenmoduleManager.findModuleSource()is called for cached modules during installation (line 1270), it returns null because thecustomModulePathsmap is empty, causing cached modules to silently skip installation. This mirrors the bug the PR aims to address.
🤖 Fix all issues with AI agents
In `@tools/cli/lib/ui.js`:
- Line 339: The CLI branches that build the sources array (where const sources =
customPaths.map((p, i) => ({ path: p, id: selectedModuleIds[i], name:
selectedModuleIds[i] }))) are setting name to the module code only; update both
CLI mappings to capture the module metadata (moduleMeta) alongside
selectedModuleIds and set name: moduleMeta.name || moduleMeta.code (matching
promptCustomContentSource()). Ensure you retrieve moduleMeta for each
selectedModuleIds[i] (e.g., from the same module lookup used elsewhere) and use
it when constructing the objects in both places where sources is created.
- Around line 338-350: Remove the unused "paths" property from the
customContentConfig object construction so the CLI branch produces the same
shape as promptCustomContentSource(); specifically, in the code that builds
customModuleResult (the customContentConfig object created alongside
selectedCustomModules), drop paths: customPaths and rely on sources (objects
with path, id, name) and selectedFiles for path information; do the same removal
in the other CLI branch that sets customContentConfig to ensure both branches
match promptCustomContentSource()'s shape.
🧹 Nitpick comments (3)
tools/cli/lib/ui.js (3)
297-351: Near-identical ~50-line blocks duplicated across modify and fresh-install paths.The custom-content-from-CLI logic (path validation loop, metadata reading,
sources/selectedFilesconstruction) is copy-pasted between the modify flow (lines 297–351) and the fresh-install flow (lines 442–492). Any future fix (e.g., thenamebug above) must be applied in both places, and they will inevitably drift.Consider extracting a shared helper, e.g.
buildCustomContentConfigFromPaths(paths), that both code paths call.Also applies to: 442-492
338-350: Modify flow buildssourcesvia index-correlated parallel arrays.
customPathsandselectedModuleIdsare two parallel arrays kept in sync by push order inside the same loop. This works today, but it's a fragile pattern — any refactor that reorders or filters one array without the other silently corrupts the mapping. The fresh-install path (line 484) has the same pattern. Using a single array of objects (as suggested in thenamefix above) eliminates this class of bug.
309-336:require('yaml')called inside the loop on every iteration.Lines 322 and 467 do
const yaml = require('yaml')insideforloops. Node cachesrequirecalls, so there's no correctness issue, but it's unconventional — the require should be hoisted to the top of the function (or the file) for clarity. This is pre-existing, not introduced by this PR, so low priority.Also applies to: 454-481
| if (customPaths.length > 0) { | ||
| const sources = customPaths.map((p, i) => ({ path: p, id: selectedModuleIds[i], name: selectedModuleIds[i] })); | ||
| customModuleResult = { | ||
| selectedCustomModules: selectedModuleIds, | ||
| customContentConfig: { | ||
| hasCustomContent: true, | ||
| selected: true, | ||
| paths: customPaths, | ||
| sources: sources, | ||
| selectedFiles: customPaths.map((p) => path.join(p, 'module.yaml')), | ||
| selectedModuleIds: selectedModuleIds, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check file size and read the relevant sections
wc -l tools/cli/lib/ui.jsRepository: bmad-code-org/BMAD-METHOD
Length of output: 93
🏁 Script executed:
# Read the specific lines mentioned in the review comment
echo "=== Lines 338-350 ==="
sed -n '338,350p' tools/cli/lib/ui.js
echo "=== Lines 483-492 ==="
sed -n '483,492p' tools/cli/lib/ui.js
echo "=== Lines 1452-1527 (promptCustomContentSource) ==="
sed -n '1452,1527p' tools/cli/lib/ui.jsRepository: bmad-code-org/BMAD-METHOD
Length of output: 3928
🏁 Script executed:
# Search for where customContentConfig is consumed
rg -n 'customContentConfig' tools/cli/lib/ui.js -A 3 -B 3Repository: bmad-code-org/BMAD-METHOD
Length of output: 6029
🏁 Script executed:
# Search for paths property usage in customContentConfig context
rg -n '\.paths' tools/cli/lib/ui.js -C 2Repository: bmad-code-org/BMAD-METHOD
Length of output: 51
🏁 Script executed:
# Search more broadly for how customContentConfig is used
rg -n 'customContent\.' tools/cli/lib/ui.js -C 2Repository: bmad-code-org/BMAD-METHOD
Length of output: 51
🏁 Script executed:
# Search for where the install or processing happens with customContent
rg -n 'customContent:' tools/cli/lib/ui.js -B 5 -A 5Repository: bmad-code-org/BMAD-METHOD
Length of output: 1612
🏁 Script executed:
# Look for files that might consume customContentConfig outside ui.js
fd --type=f -name '*.js' | xargs grep -l 'customContent' | head -20Repository: bmad-code-org/BMAD-METHOD
Length of output: 299
🏁 Script executed:
# Check what properties are actually accessed on customContentConfig
rg -n 'customContentConfig\.' tools/cli/lib/ui.js -C 1 | head -100Repository: bmad-code-org/BMAD-METHOD
Length of output: 1843
🏁 Script executed:
# Find and check files that consume customContent to see if they use paths
find tools -name '*.js' -type f | xargs grep -l 'customContent' 2>/dev/nullRepository: bmad-code-org/BMAD-METHOD
Length of output: 175
🏁 Script executed:
# Check the installer or main entry point
rg -n '\.paths' tools/ --type=js -C 2 | head -50Repository: bmad-code-org/BMAD-METHOD
Length of output: 745
🏁 Script executed:
# Look at what the installer does with customContent
fd --type=f '*.js' tools | xargs grep -l 'install\|installer' | head -5Repository: bmad-code-org/BMAD-METHOD
Length of output: 352
🏁 Script executed:
# Check imports in ui.js to understand the flow
head -50 tools/cli/lib/ui.jsRepository: bmad-code-org/BMAD-METHOD
Length of output: 1843
🏁 Script executed:
# Check the installer.js to see how customContent is consumed
sed -n '1,100p' tools/cli/installers/lib/core/installer.jsRepository: bmad-code-org/BMAD-METHOD
Length of output: 4488
🏁 Script executed:
# Search for where customContent is used in installer.js
rg -n 'customContent' tools/cli/installers/lib/core/installer.js -C 3 | head -80Repository: bmad-code-org/BMAD-METHOD
Length of output: 2159
🏁 Script executed:
# Check modules/manager.js for customContent usage
rg -n 'customContent\|paths' tools/cli/installers/lib/modules/manager.js -C 2 | head -60Repository: bmad-code-org/BMAD-METHOD
Length of output: 51
🏁 Script executed:
# Search more broadly for where the paths property from customContentConfig would be used
rg -n 'customPaths\|paths\[' tools/cli/lib/ui.js -C 2Repository: bmad-code-org/BMAD-METHOD
Length of output: 51
🏁 Script executed:
# Check if paths property is ever accessed anywhere in the codebase
rg -n '\bpaths\b' tools/cli/installers/lib/core/installer.js -C 2 | grep -A 3 -B 3 'customContent\|custom'Repository: bmad-code-org/BMAD-METHOD
Length of output: 895
🏁 Script executed:
# Search for any usage of the paths property at all in custom content context
rg -n 'customContent.*paths\|paths.*customContent' tools/ --type=jsRepository: bmad-code-org/BMAD-METHOD
Length of output: 51
🏁 Script executed:
# Let's check if sources property is the replacement for paths
rg -n '\.sources' tools/cli/installers/lib/modules/manager.js -C 2Repository: bmad-code-org/BMAD-METHOD
Length of output: 51
🏁 Script executed:
# Check the full context around custom content handling in installer
sed -n '303,340p' tools/cli/installers/lib/core/installer.jsRepository: bmad-code-org/BMAD-METHOD
Length of output: 1987
Remove the unused paths property from customContentConfig in both CLI branches.
Lines 345 and 488 set paths: customPaths on customContentConfig, but promptCustomContentSource() never sets this property, and the installer never consumes it. All necessary path information is already provided via sources (containing source objects with path, id, name) and selectedFiles. Remove paths to eliminate shape divergence between CLI and interactive flows.
🤖 Prompt for AI Agents
In `@tools/cli/lib/ui.js` around lines 338 - 350, Remove the unused "paths"
property from the customContentConfig object construction so the CLI branch
produces the same shape as promptCustomContentSource(); specifically, in the
code that builds customModuleResult (the customContentConfig object created
alongside selectedCustomModules), drop paths: customPaths and rely on sources
(objects with path, id, name) and selectedFiles for path information; do the
same removal in the other CLI branch that sets customContentConfig to ensure
both branches match promptCustomContentSource()'s shape.
tools/cli/lib/ui.js
Outdated
| } | ||
|
|
||
| if (customPaths.length > 0) { | ||
| const sources = customPaths.map((p, i) => ({ path: p, id: selectedModuleIds[i], name: selectedModuleIds[i] })); |
There was a problem hiding this comment.
name in sources always equals code, diverging from the interactive flow.
In promptCustomContentSource() (line 1506), the interactive path sets name: moduleData.name || moduleData.code. Here, both CLI paths set name: selectedModuleIds[i], which is always moduleMeta.code. If a module's module.yaml has a human-readable name field (e.g., "Pantheon Custom Module"), it's silently discarded and the terse code is used instead.
This means downstream consumers that display or key on sources[].name will behave differently depending on whether the user went through the interactive or CLI path.
Proposed fix (apply to both line 339 and line 484)
You'd need to capture moduleMeta.name alongside moduleMeta.code in the loop. For example, collect objects instead of bare IDs:
const customPaths = [];
- const selectedModuleIds = [];
+ const selectedModuleEntries = []; // { id, name }
for (const customPath of paths) {
...
customPaths.push(expandedPath);
- selectedModuleIds.push(moduleMeta.code);
+ selectedModuleEntries.push({ id: moduleMeta.code, name: moduleMeta.name || moduleMeta.code });
}
if (customPaths.length > 0) {
- const sources = customPaths.map((p, i) => ({ path: p, id: selectedModuleIds[i], name: selectedModuleIds[i] }));
+ const selectedModuleIds = selectedModuleEntries.map(e => e.id);
+ const sources = customPaths.map((p, i) => ({ path: p, id: selectedModuleEntries[i].id, name: selectedModuleEntries[i].name }));
...
}Also applies to: 484-484
🤖 Prompt for AI Agents
In `@tools/cli/lib/ui.js` at line 339, The CLI branches that build the sources
array (where const sources = customPaths.map((p, i) => ({ path: p, id:
selectedModuleIds[i], name: selectedModuleIds[i] }))) are setting name to the
module code only; update both CLI mappings to capture the module metadata
(moduleMeta) alongside selectedModuleIds and set name: moduleMeta.name ||
moduleMeta.code (matching promptCustomContentSource()). Ensure you retrieve
moduleMeta for each selectedModuleIds[i] (e.g., from the same module lookup used
elsewhere) and use it when constructing the objects in both places where sources
is created.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
- Use module display name in sources (name: moduleMeta.name || code) - Add null guard for empty module.yaml (yaml.parse returns null) - Remove unused paths property to match promptCustomContentSource shape
…warn) and removed paths from the config to match promptCustomContentSource()
* fix custom install bug * fix manager.js * From PR #1624: added empty module.yaml handling (skip + warn) and removed paths from the config to match promptCustomContentSource() * fix: custom-content quick-update ENOENT, pass --custom-content through, add PR#1624 improvements to allow update installs to work using non-interactive mode
3 participants
What
Fixes the non-interactive
--custom-contentCLI flag so custom modules install correctly. Both CLI code paths (modify and fresh install) now produce the same config shape as the interactivepromptCustomContentSource()method.Why
The
--custom-contentflag builds acustomContentConfigmissingsources,selected, andselectedFilesproperties. The installer expects these (set by the interactive path), sofindModuleSource()never finds the custom module and throws "Source for module X is not available".Fixes #1623
How
sources,selected, andselectedFilesto the config object in both the "modify existing install" flow (~line 338) and "fresh install" flow (~line 481) intools/cli/lib/ui.jspromptCustomContentSource()producesTesting
npm test— schemas, refs, install component tests, lint, markdownlint, prettier)npx bmad-method install --directory . --modules bmm,pantheon --custom-content ~/git/pantheon/src --tools claude-code --user-name Test --yesnow installs the custom module successfully