refactor: remove legacy YAML/XML workflow engine plumbing#1864
Merged
alexeyv merged 12 commits intobmad-code-org:mainfrom Mar 9, 2026
Merged
refactor: remove legacy YAML/XML workflow engine plumbing#1864alexeyv merged 12 commits intobmad-code-org:mainfrom
alexeyv merged 12 commits intobmad-code-org:mainfrom
Conversation
…view guidelines All workflows have been converted to markdown. Remove workflow.yaml, workflow.xml, and config_source references from Augment review rules. Drop the entire xml_workflows section (5 rules) and the YAML-specific standard_workflow_instructions rule.
…located markdown Convert the discover_inputs XML protocol (FULL_LOAD, SELECTIVE_LOAD, INDEX_GUIDED strategies) into standalone markdown files placed alongside the two workflows that use it (create-story, code-review). Replace <invoke-protocol> tags with explicit file references. This decouples the workflows from workflow.xml, enabling its deletion in a follow-up. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove 5 files made obsolete by the workflow.yaml → workflow.md migration: - workflow.xml (the YAML workflow interpreter engine) - dev-story/instructions.xml (superseded by workflow.md) - 3 installer templates for YAML workflow command generation References in CLI code will be cleaned up in follow-up commits. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These handler fragments were deleted — the exec handler already covers loading .md workflow files directly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
@coderabbitai review |
🤖 Augment PR SummarySummary: This PR continues the migration away from the legacy YAML/XML workflow engine by treating Changes:
Technical Notes: Workflow discovery now depends on YAML frontmatter in markdown files; any remaining workflow/validate-workflow handler references should be kept consistent across compiler/analyzer and agent-components. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThis PR performs a major architectural migration from XML/YAML-based workflows to Markdown-based workflow files with frontmatter. The legacy Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
tools/cli/installers/lib/ide/shared/workflow-command-generator.js (6)
149-154:⚠️ Potential issue | 🟡 MinorMissing descriptions will render as the literal string
undefined.
collectWorkflowArtifacts()already falls back to${workflow.name} workflow, but this method interpolatesworkflow.descriptiondirectly. That leaksundefinedinto user-facing command content when the CSV omits the field.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js` around lines 149 - 154, The template replacement chain in workflow-command-generator.js uses workflow.description directly which can render "undefined" for missing CSV descriptions; update the replacement to use a safe fallback (e.g., use workflow.description || `${workflow.name} workflow`) so the .replaceAll('{{description}}', ...) call substitutes a sensible default; locate the replacement sequence (the return template.replaceAll(...).replaceAll('{{description}}', ...).replaceAll('{{workflow_path}}', workflowPath).replaceAll('_bmad', this.bmadFolderName)) and change the description argument accordingly to mirror collectWorkflowArtifacts()'s fallback.
73-79:⚠️ Potential issue | 🟠 MajorThis branch ignores custom BMAD folder names.
The constructor takes
bmadFolderName, but this normalization still strips only the literal_bmad/. Any non-default install root will keep the wrong prefix here and diverge from the rest of the generated paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js` around lines 73 - 79, The normalization currently strips only the literal '_bmad/' which breaks custom BMAD roots; update the logic that normalizes workflowRelPath to use the constructor-provided bmadFolderName instead of the hard-coded '_bmad/'. Specifically, replace the includes('_bmad/') and split(/_bmad\//) checks with tests that use bmadFolderName + '/' (or an escaped RegExp built from bmadFolderName) so workflowRelPath is correctly trimmed for custom folder names; ensure you update references around workflowRelPath handling in the same function to keep path construction consistent with the constructor argument.
87-97:⚠️ Potential issue | 🟠 MajorMalformed manifest rows are not rejected before file generation.
name,module, andpathare consumed immediately for filenames, directories, and path transforms, but none are validated. One incomplete CSV row is enough to throw mid-install or emit broken command artifacts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js` around lines 87 - 97, The code is using workflow.name, workflow.module and workflow.path directly when building artifacts (in the artifacts.push block and when computing workflowRelPath) without validating CSV rows; add validation at the start of the generator for each workflow entry to ensure required fields (name, module, path) are non-empty and sane (e.g., strings without path traversal), and either skip malformed rows with a logged warning or throw a clear error; after validation proceed to build artifacts using workflow.name, workflow.module, workflow.path, workflowRelPath and commandContent as before so generation never attempts to write files from incomplete input.
80-85:⚠️ Potential issue | 🟠 MajorThe
/src/rewrite can grab the wrong segment from the checkout path.
/\/src\/([^/]+)\/(.+)/matches the first/src/anywhere in the string. A repo checked out under something like/Users/alex/src/repos/.../src/bmm/...will normalize torepos/...instead of the workflow path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js` around lines 80 - 85, The current regex that rewrites workflowRelPath (/\/src\/([^/]+)\/(.+)/) matches the first "/src/" in the string and thus can pick the wrong segment; update the logic that normalizes workflowRelPath (the block that tests workflowRelPath.includes('/src/') and assigns workflowRelPath = `${match[1]}/${match[2]}`) to target the last "/src/" instead of the first — e.g. either compute lastIndexOf('/src/') and substring from there or use a greedy prefix regex like /^.*\/src\/([^/]+)\/(.+)/ so the capture groups come from the final src/ segment and produce the correct `${match[1]}/${match[2]}` result.
129-145:⚠️ Potential issue | 🟠 MajorThis rewrite is still POSIX-only.
workflow.pathis matched before any separator normalization, so Windows manifest entries likeC:\repo\src\bmm\...will not hit either branch. The generated command will leak the machine-local source path instead of{project-root}/....🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js` around lines 129 - 145, workflow.path is checked with POSIX separators so Windows paths like C:\repo\src\bmm\... don't match and the original source path can leak; normalize separators before matching by converting backslashes to forward slashes (operate on the workflowPath local variable) and then run the existing /src/bmm/ and /src/core/ regex branches (references: workflow.path, workflowPath, the match calls and this.bmadFolderName usage) so Windows manifests are rewritten into the {project-root}/_bmad/... form instead of leaking machine-local paths.
69-85:⚠️ Potential issue | 🟠 Major
workflowPathstops one directory too early for source-tree inputs.Line 84 rewrites
/src/<module>/...to<module>/..., but the field is documented as relative to the actual installed workflow file. That drops the BMAD install root, so any consumer ofartifact.workflowPathwill point outside{project-root}/${this.bmadFolderName}.Suggested fix
} else if (workflowRelPath.includes('/src/')) { // Normalize source paths (e.g. .../src/bmm/...) to relative module path (e.g. bmm/...) const match = workflowRelPath.match(/\/src\/([^/]+)\/(.+)/); if (match) { - workflowRelPath = `${match[1]}/${match[2]}`; + workflowRelPath = `${this.bmadFolderName}/${match[1]}/${match[2]}`; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js` around lines 69 - 85, The path normalization for source-tree inputs currently strips the BMAD install root by rewriting matches of /src/<module>/... to <module>/..., causing artifact.workflowPath to point outside the BMAD folder; update the logic that handles the /src/ match (the regex match = workflowRelPath.match(/\/src\/([^/]+)\/(.+)/) and the assignment to workflowRelPath) to preserve or prepend the BMAD install root (this.bmadFolderName) so the resulting workflowRelPath is rooted under {this.bmadFolderName}/<module>/..., and ensure the same behavior works for both absolute and relative input paths and for the other branch that strips _bmad/.src/bmm/workflows/4-implementation/create-story/checklist.md (1)
49-55:⚠️ Potential issue | 🟡 Minor
Validation frameworkis no longer an actionable input.In fresh-context mode, Lines 49-55 list this as a required input, but “the workflow's checklist execution system” is just a label. It does not tell the agent what file or instructions to load, so the checklist now depends on invisible framework behavior.
🤖 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/checklist.md` around lines 49 - 55, Update the "Required Inputs" section in create-story/checklist.md so the "Validation framework" entry becomes an actionable input: replace the vague label with a concrete file or instruction reference (e.g., validation_rules_file, checklist_runner_config, or path to the checklist execution script) and describe the expected format or loader to use; ensure the entry points to the actual file or command the workflow should load/run (refer to the "Required Inputs" header and the "Validation framework" bullet) so agents in fresh-context mode can deterministically find and execute the checklist.tools/cli/lib/agent-analyzer.js (2)
53-65:⚠️ Potential issue | 🟠 MajorLegacy
workflow:menu items are now orphaned.
tools/cli/lib/agent/compiler.jsstill emitsworkflow="..."for non-multi menu items, but this analyzer no longer addsworkflowtousedAttributesin the legacy path.ActivationBuilder.buildHandlers()only loads fragments named inusedAttributes, so any remaining legacy agents will compile without the handler instructions their menus still reference.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/lib/agent-analyzer.js` around lines 53 - 65, The analyzer misses legacy menu items' workflow attribute, so update the legacy-path attribute checks in tools/cli/lib/agent-analyzer.js (the block that currently checks item.exec, item.tmpl, item.data, item.action) to also detect item.workflow and add 'workflow' to profile.usedAttributes; this ensures ActivationBuilder.buildHandlers() will load the workflow fragments that tools/cli/lib/agent/compiler.js still emits for non-multi menu items.
37-46:⚠️ Potential issue | 🔴 CriticalCritical mismatch: agent YAML still defines workflow handlers, but analyzer no longer detects them.
Lines 37–46 now record only
execandactionattributes, dropping workflow detection. However, multiple.agent.yamlfiles actively useworkflow:andvalidate-workflow:in menu items (e.g., dev.agent.yaml, sm.agent.yaml, qa.agent.yaml, tech-writer.agent.yaml, analyst.agent.yaml, pm.agent.yaml, quick-flow-solo-dev.agent.yaml). If activation-builder loads handler fragments strictly fromusedAttributes, these workflow handlers will fail to load on agent startup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/lib/agent-analyzer.js` around lines 37 - 46, The analyzer loop over item.triggers is only adding 'exec' and 'action' to profile.usedAttributes and ignores workflow-related handler keys, causing workflow/validate-workflow handlers to be omitted; update the logic in the loop that inspects exec.type and the exec object (the code around item.triggers, for (const triggerGroup of item.triggers), for (const [triggerName, execArray] of Object.entries(triggerGroup)) and the checks using exec.type) to also detect and add 'workflow' and 'validate-workflow' (or any other handler keys used in .agent.yaml) to profile.usedAttributes whenever those keys are present or when exec.type equals those handler names so activation-builder will load workflow handler fragments.tools/cli/installers/lib/modules/manager.js (2)
1091-1102:⚠️ Potential issue | 🔴 CriticalLine 1097 reads the module capture, not the workflow subpath.
For the regex on Lines 1091-1092, group 2 is the destination module and group 3 is the workflow subpath from the example comment. Using
installMatch[2]makesactualDestWorkflowPathland under<target>/workflows/<module>instead of the requested workflow directory.Proposed fix
- const installWorkflowSubPath = installMatch[2]; + const installWorkflowSubPath = installMatch[3];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/modules/manager.js` around lines 1091 - 1102, The code is using the wrong regex capture for the dest workflow subpath: installMatch[2] is the destination module while installMatch[3] is the requested workflow path, so change the assignment of installWorkflowSubPath to use installMatch[3] (and keep the existing .replace(/\/workflow\.md$/, '') logic) so actualDestWorkflowPath builds as path.join(targetPath, 'workflows', installMatch[3].replace(/\/workflow\.md$/, '')); verify installMatch is checked earlier and leave the parse failure log and continue behavior unchanged.
1046-1049:⚠️ Potential issue | 🟠 MajorVendoring only inspects top-level agent YAMLs.
Lines 1047-1048 do a single
readdir()onagents/, whilecompileModuleAgents()later usesfindAgentFiles()recursively. A nested.agent.yamlcan therefore compile successfully while itsworkflow-installdependencies are never copied.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/modules/manager.js` around lines 1046 - 1049, The vendoring step currently uses fs.readdir on sourceAgentsPath to collect only top-level agent YAMLs (agentFiles/yamlFiles) which misses nested .agent.yaml files and their workflow-install dependencies; replace the single-level read with the existing recursive helper findAgentFiles(sourceAgentsPath) (or call compileModuleAgents()’s discovery logic) to enumerate all agent YAMLs under sourceAgentsPath, then filter for '.agent.yaml' / '.yaml' as before so nested agents and their workflow-install dependencies are discovered and copied correctly (update any code that references agentFiles/yamlFiles to use the new recursive list).tools/cli/installers/lib/core/manifest-generator.js (1)
369-375:⚠️ Potential issue | 🟡 MinorNormalize quoted
standalone: "false"here too.
tools/cli/installers/lib/ide/_base-ide.jstreats both booleanfalseand string'false'as non-standalone, but this collector only checks the boolean on Line 370. A quoted value will therefore be hidden from IDE discovery but still published intoworkflow-manifest.csv.Proposed fix
- if (workflow.standalone === false) { + if (workflow.standalone === false || workflow.standalone === 'false') { if (debug) { console.log(`[DEBUG] Skipped (standalone=false): ${workflow.name}`); } continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/core/manifest-generator.js` around lines 369 - 375, The check for non-standalone workflows only treats the boolean false and misses the string "false"; update the condition that uses workflow.standalone so it treats both boolean false and string 'false' as non-standalone (e.g., replace the current if (workflow.standalone === false) with a normalized check like if (workflow.standalone === false || String(workflow.standalone).toLowerCase() === 'false')), keeping the existing debug console.log(`[DEBUG] Skipped (standalone=false): ${workflow.name}`) behavior intact.
🧹 Nitpick comments (6)
tools/cli/installers/lib/ide/shared/workflow-command-generator.js (3)
124-127: Template load failures need actionable context.If
workflow-commander.mdis moved or omitted, this throws a bareENOENTwith no clue which workflow generation path failed. Wrap the read with a message that includes the template path and current workflow name/module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js` around lines 124 - 127, When loading the template via templatePath and fs.readFile for 'workflow-commander.md', wrap the read in a try/catch and rethrow or log an error that includes the templatePath and the current workflow identifier (e.g., workflow.name or moduleName) so failures show which workflow generation path failed; update the code around templatePath and the template = await fs.readFile(...) call to include this contextual information in the error message.
213-216: The launcher now hardcodes an unconditional persistence rule.“Save outputs after EACH section” is broader than the actual workflow contract. Some workflows only persist at explicit checkpoints, so baking this into every generated README will cause incorrect writes or duplicate saves.
Based on learnings: the "Save outputs after EACH section when generating any documents from templates" directive is a pre-existing pattern that should be made conditional and idempotent across affected templates.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js` around lines 213 - 216, The README generator in workflow-command-generator.js is unconditionally inserting the "Save outputs after EACH section" persistence directive; make this conditional and idempotent by: update the function that composes the README (e.g., generateReadme or renderWorkflowTemplate) to only insert the per-section persistence line when the workflow/template metadata explicitly requests it (e.g., workflow.config.persistOutputPerSection === true or template contains an explicit checkpoint flag), and add a small dedupe step (e.g., ensureUniqueDirective or normalizePersistenceDirectives) before writing so repeated runs don't duplicate the line. Ensure the new logic checks both workflow metadata and template markers, and that the insertion point is the same place currently used so behavior remains stable when the flag is absent.
124-127: The fixed template should be cached, not re-read for every workflow.Now that the path is constant, this file does unnecessary disk I/O once per workflow. Cache the template on the generator instance or load it once per batch.
Suggested refactor
class WorkflowCommandGenerator { constructor(bmadFolderName = BMAD_FOLDER_NAME) { this.bmadFolderName = bmadFolderName; + this.workflowCommandTemplate = null; } @@ async generateCommandContent(workflow, bmadDir) { const templatePath = path.join(__dirname, '../templates/workflow-commander.md'); // Load the template - const template = await fs.readFile(templatePath, 'utf8'); + if (!this.workflowCommandTemplate) { + this.workflowCommandTemplate = await fs.readFile(templatePath, 'utf8'); + } + const template = this.workflowCommandTemplate;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js` around lines 124 - 127, The code currently reads the constant template file for every workflow via templatePath and await fs.readFile; change this to cache the loaded template so you avoid repeated disk I/O. Implement a module-level or instance-level cache (e.g., a module-scoped cachedTemplate variable or this.cachedTemplate on the generator class) and on first use load the template with fs.readFile into that cache, then return the cached string for subsequent calls instead of re-reading the file in the code that references templatePath and template.docs/reference/commands.md (1)
30-30: Call this the workflow file, not a workflow config.The installed skill loads
workflow.mdas the entry document, so “workflow config” still sounds like the retired engine-handoff model. “Workflow definition” orworkflow.mdwould be clearer. Based on learnings:workflow.mdis always the canonical entry file for verbatim skill directories.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/commands.md` at line 30, Update the phrasing in the docs where it currently says "workflow config" (the table cell under "**Workflow skill**") to use "workflow file" or explicitly "workflow.md" (or "workflow definition") so it clearly reflects that the installed skill loads workflow.md as the canonical entry document; replace the phrase "Loads the workflow config and follows its steps" with something like "Loads the workflow.md entry file and follows its steps" to remove ambiguity with the old engine-handoff model.src/bmm/workflows/4-implementation/code-review/discover-inputs.md (1)
1-88: Extract this shared protocol instead of copy/pasting it.This file and
src/bmm/workflows/4-implementation/create-story/discover-inputs.mdare effectively the same instructions. Any fix to selection rules, ordering, or error handling now has to be kept in sync manually, which is how drift gets reintroduced. Move the protocol to one shared source or generate these files from a template.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/code-review/discover-inputs.md` around lines 1 - 88, The two nearly identical protocols in src/bmm/workflows/4-implementation/code-review/discover-inputs.md and src/bmm/workflows/4-implementation/create-story/discover-inputs.md should be deduplicated: extract the shared content into a single canonical resource (e.g., src/bmm/workflows/shared/discover-inputs.md or a templated source) and replace the duplicated files with a short include/loader that pulls the shared protocol (or generate both files from the single template at build time); update any consumers to reference the new shared symbol instead of the duplicated copy so selection rules, ordering, and error handling are maintained in one place.tools/cli/lib/agent/compiler.js (1)
211-248: KeepprocessExecArray()andbuildNestedHandlers()in sync.After Line 232, this parser only carries the route-based path forward, while
buildNestedHandlers()still has workflow-specific branches. Clean up the dead branches or restore the missing fields here before the next handler migration turns the mismatch into a runtime bug.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/lib/agent/compiler.js` around lines 211 - 248, processExecArray currently only forwards route, data, action, type and description but omits workflow-related fields that buildNestedHandlers still expects; update processExecArray to mirror buildNestedHandlers by also capturing workflow-specific properties (e.g., exec.workflow, exec.path, exec.workflowStep or other workflow-related keys used by buildNestedHandlers) and ensure result.workflow (and any other workflow fields buildNestedHandlers reads) are set when present, or remove the workflow branches from buildNestedHandlers if you prefer to drop workflow support—make the two functions consistent so no workflow fields are lost during the handler migration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/reference/commands.md`:
- Line 30: Replace any remaining "engine" phrasing so the doc is consistent:
change the table row with "**Workflow skill**" and any sentences that say "load
the workflow engine" or "load the workflow engine and pass a specific workflow
configuration" (e.g., the paragraph at lines ~91-92) to instead say that
workflow skills "load the workflow config and follow its steps" or "receive/load
a workflow configuration and execute its steps" so the page consistently
describes the new flow without mentioning a workflow engine.
In `@src/bmm/workflows/4-implementation/create-story/discover-inputs.md`:
- Around line 63-67: The fallback that loads all glob hits into
{pattern_name_content} must instead select a single canonical file or prompt on
ambiguity: update the logic in the whole-pattern fallback (the step that handles
`{planning_artifacts}/*prd*.md` and populates `{pattern_name_content}`) to (1)
filter out drafts/archived files (e.g., names/paths containing "draft" or
"archive"), (2) prefer the most recent or highest-versioned file when versioned
copies exist, and (3) if multiple candidates still remain, either apply a
deterministic tie-breaker (e.g., prefer README or highest lexical precedence) or
surface a user prompt/choice. Ensure the selected file’s full contents are
assigned to `{pattern_name_content}` (not all matches), and document the
canonical-choice rules where the whole-document fallback is implemented.
In `@src/utility/agent-components/activation-steps.txt`:
- Line 14: Update the EXECUTE_STEP dispatch contract to include the "multi"
execution type so selected menu items with type="multi" are routed to the
runtime's multi handler instead of falling through; specifically, edit the step
whose n="{EXECUTE_STEP}" that currently lists workflow, exec, tmpl, data, action
and add "multi" to that enumeration and any matching dispatch logic that checks
those attribute values so it invokes the dedicated multi handler.
In `@src/utility/agent-components/handler-multi.txt`:
- Around line 8-10: Multi-only multi-trigger items can reference handler
type="workflow" that never gets loaded because tools/cli/lib/agent-analyzer.js
doesn't add 'workflow' when inspecting nested multi triggers and
tools/cli/lib/activation-builder.js only loads fragments from
profile.usedAttributes; update the loading logic so referenced workflow handlers
are always included. Specifically, in tools/cli/lib/agent-analyzer.js, detect
nested multi triggers and add 'workflow' to the set of available handlers when
any nested handler string or pattern points to a workflow; alternatively (or
additionally), update tools/cli/lib/activation-builder.js to treat
handler-workflow.txt fragments referenced by multi items as used (override
profile.usedAttributes for multi nested handler references) so
handler-workflow.txt is guaranteed to be loaded into the activation block.
Ensure references to handler type="workflow" and handler-workflow.txt are
handled consistently across agent-analyzer.js and activation-builder.js.
In `@tools/cli/installers/lib/core/manifest-generator.js`:
- Around line 347-354: The code silently skips markdown files lacking YAML
frontmatter by continuing when frontmatterMatch is falsy; change this to surface
an error for missing frontmatter (for example by emitting a clear
console.error/processLogger.error including fullPath and the filename like
workflow.md and either throwing or recording the failure) instead of continuing
silently — update the block around frontmatterMatch (and any surrounding
function that handles parsing/manifest generation) to log the error and fail the
manifest generation or add the file to an errors list so missing frontmatter is
visible in normal installs (retain the existing debug log but do not rely on
it).
In `@tools/cli/installers/lib/ide/_base-ide.js`:
- Around line 349-353: The code currently silently skips files when frontmatter
is missing (frontmatterMatch) and swallows read/parse exceptions; instead, in
the try block that reads fullPath with fs.readFile and parses YAML (yaml.parse),
throw or propagate a descriptive Error when frontmatterMatch is falsy and do not
use continue to hide the failure, and rethrow or surface any exceptions caught
during reading/parsing rather than swallowing them (update the surrounding
try/catch to log/throw an Error that includes fullPath and the underlying error
message so callers of this routine can detect malformed workflow.md files).
- Around line 343-346: Recursion in findWorkflowFiles currently computes
relativePath against the current subdirectory (dir) which causes nested
workflows to get paths anchored to leaf dirs; change the recursive call so it
passes down the original scan root (or preserve the original root in a parameter
like rootDir) and compute relativePath using that root, ensuring relativePath is
always derived from the original workflows root rather than dir when pushing
items into workflows; update any places that compute relativePath (references to
relativePath, dir, fullPath in findWorkflowFiles) to use the preserved root.
In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js`:
- Around line 124-127: The generateCommandContent function currently renders a
hardcoded template (workflow-commander.md) and sets artifact.content but
writeWorkflowArtifacts later ignores artifact.content and always calls
renderTemplate with the installer config template; to fix, remove the hardcoded
template rendering from generateCommandContent (or delete the function if
unused) so you don’t render wasted content, or alternatively modify
writeWorkflowArtifacts to prefer artifact.content (if present) before calling
renderTemplate; ensure changes reference generateCommandContent,
collectWorkflowArtifacts, writeWorkflowArtifacts, renderTemplate, and
artifact.content so the flow either supplies pre-rendered artifact.content or
always uses the config-driven render path consistently.
---
Outside diff comments:
In `@src/bmm/workflows/4-implementation/create-story/checklist.md`:
- Around line 49-55: Update the "Required Inputs" section in
create-story/checklist.md so the "Validation framework" entry becomes an
actionable input: replace the vague label with a concrete file or instruction
reference (e.g., validation_rules_file, checklist_runner_config, or path to the
checklist execution script) and describe the expected format or loader to use;
ensure the entry points to the actual file or command the workflow should
load/run (refer to the "Required Inputs" header and the "Validation framework"
bullet) so agents in fresh-context mode can deterministically find and execute
the checklist.
In `@tools/cli/installers/lib/core/manifest-generator.js`:
- Around line 369-375: The check for non-standalone workflows only treats the
boolean false and misses the string "false"; update the condition that uses
workflow.standalone so it treats both boolean false and string 'false' as
non-standalone (e.g., replace the current if (workflow.standalone === false)
with a normalized check like if (workflow.standalone === false ||
String(workflow.standalone).toLowerCase() === 'false')), keeping the existing
debug console.log(`[DEBUG] Skipped (standalone=false): ${workflow.name}`)
behavior intact.
In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js`:
- Around line 149-154: The template replacement chain in
workflow-command-generator.js uses workflow.description directly which can
render "undefined" for missing CSV descriptions; update the replacement to use a
safe fallback (e.g., use workflow.description || `${workflow.name} workflow`) so
the .replaceAll('{{description}}', ...) call substitutes a sensible default;
locate the replacement sequence (the return
template.replaceAll(...).replaceAll('{{description}}',
...).replaceAll('{{workflow_path}}', workflowPath).replaceAll('_bmad',
this.bmadFolderName)) and change the description argument accordingly to mirror
collectWorkflowArtifacts()'s fallback.
- Around line 73-79: The normalization currently strips only the literal
'_bmad/' which breaks custom BMAD roots; update the logic that normalizes
workflowRelPath to use the constructor-provided bmadFolderName instead of the
hard-coded '_bmad/'. Specifically, replace the includes('_bmad/') and
split(/_bmad\//) checks with tests that use bmadFolderName + '/' (or an escaped
RegExp built from bmadFolderName) so workflowRelPath is correctly trimmed for
custom folder names; ensure you update references around workflowRelPath
handling in the same function to keep path construction consistent with the
constructor argument.
- Around line 87-97: The code is using workflow.name, workflow.module and
workflow.path directly when building artifacts (in the artifacts.push block and
when computing workflowRelPath) without validating CSV rows; add validation at
the start of the generator for each workflow entry to ensure required fields
(name, module, path) are non-empty and sane (e.g., strings without path
traversal), and either skip malformed rows with a logged warning or throw a
clear error; after validation proceed to build artifacts using workflow.name,
workflow.module, workflow.path, workflowRelPath and commandContent as before so
generation never attempts to write files from incomplete input.
- Around line 80-85: The current regex that rewrites workflowRelPath
(/\/src\/([^/]+)\/(.+)/) matches the first "/src/" in the string and thus can
pick the wrong segment; update the logic that normalizes workflowRelPath (the
block that tests workflowRelPath.includes('/src/') and assigns workflowRelPath =
`${match[1]}/${match[2]}`) to target the last "/src/" instead of the first —
e.g. either compute lastIndexOf('/src/') and substring from there or use a
greedy prefix regex like /^.*\/src\/([^/]+)\/(.+)/ so the capture groups come
from the final src/ segment and produce the correct `${match[1]}/${match[2]}`
result.
- Around line 129-145: workflow.path is checked with POSIX separators so Windows
paths like C:\repo\src\bmm\... don't match and the original source path can
leak; normalize separators before matching by converting backslashes to forward
slashes (operate on the workflowPath local variable) and then run the existing
/src/bmm/ and /src/core/ regex branches (references: workflow.path,
workflowPath, the match calls and this.bmadFolderName usage) so Windows
manifests are rewritten into the {project-root}/_bmad/... form instead of
leaking machine-local paths.
- Around line 69-85: The path normalization for source-tree inputs currently
strips the BMAD install root by rewriting matches of /src/<module>/... to
<module>/..., causing artifact.workflowPath to point outside the BMAD folder;
update the logic that handles the /src/ match (the regex match =
workflowRelPath.match(/\/src\/([^/]+)\/(.+)/) and the assignment to
workflowRelPath) to preserve or prepend the BMAD install root
(this.bmadFolderName) so the resulting workflowRelPath is rooted under
{this.bmadFolderName}/<module>/..., and ensure the same behavior works for both
absolute and relative input paths and for the other branch that strips _bmad/.
In `@tools/cli/installers/lib/modules/manager.js`:
- Around line 1091-1102: The code is using the wrong regex capture for the dest
workflow subpath: installMatch[2] is the destination module while
installMatch[3] is the requested workflow path, so change the assignment of
installWorkflowSubPath to use installMatch[3] (and keep the existing
.replace(/\/workflow\.md$/, '') logic) so actualDestWorkflowPath builds as
path.join(targetPath, 'workflows', installMatch[3].replace(/\/workflow\.md$/,
'')); verify installMatch is checked earlier and leave the parse failure log and
continue behavior unchanged.
- Around line 1046-1049: The vendoring step currently uses fs.readdir on
sourceAgentsPath to collect only top-level agent YAMLs (agentFiles/yamlFiles)
which misses nested .agent.yaml files and their workflow-install dependencies;
replace the single-level read with the existing recursive helper
findAgentFiles(sourceAgentsPath) (or call compileModuleAgents()’s discovery
logic) to enumerate all agent YAMLs under sourceAgentsPath, then filter for
'.agent.yaml' / '.yaml' as before so nested agents and their workflow-install
dependencies are discovered and copied correctly (update any code that
references agentFiles/yamlFiles to use the new recursive list).
In `@tools/cli/lib/agent-analyzer.js`:
- Around line 53-65: The analyzer misses legacy menu items' workflow attribute,
so update the legacy-path attribute checks in tools/cli/lib/agent-analyzer.js
(the block that currently checks item.exec, item.tmpl, item.data, item.action)
to also detect item.workflow and add 'workflow' to profile.usedAttributes; this
ensures ActivationBuilder.buildHandlers() will load the workflow fragments that
tools/cli/lib/agent/compiler.js still emits for non-multi menu items.
- Around line 37-46: The analyzer loop over item.triggers is only adding 'exec'
and 'action' to profile.usedAttributes and ignores workflow-related handler
keys, causing workflow/validate-workflow handlers to be omitted; update the
logic in the loop that inspects exec.type and the exec object (the code around
item.triggers, for (const triggerGroup of item.triggers), for (const
[triggerName, execArray] of Object.entries(triggerGroup)) and the checks using
exec.type) to also detect and add 'workflow' and 'validate-workflow' (or any
other handler keys used in .agent.yaml) to profile.usedAttributes whenever those
keys are present or when exec.type equals those handler names so
activation-builder will load workflow handler fragments.
---
Nitpick comments:
In `@docs/reference/commands.md`:
- Line 30: Update the phrasing in the docs where it currently says "workflow
config" (the table cell under "**Workflow skill**") to use "workflow file" or
explicitly "workflow.md" (or "workflow definition") so it clearly reflects that
the installed skill loads workflow.md as the canonical entry document; replace
the phrase "Loads the workflow config and follows its steps" with something like
"Loads the workflow.md entry file and follows its steps" to remove ambiguity
with the old engine-handoff model.
In `@src/bmm/workflows/4-implementation/code-review/discover-inputs.md`:
- Around line 1-88: The two nearly identical protocols in
src/bmm/workflows/4-implementation/code-review/discover-inputs.md and
src/bmm/workflows/4-implementation/create-story/discover-inputs.md should be
deduplicated: extract the shared content into a single canonical resource (e.g.,
src/bmm/workflows/shared/discover-inputs.md or a templated source) and replace
the duplicated files with a short include/loader that pulls the shared protocol
(or generate both files from the single template at build time); update any
consumers to reference the new shared symbol instead of the duplicated copy so
selection rules, ordering, and error handling are maintained in one place.
In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js`:
- Around line 124-127: When loading the template via templatePath and
fs.readFile for 'workflow-commander.md', wrap the read in a try/catch and
rethrow or log an error that includes the templatePath and the current workflow
identifier (e.g., workflow.name or moduleName) so failures show which workflow
generation path failed; update the code around templatePath and the template =
await fs.readFile(...) call to include this contextual information in the error
message.
- Around line 213-216: The README generator in workflow-command-generator.js is
unconditionally inserting the "Save outputs after EACH section" persistence
directive; make this conditional and idempotent by: update the function that
composes the README (e.g., generateReadme or renderWorkflowTemplate) to only
insert the per-section persistence line when the workflow/template metadata
explicitly requests it (e.g., workflow.config.persistOutputPerSection === true
or template contains an explicit checkpoint flag), and add a small dedupe step
(e.g., ensureUniqueDirective or normalizePersistenceDirectives) before writing
so repeated runs don't duplicate the line. Ensure the new logic checks both
workflow metadata and template markers, and that the insertion point is the same
place currently used so behavior remains stable when the flag is absent.
- Around line 124-127: The code currently reads the constant template file for
every workflow via templatePath and await fs.readFile; change this to cache the
loaded template so you avoid repeated disk I/O. Implement a module-level or
instance-level cache (e.g., a module-scoped cachedTemplate variable or
this.cachedTemplate on the generator class) and on first use load the template
with fs.readFile into that cache, then return the cached string for subsequent
calls instead of re-reading the file in the code that references templatePath
and template.
In `@tools/cli/lib/agent/compiler.js`:
- Around line 211-248: processExecArray currently only forwards route, data,
action, type and description but omits workflow-related fields that
buildNestedHandlers still expects; update processExecArray to mirror
buildNestedHandlers by also capturing workflow-specific properties (e.g.,
exec.workflow, exec.path, exec.workflowStep or other workflow-related keys used
by buildNestedHandlers) and ensure result.workflow (and any other workflow
fields buildNestedHandlers reads) are set when present, or remove the workflow
branches from buildNestedHandlers if you prefer to drop workflow support—make
the two functions consistent so no workflow fields are lost during the handler
migration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 92ba740c-8640-4c8f-8cdf-7d0258a68053
⛔ Files ignored due to path filters (2)
.augment/code_review_guidelines.yamlis excluded by!.augment/**test/fixtures/file-refs-csv/valid/bmm-style.csvis excluded by!**/*.csv,!test/fixtures/**
📒 Files selected for processing (25)
docs/reference/commands.mdsrc/bmm/workflows/4-implementation/code-review/discover-inputs.mdsrc/bmm/workflows/4-implementation/code-review/workflow.mdsrc/bmm/workflows/4-implementation/create-story/checklist.mdsrc/bmm/workflows/4-implementation/create-story/discover-inputs.mdsrc/bmm/workflows/4-implementation/create-story/workflow.mdsrc/bmm/workflows/4-implementation/dev-story/instructions.xmlsrc/core/tasks/bmad-skill-manifest.yamlsrc/core/tasks/workflow.xmlsrc/utility/agent-components/activation-steps.txtsrc/utility/agent-components/handler-multi.txtsrc/utility/agent-components/handler-validate-workflow.txtsrc/utility/agent-components/handler-workflow.txttest/test-file-refs-csv.jstools/cli/installers/lib/core/manifest-generator.jstools/cli/installers/lib/ide/_base-ide.jstools/cli/installers/lib/ide/_config-driven.jstools/cli/installers/lib/ide/shared/path-utils.jstools/cli/installers/lib/ide/shared/workflow-command-generator.jstools/cli/installers/lib/ide/templates/combined/default-workflow-yaml.mdtools/cli/installers/lib/ide/templates/combined/kiro-workflow-yaml.mdtools/cli/installers/lib/ide/templates/workflow-command-template.mdtools/cli/installers/lib/modules/manager.jstools/cli/lib/agent-analyzer.jstools/cli/lib/agent/compiler.js
💤 Files with no reviewable changes (8)
- src/utility/agent-components/handler-validate-workflow.txt
- tools/cli/installers/lib/ide/templates/combined/kiro-workflow-yaml.md
- src/bmm/workflows/4-implementation/dev-story/instructions.xml
- tools/cli/installers/lib/ide/templates/combined/default-workflow-yaml.md
- tools/cli/installers/lib/ide/templates/workflow-command-template.md
- src/core/tasks/workflow.xml
- src/utility/agent-components/handler-workflow.txt
- src/core/tasks/bmad-skill-manifest.yaml
tools/cli/installers/lib/ide/shared/workflow-command-generator.js
Outdated
Show resolved
Hide resolved
- Fix regex capture group index in module manager workflow path parsing - Remove stale workflow handler references from handler-multi.txt - Replace workflow with multi in activation-steps dispatch contract - Remove dead validate-workflow emission from compiler and xml-builder - Align commands.md wording to remove engine references - Fix relativePath anchoring in _base-ide.js recursive directory scans - Remove dead code from workflow-command-generator (unused template, generateCommandContent, writeColonArtifacts, writeDashArtifacts) - Delete unused workflow-commander.md template - Add regression test for workflow path regex
nikolasdehor
added a commit
to nikolasdehor/BMAD-METHOD
that referenced
this pull request
Mar 11, 2026
The validate-workflow task was accidentally deleted during the XML-to-native-skill refactor in bmad-code-org#1864. The schema, handler, and test fixtures still reference it, creating broken invocations. Recreates the task as a native skill directory following the established pattern (SKILL.md + bmad-skill-manifest.yaml + workflow.md), with the original validation logic preserved. Fixes bmad-code-org#1530
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Context
All 9 workflow.yaml files have been converted to workflow.md. The YAML workflow engine (workflow.xml) and its supporting plumbing are now dead code.
Test plan