fix(create-story): replace missing validate-workflow invoke with checklist action#1837
Conversation
…icit checklist action
🤖 Augment PR SummarySummary: Updates the create-story workflow’s finalization step to remove a stale reference to a deleted validation task. Changes: Replaces the missing 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
|
|
||
| <step n="6" goal="Update sprint status and finalize"> | ||
| <invoke-task>Validate against checklist at {installed_path}/checklist.md using _bmad/core/tasks/validate-workflow.xml</invoke-task> | ||
| <action>Validate the newly created story file {story_file} against {installed_path}/checklist.md and apply any required fixes before finalizing</action> |
There was a problem hiding this comment.
{story_file} is referenced here, but later in this workflow the story file placeholder is {{story_file}}; if these are different interpolation syntaxes, this line may not resolve to the actual generated file and the validation step could be skipped or applied to the wrong path.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
📝 WalkthroughWalkthroughReplaces step 6 validation mechanism in the create-story workflow by removing external workflow file invocation and substituting direct inline validation action that validates the newly created story against a checklist and applies required fixes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/bmm/workflows/4-implementation/create-story/instructions.xml (4)
264-265:⚠️ Potential issue | 🟠 MajorTemplate initialization uses
{default_output_file}but validation uses{story_file}.Step 5 initializes and populates
{default_output_file}:<action>Initialize from template.md: {default_output_file}</action>But step 6 validates
{story_file}. Either these should be the same variable, or there should be an explicit assignment connecting them. This disconnect could cause validation to target a different or nonexistent file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/create-story/instructions.xml` around lines 264 - 265, The template init and validation use different variables: the template action and the template-output use {default_output_file} while the validation step references {story_file}; update instructions.xml so both steps refer to the same variable (either rename all references to {story_file} or assign {story_file} = {default_output_file} before validation) and ensure the <action>Initialize from template.md: {default_output_file}</action> and <template-output file="{default_output_file}">story_header</template-output> entries are consistent with the validator's target variable ({story_file}) so validation runs against the actual initialized file.
21-24:⚠️ Potential issue | 🟡 MinorInconsistent variable notation:
{{double}}vs{single}braces.The file mixes:
{{story_path}},{{epic_num}},{{story_key}}— double braces{story_file},{installed_path},{default_output_file}— single bracesThis inconsistency suggests either different variable scopes/mechanisms or a syntax error. If these are meant to be different (e.g., user-input vs workflow-config), this should be documented. Otherwise, standardize to one notation.
Also applies to: 314-314, 328-333
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/create-story/instructions.xml` around lines 21 - 24, The template mixes two variable syntaxes (double-brace like {{story_path}}, {{epic_num}}, {{story_key}} and single-brace like {story_file}, {installed_path}, {default_output_file}), causing ambiguity or parser errors; pick one notation (preferably the existing double-brace convention used in the parse step) and replace all single-brace occurrences with the double-brace form across this instruction file (including the parse step "Parse user-provided story path" and the other occurrences around the referenced blocks), or if single-brace variables are intentionally different, add a short comment documenting the two scopes and update the workflow/template parser usage accordingly so all variable references (story_path, epic_num, story_num, story_key, story_file, installed_path, default_output_file) are consistent and unambiguous.
178-178:⚠️ Potential issue | 🟡 MinorUnprofessional language in workflow file.
Line 178 contains "fuckups" which is inappropriate for production code that may be visible to customers, auditors, or in logs.
Proposed fix
- <critical>🔬 EXHAUSTIVE ARTIFACT ANALYSIS - This is where you prevent future developer fuckups!</critical> + <critical>🔬 EXHAUSTIVE ARTIFACT ANALYSIS - This is where you prevent future developer mistakes!</critical>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/create-story/instructions.xml` at line 178, Replace the unprofessional word in the <critical> element that currently reads "🔬 EXHAUSTIVE ARTIFACT ANALYSIS - This is where you prevent future developer fuckups!" with a neutral, professional phrase (e.g., "🔬 EXHAUSTIVE ARTIFACT ANALYSIS - This is where you prevent future developer mistakes" or "…prevent future developer errors/regressions") so the <critical> tag content is appropriate for production; update the exact string in instructions.xml (the <critical> element) wherever it appears and run a quick search to ensure no other occurrences of "fuckups" remain.
59-117:⚠️ Potential issue | 🟠 MajorRemove duplicate orphaned code block at lines 119-174.
Lines 59-117 are correctly wrapped in
<check if="no user input provided">, but lines 119-174 contain an exact duplicate of the same sprint-status parsing, story extraction, and epic status-checking logic without any conditional wrapper. After the</check>closing tag on line 118, lines 119-174 execute unconditionally and redundantly repeat the entire block, creating dead code or forcing duplicate execution.Delete lines 119-174 entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/create-story/instructions.xml` around lines 59 - 117, The file contains a duplicated, unconditional copy of the sprint-status parsing and story extraction block that was already correctly wrapped in <check if="no user input provided"> … </check> (the original block references sprint_status, story_key, story_id, epic_num, story_num, and checks/updates epic-{{epic_num}} status); remove the entire duplicate block following the closing </check> (the orphaned lines that repeat the parsing, FIRST story selection, epic status checks and updates) so only the single guarded <check if="no user input provided"> block remains and all references (sprint_status, story_key, story_id, epic_num, epic-{{epic_num}}) are preserved in the original section.
🧹 Nitpick comments (2)
src/bmm/workflows/4-implementation/create-story/instructions.xml (2)
313-327: Step 6 lacks atomicity — partial state corruption possible.The step performs multiple state-changing operations without transaction semantics:
- Validate and apply fixes to story file
- Save story document
- Update sprint_status to "ready-for-dev"
If the workflow fails between saving the story (line 315) and updating sprint status (line 323), the system ends up with a saved story that isn't tracked as ready. Conversely, if status updates but save failed, the tracking claims completion that didn't happen.
Consider:
- Saving to a temp file first, then atomic rename
- Or reversing the order: update status to "validating", save, then update to "ready-for-dev"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/create-story/instructions.xml` around lines 313 - 327, The step 6 workflow is not atomic: implement atomic story save and transactional sprint_status updates to avoid partial state; when handling {story_file} and {{sprint_status}} modify the sequence in create-story/instructions.xml so you first write the story to a temp file and atomically rename it into place (preserve checklist validation and fixes), then update development_status[{{story_key}}] using an interim status transition (e.g., set to "validating" → save sprint_status preserving STATUS DEFINITIONS → verify story file exists → set to "ready-for-dev" and save), or alternatively perform the sprint_status update last but only after a successful atomic rename, ensuring every save preserves ALL comments/structure and rolls back or aborts the sprint_status change on any failure.
338-340: Hardcoded next-step instructions referencedev-storyandcode-reviewwithout validation.The output suggests running
dev-storyandcode-reviewworkflows, but doesn't verify these exist or are installed. Given the BMAD modular architecture, these could be optional modules. A missing-module scenario would leave users with invalid instructions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/create-story/instructions.xml` around lines 338 - 340, Update the hardcoded next-step text in instructions.xml so it only references optional workflows if they exist: before emitting the lines that mention "dev-story" and "code-review" (and the optional "/bmad:tea:automate" suggestion), check the BMAD module registry or installed-workflows API and conditionally include each instruction; if a workflow is missing, replace the direct command with a safe alternative message (e.g., "install or enable X module" or omit the step). Locate the strings "dev-story", "code-review", and "/bmad:tea:automate" in instructions.xml and implement the conditional inclusion logic so users never see instructions for workflows that aren't installed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bmm/workflows/4-implementation/create-story/instructions.xml`:
- Around line 314-315: The workflow currently unconditionally executes the "Save
story document unconditionally" action even if the preceding "Validate the newly
created story file {story_file} against {installed_path}/checklist.md and apply
any required fixes before finalizing" step finds unfixable issues; add a
conditional check after the validation step that tests for "validation failed
with unfixable issues" and, when true, emits a clear output like "🚫 Validation
found critical issues that cannot be auto-remediated" and transitions to a
HALT/manual-intervention branch instead of saving—update the validation step to
return a status flag (e.g., validation_result) and branch on that flag before
running the "Save story document unconditionally" action.
- Line 315: The "Save story document unconditionally" action ignores validation
results; change the workflow so the save occurs conditionally after validation
succeeds, or if you must always persist, augment the saved document and logs
with validation outcome and issues. Update the action entry "Save story document
unconditionally" to either "Save story document on successful validation" and
only call the save step when validation passes, or keep the unconditional save
but add writing of validation metadata (e.g., story.validation.status and
story.validation.issues) into the story object and emit a clear log entry (e.g.,
via the workflow logger) showing pass/fail and errors so consumers can detect
invalid state. Ensure the validation step and the save step names (validation,
saveStoryDocument) are the controlling symbols referenced in the workflow so the
behavior is deterministic.
- Line 314: The workflow references an undefined variable {story_file}; replace
that with the existing {default_output_file} (defined in workflow.yaml as
{implementation_artifacts}/{{story_key}}.md) wherever it appears: update the
<action> text to mention {default_output_file} instead of {story_file}, and
change the output/template placeholders {{story_file}} to
{{default_output_file}} so all references point to the defined variable used by
the step that creates the story file.
---
Outside diff comments:
In `@src/bmm/workflows/4-implementation/create-story/instructions.xml`:
- Around line 264-265: The template init and validation use different variables:
the template action and the template-output use {default_output_file} while the
validation step references {story_file}; update instructions.xml so both steps
refer to the same variable (either rename all references to {story_file} or
assign {story_file} = {default_output_file} before validation) and ensure the
<action>Initialize from template.md: {default_output_file}</action> and
<template-output file="{default_output_file}">story_header</template-output>
entries are consistent with the validator's target variable ({story_file}) so
validation runs against the actual initialized file.
- Around line 21-24: The template mixes two variable syntaxes (double-brace like
{{story_path}}, {{epic_num}}, {{story_key}} and single-brace like {story_file},
{installed_path}, {default_output_file}), causing ambiguity or parser errors;
pick one notation (preferably the existing double-brace convention used in the
parse step) and replace all single-brace occurrences with the double-brace form
across this instruction file (including the parse step "Parse user-provided
story path" and the other occurrences around the referenced blocks), or if
single-brace variables are intentionally different, add a short comment
documenting the two scopes and update the workflow/template parser usage
accordingly so all variable references (story_path, epic_num, story_num,
story_key, story_file, installed_path, default_output_file) are consistent and
unambiguous.
- Line 178: Replace the unprofessional word in the <critical> element that
currently reads "🔬 EXHAUSTIVE ARTIFACT ANALYSIS - This is where you prevent
future developer fuckups!" with a neutral, professional phrase (e.g., "🔬
EXHAUSTIVE ARTIFACT ANALYSIS - This is where you prevent future developer
mistakes" or "…prevent future developer errors/regressions") so the <critical>
tag content is appropriate for production; update the exact string in
instructions.xml (the <critical> element) wherever it appears and run a quick
search to ensure no other occurrences of "fuckups" remain.
- Around line 59-117: The file contains a duplicated, unconditional copy of the
sprint-status parsing and story extraction block that was already correctly
wrapped in <check if="no user input provided"> … </check> (the original block
references sprint_status, story_key, story_id, epic_num, story_num, and
checks/updates epic-{{epic_num}} status); remove the entire duplicate block
following the closing </check> (the orphaned lines that repeat the parsing,
FIRST story selection, epic status checks and updates) so only the single
guarded <check if="no user input provided"> block remains and all references
(sprint_status, story_key, story_id, epic_num, epic-{{epic_num}}) are preserved
in the original section.
---
Nitpick comments:
In `@src/bmm/workflows/4-implementation/create-story/instructions.xml`:
- Around line 313-327: The step 6 workflow is not atomic: implement atomic story
save and transactional sprint_status updates to avoid partial state; when
handling {story_file} and {{sprint_status}} modify the sequence in
create-story/instructions.xml so you first write the story to a temp file and
atomically rename it into place (preserve checklist validation and fixes), then
update development_status[{{story_key}}] using an interim status transition
(e.g., set to "validating" → save sprint_status preserving STATUS DEFINITIONS →
verify story file exists → set to "ready-for-dev" and save), or alternatively
perform the sprint_status update last but only after a successful atomic rename,
ensuring every save preserves ALL comments/structure and rolls back or aborts
the sprint_status change on any failure.
- Around line 338-340: Update the hardcoded next-step text in instructions.xml
so it only references optional workflows if they exist: before emitting the
lines that mention "dev-story" and "code-review" (and the optional
"/bmad:tea:automate" suggestion), check the BMAD module registry or
installed-workflows API and conditionally include each instruction; if a
workflow is missing, replace the direct command with a safe alternative message
(e.g., "install or enable X module" or omit the step). Locate the strings
"dev-story", "code-review", and "/bmad:tea:automate" in instructions.xml and
implement the conditional inclusion logic so users never see instructions for
workflows that aren't installed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0c86617f-44fa-44af-b4aa-fd39463f7020
📒 Files selected for processing (1)
src/bmm/workflows/4-implementation/create-story/instructions.xml
| <action>Validate the newly created story file {story_file} against {installed_path}/checklist.md and apply any required fixes before finalizing</action> | ||
| <action>Save story document unconditionally</action> |
There was a problem hiding this comment.
No error handling path if validation fails or fixes cannot be applied.
The flow unconditionally proceeds to "Save story document unconditionally" on line 315 regardless of validation outcome. If validation identifies critical gaps that cannot be auto-fixed, the workflow will still save and publish a defective story file.
Consider adding:
<check if="validation failed with unfixable issues">
<output>🚫 Validation found critical issues that cannot be auto-remediated</output>
<action>HALT - Manual intervention required</action>
</check>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bmm/workflows/4-implementation/create-story/instructions.xml` around
lines 314 - 315, The workflow currently unconditionally executes the "Save story
document unconditionally" action even if the preceding "Validate the newly
created story file {story_file} against {installed_path}/checklist.md and apply
any required fixes before finalizing" step finds unfixable issues; add a
conditional check after the validation step that tests for "validation failed
with unfixable issues" and, when true, emits a clear output like "🚫 Validation
found critical issues that cannot be auto-remediated" and transitions to a
HALT/manual-intervention branch instead of saving—update the validation step to
return a status flag (e.g., validation_result) and branch on that flag before
running the "Save story document unconditionally" action.
|
|
||
| <step n="6" goal="Update sprint status and finalize"> | ||
| <invoke-task>Validate against checklist at {installed_path}/checklist.md using _bmad/core/tasks/validate-workflow.xml</invoke-task> | ||
| <action>Validate the newly created story file {story_file} against {installed_path}/checklist.md and apply any required fixes before finalizing</action> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for story_file variable definition or assignment in workflow files
rg -n 'story_file' src/bmm/workflows/4-implementation/create-story/ -A 2 -B 2Repository: bmad-code-org/BMAD-METHOD
Length of output: 2900
🏁 Script executed:
# Also check workflow.yaml for variable declarations
fd -e yaml -e xml src/bmm/workflows/4-implementation/create-story/ -x cat -n {}Repository: bmad-code-org/BMAD-METHOD
Length of output: 566
🏁 Script executed:
cat -n src/bmm/workflows/4-implementation/create-story/instructions.xml | head -150Repository: bmad-code-org/BMAD-METHOD
Length of output: 9392
🏁 Script executed:
cat -n src/bmm/workflows/4-implementation/create-story/instructions.xml | tail -n +150 | head -200Repository: bmad-code-org/BMAD-METHOD
Length of output: 12253
🏁 Script executed:
cat -n src/bmm/workflows/4-implementation/create-story/workflow.yamlRepository: bmad-code-org/BMAD-METHOD
Length of output: 3147
🏁 Script executed:
# Check if there's any assignment of story_file or story_file_path in workflow.yaml or other config files
rg -n 'story_file|story_file_path' src/bmm/workflows/4-implementation/create-story/ --type yaml --type mdRepository: bmad-code-org/BMAD-METHOD
Length of output: 337
Replace undefined variable {story_file} with {default_output_file} at line 314, and fix corresponding output references at lines 333 and 337
The action at line 314 references {story_file}, but this variable is never defined or assigned anywhere in the workflow. Step 5 creates the story file and writes output to {default_output_file} (defined in workflow.yaml as {implementation_artifacts}/{{story_key}}.md), not to any variable called story_file. Additionally, the output block at lines 333 and 337 references {{story_file}} without prior assignment, which will produce undefined variable errors.
Update line 314 to:
<action>Validate the newly created story file {default_output_file} against {installed_path}/checklist.md and apply any required fixes before finalizing</action>
And update lines 333 and 337 to use {{default_output_file}} instead of {{story_file}}.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bmm/workflows/4-implementation/create-story/instructions.xml` at line
314, The workflow references an undefined variable {story_file}; replace that
with the existing {default_output_file} (defined in workflow.yaml as
{implementation_artifacts}/{{story_key}}.md) wherever it appears: update the
<action> text to mention {default_output_file} instead of {story_file}, and
change the output/template placeholders {{story_file}} to
{{default_output_file}} so all references point to the defined variable used by
the step that creates the story file.
| <step n="6" goal="Update sprint status and finalize"> | ||
| <invoke-task>Validate against checklist at {installed_path}/checklist.md using _bmad/core/tasks/validate-workflow.xml</invoke-task> | ||
| <action>Validate the newly created story file {story_file} against {installed_path}/checklist.md and apply any required fixes before finalizing</action> | ||
| <action>Save story document unconditionally</action> |
There was a problem hiding this comment.
"Save story document unconditionally" contradicts defensive workflow design.
Saving unconditionally after validation suggests validation results are ignored. If the intent is "always persist whatever state we have," this should at least log whether validation passed or note issues in the story file itself.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bmm/workflows/4-implementation/create-story/instructions.xml` at line
315, The "Save story document unconditionally" action ignores validation
results; change the workflow so the save occurs conditionally after validation
succeeds, or if you must always persist, augment the saved document and logs
with validation outcome and issues. Update the action entry "Save story document
unconditionally" to either "Save story document on successful validation" and
only call the save step when validation passes, or keep the unconditional save
but add writing of validation metadata (e.g., story.validation.status and
story.validation.issues) into the story object and emit a clear log entry (e.g.,
via the workflow logger) showing pass/fail and errors so consumers can detect
invalid state. Ensure the validation step and the save step names (validation,
saveStoryDocument) are the controlling symbols referenced in the workflow so the
behavior is deterministic.
2 participants
Summary
invoke-taskreference to non-existent_bmad/core/tasks/validate-workflow.xml{installed_path}/checklist.mdfor the generated{story_file}Why
validate-workflow.xmlno longer exists, so the old instruction was stale and misleadingVerification
src/bmm/workflows/4-implementation/create-story/instructions.xmla5c573d4contains1 insertion(+), 1 deletion(-)