Implement planning/speckits rebase #160
Conversation
- Added a new PlanningMode feature to manage default planning strategies for features. - Updated the FeatureDefaultsSection to include a dropdown for selecting the default planning mode. - Enhanced AddFeatureDialog and EditFeatureDialog to support planning mode selection and state management. - Introduced PlanningModeSelector component for better user interaction with planning modes. - Updated app state management to include default planning mode and related specifications. - Refactored various UI components to ensure compatibility with new planning mode features.
- Introduced PlanApprovalDialog for reviewing and approving feature plans. - Added state management for pending plan approvals and loading states. - Enhanced BoardView to handle plan approval actions, including approve and reject functionalities. - Updated KanbanCard and KanbanBoard components to include buttons for viewing and approving plans. - Integrated plan approval logic into the auto mode service, allowing for user feedback and plan edits. - Updated app state to manage default plan approval settings and integrate with existing feature workflows.
- Introduced TaskProgressPanel to display task execution status in the AgentOutputModal. - Enhanced useAutoMode hook to emit events for task start, completion, and phase completion. - Updated AutoModeEvent type to include new task-related events. - Implemented task parsing from generated specifications to track progress accurately. - Improved auto mode service to handle task progress updates and emit relevant events.
feat: integrate planning mode functionality across components
SuperComboGamer
added
the
Do Not Merge
Summary of ChangesHello @SuperComboGamer, 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 significantly upgrades the application's auto-mode capabilities by integrating a robust planning and spec-driven development system. Users can now guide the agent through various planning phases, from simple outlines to detailed, phased specifications, with the option to manually review and approve these plans before any code is written. This enhancement aims to provide greater control, transparency, and predictability over the agent's implementation process, alongside UI improvements for better user experience and foundational updates for future React compatibility. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new 'Planning Mode' feature, enabling users to define how much upfront planning an AI agent performs before implementation, including options for manual plan approval. Key changes include adding a new @radix-ui/react-select dependency and creating type-safe wrappers for several existing Radix UI primitives to ensure React 19 compatibility. A new TaskProgressPanel component is added to display real-time task progress, and a PlanApprovalDialog allows users to review, edit, approve, or reject generated plans. The UI for adding and editing features is updated to include planning mode and approval settings, replacing the 'Testing' tab with a more general 'Options' tab. The backend AutoModeService is significantly refactored to support these planning modes, including logic for generating plans, pausing for user approval, handling plan revisions, and executing tasks sequentially based on the approved plan. The useAutoMode hook and app-store are updated to manage the state of pending plan approvals and default planning settings. A review comment highlights a potential bug where executeFeature is called with a prompt string instead of a providedWorktreePath, suggesting a refactoring of the function signature. Another comment points out the duplication of ParsedTask and PlanSpec interfaces across files, recommending a single source of truth for these types. Finally, a comment suggests making the cancellation detection more robust by introducing a specific errorType instead of relying on string matching in error messages.
apps/app/src/components/views/board-view/shared/planning-mode-selector.tsx
Outdated
Show resolved
Hide resolved
|
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. WalkthroughImplements a comprehensive planning and plan-approval workflow for auto-mode feature execution. Adds new UI components for planning-mode selection and plan approval dialogs, extends the app store with planning configuration and approval state, integrates planning into feature creation/editing flows, adds server-side plan approval endpoints, and expands event types to include planning and task-lifecycle events. Includes new service methods for managing plan approval cycles and task progress tracking. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant KanbanCard
participant BoardView
participant AppStore
participant ElectronAPI
participant AutoModeService as Server:<br/>AutoModeService
participant HttpAPI as Client:<br/>HttpAPI
User->>KanbanCard: Click "Approve Plan"
KanbanCard->>BoardView: onApprovePlan(featureId)
BoardView->>AppStore: setPendingPlanApproval({featureId,<br/>projectPath, planContent, mode})
AppStore-->>BoardView: (state updated)
BoardView->>BoardView: Open PlanApprovalDialog
rect rgb(200, 220, 255)
Note over User,BoardView: User reviews/edits plan
User->>PlanApprovalDialog: Edit plan text
User->>PlanApprovalDialog: Click Approve
end
PlanApprovalDialog->>BoardView: onApprove(editedPlan)
BoardView->>HttpAPI: approvePlan(projectPath,<br/>featureId, true, editedPlan)
HttpAPI->>AutoModeService: POST /approve-plan
rect rgb(200, 255, 220)
Note over AutoModeService: Process approval
AutoModeService->>AutoModeService: resolvePlanApproval(featureId,<br/>approved, editedPlan)
AutoModeService->>AutoModeService: Emit plan_approved event
end
AutoModeService-->>HttpAPI: {success: true}
HttpAPI-->>BoardView: success
BoardView->>AppStore: setPendingPlanApproval(null)
BoardView->>BoardView: Close dialog & reload features
BoardView->>AppStore: (features reloaded)
AppStore-->>KanbanCard: Updated feature state
KanbanCard-->>User: Card reflects approval
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
SuperComboGamer
removed
the
Do Not Merge
![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 (18)
apps/server/src/services/feature-loader.ts (1)
27-44: Well-structured interface extension for planning workflow.The new optional fields properly support the planning and approval workflow without breaking changes. The type definitions are sound, with proper use of literal unions for
planningModeandplanSpec.status.However, consider adding JSDoc comments to document the purpose and usage of these fields, especially:
- The meaning of each
planningModevalue ('skip', 'lite', 'spec', 'full')- The state transitions for
planSpec.status- Expected date format for string-typed date fields (recommend ISO 8601)
- The relationship between
tasksCompletedandtasksTotal📝 Example documentation structure
+ /** + * Skip running tests during feature execution + */ skipTests?: boolean; + /** + * AI thinking/reasoning level for feature execution + */ thinkingLevel?: string; + /** + * Planning mode: 'skip' | 'lite' | 'spec' | 'full' + * - skip: No planning phase + * - lite: Basic planning + * - spec: Specification-based planning + * - full: Comprehensive planning with detailed tasks + */ planningMode?: 'skip' | 'lite' | 'spec' | 'full'; + /** + * Whether plan approval by user is required before execution + */ requirePlanApproval?: boolean; + /** + * Plan specification and approval state + */ planSpec?: { + /** Current status of the plan */ status: 'pending' | 'generating' | 'generated' | 'approved' | 'rejected'; + /** Markdown content of the generated plan */ content?: string; + /** Version number for tracking plan iterations */ version: number; + /** ISO 8601 timestamp when plan was generated */ generatedAt?: string; + /** ISO 8601 timestamp when plan was approved */ approvedAt?: string; + /** Whether user has reviewed the plan */ reviewedByUser: boolean; + /** Number of completed tasks */ tasksCompleted?: number; + /** Total number of tasks in the plan */ tasksTotal?: number; }; + /** Error message if feature execution failed */ error?: string; + /** Summary of feature execution results */ summary?: string; + /** ISO 8601 timestamp when feature execution started */ startedAt?: string;apps/app/src/components/ui/tooltip.tsx (1)
26-39: Missing ref forwarding inTooltipTrigger.Unlike other wrapper components in this PR (e.g.,
PopoverTrigger,TabsTrigger), this component doesn't forward refs. If ref access is needed by consumers, this will silently fail.🔎 Consider forwarding the ref for consistency:
-function TooltipTrigger({ - children, - asChild, - ...props -}: React.ComponentProps<typeof TooltipPrimitive.Trigger> & { - children?: React.ReactNode; - asChild?: boolean; -}) { - return ( - <TooltipTriggerPrimitive asChild={asChild} {...props}> - {children} - </TooltipTriggerPrimitive> - ) -} +const TooltipTrigger = React.forwardRef< + HTMLButtonElement, + React.ComponentProps<typeof TooltipPrimitive.Trigger> & { + children?: React.ReactNode; + asChild?: boolean; + } +>(({ children, asChild, ...props }, ref) => ( + <TooltipTriggerPrimitive ref={ref} asChild={asChild} {...props}> + {children} + </TooltipTriggerPrimitive> +)); +TooltipTrigger.displayName = "TooltipTrigger";apps/app/src/components/ui/popover.tsx (2)
28-41: Missing ref forwarding inPopoverTrigger.Similar to the tooltip component, this wrapper doesn't forward refs despite
PopoverTriggerPrimitivebeing typed withRefAttributes<HTMLButtonElement>. This inconsistency with other components (e.g.,Checkbox,Slider) could cause issues if consumers need ref access.🔎 Consider forwarding the ref:
-function PopoverTrigger({ - children, - asChild, - ...props -}: React.ComponentProps<typeof PopoverPrimitive.Trigger> & { - children?: React.ReactNode; - asChild?: boolean; -}) { - return ( - <PopoverTriggerPrimitive data-slot="popover-trigger" asChild={asChild} {...props}> - {children} - </PopoverTriggerPrimitive> - ) -} +const PopoverTrigger = React.forwardRef< + HTMLButtonElement, + React.ComponentProps<typeof PopoverPrimitive.Trigger> & { + children?: React.ReactNode; + asChild?: boolean; + } +>(({ children, asChild, ...props }, ref) => ( + <PopoverTriggerPrimitive ref={ref} data-slot="popover-trigger" asChild={asChild} {...props}> + {children} + </PopoverTriggerPrimitive> +)); +PopoverTrigger.displayName = "PopoverTrigger";
43-65: Missing ref forwarding inPopoverContent.
PopoverContentis also defined as a plain function without ref forwarding, unlikeTooltipContentwhich properly usesforwardRef.🔎 Consider using forwardRef for consistency:
-function PopoverContent({ - className, - align = "center", - sideOffset = 4, - ...props -}: React.ComponentProps<typeof PopoverPrimitive.Content> & { - className?: string; -}) { +const PopoverContent = React.forwardRef< + HTMLDivElement, + React.ComponentProps<typeof PopoverPrimitive.Content> & { + className?: string; + } +>(({ className, align = "center", sideOffset = 4, ...props }, ref) => ( return ( <PopoverPrimitive.Portal> - <PopoverContentPrimitive + <PopoverContentPrimitive ref={ref} ... /> </PopoverPrimitive.Portal> ) -} +)); +PopoverContent.displayName = "PopoverContent";apps/app/src/components/ui/tabs.tsx (1)
37-122: Missing ref forwarding across all Tabs components.None of the Tabs components (
Tabs,TabsList,TabsTrigger,TabsContent) forward refs, unlikeCheckboxandSliderin this PR which useforwardRef. This inconsistency may cause issues if consumers need imperative access to the underlying DOM elements.Consider whether ref forwarding is needed for these components. If not, the
RefAttributesin the primitive wrapper types are unnecessary.apps/app/src/components/ui/dropdown-menu.tsx (1)
127-164: Inconsistent wrapper usage for SubContent and Content.
DropdownMenuSubContentandDropdownMenuContentuseDropdownMenuPrimitive.SubContentandDropdownMenuPrimitive.Contentdirectly, while other components use the newly defined wrapper primitives. If the wrappers are needed for React 19 compatibility, consider adding wrappers for these as well for consistency. If they work without wrappers due to props spreading, this is fine but worth documenting.apps/app/src/components/ui/task-progress-panel.tsx (1)
51-61: Avoid usinganytype for task mapping.The
anytype on line 51 bypasses TypeScript's type checking. Consider defining a proper type for the planSpec tasks to improve type safety.🔎 Suggested improvement
+interface PlanSpecTask { + id: string; + description: string; + filePath?: string; + phase?: string; +} - const initialTasks: TaskInfo[] = planTasks.map((t: any, index: number) => ({ + const initialTasks: TaskInfo[] = planTasks.map((t: PlanSpecTask, index: number) => ({apps/server/src/lib/error-handler.ts (1)
30-38: Clarify overlap between abort and cancellation detection.The keyword "aborted" appears in both
isAbortError(line 20 checksmessage.includes("abort")) andisCancellationError(line 36). Due to the priority order inclassifyError, abort takes precedence over cancellation, which is likely intentional. Consider documenting this precedence or removing "aborted" fromisCancellationErrorsince abort errors are already handled separately.🔎 Option: Remove "aborted" from cancellation check to avoid overlap
export function isCancellationError(errorMessage: string): boolean { const lowerMessage = errorMessage.toLowerCase(); return ( lowerMessage.includes("cancelled") || lowerMessage.includes("canceled") || - lowerMessage.includes("stopped") || - lowerMessage.includes("aborted") + lowerMessage.includes("stopped") ); }Alternatively, add a comment explaining the intentional overlap and precedence.
apps/app/src/components/views/board-view/dialogs/agent-output-modal.tsx (1)
185-190: Minor inconsistency: Unused field check.The
plan_approval_requiredcase checks for'planningMode' in eventbut doesn't actually use the field. Consider either removing the check or including the planning mode in the message for consistency with theplanning_startedhandler above.🔎 Suggested improvement:
- case "plan_approval_required": - // Show when plan requires approval - if ("planningMode" in event) { - newContent = `\n⏸️ Plan generated - waiting for your approval...\n`; - } - break; + case "plan_approval_required": + // Show when plan requires approval + if ("planningMode" in event) { + const modeLabel = + event.planningMode === "lite" + ? "Lite" + : event.planningMode === "spec" + ? "Spec" + : "Full"; + newContent = `\n⏸️ Plan generated (${modeLabel}) - waiting for your approval...\n`; + } + break;apps/server/src/routes/auto-mode/routes/approve-plan.ts (1)
58-64: Consider more specific error status codes.The error response always returns
500regardless of the failure reason. Consider using more specific status codes for better API semantics:
404if the feature is not found409if the approval request is no longer valid (e.g., already processed)500for unexpected server errorsThis would help clients distinguish between client errors (which shouldn't be retried) and server errors (which might succeed on retry).
🔎 Example implementation:
if (!result.success) { + // Determine appropriate status code based on error message + let statusCode = 500; + if (result.error?.includes('not found')) { + statusCode = 404; + } else if (result.error?.includes('No pending approval') || result.error?.includes('already')) { + statusCode = 409; + } + - res.status(500).json({ + res.status(statusCode).json({ success: false, error: result.error, });apps/app/src/components/views/board-view.tsx (1)
400-452: Consider awaitingloadFeatures()for consistency.The
loadFeatures()call on line 440 is not awaited. While the local state is updated immediately viaupdateFeature, a fast user interaction beforeloadFeatures()completes could show stale data. This pattern is acceptable if you want the UI to feel snappy, but consider awaiting it if server synchronization is critical.The overall implementation is correct.
apps/app/src/components/views/board-view/shared/planning-mode-selector.tsx (1)
14-17: ConsolidatePlanningModetype to a single source of truth.
PlanningModeis defined here and also inapps/app/src/store/app-store.ts(line 266). Having duplicate type definitions can lead to drift and maintenance issues. Consider re-exporting from the store instead:-export type PlanningMode = 'skip' | 'lite' | 'spec' | 'full'; - -// Re-export for backwards compatibility -export type { ParsedTask, PlanSpec } from "@/store/app-store"; +// Re-export types from store for single source of truth +export type { PlanningMode, ParsedTask, PlanSpec } from "@/store/app-store";This aligns with the existing pattern of re-exporting
ParsedTaskandPlanSpec.apps/app/src/components/views/settings-view/feature-defaults/feature-defaults-section.tsx (2)
16-16: Consider importing PlanningMode from shared location.The
PlanningModetype is redefined locally but appears to match the type exported from@/store/app-store(line 41 in edit-feature-dialog.tsx imports it). Consider importing the type instead of redefining it to ensure consistency and single source of truth.🔎 Apply this diff to import the type:
+import type { PlanningMode } from "@/store/app-store"; -type PlanningMode = 'skip' | 'lite' | 'spec' | 'full';
70-135: Consider using a map for icon and color selection.The current implementation uses nested ternary chains (lines 74-78, 79-82, 129-133) to determine icons, colors, and descriptions. While functional, a configuration map would improve maintainability and reduce duplication.
🔎 Example refactor using a configuration map:
const PLANNING_MODE_CONFIG = { skip: { icon: Zap, color: 'emerald', bgClass: 'bg-emerald-500/10', iconClass: 'text-emerald-500', description: 'Jump straight to implementation without upfront planning.', }, lite: { icon: ClipboardList, color: 'blue', bgClass: 'bg-blue-500/10', iconClass: 'text-blue-500', description: 'Create a quick planning outline with tasks before building.', }, spec: { icon: FileText, color: 'purple', bgClass: 'bg-purple-500/10', iconClass: 'text-purple-500', description: 'Generate a specification with acceptance criteria for approval.', }, full: { icon: ScrollText, color: 'amber', bgClass: 'bg-amber-500/10', iconClass: 'text-amber-500', description: 'Create comprehensive spec with phased implementation plan.', }, } as const; const config = PLANNING_MODE_CONFIG[defaultPlanningMode]; const Icon = config.icon; // Usage: <div className={cn("...", config.bgClass)}> <Icon className={cn("w-5 h-5", config.iconClass)} /> </div> <p className="...">{config.description}</p>apps/server/src/services/auto-mode-service.ts (4)
34-56: Type definitions are consistent with app-store.The
PlanningMode,ParsedTask, andPlanSpectypes mirror those inapps/app/src/store/app-store.ts(lines 311-332). Consider extracting these to a shared types package to avoid duplication and ensure consistency.
272-296: Regex pattern may not handle all edge cases.The regex pattern on line 274 uses
[^|]+which is greedy and will match the description. However, if the description contains a|character, it will break parsing. Consider escaping or documenting this limitation.🔎 Suggested improvement for edge case handling:
- const taskMatch = line.match(/- \[ \] (T\d{3}):\s*([^|]+)(?:\|\s*File:\s*(.+))?$/); + // Note: Description cannot contain '|' character as it's used as delimiter + const taskMatch = line.match(/- \[ \] (T\d{3}):\s*([^|]+?)(?:\s*\|\s*File:\s*(.+))?$/);The
+?makes the description non-greedy and adds optional whitespace around the delimiter.
1197-1271: Recovery logic is robust but consider timeout.The recovery path (lines 1211-1267) handles the case where the server restarted and lost the in-memory pending approval. This is good resilience. However, if a user never approves/rejects a plan, the entry stays in
pendingApprovalsindefinitely.Consider adding a timeout mechanism or cleanup during periodic health checks to prevent unbounded memory growth in long-running server instances.
1919-2073: Plan revision loop lacks iteration limit.The
while (!planApproved)loop on line 1919 could theoretically run indefinitely if the user keeps providing feedback without approving. While in practice users would eventually approve or cancel, consider adding a maximum revision count to prevent potential runaway scenarios.🔎 Suggested safeguard:
+ const MAX_PLAN_REVISIONS = 10; let planApproved = false; + let revisionCount = 0; - while (!planApproved) { + while (!planApproved && revisionCount < MAX_PLAN_REVISIONS) { console.log(`[AutoMode] Spec v${planVersion} generated for feature ${featureId}, waiting for approval`); // ... existing code ... } else { // User wants revisions - regenerate the plan console.log(`[AutoMode] Plan v${planVersion} rejected with feedback for feature ${featureId}, regenerating...`); planVersion++; + revisionCount++; // ... rest of revision logic ... } } + + if (revisionCount >= MAX_PLAN_REVISIONS) { + throw new Error(`Maximum plan revisions (${MAX_PLAN_REVISIONS}) exceeded`); + }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (38)
apps/app/package.json(1 hunks)apps/app/src/components/ui/checkbox.tsx(2 hunks)apps/app/src/components/ui/dialog.tsx(6 hunks)apps/app/src/components/ui/dropdown-menu.tsx(6 hunks)apps/app/src/components/ui/popover.tsx(1 hunks)apps/app/src/components/ui/select.tsx(1 hunks)apps/app/src/components/ui/slider.tsx(2 hunks)apps/app/src/components/ui/tabs.tsx(2 hunks)apps/app/src/components/ui/task-progress-panel.tsx(1 hunks)apps/app/src/components/ui/tooltip.tsx(1 hunks)apps/app/src/components/views/board-view.tsx(7 hunks)apps/app/src/components/views/board-view/components/kanban-card.tsx(7 hunks)apps/app/src/components/views/board-view/dialogs/add-feature-dialog.tsx(10 hunks)apps/app/src/components/views/board-view/dialogs/agent-output-modal.tsx(3 hunks)apps/app/src/components/views/board-view/dialogs/edit-feature-dialog.tsx(8 hunks)apps/app/src/components/views/board-view/dialogs/follow-up-dialog.tsx(1 hunks)apps/app/src/components/views/board-view/dialogs/index.ts(1 hunks)apps/app/src/components/views/board-view/dialogs/plan-approval-dialog.tsx(1 hunks)apps/app/src/components/views/board-view/hooks/use-board-actions.ts(3 hunks)apps/app/src/components/views/board-view/hooks/use-board-features.ts(1 hunks)apps/app/src/components/views/board-view/kanban-board.tsx(3 hunks)apps/app/src/components/views/board-view/shared/index.ts(1 hunks)apps/app/src/components/views/board-view/shared/planning-mode-selector.tsx(1 hunks)apps/app/src/components/views/settings-view.tsx(2 hunks)apps/app/src/components/views/settings-view/feature-defaults/feature-defaults-section.tsx(2 hunks)apps/app/src/hooks/use-auto-mode.ts(6 hunks)apps/app/src/lib/electron.ts(2 hunks)apps/app/src/lib/http-api-client.ts(1 hunks)apps/app/src/store/app-store.ts(7 hunks)apps/app/src/types/electron.d.ts(3 hunks)apps/server/src/lib/error-handler.ts(5 hunks)apps/server/src/routes/auto-mode/index.ts(2 hunks)apps/server/src/routes/auto-mode/routes/approve-plan.ts(1 hunks)apps/server/src/services/auto-mode-service.ts(23 hunks)apps/server/src/services/feature-loader.ts(1 hunks)apps/server/tests/integration/services/auto-mode-service.integration.test.ts(1 hunks)apps/server/tests/unit/services/auto-mode-service-planning.test.ts(1 hunks)apps/server/tests/unit/services/auto-mode-task-parsing.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (14)
apps/app/src/components/views/board-view/dialogs/agent-output-modal.tsx (2)
apps/app/src/types/electron.d.ts (1)
AutoModeEvent(162-306)apps/app/src/components/ui/task-progress-panel.tsx (1)
TaskProgressPanel(24-274)
apps/server/tests/unit/services/auto-mode-service-planning.test.ts (1)
apps/server/src/services/auto-mode-service.ts (2)
getPlanningPromptPrefix(1586-1605)buildFeaturePrompt(1607-1679)
apps/server/src/routes/auto-mode/index.ts (1)
apps/server/src/routes/auto-mode/routes/approve-plan.ts (1)
createApprovePlanHandler(12-78)
apps/app/src/components/views/board-view/components/kanban-card.tsx (1)
apps/app/src/components/ui/button.tsx (1)
Button(116-116)
apps/app/src/components/views/settings-view/feature-defaults/feature-defaults-section.tsx (6)
apps/app/src/components/views/board-view/shared/planning-mode-selector.tsx (1)
PlanningMode(14-14)apps/app/src/store/app-store.ts (1)
PlanningMode(267-267)apps/app/src/lib/utils.ts (1)
cn(5-7)apps/app/src/components/ui/label.tsx (1)
Label(24-24)apps/app/src/components/ui/select.tsx (5)
Select(150-150)SelectTrigger(153-153)SelectValue(152-152)SelectContent(154-154)SelectItem(156-156)apps/app/src/components/ui/checkbox.tsx (1)
Checkbox(56-56)
apps/app/src/hooks/use-auto-mode.ts (1)
apps/app/src/types/electron.d.ts (1)
AutoModeEvent(162-306)
apps/server/src/services/auto-mode-service.ts (5)
apps/app/src/store/app-store.ts (4)
PlanningMode(267-267)ParsedTask(312-318)PlanSpec(321-332)Feature(281-309)apps/server/src/services/feature-loader.ts (1)
Feature(15-45)apps/server/src/lib/events.ts (1)
EventEmitter(31-34)apps/server/src/lib/error-handler.ts (1)
classifyError(78-105)apps/server/src/providers/types.ts (1)
ExecuteOptions(25-36)
apps/app/src/components/ui/checkbox.tsx (1)
apps/app/src/lib/utils.ts (1)
cn(5-7)
apps/server/tests/integration/services/auto-mode-service.integration.test.ts (2)
apps/server/tests/integration/helpers/git-test-repo.ts (1)
createTestFeature(76-89)apps/server/src/providers/provider-factory.ts (1)
ProviderFactory(13-115)
apps/app/src/components/views/board-view/hooks/use-board-actions.ts (2)
apps/app/src/components/views/board-view/shared/planning-mode-selector.tsx (1)
PlanningMode(14-14)apps/app/src/store/app-store.ts (1)
PlanningMode(267-267)
apps/app/src/components/ui/dialog.tsx (1)
apps/app/src/lib/utils.ts (1)
cn(5-7)
apps/app/src/components/ui/select.tsx (1)
apps/app/src/lib/utils.ts (1)
cn(5-7)
apps/app/src/components/ui/dropdown-menu.tsx (1)
apps/app/src/lib/utils.ts (1)
cn(5-7)
apps/app/src/components/ui/tabs.tsx (1)
apps/app/src/lib/utils.ts (1)
cn(5-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (92)
apps/app/src/components/views/board-view/dialogs/follow-up-dialog.tsx (1)
61-61: LGTM! Good explicit typing.The explicit
React.KeyboardEventtype annotation improves type safety and code clarity without any behavioral changes.apps/app/package.json (1)
48-48: LGTM!The dependency addition is correctly placed in alphabetical order and uses a caret version range consistent with other Radix UI dependencies in the project.
apps/app/src/components/ui/slider.tsx (1)
7-33: LGTM!The type-safe wrappers follow a consistent pattern and correctly add explicit typing for
children,className, and ref attributes to improve React 19 compatibility with Radix UI primitives.apps/app/src/components/ui/tooltip.tsx (1)
41-69: LGTM!
TooltipContentcorrectly usesforwardRefand properly forwards the ref toTooltipContentPrimitive.apps/app/src/components/ui/checkbox.tsx (2)
16-28: LGTM!The type-safe wrappers are correctly defined with explicit typing for React 19 compatibility.
38-43: Verify: Consumers may need the indeterminate state.The normalization coerces
"indeterminate"tofalse, which simplifies the consumer API but loses the ability to detect when the checkbox transitions to/from indeterminate. Ensure this behavior aligns with how the component is used elsewhere.#!/bin/bash # Check if any consumers rely on detecting indeterminate state in onCheckedChange rg -n "onCheckedChange" --type=tsx -C3 | grep -i "indeterminate" || echo "No indeterminate handling found in onCheckedChange consumers" rg -n "<Checkbox" --type=tsx -B2 -A5 | head -50apps/app/src/components/ui/select.tsx (1)
1-160: LGTM!Well-structured Select component implementation. Unlike other files in this PR, this correctly uses
forwardRefthroughout all components, providing proper ref forwarding to consumers. The direct usage of Radix primitives without the type-safe wrapper pattern is cleaner and works well here.apps/app/src/components/ui/dialog.tsx (5)
9-37: Type-safe wrappers approach looks reasonable for React 19 compatibility.The pattern of casting Radix primitives to
ForwardRefExoticComponentwith extended props is a valid workaround for React 19's stricter JSX type inference. This ensureschildrenandclassNameare explicitly typed.Minor organization suggestion: consider grouping all primitive wrappers together (DialogOverlayPrimitive at line 63-67 is separated from the others).
63-88: LGTM!The
DialogOverlaycorrectly uses the wrapper primitive and properly merges className withcn().
90-151: Well-structured enhancement withshowCloseButtonandcompactprops.The logic for handling custom max-width detection and conditional styling is clear. The implementation correctly:
- Defaults
showCloseButtontotruefor backward compatibility- Uses
compactto toggle between dense and standard layouts- Detects custom
max-w-classes to avoid conflicting width constraints
176-193: LGTM!The explicit
childrenprop handling ensures proper rendering with the wrapper primitive.
195-215: LGTM with minor observation.The implementation is correct. Note that the
titleprop here is the standard HTMLtitleattribute (for hover tooltips), which is already included inComponentPropsWithoutRef. The explicit type addition is redundant but doesn't cause issues.apps/app/src/components/ui/dropdown-menu.tsx (6)
9-68: Type-safe wrappers follow consistent pattern.The wrapper approach mirrors the dialog component. Minor redundancy:
React.HTMLAttributes<HTMLDivElement>inDropdownMenuItemPrimitive(line 34) andDropdownMenuRadioItemPrimitive(line 41) is already included viaComponentPropsWithoutRef, so the intersection is unnecessary but harmless.
72-102: LGTM!The
DropdownMenuTriggerandDropdownMenuRadioGroupwrapper functions correctly forward props includingchildrento their respective primitives.
166-185: LGTM!The
DropdownMenuItemcorrectly uses the wrapper primitive and explicitly renders children.
187-236: LGTM!Both
DropdownMenuCheckboxItemandDropdownMenuRadioItemcorrectly use their respective wrapper primitives, includingDropdownMenuItemIndicatorPrimitivefor the indicator icons.
238-272: LGTM!
DropdownMenuLabelandDropdownMenuSeparatorcorrectly use their wrapper primitives with appropriate prop handling.
287-303: LGTM!All components are properly exported, including the newly wrapped
DropdownMenuTriggerandDropdownMenuRadioGroup.apps/app/src/lib/http-api-client.ts (1)
579-592: LGTM!The
approvePlanmethod implementation correctly follows the established pattern used by otherautoModemethods. The signature matches theAutoModeAPIinterface definition, and the POST body includes all required and optional parameters.apps/app/src/components/ui/task-progress-panel.tsx (2)
178-188: LGTM - Nice circular progress implementation.The SVG-based circular progress indicator with
strokeDasharrayandstrokeDashoffsetis a clean implementation. The math is correct: circumference ≈ 2πr = 2π×10 ≈ 62.8 (rounded to 63).
93-121: The heuristic for marking previous tasks as completed assumes sequential execution and could incorrectly mark tasks if they run concurrently or events arrive out of order.Lines 105-107 mark earlier tasks as completed when moving to a later task. This logic assumes tasks execute strictly in order and that events arrive sequentially. If the planning workflow can execute multiple tasks concurrently or events arrive out of order (e.g., task 3 starts before task 2), this heuristic will incorrectly mark tasks as completed. The fallback to add new tasks (lines 112-120) provides some resilience, but the state logic remains fragile. Consider adding a mechanism to verify task order or track completion status more explicitly rather than inferring it from list position.
apps/server/src/lib/error-handler.ts (1)
58-68: LGTM - Clean extension of error classification.The
ErrorTypeunion andErrorInfointerface are cleanly extended to support cancellation errors. The newisCancellationfield inErrorInfoprovides explicit access to the cancellation status.apps/app/src/types/electron.d.ts (2)
242-306: LGTM - Comprehensive event type definitions.The new
AutoModeEventvariants are well-structured with consistent patterns:
- All include
featureIdas required- Optional
projectPathfor context- Task events include progress tracking fields (
taskIndex,tasksTotal,tasksCompleted)- Planning events properly distinguish between
planningModevalues ("lite" | "spec" | "full")
473-482: LGTM - Clean API method signature.The
approvePlanmethod signature properly defines required (projectPath,featureId,approved) and optional (editedPlan,feedback) parameters, matching the server-side handler expectations.apps/server/src/routes/auto-mode/index.ts (1)
20-20: LGTM - Clean route integration.The new
/approve-planendpoint follows the established pattern for route registration in this file. The handler (per the code snippet) includes proper validation for required fields, appropriate error handling, and logging.Also applies to: 39-39
apps/app/src/components/views/board-view/shared/index.ts (1)
7-7: LGTM!The export addition follows the established barrel pattern and properly exposes the planning-mode-selector module.
apps/app/src/components/views/board-view/dialogs/index.ts (1)
9-9: LGTM!The export follows the existing pattern for dialog components.
apps/app/src/components/views/board-view/kanban-board.tsx (1)
50-51: LGTM!The callback props are properly threaded through the component hierarchy following the established pattern used by other action handlers like
onEdit,onDelete, etc.Also applies to: 82-83, 196-197
apps/app/src/components/views/board-view/hooks/use-board-features.ts (1)
213-217: LGTM!The event handler follows the same pattern as the
auto_mode_feature_completehandler above it, ensuring the board refreshes to display the approval button when a plan is ready for review.apps/app/src/components/views/board-view/dialogs/agent-output-modal.tsx (2)
173-230: LGTM with solid defensive programming!The new event handlers provide comprehensive coverage of the planning workflow. The use of type narrowing with
Extract<AutoModeEvent, { type: "..." }>is excellent for type safety, and the defensive field checks using theinoperator prevent runtime errors.
350-355: LGTM!The TaskProgressPanel is well-integrated and properly positioned to show task progress without disrupting the existing modal layout.
apps/server/src/routes/auto-mode/routes/approve-plan.ts (2)
23-37: LGTM!Input validation is thorough with appropriate error messages and status codes.
39-56: Excellent recovery design!The comment and implementation show good awareness of the server restart scenario. Delegating recovery logic to
resolvePlanApprovalkeeps this endpoint simple while supporting resilience.apps/server/tests/integration/services/auto-mode-service.integration.test.ts (1)
543-738: Excellent test coverage for planning modes!The test suite comprehensively covers:
- All planning mode variants (skip, lite, spec, full)
- Approval workflows and edge cases
- Event emission verification
- Error handling for non-existent features
The tests follow consistent patterns and use appropriate timeouts for integration testing.
apps/server/tests/unit/services/auto-mode-service-planning.test.ts (5)
21-107: Comprehensive getPlanningPromptPrefix test coverage!The tests validate:
- All planning modes (skip, lite, spec, full)
- Lite mode with/without approval requirement
- Correct structure and content markers for each mode
- Separator and header inclusion
109-147: Indirect testing approach is pragmatic.Since
parseTasksFromSpecis a module-level function, testing through content validation is a reasonable approach. The tests verify the expected task format and phase structure are present in spec content.
149-199: Good coverage of error paths!These tests validate that the approval flow handles edge cases gracefully:
- Non-existent approvals
- Cancellation without errors
- Resolution failures with appropriate error messages
201-274: Thorough buildFeaturePrompt and extractTitle validation!The tests cover:
- Required fields (ID, description, spec)
- Optional fields (images)
- Summary tag instructions
- Title extraction edge cases (empty, short, long with truncation)
276-331: LGTM!The structural tests ensure all planning modes have the expected sections and instructions, providing good regression protection.
apps/app/src/components/views/board-view/hooks/use-board-actions.ts (2)
7-7: LGTM!The
PlanningModeimport and its integration intohandleAddFeatureare correctly implemented. The newplanningModeandrequirePlanApprovalfields are properly included in the feature data and propagated to the store.Also applies to: 154-155
200-201: LGTM!The optional
planningModeandrequirePlanApprovalfields inhandleUpdateFeatureallow partial updates, which is appropriate for the edit flow.apps/app/src/components/views/board-view.tsx (3)
454-501: Same observation for reject flow.The
loadFeatures()call on line 489 follows the same fire-and-forget pattern as the approve flow. The local state update ensures immediate UI feedback, which is a reasonable trade-off.
503-522: LGTM!The
handleOpenApprovalDialogcorrectly constructs the pending approval state from the feature's existing plan data, with appropriate type narrowing for theapprovalMode.
754-780: LGTM!The dual dialog approach (one for interactive approval, one for read-only viewing) is clean. The view-only dialog appropriately uses no-op handlers and passes
viewOnly={true}.apps/app/src/components/views/board-view/shared/planning-mode-selector.tsx (3)
77-96: LGTM!The component props are well-structured with sensible defaults. The derived state (
requiresApproval,canGenerate,hasSpec) cleanly encapsulates the mode logic.
133-194: LGTM!The mode selection cards are well-implemented with proper accessibility (
type="button",data-testid), visual feedback for selected state, and responsive grid layout.
214-331: LGTM!The spec preview panel has appropriate conditional rendering based on
planSpec.status, with clear visual indicators for each state (generating, approved, ready for review). The action buttons are contextually shown.apps/app/src/components/views/board-view/dialogs/add-feature-dialog.tsx (4)
115-132: LGTM!The planning mode state is correctly initialized from store defaults and synced when the dialog opens. The dependency array properly includes all relevant defaults.
157-158: LGTM!The planning mode values are correctly passed to
onAddand reset to defaults after submission, maintaining consistent behavior across add operations.Also applies to: 174-175
469-493: LGTM!The Options tab layout is clean with a clear visual separation between the planning mode selector and testing section. The
compactprop appropriately adapts the selector for dialog context.
246-257: Use the genericEventtype instead ofCustomEventfor the event handlers.The
onPointerDownOutsideandonInteractOutsidehandlers should acceptEventtype, notCustomEvent. While Radix Dialog internally uses a PointerDownOutsideEvent type for these handlers, it was not exported in the public API. UsingCustomEventis semantically incorrect since these are not custom events. Either leave the handlers untyped or explicitly type them asEventto match the pattern used in Radix documentation and community implementations.Likely an incorrect or invalid review comment.
apps/app/src/components/views/board-view/dialogs/plan-approval-dialog.tsx (5)
40-53: LGTM!The state initialization and reset logic is correct. The
useEffectproperly syncseditedPlanwhenplanContentchanges, handling the case where the component is already mounted when new plan content arrives.
55-67: LGTM!The approve/reject handlers are well-designed:
handleApproveavoids unnecessary data transfer by only passing the edited plan when actually modified.handleRejectimplements a thoughtful two-phase UX (first click shows feedback input, second click submits).
74-78: LGTM!Preventing dialog close during loading operations is good UX practice to avoid data loss or inconsistent states.
129-144: LGTM!The content area correctly switches between editable textarea and Markdown render based on mode. The fallback message "No plan content available." handles edge cases gracefully.
165-216: LGTM!The footer button logic cleanly handles all three states:
viewOnly- simple close buttonshowRejectFeedback- back/submit buttons with dynamic label ("Revise Plan" vs "Cancel Feature")- Default - reject/approve buttons
The loading states are properly reflected in button disabled states and spinner icons.
apps/app/src/components/views/settings-view.tsx (1)
43-46: LGTM! Clean integration of planning defaults.The planning mode and approval settings are correctly wired from the store through to the FeatureDefaultsSection component. The pattern is consistent with the existing code for other settings.
Also applies to: 129-130, 135-136
apps/server/tests/unit/services/auto-mode-task-parsing.test.ts (3)
42-89: LGTM! Robust task parsing with good fallback logic.The parsing function correctly handles:
- Tasks within ```tasks code blocks
- Phase headers (e.g.,
## Phase 1: Foundation)- Fallback to searching for task lines anywhere in content
- Tasks with and without file paths
The phase header regex (
/^##\s*(.+)$/) flexibly matches any##header, not just those starting with "Phase", which provides good forward compatibility.
91-345: LGTM! Comprehensive test coverage.The test suite covers:
- Valid task parsing with various formats
- Invalid input handling
- Phase-based task organization
- Task order preservation
- Different task ID formats
- Expected output patterns for different planning modes (lite, spec, full)
The tests provide good confidence in the parsing logic's correctness and robustness.
16-40: The task ID pattern consistency is confirmed. Both the test file and production code (auto-mode-service.ts) use the identical regex patternT\d{3}for task ID validation, ensuring strict 3-digit format across the codebase. No alignment issues found.apps/app/src/components/views/board-view/components/kanban-card.tsx (5)
108-109: LGTM! Props addition follows existing patterns.The new
onViewPlanandonApprovePlancallbacks are correctly typed as optional and follow the same pattern as other action callbacks in the component.Also applies to: 135-136
886-902: LGTM! Approve Plan button properly highlights urgent action.The button correctly:
- Checks both
planSpec.status === 'generated'and callback availability- Uses
animate-pulseto draw attention when plan approval is needed- Applies responsive layout with
flex-1 min-w-0andshrink-0on icon- Includes clear test ID for automated testing
946-962: LGTM! Consistent Approve Plan button for paused features.The button maintains consistency with the running task variant and correctly handles the case where a feature is in progress but waiting for plan approval (not actively running).
1117-1132: LGTM! View Plan button appropriately styled for backlog.The button correctly:
- Checks for
planSpec.contentpresence (appropriate for backlog state)- Uses Eye icon for viewing action
- Includes compact styling (
px-2) for icon-only display- Provides tooltip via
titleattribute
883-883: LGTM! Layout improvements enhance responsiveness.The changes improve the card's layout:
flex-wrapallows action buttons to wrap gracefully on smaller screensshrink-0on icons prevents icon distortiontruncateon button text prevents overflow while maintaining icon visibilityThese are good responsive design practices.
Also applies to: 915-917, 1041-1042
apps/app/src/hooks/use-auto-mode.ts (4)
7-10: LGTM! Type guard correctly narrows event type.The type guard follows TypeScript best practices by using a type predicate with
Extractto narrow theAutoModeEventunion type.
158-167: LGTM! Error handling correctly distinguishes cancellations from errors.The implementation properly uses
errorTypeto detect user-initiated cancellations and aborts, avoiding the fragile string-matching approach. Cancellations are logged as info rather than errors, and the feature is removed from running tasks without triggering error UI.
234-304: LGTM! Plan approval event handlers comprehensively track planning lifecycle.The handlers correctly:
- Use the type guard to narrow event types
- Set pending approval state with proper fallback for project path
- Distinguish between user approval (with/without edits), auto-approval, and revision requests
- Track plan versions for revision cycles
- Log appropriate activities for UI feedback
Note that
currentProject?.pathis included in the effect's dependency array (line 364), ensuring the handlers have access to the current project context.
306-350: LGTM! Task and phase progress handlers provide detailed tracking.The handlers effectively:
- Use type assertions to access event-specific properties (taskId, taskDescription, tasksCompleted, tasksTotal, phaseNumber)
- Provide informative logging with visual indicators (▶ for start, ✓ for complete)
- Track progress counts for task completion
- Add activities for UI feedback
The type assertions are appropriate given the discriminated union nature of
AutoModeEvent.apps/app/src/lib/electron.ts (2)
261-267: LGTM! API signature clearly models plan approval workflow.The
approvePlanmethod signature appropriately captures:
- Required parameters:
projectPath,featureId,approved- Optional parameters:
editedPlan(for user modifications),feedback(for rejection/revision reasons)- Standard return type matching other API methods
The signature supports both approval and rejection flows with proper context.
1706-1721: LGTM! Mock implementation provides appropriate logging.The mock correctly:
- Logs approval parameters for debugging
- Uses
"[edited]"placeholder for edited plans (avoids logging large content)- Returns success immediately (appropriate for mock)
Note: The mock doesn't emit events or update state, but this is consistent with other mock methods and acceptable since the real backend implementation handles the full workflow.
apps/app/src/components/views/board-view/dialogs/edit-feature-dialog.tsx (5)
41-41: LGTM! State initialization properly syncs with feature data.The planning mode state:
- Defaults to
'skip'(consistent with app defaults)- Syncs with feature data via
useEffectwhen feature changes- Uses proper fallback chains (
feature?.planningMode ?? 'skip')Also applies to: 49-49, 106-107, 114-117
74-75: LGTM! Planning mode included in update payload.The
onUpdateinterface andhandleUpdateimplementation correctly includeplanningModeandrequirePlanApprovalfrom local state, allowing users to modify these settings before saving.Also applies to: 143-144
26-26: LGTM! Tab renamed to better reflect expanded content.Renaming from "testing" to "options" is appropriate since the tab now contains planning mode settings in addition to testing configuration. The
SlidersHorizontalicon is a standard choice for options/settings.Also applies to: 252-255
466-492: LGTM! Options tab cleanly integrates planning and testing settings.The tab structure:
- Uses
PlanningModeSelectorwith all required props correctly wired- Includes visual separator between planning and testing sections
- Preserves existing
TestingTabContentfunctionality- Uses
compactprop for appropriate sizing in dialog context
222-222: LGTM! Improved type safety for event handlers.Explicitly typing the event handlers as
CustomEventimproves type safety and aligns with shadcn/ui Dialog component expectations.Also applies to: 228-228
apps/app/src/components/views/settings-view/feature-defaults/feature-defaults-section.tsx (3)
23-24: LGTM! Props correctly typed and wired.The new planning-related props follow the existing patterns:
- State props:
defaultPlanningMode,defaultRequirePlanApproval- Callbacks:
onDefaultPlanningModeChange,onDefaultRequirePlanApprovalChange- All properly typed and destructured
Also applies to: 29-30, 37-40, 44-45
137-167: LGTM! Approval UI correctly conditionally rendered.The approval checkbox:
- Only appears when planning mode is not 'skip' (appropriate since skip has no plan to approve)
- Has proper label association via
htmlForandid- Includes clear descriptive text explaining the feature
- Uses appropriate icons (
ShieldCheck)
165-165: LGTM! Separator logic maintains consistent spacing.The dual separator approach ensures proper visual separation regardless of whether the approval checkbox is visible:
- Line 165: Separator after approval UI (when shown)
- Line 170: Separator when approval UI is hidden (skip mode)
Also applies to: 170-170
apps/server/src/services/auto-mode-service.ts (7)
58-212: Well-structured planning prompts.The prompts clearly define the expected output format with parseable markers (
[SPEC_GENERATED],[TASK_START],[TASK_COMPLETE]). The task block syntax withT###IDs enables structured parsing.
322-327: LGTM!The
PendingApprovalinterface correctly captures the Promise resolver pattern needed for the async approval workflow.
457-466: Good refactor addressing the past review comment.The method signature now uses an
optionsobject for the sixth parameter instead of positional arguments. This resolves the previous issue wherecontinuationPromptwas being passed asprovidedWorktreePath. The options pattern is more extensible and type-safe.
639-643: LGTM!Properly cancels pending plan approvals before aborting the feature, preventing orphaned promises in the pendingApprovals map.
2106-2206: Multi-agent task execution is well-structured.Good practices observed:
- Abort check before each task (line 2114)
- Turn limit per task prevents runaway (line 2141)
- Proper progress events for UI updates
- Phase completion detection for grouped tasks
One consideration: if a task fails (throws), the entire feature fails. Consider whether partial completion should be preserved.
2335-2405: Well-designed task prompt structure.The prompt effectively provides focused context by showing completed tasks, current task details, and a preview of upcoming work. The full plan is wrapped in a
<details>tag to keep the prompt concise while maintaining access to full context.
2307-2333: LGTM!The method now correctly passes the continuation prompt via the options object, addressing the previous review comment about argument mismatch.
apps/app/src/store/app-store.ts (6)
266-332: LGTM!The planning-related types are well-defined and match the server-side definitions. The
PlanSpecinterface comprehensively captures the plan lifecycle state including task progress tracking.
492-503: LGTM!The
pendingPlanApprovaltype correctly excludes'skip'fromplanningModesince skip mode doesn't generate plans requiring approval. The state structure aligns well with the server-side event payloads.
733-742: LGTM!Standard setter actions for the new planning state. The action signatures correctly match the state types.
832-834: LGTM!Sensible defaults:
'skip'disables planning by default, andrequirePlanApproval: falseis consistent with this. Theas PlanningModeassertion is necessary for TypeScript to infer the correct union type.
2243-2247: LGTM!Simple, idiomatic zustand setter implementations.
2317-2318: LGTM!Correctly persists user preferences (
defaultPlanningMode,defaultRequirePlanApproval) while excluding transient state (pendingPlanApproval). Pending approval is session-specific and should be cleared on app reload.
| const featurePath = path.join( | ||
| projectPath, | ||
| ".automaker", | ||
| "features", | ||
| featureId, | ||
| "feature.json" | ||
| ); | ||
|
|
||
| try { | ||
| const data = await fs.readFile(featurePath, "utf-8"); | ||
| const feature = JSON.parse(data); | ||
|
|
||
| // Initialize planSpec if it doesn't exist | ||
| if (!feature.planSpec) { | ||
| feature.planSpec = { | ||
| status: 'pending', | ||
| version: 1, | ||
| reviewedByUser: false, | ||
| }; | ||
| } | ||
|
|
||
| // Apply updates | ||
| Object.assign(feature.planSpec, updates); | ||
|
|
||
| // If content is being updated and it's a new version, increment version | ||
| if (updates.content && updates.content !== feature.planSpec.content) { | ||
| feature.planSpec.version = (feature.planSpec.version || 0) + 1; | ||
| } | ||
|
|
||
| feature.updatedAt = new Date().toISOString(); | ||
| await fs.writeFile(featurePath, JSON.stringify(feature, null, 2)); | ||
| } catch (error) { | ||
| console.error(`[AutoMode] Failed to update planSpec for ${featureId}:`, error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Use getFeatureDir helper for consistency.
Lines 1479-1484 construct the feature path manually instead of using the getFeatureDir helper (imported and used elsewhere, e.g., line 655). This could lead to inconsistencies if the path structure changes.
Also, the version increment logic on lines 1504-1506 has a subtle issue: it checks updates.content !== feature.planSpec.content, but Object.assign on line 1501 already merged updates into feature.planSpec, so this comparison may not work as intended.
🔎 Suggested fix:
+ import { getFeatureDir } from "../lib/automaker-paths.js";
+ // ... already imported at top
private async updateFeaturePlanSpec(
projectPath: string,
featureId: string,
updates: Partial<PlanSpec>
): Promise<void> {
- const featurePath = path.join(
- projectPath,
- ".automaker",
- "features",
- featureId,
- "feature.json"
- );
+ const featureDir = getFeatureDir(projectPath, featureId);
+ const featurePath = path.join(featureDir, "feature.json");
try {
const data = await fs.readFile(featurePath, "utf-8");
const feature = JSON.parse(data);
// Initialize planSpec if it doesn't exist
if (!feature.planSpec) {
feature.planSpec = {
status: 'pending',
version: 1,
reviewedByUser: false,
};
}
+ // Check for content change before merging
+ const contentChanged = updates.content && updates.content !== feature.planSpec.content;
+
// Apply updates
Object.assign(feature.planSpec, updates);
- // If content is being updated and it's a new version, increment version
- if (updates.content && updates.content !== feature.planSpec.content) {
+ if (contentChanged) {
feature.planSpec.version = (feature.planSpec.version || 0) + 1;
}🤖 Prompt for AI Agents
In apps/server/src/services/auto-mode-service.ts around lines 1479 to 1513,
replace the manual construction of featurePath with the existing
getFeatureDir(projectPath, featureId) helper (and join "feature.json" to it) to
ensure consistent path logic; before merging updates, capture the oldContent
(e.g., const oldContent = feature.planSpec?.content) then apply
Object.assign(feature.planSpec, updates) and only increment
feature.planSpec.version when updates.content is provided and different from
oldContent (e.g., if (updates.content && updates.content !== oldContent)
feature.planSpec.version = (feature.planSpec.version || 0) + 1); keep planSpec
initialization and updatedAt/writeFile logic and preserve the catch/error
handling.
Merge latest features from main including: - PR #161 (worktree-confusion): Clarified branch handling in dialogs - PR #160 (speckits-rebase): Planning mode functionality Resolved conflicts: - add-feature-dialog.tsx: Combined TanStack Router navigation with branch selection state - worktree-integration.spec.ts: Updated tests for new worktree behavior (created at execution time) - package-lock.json: Regenerated after merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.