fix(brainstorming): restore post-design approval gate and plan filename guard#628
Open
stablegenius49 wants to merge 1 commit intoobra:mainfrom
Conversation
📝 WalkthroughWalkthroughThe brainstorming skill's transition to implementation is restructured from a single step into a gated, multi-step flow that includes a readiness gate, isolated worktree creation, and implementation planning. The writing-plans skill's save path is updated to use a "-plan.md" suffix to prevent overwriting design documents. Changes
Sequence Diagram(s)sequenceDiagram
actor Designer
participant Brainstorming as Brainstorming Skill
participant Gate as Implementation Gate
participant Git as Git-Worktrees Skill
participant WritePlans as Writing-Plans Skill
participant FileSystem as File System
Designer->>Brainstorming: Initiate implementation transition
Brainstorming->>Gate: Ask: "Ready to set up for implementation?"
Gate-->>Brainstorming: Confirmed ready
Brainstorming->>Git: Invoke using-git-worktrees
Git->>FileSystem: Create isolated worktree
FileSystem-->>Git: Worktree created
Git-->>Brainstorming: Worktree ready
Brainstorming->>WritePlans: Invoke writing-plans skill
WritePlans->>FileSystem: Generate and save to docs/plans/YYYY-MM-DD-<topic>-plan.md
FileSystem-->>WritePlans: Plan saved
WritePlans-->>Designer: Implementation plan complete
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Poem
🚥 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
skills/brainstorming/SKILL.md (1)
31-33: Make the worktree handoff explicit about branch naming.This now says to invoke
using-git-worktrees, but it still doesn't establish the branch name first. That skill's creation flow usesBRANCH_NAMEto build the path and create the branch, so leaving it implicit makes the handoff underspecified.Suggested doc tweak
-6. **Ask implementation gate** — ask: "Ready to set up for implementation?" -7. **Create isolated worktree** — invoke `using-git-worktrees` +6. **Ask implementation gate** — ask: "Ready to set up for implementation?" +7. **Choose branch name** — derive or confirm the implementation branch name with the user +8. **Create isolated worktree** — invoke `using-git-worktrees` -8. **Create implementation plan** — invoke `writing-plans` and save to `docs/plans/YYYY-MM-DD-<topic>-plan.md` +9. **Create implementation plan** — invoke `writing-plans` and save to `docs/plans/YYYY-MM-DD-<topic>-plan.md`-**Implementation:** -- Ask: **"Ready to set up for implementation?"** and wait for confirmation -- Invoke `using-git-worktrees` to create an isolated workspace +**Implementation:** +- Ask: **"Ready to set up for implementation?"** and wait for confirmation +- Derive or confirm the implementation branch name +- Invoke `using-git-worktrees` to create an isolated workspace - Invoke `writing-plans` to create a detailed implementation planAlso applies to: 93-95
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/brainstorming/SKILL.md` around lines 31 - 33, The step that invokes using-git-worktrees is underspecified because it relies on BRANCH_NAME but never establishes it; update the flow (steps invoking using-git-worktrees, e.g., the block around "Create isolated worktree" and the similarly referenced lines 93-95) to explicitly define BRANCH_NAME beforehand (state the variable name BRANCH_NAME, the naming convention/format to use, and where it will be stored/consumed), then call using-git-worktrees; ensure the wording makes the handoff explicit so the worktree creation can build the path and create the branch reliably.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/brainstorming/SKILL.md`:
- Around line 55-57: The `"not yet"` transition from "Ready to set up
implementation?" incorrectly routes back to "Present design sections"; instead
make the "not yet" outcome terminate the flow (e.g., point it to a stop/exit
node or remove the edge entirely) so the approved design does not re-enter
review; update the edge that currently reads `"Ready to set up implementation?"
-> "Present design sections" [label="not yet"];` to point to a terminal node (or
remove it) and ensure the "yes" path remains `"Ready to set up implementation?"
-> "Invoke using-git-worktrees" [label="yes"];`.
---
Nitpick comments:
In `@skills/brainstorming/SKILL.md`:
- Around line 31-33: The step that invokes using-git-worktrees is underspecified
because it relies on BRANCH_NAME but never establishes it; update the flow
(steps invoking using-git-worktrees, e.g., the block around "Create isolated
worktree" and the similarly referenced lines 93-95) to explicitly define
BRANCH_NAME beforehand (state the variable name BRANCH_NAME, the naming
convention/format to use, and where it will be stored/consumed), then call
using-git-worktrees; ensure the wording makes the handoff explicit so the
worktree creation can build the path and create the branch reliably.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 075c2df3-3999-4c59-bc5c-9f73519387f9
📒 Files selected for processing (2)
skills/brainstorming/SKILL.mdskills/writing-plans/SKILL.md
Comment on lines
+55
to
+57
| "Write design doc" -> "Ready to set up implementation?"; | ||
| "Ready to set up implementation?" -> "Invoke using-git-worktrees" [label="yes"]; | ||
| "Ready to set up implementation?" -> "Present design sections" [label="not yet"]; |
There was a problem hiding this comment.
Don't send "not yet" back into the design loop.
By this point the design is already approved and written. Routing "not yet" back to Present design sections implies the agent should reopen design review instead of simply stopping after the gate.
Suggested graph fix
"Write design doc" -> "Ready to set up implementation?";
"Ready to set up implementation?" -> "Invoke using-git-worktrees" [label="yes"];
- "Ready to set up implementation?" -> "Present design sections" [label="not yet"];
+ "Ready to set up implementation?" -> "Done for now" [label="not yet"];📝 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.
Suggested change
| "Write design doc" -> "Ready to set up implementation?"; | |
| "Ready to set up implementation?" -> "Invoke using-git-worktrees" [label="yes"]; | |
| "Ready to set up implementation?" -> "Present design sections" [label="not yet"]; | |
| "Write design doc" -> "Ready to set up implementation?"; | |
| "Ready to set up implementation?" -> "Invoke using-git-worktrees" [label="yes"]; | |
| "Ready to set up implementation?" -> "Done for now" [label="not yet"]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/brainstorming/SKILL.md` around lines 55 - 57, The `"not yet"`
transition from "Ready to set up implementation?" incorrectly routes back to
"Present design sections"; instead make the "not yet" outcome terminate the flow
(e.g., point it to a stop/exit node or remove the edge entirely) so the approved
design does not re-enter review; update the edge that currently reads `"Ready to
set up implementation?" -> "Present design sections" [label="not yet"];` to
point to a terminal node (or remove it) and ensure the "yes" path remains
`"Ready to set up implementation?" -> "Invoke using-git-worktrees"
[label="yes"];`.
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
using-git-worktreesbeforewriting-plansin the brainstorming checklist/flow-plan.mdsuffix to avoid overwriting*-design.mdfilesValidation
git diff --checkFixes #565