feat: add monorepo support with dynamic project context#1691
feat: add monorepo support with dynamic project context#1691serrnovik wants to merge 17 commits intobmad-code-org:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds monorepo support: new set-project workflow, centralized monorepo context logic, installer/template placeholder injection, many workflow docs updated to load {main_config} and conditionally override output paths via _bmad/.current_project, plus new validation tests and installer wiring. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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: 13
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (32)
tools/cli/installers/lib/ide/github-copilot.js (8)
688-692:⚠️ Potential issue | 🟡 Minor
fs.writeFileused directly instead ofthis.writeFile, inconsistent with the rest of the classEvery other write operation in this file uses
this.writeFile(...), but the cleanup path incleanupCopilotInstructionsbypasses it. Whateverthis.writeFileabstracts (logging, error handling, directory creation) is silently skipped here.🐛 Proposed fix
- await fs.writeFile(instructionsPath, cleaned, 'utf8'); + await this.writeFile(instructionsPath, cleaned);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/github-copilot.js` around lines 688 - 692, The cleanup path in cleanupCopilotInstructions uses fs.writeFile(instructionsPath, cleaned, 'utf8') directly, bypassing the class abstraction this.writeFile (which handles logging, errors and directory setup); change that call to use this.writeFile(instructionsPath, cleaned, 'utf8') so the same error handling/side-effects occur, keeping references to instructionsPath and cleaned and ensuring behavior matches other write operations in the class.
582-586:⚠️ Potential issue | 🟡 MinorTool names read from existing permissions files are embedded in YAML without sanitization
return '[' + tools.map((t) => `'${t}'`).join(', ') + ']';Values from
existingToolPermissionscome from untrusted file content parsed byyaml.parse. A tool entry containing a single quote (e.g.,read'n'write) would produce malformed YAML frontmatter. There is also no allowlist validation — an unexpected value passes through silently.🛡️ Proposed fix: sanitize and allowlist
+ static VALID_TOOLS = new Set(['read', 'edit', 'search', 'execute', 'terminal']); + getToolsForFile(fileName) { const defaultTools = ['read', 'edit', 'search', 'execute']; const tools = (this.existingToolPermissions && this.existingToolPermissions.get(fileName)) || defaultTools; - return '[' + tools.map((t) => `'${t}'`).join(', ') + ']'; + const safe = tools.filter((t) => typeof t === 'string' && GitHubCopilotSetup.VALID_TOOLS.has(t)); + const effective = safe.length > 0 ? safe : defaultTools; + return '[' + effective.map((t) => `'${t}'`).join(', ') + ']'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/github-copilot.js` around lines 582 - 586, getToolsForFile currently returns tool names from existingToolPermissions directly into YAML-like string which allows untrusted values (e.g., containing single quotes) and unexpected tool names; update getToolsForFile to validate each tool against an allowlist (e.g., ['read','edit','search','execute']), filter out or default any invalid entries, and escape any single quotes in allowed tool names before joining so the resulting string is safe for embedding in YAML frontmatter; reference the getToolsForFile function and ensure the logic uses the existingToolPermissions lookup but applies allowlist validation and proper single-quote escaping for each tool.
482-490:⚠️ Potential issue | 🟡 Minor
indexOf(markerEnd)searches from the start of the file, not from after the start markerconst endIdx = existing.indexOf(markerEnd);If the BMAD section content ever contains the literal string
<!-- BMAD:END -->(e.g., in an inline documentation example or a prior bad write),indexOfwill match the wrong occurrence and the wrong slice of content will be replaced. The search should start atstartIdx + markerStart.length.🐛 Proposed fix
- const endIdx = existing.indexOf(markerEnd); + const endIdx = existing.indexOf(markerEnd, startIdx + markerStart.length);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/github-copilot.js` around lines 482 - 490, The end marker search uses existing.indexOf(markerEnd) which can match an earlier occurrence; update the end marker lookup to search after the start marker (e.g., compute endIdx using existing.indexOf(markerEnd, startIdx + markerStart.length) or search inside the substring starting at startIdx + markerStart.length), then keep the existing guards (endIdx !== -1 && endIdx > startIdx) and rebuild before/after/merged as before using the corrected endIdx; reference symbols: existing, instructionsPath, markerStart, markerEnd, startIdx, endIdx, before, after, merged.
493-497:⚠️ Potential issue | 🟡 Minor
.bakfile is silently overwritten on repeated installs, destroying the original user backupawait fs.copy(instructionsPath, backupPath);
fs.copyin fs-extra overwrites the destination by default. If the user ran the installer previously (creating a.bakfrom their original custom instructions), a second install overwrites that backup with the BMAD-managed content — the original user content is gone permanently with no warning.🐛 Proposed fix: skip overwriting an existing backup
- await fs.copy(instructionsPath, backupPath); - if (!options.silent) await prompts.log.warn(` Backed up copilot-instructions.md → .bak`); + if (await fs.pathExists(backupPath)) { + if (!options.silent) await prompts.log.warn(` Skipping backup: .bak already exists (preserving original)`); + } else { + await fs.copy(instructionsPath, backupPath); + if (!options.silent) await prompts.log.warn(` Backed up copilot-instructions.md → .bak`); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/github-copilot.js` around lines 493 - 497, The current backup logic uses fs.copy(instructionsPath, backupPath) which will silently overwrite an existing .bak; update the code around backupPath/instructionsPath to first check for the backup's existence (e.g., await fs.pathExists(backupPath)) and only perform fs.copy if the backup does not already exist; if it exists, skip the copy and surface a message via prompts.log.warn or prompts.log.info (respecting options.silent) indicating the backup was preserved and not overwritten so users won't lose their original custom instructions; keep the existing writeFile(instructionsPath, `${markedContent}\n`) behavior untouched.
605-617:⚠️ Potential issue | 🟠 MajorAgents cleanup pattern deletes any
bmad*.mdfile, not justbmad*.agent.mdif (file.startsWith('bmad') && (file.endsWith('.agent.md') || file.endsWith('.md'))) {The second condition (
file.endsWith('.md')) is a superset — it matches files likebmad-notes.mdorbmad-readme.mdthat are not BMAD-generated agent files. Compare with the prompts cleanup at line 627, which is correctly scoped tobmad-*.prompt.md. This can silently destroy non-BMAD files in the.github/agents/directory.🐛 Proposed fix
- if (file.startsWith('bmad') && (file.endsWith('.agent.md') || file.endsWith('.md'))) { + if (file.startsWith('bmad') && file.endsWith('.agent.md')) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/github-copilot.js` around lines 605 - 617, The current cleanup loop over files in agentsDir uses a too-broad condition (file.startsWith('bmad') && (file.endsWith('.agent.md') || file.endsWith('.md'))) which will delete non-agent files like bmad-notes.md; change the conditional in that loop to only match BMAD agent files (for example: file.startsWith('bmad-') && file.endsWith('.agent.md')) so only files like bmad-*.agent.md are removed; keep the removed counter and the prompts.log.message behavior unchanged and update the condition in the loop that iterates over files to use the stricter pattern.
534-536:⚠️ Potential issue | 🟡 Minor
escapeYamlSingleQuotedoes not handle newlines; a newline in a CSV metadata field breaks YAML frontmatterreturn (value || '').replaceAll("'", "''");YAML single-quoted scalars cannot contain literal newlines (the spec requires them to be folded or use multi-line syntax). If
manifestEntry.capabilitiesor any other field sourced fromagent-manifest.csvcontains a\n, the generated frontmatter will be malformed and unparseable by most YAML parsers.🐛 Proposed fix: strip/collapse newlines
escapeYamlSingleQuote(value) { - return (value || '').replaceAll("'", "''"); + return (value || '').replace(/[\r\n]+/g, ' ').replaceAll("'", "''"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/github-copilot.js` around lines 534 - 536, The escapeYamlSingleQuote function currently only doubles single quotes and doesn't handle newlines, which breaks YAML single-quoted frontmatter; update escapeYamlSingleQuote to first normalize/collapse all newline sequences (\r\n, \r, \n) into a single space (or trim surrounding whitespace) and then perform the single-quote doubling, so values like manifestEntry.capabilities become a single-line safe YAML single-quoted scalar; modify the escapeYamlSingleQuote implementation accordingly.
462-462:⚠️ Potential issue | 🟠 Major
{context}is used in the output paths (lines 440-442) but is absent from the Key Conventions session variables listLine 462 instructs the AI to store session variables including
{output_folder},{planning_artifacts}, and{implementation_artifacts}, but{context}— which is now embedded in all three of those values — is never mentioned. Without an explicit instruction telling the AI where to read{context}from (e.g., "read_bmad/.current_projectand store its value as{context}"), the AI will either leave the literal string{context}in every output path or guess incorrectly.🐛 Proposed addition to Key Conventions (only when monorepo mode is active)
- Store all config fields as session variables: `{user_name}`, `{communication_language}`, `{output_folder}`, `{planning_artifacts}`, `{implementation_artifacts}`, `{project_knowledge}` +- **Monorepo context**: If `_bmad/.current_project` exists, read its contents (trimmed) and store as `{context}`; substitute `{context}` into `{output_folder}`, `{planning_artifacts}`, and `{implementation_artifacts}` paths. If absent, `{context}` is empty.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/github-copilot.js` at line 462, The Key Conventions session variables list is missing the {context} variable while output paths (e.g., values for {output_folder}, {planning_artifacts}, {implementation_artifacts}) embed {context}; update the session variables list in the installer code (the block that enumerates stored session variables such as {user_name}, {communication_language}, ...) to include {context} and add an explicit instruction to read the current project context from _bmad/.current_project (or equivalent monorepo sentinel) and store that value as {context} so the AI substitutes it into output paths instead of leaving the literal "{context}".
558-562:⚠️ Potential issue | 🟡 MinorFrontmatter regex assumes LF-only line endings; breaks silently on CRLF files
const fmMatch = content.match(/^---\n([\s\S]*?)\n---/);Files written on Windows or checked out with
core.autocrlf=trueuse\r\n. The regex requires\nas delimiter, so the match fails and any existing tool permissions in those files are not preserved across reinstalls.🐛 Proposed fix
- const fmMatch = content.match(/^---\n([\s\S]*?)\n---/); + const fmMatch = content.match(/^---\r?\n([\s\S]*?)\r?\n---/);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/github-copilot.js` around lines 558 - 562, The frontmatter regex used to populate fmMatch assumes LF-only line endings and fails on CRLF files; update the regex in the block that sets fmMatch so the newline delimiters accept optional CR (i.e. allow CRLF or LF) by replacing the literal '\n' delimiters with a pattern that permits an optional '\r' before '\n' (and keep the non-greedy [\s\S]*? capture), so existing tool permissions are preserved for files with Windows line endings.src/bmm/workflows/document-project/workflows/deep-dive-instructions.md (1)
195-197:⚠️ Potential issue | 🔴 Critical
output_folderoverride does not affect any write operation in this workflow.All file write operations in this workflow use
{project_knowledge}as the output base path (Lines 197, 244, 264, 288). The monorepo context check (Line 260) overridesoutput_folder, which is a different variable never referenced as a write target anywhere in this file. Even if the context check were correctly placed and properly tagged, deep-dive documentation would still be written to the pre-monorepo{project_knowledge}path.The override should target
{project_knowledge}(or whatever variable maps to it in the config), notoutput_folder, to actually redirect output for monorepo projects.Also applies to: 260-260
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/document-project/workflows/deep-dive-instructions.md` around lines 195 - 197, The monorepo override currently sets output_folder but all write actions use {project_knowledge}; update the monorepo context check so it assigns the monorepo path to the same variable used by write operations (i.e., set or override {project_knowledge} instead of output_folder) so that actions like writing deep-dive-{{sanitized_target_name}}.md actually go to the monorepo target; verify any references that compute the monorepo path are applied to {project_knowledge} and not to output_folder.src/bmm/workflows/4-implementation/dev-story/instructions.xml (5)
378-379: 🛠️ Refactor suggestion | 🟠 MajorStep 10 re-executes the DoD validation that Step 9 already ran with HALT conditions
Step 9 (lines 336–348) runs the full enhanced definition-of-done checklist and gates on it with four
HALTactions (lines 372–375). Step 10 begins by running the same checklist again (line 379). Because Step 10 is only reachable after Step 9 succeeds, this re-execution is either dead work or implicitly signals that Step 9's validation is insufficient — neither is acceptable. Remove the re-execution in Step 10, or clarify what additional validation Step 10 is intended to perform beyond Step 9.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/dev-story/instructions.xml` around lines 378 - 379, Step 10 currently re-runs the enhanced definition-of-done checklist that Step 9 already executes and gates with HALT actions; remove the duplicate checklist execution from the <step n="10"> action or replace it with a clarified, distinct validation action explaining what additional checks or communications Step 10 performs beyond Step 9 (e.g., post-release verification, stakeholder notification, or support triage), and update the <action> text in <step n="10"> to explicitly state that distinct responsibility so the duplicate validation is eliminated or clearly differentiated from <step n="9">.
135-137:⚠️ Potential issue | 🟠 Major
goto step="6"labeled "Completion sequence" is mislabeled and likely targets the wrong stepWhen no incomplete tasks remain, the workflow jumps to Step 6 (
goal="Author comprehensive tests"). The labelCompletion sequencestrongly implies the intent is Step 9 (goal="Story completion and mark for review"). Sending an already-complete story through Step 6 forces redundant test authoring, then Steps 7–8 re-validate, before finally reaching Step 9. If this path is intentional (re-run test suite on already-done stories), the label must be corrected; if unintentional, the target step number is wrong.🐛 Proposed fix (if intent is to go directly to completion)
<action if="no incomplete tasks"> - <goto step="6">Completion sequence</goto> + <goto step="9">Completion sequence</goto> </action>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/dev-story/instructions.xml` around lines 135 - 137, The goto action with attribute goto step="6" and inner text "Completion sequence" is mislabeled and likely points to the wrong step; either update the target to step="9" (so the <action if="no incomplete tasks"><goto step="9">Completion sequence</goto></action> jumps directly to the "Story completion and mark for review" step) or, if the intent is to re-run tests, change the inner label to reflect that intent (e.g., "Re-run tests and continue") while keeping step="6"; modify the <action> block accordingly to ensure the label and the goto step attribute match the intended workflow.
142-153: 🛠️ Refactor suggestion | 🟠 MajorStep 2 verbatim duplicates the four context-loading actions already completed in Step 1
Lines 146–149 repeat exactly the actions from lines 127–131:
- "Parse sections: Story, Acceptance Criteria…"
- "Load comprehensive context from story file's Dev Notes section"
- "Extract developer guidance from Dev Notes…"
- "Use enhanced story context to inform implementation…"
This means the agent reads and parses the story file twice in sequence with no intervening state change. Beyond the redundant I/O, the two copies will diverge over time as the workflow evolves, creating inconsistency. Step 2 should either be eliminated or scoped to loading context not already handled in Step 1 (e.g.,
{project_context}, which appears only on line 145).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/dev-story/instructions.xml` around lines 142 - 153, Step 2 currently duplicates the four context-loading actions ("Parse sections: Story, Acceptance Criteria, Tasks/Subtasks, Dev Notes, Dev Agent Record, File List, Change Log, Status"; "Load comprehensive context from story file's Dev Notes section"; "Extract developer guidance from Dev Notes: architecture requirements, previous learnings, technical specifications"; "Use enhanced story context to inform implementation decisions and approaches") that were already performed in Step 1; remove the duplicated actions from step n="2" and leave only the unique action "Load {project_context} for coding standards and project-wide patterns (if exists)" (or alternatively collapse Step 2 into a single note that explicitly scopes it to project_context only), and update the Step 2 <output> to reflect that only project-level context was loaded to avoid redundant parsing and I/O.
382-384:⚠️ Potential issue | 🟡 MinorRemoval of story file path and status from completion messaging is an unacknowledged UX regression
Per the AI summary, two lines were removed from the completion output: the story file path and the current status. Step 10 now tells the user implementation is complete and summarizes accomplishments (line 383), but omits where the story file lives and what its new status is. In a monorepo context — where
output_foldermay have been redirected to a project-specific subdirectory — this information is more critical, not less. Users have no way to confirm the correct artifact was updated in the expected location.➕ Proposed addition to completion output
<action>Communicate to {user_name} that story implementation is complete and ready for review</action> <action>Summarize key accomplishments: story ID, story key, title, key changes made, tests added, files modified</action> + <action>Include story file path and current status ("review") in the completion summary</action> + <check if="output_folder was overridden (project_suffix is set)"> + <action>Note the active project context and output directory in the completion message</action> + </check>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/dev-story/instructions.xml` around lines 382 - 384, The completion messaging removed the story file path and current status causing a UX regression; update the <action> elements that read "Communicate to {user_name} that story implementation is complete and ready for review" and "Summarize key accomplishments: story ID, story key, title, key changes made, tests added, files modified" to also include the story file path (use {output_folder} or the story file variable) and the new status (use {story_status} or equivalent), so the final summary explicitly states where the artifact was written and its current status for verification in monorepos.
351-360:⚠️ Potential issue | 🟠 Major
{sprint_status}(single-brace) in Step 9 and Step 10 is inconsistent with{{sprint_status}}(double-brace) used everywhere elseThroughout the file (lines 29, 31, 41, 55, 67, 72, 78, 197–199, 209, 215, 220, 223–224), the sprint status reference uses
{{sprint_status}}— a runtime workflow variable. Lines 351, 360, and 407 silently switch to{sprint_status}. If single-brace resolves to a config/template value and double-brace to a runtime variable, the conditional checks in Step 9 may evaluate against a literal string or the wrong value, silently bypassing the sprint-status update or triggering it incorrectly.🐛 Proposed fix (lines 351, 360, 407)
- <check if="{sprint_status} file exists AND {{current_sprint_status}} != 'no-sprint-tracking'"> - <action>Load the FULL file: {sprint_status}</action> + <check if="{{sprint_status}} file exists AND {{current_sprint_status}} != 'no-sprint-tracking'"> + <action>Load the FULL file: {{sprint_status}}</action>- <check if="{sprint_status} file does NOT exist OR {{current_sprint_status}} == 'no-sprint-tracking'"> + <check if="{{sprint_status}} file does NOT exist OR {{current_sprint_status}} == 'no-sprint-tracking'">- <check if="{sprint_status} file exists"> + <check if="{{sprint_status}} file exists">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/dev-story/instructions.xml` around lines 351 - 360, Replace the stray single-brace template tokens with the runtime variable form so the checks use the actual workflow variable: change all occurrences of "{sprint_status}" in the check condition(s) and in the action "Load the FULL file: {sprint_status}" to "{{sprint_status}}", ensuring the check if attribute and the Load action reference the same runtime variable used elsewhere (note the surrounding elements: the <check if="..."> condition and the "Load the FULL file" action that currently use single-brace); also apply the same replacement for the other mismatched occurrence later in the file so the conditional logic and file load operate on {{sprint_status}} consistently with development_status[{{story_key}}].src/bmm/workflows/4-implementation/create-story/instructions.xml (4)
29-179:⚠️ Potential issue | 🟠 Major
GOTO step 2areferences a step label that does not exist.
GOTO step 2aappears on lines 29, 55, 60, 122, and 179. The only step following Step 1 is<step n="2">(line 182). There is no<step n="2a">anywhere in the file. If the workflow engine performs a literal step-label lookup, every one of these GOTO directives will fail. If it silently resolves2ato2, the behavior is accidentally correct but the label is still wrong and misleading to anyone maintaining this file.🐛 Proposed fix (all occurrences)
- <action>GOTO step 2a</action> + <action>GOTO step 2</action>(Apply to all five occurrences at lines 29, 55, 60, 122, and 179.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/create-story/instructions.xml` around lines 29 - 179, The GOTO targets "step 2a" in multiple places (the directives "GOTO step 2a" at occurrences tied to the checks and actions) do not match any actual step label (there is only <step n="2">); update each "GOTO step 2a" occurrence to "GOTO step 2" so the workflow engine resolves correctly (or alternatively add a matching <step n="2a"> if a distinct substep is intended), specifically modify the five "GOTO step 2a" tokens referenced in the diff to point at the existing step n="2" (or create step n="2a" and wire it as intended).
319-319:⚠️ Potential issue | 🟠 Major
validate-workflow.xmlis referenced without a{project-root}prefix, inconsistent with every other absolute path in this file.Line 2 uses
{project-root}/_bmad/core/tasks/workflow.xml.
Line 23 uses{project-root}/_bmad-output/....
Line 319 references_bmad/core/tasks/validate-workflow.xmlwithout any prefix.In a monorepo context (the entire motivation of this PR), if the agent's working directory is not the project root, this bare relative path will fail to resolve. The
{project-root}prefix ensures correct resolution regardless of where the workflow engine is invoked from.🐛 Proposed fix
- <invoke-task>Validate against checklist at {installed_path}/checklist.md using _bmad/core/tasks/validate-workflow.xml</invoke-task> + <invoke-task>Validate against checklist at {installed_path}/checklist.md using {project-root}/_bmad/core/tasks/validate-workflow.xml</invoke-task>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/create-story/instructions.xml` at line 319, The invoke-task element referencing "_bmad/core/tasks/validate-workflow.xml" is missing the {project-root} prefix and will fail if the engine is not run from the repo root; update the <invoke-task> entry that currently reads "Validate against checklist at {installed_path}/checklist.md using _bmad/core/tasks/validate-workflow.xml" to use an absolute path consistent with the other entries by adding the "{project-root}/" prefix (i.e., reference "{project-root}/_bmad/core/tasks/validate-workflow.xml") so the workflow engine can resolve the validate-workflow.xml file reliably.
183-183:⚠️ Potential issue | 🟡 MinorProfanity in a public-facing workflow instruction.
Line 183 contains explicit profanity ("fuckups") inside a
<critical>tag that is rendered directly as agent instruction text. This is inappropriate for an open-source project used in professional development environments.✏️ Proposed fix
- <critical>🔬 EXHAUSTIVE ARTIFACT ANALYSIS - This is where you prevent future developer fuckups!</critical> + <critical>🔬 EXHAUSTIVE ARTIFACT ANALYSIS - This is where you prevent future developer errors and implementation failures!</critical>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/create-story/instructions.xml` at line 183, Replace the profanity found inside the <critical> element (the string "fuckups" on line containing <critical>🔬 EXHAUSTIVE ARTIFACT ANALYSIS - This is where you prevent future developer fuckups!</critical>) with a professional alternative; update the <critical> text to something like "prevent future developer mistakes" or "prevent future regressions" ensuring tone is professional and suitable for public-facing workflow instructions in create-story/instructions.xml.
64-179:⚠️ Potential issue | 🟠 MajorDead code: lines 124–179 are an unreachable duplicate of the
<check if="no user input provided">block.Trace the control flow from the top of Step 1:
- Lines 26–30: User provided
story_pathor epic-story number →GOTO step 2a(exits step 1).- Lines 32–62: Sprint status file does NOT exist → prompts user; all paths end in
GOTOorHALT.- Lines 64–123:
no user input provided— at this point, line 26 has already handled all user-input cases, so this condition is always true when reached; the block ends withGOTO step 2a.Lines 124–179 are never reached: any execution path that reaches line 124 would have already exited via line 122. The code there is an exact structural duplicate (load sprint_status, parse backlog story, update epic status, GOTO), which also means bugs fixed in one copy will silently persist in the dead copy, creating future maintenance confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/create-story/instructions.xml` around lines 64 - 179, There is an unreachable duplicate of the "<check if=\"no user input provided\">" block followed by a repeated sequence that ends with "GOTO step 2a"; remove the duplicate block (the second occurrence that repeats loading {{sprint_status}}, parsing development_status, finding the first backlog story, extracting epic_num/story_num/story_title, setting {{story_id}}, checking/updating epic status and final "GOTO step 2a") so only the first authoritative "no user input provided" handling remains; ensure references like story_key, {{story_id}}, epic-{{epic_num}}, and the checks for epic status remain intact and that any shared side-effects (updating sprint_status) are preserved; run relevant unit/integration checks that exercise step 1 to confirm behavior is unchanged.src/bmm/workflows/4-implementation/retrospective/instructions.md (5)
724-728:⚠️ Potential issue | 🟠 MajorSteps 7, 8, 10, and 12 output blocks explicitly instruct the agent to emit time estimates, directly contradicting the
<critical>directive at line 17.Line 17 categorically prohibits mentioning "hours, days, weeks, months, or ANY time-based predictions." Yet the templated dialogue in these steps instructs the agent to output
"estimated {{hours_1}} hours","Total critical prep effort: {{critical_hours}} hours ({{critical_days}} days)", and"Execute Preparation Sprint (Est: {{prep_days}} days)"in Steps 7, 8, 10, and 12 respectively.The agent will resolve the output templates, produce time estimates, and violate the critical. Either the critical directive should be narrowed to exclude retrospective/prep planning contexts, or all
{{hours_*}}/{{days_*}}/{{critical_*_days}}variables throughout the workflow must be replaced with effort-sizing units (story points, complexity tiers, etc.).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/retrospective/instructions.md` around lines 724 - 728, The templates in Steps 7, 8, 10, and 12 currently emit time-based estimates (e.g., "{{hours_1}} hours", "{{critical_hours}} hours", "{{critical_days}} days", "{{prep_days}} days") which conflicts with the <critical> directive prohibiting time-based predictions; update those output blocks to use effort-sizing units instead (e.g., story points or complexity tiers) by replacing all "{{hours_*}}", "{{days_*}}", and "{{critical_*_days}}" variables with semantic effort variables (e.g., "{{points_1}}", "{{critical_points}}", "{{prep_complexity}}") and adjust any corresponding references/templating in Steps 7, 8, 10, and 12 to render the new units consistently; alternatively, if you prefer to permit time estimates, narrow the <critical> directive scope to explicitly allow retrospective/prep planning contexts and document that exception.
908-945:⚠️ Potential issue | 🟠 MajorStep 8 renders next-epic prep and critical-path sections unconditionally, even when
{{next_epic_exists}} == false.Step 7 routes to Step 8 in both the "next epic exists" and "next epic does not exist" code paths. Step 8's
🚀 EPIC {{next_epic_num}} PREPARATION TASKSand⚠️ CRITICAL PATHoutput blocks (lines 908–945) have no{{#ifnext_epic_exists}}guard. When there is no next epic, the agent will output a preparation checklist and critical-path section with unresolved placeholder variables ({{setup_task_1}}, etc.), producing garbage output.🐛 Proposed fix
+{{`#if` next_epic_exists}} ═══════════════════════════════════════════════════════════ 🚀 EPIC {{next_epic_num}} PREPARATION TASKS: ═══════════════════════════════════════════════════════════ ... ═══════════════════════════════════════════════════════════ ⚠️ CRITICAL PATH: ═══════════════════════════════════════════════════════════ ... +{{/if}}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/retrospective/instructions.md` around lines 908 - 945, Step 8 unconditionally renders the "🚀 EPIC {{next_epic_num}} PREPARATION TASKS" and "⚠️ CRITICAL PATH" blocks even when {{next_epic_exists}} is false; wrap those entire sections (the blocks that include {{setup_task_1}}..{{total_hours}} and {{critical_item_1}}..{{critical_deadline_2}}) in a conditional guard so they only render when next_epic_exists is true (e.g., add {{`#if` next_epic_exists}} ... {{/if}} around the two output blocks), or alternatively change Step 7 routing so it does not advance to Step 8 when next_epic_exists is false; ensure no unresolved placeholders appear when next_epic_exists == false by guarding the blocks referenced as "🚀 EPIC {{next_epic_num}} PREPARATION TASKS" and "⚠️ CRITICAL PATH".
248-250:⚠️ Potential issue | 🟡 Minor"Calculate average completion time per story" risks surfacing time-based metrics that conflict with the line 17 critical.
The velocity patterns analysis instructs the agent to "calculate average completion time" and provide examples referencing estimates ("3x longer than estimated"). While line 17 targets future predictions, directing the agent to calculate and name time-based completion metrics during facilitated dialogue creates a grey zone where the agent may naturally volunteer day/hour counts in the retrospective discussion, violating the spirit of the critical directive.
Consider replacing time-based velocity language with story-point velocity or relative complexity comparisons (e.g., "Story X completed in 2x the expected points").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/retrospective/instructions.md` around lines 248 - 250, Replace the time-based velocity language in the retrospective instructions (the bullets "Calculate average completion time per story" and "Note velocity trends (e.g., 'First 2 stories took 3x longer than estimated')") with guidance that uses story-point velocity or relative complexity comparisons instead of absolute time; update the second and third bullet to ask for metrics like "average story-point completion per sprint" and "relative completion vs. estimated points (e.g., 'Story X completed at 2x estimated points')" and remove any phrasing that encourages reporting days/hours to avoid conflicting with the line 17 critical.
184-184:⚠️ Potential issue | 🟡 Minor
{{story_num}}is used as a loop variable in the file path pattern but is never declared or bound.The instruction says "For each story in epic {{epic_number}}, read the complete story file from
{implementation_artifacts}/{{epic_number}}-{{story_num}}-*.md" but never tells the agent how{{story_num}}is sourced (from the sprint-status file? from directory listing?). The agent is left to infer the loop variable, which may result in unresolved placeholder paths or hallucinated story numbers.🐛 Proposed fix
-<action>For each story in epic {{epic_number}}, read the complete story file from {implementation_artifacts}/{{epic_number}}-{{story_num}}-*.md</action> +<action>For each story key found for epic {{epic_number}} in {sprint_status_file} (keys matching pattern "{{epic_number}}-*"), extract the story number as {{story_num}} and read the corresponding file: {implementation_artifacts}/{{epic_number}}-{{story_num}}-*.md</action>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/retrospective/instructions.md` at line 184, The template uses an undeclared loop variable {{story_num}} in the path {implementation_artifacts}/{{epic_number}}-{{story_num}}-*.md; declare and bind {{story_num}} by iterating over a concrete source (e.g., the epic's story list or directory listing) before using it. Update the instruction to state how to obtain story numbers—either "for each story in {{epic.stories}} (use story.number as {{story_num}})" or "list files in {implementation_artifacts} matching {{epic_number}}-*-*.md and extract {{story_num}} via regex"—so the agent has a clear binding for {{story_num}} when reading story files.
1336-1340:⚠️ Potential issue | 🟠 Major
{date}is used 4 times for filename construction but is never defined, initialized, or formatted.The variable
{date}appears inepic-{{epic_number}}-retro-{date}.mdthroughout Steps 11 and 12 but there is no instruction in the Configuration Loading section (or anywhere else) telling the agent how to obtain the date or what format to use. The agent will invent a format, producing inconsistent filenames (e.g.,retro-2025-1-15.mdone run,retro-January-15-2025.mdanother). This directly undermines Step 3's file search patternepic-{{prev_epic_num}}-retro-*.md, which must match whatever format was used when the previous retro was saved.🐛 Proposed fix — define date format in Configuration Loading
Load and read full config from {main_config} and resolve basic variables. +Set `{date}` = current date in `YYYY-MM-DD` format (e.g., 2025-01-15).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/retrospective/instructions.md` around lines 1336 - 1340, The filename template epic-{{epic_number}}-retro-{date}.md uses an undefined `{date}` leading to inconsistent filenames; add a defined, documented date format in the Configuration Loading section (e.g., set a config key like `date_format = "YYYY-MM-DD"` or ISO 8601) and initialize a `{date}` variable by formatting the current date with that format, then update the filename construction in Steps 11/12 to use that initialized `{date}`; ensure the Step 3 search pattern `epic-{{prev_epic_num}}-retro-*.md` will match by choosing a single explicit format and mentioning the exact `date_format` symbol in the config so agents produce consistent names.src/bmm/workflows/document-project/workflows/full-scan-instructions.md (1)
88-123:⚠️ Potential issue | 🔴 CriticalConfiguration Loading block is injected inside the
<ask>user prompt, corrupting the scan-level selection menu, and leaving a garbled text fragment on line 106.The
## 1. Configuration Loadingblock (lines 98–105) was inserted in the middle of the<ask>Choose your scan depth level:tag that spans lines 89–123. Concretely:
- The executing agent will display the monorepo context check instructions verbatim to the user as part of the scan-level options, interspersed between the Quick Scan and Deep Scan descriptions. This completely breaks the user-facing workflow.
- Line 106 (
json, etc.)) is a truncated remnant of the original Quick Scan description: "File reading: Minimal (configs, README, package.json, etc.)" was split mid-sentence by the insertion. This garbled fragment is now shown to the user as part of the menu.The Configuration Loading block must be extracted to a dedicated step before the
<check if="workflow_mode == initial_scan OR workflow_mode == full_rescan">check (e.g., as a new<step n="0.4">), and the Quick Scan description must be restored to its original, untruncated form.🔧 Proposed fix (structural outline)
Add a new step before step 0.6:
+<step n="0.4" goal="Load configuration and apply monorepo context"> + <action>Load and read full config from {main_config} and resolve basic variables.</action> + <check if="_bmad/.current_project exists"> + <action>Read content as project_suffix</action> + <action>Trim whitespace from project_suffix</action> + <action>Override output_folder to {project-root}/_bmad-output/{project_suffix}</action> + </check> +</step> + <step n="0.5" goal="Load documentation requirements...">Restore the Quick Scan description inside
<ask>:-## 1. Configuration Loading - -Load and read full config from {main_config} and resolve basic variables. - -**Monorepo Context Check:** -1. Check if `_bmad/.current_project exists`. -2. If it exists, read its content as `{project_suffix}` and override output folder: - - `output_folder`: `{project-root}/_bmad-output/{project_suffix}` -json, etc.) +- File reading: Minimal (configs, README, package.json, etc.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/document-project/workflows/full-scan-instructions.md` around lines 88 - 123, The Configuration Loading block was accidentally inserted inside the <ask> user prompt (within the <check if="workflow_mode == initial_scan OR workflow_mode == full_rescan"> section), corrupting the scan-level menu and truncating the Quick Scan description; fix it by extracting the entire "Configuration Loading" section out of the <ask> and placing it into a new pre-check step (e.g., <step n="0.4">) that runs before the existing <check if="workflow_mode == initial_scan OR workflow_mode == full_rescan">, and restore the Quick Scan "File reading: Minimal (configs, README, package.json, etc.)" line to its full, original text inside the <ask> block so the menu displays correctly.src/bmm/workflows/4-implementation/correct-course/instructions.md (1)
36-55:⚠️ Potential issue | 🔴 CriticalConfiguration Loading block is injected into the middle of Step 2 — too late and structurally wrong.
The
### 1. Configuration Loadingsection (lines 43–50) is embedded inside<step n="2" goal="Execute Change Analysis Checklist">, sandwiched between the checklist recording instructions and two<action>tags. This means:
- Steps 1 and 0.5 (
Initialize Change NavigationandDiscover and load project documents) execute beforeoutput_folderis set — any file writes in those steps use the wrong path.- The heading renders as free Markdown inside an XML step body, making the semantic intent unclear to the executing agent.
Compare the correct pattern used in
create-epics-and-stories/workflow.mdandcreate-ux-design/workflow.md, where Configuration Loading is a dedicated top-level## INITIALIZATION SEQUENCEsection before any step.🔧 Proposed fix — extract to a top-level initialization section before Step 1
+## INITIALIZATION SEQUENCE + +### 1. Configuration Loading + +Load and read full config from {main_config} and resolve basic variables. + +**Monorepo Context Check:** +1. Check if `_bmad/.current_project` exists. +2. If it exists, read its content as `{project_suffix}` and override output folder: + - `output_folder`: `{project-root}/_bmad-output/{project_suffix}` + +--- + <workflow> <step n="1" goal="Initialize Change Navigation">And remove lines 43–50 from inside
<step n="2">.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/correct-course/instructions.md` around lines 36 - 55, The "### 1. Configuration Loading" block is incorrectly embedded inside <step n="2" goal="Execute Change Analysis Checklist"> (between the checklist items and <action> tags), causing output_folder to be set too late; move that entire Configuration Loading section (including checks for _bmad/.current_project, reading {project_suffix}, and overriding output_folder to `{project-root}/_bmad-output/{project_suffix}` and the reference to {main_config}) into a new top-level initialization section placed before any <step> (e.g., an "## INITIALIZATION SEQUENCE" before Step 1), and remove the Configuration Loading block from inside <step n="2"> so that functions/processes that run in Initialize Change Navigation and Discover and load project documents use the correct output_folder.src/bmm/workflows/generate-project-context/workflow.md (1)
30-41:⚠️ Potential issue | 🟠 MajorDuplicate config-load instruction — residual old block creates ambiguity
Lines 30 and 37 both instruct the agent to load configuration:
- Line 30 (new): "Load and read full config from
{main_config}and resolve basic variables."- Line 37 (old, unchanged): "Load config from
{project-root}/_bmad/bmm/config.yamland resolve: …"If both lines are executed, the agent loads config twice; if only one is executed, it's unclear which survives. The old block needs to be removed — it was the original config instruction that the
{main_config}pattern was meant to replace.🔧 Proposed fix
-Load config from `{project-root}/_bmad/bmm/config.yaml` and resolve: - -- `project_name`, `output_folder`, `user_name` -- `communication_language`, `document_output_language`, `user_skill_level` -- `date` as system-generated current datetime -- ✅ YOU MUST ALWAYS SPEAK OUTPUT In your Agent communication style with the config `{communication_language}` +- `project_name`, `output_folder`, `user_name` +- `communication_language`, `document_output_language`, `user_skill_level` +- `date` as system-generated current datetime +- ✅ YOU MUST ALWAYS SPEAK OUTPUT In your Agent communication style with the config `{communication_language}`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/generate-project-context/workflow.md` around lines 30 - 41, Remove the redundant older config-load block that references "{project-root}/_bmad/bmm/config.yaml" and keep the new unified instruction that loads and reads the full config from "{main_config}"; update the workflow text (in workflow.md) so it only describes loading from "{main_config}", resolving the listed variables (project_name, output_folder, user_name, communication_language, document_output_language, user_skill_level, date), and include the monorepo check for "_bmad/.current_project" that overrides output_folder using the read {project_suffix}.src/bmm/workflows/4-implementation/sprint-planning/instructions.md (1)
35-60:⚠️ Potential issue | 🟡 MinorMove step 0.5 before step 1 for maintainability and reader clarity
Step 0.5 (lines 57–60) invokes
discover_inputsto populate{epics_content}which step 1 (lines 35–55) depends on. Although the workflow engine processes steps by numeric order (not document order), readers and maintainers will assume top-to-bottom execution and become confused when step 1 appears first. Reordering improves clarity and reduces the risk of future steps being placed in the wrong position.🔧 Proposed fix — move step 0.5 before step 1
+ <step n="0.5" goal="Discover and load project documents"> + <invoke-protocol name="discover_inputs" /> + <note>After discovery, these content variables are available: {epics_content} (all epics loaded - uses FULL_LOAD strategy)</note> + </step> + <step n="1" goal="Parse epic files and extract all work items"> ... </step> - - <step n="0.5" goal="Discover and load project documents"> - <invoke-protocol name="discover_inputs" /> - <note>After discovery, these content variables are available: {epics_content} (all epics loaded - uses FULL_LOAD strategy)</note> - </step>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-planning/instructions.md` around lines 35 - 60, Reorder the workflow so the discovery step runs before parsing: move the step element with n="0.5" that invokes-protocol name="discover_inputs" (which populates {epics_content}) to appear before the step element with n="1" that parses epics and extracts work items; ensure the discover_inputs step remains unchanged (it provides {epics_content} via FULL_LOAD) and that the parsing step (the one that loads {project_context}, searches {epics_location} for `{epics_pattern}`, and extracts Epic/Story IDs and converts story IDs to kebab-case) follows it, so readers see dependency order top-to-bottom.src/bmm/workflows/3-solutioning/create-architecture/workflow.md (2)
1-4:⚠️ Potential issue | 🔴 Critical
main_configis missing from the frontmatter —{main_config}on line 30 will be unresolved.Every other BMM workflow file updated in this PR (
workflow-domain-research.md,create-product-brief/workflow.md,workflow-market-research.md,workflow-edit-prd.md,quick-spec/workflow.md,workflow-technical-research.md) definesmain_config: '{project-root}/_bmad/bmm/config.yaml'in YAML frontmatter. This file is the only one that does not. The first initialization instruction references{main_config}directly; without the frontmatter key, the variable is undefined and the config load silently fails or the agent errors out.🐛 Proposed fix
--- name: create-architecture description: Collaborative architectural decision facilitation for AI-agent consistency. Replaces template-driven architecture with intelligent, adaptive conversation that produces a decision-focused architecture document optimized for preventing agent conflicts. +main_config: '{project-root}/_bmad/bmm/config.yaml' ---🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/3-solutioning/create-architecture/workflow.md` around lines 1 - 4, Add the missing YAML frontmatter key main_config so the {main_config} placeholder resolves: update the frontmatter at the top of this workflow to include main_config: '{project-root}/_bmad/bmm/config.yaml' (matching the other BMM workflow files) so references to {main_config} in the initialization step (the {main_config} placeholder) are defined and the config loads correctly.
28-41:⚠️ Potential issue | 🟠 MajorDuplicate config-loading instruction renders the initialization sequence ambiguous, and
{project-root}/is missing from the context check.Two issues:
Redundant load: Line 30 says "Load and read full config from
{main_config}" (undefined until the frontmatter fix is applied). Then line 37 says "Load config from{project-root}/_bmad/bmm/config.yamland resolve:" — the same file again. An AI agent following these instructions will attempt to load the config, apply the monorepo override, and then load the config again, creating ambiguity about which variable values are authoritative after the monorepo override. Oncemain_configis added to frontmatter, lines 37-41 should be replaced by a simple variable list under the single load at line 30.Missing
{project-root}/prefix on_bmad/.current_project(line 33) — same root cause flagged across all files.🛠️ Proposed fix
### 1. Configuration Loading Load and read full config from {main_config} and resolve basic variables. **Monorepo Context Check:** -1. Check if `_bmad/.current_project` exists. +1. Check if `{project-root}/_bmad/.current_project` exists. 2. If it exists, read its content as `{project_suffix}` and override output folder: - `output_folder`: `{project-root}/_bmad-output/{project_suffix}` -Load config from `{project-root}/_bmad/bmm/config.yaml` and resolve: - - `project_name`, `output_folder`, `planning_artifacts`, `user_name`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/3-solutioning/create-architecture/workflow.md` around lines 28 - 41, The workflow duplicates the config load: replace the second "Load config from {project-root}/_bmad/bmm/config.yaml and resolve:" block (lines 37–41) with a simple list of resolved variables under the initial "Load and read full config from {main_config}" step, ensuring a single authoritative load of {main_config} (which will point to {project-root}/_bmad/bmm/config.yaml once frontmatter is fixed); also fix the monorepo context check by changing `_bmad/.current_project` to `{project-root}/_bmad/.current_project` so the check and subsequent override of output_folder (`output_folder`: `{project-root}/_bmad-output/{project_suffix}`) correctly reference the project root.src/core/workflows/brainstorming/workflow.md (2)
35-48:⚠️ Potential issue | 🟠 Major
{main_config}undefined (broken first load) and duplicate config-loading instruction;{project-root}/prefix missing.Three compounding issues:
- Undefined variable: Line 37 says "Load and read full config from
{main_config}" —{main_config}is not in the frontmatter, so this is a no-op or an error.- Redundant load: Line 44 then gives an explicit "Load config from
{project-root}/_bmad/core/config.yaml" — a second load of the same file (once the frontmatter is fixed). Same ambiguity as increate-architecture/workflow.md: the agent doesn't know which load's variable values take precedence relative to the monorepo override applied in between. Remove the second explicit load directive and rely solely on{main_config}once the frontmatter is correct.- Missing
{project-root}/prefix on line 40's_bmad/.current_project— same root cause flagged across all files in this PR.🛠️ Proposed fix
Load and read full config from {main_config} and resolve basic variables. **Monorepo Context Check:** -1. Check if `_bmad/.current_project` exists. +1. Check if `{project-root}/_bmad/.current_project` exists. 2. If it exists, read its content as `{project_suffix}` and override output folder: - `output_folder`: `{project-root}/_bmad-output/{project_suffix}` -Load config from `{project-root}/_bmad/core/config.yaml` and resolve: - - `project_name`, `output_folder`, `user_name`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/workflows/brainstorming/workflow.md` around lines 35 - 48, The workflow currently references an undefined {main_config}, duplicates a later explicit load, and omits the {project-root}/ prefix for the monorepo check; fix by adding a proper {main_config} entry to the frontmatter (pointing to {project-root}/_bmad/core/config.yaml), remove the redundant "Load config from {project-root}/_bmad/core/config.yaml" instruction so only {main_config} is used, and update the monorepo check to use the full path "{project-root}/_bmad/.current_project" (ensure any mention of _bmad paths consistently includes the {project-root}/ prefix); keep logic that if {project_suffix} exists it overrides output_folder after loading {main_config}.
1-5:⚠️ Potential issue | 🔴 Critical
main_configmissing from frontmatter; and if added, it must point to the core config, not the BMM config.Line 37 references
{main_config}, which is undefined in this file's frontmatter. Unlike BMM workflow files (which rightly definemain_config: '{project-root}/_bmad/bmm/config.yaml'), this is a core workflow undersrc/core/. Line 44 correctly names the core config path{project-root}/_bmad/core/config.yaml. Ifmain_configis added here by copying the BMM frontmatter pattern, the agent will be directed to the wrong config file. The field must be added with the core-specific path.🐛 Proposed fix
--- name: brainstorming description: Facilitate interactive brainstorming sessions using diverse creative techniques and ideation methods context_file: '' # Optional context file path for project-specific guidance +main_config: '{project-root}/_bmad/core/config.yaml' ---🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/workflows/brainstorming/workflow.md` around lines 1 - 5, The frontmatter is missing the main_config key referenced by {main_config}; add a main_config entry in the frontmatter and set it to the core workflow config (not the BMM config) so {main_config} resolves to the core config used by src/core workflows; ensure the added main_config value references the core config location and not the BMM config path.src/bmm/workflows/bmad-quick-flow/quick-dev/workflow.md (1)
40-40:⚠️ Potential issue | 🟡 MinorMandatory communication directive is misplaced inside the config-loading block.
The
✅ YOU MUST ALWAYS SPEAK OUTPUT…directive (with a randomly uppercased "In") is buried inside the variable-resolution list. It reads as if it were another config value to resolve, rather than a standing behavioral rule. The random capitalization ("In your Agent communication style") further undermines clarity. Move it to a dedicated Behavioral Rules or Communication section so it isn't drowned out.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/bmad-quick-flow/quick-dev/workflow.md` at line 40, The directive line "✅ YOU MUST ALWAYS SPEAK OUTPUT In your Agent communication style with the config `{communication_language}`" is misplaced inside the variable-resolution/config-loading list and should be moved to a dedicated Behavioral Rules or Communication section; remove it from the variable list so it isn't parsed as a config value, correct the capitalization to "in" (or reword to "in your agent communication style") and place it under a clearly labeled "Behavioral Rules" or "Communication" heading so tools and readers won't treat it as a config resolution entry.
src/bmm/workflows/1-analysis/research/workflow-domain-research.md
Outdated
Show resolved
Hide resolved
src/bmm/workflows/1-analysis/research/workflow-market-research.md
Outdated
Show resolved
Hide resolved
src/bmm/workflows/1-analysis/research/workflow-technical-research.md
Outdated
Show resolved
Hide resolved
src/bmm/workflows/2-plan-workflows/create-prd/workflow-edit-prd.md
Outdated
Show resolved
Hide resolved
| ## 1. Configuration Loading | ||
|
|
||
| Load and read full config from {main_config} and resolve basic variables. | ||
|
|
||
| **Monorepo Context Check:** | ||
| 1. Check if `_bmad/.current_project exists`. | ||
| 2. If it exists, read its content as `{project_suffix}` and override output folder: | ||
| - `output_folder`: `{project-root}/_bmad-output/{project_suffix}` |
There was a problem hiding this comment.
{implementation_artifacts} is resolved before the output_folder override, so retrospective files are still written to the wrong location in monorepo mode.
The instructions eagerly resolve all config variables at line 8 ("resolve basic variables"), then override output_folder at lines 12-13. However, {implementation_artifacts} — used in every file-save action in Step 11 and referenced throughout the workflow — is almost certainly derived from output_folder at config-load time and already holds its pre-override path by the time the monorepo check runs.
This means the entire purpose of the feature is defeated for this workflow: the retrospective document, sprint-status updates, and all artifact paths in Step 11 will still be written to the original non-project-specific directory.
The fix is to explicitly re-derive {implementation_artifacts} (and any other path variables that depend on output_folder) after the override, or to instruct the agent to substitute {project_suffix} into artifact paths directly.
🐛 Proposed fix
**Monorepo Context Check:**
1. Check if `_bmad/.current_project` exists.
2. If it exists, read its content as `{project_suffix}` and override output folder:
- `output_folder`: `{project-root}/_bmad-output/{project_suffix}`
+3. Re-derive all path variables that depend on `output_folder`:
+ - `implementation_artifacts`: `{output_folder}/stories`
+ - (apply the same re-derivation to any other path variable sourced from `output_folder`)📝 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.
| ## 1. Configuration Loading | |
| Load and read full config from {main_config} and resolve basic variables. | |
| **Monorepo Context Check:** | |
| 1. Check if `_bmad/.current_project exists`. | |
| 2. If it exists, read its content as `{project_suffix}` and override output folder: | |
| - `output_folder`: `{project-root}/_bmad-output/{project_suffix}` | |
| ## 1. Configuration Loading | |
| Load and read full config from {main_config} and resolve basic variables. | |
| **Monorepo Context Check:** | |
| 1. Check if `_bmad/.current_project exists`. | |
| 2. If it exists, read its content as `{project_suffix}` and override output folder: | |
| - `output_folder`: `{project-root}/_bmad-output/{project_suffix}` | |
| 3. Re-derive all path variables that depend on `output_folder`: | |
| - `implementation_artifacts`: `{output_folder}/stories` | |
| - (apply the same re-derivation to any other path variable sourced from `output_folder`) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bmm/workflows/4-implementation/retrospective/instructions.md` around
lines 6 - 13, The workflow resolves config variables too early causing
{implementation_artifacts} to retain the pre-override path; after the monorepo
check that reads _bmad/.current_project and sets project_suffix and overrides
output_folder, re-derive any dependent path variables (explicitly
{implementation_artifacts} and any other variables computed from output_folder)
so they reflect the new output_folder, or alternatively substitute
{project_suffix} directly into artifact paths used in Step 11
(retrospective/sprint-status saves) to ensure files are written into
_bmad-output/{project_suffix}.
| </step> | ||
|
|
||
| ## 1. Configuration Loading | ||
|
|
||
| Load and read full config from {main_config} and resolve basic variables. | ||
|
|
||
| **Monorepo Context Check:** | ||
| 1. Check if `_bmad/.current_project exists`. | ||
| 2. If it exists, read its content as `{project_suffix}` and override output folder: | ||
| - `output_folder`: `{project-root}/_bmad-output/{project_suffix}` | ||
|
|
||
| <step n="3" goal="Check for existing documentation and determine workflow mode" if="resume_mode == false"> |
There was a problem hiding this comment.
Configuration Loading block placed outside <step> tags — breaks the XML workflow structure, and step 2 is missing
Lines 82–89 drop a raw Markdown heading and text block between </step> (end of step 1, line 80) and <step n="3"> (line 91), without wrapping it in a <step> element. The entire rest of this file uses <step n="X"> elements inside a <workflow> block. An execution engine parsing the <workflow> element will either skip this block entirely (treating it as free text outside step scope) or fail to process it at runtime.
Additionally:
- Step numbering jumps directly from
n="1"ton="3"— step 2 is absent. If the configuration loading was intended to replace a removed step 2, it must be wrapped in a proper<step>tag. - Line 87 has the same backtick bug:
`_bmad/.current_project exists`→ should be`_bmad/.current_project` exists.
🔧 Proposed fix — wrap in a step tag
-## 1. Configuration Loading
-
-Load and read full config from {main_config} and resolve basic variables.
-
-**Monorepo Context Check:**
-1. Check if `_bmad/.current_project exists`.
-2. If it exists, read its content as `{project_suffix}` and override output folder:
- - `output_folder`: `{project-root}/_bmad-output/{project_suffix}`
+<step n="2" goal="Load configuration and apply monorepo context">
+<action>Load and read full config from {main_config} and resolve basic variables.</action>
+
+**Monorepo Context Check:**
+1. Check if `_bmad/.current_project` exists.
+2. If it exists, read its content as `{project_suffix}` and override output folder:
+ - `output_folder`: `{project-root}/_bmad-output/{project_suffix}`
+</step>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bmm/workflows/document-project/instructions.md` around lines 80 - 91,
Wrap the "Configuration Loading" markdown block into a proper workflow step by
adding a <step n="2"> ... </step> around it (so steps go 1,2,3 sequentially) and
ensure this step is inside the existing <workflow> block; also correct the
backtick misuse so the path reads `_bmad/.current_project` (not ``
`_bmad/.current_project exists` ``) and verify the step's goal/attributes match
the other steps' conventions.
src/bmm/workflows/document-project/workflows/deep-dive-instructions.md
Outdated
Show resolved
Hide resolved
…tization, and documentation fixes
- Re-derive dependent paths in retrospective workflow after monorepo override - Add max length validation for project_suffix in dev-story - Fix premature output path computation in create-story workflow
|
@coderabbitai review |
|
I really like this idea. A few things to consider - this could be powerful just beyond monorepo. Right now for example, I already use a dev folder with bmad installed in it and have all my separate projects underneath it. this could potentially help improve working like that. Additionally we are looking towards support user ~ skill installation (all soon will be bundle-able skills) - this may or may not play relevance in that - need to give it some thought. The workflow and switching mechanism - I love the simplicity of this. But it belongs in BMad Core, not under BMad Method. This should be supported and work regardless of what modules are installed. I would like to figure out a way to support this without the need to add the condition into every workflow. One thing that every workflow has in common (almost all) is:
Aside from very few simple skills/workflows - those 2 needs exist, or at least there is no harm in resolving those. So one idea is potentially inject the resolution of those into every command/skill file itself. This way, we could keep the prompt in logic in one place, and at install time, when generating all the skills, this could go right into the start of the skill body before routing to the workflow itself. |
…nd dynamic path adjustments in workflows.
…licated checks from numerous workflow files and updating related tests.
Thank you for the feedback. I've tried to do better based on it. Now there is no duplication. And I inject logic during installation into workflows. Must be more manageable now without loosing functionality. i've also added one more feature Motivation: I might need to run 2 agents in parallel. In the same repository - now it is possible. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (24)
src/bmm/workflows/4-implementation/sprint-status/instructions.md (9)
167-182:⚠️ Potential issue | 🟡 MinorData mode output is missing retrospective counts despite Step 2 explicitly computing them.
Step 2 (line 49) parses retrospective statuses and counts
optionalanddoneretrospectives. Step 20 emits counts for stories (lines 172-176) and epics (lines 177-179) but provides no<template-output>for retrospective counts. Callers using data mode to assess sprint completeness (e.g., deciding whether to trigger a retrospective workflow) cannot determine the retrospective state without re-parsing the file.🔧 Proposed fix
<template-output>epic_done = {{epic_done}}</template-output> + <template-output>retro_optional = {{retro_optional}}</template-output> + <template-output>retro_done = {{retro_done}}</template-output> <template-output>risks = {{risks}}</template-output>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-status/instructions.md` around lines 167 - 182, Step 20's Data mode output omits the retrospective counts computed in Step 2, so add template-output entries for the retrospective counters (use the same names as computed in Step 2) so callers can read them without re-parsing; specifically update the Step n="20" block to include template-output tags for the retrospective counts (e.g., retrospective_optional and retrospective_done or whatever exact variable names Step 2 uses) alongside the existing count_* and epic_* outputs.
93-103:⚠️ Potential issue | 🟠 MajorStep 3 computes
next_agentbut it is never emitted in data mode (Step 20).Line 102 explicitly records
next_agentas part of the recommendation output. However, Step 20's<template-output>block (lines 170-181) listsnext_workflow_idandnext_story_idbut omitsnext_agent. Any caller invoking data mode to route work to the correct agent will not receive this field, forcing callers to re-derive it themselves or hardcode assumptions.Additionally, the priority list at lines 96-101 does not guard against an empty
development_statusmap. If parsing produces zero stories, zero epics, and zero retrospectives, none of items 1–5 trigger, the else clause fires, and the congratulation message is displayed — a misleading success message for a legitimately empty/newly-initialized sprint file.🔧 Suggested additions
In Step 20, add the missing output:
<template-output>next_story_id = {{next_story_id}}</template-output> + <template-output>next_agent = {{next_agent}}</template-output> <template-output>count_backlog = {{count_backlog}}</template-output>In Step 3, add a guard:
+ <check if="development_status is empty"> + <action>Set next_workflow_id = "sprint-planning", next_story_id = "", next_agent = "SM"</action> + <action>Return to caller with this recommendation</action> + </check> <action>Pick the next recommended workflow using priority:</action>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-status/instructions.md` around lines 93 - 103, Step 3 currently computes next_agent (and sets next_story_id/next_workflow_id) but Step 20's <template-output> omits next_agent and the priority logic doesn't handle an empty development_status map; update the template-output block to include next_agent so data-mode callers receive it, and modify the selection guard in the "Select next action recommendation" step (the priority check over development_status / retrospective lists) to detect empty development_status/epics/retrospectives and, instead of emitting the congratulatory "all done" message, emit a clear no-items-found output (e.g., next_workflow_id=null/none, next_story_id=null/none and next_agent=null/none) or an explicit "no work" status so callers can route appropriately. Ensure you reference/modify the next_agent variable, the selection logic in Step 3, and the <template-output> block in Step 20.
83-90:⚠️ Potential issue | 🟡 MinorStale timestamp comparison has no format specification — agent will infer or guess.
Line 88 instructs: "IF
generatedtimestamp is more than 7 days old: warn." Thegeneratedfield's expected format (ISO 8601, RFC 2822, YYYY-MM-DD, Unix epoch, etc.) is never specified. An LLM agent interpreting this will either attempt to parse by heuristic (unreliable across format variants) or silently skip the check if the format is unrecognized. The risk of a false positive ("file is stale" when it isn't) or false negative (stale file passes unchecked) is non-trivial.The workflow.yaml or Step 2 should specify the expected
generatedtimestamp format and the comparison basis (e.g., UTC wall clock at execution time).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-status/instructions.md` around lines 83 - 90, Specify the exact timestamp format and comparison basis for the `generated` field used in the stale check: state that `generated` must be ISO 8601 (e.g., RFC 3339) in UTC and that the worker should compare it to the current UTC wall-clock time; update the "IF `generated` timestamp is more than 7 days old" rule to explicitly parse `generated` as ISO 8601 UTC and compute now_utc - generated_utc > 7 days, and add a fallback behavior (fail the check or log a parsing error) if the timestamp is not valid; reference the `generated` field and the "sprint-status.yaml may be stale" warning so the agent can find and enforce the rule.
83-91:⚠️ Potential issue | 🟡 MinorOrphan story detection logic relies on undocumented key naming conventions.
Line 89 instructs: "IF any story key doesn't match an epic pattern (e.g., story
5-1-...but noepic-5): warn 'orphaned story detected'." The rule assumes a{epic_number}-{story_number}-{slug}format for story keys and anepic-{epic_number}format for epic keys. These conventions are never formally defined in this file. If a project uses story keys of a different shape, the orphan check will misfire silently (false positives or false negatives). The expected key schema should be explicitly documented or referenced.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-status/instructions.md` around lines 83 - 91, The orphan-detection rule that raises "orphaned story detected" depends on an assumed key schema (story keys like "{epic_number}-{story_number}-{slug}" and epic keys like "epic-{epic_number}") but that schema is undocumented; either document the expected key format in this file (referencing the rule and examples shown such as "5-1-..." and "epic-5") or make the check configurable/robust by describing the pattern parameters (regex or naming convention) and how to override them so projects with different key shapes won't get false positives/negatives; update the instructions around the "IF any story key doesn't match an epic pattern" line to include the formal schema or a link to configuration docs and an example override.
11-25:⚠️ Potential issue | 🟠 MajorStep 0 silently falls through on any unrecognized
modevalue.The three
<check>branches coverdata,validate, andinteractiveexclusively. If{{mode}}is set to anything else (e.g.,"export","debug", a typo, or an empty string that is not the literal"interactive"), none of the checks fire and the step exits without proceeding — leaving the workflow in a limbo state with no user feedback and no exit.🛡️ Proposed fix: add an else/error branch
<check if="mode == interactive"> <action>Continue to Step 1</action> </check> + + <check if="mode is none of [data, validate, interactive]"> + <output>❌ Unknown mode "{{mode}}". Valid modes are: interactive, validate, data.</output> + <action>Exit workflow</action> + </check> </step>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-status/instructions.md` around lines 11 - 25, Step 0 currently checks only for "data", "validate", and "interactive" and silently falls through on any other value of {{mode}}; add an explicit else/error branch in Step 0 that handles unrecognized modes (e.g., a final <check if="true"> or an explicit else) to either set mode = "interactive" as a safe default and continue to Step 1 or to log/raise an error and exit; update the Step 0 actions to emit a clear message referencing {{mode}} so users are informed when an invalid mode is provided and the workflow won't be left in limbo.
130-141:⚠️ Potential issue | 🟡 MinorChoice 1 ("Run recommended workflow now") only prints a message — it does not run the workflow.
The menu prompt at line 132 says "Run recommended workflow now," but the
<check if="choice == 1">block at lines 138-141 only outputs a text string telling the user to manually type a slash command. The agent does not invoke or transition to the recommended workflow. This contradicts the stated option, creating a misleading user experience — a user who selects "1" expecting automation instead receives a copy-paste suggestion.Either rename the option to "Show recommended command" or implement an actual workflow invocation if the execution engine supports it. At minimum, the prompt text should be honest about what will happen.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-status/instructions.md` around lines 130 - 141, The menu option label in step n="5" is misleading because selecting choice == 1 only emits an instructional string; update the UI to be honest by renaming option 1 from "Run recommended workflow now" to "Show recommended command" (or similar) in the <ask> text, and adjust the <check if="choice == 1"> <output> text to clearly state it will only display the slash command using {{next_workflow_id}} and {{next_story_id}}; alternatively, if the engine supports executing workflows, replace the <output> block for choice == 1 with a call/transition to execute the workflow (invoking /bmad:bmm:workflows:{{next_workflow_id}} and setting story_key={{next_story_id}} when applicable) so selection actually runs the recommended workflow.
57-81:⚠️ Potential issue | 🟠 MajorInteractive user correction block in Step 2 bleeds into data mode via "same as Step 2" reference in Step 20.
Step 20 (line 168) instructs: "Load and parse
{sprint_status_file}same as Step 2." Step 2 includes an<ask>block that prompts the user to interactively correct unrecognized statuses (lines 71-76). Whensprint-statusis invoked in data mode by another workflow (e.g., as a sub-call to retrieve counts), this interactive prompt will block automation — the calling workflow will wait indefinitely for a human response that is never expected.Data mode must define its own parse behavior that skips interactive corrections (either silently accepting unknown statuses or returning an error template-output) rather than delegating to the interactive Step 2 procedure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-status/instructions.md` around lines 57 - 81, The interactive correction (<ask> block under Step 2 that uses invalid_entries) must not run when invoked in data mode from Step 20; change the Step 20 instruction ("Load and parse {sprint_status_file} same as Step 2") to call a non-interactive parsing branch instead—implement a separate parse path that uses the same validation logic (invalid_entries) but omits the <ask> correction block and either returns a structured error/template of unrecognized statuses or silently accepts/normalizes them; update the workflow to reference the new non-interactive parser (e.g., "parse_sprint_status_data_mode") when invoked as data-mode/sub-call, leaving the interactive Step 2 unchanged for user runs.
188-213:⚠️ Potential issue | 🟡 MinorValidate mode checks field presence for
story_locationbut not that the path actually exists.Step 30 (lines 199-205) validates that required metadata fields exist, including
story_location. However, it does not verify that the path value ofstory_locationresolves to an actual directory or file on disk. A sprint-status.yaml withstory_location: /nonexistent/pathpasses validation without warning. The same gap applies to thegeneratedfield — its value is accepted without timestamp format validation.A complete validate mode should include a secondary check:
🔧 Suggested addition after the metadata-fields check
<action>Validate required metadata fields exist: generated, project, project_key, tracking_system, story_location</action> <check if="any required field missing"> ... </check> + +<action>Verify that the path in story_location exists on disk</action> +<check if="story_location path does not exist"> + <template-output>is_valid = false</template-output> + <template-output>error = "story_location path does not exist: {{story_location}}"</template-output> + <template-output>suggestion = "Check story_location in sprint-status.yaml or re-run sprint-planning"</template-output> + <action>Return</action> +</check>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-status/instructions.md` around lines 188 - 213, Step 30's validation currently only checks for presence of metadata keys; update the "Validate sprint-status file" logic to also verify that the value of story_location resolves to an existing file or directory on disk and to validate that generated is a valid timestamp (e.g., ISO-8601) before accepting it. Concretely, after the "Validate required metadata fields exist" check, add a check that attempts to resolve and stat the path in story_location and sets is_valid=false with an appropriate error/suggestion if it does not exist, and add a timestamp-parse/format check for generated that sets is_valid=false with an explanatory error/suggestion on parse failure; keep the same template-output keys (is_valid, error, suggestion) and return behavior as in the existing checks.
105-128:⚠️ Potential issue | 🟡 Minor
{sprint_status_file}in<output>block uses single-brace syntax, inconsistent with{{double-brace}}template variables.Line 112 displays
{sprint_status_file}(single braces) inside an<output>block, while all other user-facing template values in the same block use{{double-brace}}Handlebars syntax (lines 109-117). Single-brace variables appear to be agent-instruction placeholders resolved at a different stage than Handlebars template variables. Inside an<output>block, the agent may render this as a literal{sprint_status_file}string rather than the resolved path, giving users a confusing display.🔧 Proposed fix
- - Status file: {sprint_status_file} + - Status file: {{sprint_status_file}}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/sprint-status/instructions.md` around lines 105 - 128, In the <output> block inside the step with goal "Display summary" (step n="4") replace the single-brace placeholder {sprint_status_file} with the Handlebars double-brace form {{sprint_status_file}} so it is rendered consistently with the other template variables (e.g., {{project}}, {{count_backlog}}) and not treated as an unresolved agent-instruction placeholder; ensure no other single-brace placeholders remain in that <output> block.src/bmm/workflows/1-analysis/research/workflow-technical-research.md (3)
59-59:⚠️ Potential issue | 🟡 MinorSpurious mid-sentence capitalization in the communication language directive may cause incorrect AI parsing.
SPEAK OUTPUT In your Agent communication stylehas an errant capitalIon "In". In this position, an LLM may parse the instruction as two disconnected clauses —YOU MUST ALWAYS SPEAK OUTPUTandIn your Agent communication style...— potentially weakening the constraint. This is an AI-facing instruction, not a header, so accurate grammar matters.🛠️ Proposed fix
-**✅ YOU MUST ALWAYS SPEAK OUTPUT In your Agent communication style with the config `{communication_language}`** +**✅ YOU MUST ALWAYS SPEAK OUTPUT in your Agent communication style with the config `{communication_language}`**🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/1-analysis/research/workflow-technical-research.md` at line 59, The directive text contains a spurious capital "I" in "SPEAK OUTPUT In your Agent communication style" which risks incorrect LLM parsing; update the phrase to use correct sentence case/spacing (e.g., "SPEAK OUTPUT in your Agent communication style" or "SPEAK OUTPUT: in your Agent communication style") so it reads as a single clear instruction; locate and replace the exact string "SPEAK OUTPUT In your Agent communication style with the config `{communication_language}`" in the document to ensure the AI-facing constraint is grammatically correct and unambiguous.
24-27:⚠️ Potential issue | 🟡 Minor
dateis contradictorily classified and referenced with the wrong brace convention.Two entangled problems here:
Wrong list placement (lines 26–27):
dateis listed alongside config-file variables (project_name,output_folder,planning_artifacts, etc.) but then immediately qualified as "a system-generated value." Config loading is about reading from{main_config}; system-generated values are computed by the agent at runtime. Mixing them in the same bullet block invites an agent to attempt to readdatefrom the YAML config, where it won't exist.Brace mismatch with line 54: The established single-brace convention (
{var}) indicates a config-resolved variable; double-brace ({{var}}) denotes a runtime-set variable. Ifdateis being initialized during the config-loading step, it should appear as{date}in the output path — but line 54 uses{{date}}. Conversely, if it's purely runtime, it must be removed from the config block entirely.🛠️ Proposed fix
-## CONFIGURATION - -### 1. Configuration Loading - -Load and read full config from {main_config} and resolve basic variables. - - -- `project_name`, `output_folder`, `planning_artifacts`, `user_name` -- `communication_language`, `document_output_language`, `user_skill_level` -- `date` as a system-generated value +## CONFIGURATION + +### 1. Configuration Loading + +Load and read full config from {main_config} and resolve basic variables. + +- `project_name`, `output_folder`, `planning_artifacts`, `user_name` +- `communication_language`, `document_output_language`, `user_skill_level` +- Set `date` = today's date (system-generated, not from config)And in line 54, change
{{date}}→{date}to align with the config-resolved single-brace convention (assumingdateis now resolved in the loading step), or keep{{date}}ifdateis purely a runtime variable and remove it from the config block.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/1-analysis/research/workflow-technical-research.md` around lines 24 - 27, The `date` variable is incorrectly grouped with config-file keys (`project_name`, `output_folder`, `planning_artifacts`, `user_name`) while also being described as system-generated, and its brace convention is inconsistent (`{date}` vs `{{date}}`); fix by deciding whether `date` is config-resolved or runtime-generated, then update the document accordingly: if `date` is resolved during config loading, move it into the config list and use the single-brace form `{date}` in the output path; if `date` is runtime-generated, remove it from the config keys list and keep the double-brace `{{date}}` usage in the output path so the conventions for `{...}` (config) and `{{...}}` (runtime) remain consistent across the file.
54-54:⚠️ Potential issue | 🟠 Major
{planning_artifacts}may not reflect a monorepo-overriddenoutput_folder, silently routing artifacts to the wrong directory.Step 4 writes the starter file to
{planning_artifacts}/research/…. Ifplanning_artifactsis derived fromoutput_folderat config-load time, and the monorepo override adjustsoutput_folderafterplanning_artifactsis already bound, the output lands in the default (non-monorepo) path regardless.The instruction must either: (a) re-derive
planning_artifactsafter anyoutput_folderoverride is applied, or (b) compose the output path from the (potentially overridden){output_folder}directly rather than the pre-bound{planning_artifacts}.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/1-analysis/research/workflow-technical-research.md` at line 54, The starter file path uses the pre-bound planning_artifacts variable which can be stale if a monorepo overrides output_folder later; update the write logic that creates "{planning_artifacts}/research/technical-{{research_topic}}-research-{{date}}.md" to instead resolve the path from the current, possibly overridden output_folder at the time of file creation (or explicitly re-derive planning_artifacts after applying overrides), ensuring you compose the final path from output_folder + "/research/technical-{{research_topic}}-research-{{date}}.md" rather than the original planning_artifacts binding.src/bmm/workflows/2-plan-workflows/create-prd/workflow-validate-prd.md (6)
62-62:⚠️ Potential issue | 🟠 MajorHard-coded English output on Line 62 contradicts the
{communication_language}requirement stated on Line 58.Line 58 mandates the agent always speak in the configured
{communication_language}. Four lines later, a verbatim English string is emitted unconditionally. For non-English deployments, this will surface an English announcement in an otherwise localized workflow. The string should either be removed (the step file announces mode implicitly) or replaced with an instruction to announce mode in{communication_language}.✏️ Proposed fix
-"**Validate Mode: Validating an existing PRD against BMAD standards.**" +Announce to the user in `{communication_language}` that you are operating in Validate Mode: validating an existing PRD against BMAD standards.🤖 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-validate-prd.md` at line 62, The hard-coded English string "**Validate Mode: Validating an existing PRD against BMAD standards.**" should not be emitted unconditionally; replace it with either remove the line entirely or change it to a language-aware placeholder that instructs the agent to announce the mode in the configured {communication_language} (e.g., "Announce validate mode in {communication_language}"), ensuring the agent uses the {communication_language} variable rather than emitting fixed English.
5-5:⚠️ Potential issue | 🟡 MinorInconsistent path notation between frontmatter (Line 5) and inline hint (Line 64).
The
validateWorkflowfrontmatter value includes a./prefix (./steps-v/step-v-01-discovery.md), but the parenthetical hint on Line 64 omits it (steps-v/step-v-01-discovery.md). These are typically equivalent, but the discrepancy could cause confusion: an agent comparing them may not recognize they refer to the same file, or path-resolution logic that treats./and bare-relative paths differently may behave inconsistently across environments.✏️ Proposed fix — normalize the hint to match frontmatter exactly
-Then read fully and follow: `{validateWorkflow}` (steps-v/step-v-01-discovery.md) +Then read fully and follow: `{validateWorkflow}` (./steps-v/step-v-01-discovery.md)Also applies to: 64-64
🤖 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-validate-prd.md` at line 5, The frontmatter key validateWorkflow uses a ./-prefixed path ('./steps-v/step-v-01-discovery.md') but the inline hint on Line 64 uses the bare-relative path ('steps-v/step-v-01-discovery.md'); update the inline hint to use the exact same ./-prefixed form to normalize paths and avoid mismatch when comparing validateWorkflow to the hint (search for validateWorkflow and the parenthetical hint around Line 64 to locate the strings to change).
51-56:⚠️ Potential issue | 🟡 MinorNewly added blank line (Line 52) creates a double gap that structurally severs the variable list from its instruction.
Before this change there was presumably a single blank line between the instruction and the variable list. The addition of line 52 now produces two consecutive blank lines, which in Markdown signals a new paragraph/section boundary. An AI agent reading the instruction on line 51 then encountering a double blank gap before the list on line 54 may interpret the list as supplementary context rather than the required scope of "basic variables" to resolve.
✏️ Proposed fix
-Load and read full config from {main_config} and resolve basic variables. - +Load and read full config from {main_config} and resolve all required variables: - `project_name`, `output_folder`, `planning_artifacts`, `user_name`🤖 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-validate-prd.md` around lines 51 - 56, Remove the unintended extra blank line between the instruction "Load and read full config from {main_config} and resolve basic variables." and the bulleted list so the list remains the direct scope of that instruction; specifically, collapse the two consecutive blank lines into a single blank line (or no blank line) so the items `project_name`, `output_folder`, `planning_artifacts`, `user_name`, `communication_language`, `document_output_language`, `user_skill_level`, and `date` are visually and structurally attached to the instruction in workflow-validate-prd.md.
49-58:⚠️ Potential issue | 🟡 MinorWorkflow initialization documentation doesn't clarify that inline
#project:overrides are pre-processed before this workflow starts.The inline project override mechanism (
#project:NAMEor#p:NAME) is implemented in the shared context-logic component withpriority="before-config", meaning it resolves the override before the "Configuration Loading" step executes. The workflow's initialization sequence (lines 49-58) should acknowledge this, either by:
- Adding a note that "Inline project overrides are applied before configuration loading"
- Or referencing the context-logic documentation for users invoking with
#project:syntaxCurrently, a developer reading just this workflow file would have no indication that inline project overrides are supported at invocation time.
🤖 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-validate-prd.md` around lines 49 - 58, Update the "Configuration Loading" section in workflow-validate-prd.md to explicitly state that inline project overrides (e.g., `#project:NAME` or `#p:NAME`) are pre-processed before this workflow runs by the shared context-logic component (priority="before-config"); either add a short note like "Inline project overrides are applied before configuration loading" or add a brief reference/link to the context-logic documentation so readers know overrides are resolved prior to reading `project_name`, `output_folder`, and other config values.
4-4:⚠️ Potential issue | 🟡 MinorThe
{project-root}assumption is by design and not a circular dependency, but workflow documentation lacks critical monorepo guidance.
{project-root}is a system variable injected by the IDE/tooling layer at installation time (resolving to workspace root) and is available before config loading. However, the workflow-validate-prd.md does not mention:
- Monorepo context switching — The
set-projectworkflow enables per-project context via#project:NAMEsyntax, butworkflow-validate-prd.mdnever references this.- System variable injection — No explanation that
{project-root}is pre-injected by the IDE, which could confuse users in monorepo setups where workspace root ≠ logical project root.Add a section under "INITIALIZATION SEQUENCE" documenting monorepo support: if working in a monorepo, use the
set-projectworkflow or inline#project:NAMEoverride before running validation to ensure the config loads from the correct project context.🤖 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-validate-prd.md` at line 4, Add a short "Monorepo support" subsection under INITIALIZATION SEQUENCE in workflow-validate-prd.md that documents (1) that the system variable {project-root} is injected by the IDE/tooling at install time and resolves to the workspace root, and (2) how to switch logical project context in a monorepo by running the set-project workflow or using an inline `#project`:NAME override before running the validation workflow so the main_config (`{project-root}/_bmad/bmm/config.yaml`) resolves to the correct project; reference the set-project workflow name and the `#project`:NAME syntax so readers know the exact commands to use.
49-56:⚠️ Potential issue | 🟠 MajorAdd monorepo context check to initialization sequence.
The initialization sequence reads
{main_config}and resolves variables, but contains no instruction to check for_bmad/.current_projector overrideoutput_folderbased on its contents—despite this being the stated purpose of the PR. The architecture uses centralized context logic that's injected into workflows containing the{{monorepo_context_logic}}placeholder, or explicit checks like those indeep-dive-instructions.md. This workflow has neither. An agent executing this workflow has no signal thatoutput_folderis conditionally overridden when a project context file exists.Add either an explicit monorepo context check (matching the pattern in peer workflows) or include the centralized injection placeholder in the initialization sequence to ensure proper artifact routing in monorepo setups.
🤖 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-validate-prd.md` around lines 49 - 56, The initialization step in workflow-validate-prd.md is missing the monorepo context handling so output_folder won't be conditionally overridden; update the initialization sequence to either insert the centralized placeholder {{monorepo_context_logic}} or add an explicit check that reads _bmad/.current_project and, if present, overrides output_folder (and adjusts project_name/planning_artifacts as peer workflows do); ensure this logic runs immediately after loading {main_config} and before any artifact routing or date resolution so downstream steps use the corrected output_folder.src/bmm/workflows/generate-project-context/workflow.md (1)
1-4:⚠️ Potential issue | 🔴 Critical
{main_config}referenced without a declaration in the YAML frontmatter.The frontmatter only declares
nameanddescription. The new line 30 references{main_config}, which has no resolved value here — the agent will receive the literal placeholder string and fail to load the configuration. Comparecreate-product-brief/workflow.mdline 4, which correctly addsmain_config: '{project-root}/_bmad/bmm/config.yaml'to frontmatter.🐛 Proposed fix — add the missing frontmatter key
--- name: generate-project-context description: Creates a concise project-context.md file with critical rules and patterns that AI agents must follow when implementing code. Optimized for LLM context efficiency. +main_config: '{project-root}/_bmad/bmm/config.yaml' ---Also applies to: 28-30
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/generate-project-context/workflow.md` around lines 1 - 4, The YAML frontmatter for the generate-project-context workflow is missing the main_config key referenced later as {main_config}; add a frontmatter entry main_config: '{project-root}/_bmad/bmm/config.yaml' to the top of the workflow (same style used in create-product-brief/workflow.md) so the placeholder resolves at runtime and the agent receives the correct config path.src/bmm/workflows/3-solutioning/create-architecture/workflow.md (1)
1-4:⚠️ Potential issue | 🔴 Critical
{main_config}is both undefined in this file's frontmatter AND duplicated by the retained hardcoded-path instruction.This workflow has two separate problems introduced together:
Undefined placeholder: The frontmatter (lines 1–4) contains only
nameanddescription. Line 30 references{main_config}, which has no resolution source in this file. Contrastcreate-product-brief/workflow.mdwheremain_configwas explicitly added to the frontmatter in this same PR.Stale duplicate load instruction: Line 32 (unchanged from before this PR) still tells the agent to
Load config from '{project-root}/_bmad/bmm/config.yaml' and resolve:. The new line 30 was inserted on top without removing the old instruction, so the agent receives two "Load config" directives in the same section.🐛 Proposed fix — add frontmatter key and remove the duplicate
--- name: create-architecture description: Collaborative architectural decision facilitation... +main_config: '{project-root}/_bmad/bmm/config.yaml' ---### 1. Configuration Loading Load and read full config from {main_config} and resolve basic variables. -Load config from `{project-root}/_bmad/bmm/config.yaml` and resolve: - -- `project_name`, `output_folder`, `planning_artifacts`, `user_name` +- `project_name`, `output_folder`, `planning_artifacts`, `user_name`Also applies to: 28-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/3-solutioning/create-architecture/workflow.md` around lines 1 - 4, Add a frontmatter key named main_config to this create-architecture workflow (same format used in create-product-brief) so the {main_config} placeholder is defined, and remove the stale duplicate "Load config from '{project-root}/_bmad/bmm/config.yaml' and resolve:" instruction so only the new load/resolve directive referencing {main_config} remains; update the frontmatter and the workflow body around the create-architecture document to reference the single main_config source consistently.src/bmm/workflows/bmad-quick-flow/quick-spec/workflow.md (1)
67-76:⚠️ Potential issue | 🟠 MajorTwo competing "Load config" instructions in the same section — the old hardcoded path was not removed.
Line 69 (new) tells the agent to load from
{main_config}and resolve "basic variables." Line 71 (also changed in this PR) then separately instructs the agent to load from the explicit hardcoded path{project-root}/_bmad/bmm/config.yamland resolve a larger set of variables (includingproject_context). An agent executing this section will either load the config twice or be left uncertain which variable scope is authoritative.The old explicit-path instruction should be removed; its variable list should be reconciled with the "basic" scope or explicitly included in the
{main_config}bullet list.🐛 Proposed fix — remove the duplicate explicit-path instruction
### 1. Configuration Loading Load and read full config from {main_config} and resolve basic variables. -Load config from `{project-root}/_bmad/bmm/config.yaml` and resolve: -- `project_name`, `planning_artifacts`, `implementation_artifacts`, `user_name` -- `communication_language`, `document_output_language`, `user_skill_level` -- `date` as system-generated current datetime -- `project_context` = `**/project-context.md` (load if exists) -- ✅ YOU MUST ALWAYS SPEAK OUTPUT In your Agent communication style with the config `{communication_language}` +- `project_name`, `planning_artifacts`, `implementation_artifacts`, `user_name` +- `communication_language`, `document_output_language`, `user_skill_level` +- `date` as system-generated current datetime +- `project_context` = `**/project-context.md` (load if exists) +- ✅ YOU MUST ALWAYS SPEAK OUTPUT In your Agent communication style with the config `{communication_language}`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/bmad-quick-flow/quick-spec/workflow.md` around lines 67 - 76, Remove the duplicate hardcoded-load instruction that references {project-root}/_bmad/bmm/config.yaml and reconcile its variable list into the primary {main_config} loading step: keep a single "Load and read full config from {main_config}" bullet and add the missing variables (`project_name`, `planning_artifacts`, `implementation_artifacts`, `user_name`, `communication_language`, `document_output_language`, `user_skill_level`, `date`, and `project_context = **/project-context.md`) to that bullet so the agent only loads once from {main_config} and always uses {communication_language} for agent output.src/bmm/workflows/4-implementation/create-story/instructions.xml (1)
333-348:⚠️ Potential issue | 🔴 Critical
{{story_file}}is an unresolved placeholder — the completion output will display literal{{story_file}}text to the user.
{{story_file}}is never assigned anywhere in this workflow. Step 5 writes to{target_story_file}, but Step 6's completion output still references{{story_file}}(lines 338 and 342). This looks like an incomplete rename — all{default_output_file}references in<template-output>blocks were replaced with{target_story_file}, but the two occurrences in the Step 6 summary output were missed.🐛 Proposed fix
**Story Details:** - Story ID: {{story_id}} - Story Key: {{story_key}} - - File: {{story_file}} + - File: {target_story_file} - Status: ready-for-dev **Next Steps:** - 1. Review the comprehensive story in {{story_file}} + 1. Review the comprehensive story in {target_story_file}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/create-story/instructions.xml` around lines 333 - 348, The completion output still references the unresolved placeholder {{story_file}}; locate the Step 6 summary output in the <output> block (the template-output for the final-user summary) and replace the literal {{story_file}} occurrences with the correct variable {target_story_file} (or ensure {target_story_file} is assigned to a template variable named story_file before rendering) so the summary displays the actual file path; update the template-output to consistently use {target_story_file} (or map story_file → target_story_file) to fix the incomplete rename.src/core/tasks/workflow.xml (1)
42-47:⚠️ Potential issue | 🟡 MinorDead instruction inside a conditional substep —
if="template-workflow"makes the action-workflow branch unreachableSubstep 1d is gated with
if="template-workflow", so it never executes for action-workflows. Yet its body contains<action>If action-workflow → Skip file creation</action>. This instruction is unreachable from within this substep and could confuse an AI agent reading the workflow definition, potentially causing it to attempt to reconcile a condition that cannot be true.Either remove the
ifattribute so both cases are handled inside this single substep, or remove the unreachable action-workflow instruction entirely.🛠️ Option A — drop the conditional, handle both cases in one substep
- <substep n="1d" title="Initialize Output" if="template-workflow"> + <substep n="1d" title="Initialize Output"> <action>Resolve default_output_file path with all variables and {{date}}</action> <action>Create output directory if doesn't exist</action> <action>If template-workflow → Write template to output file with placeholders</action> <action>If action-workflow → Skip file creation</action> </substep>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/tasks/workflow.xml` around lines 42 - 47, Substep n="1d" is currently restricted by if="template-workflow" yet contains an unreachable action "<action>If action-workflow → Skip file creation</action>"; remove the dead instruction or broaden the substep so it applies to both workflows. Either delete the unreachable action-workflow line from substep n="1d", or remove the if="template-workflow" attribute so the substep handles both cases and implement explicit branching inside the substep (keep the "Resolve default_output_file" and "Create output directory" actions and then conditionally write template vs skip file creation).tools/cli/installers/lib/core/installer.js (1)
65-81:⚠️ Potential issue | 🟡 MinorStale
@paramin JSDoc — parameter doesn't exist in the method signatureLine 71 documents
@param {string} bmadFolderNamewith a default of'bmad', butcopyFileWithPlaceholderReplacement(sourcePath, targetPath)takes no such parameter. The function resolves the folder name fromthis.bmadFolderName. The declared default'bmad'also conflicts with the actual value (BMAD_FOLDER_NAME, which is'_bmad').🛠️ Proposed fix
- * `@param` {string} bmadFolderName - User's chosen bmad folder name (default: 'bmad') - * `@returns` {Promise<void>} Resolves when file copy and transformation complete + * `@returns` {Promise<void>} Resolves when file copy and transformation complete + * `@note` Uses this.bmadFolderName (default: BMAD_FOLDER_NAME = '_bmad') for _bmad placeholder replacement🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/core/installer.js` around lines 65 - 81, The JSDoc for copyFileWithPlaceholderReplacement incorrectly documents a non-existent parameter bmadFolderName and a wrong default; remove the stale "@param {string} bmadFolderName" entry (or replace it with a brief note that the method uses this.bmadFolderName / BMAD_FOLDER_NAME) so the doc matches the actual signature copyFileWithPlaceholderReplacement(sourcePath, targetPath) and the real folder name (BMAD_FOLDER_NAME = '_bmad'); ensure any mention of defaults reflects the class property this.bmadFolderName rather than a literal 'bmad'.
🧹 Nitpick comments (5)
src/bmm/workflows/2-plan-workflows/create-prd/workflow-validate-prd.md (3)
64-64: Hard-coded path hint on Line 64 is redundant with the{validateWorkflow}variable and will silently diverge if the frontmatter is updated.The parenthetical
(steps-v/step-v-01-discovery.md)simply restates the resolved value of{validateWorkflow}. Any future rename or relocation of the step file would require updating both the frontmatter variable and this inline hint. An AI following the literal hint when the variable has changed would load the wrong step file — precisely the type of failure the variable abstraction is meant to prevent.✏️ Proposed fix — remove the redundant literal hint
-Then read fully and follow: `{validateWorkflow}` (steps-v/step-v-01-discovery.md) +Then read fully and follow: `{validateWorkflow}`🤖 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-validate-prd.md` at line 64, The inline hard-coded path "(steps-v/step-v-01-discovery.md)" duplicates the `{validateWorkflow}` frontmatter variable and risks divergence; remove the parenthetical hint so the document relies solely on the `{validateWorkflow}` variable (leave the sentence "Then read fully and follow: `{validateWorkflow}`" intact) to prevent future mismatches when the step file is renamed or relocated.
49-56: No error handling if{main_config}cannot be loaded — a realistic failure mode in monorepo setups.The initialization sequence instructs the agent to load
{main_config}but provides no guidance for failure cases: file not found, YAML parse error, missing required keys. In a monorepo context (exactly this PR's target use case), an incorrectly set{project-root}will silently yield a missing config. The agent will then have nooutput_folder,project_name, or language settings — and will almost certainly hallucinate placeholder values and continue, writing outputs to unpredictable paths.Add an explicit failure branch, for example:
If `{main_config}` cannot be read or is missing required keys, halt immediately and inform the user of the missing configuration file path. Do not proceed with default or assumed values.🤖 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-validate-prd.md` around lines 49 - 56, The workflow step that loads {main_config} must explicitly handle failure modes: if reading/parsing {main_config} fails or required keys (project_name, output_folder, planning_artifacts, user_name, communication_language, document_output_language, user_skill_level) are missing, immediately halt and return a clear error to the user (including the expected {main_config} path and which keys are missing) instead of continuing with defaults; update the "Configuration Loading" logic to validate the parsed config, implement a failure branch that stops execution and reports the problem, and ensure the generated date is only used after successful validation.
22-26: ThestepsCompletedtracking condition ("when a workflow produces a document") conflicts with the unconditionalstepsCompletedmandate in Step Processing Rules.Line 25 states state tracking should be done "when a workflow produces a document" — a conditional. Lines 34 and 42 in Step Processing Rules unconditionally mandate updating
stepsCompletedbefore loading the next step and when writing final output. For a validation workflow that may or may not write a persistent output file, an AI agent seeing the conditional on line 25 might skip state tracking entirely, preventing resumability. The condition should either be removed or the validate workflow should explicitly document whether it produces an output file.🤖 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-validate-prd.md` around lines 22 - 26, The "stepsCompleted" guidance is inconsistent: either remove the conditional phrase "when a workflow produces a document" or explicitly state that the validate workflow does (or does not) produce a persistent output and therefore must update stepsCompleted; update the Step Processing Rules to match (so the unconditional mandates to update stepsCompleted before loading the next step and when writing final output are consistent with the workflow description), and ensure all references to stepsCompleted in the validate workflow and Step Processing Rules are identical so an agent cannot skip state tracking.tools/cli/installers/lib/ide/_config-driven.js (1)
394-395:requirecalled insiderenderTemplate— hoisted to module scope.
renderTemplateis invoked once per artifact (insidewriteAgentArtifacts,writeWorkflowArtifacts,writeTaskToolArtifactsloops). Callingrequireon every invocation is a code smell; the module is cached by Node.js, but the intent and maintainability both suffer. Move it to the top of the file alongside the other requires.♻️ Proposed refactor
At the top of the file (line ~8, alongside the other requires):
const { TaskToolCommandGenerator } = require('./shared/task-tool-command-generator'); +const { MONOREPO_CONTEXT_LOGIC } = require('./shared/context-logic');Inside
renderTemplate, remove the inline require:- const { MONOREPO_CONTEXT_LOGIC } = require('./shared/context-logic'); - let rendered = template🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 394 - 395, The inline require of MONOREPO_CONTEXT_LOGIC inside renderTemplate should be hoisted to module scope: add const { MONOREPO_CONTEXT_LOGIC } = require('./shared/context-logic'); alongside the other top-level requires, then remove the in-function require from renderTemplate so the function uses the hoisted constant; update any references in renderTemplate to use MONOREPO_CONTEXT_LOGIC directly.test/test-monorepo-validation.js (1)
24-31: Duplicate test infrastructure across both test files
test-monorepo-validation.js(lines 24–47) andtest-context-logic-integration.js(lines 17–38) define byte-for-byte identical ANSI color tables and near-identical assertion helpers (assert/okwith the same signature). If the output format or color codes need to change, two files must be updated in lockstep.Extract a shared
test/test-helpers.jswithcolors, anok()helper, andrunSummary()so both suites import from one place.Also applies to: 36-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/test-monorepo-validation.js` around lines 24 - 31, Extract the duplicated ANSI color table (colors), the ok/assert helper, and runSummary into a single shared module and have both test suites import them instead of redefining; specifically, create a module that exports colors, ok(), and runSummary, update the two test files that currently define the identical colors object and ok helper to import those symbols, and remove the duplicated definitions in each test so changes to color codes or output format are made in one place.
src/bmm/workflows/2-plan-workflows/create-prd/workflow-validate-prd.md
Outdated
Show resolved
Hide resolved
| .replaceAll('{{monorepo_context_logic}}', MONOREPO_CONTEXT_LOGIC); | ||
|
|
||
| // Replace _bmad placeholder with actual folder name | ||
| rendered = rendered.replaceAll('_bmad', this.bmadFolderName); |
There was a problem hiding this comment.
_bmad-output path in MONOREPO_CONTEXT_LOGIC is corrupted by the broad replaceAll('_bmad', ...) replacement for any non-default bmadFolderName.
After MONOREPO_CONTEXT_LOGIC is injected at line 402, line 405 runs rendered.replaceAll('_bmad', this.bmadFolderName) as a raw substring replacement across the entire rendered string. MONOREPO_CONTEXT_LOGIC contains two distinct _bmad substrings:
| String in MONOREPO_CONTEXT_LOGIC | Intended result | Actual result (e.g. bmadFolderName = '.bmad') |
|---|---|---|
{project-root}/_bmad/.current_project |
{project-root}/.bmad/.current_project ✅ |
{project-root}/.bmad/.current_project ✅ |
{project-root}/_bmad-output/{project_suffix} |
unchanged (_bmad-output is a fixed output dir convention) |
{project-root}/.bmad-output/{project_suffix} ❌ |
The monorepo output directory _bmad-output is a fixed convention independent of the config folder name. Renaming _bmad → bmadFolderName inside it silently breaks project output routing for any installation where the BMAD folder is customized.
🐛 Option A — use `{{bmadFolderName}}` in `context-logic.js` for the config-dir reference (preferred)
In tools/cli/installers/lib/ide/shared/context-logic.js, replace the literal _bmad config-dir references with the template placeholder so only those resolve, while _bmad-output stays literal:
- <check if="project_suffix not yet set AND {project-root}/_bmad/.current_project exists">
- <action>Read {project-root}/_bmad/.current_project as project_suffix</action>
+ <check if="project_suffix not yet set AND {project-root}/{{bmadFolderName}}/.current_project exists">
+ <action>Read {project-root}/{{bmadFolderName}}/.current_project as project_suffix</action>Line 405 in _config-driven.js can then be kept as-is (or removed if _bmad no longer appears in templates literally); {{bmadFolderName}} will be substituted by the existing line 408.
Option B — make the replacement more precise in _config-driven.js (minimal change):
- rendered = rendered.replaceAll('_bmad', this.bmadFolderName);
+ // Use word-boundary replacement to avoid corrupting '_bmad-output'
+ rendered = rendered.replace(/_bmad(?!-output\b)/g, this.bmadFolderName);📝 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.
| .replaceAll('{{monorepo_context_logic}}', MONOREPO_CONTEXT_LOGIC); | |
| // Replace _bmad placeholder with actual folder name | |
| rendered = rendered.replaceAll('_bmad', this.bmadFolderName); | |
| .replaceAll('{{monorepo_context_logic}}', MONOREPO_CONTEXT_LOGIC); | |
| // Use word-boundary replacement to avoid corrupting '_bmad-output' | |
| rendered = rendered.replace(/_bmad(?!-output\b)/g, this.bmadFolderName); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 402 - 405, The
global replaceAll('_bmad', this.bmadFolderName) corrupts the fixed output dir
`_bmad-output`; update the template and replacement: change the config-dir
literal `_bmad` occurrences inside MONOREPO_CONTEXT_LOGIC to use a placeholder
like `{{bmadFolderName}}` (so only config-dir references are substituted) and
keep the `_bmad-output` literal unchanged, then let the existing
.replaceAll('{{bmadFolderName}}', this.bmadFolderName) perform the substitution;
alternatively, if you prefer a minimal patch in _config-driven.js, make the
replacement precise (e.g., only replace path segment patterns such as '/_bmad/'
or use a token boundary) instead of a blind replaceAll('_bmad', ...).
There was a problem hiding this comment.
Rate Limit Exceeded
@serrnovik have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 26 seconds before sending another message.
| <check if="project_suffix contains '..' OR starts with '/' OR starts with '\\'"> | ||
| <output>🚫 Security Error: Invalid project context — path traversal detected.</output> | ||
| <action>HALT</action> | ||
| </check> | ||
| <check if="project_suffix matching regex [^a-zA-Z0-9._-]"> | ||
| <output>🚫 Error: project_suffix must only contain alphanumeric characters, dots, dashes, or underscores.</output> | ||
| <action>HALT</action> | ||
| </check> |
There was a problem hiding this comment.
Regex check is inconsistent with the inline fallback checks in both advanced-elicitation/workflow.xml and deep-dive-instructions.md
Three divergences exist between the centralized version and the inline checks:
context-logic.js |
Inline XML checks | |
|---|---|---|
| Verb | matching |
matches |
| Quoting | [^a-zA-Z0-9._-] (unquoted) |
`'[^a-zA-Z0-9._-] |
| Empty guard | absent | ^\s*$ present |
The empty-string case is handled implicitly by the outer <check if="project_suffix is set"> wrapper, so it's not an active bug. But the two representations will diverge when one is updated without the other. Since MONOREPO_CONTEXT_LOGIC is the canonical single source of truth, the inline checks were clearly hand-edited separately — and given this inconsistency already exists, the drift will only widen.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/cli/installers/lib/ide/shared/context-logic.js` around lines 28 - 35,
Update the centralized MONOREPO_CONTEXT_LOGIC to match the inline XML checks:
change the check condition on project_suffix from "matching regex
[^a-zA-Z0-9._-]" to use the same operator and quoted pattern as the inline
checks (use "matches" and the pattern "'[^a-zA-Z0-9._-]|^\s*$'"), so the verb
and regex string are identical to the inline checks and prevent future drift
while keeping the existing outer "project_suffix is set" guard intact.
There was a problem hiding this comment.
Rate Limit Exceeded
@serrnovik have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 32 seconds before sending another message.
| <steps CRITICAL="TRUE"> | ||
| 0. {{monorepo_context_logic}} | ||
|
|
There was a problem hiding this comment.
{{monorepo_context_logic}} is injected INSIDE <steps> as step 0 — structurally inconsistent with every other template and contradicts priority="before-config".
Every other template (workflow-command-template.md, workflow-commander.md, default-workflow.md, windsurf-workflow.md) places the monorepo context block as a standalone pre-execution block outside any structural wrapper. Here it is embedded inside <steps CRITICAL="TRUE">, where the rendered <monorepo-context-check> XML becomes a child element of <steps>. An AI agent parsing the <steps> XML may treat the monorepo check as step 0 of the YAML workflow execution sequence rather than a prerequisite initialization, violating the declared priority="before-config" semantics.
🐛 Proposed fix
---
name: '{{name}}'
description: '{{description}}'
disable-model-invocation: true
---
+{{monorepo_context_logic}}
+
IT IS CRITICAL THAT YOU FOLLOW THESE STEPS - while staying in character as the current agent persona you may have loaded:
<steps CRITICAL="TRUE">
-0. {{monorepo_context_logic}}
-
1. Always LOAD the FULL @{project-root}/{{bmadFolderName}}/core/tasks/workflow.xml📝 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.
| <steps CRITICAL="TRUE"> | |
| 0. {{monorepo_context_logic}} | |
| --- | |
| name: '{{name}}' | |
| description: '{{description}}' | |
| disable-model-invocation: true | |
| --- | |
| {{monorepo_context_logic}} | |
| IT IS CRITICAL THAT YOU FOLLOW THESE STEPS - while staying in character as the current agent persona you may have loaded: | |
| <steps CRITICAL="TRUE"> | |
| 1. Always LOAD the FULL @{project-root}/{{bmadFolderName}}/core/tasks/workflow.xml |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/cli/installers/lib/ide/templates/combined/default-workflow-yaml.md`
around lines 9 - 11, The injected {{monorepo_context_logic}} must not be placed
inside the <steps CRITICAL="TRUE"> wrapper; remove it from being step 0 and
render it as a standalone pre-execution block (the <monorepo-context-check>
element) before the <steps> element so it respects priority="before-config" and
does not appear as a workflow step; update the template that generates
default-workflow-yaml.md to output {{monorepo_context_logic}} immediately before
the <steps ...> section instead of inside it.
There was a problem hiding this comment.
Rate Limit Exceeded
@serrnovik have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 18 seconds before sending another message.
| IT IS CRITICAL THAT YOU FOLLOW THIS COMMAND: LOAD the FULL #[[file:{{bmadFolderName}}/{{path}}]], READ its entire contents and follow its directions exactly! | ||
| {{monorepo_context_logic}} | ||
|
|
||
| IT IS CRITICAL THAT YOU FOLLOW THIS COMMAND: LOAD the FULL @{project-root}/{{bmadFolderName}}/{{path}}, READ its entire contents and follow its directions exactly! |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for [[file:...]] patterns in templates
echo "=== Searching for [[file:...]] syntax ==="
rg -n '\[\[file:' tools/cli/installers/lib/ide/templates/ --type md -A1
echo ""
echo "=== Searching for @{project-root} syntax ==="
rg -n '@{project-root}' tools/cli/installers/lib/ide/templates/ --type md
echo ""
echo "=== Examine the actual file ==="
cat -n tools/cli/installers/lib/ide/templates/combined/kiro-workflow.mdRepository: bmad-code-org/BMAD-METHOD
Length of output: 1836
🏁 Script executed:
#!/bin/bash
# Double-check: search for any use of @{ or project-root in templates to confirm isolation
echo "=== Searching for any @ prefix patterns ==="
rg -n '@\{' tools/cli/installers/lib/ide/templates/ --type md
echo ""
echo "=== Searching for 'project-root' text anywhere ==="
rg -n 'project-root' tools/cli/installers/lib/ide/templates/ --type md
echo ""
echo "=== Get full context around kiro-workflow.md ==="
cat -n tools/cli/installers/lib/ide/templates/combined/kiro-workflow.mdRepository: bmad-code-org/BMAD-METHOD
Length of output: 5161
Inconsistent file-reference syntax within Kiro templates—align kiro-workflow.md with other Kiro-specific templates
All other Kiro-specific templates use #[[file:{{bmadFolderName}}/{{path}}]] for file references:
- kiro-tool.md (line 7)
- kiro-task.md (line 7)
- kiro-agent.md (line 10)
- kiro-workflow-yaml.md (lines 10–11)
Line 9 of kiro-workflow.md uses @{project-root}/{{bmadFolderName}}/{{path}} instead. If #[[file:...]] is Kiro's standard file-inclusion mechanism, this deviation may break content auto-loading. Align with the established Kiro pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/cli/installers/lib/ide/templates/combined/kiro-workflow.md` at line 9,
Replace the non-standard file-reference token in kiro-workflow.md (the string
starting "LOAD the FULL @{project-root}/{{bmadFolderName}}/{{path}}") with
Kiro's standard inclusion syntax used by the other templates: use
#[[file:{{bmadFolderName}}/{{path}}]] in that sentence so it reads "LOAD the
FULL #[[file:{{bmadFolderName}}/{{path}}]]" (preserving the surrounding
instruction text), and then verify the updated line matches the pattern used in
kiro-tool.md, kiro-task.md, kiro-agent.md, and kiro-workflow-yaml.md.
There was a problem hiding this comment.
Rate Limit Exceeded
@serrnovik have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 17 seconds before sending another message.
| WORKFLOW INSTRUCTIONS: | ||
| {{monorepo_context_logic}} | ||
|
|
||
| 1. LOAD the workflow file from {project-root}/{{bmadFolderName}}/{{path}} |
There was a problem hiding this comment.
{{monorepo_context_logic}} is embedded under WORKFLOW INSTRUCTIONS: — semantically misplaced vs every other template.
All other updated templates (default-workflow.md, workflow-command-template.md, workflow-commander.md) inject the monorepo context block as a top-level pre-execution block, before any heading. Here it sits inside the "WORKFLOW INSTRUCTIONS:" section, which signals to the AI that context detection is an optional workflow step rather than a mandatory initialization prerequisite. This inconsistency could cause an AI agent to skip or defer the context check.
🐛 Proposed fix
---
name: '{{name}}'
description: '{{description}}'
---
+{{monorepo_context_logic}}
+
Execute the BMAD '{{name}}' workflow.
CRITICAL: You must load and follow the workflow definition exactly.
WORKFLOW INSTRUCTIONS:
-{{monorepo_context_logic}}
1. LOAD the workflow file from {project-root}/{{bmadFolderName}}/{{path}}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/cli/installers/lib/ide/templates/combined/opencode-workflow.md` around
lines 10 - 13, The monorepo context block `{{monorepo_context_logic}}` is
currently embedded under the "WORKFLOW INSTRUCTIONS:" section which misleads
agents into treating context detection as a workflow step; move
`{{monorepo_context_logic}}` out of the "WORKFLOW INSTRUCTIONS:" block and place
it as a top-level pre-execution block before any headings (i.e., before the
"WORKFLOW INSTRUCTIONS:" line) to match the placement used by
`default-workflow.md`, `workflow-command-template.md`, and
`workflow-commander.md` so context detection runs as a mandatory initialization
step.
There was a problem hiding this comment.
Rate Limit Exceeded
@serrnovik have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 9 seconds before sending another message.
… order, and template paths
580dc67 to
89e310a
Compare
What
Implemented monorepo support for BMAD-METHOD, introducing a dynamic artifact pathing mechanism based on a local project context file. This update allows workflows to automatically route their outputs to project-specific subdirectories within
_bmad-output/.Why
This change enables cleaner organization and branch switching in monorepo environments by centralizing methodology files while isolating project artifacts. It avoids methodology duplication and re-configuration overhead when working across multiple projects in a single shared repository.
How
_bmad/.current_projectand override theoutput_folderdynamically./set-projectworkflow to comfortably set, clear, and sanitize project contexts from the IDE.Testing
Added a new
test:monorepovalidation suite in BMAD-METHOD to verify that all 31 core/BMM workflows correctly implement the required context logic.