feat(skills): TOML-based skill customization system#2262
Closed
feat(skills): TOML-based skill customization system#2262
Conversation
Replace YAML customization with a three-layer TOML override model (skill defaults → team overrides → personal preferences). Each skill now ships a customize.toml with its full customization surface and a resolve-customization.py script that merges layers just-in-time during activation. Personal .user.toml files are gitignored; team .toml files are committed. Updates all 7 agent skills and product-brief workflow with new activation steps, customization defaults, and merge scripts. Rewrites the customize-bmad how-to guide for the new system.
🤖 Augment PR SummarySummary: This PR introduces a TOML-based, three-layer customization system for BMAD skills and workflows, replacing the previous YAML-based approach. Changes:
Technical Notes: The resolver outputs merged JSON and supports JIT field extraction via repeatable 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
|
|
||
| ## Communication Style | ||
| If script unavailable, read these sections from the following files | ||
| (first found wins, most specific first): |
There was a problem hiding this comment.
The fallback guidance “first found wins” conflicts with the sparse-override/merge semantics described for TOML layers; reading only the most-specific file will drop inherited fields (e.g., overriding only persona.displayName would lose default title/icon). This seems likely to break customization behavior specifically in environments where the resolver script can’t be run. (Guideline: skill_validation)
Other locations where this applies: src/bmm-skills/1-analysis/bmad-agent-tech-writer/SKILL.md:15, src/bmm-skills/2-plan-workflows/bmad-agent-pm/SKILL.md:15, src/bmm-skills/2-plan-workflows/bmad-agent-ux-designer/SKILL.md:15, src/bmm-skills/3-solutioning/bmad-agent-architect/SKILL.md:15, src/bmm-skills/4-implementation/bmad-agent-dev/SKILL.md:15, src/bmm-skills/1-analysis/bmad-product-brief/SKILL.md:25
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| } | ||
|
|
||
| // Skip scripts directory - contains build/customization scripts not needed at install time | ||
| if (file.startsWith('scripts/') || file === 'scripts') { |
There was a problem hiding this comment.
Skipping the scripts/ directory at install time appears to prevent ./scripts/resolve-customization.py (referenced throughout updated SKILL.md/docs) from being copied into installed skills, which would make the documented “Run: python ./scripts/resolve-customization.py …” step fail. This also seems to contradict the PR’s stated/test-planned expectation that scripts/ are copied for each skill during install.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| with open(path, "rb") as f: | ||
| return tomllib.load(f) | ||
| except Exception as exc: | ||
| print(f"warning: failed to parse {path}: {exc}", file=sys.stderr) |
There was a problem hiding this comment.
load_toml() treats TOML parse errors the same as “file doesn’t exist” (returns {}) and the script still exits 0, which can silently drop customizations and produce misleading “successful” output. This likely needs clearer failure signaling since it’s user-facing tooling. (Guideline: script_error_handling)
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
The file reference validator was flagging {project-root}/_bmad/customizations/
paths in SKILL.md files as broken. These are user-created override files that
only exist at install time, not in the source tree.
📝 WalkthroughWalkthroughThis PR introduces a TOML-based skill customization system. Skills now feature Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bmm-skills/1-analysis/bmad-product-brief/prompts/finalize.md (1)
45-45:⚠️ Potential issue | 🟠 MajorCompletion output still hardcodes paths instead of resolved config paths.
You resolve
config.outputFile/config.distillateFile, but Lines 71-72 and Line 81 still report fixed{planning_artifacts}paths, and Line 45 hardcodes distillatesource. This breaks correctness when custom output paths are used.Suggested fix
-source: "product-brief-{project_name}.md" +source: "{config.outputFile}" ... -**Executive Brief:** `{planning_artifacts}/product-brief-{project_name}.md` -[If distillate created:] **Detail Pack:** `{planning_artifacts}/product-brief-{project_name}-distillate.md` +**Executive Brief:** `{config.outputFile}` +[If distillate created:] **Detail Pack:** `{config.distillateFile}` ... - "brief": "{planning_artifacts}/product-brief-{project_name}.md", - "distillate": "{path or null}", + "brief": "{config.outputFile}", + "distillate": "{config.distillateFile or null}",Also applies to: 71-72, 81-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm-skills/1-analysis/bmad-product-brief/prompts/finalize.md` at line 45, The prompt hardcodes the distillate/source path and planning artifact placeholders ("source: \"product-brief-{project_name}.md\"" and "{planning_artifacts}") instead of using the resolved config values; update the prompt text to interpolate the resolved config variables (use config.outputFile and config.distillateFile) wherever the prompt currently emits "product-brief-{project_name}.md", "{planning_artifacts}" or other fixed paths so the generated instructions reflect custom output/distillate paths (replace every occurrence of those literals in this prompt template with the corresponding config.outputFile or config.distillateFile references).
🧹 Nitpick comments (5)
src/bmm-skills/_shared/scripts/resolve-customization.py (3)
24-24: Remove unusedosimport.🧹 Proposed fix
import argparse import json -import os import sys import tomllib🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm-skills/_shared/scripts/resolve-customization.py` at line 24, The file imports the unused module os in resolve-customization.py; remove the unused import statement (delete the line "import os") so the module only imports what it actually uses and avoids lint warnings in the top-level imports for that file.
1-182: This_sharedversion should be the single source of truth.Since this file exists under
_shared/scripts/, consider having each skill invoke this shared script rather than maintaining identical copies in every skill directory. This would centralize the merge logic and simplify future maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm-skills/_shared/scripts/resolve-customization.py` around lines 1 - 182, This file under src/bmm-skills/_shared/scripts/resolve-customization.py should be the single source of truth; remove/stop updating per-skill copies and make each skill invoke this shared script instead. Replace duplicated per-skill scripts with a small wrapper that execs or imports and calls the shared main() (or shells out to the shared script path), ensure the wrapper passes the same CLI arguments so argparse in main() works, and keep find_project_root(), load_toml(), deep_merge(), extract_key() intact in the shared module; update any skill-level CI/entrypoints to call the shared script location rather than duplicated files. Ensure the wrapper/resolver preserves behavior (defaults_path resolution relative to script_dir) or adjust main() so resolution works when invoked from other cwd contexts.
47-52: Consider narrowing the exception type.The current
except Exceptionis intentionally broad for graceful degradation, which is reasonable. However, you could narrow totomllib.TOMLDecodeErrorto catch only TOML parsing failures while letting unexpected errors (e.g., permission denied) propagate:🛡️ Optional refinement
- except Exception as exc: + except tomllib.TOMLDecodeError as exc: print(f"warning: failed to parse {path}: {exc}", file=sys.stderr) return {}This would distinguish "malformed TOML" (recoverable) from "I/O error" (potentially worth surfacing).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm-skills/_shared/scripts/resolve-customization.py` around lines 47 - 52, The broad except Exception should be narrowed so only TOML parsing errors are caught: replace the generic except in the block that opens and loads the file (the with open(path, "rb") as f: return tomllib.load(f) section) with an except tomllib.TOMLDecodeError as exc that prints the warning and returns {} so malformed TOML is handled gracefully while letting I/O errors (e.g., permission denied, missing file) propagate; ensure tomllib.TOMLDecodeError is referenced exactly.src/bmm-skills/2-plan-workflows/bmad-agent-ux-designer/scripts/resolve-customization.py (2)
24-24: Remove unusedosimport.The
osmodule is imported but never used in this script.🧹 Proposed fix
import argparse import json -import os import sys import tomllib🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm-skills/2-plan-workflows/bmad-agent-ux-designer/scripts/resolve-customization.py` at line 24, The file imports the unused module os at the top of resolve-customization.py; remove the unused import statement ("import os") from the module to eliminate the unused dependency and silence linters, ensuring no other code references os in functions or variables within resolve-customization.py (if any references exist, replace them with appropriate alternatives before removing).
1-182: Consider consolidating duplicated resolver scripts.This script is duplicated verbatim across multiple skill directories (ux-designer, tech-writer, product-brief, analyst, pm, architect) while an identical copy exists at
src/bmm-skills/_shared/scripts/resolve-customization.py. Each skill's SKILL.md could invoke the shared script instead:python {skill-dir}/../_shared/scripts/resolve-customization.py {skill-name} --key ...Alternatively, have the installer symlink or copy from
_shared/at install time. This would eliminate maintenance burden when the merge logic needs updates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm-skills/2-plan-workflows/bmad-agent-ux-designer/scripts/resolve-customization.py` around lines 1 - 182, This repository contains verbatim duplicate resolve-customization.py copies; consolidate by removing the per-skill copies and updating each skill to call the single shared script (src/bmm-skills/_shared/scripts/resolve-customization.py) or make the installer create a symlink/copy into the skill folder; update SKILL.md invocation examples to run the shared script (or document the symlink location) and ensure any references to functions like main(), deep_merge(), load_toml(), and extract_key() remain satisfied by the shared file so no behavior changes occur.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bmm-skills/1-analysis/bmad-agent-analyst/SKILL.md`:
- Around line 16-17: The two entries in SKILL.md (the literal backticked paths
`{project-root}/_bmad/customizations/bmad-agent-analyst.user.toml` and
`{project-root}/_bmad/customizations/bmad-agent-analyst.toml`) are being
interpreted as repository refs by the strict validator; change them to non-path
tokenized wording by splitting directory and filename (e.g., refer to the
_bmad/customizations directory and the bmad-agent-analyst.user.toml and
bmad-agent-analyst.toml files separately) so the text remains instructional but
does not look like a repo reference; update the lines in SKILL.md accordingly
(look for the exact strings shown) and keep the rest of the document unchanged.
In `@src/bmm-skills/1-analysis/bmad-agent-tech-writer/SKILL.md`:
- Around line 16-17: The two bullet lines listing
`{project-root}/_bmad/customizations/bmad-agent-tech-writer.user.toml` and
`{project-root}/_bmad/customizations/bmad-agent-tech-writer.toml` are being
interpreted as repo file refs by validate-file-refs; change them into plain
instructional text that mentions the directory and filenames without forming
resolvable paths—for example: "Look for files named
bmad-agent-tech-writer.user.toml and bmad-agent-tech-writer.toml in the
_bmad/customizations directory under your project root (if present)." Update
SKILL.md to replace the two path lines with that non-resolvable descriptive
wording.
In `@src/bmm-skills/1-analysis/bmad-product-brief/prompts/draft-and-review.md`:
- Around line 46-54: Update the review orchestration and docs so disabled lenses
are consistently honored: where the prompt references “all three review passes”
or requires being “reviewed by all three lenses” (the "Graceful Degradation" and
"Stage Complete" rules), change the logic to compute the active lenses from the
resolved review settings (review.skepticReview, review.opportunityReview,
review.contextualReview, and review.contextualReviewLens) and require completion
only for those active lenses; update the fallback behavior so contextualReview
selection only runs when review.contextualReview is true and
review.contextualReviewLens is empty (pick the most useful third lens), and
adjust text to document the TOML review customization behavior and the new
operational rule that only enabled lenses are required for stage completion.
In `@src/bmm-skills/1-analysis/bmad-product-brief/SKILL.md`:
- Around line 26-27: The two path-like references in SKILL.md (the lines listing
`{project-root}/_bmad/customizations/bmad-product-brief.user.toml` and
`{project-root}/_bmad/customizations/bmad-product-brief.toml`) are being parsed
as repo file refs; rewrite them to avoid slash-based paths while preserving
guidance—e.g., describe them as "a file named bmad-product-brief.user.toml
inside the _bmad/customizations directory (if present)" and "a file named
bmad-product-brief.toml inside the _bmad/customizations directory (if present)";
update the lines in SKILL.md so they refer to filenames and locations in plain
prose rather than path-like strings.
In
`@src/bmm-skills/2-plan-workflows/bmad-agent-pm/scripts/resolve-customization.py`:
- Around line 92-95: The merge logic treats an explicit empty list override as
an atomic replace, which clears capabilities; update the branch that calls
_is_menu_array and merge_menu so empty lists intended as menu overrides are
handled by merge-by-code: change the condition to also recognize empty list menu
overrides (e.g. treat (key == "menu" and isinstance(over_val, list) and
isinstance(base_val, list)) as a menu-array case) and call merge_menu(base_val,
over_val) instead of assigning over_val directly; reference the existing symbols
_is_menu_array, merge_menu, merged, over_val, and base_val when making this
change.
- Around line 50-52: The except block in load_toml is too broad; narrow it to
only handle parse/file errors (e.g., toml decoding and I/O errors) instead of
catching all Exceptions. Update the except clause in load_toml to catch specific
exceptions such as toml.TomlDecodeError and OSError (or
FileNotFoundError/PermissionError) and keep the existing print to stderr and
return {} behavior for those cases, so programming errors still surface
normally.
In `@src/bmm-skills/2-plan-workflows/bmad-agent-pm/SKILL.md`:
- Around line 16-17: The two example paths
"`{project-root}/_bmad/customizations/bmad-agent-pm.user.toml`" and
"`{project-root}/_bmad/customizations/bmad-agent-pm.toml`" are being picked up
as in-repo refs by strict validation; update those strings in SKILL.md to avoid
curly-brace refs (for example replace `{project-root}` with `<project-root>` or
escape the braces) so the lines become
"`<project-root>/_bmad/customizations/bmad-agent-pm.user.toml`" and
"`<project-root>/_bmad/customizations/bmad-agent-pm.toml`", preserving the
inline code formatting but removing the curly-brace reference pattern.
In `@src/bmm-skills/3-solutioning/bmad-agent-architect/SKILL.md`:
- Around line 16-17: Rewrite the two fallback path lines in SKILL.md so they no
longer render as repo links but preserve the exact runtime fallback semantics;
specifically change the literal backticked paths
`{project-root}/_bmad/customizations/bmad-agent-architect.user.toml` and
`{project-root}/_bmad/customizations/bmad-agent-architect.toml` to a non-parsing
form (for example: plain text with surrounding parentheses, prefixed with the
word "literal", or replace braces with placeholders like
PROJECT_ROOT/_bmad/customizations/...) so `validate-file-refs (STRICT)` stops
flagging them while the meaning — lookup of those two files in that fallback
order — remains clear. Ensure the replacements appear where the current lines
reference those two exact filenames (the lines containing the user.toml and
bmad-agent-architect.toml entries) and keep the numbered list and ordering
unchanged.
In
`@src/bmm-skills/4-implementation/bmad-agent-dev/scripts/resolve-customization.py`:
- Around line 92-95: The current branch skips merge_menu when the override menu
is an explicit empty list because _is_menu_array(over_val) returns false for
empty lists; update the conditional so empty override menus are treated as menu
arrays and still call merge_menu. Concretely, replace the condition using
_is_menu_array(over_val) and _is_menu_array(base_val) with one that detects the
'menu' key and treats an empty list as a menu (e.g., key == "menu" and
(_is_menu_array(base_val) or isinstance(over_val, list))), then call
merge_menu(base_val, over_val) so merge_menu handles replacing/adding codes
rather than doing an atomic replacement via merged[key] = over_val.
- Around line 129-131: The CLI help for the 'skill_name' argparse parameter
references PM/product-brief examples which are misleading for the dev resolver;
update the help text in the add_argument call that defines "skill_name" (the
argparse add_argument for 'skill_name') to mention the dev resolver context and
provide appropriate examples such as dev-specific identifiers (e.g.
"bmad-agent-dev", "bmad-dev-skill") or a short note like "use dev resolver skill
id" so users see accurate examples for this script.
- Around line 50-52: The broad "except Exception as exc" in the TOML
parsing/reading block should be narrowed to only expected I/O and parse errors
so programming errors propagate; replace that broad handler with specific
exceptions such as FileNotFoundError and OSError plus the TOML library's decode
error (e.g., toml.TomlDecodeError or tomllib.TOMLDecodeError depending on which
parser is imported) and ensure the appropriate decoder type is imported/checked;
keep the same warning message and return {} for those cases and let all other
exceptions bubble up.
In `@src/bmm-skills/4-implementation/bmad-agent-dev/SKILL.md`:
- Around line 16-17: The two example file paths in SKILL.md are being
interpreted as repo file refs; update the lines that currently read
`{project-root}/_bmad/customizations/bmad-agent-dev.user.toml` and
`{project-root}/_bmad/customizations/bmad-agent-dev.toml` to a non-ref format
such as replacing the braces with a placeholder token or angle brackets (e.g.
<project-root>/_bmad/customizations/bmad-agent-dev.user.toml and
<project-root>/_bmad/customizations/bmad-agent-dev.toml) or a plain-text form
like project-root/_bmad/customizations/... so the validate-file-refs --strict
checker no longer treats them as repository file references.
In `@tools/installer/modules/official-modules.js`:
- Around line 457-460: The current filter only skips top-level "scripts/" but
misses nested script paths returned by getFileList(); update the check inside
the loop that iterates over file (from getFileList()) to skip any path that
contains a scripts segment (e.g., file.startsWith('scripts/') ||
file.includes('/scripts/') ), and remove or ignore the unnecessary file ===
'scripts' branch; modify the condition in the loop body (the one referencing
file.startsWith('scripts/')) so nested "*/scripts/*" entries are also excluded.
---
Outside diff comments:
In `@src/bmm-skills/1-analysis/bmad-product-brief/prompts/finalize.md`:
- Line 45: The prompt hardcodes the distillate/source path and planning artifact
placeholders ("source: \"product-brief-{project_name}.md\"" and
"{planning_artifacts}") instead of using the resolved config values; update the
prompt text to interpolate the resolved config variables (use config.outputFile
and config.distillateFile) wherever the prompt currently emits
"product-brief-{project_name}.md", "{planning_artifacts}" or other fixed paths
so the generated instructions reflect custom output/distillate paths (replace
every occurrence of those literals in this prompt template with the
corresponding config.outputFile or config.distillateFile references).
---
Nitpick comments:
In `@src/bmm-skills/_shared/scripts/resolve-customization.py`:
- Line 24: The file imports the unused module os in resolve-customization.py;
remove the unused import statement (delete the line "import os") so the module
only imports what it actually uses and avoids lint warnings in the top-level
imports for that file.
- Around line 1-182: This file under
src/bmm-skills/_shared/scripts/resolve-customization.py should be the single
source of truth; remove/stop updating per-skill copies and make each skill
invoke this shared script instead. Replace duplicated per-skill scripts with a
small wrapper that execs or imports and calls the shared main() (or shells out
to the shared script path), ensure the wrapper passes the same CLI arguments so
argparse in main() works, and keep find_project_root(), load_toml(),
deep_merge(), extract_key() intact in the shared module; update any skill-level
CI/entrypoints to call the shared script location rather than duplicated files.
Ensure the wrapper/resolver preserves behavior (defaults_path resolution
relative to script_dir) or adjust main() so resolution works when invoked from
other cwd contexts.
- Around line 47-52: The broad except Exception should be narrowed so only TOML
parsing errors are caught: replace the generic except in the block that opens
and loads the file (the with open(path, "rb") as f: return tomllib.load(f)
section) with an except tomllib.TOMLDecodeError as exc that prints the warning
and returns {} so malformed TOML is handled gracefully while letting I/O errors
(e.g., permission denied, missing file) propagate; ensure
tomllib.TOMLDecodeError is referenced exactly.
In
`@src/bmm-skills/2-plan-workflows/bmad-agent-ux-designer/scripts/resolve-customization.py`:
- Line 24: The file imports the unused module os at the top of
resolve-customization.py; remove the unused import statement ("import os") from
the module to eliminate the unused dependency and silence linters, ensuring no
other code references os in functions or variables within
resolve-customization.py (if any references exist, replace them with appropriate
alternatives before removing).
- Around line 1-182: This repository contains verbatim duplicate
resolve-customization.py copies; consolidate by removing the per-skill copies
and updating each skill to call the single shared script
(src/bmm-skills/_shared/scripts/resolve-customization.py) or make the installer
create a symlink/copy into the skill folder; update SKILL.md invocation examples
to run the shared script (or document the symlink location) and ensure any
references to functions like main(), deep_merge(), load_toml(), and
extract_key() remain satisfied by the shared file so no behavior changes occur.
🪄 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: c0a6676c-2dcf-4f3f-8272-8bb002ed3412
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!package-lock.json
📒 Files selected for processing (27)
.gitignoredocs/how-to/customize-bmad.mdsrc/bmm-skills/1-analysis/bmad-agent-analyst/SKILL.mdsrc/bmm-skills/1-analysis/bmad-agent-analyst/customize.tomlsrc/bmm-skills/1-analysis/bmad-agent-analyst/scripts/resolve-customization.pysrc/bmm-skills/1-analysis/bmad-agent-tech-writer/SKILL.mdsrc/bmm-skills/1-analysis/bmad-agent-tech-writer/customize.tomlsrc/bmm-skills/1-analysis/bmad-agent-tech-writer/scripts/resolve-customization.pysrc/bmm-skills/1-analysis/bmad-product-brief/SKILL.mdsrc/bmm-skills/1-analysis/bmad-product-brief/customize.tomlsrc/bmm-skills/1-analysis/bmad-product-brief/prompts/draft-and-review.mdsrc/bmm-skills/1-analysis/bmad-product-brief/prompts/finalize.mdsrc/bmm-skills/1-analysis/bmad-product-brief/scripts/resolve-customization.pysrc/bmm-skills/2-plan-workflows/bmad-agent-pm/SKILL.mdsrc/bmm-skills/2-plan-workflows/bmad-agent-pm/customize.tomlsrc/bmm-skills/2-plan-workflows/bmad-agent-pm/scripts/resolve-customization.pysrc/bmm-skills/2-plan-workflows/bmad-agent-ux-designer/SKILL.mdsrc/bmm-skills/2-plan-workflows/bmad-agent-ux-designer/customize.tomlsrc/bmm-skills/2-plan-workflows/bmad-agent-ux-designer/scripts/resolve-customization.pysrc/bmm-skills/3-solutioning/bmad-agent-architect/SKILL.mdsrc/bmm-skills/3-solutioning/bmad-agent-architect/customize.tomlsrc/bmm-skills/3-solutioning/bmad-agent-architect/scripts/resolve-customization.pysrc/bmm-skills/4-implementation/bmad-agent-dev/SKILL.mdsrc/bmm-skills/4-implementation/bmad-agent-dev/customize.tomlsrc/bmm-skills/4-implementation/bmad-agent-dev/scripts/resolve-customization.pysrc/bmm-skills/_shared/scripts/resolve-customization.pytools/installer/modules/official-modules.js
Comment on lines
+16
to
+17
| 1. `{project-root}/_bmad/customizations/bmad-agent-analyst.user.toml` (if exists) | ||
| 2. `{project-root}/_bmad/customizations/bmad-agent-analyst.toml` (if exists) |
There was a problem hiding this comment.
Strict ref validation fails on these customization-file entries.
Line 16 and Line 17 are parsed as broken repository references by the strict validator, causing CI failure. Please switch to non-path tokenized wording (directory + filename) to preserve instructions without triggering ref checks.
As per coding guidelines, source-file changes that alter configuration behavior must remain valid and releasable; current strict validation failures block that.
🧰 Tools
🪛 GitHub Actions: Quality & Validation
[error] 16-16: validate-file-refs (STRICT) found a broken reference: customizations/bmad-agent-analyst.user.toml → src/customizations/bmad-agent-analyst.user.toml. Target not found.
[error] 17-17: validate-file-refs (STRICT) found a broken reference: customizations/bmad-agent-analyst.toml → src/customizations/bmad-agent-analyst.toml. Target not found.
🪛 GitHub Check: validate
[warning] 17-17:
Broken reference: customizations/bmad-agent-analyst.toml → src/customizations/bmad-agent-analyst.toml
[warning] 16-16:
Broken reference: customizations/bmad-agent-analyst.user.toml → src/customizations/bmad-agent-analyst.user.toml
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bmm-skills/1-analysis/bmad-agent-analyst/SKILL.md` around lines 16 - 17,
The two entries in SKILL.md (the literal backticked paths
`{project-root}/_bmad/customizations/bmad-agent-analyst.user.toml` and
`{project-root}/_bmad/customizations/bmad-agent-analyst.toml`) are being
interpreted as repository refs by the strict validator; change them to non-path
tokenized wording by splitting directory and filename (e.g., refer to the
_bmad/customizations directory and the bmad-agent-analyst.user.toml and
bmad-agent-analyst.toml files separately) so the text remains instructional but
does not look like a repo reference; update the lines in SKILL.md accordingly
(look for the exact strings shown) and keep the rest of the document unchanged.
Comment on lines
+16
to
+17
| 1. `{project-root}/_bmad/customizations/bmad-agent-tech-writer.user.toml` (if exists) | ||
| 2. `{project-root}/_bmad/customizations/bmad-agent-tech-writer.toml` (if exists) |
There was a problem hiding this comment.
Broken reference paths are failing CI here as well.
Line 16 and Line 17 are flagged by strict validate-file-refs. These examples need to be reformatted so they are instructional text, not resolvable repo refs.
🧰 Tools
🪛 GitHub Actions: Quality & Validation
[error] 16-16: validate-file-refs (STRICT) found a broken reference: customizations/bmad-agent-tech-writer.user.toml → src/customizations/bmad-agent-tech-writer.user.toml. Target not found.
[error] 17-17: validate-file-refs (STRICT) found a broken reference: customizations/bmad-agent-tech-writer.toml → src/customizations/bmad-agent-tech-writer.toml. Target not found.
🪛 GitHub Check: validate
[warning] 17-17:
Broken reference: customizations/bmad-agent-tech-writer.toml → src/customizations/bmad-agent-tech-writer.toml
[warning] 16-16:
Broken reference: customizations/bmad-agent-tech-writer.user.toml → src/customizations/bmad-agent-tech-writer.user.toml
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bmm-skills/1-analysis/bmad-agent-tech-writer/SKILL.md` around lines 16 -
17, The two bullet lines listing
`{project-root}/_bmad/customizations/bmad-agent-tech-writer.user.toml` and
`{project-root}/_bmad/customizations/bmad-agent-tech-writer.toml` are being
interpreted as repo file refs by validate-file-refs; change them into plain
instructional text that mentions the directory and filenames without forming
resolvable paths—for example: "Look for files named
bmad-agent-tech-writer.user.toml and bmad-agent-tech-writer.toml in the
_bmad/customizations directory under your project root (if present)." Update
SKILL.md to replace the two path lines with that non-resolvable descriptive
wording.
Comment on lines
+46
to
+54
| Before showing the draft to the user, run it through multiple review lenses in parallel. Use the resolved `review` config to determine which lenses to run. | ||
|
|
||
| **Launch in parallel:** | ||
| **Launch in parallel** (skip any where the resolved config disables them): | ||
|
|
||
| 1. **Skeptic Reviewer** (`../agents/skeptic-reviewer.md`) — "What's missing? What assumptions are untested? What could go wrong? Where is the brief vague or hand-wavy?" | ||
| 1. **Skeptic Reviewer** (`../agents/skeptic-reviewer.md`) -- if `review.skepticReview` is true: "What's missing? What assumptions are untested? What could go wrong? Where is the brief vague or hand-wavy?" | ||
|
|
||
| 2. **Opportunity Reviewer** (`../agents/opportunity-reviewer.md`) — "What adjacent value propositions are being missed? What market angles or partnerships could strengthen this? What's underemphasized?" | ||
| 2. **Opportunity Reviewer** (`../agents/opportunity-reviewer.md`) -- if `review.opportunityReview` is true: "What adjacent value propositions are being missed? What market angles or partnerships could strengthen this? What's underemphasized?" | ||
|
|
||
| 3. **Contextual Reviewer** — You (the main agent) pick the most useful third lens based on THIS specific product. Choose the lens that addresses the SINGLE BIGGEST RISK that the skeptic and opportunity reviewers won't naturally catch. Examples: | ||
| 3. **Contextual Reviewer** -- if `review.contextualReview` is true: If `review.contextualReviewLens` is not empty, use that specific lens. Otherwise, you (the main agent) pick the most useful third lens based on THIS specific product. Choose the lens that addresses the SINGLE BIGGEST RISK that the skeptic and opportunity reviewers won't naturally catch. Examples: |
There was a problem hiding this comment.
Review-flag logic conflicts with fallback/completion rules.
You now allow disabling lenses via review.*, but downstream instructions still require “all three review passes” (Graceful Degradation) and “reviewed by all three lenses” (Stage Complete). That creates contradictory behavior when a lens is intentionally disabled.
As per coding guidelines: “Document the new TOML customization system … [review] settings …” including operational behavior consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bmm-skills/1-analysis/bmad-product-brief/prompts/draft-and-review.md`
around lines 46 - 54, Update the review orchestration and docs so disabled
lenses are consistently honored: where the prompt references “all three review
passes” or requires being “reviewed by all three lenses” (the "Graceful
Degradation" and "Stage Complete" rules), change the logic to compute the active
lenses from the resolved review settings (review.skepticReview,
review.opportunityReview, review.contextualReview, and
review.contextualReviewLens) and require completion only for those active
lenses; update the fallback behavior so contextualReview selection only runs
when review.contextualReview is true and review.contextualReviewLens is empty
(pick the most useful third lens), and adjust text to document the TOML review
customization behavior and the new operational rule that only enabled lenses are
required for stage completion.
Comment on lines
+26
to
+27
| 1. `{project-root}/_bmad/customizations/bmad-product-brief.user.toml` (if exists) | ||
| 2. `{project-root}/_bmad/customizations/bmad-product-brief.toml` (if exists) |
There was a problem hiding this comment.
CI-blocking broken file refs in activation fallback paths.
Lines 26-27 are currently parsed as repository file references by validate-file-refs (STRICT), and the job fails. Please rewrite these lines to avoid path-like repo refs while preserving runtime guidance.
Suggested wording adjustment
-1. `{project-root}/_bmad/customizations/bmad-product-brief.user.toml` (if exists)
-2. `{project-root}/_bmad/customizations/bmad-product-brief.toml` (if exists)
+1. User override file under `{project-root}/_bmad/customizations` named `bmad-product-brief.user.toml` (if exists)
+2. Team override file under `{project-root}/_bmad/customizations` named `bmad-product-brief.toml` (if exists)🧰 Tools
🪛 GitHub Actions: Quality & Validation
[error] 26-26: validate-file-refs (STRICT) found a broken reference: customizations/bmad-product-brief.user.toml → src/customizations/bmad-product-brief.user.toml. Target not found.
[error] 27-27: validate-file-refs (STRICT) found a broken reference: customizations/bmad-product-brief.toml → src/customizations/bmad-product-brief.toml. Target not found.
🪛 GitHub Check: validate
[warning] 27-27:
Broken reference: customizations/bmad-product-brief.toml → src/customizations/bmad-product-brief.toml
[warning] 26-26:
Broken reference: customizations/bmad-product-brief.user.toml → src/customizations/bmad-product-brief.user.toml
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bmm-skills/1-analysis/bmad-product-brief/SKILL.md` around lines 26 - 27,
The two path-like references in SKILL.md (the lines listing
`{project-root}/_bmad/customizations/bmad-product-brief.user.toml` and
`{project-root}/_bmad/customizations/bmad-product-brief.toml`) are being parsed
as repo file refs; rewrite them to avoid slash-based paths while preserving
guidance—e.g., describe them as "a file named bmad-product-brief.user.toml
inside the _bmad/customizations directory (if present)" and "a file named
bmad-product-brief.toml inside the _bmad/customizations directory (if present)";
update the lines in SKILL.md so they refer to filenames and locations in plain
prose rather than path-like strings.
Comment on lines
+50
to
+52
| except Exception as exc: | ||
| print(f"warning: failed to parse {path}: {exc}", file=sys.stderr) | ||
| return {} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify broad exception handlers in resolver scripts.
rg -n -C2 --type=py 'except Exception as exc:' src/bmm-skills/**/scripts/resolve-customization.pyRepository: bmad-code-org/BMAD-METHOD
Length of output: 275
Narrow the exception handler in load_toml().
Catching Exception here is too broad and can mask unexpected programming errors unrelated to TOML parsing or file I/O, making failures harder to diagnose.
💡 Proposed fix
- except Exception as exc:
+ except (tomllib.TOMLDecodeError, OSError) as exc:
print(f"warning: failed to parse {path}: {exc}", file=sys.stderr)
return {}📝 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
| except Exception as exc: | |
| print(f"warning: failed to parse {path}: {exc}", file=sys.stderr) | |
| return {} | |
| except (tomllib.TOMLDecodeError, OSError) as exc: | |
| print(f"warning: failed to parse {path}: {exc}", file=sys.stderr) | |
| return {} |
🧰 Tools
🪛 Ruff (0.15.9)
[warning] 50-50: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/bmm-skills/2-plan-workflows/bmad-agent-pm/scripts/resolve-customization.py`
around lines 50 - 52, The except block in load_toml is too broad; narrow it to
only handle parse/file errors (e.g., toml decoding and I/O errors) instead of
catching all Exceptions. Update the except clause in load_toml to catch specific
exceptions such as toml.TomlDecodeError and OSError (or
FileNotFoundError/PermissionError) and keep the existing print to stderr and
return {} behavior for those cases, so programming errors still surface
normally.
Comment on lines
+50
to
+52
| except Exception as exc: | ||
| print(f"warning: failed to parse {path}: {exc}", file=sys.stderr) | ||
| return {} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify broad exception handlers in resolver scripts.
rg -n -C2 --type=py 'except Exception as exc:' src/bmm-skills/**/scripts/resolve-customization.pyRepository: bmad-code-org/BMAD-METHOD
Length of output: 275
🏁 Script executed:
# Read the resolve-customization.py file to understand the full context
head -80 src/bmm-skills/4-implementation/bmad-agent-dev/scripts/resolve-customization.pyRepository: bmad-code-org/BMAD-METHOD
Length of output: 2653
🏁 Script executed:
# Check Python version requirements to confirm tomllib availability
# tomllib was added in Python 3.11; check if there's a fallback or version specification
grep -r "python_requires\|import toml" . --include="*.py" --include="*.toml" | head -20Repository: bmad-code-org/BMAD-METHOD
Length of output: 828
🏁 Script executed:
# Verify correct exception type name in tomllib module
python3 << 'EOF'
try:
import tomllib
# Check what exceptions tomllib defines
print("tomllib module found")
print("Has TOMLDecodeError:", hasattr(tomllib, 'TOMLDecodeError'))
print("Exception name:", 'tomllib.TOMLDecodeError' if hasattr(tomllib, 'TOMLDecodeError') else 'NOT FOUND')
except ImportError as e:
print(f"tomllib import failed: {e}")
# Try fallback
try:
import tomli as tomllib
print("Using tomli fallback")
print("Has TOMLDecodeError:", hasattr(tomllib, 'TOMLDecodeError'))
except ImportError:
print("No toml library found")
EOFRepository: bmad-code-org/BMAD-METHOD
Length of output: 155
🏁 Script executed:
# Check if all resolve-customization.py files have the same broad exception handler
for file in $(find src/bmm-skills -name "resolve-customization.py" -type f); do
echo "=== $file ==="
sed -n '45,55p' "$file"
doneRepository: bmad-code-org/BMAD-METHOD
Length of output: 3364
🏁 Script executed:
# Verify that OSError covers all file opening errors and tomllib.TOMLDecodeError covers parsing
python3 << 'EOF'
import tomllib
import os
# Check exception hierarchy
print("OSError covers file operations:")
print(" - FileNotFoundError is subclass of OSError:", issubclass(FileNotFoundError, OSError))
print(" - PermissionError is subclass of OSError:", issubclass(PermissionError, OSError))
print(" - IsADirectoryError is subclass of OSError:", issubclass(IsADirectoryError, OSError))
print("\nFocus: tomllib.load() signature and potential exceptions")
help(tomllib.load)
EOFRepository: bmad-code-org/BMAD-METHOD
Length of output: 494
Narrow exception handler to expected failures in TOML loading.
The broad except Exception handler silently suppresses unexpected bugs (ImportError, AttributeError, etc.) and masks programming errors. Only file and parse errors should be downgraded to warnings.
Proposed fix
- except Exception as exc:
+ except (tomllib.TOMLDecodeError, OSError) as exc:
print(f"warning: failed to parse {path}: {exc}", file=sys.stderr)
return {}📝 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
| except Exception as exc: | |
| print(f"warning: failed to parse {path}: {exc}", file=sys.stderr) | |
| return {} | |
| except (tomllib.TOMLDecodeError, OSError) as exc: | |
| print(f"warning: failed to parse {path}: {exc}", file=sys.stderr) | |
| return {} |
🧰 Tools
🪛 Ruff (0.15.9)
[warning] 50-50: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/bmm-skills/4-implementation/bmad-agent-dev/scripts/resolve-customization.py`
around lines 50 - 52, The broad "except Exception as exc" in the TOML
parsing/reading block should be narrowed to only expected I/O and parse errors
so programming errors propagate; replace that broad handler with specific
exceptions such as FileNotFoundError and OSError plus the TOML library's decode
error (e.g., toml.TomlDecodeError or tomllib.TOMLDecodeError depending on which
parser is imported) and ensure the appropriate decoder type is imported/checked;
keep the same warning message and return {} for those cases and let all other
exceptions bubble up.
Comment on lines
+92
to
+95
| elif _is_menu_array(over_val) and _is_menu_array(base_val): | ||
| merged[key] = merge_menu(base_val, over_val) | ||
| else: | ||
| merged[key] = over_val |
There was a problem hiding this comment.
Empty menu overrides bypass merge-by-code and replace the menu outright.
When menu is explicitly empty, this branch is skipped and the code falls back to atomic list replacement. That can unintentionally remove all capabilities.
💡 Proposed fix
- elif _is_menu_array(over_val) and _is_menu_array(base_val):
+ elif key == "menu" and isinstance(base_val, list) and isinstance(over_val, list):
merged[key] = merge_menu(base_val, over_val)📝 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
| elif _is_menu_array(over_val) and _is_menu_array(base_val): | |
| merged[key] = merge_menu(base_val, over_val) | |
| else: | |
| merged[key] = over_val | |
| elif key == "menu" and isinstance(base_val, list) and isinstance(over_val, list): | |
| merged[key] = merge_menu(base_val, over_val) | |
| else: | |
| merged[key] = over_val |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/bmm-skills/4-implementation/bmad-agent-dev/scripts/resolve-customization.py`
around lines 92 - 95, The current branch skips merge_menu when the override menu
is an explicit empty list because _is_menu_array(over_val) returns false for
empty lists; update the conditional so empty override menus are treated as menu
arrays and still call merge_menu. Concretely, replace the condition using
_is_menu_array(over_val) and _is_menu_array(base_val) with one that detects the
'menu' key and treats an empty list as a menu (e.g., key == "menu" and
(_is_menu_array(base_val) or isinstance(over_val, list))), then call
merge_menu(base_val, over_val) so merge_menu handles replacing/adding codes
rather than doing an atomic replacement via merged[key] = over_val.
Comment on lines
+129
to
+131
| "skill_name", | ||
| help="Skill identifier (e.g. bmad-agent-pm, bmad-product-brief)", | ||
| ) |
There was a problem hiding this comment.
Update CLI help examples to match the dev resolver context.
The examples mention PM/product-brief identifiers inside the dev agent resolver, which is misleading.
💡 Proposed fix
- help="Skill identifier (e.g. bmad-agent-pm, bmad-product-brief)",
+ help="Skill identifier (e.g. bmad-agent-dev)",📝 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
| "skill_name", | |
| help="Skill identifier (e.g. bmad-agent-pm, bmad-product-brief)", | |
| ) | |
| "skill_name", | |
| help="Skill identifier (e.g. bmad-agent-dev)", | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/bmm-skills/4-implementation/bmad-agent-dev/scripts/resolve-customization.py`
around lines 129 - 131, The CLI help for the 'skill_name' argparse parameter
references PM/product-brief examples which are misleading for the dev resolver;
update the help text in the add_argument call that defines "skill_name" (the
argparse add_argument for 'skill_name') to mention the dev resolver context and
provide appropriate examples such as dev-specific identifiers (e.g.
"bmad-agent-dev", "bmad-dev-skill") or a short note like "use dev resolver skill
id" so users see accurate examples for this script.
…kills Add customize.toml with stock fields (inject before/after, additional_resources) to all 34 remaining workflow and core skills. Copy resolve-customization.py script into every skill's scripts/ directory. Add customization resolve and inject points to all workflow SKILL.md files. Strip fallback blocks from all SKILL.md files since the script ships with every skill.
This was referenced Apr 14, 2026
- Narrow except Exception to (tomllib.TOMLDecodeError, OSError) in resolve-customization.py and all copies - Re-add scripts/ exclusion to _config-driven.js IDE installer path - Update draft-and-review.md to reference "enabled lenses" not "all three"
Remove the scripts/ exclusion from both installer paths so that resolve-customization.py is available at the install destination where SKILL.md references it.
- Drop ./ prefix from script paths (use scripts/ not ./scripts/) - Use python3 instead of python for explicitness - Add Available Scripts listing to all SKILL.md files
- Fix merge_menu KeyError crash when menu items missing 'code' key - Fix _is_menu_array to check ALL elements, not just first - Remove unused import os from resolve-customization.py - Remove inject.after from agent activation (agents have no completion point; inject.after only makes sense for workflows)
- Add type: ignore[arg-type] to merge_menu call (Pylance narrowing limitation) - Reword inject.before in workflows: "prepend to active instructions and follow it" - Reword inject.after in workflows: "append to active instructions and follow it" - Make additional_resources lazy: note list but don't eagerly load
Collaborator
Author
|
Closing - will resubmit as a smaller, focused PR |
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
customize.toml) → team overrides (_bmad/customizations/{name}.toml) → personal preferences (_bmad/customizations/{name}.user.toml, gitignored)resolve-customization.pyscript that merges layers just-in-time during activation, with sparse override semantics (only specify what you change) and merge-by-code for menu itemscustomize-bmadhow-to guide for the new systemTest plan
npx bmad-method installon a clean project and verifycustomize.tomlandscripts/are copied to.claude/skills/for each agent_bmad/customizations/bmad-agent-pm.tomlwith persona overrides, activate PM agent, verify merged persona_bmad/customizations/bmad-agent-pm.user.tomlwith personal overrides, verify three-layer merge priority.user.tomlfiles are gitignored🤖 Generated with Claude Code