Convert sprint-planning workflow to native skill packaging#1944
Convert sprint-planning workflow to native skill packaging#1944
Conversation
🤖 Augment PR SummarySummary: Converts the BMM sprint-planning workflow into a native skill directory ( Changes: Renames the workflow folder/frontmatter, adds 🤖 Was this summary useful? React with 👍 or 👎 |
| @@ -1,5 +1,5 @@ | |||
| --- | |||
| name: sprint-planning | |||
| name: bmad-sprint-planning | |||
There was a problem hiding this comment.
Now that the workflow is bmad-sprint-planning, consider updating other in-repo instructions that still tell users to run sprint-planning (or /bmad:bmm:workflows:sprint-planning), since those commands/paths may no longer exist after this migration. Otherwise users may hit dead ends when other workflows (e.g. sprint-status/create-story) direct them to re-run sprint planning.
Severity: medium
Other Locations
src/bmm/workflows/4-implementation/sprint-status/workflow.md:68src/bmm/workflows/4-implementation/sprint-status/workflow.md:229src/bmm/workflows/4-implementation/create-story/workflow.md:74src/bmm/workflows/4-implementation/create-story/workflow.md:85docs/zh-cn/tutorials/getting-started.md:181docs/zh-cn/tutorials/getting-started.md:230docs/zh-cn/how-to/upgrade-to-v6.md:61
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
📝 WalkthroughWalkthroughThis PR migrates the sprint-planning workflow to a new directory structure named bmad-sprint-planning. Changes include updating the Scrum Master agent workflow reference path, creating new SKILL.md and bmad-skill-manifest.yaml files in the new directory, updating workflow metadata (name and installed_path), and removing metadata from the old manifest. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/bmm/workflows/4-implementation/bmad-sprint-planning/workflow.md (3)
60-68:⚠️ Potential issue | 🟡 MinorNo fallback when neither whole nor sharded epics are found.
The discovery logic describes checking for whole document first, then sharded version, but doesn't specify behavior when neither exists. Should the workflow HALT with an error, prompt the user, or proceed with empty status?
Suggested addition for error handling
4. **Priority**: If both whole and sharded versions exist, use the whole document +5. **Not Found**: If no epic files are found matching the patterns, HALT and inform {user_name} that epics must be created first🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/bmad-sprint-planning/workflow.md` around lines 60 - 68, The discovery flow for epics (the steps referencing "Search for whole document first", "epics/index.md" and "Read index.md") lacks a defined fallback; update the workflow to explicitly handle the case when neither a whole epics file nor a sharded epics/index.md is found by failing fast with a clear error message and instructions for the user (or prompting for next action). Modify the logic that checks for whole-document files (e.g., epics.md, bmm-epics.md, *epic*.md) and the branch that checks for epics/index.md so that after both checks fail it emits a descriptive error (or user prompt) and halts further processing instead of continuing silently with empty status.
176-181:⚠️ Potential issue | 🟡 MinorInconsistent variable syntax in YAML template.
The variable placeholders use inconsistent syntax:
- Lines 140-145 (comments):
{date},{project_name}— no spaces- Lines 176-181 (YAML fields):
{ date },{ project_name }— with spacesThis inconsistency could cause issues if the LLM or any tooling expects a specific placeholder format. The spaced version
{ date }might also be misinterpreted as YAML flow mapping syntax.Proposed fix to standardize 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/bmm/workflows/4-implementation/bmad-sprint-planning/workflow.md` around lines 176 - 181, The YAML template uses inconsistent placeholder syntax: change the spaced placeholders in the YAML fields (keys generated, last_updated, project, project_key, tracking_system, story_location) from the form "{ date }", "{ project_name }", etc. to the standardized no-space form "{date}", "{project_name}", etc.; update those six fields so all placeholders match the existing non-spaced format used elsewhere in the file to avoid parsing or LLM interpretation issues.
48-48:⚠️ Potential issue | 🟡 MinorGlob pattern could match multiple files.
The
**/project-context.mdglob could match multiple files if there are project-context.md files in subdirectories. The "(load if exists)" qualifier doesn't specify behavior when multiple matches occur. Consider using a more specific path or documenting the expected resolution.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/bmad-sprint-planning/workflow.md` at line 48, The glob pattern `**/project-context.md` can match multiple files; update the workflow to use a specific path or define deterministic resolution: replace `**/project-context.md` with a single explicit path (e.g., `./project-context.md` or `project-context.md` at the intended directory) or document and implement behavior for multiple matches (e.g., "load the first match sorted by path" or "throw an error if more than one exists"); ensure the `(load if exists)` note is updated to state the chosen deterministic behavior and reference `project_context` so readers know which policy applies.
🧹 Nitpick comments (3)
src/bmm/workflows/4-implementation/bmad-sprint-planning/workflow.md (3)
34-35: Redundant path variables?Both
story_locationandstory_location_absoluteare set to{implementation_artifacts}. If these always resolve to the same value, consider consolidating to reduce confusion. Alternatively, if there's a distinction (e.g., one for display vs. filesystem operations), document the difference.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/bmad-sprint-planning/workflow.md` around lines 34 - 35, The two variables story_location and story_location_absolute are both assigned {implementation_artifacts}; either consolidate them into a single canonical variable (e.g., keep story_location and remove story_location_absolute, updating all references to use story_location) or explicitly document and implement their distinct roles (e.g., story_location for user-facing/display paths and story_location_absolute for filesystem/absolute paths) and ensure their assignments and downstream usages (in templates or scripts referencing story_location and story_location_absolute) reflect that distinction.
204-207: Multiple variable syntax conventions used.The workflow uses three different placeholder syntaxes:
- Single braces:
{date},{user_name},{status_file}— path/config variables- Double braces:
{{epic_count}},{{story_count}}— computed values- Spaced braces:
{ date }(lines 176-181) — unclear intentWhile double braces for computed values may be intentional to distinguish them from config variables, this should be explicitly documented to avoid confusion for maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/bmad-sprint-planning/workflow.md` around lines 204 - 207, Normalize and document the placeholder conventions used in this workflow: treat single-brace tokens like {date}, {user_name}, {status_file} as path/config variables and double-brace tokens like {{epic_count}}, {{story_count}} as computed values; update occurrences of spaced-brace forms such as { date } to the chosen standard (prefer {date} for config and {{epic_count}} for computed) and add a short note near the top of workflow.md explaining this convention so maintainers understand the distinction between {date}, {{epic_count}}, and malformed { date } usages.
69-69: Fuzzy matching instruction lacks specificity.The "fuzzy matching" directive provides no guidance on acceptable variations. This relies entirely on the LLM's interpretation, which could lead to inconsistent behavior across different models or sessions. Consider providing explicit examples of acceptable variations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/bmad-sprint-planning/workflow.md` at line 69, Update the "Fuzzy matching" directive to specify exact acceptable variations and matching rules: state that matching should be case-insensitive and accept common separators (dash, underscore, space), allow optional prefixes/suffixes (e.g., "bmm-", "team-", "-draft"), and optional file extensions (".md"); add concrete examples such as "epics.md", "bmm-epics.md", "user_stories.md", "team epics draft.md" and describe that substring matches or token-boundary matches are valid (or define that only full-token matches are allowed) so the LLM has an explicit rule set to follow when resolving document names instead of relying on implicit behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/bmm/workflows/4-implementation/bmad-sprint-planning/workflow.md`:
- Around line 60-68: The discovery flow for epics (the steps referencing "Search
for whole document first", "epics/index.md" and "Read index.md") lacks a defined
fallback; update the workflow to explicitly handle the case when neither a whole
epics file nor a sharded epics/index.md is found by failing fast with a clear
error message and instructions for the user (or prompting for next action).
Modify the logic that checks for whole-document files (e.g., epics.md,
bmm-epics.md, *epic*.md) and the branch that checks for epics/index.md so that
after both checks fail it emits a descriptive error (or user prompt) and halts
further processing instead of continuing silently with empty status.
- Around line 176-181: The YAML template uses inconsistent placeholder syntax:
change the spaced placeholders in the YAML fields (keys generated, last_updated,
project, project_key, tracking_system, story_location) from the form "{ date }",
"{ project_name }", etc. to the standardized no-space form "{date}",
"{project_name}", etc.; update those six fields so all placeholders match the
existing non-spaced format used elsewhere in the file to avoid parsing or LLM
interpretation issues.
- Line 48: The glob pattern `**/project-context.md` can match multiple files;
update the workflow to use a specific path or define deterministic resolution:
replace `**/project-context.md` with a single explicit path (e.g.,
`./project-context.md` or `project-context.md` at the intended directory) or
document and implement behavior for multiple matches (e.g., "load the first
match sorted by path" or "throw an error if more than one exists"); ensure the
`(load if exists)` note is updated to state the chosen deterministic behavior
and reference `project_context` so readers know which policy applies.
---
Nitpick comments:
In `@src/bmm/workflows/4-implementation/bmad-sprint-planning/workflow.md`:
- Around line 34-35: The two variables story_location and
story_location_absolute are both assigned {implementation_artifacts}; either
consolidate them into a single canonical variable (e.g., keep story_location and
remove story_location_absolute, updating all references to use story_location)
or explicitly document and implement their distinct roles (e.g., story_location
for user-facing/display paths and story_location_absolute for
filesystem/absolute paths) and ensure their assignments and downstream usages
(in templates or scripts referencing story_location and story_location_absolute)
reflect that distinction.
- Around line 204-207: Normalize and document the placeholder conventions used
in this workflow: treat single-brace tokens like {date}, {user_name},
{status_file} as path/config variables and double-brace tokens like
{{epic_count}}, {{story_count}} as computed values; update occurrences of
spaced-brace forms such as { date } to the chosen standard (prefer {date} for
config and {{epic_count}} for computed) and add a short note near the top of
workflow.md explaining this convention so maintainers understand the distinction
between {date}, {{epic_count}}, and malformed { date } usages.
- Line 69: Update the "Fuzzy matching" directive to specify exact acceptable
variations and matching rules: state that matching should be case-insensitive
and accept common separators (dash, underscore, space), allow optional
prefixes/suffixes (e.g., "bmm-", "team-", "-draft"), and optional file
extensions (".md"); add concrete examples such as "epics.md", "bmm-epics.md",
"user_stories.md", "team epics draft.md" and describe that substring matches or
token-boundary matches are valid (or define that only full-token matches are
allowed) so the LLM has an explicit rule set to follow when resolving document
names instead of relying on implicit behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2e90fa9c-a2e3-4f82-83df-13b4e4f1e104
⛔ Files ignored due to path filters (1)
src/bmm/module-help.csvis excluded by!**/*.csv
📒 Files selected for processing (7)
src/bmm/agents/sm.agent.yamlsrc/bmm/workflows/4-implementation/bmad-sprint-planning/SKILL.mdsrc/bmm/workflows/4-implementation/bmad-sprint-planning/bmad-skill-manifest.yamlsrc/bmm/workflows/4-implementation/bmad-sprint-planning/checklist.mdsrc/bmm/workflows/4-implementation/bmad-sprint-planning/sprint-status-template.yamlsrc/bmm/workflows/4-implementation/bmad-sprint-planning/workflow.mdsrc/bmm/workflows/4-implementation/sprint-planning/bmad-skill-manifest.yaml
💤 Files with no reviewable changes (1)
- src/bmm/workflows/4-implementation/sprint-planning/bmad-skill-manifest.yaml
bfacca5 to
e30685b
Compare
e30685b to
23ba421
Compare
Summary
src/bmm/workflows/4-implementation/sprint-planninginto native skill directorysrc/bmm/workflows/4-implementation/bmad-sprint-planningtype: skilland addSKILL.mdmodule-help.csvandsm.agent.yamlValidation
node tools/cli/bmad-cli.js install --directory /Users/alex/src/bmad --modules bmm --tools claude-code --yes/Users/alex/src/bmad/.claude/skills/bmad-sprint-planning/_bmad/_config/skill-manifest.csv; absent from_bmad/_config/workflow-manifest.csvworkflow.md; exact parity PASS forchecklist.mdandsprint-status-template.yamlnpm test