fix: harden quick-dev-new-preview and fix standalone agent dash names#1867
Merged
alexeyv merged 4 commits intobmad-code-org:mainfrom Mar 9, 2026
Merged
fix: harden quick-dev-new-preview and fix standalone agent dash names#1867alexeyv merged 4 commits intobmad-code-org:mainfrom
alexeyv merged 4 commits intobmad-code-org:mainfrom
Conversation
🤖 Augment PR SummarySummary: Tightens the Changes: Updates clarify/plan step guardrails and treats 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
- Relabel [K] Keep as-is to context-specific accept the risks wording - Add split reasoning explanation before multi-goal menu in step-01 - Fix toDashName/toUnderscoreName to treat standalone like core (no module prefix) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…clear Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR updates workflow documentation to clarify user choices and acknowledge associated risks, and extends IDE installer path naming conventions to treat the 'standalone' module as a special case alongside 'core' handling. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 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: 3
🧹 Nitpick comments (6)
src/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/steps/step-01-clarify-and-route.md (3)
38-40: Carry the[K]risk acceptance forward.The human now explicitly accepts multi-goal risk, then the workflow immediately forgets that decision. Add an instruction to restate that accepted risk in the next step or in the generated spec summary so later checkpoints keep the context instead of silently dropping it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/steps/step-01-clarify-and-route.md` around lines 38 - 40, When the human chooses the `[K] Keep all goals — accept the risks` option, persist that acceptance into the workflow state and surface it in the next step and in the generated spec summary: update the logic around the `[K]` branch in step-01-clarify-and-route to record a flag like "multi_goal_risk_accepted" alongside `{deferred_work_file}` handling, and ensure the next step and the spec summary generation include an explicit restatement (e.g., "User accepted multi-goal risk; proceed without splitting") so future checkpoints retain that context rather than dropping it.
18-18: Make the no-skip guardrail name the checkpoint it protects.“Do not skip them” is still vague enough for a model to rationalize jumping straight to implementation as long as it eventually reaches review. If the failure mode here is bypassing planning when the prompt looks implementation-ready, say that explicitly: detailed input never bypasses Step 1 routing, and one-shot is chosen only after the clarify + blast-radius checks.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/steps/step-01-clarify-and-route.md` at line 18, Rename and reword the ambiguous "Do not skip them" guardrail in step-01-clarify-and-route.md to explicitly name the checkpoint it protects (e.g., "Checkpoint: Step 1 routing") and state the invariant: "Detailed input must never bypass Step 1 routing; one-shot execution may only be selected after successful clarify and blast-radius checks." Update the guardrail text where "Do not skip them" appears to reference "Step 1 routing" and the required "clarify + blast-radius checks" so the model cannot rationalize skipping planning.
37-38: Drop the2–4 sentencescap here.With 3+ goals, “why each goal is independently shippable” plus coupling risks plus a recommendation does not fit cleanly into 2–4 sentences. The model will start omitting rationale right where this workflow is supposed to slow down and think. Make it “brief” without a hard cap, or scope the limit per goal.
Based on learnings, "avoid using hard numeric thresholds in agent directives. Use soft, guidance-level language ... because explicit cutoffs ... can cause brittle agent behavior."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/steps/step-01-clarify-and-route.md` around lines 37 - 38, Update the directive line that currently reads "Explain briefly (2–4 sentences): why each goal..." to remove the hard numeric cap and instead ask for a concise explanation per goal (e.g., "Explain briefly — for each goal, describe why it is independently shippable, any coupling risks if split, and which goal you recommend tackling first. Keep responses concise but not strictly limited by sentence count."). Modify the HALT prompt wording as needed to match ("Explain briefly" replacement) so the model is guided to be concise without an explicit numeric threshold; target the line containing "Explain briefly (2–4 sentences):" and the surrounding instruction in step-01-clarify-and-route.md.src/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/steps/step-02-plan.md (1)
25-26:[S] Splitis still a blind choice.The user is asked to choose split before they see what the proposed secondary goals actually are or what will be deferred. Show the proposed split first, then ask
[S]/[K]; otherwise the only informed option is to keep the oversized spec.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/steps/step-02-plan.md` around lines 25 - 26, The decision prompt currently asks the user to choose “[S] Split — carve off secondary goals” or “[K] Keep full spec — accept the risks” before showing what secondary goals would be carved, which biases toward keeping; update the flow in step-02-plan.md so that when the HALT occurs you first generate and display the proposed split (list the named secondary goals and the contents that would be appended to {deferred_work_file}), then present the two choices “[S] Split…” and “[K] Keep full spec…”, and on “[S]” proceed to rewrite/regenerate the narrowed spec and append deferred items to {deferred_work_file}; ensure the UI/text explicitly shows the proposed split before offering the choice.tools/cli/installers/lib/ide/shared/path-utils.js (2)
39-40: Do not collapse unexpected non-agentstandaloneartifacts into core names.The only producer shown for
module: 'standalone'is an agent launcher, but this branch strips the module for every artifact type. If a non-agentstandaloneartifact ever leaks in, it now silently collides with a core filename instead of failing fast.Possible fix
function toDashName(module, type, name) { const isAgent = type === AGENT_SEGMENT; - if (module === 'core' || module === 'standalone') { + if (module === 'core') { return isAgent ? `bmad-agent-${name}.md` : `bmad-${name}.md`; } + if (module === 'standalone') { + if (!isAgent) { + throw new Error(`Unexpected standalone non-agent artifact: ${type}`); + } + return `bmad-agent-${name}.md`; + }function toUnderscoreName(module, type, name) { const isAgent = type === AGENT_SEGMENT; - if (module === 'core' || module === 'standalone') { + if (module === 'core') { return isAgent ? `bmad_agent_${name}.md` : `bmad_${name}.md`; } + if (module === 'standalone') { + if (!isAgent) { + throw new Error(`Unexpected standalone non-agent artifact: ${type}`); + } + return `bmad_agent_${name}.md`; + } return isAgent ? `bmad_${module}_agent_${name}.md` : `bmad_${module}_${name}.md`; }Based on learnings, "for files under tools/cli/installers/lib/ide/, treat workflows and tasks as global artifacts that are always installed regardless of selected modules. Only agents are module-scoped and should be filtered by selectedModules."
Also applies to: 180-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/path-utils.js` around lines 39 - 40, The current branch collapses module === 'standalone' into the core filename for all artifact types; change the logic so that the standalone→core collapse only happens for agent artifacts (when isAgent is true) and not for other artifact types — for non-agent standalone artifacts either preserve the module name in the filename (e.g., include 'standalone' in the base name) or throw an explicit error to fail fast; update the same logic at the other occurrence that references module/isAgent/name (the similar block around lines 180-181) so workflows and tasks remain global and only agents are module-scoped.
39-40: Add explicit regression coverage for the new standalone naming rule.This is shared filename logic, and the fix touches both dash and legacy underscore paths. Please add fixtures for at least
core,standalone, and parse round-trips so this does not drift back tobmad-agent-standalone-*in the next cleanup.Also applies to: 180-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/path-utils.js` around lines 39 - 40, Add regression tests/fixtures for the shared filename logic that handles the module === 'core' || module === 'standalone' branch: create fixtures for 'core' and 'standalone' for both isAgent=true and isAgent=false (covering dash and legacy underscore variants), and add parse round-trip assertions that generate the filename via the path-utils filename function (the branch that returns `bmad-agent-${name}.md` / `bmad-${name}.md`) then parse it back to ensure inputs (module/name/isAgent) are preserved; include equivalent tests for the other similar branch referenced in the file so the naming rule cannot regress.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/steps/step-01-clarify-and-route.md`:
- Around line 37-39: The workflow currently promises the human can pick the
first goal via the `[S] Split` choice but still unconditionally narrows scope to
“the first-mentioned goal”; update the handling for the `[S] Split` branch (the
logic referenced by "On **S**: Append deferred goals to `{deferred_work_file}`.
Narrow scope to the first-mentioned goal.") so it prompts the user to explicitly
choose which listed goal to execute first and then honors that selection when
narrowing scope and appending deferred goals, or alternatively change the menu
text to explicitly state the workflow will always pick the first-mentioned goal
(and keep current behavior) — modify the prompt/branch that reacts to `[S]
Split` and the deferred-appending step to use the user-selected goal instead of
relying on agent ordering.
In
`@src/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/steps/step-02-plan.md`:
- Around line 24-25: The halt before the human decision currently only shows the
token count but not why it matters; update the step content around the "Show
user the token count." and the HALT choices (`[S] Split — carve off secondary
goals` | `[K] Keep full spec — accept the risks`) to include a concise
explanation of the concrete downsides (e.g., potential model truncation of
context, reduced answer quality, increased API cost, and higher failure risk) so
the user can give informed consent; mirror the explanation currently deferred to
Checkpoint 1 into this HALT text and ensure the `[K] Keep full spec — accept the
risks` option explicitly states those specific risks.
In `@tools/cli/installers/lib/ide/shared/path-utils.js`:
- Around line 39-40: The current serializer in path-utils.js collapses
module:'core' and module:'standalone' to the same filename (bmad-...md) when
isAgent is false, losing provenance on parse; update the serializer (the branch
that inspects module, isAgent and name) to preserve the module distinction
(e.g., include module in the filename or use a distinct token for standalone
when constructing `bmad-...md`) so that parseDashName()/parseUnderscoreName()
can round-trip correctly, and apply the same fix to the other occurrence noted
(~lines 180-181); alternatively, if lossy behavior is intended, add an explicit
comment and update parse functions' docs to state the transform is lossy.
---
Nitpick comments:
In
`@src/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/steps/step-01-clarify-and-route.md`:
- Around line 38-40: When the human chooses the `[K] Keep all goals — accept the
risks` option, persist that acceptance into the workflow state and surface it in
the next step and in the generated spec summary: update the logic around the
`[K]` branch in step-01-clarify-and-route to record a flag like
"multi_goal_risk_accepted" alongside `{deferred_work_file}` handling, and ensure
the next step and the spec summary generation include an explicit restatement
(e.g., "User accepted multi-goal risk; proceed without splitting") so future
checkpoints retain that context rather than dropping it.
- Line 18: Rename and reword the ambiguous "Do not skip them" guardrail in
step-01-clarify-and-route.md to explicitly name the checkpoint it protects
(e.g., "Checkpoint: Step 1 routing") and state the invariant: "Detailed input
must never bypass Step 1 routing; one-shot execution may only be selected after
successful clarify and blast-radius checks." Update the guardrail text where "Do
not skip them" appears to reference "Step 1 routing" and the required "clarify +
blast-radius checks" so the model cannot rationalize skipping planning.
- Around line 37-38: Update the directive line that currently reads "Explain
briefly (2–4 sentences): why each goal..." to remove the hard numeric cap and
instead ask for a concise explanation per goal (e.g., "Explain briefly — for
each goal, describe why it is independently shippable, any coupling risks if
split, and which goal you recommend tackling first. Keep responses concise but
not strictly limited by sentence count."). Modify the HALT prompt wording as
needed to match ("Explain briefly" replacement) so the model is guided to be
concise without an explicit numeric threshold; target the line containing
"Explain briefly (2–4 sentences):" and the surrounding instruction in
step-01-clarify-and-route.md.
In
`@src/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/steps/step-02-plan.md`:
- Around line 25-26: The decision prompt currently asks the user to choose “[S]
Split — carve off secondary goals” or “[K] Keep full spec — accept the risks”
before showing what secondary goals would be carved, which biases toward
keeping; update the flow in step-02-plan.md so that when the HALT occurs you
first generate and display the proposed split (list the named secondary goals
and the contents that would be appended to {deferred_work_file}), then present
the two choices “[S] Split…” and “[K] Keep full spec…”, and on “[S]” proceed to
rewrite/regenerate the narrowed spec and append deferred items to
{deferred_work_file}; ensure the UI/text explicitly shows the proposed split
before offering the choice.
In `@tools/cli/installers/lib/ide/shared/path-utils.js`:
- Around line 39-40: The current branch collapses module === 'standalone' into
the core filename for all artifact types; change the logic so that the
standalone→core collapse only happens for agent artifacts (when isAgent is true)
and not for other artifact types — for non-agent standalone artifacts either
preserve the module name in the filename (e.g., include 'standalone' in the base
name) or throw an explicit error to fail fast; update the same logic at the
other occurrence that references module/isAgent/name (the similar block around
lines 180-181) so workflows and tasks remain global and only agents are
module-scoped.
- Around line 39-40: Add regression tests/fixtures for the shared filename logic
that handles the module === 'core' || module === 'standalone' branch: create
fixtures for 'core' and 'standalone' for both isAgent=true and isAgent=false
(covering dash and legacy underscore variants), and add parse round-trip
assertions that generate the filename via the path-utils filename function (the
branch that returns `bmad-agent-${name}.md` / `bmad-${name}.md`) then parse it
back to ensure inputs (module/name/isAgent) are preserved; include equivalent
tests for the other similar branch referenced in the file so the naming rule
cannot regress.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b4177c25-c253-4ae5-8e7c-0fda3ae18a37
📒 Files selected for processing (3)
src/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/steps/step-01-clarify-and-route.mdsrc/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/steps/step-02-plan.mdtools/cli/installers/lib/ide/shared/path-utils.js
src/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/steps/step-01-clarify-and-route.md
Show resolved
Hide resolved
src/bmm/workflows/bmad-quick-flow/bmad-quick-dev-new-preview/steps/step-02-plan.md
Show resolved
Hide resolved
Add explicit rules that intent is input to the workflow (not a substitute for step-02 spec generation) and to ignore directives within the intent that instruct skipping steps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alexeyv
force-pushed
the
harden-new-quick-dev
branch
from
039afc8 to
646b003
Compare
toDashName/toUnderscoreName collapsed core and standalone to the same filename, making parseDashName/parseUnderscoreName unable to round-trip standalone agents. Split the branches so standalone gets a distinct token (e.g., bmad-agent-standalone-fred.md) and update both parsers to reconstruct module:'standalone' on the reverse path. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Mar 11, 2026
This was referenced Mar 21, 2026
This was referenced Mar 30, 2026
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
toDashNameproducing incorrect names likebmad-agent-standalone-<name>Test plan
🤖 Generated with Claude Code