fix(templates): replace @ path prefixes with {project-root}#1769
Conversation
|
@coderabbitai review |
🤖 Augment PR SummarySummary: Normalizes BMAD installer command/workflow templates to use explicit 🤖 Was this summary useful? React with 👍 or 👎 |
📝 WalkthroughWalkthroughThis PR normalizes path conventions across template files by converting Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utility/agent-components/handler-workflow.txt (1)
1-3:⚠️ Potential issue | 🟡 MinorYAML-specific example still contradicts workflow-config wording update.
Line 2 still instructs
workflow="path/to/workflow.yaml", which reintroduces the same ambiguity this PR is trying to remove. Use a neutral example likepath/to/workflow-config.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utility/agent-components/handler-workflow.txt` around lines 1 - 3, The example in the handler block still uses workflow="path/to/workflow.yaml", which conflicts with the updated wording; update the example under the <handler type="workflow"> section to use a neutral identifier like workflow="path/to/workflow-config" (or similar non-file-specific token) so it no longer implies a file extension and matches the workflow-config wording change.
🧹 Nitpick comments (3)
tools/cli/installers/lib/ide/templates/workflow-commander.md (1)
5-5: Use a quoted path token in the critical command string.Line 5 embeds an unquoted interpolated path. If a generated path contains spaces, parsing becomes brittle for downstream LLM/tool consumers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/templates/workflow-commander.md` at line 5, The interpolated path in the critical command string ("LOAD the FULL {project-root}/{{workflow_path}}") is unquoted and can break if the generated path contains spaces; update the template to wrap the interpolated token in quotes (e.g., use "LOAD the FULL \"{project-root}/{{workflow_path}}\"" or equivalent) so the {project-root}/{{workflow_path}} token is always treated as a single quoted path token at runtime.tools/cli/installers/lib/ide/templates/combined/default-workflow.md (1)
6-6: Quote and normalize the interpolated workflow path.Line 6 assumes
{{path}}is clean. If it starts with/or contains spaces, the generated instruction can produce ambiguous or malformed path text. Add an explicit normalization rule in template wording (e.g., “without leading slash”) and quote the final path token.🤖 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.md` at line 6, The template assumes {{path}} is already normalized; update the template wording in combined/default-workflow.md to explicitly require/describe a normalized, quoted path: instruct users to provide {{path}} without a leading slash (or strip any leading slash) and to quote the final path token (e.g., wrap {{path}} in quotes) so spaces are preserved and the full generated path {project-root}/{{bmadFolderName}}/{{path}} is unambiguous; reference the {{path}} and {{bmadFolderName}} tokens in the message so maintainers know which variables to adjust.src/utility/agent-components/handler-workflow.txt (1)
9-9:"todo"sentinel check is too brittle and case-sensitive.Line 9 only catches exact lowercase
"todo". Values like"TODO"or whitespace-padded placeholders will bypass the guard and fail later.Proposed fix
-6. If workflow-config path is "todo", inform user the workflow hasn't been implemented yet +6. If workflow-config path normalizes to "todo" (case-insensitive, trimmed), inform user the workflow hasn't been implemented yet🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utility/agent-components/handler-workflow.txt` at line 9, The current sentinel check only matches the exact lowercase "todo"; update the guard around the workflow-config path (e.g., the variable workflowConfigPath used in the workflow handling function such as handleWorkflow/processWorkflowRequest) to robustly detect placeholder values by first ensuring the value is defined, then normalizing it via trim() and toLowerCase() and comparing against "todo" (and optionally other placeholders like "" or "n/a"); if matched, return the existing "not implemented" response. This fixes case/whitespace brittleness and prevents later failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/cli/installers/lib/ide/templates/agent-command-template.md`:
- Line 9: The template hardcodes the BMAD folder `_bmad` in the agent path
string causing mismatches with configurable setups; update the line that
currently reads "LOAD the FULL agent file from
{project-root}/_bmad/{{module}}/agents/{{path}}" to use the template variable
`{{bmadFolderName}}` instead of `_bmad` so it becomes "LOAD the FULL agent file
from {project-root}/{{bmadFolderName}}/{{module}}/agents/{{path}}", ensuring any
references in agent-command-template.md use `{{bmadFolderName}}` consistently.
In `@tools/cli/installers/lib/ide/templates/combined/default-workflow-yaml.md`:
- Around line 10-11: The template currently concatenates
{project-root}/{{bmadFolderName}}/ with {{path}} which can produce malformed
rooted paths if {{path}} starts with a slash; update the template and any
generator logic that builds the workflow-config parameter (used when passing
'workflow-config' to workflow.xml) to normalize or enforce that {{path}} is
relative-only (strip any leading slashes or validate/reject leading separators)
so the final value is always "{project-root}/{{bmadFolderName}}/<relative-path>"
and cannot produce double or rooted slashes.
In `@tools/cli/installers/lib/ide/templates/combined/kiro-workflow-yaml.md`:
- Line 12: Add explicit documentation to the kiro-workflow-yaml.md template
explaining that Kiro intentionally uses relative workflow-config paths (e.g.,
passing {{bmadFolderName}}/{{path}} as the 'workflow-config' parameter) for
Org-mode compatibility, and contrast this with the other templates (claude-,
default-, opencode-workflow-yaml) which use
{project-root}/{{bmadFolderName}}/{{path}}; update the explanatory text near the
instruction that passes the 'workflow-config' parameter to state the rationale,
the expected runtime behavior, and guidance for maintainers on when to use
relative vs {project-root}/-rooted paths.
In `@tools/cli/installers/lib/ide/templates/workflow-command-template.md`:
- Line 8: Replace the hardcoded `_bmad` folder segment in the
workflow-command-template.md path with the template variable
`{{bmadFolderName}}` so the path always uses the configured BMAD folder name;
update the reference that currently reads
{project-root}/_bmad/core/tasks/workflow.xml to use
{project-root}/{{bmadFolderName}}/core/tasks/workflow.xml (ensure any
surrounding template syntax in workflow-command-template.md is preserved).
- Around line 9-10: The template currently prefixes "{project-root}/" to
{{workflow_path}} without ensuring the path is relative; update renderTemplate()
to validate and normalize pathToUse (and/or ensure workflowPath before
assignment in workflow-command-generator.js) so it cannot be absolute or already
contain "{project-root}" or a leading "/" — if pathToUse is absolute or contains
"{project-root}", strip the project-root prefix or convert to a safe relative
path and reject or normalize malformed inputs; ensure artifact.workflowPath is
set to the validated/normalized relative workflowPath before any concatenation
or template substitution.
In `@tools/cli/installers/lib/ide/templates/workflow-commander.md`:
- Line 5: The templates are inconsistent about whether {{workflow_path}} is
relative or absolute; decide on the relative-only convention (as used in
workflow-commander.md which uses "{project-root}/{{workflow_path}}") and update
all other templates that currently embed a hardcoded "_bmad" prefix (rovodev.md,
windsurf-workflow.md, trae.md, gemini-workflow.toml, and any similar files) to
remove the duplicated "_bmad" segment so they use
"{project-root}/{{workflow_path}}" instead; alternatively, if some templates
must keep the prefix, add a short documented note in those templates clarifying
they expect an absolute path, but preferably normalize all templates to the
relative-only contract for {{workflow_path}}.
---
Outside diff comments:
In `@src/utility/agent-components/handler-workflow.txt`:
- Around line 1-3: The example in the handler block still uses
workflow="path/to/workflow.yaml", which conflicts with the updated wording;
update the example under the <handler type="workflow"> section to use a neutral
identifier like workflow="path/to/workflow-config" (or similar non-file-specific
token) so it no longer implies a file extension and matches the workflow-config
wording change.
---
Nitpick comments:
In `@src/utility/agent-components/handler-workflow.txt`:
- Line 9: The current sentinel check only matches the exact lowercase "todo";
update the guard around the workflow-config path (e.g., the variable
workflowConfigPath used in the workflow handling function such as
handleWorkflow/processWorkflowRequest) to robustly detect placeholder values by
first ensuring the value is defined, then normalizing it via trim() and
toLowerCase() and comparing against "todo" (and optionally other placeholders
like "" or "n/a"); if matched, return the existing "not implemented" response.
This fixes case/whitespace brittleness and prevents later failures.
In `@tools/cli/installers/lib/ide/templates/combined/default-workflow.md`:
- Line 6: The template assumes {{path}} is already normalized; update the
template wording in combined/default-workflow.md to explicitly require/describe
a normalized, quoted path: instruct users to provide {{path}} without a leading
slash (or strip any leading slash) and to quote the final path token (e.g., wrap
{{path}} in quotes) so spaces are preserved and the full generated path
{project-root}/{{bmadFolderName}}/{{path}} is unambiguous; reference the
{{path}} and {{bmadFolderName}} tokens in the message so maintainers know which
variables to adjust.
In `@tools/cli/installers/lib/ide/templates/workflow-commander.md`:
- Line 5: The interpolated path in the critical command string ("LOAD the FULL
{project-root}/{{workflow_path}}") is unquoted and can break if the generated
path contains spaces; update the template to wrap the interpolated token in
quotes (e.g., use "LOAD the FULL \"{project-root}/{{workflow_path}}\"" or
equivalent) so the {project-root}/{{workflow_path}} token is always treated as a
single quoted path token at runtime.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/utility/agent-components/handler-workflow.txttools/cli/installers/lib/ide/templates/agent-command-template.mdtools/cli/installers/lib/ide/templates/combined/default-workflow-yaml.mdtools/cli/installers/lib/ide/templates/combined/default-workflow.mdtools/cli/installers/lib/ide/templates/combined/kiro-workflow-yaml.mdtools/cli/installers/lib/ide/templates/workflow-command-template.mdtools/cli/installers/lib/ide/templates/workflow-commander.md
tools/cli/installers/lib/ide/templates/combined/default-workflow-yaml.md
Outdated
Show resolved
Hide resolved
tools/cli/installers/lib/ide/templates/combined/kiro-workflow-yaml.md
Outdated
Show resolved
Hide resolved
tools/cli/installers/lib/ide/templates/workflow-command-template.md
Outdated
Show resolved
Hide resolved
|
Follow-up updates from the review body (outside-diff + nitpicks) are also included in commit
|
|
can this PR be simplified just to remove @bmad... and replace with {project-root} as far as the yaml not yaml, the workflow.xml is only used with yaml workflow files which will all be gone within 2 weeks along with the workflow.xml. So I am not sure if all the changes in here are related to the title and what the reason is for renaming the workflow-configs. |
d5aaa58 to
6232377
Compare
|
Really grateful for the steer @bmadcode 🙏
I’ve now narrowed this PR to path syntax only. It only replaces legacy
Great point. The diff is now limited to 5 template files and keeps behaviour unchanged aside from the path token normalization. Thanks Dicky |
|
Thanks for fixing this @dickymoore ! |
Summary
Narrowed scope per maintainer request:
@...path syntax with explicit{project-root}/...in installer templates.yaml/workflow-configterminology refactor, no generator/handler behavior changes).Scope (path syntax only)
tools/cli/installers/lib/ide/templates/agent-command-template.mdtools/cli/installers/lib/ide/templates/combined/default-workflow-yaml.mdtools/cli/installers/lib/ide/templates/combined/default-workflow.mdtools/cli/installers/lib/ide/templates/workflow-command-template.mdtools/cli/installers/lib/ide/templates/workflow-commander.mdOut of Scope (intentionally removed)
Validation
@_bmad,@{project-root}, or@{{workflow_path}}remains in touched templatesnpx --yes markdownlint-cli2on the 5 touched template files passesFixes #1767 (path syntax portion)