feat(skills): add TOML-based skill customization system#20
feat(skills): add TOML-based skill customization system#20
Conversation
Add customize.toml to all 35 skills (7 agents with full persona + metadata, 28 workflows with stock fields). Include resolve-customization.py script in each skill's scripts/ directory. Add customization resolve and inject points to all workflow SKILL.md files.
WalkthroughThis pull request standardizes per-skill customization by adding default Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / CLI
participant Resolver as scripts/resolve-customization.py
participant FS as Project FS (defaults & _bmad/customizations/)
participant Agent as Agent/Workflow Activation
participant User as User
Dev->>Resolver: run resolver (skill_name, --key ...)
Resolver->>FS: read skill defaults (./customize.toml)
Resolver->>FS: if project root found, read team and user overrides
Resolver->>Resolver: deep_merge (dicts, menu-by-code, arrays override)
Resolver->>Dev: emit merged JSON (or selected keys)
Dev->>Agent: start activation with resolved JSON
Agent->>Agent: apply persona, inject.before, load additional_resources
Agent->>User: greet and present merged capabilities menu
User->>Agent: select capability (must match exact registered code)
Note: rectangles/colors are implicit in Mermaid actors; interactions focus on resolver → filesystem → agent flow. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
|
🤖 Augment PR SummarySummary: Introduces a TOML-based customization system for GDS skills (agents + workflows), enabling per-user and per-team overrides on top of skill defaults. Changes:
Technical Notes: Merge rules are recursive for tables, treat non-menu arrays as atomic replace, and support dotted key extraction via 🤖 Was this summary useful? React with 👍 or 👎 |
| user: dict[str, Any] = {} | ||
| if project_root is not None: | ||
| customizations_dir = project_root / "_bmad" / "customizations" | ||
| team = load_toml(customizations_dir / f"{args.skill_name}.toml") |
There was a problem hiding this comment.
defaults_path is derived from the script’s directory, but the override filenames are derived from user-provided skill_name; if the caller passes a mismatched name, this can silently merge defaults from one skill with overrides from another. Consider guarding against this to prevent hard-to-debug configuration mixups.
Severity: medium
Other Locations
src/agents/gds-agent-game-architect/scripts/resolve-customization.py:159src/agents/gds-agent-game-designer/scripts/resolve-customization.py:159src/agents/gds-agent-game-dev/scripts/resolve-customization.py:159src/agents/gds-agent-game-qa/scripts/resolve-customization.py:159src/agents/gds-agent-game-scrum-master/scripts/resolve-customization.py:159src/agents/gds-agent-game-solo-dev/scripts/resolve-customization.py:159src/agents/gds-agent-tech-writer/scripts/resolve-customization.py:159src/workflows/1-preproduction/gds-create-game-brief/scripts/resolve-customization.py:159src/workflows/1-preproduction/research/gds-domain-research/scripts/resolve-customization.py:159src/workflows/2-design/gds-create-gdd/scripts/resolve-customization.py:159src/workflows/2-design/gds-create-narrative/scripts/resolve-customization.py:159src/workflows/2-design/gds-create-ux-design/scripts/resolve-customization.py:159src/workflows/3-technical/gds-check-implementation-readiness/scripts/resolve-customization.py:159src/workflows/3-technical/gds-create-epics-and-stories/scripts/resolve-customization.py:159src/workflows/3-technical/gds-game-architecture/scripts/resolve-customization.py:159src/workflows/3-technical/gds-generate-project-context/scripts/resolve-customization.py:159src/workflows/4-production/gds-code-review/scripts/resolve-customization.py:159src/workflows/4-production/gds-correct-course/scripts/resolve-customization.py:159src/workflows/4-production/gds-create-story/scripts/resolve-customization.py:159src/workflows/4-production/gds-dev-story/scripts/resolve-customization.py:159src/workflows/4-production/gds-retrospective/scripts/resolve-customization.py:159src/workflows/4-production/gds-sprint-planning/scripts/resolve-customization.py:159src/workflows/4-production/gds-sprint-status/scripts/resolve-customization.py:159src/workflows/gametest/gds-e2e-scaffold/scripts/resolve-customization.py:159src/workflows/gametest/gds-performance-test/scripts/resolve-customization.py:159src/workflows/gametest/gds-playtest-plan/scripts/resolve-customization.py:159src/workflows/gametest/gds-test-automate/scripts/resolve-customization.py:159src/workflows/gametest/gds-test-design/scripts/resolve-customization.py:159src/workflows/gametest/gds-test-framework/scripts/resolve-customization.py:159src/workflows/gametest/gds-test-review/scripts/resolve-customization.py:159src/workflows/gds-document-project/scripts/resolve-customization.py:159src/workflows/gds-quick-flow/gds-quick-dev-new-preview/scripts/resolve-customization.py:159src/workflows/gds-quick-flow/gds-quick-dev/scripts/resolve-customization.py:159src/workflows/gds-quick-flow/gds-quick-spec/scripts/resolve-customization.py:159
🤖 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() returns {} on parse errors (only emitting a warning), which can make a broken customization file look like “customization doesn’t work” while still exiting successfully. Consider whether parse failures (especially for the defaults file) should be treated as hard errors to avoid silently dropping configuration.
Severity: medium
Other Locations
src/agents/gds-agent-game-architect/scripts/resolve-customization.py:51src/agents/gds-agent-game-designer/scripts/resolve-customization.py:51src/agents/gds-agent-game-dev/scripts/resolve-customization.py:51src/agents/gds-agent-game-qa/scripts/resolve-customization.py:51src/agents/gds-agent-game-scrum-master/scripts/resolve-customization.py:51src/agents/gds-agent-game-solo-dev/scripts/resolve-customization.py:51src/agents/gds-agent-tech-writer/scripts/resolve-customization.py:51src/workflows/1-preproduction/gds-create-game-brief/scripts/resolve-customization.py:51src/workflows/1-preproduction/research/gds-domain-research/scripts/resolve-customization.py:51src/workflows/2-design/gds-create-gdd/scripts/resolve-customization.py:51src/workflows/2-design/gds-create-narrative/scripts/resolve-customization.py:51src/workflows/2-design/gds-create-ux-design/scripts/resolve-customization.py:51src/workflows/3-technical/gds-check-implementation-readiness/scripts/resolve-customization.py:51src/workflows/3-technical/gds-create-epics-and-stories/scripts/resolve-customization.py:51src/workflows/3-technical/gds-game-architecture/scripts/resolve-customization.py:51src/workflows/3-technical/gds-generate-project-context/scripts/resolve-customization.py:51src/workflows/4-production/gds-code-review/scripts/resolve-customization.py:51src/workflows/4-production/gds-correct-course/scripts/resolve-customization.py:51src/workflows/4-production/gds-create-story/scripts/resolve-customization.py:51src/workflows/4-production/gds-dev-story/scripts/resolve-customization.py:51src/workflows/4-production/gds-retrospective/scripts/resolve-customization.py:51src/workflows/4-production/gds-sprint-planning/scripts/resolve-customization.py:51src/workflows/4-production/gds-sprint-status/scripts/resolve-customization.py:51src/workflows/gametest/gds-e2e-scaffold/scripts/resolve-customization.py:51src/workflows/gametest/gds-performance-test/scripts/resolve-customization.py:51src/workflows/gametest/gds-playtest-plan/scripts/resolve-customization.py:51src/workflows/gametest/gds-test-automate/scripts/resolve-customization.py:51src/workflows/gametest/gds-test-design/scripts/resolve-customization.py:51src/workflows/gametest/gds-test-framework/scripts/resolve-customization.py:51src/workflows/gametest/gds-test-review/scripts/resolve-customization.py:51src/workflows/gds-document-project/scripts/resolve-customization.py:51src/workflows/gds-quick-flow/gds-quick-dev-new-preview/scripts/resolve-customization.py:51src/workflows/gds-quick-flow/gds-quick-dev/scripts/resolve-customization.py:51src/workflows/gds-quick-flow/gds-quick-spec/scripts/resolve-customization.py:51
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| """Merge-by-code: matching codes replace; new codes append.""" | ||
| result_by_code: dict[str, dict] = {item["code"]: dict(item) for item in base} | ||
| for item in override: | ||
| result_by_code[item["code"]] = dict(item) |
There was a problem hiding this comment.
merge_menu() assumes every entry has a code key; since _is_menu_array() only checks the first element, a malformed later element can raise KeyError here and abort customization resolution. Consider whether this should be validated/handled more defensively to keep the resolver robust against partial user edits.
Severity: medium
Other Locations
src/agents/gds-agent-game-architect/scripts/resolve-customization.py:73src/agents/gds-agent-game-designer/scripts/resolve-customization.py:73src/agents/gds-agent-game-dev/scripts/resolve-customization.py:73src/agents/gds-agent-game-qa/scripts/resolve-customization.py:73src/agents/gds-agent-game-scrum-master/scripts/resolve-customization.py:73src/agents/gds-agent-game-solo-dev/scripts/resolve-customization.py:73src/agents/gds-agent-tech-writer/scripts/resolve-customization.py:73src/workflows/1-preproduction/gds-create-game-brief/scripts/resolve-customization.py:73src/workflows/1-preproduction/research/gds-domain-research/scripts/resolve-customization.py:73src/workflows/2-design/gds-create-gdd/scripts/resolve-customization.py:73src/workflows/2-design/gds-create-narrative/scripts/resolve-customization.py:73src/workflows/2-design/gds-create-ux-design/scripts/resolve-customization.py:73src/workflows/3-technical/gds-check-implementation-readiness/scripts/resolve-customization.py:73src/workflows/3-technical/gds-create-epics-and-stories/scripts/resolve-customization.py:73src/workflows/3-technical/gds-game-architecture/scripts/resolve-customization.py:73src/workflows/3-technical/gds-generate-project-context/scripts/resolve-customization.py:73src/workflows/4-production/gds-code-review/scripts/resolve-customization.py:73src/workflows/4-production/gds-correct-course/scripts/resolve-customization.py:73src/workflows/4-production/gds-create-story/scripts/resolve-customization.py:73src/workflows/4-production/gds-dev-story/scripts/resolve-customization.py:73src/workflows/4-production/gds-retrospective/scripts/resolve-customization.py:73src/workflows/4-production/gds-sprint-planning/scripts/resolve-customization.py:73src/workflows/4-production/gds-sprint-status/scripts/resolve-customization.py:73src/workflows/gametest/gds-e2e-scaffold/scripts/resolve-customization.py:73src/workflows/gametest/gds-performance-test/scripts/resolve-customization.py:73src/workflows/gametest/gds-playtest-plan/scripts/resolve-customization.py:73src/workflows/gametest/gds-test-automate/scripts/resolve-customization.py:73src/workflows/gametest/gds-test-design/scripts/resolve-customization.py:73src/workflows/gametest/gds-test-framework/scripts/resolve-customization.py:73src/workflows/gametest/gds-test-review/scripts/resolve-customization.py:73src/workflows/gds-document-project/scripts/resolve-customization.py:73src/workflows/gds-quick-flow/gds-quick-dev-new-preview/scripts/resolve-customization.py:73src/workflows/gds-quick-flow/gds-quick-dev/scripts/resolve-customization.py:73src/workflows/gds-quick-flow/gds-quick-spec/scripts/resolve-customization.py:73
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
Change except Exception to except (tomllib.TOMLDecodeError, OSError) for clearer failure signaling in user-facing tooling.
Refactor all 7 GDS agent SKILL.md files to resolve persona, inject, additional_resources, and menu from customize.toml at activation. Remove hardcoded Identity, Communication Style, and Principles sections. Preserve Critical Actions sections where present.
- 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 and customize.toml
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/agents/gds-agent-game-dev/SKILL.md (1)
45-57:⚠️ Potential issue | 🔴 CriticalFix capabilities table: Two skill names are not registered.
The verification reveals that the Capabilities table references two skills that do not exist in the codebase:
gds-quick-prototype— not foundbmad-advanced-elicitation— not foundThree skills are correctly registered:
gds-dev-story✓gds-code-review✓gds-quick-dev✓Since the documentation states "invoke the corresponding skill by its exact registered name" and "DO NOT invent capabilities on the fly," remove or replace the unregistered skills or ensure they are implemented before documenting them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/gds-agent-game-dev/SKILL.md` around lines 45 - 57, The Capabilities table lists two unregistered skills (gds-quick-prototype and bmad-advanced-elicitation); update SKILL.md to either remove those two entries or replace them with the exact registered skill names, ensuring the table entries and the "CRITICAL Handling" instruction still reference only exact, implemented skill names (e.g., keep gds-dev-story, gds-code-review, gds-quick-dev) and do not invent new IDs unless the corresponding skill implementations (matching the exact names) are added to the codebase.src/agents/gds-agent-game-designer/SKILL.md (1)
44-56:⚠️ Potential issue | 🔴 CriticalRemove or register the
gds-quick-prototypeskill.The capabilities table references
gds-quick-prototype, but this skill is not registered in the system. Four of the five referenced skills exist at their expected locations insrc/workflows/, butgds-quick-prototypehas no corresponding directory. Since the SKILL.md instructions state "invoke the corresponding skill by its exact registered name" and "DO NOT invent capabilities on the fly", users will encounter an error if they attempt to invoke this capability. Either remove the entry from the capabilities table or create the missing skill directory.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/gds-agent-game-designer/SKILL.md` around lines 44 - 56, The Capabilities table in SKILL.md lists gds-quick-prototype but that skill isn't registered; either remove the "QP | Rapid game prototyping | gds-quick-prototype" row from the table or create and register the missing skill with the exact name gds-quick-prototype (add its workflow/handler, manifest/registration entry and any expected exports so it can be invoked by name), and verify SKILL.md and the skill registry reference the identical string "gds-quick-prototype".src/agents/gds-agent-game-solo-dev/SKILL.md (1)
40-54:⚠️ Potential issue | 🔴 CriticalTwo referenced skills are unregistered and unavailable: remove them from the capabilities table or implement them.
The capabilities table references
gds-quick-prototypeandbmad-advanced-elicitation, but neither skill has a registered implementation in the codebase. Only 5 of the 7 listed skills exist:
- ✗
gds-quick-prototype— NO DIRECTORY FOUND- ✓
gds-quick-dev— exists atsrc/workflows/gds-quick-flow/gds-quick-dev/- ✓
gds-quick-spec— exists atsrc/workflows/gds-quick-flow/gds-quick-spec/- ✓
gds-code-review— exists atsrc/workflows/4-production/gds-code-review/- ✓
gds-test-framework— exists atsrc/workflows/gametest/gds-test-framework/- ✓
gds-quick-dev-new-preview— exists atsrc/workflows/gds-quick-flow/gds-quick-dev-new-preview/- ✗
bmad-advanced-elicitation— NO DIRECTORY FOUNDPer the instruction "invoke the corresponding skill by its exact registered name... DO NOT invent capabilities on the fly," this breaks the contract—users cannot invoke non-existent skills.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/gds-agent-game-solo-dev/SKILL.md` around lines 40 - 54, The Capabilities table references two unregistered skills ("gds-quick-prototype" / code QP and "bmad-advanced-elicitation" / code AE) which breaks the invoke-by-exact-name contract; either remove the QP and AE rows from the table in SKILL.md or implement and register those skills under the exact names gds-quick-prototype and bmad-advanced-elicitation (ensuring their workflow directories and registration match the other entries like gds-quick-dev, gds-quick-spec, gds-code-review, gds-test-framework, and gds-quick-dev-new-preview) so users can invoke them exactly as listed.
♻️ Duplicate comments (6)
src/workflows/3-technical/gds-create-epics-and-stories/SKILL.md (1)
6-26:⚠️ Potential issue | 🟡 MinorClarify the working directory for script execution.
Same working directory ambiguity as other workflow SKILL.md files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/3-technical/gds-create-epics-and-stories/SKILL.md` around lines 6 - 26, The SKILL.md is ambiguous about which working directory to run scripts like scripts/resolve-customization.py from; update the document (references: scripts/resolve-customization.py, commands shown using python3) to explicitly state the working directory (e.g., “run from repository root”) and clarify whether paths like ./workflow.md and listed resource files are relative to that root or to the workflow folder, and add a single example command showing the full relative invocation from the specified directory to remove ambiguity.src/workflows/3-technical/gds-check-implementation-readiness/SKILL.md (1)
6-26:⚠️ Potential issue | 🟡 MinorClarify the working directory for script execution.
Same working directory ambiguity as other workflow SKILL.md files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/3-technical/gds-check-implementation-readiness/SKILL.md` around lines 6 - 26, The instructions in SKILL.md are ambiguous about where to run scripts; update the SKILL.md content to explicitly state the working directory and how to execute the script: specify that scripts/resolve-customization.py should be invoked from the repository root (or indicate if it must be run from the scripts/ directory), and clarify whether paths in --key values (inject, additional_resources, inject.after) and references in ./workflow.md are relative to that root or another directory; mention the expected cwd for reading files listed in additional_resources and for resolving inject.before/after so users know where to run the python command and where file paths are resolved.src/workflows/2-design/gds-create-narrative/SKILL.md (1)
6-26:⚠️ Potential issue | 🟡 MinorClarify the working directory for script execution.
Same working directory ambiguity as other workflow SKILL.md files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/2-design/gds-create-narrative/SKILL.md` around lines 6 - 26, The instructions are ambiguous about the working directory when running scripts; clarify where to run scripts/resolve-customization.py (e.g., from repo root) and how file paths in additional_resources are resolved. Update SKILL.md to state: run commands from the repository root (or specify alternative), that paths in additional_resources are relative to that working directory (or absolute), and ensure references to ./workflow.md are resolved from the same base directory; mention the exact command examples used (python3 scripts/resolve-customization.py gds-create-narrative --key inject --key additional_resources and python3 scripts/resolve-customization.py gds-create-narrative --key inject.after) so readers know the expected CWD and path resolution for inject.before, additional_resources, and inject.after.src/workflows/3-technical/gds-generate-project-context/SKILL.md (1)
6-26:⚠️ Potential issue | 🟡 MinorClarify the working directory for script execution.
Same issue as in
gds-game-architecture/SKILL.md: the working directory for runningscripts/resolve-customization.pyis not specified, which may confuse users.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/3-technical/gds-generate-project-context/SKILL.md` around lines 6 - 26, The instructions omit the working directory for running scripts/resolve-customization.py; update the documentation around the two command invocations (the Resolve Customization command that calls "python3 scripts/resolve-customization.py gds-generate-project-context --key inject --key additional_resources" and the Post-Workflow command that calls "python3 scripts/resolve-customization.py gds-generate-project-context --key inject.after") to explicitly state they must be executed from the repository root (or specify the exact working directory used by your workflows), and add a short note telling users how to run the command from a different cwd (e.g., prepend the repo root path or cd into repo root) so the script path resolves correctly.src/workflows/4-production/gds-retrospective/SKILL.md (1)
6-26:⚠️ Potential issue | 🟡 MinorClarify the working directory for script execution.
Same working directory ambiguity as other workflow SKILL.md files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/4-production/gds-retrospective/SKILL.md` around lines 6 - 26, Clarify the working directory for the resolve-customization commands by stating where to run scripts/resolve-customization.py (e.g., "run from the repository root" or "run from the scripts/ directory") and update the three Run lines that reference "python3 scripts/resolve-customization.py gds-retrospective ..." so they explicitly specify the required working directory or use an explicit path (e.g., "./scripts/resolve-customization.py" vs "scripts/...") and add a short note in the "Available Scripts" or "Resolve Customization" and "Post-Workflow Customization" sections indicating the expected CWD and any environment assumptions; ensure references to the script name scripts/resolve-customization.py and keys (--key inject, --key additional_resources, --key inject.after) are preserved.src/workflows/gametest/gds-test-automate/SKILL.md (1)
6-26:⚠️ Potential issue | 🟡 MinorClarify the working directory for script execution.
Same working directory ambiguity issue as noted in other SKILL.md files.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/gametest/gds-test-automate/SKILL.md` around lines 6 - 26, Clarify which working directory to use when running the resolve-customization script: state explicitly that the commands (e.g., python3 scripts/resolve-customization.py gds-test-automate --key inject --key additional_resources and python3 scripts/resolve-customization.py gds-test-automate --key inject.after) must be executed from the repository root (or specify an alternative directory such as the scripts directory) and show the exact shell steps to ensure correctness (e.g., cd to repo root || cd scripts) so readers know where relative paths resolve and which directory contains scripts/resolve-customization.py.
🟡 Minor comments (4)
src/workflows/gds-quick-flow/gds-quick-dev-new-preview/SKILL.md-16-17 (1)
16-17:⚠️ Potential issue | 🟡 MinorDocument the base path for
additional_resources.These instructions say to read each listed file, but they don't repeat that the paths are relative to
{project-root}. Without that, callers can resolve them from the wrong directory and miss the intended resources.Suggested fix
-If `additional_resources` is not empty, read each listed file and incorporate as reference context. +If `additional_resources` is not empty, read each listed file relative to `{project-root}` and incorporate its contents as reference context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/gds-quick-flow/gds-quick-dev-new-preview/SKILL.md` around lines 16 - 17, Update the SKILL.md text around the handling of inject.before and additional_resources to explicitly state that file paths in additional_resources are resolved relative to the project root (i.e., `{project-root}`); mention that the system will read each listed file from `{project-root}` and incorporate its contents as reference context, and clarify that inject.before content is treated as higher-priority context so callers should provide relative paths from `{project-root}` when listing additional_resources.src/workflows/3-technical/gds-game-architecture/SKILL.md-6-26 (1)
6-26:⚠️ Potential issue | 🟡 MinorClarify the working directory for script execution.
The documentation instructs users to run
python3 scripts/resolve-customization.pybut doesn't specify the working directory. Users may be unclear whether to execute the script from the skill's directory (e.g.,src/workflows/3-technical/gds-game-architecture/), the project root, or elsewhere.Consider adding a brief note indicating the expected working directory, such as:
Run from the skill directory: `python3 scripts/resolve-customization.py gds-game-architecture --key inject --key additional_resources`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/3-technical/gds-game-architecture/SKILL.md` around lines 6 - 26, The docs lack the working-directory context for running scripts/resolve-customization.py; update SKILL.md to state where to run the commands (e.g., run from the repository root or the skill directory) and give explicit examples for the three commands referencing scripts/resolve-customization.py and the skill key gds-game-architecture, and mention that resolved keys inject, additional_resources and inject.after are produced as JSON to be consumed by the workflow referenced in ./workflow.md so users know whether to run the command from the skill directory (src/workflows/3-technical/gds-game-architecture/) or the project root.src/agents/gds-agent-game-qa/SKILL.md-44-45 (1)
44-45:⚠️ Potential issue | 🟡 MinorMake
project-context.mdresolution deterministic.Line 44's
**/project-context.mdsearch can match multiple files in a monorepo, but the activation flow never says which one wins. Please define a stable rule, e.g. nearest to the current working directory, so the agent does not load different standards depending on search order.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/gds-agent-game-qa/SKILL.md` around lines 44 - 45, The glob "**/project-context.md" in the "Load project context" step is ambiguous in monorepos; change the resolution to be deterministic by: search for all matches to "**/project-context.md", then select the single file nearest to the current working directory (fewest directory levels from CWD); if there is a tie, break ties deterministically (e.g., choose the lexicographically smallest path or prefer workspace root), update the "Load project context" step and any implementation that performs the search to apply this selection logic, and document the deterministic rule in SKILL.md.src/agents/gds-agent-game-dev/scripts/resolve-customization.py-91-96 (1)
91-96:⚠️ Potential issue | 🟡 MinorRestrict merge-by-code to the
menufield.Lines 93-94 currently apply menu semantics to any list of dicts that happens to contain a
codekey, even though the documented rule is[[menu]]only. A future non-menu array withcodeentries would merge unexpectedly instead of replacing atomically.Suggested fix
- elif _is_menu_array(over_val) and _is_menu_array(base_val): + elif key == "menu" and _is_menu_array(over_val) and _is_menu_array(base_val): merged[key] = merge_menu(base_val, over_val)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/gds-agent-game-dev/scripts/resolve-customization.py` around lines 91 - 96, The current merge treats any list-of-dicts with a "code" key as a menu by using _is_menu_array, causing unintended merges; modify the conditional in deep_merge so merge_menu is only used when the current key is exactly "menu" (i.e., change the elif to require key == "menu" && _is_menu_array(over_val) && _is_menu_array(base_val)), otherwise fall back to replacing with over_val; update references to deep_merge, _is_menu_array, merge_menu, over_val, base_val, and merged to implement this restriction.
🧹 Nitpick comments (12)
src/workflows/gametest/gds-playtest-plan/scripts/resolve-customization.py (1)
168-174: Warn on unresolved--keyrequests instead of silently dropping them.Silent omission makes typos in dotted keys hard to detect during workflow wiring.
🛠️ Suggested improvement
if args.keys: result = {} for key in args.keys: value = extract_key(merged, key) - if value is not None: - result[key] = value + if value is None: + print(f"warning: unresolved key '{key}'", file=sys.stderr) + continue + result[key] = value json.dump(result, sys.stdout, indent=2, ensure_ascii=False)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/gametest/gds-playtest-plan/scripts/resolve-customization.py` around lines 168 - 174, The current loop over args.keys silently drops keys when extract_key(merged, key) returns None; change it to emit a warning for each unresolved key so typos are visible. In the block that iterates over args.keys (and uses extract_key and merged to build result), when value is None write a warning to stderr or via logging that includes the requested key name and context (e.g., "unresolved key: <key>") instead of silently skipping it; still add found keys to result and keep the existing json.dump behavior.src/workflows/4-production/gds-dev-story/scripts/resolve-customization.py (2)
67-75: Menu item ordering may vary.
merge_menubuilds results using a dict keyed bycode. While Python 3.7+ preserves insertion order, the final order will be: base items (in their original order, minus any replaced by override), then new override items appended. If strict ordering is required for menus, this may need adjustment.If menu order matters, consider preserving original base order and appending only genuinely new items:
♻️ Order-preserving alternative
def merge_menu(base: list[dict], override: list[dict]) -> list[dict]: """Merge-by-code: matching codes replace; new codes append.""" - result_by_code: dict[str, dict] = {item["code"]: dict(item) for item in base if "code" in item} + override_by_code = {item["code"]: dict(item) for item in override if "code" in item} + result = [] + seen_codes = set() + # Preserve base order, replacing with overrides where matched + for item in base: + if "code" not in item: + continue + code = item["code"] + seen_codes.add(code) + result.append(override_by_code.get(code, dict(item))) + # Append new items from override for item in override: if "code" not in item: print(f"warning: menu item missing 'code' key, skipping: {item}", file=sys.stderr) continue - result_by_code[item["code"]] = dict(item) - return list(result_by_code.values()) + if item["code"] not in seen_codes: + result.append(dict(item)) + return result🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/4-production/gds-dev-story/scripts/resolve-customization.py` around lines 67 - 75, merge_menu currently collects items into result_by_code which yields base items (with replacements) followed by override-only items in arbitrary dict value order; to preserve menu ordering change the algorithm in merge_menu to iterate base in order and for each base item yield the override item if override contains the same "code" (replace in-place), otherwise yield the base item, then iterate override in order and append only items whose "code" is not present in the base (new items), using the same "code" checks and stderr warnings for missing codes; reference merge_menu and result_by_code to locate where to implement this ordering-preserving merge.
1-183: Implementation is solid; consider consolidating duplicated scripts.The three-layer merge logic is well-implemented with appropriate error handling. However, this script is duplicated across 35 skill directories. If a bug is found or an enhancement is needed, all copies must be updated.
Consider consolidating into a single shared script that can be invoked with the skill directory as an argument, or symlinked from each skill's
scripts/directory. This would reduce maintenance burden while preserving the per-skill invocation pattern.Example approach:
# Single shared script at: # _bmad/scripts/resolve-customization.py # Per-skill invocation: python3 _bmad/scripts/resolve-customization.py --skill-dir src/workflows/4-production/gds-dev-story gds-dev-storyAlternatively, if the current approach is intentional for self-contained skill packaging, that's a valid trade-off—just document the update process when changes are needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/4-production/gds-dev-story/scripts/resolve-customization.py` around lines 1 - 183, The repo has 35 copies of this script—consolidate into a single shared script and update per-skill invocations to call it (or symlink it) to avoid duplicate maintenance. Extract the core functions (find_project_root, load_toml, _is_menu_array, merge_menu, deep_merge, extract_key) and the CLI flow around defaults_path/project_root into one shared _bmad/scripts/resolve-customization.py, change the entrypoint to accept either --skill-dir (path to skill containing customize.toml) and skill_name (for looking up project overrides) or preserve the existing positional skill_name but compute defaults_path from --skill-dir; keep the same JSON output behavior and flags (args.keys), and update each skill's scripts/resolve-customization.py to become a tiny shim that calls the shared script (or create a symlink) so invocation and behavior remain identical.src/workflows/2-design/gds-create-ux-design/scripts/resolve-customization.py (1)
1-183: Consider centralizing resolver logic instead of copying per-skill scripts.This implementation is duplicated across many skills; a shared resolver module/script would reduce drift and make bug fixes (like merge edge cases) one-and-done.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/2-design/gds-create-ux-design/scripts/resolve-customization.py` around lines 1 - 183, This repo has duplicate per-skill resolver scripts; extract the core logic (load_toml, find_project_root, deep_merge, merge_menu, _is_menu_array, extract_key, and the main merge flow used in main) into a single shared module (e.g., bmad.resolve_customization) that exposes a function like resolve_customization(skill_name: str, start_dir: Path) -> dict and an optional resolve_keys(data, keys) helper; update each per-skill script to import and call that function (keeping only CLI thin-wrapper code), remove duplicated functions from skill scripts, and add unit tests covering merge_menu, deep_merge, and dotted-key extraction to prevent future drift. Ensure public API preserves behavior (defaults <- team <- user, menu merge-by-code, arrays atomic replace, and CLI --key handling) and update any scripts to use the new module's JSON output logic.src/workflows/3-technical/gds-create-epics-and-stories/scripts/resolve-customization.py (1)
30-183: Extract this resolver into one shared implementation.This file is effectively the same as the other
scripts/resolve-customization.pycopies in the PR. Every behavior change now needs dozens of manual edits, which makes drift likely the next time merge logic changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/3-technical/gds-create-epics-and-stories/scripts/resolve-customization.py` around lines 30 - 183, This file duplicates resolver logic across the repo; extract the shared implementation so other copies import it instead of drifting. Move the core functions (find_project_root, load_toml, _is_menu_array, merge_menu, deep_merge, extract_key and the merge/load logic) into a single shared module (e.g. resolve_customization/shared or bmad.resolve_customization) exposing a resolve_customizations(skill_name, keys=None, cwd=None) or a main_entry function, then change this script's main to be a thin CLI wrapper that imports and calls that shared function (keeping argument parsing here). Update other scripts to import the same shared module and remove duplicate function bodies from their files.src/workflows/1-preproduction/gds-brainstorm-game/scripts/resolve-customization.py (1)
1-183: Consider centralizing the resolver instead of copying it into every skill.This file is effectively the same implementation being added in many places. Any future merge bug or security fix now needs to be patched across every copy, which is easy to miss.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/1-preproduction/gds-brainstorm-game/scripts/resolve-customization.py` around lines 1 - 183, This script duplicates a resolver used across many skills; extract the common logic (find_project_root, load_toml, _is_menu_array, merge_menu, deep_merge, extract_key and the TOML merge/output behavior) into a single shared module (e.g., bmad.customization.resolver) and replace per-skill copies with a small wrapper that imports the shared resolver and invokes a single function/CLI entrypoint (e.g., resolve_customization(skill_name, keys=None) or main()) so fixes apply centrally; ensure the shared resolver still computes defaults_path relative to the caller's skill directory (accepting an optional skill_dir or defaults_path argument) and update the script to only parse CLI args and call the shared API.src/agents/gds-agent-game-designer/customize.toml (1)
47-50: Remove or complete the empty “Menu customization docs” section.Line 48 introduces a menu customization section, but no menu defaults or guidance follow. This creates avoidable ambiguity for users editing overrides.
Minimal cleanup option
-# ────────────────────────────────────────────────────────────────── -# Menu customization docs -# ────────────────────────────────────────────────────────────────── - # ────────────────────────────────────────────────────────────────── # Injected prompts # ──────────────────────────────────────────────────────────────────🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/gds-agent-game-designer/customize.toml` around lines 47 - 50, The "# Menu customization docs" placeholder is empty and should be removed or completed: either delete that header and any blank lines to avoid confusion, or replace it with a short, concrete guidance block that documents the expected menu override keys (e.g., items, order, labels) and the default values/behavior used by this agent; update the section where "# Menu customization docs" appears to include one or two brief examples and a note about precedence so users know how to write overrides.src/workflows/4-production/gds-sprint-planning/SKILL.md (1)
10-25: Clarify command execution directory.Please explicitly state these commands are run from the
src/workflows/4-production/gds-sprint-planning/directory to avoid path-resolution mistakes.Suggested docs tweak
## Resolve Customization Resolve `inject` and `additional_resources` from customization: -Run: `python3 scripts/resolve-customization.py gds-sprint-planning --key inject --key additional_resources` +Run from `src/workflows/4-production/gds-sprint-planning/`: +`python3 scripts/resolve-customization.py gds-sprint-planning --key inject --key additional_resources` Use the JSON output as resolved values. @@ After the workflow completes, resolve `inject.after` from customization: -Run: `python3 scripts/resolve-customization.py gds-sprint-planning --key inject.after` +Run from `src/workflows/4-production/gds-sprint-planning/`: +`python3 scripts/resolve-customization.py gds-sprint-planning --key inject.after`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/4-production/gds-sprint-planning/SKILL.md` around lines 10 - 25, The document lacks an explicit working-directory for the resolve commands; update SKILL.md (the "Resolve Customization" and "Post-Workflow Customization" sections) to state that the following commands must be executed from the src/workflows/4-production/gds-sprint-planning/ directory, and clarify that the scripts referenced (resolve-customization.py) are relative to that directory when resolving keys inject, additional_resources, inject.before and inject.after and when reading files referenced by additional_resources; ensure the note also points to ./workflow.md for subsequent steps so readers know the expected cwd for all referenced paths.src/agents/gds-agent-game-architect/customize.toml (1)
47-49: Consider removing the empty placeholder section.The "Menu customization docs" section header with no content appears to be leftover scaffolding. If there's no immediate plan to document menu customization here, consider removing these lines to keep the file clean.
🧹 Proposed cleanup
-# ────────────────────────────────────────────────────────────────── -# Menu customization docs -# ────────────────────────────────────────────────────────────────── -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/gds-agent-game-architect/customize.toml` around lines 47 - 49, Remove the empty placeholder section containing the "Menu customization docs" header and its surrounding separator comment blocks (the three-comment separator lines and the "Menu customization docs" line) so the TOML file no longer contains unused scaffolding; if you plan to add content later, add it as a real subsection instead of leaving blank headers.src/agents/gds-agent-game-scrum-master/SKILL.md (1)
43-56: Avoid a second source of truth for the capabilities menu.Step 1 resolves
menufrom customization, but Line 43 still starts from a hardcoded table. That means default capabilities now live in bothcustomize.tomlandSKILL.md, so they can drift. Prefer presenting the resolvedmenudirectly, or clearly treat the markdown table as generated from the TOML defaults.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/gds-agent-game-scrum-master/SKILL.md` around lines 43 - 56, The hardcoded capabilities table in SKILL.md duplicates defaults stored in customize.toml causing a second source of truth; update SKILL.md to instead present the resolved `menu` (the merged result used at runtime) or clearly indicate the table is generated from TOML defaults. Specifically, replace the static table block (the table under "Capabilities") with a dynamic representation or a statement that prints/embeds the resolved `menu`, and ensure the documentation references the `menu` resolution logic (the same `menu` variable/process used when merging customization) so only the resolved `menu` is shown to users.src/workflows/gds-quick-flow/gds-quick-dev/scripts/resolve-customization.py (1)
1-183: Please centralize the resolver implementation.This file is one of many near-identical copies of the same resolver. The recent menu fixes already had to be replayed across every copy, so future hardening will have the same fan-out cost. A shared module or single repo-level script would make follow-up fixes much safer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/gds-quick-flow/gds-quick-dev/scripts/resolve-customization.py` around lines 1 - 183, This resolver is a near-duplicate of many copies; centralize it into a single shared module (e.g., expose a resolver function) and have this script become a thin CLI wrapper that imports and calls that shared implementation: extract the functions find_project_root, load_toml, _is_menu_array, merge_menu, deep_merge, extract_key and the merging logic into a repo-level module (e.g., bmad.customization.resolve or similar) with a clear API like resolve_customization(skill_name: str, start_path: Path | None) -> dict, and modify this file to import that API and only handle CLI args, path-to-skill resolution and JSON printing; preserve behavior for defaults_path resolution (allow passing skill_dir or defaults_path into the shared API) and keep warnings/IO behavior consistent so other scripts can reuse fixes (menu merging etc.) without duplicated code.src/agents/gds-agent-game-dev/scripts/resolve-customization.py (1)
1-183: Move the resolver into shared code instead of copying it per skill.This PR adds the same resolver implementation to many directories. Any future fix to merge semantics, path resolution, or error handling now needs the same edit everywhere.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/gds-agent-game-dev/scripts/resolve-customization.py` around lines 1 - 183, This file duplicates resolver logic across skills; extract the shared logic (find_project_root, load_toml, _is_menu_array, merge_menu, deep_merge, extract_key and the merge/load behavior) into a single shared module (e.g., a package module like bmad.customization or bmad_tools.customize) and export a simple API function (e.g., resolve_customization(skill_name, start_path=... , keys=None) or resolve_and_dump(args)) that the per-skill script can call; then replace each copied resolve-customization.py with a tiny wrapper that imports the shared function and handles CLI arg parsing (or have the shared module parse args) so fixes to merge semantics, path resolution, or error handling are changed once in the shared module rather than in every duplicate file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: be444696-fbe3-4ccd-892a-565d0aa56009
📒 Files selected for processing (105)
src/agents/gds-agent-game-architect/SKILL.mdsrc/agents/gds-agent-game-architect/customize.tomlsrc/agents/gds-agent-game-architect/scripts/resolve-customization.pysrc/agents/gds-agent-game-designer/SKILL.mdsrc/agents/gds-agent-game-designer/customize.tomlsrc/agents/gds-agent-game-designer/scripts/resolve-customization.pysrc/agents/gds-agent-game-dev/SKILL.mdsrc/agents/gds-agent-game-dev/customize.tomlsrc/agents/gds-agent-game-dev/scripts/resolve-customization.pysrc/agents/gds-agent-game-qa/SKILL.mdsrc/agents/gds-agent-game-qa/customize.tomlsrc/agents/gds-agent-game-qa/scripts/resolve-customization.pysrc/agents/gds-agent-game-scrum-master/SKILL.mdsrc/agents/gds-agent-game-scrum-master/customize.tomlsrc/agents/gds-agent-game-scrum-master/scripts/resolve-customization.pysrc/agents/gds-agent-game-solo-dev/SKILL.mdsrc/agents/gds-agent-game-solo-dev/customize.tomlsrc/agents/gds-agent-game-solo-dev/scripts/resolve-customization.pysrc/agents/gds-agent-tech-writer/SKILL.mdsrc/agents/gds-agent-tech-writer/customize.tomlsrc/agents/gds-agent-tech-writer/scripts/resolve-customization.pysrc/workflows/1-preproduction/gds-brainstorm-game/SKILL.mdsrc/workflows/1-preproduction/gds-brainstorm-game/customize.tomlsrc/workflows/1-preproduction/gds-brainstorm-game/scripts/resolve-customization.pysrc/workflows/1-preproduction/gds-create-game-brief/SKILL.mdsrc/workflows/1-preproduction/gds-create-game-brief/customize.tomlsrc/workflows/1-preproduction/gds-create-game-brief/scripts/resolve-customization.pysrc/workflows/1-preproduction/research/gds-domain-research/SKILL.mdsrc/workflows/1-preproduction/research/gds-domain-research/customize.tomlsrc/workflows/1-preproduction/research/gds-domain-research/scripts/resolve-customization.pysrc/workflows/2-design/gds-create-gdd/SKILL.mdsrc/workflows/2-design/gds-create-gdd/customize.tomlsrc/workflows/2-design/gds-create-gdd/scripts/resolve-customization.pysrc/workflows/2-design/gds-create-narrative/SKILL.mdsrc/workflows/2-design/gds-create-narrative/customize.tomlsrc/workflows/2-design/gds-create-narrative/scripts/resolve-customization.pysrc/workflows/2-design/gds-create-ux-design/SKILL.mdsrc/workflows/2-design/gds-create-ux-design/customize.tomlsrc/workflows/2-design/gds-create-ux-design/scripts/resolve-customization.pysrc/workflows/3-technical/gds-check-implementation-readiness/SKILL.mdsrc/workflows/3-technical/gds-check-implementation-readiness/customize.tomlsrc/workflows/3-technical/gds-check-implementation-readiness/scripts/resolve-customization.pysrc/workflows/3-technical/gds-create-epics-and-stories/SKILL.mdsrc/workflows/3-technical/gds-create-epics-and-stories/customize.tomlsrc/workflows/3-technical/gds-create-epics-and-stories/scripts/resolve-customization.pysrc/workflows/3-technical/gds-game-architecture/SKILL.mdsrc/workflows/3-technical/gds-game-architecture/customize.tomlsrc/workflows/3-technical/gds-game-architecture/scripts/resolve-customization.pysrc/workflows/3-technical/gds-generate-project-context/SKILL.mdsrc/workflows/3-technical/gds-generate-project-context/customize.tomlsrc/workflows/3-technical/gds-generate-project-context/scripts/resolve-customization.pysrc/workflows/4-production/gds-code-review/SKILL.mdsrc/workflows/4-production/gds-code-review/customize.tomlsrc/workflows/4-production/gds-code-review/scripts/resolve-customization.pysrc/workflows/4-production/gds-correct-course/SKILL.mdsrc/workflows/4-production/gds-correct-course/customize.tomlsrc/workflows/4-production/gds-correct-course/scripts/resolve-customization.pysrc/workflows/4-production/gds-create-story/SKILL.mdsrc/workflows/4-production/gds-create-story/customize.tomlsrc/workflows/4-production/gds-create-story/scripts/resolve-customization.pysrc/workflows/4-production/gds-dev-story/SKILL.mdsrc/workflows/4-production/gds-dev-story/customize.tomlsrc/workflows/4-production/gds-dev-story/scripts/resolve-customization.pysrc/workflows/4-production/gds-retrospective/SKILL.mdsrc/workflows/4-production/gds-retrospective/customize.tomlsrc/workflows/4-production/gds-retrospective/scripts/resolve-customization.pysrc/workflows/4-production/gds-sprint-planning/SKILL.mdsrc/workflows/4-production/gds-sprint-planning/customize.tomlsrc/workflows/4-production/gds-sprint-planning/scripts/resolve-customization.pysrc/workflows/4-production/gds-sprint-status/SKILL.mdsrc/workflows/4-production/gds-sprint-status/customize.tomlsrc/workflows/4-production/gds-sprint-status/scripts/resolve-customization.pysrc/workflows/gametest/gds-e2e-scaffold/SKILL.mdsrc/workflows/gametest/gds-e2e-scaffold/customize.tomlsrc/workflows/gametest/gds-e2e-scaffold/scripts/resolve-customization.pysrc/workflows/gametest/gds-performance-test/SKILL.mdsrc/workflows/gametest/gds-performance-test/customize.tomlsrc/workflows/gametest/gds-performance-test/scripts/resolve-customization.pysrc/workflows/gametest/gds-playtest-plan/SKILL.mdsrc/workflows/gametest/gds-playtest-plan/customize.tomlsrc/workflows/gametest/gds-playtest-plan/scripts/resolve-customization.pysrc/workflows/gametest/gds-test-automate/SKILL.mdsrc/workflows/gametest/gds-test-automate/customize.tomlsrc/workflows/gametest/gds-test-automate/scripts/resolve-customization.pysrc/workflows/gametest/gds-test-design/SKILL.mdsrc/workflows/gametest/gds-test-design/customize.tomlsrc/workflows/gametest/gds-test-design/scripts/resolve-customization.pysrc/workflows/gametest/gds-test-framework/SKILL.mdsrc/workflows/gametest/gds-test-framework/customize.tomlsrc/workflows/gametest/gds-test-framework/scripts/resolve-customization.pysrc/workflows/gametest/gds-test-review/SKILL.mdsrc/workflows/gametest/gds-test-review/customize.tomlsrc/workflows/gametest/gds-test-review/scripts/resolve-customization.pysrc/workflows/gds-document-project/SKILL.mdsrc/workflows/gds-document-project/customize.tomlsrc/workflows/gds-document-project/scripts/resolve-customization.pysrc/workflows/gds-quick-flow/gds-quick-dev-new-preview/SKILL.mdsrc/workflows/gds-quick-flow/gds-quick-dev-new-preview/customize.tomlsrc/workflows/gds-quick-flow/gds-quick-dev-new-preview/scripts/resolve-customization.pysrc/workflows/gds-quick-flow/gds-quick-dev/SKILL.mdsrc/workflows/gds-quick-flow/gds-quick-dev/customize.tomlsrc/workflows/gds-quick-flow/gds-quick-dev/scripts/resolve-customization.pysrc/workflows/gds-quick-flow/gds-quick-spec/SKILL.mdsrc/workflows/gds-quick-flow/gds-quick-spec/customize.tomlsrc/workflows/gds-quick-flow/gds-quick-spec/scripts/resolve-customization.py
- 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
|
I'm leaving this open for now - it appears to be in active development on the BMAD-CODE side and the PR I've pushed supercedes much of the work (such as combining agents and workflows). |
Unblocks markdownlint MD034/no-bare-urls failure on CI. Same fix as the one already on main (via PR #21). Cherry-picked here so this branch's CI passes; a future rebase onto main will absorb it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 20
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (2)
src/agents/gds-agent-game-qa/scripts/resolve-customization.py-129-132 (1)
129-132:⚠️ Potential issue | 🟡 MinorValidate
skill_nameto prevent path traversal outside customizations directory.At Lines 160-161,
args.skill_nameis interpolated directly into a filename. Inputs like../../xcan escape_bmad/customizationsand read unintended files.Proposed fix
import argparse import json +import re import sys import tomllib @@ args = parser.parse_args() + if not re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9._-]*", args.skill_name): + parser.error("skill_name must match [A-Za-z0-9][A-Za-z0-9._-]*")Also applies to: 140-141, 159-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/gds-agent-game-qa/scripts/resolve-customization.py` around lines 129 - 132, Inputs in parser.add_argument are used verbatim (args.skill_name) to build filenames under the _bmad/customizations directory, allowing path traversal (e.g. ../../). Validate and sanitize args.skill_name before using it: reject any value containing path separators or suspicious sequences and enforce a strict whitelist pattern (e.g. ^[A-Za-z0-9_-]+$), or map skill names to known allowed entries; when building the file path use pathlib.Path to join and resolve (Path(custom_dir) / args.skill_name).resolve() and ensure the resolved path is a child of the customizations directory (compare .resolve().is_relative_to(custom_dir.resolve()) or check prefix), and apply this validation wherever args.skill_name is interpolated into filenames (references: parser.add_argument, args.skill_name, the _bmad/customizations file operations).src/agents/gds-agent-game-solo-dev/scripts/resolve-customization.py-148-152 (1)
148-152:⚠️ Potential issue | 🟡 MinorPrefer skill-local root when CWD root is not a BMAD project.
Line 148 trusts the current working directory first. If this script is launched from another git repo, customization files for this skill can be silently missed.
Suggested fix
- project_root = find_project_root(Path.cwd()) - if project_root is None: - # Try from the skill directory as fallback - project_root = find_project_root(skill_dir) + project_root = find_project_root(Path.cwd()) + if project_root is None or not (project_root / "_bmad" / "customizations").is_dir(): + # Fall back to the script's repository when CWD is outside a BMAD project + project_root = find_project_root(skill_dir)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/gds-agent-game-solo-dev/scripts/resolve-customization.py` around lines 148 - 152, The current lookup prefers find_project_root(Path.cwd()) which can pick up a different repo and miss this skill's customization; change the resolution to prefer the skill-local root first by calling find_project_root(skill_dir) and using that if present, then fall back to find_project_root(Path.cwd()); update the project_root assignment and any downstream logic that uses project_root (references: find_project_root, project_root, Path.cwd(), skill_dir) so the skill's local BMAD project is selected when available.
🧹 Nitpick comments (7)
src/workflows/4-production/gds-sprint-status/scripts/resolve-customization.py (1)
42-51: Add a strict mode for TOML load failures.Returning
{}on parse/read errors can silently mask broken customization. A--strictflag (non-zero exit on failure) would improve CI safety while keeping current behavior as default.🧪 Proposed direction
-def load_toml(path: Path) -> dict[str, Any]: +def load_toml(path: Path, *, strict: bool = False) -> dict[str, Any]: @@ - except (tomllib.TOMLDecodeError, OSError) as exc: + except (tomllib.TOMLDecodeError, OSError) as exc: + if strict: + raise print(f"warning: failed to parse {path}: {exc}", file=sys.stderr) return {}# in argparse parser.add_argument("--strict", action="store_true", help="Fail on TOML parse/read errors.") # and pass strict=args.strict to each load_toml(...) call🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/4-production/gds-sprint-status/scripts/resolve-customization.py` around lines 42 - 51, Introduce a --strict mode that makes TOML load failures fatal: add parser.add_argument("--strict", action="store_true", help="Fail on TOML parse/read errors.") and thread the resulting flag into each load_toml(...) call, then modify load_toml(path: Path, strict: bool = False) to keep current behavior (return {}) on non-strict errors but, when strict is True, log the error and exit non-zero (sys.exit(1)) or re-raise the exception instead of returning {}; update all call sites to pass strict=args.strict.src/agents/gds-agent-game-designer/scripts/resolve-customization.py (1)
149-152: Emit a warning when project root is unresolved.If both root lookups fail, overrides are skipped silently. Adding a warning to stderr would make misconfigured invocation context easier to diagnose.
🧭 Suggested diagnostic
@@ project_root = find_project_root(Path.cwd()) if project_root is None: # Try from the skill directory as fallback project_root = find_project_root(skill_dir) + if project_root is None: + print( + "warning: project root not found; using skill defaults only (team/user overrides skipped)", + file=sys.stderr, + )Also applies to: 158-162
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/gds-agent-game-designer/scripts/resolve-customization.py` around lines 149 - 152, The code currently falls back to project_root = find_project_root(skill_dir) and silently skips overrides if project_root remains None; update the logic in resolve-customization.py so that after both lookups (the initial project_root check and the fallback using find_project_root(skill_dir)) you detect when project_root is still None and emit a clear warning to stderr (e.g., using sys.stderr.write or logging.warning) indicating that project root could not be resolved and overrides will be skipped; reference the project_root variable, the find_project_root call, and the overrides handling branch so the warning is emitted in both the block at the existing fallback (project_root = find_project_root(skill_dir)) and the similar block around lines 158-162.src/workflows/gds-quick-flow/gds-quick-dev/scripts/resolve-customization.py (1)
67-75: Preserve base menu fields when overriding bycode.Line 74 fully replaces matching menu items. If an override item is partial, default fields are dropped. Prefer deep-merging matching items by
code.Proposed refactor
def merge_menu(base: list[dict], override: list[dict]) -> list[dict]: """Merge-by-code: matching codes replace; new codes append.""" result_by_code: dict[str, dict] = {item["code"]: dict(item) for item in base if "code" in item} for item in override: if "code" not in item: print(f"warning: menu item missing 'code' key, skipping: {item}", file=sys.stderr) continue - result_by_code[item["code"]] = dict(item) + code = item["code"] + if code in result_by_code: + result_by_code[code] = deep_merge(result_by_code[code], dict(item)) + else: + result_by_code[code] = dict(item) return list(result_by_code.values())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/gds-quick-flow/gds-quick-dev/scripts/resolve-customization.py` around lines 67 - 75, The merge_menu function currently replaces a base item entirely when an override with the same "code" exists; change it to deep-merge matching items so base fields are preserved when overrides are partial: in merge_menu, when building result_by_code for base use the existing {item["code"]: dict(item)} approach, but when processing override items (in the for item in override loop) detect existing result_by_code[item["code"]] and perform a recursive/deep merge of dict values (and lists if needed) rather than assigning dict(item) directly; implement a helper like deep_merge_dicts(a: dict, b: dict) and call result_by_code[item["code"]] = deep_merge_dicts(result_by_code[item["code"]], dict(item)) so only provided override keys replace base keys and nested structures are merged.src/workflows/4-production/gds-code-review/scripts/resolve-customization.py (1)
148-152: Prefer resolving project root from the script location first.Using
Path.cwd()first at Line [148] can bind to an unrelated repo when the script is invoked from outside this project. Resolve fromskill_dirfirst, then optionally fall back tocwd.♻️ Proposed change
- project_root = find_project_root(Path.cwd()) - if project_root is None: - # Try from the skill directory as fallback - project_root = find_project_root(skill_dir) + # Prefer script-local discovery to avoid picking an unrelated cwd repository. + project_root = find_project_root(skill_dir) + if project_root is None: + project_root = find_project_root(Path.cwd())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/4-production/gds-code-review/scripts/resolve-customization.py` around lines 148 - 152, The code currently resolves project_root by calling find_project_root(Path.cwd()) first which can pick up an unrelated repo; change the order so you call find_project_root(skill_dir) first and assign to project_root, and only if that returns None call find_project_root(Path.cwd()) as a fallback; update the variables around project_root, find_project_root, Path.cwd(), and skill_dir accordingly so the script prefers the script's directory before the current working directory.src/workflows/1-preproduction/gds-brainstorm-game/scripts/resolve-customization.py (1)
148-152: Preferskill_diras the primary root-discovery anchor.Current order is cwd-first, so running from an unrelated git repo can resolve the wrong root and miss intended
_bmad/customizations.🔧 Proposed refactor
- project_root = find_project_root(Path.cwd()) - if project_root is None: - # Try from the skill directory as fallback - project_root = find_project_root(skill_dir) + project_root = find_project_root(skill_dir) + if project_root is None: + # Fallback for ad-hoc execution from outside the repo tree + project_root = find_project_root(Path.cwd())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/1-preproduction/gds-brainstorm-game/scripts/resolve-customization.py` around lines 148 - 152, The root discovery currently calls find_project_root(Path.cwd()) first which can pick up an unrelated repo; change the lookup order to call find_project_root(skill_dir) as the primary anchor and only call find_project_root(Path.cwd()) as the fallback if that returns None, updating the surrounding logic/comments accordingly; target the project_root variable and the two find_project_root calls (using skill_dir and Path.cwd()) to implement this swap.src/workflows/3-technical/gds-generate-project-context/scripts/resolve-customization.py (1)
130-132: Fail fast whenskill_namedoes not match this resolver’s skill directory.Line 145 always loads defaults from this script’s parent skill, but Line 130-Line 132 accepts any
skill_name. A typo can silently merge the wrong override file into the right defaults. Add a strict equality check to avoid hard-to-trace config drift.✅ Proposed guard
args = parser.parse_args() + expected_skill = skill_dir.name + if args.skill_name != expected_skill: + parser.error(f"skill_name must be '{expected_skill}' for this resolver")Also applies to: 143-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/3-technical/gds-generate-project-context/scripts/resolve-customization.py` around lines 130 - 132, Add a strict equality guard to reject mismatched skill names so that the CLI arg "skill_name" must equal this resolver's parent skill directory before loading defaults; specifically, in the code that parses the "skill_name" argument and before the block that loads defaults from this script’s parent skill, compare the provided skill_name to the resolver’s own skill id (derived from the parent directory or existing constant) and raise/exit with an error if they differ to prevent silent merging of wrong override files.src/agents/gds-agent-tech-writer/scripts/resolve-customization.py (1)
130-132: Validateskill_nameagainst the resolver’s owning skill directory.Line 130 and Line 160 allow accidental cross-skill merges (defaults from this skill + overrides from another skill name). Add a mismatch guard to prevent silently producing inconsistent configs.
💡 Proposed fix
@@ - args = parser.parse_args() - # Locate the skill's own customize.toml (one level up from scripts/) script_dir = Path(__file__).resolve().parent skill_dir = script_dir.parent defaults_path = skill_dir / "customize.toml" + args = parser.parse_args() + if args.skill_name != skill_dir.name: + parser.error( + f"skill_name '{args.skill_name}' does not match resolver skill '{skill_dir.name}'" + )Also applies to: 160-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/gds-agent-tech-writer/scripts/resolve-customization.py` around lines 130 - 132, The CLI accepts a skill_name argument but does not guard against mixing defaults from the resolver's owning skill directory with overrides from a different skill; add a validation step that compares the parsed skill_name argument to the resolver's owning skill identifier (e.g., the resolver's skill_dir or owning_skill variable) and aborts with a clear error if they differ to prevent cross-skill merges. Insert this check immediately after parsing args (where "skill_name" is read) and also before applying overrides/merging configs (the code path that combines defaults and overrides) so both code paths reject mismatched skill names. Ensure the error returns non-zero/raises SystemExit with a descriptive message mentioning both names.
🤖 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/agents/gds-agent-game-designer/scripts/resolve-customization.py`:
- Around line 130-132: The code currently interpolates args.skill_name directly
into customization paths, enabling path traversal; add validation immediately
after parsing (where args.skill_name is used) to reject any value that contains
path separators or unsafe characters (allow only a strict whitelist like
alphanumerics, hyphen and underscore, e.g. regex ^[A-Za-z0-9_-]+$) and
return/exit with an error if it fails; additionally, when building the override
file paths for _bmad/customizations, construct paths with os.path.join and then
resolve them with os.path.realpath and verify the resolved path starts with the
resolved base directory (realpath of _bmad/customizations) to enforce no
escaping. Ensure you reference args.skill_name and the code that builds the
customization path (the path interpolation around "_bmad/customizations" and any
functions that read the TOML) when applying these checks.
In `@src/agents/gds-agent-game-qa/scripts/resolve-customization.py`:
- Around line 58-64: The current strict guard in _is_menu_array causes the
menu-merge to skip merge-by-code when any override item is malformed, leading to
full-array replacement and loss of defaults; update the merge logic so that if
the base/menu array items all have "code" and the override array contains at
least one item with a "code", you perform a merge-by-code for those override
items that include "code" (overwriting matching default entries) while
preserving default entries for missing/unknown override codes, and emit a
warning for each malformed override item (those missing "code") instead of
falling back to full replacement; keep _is_menu_array as a precise predicate for
“all items are valid” but change the calling merge routine to support
partial-code merges and warnings.
In `@src/agents/gds-agent-game-solo-dev/scripts/resolve-customization.py`:
- Around line 160-161: The code constructs file paths using raw CLI input
args.skill_name when calling load_toml for team and user files, which allows
path traversal; validate or sanitize args.skill_name before building paths:
ensure it contains only allowed characters (e.g., alphanumeric, hyphen,
underscore) and rejects any path separators or "..", or alternatively normalize
the candidate with Path(args.skill_name).name or resolve the joined path and
verify it is a descendant of customizations_dir; apply this check before using
args.skill_name in the calls to load_toml so team and user file paths cannot
escape _bmad/customizations.
In `@src/agents/gds-agent-tech-writer/scripts/resolve-customization.py`:
- Around line 174-176: The JSON dumps of `result` and `merged` can raise
TypeError for TOML-parsed temporal objects; add a custom JSON serializer and
pass it to `json.dump` (e.g., use a helper like `json_default` that checks for
`datetime.date`, `datetime.time`, `datetime.datetime` and returns
`isoformat()`), then call `json.dump(result, sys.stdout, ...,
default=json_default)` and `json.dump(merged, sys.stdout, ...,
default=json_default)` so temporal types are converted to ISO strings before
dumping; add the `json_default` helper near the top of
`resolve-customization.py` (or next to the current dump calls) and import
`datetime` as needed.
In
`@src/workflows/1-preproduction/gds-brainstorm-game/scripts/resolve-customization.py`:
- Around line 58-64: The current predicate _is_menu_array only returns True when
every item is a dict with a "code", which causes malformed overrides to trigger
atomic replacement; change _is_menu_array to return True for any non-empty list
that contains at least one dict item with a "code" (e.g., use
any(isinstance(item, dict) and "code" in item for item in value) rather than
all(...)), so that deep_merge will treat it as a menu array and let merge_menu
handle validation/cleanup of malformed items; apply the same change to the other
analogous predicates/uses of this check found near the other menu checks so all
menu-detection logic is consistent.
In `@src/workflows/2-design/gds-create-gdd/scripts/resolve-customization.py`:
- Around line 159-161: The code builds TOML file paths by interpolating
args.skill_name into customizations_dir and calling load_toml
(customizations_dir / f"{args.skill_name}.toml" and .user.toml), which allows
path traversal; validate or sanitize args.skill_name before use (e.g., allow
only a strict whitelist regex like [A-Za-z0-9_-]+ or normalize then resolve the
resulting path and verify it is a child of customizations_dir), and if
validation fails raise an error; update the code that constructs paths and calls
load_toml (referencing customizations_dir, args.skill_name, and load_toml) to
perform this check so no file outside the _bmad/customizations directory can be
opened.
In
`@src/workflows/2-design/gds-create-ux-design/scripts/resolve-customization.py`:
- Around line 42-51: The current load_toml function swallows parse/IO errors and
returns {} which masks a missing/invalid defaults TOML; change load_toml to
accept a fail_on_error: bool (default False) and when fail_on_error is True
re-raise the caught exception (or raise a new RuntimeError) instead of returning
{} after logging; then call load_toml(..., fail_on_error=True) where the
defaults file is loaded so the process fails fast on unreadable/invalid
defaults.
- Around line 93-94: The current branch calls merge_menu whenever both values
are lists of dicts with codes, which can wrongly merge non-menu arrays; update
the conditional in the merge logic (the branch using _is_menu_array(over_val)
and _is_menu_array(base_val) that calls merge_menu) to only call merge_menu when
the current key is exactly "menu" (i.e., key == "menu") and otherwise fall back
to the existing override/replace behavior; adjust the condition so
merge_menu(base_val, over_val) is invoked only for key "menu" while other
list-of-dict cases are not merged-by-code.
In
`@src/workflows/3-technical/gds-game-architecture/scripts/resolve-customization.py`:
- Around line 130-132: The CLI argument skill_name is used directly to build
filesystem paths and can enable path traversal; validate and sanitize it before
use: in the argument handling code that reads the skill_name variable (the
parser add_argument named "skill_name" and the code that constructs paths around
lines where files under "_bmad/customizations" are accessed), reject values
containing path separators or traversal sequences and allow only a safe
whitelist (e.g. a regex like ^[a-z0-9_-]+$), or alternatively normalize/join
with os.path.normpath and ensure the resulting path remains inside the base
directory (compare os.path.commonpath or commonprefix with the base dir). Return
a clear error/exit when validation fails so subsequent path joins (the code that
uses skill_name to read files) cannot escape the intended directory.
- Around line 42-51: The current load_toml function swallows missing/invalid
files and returns {} which is fine for optional overrides but unsafe for
required defaults like customize.toml; change load_toml to accept a required:
bool parameter (default False) and if required is True then raise an exception
(or re-raise the decoding/IO error) when the file is missing or fails to parse
instead of printing a warning and returning {}; update callers that load the
required defaults (the caller that reads customize.toml—see the usage around
lines referenced in the review) to call load_toml(required=True) while leaving
other callers as optional (required=False).
- Around line 148-151: The project root lookup currently calls
find_project_root(Path.cwd()) first which can pick up the wrong repository when
the script is invoked outside the skill folder; change the resolution order to
call find_project_root(skill_dir) first and only if that returns None call
find_project_root(Path.cwd()), updating the logic that sets project_root
(references: find_project_root, project_root, skill_dir, Path.cwd()) so the
script prefers the script's skill directory as the primary root and falls back
to the current working directory.
In
`@src/workflows/3-technical/gds-generate-project-context/scripts/resolve-customization.py`:
- Around line 58-64: The predicate _is_menu_array is too strict—requiring every
item to have "code" causes deep_merge to skip the menu-merge path when some
overrides lack codes; change _is_menu_array to return True when value is a
non-empty list of dicts and at least one item contains the "code" key (so we
still recognize menus even if some entries are malformed), and update callers
(merge_menu and deep_merge) to use this revised predicate so the warning/merge
logic for missing codes runs instead of doing an atomic replace.
- Around line 158-161: The code interpolates args.skill_name directly into paths
when calling load_toml (customizations_dir / f"{args.skill_name}.toml" and
f"{args.skill_name}.user.toml"), allowing path traversal; validate or sanitize
args.skill_name before use (e.g., restrict to a safe pattern such as
/^[A-Za-z0-9_-]+$/ or otherwise canonicalize and ensure the resolved path is
inside customizations_dir) and reject or escape invalid values so load_toml only
opens files under the intended customizations_dir; implement this check right
after args are parsed and before building the paths.
In `@src/workflows/4-production/gds-code-review/scripts/resolve-customization.py`:
- Around line 129-132: Validate the skill_name argument (declared with
parser.add_argument("skill_name")) before using it to load overrides: ensure it
matches an allowed pattern (e.g. only lowercase letters, numbers and hyphens via
regex r'^[a-z0-9-]+$') and/or is present in the current workflow's known skill
list, and abort with a clear error if it doesn't; then use the validated
skill_name when building paths for loading customize.toml and override files so
a typo cannot cause cross-skill file loading (adjust the load logic that
references customize.toml and the override file paths to use the validated
value).
In
`@src/workflows/4-production/gds-sprint-status/scripts/resolve-customization.py`:
- Around line 129-132: The code uses the user-supplied variable skill_name (from
parser.add_argument) directly in filesystem path construction, allowing values
with path separators or '..' to escape _bmad/customizations; fix by
validating/sanitizing skill_name before any path joining: implement an argparse
type or post-parse check that rejects values containing os.path.sep,
os.path.altsep, or any '..' segments (or require a strict whitelist regex such
as r'^[a-z0-9-]+$'), or normalize with os.path.normpath and assert the
normalized name equals the original basename, and on failure call parser.error
or raise argparse.ArgumentTypeError; apply the same validation where skill_name
is later used to build paths (the uses at the parser.add_argument site and the
later path construction).
In `@src/workflows/gametest/gds-e2e-scaffold/scripts/resolve-customization.py`:
- Around line 174-176: The JSON dump will fail if TOML-parsed data (from
tomllib) contains datetime.date/time/datetime objects; update the serialization
before calling json.dump for both `result` and `merged` by converting any
datetime/date/time values to ISO-8601 strings (or otherwise JSON-serializable
types). Implement a small sanitizer that recursively walks the parsed data
structure and replaces instances of `datetime.date`, `datetime.time`, and
`datetime.datetime` with `.isoformat()` strings, then pass the sanitized object
to `json.dump` where `result` or `merged` are used.
In `@src/workflows/gds-quick-flow/gds-quick-dev/scripts/resolve-customization.py`:
- Around line 42-51: The load_toml function currently swallows parsing/read
errors and returns {} which lets execution continue; change it so that if the
file exists but tomllib.TOMLDecodeError or OSError occurs it fails fast: either
re-raise the exception or call sys.exit(1) with a descriptive error message
instead of returning an empty dict. Update load_toml (the function that opens
and calls tomllib.load) to distinguish "file not found" (keep returning {}) from
"file present but unreadable/corrupt" (exit non-zero or raise), and ensure the
error message includes the path and exception details.
- Around line 130-132: Validate args.skill_name before using it to build file
paths: reject any values containing path separators or traversal tokens (e.g.,
'/', '\', '..') and only allow a safe whitelist (e.g., letters, numbers, hyphen,
underscore). Update the argument handling in resolve-customization.py (the
parser where "skill_name" is defined and any code that reads files using
args.skill_name for filenames like the ones constructed around line 160/161) to
perform this check and raise/exit with a clear error if validation fails so the
unsafe value is never interpolated into file paths.
---
Minor comments:
In `@src/agents/gds-agent-game-qa/scripts/resolve-customization.py`:
- Around line 129-132: Inputs in parser.add_argument are used verbatim
(args.skill_name) to build filenames under the _bmad/customizations directory,
allowing path traversal (e.g. ../../). Validate and sanitize args.skill_name
before using it: reject any value containing path separators or suspicious
sequences and enforce a strict whitelist pattern (e.g. ^[A-Za-z0-9_-]+$), or map
skill names to known allowed entries; when building the file path use
pathlib.Path to join and resolve (Path(custom_dir) / args.skill_name).resolve()
and ensure the resolved path is a child of the customizations directory (compare
.resolve().is_relative_to(custom_dir.resolve()) or check prefix), and apply this
validation wherever args.skill_name is interpolated into filenames (references:
parser.add_argument, args.skill_name, the _bmad/customizations file operations).
In `@src/agents/gds-agent-game-solo-dev/scripts/resolve-customization.py`:
- Around line 148-152: The current lookup prefers find_project_root(Path.cwd())
which can pick up a different repo and miss this skill's customization; change
the resolution to prefer the skill-local root first by calling
find_project_root(skill_dir) and using that if present, then fall back to
find_project_root(Path.cwd()); update the project_root assignment and any
downstream logic that uses project_root (references: find_project_root,
project_root, Path.cwd(), skill_dir) so the skill's local BMAD project is
selected when available.
---
Nitpick comments:
In `@src/agents/gds-agent-game-designer/scripts/resolve-customization.py`:
- Around line 149-152: The code currently falls back to project_root =
find_project_root(skill_dir) and silently skips overrides if project_root
remains None; update the logic in resolve-customization.py so that after both
lookups (the initial project_root check and the fallback using
find_project_root(skill_dir)) you detect when project_root is still None and
emit a clear warning to stderr (e.g., using sys.stderr.write or logging.warning)
indicating that project root could not be resolved and overrides will be
skipped; reference the project_root variable, the find_project_root call, and
the overrides handling branch so the warning is emitted in both the block at the
existing fallback (project_root = find_project_root(skill_dir)) and the similar
block around lines 158-162.
In `@src/agents/gds-agent-tech-writer/scripts/resolve-customization.py`:
- Around line 130-132: The CLI accepts a skill_name argument but does not guard
against mixing defaults from the resolver's owning skill directory with
overrides from a different skill; add a validation step that compares the parsed
skill_name argument to the resolver's owning skill identifier (e.g., the
resolver's skill_dir or owning_skill variable) and aborts with a clear error if
they differ to prevent cross-skill merges. Insert this check immediately after
parsing args (where "skill_name" is read) and also before applying
overrides/merging configs (the code path that combines defaults and overrides)
so both code paths reject mismatched skill names. Ensure the error returns
non-zero/raises SystemExit with a descriptive message mentioning both names.
In
`@src/workflows/1-preproduction/gds-brainstorm-game/scripts/resolve-customization.py`:
- Around line 148-152: The root discovery currently calls
find_project_root(Path.cwd()) first which can pick up an unrelated repo; change
the lookup order to call find_project_root(skill_dir) as the primary anchor and
only call find_project_root(Path.cwd()) as the fallback if that returns None,
updating the surrounding logic/comments accordingly; target the project_root
variable and the two find_project_root calls (using skill_dir and Path.cwd()) to
implement this swap.
In
`@src/workflows/3-technical/gds-generate-project-context/scripts/resolve-customization.py`:
- Around line 130-132: Add a strict equality guard to reject mismatched skill
names so that the CLI arg "skill_name" must equal this resolver's parent skill
directory before loading defaults; specifically, in the code that parses the
"skill_name" argument and before the block that loads defaults from this
script’s parent skill, compare the provided skill_name to the resolver’s own
skill id (derived from the parent directory or existing constant) and raise/exit
with an error if they differ to prevent silent merging of wrong override files.
In `@src/workflows/4-production/gds-code-review/scripts/resolve-customization.py`:
- Around line 148-152: The code currently resolves project_root by calling
find_project_root(Path.cwd()) first which can pick up an unrelated repo; change
the order so you call find_project_root(skill_dir) first and assign to
project_root, and only if that returns None call find_project_root(Path.cwd())
as a fallback; update the variables around project_root, find_project_root,
Path.cwd(), and skill_dir accordingly so the script prefers the script's
directory before the current working directory.
In
`@src/workflows/4-production/gds-sprint-status/scripts/resolve-customization.py`:
- Around line 42-51: Introduce a --strict mode that makes TOML load failures
fatal: add parser.add_argument("--strict", action="store_true", help="Fail on
TOML parse/read errors.") and thread the resulting flag into each load_toml(...)
call, then modify load_toml(path: Path, strict: bool = False) to keep current
behavior (return {}) on non-strict errors but, when strict is True, log the
error and exit non-zero (sys.exit(1)) or re-raise the exception instead of
returning {}; update all call sites to pass strict=args.strict.
In `@src/workflows/gds-quick-flow/gds-quick-dev/scripts/resolve-customization.py`:
- Around line 67-75: The merge_menu function currently replaces a base item
entirely when an override with the same "code" exists; change it to deep-merge
matching items so base fields are preserved when overrides are partial: in
merge_menu, when building result_by_code for base use the existing
{item["code"]: dict(item)} approach, but when processing override items (in the
for item in override loop) detect existing result_by_code[item["code"]] and
perform a recursive/deep merge of dict values (and lists if needed) rather than
assigning dict(item) directly; implement a helper like deep_merge_dicts(a: dict,
b: dict) and call result_by_code[item["code"]] =
deep_merge_dicts(result_by_code[item["code"]], dict(item)) so only provided
override keys replace base keys and nested structures are merged.
🪄 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: 18092854-4718-45b8-9c08-5ba8f605340b
📒 Files selected for processing (64)
README.mdsrc/agents/gds-agent-game-architect/scripts/resolve-customization.pysrc/agents/gds-agent-game-designer/scripts/resolve-customization.pysrc/agents/gds-agent-game-dev/scripts/resolve-customization.pysrc/agents/gds-agent-game-qa/scripts/resolve-customization.pysrc/agents/gds-agent-game-scrum-master/scripts/resolve-customization.pysrc/agents/gds-agent-game-solo-dev/scripts/resolve-customization.pysrc/agents/gds-agent-tech-writer/scripts/resolve-customization.pysrc/workflows/1-preproduction/gds-brainstorm-game/SKILL.mdsrc/workflows/1-preproduction/gds-brainstorm-game/scripts/resolve-customization.pysrc/workflows/1-preproduction/gds-create-game-brief/SKILL.mdsrc/workflows/1-preproduction/gds-create-game-brief/scripts/resolve-customization.pysrc/workflows/1-preproduction/research/gds-domain-research/SKILL.mdsrc/workflows/1-preproduction/research/gds-domain-research/scripts/resolve-customization.pysrc/workflows/2-design/gds-create-gdd/SKILL.mdsrc/workflows/2-design/gds-create-gdd/scripts/resolve-customization.pysrc/workflows/2-design/gds-create-narrative/SKILL.mdsrc/workflows/2-design/gds-create-narrative/scripts/resolve-customization.pysrc/workflows/2-design/gds-create-ux-design/SKILL.mdsrc/workflows/2-design/gds-create-ux-design/scripts/resolve-customization.pysrc/workflows/3-technical/gds-check-implementation-readiness/SKILL.mdsrc/workflows/3-technical/gds-check-implementation-readiness/scripts/resolve-customization.pysrc/workflows/3-technical/gds-create-epics-and-stories/SKILL.mdsrc/workflows/3-technical/gds-create-epics-and-stories/scripts/resolve-customization.pysrc/workflows/3-technical/gds-game-architecture/SKILL.mdsrc/workflows/3-technical/gds-game-architecture/scripts/resolve-customization.pysrc/workflows/3-technical/gds-generate-project-context/SKILL.mdsrc/workflows/3-technical/gds-generate-project-context/scripts/resolve-customization.pysrc/workflows/4-production/gds-code-review/SKILL.mdsrc/workflows/4-production/gds-code-review/scripts/resolve-customization.pysrc/workflows/4-production/gds-correct-course/SKILL.mdsrc/workflows/4-production/gds-correct-course/scripts/resolve-customization.pysrc/workflows/4-production/gds-create-story/SKILL.mdsrc/workflows/4-production/gds-create-story/scripts/resolve-customization.pysrc/workflows/4-production/gds-dev-story/SKILL.mdsrc/workflows/4-production/gds-dev-story/scripts/resolve-customization.pysrc/workflows/4-production/gds-retrospective/SKILL.mdsrc/workflows/4-production/gds-retrospective/scripts/resolve-customization.pysrc/workflows/4-production/gds-sprint-planning/SKILL.mdsrc/workflows/4-production/gds-sprint-planning/scripts/resolve-customization.pysrc/workflows/4-production/gds-sprint-status/SKILL.mdsrc/workflows/4-production/gds-sprint-status/scripts/resolve-customization.pysrc/workflows/gametest/gds-e2e-scaffold/SKILL.mdsrc/workflows/gametest/gds-e2e-scaffold/scripts/resolve-customization.pysrc/workflows/gametest/gds-performance-test/SKILL.mdsrc/workflows/gametest/gds-performance-test/scripts/resolve-customization.pysrc/workflows/gametest/gds-playtest-plan/SKILL.mdsrc/workflows/gametest/gds-playtest-plan/scripts/resolve-customization.pysrc/workflows/gametest/gds-test-automate/SKILL.mdsrc/workflows/gametest/gds-test-automate/scripts/resolve-customization.pysrc/workflows/gametest/gds-test-design/SKILL.mdsrc/workflows/gametest/gds-test-design/scripts/resolve-customization.pysrc/workflows/gametest/gds-test-framework/SKILL.mdsrc/workflows/gametest/gds-test-framework/scripts/resolve-customization.pysrc/workflows/gametest/gds-test-review/SKILL.mdsrc/workflows/gametest/gds-test-review/scripts/resolve-customization.pysrc/workflows/gds-document-project/SKILL.mdsrc/workflows/gds-document-project/scripts/resolve-customization.pysrc/workflows/gds-quick-flow/gds-quick-dev-new-preview/SKILL.mdsrc/workflows/gds-quick-flow/gds-quick-dev-new-preview/scripts/resolve-customization.pysrc/workflows/gds-quick-flow/gds-quick-dev/SKILL.mdsrc/workflows/gds-quick-flow/gds-quick-dev/scripts/resolve-customization.pysrc/workflows/gds-quick-flow/gds-quick-spec/SKILL.mdsrc/workflows/gds-quick-flow/gds-quick-spec/scripts/resolve-customization.py
✅ Files skipped from review due to trivial changes (32)
- README.md
- src/workflows/4-production/gds-create-story/SKILL.md
- src/workflows/3-technical/gds-generate-project-context/SKILL.md
- src/workflows/1-preproduction/gds-create-game-brief/SKILL.md
- src/workflows/4-production/gds-dev-story/SKILL.md
- src/workflows/2-design/gds-create-gdd/SKILL.md
- src/workflows/gametest/gds-test-automate/SKILL.md
- src/workflows/2-design/gds-create-ux-design/SKILL.md
- src/workflows/gametest/gds-playtest-plan/SKILL.md
- src/workflows/2-design/gds-create-narrative/SKILL.md
- src/workflows/gds-quick-flow/gds-quick-dev-new-preview/SKILL.md
- src/workflows/3-technical/gds-create-epics-and-stories/SKILL.md
- src/workflows/gds-document-project/SKILL.md
- src/workflows/gametest/gds-performance-test/SKILL.md
- src/workflows/gds-quick-flow/gds-quick-spec/SKILL.md
- src/workflows/gametest/gds-e2e-scaffold/SKILL.md
- src/workflows/1-preproduction/research/gds-domain-research/SKILL.md
- src/workflows/4-production/gds-correct-course/SKILL.md
- src/workflows/4-production/gds-sprint-status/SKILL.md
- src/workflows/4-production/gds-code-review/SKILL.md
- src/workflows/4-production/gds-sprint-planning/SKILL.md
- src/workflows/gametest/gds-test-review/SKILL.md
- src/workflows/gametest/gds-test-design/SKILL.md
- src/workflows/gametest/gds-performance-test/scripts/resolve-customization.py
- src/workflows/4-production/gds-dev-story/scripts/resolve-customization.py
- src/workflows/4-production/gds-sprint-planning/scripts/resolve-customization.py
- src/workflows/4-production/gds-retrospective/scripts/resolve-customization.py
- src/workflows/gametest/gds-test-review/scripts/resolve-customization.py
- src/agents/gds-agent-game-architect/scripts/resolve-customization.py
- src/workflows/gametest/gds-test-framework/scripts/resolve-customization.py
- src/workflows/gds-quick-flow/gds-quick-dev-new-preview/scripts/resolve-customization.py
- src/workflows/3-technical/gds-check-implementation-readiness/scripts/resolve-customization.py
🚧 Files skipped from review as they are similar to previous changes (18)
- src/workflows/gds-quick-flow/gds-quick-dev/SKILL.md
- src/workflows/gametest/gds-test-framework/SKILL.md
- src/workflows/4-production/gds-retrospective/SKILL.md
- src/workflows/1-preproduction/gds-brainstorm-game/SKILL.md
- src/workflows/3-technical/gds-game-architecture/SKILL.md
- src/workflows/gametest/gds-test-design/scripts/resolve-customization.py
- src/workflows/3-technical/gds-check-implementation-readiness/SKILL.md
- src/workflows/gds-document-project/scripts/resolve-customization.py
- src/agents/gds-agent-game-dev/scripts/resolve-customization.py
- src/workflows/1-preproduction/research/gds-domain-research/scripts/resolve-customization.py
- src/workflows/2-design/gds-create-narrative/scripts/resolve-customization.py
- src/workflows/3-technical/gds-create-epics-and-stories/scripts/resolve-customization.py
- src/workflows/gds-quick-flow/gds-quick-spec/scripts/resolve-customization.py
- src/workflows/1-preproduction/gds-create-game-brief/scripts/resolve-customization.py
- src/workflows/4-production/gds-create-story/scripts/resolve-customization.py
- src/workflows/4-production/gds-correct-course/scripts/resolve-customization.py
- src/workflows/gametest/gds-playtest-plan/scripts/resolve-customization.py
- src/workflows/gametest/gds-test-automate/scripts/resolve-customization.py
| "skill_name", | ||
| help="Skill identifier (e.g. bmad-agent-pm, bmad-product-brief)", | ||
| ) |
There was a problem hiding this comment.
Validate skill_name before building override file paths.
At Line 160 and Line 161, args.skill_name is interpolated directly into paths. A crafted value containing path separators (e.g., ../) can escape _bmad/customizations and read unintended TOML files.
🔐 Proposed hardening
@@
import argparse
import json
+import re
import sys
import tomllib
@@
args = parser.parse_args()
+ if not re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9._-]*", args.skill_name):
+ parser.error("skill_name may only contain letters, digits, dot, underscore, and hyphen")
@@
if project_root is not None:
customizations_dir = project_root / "_bmad" / "customizations"
team = load_toml(customizations_dir / f"{args.skill_name}.toml")
user = load_toml(customizations_dir / f"{args.skill_name}.user.toml")Also applies to: 159-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/agents/gds-agent-game-designer/scripts/resolve-customization.py` around
lines 130 - 132, The code currently interpolates args.skill_name directly into
customization paths, enabling path traversal; add validation immediately after
parsing (where args.skill_name is used) to reject any value that contains path
separators or unsafe characters (allow only a strict whitelist like
alphanumerics, hyphen and underscore, e.g. regex ^[A-Za-z0-9_-]+$) and
return/exit with an error if it fails; additionally, when building the override
file paths for _bmad/customizations, construct paths with os.path.join and then
resolve them with os.path.realpath and verify the resolved path starts with the
resolved base directory (realpath of _bmad/customizations) to enforce no
escaping. Ensure you reference args.skill_name and the code that builds the
customization path (the path interpolation around "_bmad/customizations" and any
functions that read the TOML) when applying these checks.
| def _is_menu_array(value: Any) -> bool: | ||
| """True when *value* is a non-empty list where ALL items are dicts with a ``code`` key.""" | ||
| return ( | ||
| isinstance(value, list) | ||
| and len(value) > 0 | ||
| and all(isinstance(item, dict) and "code" in item for item in value) | ||
| ) |
There was a problem hiding this comment.
menu merge can silently downgrade to full replacement on malformed override entries.
At Line 93, merge-by-code only runs when all items in both arrays have code. If one override item is malformed, merge is skipped and Line 96 replaces the whole array, which can drop default menu entries. Also, the warning path at Line 71 is mostly bypassed by the strict guard.
Proposed fix
def merge_menu(base: list[dict], override: list[dict]) -> list[dict]:
"""Merge-by-code: matching codes replace; new codes append."""
- result_by_code: dict[str, dict] = {item["code"]: dict(item) for item in base if "code" in item}
+ result_by_code: dict[str, dict] = {
+ str(item["code"]): dict(item)
+ for item in base
+ if isinstance(item, dict) and "code" in item
+ }
for item in override:
- if "code" not in item:
+ if not isinstance(item, dict) or "code" not in item:
print(f"warning: menu item missing 'code' key, skipping: {item}", file=sys.stderr)
continue
- result_by_code[item["code"]] = dict(item)
+ result_by_code[str(item["code"])] = dict(item)
return list(result_by_code.values())
@@
- elif _is_menu_array(over_val) and _is_menu_array(base_val):
- merged[key] = merge_menu(base_val, over_val) # type: ignore[arg-type]
+ elif key == "menu" and isinstance(base_val, list) and isinstance(over_val, list):
+ merged[key] = [] if len(over_val) == 0 else merge_menu(base_val, over_val)Also applies to: 67-75, 93-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/agents/gds-agent-game-qa/scripts/resolve-customization.py` around lines
58 - 64, The current strict guard in _is_menu_array causes the menu-merge to
skip merge-by-code when any override item is malformed, leading to full-array
replacement and loss of defaults; update the merge logic so that if the
base/menu array items all have "code" and the override array contains at least
one item with a "code", you perform a merge-by-code for those override items
that include "code" (overwriting matching default entries) while preserving
default entries for missing/unknown override codes, and emit a warning for each
malformed override item (those missing "code") instead of falling back to full
replacement; keep _is_menu_array as a precise predicate for “all items are
valid” but change the calling merge routine to support partial-code merges and
warnings.
| def _is_menu_array(value: Any) -> bool: | ||
| """True when *value* is a non-empty list where ALL items are dicts with a ``code`` key.""" | ||
| return ( | ||
| isinstance(value, list) | ||
| and len(value) > 0 | ||
| and all(isinstance(item, dict) and "code" in item for item in value) | ||
| ) |
There was a problem hiding this comment.
Menu merge guard drops into full replacement when one override item is invalid.
At Line 58–64, _is_menu_array requires all items to have code. At Line 93–96, that means one bad override item bypasses merge_menu and replaces the full menu, instead of skipping only invalid items. This can unintentionally discard base/team menu entries.
Suggested fix
-def _is_menu_array(value: Any) -> bool:
- """True when *value* is a non-empty list where ALL items are dicts with a ``code`` key."""
- return (
- isinstance(value, list)
- and len(value) > 0
- and all(isinstance(item, dict) and "code" in item for item in value)
- )
+def _is_menu_array(value: Any) -> bool:
+ """True when *value* is a list of TOML tables (dicts)."""
+ return isinstance(value, list) and all(isinstance(item, dict) for item in value)
@@
- elif _is_menu_array(over_val) and _is_menu_array(base_val):
+ elif key == "menu" and _is_menu_array(base_val) and _is_menu_array(over_val):
merged[key] = merge_menu(base_val, over_val) # type: ignore[arg-type]Also applies to: 93-96
| team = load_toml(customizations_dir / f"{args.skill_name}.toml") | ||
| user = load_toml(customizations_dir / f"{args.skill_name}.user.toml") |
There was a problem hiding this comment.
Validate skill_name before building filesystem paths.
Lines 160-161 use raw CLI input in path construction. A crafted value like ../../foo can escape _bmad/customizations and read unintended TOML files.
Suggested fix
import argparse
import json
+import re
import sys
import tomllib
@@
args = parser.parse_args()
+ if not re.fullmatch(r"[a-z0-9][a-z0-9-]*", args.skill_name):
+ parser.error("skill_name must contain only lowercase letters, numbers, and hyphens")
@@
if project_root is not None:
customizations_dir = project_root / "_bmad" / "customizations"
- team = load_toml(customizations_dir / f"{args.skill_name}.toml")
- user = load_toml(customizations_dir / f"{args.skill_name}.user.toml")
+ base_dir = customizations_dir.resolve()
+ team_path = (customizations_dir / f"{args.skill_name}.toml").resolve()
+ user_path = (customizations_dir / f"{args.skill_name}.user.toml").resolve()
+ if not team_path.is_relative_to(base_dir) or not user_path.is_relative_to(base_dir):
+ parser.error("invalid skill_name path")
+ team = load_toml(team_path)
+ user = load_toml(user_path)📝 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.
| team = load_toml(customizations_dir / f"{args.skill_name}.toml") | |
| user = load_toml(customizations_dir / f"{args.skill_name}.user.toml") | |
| base_dir = customizations_dir.resolve() | |
| team_path = (customizations_dir / f"{args.skill_name}.toml").resolve() | |
| user_path = (customizations_dir / f"{args.skill_name}.user.toml").resolve() | |
| if not team_path.is_relative_to(base_dir) or not user_path.is_relative_to(base_dir): | |
| parser.error("invalid skill_name path") | |
| team = load_toml(team_path) | |
| user = load_toml(user_path) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/agents/gds-agent-game-solo-dev/scripts/resolve-customization.py` around
lines 160 - 161, The code constructs file paths using raw CLI input
args.skill_name when calling load_toml for team and user files, which allows
path traversal; validate or sanitize args.skill_name before building paths:
ensure it contains only allowed characters (e.g., alphanumeric, hyphen,
underscore) and rejects any path separators or "..", or alternatively normalize
the candidate with Path(args.skill_name).name or resolve the joined path and
verify it is a descendant of customizations_dir; apply this check before using
args.skill_name in the calls to load_toml so team and user file paths cannot
escape _bmad/customizations.
| json.dump(result, sys.stdout, indent=2, ensure_ascii=False) | ||
| else: | ||
| json.dump(merged, sys.stdout, indent=2, ensure_ascii=False) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verifies stdlib behavior: TOML date parses, then json.dumps fails without conversion.
python - <<'PY'
import tomllib, json
data = tomllib.loads("released = 2026-04-15")
print(type(data["released"]).__name__) # expected: date
try:
json.dumps(data)
print("UNEXPECTED: serialization succeeded")
except TypeError as e:
print("EXPECTED TypeError:", e)
PYRepository: bmad-code-org/bmad-module-game-dev-studio
Length of output: 154
🏁 Script executed:
# Verify the file exists and check its content at the specified lines
find . -path "*src/agents/gds-agent-tech-writer/scripts/resolve-customization.py" -type fRepository: bmad-code-org/bmad-module-game-dev-studio
Length of output: 152
🏁 Script executed:
# If file exists, check lines around 174-176
if [ -f "src/agents/gds-agent-tech-writer/scripts/resolve-customization.py" ]; then
echo "=== File content around lines 174-176 ==="
sed -n '170,180p' "src/agents/gds-agent-tech-writer/scripts/resolve-customization.py"
echo ""
echo "=== Checking for tomllib import ==="
grep -n "import tomllib\|from tomllib" "src/agents/gds-agent-tech-writer/scripts/resolve-customization.py" || echo "No tomllib import found"
else
echo "File not found at expected path"
# Search for the file anywhere in the repo
find . -name "resolve-customization.py" -type f 2>/dev/null
fiRepository: bmad-code-org/bmad-module-game-dev-studio
Length of output: 542
🏁 Script executed:
# Check for TOML files and their content for date/time values
find . -name "*.toml" -type f | head -20 | while read toml_file; do
if grep -E '^\s*\w+\s*=\s*[0-9]{4}-[0-9]{2}-[0-9]{2}' "$toml_file" > /dev/null 2>&1; then
echo "=== $toml_file has date values ==="
grep -E '^\s*\w+\s*=\s*[0-9]{4}-[0-9]{2}-[0-9]{2}' "$toml_file"
fi
doneRepository: bmad-code-org/bmad-module-game-dev-studio
Length of output: 67
🏁 Script executed:
# Search for all TOML files more broadly
find . -name "*.toml" -type f 2>/dev/null | head -30Repository: bmad-code-org/bmad-module-game-dev-studio
Length of output: 1895
🏁 Script executed:
# Check the full content of the resolve-customization.py file to understand data flow
wc -l "src/agents/gds-agent-tech-writer/scripts/resolve-customization.py"Repository: bmad-code-org/bmad-module-game-dev-studio
Length of output: 154
🏁 Script executed:
# Get the load_toml and deep_merge functions to understand data flow
sed -n '1,100p' "src/agents/gds-agent-tech-writer/scripts/resolve-customization.py" | head -60Repository: bmad-code-org/bmad-module-game-dev-studio
Length of output: 2071
🏁 Script executed:
# Check all TOML files for date/time values
for toml_file in ./src/agents/gds-agent-game-architect/customize.toml \
./src/agents/gds-agent-tech-writer/customize.toml \
./src/workflows/2-design/gds-create-gdd/customize.toml; do
if [ -f "$toml_file" ]; then
echo "=== $toml_file ==="
cat "$toml_file" | head -50
echo ""
fi
doneRepository: bmad-code-org/bmad-module-game-dev-studio
Length of output: 6525
🏁 Script executed:
# Search all TOML files for patterns that look like dates or times
grep -rn '\= *[0-9]\{4\}-[0-9]\{2\}-[0-9]\{2\}\|[0-9]\{2\}:[0-9]\{2\}:[0-9]\{2\}' ./src --include="*.toml" 2>/dev/null | head -20Repository: bmad-code-org/bmad-module-game-dev-studio
Length of output: 67
🏁 Script executed:
# Review the full resolve-customization.py to understand data flow from load to json.dump
cat -n "src/agents/gds-agent-tech-writer/scripts/resolve-customization.py" | tail -100Repository: bmad-code-org/bmad-module-game-dev-studio
Length of output: 4144
Handle TOML temporal types before JSON dumping.
Lines 174 and 176 can raise TypeError when TOML contains date/time/datetime values (parsed by tomllib into Python temporal objects). While the current codebase does not have temporal values in any TOML files, this is a latent correctness issue that will break the resolver if temporal values are added in the future.
Suggested fix
@@
import argparse
import json
import sys
import tomllib
from pathlib import Path
from typing import Any
+from datetime import date, datetime, time
@@
def extract_key(data: dict[str, Any], dotted_key: str) -> Any:
@@
return current
+
+
+def to_jsonable(value: Any) -> Any:
+ """Convert TOML-parsed values into JSON-serializable structures."""
+ if isinstance(value, dict):
+ return {k: to_jsonable(v) for k, v in value.items()}
+ if isinstance(value, list):
+ return [to_jsonable(v) for v in value]
+ if isinstance(value, (datetime, date, time)):
+ return value.isoformat()
+ return value
@@
if args.keys:
result = {}
for key in args.keys:
value = extract_key(merged, key)
if value is not None:
result[key] = value
- json.dump(result, sys.stdout, indent=2, ensure_ascii=False)
+ json.dump(to_jsonable(result), sys.stdout, indent=2, ensure_ascii=False)
else:
- json.dump(merged, sys.stdout, indent=2, ensure_ascii=False)
+ json.dump(to_jsonable(merged), sys.stdout, indent=2, ensure_ascii=False)📝 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.
| json.dump(result, sys.stdout, indent=2, ensure_ascii=False) | |
| else: | |
| json.dump(merged, sys.stdout, indent=2, ensure_ascii=False) | |
| if args.keys: | |
| result = {} | |
| for key in args.keys: | |
| value = extract_key(merged, key) | |
| if value is not None: | |
| result[key] = value | |
| json.dump(to_jsonable(result), sys.stdout, indent=2, ensure_ascii=False) | |
| else: | |
| json.dump(to_jsonable(merged), sys.stdout, indent=2, ensure_ascii=False) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/agents/gds-agent-tech-writer/scripts/resolve-customization.py` around
lines 174 - 176, The JSON dumps of `result` and `merged` can raise TypeError for
TOML-parsed temporal objects; add a custom JSON serializer and pass it to
`json.dump` (e.g., use a helper like `json_default` that checks for
`datetime.date`, `datetime.time`, `datetime.datetime` and returns
`isoformat()`), then call `json.dump(result, sys.stdout, ...,
default=json_default)` and `json.dump(merged, sys.stdout, ...,
default=json_default)` so temporal types are converted to ISO strings before
dumping; add the `json_default` helper near the top of
`resolve-customization.py` (or next to the current dump calls) and import
`datetime` as needed.
| parser.add_argument( | ||
| "skill_name", | ||
| help="Skill identifier (e.g. bmad-agent-pm, bmad-product-brief)", | ||
| ) |
There was a problem hiding this comment.
Validate skill_name to prevent cross-skill merge mistakes.
Line [145] always loads this workflow’s customize.toml, while Lines [160]-[161] load override files based on user input. A typo silently mixes unrelated skill layers.
🐛 Proposed guard
args = parser.parse_args()
# Locate the skill's own customize.toml (one level up from scripts/)
script_dir = Path(__file__).resolve().parent
skill_dir = script_dir.parent
+ expected_skill_name = skill_dir.name
+ if args.skill_name != expected_skill_name:
+ print(
+ f"error: skill_name '{args.skill_name}' does not match this resolver's skill "
+ f"directory '{expected_skill_name}'",
+ file=sys.stderr,
+ )
+ sys.exit(2)
defaults_path = skill_dir / "customize.toml"Also applies to: 145-145, 160-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/workflows/4-production/gds-code-review/scripts/resolve-customization.py`
around lines 129 - 132, Validate the skill_name argument (declared with
parser.add_argument("skill_name")) before using it to load overrides: ensure it
matches an allowed pattern (e.g. only lowercase letters, numbers and hyphens via
regex r'^[a-z0-9-]+$') and/or is present in the current workflow's known skill
list, and abort with a clear error if it doesn't; then use the validated
skill_name when building paths for loading customize.toml and override files so
a typo cannot cause cross-skill file loading (adjust the load logic that
references customize.toml and the override file paths to use the validated
value).
| parser.add_argument( | ||
| "skill_name", | ||
| help="Skill identifier (e.g. bmad-agent-pm, bmad-product-brief)", | ||
| ) |
There was a problem hiding this comment.
Validate skill_name before building file paths.
skill_name is used directly in path construction; values containing separators or .. can escape _bmad/customizations and read unintended files.
🔒 Proposed fix
@@
import argparse
import json
+import re
import sys
@@
def main() -> None:
@@
args = parser.parse_args()
+ if not re.fullmatch(r"[a-z0-9-]+", args.skill_name):
+ parser.error("skill_name must match ^[a-z0-9-]+$")
@@
customizations_dir = project_root / "_bmad" / "customizations"
team = load_toml(customizations_dir / f"{args.skill_name}.toml")
user = load_toml(customizations_dir / f"{args.skill_name}.user.toml")Also applies to: 160-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/workflows/4-production/gds-sprint-status/scripts/resolve-customization.py`
around lines 129 - 132, The code uses the user-supplied variable skill_name
(from parser.add_argument) directly in filesystem path construction, allowing
values with path separators or '..' to escape _bmad/customizations; fix by
validating/sanitizing skill_name before any path joining: implement an argparse
type or post-parse check that rejects values containing os.path.sep,
os.path.altsep, or any '..' segments (or require a strict whitelist regex such
as r'^[a-z0-9-]+$'), or normalize with os.path.normpath and assert the
normalized name equals the original basename, and on failure call parser.error
or raise argparse.ArgumentTypeError; apply the same validation where skill_name
is later used to build paths (the uses at the parser.add_argument site and the
later path construction).
| json.dump(result, sys.stdout, indent=2, ensure_ascii=False) | ||
| else: | ||
| json.dump(merged, sys.stdout, indent=2, ensure_ascii=False) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
from pathlib import Path
import tomllib
import datetime as dt
def walk(v, path="$"):
issues = []
if isinstance(v, dict):
for k, vv in v.items():
issues.extend(walk(vv, f"{path}.{k}"))
elif isinstance(v, list):
for i, vv in enumerate(v):
issues.extend(walk(vv, f"{path}[{i}]"))
elif isinstance(v, (dt.date, dt.time, dt.datetime)):
issues.append((path, type(v).__name__, v.isoformat()))
return issues
toml_files = sorted(Path(".").rglob("*.toml"))
found = False
for f in toml_files:
try:
data = tomllib.loads(f.read_text(encoding="utf-8"))
except Exception:
continue
issues = walk(data)
if issues:
found = True
print(f"\n{f}:")
for p, t, val in issues:
print(f" - {p}: {t} = {val}")
if not found:
print("No TOML date/time/datetime values detected.")
PYRepository: bmad-code-org/bmad-module-game-dev-studio
Length of output: 128
🏁 Script executed:
cat -n src/workflows/gametest/gds-e2e-scaffold/scripts/resolve-customization.py | sed -n '170,180p'Repository: bmad-code-org/bmad-module-game-dev-studio
Length of output: 522
🏁 Script executed:
# Check what files are processed by this script and if any are guaranteed to have date/time types
rg -B 5 -A 5 "tomllib" src/workflows/gametest/gds-e2e-scaffold/scripts/resolve-customization.pyRepository: bmad-code-org/bmad-module-game-dev-studio
Length of output: 699
Handle TOML date/time types before JSON serialization.
json.dump() will fail if any TOML file contains date/time values (valid per TOML spec), as tomllib returns datetime.date, datetime.time, and datetime.datetime objects which are not JSON-serializable. While no existing TOML files in the repository currently contain such values, this should be addressed to prevent runtime failures when date/time fields are added.
💡 Proposed fix
+import datetime as dt
import json
import sys
import tomllib
@@
+def _json_default(value: Any) -> str:
+ if isinstance(value, (dt.date, dt.time, dt.datetime)):
+ return value.isoformat()
+ raise TypeError(f"Object of type {type(value).__name__} is not JSON serializable")
+
@@
- json.dump(result, sys.stdout, indent=2, ensure_ascii=False)
+ json.dump(result, sys.stdout, indent=2, ensure_ascii=False, default=_json_default)
else:
- json.dump(merged, sys.stdout, indent=2, ensure_ascii=False)
+ json.dump(merged, sys.stdout, indent=2, ensure_ascii=False, default=_json_default)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/workflows/gametest/gds-e2e-scaffold/scripts/resolve-customization.py`
around lines 174 - 176, The JSON dump will fail if TOML-parsed data (from
tomllib) contains datetime.date/time/datetime objects; update the serialization
before calling json.dump for both `result` and `merged` by converting any
datetime/date/time values to ISO-8601 strings (or otherwise JSON-serializable
types). Implement a small sanitizer that recursively walks the parsed data
structure and replaces instances of `datetime.date`, `datetime.time`, and
`datetime.datetime` with `.isoformat()` strings, then pass the sanitized object
to `json.dump` where `result` or `merged` are used.
| def load_toml(path: Path) -> dict[str, Any]: | ||
| """Return parsed TOML or empty dict if the file doesn't exist.""" | ||
| if not path.is_file(): | ||
| return {} | ||
| try: | ||
| with open(path, "rb") as f: | ||
| return tomllib.load(f) | ||
| except (tomllib.TOMLDecodeError, OSError) as exc: | ||
| print(f"warning: failed to parse {path}: {exc}", file=sys.stderr) | ||
| return {} |
There was a problem hiding this comment.
Fail fast when default customize.toml is unreadable.
Line 154 loads defaults via a loader that always degrades to {}. If the skill default file is missing/corrupt, the resolver should exit non-zero instead of silently continuing.
Proposed fix
-def load_toml(path: Path) -> dict[str, Any]:
- """Return parsed TOML or empty dict if the file doesn't exist."""
+def load_toml(path: Path, *, required: bool = False) -> dict[str, Any]:
+ """Return parsed TOML; optionally fail hard for required files."""
if not path.is_file():
+ if required:
+ print(f"error: required TOML file not found: {path}", file=sys.stderr)
+ raise SystemExit(2)
return {}
try:
with open(path, "rb") as f:
return tomllib.load(f)
except (tomllib.TOMLDecodeError, OSError) as exc:
- print(f"warning: failed to parse {path}: {exc}", file=sys.stderr)
- return {}
+ level = "error" if required else "warning"
+ print(f"{level}: failed to parse {path}: {exc}", file=sys.stderr)
+ if required:
+ raise SystemExit(2)
+ return {}
@@
- defaults = load_toml(defaults_path)
+ defaults = load_toml(defaults_path, required=True)Also applies to: 154-155
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/workflows/gds-quick-flow/gds-quick-dev/scripts/resolve-customization.py`
around lines 42 - 51, The load_toml function currently swallows parsing/read
errors and returns {} which lets execution continue; change it so that if the
file exists but tomllib.TOMLDecodeError or OSError occurs it fails fast: either
re-raise the exception or call sys.exit(1) with a descriptive error message
instead of returning an empty dict. Update load_toml (the function that opens
and calls tomllib.load) to distinguish "file not found" (keep returning {}) from
"file present but unreadable/corrupt" (exit non-zero or raise), and ensure the
error message includes the path and exception details.
| "skill_name", | ||
| help="Skill identifier (e.g. bmad-agent-pm, bmad-product-brief)", | ||
| ) |
There was a problem hiding this comment.
Validate skill_name before using it in path construction.
Line 160 and Line 161 interpolate args.skill_name directly into filenames. Reject path separators / traversal tokens to avoid reading unintended files.
Proposed fix
import argparse
import json
+import re
import sys
@@
+SKILL_NAME_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*$")
+
@@
args = parser.parse_args()
+ if not SKILL_NAME_RE.fullmatch(args.skill_name):
+ parser.error("skill_name contains invalid characters")Also applies to: 160-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/workflows/gds-quick-flow/gds-quick-dev/scripts/resolve-customization.py`
around lines 130 - 132, Validate args.skill_name before using it to build file
paths: reject any values containing path separators or traversal tokens (e.g.,
'/', '\', '..') and only allow a safe whitelist (e.g., letters, numbers, hyphen,
underscore). Update the argument handling in resolve-customization.py (the
parser where "skill_name" is defined and any code that reads files using
args.skill_name for filenames like the ones constructed around line 160/161) to
perform this check and raise/exit with a clear error if validation fails so the
unsafe value is never interpolated into file paths.
Summary
customize.tomlto all 35 skills: 7 agents with full persona + metadata sections, 28 workflows with stock fieldsresolve-customization.pyscript in each skill'sscripts/directory for three-layer TOML merge (user > team > defaults)Part of the cross-module skill customization initiative. See bmad-code-org/BMAD-METHOD#2262 for the core implementation.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation