refactor(skills): convert create-prd to native skill directory#1984
refactor(skills): convert create-prd to native skill directory#1984
Conversation
🤖 Augment PR SummarySummary: Refactors the legacy Changes:
Technical Notes: The new skill uses step-file architecture with continuation support via 🤖 Was this summary useful? React with 👍 or 👎 |
|
|
||
| ### 2. Handle Continuation (If Document Exists) | ||
|
|
||
| If the document exists and has frontmatter with `stepsCompleted` BUT `step-11-complete` is NOT in the list, follow the Continuation Protocol since the document is incomplete: |
There was a problem hiding this comment.
The continuation gate checks for step-11-complete, but this skill’s terminal step file is step-12-complete.md (and step 11 is step-11-polish.md). This can cause completed workflows to be treated as “incomplete” and route into continuation incorrectly.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| ### 4. Handle Workflow Completion | ||
|
|
||
| **If `stepsCompleted` array contains `"step-11-complete.md"`:** |
There was a problem hiding this comment.
Completion detection looks for step-11-complete.md, which doesn’t exist in this skill; the final step is step-12-complete.md. If a PRD is already complete, this can prevent the “already completed” branch and may later fail when trying to derive nextStepFile from the last step.
Severity: high
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| Update the main workflow status file if there is one: | ||
|
|
||
| - Load `{status_file}` from workflow configuration (if exists) | ||
| - Update workflow_status["prd"] = "{default_output_file}" |
There was a problem hiding this comment.
This status update writes workflow_status["prd"] = "{default_output_file}", but this workflow’s output path is outputFile: {planning_artifacts}/prd.md and default_output_file isn’t defined in this step/workflow. If the variable isn’t provided by config, the status file may end up with an unresolved placeholder.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
|
||
| **Available Validation Workflows:** | ||
|
|
||
| **Option 1: Check Implementation Readiness** (`{checkImplementationReadinessWorkflow}`) |
There was a problem hiding this comment.
{checkImplementationReadinessWorkflow} is referenced as a callable option, but it’s not defined in this step’s frontmatter and may not be present in the loaded config either. If it’s undefined at runtime, the validation option becomes unusable/unclear.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| **If complexity is LOW:** | ||
|
|
||
| Offer to skip: | ||
| "The domain complexity from our discovery is low. We may not need deep domain-specific requirements. Would you like to: |
There was a problem hiding this comment.
When complexity is low, the prompt offers a [D] Do domain exploration anyway option, but later menu handling only defines A/P/C and there’s no documented behavior for D. This could leave the agent without a deterministic branch if the user selects D.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| @@ -0,0 +1,197 @@ | |||
| # BMAD PRD Purpose | |||
|
|
|||
| **The PRD is the top of the required funnel that feeds all subsequent product development work in rhw BMad Method.** | |||
There was a problem hiding this comment.
The opening sentence has a likely typo (“rhw BMad Method”) in a prominent bolded statement. Since this file is used as a quality reference during polishing, small errors here can propagate into generated output.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
📝 WalkthroughWalkthroughThis PR converts the Create PRD workflow from a file-path-based workflow to a native skill architecture. It updates the PM agent to invoke the skill via Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
📝 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.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (7)
src/core/tasks/bmad-create-prd/steps-c/step-05-domain.md-79-79 (1)
79-79:⚠️ Potential issue | 🟡 MinorDomain label examples are likely out of sync with lookup keys
Line 79 uses “education,” while lookup in Lines 99-100 depends on exact domain matching from step-02. Your CSV uses
edtech, noteducation; exact-match lookup can miss.Proposed fix
-- What's the specific domain? (healthcare, fintech, education, etc.) +- What's the specific domain key? (healthcare, fintech, edtech, govtech, etc.)Also applies to: 99-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-05-domain.md` at line 79, The domain label "education" used in step-05-domain.md does not match the lookup key used in step-02/CSV which is "edtech", causing exact-match lookups to fail; update the domain label "education" to the canonical key "edtech" (or add a small mapping/alias handling so "education" maps to "edtech") and ensure the lookup code/path that reads the CSV uses the same canonical values; target the literal "education" in step-05-domain.md and the lookup logic that expects exact matches to align them.src/core/tasks/bmad-create-prd/steps-c/step-03-success.md-16-16 (1)
16-16:⚠️ Potential issue | 🟡 MinorProgress counters are inconsistent with the 13-step flow.
Line 16 and Line 176 still say
of 11, while surrounding steps useof 13. This will mislead users about position and remaining work.Suggested correction
- **Progress: Step 3 of 11** - Next: User Journey Mapping + **Progress: Step 3 of 13** - Next: User Journey Mapping @@ - Display: "**Select:** [A] Advanced Elicitation [P] Party Mode [C] Continue to User Journey Mapping (Step 4 of 11)" + Display: "**Select:** [A] Advanced Elicitation [P] Party Mode [C] Continue to User Journey Mapping (Step 4 of 13)"Also applies to: 176-176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-03-success.md` at line 16, The progress counters in the markdown header text "**Progress: Step 3 of 11** - Next: User Journey Mapping" (and the duplicate occurrence later in the file) are wrong for a 13-step flow; update those literal strings to "**Progress: Step 3 of 13** - Next: User Journey Mapping" (and the second occurrence at the other location) and search the file for any remaining "of 11" occurrences to replace with "of 13" so the displayed step count matches the 13-step flow.src/core/tasks/bmad-create-prd/data/prd-purpose.md-3-3 (1)
3-3:⚠️ Potential issue | 🟡 MinorTypo in opening statement.
Line 3 contains "rhw BMad Method" which should be "the BMAD Method".
📝 Fix typo
-**The PRD is the top of the required funnel that feeds all subsequent product development work in rhw BMad Method.** +**The PRD is the top of the required funnel that feeds all subsequent product development work in the BMAD Method.**🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/data/prd-purpose.md` at line 3, Replace the typo in the PRD opening line: change "rhw BMad Method" to "the BMAD Method" so the sentence reads "The PRD is the top of the required funnel that feeds all subsequent product development work in the BMAD Method."; update the sentence in prd-purpose.md accordingly.src/core/tasks/bmad-create-prd/data/prd-purpose.md-36-38 (1)
36-38:⚠️ Potential issue | 🟡 MinorContradictory messaging about user stories.
Lines 36-38 label the User Stories step as "(future: User Stories)", implying it's not yet implemented. However, line 172 describes "FRs → user stories (1 FR could map to 1-3 stories potentially)" as a current downstream artifact, not a future one. This contradiction will confuse readers about whether story generation is available.
Either remove "(future:" from line 37 if user story generation is implemented, or change line 172 to match the future tense.
Also applies to: 172-172
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/data/prd-purpose.md` around lines 36 - 38, The two lines conflict about user story availability: update the header string "Vision → Success Criteria → User Journeys → Functional Requirements → (future: User Stories)" and the later line that reads "FRs → user stories (1 FR could map to 1-3 stories potentially)" to be consistent; either remove "(future:" from the header to indicate user story generation is currently supported, or change the later "FRs → user stories..." phrasing to future tense (e.g., "FRs → will map to user stories...") so both statements match. Locate and edit the exact phrases "Vision → Success Criteria → User Journeys → Functional Requirements → (future: User Stories)" and "FRs → user stories (1 FR could map to 1-3 stories potentially)" in prd-purpose.md to make their tense/availability consistent.src/core/tasks/bmad-create-prd/steps-c/step-01b-continue.md-67-69 (1)
67-69:⚠️ Potential issue | 🟡 MinorMissing error handling for document reloading.
Lines 67-69 instruct reloading all documents from
inputDocuments, but there's no guidance for handling missing, moved, or corrupted files. In a real-world scenario, files may have been deleted or relocated between sessions.🛡️ Add graceful degradation
**Context Reloading:** - For each document in `inputDocuments`, load the complete file + - If any document cannot be loaded, inform the user which files are missing and ask if they want to proceed without them or update the path - This ensures you have full context for continuation - Don't discover new documents - only reload what was previously processed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-01b-continue.md` around lines 67 - 69, The reload step for inputDocuments lacks error handling; update the document reload logic (the loop that iterates inputDocuments and calls the loader such as loadDocument/reloadDocument) to wrap each load in a try/catch, detect common failures (file not found/moved, permission denied, corrupted read/parse), and on failure log a warning with the file id and error, set a per-document status/metadata (e.g., reloaded: false, errorMessage: "...", errorType: "Missing"|"Corrupted"), and continue processing the remaining documents without attempting to auto-discover new files; optionally implement a single configurable fallback lookup attempt (e.g., lookupFallbackPath(documentId)) before marking as missing, but do not change the rule “Don't discover new documents.” Ensure the code uses the existing symbols inputDocuments and the document loader function names to locate and modify the reload loop.src/core/tasks/bmad-create-prd/steps-c/step-04-journeys.md-159-160 (1)
159-160:⚠️ Potential issue | 🟡 MinorInconsistent user interaction pattern.
Lines 159-160 instruct asking the user "Accept these improvements to the user journeys? (y/n)" and "Accept these changes to the user journeys? (y/n)", but the menu display at line 156 shows
[A] Advanced Elicitation [P] Party Mode [C] Continue.Switching from menu letters (A/P/C) to y/n creates UI inconsistency and may confuse users about what input format is expected.
🔧 Make interaction pattern consistent
-- IF A: Read fully and follow: {advancedElicitationTask} with the current journey content, process the enhanced journey insights that come back, ask user "Accept these improvements to the user journeys? (y/n)", if yes update content with improvements then redisplay menu, if no keep original content then redisplay menu +- IF A: Read fully and follow: {advancedElicitationTask} with the current journey content, process the enhanced journey insights that come back, display the improvements, then ask user to either [Y] Accept improvements or [N] Keep original, if Y update content with improvements then redisplay menu, if N keep original content then redisplay menu -- IF P: Read fully and follow: {partyModeWorkflow} with the current journeys, process the collaborative journey improvements and additions, ask user "Accept these changes to the user journeys? (y/n)", if yes update content with improvements then redisplay menu, if no keep original content then redisplay menu +- IF P: Read fully and follow: {partyModeWorkflow} with the current journeys, process the collaborative journey improvements and additions, display the changes, then ask user to either [Y] Accept changes or [N] Keep original, if Y update content with improvements then redisplay menu, if N keep original content then redisplay menu🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-04-journeys.md` around lines 159 - 160, The acceptance prompts after running {advancedElicitationTask} and {partyModeWorkflow} use "y/n" which conflicts with the menu choice letters [A]/[P]/[C]; change those prompts to match the menu convention — e.g. after processing the enhanced journeys, prompt "Accept these improvements to the user journeys? (A = accept, C = keep original)" (and use the same phrasing for partyModeWorkflow), handle A to update content and C to keep original, then redisplay the menu so input format is consistent with the existing [A]/[P]/[C] UI.src/core/tasks/bmad-create-prd/steps-c/step-01-init.md-16-16 (1)
16-16:⚠️ Potential issue | 🟡 MinorStep progress denominator is inconsistent with later steps
This file says “Step 1 of 11”, while
step-11-polish.mdstates “Step 11 of 12”. Mixed progress cardinality will produce contradictory UX and checkpoint logic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-01-init.md` at line 16, The progress denominator in step-01-init.md ("Progress: Step 1 of 11") is inconsistent with step-11-polish.md ("Step 11 of 12"); update the "Step 1 of 11" text in src/core/tasks/bmad-create-prd/steps-c/step-01-init.md to use the correct total (make it "Step 1 of 12" if the workflow has 12 steps) and then scan all other step-*.md files (including step-11-polish.md) to ensure every "Step X of Y" uses the same total so progress UI/checkpoint logic is consistent.
🧹 Nitpick comments (12)
src/core/tasks/bmad-create-prd/steps-c/step-05-domain.md (5)
96-106: Tool-invocation prompt is ambiguous for deterministic executionThe quoted block at Lines 96-106 mixes instruction text and pseudo-output constraints, but does not define a strict invocation mechanism. This tends to produce free-form text instead of reliable structured extraction.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-05-domain.md` around lines 96 - 106, The prompt in step-05-domain.md is ambiguous for deterministic tool invocation; update the block to explicitly call the CSV lookup tool with exact parameters and expected schema instead of free-form instructions — e.g., invoke a tool (name it clearly, e.g., lookupCsvRow) with arguments: csv=domainComplexityCSV, matchField="domain", matchValue={{domainFromStep02}}, fields=["domain","complexity","typical_concerns","compliance_requirements"], outputFormat="yaml", and require the tool to return only the single YAML object; remove the loose phrases like "Your task" and "Return ONLY" and replace them with the concrete function call pattern so automation always produces the single matching row.
54-55: Frontmatter update target is underspecified and riskyYou require frontmatter updates on continue, but this step appends into
{planning_artifacts}/prd.md(not guaranteed to have mutable frontmatter/stepsCompleted in expected shape). Define exact mutation contract to avoid corrupting docs.Also applies to: 160-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-05-domain.md` around lines 54 - 55, The frontmatter update currently appends this step name to stepsCompleted in {planning_artifacts}/prd.md without validation; change the continue handler (the logic that updates prd.md from step-05-domain.md) to first read and parse YAML frontmatter, validate that a mutable stepsCompleted array exists (or create it safely), then append this step id only if not present, write back atomically (temp file + rename) and fail with a clear error if prd.md has no frontmatter or unexpected shape; implement this as a reusable function (e.g., updateStepsCompleted or ensureFrontmatterSchema) so callers can validate/mutate other docs safely.
170-176: Append operation is not idempotent and will duplicate section on rerunsOn repeated executions, this always appends another
## Domain-Specific Requirementsblock. That can produce conflicting duplicate sections in PRD.Proposed fix
-When user selects 'C', append to `{outputFile}`: +When user selects 'C', upsert in `{outputFile}`: +- If `## Domain-Specific Requirements` exists, replace that section +- Otherwise append a new section at the end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-05-domain.md` around lines 170 - 176, The current append behavior in step-05-domain.md will duplicate the "## Domain-Specific Requirements" block on every run; change the operation that writes to {outputFile} so it is idempotent by performing an upsert: read {outputFile}, detect an existing "## Domain-Specific Requirements" heading and replace that entire section with the new "{{discovered domain requirements}}" content, and only append the heading+content if no existing section is found; reference the "## Domain-Specific Requirements" heading and {outputFile} in your implementation so the write logic is a replace-or-append rather than blind append.
32-33: Language constraints can deadlock when placeholders are unsetHard “ALWAYS SPEAK/WRITE in
{communication_language}/{document_output_language}” without fallback can block execution if variables are absent upstream. Add default behavior to preserve step continuity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-05-domain.md` around lines 32 - 33, The two hard constraints lines "✅ YOU MUST ALWAYS SPEAK OUTPUT In your Agent communication style with the config `{communication_language}`" and "✅ YOU MUST ALWAYS WRITE all artifact and document content in `{document_output_language}`" can deadlock when placeholders are unset; update these rules to include a clear fallback (for example: use the provided `{communication_language}`/`{document_output_language}` when set, otherwise default to a specified language such as English) so the step continues if variables are missing, and ensure the wording in the step (the two quoted strings) explicitly documents the fallback behavior.
155-162: Menu parser does not define normalization for user input variantsLine 155 exposes symbolic options; Lines 157-162 omit normalization (
a/A, full words like “continue”, whitespace). Without explicit normalization, menu behavior is brittle in conversational runs.Proposed fix
#### Menu Handling Logic: +- Normalize user choice (trim, lowercase) and map aliases: +- "a"/"advanced" -> A, "p"/"party" -> P, "c"/"continue"/"skip" -> C, "d"/"domain" -> D🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-05-domain.md` around lines 155 - 162, The menu handler for the "Display" menu must normalize user input before branching: update the parser that reads the Display options so it trims whitespace and lowercases input, and match against accepted aliases for each branch (e.g., for Advanced Elicitation accept "a", "A", "advanced", "advanced elicitation"; for Party Mode accept "p", "party", "party mode"; for Continue accept "c", "continue") then route to the existing handlers advancedElicitationTask, partyModeWorkflow, or the save-and-proceed flow that writes to outputFile and loads nextStepFile; ensure unknown inputs fall through to the help/redisplay path and that redisplay logic still triggers after a completed branch.src/core/tasks/bmad-create-prd/steps-c/step-02c-executive-summary.md (1)
49-49: Avoid asking the model to expose internal analysis.Line 49 should be reframed to “brief rationale summary” rather than “show your analysis.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-02c-executive-summary.md` at line 49, Replace the line that currently reads "🎯 Show your analysis before taking any action" in step-02c-executive-summary.md with wording that avoids requesting internal chain-of-thought; change it to "🎯 Brief rationale summary" (or "Brief rationale summary:") so the step asks for a concise rationale rather than exposing internal analysis.src/core/tasks/bmad-create-prd/steps-c/step-03-success.md (1)
33-33: Analysis exposure directive should be narrowed.Line 33 should request a short external rationale, not internal analysis output.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-03-success.md` at line 33, Update the instruction text "🎯 Show your analysis before taking any action" to request a concise external rationale rather than internal chain-of-thought; replace it with a short prompt like "🎯 Provide a brief rationale for your planned action (concise, external justification — do not include internal chain-of-thought or detailed analysis)" so the file's guidance (step-03-success.md) asks for a short, external rationale only.src/core/tasks/bmad-create-prd/steps-c/step-02b-vision.md (2)
10-12: Party Mode target is path-coupled while other auxiliary flows are skill-coupled.Line 11 hardcodes a filesystem path, while Line 10 already uses
skill:indirection. This makes the step less portable across installer layouts and future workflow-to-skill migrations.Suggested consistency fix
- partyModeWorkflow: '{project-root}/_bmad/core/workflows/bmad-party-mode/workflow.md' + partyModeWorkflow: 'skill:bmad-party-mode'Also applies to: 117-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-02b-vision.md` around lines 10 - 12, The partyModeWorkflow entry is path-coupled while advancedElicitationTask uses the skill: indirection; update partyModeWorkflow to use the same skill indirection (e.g., replace the filesystem path '{project-root}/_bmad/core/workflows/bmad-party-mode/workflow.md' with a skill reference like 'skill:bmad-party-mode') so the step is portable and consistent with advancedElicitationTask; apply the same change to the other occurrences mentioned (lines 117-119) to keep all auxiliary flows skill-coupled.
50-50: “Show your analysis” instruction is unsafe as written.Line 50 explicitly asks for analysis exposure. Replace with concise user-facing rationale to avoid leaking internal reasoning traces.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-02b-vision.md` at line 50, The phrase "🎯 Show your analysis before taking any action" in step-02b-vision.md is asking for internal chain-of-thought; replace that line with a concise, user-facing instruction such as "🎯 Provide a brief, user-facing rationale for the chosen action" (or similar wording) that requests a short explanation of the action without exposing internal analysis or chain-of-thought; ensure the new text appears where the original bullet is to preserve flow.src/core/tasks/bmad-create-prd/templates/prd-template.md (1)
1-10: Template lacks deterministic section anchors for safe re-entry.Current scaffold is too thin for idempotent updates. Multiple steps append by headers; on retries, this can duplicate top-level sections and make later edits ambiguous.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/templates/prd-template.md` around lines 1 - 10, The PRD template lacks deterministic anchors, causing duplicate top-level sections on retries; update the template (prd-template.md) to include fixed, machine-readable section anchors/tokens for each major section (e.g., add HTML comment anchors or explicit IDs next to headers like "## Overview <!-- prd:overview -->" or "## Goals <!-- prd:goals -->") and ensure the frontmatter (stepsCompleted, inputDocuments) is used to track which anchored sections were populated so append/replace logic can target those anchors reliably instead of blindly appending headers.src/bmm/workflows/2-plan-workflows/create-prd/workflow-create-prd.md (1)
4-6: Deprecated workflow still advertises a likely-dead local step chain.Line 6 points to
./steps-c/step-01-init.mdeven though the active flow moved toskill:bmad-create-prd. Keeping this route in a non-standalone file creates a failure-prone fallback path for direct/manual invocations.Suggested hardening
--- name: create-prd description: 'Create a PRD from scratch. Use when the user says "lets create a product requirements document" or "I want to create a new PRD"' standalone: false main_config: '{project-root}/_bmad/bmm/config.yaml' -nextStep: './steps-c/step-01-init.md' --- @@ -## 2. Route to Create Workflow +## 2. Route to Create Workflow (Deprecated Wrapper) @@ -"**Create Mode: Creating a new PRD from scratch.**" +"**Create Mode: Creating a new PRD from scratch.**" -Read fully and follow: `{nextStep}` (steps-c/step-01-init.md) +This workflow is deprecated as an entrypoint. Read fully and follow: `skill:bmad-create-prd`Also applies to: 61-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/2-plan-workflows/create-prd/workflow-create-prd.md` around lines 4 - 6, The workflow file incorrectly points nextStep to the deprecated local chain ("./steps-c/step-01-init.md") creating a fallback path; update the nextStep entry to reference the active flow ("skill:bmad-create-prd") or remove the local nextStep and set standalone: true so the workflow uses the skill-based route, and mirror the same change for any other occurrences of nextStep/standalone later in the file to eliminate the stale local step chain.src/core/tasks/bmad-create-prd/steps-c/step-10-nonfunctional.md (1)
8-8: Unused frontmatter variable.Line 8 declares
purposeFilein frontmatter but it's never referenced in the step instructions. Either this variable should be loaded and used to guide the NFR elicitation, or it should be removed to avoid confusion.If
purposeFileis intended to provide context (likely src/core/tasks/bmad-create-prd/data/prd-purpose.md based on the directory structure), add an instruction to load it:### 1. Load NFR Guidance Read {purposeFile} to understand BMAD NFR quality standards and anti-patterns before beginning discovery.Otherwise, remove the unused declaration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/bmad-create-prd/steps-c/step-10-nonfunctional.md` at line 8, The frontmatter variable purposeFile in step-10-nonfunctional.md is declared but never used; either remove the declaration or add a clear step that loads and uses it to guide elicitation—specifically update the step content to include a prompt such as "Load and read {purposeFile} to understand BMAD NFR quality standards and anti-patterns before beginning discovery" so that purposeFile is actually referenced and informs the non-functional requirements elicitation.
🤖 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/core/tasks/bmad-create-prd/steps-c/step-01-init.md`:
- Around line 154-166: The step requires documentCounts but the frontmatter
contract only guarantees stepsCompleted and inputDocuments (template
prd-template.md), so add a stable contract: update the PRD template frontmatter
to include a documentCounts field (defaulting to an empty map/object) and modify
the step logic in step-01-init.md to validate that frontmatter.documentCounts
exists (create it if missing) before allowing the C/continue action; ensure the
code paths that append this step to stepsCompleted also update
frontmatter.documentCounts and persist the frontmatter so the subsequent read of
{nextStepFile} is safe.
- Around line 85-87: The current logic treats "if no stepsCompleted" as a fresh
setup and unconditionally copies the template over {outputFile}, which can
overwrite existing user-authored content lacking valid frontmatter; update the
code that handles the "if no stepsCompleted" branch (the Input Document
Discovery path that checks stepsCompleted/frontmatter) to first detect whether
{outputFile} exists and contains non-template/user content (e.g., check for any
content beyond valid frontmatter or a file modification date) and if so prompt
for confirmation or create a new file variant (e.g., {outputFile}.new) instead
of overwriting; ensure this guard is applied to the same logic referenced at the
second occurrence (lines noted as also applies to 117-118) so both code paths
that copy the template respect the confirmation/new-file safeguard rather than
silently overwriting.
- Around line 133-136: The report template in step-01-init.md references
brainstormingCount but no discovery rule populates it, causing inconsistent
reporting; either add a discovery rule that searches for brainstorming artifacts
and sets brainstormingCount (e.g., add a "brainstorming" pattern to the
discovery configuration that increments brainstormingCount when files match) or
remove the Brainstorming line from the template; update the discovery logic
where briefCount/researchCount/projectDocsCount are produced so it also produces
brainstormingCount (same naming) to keep template and state consistent.
- Line 74: The continuation gate currently looks only for the sentinel
"step-11-complete" and will misroute PRDs now that the workflow uses
"step-12-complete.md"; update the continuation check in the Continuation
Protocol to consider the new sentinel (e.g., check for "step-12-complete" or
accept either "step-11-complete" OR "step-12-complete") so completed documents
with the newer sentinel are not treated as incomplete. Ensure the check
references the exact sentinel string "step-12-complete" used in the repository.
In `@src/core/tasks/bmad-create-prd/steps-c/step-01b-continue.md`:
- Line 87: The check in step-01b-continue.md that tests the stepsCompleted array
for "step-11-complete.md" is using the wrong final-step name; update the
condition to look for "step-12-complete.md" instead so the workflow recognizes
completion (locate the stepsCompleted membership check in step-01b-continue.md
and replace the "step-11-complete.md" literal with "step-12-complete.md").
- Around line 74-77: The resume logic that reads the last element of
stepsCompleted and extracts nextStepFile must validate and handle errors: when
loading the last completed step file (the filename from stepsCompleted) wrap the
load/parse in a try/catch and handle file-not-found or parse errors; after
parsing frontmatter, explicitly check that nextStepFile exists and is a
non-empty string and if not throw/log a clear error or return a controlled
failure state; ensure any fallback behavior (e.g., start from a default step or
abort with diagnostics) is explicit and documented so functions that reference
stepsCompleted, lastCompletedStepFile, and nextStepFile never proceed with
undefined values.
- Around line 58-59: The doc references accessing the last element of the
stepsCompleted array without validating it; update the code that reads
stepsCompleted (the expression stepsCompleted[stepsCompleted.length - 1]) to
first ensure Array.isArray(stepsCompleted) and stepsCompleted.length > 0, and
handle the empty/non-array case (return a clear error, use a safe default, or
skip the step flow) so the code never attempts to index into an empty or invalid
array.
In `@src/core/tasks/bmad-create-prd/steps-c/step-02-discovery.md`:
- Line 93: Replace the non-standard Mustache/Handlebars conditional snippet
"{{if projectDocsCount > 0}}...{{else}}...{{/if}}" with an explicit instruction
the agent can execute: check the numeric variable projectDocsCount and, if > 0
output the brownfield sentence "This is a brownfield project - I'll focus on
understanding what you want to add or change.", otherwise output the greenfield
sentence "This is a greenfield project - I'll help you define the full product
vision."; ensure the rendered step text contains only the chosen sentence (no
raw template markers) and reference the variable projectDocsCount when
implementing the conditional logic.
In `@src/core/tasks/bmad-create-prd/steps-c/step-02b-vision.md`:
- Around line 50-53: The frontmatter update rule currently unconditionally
appends this step to stepsCompleted; change it to only append the current step
after the user explicitly selects "C" from the A/P/C menu (i.e., when vision
discovery is fully complete) so resume state isn't corrupted by partial A or P
flows; locate and modify the logic that performs the stepsCompleted update (the
frontmatter write/update for "stepsCompleted") to be executed inside the branch
that handles the "C" selection, and apply the same conditional change to the
other occurrences referenced (the similar stepsCompleted updates near the A/P/C
handling blocks).
In `@src/core/tasks/bmad-create-prd/steps-c/step-02c-executive-summary.md`:
- Around line 119-133: The current "C" branch always appends the markdown block
(including the headers "## Executive Summary" and "## Project Classification"),
causing duplicated sections on re-entry; change the behavior to upsert/replace
these sections instead of appending: when handling the 'C' selection, search the
document for existing "## Executive Summary" and "## Project Classification"
headers and replace the content between those headers (or replace the existing
header+block) with the new {vision_alignment_content},
{product_differentiator_content}, and {project_classification_content}; if a
header is missing, insert the corresponding section — update the code path that
constructs/appends the markdown so it performs header-aware replace (upsert)
rather than blind append.
- Around line 51-53: The frontmatter mutation currently described can be
misapplied during A/P loops; ensure the code that appends to stepsCompleted only
runs when the user explicitly selects the 'C' (Continue) menu option — do not
update stepsCompleted for 'A' (Amend) or 'P' (Preview) flows — and enforce that
the next-step loader is blocked until 'C' is chosen; update the documentation in
step-02c-executive-summary.md to state that stepsCompleted is mutated strictly
on 'C' selection and that loading the subsequent step is forbidden until that
choice is made, referencing the stepsCompleted field and the menu options 'C',
'A', and 'P'.
In `@src/core/tasks/bmad-create-prd/steps-c/step-03-success.md`:
- Around line 181-182: The C-path that appends the final content to {outputFile}
in step-03-success.md must be made idempotent: before appending, load the
existing file, parse its frontmatter and body, check the stepsCompleted array
and the presence of the target sections (e.g., "## Success Criteria" and "##
Product Scope"), and only append missing sections and add this step name to
stepsCompleted if they are not already present; then write back the updated
frontmatter and body and continue to {nextStepFile}; if any sections already
exist, skip appending them to avoid duplicates and still ensure the step name is
present in stepsCompleted.
In `@src/core/tasks/bmad-create-prd/steps-c/step-04-journeys.md`:
- Line 180: There is a contradiction between the "SUCCESS METRICS" header that
requires "Minimum 3-4 compelling narrative journeys covering different user
types" and the "JOURNEY TYPES TO ENSURE" section that lists five specific
journeys; reconcile by making one of two edits: either change the SUCCESS
METRICS line to read "Minimum 5 journey types as specified in JOURNEY TYPES TO
ENSURE section" so both sections require five, or change the "JOURNEY TYPES TO
ENSURE" header text from "Minimum Coverage" to "Recommended Coverage" and append
"(if applicable)" to the fifth journey "API/Integration" so it is optional;
update only the SUCCESS METRICS line or the JOURNEY TYPES TO ENSURE header and
the API/Integration line accordingly (refer to the headings "SUCCESS METRICS"
and "JOURNEY TYPES TO ENSURE" and the "API/Integration" journey to locate the
changes).
In `@src/core/tasks/bmad-create-prd/steps-c/step-05-domain.md`:
- Around line 84-87: The markdown presents menu option "D" ("Do domain
exploration anyway") but the runtime menu logic only handles A/P/C, making D
unreachable; either remove the "D" choice from the text or update the menu
handler to accept and route "D" into the domain exploration branch. Locate the
menu selection handling that maps choices A/P/C to their steps (the function or
switch that processes the A/P/C choices) and add a case for "D" that invokes the
same domain exploration flow as the existing domain path (or, if intentional to
skip, wire it to the Innovation step), or alternatively delete the "D" line from
the markdown so UI and logic remain consistent.
- Around line 102-104: Update the output field names in step-05-domain.md to
match the CSV schema: replace the requested typical_concerns with key_concerns
and remove or populate compliance_requirements (since the CSV has no dedicated
column) — either omit compliance_requirements from the YAML output or include it
with a null/empty value or derived content; ensure the instruction now returns:
domain, complexity, key_concerns (and compliance_requirements only if explicitly
derived or left empty).
In `@src/core/tasks/bmad-create-prd/steps-c/step-06-innovation.md`:
- Around line 66-72: The markdown step currently extracts web_search_triggers
from projectTypesCSV and later references it, but the declared return schema
omits web_search_triggers; update the step-06-innovation return schema to
include web_search_triggers (same type/format as extracted, e.g., string list or
semicolon-separated string), ensure the extraction logic that reads project_type
and pulls innovation_signals and web_search_triggers is preserved, and confirm
the downstream consumer (the step that uses web_search_triggers around the other
referenced lines) reads this field from the step output rather than assuming
implicit availability.
- Around line 13-15: The task reference partyModeWorkflow still uses an absolute
project-root path; update it to the relative/skill-style used elsewhere (e.g.,
replace "{project-root}/_bmad/core/workflows/bmad-party-mode/workflow.md" with
the skill-style reference consistent with advancedElicitationTask). Locate the
partyModeWorkflow entry in step-06-innovation.md and change it to the same
relative/skill format used by other tasks (match the pattern of
'skill:<skill-name>' or the repo's converted workflow reference style) so all
task references are migrated consistently.
In `@src/core/tasks/bmad-create-prd/steps-c/step-07-project-type.md`:
- Around line 62-63: The step uses the template key {{projectTypeFromStep02}}
which may be undefined; update the step logic (the place that renders or reads
step-07-project-type.md) to resolve project type with fallbacks and validation:
attempt keys in order projectTypeFromStep02, project_type, projectType and if
none exist fail early with a clear error/log entry; ensure the CSV lookup code
(the function or template renderer that matches "Find row where project_type
matches ..." or any caller of step-07-project-type.md) checks the resolved value
before performing the CSV match and returns a helpful error message indicating
the missing project-type state key.
- Around line 13-15: Replace the absolute project-root path in partyModeWorkflow
with the skill-relative reference: change partyModeWorkflow from
'{project-root}/_bmad/core/workflows/bmad-party-mode/workflow.md' to the skill
form (e.g. 'skill:bmad-party-mode/workflow.md') so it matches the migrated
pattern used by advancedElicitationTask and other entries.
---
Minor comments:
In `@src/core/tasks/bmad-create-prd/data/prd-purpose.md`:
- Line 3: Replace the typo in the PRD opening line: change "rhw BMad Method" to
"the BMAD Method" so the sentence reads "The PRD is the top of the required
funnel that feeds all subsequent product development work in the BMAD Method.";
update the sentence in prd-purpose.md accordingly.
- Around line 36-38: The two lines conflict about user story availability:
update the header string "Vision → Success Criteria → User Journeys → Functional
Requirements → (future: User Stories)" and the later line that reads "FRs → user
stories (1 FR could map to 1-3 stories potentially)" to be consistent; either
remove "(future:" from the header to indicate user story generation is currently
supported, or change the later "FRs → user stories..." phrasing to future tense
(e.g., "FRs → will map to user stories...") so both statements match. Locate and
edit the exact phrases "Vision → Success Criteria → User Journeys → Functional
Requirements → (future: User Stories)" and "FRs → user stories (1 FR could map
to 1-3 stories potentially)" in prd-purpose.md to make their tense/availability
consistent.
In `@src/core/tasks/bmad-create-prd/steps-c/step-01-init.md`:
- Line 16: The progress denominator in step-01-init.md ("Progress: Step 1 of
11") is inconsistent with step-11-polish.md ("Step 11 of 12"); update the "Step
1 of 11" text in src/core/tasks/bmad-create-prd/steps-c/step-01-init.md to use
the correct total (make it "Step 1 of 12" if the workflow has 12 steps) and then
scan all other step-*.md files (including step-11-polish.md) to ensure every
"Step X of Y" uses the same total so progress UI/checkpoint logic is consistent.
In `@src/core/tasks/bmad-create-prd/steps-c/step-01b-continue.md`:
- Around line 67-69: The reload step for inputDocuments lacks error handling;
update the document reload logic (the loop that iterates inputDocuments and
calls the loader such as loadDocument/reloadDocument) to wrap each load in a
try/catch, detect common failures (file not found/moved, permission denied,
corrupted read/parse), and on failure log a warning with the file id and error,
set a per-document status/metadata (e.g., reloaded: false, errorMessage: "...",
errorType: "Missing"|"Corrupted"), and continue processing the remaining
documents without attempting to auto-discover new files; optionally implement a
single configurable fallback lookup attempt (e.g.,
lookupFallbackPath(documentId)) before marking as missing, but do not change the
rule “Don't discover new documents.” Ensure the code uses the existing symbols
inputDocuments and the document loader function names to locate and modify the
reload loop.
In `@src/core/tasks/bmad-create-prd/steps-c/step-03-success.md`:
- Line 16: The progress counters in the markdown header text "**Progress: Step 3
of 11** - Next: User Journey Mapping" (and the duplicate occurrence later in the
file) are wrong for a 13-step flow; update those literal strings to "**Progress:
Step 3 of 13** - Next: User Journey Mapping" (and the second occurrence at the
other location) and search the file for any remaining "of 11" occurrences to
replace with "of 13" so the displayed step count matches the 13-step flow.
In `@src/core/tasks/bmad-create-prd/steps-c/step-04-journeys.md`:
- Around line 159-160: The acceptance prompts after running
{advancedElicitationTask} and {partyModeWorkflow} use "y/n" which conflicts with
the menu choice letters [A]/[P]/[C]; change those prompts to match the menu
convention — e.g. after processing the enhanced journeys, prompt "Accept these
improvements to the user journeys? (A = accept, C = keep original)" (and use the
same phrasing for partyModeWorkflow), handle A to update content and C to keep
original, then redisplay the menu so input format is consistent with the
existing [A]/[P]/[C] UI.
In `@src/core/tasks/bmad-create-prd/steps-c/step-05-domain.md`:
- Line 79: The domain label "education" used in step-05-domain.md does not match
the lookup key used in step-02/CSV which is "edtech", causing exact-match
lookups to fail; update the domain label "education" to the canonical key
"edtech" (or add a small mapping/alias handling so "education" maps to "edtech")
and ensure the lookup code/path that reads the CSV uses the same canonical
values; target the literal "education" in step-05-domain.md and the lookup logic
that expects exact matches to align them.
---
Nitpick comments:
In `@src/bmm/workflows/2-plan-workflows/create-prd/workflow-create-prd.md`:
- Around line 4-6: The workflow file incorrectly points nextStep to the
deprecated local chain ("./steps-c/step-01-init.md") creating a fallback path;
update the nextStep entry to reference the active flow ("skill:bmad-create-prd")
or remove the local nextStep and set standalone: true so the workflow uses the
skill-based route, and mirror the same change for any other occurrences of
nextStep/standalone later in the file to eliminate the stale local step chain.
In `@src/core/tasks/bmad-create-prd/steps-c/step-02b-vision.md`:
- Around line 10-12: The partyModeWorkflow entry is path-coupled while
advancedElicitationTask uses the skill: indirection; update partyModeWorkflow to
use the same skill indirection (e.g., replace the filesystem path
'{project-root}/_bmad/core/workflows/bmad-party-mode/workflow.md' with a skill
reference like 'skill:bmad-party-mode') so the step is portable and consistent
with advancedElicitationTask; apply the same change to the other occurrences
mentioned (lines 117-119) to keep all auxiliary flows skill-coupled.
- Line 50: The phrase "🎯 Show your analysis before taking any action" in
step-02b-vision.md is asking for internal chain-of-thought; replace that line
with a concise, user-facing instruction such as "🎯 Provide a brief, user-facing
rationale for the chosen action" (or similar wording) that requests a short
explanation of the action without exposing internal analysis or
chain-of-thought; ensure the new text appears where the original bullet is to
preserve flow.
In `@src/core/tasks/bmad-create-prd/steps-c/step-02c-executive-summary.md`:
- Line 49: Replace the line that currently reads "🎯 Show your analysis before
taking any action" in step-02c-executive-summary.md with wording that avoids
requesting internal chain-of-thought; change it to "🎯 Brief rationale summary"
(or "Brief rationale summary:") so the step asks for a concise rationale rather
than exposing internal analysis.
In `@src/core/tasks/bmad-create-prd/steps-c/step-03-success.md`:
- Line 33: Update the instruction text "🎯 Show your analysis before taking any
action" to request a concise external rationale rather than internal
chain-of-thought; replace it with a short prompt like "🎯 Provide a brief
rationale for your planned action (concise, external justification — do not
include internal chain-of-thought or detailed analysis)" so the file's guidance
(step-03-success.md) asks for a short, external rationale only.
In `@src/core/tasks/bmad-create-prd/steps-c/step-05-domain.md`:
- Around line 96-106: The prompt in step-05-domain.md is ambiguous for
deterministic tool invocation; update the block to explicitly call the CSV
lookup tool with exact parameters and expected schema instead of free-form
instructions — e.g., invoke a tool (name it clearly, e.g., lookupCsvRow) with
arguments: csv=domainComplexityCSV, matchField="domain",
matchValue={{domainFromStep02}},
fields=["domain","complexity","typical_concerns","compliance_requirements"],
outputFormat="yaml", and require the tool to return only the single YAML object;
remove the loose phrases like "Your task" and "Return ONLY" and replace them
with the concrete function call pattern so automation always produces the single
matching row.
- Around line 54-55: The frontmatter update currently appends this step name to
stepsCompleted in {planning_artifacts}/prd.md without validation; change the
continue handler (the logic that updates prd.md from step-05-domain.md) to first
read and parse YAML frontmatter, validate that a mutable stepsCompleted array
exists (or create it safely), then append this step id only if not present,
write back atomically (temp file + rename) and fail with a clear error if prd.md
has no frontmatter or unexpected shape; implement this as a reusable function
(e.g., updateStepsCompleted or ensureFrontmatterSchema) so callers can
validate/mutate other docs safely.
- Around line 170-176: The current append behavior in step-05-domain.md will
duplicate the "## Domain-Specific Requirements" block on every run; change the
operation that writes to {outputFile} so it is idempotent by performing an
upsert: read {outputFile}, detect an existing "## Domain-Specific Requirements"
heading and replace that entire section with the new "{{discovered domain
requirements}}" content, and only append the heading+content if no existing
section is found; reference the "## Domain-Specific Requirements" heading and
{outputFile} in your implementation so the write logic is a replace-or-append
rather than blind append.
- Around line 32-33: The two hard constraints lines "✅ YOU MUST ALWAYS SPEAK
OUTPUT In your Agent communication style with the config
`{communication_language}`" and "✅ YOU MUST ALWAYS WRITE all artifact and
document content in `{document_output_language}`" can deadlock when placeholders
are unset; update these rules to include a clear fallback (for example: use the
provided `{communication_language}`/`{document_output_language}` when set,
otherwise default to a specified language such as English) so the step continues
if variables are missing, and ensure the wording in the step (the two quoted
strings) explicitly documents the fallback behavior.
- Around line 155-162: The menu handler for the "Display" menu must normalize
user input before branching: update the parser that reads the Display options so
it trims whitespace and lowercases input, and match against accepted aliases for
each branch (e.g., for Advanced Elicitation accept "a", "A", "advanced",
"advanced elicitation"; for Party Mode accept "p", "party", "party mode"; for
Continue accept "c", "continue") then route to the existing handlers
advancedElicitationTask, partyModeWorkflow, or the save-and-proceed flow that
writes to outputFile and loads nextStepFile; ensure unknown inputs fall through
to the help/redisplay path and that redisplay logic still triggers after a
completed branch.
In `@src/core/tasks/bmad-create-prd/steps-c/step-10-nonfunctional.md`:
- Line 8: The frontmatter variable purposeFile in step-10-nonfunctional.md is
declared but never used; either remove the declaration or add a clear step that
loads and uses it to guide elicitation—specifically update the step content to
include a prompt such as "Load and read {purposeFile} to understand BMAD NFR
quality standards and anti-patterns before beginning discovery" so that
purposeFile is actually referenced and informs the non-functional requirements
elicitation.
In `@src/core/tasks/bmad-create-prd/templates/prd-template.md`:
- Around line 1-10: The PRD template lacks deterministic anchors, causing
duplicate top-level sections on retries; update the template (prd-template.md)
to include fixed, machine-readable section anchors/tokens for each major section
(e.g., add HTML comment anchors or explicit IDs next to headers like "##
Overview <!-- prd:overview -->" or "## Goals <!-- prd:goals -->") and ensure the
frontmatter (stepsCompleted, inputDocuments) is used to track which anchored
sections were populated so append/replace logic can target those anchors
reliably instead of blindly appending headers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0c65575b-93e4-447f-92a7-8bea612d8b27
⛔ Files ignored due to path filters (3)
src/bmm/module-help.csvis excluded by!**/*.csvsrc/core/tasks/bmad-create-prd/data/domain-complexity.csvis excluded by!**/*.csvsrc/core/tasks/bmad-create-prd/data/project-types.csvis excluded by!**/*.csv
📒 Files selected for processing (23)
src/bmm/agents/pm.agent.yamlsrc/bmm/workflows/2-plan-workflows/create-prd/bmad-skill-manifest.yamlsrc/bmm/workflows/2-plan-workflows/create-prd/workflow-create-prd.mdsrc/core/tasks/bmad-create-prd/SKILL.mdsrc/core/tasks/bmad-create-prd/bmad-skill-manifest.yamlsrc/core/tasks/bmad-create-prd/data/prd-purpose.mdsrc/core/tasks/bmad-create-prd/steps-c/step-01-init.mdsrc/core/tasks/bmad-create-prd/steps-c/step-01b-continue.mdsrc/core/tasks/bmad-create-prd/steps-c/step-02-discovery.mdsrc/core/tasks/bmad-create-prd/steps-c/step-02b-vision.mdsrc/core/tasks/bmad-create-prd/steps-c/step-02c-executive-summary.mdsrc/core/tasks/bmad-create-prd/steps-c/step-03-success.mdsrc/core/tasks/bmad-create-prd/steps-c/step-04-journeys.mdsrc/core/tasks/bmad-create-prd/steps-c/step-05-domain.mdsrc/core/tasks/bmad-create-prd/steps-c/step-06-innovation.mdsrc/core/tasks/bmad-create-prd/steps-c/step-07-project-type.mdsrc/core/tasks/bmad-create-prd/steps-c/step-08-scoping.mdsrc/core/tasks/bmad-create-prd/steps-c/step-09-functional.mdsrc/core/tasks/bmad-create-prd/steps-c/step-10-nonfunctional.mdsrc/core/tasks/bmad-create-prd/steps-c/step-11-polish.mdsrc/core/tasks/bmad-create-prd/steps-c/step-12-complete.mdsrc/core/tasks/bmad-create-prd/templates/prd-template.mdsrc/core/tasks/bmad-create-prd/workflow.md
💤 Files with no reviewable changes (1)
- src/bmm/workflows/2-plan-workflows/create-prd/bmad-skill-manifest.yaml
|
|
||
| ### 2. Handle Continuation (If Document Exists) | ||
|
|
||
| If the document exists and has frontmatter with `stepsCompleted` BUT `step-11-complete` is NOT in the list, follow the Continuation Protocol since the document is incomplete: |
There was a problem hiding this comment.
Completion sentinel is stale and can misroute completed workflows
The continuation gate checks for step-11-complete, but this workflow now includes step-12-complete.md. A completed PRD can be misclassified as incomplete and incorrectly routed into continuation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/tasks/bmad-create-prd/steps-c/step-01-init.md` at line 74, The
continuation gate currently looks only for the sentinel "step-11-complete" and
will misroute PRDs now that the workflow uses "step-12-complete.md"; update the
continuation check in the Continuation Protocol to consider the new sentinel
(e.g., check for "step-12-complete" or accept either "step-11-complete" OR
"step-12-complete") so completed documents with the newer sentinel are not
treated as incomplete. Ensure the check references the exact sentinel string
"step-12-complete" used in the repository.
| If no document exists or no `stepsCompleted` in frontmatter: | ||
|
|
||
| #### A. Input Document Discovery |
There was a problem hiding this comment.
Existing document can be overwritten without confirmation
if no stepsCompleted falls into fresh setup, then immediately copies template to {outputFile}. If {outputFile} already has user-authored content but missing/invalid frontmatter, this path destroys it.
Proposed guard
-If no document exists or no `stepsCompleted` in frontmatter:
+If no document exists:
+ proceed with fresh setup
+If document exists but `stepsCompleted` is missing/invalid:
+ pause and ask user whether to (a) back up and reinitialize, (b) continue manually, or (c) abortAlso applies to: 117-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/tasks/bmad-create-prd/steps-c/step-01-init.md` around lines 85 - 87,
The current logic treats "if no stepsCompleted" as a fresh setup and
unconditionally copies the template over {outputFile}, which can overwrite
existing user-authored content lacking valid frontmatter; update the code that
handles the "if no stepsCompleted" branch (the Input Document Discovery path
that checks stepsCompleted/frontmatter) to first detect whether {outputFile}
exists and contains non-template/user content (e.g., check for any content
beyond valid frontmatter or a file modification date) and if so prompt for
confirmation or create a new file variant (e.g., {outputFile}.new) instead of
overwriting; ensure this guard is applied to the same logic referenced at the
second occurrence (lines noted as also applies to 117-118) so both code paths
that copy the template respect the confirmation/new-file safeguard rather than
silently overwriting.
| - Product briefs: {{briefCount}} files {if briefCount > 0}✓ loaded{else}(none found){/if} | ||
| - Research: {{researchCount}} files {if researchCount > 0}✓ loaded{else}(none found){/if} | ||
| - Brainstorming: {{brainstormingCount}} files {if brainstormingCount > 0}✓ loaded{else}(none found){/if} | ||
| - Project docs: {{projectDocsCount}} files {if projectDocsCount > 0}✓ loaded (brownfield project){else}(none found - greenfield project){/if} |
There was a problem hiding this comment.
Report includes brainstormingCount without a corresponding discovery rule
The discovery list never defines a brainstorming search pattern, yet the setup report claims a brainstorming count. This creates inconsistent user reporting and state accounting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/tasks/bmad-create-prd/steps-c/step-01-init.md` around lines 133 -
136, The report template in step-01-init.md references brainstormingCount but no
discovery rule populates it, causing inconsistent reporting; either add a
discovery rule that searches for brainstorming artifacts and sets
brainstormingCount (e.g., add a "brainstorming" pattern to the discovery
configuration that increments brainstormingCount when files match) or remove the
Brainstorming line from the template; update the discovery logic where
briefCount/researchCount/projectDocsCount are produced so it also produces
brainstormingCount (same naming) to keep template and state consistent.
| - IF C: Update output file frontmatter, adding this step name to the end of the list of stepsCompleted, then read fully and follow: {nextStepFile} | ||
| - IF user provides additional files: Load them, update inputDocuments and documentCounts, redisplay report | ||
| - IF user asks questions: Answer and redisplay menu | ||
|
|
||
| #### EXECUTION RULES: | ||
|
|
||
| - ALWAYS halt and wait for user input after presenting menu | ||
| - ONLY proceed to next step when user selects 'C' | ||
|
|
||
| ## CRITICAL STEP COMPLETION NOTE | ||
|
|
||
| ONLY WHEN [C continue option] is selected and [frontmatter properly updated with this step added to stepsCompleted and documentCounts], will you then read fully and follow: `{nextStepFile}` to begin project discovery. | ||
|
|
There was a problem hiding this comment.
documentCounts is required but never contract-defined
You require updating documentCounts before continuation, but the template only guarantees stepsCompleted and inputDocuments (src/core/tasks/bmad-create-prd/templates/prd-template.md:1-4). This creates a brittle state contract across steps.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/tasks/bmad-create-prd/steps-c/step-01-init.md` around lines 154 -
166, The step requires documentCounts but the frontmatter contract only
guarantees stepsCompleted and inputDocuments (template prd-template.md), so add
a stable contract: update the PRD template frontmatter to include a
documentCounts field (defaulting to an empty map/object) and modify the step
logic in step-01-init.md to validate that frontmatter.documentCounts exists
(create it if missing) before allowing the C/continue action; ensure the code
paths that append this step to stepsCompleted also update
frontmatter.documentCounts and persist the frontmatter so the subsequent read of
{nextStepFile} is safe.
| - `stepsCompleted`: Array of completed step filenames | ||
| - Last element of `stepsCompleted` array: The most recently completed step |
There was a problem hiding this comment.
No array bounds check before accessing last element.
Lines 58-59 instruct the agent to access the last element of the stepsCompleted array, but there's no validation that the array is non-empty. If the array is empty (corrupted frontmatter or initialization error), this will cause undefined behavior.
🛡️ Add bounds check
**State Assessment:**
Review the frontmatter to understand:
- `stepsCompleted`: Array of completed step filenames
+ - If array is empty, this is a corrupted state - inform user and suggest restarting from step-01-init.md
- Last element of `stepsCompleted` array: The most recently completed step📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - `stepsCompleted`: Array of completed step filenames | |
| - Last element of `stepsCompleted` array: The most recently completed step | |
| **State Assessment:** | |
| Review the frontmatter to understand: | |
| - `stepsCompleted`: Array of completed step filenames | |
| - If array is empty, this is a corrupted state - inform user and suggest restarting from step-01-init.md | |
| - Last element of `stepsCompleted` array: The most recently completed step |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/tasks/bmad-create-prd/steps-c/step-01b-continue.md` around lines 58
- 59, The doc references accessing the last element of the stepsCompleted array
without validating it; update the code that reads stepsCompleted (the expression
stepsCompleted[stepsCompleted.length - 1]) to first ensure
Array.isArray(stepsCompleted) and stepsCompleted.length > 0, and handle the
empty/non-array case (return a clear error, use a safe default, or skip the step
flow) so the code never attempts to index into an empty or invalid array.
| Return ONLY the matching row as a YAML-formatted object with these fields: | ||
| domain, complexity, typical_concerns, compliance_requirements | ||
|
|
There was a problem hiding this comment.
Requested lookup fields do not match actual CSV schema
Lines 102-104 require typical_concerns and compliance_requirements, but the referenced CSV exposes key_concerns and has no dedicated compliance_requirements column. This will fail extraction.
Proposed fix
Return ONLY the matching row as a YAML-formatted object with these fields:
-domain, complexity, typical_concerns, compliance_requirements
+domain, complexity, key_concerns, required_knowledge, special_sections🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/tasks/bmad-create-prd/steps-c/step-05-domain.md` around lines 102 -
104, Update the output field names in step-05-domain.md to match the CSV schema:
replace the requested typical_concerns with key_concerns and remove or populate
compliance_requirements (since the CSV has no dedicated column) — either omit
compliance_requirements from the YAML output or include it with a null/empty
value or derived content; ensure the instruction now returns: domain,
complexity, key_concerns (and compliance_requirements only if explicitly derived
or left empty).
| advancedElicitationTask: 'skill:bmad-advanced-elicitation' | ||
| partyModeWorkflow: '{project-root}/_bmad/core/workflows/bmad-party-mode/workflow.md' | ||
| --- |
There was a problem hiding this comment.
Path migration is incomplete in task references
partyModeWorkflow still uses an absolute {project-root}/_bmad/... path instead of the relative/skill style adopted in this conversion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/tasks/bmad-create-prd/steps-c/step-06-innovation.md` around lines 13
- 15, The task reference partyModeWorkflow still uses an absolute project-root
path; update it to the relative/skill-style used elsewhere (e.g., replace
"{project-root}/_bmad/core/workflows/bmad-party-mode/workflow.md" with the
skill-style reference consistent with advancedElicitationTask). Locate the
partyModeWorkflow entry in step-06-innovation.md and change it to the same
relative/skill format used by other tasks (match the pattern of
'skill:<skill-name>' or the repo's converted workflow reference style) so all
task references are migrated consistently.
| Load innovation signals specific to this project type: | ||
|
|
||
| - Load `{projectTypesCSV}` completely | ||
| - Find the row where `project_type` matches detected type from step-02 | ||
| - Extract `innovation_signals` (semicolon-separated list) | ||
| - Extract `web_search_triggers` for potential innovation research | ||
|
|
There was a problem hiding this comment.
Data contract is internally inconsistent for web_search_triggers
Step 1 says to extract web_search_triggers, and Step 4 uses it, but the declared return schema omits it. The downstream research instruction can fail because the field may never be returned.
Also applies to: 117-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/tasks/bmad-create-prd/steps-c/step-06-innovation.md` around lines 66
- 72, The markdown step currently extracts web_search_triggers from
projectTypesCSV and later references it, but the declared return schema omits
web_search_triggers; update the step-06-innovation return schema to include
web_search_triggers (same type/format as extracted, e.g., string list or
semicolon-separated string), ensure the extraction logic that reads project_type
and pulls innovation_signals and web_search_triggers is preserved, and confirm
the downstream consumer (the step that uses web_search_triggers around the other
referenced lines) reads this field from the step output rather than assuming
implicit availability.
| advancedElicitationTask: 'skill:bmad-advanced-elicitation' | ||
| partyModeWorkflow: '{project-root}/_bmad/core/workflows/bmad-party-mode/workflow.md' | ||
| --- |
There was a problem hiding this comment.
Absolute party-mode reference is still present
partyModeWorkflow wasn’t converted to relative/skill form, leaving this step inconsistent with the migration objective.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/tasks/bmad-create-prd/steps-c/step-07-project-type.md` around lines
13 - 15, Replace the absolute project-root path in partyModeWorkflow with the
skill-relative reference: change partyModeWorkflow from
'{project-root}/_bmad/core/workflows/bmad-party-mode/workflow.md' to the skill
form (e.g. 'skill:bmad-party-mode/workflow.md') so it matches the migrated
pattern used by advancedElicitationTask and other entries.
| - Find row where project_type matches {{projectTypeFromStep02}} | ||
|
|
There was a problem hiding this comment.
Cross-step state key is undefined and may fail lookup
{{projectTypeFromStep02}} is used as if guaranteed, but this step never defines fallback keys (project_type, projectType, etc.). A key mismatch means no CSV match and degraded behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/tasks/bmad-create-prd/steps-c/step-07-project-type.md` around lines
62 - 63, The step uses the template key {{projectTypeFromStep02}} which may be
undefined; update the step logic (the place that renders or reads
step-07-project-type.md) to resolve project type with fallbacks and validation:
attempt keys in order projectTypeFromStep02, project_type, projectType and if
none exist fail early with a clear error/log entry; ensure the CSV lookup code
(the function or template renderer that matches "Find row where project_type
matches ..." or any caller of step-07-project-type.md) checks the resolved value
before performing the CSV match and returns a helpful error message indicating
the missing project-type state key.
1bc0f8d to
844f8a5
Compare
Move the create-prd workflow into src/core/tasks/bmad-create-prd/ as a
self-contained native skill with SKILL.md, workflow.md, steps-c/, data/,
and templates/. Update all internal path references from absolute
{project-root}/_bmad/... to relative paths. Mark the source
workflow-create-prd.md as standalone:false to prevent duplicate manifest
entries. Update pm.agent.yaml and module-help.csv to use skill:bmad-create-prd.
Strip name/description from all step frontmatter (STEP-06), remove intra-skill path variables and inline them (PATH-04), move outputFile to workflow.md (WF-03), fix stale step-11-complete refs (REF-02), fix product_knowledge typo (REF-01), rewrite continuation logic to use a lookup table, and strip legacy source workflow frontmatter. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
844f8a5 to
b42f1e7
Compare
Summary
create-prdworkflow fromsrc/bmm/workflows/2-plan-workflows/create-prd/workflow-create-prd.mdinto a self-contained native skill atsrc/core/tasks/bmad-create-prd/{project-root}/_bmad/bmm/workflows/...path references in step files to relative paths (./,../data/, etc.)pm.agent.yamlandmodule-help.csvto referenceskill:bmad-create-prdinstead of the old workflow pathworkflow-create-prd.mdasstandalone: falseand remove its entry from the sharedbmad-skill-manifest.yamlto prevent duplicate manifest entriesvalidationFlowcross-reference instep-12-complete.md(points tosteps-v/which belongs to the separate validate-prd workflow)New skill structure
Verification
.claude/skills/bmad-create-prd/with all expected filesskill-manifest.csvcontainsbmad-create-prdentryworkflow-manifest.csvno longer contains acreate-prdrowTest plan
bmad-create-prdskill installs correctly on a fresh project