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,15 @@ 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,52 @@ 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, renameSync, unlinkSync } 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";
@@ -100,6 +100,26 @@ function validateStatusTransition(
   return true;
 }
 
+/**
+ * Atomic file write to prevent corruption on crash/interrupt.
+ * Writes to a temporary file first, then atomically renames to target.
+ */
+function atomicWriteFileSync(filePath: string, content: string): void {
+  const tempPath = `${filePath}.${process.pid}.tmp`;
+  try {
+    writeFileSync(tempPath, content, "utf-8");
+    renameSync(tempPath, filePath);
+  } catch (error) {
+    // Clean up temp file if rename failed
+    try {
+      unlinkSync(tempPath);
+    } catch {
+      // Ignore cleanup errors
+    }
+    throw error;
+  }
+}
+
 /**
  * Register all agent-events-related IPC handlers
  */
@@ -135,7 +155,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 +173,170 @@ 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
+          // Also persist to worktree if it exists, since getTasks() prefers worktree version
+          const worktreePath = findTaskWorktree(exitProject.path, exitTask.specId);
+          const worktreeSpecDir = worktreePath ? path.join(worktreePath, specsBaseDir, exitTask.specId) : null;
+
+          // Prepare updated content
+          metadata.stoppedForPlanReview = true;
+          planContent.status = "stopped";
+          planContent.planStatus = "awaiting_review";
+
+          const metadataJson = JSON.stringify(metadata, null, 2);
+          const planJson = JSON.stringify(planContent, null, 2);
+
+          // Write to main project (atomic write to prevent corruption)
+          try {
+            atomicWriteFileSync(metadataPath, metadataJson);
+          } catch (err) {
+            console.error(`[Task ${taskId}] Failed to update task_metadata.json:`, err);
+          }
+
+          try {
+            atomicWriteFileSync(planPath, planJson);
+          } catch (err) {
+            console.error(`[Task ${taskId}] Failed to update implementation_plan.json:`, err);
+          }
+
+          // Also write to worktree if it exists (for consistency with getTasks())
+          if (worktreeSpecDir) {
+            const worktreeMetadataPath = path.join(worktreeSpecDir, "task_metadata.json");
+            const worktreePlanPath = path.join(worktreeSpecDir, AUTO_BUILD_PATHS.IMPLEMENTATION_PLAN);
+
+            try {
+              if (existsSync(worktreeMetadataPath)) {
+                atomicWriteFileSync(worktreeMetadataPath, metadataJson);
+              }
+            } catch (err) {
+              console.error(`[Task ${taskId}] Failed to update worktree task_metadata.json:`, err);
+            }
+
+            try {
+              if (existsSync(worktreePlanPath)) {
+                atomicWriteFileSync(worktreePlanPath, planJson);
+              }
+            } catch (err) {
+              console.error(`[Task ${taskId}] Failed to update worktree 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
+          // First, update the plan file status to in_progress so UI stays consistent
+          planContent.status = "in_progress";
+          planContent.planStatus = "in_progress";
+
+          const planJson = JSON.stringify(planContent, null, 2);
+
+          // Write to main project (atomic write to prevent corruption)
+          try {
+            atomicWriteFileSync(planPath, planJson);
+          } catch (err) {
+            console.error(`[Task ${taskId}] Failed to update implementation_plan.json for coding transition:`, err);
+          }
+
+          // Also write to worktree if it exists (for consistency with getTasks())
+          const worktreePath = findTaskWorktree(exitProject.path, exitTask.specId);
+          if (worktreePath) {
+            const worktreePlanPath = path.join(worktreePath, specsBaseDir, exitTask.specId, AUTO_BUILD_PATHS.IMPLEMENTATION_PLAN);
+            try {
+              if (existsSync(worktreePlanPath)) {
+                atomicWriteFileSync(worktreePlanPath, planJson);
+              }
+            } catch (err) {
+              console.error(`[Task ${taskId}] Failed to update worktree implementation_plan.json for coding transition:`, err);
+            }
+          }
+
+          // Invalidate cache so UI reflects the new status
+          projectStore.invalidateTasksCache(exitProject.id);
+
+          // Start 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
+            }
+          );
+
+          // Notify renderer that task is continuing to coding phase
+          // This ensures UI shows in_progress status immediately
+          safeSendToRenderer(
+            getMainWindow,
+            IPC_CHANNELS.TASK_STATUS_CHANGE,
+            taskId,
+            "in_progress" as TaskStatus,
+            exitProjectId
+          );
+
+          return;
+        }
+      }
     }
 
     let task: Task | undefined;