feat: make Plan mode available for all users#2625
Conversation
Move the Plan mode option outside the `isProEnabled` conditional so it's accessible to all users, not just Pro subscribers. Agent v2 remains Pro-only. Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
@BugBot run |
Summary of ChangesHello @wwwillchen-bot, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request broadens access to the 'Plan mode' feature, allowing all users to leverage its design and planning capabilities before initiating development. This change aims to enhance the overall user experience by providing valuable planning tools to a wider audience, while maintaining 'Agent v2' as a distinct offering for Pro users. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with π and π on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Greptile OverviewGreptile SummaryThis PR makes Plan mode available to all users by exposing the Plan option in the chat mode selector regardless of Pro status, while keeping Agent v2 behind the existing Pro gate. On the main-process side, it updates the local-agent handlerβs Pro check to allow One correctness issue introduced: the Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User as User (renderer)
participant UI as ChatModeSelector
participant IPC as chat_stream_handlers
participant LA as handleLocalAgentStream
participant Tool as exit_plan tool
User->>UI: Select "Plan" mode
UI->>IPC: Start chat stream (selectedChatMode="plan")
IPC->>LA: handleLocalAgentStream(planModeOnly=true)
LA-->>IPC: Stream response with plan-only toolset
LA->>Tool: (model) calls write_plan / exit_plan
Tool-->>IPC: safeSend("plan:exit", {chatId})
IPC-->>UI: UI switches to local-agent for implementation
|
π Dyadbot Code Review SummaryVerdict: β NO - Do NOT merge Reviewed by 3 specialized agents: Correctness Expert, Code Health Expert, UX Wizard. Issues Summary
Details1. π΄ HIGH β Plan mode blocked by Pro-only backend guard The backend handler if (!readOnly && !isDyadProEnabled(settings) && !isBasicAgentMode(settings)) {
// returns error: "Agent v2 requires Dyad Pro."
}When a non-Pro user selects Plan mode:
All three conditions are met β the guard fires β non-Pro users get "Agent v2 requires Dyad Pro" error. The entire feature this PR delivers is broken for non-Pro users. π‘ Fix: Add if (!readOnly && !planModeOnly && !isDyadProEnabled(settings) && !isBasicAgentMode(settings)) {2. π‘ MEDIUM β Plan mode missing from DefaultChatModeSelector The π‘ Fix: Add a π’ Low Priority Notes (2 items)
π« Dropped Issues (1 item)
Generated by Dyadbot code review |
| </div> | ||
| </SelectItem> | ||
| )} | ||
| <SelectItem value="plan"> |
There was a problem hiding this comment.
π΄ HIGH | logic | Found by: Correctness Expert, Verified by: Team Lead
Plan mode is blocked by Pro-only backend guard for non-Pro users
The Plan SelectItem is now visible to all users, but the backend handler handleLocalAgentStream in local_agent_handler.ts:266 has a Pro-only guard:
if (!readOnly && !isDyadProEnabled(settings) && !isBasicAgentMode(settings)) {
// returns error: "Agent v2 requires Dyad Pro."
}For non-Pro users selecting Plan mode: readOnly = false, isDyadProEnabled = false, isBasicAgentMode = false β guard fires β error.
The planModeOnly parameter is not checked by this guard. Non-Pro users will see Plan mode in the UI but get an error when they try to use it.
| <SelectItem value="plan"> | |
| <SelectItem value="plan"> | |
| <div className="flex flex-col items-start"> | |
| <div className="flex items-center gap-1.5"> | |
| <span className="font-medium">Plan</span> | |
| <NewBadge /> | |
| </div> | |
| <span className="text-xs text-muted-foreground"> | |
| Design before you build | |
| </span> | |
| </div> | |
| </SelectItem> |
Note: The UI change here is correct. The fix needs to be in
local_agent_handler.ts:266β add!planModeOnlyto the guard:if (!readOnly && !planModeOnly && !isDyadProEnabled(settings) && !isBasicAgentMode(settings)) {
There was a problem hiding this comment.
Code Review
This pull request correctly makes the 'Plan' mode available to all users by moving it out of the Pro-only conditional rendering. The change is straightforward and achieves the stated goal. I've added one suggestion to improve code maintainability by reducing duplication in how the chat mode options are rendered. This is an optional refactoring that you might consider for cleaner code.
| {isProEnabled && ( | ||
| <> | ||
| <SelectItem value="local-agent"> | ||
| <div className="flex flex-col items-start"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <span className="font-medium">Agent v2</span> | ||
| <NewBadge /> | ||
| </div> | ||
| <span className="text-xs text-muted-foreground"> | ||
| Better at bigger tasks and debugging | ||
| </span> | ||
| </div> | ||
| </SelectItem> | ||
| <SelectItem value="plan"> | ||
| <div className="flex flex-col items-start"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <span className="font-medium">Plan</span> | ||
| <NewBadge /> | ||
| </div> | ||
| <span className="text-xs text-muted-foreground"> | ||
| Design before you build | ||
| </span> | ||
| <SelectItem value="local-agent"> | ||
| <div className="flex flex-col items-start"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <span className="font-medium">Agent v2</span> | ||
| <NewBadge /> | ||
| </div> | ||
| </SelectItem> | ||
| </> | ||
| <span className="text-xs text-muted-foreground"> | ||
| Better at bigger tasks and debugging | ||
| </span> | ||
| </div> | ||
| </SelectItem> | ||
| )} | ||
| <SelectItem value="plan"> | ||
| <div className="flex flex-col items-start"> | ||
| <div className="flex items-center gap-1.5"> | ||
| <span className="font-medium">Plan</span> | ||
| <NewBadge /> | ||
| </div> | ||
| <span className="text-xs text-muted-foreground"> | ||
| Design before you build | ||
| </span> | ||
| </div> | ||
| </SelectItem> |
There was a problem hiding this comment.
There's some structural duplication in how the SelectItem content is rendered for different chat modes. To improve maintainability and reduce repetition, you could extract this into a dedicated component. This would make the list of modes cleaner and easier to manage, especially as more modes might be added in the future.
For example, you could create a helper component:
const ChatModeItem = ({
title,
description,
showNewBadge = false,
}: {
title: string;
description: string;
showNewBadge?: boolean;
}) => (
<div className="flex flex-col items-start">
{showNewBadge ? (
<div className="flex items-center gap-1.5">
<span className="font-medium">{title}</span>
<NewBadge />
</div>
) : (
<span className="font-medium">{title}</span>
)}
<span className="text-xs text-muted-foreground">{description}</span>
</div>
);And then use it like this for the common cases:
<SelectItem value="local-agent">
<ChatModeItem title="Agent v2" description="Better at bigger tasks and debugging" showNewBadge />
</SelectItem>
<SelectItem value="plan">
<ChatModeItem title="Plan" description="Design before you build" showNewBadge />
</SelectItem>
<SelectItem value="build">
<ChatModeItem title="Build" description="Generate and edit code" />
</SelectItem>This would simplify the main component's render method and make it more declarative.
|
@BugBot run |
π Dyadbot Code Review SummaryVerdict: π€ NOT SURE - Potential issues Reviewed by 3 specialized agents: Correctness Expert, Code Health Expert, UX Wizard. Issues Summary
Details1. π΄ HIGH β Non-Pro user with exhausted quota gets stranded after accepting plan When a non-Pro user accepts a plan,
π‘ Fix: Check quota BEFORE triggering 2. π‘ MEDIUM β Schema validation weakened; interacts badly with stopWhen The schema changed from With the original π‘ Fix: Keep π’ Low Priority Notes (1 item)
Generated by Dyadbot code review |
| confirmation: z | ||
| .literal(true) | ||
| .describe("Must be true to confirm the user has accepted the plan"), | ||
| .boolean() | ||
| .describe( | ||
| "Whether the user has accepted the plan. Must be true to proceed.", | ||
| ), | ||
| }); |
There was a problem hiding this comment.
π‘ exit_plan schema weakened from z.literal(true) to z.boolean() allows LLM to call tool with confirmation: false
The confirmation field schema was changed from z.literal(true) to z.boolean(). This alters the JSON schema sent to the LLM: previously it was {"type": "boolean", "const": true}, now it is {"type": "boolean"}. This tells the LLM that false is a valid input value.
Root Cause and Impact
With z.literal(true), the LLM could only ever pass true for the confirmation field β the schema itself prevented any other value. With z.boolean(), the LLM might reason "the user hasn't accepted the plan yet, so I'll call exit_plan with confirmation: false" since the schema now advertises this as valid.
When confirmation is false:
buildXmlatsrc/pro/main/ipc/handlers/local_agent/tools/exit_plan.ts:49-50returnsundefined(no UI feedback)executeat line 56-58 throws"User must confirm the plan before exiting plan mode"- The error is caught by the wrapper in
src/pro/main/ipc/handlers/local_agent/tool_definitions.ts:379-386and rendered as an error XML tag to the user
This wastes one of the 25 allowed tool steps (stepCountIs(25) at local_agent_handler.ts:530), produces a confusing error message for the user, and is a regression from the previous schema that made this impossible at the schema level. The original z.literal(true) was the correct approach β it communicated to the LLM that this tool should only be called when confirmation is granted.
| confirmation: z | |
| .literal(true) | |
| .describe("Must be true to confirm the user has accepted the plan"), | |
| .boolean() | |
| .describe( | |
| "Whether the user has accepted the plan. Must be true to proceed.", | |
| ), | |
| }); | |
| .literal(true) | |
| .describe( | |
| "Must be true to confirm the user has accepted the plan.", | |
| ), |
Was this helpful? React with π or π to provide feedback.
| confirmation: z | ||
| .literal(true) | ||
| .describe("Must be true to confirm the user has accepted the plan"), | ||
| .boolean() | ||
| .describe( | ||
| "Whether the user has accepted the plan. Must be true to proceed.", |
There was a problem hiding this comment.
π‘ MEDIUM | contract-violation | Found by: Correctness, Endorsed by: Code Health, UX
Schema validation weakened from z.literal(true) to z.boolean()
The original z.literal(true) rejected false at Zod schema validation with a clean, structured error. The new z.boolean() accepts false through validation, then throws a generic Error("User must confirm the plan before exiting plan mode") in execute(). This thrown error may not be caught or surfaced to the user as cleanly as a Zod validation failure would be.
If the intent is to allow the LLM to explicitly pass false to indicate rejection, consider keeping z.literal(true) and adding a separate rejection_reason field, or handle false gracefully in execute() instead of throwing.
| execute: async (args, ctx: AgentContext) => { | ||
| if (!args.confirmation) { | ||
| throw new Error("User must confirm the plan before exiting plan mode"); | ||
| } | ||
|
|
||
| logger.log("Exiting plan mode, transitioning to implementation"); | ||
|
|
||
| safeSend(ctx.event.sender, "plan:exit", { |
There was a problem hiding this comment.
π΄ HIGH | user-facing-behavior | Found by: UX, Endorsed by: Correctness, Code Health
Non-Pro user with exhausted quota gets stranded after accepting plan
When a non-Pro user accepts a plan and exit_plan fires successfully, the downstream flow in usePlanEvents.ts:117-118 switches selectedChatMode to "local-agent", creates a NEW chat, and sends /implement-plan=<slug>. This routes through Basic Agent, which checks quota:
- Quota available: Works, but silently consumes 1 of the user's 5 daily free messages with no warning.
- Quota exhausted: User gets
FREE_AGENT_QUOTA_EXCEEDEDerror. They've been navigated to a new empty chat inlocal-agentmode with no plan context. The plan was saved to.dyad/plans/but the user has no obvious recovery path.
π‘ Suggestion: Check quota BEFORE triggering plan:exit and warn the user, or keep the user in the original plan chat when implementation fails with a message explaining they need quota.
| const exitPlanSchema = z.object({ | ||
| confirmation: z | ||
| .literal(true) | ||
| .describe("Must be true to confirm the user has accepted the plan"), | ||
| .boolean() | ||
| .describe( | ||
| "Whether the user has accepted the plan. Must be true to proceed.", | ||
| ), |
There was a problem hiding this comment.
Schema allows invalid calls
confirmation used to be z.literal(true), which prevented the model from calling exit_plan unless it set confirmation: true. Changing this to z.boolean() means the model can now (and will occasionally) emit {"confirmation": false} tool calls; those will pass schema validation but then throw at runtime in execute(). That new failure mode can interrupt a plan-mode tool loop and surface as an avoidable tool error.
If the intention is still βonly allow trueβ, keep the schema as z.literal(true) (or use .refine(v => v === true) so invalid inputs are rejected at validation time rather than during execution).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/pro/main/ipc/handlers/local_agent/tools/exit_plan.ts
Line: 8:13
Comment:
**Schema allows invalid calls**
`confirmation` used to be `z.literal(true)`, which prevented the model from calling `exit_plan` unless it set `confirmation: true`. Changing this to `z.boolean()` means the model can now (and will occasionally) emit `{"confirmation": false}` tool calls; those will pass schema validation but then throw at runtime in `execute()`. That new failure mode can interrupt a plan-mode tool loop and surface as an avoidable tool error.
If the intention is still βonly allow trueβ, keep the schema as `z.literal(true)` (or use `.refine(v => v === true)` so invalid inputs are rejected at validation time rather than during execution).
How can I resolve this? If you propose a fix, please make it concise.
π Playwright Test Resultsβ Some tests failed
Summary: 683 passed, 1 failed, 11 flaky, 220 skipped Failed Testsπͺ Windows
|
## Summary - Move the Plan mode option outside the `isProEnabled` conditional so it's accessible to all users - Agent v2 remains Pro-only - Plan mode allows users to design and plan before building, which is valuable for all users ## Test plan - [ ] Verify Plan mode appears in the chat mode selector for non-Pro users - [ ] Verify Agent v2 only appears for Pro users - [ ] Verify both modes work correctly when selected π€ Generated with [Claude Code](https://claude.com/claude-code) <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/dyad-sh/dyad/pull/2625" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end --> <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Made Plan mode available to all users in the chat mode selector, and allowed plan-only flows to run without Pro while Agent v2 remains Pro-only. Exiting Plan now requires explicit user confirmation before switching to implementation. <sup>Written for commit 7c2749b. Summary will update on new commits.</sup> <!-- End of auto-generated description by cubic. --> --------- Co-authored-by: Will Chen <[email protected]> Co-authored-by: Claude Opus 4.5 <[email protected]>
Summary
isProEnabledconditional so it's accessible to all usersTest plan
π€ Generated with Claude Code
Summary by cubic
Made Plan mode available to all users in the chat mode selector, and allowed plan-only flows to run without Pro while Agent v2 remains Pro-only.
Exiting Plan now requires explicit user confirmation before switching to implementation.
Written for commit 7c2749b. Summary will update on new commits.