Skip to content

fix(skill): clean up bmad-create-architecture validation findings #1987

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alexeyv merged 2 commits into from
Mar 14, 2026
Merged

fix(skill): clean up bmad-create-architecture validation findings #1987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
089c19d
fix(skill): clean up bmad-create-architecture validation findings
alexeyv Mar 14, 2026
7e6c26c
Merge branch 'main' into fix/create-architecture-skill-cleanup
alexeyv Mar 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/bmm/workflows/3-solutioning/bmad-create-architecture/steps/step-01-init.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ If no document exists or no `stepsCompleted` in frontmatter:
Discover and load context documents using smart discovery. Documents can be in the following locations:
- {planning_artifacts}/**
- {output_folder}/**
- {product_knowledge}/**
- docs/**
- {project_knowledge}/**
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

{project_knowledge} introduces a variable-contract compatibility risk.

This rename can break runs where upstream config/context still exposes {product_knowledge} (which is still present in sibling workflow steps). Add a compatibility fallback (check both keys) or a migration note in the step contract.

Based on learnings: converted workflow families still contain {product_knowledge} usages (e.g., src/bmm/workflows/2-plan-workflows/bmad-create-ux-design/steps/step-01-init.md:55-71).

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@src/bmm/workflows/3-solutioning/bmad-create-architecture/steps/step-01-init.md`
at line 60, The step changed the contract key from {product_knowledge} to
{project_knowledge}, which can break consumers still providing the old key;
update the step (step-01-init.md) to accept both keys by checking for
{project_knowledge} first and falling back to {product_knowledge} (or vice
versa) when reading the input, and add a short migration note in the step’s
header documenting the rename and advising downstream workflows to migrate to
{project_knowledge}; ensure references in this file and any related helpers
consistently handle both keys until migration is complete.

Copy link
Copy Markdown

@augmentcode augmentcode bot Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{project_knowledge} is used here for discovery, but workflow.md doesn’t list project_knowledge in its “Load config … and resolve” section; is it guaranteed to be available for this workflow? If not, this placeholder may not resolve and document discovery could silently miss the intended folder.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

- {project-root}/docs/**
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Hard-pinning to {project-root}/docs/** removes resilience to alternate docs roots.

You dropped the bare docs/** pattern entirely. If {project-root} resolution is unavailable or misconfigured in an execution context, discovery for docs collapses. Keep a fallback probe for docs/**.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@src/bmm/workflows/3-solutioning/bmad-create-architecture/steps/step-01-init.md`
at line 61, The glob pattern list currently hard-pins documentation discovery to
"{project-root}/docs/**" and removed the bare "docs/**" fallback; restore
resilience by adding back the plain "docs/**" pattern alongside
"{project-root}/docs/**" so discovery works even when {project-root} resolution
fails—update the pattern array or docs globs referenced in step-01-init.md to
include both "{project-root}/docs/**" and "docs/**".


Also - when searching - documents can be a single markdown file, or a folder with an index and multiple files. For Example, if searching for `*foo*.md` and not found, also search for a folder called *foo*/index.md (which indicates sharded content)

Expand All @@ -67,7 +67,7 @@ Try to discover the following:
- Product Requirements Document (`*prd*.md`)
- UX Design (`*ux-design*.md`) and other
- Research Documents (`*research*.md`)
- Project Documentation (generally multiple documents might be found for this in the `{product_knowledge}` or `docs` folder.)
- Project Documentation (generally multiple documents might be found for this in the `{project_knowledge}` or `{project-root}/docs` folder.)
- Project Context (`**/project-context.md`)

<critical>Confirm what you have found with the user, along with asking if the user wants to provide anything else. Only after this confirmation will you proceed to follow the loading rules</critical>
Expand Down Expand Up @@ -95,7 +95,7 @@ Before proceeding, verify we have the essential inputs:

#### C. Create Initial Document

Copy the template from `{installed_path}/architecture-decision-template.md` to `{planning_artifacts}/architecture.md`
Copy the template from `../architecture-decision-template.md` to `{planning_artifacts}/architecture.md`
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Copy failure path is missing but success is reported unconditionally later.

The step later states “Created ... from template” regardless of copy success. Add a mandatory success check before any completion/reporting branch.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@src/bmm/workflows/3-solutioning/bmad-create-architecture/steps/step-01-init.md`
at line 98, The step unconditionally reports "Created ... from template" even if
the copy failed; update the logic that copies
../architecture-decision-template.md to {planning_artifacts}/architecture.md to
check the copy operation result (e.g., check the command exit code or verify the
destination file exists and is readable) and only proceed to the success/report
branch when that check passes; if the check fails, return/raise an error or
enter the failure reporting branch so the step reports the copy failure instead
of success.


#### D. Complete Initialization and Report

Expand Down
6 changes: 0 additions & 6 deletions src/bmm/workflows/3-solutioning/bmad-create-architecture/workflow.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,6 @@ Load config from `{project-root}/_bmad/bmm/config.yaml` and resolve:
- `date` as system-generated current datetime
- ✅ YOU MUST ALWAYS SPEAK OUTPUT In your Agent communication style with the config `{communication_language}`

### Paths

- `installed_path` = `.`
- `template_path` = `{installed_path}/architecture-decision-template.md`
- `data_files_path` = `{installed_path}/data/`

---

## EXECUTION
Expand Down
Loading