Conversation
🤖 Augment PR SummarySummary: This PR converts the BMM domain research workflow into a first-class skill and fixes metadata references. Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
| - **CONTENT ALREADY WRITTEN TO DOCUMENT** | ||
| - Update frontmatter: `stepsCompleted: [1, 2, 3, 4, 5]` | ||
| - Load: `{project-root}/_bmad/bmm/workflows/1-analysis/research/domain-steps/step-06-research-synthesis.md` | ||
| - Load: `./step-06-research-synthesis.md` |
There was a problem hiding this comment.
Minor workflow-state inconsistency: in step-06-research-synthesis.md the completion block updates stepsCompleted to [1, 2, 3, 4, 5], which seems like it should include step 6 and could break resume/completion logic.
Severity: medium
Other Locations
src/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-06-research-synthesis.md:378
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
📝 WalkthroughWalkthroughConverts the domain-research workflow into a native skill by adding SKILL.md and a skill-type manifest. Updates the analyst agent to reference the new skill instead of the workflow. Converts absolute file paths to relative paths across domain research steps. Removes the workflow entry from the research manifest and adds a new research template file. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-05-technical-trends.md (1)
226-233:⚠️ Potential issue | 🟠 MajorContradictory completion language vs. step-06 reference.
Lines 226-233 state "Research workflow complete" and list next actions as if research is finished, but line 177 explicitly loads step-06 for research synthesis. This inconsistency will confuse the executing agent.
If step-06 exists, update this section to remove the premature "complete" language. If step-06 is intended to be removed, update line 177 to mark the workflow as complete without loading another step.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-05-technical-trends.md` around lines 226 - 233, The "## NEXT STEPS:" block in step-05-technical-trends.md currently says "Research workflow complete" but the file also references loading step-06 (research synthesis); resolve the contradiction by either removing the premature "Research workflow complete" wording and replacing it with a transitional line that indicates "Proceed to step-06: Research synthesis" (or similar) so the agent continues, or if step-06 should not run, remove the step-06 load reference and keep the "Research workflow complete" language; update the "## NEXT STEPS:" paragraph and the place that mentions loading step-06 so both consistently indicate whether the workflow continues to step-06 or is finished.
🧹 Nitpick comments (4)
src/bmm/agents/analyst.agent.yaml (1)
29-31: Skill reference looks correct, but note inconsistency with sibling workflows.The
skill:bmad-domain-researchreference correctly matches the skill directory name and will resolve via the manifest discovery. However, the sibling research workflows (MR at line 26, TR at line 34) still use absolute{project-root}paths.If these will eventually be converted to skills as well, consider tracking this as a follow-up to maintain consistent invocation patterns.
,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/agents/analyst.agent.yaml` around lines 29 - 31, The exec entry for the DR workflow correctly uses the skill invocation "skill:bmad-domain-research"; update the sibling MR and TR workflows so their exec fields use the equivalent skill-style references (e.g., "skill:..." matching each skill directory name) instead of absolute {project-root} paths, or add a follow-up task to convert MR and TR to skills and standardize all exec entries; look for the MR and TR trigger blocks and their exec fields in this YAML to make the change.src/bmm/workflows/1-analysis/research/bmad-domain-research/research.template.md (3)
4-4: HardcodedworkflowType: 'research'reduces template reusability.If this template is ever adapted for a different workflow type (e.g.,
investigation,audit,analysis), the hardcoded'research'literal creates friction. Should be a template variable{{workflowType}}or{{workflow_type}}with a default value.♻️ Proposed refactor
-workflowType: 'research' +workflowType: '{{workflow_type}}'Then ensure the invoking workflow sets
workflow_type: 'research'as a default parameter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/1-analysis/research/bmad-domain-research/research.template.md` at line 4, The template currently hardcodes workflowType: 'research', reducing reusability; change it to a template variable like {{workflowType}} or {{workflow_type}} with a sensible default (e.g., 'research') so callers can override it, and update any invoking workflow to pass workflow_type: 'research' as the default parameter; locate the hardcoded literal workflowType in the template (and any references that expect 'research') and replace with the chosen template variable, ensuring the consuming workflow sets the corresponding default parameter.
1-29: No atomic write or idempotency guidance for template population.Based on learnings, BMAD-METHOD templates should address atomic writes, idempotent saves, and sensitive data redaction. This template provides none of that:
- When a step appends content, is it atomic (write to temp file, then rename)? Or do partial writes corrupt the file if the step crashes?
- If a user re-runs a step, does content get duplicated? Should steps check
stepsCompletedbefore appending?- Should
user_name,research_topic, or other fields be redacted from logs or sensitive contexts?These operational concerns should be documented in a template header comment or referenced external guide.
Based on learnings from n-zhi (PR
#1817): Templates should specify atomic write semantics, idempotent step execution, and sensitive key redaction.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/1-analysis/research/bmad-domain-research/research.template.md` around lines 1 - 29, The template lacks operational guidance for atomic writes, idempotency, and sensitive-data redaction; update the template header (near the front of the file containing stepsCompleted, inputDocuments, web_research_enabled, source_verification, user_name, research_topic, research_goals, date) to state: write operations must be atomic (write to a temp file then rename), steps must be idempotent (each step should check and update stepsCompleted to avoid duplicate appends on re-run), and list which keys are sensitive (user_name, research_topic, research_goals, inputDocuments) with required redaction/handling rules or a link to the central guide (e.g., “see BMAD-METHOD operational guide”) so implementers know how to perform safe appends and redact sensitive fields.
29-29: Non-specific comment provides zero operational guidance."Content will be appended sequentially through research workflow steps" tells us nothing useful:
- Which steps? Steps 1-6? Specific step numbers?
- What content? Findings? References? Analysis?
- Where exactly? After this comment? Before it? In specific sections?
- What's the append mechanism? Markdown concatenation? Section insertion?
This comment is write-only documentation that will never help anyone debug a malformed output.
📝 More specific comment
-<!-- Content will be appended sequentially through research workflow steps --> +<!-- Steps 2-5 append their findings sections below this line. Step 6 replaces the Research Overview placeholder and adds Research Synthesis. -->🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/1-analysis/research/bmad-domain-research/research.template.md` at line 29, The template comment "Content will be appended sequentially through research workflow steps" in research.template.md is too vague; replace it with a concrete template instruction that specifies which workflow steps (e.g., steps 1–6 or named steps), what kinds of content to append (findings, references, citations, analysis), exactly where to insert content (e.g., under a specific heading or placeholder like "## Step {n} Results"), and the append mechanism (e.g., markdown section insertion, replace placeholder tokens like {{step-1}}, or append at EOF). Update the text around the placeholder in the template so consumers and automation know the expected content format, required metadata (author, date, source), and insertion rules (order, delimiter), referencing the template file research.template.md and the placeholder comment to locate the change.
🤖 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/1-analysis/research/bmad-domain-research/research.template.md`:
- Around line 1-13: Add a schema_version (or template_version) field to the
frontmatter of research.template.md (alongside keys like stepsCompleted,
inputDocuments, workflowType, research_type, research_topic, etc.) and set a
sensible default (e.g., schema_version: 1). Update any code that generates or
consumes this template to read and validate schema_version and to increment it
whenever the template/frontmatter changes, and add a short migration
note/comments so future schema changes include a version bump and migration
path.
- Line 5: The template sets lastStep: 1 while stepsCompleted: [] which is
contradictory; update the template so lastStep reflects no progress—either set
lastStep: 0 or remove the lastStep field until the first step is completed, and
ensure any code that reads lastStep (e.g., consumers that expect a 0-based
sentinel) treats missing lastStep as 0 to avoid misinterpreting step 1 as
completed; change the value in the research.template.md to keep lastStep
consistent with stepsCompleted.
- Around line 21-29: The template only declares "Research Overview" and lacks a
placeholder for the required "Research Synthesis" (which
step-06-research-synthesis.md expects), so update research.template.md to
include explicit section placeholders/headings such as "## Research Synthesis"
(for the executive summary) and other expected slots (e.g., "## Findings", "##
Methods", "## Recommendations") or clear HTML comments where steps will append
content; ensure the new "## Research Synthesis" heading is present and labelled
clearly so automated steps can locate and append the full executive summary.
- Line 2: The final completion action under "If 'C' (Complete Research)"
incorrectly omits step 6 from the stepsCompleted array; update the YAML entry
stepsCompleted in that completion block so it matches the EXECUTION PROTOCOLS
pattern and includes 6 (i.e., stepsCompleted: [1, 2, 3, 4, 5, 6]) to ensure step
6 marks itself as completed; locate the block labeled "If 'C' (Complete
Research)" in research.template.md and change the stepsCompleted array there
accordingly.
---
Outside diff comments:
In
`@src/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-05-technical-trends.md`:
- Around line 226-233: The "## NEXT STEPS:" block in step-05-technical-trends.md
currently says "Research workflow complete" but the file also references loading
step-06 (research synthesis); resolve the contradiction by either removing the
premature "Research workflow complete" wording and replacing it with a
transitional line that indicates "Proceed to step-06: Research synthesis" (or
similar) so the agent continues, or if step-06 should not run, remove the
step-06 load reference and keep the "Research workflow complete" language;
update the "## NEXT STEPS:" paragraph and the place that mentions loading
step-06 so both consistently indicate whether the workflow continues to step-06
or is finished.
---
Nitpick comments:
In `@src/bmm/agents/analyst.agent.yaml`:
- Around line 29-31: The exec entry for the DR workflow correctly uses the skill
invocation "skill:bmad-domain-research"; update the sibling MR and TR workflows
so their exec fields use the equivalent skill-style references (e.g.,
"skill:..." matching each skill directory name) instead of absolute
{project-root} paths, or add a follow-up task to convert MR and TR to skills and
standardize all exec entries; look for the MR and TR trigger blocks and their
exec fields in this YAML to make the change.
In
`@src/bmm/workflows/1-analysis/research/bmad-domain-research/research.template.md`:
- Line 4: The template currently hardcodes workflowType: 'research', reducing
reusability; change it to a template variable like {{workflowType}} or
{{workflow_type}} with a sensible default (e.g., 'research') so callers can
override it, and update any invoking workflow to pass workflow_type: 'research'
as the default parameter; locate the hardcoded literal workflowType in the
template (and any references that expect 'research') and replace with the chosen
template variable, ensuring the consuming workflow sets the corresponding
default parameter.
- Around line 1-29: The template lacks operational guidance for atomic writes,
idempotency, and sensitive-data redaction; update the template header (near the
front of the file containing stepsCompleted, inputDocuments,
web_research_enabled, source_verification, user_name, research_topic,
research_goals, date) to state: write operations must be atomic (write to a temp
file then rename), steps must be idempotent (each step should check and update
stepsCompleted to avoid duplicate appends on re-run), and list which keys are
sensitive (user_name, research_topic, research_goals, inputDocuments) with
required redaction/handling rules or a link to the central guide (e.g., “see
BMAD-METHOD operational guide”) so implementers know how to perform safe appends
and redact sensitive fields.
- Line 29: The template comment "Content will be appended sequentially through
research workflow steps" in research.template.md is too vague; replace it with a
concrete template instruction that specifies which workflow steps (e.g., steps
1–6 or named steps), what kinds of content to append (findings, references,
citations, analysis), exactly where to insert content (e.g., under a specific
heading or placeholder like "## Step {n} Results"), and the append mechanism
(e.g., markdown section insertion, replace placeholder tokens like {{step-1}},
or append at EOF). Update the text around the placeholder in the template so
consumers and automation know the expected content format, required metadata
(author, date, source), and insertion rules (order, delimiter), referencing the
template file research.template.md and the placeholder comment to locate the
change.
🪄 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: 9dafec59-7b40-4174-81f6-0e02fa2c5a13
⛔ Files ignored due to path filters (1)
src/bmm/module-help.csvis excluded by!**/*.csv
📒 Files selected for processing (12)
src/bmm/agents/analyst.agent.yamlsrc/bmm/workflows/1-analysis/research/bmad-domain-research/SKILL.mdsrc/bmm/workflows/1-analysis/research/bmad-domain-research/bmad-skill-manifest.yamlsrc/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-01-init.mdsrc/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-02-domain-analysis.mdsrc/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-03-competitive-landscape.mdsrc/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-04-regulatory-focus.mdsrc/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-05-technical-trends.mdsrc/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-06-research-synthesis.mdsrc/bmm/workflows/1-analysis/research/bmad-domain-research/research.template.mdsrc/bmm/workflows/1-analysis/research/bmad-domain-research/workflow.mdsrc/bmm/workflows/1-analysis/research/bmad-skill-manifest.yaml
💤 Files with no reviewable changes (2)
- src/bmm/workflows/1-analysis/research/bmad-domain-research/workflow.md
- src/bmm/workflows/1-analysis/research/bmad-skill-manifest.yaml
| --- | ||
| stepsCompleted: [] | ||
| inputDocuments: [] | ||
| workflowType: 'research' | ||
| lastStep: 1 | ||
| research_type: '{{research_type}}' | ||
| research_topic: '{{research_topic}}' | ||
| research_goals: '{{research_goals}}' | ||
| user_name: '{{user_name}}' | ||
| date: '{{date}}' | ||
| web_research_enabled: true | ||
| source_verification: true | ||
| --- |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
No schema version field for template frontmatter.
This template defines a frontmatter schema with specific keys (stepsCompleted, inputDocuments, workflowType, etc.), but there's no schema_version or template_version field. When the schema evolves—new required keys, renamed fields, changed semantics—there's no way to distinguish which version of the template a given research file was generated from. This will break backward compatibility and make migrations painful.
➕ Proposed addition
---
+template_version: '1.0.0'
stepsCompleted: []📝 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: [] | |
| inputDocuments: [] | |
| workflowType: 'research' | |
| lastStep: 1 | |
| research_type: '{{research_type}}' | |
| research_topic: '{{research_topic}}' | |
| research_goals: '{{research_goals}}' | |
| user_name: '{{user_name}}' | |
| date: '{{date}}' | |
| web_research_enabled: true | |
| source_verification: true | |
| --- | |
| --- | |
| template_version: '1.0.0' | |
| stepsCompleted: [] | |
| inputDocuments: [] | |
| workflowType: 'research' | |
| lastStep: 1 | |
| research_type: '{{research_type}}' | |
| research_topic: '{{research_topic}}' | |
| research_goals: '{{research_goals}}' | |
| user_name: '{{user_name}}' | |
| date: '{{date}}' | |
| web_research_enabled: true | |
| source_verification: true | |
| --- |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/bmm/workflows/1-analysis/research/bmad-domain-research/research.template.md`
around lines 1 - 13, Add a schema_version (or template_version) field to the
frontmatter of research.template.md (alongside keys like stepsCompleted,
inputDocuments, workflowType, research_type, research_topic, etc.) and set a
sensible default (e.g., schema_version: 1). Update any code that generates or
consumes this template to read and validate schema_version and to increment it
whenever the template/frontmatter changes, and add a short migration
note/comments so future schema changes include a version bump and migration
path.
| @@ -0,0 +1,29 @@ | |||
| --- | |||
| stepsCompleted: [] | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify stepsCompleted inconsistency in step-06-research-synthesis.md
rg -n -C3 'stepsCompleted.*\[1.*2.*3.*4.*5' src/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-06-research-synthesis.mdRepository: bmad-code-org/BMAD-METHOD
Length of output: 1032
🏁 Script executed:
cat -n src/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-06-research-synthesis.md | sed -n '15,30p'Repository: bmad-code-org/BMAD-METHOD
Length of output: 930
🏁 Script executed:
cat -n src/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-06-research-synthesis.md | sed -n '370,385p'Repository: bmad-code-org/BMAD-METHOD
Length of output: 1111
🏁 Script executed:
cat -n src/bmm/workflows/1-analysis/research/bmad-domain-research/research.template.md | head -5Repository: bmad-code-org/BMAD-METHOD
Length of output: 182
🏁 Script executed:
# Check other steps to see if they follow a similar pattern with stepsCompleted
rg -n 'stepsCompleted' src/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/ --no-heading | head -20Repository: bmad-code-org/BMAD-METHOD
Length of output: 2076
🏁 Script executed:
rg -n -i 'continue|Continue' src/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-06-research-synthesis.mdRepository: bmad-code-org/BMAD-METHOD
Length of output: 188
🏁 Script executed:
cat -n src/bmm/workflows/1-analysis/research/bmad-domain-research/domain-steps/step-06-research-synthesis.md | sed -n '360,385p'Repository: bmad-code-org/BMAD-METHOD
Length of output: 1489
Step 6 omits itself from stepsCompleted during final completion.
Line 21 (EXECUTION PROTOCOLS) correctly specifies stepsCompleted: [1, 2, 3, 4, 5, 6], but line 378 (final completion action under "If 'C' (Complete Research)") incorrectly writes stepsCompleted: [1, 2, 3, 4, 5]. This breaks the established pattern where steps 1–5 each include themselves in stepsCompleted upon completion, causing workflow state tracking to be incomplete when step 6 finishes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/bmm/workflows/1-analysis/research/bmad-domain-research/research.template.md`
at line 2, The final completion action under "If 'C' (Complete Research)"
incorrectly omits step 6 from the stepsCompleted array; update the YAML entry
stepsCompleted in that completion block so it matches the EXECUTION PROTOCOLS
pattern and includes 6 (i.e., stepsCompleted: [1, 2, 3, 4, 5, 6]) to ensure step
6 marks itself as completed; locate the block labeled "If 'C' (Complete
Research)" in research.template.md and change the stepsCompleted array there
accordingly.
| stepsCompleted: [] | ||
| inputDocuments: [] | ||
| workflowType: 'research' | ||
| lastStep: 1 |
There was a problem hiding this comment.
Contradictory initial state: lastStep: 1 with empty stepsCompleted.
If no steps have been completed yet (stepsCompleted: []), why is lastStep: 1? This creates ambiguity about whether step 1 has been executed. Should initialize to lastStep: 0 or omit this field until the first step completes.
🔧 Proposed fix
-lastStep: 1
+lastStep: 0📝 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.
| lastStep: 1 | |
| version: '1.0' | |
| workflowType: 'research' | |
| stepsCompleted: [] | |
| lastStep: 0 | |
| web_research_enabled: true | |
| source_verification: true | |
| inputDocuments: [] | |
| research_date: '{{date}}' | |
| research_topic: '{{research_topic}}' | |
| researcher_notes: '' | |
| summary_status: 'pending' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/bmm/workflows/1-analysis/research/bmad-domain-research/research.template.md`
at line 5, The template sets lastStep: 1 while stepsCompleted: [] which is
contradictory; update the template so lastStep reflects no progress—either set
lastStep: 0 or remove the lastStep field until the first step is completed, and
ensure any code that reads lastStep (e.g., consumers that expect a 0-based
sentinel) treats missing lastStep as 0 to avoid misinterpreting step 1 as
completed; change the value in the research.template.md to keep lastStep
consistent with stepsCompleted.
| --- | ||
|
|
||
| ## Research Overview | ||
|
|
||
| [Research overview and methodology will be appended here] | ||
|
|
||
| --- | ||
|
|
||
| <!-- Content will be appended sequentially through research workflow steps --> |
There was a problem hiding this comment.
Incomplete document structure: missing expected sections.
The template defines only a "Research Overview" section, but according to the context snippet from step-06-research-synthesis.md, the workflow expects a "Research Synthesis" section that contains the full executive summary. Where is this section in the template?
If steps are supposed to append sections dynamically, the template should define section placeholders or comments indicating where each section will be inserted. Otherwise, the document structure is undefined and fragile.
📋 Suggested structure additions
## Research Overview
[Research overview and methodology will be appended here]
+---
+
+## Research Synthesis
+
+[Executive summary and synthesis will be appended by step 6]
+
---
<!-- Content will be appended sequentially through research workflow steps -->🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/bmm/workflows/1-analysis/research/bmad-domain-research/research.template.md`
around lines 21 - 29, The template only declares "Research Overview" and lacks a
placeholder for the required "Research Synthesis" (which
step-06-research-synthesis.md expects), so update research.template.md to
include explicit section placeholders/headings such as "## Research Synthesis"
(for the executive summary) and other expected slots (e.g., "## Findings", "##
Methods", "## Recommendations") or clear HTML comments where steps will append
content; ensure the new "## Research Synthesis" heading is present and labelled
clearly so automated steps can locate and append the full executive summary.
|
Closing as duplicate of #1928. This patch was pushed to the existing branch and is tracked there. |
Summary
skill:bmad-domain-research/Users/alex/src/bmad/_bmadand verify the installed skill outputValidation
node tools/cli/bmad-cli.js install --directory /Users/alex/src/bmad --modules bmm --tools claude-code --yesnpm ci && npm run format:check && npm run lint && npm run lint:md && npm run docs:build && npm run validate:schemas && npm run test:schemas && npm run test:install && npm run validate:refs