feat(workflows): adopt customize.toml pattern for workflow skills#23
feat(workflows): adopt customize.toml pattern for workflow skills#23
Conversation
Merge redirect-only SKILL.md with workflow.md content into a single SKILL.md per workflow skill, add Conventions + On Activation (resolve customization, prepend/append steps, persistent_facts, config load, greet), and wire workflow.on_complete into each workflow's terminal step. External-style workflows (steps-c/, steps-v/, steps-e/, etc.) get the on_complete resolver appended to the final step file; inline workflows get an <action> inside the final <step> block. gds-sprint-status has three terminal branches (main flow step 5, data mode step 20, validate mode step 30) — on_complete wired at each. gds-document-project dispatches into instructions.md + deep-dive-instructions.md + full-scan-instructions.md; on_complete wired at all three terminal points so the hook fires regardless of execution path. Applies the same customization surface shipped for the GDS agent skills in #22.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 40 minutes and 1 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
WalkthroughWorkflow definitions across 40+ skill directories have been restructured: content previously distributed across Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
This pull request is abnormally large and would use a significant amount of tokens to review. If you still wish to review it, comment "augment review" and we will review it. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (21)
src/workflows/gametest/gds-playtest-plan/SKILL.md-76-78 (1)
76-78:⚠️ Potential issue | 🟠 MajorReplace undefined placeholders and remove duplicated initialization block.
{installed_path}and{module_config}are not defined in the Conventions section, and this block also repeats config-load/greet behavior already handled in On Activation. This can cause unresolved path variables and double-greeting behavior.Suggested fix
-**Supporting Components**: -- Validation: `{installed_path}/checklist.md` -- Template: `{installed_path}/playtest-template.md` +**Supporting Components**: +- Validation: `{skill-root}/checklist.md` +- Template: `{skill-root}/playtest-template.md` - Knowledge Base: `knowledge/playtesting.md` @@ -Load and resolve configuration from `{module_config}`: - -```yaml -output_folder: {from config} -user_name: {from config} -communication_language: {from config} -document_output_language: {from config} -game_dev_experience: {from config} -date: {system-generated} -``` - Resolve workflow variables: ```yaml playtest_type: "internal" # internal | external | focused session_duration: 60 # minutes participant_count: 5
-Greet the user by name (
user_name) and confirm the playtest type and scope before proceeding.
+Confirm the playtest type and scope before proceeding.</details> Also applies to: 85-94, 103-103 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@src/workflows/gametest/gds-playtest-plan/SKILL.mdaround lines 76 - 78, The
Validation/Template/Knowledge Base lines use undefined placeholders
{installed_path}and{module_config}and duplicate initialization
(config-load/greet) already performed in the On Activation section; remove or
replace those placeholders with resolved workflow variables (e.g., use values
derived from config such as output_folder, user_name, communication_language,
document_output_language, game_dev_experience and system-generated date) and
delete the duplicated greeting/config-load steps so only On Activation performs
initialization; update the Validation/Template/Knowledge Base entries to
reference concrete paths or the resolved variable names (e.g., playtest_type,
session_duration, participant_count) and apply the same replacements in the
other affected blocks (lines noted in the review).</details> </blockquote></details> <details> <summary>src/workflows/gametest/gds-test-review/SKILL.md-75-76 (1)</summary><blockquote> `75-76`: _⚠️ Potential issue_ | _🟠 Major_ **Unify placeholder variables to match the established conventions in this file.** Lines 75–76 use `{installed_path}`, and line 83 uses `{module_config}`, but the conventions block (lines 14–15) establishes `{skill-root}` and `{project-root}` as the documented placeholders for this workflow. Replace the non-standard placeholders with the file's stated conventions to ensure paths resolve correctly at runtime. <details> <summary>Suggested patch</summary> ```diff -**Supporting Components**: -- Validation: `{installed_path}/checklist.md` -- Template: `{installed_path}/test-review-template.md` +**Supporting Components**: +- Validation: `{skill-root}/checklist.md` +- Template: `{skill-root}/test-review-template.md` @@ -Load and resolve configuration from `{module_config}`: +Load and resolve configuration from `{project-root}/_bmad/gds/config.yaml`:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/gametest/gds-test-review/SKILL.md` around lines 75 - 76, Replace the non-standard placeholders used in the Validation and Template lines: change `{installed_path}` to the established `{skill-root}` placeholder (update both occurrences in the "Validation: `{installed_path}/checklist.md`" and "Template: `{installed_path}/test-review-template.md`" lines) and replace `{module_config}` with the documented `{project-root}` placeholder where it appears; ensure the literal strings "Validation:" and "Template:" remain unchanged so the workflow uses the file's declared `{skill-root}`/`{project-root}` conventions.src/workflows/gametest/gds-test-design/SKILL.md-76-77 (1)
76-77:⚠️ Potential issue | 🟠 MajorAdd missing placeholder definitions to resolve inconsistency with similar workflows.
{installed_path}and{module_config}are used in this skill but are not defined in the Conventions or Paths sections. Other similar workflows (e.g.,gds-test-automate,gds-performance-test) explicitly defineinstalled_path = {skill_root}. Define these placeholders to match the pattern used elsewhere and prevent runtime resolution failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/gametest/gds-test-design/SKILL.md` around lines 76 - 77, Add definitions for the placeholders used in the SKILL.md Conventions/Paths section: declare installed_path = {skill_root} (to match other workflows like gds-test-automate and gds-performance-test) and add a module_config placeholder (e.g., module_config = {installed_path}/module_config or module_config = {skill_root}/module_config.yaml) so that `{installed_path}` and `{module_config}` used in the document (e.g., in Validation: `{installed_path}/checklist.md` and Template: `{installed_path}/test-design-template.md`) are explicitly defined and will resolve at runtime.src/workflows/4-production/gds-correct-course/SKILL.md-276-292 (1)
276-292:⚠️ Potential issue | 🟠 MajorMove handoff confirmation outside the Major-only branch.
Confirm handoff completionandDocument handoffcurrently run only for Major scope. Minor and Moderate routes should also confirm/log the handoff before Step 6 summarizes completion.🐛 Proposed fix
<check if="Major scope"> <action>Route to: Product Manager / Solution Architect</action> <action>Deliverables: Complete Sprint Change Proposal + escalation notice</action> - -<action>Confirm handoff completion and next steps with user</action> -<action>Document handoff in workflow execution log</action> </check> + +<action>Confirm handoff completion and next steps with user</action> +<action>Document handoff in workflow execution log</action>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/4-production/gds-correct-course/SKILL.md` around lines 276 - 292, Move the two actions "<action>Confirm handoff completion and next steps with user</action>" and "<action>Document handoff in workflow execution log</action>" out of the Major-only <check if="Major scope"> branch so they run for all scopes; update the SKILL.md workflow by placing those two action nodes after the three <check if="..."> blocks (or by adding identical action nodes to the Minor and Moderate branches) so that Minor, Moderate and Major routes all perform handoff confirmation and logging before Step 6.src/workflows/4-production/gds-correct-course/SKILL.md-120-133 (1)
120-133:⚠️ Potential issue | 🟠 MajorAlign the halt condition with the documented required documents.
This step requires Architecture and UI/UX, but the discovery rules say only GDD and Epics are essential. As written, the workflow can halt even when optional artifacts are simply unavailable.
🐛 Proposed fix
- <action>Verify access to required project documents:</action> + <action>Verify access to required project documents:</action> - GDD (Game Design Document) - Current Epics and Stories + <action>Check optional project documents if available:</action> - Architecture documentation - UI/UX specifications - Narrative Design documentation @@ -<action if="core documents are unavailable">HALT: "Need access to project documents (GDD, Epics, Architecture, UI/UX) to assess change impact. Please ensure these documents are accessible."</action> +<action if="GDD or Epics are unavailable">HALT: "Need access to core project documents (GDD and Epics) to assess change impact. Please ensure these documents are accessible."</action>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/4-production/gds-correct-course/SKILL.md` around lines 120 - 133, Update the HALT action with condition action if="core documents are unavailable" so it only enforces availability of the essential discovery artifacts (GDD and Current Epics and Stories) rather than optional ones; change the halt message to request the GDD and Epics specifically and convert Architecture documentation, UI/UX specifications, and Narrative Design documentation into non-blocking warnings or guidance rather than blockers. Locate the "Verify access to required project documents:" block and the two HALT actions (action if="change trigger is unclear" and action if="core documents are unavailable") and ensure only the core-docs HALT references GDD and Current Epics and Stories, while optional artifacts are noted as recommended but not required.src/workflows/gametest/gds-test-framework/SKILL.md-82-91 (1)
82-91:⚠️ Potential issue | 🟠 MajorUndefined variable and confusing config reference.
Line 82 references
{module_config}, which is not defined in the Conventions section (lines 11-16) or anywhere else in the workflow. Additionally, the config has already been loaded in the "On Activation" section (line 42) from{project-root}/_bmad/gds/config.yaml.This section appears to be a reference/reminder of available config variables rather than an execution instruction, but the phrasing "Load and resolve configuration from
{module_config}" reads as a directive and could lead implementers to load the config twice.📝 Proposed fix to clarify this as reference documentation
-Load and resolve configuration from `{module_config}`: +**Configuration variables available** (loaded during On Activation Step 4): ```yaml output_folder: {from config} user_name: {from config} communication_language: {from config} document_output_language: {from config} game_dev_experience: {from config} date: {system-generated}</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@src/workflows/gametest/gds-test-framework/SKILL.mdaround lines 82 - 91,
Replace the misleading directive "Load and resolve configuration from
{module_config}" with a clear note that the YAML block is a reference/example
of resolved config values (not an instruction to reload); mention that actual
config is loaded in the "On Activation" section from
{project-root}/_bmad/gds/config.yamland retain the YAML keys as illustrative
placeholders (output_folder,user_name,communication_language,
document_output_language,game_dev_experience,date) so implementers
understand these are sample values rather than a second load operation.</details> </blockquote></details> <details> <summary>src/workflows/gds-quick-flow/gds-quick-dev/SKILL.md-108-117 (1)</summary><blockquote> `108-117`: _⚠️ Potential issue_ | _🟠 Major_ **Replace the unresolved `{main_config}` placeholder.** `{main_config}` is not defined in this skill, while activation already standardizes on `{project-root}/_bmad/gds/config.yaml`. As written, the initialization reload can stall or depend on an undeclared variable. <details> <summary>Proposed fix</summary> ```diff -Load and read full config from `{main_config}` and resolve: +Load and read full config from `{project-root}/_bmad/gds/config.yaml` and resolve:🤖 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/SKILL.md` around lines 108 - 117, The placeholder `{main_config}` is undefined and should be replaced with the standardized config path used by activation; update the doc/init step to reference `{project-root}/_bmad/gds/config.yaml` (or introduce and document a `main_config` variable that points to that path) so `project_name`, `planning_artifacts`, `implementation_artifacts`, etc. are loaded from a concrete source; ensure `sprint_status` and `project_context` resolution logic continues to derive from the resolved `implementation_artifacts` and that existence checks (for project-context.md and CLAUDE.md/memory files) remain intact.src/workflows/gametest/gds-test-automate/SKILL.md-82-103 (1)
82-103:⚠️ Potential issue | 🟠 MajorUse the defined
{skill-root}placeholder.Line 84 uses
{skill_root}, but the conventions define{skill-root}. This leaves{installed_path}unresolved or wrong, which breaks the checklist and knowledge fragment paths.Proposed fix
- `installed_path` = `{skill_root}` + `installed_path` = `{skill-root}`🤖 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 82 - 103, The placeholder name is inconsistent: SKILL.md defines `{skill-root}` but the Paths section uses `{skill_root}` for `installed_path`, causing unresolved paths; update the `installed_path` and all uses (`installed_path`, `validation`, `test_dir`, `source_dir`, `default_output_file`, and the Knowledge Fragments entries for Unity/Unreal/Godot/E2E) to reference the correct `{skill-root}` placeholder (replace `{skill_root}` with `{skill-root}`) so `installed_path` resolves and checklist/knowledge fragment paths are correct.src/workflows/4-production/gds-create-story/SKILL.md-99-253 (1)
99-253:⚠️ Potential issue | 🟠 MajorFix Step 1 routing before story discovery can proceed.
This step jumps to
step 2a, but nostep 2aexists, and the backlog discovery block is duplicated after the guarded branch. Also, thedoneepic check only runs for the first story in an epic, so later stories can still be created under a completed epic. Route tostep 2, remove the duplicate discovery block, and validate epic status before the first-story-only update.Proposed direction
- <action>GOTO step 2a</action> + <action>GOTO step 2</action> @@ - <action>GOTO step 2a</action> + <action>GOTO step 2</action> @@ - <action>GOTO step 2a</action> + <action>GOTO step 2</action> @@ - <action>GOTO step 2a</action> + <action>GOTO step 2</action>Then remove the second, unguarded backlog-discovery block at Lines 198-253 and perform the epic status validation once before the first-story-only status transition.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/4-production/gds-create-story/SKILL.md` around lines 99 - 253, Step 1 currently GOTO's "step 2a" (which doesn't exist), duplicates the backlog-discovery block, and only checks epic 'done' status when it's the first story; change all GOTO targets in Step 1 from "step 2a" to "step 2", remove the duplicated unguarded backlog-discovery block (the second copy that begins at the later Load FULL file actions), and move the epic status validation (the check for epic status 'done' and invalid statuses) to run immediately after you extract epic_num/story_num/story_key from the discovered backlog story (before performing the "first story in epic" first-story-only transition); update any references to {{story_key}}, {{epic_num}}, and {{sprint_status}} accordingly so the validation always runs for every discovered story.src/workflows/2-design/gds-create-prd/SKILL.md-93-107 (1)
93-107:⚠️ Potential issue | 🟠 MajorResolve startup placeholders before routing.
{main_config}and{nextStep}are not defined in this skill. Use the explicit config and first-step paths so activation can deterministically enter the PRD workflow.Proposed fix
-Load and read full config from {main_config} and resolve: +Load and read full config from `{project-root}/_bmad/gds/config.yaml` and resolve: @@ -Read fully and follow: `{nextStep}` (steps-c/step-01-init.md) +Read fully and follow: `steps-c/step-01-init.md`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/2-design/gds-create-prd/SKILL.md` around lines 93 - 107, The startup placeholders {main_config} and {nextStep} must be resolved to explicit values so activation deterministically enters Create Mode: update the "Configuration Loading" and "Route to Create Workflow" sections in SKILL.md to reference the concrete config file/key (e.g., the actual main config object name or filename used by the system) and the explicit first-step path for Create Mode (the exact step-c/step-01-init.md path), replacing {main_config} and {nextStep}; ensure the text still documents loading project_name/output_folder/planning_artifacts/user_name and enforcing communication_language as the agent's speaking language.src/workflows/gametest/gds-test-automate/SKILL.md-202-228 (1)
202-228:⚠️ Potential issue | 🟠 MajorRemove the backslashes from the Godot
_sutcalls in the GDScript template.The template will generate
\_sutliterally in GDScript code, which is invalid syntax. Markdown code blocks do not require escaping underscores.Required fix
- var result = \_sut.{method_name}({parameters}) + var result = _sut.{method_name}({parameters}) @@ - var result = \_sut.{method_name}(tc.input) + var result = _sut.{method_name}(tc.input)🤖 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 202 - 228, The GDScript template incorrectly escapes the `_sut` identifier (producing `\_sut`) in test functions; update the template in SKILL.md so all occurrences use the plain `_sut` identifier (e.g., in before_each/after_each initialization, the `test_{method_name}_when_{condition}_should_{expectation}` act line `var result = _sut.{method_name}({parameters})` and the parameterized test loop `var result = _sut.{method_name}(tc.input)`), removing the backslashes so generated GDScript is valid.src/workflows/4-production/gds-code-review/SKILL.md-90-90 (1)
90-90:⚠️ Potential issue | 🟠 Major
{main_config}placeholder is undefined and violates documented conventions.Line 90 references
{main_config}, but this placeholder is not defined anywhere in the codebase (not in Conventions, not in customize.toml, not in any config files). The file documents only three valid placeholders in its Conventions section:{skill-root},{project-root}, and{skill-name}. This same undefined placeholder appears consistently across 11 workflow files, indicating a systematic architectural issue.Line 43 of this same file explicitly uses
{project-root}/_bmad/gds/config.yaml, which is the documented pattern. Use the same explicit path on Line 90 for consistency and reliability.Suggested fix
-Load and read full config from `{main_config}` and resolve: +Load and read full config from `{project-root}/_bmad/gds/config.yaml` and resolve:🤖 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/SKILL.md` at line 90, The text uses an undefined placeholder `{main_config}` (violating the Conventions which only define `{skill-root}`, `{project-root}`, and `{skill-name}`); replace occurrences of `{main_config}` in SKILL.md (and the other affected workflow files) with the explicit documented path `{project-root}/_bmad/gds/config.yaml` so the config reference matches the pattern already used (e.g., the existing `{project-root}/_bmad/gds/config.yaml` usage) and restore consistency across all 11 workflow files.src/workflows/3-technical/gds-generate-project-context/SKILL.md-74-75 (1)
74-75:⚠️ Potential issue | 🟠 MajorPlaceholder mismatch likely breaks path resolution.
Conventions define
{skill-root}(hyphen), but this section uses{skill_root}(underscore). That can leaveinstalled_pathunresolved and break template loading.Proposed fix
-- `installed_path` = `{skill_root}` +- `installed_path` = `{skill-root}`🤖 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 74 - 75, The installed_path/template_path placeholders use the wrong placeholder name `{skill_root}`; update the placeholder to match the convention `{skill-root}` so `installed_path` and `template_path` resolve correctly (look for the lines that set `installed_path` and `template_path` in SKILL.md and replace `{skill_root}` with `{skill-root}` so the template loader receives the correct path).src/workflows/2-design/gds-validate-prd/SKILL.md-95-95 (1)
95-95:⚠️ Potential issue | 🟠 MajorUse explicit config path for consistency with Step 4 activation flow.
{main_config}is undefined throughout the codebase. Step 4 (On Activation) uses the explicit path{project-root}/_bmad/gds/config.yaml, but the INITIALIZATION SEQUENCE section uses an undefined placeholder instead. This pattern appears across all workflow files and creates ambiguity in runtime initialization.Suggested fix
-Load and read full config from {main_config} and resolve: +Load and read full config from `{project-root}/_bmad/gds/config.yaml` and resolve:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/2-design/gds-validate-prd/SKILL.md` at line 95, Replace the undefined placeholder `{main_config}` with the explicit config path `{project-root}/_bmad/gds/config.yaml` wherever it appears (e.g., in the INITIALIZATION SEQUENCE section of SKILL.md and other workflow files) so the initialization flow matches Step 4 (On Activation); update the text "Load and read full config from {main_config} and resolve:" to "Load and read full config from {project-root}/_bmad/gds/config.yaml and resolve:" and ensure any references or examples use that exact path for consistency.src/workflows/2-design/gds-create-narrative/SKILL.md-98-98 (1)
98-98:⚠️ Potential issue | 🟠 MajorAdd
{main_config}to the Conventions section or replace with the documented config path.
{main_config}is referenced in the INITIALIZATION SEQUENCE but is not defined in the Conventions section. However, Step 4 of "On Activation" explicitly documents the config source as{project-root}/_bmad/gds/config.yaml. Either:
- Add
{main_config}to Conventions (lines 14–19) with value{project-root}/_bmad/gds/config.yaml, or- Replace
{main_config}with the explicit path to match Step 4.This inconsistency exists across multiple SKILL.md files and creates ambiguity about variable injection at runtime.
🤖 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` at line 98, The document references the placeholder {main_config} in the INITIALIZATION SEQUENCE but that token is not defined in the Conventions section; either add a Conventions entry defining {main_config} = {project-root}/_bmad/gds/config.yaml or replace every occurrence of {main_config} in this SKILL.md (and the other SKILL.md files with the same issue) with the explicit path {project-root}/_bmad/gds/config.yaml so the config source is unambiguous (update references in the "Conventions" section and the "INITIALIZATION SEQUENCE"/"On Activation" steps accordingly).src/workflows/2-design/gds-validate-gdd/SKILL.md-92-99 (1)
92-99:⚠️ Potential issue | 🟠 MajorDuplicate configuration loading with undefined variable.
Similar to the domain-research workflow, configuration is loaded twice:
- Activation Step 4 (lines 43-48) loads minimal config from
{project-root}/_bmad/gds/config.yaml- Initialization Sequence (lines 95-99) attempts to load from undefined
{main_config}The
{main_config}variable is never defined. Additionally, the extensive field list in the initialization sequence (project_name,output_folder,planning_artifacts,document_output_language,game_dev_experience) is not loaded during the activation phase.🔧 Proposed fix to consolidate config loading
Expand the activation Step 4 (around line 43) to include all required fields:
### Step 4: Load Config Load config from `{project-root}/_bmad/gds/config.yaml` and resolve: +- `project_name` +- `output_folder` +- `planning_artifacts` - `user_name` - `communication_language` +- `document_output_language` +- `game_dev_experience` +- `date` as the system-generated current datetimeThen update the INITIALIZATION SEQUENCE section to remove the duplicate loading:
## INITIALIZATION SEQUENCE ### 1. Configuration Loading -Load and read full config from {main_config} and resolve: - -- `project_name`, `output_folder`, `planning_artifacts`, `user_name` -- `communication_language`, `document_output_language`, `game_dev_experience` -- `date` as system-generated current datetime +Configuration was loaded during activation (see "On Activation" section above).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/2-design/gds-validate-gdd/SKILL.md` around lines 92 - 99, Consolidate and fix duplicate config loading by expanding Activation Step 4 to load the full config (ensure you read {project-root}/_bmad/gds/config.yaml into the same config object) and populate the fields project_name, output_folder, planning_artifacts, user_name, communication_language, document_output_language, game_dev_experience, and date (set to current datetime); then remove the redundant Initialization Sequence load that references the undefined {main_config} variable and update any subsequent references to use the activation-step config instead (refer to "Activation Step 4" and "INITIALIZATION SEQUENCE" in SKILL.md and remove any use of {main_config}).src/workflows/1-preproduction/research/gds-domain-research/SKILL.md-64-69 (1)
64-69:⚠️ Potential issue | 🟠 MajorDuplicate configuration loading with undefined variable.
Configuration is loaded twice with inconsistencies:
- Activation Step 4 (lines 42-48) loads config from
{project-root}/_bmad/gds/config.yaml- CONFIGURATION section (lines 64-69) loads config from undefined
{module_config}This creates confusion about which config source to use. The
{module_config}variable is never defined, and the extended field list in the CONFIGURATION section (project_name,output_folder,document_output_language,game_dev_experience) is not loaded during activation.🔧 Proposed fix to consolidate config loading
Option 1: Extend the activation Step 4 to include all needed fields:
In the "Step 4: Load Config" section around line 42, expand to:
### Step 4: Load Config Load config from `{project-root}/_bmad/gds/config.yaml` and resolve: +- `project_name` +- `output_folder` - `user_name` - `communication_language` +- `document_output_language` +- `game_dev_experience` - `planning_artifacts` - `date` as the system-generated current datetimeThen remove the duplicate CONFIGURATION section (lines 64-69).
Option 2: Keep both sections but fix the reference and clarify intent:
## CONFIGURATION -Load config from `{module_config}` and resolve: +Extended config (already loaded during activation, listed here for reference): - `project_name`, `output_folder`, `planning_artifacts`, `user_name` - `communication_language`, `document_output_language`, `game_dev_experience` - `date` as a system-generated value🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/1-preproduction/research/gds-domain-research/SKILL.md` around lines 64 - 69, The CONFIGURATION section references an undefined `{module_config}` and duplicates config loading; consolidate by extending the existing "Step 4: Load Config" (the activation step that reads `{project-root}/_bmad/gds/config.yaml`) to load all required fields (`project_name`, `output_folder`, `planning_artifacts`, `user_name`, `communication_language`, `document_output_language`, `game_dev_experience`, and `date` as a system-generated value) and then remove the duplicate CONFIGURATION block; alternatively, if you must keep a separate CONFIGURATION section, replace `{module_config}` with the same `{project-root}/_bmad/gds/config.yaml` reference and ensure that the extra fields are actually loaded in the activation code that performs the config read.src/workflows/3-technical/gds-check-implementation-readiness/SKILL.md-88-99 (1)
88-99:⚠️ Potential issue | 🟠 Major
{module_config}undefined after workflow.md removal — duplicates On Activation config loading.This section references
{module_config}which is not bound anywhere in the new SKILL.md. On Activation Step 4 already loads{project-root}/_bmad/gds/config.yaml. Either drop this legacy block or rewrite it to read the remaining fields (project_name,output_folder,planning_artifacts,document_output_language) from the same concrete path.🤖 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 88 - 99, The SKILL.md references an undefined token `{module_config}`; update this initialization section to either remove the legacy `{module_config}` mention or change it to explicitly read the remaining fields (`project_name`, `output_folder`, `planning_artifacts`, `document_output_language`) from the concrete config used on activation (`{project-root}/_bmad/gds/config.yaml`) so there is no duplicate/undefined loading; locate the block in SKILL.md that lists "Module Configuration Loading" and replace `{module_config}` with the concrete config path or remove the block if redundant with Activation Step 4.src/workflows/1-preproduction/gds-create-game-brief/SKILL.md-93-106 (1)
93-106:⚠️ Potential issue | 🟠 Major
{main_config}undefined — duplicates On Activation config load.The On Activation block (Steps 4–5) already resolves
user_nameandcommunication_languagefrom{project-root}/_bmad/gds/config.yaml. This legacy INITIALIZATION SEQUENCE then asks the agent to reload from{main_config}, which has no binding in this SKILL.md afterworkflow.mdwas removed. Either fold the extra fields (project_name,output_folder,document_output_language,game_dev_experience,date) into Step 4's resolution list against the known config path, or strip this block. Same pattern recurs across the other restructured SKILL.md files in this PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/1-preproduction/gds-create-game-brief/SKILL.md` around lines 93 - 106, The INITIALIZATION SEQUENCE references an undefined `{main_config}` and redundantly reloads values already resolved in the On Activation block (Steps 4–5) from `{project-root}/_bmad/gds/config.yaml`; remove the `{main_config}` reload or fold its required fields (`project_name`, `output_folder`, `document_output_language`, `game_dev_experience`, `date`) into the existing On Activation resolution so Step 4–5 explicitly resolves those keys from `{project-root}/_bmad/gds/config.yaml`, and update the instruction that invokes `steps/step-01-init.md` to rely on those resolved values (and delete any leftover `{main_config}` references).src/workflows/3-technical/gds-create-epics-and-stories/SKILL.md-92-103 (1)
92-103:⚠️ Potential issue | 🟠 MajorINITIALIZATION SEQUENCE duplicates the On Activation config/greet flow.
On Activation Step 4/5 already resolves
user_name/communication_languagefrom{project-root}/_bmad/gds/config.yamland greets. This section then asks the agent to reload from{module_config}(undefined in this file afterworkflow.mddeletion) and re-assert language. Consider reducing section 1 to just the fields this workflow additionally needs (project_name,output_folder,planning_artifacts,document_output_language), sourced from the already-known config path, or remove it entirely if On Activation is sufficient. Same concern applies to the sibling 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 92 - 103, The INITIALIZATION SEQUENCE duplicates the On Activation config/greet flow by reloading undefined `{module_config}` and re-asserting language; update SKILL.md to stop re-reading the full config and instead only require the additional fields this workflow needs—`project_name`, `output_folder`, `planning_artifacts`, `document_output_language`—and source them from the already-resolved `{project-root}/_bmad/gds/config.yaml` (as handled in On Activation Step 4/5), or remove the entire Initialization section if On Activation already supplies everything; ensure references to `{module_config}` are deleted and similar changes are applied to sibling SKILL.md files.src/workflows/2-design/gds-edit-gdd/SKILL.md-92-110 (1)
92-110:⚠️ Potential issue | 🟠 MajorLegacy INITIALIZATION SEQUENCE duplicates/contradicts the new On Activation block.
On Activation (Step 4) already loads config from the hardcoded
{project-root}/_bmad/gds/config.yamland greets the user. The remaining INITIALIZATION SEQUENCE then tells the agent to reload config from{main_config}(undefined in this document) and re-assert speaking language. Two issues:
{main_config}is never defined here — previously it was resolved fromworkflow.mdcontext that has been deleted. The agent has no binding for it.{editWorkflow}on Line 110 is also undefined; only the parenthetical hintsteps-e/step-e-01-discovery.mdis authoritative.- Re-loading config after On Activation is at best redundant, at worst conflicting if paths differ.
Consider collapsing INITIALIZATION SEQUENCE into a minimal "Route to Edit Workflow" section that directly reads
steps-e/step-e-01-discovery.md, since config is already resolved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/2-design/gds-edit-gdd/SKILL.md` around lines 92 - 110, The INITIALIZATION SEQUENCE duplicates and references undefined tokens ({main_config}, {editWorkflow}) and conflicts with the On Activation config load; remove the redundant config reload and undefined tokens, collapse the section into a minimal "Route to Edit Workflow" that prompts for the GDD path (default: `{planning_artifacts}/gdd.md`) and then directly loads and follows the authoritative file `steps-e/step-e-01-discovery.md`; keep a single explicit note that the agent must always speak in the configured `{communication_language}` (but do not attempt to re-resolve or reload config here since On Activation already handles it).
🟡 Minor comments (17)
src/workflows/4-production/gds-correct-course/SKILL.md-312-312 (1)
312-312:⚠️ Potential issue | 🟡 MinorGive
on_completethe same fallback path as activation.Activation defines a manual fallback when
resolve_customization.pyfails, but the terminal hook depends only on the script. If the resolver is unavailable, a configuredworkflow.on_completecan be skipped.🛠️ Proposed fix
-<action>Run: `python3 {project-root}/_bmad/scripts/resolve_customization.py --skill {skill-root} --key workflow.on_complete` — if the resolved value is non-empty, follow it as the final terminal instruction before exiting.</action> +<action>Resolve `workflow.on_complete` from the already-resolved `{workflow}` block. If unavailable, run: `python3 {project-root}/_bmad/scripts/resolve_customization.py --skill {skill-root} --key workflow.on_complete`; if the script fails, apply the same base → team → user fallback merge from activation. If the resolved value is non-empty, follow it as the final terminal instruction before exiting.</action>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/4-production/gds-correct-course/SKILL.md` at line 312, Ensure the terminal hook uses the same manual fallback as activation: when invoking resolve_customization.py for workflow.on_complete (the terminal hook) handle both script failure and empty result by falling back to the configured manual on_complete path (the same manual fallback used by on_activate), i.e., update the logic that runs resolve_customization.py (resolve_customization.py --skill {skill-root} --key workflow.on_complete) so that any exception, non-zero exit, or empty output causes the code to use the configured workflow.on_complete manual value (or the activation manual fallback) as the final terminal instruction before exiting.src/workflows/gametest/gds-performance-test/SKILL.md-69-69 (1)
69-69:⚠️ Potential issue | 🟡 MinorFix the path reference to the knowledge base file.
The file
knowledge/performance-testing.mdexists in the repository atsrc/agents/gds-agent-game-dev/gametest/knowledge/performance-testing.md, but the reference path in SKILL.md appears incorrect. Update the reference to the correct relative path from the workflow file's location, or clarify the intended resolution mechanism if this uses a different path resolution strategy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/gametest/gds-performance-test/SKILL.md` at line 69, SKILL.md currently points to a nonexistent knowledge/performance-testing.md; update the reference in src/workflows/gametest/gds-performance-test/SKILL.md to the correct relative path ../../../agents/gds-agent-game-dev/gametest/knowledge/performance-testing.md so it resolves to the actual file src/agents/gds-agent-game-dev/gametest/knowledge/performance-testing.md, replacing the existing `knowledge/performance-testing.md` token.src/workflows/gametest/gds-performance-test/SKILL.md-83-83 (1)
83-83:⚠️ Potential issue | 🟡 MinorUse
{skill-root}(hyphenated) to match the documented convention at line 17.The Conventions section explicitly defines the placeholder as
{skill-root}(with hyphen), and the resolution script at line 25 uses--skill {skill-root}with the hyphenated form. Line 83 should align with this:-- `installed_path` = `{skill_root}` +- `installed_path` = `{skill-root}`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/gametest/gds-performance-test/SKILL.md` at line 83, The placeholder on the installed_path line uses `{skill_root}` (underscore) but must follow the documented convention `{skill-root}` (hyphen) used elsewhere (see Conventions and the resolution script `--skill {skill-root}`); update the `installed_path` entry to use `{skill-root}` so the placeholder is consistent with `installed_path`, the Conventions section, and the resolution script.src/workflows/4-production/gds-create-story/SKILL.md-424-441 (1)
424-441:⚠️ Potential issue | 🟡 MinorUse the defined output-file variable in the completion message.
{{story_file}}is not defined in this skill; the story path is tracked as{default_output_file}. The current report can show unresolved placeholders to the user.Proposed fix
- - File: {{story_file}} + - File: {default_output_file} @@ - 1. Review the comprehensive story in {{story_file}} + 1. Review the comprehensive story in {default_output_file}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/4-production/gds-create-story/SKILL.md` around lines 424 - 441, The completion message uses an undefined placeholder {{story_file}}; replace it with the skill's tracked output variable {default_output_file} in the output block so the final report shows the correct story path (update the output text that currently lists "- File: {{story_file}}" to use {default_output_file}); keep the rest of the message and the existing Run command (python3 ... resolve_customization.py) unchanged so customization logic still runs.src/workflows/1-preproduction/gds-brainstorm-game/SKILL.md-66-66 (1)
66-66:⚠️ Potential issue | 🟡 MinorMinor wording fix: “self-contained” (hyphenated).
🤖 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/SKILL.md` at line 66, Update the wording in the SKILL.md "Micro-file Design" line: replace the phrase "self contained" with the hyphenated form "self-contained" so the sentence reads "**Micro-file Design**: Each step is a self-contained instruction file that is a part of an overall workflow that must be followed exactly"; edit the exact line containing "Micro-file Design" to apply this change.src/workflows/2-design/gds-create-narrative/SKILL.md-66-66 (1)
66-66:⚠️ Potential issue | 🟡 MinorMinor wording fix: use “self-contained”.
🤖 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` at line 66, Update the wording in the "Micro-file Design" bullet where it currently reads "self contained" to use the hyphenated form "self-contained"; locate the bullet starting with "**Micro-file Design**" in SKILL.md and replace "self contained instruction file" with "self-contained instruction file" so the phrasing is correct.src/workflows/2-design/gds-validate-prd/SKILL.md-66-66 (1)
66-66:⚠️ Potential issue | 🟡 MinorFix compound adjective typo.
Use “self-contained” here for consistency/readability.
Suggested wording fix
-- **Micro-file Design**: Each step is a self contained instruction file that is a part of an overall workflow that must be followed exactly +- **Micro-file Design**: Each step is a self-contained instruction file that is a part of an overall workflow that must be followed exactly🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/2-design/gds-validate-prd/SKILL.md` at line 66, Update the phrase in the "Micro-file Design" bullet in SKILL.md to use the correct compound adjective "self-contained" instead of "self contained"; edit the line that currently reads "**Micro-file Design**: Each step is a self contained instruction file..." so it reads "**Micro-file Design**: Each step is a self-contained instruction file..." to improve consistency and readability.src/workflows/1-preproduction/gds-brainstorm-game/SKILL.md-100-100 (1)
100-100:⚠️ Potential issue | 🟡 MinorDefine or replace
{main_config}with the explicit config path.Line 100 references
{main_config}, which is not defined in the Conventions section like other template variables ({skill-root},{project-root},{skill-name}). Step 4 already documents the actual config location as{project-root}/_bmad/gds/config.yaml. Line 100 should either define{main_config}in Conventions or use the explicit path for clarity.🤖 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/SKILL.md` at line 100, Replace the ambiguous placeholder {main_config} in SKILL.md with an explicit path or add it to the Conventions; specifically, update the "Load and read full config from {main_config} and resolve:" phrase to either "Load and read full config from {project-root}/_bmad/gds/config.yaml and resolve:" or add a new Conventions entry defining {main_config} = {project-root}/_bmad/gds/config.yaml so references (the line using {main_config} and the existing Step 4) are consistent and unambiguous.src/workflows/2-design/gds-validate-gdd/SKILL.md-66-66 (1)
66-66:⚠️ Potential issue | 🟡 MinorGrammar: missing hyphen in compound adjective.
The phrase "self contained" should be hyphenated as "self-contained" when used as a compound adjective modifying "instruction file".
📝 Proposed fix
-- **Micro-file Design**: Each step is a self contained instruction file that is a part of an overall workflow that must be followed exactly +- **Micro-file Design**: Each step is a self-contained instruction file that is a part of an overall workflow that must be followed exactly🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/2-design/gds-validate-gdd/SKILL.md` at line 66, Update the phrasing in SKILL.md where the sentence reads "Each step is a self contained instruction file..." to use the compound adjective form "self-contained" (i.e., change "self contained" to "self-contained") so the line becomes "**Micro-file Design**: Each step is a self-contained instruction file that is a part of an overall workflow that must be followed exactly".src/workflows/4-production/gds-sprint-planning/SKILL.md-209-214 (1)
209-214:⚠️ Potential issue | 🟡 MinorInconsistent placeholder syntax in YAML template.
The placeholders in lines 209-214 use spaces around the variable names (
{ date },{ project_name }), while the rest of the file consistently uses no spaces ({project-root},{user_name},{communication_language}). This inconsistency may confuse the agent during template substitution.🔧 Proposed fix for consistent placeholder syntax
-generated: { date } -last_updated: { date } -project: { project_name } -project_key: { project_key } -tracking_system: { tracking_system } -story_location: { story_location } +generated: {date} +last_updated: {date} +project: {project_name} +project_key: {project_key} +tracking_system: {tracking_system} +story_location: {story_location}🤖 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 209 - 214, The YAML placeholders in the block using generated: { date }, last_updated: { date }, project: { project_name }, project_key: { project_key }, tracking_system: { tracking_system }, story_location: { story_location } are inconsistent with the rest of the file which uses no spaces inside braces (e.g. {project-root}, {user_name}); update these placeholders to remove the spaces so they read {date}, {project_name}, {project_key}, {tracking_system}, and {story_location} to match the existing placeholder syntax and ensure consistent template substitution.src/workflows/4-production/gds-sprint-status/customize.toml-37-38 (1)
37-38:⚠️ Potential issue | 🟡 MinorIncomplete terminal step documentation.
The comment states
on_completeexecutes at "Step 30 (Validate sprint-status file)", but per the PR objectives,gds-sprint-statushas three terminal branches whereon_completefires: main flow step 5, data-mode step 20, and validate-mode step 30. The comment should clarify that the hook executes at all terminal points, not just Step 30.📝 Proposed fix for complete documentation
-# Scalar: executed when the workflow reaches Step 30 (Validate sprint-status file), -# after the final outputs are produced. Override wins. +# Scalar: executed when the workflow reaches any terminal step +# (Step 5 main flow, Step 20 data-mode, or Step 30 validate-mode), +# after the final outputs are produced. Override wins.🤖 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/customize.toml` around lines 37 - 38, The comment for on_complete in the gds-sprint-status customize.toml is misleading because it only mentions Step 30 (Validate sprint-status file) even though the on_complete hook fires at all terminal branches (main flow step 5, data-mode step 20, and validate-mode step 30); update the documentation comment to state that on_complete runs whenever the workflow reaches any terminal step (specifically list main flow step 5, data-mode step 20, and validate-mode step 30) and note that override wins. Ensure the comment references the on_complete hook and the three terminal branches by name so future readers understand all trigger points.src/workflows/3-technical/gds-check-implementation-readiness/SKILL.md-62-62 (1)
62-62:⚠️ Potential issue | 🟡 MinorTypo: "adhere too" → "adhere to".
Same nit as in the sibling
gds-create-epics-and-stories/SKILL.md.🤖 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` at line 62, Fix the typo in the SKILL.md under the "Micro-file Design" bullet: change "adhere too" to "adhere to" in the sentence "Each step of the overall goal is a self contained instruction file that you will adhere too 1 file as directed at a time" (look for the "Micro-file Design" heading and that exact sentence in SKILL.md).src/workflows/3-technical/gds-create-epics-and-stories/SKILL.md-66-66 (1)
66-66:⚠️ Potential issue | 🟡 MinorTypo: "adhere too" → "adhere to".
-- **Micro-file Design**: Each step of the overall goal is a self contained instruction file that you will adhere too 1 file as directed at a time +- **Micro-file Design**: Each step of the overall goal is a self-contained instruction file that you adhere to, one file at a time as directed🤖 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` at line 66, Fix the typo in the SKILL.md "Micro-file Design" line: change "adhere too" to "adhere to" so the sentence reads "Each step of the overall goal is a self contained instruction file that you will adhere to 1 file as directed at a time"; update the exact text in the SKILL.md entry containing "Micro-file Design" to replace "too" with "to".src/workflows/3-technical/gds-game-architecture/SKILL.md-73-73 (1)
73-73:⚠️ Potential issue | 🟡 MinorVariable name inconsistent with Conventions:
{skill_root}vs{skill-root}.Line 73 writes
installed_path = {skill_root}(underscore), but the Conventions block on Line 17 defines{skill-root}(hyphen). Downstream steps resolving{installed_path}may get an unresolved token. Use the hyphenated form for consistency:-- `installed_path` = `{skill_root}` +- `installed_path` = `{skill-root}`🤖 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` at line 73, The variable token used for installed_path is inconsistent: change the underscore token `{skill_root}` to the hyphenated convention `{skill-root}` so `installed_path` uses the same token as defined in the Conventions block; update the occurrence of `installed_path = {skill_root}` to `installed_path = {skill-root}` to ensure downstream resolvers find the correct token.src/workflows/4-production/gds-dev-story/SKILL.md-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorMinor grammar: hyphenate compound adjective.
The phrase "context filled story spec file" should use a hyphen: "context-filled story spec file".
✏️ Proposed fix
-**Goal:** Execute story implementation following a context filled story spec file. +**Goal:** Execute story implementation following a context-filled story spec file.🤖 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/SKILL.md` at line 8, Update the phrase "context filled story spec file" in SKILL.md to use a hyphenated compound adjective: replace it with "context-filled story spec file" (locate the Goal line containing "Execute story implementation following a context filled story spec file.").src/workflows/2-design/gds-edit-prd/SKILL.md-66-66 (1)
66-66:⚠️ Potential issue | 🟡 MinorMinor grammar: hyphenate compound adjective.
The phrase "self contained instruction file" should use a hyphen: "self-contained instruction file".
✏️ Proposed fix
-- **Micro-file Design**: Each step is a self contained instruction file that is a part of an overall workflow that must be followed exactly +- **Micro-file Design**: Each step is a self-contained instruction file that is a part of an overall workflow that must be followed exactly🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/2-design/gds-edit-prd/SKILL.md` at line 66, Minor grammar: hyphenate the compound adjective "self contained" to "self-contained" in the SKILL.md step description; locate the string "- **Micro-file Design**: Each step is a self contained instruction file that is a part of an overall workflow that must be followed exactly" and update it so the phrase reads "self-contained instruction file" (preserve surrounding wording and Markdown formatting).src/workflows/2-design/gds-create-gdd/SKILL.md-66-66 (1)
66-66:⚠️ Potential issue | 🟡 MinorMinor grammar: hyphenate compound adjective.
The phrase "self contained instruction file" should use a hyphen: "self-contained instruction file".
✏️ Proposed fix
-- **Micro-file Design**: Each step is a self contained instruction file that is a part of an overall workflow that must be followed exactly +- **Micro-file Design**: Each step is a self-contained instruction file that is a part of an overall workflow that must be followed exactly🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/workflows/2-design/gds-create-gdd/SKILL.md` at line 66, Replace the phrase "self contained instruction file" with the hyphenated form "self-contained instruction file" in the SKILL.md text (the sentence starting "**Micro-file Design**: Each step..."), ensuring the compound adjective is hyphenated.
| ```csharp | ||
| [UnityTest] | ||
| public IEnumerator Performance_CombatScene_MaintainsFPS() | ||
| { | ||
| using (Measure.ProfilerMarkers(new[] { "Main Thread" })) | ||
| { | ||
| SceneManager.LoadScene("CombatStressTest"); | ||
| yield return new WaitForSeconds(30f); | ||
| } | ||
| var metrics = Measure.Custom(new SampleGroupDefinition("FPS")); | ||
| Assert.Greater(metrics.Median, 30, "FPS should stay above 30"); | ||
| } | ||
| ``` | ||
| </action> |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Unity Performance Testing package Measure.ProfilerMarkers and Measure.Custom API documentation
💡 Result:
Unity Performance Testing package (com.unity.test-framework.performance, latest version 3.2.1 as of 2026-03-12, compatible with Unity 6) provides Measure.ProfilerMarkers and Measure.Custom APIs for performance tests. Measure.ProfilerMarkers: Records profiler markers automatically within a using scope. Names must match profiler marker labels. Sampled once per frame; multiple invocations per frame sum up. Supports string[] of marker names or SampleGroup[] for custom units. Example 1 (string[]): [Test, Performance] public void Test { string[] markers = {"Instantiate", "Instantiate.Copy", "Instantiate.Produce", "Instantiate.Awake"}; using (Measure.ProfilerMarkers(markers)) { // code } } Example 2 (SampleGroup[]): [UnityTest, Performance] public IEnumerator Test { var sampleGroups = new[] { new SampleGroup("Instantiate", SampleUnit.Second), new SampleGroup("Instantiate.Copy", SampleUnit.Nanosecond), new SampleGroup("Instantiate.Awake", SampleUnit.Microsecond) }; using (Measure.ProfilerMarkers(sampleGroups)) { // code } } Limitations: Not supported in Edit Mode tests, Deep Profiling, or Profiler.BeginSample markers (use ProfilerMarker instead). Recent update (3.2.1) uses ProfilerRecorder API. Measure.Custom(sampleGroup, value): Records custom metric values (double) outside frame/method/profiler, e.g., memory. Example: [Test, Performance] public void Test { var sampleGroup = new SampleGroup("TotalAllocatedMemory", SampleUnit.Megabyte, false); var allocatedMemory = UnityEngine.Profiling.Profiler.GetTotalAllocatedMemoryLong / 1048576f; Measure.Custom(sampleGroup, allocatedMemory); } Usage requires [Test, Performance] attribute and using Unity.PerformanceTesting;. Tests run in development player builds.
Citations:
- 1: https://docs.unity3d.com/Packages/com.unity.test-framework.performance@3.2/manual/measure-profile-markers.html
- 2: https://docs.unity3d.com/Packages/com.unity.test-framework.performance@3.0/manual/measure-profile-markers.html
- 3: https://docs.unity3d.com/Packages/com.unity.test-framework.performance@3.4/manual/measure-custom.html
- 4: https://docs.unity3d.com/Packages/com.unity.test-framework.performance@3.0/manual/measure-custom.html
- 5: https://docs.unity3d.com/Manual/com.unity.test-framework.performance.html
- 6: https://docs.unity3d.com/Packages/com.unity.test-framework.performance@3.2/changelog/CHANGELOG.html
Fix the Measure.Custom() call—it requires two parameters, not one.
The Measure.Custom() method signature is Measure.Custom(SampleGroup sampleGroup, double value). The code example calls it with only one argument and attempts to access a .Median property on the return value, which does not match the documented API. Additionally, the code uses SampleGroupDefinition instead of the documented SampleGroup class. Correct the example to provide both the sample group definition and the measured value as separate parameters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/workflows/gametest/gds-performance-test/SKILL.md` around lines 206 - 219,
The test calls Measure.Custom incorrectly with a SampleGroupDefinition and one
argument; update Performance_CombatScene_MaintainsFPS to create and pass a
SampleGroup (not SampleGroupDefinition) and a numeric value to Measure.Custom
(Measure.Custom(SampleGroup, double)), e.g. construct a SampleGroup for "FPS"
and pass the measured FPS value as the second parameter, assign the result to
metrics, and then assert on metrics.Median; ensure you update the SampleGroup
creation and the Measure.Custom call accordingly.
There was a problem hiding this comment.
Acknowledged — the Measure.Custom(SampleGroupDefinition, ...).Median example is incorrect per the current Unity Performance Testing API (Measure.Custom(SampleGroup, double) returns void, no .Median property). However, this bug is in the pre-existing workflow content that was merged verbatim from the legacy workflow.md into SKILL.md; it pre-dates this PR, which only adds the customization scaffolding. Tracking as a separate fix so this PR stays focused on the customize.toml pattern rollout.
There was a problem hiding this comment.
@bmadcode, understood — thanks for the context. Since the incorrect Measure.Custom example is pre-existing and out of scope for this PR, would you like me to open a follow-up GitHub issue to track the fix? I can file it with the specific correction needed (SampleGroup instead of SampleGroupDefinition, Measure.Custom(SampleGroup, double) with a measured value, and removing the non-existent .Median return), so it doesn't get lost.
- gds-e2e-scaffold, gds-document-project: fix `{skill_root}` typo
(underscore) to `{skill-root}` (dash) in `installed_path` so
downstream references like `{installed_path}/checklist.md` resolve
correctly against the convention declared above.
- deep-dive-instructions.md: move the workflow.on_complete resolver
from the end of Step 13a (area-selection confirmation loop) to
Step 13g (Finish) so the hook only fires when the user actually
exits the workflow, not during mid-workflow navigation.
1 participant
Summary
SKILL.mdwith itsworkflow.mdinto a single integratedSKILL.mdacross all 31 GDS workflow skills (15steps-c//steps-v//steps-e/external-step skills + 16 inline-step skills).Conventionsblock,On Activationsection (resolve customization → prepend steps → persistent_facts → config load from_bmad/gds/config.yaml→ greet → append steps).workflow.on_completeinto each workflow's terminal step — external workflows get## On Completeappended to the finalstep-NN-*.md; inline workflows get an<action>inside the final<step>block.customize.tomlper workflow under the[workflow]namespace so overrides from_bmad/custom/*.tomland*.user.tomlmerge per the standard BMad structural rules.Mirrors the customization surface shipped for the GDS agent skills in #22.
Branching-terminal handling
gds-sprint-statushas three terminal branches (main flow step 5, data-mode step 20, validate-mode step 30) —on_completewired at each so the hook fires regardless of which mode was invoked.gds-document-projectdispatches intoinstructions.md+deep-dive-instructions.md+full-scan-instructions.md—on_completewired at all three terminal points.Test plan
On Activationsequence (config loads from_bmad/gds/config.yaml, greet fires).on_completein_bmad/custom/gds-create-gdd.user.tomland confirm it fires at the end ofsteps-c/step-14-complete.md.on_completecheck for one inline workflow (e.g.gds-retrospectiveat step 12).gds-sprint-status, confirmon_completefires in all three modes (main, data, validate).persistent_facts = ["..."]) and confirm it appends rather than replacing the default.gds-agent-game-designerto confirm the agent→workflow handoff still works after the skill restructure.Summary by CodeRabbit
New Features
workflow.on_complete) for custom post-workflow actions.Refactor