fix: Human review checkpoint using stopped status (Issue #1231)

.gitignore

@@ -7,6 +7,12 @@
 Thumbs.db
 ehthumbs.db
 Desktop.ini
+nul
+
+# ===========================
+# Local Development
+# ===========================
+.bill/
 
 # ===========================
 # Security - Environment & Secrets

apps/backend/implementation_plan/plan.py

@@ -203,9 +203,12 @@ def update_status_from_subtasks(self):
             self.planStatus = "in_progress"
         else:
             # All subtasks pending
-            # Preserve human_review/review status if it's for plan approval stage
+            # Preserve review status if it's for plan approval stage
             # (spec is complete, waiting for user to approve before coding starts)
-            if self.status == "human_review" and self.planStatus == "review":
+            # This covers both:
+            # - human_review + review: legacy plan review state
+            # - stopped + awaiting_review: new plan review state (set by spec_runner.py --no-build)
+            if self.status in ("human_review", "stopped") and self.planStatus in ("review", "awaiting_review"):
                 # Keep the plan approval status - don't reset to backlog
                 pass
             else:

apps/backend/runners/spec_runner.py

@@ -46,6 +46,7 @@
 
 import asyncio
 import io
+import json
 import os
 from pathlib import Path
 
@@ -372,6 +373,34 @@ def main():
             # Execute run.py - replace current process
             os.execv(sys.executable, run_cmd)
 
+        else:
+            # --no-build specified: Set planStatus to 'awaiting_review' to signal frontend
+            # that planning is complete and user review is required before coding starts
+            debug("spec_runner", "--no-build flag detected, setting awaiting_review status")
+            print()
+            print_status("--no-build flag: Setting plan to awaiting review", "info")
+
+            plan_path = orchestrator.spec_dir / "implementation_plan.json"
+            debug("spec_runner", f"Looking for plan file at: {plan_path}")
+
+            if plan_path.exists():
+                try:
+                    plan_data = json.loads(plan_path.read_text(encoding="utf-8"))
+                    plan_data["planStatus"] = "awaiting_review"
+                    plan_data["status"] = "stopped"  # Mark as stopped for review
+                    plan_path.write_text(json.dumps(plan_data, indent=2, ensure_ascii=False), encoding="utf-8")
+                    debug("spec_runner", "Set planStatus to 'awaiting_review' for frontend")
+                    print()
+                    print_status("Planning complete. Awaiting review before coding.", "success")
+                    print(f"  {muted('Review the spec at:')} {orchestrator.spec_dir / 'spec.md'}")
+                    print(f"  {muted('When ready, restart the task in the UI to begin coding.')}")
+                except (json.JSONDecodeError, OSError) as e:
+                    debug_error("spec_runner", f"Failed to update plan status: {e}")
+                    print_status(f"Failed to update plan status: {e}", "error")
+            else:
+                debug_error("spec_runner", f"Plan file not found at: {plan_path}")
+                print_status(f"Warning: implementation_plan.json not found at {plan_path}", "warning")
+
         sys.exit(0)
 
     except KeyboardInterrupt:

apps/backend/task_logger/storage.py

@@ -129,6 +129,7 @@ def update_phase_status(
             self._data["phases"][phase]["status"] = status
             if completed_at:
                 self._data["phases"][phase]["completed_at"] = completed_at
+            self.save()
 
     def set_phase_started(self, phase: str, started_at: str) -> None:
         """

apps/frontend/src/main/agent/agent-manager.ts

@@ -148,7 +148,14 @@ export class AgentManager extends EventEmitter {
     }
 
     // Check if user requires review before coding
-    if (!metadata?.requireReviewBeforeCoding) {
+    if (metadata?.requireReviewBeforeCoding) {
+      // Human review required: Stop after spec creation, don't start build
+      // Frontend will handle the review gate and resume coding after approval
+      // NOTE: Also pass --auto-approve so the spec gets approved during creation
+      // (otherwise spec_runner exits early with "spec not approved" error)
+      args.push('--no-build');
+      args.push('--auto-approve');
+    } else {
       // Auto-approve: When user starts a task from the UI without requiring review
       args.push('--auto-approve');
     }
@@ -175,8 +182,9 @@ export class AgentManager extends EventEmitter {
     // Store context for potential restart
     this.storeTaskContext(taskId, projectPath, '', {}, true, taskDescription, specDir, metadata, baseBranch);
 
-    // Note: This is spec-creation but it chains to task-execution via run.py
-    await this.processManager.spawnProcess(taskId, autoBuildSource, args, combinedEnv, 'task-execution');
+    // Use 'spec-creation' processType so exit handler can distinguish planning from coding
+    // This prevents infinite loop when auto-continue triggers after coding completes
+    await this.processManager.spawnProcess(taskId, autoBuildSource, args, combinedEnv, 'spec-creation');
   }
 
   /**

apps/frontend/src/main/ipc-handlers/agent-events-handlers.ts

@@ -1,6 +1,6 @@
 import type { BrowserWindow } from "electron";
 import path from "path";
-import { existsSync } from "fs";
+import { existsSync, readFileSync, writeFileSync } from "fs";
 import { IPC_CHANNELS, AUTO_BUILD_PATHS, getSpecsDir } from "../../shared/constants";
 import {
   wouldPhaseRegress,
@@ -22,7 +22,7 @@ import { titleGenerator } from "../title-generator";
 import { fileWatcher } from "../file-watcher";
 import { projectStore } from "../project-store";
 import { notificationService } from "../notification-service";
-import { persistPlanStatusSync, getPlanPath } from "./task/plan-file-utils";
+import { persistPlanStatusSync, getPlanPath, createPlanIfNotExists } from "./task/plan-file-utils";
 import { findTaskWorktree } from "../worktree-paths";
 import { findTaskAndProject } from "./task/shared";
 import { safeSendToRenderer } from "./utils";
@@ -135,7 +135,7 @@ export function registerAgenteventsHandlers(
 
   agentManager.on("exit", (taskId: string, code: number | null, processType: ProcessType) => {
     // Get project info early for multi-project filtering (issue #723)
-    const { project: exitProject } = findTaskAndProject(taskId);
+    const { project: exitProject, task: exitTask } = findTaskAndProject(taskId);
     const exitProjectId = exitProject?.id;
 
     // Send final plan state to renderer BEFORE unwatching
@@ -153,9 +153,97 @@ export function registerAgenteventsHandlers(
 
     fileWatcher.unwatch(taskId);
 
-    if (processType === "spec-creation") {
-      console.warn(`[Task ${taskId}] Spec creation completed with code ${code}`);
-      return;
+    // FIX: Planning-to-coding transition (Issue #1231)
+    // Only run this logic for spec-creation process (planning phase).
+    // This prevents infinite loop: without this check, when coding (task-execution) completes,
+    // this handler would incorrectly call startTaskExecution again, causing an infinite loop.
+    // The processType check ensures we only auto-continue from planning → coding, not coding → coding.
+    if (processType === 'spec-creation' && (code === 0 || code === 1) && exitProject && exitTask) {
+      const specsBaseDir = getSpecsDir(exitProject.autoBuildPath);
+      const specDir = path.join(exitProject.path, specsBaseDir, exitTask.specId);
+      const specFilePath = path.join(specDir, AUTO_BUILD_PATHS.SPEC_FILE);
+      const planPath = path.join(specDir, AUTO_BUILD_PATHS.IMPLEMENTATION_PLAN);
+      const metadataPath = path.join(specDir, "task_metadata.json");
+
+      // Check if spec and plan exist (planning completed)
+      const hasSpec = existsSync(specFilePath);
+      const hasPlan = existsSync(planPath);
+
+      if (hasSpec && hasPlan) {
+        // Read metadata and plan once to avoid multiple file reads
+        let metadata: Record<string, unknown> = {};
+        let planContent: Record<string, unknown> = {};
+
+        if (existsSync(metadataPath)) {
+          try {
+            metadata = JSON.parse(readFileSync(metadataPath, "utf-8"));
+          } catch (err) {
+            console.error(`[Task ${taskId}] Failed to read metadata:`, err);
+          }
+        }
+
+        try {
+          planContent = JSON.parse(readFileSync(planPath, "utf-8"));
+        } catch (err) {
+          console.error(`[Task ${taskId}] Failed to read plan:`, err);
+        }
+
+        const requireReviewBeforeCoding = metadata.requireReviewBeforeCoding === true;
+        const allSubtasks = ((planContent.phases as Array<{ subtasks?: unknown[] }>) || [])
+          .flatMap((p) => p.subtasks || []);
+        const planHasSubtasks = allSubtasks.length > 0;
+
+        if (planHasSubtasks && requireReviewBeforeCoding) {
+          // User wants to review before coding - STOP the task
+          // Note: spec_runner.py with --no-build already sets these values, but we set them
+          // here as a fallback in case the Python process didn't complete the update
+          try {
+            metadata.stoppedForPlanReview = true;
+            writeFileSync(metadataPath, JSON.stringify(metadata, null, 2));
+          } catch (err) {
+            console.error(`[Task ${taskId}] Failed to update task_metadata.json:`, err);
+          }
+
+          try {
+            planContent.status = "stopped";
+            planContent.planStatus = "awaiting_review";
+            writeFileSync(planPath, JSON.stringify(planContent, null, 2));
+          } catch (err) {
+            console.error(`[Task ${taskId}] Failed to update implementation_plan.json:`, err);
+          }
+
+          // Invalidate cache so UI reflects the new status
+          projectStore.invalidateTasksCache(exitProject.id);
+
+          // Notify renderer of status change
+          safeSendToRenderer(
+            getMainWindow,
+            IPC_CHANNELS.TASK_STATUS_CHANGE,
+            taskId,
+            "stopped" as TaskStatus,
+            exitProjectId
+          );
+
+          return;
+        }
+
+        if (planHasSubtasks && !requireReviewBeforeCoding) {
+          // Auto-continue to coding phase
+          const baseBranch = (exitTask.metadata?.baseBranch as string | undefined) || exitProject.settings?.mainBranch;
+          agentManager.startTaskExecution(
+            taskId,
+            exitProject.path,
+            exitTask.specId,
+            {
+              parallel: false,
+              workers: 1,
+              baseBranch,
+              useWorktree: exitTask.metadata?.useWorktree as boolean | undefined
+            }
+          );
+          return;
+        }
+      }
     }
 
     let task: Task | undefined;

apps/frontend/src/main/ipc-handlers/settings-handlers.ts

@@ -526,6 +526,51 @@ export function registerSettingsHandlers(
     }
   );
 
+  ipcMain.handle(
+    IPC_CHANNELS.SHELL_OPEN_PATH,
+    async (_, filePath: string): Promise<IPCResult<string>> => {
+      try {
+        // Validate filePath input
+        if (!filePath || typeof filePath !== 'string' || filePath.trim() === '') {
+          return {
+            success: false,
+            error: 'File path is required and must be a non-empty string'
+          };
+        }
+
+        // Resolve to absolute path
+        const resolvedPath = path.resolve(filePath);
+
+        // Verify path exists
+        if (!existsSync(resolvedPath)) {
+          return {
+            success: false,
+            error: `File does not exist: ${resolvedPath}`
+          };
+        }
+
+        // Open with system default application
+        const result = await shell.openPath(resolvedPath);
+
+        // shell.openPath returns empty string on success, error message on failure
+        if (result) {
+          return {
+            success: false,
+            error: result
+          };
+        }
+
+        return { success: true, data: resolvedPath };
+      } catch (error) {
+        const errorMsg = error instanceof Error ? error.message : 'Unknown error';
+        return {
+          success: false,
+          error: `Failed to open file: ${errorMsg}`
+        };
+      }
+    }
+  );
+
   // ============================================
   // Auto-Build Source Environment Operations
   // ============================================

apps/frontend/src/main/ipc-handlers/task/execution-handlers.ts

@@ -221,6 +221,21 @@ export function registerTaskExecutionHandlers(
           // Use default description
         }
 
+        // Clear stoppedForPlanReview flag if it was set (user is resuming after review)
+        if (task.metadata?.stoppedForPlanReview) {
+          const metadataPath = path.join(specDir, 'task_metadata.json');
+          try {
+            if (existsSync(metadataPath)) {
+              const metadata = JSON.parse(readFileSync(metadataPath, 'utf-8'));
+              metadata.stoppedForPlanReview = false;
+              writeFileSync(metadataPath, JSON.stringify(metadata, null, 2));
+              console.warn('[TASK_START] Cleared stoppedForPlanReview flag');
+            }
+          } catch (err) {
+            console.error('[TASK_START] Failed to clear stoppedForPlanReview flag:', err);
+          }
+        }
+
         console.warn('[TASK_START] Starting task execution (no subtasks) for:', task.specId);
         // Start task execution which will create the implementation plan
         // Note: No parallel mode for planning phase - parallel only makes sense with multiple subtasks

apps/frontend/src/preload/api/modules/shell-api.ts

@@ -8,6 +8,7 @@ import type { IPCResult } from '../../../shared/types';
 export interface ShellAPI {
   openExternal: (url: string) => Promise<void>;
   openTerminal: (dirPath: string) => Promise<IPCResult<void>>;
+  openPath: (filePath: string) => Promise<IPCResult<string>>;
 }
 
 /**
@@ -17,5 +18,7 @@ export const createShellAPI = (): ShellAPI => ({
   openExternal: (url: string): Promise<void> =>
     invokeIpc(IPC_CHANNELS.SHELL_OPEN_EXTERNAL, url),
   openTerminal: (dirPath: string): Promise<IPCResult<void>> =>
-    invokeIpc(IPC_CHANNELS.SHELL_OPEN_TERMINAL, dirPath)
+    invokeIpc(IPC_CHANNELS.SHELL_OPEN_TERMINAL, dirPath),
+  openPath: (filePath: string): Promise<IPCResult<string>> =>
+    invokeIpc(IPC_CHANNELS.SHELL_OPEN_PATH, filePath)
 });

apps/frontend/src/renderer/__tests__/task-order.test.ts

@@ -32,6 +32,7 @@
     human_review: [],
     pr_created: [],
     done: [],
+    stopped: [],
     ...overrides
   };
 }
@@ -243,7 +244,7 @@
     it('should create empty task order if no stored order exists', () => {
       useTaskStore.getState().loadTaskOrder('project-1');
 
       expect(useTaskStore.getState().taskOrder).toEqual({
         backlog: [],
         in_progress: [],
         ai_review: [],
@@ -275,7 +276,7 @@
       useTaskStore.getState().loadTaskOrder('project-1');
 
       // Should fall back to empty order state
       expect(useTaskStore.getState().taskOrder).toEqual({
         backlog: [],
         in_progress: [],
         ai_review: [],
@@ -295,13 +296,13 @@
       // Mock localStorage.getItem to throw
       const originalGetItem = localStorage.getItem;
       localStorage.getItem = vi.fn(() => {
         throw new Error('Storage quota exceeded');
       });
 
       useTaskStore.getState().loadTaskOrder('project-1');
 
       // Should fall back to empty order state
       expect(useTaskStore.getState().taskOrder).toEqual({
         backlog: [],
         in_progress: [],
         ai_review: [],
@@ -495,7 +496,8 @@
         ai_review: [],
         human_review: [],
         pr_created: [],
-        done: []
+        done: [],
+        stopped: []
       } as TaskOrderState;
       useTaskStore.setState({ taskOrder: order });
 
@@ -586,7 +588,7 @@
       useTaskStore.getState().loadTaskOrder('project-1');
 
       // Empty string causes JSON.parse to throw - should fall back to empty order
       expect(useTaskStore.getState().taskOrder).toEqual({
         backlog: [],
         in_progress: [],
         ai_review: [],

apps/frontend/src/renderer/components/KanbanBoard.tsx

@@ -43,10 +43,13 @@ function isValidDropColumn(id: string): id is typeof TASK_STATUS_COLUMNS[number]
 /**
  * Get the visual column for a task status.
  * pr_created tasks are displayed in the 'done' column, so we map them accordingly.
+ * stopped tasks are displayed in the 'backlog' column.
  * This is used to compare visual positions during drag-and-drop operations.
  */
 function getVisualColumn(status: TaskStatus): typeof TASK_STATUS_COLUMNS[number] {
-  return status === 'pr_created' ? 'done' : status;
+  if (status === 'pr_created') return 'done';
+  if (status === 'stopped') return 'backlog';
+  return status;
 }
 
 interface KanbanBoardProps {
@@ -478,6 +481,7 @@ export function KanbanBoard({ tasks, onTaskClick, onNewTaskClick, onRefresh, isR
 
   const tasksByStatus = useMemo(() => {
     // Note: pr_created tasks are shown in the 'done' column since they're essentially complete
+    // Note: stopped tasks are shown in the 'backlog' column
     const grouped: Record<typeof TASK_STATUS_COLUMNS[number], Task[]> = {
       backlog: [],
       in_progress: [],
@@ -487,8 +491,8 @@ export function KanbanBoard({ tasks, onTaskClick, onNewTaskClick, onRefresh, isR
     };
 
     filteredTasks.forEach((task) => {
-      // Map pr_created tasks to the done column
-      const targetColumn = task.status === 'pr_created' ? 'done' : task.status;
+      // Map pr_created tasks to the done column, stopped tasks to backlog
+      const targetColumn = getVisualColumn(task.status);
       if (grouped[targetColumn]) {
         grouped[targetColumn].push(task);
       }
@@ -741,7 +745,8 @@ export function KanbanBoard({ tasks, onTaskClick, onNewTaskClick, onRefresh, isR
       ai_review: [],
       human_review: [],
       pr_created: [],
-      done: []
+      done: [],
+      stopped: []
     };
 
     for (const status of Object.keys(taskOrder) as Array<keyof typeof taskOrder>) {