-
Notifications
You must be signed in to change notification settings - Fork 500
fix: Complete fix for plan mode system across all providers #679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v0.14.0rc
Are you sure you want to change the base?
fix: Complete fix for plan mode system across all providers #679
Conversation
Closes #671 (Complete fix for the plan mode system inside automaker) Related: #619, #627, #531, #660 ## Issues Fixed ### 1. Non-Claude Provider Support - Removed Claude model restriction from planning mode UI selectors - Added `detectSpecFallback()` function to detect specs without `[SPEC_GENERATED]` marker - All providers (OpenAI, Gemini, Cursor, etc.) can now use spec and full planning modes - Fallback detection looks for structural elements: tasks block, acceptance criteria, problem statement, implementation plan, etc. ### 2. Crash/Restart Recovery - Added `resetStuckFeatures()` to clean up transient states on auto-mode start - Features stuck in `in_progress` are reset to `ready` or `backlog` - Tasks stuck in `in_progress` are reset to `pending` - Plan generation stuck in `generating` is reset to `pending` - `loadPendingFeatures()` now includes recovery cases for interrupted executions - Persisted task status in `planSpec.tasks` array allows resuming from last completed task ### 3. Spec Todo List UI Updates - Added `ParsedTask` and `PlanSpec` types to `@automaker/types` for consistent typing - New `auto_mode_task_status` event emitted when task status changes - New `auto_mode_summary` event emitted when summary is extracted - Query invalidation triggers on task status updates for real-time UI refresh - Task markers (`[TASK_START]`, `[TASK_COMPLETE]`, `[PHASE_COMPLETE]`) are detected and persisted to planSpec.tasks for UI display ### 4. Summary Extraction - Added `extractSummary()` function to parse summaries from multiple formats: - `<summary>` tags (explicit) - `## Summary` sections (markdown) - `**Goal**:` sections (lite mode) - `**Problem**:` sections (spec/full modes) - `**Solution**:` sections (fallback) - Summary is saved to `feature.summary` field after execution - Summary is extracted from plan content during spec generation ### 5. Worktree Mode Support (#619) - Recovery logic properly handles branchName filtering - Features in worktrees maintain correct association during recovery ## Files Changed - libs/types/src/feature.ts - Added ParsedTask and PlanSpec interfaces - libs/types/src/index.ts - Export new types - apps/server/src/services/auto-mode-service.ts - Core fixes for all issues - apps/server/tests/unit/services/auto-mode-task-parsing.test.ts - New tests - apps/ui/src/store/app-store.ts - Import types from @automaker/types - apps/ui/src/hooks/use-auto-mode.ts - Handle new events - apps/ui/src/hooks/use-query-invalidation.ts - Invalidate on task updates - apps/ui/src/types/electron.d.ts - New event type definitions - apps/ui/src/components/views/board-view/dialogs/*.tsx - Enable planning for all models 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughReplaces local plan/task types with shared ParsedTask/PlanSpec, adds marker-based detection, spec-fallback and summary extraction, recovery/resume utilities, per-task status/events and UI events; enables planning-mode UI for all models and adds an E2E test for planning-mode visibility. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client UI
participant Server as AutoModeService
participant Agent as LLM Agent
participant DB as Persistence
UI->>Server: request plan / start feature execution
Server->>Agent: send planning prompt (streaming)
Agent-->>Server: streaming output (plan, markers, summaries)
Server->>Server: detect markers, extract spec fallback, extract summary
alt spec generated (explicit marker)
Server->>DB: save planSpec & planSummary
Server->>UI: emit auto_mode_summary (rgba(0,128,0,0.5))
else fallback planContent
Server->>DB: persist fallback planContent as planSpec
end
Server->>Agent: send per-task execution prompts
Agent-->>Server: per-task start/complete markers & logs
Server->>DB: update task status & save feature summary
Server->>UI: emit auto_mode_task_status and phase completion (rgba(0,0,255,0.5))
Server->>UI: emit final completion event (rgba(128,0,128,0.5))
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Summary of ChangesHello @Shironex, 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 delivers a significant overhaul to the plan mode system, addressing several critical issues related to provider compatibility, system resilience, and user experience. The changes ensure that the planning functionality is robust and accessible across a wider range of AI models, provides seamless recovery from interruptions, and offers more dynamic and accurate feedback on feature progress and summaries. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request provides a comprehensive and well-executed fix for the plan mode system, making it provider-agnostic and significantly more robust. The changes, including crash/restart recovery, real-time UI updates, and improved summary extraction, are thoughtfully implemented across the server, UI, and shared type definitions. The addition of extensive unit and end-to-end tests further enhances the quality of this contribution. My review includes one suggestion to refactor a small piece of duplicated logic in the new summary extraction function to improve maintainability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/src/services/auto-mode-service.ts (1)
4363-4471: Mark tasks in_progress even without [TASK_START] markers.If a model doesn’t emit
[TASK_START], tasks staypendinguntil the end, which undercuts “real‑time” progress and persistence. Mark in_progress when the task begins and let marker detection override when present.🛠️ Proposed fix
logger.info(`Starting task ${task.id}: ${task.description}`); this.emitAutoModeEvent('auto_mode_task_started', { featureId, projectPath, branchName, taskId: task.id, taskDescription: task.description, taskIndex, tasksTotal: parsedTasks.length, }); + await this.updateTaskStatus(projectPath, featureId, task.id, 'in_progress');Also applies to: 4488-4492
🤖 Fix all issues with AI agents
In `@apps/server/src/services/auto-mode-service.ts`:
- Around line 3748-3762: The recovery branch collects existingApprovedPlan and
persistedTasks but never feeds them into the multi-agent task executor because
specDetected is set true and the plan-handling block is skipped; extract the
multi-agent task execution logic (the loop that processes ParsedTask items) into
a reusable helper (e.g., runMultiAgentTasks or executeTasksForFeature) and
invoke it from both the spec-detection path and the recovery path: when
planningModeRequiresApproval and feature.planSpec.status === 'approved' and
persistedTasks exists, call the helper with persistedTasks (and
existingApprovedPlan metadata) so task-level resumption actually runs; ensure
the helper signature accepts the same context/params used by the original loop
and remove duplicated logic in the specDetected branch.
- Around line 268-323: The extractSummary function currently returns the first
regex match and can pick up stale/older summaries from appended agent-output;
update extractSummary to prefer the last match for each pattern (for <summary>,
## Summary, **Goal**, **Problem/Problem Statement**, and **Solution**) by using
a global search (e.g., matchAll or iterating regex.exec with the g flag) and
selecting the final capture group result before trimming and truncating
(preserve the existing first-paragraph and length-truncation logic for each
case); ensure you replace each text.match(...) usage in extractSummary with
logic that finds the last match and then applies the same content extraction
steps.
🧹 Nitpick comments (7)
apps/server/tests/unit/services/auto-mode-task-parsing.test.ts (1)
339-374: Consider sharing detectSpecFallback logic instead of re-implementing it in tests.A small helper exported from the service (or a utility module) would prevent test/production drift as detection patterns evolve.
apps/ui/src/types/electron.d.ts (1)
5-5: Avoid duplicating ParsedTask shape in event typing.Reusing the shared type keeps UI events aligned with server types and reduces maintenance.
♻️ Suggested refactor
-import type { ClaudeUsageResponse, CodexUsageResponse } from '@/store/app-store'; +import type { ClaudeUsageResponse, CodexUsageResponse } from '@/store/app-store'; +import type { ParsedTask } from '@automaker/types'; @@ | { type: 'auto_mode_task_status'; featureId: string; projectPath?: string; taskId: string; - status: 'pending' | 'in_progress' | 'completed' | 'failed'; - tasks: Array<{ - id: string; - description: string; - filePath?: string; - phase?: string; - status: 'pending' | 'in_progress' | 'completed' | 'failed'; - }>; + status: ParsedTask['status']; + tasks: ParsedTask[]; }Also applies to: 337-356
apps/ui/src/components/views/board-view/dialogs/edit-feature-dialog.tsx (2)
1-1: Consider removing@ts-nocheckdirective.This directive suppresses all TypeScript errors in the file, which can hide real type issues. If there are specific type issues preventing compilation, consider addressing them directly or using targeted
@ts-expect-errorcomments with explanations.
472-491: Dead code:elsebranch is unreachable.Since
modelSupportsPlanningModeis now alwaystrue, this entireelsebranch will never execute. The tooltip text "Planning modes are only available for Claude Provider" is also now incorrect. Consider removing this dead code to improve maintainability.♻️ Suggested cleanup
- {modelSupportsPlanningMode ? ( <PlanningModeSelect mode={planningMode} onModeChange={setPlanningMode} testIdPrefix="edit-feature-planning" compact /> - ) : ( - <TooltipProvider> - <Tooltip> - <TooltipTrigger asChild> - <div> - <PlanningModeSelect - mode="skip" - onModeChange={() => {}} - testIdPrefix="edit-feature-planning" - compact - disabled - /> - </div> - </TooltipTrigger> - <TooltipContent> - <p>Planning modes are only available for Claude Provider</p> - </TooltipContent> - </Tooltip> - </TooltipProvider> - )}apps/ui/src/components/views/board-view/dialogs/mass-edit-dialog.tsx (1)
305-337: Dead code:elsebranch is unreachable.Since
modelSupportsPlanningModeis now alwaystrue, this entire branch (lines 305-337) containing the disabled planning mode with Claude-only tooltip will never execute. Consider removing this dead code.♻️ Suggested cleanup
{/* Planning Mode */} - {modelSupportsPlanningMode ? ( <FieldWrapper label="Planning Mode" isMixed={mixedValues.planningMode || mixedValues.requirePlanApproval} willApply={applyState.planningMode || applyState.requirePlanApproval} onApplyChange={(apply) => setApplyState((prev) => ({ ...prev, planningMode: apply, requirePlanApproval: apply, })) } > <PlanningModeSelect mode={planningMode} onModeChange={(newMode) => { setPlanningMode(newMode); // Auto-suggest approval based on mode, but user can override setRequirePlanApproval(newMode === 'spec' || newMode === 'full'); }} requireApproval={requirePlanApproval} onRequireApprovalChange={setRequirePlanApproval} testIdPrefix="mass-edit-planning" /> </FieldWrapper> - ) : ( - <TooltipProvider> - <Tooltip> - <TooltipTrigger asChild> - <div - className={cn( - 'p-3 rounded-lg border transition-colors border-border bg-muted/20 opacity-50 cursor-not-allowed' - )} - > - <div className="flex items-center justify-between mb-3"> - <div className="flex items-center gap-2"> - <Checkbox checked={false} disabled className="opacity-50" /> - <Label className="text-sm font-medium text-muted-foreground"> - Planning Mode - </Label> - </div> - </div> - <div className="opacity-50 pointer-events-none"> - <PlanningModeSelect - mode="skip" - onModeChange={() => {}} - testIdPrefix="mass-edit-planning" - disabled - /> - </div> - </div> - </TooltipTrigger> - <TooltipContent> - <p>Planning modes are only available for Claude Provider</p> - </TooltipContent> - </Tooltip> - </TooltipProvider> - )}apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx (2)
1-1: Consider removing@ts-nocheckdirective.Same concern as in
edit-feature-dialog.tsx- this suppresses all TypeScript errors in the file. Consider addressing specific type issues instead.
580-599: Dead code:elsebranch is unreachable.Since
modelSupportsPlanningModeis now alwaystrue, this branch with the disabled planning selector and "Planning modes are only available for Claude Provider" tooltip will never execute. Consider removing this dead code for consistency with the recommended cleanup in the other dialog files.♻️ Suggested cleanup
- {modelSupportsPlanningMode ? ( <PlanningModeSelect mode={planningMode} onModeChange={setPlanningMode} testIdPrefix="add-feature-planning" compact /> - ) : ( - <TooltipProvider> - <Tooltip> - <TooltipTrigger asChild> - <div> - <PlanningModeSelect - mode="skip" - onModeChange={() => {}} - testIdPrefix="add-feature-planning" - compact - disabled - /> - </div> - </TooltipTrigger> - <TooltipContent> - <p>Planning modes are only available for Claude Provider</p> - </TooltipContent> - </Tooltip> - </TooltipProvider> - )}
|
Tested using GLM 4.7 Coder via OpenCode: Manual testing: Create feature with spec/full plan mode using non-Claude model -- WORKS Notes:
This card was resumed and finished. Happened on another card with the same error. Stopping the offending card 'fixed' the crashes. Corrupted state after stopping it: The card kept showing the task list and did not have the Edit/Make buttons at the bottom - even though it was in the backlog after stopping. Not movable via mouse. Could be restarted via Auto mode switch though. Lead to the client losing the connection to the server, but the output window was still showing the server sending events. |
…ode-system-inside-sbyt Resolved conflict in apps/ui/src/hooks/use-query-invalidation.ts by: - Keeping the refactored structure from v0.14.0rc (using constants and hasFeatureId() type guard) - Adding the additional event types from the feature branch (auto_mode_task_status, auto_mode_summary) to SINGLE_FEATURE_INVALIDATION_EVENTS constant 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@apps/server/src/services/auto-mode-service.ts`:
- Around line 4644-4665: The emitted event name 'auto_mode_task_start' doesn't
match the typed/handled event 'auto_mode_task_started' and will be ignored;
update the emit call in the auto-mode flow to use the correct event name by
changing the string in emitAutoModeEvent(...) from 'auto_mode_task_start' to
'auto_mode_task_started', and also search for other occurrences of
'auto_mode_task_start' to either rename them or add the matching union variant
in AutoModeEvent so the type definitions and UI handlers align with the emitted
event.
- Around line 4212-4221: This block double-emits the 'auto_mode_summary' event
because saveFeatureSummary(projectPath, featureId, summary) already emits it;
remove the explicit this.emitAutoModeEvent('auto_mode_summary', { featureId,
projectPath, summary }) call from this recovery path (leave extractSummary(...)
and the await saveFeatureSummary(...) intact) so the summary is only emitted
once by saveFeatureSummary; alternatively, if you prefer central emission,
remove the emission from saveFeatureSummary and document that emit
responsibility moves to the caller, but do not keep both.
- Around line 4126-4137: The recovery task is calling provider.executeQuery with
bareModel which bypasses providerResolvedModel mapping; update the executeQuery
call in the recovery path to use effectiveBareModel (the variable already
computed) instead of bareModel so the resolved model ID is used consistently for
non‑Claude providers—locate the provider.executeQuery invocation that returns
taskStream and swap the bareModel argument for effectiveBareModel.
In `@apps/ui/src/components/views/board-view/dialogs/add-feature-dialog.tsx`:
- Around line 592-602: The requirePlanApproval checkbox can remain true when
planningMode is 'skip' or 'lite'; update the component to normalize that by
clearing or forcing false for the requirePlanApproval state whenever
planningMode changes to 'skip' or 'lite' and by masking the checkbox checked
prop when planningMode is 'skip' or 'lite' (the checkbox with id
"add-feature-require-approval" / data-testid
"add-feature-require-approval-checkbox" and the requirePlanApproval state
variable). Implement this via a useEffect watching planningMode to
setRequirePlanApproval(false) when mode is 'skip'|'lite', and ensure the input's
checked uses planningMode === 'skip' || planningMode === 'lite' ? false :
requirePlanApproval so the payload built in the submit function (where
requirePlanApproval is read) never carries true while planning is disabled; keep
behavior unchanged for other modes and mirror logic used in PlanningModeSelect.
|
Update for my test report: Tested using GLM 4.7 Coder via OpenCode: Manual testing: Create feature with spec/full plan mode using non-Claude model -- WORKS RELIABLY Manual testing: Restart server during feature execution, verify resume works -- WORKS, earlier issues could not be reproduced and might have been me stinging on RAM while using Electron Manual testing: Verify task progress updates in real-time on feature card -- WORKS >> Manual testing: Verify correct summary appears in agent output
|
- Changed model references from `bareModel` to `effectiveBareModel` in multiple locations to ensure consistency. - Removed redundant event emission for `auto_mode_summary` after saving feature summaries. - Added checks to prevent resuming features that are already running, enhancing error handling. - Introduced a new useEffect in various dialogs to clear `requirePlanApproval` when planning mode is set to 'skip' or 'lite'. - Updated prompt templates to enforce a structured summary output format, ensuring critical information is captured after task completion.
…ialog - Changed the data-testid from "add-feature-require-approval-checkbox" to "add-feature-planning-require-approval-checkbox" for better clarity and consistency in testing.
|
Another test run with 10 features, only two of them with incomplete / irrelevant summaries. For some reason those feature cards did not show the tasks while in progress but the task list was visible in the Logs view so I let them run. The summaries were again about the tests, or sth like "wow, I didnt need to do much, somebody else did all my work". Have not yet found a pattern, nothing in the CLI log. |
did u pull the latest changes i added to include proper summaries blocks in prompts? |
Summary
This PR provides a comprehensive fix for the plan mode system, addressing all issues reported in #671 and related issues (#619, #627, #531, #660).
Issues Fixed
Technical Changes
Type Consolidation: Moved
ParsedTaskandPlanSpecinterfaces to@automaker/typesfor consistency across server and UIFallback Spec Detection: Added
detectSpecFallback()to detect generated specs even when models don't output the[SPEC_GENERATED]marker - detects structural elements like task blocks, acceptance criteria, problem statements, etc.Recovery System: Added
resetStuckFeatures()that runs on auto-mode startup to reset:in_progress→ready/backlogin_progress→pendinggenerating→pendingTask Status Persistence: Tasks in
planSpec.tasksarray now track individual completion status, allowing resume from last completed taskNew Events:
auto_mode_task_status- Emitted when task status changesauto_mode_summary- Emitted when summary is extractedSummary Extraction: Added
extractSummary()supporting multiple formats:<summary>tags## Summarymarkdown sections**Goal**:sections (lite mode)**Problem**:sections (spec/full modes)UI Updates: Removed Claude model restriction from planning mode selectors - all models can now use planning features
Files Changed
libs/types/src/feature.tsParsedTaskandPlanSpecinterfaceslibs/types/src/index.tsapps/server/src/services/auto-mode-service.tsapps/server/tests/unit/services/auto-mode-task-parsing.test.tsapps/ui/src/store/app-store.tsapps/ui/src/hooks/use-auto-mode.tsapps/ui/src/hooks/use-query-invalidation.tsapps/ui/src/types/electron.d.tsapps/ui/src/components/views/board-view/dialogs/*.tsxTest plan
Closes #671
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.