Add module tools: setup skill, config system, and builder updates#34
Merged
Add module tools: setup skill, config system, and builder updates#34
Conversation
Delete bmad-manifest.json, bmad-skill-manifest.yaml, manifest.py, bmad-manifest-schema.json, and metadata-reference.md files across both builders and samples. Remove all references from docs, build processes, quality scans, prepass scripts, templates, and references.
New skill: bmad-builder-config
- Simple workflow that installs/configures the bmb module into a project
- Creates/updates shared {project-root}/_bmad/config.yaml with module settings
- Merges module-help.csv entries with anti-zombie pattern (removes stale
entries before writing fresh ones)
- Two deterministic Python scripts: merge-config.py and merge-help-csv.py
- 22 unit tests covering fresh install, update, anti-zombie, round-trip
- Outcome-based SKILL.md (~39 lines) — scripts are self-documenting via --help
- Both lint gates pass clean (path-standards, script-standards)
Remove bmad-init from entire codebase (13 files):
- bmad-init skill no longer exists; config is read directly from
{project-root}/_bmad/config.yaml
- Templates: removed {if-bmad-init}/{/if-bmad-init} conditional blocks,
replaced with unconditional config loading from config.yaml
- Template substitution rules: removed bmad-init conditional section
- Classification reference: removed bmad-init mentions from type descriptions
- Complex workflow patterns: replaced bmad-init invocation pattern with
direct config file reading
- Build process (both builders): removed bmad-init as default external skill,
updated standalone field description
- Quality scanners: replaced "bmad-init variables" with "config variables"
in prompt-craft, workflow-integrity, and enhancement-opportunities scanners
- Script opportunities reference: updated variable tracking to reference
config.yaml directly
- Standard fields: updated standalone field from "Opts out of bmad-init?"
to "Fully standalone, no config needed?"
Update module-help.csv:
- New CSV column format (module,agent-name,skill-name,display-name,...)
- Added bmad-builder-config entry (Configure Builder, CB)
…it and legacy migration Renames the skill directory and updates all references (display name, menu code, descriptions). Splits configuration into shared config.yaml and personal config.user.yaml for gitignored user settings. Adds legacy config migration that carries forward matching values then removes old files. Expands SKILL.md with argument-based headless mode, default priority chain, and an outcome section requiring consistent use of user name and communication language. Updates both merge scripts and their tests for the new flags and features.
Explains when modules need configuration vs. sensible defaults or agent memory, how the setup skill writes config files and registers with the help system, and when standalone skills can skip the overhead entirely. Includes sample prompt for upcoming module scaffolding tooling.
Rewrites On Activation for agent-builder and workflow-builder to load config from _bmad/config.yaml and config.user.yaml with fallback defaults when config is missing. Also adds .ruff* to gitignore.
|
Caution Review failedPull request was closed or merged during review Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR removes per-skill manifests and manifest tooling, centralizes runtime config to Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant BuilderSkill as "Builder Skill (agent/workflow)"
participant SetupSkill as "bmad-builder-setup"
participant MergeConfig as "merge-config.py"
participant MergeHelp as "merge-help-csv.py"
participant Repo as "Project (_bmad/ files)"
rect rgba(135,206,235,0.5)
User->>BuilderSkill: Activate (may pass --headless)
BuilderSkill->>Repo: Read `_bmad/config.yaml` + `_bmad/config.user.yaml`
alt config missing
BuilderSkill->>User: Inform setup availability
end
end
rect rgba(144,238,144,0.5)
User->>SetupSkill: Run setup (provide answers / legacy dir)
SetupSkill->>MergeConfig: Invoke merge-config (module YAML + answers)
MergeConfig->>Repo: Update `_bmad/config.yaml` and `_bmad/config.user.yaml` (anti-zombie)
MergeConfig->>User: Print JSON summary
SetupSkill->>MergeHelp: Invoke merge-help-csv (help CSV)
MergeHelp->>Repo: Update `_bmad/module-help.csv` (anti-zombie)
MergeHelp->>User: Print JSON summary
end
rect rgba(255,182,193,0.5)
BuilderSkill->>Repo: Subsequent activations read updated config and help CSV
BuilderSkill->>User: Operate using config values and prompt files
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
🤖 Augment PR SummarySummary: Adds a first-class module setup skill for BMad Builder, migrates legacy per-module configs, and removes the old manifest-based metadata approach. Changes:
Technical Notes: Merge scripts use an anti-zombie approach (remove existing module section/rows before writing) and can optionally clean up legacy files under 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| args.legacy_dir, module_code, module_yaml, args.verbose | ||
| ) | ||
| if legacy_core or legacy_module: | ||
| answers = apply_legacy_defaults(answers, legacy_core, legacy_module) |
There was a problem hiding this comment.
When --legacy-dir is present, apply_legacy_defaults() can inject a core block even if the caller intentionally omitted core (e.g., because shared core already exists), and then merge_config() may overwrite existing shared core keys (like output_folder / document_output_language) with legacy values.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| 2. Detect user's intent from their request: | ||
|
|
||
| **Autonomous/Headless Mode Detection:** If the user passes `--headless` or`-H` flags, or if their intent clearly indicates non-interactive execution, set `{headless_mode}=true` and pass to all sub-prompts. | ||
| 2. Load config from `{project-root}/_bmad/config.yaml` (bmb section) and `config.user.yaml`. If missing, note that `bmad-builder-setup` is available and continue with fallbacks: |
There was a problem hiding this comment.
This instruction references config.user.yaml without the {project-root}/_bmad/ prefix, which may cause lookups relative to the skill directory rather than the project’s _bmad folder.
Severity: medium
Other Locations
src/skills/bmad-workflow-builder/SKILL.md:23
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| ## On Activation | ||
|
|
||
| 1. **Load config via bmad-init skill** — Store all returned vars for use: | ||
| 1. **Load config** from `{project-root}/_bmad/config.yaml` — Store all returned vars for use: |
There was a problem hiding this comment.
The template says {user_name} / {communication_language} come from {project-root}/_bmad/config.yaml, but this PR’s setup flow writes those user-only keys exclusively to {project-root}/_bmad/config.user.yaml, so generated skills may not be able to resolve them.
Severity: medium
Other Locations
src/skills/bmad-workflow-builder/assets/SKILL-template.md:31src/skills/bmad-workflow-builder/assets/SKILL-template.md:70
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| - module: {bmad-module-code} | ||
| - vars: user_name:BMad,communication_language:English,document_output_language:English,output_folder:{project-root}/_bmad-output,{output-location-variable}:{default-output-path} | ||
| Load config from {project-root}/_bmad/config.yaml | ||
| - Read the `core` section for shared variables (user_name, communication_language, etc.) |
There was a problem hiding this comment.
The config loader guidance references a core section inside config.yaml, but merge-config.py writes shared core keys at the root level (and migrates any legacy core: block up), so consumers following this doc may read the wrong structure.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| - Dependency graphing (parsing imports, references, skill entries) | ||
| - Memory structure validation (required sections, path correctness) | ||
| - Access boundary extraction and verification | ||
| - Dependency graphing (parsing imports, references, skill entries) |
There was a problem hiding this comment.
Dependency graphing (parsing imports, references, skill entries) appears twice in the “Common script-worthy operations” list, which looks unintentional and may confuse the build guidance.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/skills/bmad-workflow-builder/scripts/prepass-execution-deps.py (1)
164-182:⚠️ Potential issue | 🔴 CriticalDependency graph extraction is currently non-functional.
dep_graph/prefer_afterare never populated after stage discovery, so cycle/redundancy analysis is effectively disabled andhard_dependenciesalways returns empty.Suggested fix direction
def scan_execution_deps(skill_path: Path) -> dict: - # Build dependency graph from skill structure - dep_graph: dict[str, list[str]] = {} - prefer_after: dict[str, list[str]] = {} + # Build dependency graph from skill structure + dep_graph: dict[str, list[str]] = {} + prefer_after: dict[str, list[str]] = {} all_stages: set[str] = set() - # Check for stage-level prompt files at skill root - for f in sorted(skill_path.iterdir()): - if f.is_file() and f.suffix == '.md' and f.name != 'SKILL.md': - all_stages.add(f.stem) + stage_files = [ + f for f in sorted(skill_path.iterdir()) + if f.is_file() and f.suffix == '.md' and f.name != 'SKILL.md' + ] + for f in stage_files: + all_stages.add(f.stem) + dep_graph.setdefault(f.stem, []) + prefer_after.setdefault(f.stem, []) + + # TODO: parse per-stage dependency hints from markdown/frontmatter + # and populate dep_graph / prefer_after before running graph analyses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/skills/bmad-workflow-builder/scripts/prepass-execution-deps.py` around lines 164 - 182, The dep_graph and prefer_after maps are never filled after discovering stages, so detect_cycles, find_transitive_redundancy and find_parallel_groups operate on an empty graph and hard_dependencies ends up empty; modify the stage-scanning logic (the loop that fills all_stages) to also open each stage file and extract dependency metadata into dep_graph (hard dependencies) and prefer_after (soft/after relationships) before calling detect_cycles, find_transitive_redundancy and find_parallel_groups — parse whatever marker you use in stage files (frontmatter keys, "requires"/"after" lines, or similar) and ensure every discovered stage is present as a key in dep_graph (with empty list if none) so downstream functions see the actual graph.src/skills/bmad-agent-builder/quality-scan-structure.md (1)
140-152:⚠️ Potential issue | 🟡 MinorInvalid JSON: trailing comma in example output.
The JSON example has a trailing comma on line 145 (
"has_headless": false,) which is invalid JSON syntax. This could confuse implementations following this schema.🐛 Fix the trailing comma
"assessments": { "sections_found": ["Overview", "Identity"], "capabilities_count": 0, "has_memory": false, - "has_headless": false, + "has_headless": false },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/skills/bmad-agent-builder/quality-scan-structure.md` around lines 140 - 152, The JSON example under the "assessments" object contains an invalid trailing comma after the "has_headless" property; remove that trailing comma so the object is valid JSON (locate the "assessments" block and the "has_headless" key in the example output and delete the comma following false).src/skills/bmad-agent-builder/references/script-opportunities-reference.md (1)
170-178:⚠️ Potential issue | 🟡 MinorFix the activation step numbering gap.
Line 177 jumps from
6to8(“Present menu”). Please renumber to keep the sequence contiguous for readers and for any script/checklist derived from this list.✏️ Suggested doc fix
-6. Greet -8. Present menu +6. Greet +7. Present menu🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/skills/bmad-agent-builder/references/script-opportunities-reference.md` around lines 170 - 178, The ordered checklist has a numbering gap: after "6. Greet" it jumps to "8. Present menu"; change "8. Present menu" to "7. Present menu" so the sequence reads 1. Activation mode detection, 2. Config loading, 3. First-run check, 4. Access boundaries load, 5. Memory load, 6. Greet, 7. Present menu, and update any adjacent list numbering if present to keep the sequence contiguous (refer to the list items "Activation mode detection", "Greet", and "Present menu" to locate the exact spot).
🧹 Nitpick comments (5)
src/skills/bmad-builder-setup/scripts/merge-config.py (1)
266-268: Consider moving_CORE_USER_KEYSdefinition earlier in the file.
_CORE_USER_KEYSis used inmerge_config()at line 225 but defined at line 267. While this works (the function body isn't executed until called), placing constants near the top of the module (alongside_CORE_KEYSat line 88) improves readability and follows the convention of defining constants before their use.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/skills/bmad-builder-setup/scripts/merge-config.py` around lines 266 - 268, Move the constant definition of _CORE_USER_KEYS so it appears before its use in merge_config — e.g., relocate the _CORE_USER_KEYS = ("user_name", "communication_language") declaration up near the existing _CORE_KEYS constant (around where _CORE_KEYS is defined) so constants are declared prior to being referenced by the merge_config function; ensure no logic changes, only reposition the constant.src/skills/bmad-agent-builder/scripts/prepass-execution-deps.py (1)
202-212: Dependency graph functions are now effectively no-ops.With
dep_graphalways empty,detect_cycles(),find_transitive_redundancy(), and partiallyfind_parallel_groups()will never produce meaningful results. The graph-related functions (lines 33-110) are now dead code.If these capabilities are being removed intentionally with the manifest, consider removing the unused functions to reduce maintenance burden. If dependency analysis will be restored via a different mechanism, a TODO comment would clarify intent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/skills/bmad-agent-builder/scripts/prepass-execution-deps.py` around lines 202 - 212, The dependency graph (dep_graph) is never populated so functions detect_cycles, find_transitive_redundancy, and find_parallel_groups become dead code; either restore dep_graph population from your manifest/prompt metadata or remove/mark the unused functions. Locate where dep_graph, prefer_after and all_stages are initialized and either (A) implement logic to populate dep_graph by parsing prompt files or the skill manifest (so detect_cycles and find_transitive_redundancy operate on real data), or (B) remove or annotate detect_cycles, find_transitive_redundancy, and find_parallel_groups with a TODO/comment indicating dependency analysis is intentionally disabled. Ensure references to prompts_dir and f.stem are used in the chosen approach so stage names feed into dep_graph if you restore analysis..gitignore (1)
21-21: Consider narrowing the pattern to.ruff_cache/if config files should be tracked.The pattern
.ruff*will ignore both cache directories (.ruff_cache/) and configuration files (.ruff.toml). If Ruff configuration should be committed to the repository, consider using.ruff_cache/instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.gitignore at line 21, The .gitignore entry `.ruff*` is too broad and also ignores configuration files like `.ruff.toml`; replace or narrow it to `.ruff_cache/` (or add a separate `.ruff_cache/` line and remove `.ruff*`) so cache directories are ignored but Ruff config files (`.ruff.toml`) remain tracked; update the ignore pattern accordingly and ensure no other rules still match `.ruff.toml`.src/skills/bmad-agent-builder/build-process.md (1)
37-41: Remove the duplicated dependency-graphing bullet.Line 37 and Line 40 repeat the same guidance verbatim; keeping one avoids redundant prompting behavior.
Suggested cleanup
- Dependency graphing (parsing imports, references, skill entries) - Memory structure validation (required sections, path correctness) - Access boundary extraction and verification -- Dependency graphing (parsing imports, references, skill entries) - Pre-processing for LLM capabilities (extract compact metrics from large files so the LLM works from structured data, not raw content)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/skills/bmad-agent-builder/build-process.md` around lines 37 - 41, The markdown contains a duplicated bullet "Dependency graphing (parsing imports, references, skill entries)"; remove the second occurrence (the redundant line) so the list only includes that item once and retains the other bullets ("Memory structure validation...", "Access boundary extraction...", "Pre-processing for LLM capabilities..."), ensuring no extra blank lines are introduced.src/skills/bmad-agent-builder/scripts/prepass-prompt-metrics.py (1)
296-300: Minor redundancy in file filtering condition.Line 300 checks both
f.name not in skip_filesandf.name != 'SKILL.md', but'SKILL.md'is already inskip_files(line 297). The second check is redundant.♻️ Optional simplification
skip_files = {'SKILL.md'} for f in sorted(skill_path.iterdir()): - if f.is_file() and f.suffix == '.md' and f.name not in skip_files and f.name != 'SKILL.md': + if f.is_file() and f.suffix == '.md' and f.name not in skip_files: data = scan_file_patterns(f, f.name)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/skills/bmad-agent-builder/scripts/prepass-prompt-metrics.py` around lines 296 - 300, The loop that iterates files redundantly checks f.name not in skip_files and f.name != 'SKILL.md' even though 'SKILL.md' is already in skip_files; remove the extra equality check so the condition is just if f.is_file() and f.suffix == '.md' and f.name not in skip_files, keeping the skip_files set (skip_files) and the iterator over skill_path.iterdir() unchanged and only referencing f and f.name in the simplified condition.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Line 17: Add a clear changelog entry documenting the removal of manifest-based
architecture: update CHANGELOG.md to include a line stating that manifest files
and related code (e.g., bmad-manifest.json, manifest schemas, manifest.py and
any manifest handling in both builders) were removed and replaced by the YAML
configuration system so users are aware of the breaking change from
manifest-based to YAML-based configuration.
In `@docs/explanation/module-configuration.md`:
- Around line 123-124: Fix the typo in the documentation sentence inside
module-configuration.md where it currently reads "components amd structure" —
change "amd" to "and" so the phrase reads "components and structure"; update the
text near the sentence that references `bmad-builder-setup` SKILL.md, scripts,
tests, and the two asset files (`module.yaml` and `module-help.csv`)
accordingly.
In `@docs/reference/workflow-patterns.md`:
- Line 12: Update the outdated init name used in the Simple Utility workflow
description: find the table cell that includes the string `bmad-init` and
replace it with `bmad-builder-setup` so the documentation reflects the current
setup/config flow; ensure the replacement preserves formatting (inline code
backticks) and the rest of the row (e.g., SKILL.md + `scripts/`) remains
unchanged.
In `@src/skills/bmad-builder-setup/assets/module.yaml`:
- Line 18: The prompt string under the prompt key in module.yaml is
ambiguous—change "Evals, Test, Quality and Planning Reports?" to a clearer
phrasing such as "Evals, Tests, Quality, and Planning Reports?" (make "Test"
plural to "Tests" and add the Oxford comma for consistency) so the prompt
properly refers to multiple tests and reads cleanly.
- Around line 1-5: The YAML file uses unquoted scalars and inconsistent
formatting; update the module.yaml so all scalar values are double-quoted (e.g.,
"code", "name", "description", "module_version", "default_selected") and ensure
consistent YAML style (spacing, indentation and line endings) to satisfy the
linter and the project's rule that all YAML must use double quotes with
avoidEscape: true; edit the entries for code, name, description, module_version
and default_selected to be double-quoted and re-run the formatter to confirm no
other lines (noted lines 7-10 and 12-20) violate the same rule.
In `@src/skills/bmad-builder-setup/scripts/merge-help-csv.py`:
- Around line 16-17: The docstring lists exit code 2 but the script never uses
it; wrap the script's main processing (the top-level logic in this module) in a
try/except that catches unexpected runtime errors, print the exception to stderr
(or log it) and call sys.exit(2) on such exceptions, and update the top-level
docstring string accordingly if you prefer removing 2 instead; specifically, add
a guarded main() function or wrap existing top-level code in try: ... except
Exception as e: print(f"Runtime error: {e}", file=sys.stderr); sys.exit(2) so
the documented "2=runtime error" is actually emitted on I/O or other runtime
failures.
In `@src/skills/bmad-builder-setup/scripts/tests/test-merge-config.py`:
- Around line 449-451: The test unpacks load_legacy_values into core, mod, files
but never uses core; update the unpacking in the test to avoid the unused
variable by either assigning it to a throwaway name (e.g., _core) or only
unpacking the needed values (e.g., mod, files) when calling load_legacy_values
in the test_merge_config test where load_legacy_values is invoked; change the
tuple assignment accordingly so the unused value is not bound to `core`.
- Around line 422-424: The test unpacks three values from load_legacy_values
into core, mod, files but never uses mod; update the unpacking in
test-merge-config.py to avoid an unused variable by either assigning a throwaway
name (e.g., core, _, files = load_legacy_values(...)) or by only capturing the
needed return values (e.g., core, files = load_legacy_values(...)) depending on
the function's return shape; keep the call to load_legacy_values and assertions
unchanged and reference load_legacy_values, core, mod, files to locate the
change.
In `@src/skills/bmad-builder-setup/scripts/tests/test-merge-help-csv.py`:
- Around line 86-88: The test unpacks read_csv_rows(path) into header and rows
but never uses header; fix by changing the unpacking to ignore the header (e.g.,
use _ or discard it) so only rows is used in assertions (reference:
read_csv_rows and the test in test-merge-help-csv.py where header, rows =
read_csv_rows(path) is written).
In `@src/skills/bmad-builder-setup/SKILL.md`:
- Line 49: The help examples in SKILL.md use inconsistent executable paths
("./scripts/merge-config.py --help" vs "scripts/merge-help-csv.py --help");
update the examples so both commands use the same explicit path (e.g., prefix
both with "./scripts/") to ensure consistency and avoid failures when run from
different working directories—edit the line containing the two command examples
to use the unified path.
In `@src/skills/bmad-workflow-builder/assets/SKILL-template.md`:
- Around line 31-34: Update the "Load config" step in SKILL-template.md to load
and merge user-scoped settings from `_bmad/config.user.yaml` in addition to
`_bmad/config.yaml` so `{user_name}` and `{communication_language}` (and
optionally `{document_output_language}`) are sourced from the user config when
present; modify the instructions for the Load config step (the block referencing
`{user_name}`, `{communication_language}`, `{document_output_language}`) to
explicitly call out loading `_bmad/config.user.yaml` and merging/overriding
values, and apply the same change to the second duplicate block (the later block
at lines ~70-74) so workflows pick up user personalization.
- Line 68: The file contains a duplicated Markdown heading "## On Activation"
(the symbol "## On Activation") causing MD024; fix it by removing or renaming
the redundant heading so only one "## On Activation" remains—either delete the
extra heading and merge its content into the original section or change it to a
distinct heading (e.g., "### On Activation Details") to preserve structure and
pass markdown-lint.
In `@src/skills/bmad-workflow-builder/quality-scan-enhancement-opportunities.md`:
- Around line 191-193: The recovery message incorrectly references the old setup
entrypoint "module-init"; update the fallback text used in Stage 2 (the JSON
object with "detail" and "action" in quality-scan-enhancement-opportunities.md)
to point users to the new setup skill "bmad-builder-setup", and while here
ensure the Stage 2 logic detects a missing _bmad/config.yaml, explains how to
run "bmad-builder-setup" to create it, and offers an option to proceed with
sensible defaults if the user agrees; locate and update the "detail" and
"action" strings that compose the missing-config fallback for Stage 2 to
implement this corrected guidance.
In `@src/skills/bmad-workflow-builder/quality-scan-skill-cohesion.md`:
- Line 302: The checklist and implementation for the cohesion scan currently
only reads SKILL.md and prompts but omits references/*.md; update the “Parallel
read batch” step and the completeness/verification logic so the scanner reads
SKILL.md, all prompt files AND references/*.md (the “list resources/” step) in
the same parallel batch, and ensure the completeness check that currently
verifies SKILL.md + prompts also verifies references/*.md is present and
accounted for in the cohesion findings.
In `@src/skills/bmad-workflow-builder/references/complex-workflow-patterns.md`:
- Around line 33-42: Update the "Config Loading Pattern" guidance so workflows
read shared keys from the core config and user-specific keys from the user
config: change instructions that say to read user_name and
communication_language from `_bmad/config.yaml` to instead read shared variables
from the `core` section of `_bmad/config.yaml` and user-specific variables
(e.g., user_name, communication_language) from `_bmad/config.user.yaml`; ensure
the doc text under the "Config Loading Pattern" heading and any repeated
occurrences (mentions of `core` and module sections) reflect this separation so
generated workflows will load personal config from the user file.
---
Outside diff comments:
In `@src/skills/bmad-agent-builder/quality-scan-structure.md`:
- Around line 140-152: The JSON example under the "assessments" object contains
an invalid trailing comma after the "has_headless" property; remove that
trailing comma so the object is valid JSON (locate the "assessments" block and
the "has_headless" key in the example output and delete the comma following
false).
In `@src/skills/bmad-agent-builder/references/script-opportunities-reference.md`:
- Around line 170-178: The ordered checklist has a numbering gap: after "6.
Greet" it jumps to "8. Present menu"; change "8. Present menu" to "7. Present
menu" so the sequence reads 1. Activation mode detection, 2. Config loading, 3.
First-run check, 4. Access boundaries load, 5. Memory load, 6. Greet, 7. Present
menu, and update any adjacent list numbering if present to keep the sequence
contiguous (refer to the list items "Activation mode detection", "Greet", and
"Present menu" to locate the exact spot).
In `@src/skills/bmad-workflow-builder/scripts/prepass-execution-deps.py`:
- Around line 164-182: The dep_graph and prefer_after maps are never filled
after discovering stages, so detect_cycles, find_transitive_redundancy and
find_parallel_groups operate on an empty graph and hard_dependencies ends up
empty; modify the stage-scanning logic (the loop that fills all_stages) to also
open each stage file and extract dependency metadata into dep_graph (hard
dependencies) and prefer_after (soft/after relationships) before calling
detect_cycles, find_transitive_redundancy and find_parallel_groups — parse
whatever marker you use in stage files (frontmatter keys, "requires"/"after"
lines, or similar) and ensure every discovered stage is present as a key in
dep_graph (with empty list if none) so downstream functions see the actual
graph.
---
Nitpick comments:
In @.gitignore:
- Line 21: The .gitignore entry `.ruff*` is too broad and also ignores
configuration files like `.ruff.toml`; replace or narrow it to `.ruff_cache/`
(or add a separate `.ruff_cache/` line and remove `.ruff*`) so cache directories
are ignored but Ruff config files (`.ruff.toml`) remain tracked; update the
ignore pattern accordingly and ensure no other rules still match `.ruff.toml`.
In `@src/skills/bmad-agent-builder/build-process.md`:
- Around line 37-41: The markdown contains a duplicated bullet "Dependency
graphing (parsing imports, references, skill entries)"; remove the second
occurrence (the redundant line) so the list only includes that item once and
retains the other bullets ("Memory structure validation...", "Access boundary
extraction...", "Pre-processing for LLM capabilities..."), ensuring no extra
blank lines are introduced.
In `@src/skills/bmad-agent-builder/scripts/prepass-execution-deps.py`:
- Around line 202-212: The dependency graph (dep_graph) is never populated so
functions detect_cycles, find_transitive_redundancy, and find_parallel_groups
become dead code; either restore dep_graph population from your manifest/prompt
metadata or remove/mark the unused functions. Locate where dep_graph,
prefer_after and all_stages are initialized and either (A) implement logic to
populate dep_graph by parsing prompt files or the skill manifest (so
detect_cycles and find_transitive_redundancy operate on real data), or (B)
remove or annotate detect_cycles, find_transitive_redundancy, and
find_parallel_groups with a TODO/comment indicating dependency analysis is
intentionally disabled. Ensure references to prompts_dir and f.stem are used in
the chosen approach so stage names feed into dep_graph if you restore analysis.
In `@src/skills/bmad-agent-builder/scripts/prepass-prompt-metrics.py`:
- Around line 296-300: The loop that iterates files redundantly checks f.name
not in skip_files and f.name != 'SKILL.md' even though 'SKILL.md' is already in
skip_files; remove the extra equality check so the condition is just if
f.is_file() and f.suffix == '.md' and f.name not in skip_files, keeping the
skip_files set (skip_files) and the iterator over skill_path.iterdir() unchanged
and only referencing f and f.name in the simplified condition.
In `@src/skills/bmad-builder-setup/scripts/merge-config.py`:
- Around line 266-268: Move the constant definition of _CORE_USER_KEYS so it
appears before its use in merge_config — e.g., relocate the _CORE_USER_KEYS =
("user_name", "communication_language") declaration up near the existing
_CORE_KEYS constant (around where _CORE_KEYS is defined) so constants are
declared prior to being referenced by the merge_config function; ensure no logic
changes, only reposition the constant.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5a699987-9695-4b52-b88b-c96bf8ce521d
⛔ Files ignored due to path filters (2)
src/module-help.csvis excluded by!**/*.csvsrc/skills/bmad-builder-setup/assets/module-help.csvis excluded by!**/*.csv
📒 Files selected for processing (66)
.gitignoreCHANGELOG.mddocs/explanation/index.mddocs/explanation/module-configuration.mddocs/explanation/what-are-bmad-agents.mddocs/explanation/what-are-skills.mddocs/explanation/what-are-workflows.mddocs/index.mddocs/reference/bmad-skill-manifest.mddocs/reference/builder-commands.mddocs/reference/index.mddocs/reference/workflow-patterns.mdsamples/bmad-agent-dream-weaver/SKILL.mdsamples/bmad-agent-dream-weaver/bmad-manifest.jsonsamples/bmad-agent-dream-weaver/init.mdsamples/bmad-excalidraw/bmad-manifest.jsonsrc/module.yamlsrc/skills/bmad-agent-builder/SKILL.mdsrc/skills/bmad-agent-builder/assets/SKILL-template.mdsrc/skills/bmad-agent-builder/assets/quality-report-template.mdsrc/skills/bmad-agent-builder/bmad-manifest.jsonsrc/skills/bmad-agent-builder/bmad-skill-manifest.yamlsrc/skills/bmad-agent-builder/build-process.mdsrc/skills/bmad-agent-builder/quality-scan-agent-cohesion.mdsrc/skills/bmad-agent-builder/quality-scan-prompt-craft.mdsrc/skills/bmad-agent-builder/quality-scan-script-opportunities.mdsrc/skills/bmad-agent-builder/quality-scan-structure.mdsrc/skills/bmad-agent-builder/references/metadata-reference.mdsrc/skills/bmad-agent-builder/references/script-opportunities-reference.mdsrc/skills/bmad-agent-builder/references/skill-best-practices.mdsrc/skills/bmad-agent-builder/references/template-substitution-rules.mdsrc/skills/bmad-agent-builder/references/universal-scan-schema.mdsrc/skills/bmad-agent-builder/report-quality-scan-creator.mdsrc/skills/bmad-agent-builder/scripts/bmad-manifest-schema.jsonsrc/skills/bmad-agent-builder/scripts/manifest.pysrc/skills/bmad-agent-builder/scripts/prepass-execution-deps.pysrc/skills/bmad-agent-builder/scripts/prepass-prompt-metrics.pysrc/skills/bmad-agent-builder/scripts/prepass-structure-capabilities.pysrc/skills/bmad-builder-setup/SKILL.mdsrc/skills/bmad-builder-setup/assets/module.yamlsrc/skills/bmad-builder-setup/scripts/merge-config.pysrc/skills/bmad-builder-setup/scripts/merge-help-csv.pysrc/skills/bmad-builder-setup/scripts/tests/test-merge-config.pysrc/skills/bmad-builder-setup/scripts/tests/test-merge-help-csv.pysrc/skills/bmad-workflow-builder/SKILL.mdsrc/skills/bmad-workflow-builder/assets/SKILL-template.mdsrc/skills/bmad-workflow-builder/bmad-manifest.jsonsrc/skills/bmad-workflow-builder/bmad-skill-manifest.yamlsrc/skills/bmad-workflow-builder/build-process.mdsrc/skills/bmad-workflow-builder/quality-scan-enhancement-opportunities.mdsrc/skills/bmad-workflow-builder/quality-scan-execution-efficiency.mdsrc/skills/bmad-workflow-builder/quality-scan-prompt-craft.mdsrc/skills/bmad-workflow-builder/quality-scan-script-opportunities.mdsrc/skills/bmad-workflow-builder/quality-scan-skill-cohesion.mdsrc/skills/bmad-workflow-builder/quality-scan-workflow-integrity.mdsrc/skills/bmad-workflow-builder/references/classification-reference.mdsrc/skills/bmad-workflow-builder/references/complex-workflow-patterns.mdsrc/skills/bmad-workflow-builder/references/metadata-reference.mdsrc/skills/bmad-workflow-builder/references/script-opportunities-reference.mdsrc/skills/bmad-workflow-builder/references/standard-fields.mdsrc/skills/bmad-workflow-builder/references/template-substitution-rules.mdsrc/skills/bmad-workflow-builder/references/universal-scan-schema.mdsrc/skills/bmad-workflow-builder/scripts/bmad-manifest-schema.jsonsrc/skills/bmad-workflow-builder/scripts/manifest.pysrc/skills/bmad-workflow-builder/scripts/prepass-execution-deps.pysrc/skills/bmad-workflow-builder/scripts/prepass-workflow-integrity.py
💤 Files with no reviewable changes (19)
- docs/explanation/what-are-bmad-agents.md
- src/skills/bmad-agent-builder/references/template-substitution-rules.md
- src/skills/bmad-agent-builder/bmad-skill-manifest.yaml
- docs/reference/index.md
- src/skills/bmad-agent-builder/assets/quality-report-template.md
- samples/bmad-excalidraw/bmad-manifest.json
- docs/reference/bmad-skill-manifest.md
- src/skills/bmad-workflow-builder/references/template-substitution-rules.md
- src/skills/bmad-agent-builder/references/metadata-reference.md
- src/skills/bmad-workflow-builder/bmad-manifest.json
- src/skills/bmad-agent-builder/bmad-manifest.json
- src/skills/bmad-workflow-builder/references/metadata-reference.md
- samples/bmad-agent-dream-weaver/bmad-manifest.json
- src/skills/bmad-agent-builder/scripts/bmad-manifest-schema.json
- src/skills/bmad-workflow-builder/scripts/prepass-workflow-integrity.py
- src/skills/bmad-workflow-builder/bmad-skill-manifest.yaml
- src/skills/bmad-workflow-builder/scripts/bmad-manifest-schema.json
- src/skills/bmad-workflow-builder/scripts/manifest.py
- src/skills/bmad-agent-builder/scripts/manifest.py
| ### Removed | ||
|
|
||
| - Obsolete sample and manifest files from old skill structure | ||
| - Obsolete sample files from old skill structure |
There was a problem hiding this comment.
Document the removal of manifest files and architecture.
The changelog entry removes the mention of manifest files, but the PR objectives indicate that manifest-related concepts and files (bmad-manifest.json, manifest schemas, manifest.py, etc.) were removed across both builders as a major architectural change. This removal should be properly documented in the changelog to inform users of the breaking change from manifest-based to YAML-based configuration.
Consider adding a dedicated entry such as:
- Manifest-based skill architecture (replaced with YAML configuration system)This helps users understand the architectural shift when upgrading.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CHANGELOG.md` at line 17, Add a clear changelog entry documenting the removal
of manifest-based architecture: update CHANGELOG.md to include a line stating
that manifest files and related code (e.g., bmad-manifest.json, manifest
schemas, manifest.py and any manifest handling in both builders) were removed
and replaced by the YAML configuration system so users are aware of the breaking
change from manifest-based to YAML-based configuration.
Comment on lines
+123
to
+124
| A decent LLM will clone the entire `bmad-builder-setup` skill components amd structure — SKILL.md, scripts, tests — updating only the skill name, description, and the two asset files (`module.yaml` and `module-help.csv`) to reflect your module. | ||
| Take the time to ensure the description that triggers it is correct, along with the module.yaml and module-help.csv |
There was a problem hiding this comment.
Fix typo in user-facing docs.
Line 123 says “components amd structure”; this should be “and”.
✏️ Suggested fix
-A decent LLM will clone the entire `bmad-builder-setup` skill components amd structure — SKILL.md, scripts, tests — updating only the skill name, description, and the two asset files (`module.yaml` and `module-help.csv`) to reflect your module.
+A decent LLM will clone the entire `bmad-builder-setup` skill components and structure — SKILL.md, scripts, tests — updating only the skill name, description, and the two asset files (`module.yaml` and `module-help.csv`) to reflect your module.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| A decent LLM will clone the entire `bmad-builder-setup` skill components amd structure — SKILL.md, scripts, tests — updating only the skill name, description, and the two asset files (`module.yaml` and `module-help.csv`) to reflect your module. | |
| Take the time to ensure the description that triggers it is correct, along with the module.yaml and module-help.csv | |
| A decent LLM will clone the entire `bmad-builder-setup` skill components and structure — SKILL.md, scripts, tests — updating only the skill name, description, and the two asset files (`module.yaml` and `module-help.csv`) to reflect your module. | |
| Take the time to ensure the description that triggers it is correct, along with the module.yaml and module-help.csv |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/explanation/module-configuration.md` around lines 123 - 124, Fix the
typo in the documentation sentence inside module-configuration.md where it
currently reads "components amd structure" — change "amd" to "and" so the phrase
reads "components and structure"; update the text near the sentence that
references `bmad-builder-setup` SKILL.md, scripts, tests, and the two asset
files (`module.yaml` and `module-help.csv`) accordingly.
| | Type | Description | Structure | | ||
| | ---- | ----------- | --------- | | ||
| | **Simple Utility** | Input/output building block. Headless, composable, often script-driven. May opt out of `bmad-init` for true standalone use | Single SKILL.md + `scripts/` | | ||
| | **Simple Utility** | Input/output building block. Headless, composable, often script-driven. May opt out of `bmad-init` for true standalone use | SKILL.md + `scripts/` | |
There was a problem hiding this comment.
Line 12 uses outdated init terminology.
The doc still says bmad-init, but this PR’s setup/config flow centers on bmad-builder-setup. Keeping old naming here can misroute users during setup.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/reference/workflow-patterns.md` at line 12, Update the outdated init
name used in the Simple Utility workflow description: find the table cell that
includes the string `bmad-init` and replace it with `bmad-builder-setup` so the
documentation reflects the current setup/config flow; ensure the replacement
preserves formatting (inline code backticks) and the rest of the row (e.g.,
SKILL.md + `scripts/`) remains unchanged.
Comment on lines
+1
to
+5
| code: bmb | ||
| name: "BMad Builder" | ||
| description: "Standard Skill Compliant Factory for BMad Agents, Workflows and Modules" | ||
| module_version: 1.0.0 | ||
| default_selected: false |
There was a problem hiding this comment.
Resolve YAML style + formatting warnings before merge.
This file currently fails formatting checks, and string scalars like Line 1 (code) and Line 4 (module_version) are not double-quoted.
✅ Suggested fix
-code: bmb
+code: "bmb"
@@
-module_version: 1.0.0
+module_version: "1.0.0"Also applies to: 7-10, 12-20
🧰 Tools
🪛 GitHub Actions: Quality & Validation
[warning] 1-1: Prettier check reported formatting issues in this file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/skills/bmad-builder-setup/assets/module.yaml` around lines 1 - 5, The
YAML file uses unquoted scalars and inconsistent formatting; update the
module.yaml so all scalar values are double-quoted (e.g., "code", "name",
"description", "module_version", "default_selected") and ensure consistent YAML
style (spacing, indentation and line endings) to satisfy the linter and the
project's rule that all YAML must use double quotes with avoidEscape: true; edit
the entries for code, name, description, module_version and default_selected to
be double-quoted and re-run the formatter to confirm no other lines (noted lines
7-10 and 12-20) violate the same rule.
| result: "{project-root}/{value}" | ||
|
|
||
| bmad_builder_reports: | ||
| prompt: "Output for Evals, Test, Quality and Planning Reports?" |
There was a problem hiding this comment.
Tighten prompt wording for clarity.
Line 18 reads “Evals, Test, Quality and Planning Reports?” — “Tests” is likely intended.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/skills/bmad-builder-setup/assets/module.yaml` at line 18, The prompt
string under the prompt key in module.yaml is ambiguous—change "Evals, Test,
Quality and Planning Reports?" to a clearer phrasing such as "Evals, Tests,
Quality, and Planning Reports?" (make "Test" plural to "Tests" and add the
Oxford comma for consistency) so the prompt properly refers to multiple tests
and reads cleanly.
Comment on lines
+191
to
193
| "detail": "Stage 2 assumes a config exists at _bmad/config.yaml. A first-timer who invokes this skill directly gets a cryptic error with no guidance on how to recover. This would frustrate new users and create abandonment.", | ||
| "action": "Add a graceful fallback in stage 2: detect missing config, explain how to run the module-init skill, and offer to proceed with defaults." | ||
| } |
There was a problem hiding this comment.
Recovery action references the wrong setup entrypoint.
Line 192 tells users to run module-init, but this PR introduces bmad-builder-setup as the configuration/setup skill. If module-init is not an alias, this creates a broken recovery path for first-time users.
Suggested patch
- "action": "Add a graceful fallback in stage 2: detect missing config, explain how to run the module-init skill, and offer to proceed with defaults."
+ "action": "Add a graceful fallback in stage 2: detect missing config, explain how to run the bmad-builder-setup skill, and offer to proceed with defaults."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "detail": "Stage 2 assumes a config exists at _bmad/config.yaml. A first-timer who invokes this skill directly gets a cryptic error with no guidance on how to recover. This would frustrate new users and create abandonment.", | |
| "action": "Add a graceful fallback in stage 2: detect missing config, explain how to run the module-init skill, and offer to proceed with defaults." | |
| } | |
| "detail": "Stage 2 assumes a config exists at _bmad/config.yaml. A first-timer who invokes this skill directly gets a cryptic error with no guidance on how to recover. This would frustrate new users and create abandonment.", | |
| "action": "Add a graceful fallback in stage 2: detect missing config, explain how to run the bmad-builder-setup skill, and offer to proceed with defaults." |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/skills/bmad-workflow-builder/quality-scan-enhancement-opportunities.md`
around lines 191 - 193, The recovery message incorrectly references the old
setup entrypoint "module-init"; update the fallback text used in Stage 2 (the
JSON object with "detail" and "action" in
quality-scan-enhancement-opportunities.md) to point users to the new setup skill
"bmad-builder-setup", and while here ensure the Stage 2 logic detects a missing
_bmad/config.yaml, explains how to run "bmad-builder-setup" to create it, and
offers an option to proceed with sensible defaults if the user agrees; locate
and update the "detail" and "action" strings that compose the missing-config
fallback for Stage 2 to implement this corrected guidance.
| ## Process | ||
|
|
||
| 1. **Parallel read batch:** Read SKILL.md, bmad-manifest.json, all prompt files, and list resources/ — in a single parallel batch | ||
| 1. **Parallel read batch:** Read SKILL.md, all prompt files, and list resources/ — in a single parallel batch |
There was a problem hiding this comment.
Align process/completeness checks with declared scan targets.
Line 302 says to “list resources/” and Line 315 only verifies SKILL.md + prompts, but this scanner’s targets also require references/*.md. This can produce incomplete cohesion findings.
Suggested doc fix
-1. **Parallel read batch:** Read SKILL.md, all prompt files, and list resources/ — in a single parallel batch
+1. **Parallel read batch:** Read SKILL.md, all prompt files, and all `references/*.md` files — in a single parallel batch
...
-- Did I read SKILL.md and ALL prompts?
+- Did I read SKILL.md, ALL prompts, and all `references/*.md` files?Also applies to: 315-315
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/skills/bmad-workflow-builder/quality-scan-skill-cohesion.md` at line 302,
The checklist and implementation for the cohesion scan currently only reads
SKILL.md and prompts but omits references/*.md; update the “Parallel read batch”
step and the completeness/verification logic so the scanner reads SKILL.md, all
prompt files AND references/*.md (the “list resources/” step) in the same
parallel batch, and ensure the completeness check that currently verifies
SKILL.md + prompts also verifies references/*.md is present and accounted for in
the cohesion findings.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
samples/bmad-excalidraw/SKILL.md (1)
36-40: Clarify greeting behavior whenuser_nameis omitted.Line 36 specifies that
{user_name}should be omitted when missing from config, but line 40 instructs to "Greet user as{user_name}" without explaining what happens when the name is unavailable. Consider adding brief guidance, such as "greet the user (using{user_name}if available)" or "greet the user generically if{user_name}is omitted."💬 Example clarification
-2. **Greet user** as `{user_name}`, speaking in `{communication_language}` +2. **Greet user** (using `{user_name}` if available), speaking in `{communication_language}`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/bmad-excalidraw/SKILL.md` around lines 36 - 40, The instruction "Greet user as {user_name}" is ambiguous when {user_name} is omitted; update the SKILL.md text to explicitly state fallback behavior (e.g., "Greet the user using {user_name} if available; otherwise use a generic greeting such as 'Hello' or 'Hi there' in {communication_language}"). Mention the placeholder {user_name} and {communication_language} so readers know to prefer the configured name and otherwise use a generic, language-matched greeting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@samples/bmad-agent-dream-weaver/SKILL.md`:
- Line 63: Update the prompt text that currently references
"{project-root}/_bmad/config.yaml and config.user.yaml" so the second filename
is fully qualified as "{project-root}/_bmad/config.user.yaml"; specifically
change the sentence that mentions loading config in SKILL.md (the line
containing "{project-root}/_bmad/config.yaml and config.user.yaml" and the
guidance for {communication_language} and {user_name}) to use the explicit full
path for config.user.yaml to avoid cwd-dependent resolution.
- Around line 81-87: The skill menu references three capability IDs that lack
corresponding markdown capability files — add capability prompt files named
dream-capture.md, lucid-coaching.md, and pattern-analysis.md (matching the IDs
used in the SKILL.md menu) containing the expected capability metadata and
prompts so the loader that opens {name}.md does not fail, or alternatively
remove/rename those IDs in the menu to match existing capability files; ensure
the filenames exactly match the IDs (dream-capture, lucid-coaching,
pattern-analysis) referenced by the menu.
In `@samples/bmad-excalidraw/SKILL.md`:
- Line 38: Update the fallback shown for `output_folder` in SKILL.md to match
the authoritative default used by the bmad-builder-setup skill: replace the
current `{project-root}/diagrams` fallback with `{project-root}/_bmad-output` so
all skills use the same output location (this aligns `output_folder` and avoids
the divergent `_bmad-output/diagrams/` vs `diagrams/diagrams/` behavior).
---
Nitpick comments:
In `@samples/bmad-excalidraw/SKILL.md`:
- Around line 36-40: The instruction "Greet user as {user_name}" is ambiguous
when {user_name} is omitted; update the SKILL.md text to explicitly state
fallback behavior (e.g., "Greet the user using {user_name} if available;
otherwise use a generic greeting such as 'Hello' or 'Hi there' in
{communication_language}"). Mention the placeholder {user_name} and
{communication_language} so readers know to prefer the configured name and
otherwise use a generic, language-matched greeting.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 474d6196-6ad1-4e94-ab81-a1e3d1f1ccc4
📒 Files selected for processing (21)
docs/explanation/what-are-bmad-agents.mdsamples/bmad-agent-dream-weaver/SKILL.mdsamples/bmad-agent-dream-weaver/dream-log.mdsamples/bmad-agent-dream-weaver/dream-seed.mdsamples/bmad-agent-dream-weaver/headless-wake.mdsamples/bmad-agent-dream-weaver/init.mdsamples/bmad-agent-dream-weaver/lucid-coach.mdsamples/bmad-agent-dream-weaver/pattern-discovery.mdsamples/bmad-agent-dream-weaver/recall-training.mdsamples/bmad-agent-dream-weaver/references/memory-system.mdsamples/bmad-excalidraw/SKILL.mdsrc/skills/bmad-agent-builder/assets/SKILL-template.mdsrc/skills/bmad-agent-builder/assets/autonomous-wake.mdsrc/skills/bmad-agent-builder/assets/init-template.mdsrc/skills/bmad-agent-builder/assets/memory-system.mdsrc/skills/bmad-agent-builder/build-process.mdsrc/skills/bmad-agent-builder/quality-scan-execution-efficiency.mdsrc/skills/bmad-agent-builder/references/standard-fields.mdsrc/skills/bmad-agent-builder/scripts/scan-path-standards.pysrc/skills/bmad-workflow-builder/references/standard-fields.mdtools/validate-file-refs.mjs
✅ Files skipped from review due to trivial changes (10)
- tools/validate-file-refs.mjs
- src/skills/bmad-agent-builder/assets/memory-system.md
- samples/bmad-agent-dream-weaver/recall-training.md
- src/skills/bmad-agent-builder/assets/autonomous-wake.md
- samples/bmad-agent-dream-weaver/lucid-coach.md
- samples/bmad-agent-dream-weaver/pattern-discovery.md
- src/skills/bmad-agent-builder/quality-scan-execution-efficiency.md
- samples/bmad-agent-dream-weaver/references/memory-system.md
- samples/bmad-agent-dream-weaver/dream-log.md
- src/skills/bmad-agent-builder/assets/init-template.md
🚧 Files skipped from review as they are similar to previous changes (5)
- samples/bmad-agent-dream-weaver/init.md
- src/skills/bmad-agent-builder/build-process.md
- src/skills/bmad-agent-builder/assets/SKILL-template.md
- src/skills/bmad-workflow-builder/references/standard-fields.md
- docs/explanation/what-are-bmad-agents.md
| - `{project-root}/_bmad/_memory/dream-weaver-sidecar/access-boundaries.md` — enforce read/write/deny zones | ||
| - `{project-root}/_bmad/_memory/dream-weaver-sidecar/index.md` — essential context and previous session | ||
| - `bmad-manifest.json` — set `{capabilities}` list | ||
| - **Load config** from `{project-root}/_bmad/config.yaml` and `config.user.yaml`. Use `{communication_language}` for all communications. For `{user_name}`: check sidecar memory first, then config — if neither has it, ask the user what they'd like to be called and store it in sidecar memory for future sessions. |
There was a problem hiding this comment.
Use an explicit full path for config.user.yaml.
Line 63 leaves config.user.yaml unqualified; make it {project-root}/_bmad/config.user.yaml to avoid cwd-dependent resolution in prompt execution.
💡 Proposed fix
- - **Load config** from `{project-root}/_bmad/config.yaml` and `config.user.yaml`. Use `{communication_language}` for all communications. For `{user_name}`: check sidecar memory first, then config — if neither has it, ask the user what they'd like to be called and store it in sidecar memory for future sessions.
+ - **Load config** from `{project-root}/_bmad/config.yaml` and `{project-root}/_bmad/config.user.yaml`. Use `{communication_language}` for all communications. For `{user_name}`: check sidecar memory first, then config — if neither has it, ask the user what they'd like to be called and store it in sidecar memory for future sessions.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - **Load config** from `{project-root}/_bmad/config.yaml` and `config.user.yaml`. Use `{communication_language}` for all communications. For `{user_name}`: check sidecar memory first, then config — if neither has it, ask the user what they'd like to be called and store it in sidecar memory for future sessions. | |
| - **Load config** from `{project-root}/_bmad/config.yaml` and `{project-root}/_bmad/config.user.yaml`. Use `{communication_language}` for all communications. For `{user_name}`: check sidecar memory first, then config — if neither has it, ask the user what they'd like to be called and store it in sidecar memory for future sessions. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@samples/bmad-agent-dream-weaver/SKILL.md` at line 63, Update the prompt text
that currently references "{project-root}/_bmad/config.yaml and
config.user.yaml" so the second filename is fully qualified as
"{project-root}/_bmad/config.user.yaml"; specifically change the sentence that
mentions loading config in SKILL.md (the line containing
"{project-root}/_bmad/config.yaml and config.user.yaml" and the guidance for
{communication_language} and {user_name}) to use the explicit full path for
config.user.yaml to avoid cwd-dependent resolution.
| 1. **Load config** from `{project-root}/_bmad/config.yaml` and `config.user.yaml`. If missing, continue with fallbacks: | ||
| - `{user_name}` — fallback: omit | ||
| - `{communication_language}` — fallback: match the user's language | ||
| - `{output_folder}` — fallback: `{project-root}/diagrams` |
There was a problem hiding this comment.
Inconsistent output_folder default creates cross-skill confusion.
The fallback value {project-root}/diagrams conflicts with the authoritative default {project-root}/_bmad-output defined in the bmad-builder-setup skill. This inconsistency means:
- Users who run setup will have diagrams saved to
_bmad-output/diagrams/(per line 23) - Users who skip setup will have diagrams saved to
diagrams/diagrams/(redundant path) - Different skills will use different output locations, causing confusion
The fallback should match the authoritative default to ensure consistent behavior across all skills.
📁 Proposed fix to align with authoritative default
- - `{output_folder}` — fallback: `{project-root}/diagrams`
+ - `{output_folder}` — fallback: `{project-root}/_bmad-output`As per coding guidelines, the bmad-builder-setup skill establishes the authoritative config contract with output_folder default of {project-root}/_bmad-output.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - `{output_folder}` — fallback: `{project-root}/diagrams` | |
| - `{output_folder}` — fallback: `{project-root}/_bmad-output` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@samples/bmad-excalidraw/SKILL.md` at line 38, Update the fallback shown for
`output_folder` in SKILL.md to match the authoritative default used by the
bmad-builder-setup skill: replace the current `{project-root}/diagrams` fallback
with `{project-root}/_bmad-output` so all skills use the same output location
(this aligns `output_folder` and avoids the divergent `_bmad-output/diagrams/`
vs `diagrams/diagrams/` behavior).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/skills/bmad-agent-builder/scripts/prepass-structure-capabilities.py (1)
221-264:⚠️ Potential issue | 🟠 MajorMemory-path consistency check is now lossy and can miss legacy
.memoryreferences.At Line 221, the regex captures from
memory/onward, so_bmad/.memory/...and_bmad/memory/...collapse to the same value. That makes the inconsistency logic at Line 243+ unable to detect mixed old/new formats during migration.💡 Proposed fix
- mem_pattern = re.compile(r'(?:memory/|sidecar/)[\w\-/]+(?:\.\w+)?') + # Preserve legacy vs new memory style so migration inconsistencies are detectable. + mem_pattern = re.compile(r'(?:_bmad/)?(?:\.?memory|sidecar)/[\w./-]+') @@ - prefixes = set() - for p in sorted_paths: - prefix = p.split('/')[0] - prefixes.add(prefix) - - memory_prefixes = {p for p in prefixes if 'memory' in p.lower()} + memory_styles = set() + sidecar_prefixes = set() + for p in sorted_paths: + if re.search(r'(?:^|/)\.memory/', p): + memory_styles.add('.memory') + elif re.search(r'(?:^|/)memory/', p): + memory_styles.add('memory') + if re.search(r'(?:^|/)sidecar/', p): + sidecar_prefixes.add('sidecar') @@ - if len(memory_prefixes) > 1: + if len(memory_styles) > 1: findings.append({ 'file': 'multiple', 'line': 0, 'severity': 'medium', 'category': 'memory-paths', - 'issue': f'Inconsistent memory path prefixes: {", ".join(sorted(memory_prefixes))}', + 'issue': f'Inconsistent memory path formats: {", ".join(sorted(memory_styles))}', + }) + + if '.memory' in memory_styles: + findings.append({ + 'file': 'multiple', 'line': 0, + 'severity': 'medium', 'category': 'memory-paths', + 'issue': 'Legacy ".memory/" path detected; migrate to "memory/"', })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/skills/bmad-agent-builder/scripts/prepass-structure-capabilities.py` around lines 221 - 264, The regex mem_pattern currently strips the leading dot in legacy ".memory" names so _bmad/.memory/... and _bmad/memory/... collapse and the inconsistency check (memory_prefixes/sidecar_prefixes over sorted_paths) misses mixed formats; update mem_pattern to preserve and distinguish ".memory" vs "memory" (i.e., match and capture the actual leading token including a possible leading dot or preceding segment) or alter the prefix extraction logic to consider the exact token (e.g., include the full first path component including any leading '.'), so memory_paths, sorted_paths and the subsequent memory_prefixes/sidecar_prefixes computation correctly detect old (.memory) vs new (memory) formats.
♻️ Duplicate comments (1)
samples/bmad-agent-dream-weaver/SKILL.md (1)
63-63:⚠️ Potential issue | 🟡 MinorUse a fully qualified path for user config.
config.user.yamlis still unqualified here; use{project-root}/_bmad/config.user.yamlto avoid cwd-dependent resolution.Proposed fix
- - **Load config** from `{project-root}/_bmad/config.yaml` and `config.user.yaml`. Use `{communication_language}` for all communications. For `{user_name}`: check sidecar memory first, then config — if neither has it, ask the user what they'd like to be called and store it in sidecar memory for future sessions. + - **Load config** from `{project-root}/_bmad/config.yaml` and `{project-root}/_bmad/config.user.yaml`. Use `{communication_language}` for all communications. For `{user_name}`: check sidecar memory first, then config — if neither has it, ask the user what they'd like to be called and store it in sidecar memory for future sessions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@samples/bmad-agent-dream-weaver/SKILL.md` at line 63, The documentation line under "Load config" references an unqualified filename "config.user.yaml" which can lead to cwd-dependent resolution; update that sentence to use the fully qualified path "{project-root}/_bmad/config.user.yaml" (so it reads something like "from {project-root}/_bmad/config.yaml and {project-root}/_bmad/config.user.yaml") and ensure the phrase that follows (about using {communication_language} and resolving {user_name} via sidecar memory then config) still refers to the same qualified config path; modify the "Load config" sentence in SKILL.md where "config.user.yaml" appears.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@samples/bmad-agent-dream-weaver/SKILL.md`:
- Around line 81-87: SKILL.md lists capability IDs dream-capture,
lucid-coaching, and pattern-analysis but the corresponding prompt files are
missing; create three new markdown files named dream-capture.md,
lucid-coaching.md, and pattern-analysis.md in the
samples/bmad-agent-dream-weaver/ directory, each containing the capability
prompt for the matching ID (title/header matching the ID, a short description of
the capability, input/output expectations, and example usage) so the skill
selector can load them when referenced by SKILL.md.
---
Outside diff comments:
In `@src/skills/bmad-agent-builder/scripts/prepass-structure-capabilities.py`:
- Around line 221-264: The regex mem_pattern currently strips the leading dot in
legacy ".memory" names so _bmad/.memory/... and _bmad/memory/... collapse and
the inconsistency check (memory_prefixes/sidecar_prefixes over sorted_paths)
misses mixed formats; update mem_pattern to preserve and distinguish ".memory"
vs "memory" (i.e., match and capture the actual leading token including a
possible leading dot or preceding segment) or alter the prefix extraction logic
to consider the exact token (e.g., include the full first path component
including any leading '.'), so memory_paths, sorted_paths and the subsequent
memory_prefixes/sidecar_prefixes computation correctly detect old (.memory) vs
new (memory) formats.
---
Duplicate comments:
In `@samples/bmad-agent-dream-weaver/SKILL.md`:
- Line 63: The documentation line under "Load config" references an unqualified
filename "config.user.yaml" which can lead to cwd-dependent resolution; update
that sentence to use the fully qualified path
"{project-root}/_bmad/config.user.yaml" (so it reads something like "from
{project-root}/_bmad/config.yaml and {project-root}/_bmad/config.user.yaml") and
ensure the phrase that follows (about using {communication_language} and
resolving {user_name} via sidecar memory then config) still refers to the same
qualified config path; modify the "Load config" sentence in SKILL.md where
"config.user.yaml" appears.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d9d00f89-16ad-4649-8261-703629c1d33c
📒 Files selected for processing (20)
docs/explanation/what-are-bmad-agents.mdsamples/bmad-agent-dream-weaver/SKILL.mdsamples/bmad-agent-dream-weaver/dream-log.mdsamples/bmad-agent-dream-weaver/dream-seed.mdsamples/bmad-agent-dream-weaver/headless-wake.mdsamples/bmad-agent-dream-weaver/init.mdsamples/bmad-agent-dream-weaver/lucid-coach.mdsamples/bmad-agent-dream-weaver/pattern-discovery.mdsamples/bmad-agent-dream-weaver/recall-training.mdsamples/bmad-agent-dream-weaver/references/memory-system.mdsrc/skills/bmad-agent-builder/assets/SKILL-template.mdsrc/skills/bmad-agent-builder/assets/autonomous-wake.mdsrc/skills/bmad-agent-builder/assets/init-template.mdsrc/skills/bmad-agent-builder/assets/memory-system.mdsrc/skills/bmad-agent-builder/build-process.mdsrc/skills/bmad-agent-builder/quality-scan-execution-efficiency.mdsrc/skills/bmad-agent-builder/references/standard-fields.mdsrc/skills/bmad-agent-builder/scripts/prepass-structure-capabilities.pysrc/skills/bmad-agent-builder/scripts/scan-path-standards.pysrc/skills/bmad-workflow-builder/references/standard-fields.md
✅ Files skipped from review due to trivial changes (7)
- src/skills/bmad-agent-builder/quality-scan-execution-efficiency.md
- samples/bmad-agent-dream-weaver/init.md
- src/skills/bmad-agent-builder/assets/memory-system.md
- samples/bmad-agent-dream-weaver/recall-training.md
- samples/bmad-agent-dream-weaver/pattern-discovery.md
- src/skills/bmad-agent-builder/references/standard-fields.md
- samples/bmad-agent-dream-weaver/headless-wake.md
🚧 Files skipped from review as they are similar to previous changes (10)
- src/skills/bmad-agent-builder/assets/init-template.md
- samples/bmad-agent-dream-weaver/dream-seed.md
- src/skills/bmad-agent-builder/assets/autonomous-wake.md
- samples/bmad-agent-dream-weaver/lucid-coach.md
- src/skills/bmad-workflow-builder/references/standard-fields.md
- src/skills/bmad-agent-builder/scripts/scan-path-standards.py
- src/skills/bmad-agent-builder/build-process.md
- docs/explanation/what-are-bmad-agents.md
- samples/bmad-agent-dream-weaver/references/memory-system.md
- src/skills/bmad-agent-builder/assets/SKILL-template.md
Comment on lines
+81
to
+87
| 1. [DC] - Capture and log a dream → dream-capture | ||
| 2. [DI] - Interpret a dream's symbols and themes → dream-interpret | ||
| 3. [RT] - Recall training exercises → recall-training | ||
| 4. [LC] - Lucid dreaming coaching → lucid-coaching | ||
| 5. [DS] - Plant dream seeds for tonight → dream-seed | ||
| 6. [PA] - Pattern analysis across dreams → pattern-analysis | ||
| 7. [SM] - Save memory → save-memory |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
skill_dir="samples/bmad-agent-dream-weaver"
skill_file="$skill_dir/SKILL.md"
python - <<'PY'
import re
from pathlib import Path
skill_file = Path("samples/bmad-agent-dream-weaver/SKILL.md")
base = skill_file.parent
text = skill_file.read_text(encoding="utf-8")
ids = re.findall(r'^\s*\d+\.\s+\[[A-Z]+\]\s+-\s+.*→\s+([a-z0-9-]+)\s*$', text, flags=re.MULTILINE)
print("Capability IDs found:", ids)
missing = []
for cid in ids:
p = base / f"{cid}.md"
ok = p.exists()
print(f"{cid:20} -> {p} :: {'OK' if ok else 'MISSING'}")
if not ok:
missing.append(cid)
if missing:
print("\nMissing prompt files:", ", ".join(missing))
raise SystemExit(1)
print("\nAll capability IDs map to existing prompt files.")
PYRepository: bmad-code-org/bmad-builder
Length of output: 856
🏁 Script executed:
#!/bin/bash
# Check lines 97-99 in SKILL.md for capability IDs
sed -n '97,99p' samples/bmad-agent-dream-weaver/SKILL.md | cat -nRepository: bmad-code-org/bmad-builder
Length of output: 324
Add missing capability prompt files for dream-capture, lucid-coaching, and pattern-analysis.
The following capability IDs in lines 81-87 reference non-existent prompt files and will fail at selection time:
dream-capture.md— missinglucid-coaching.md— missingpattern-analysis.md— missing
Create these three markdown files in samples/bmad-agent-dream-weaver/ to match the capability IDs declared in SKILL.md.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@samples/bmad-agent-dream-weaver/SKILL.md` around lines 81 - 87, SKILL.md
lists capability IDs dream-capture, lucid-coaching, and pattern-analysis but the
corresponding prompt files are missing; create three new markdown files named
dream-capture.md, lucid-coaching.md, and pattern-analysis.md in the
samples/bmad-agent-dream-weaver/ directory, each containing the capability
prompt for the matching ID (title/header matching the ID, a short description of
the capability, input/output expectations, and example usage) so the skill
selector can load them when referenced by SKILL.md.
Rewrite both agent and workflow SKILL-templates to be minimal skeletons rather than prescriptive blueprints. Templates now cover frontmatter, overview, and activation with config loading — the builder crafts everything else based on discovery phases. Key changes: - Config loading uses module/standalone pattern with sensible defaults and setup skill awareness (matching the builder's own activation) - Standardize on "headless" terminology, remove "autonomous" mixing - Headless reduced to one sentence instead of 25-line specification - Remove rigid workflow steps/stages/menus from templates - Update substitution rules, classification reference, and complex workflow patterns to match
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tools/validate-doc-links.cjs (2)
311-323: Consider usingfullMatchfor more robust link replacement.The
applyFixesfunction reconstructs the link string fromlinkTextandhref, butfullMatch(already captured at line 269) would be more reliable if the original link has any formatting variance captured by the regex.♻️ Proposed fix
function applyFixes(content, issues) { let updated = content; for (const issue of issues) { if (issue.status === 'auto-fixable' && issue.suggestedFix) { - const oldLink = `[${issue.linkText}](${issue.href})`; + const oldLink = issue.fullMatch; const newLink = `[${issue.linkText}](${issue.suggestedFix})`; updated = updated.replace(oldLink, newLink); } } return updated; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/validate-doc-links.cjs` around lines 311 - 323, The applyFixes function should use the regex-captured original match (issue.fullMatch) when swapping links instead of reconstructing the link from linkText and href; update applyFixes to build newLink as `[${issue.linkText}](${issue.suggestedFix})` and replace using issue.fullMatch if present (falling back to the constructed oldLink) so formatting captured by the regex is preserved (refer to applyFixes and the issue.fullMatch value captured earlier).
180-211: CachegetMarkdownFilesresult to avoid repeated directory traversals.
getMarkdownFiles(DOCS_ROOT)is called insidefindFileWithContext, which runs for every broken link. For repositories with many broken links, this causes redundant directory walks.♻️ Proposed fix: memoize file list
+let cachedMarkdownFiles = null; + +/** + * Get all markdown files (cached after first call) + */ +function getAllMarkdownFiles() { + if (!cachedMarkdownFiles) { + cachedMarkdownFiles = getMarkdownFiles(DOCS_ROOT); + } + return cachedMarkdownFiles; +} + /** * Search for a file with directory context */ function findFileWithContext(brokenPath) { // Extract filename and parent directory from the broken path // e.g., /tutorials/getting-started/foo/ -> parent: getting-started, file: foo.md const cleanPath = brokenPath.replace(/\/$/, '').replace(/^(\.\.\/|\.\/|\/)+/, ''); const parts = cleanPath.split('/'); const fileName = parts.at(-1) + '.md'; const parentDir = parts.length > 1 ? parts.at(-2) : null; - const allFiles = getMarkdownFiles(DOCS_ROOT); + const allFiles = getAllMarkdownFiles(); const matches = [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/validate-doc-links.cjs` around lines 180 - 211, The function findFileWithContext repeatedly calls getMarkdownFiles(DOCS_ROOT) for every broken link causing redundant directory traversals; cache or memoize that result once (e.g., compute allFiles = getMarkdownFiles(DOCS_ROOT) outside the loop or add a simple module-level/closure cache used by findFileWithContext) and then iterate over the cached allFiles instead of calling getMarkdownFiles each time; reference getMarkdownFiles, DOCS_ROOT, and findFileWithContext when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/quality.yaml:
- Around line 3-6: CI workflow removed the mandatory eslint and schema
validation jobs, so restore the missing jobs and steps named eslint and validate
(or their equivalent job IDs) in .github/workflows/quality.yaml to re-enable
ESLint checks and agent schema validation (validate:schemas, test:schemas,
validate:refs) in CI; ensure these jobs invoke the same npm scripts used locally
(e.g., npm run test, npm run test:schemas) and are configured to run on PRs so
CI gates merges when linting or YAML schema validation fail.
In `@src/skills/bmad-agent-builder/assets/SKILL-template.md`:
- Line 29: Update the sidecar memory path in SKILL-template.md to include the
project root prefix so it matches build-process.md: change the string "Memory
location: `_bmad/memory/{skillName}-sidecar/`" to use
"{project-root}/_bmad/memory/{skillName}-sidecar/" (ensure the exact token
`{project-root}` is prepended to the existing path in the SKILL-template.md
content so it aligns with the documented convention).
---
Nitpick comments:
In `@tools/validate-doc-links.cjs`:
- Around line 311-323: The applyFixes function should use the regex-captured
original match (issue.fullMatch) when swapping links instead of reconstructing
the link from linkText and href; update applyFixes to build newLink as
`[${issue.linkText}](${issue.suggestedFix})` and replace using issue.fullMatch
if present (falling back to the constructed oldLink) so formatting captured by
the regex is preserved (refer to applyFixes and the issue.fullMatch value
captured earlier).
- Around line 180-211: The function findFileWithContext repeatedly calls
getMarkdownFiles(DOCS_ROOT) for every broken link causing redundant directory
traversals; cache or memoize that result once (e.g., compute allFiles =
getMarkdownFiles(DOCS_ROOT) outside the loop or add a simple
module-level/closure cache used by findFileWithContext) and then iterate over
the cached allFiles instead of calling getMarkdownFiles each time; reference
getMarkdownFiles, DOCS_ROOT, and findFileWithContext when making this change.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 958a9f47-8600-4a9a-99a8-ea7bf02e7d02
📒 Files selected for processing (12)
.github/workflows/publish.yaml.github/workflows/quality.yaml.markdownlint-cli2.yamlpackage.jsonsrc/skills/bmad-agent-builder/assets/SKILL-template.mdsrc/skills/bmad-agent-builder/references/template-substitution-rules.mdsrc/skills/bmad-builder-setup/assets/module.yamlsrc/skills/bmad-workflow-builder/assets/SKILL-template.mdsrc/skills/bmad-workflow-builder/references/classification-reference.mdsrc/skills/bmad-workflow-builder/references/complex-workflow-patterns.mdsrc/skills/bmad-workflow-builder/references/template-substitution-rules.mdtools/validate-doc-links.cjs
✅ Files skipped from review due to trivial changes (3)
- .markdownlint-cli2.yaml
- .github/workflows/publish.yaml
- src/skills/bmad-builder-setup/assets/module.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- src/skills/bmad-workflow-builder/references/classification-reference.md
- src/skills/bmad-agent-builder/references/template-substitution-rules.md
- src/skills/bmad-workflow-builder/references/complex-workflow-patterns.md
- src/skills/bmad-workflow-builder/references/template-substitution-rules.md
Comment on lines
+3
to
+6
| # Runs quality checks on all PRs: | ||
| # - Prettier (formatting) | ||
| # - ESLint (linting) | ||
| # - markdownlint (markdown quality) | ||
| # - Schema validation (YAML structure) | ||
| # - Agent schema tests (fixture-based validation) | ||
| # - Installation component tests (compilation) | ||
| # - Bundle validation (web bundle integrity) | ||
| # - Documentation build |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the test script still includes the removed CI checks
grep -E '"test":|validate|lint' package.json | head -10Repository: bmad-code-org/bmad-builder
Length of output: 674
Remove eslint and schema validation jobs from CI, violating mandatory enforcement requirements.
The eslint and validate jobs were removed from the CI workflow, eliminating CI-level enforcement of:
- ESLint checks on JavaScript/YAML files
- Agent schema validation (
validate:schemas,test:schemas) - Cross-file reference validation (
validate:refs)
While npm run test still includes these checks locally, CI no longer gates PRs on them. This violates documented requirements: "ESLint and Prettier must pass on all JavaScript/Node.js build tools" and "All agent YAML files must be validated against the defined schema using npm run test:schemas before committing." Checks should remain enforced in CI to prevent regressions if developers skip local verification.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/quality.yaml around lines 3 - 6, CI workflow removed the
mandatory eslint and schema validation jobs, so restore the missing jobs and
steps named eslint and validate (or their equivalent job IDs) in
.github/workflows/quality.yaml to re-enable ESLint checks and agent schema
validation (validate:schemas, test:schemas, validate:refs) in CI; ensure these
jobs invoke the same npm scripts used locally (e.g., npm run test, npm run
test:schemas) and are configured to run on PRs so CI gates merges when linting
or YAML schema validation fail.
Frontmatter:
- Only name and description allowed in YAML frontmatter
- Removed argument-hint and all non-frontmatter fields (displayName, skillName, etc.)
- Split standard-fields.md into Frontmatter Fields vs Content Fields sections
- Lint script now validates frontmatter keys and flags invalid ones
Skill-internal paths:
- All skill-internal references now require ./ prefix (./references/, ./scripts/, ./assets/)
- Distinguishes skill-internal files from {project-root} paths to prevent LLM confusion
- Lint script updated: ./ is now required (was previously flagged as violation)
- Added bare-internal-path check for references/, scripts/, assets/ without ./
Progressive disclosure structure:
- All prompt files must live in ./references/, not at skill root
- SKILL.md is the only .md file at root — it serves as identity and router
- Capability sections replaced with succinct routing tables
- Removed CLI Usage section — user-facing help belongs in Overview
- Lint script now flags .md files at skill root (except SKILL.md)
Config resolution restored:
- Both builder SKILL.md files now list config keys with actual defaults
- user_name, communication_language, document_output_language, output paths
- Fixed anti-pattern in skill-best-practices: distinguishes mechanic (bad) from outcome-intent with defaults (good)
Activation flow:
- Single activation flow regardless of interactive/headless mode
- Sidecar index.md is the single entry point to memory system
- Interactive activation offers capability menu or continues from memory context
- SKILL template updated with config resolution, routing table, and memory patterns
Lint gate auto-fix:
- Build process now delegates lint to subagent if available
- Fix-and-rerun loop with max 3 attempts per script
- Lint scripts bumped to v2.0.0 with frontmatter, structure, and path checks
Also includes quality scanner and asset template refinements across both builders.
This was referenced Mar 27, 2026
3 tasks
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.
Summary
bmad-builder-setupskill — installs/configures the BMad Builder module into a project. Collects user preferences, writesconfig.yaml(shared project config) andconfig.user.yaml(gitignored personal settings), registers capabilities inmodule-help.csv. Includes anti-zombie pattern, legacy config migration, and Python scripts with full test coverage.user_nameandcommunication_languageare written exclusively toconfig.user.yaml, never toconfig.yaml. Existing user keys inconfig.yamlare actively stripped during merge._bmad/config.yamlandconfig.user.yamlwith sensible fallback defaults when config is missing.bmad-manifest.jsonfiles, manifest schemas, andmanifest.pyscripts from both builder skills. Updated docs and references accordingly.Test plan
merge-config.py— 33 unit tests passing (fresh install, update, anti-zombie, legacy migration, user key separation)merge-help-csv.py— test suite passingbmad-builder-setupon a clean project, verifyconfig.yamlhas no user-only keysSummary by CodeRabbit
New Features
Removed Features
Documentation
Chores