feat(installer): add BMad Automator module#2345
Open
bma-d wants to merge 3 commits intobmad-code-org:mainfrom
Open
feat(installer): add BMad Automator module#2345bma-d wants to merge 3 commits intobmad-code-org:mainfrom
bma-d wants to merge 3 commits intobmad-code-org:mainfrom
Conversation
🤖 Augment PR SummarySummary: Adds an experimental “BMad Automator” installer module ( Changes:
Technical Notes: Automator is currently Claude Code–only for runnable skill installation and is marked experimental throughout the registry + UI messaging. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughAdds an experimental "BMad Automator" ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant ExternalModuleManager
participant ManifestGenerator
participant IDEInstaller
participant FileSystem
User->>UI: select modules & IDEs
UI->>ExternalModuleManager: fetch selected module metadata
ExternalModuleManager-->>UI: return normalized metadata (installTargets, sourceRoot, installNote)
UI-->>User: show installNotes / warn if no compatible IDE
UI->>ManifestGenerator: proceed to build manifest with selected modules
ManifestGenerator->>ExternalModuleManager: query listAvailable() for source-root codes
ExternalModuleManager-->>ManifestGenerator: provide sourceRoot set
ManifestGenerator->>FileSystem: detect skill-only modules (sourceRoot + SKILL.md)
ManifestGenerator-->>IDEInstaller: emit manifest excluding skipped skill-only agents
IDEInstaller->>ExternalModuleManager: resolve per-skill installTargets via shouldInstallSkillRecord
IDEInstaller->>FileSystem: install supported skills to target directories (.claude/skills, etc.)
IDEInstaller-->>UI: return results (installed, skippedUnsupported counts)
UI-->>User: display final summary and warnings
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/how-to/install-bmad.md (1)
34-64:⚠️ Potential issue | 🟡 MinorDocument
bma's runtime constraints in this install flow.This page now presents
bmaas a normal selectable external module, but it still doesn't tell readers here that the module is only runnable via the Claude Code entrypoint, that Codex is worker-only, or that macOS needstmux. Add that note near the module-picker/channel sections so the new installer warnings are discoverable before users run the install.As per coding guidelines, "Preserve the Claude Code-only constraints guidance in the install narrative where relevant (tmux requirement on macOS; Claude Code entrypoint only), and ensure any “notes/warnings during install” behavior is discoverable from the docs."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/how-to/install-bmad.md` around lines 34 - 64, The docs fail to surface bma's runtime constraints in the installer narrative; add a concise notice near the module-picker/channel section (the list that mentions external modules: bmb, cis, gds, tea, bma) stating that bma is only runnable via the Claude Code entrypoint, Codex is worker-only, and macOS requires tmux to run worker sessions; preserve the existing install flow language (channels: stable/next/pinned) and add this as a visible "Note/Warning" paragraph or callout so users see these constraints before running the installer.
🧹 Nitpick comments (1)
test/test-installation-components.js (1)
112-119: Consider addingworkflow.mdfiles in the automator fixture for realism.The fixture currently creates only
SKILL.md. Addingworkflow.mdwould better mirror production skill directories and catch copy-regression classes earlier.Suggested fixture refinement
for (const skillName of ['bmad-story-automator', 'bmad-story-automator-review']) { const skillDir = path.join(fixtureDir, 'bma', skillName); await fs.ensureDir(skillDir); await fs.writeFile( path.join(skillDir, 'SKILL.md'), ['---', `name: ${skillName}`, 'description: Automator skill', '---', '', 'Automator body.'].join('\n'), ); + await fs.writeFile( + path.join(skillDir, 'workflow.md'), + `# ${skillName}\n\nAutomator workflow body.\n`, + ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/test-installation-components.js` around lines 112 - 119, In the loop that creates automator fixtures (iterating over skillName and writing SKILL.md into skillDir), also create a workflow.md file for each automator skill to better mirror production directories; update the code that uses skillDir/skillName to call fs.writeFile for path.join(skillDir, 'workflow.md') with a minimal workflow frontmatter and body (e.g., name, description, steps or a dummy workflow) so tests exercise workflow-related logic and catch copy/regression issues earlier.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/test-installation-components.js`:
- Around line 2845-2848: The test only asserts that Codex skips
'bmad-story-automator' but misses asserting the second automator
'bmad-story-automator-review'; add a negative assertion mirroring the existing
one using tempProjectDir42, fs.pathExists and path.join to check that
path.join(tempProjectDir42, '.agents', 'skills', 'bmad-story-automator-review',
'SKILL.md') does NOT exist and use a clear message like 'Codex setup skips
Claude Code-only automator review skill' so the suite detects partial filtering
regressions.
In `@tools/installer/modules/external-manager.js`:
- Around line 127-137: When normalizing module metadata in external-manager.js,
coerce installTargets, workerTargets, and requirements into arrays and drop
invalid shapes: replace the current passthroughs for installTargets,
workerTargets, and requirements with logic that if the parsed value is an array
keep it, if it's a scalar (string/number/boolean) wrap it in a single-element
array, and for null/undefined or non-scalar non-array values return an empty
array; update the object fields installTargets, workerTargets, and requirements
accordingly so downstream code (e.g., UI code calling .some()) never receives a
non-array shape.
- Around line 515-521: The current logic builds sourceRoot with
path.join(cloneDir, moduleInfo.sourceRoot) which allows "../" escapes; change it
to resolve the absolute path (e.g. path.resolve(cloneDir,
moduleInfo.sourceRoot)) and verify it is contained inside the cloneDir (e.g.
compare path.relative(cloneDir, resolvedSourceRoot) does not start with '..' or
ensure resolvedSourceRoot.startsWith(path.resolve(cloneDir) + path.sep)); if the
check fails throw an Error similar to the existing one; update the code paths
around sourceRoot, cloneDir, and moduleInfo.sourceRoot to use the
resolved+validated path before returning.
---
Outside diff comments:
In `@docs/how-to/install-bmad.md`:
- Around line 34-64: The docs fail to surface bma's runtime constraints in the
installer narrative; add a concise notice near the module-picker/channel section
(the list that mentions external modules: bmb, cis, gds, tea, bma) stating that
bma is only runnable via the Claude Code entrypoint, Codex is worker-only, and
macOS requires tmux to run worker sessions; preserve the existing install flow
language (channels: stable/next/pinned) and add this as a visible "Note/Warning"
paragraph or callout so users see these constraints before running the
installer.
---
Nitpick comments:
In `@test/test-installation-components.js`:
- Around line 112-119: In the loop that creates automator fixtures (iterating
over skillName and writing SKILL.md into skillDir), also create a workflow.md
file for each automator skill to better mirror production directories; update
the code that uses skillDir/skillName to call fs.writeFile for
path.join(skillDir, 'workflow.md') with a minimal workflow frontmatter and body
(e.g., name, description, steps or a dummy workflow) so tests exercise
workflow-related logic and catch copy/regression issues earlier.
🪄 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
Run ID: 63e8b5b1-6b81-44bc-8fb8-82a5f40cd667
📒 Files selected for processing (9)
docs/how-to/install-bmad.mddocs/reference/modules.mdtest/test-installation-components.jstools/installer/README.mdtools/installer/core/manifest-generator.jstools/installer/ide/_config-driven.jstools/installer/modules/external-manager.jstools/installer/modules/registry-fallback.yamltools/installer/ui.js
bma-d
force-pushed
the
bma-d/add-automator
branch
from
2f328b3 to
6b60599
Compare
Author
|
@coderabbitai review |
|
Tip For best results, initiate chat on the files or code changes.
Just let me know what you need! 🐰 🧠 Learnings used |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/installer/ide/_config-driven.js`:
- Around line 218-224: Wrap the call to
ExternalModuleManager.getModuleByCode(moduleName) in a try/catch so lookup
failures don't throw and abort the install; if an error occurs, log a warning
including moduleName and the error, set targets to null (fail-open so the
install proceeds on all targets), cache that null into this.moduleTargetCache
for moduleName, and continue to return the existing expression (!targets ||
targets.includes(this.name)); reference ExternalModuleManager, getModuleByCode,
moduleTargetCache, moduleName and this.name when making the change.
In `@tools/installer/ui.js`:
- Around line 961-965: The calls to ExternalModuleManager.listAvailable inside
showSelectedExternalModuleNotes (and similarly in showSelectedModuleIdeWarnings)
must be guarded so registry failures don’t abort the installer: wrap the
listAvailable() call in a try/catch, log a non-fatal warning/error if it throws,
and fall back to an empty array (or null-safe value) so the rest of the function
can continue; ensure you reference ExternalModuleManager and listAvailable in
the catch handling and do not rethrow or call process.exit.
🪄 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
Run ID: e8c6824a-0d05-419e-b3f3-2b2f15ca7b1f
📒 Files selected for processing (9)
docs/how-to/install-bmad.mddocs/reference/modules.mdtest/test-installation-components.jstools/installer/README.mdtools/installer/core/manifest-generator.jstools/installer/ide/_config-driven.jstools/installer/modules/external-manager.jstools/installer/modules/registry-fallback.yamltools/installer/ui.js
✅ Files skipped from review due to trivial changes (2)
- docs/reference/modules.md
- tools/installer/modules/registry-fallback.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/how-to/install-bmad.md
- tools/installer/README.md
- tools/installer/core/manifest-generator.js
- tools/installer/modules/external-manager.js
Comment on lines
+218
to
+224
| const { ExternalModuleManager } = require('../modules/external-manager'); | ||
| this.externalModuleManager = this.externalModuleManager || new ExternalModuleManager(); | ||
| const moduleInfo = await this.externalModuleManager.getModuleByCode(moduleName); | ||
| const targets = moduleInfo?.installTargets?.length ? moduleInfo.installTargets : null; | ||
| this.moduleTargetCache.set(moduleName, targets); | ||
|
|
||
| return !targets || targets.includes(this.name); |
There was a problem hiding this comment.
Guard module metadata lookup failures to avoid hard install aborts.
getModuleByCode() is unguarded here; if it throws (registry/file read/transient failure), the whole IDE install can fail mid-run. In tooling code, this path should handle lookup failure explicitly (e.g., warn and fail-open/fail-closed consistently) instead of crashing the install flow.
Suggested hardening
const { ExternalModuleManager } = require('../modules/external-manager');
this.externalModuleManager = this.externalModuleManager || new ExternalModuleManager();
- const moduleInfo = await this.externalModuleManager.getModuleByCode(moduleName);
+ let moduleInfo = null;
+ try {
+ moduleInfo = await this.externalModuleManager.getModuleByCode(moduleName);
+ } catch {
+ // Metadata lookup is advisory for filtering; avoid aborting entire install.
+ this.moduleTargetCache.set(moduleName, null);
+ return true;
+ }
const targets = moduleInfo?.installTargets?.length ? moduleInfo.installTargets : null;
this.moduleTargetCache.set(moduleName, targets);As per coding guidelines tools/**: Build script/tooling. Check error handling and proper exit codes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/installer/ide/_config-driven.js` around lines 218 - 224, Wrap the call
to ExternalModuleManager.getModuleByCode(moduleName) in a try/catch so lookup
failures don't throw and abort the install; if an error occurs, log a warning
including moduleName and the error, set targets to null (fail-open so the
install proceeds on all targets), cache that null into this.moduleTargetCache
for moduleName, and continue to return the existing expression (!targets ||
targets.includes(this.name)); reference ExternalModuleManager, getModuleByCode,
moduleTargetCache, moduleName and this.name when making the change.
Comment on lines
+961
to
+965
| async showSelectedExternalModuleNotes(selectedModuleIds, externalModules = null) { | ||
| if (!externalModules) { | ||
| const externalManager = new ExternalModuleManager(); | ||
| externalModules = await externalManager.listAvailable(); | ||
| } |
There was a problem hiding this comment.
Guard advisory external-module lookups so they can’t fail the install flow.
showSelectedExternalModuleNotes() and showSelectedModuleIdeWarnings() do uncaught listAvailable() calls. If registry loading fails, the installer can abort during warning/notice rendering, even though this path is non-essential.
💡 Suggested hardening
+ async _safeListExternalModules() {
+ try {
+ const externalManager = new ExternalModuleManager();
+ return await externalManager.listAvailable();
+ } catch {
+ await prompts.log.warn('Could not load external module metadata; skipping module notes/warnings.');
+ return [];
+ }
+ }
+
async showSelectedExternalModuleNotes(selectedModuleIds, externalModules = null) {
- if (!externalModules) {
- const externalManager = new ExternalModuleManager();
- externalModules = await externalManager.listAvailable();
- }
+ if (!externalModules) externalModules = await this._safeListExternalModules();
const notes = externalModules
.filter((mod) => selectedModuleIds.includes(mod.code) && mod.installNote)
.map((mod) => `${mod.name}: ${mod.installNote}`);
@@
async showSelectedModuleIdeWarnings(selectedModuleIds, selectedIdes = []) {
- const externalManager = new ExternalModuleManager();
- const externalModules = await externalManager.listAvailable();
+ const externalModules = await this._safeListExternalModules();As per coding guidelines: tools/**: Build script/tooling. Check error handling and proper exit codes.
Also applies to: 976-979
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/installer/ui.js` around lines 961 - 965, The calls to
ExternalModuleManager.listAvailable inside showSelectedExternalModuleNotes (and
similarly in showSelectedModuleIdeWarnings) must be guarded so registry failures
don’t abort the installer: wrap the listAvailable() call in a try/catch, log a
non-fatal warning/error if it throws, and fall back to an empty array (or
null-safe value) so the rest of the function can continue; ensure you reference
ExternalModuleManager and listAvailable in the catch handling and do not rethrow
or call process.exit.
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
Adds BMad Automator as an experimental installer module backed by the bmad-code-org/bmad-automator source repo. The Automator payload is kept as pure skills, with Claude Code as the only runnable entrypoint.
Why
This lets users install the Automator from the BMAD installer while keeping the Automator source maintained upstream. The installer also makes the current constraints explicit: Claude Code entrypoint only, Claude Code/Codex workers, and tmux on macOS.
How
bmain the bundled registry fallback withsource-root, target, worker, and requirement metadata; fallback entries supplement the remote official registry when missing.Testing
Ran
npm test,npm run docs:validate-links, andnpm run docs:build. Also smoke-testednode tools/installer/bmad-cli.js install --modules bma --tools claude-code --yesand the Codex skip path with--tools codex --yes.