fix: add safeguards to prevent /plan from executing without a complete plan

The AI could transition to "executing" status without writing phases,
commands, or context to MEMORY.md, causing empty plan execution.

Adds multiple defense layers:
- Block non-read/non-MEMORY.md tool calls during interview and review
- Cap LLM iterations to 3 during pre-execution phases
- Validate phases exist before /plan start allows transition
- Auto-revert to interviewing if executing with no phases
- Hijack interviewing→executing transition to "review" status,
  showing the plan to the user for approval via /plan start
- Inject staleness nudge after 2 consecutive turns without MEMORY.md update
- Add GetReviewContext() for review-phase-only system prompt injection

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

pkg/agent/context.go

@@ -83,8 +83,8 @@ Your workspace is at: %s
 3. **Memory & Plans**
    - Use memory/MEMORY.md for structured plans.
    - If Status is "interviewing": Ask clarifying questions.
-     Update Context with answers via edit_file.
+     After each answer, use edit_file to save findings to ## Context in memory/MEMORY.md.
-     When ready, organize into Phases and set Status to "executing".
+     When you have enough information, write ## Phase and ## Commands sections into MEMORY.md, then set Status to "executing".
    - If Status is "executing": Work through the current Phase's steps.
      Mark each [x] via edit_file. The system will auto-advance phases.
    - Plan format:
@@ -339,6 +339,11 @@ func (cb *ContextBuilder) GetCurrentPhase() int {
 	return cb.memory.GetCurrentPhase()
 }
 
+// GetTotalPhases returns the total number of phases in the plan.
+func (cb *ContextBuilder) GetTotalPhases() int {
+	return cb.memory.GetTotalPhases()
+}
+
 // FormatPlanDisplay returns a user-facing display of the full plan.
 func (cb *ContextBuilder) FormatPlanDisplay() string {
 	return cb.memory.FormatPlanDisplay()

pkg/agent/instance.go

@@ -30,6 +30,10 @@ type AgentInstance struct {
 	Subagents      *config.SubagentsConfig
 	SkillsFilter   []string
 	Candidates     []providers.FallbackCandidate
+
+	// Interview staleness tracking: consecutive turns where MEMORY.md was not updated.
+	interviewStaleCount  int
+	interviewMemoryLen   int
 }
 
 // NewAgentInstance creates an agent instance from config.

pkg/agent/loop.go

@@ -492,6 +492,23 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
 		opts.ChatID,
 	)
 
+	// 2b. Interview staleness nudge: if MEMORY.md hasn't been updated for
+	// several consecutive turns, inject a reminder so the AI writes its findings.
+	const interviewStaleThreshold = 2
+	if agent.ContextBuilder.GetPlanStatus() == "interviewing" && agent.interviewStaleCount >= interviewStaleThreshold {
+		messages = append(messages, providers.Message{
+			Role:    "user",
+			Content: "[System] You have been interviewing for several turns without updating memory/MEMORY.md. Please use edit_file now to save your findings to the ## Context section, or organize the plan into Phases if you have enough information.",
+		})
+	}
+
+	// 2c. Snapshot plan status and MEMORY.md size before LLM iteration.
+	preStatus := agent.ContextBuilder.GetPlanStatus()
+	var preMemoryLen int
+	if preStatus == "interviewing" {
+		preMemoryLen = len(agent.ContextBuilder.ReadMemory())
+	}
+
 	// 3. Save user message to session (use compact form if available)
 	historyMsg := opts.UserMessage
 	if opts.HistoryMessage != "" {
@@ -514,8 +531,33 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
 	// This is controlled by the tool's Silent flag and ForUser content
 
 	// 5a. Auto-advance plan phases after LLM iteration
-	if agent.ContextBuilder.HasActivePlan() && agent.ContextBuilder.GetPlanStatus() == "executing" {
+	postStatus := agent.ContextBuilder.GetPlanStatus()
-		if agent.ContextBuilder.IsPlanComplete() {
+	if agent.ContextBuilder.HasActivePlan() && postStatus == "executing" {
+		// Intercept: if AI changed status from interviewing to executing,
+		// hijack to "review" and show the plan for user approval.
+		if preStatus == "interviewing" {
+			if agent.ContextBuilder.GetTotalPhases() == 0 {
+				_ = agent.ContextBuilder.SetPlanStatus("interviewing")
+				logger.WarnCF("agent", "Reverted plan to interviewing: no phases defined",
+					map[string]interface{}{"agent_id": agent.ID})
+			} else {
+				_ = agent.ContextBuilder.SetPlanStatus("review")
+				if !constants.IsInternalChannel(opts.Channel) {
+					planDisplay := agent.ContextBuilder.FormatPlanDisplay()
+					al.bus.PublishOutbound(bus.OutboundMessage{
+						Channel:         opts.Channel,
+						ChatID:          opts.ChatID,
+						Content:         planDisplay + "\n\nUse /plan start to approve, or continue chatting to refine.",
+						SkipPlaceholder: true,
+					})
+				}
+			}
+		} else if agent.ContextBuilder.GetTotalPhases() == 0 {
+			// Safeguard: executing but no phases (shouldn't happen, but be safe).
+			_ = agent.ContextBuilder.SetPlanStatus("interviewing")
+			logger.WarnCF("agent", "Reverted plan to interviewing: no phases defined",
+				map[string]interface{}{"agent_id": agent.ID})
+		} else if agent.ContextBuilder.IsPlanComplete() {
 			_ = agent.ContextBuilder.ClearMemory()
 			if !constants.IsInternalChannel(opts.Channel) {
 				al.bus.PublishOutbound(bus.OutboundMessage{
@@ -540,7 +582,21 @@ func (al *AgentLoop) runAgentLoop(ctx context.Context, agent *AgentInstance, opt
 		}
 	}
 
-	// 5b. Handle empty response
+	// 5b. Interview staleness detection: compare MEMORY.md size after iteration.
+	if agent.ContextBuilder.GetPlanStatus() == "interviewing" {
+		postMemoryLen := len(agent.ContextBuilder.ReadMemory())
+		if postMemoryLen == preMemoryLen {
+			agent.interviewStaleCount++
+		} else {
+			agent.interviewStaleCount = 0
+		}
+		agent.interviewMemoryLen = postMemoryLen
+	} else {
+		// Reset counter when not interviewing.
+		agent.interviewStaleCount = 0
+	}
+
+	// 5c. Handle empty response
 	if finalContent == "" {
 		finalContent = opts.DefaultResponse
 	}
@@ -616,14 +672,24 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, agent *AgentInstance,
 	var finalContent string
 	lastReminderIdx := -1
 
-	for iteration < agent.MaxIterations {
+	// During pre-execution plan modes (interviewing/review), cap iterations
+	// to prevent runaway tool loops.
+	maxIter := agent.MaxIterations
+	if isPlanPreExecution(agent.ContextBuilder.GetPlanStatus()) {
+		const interviewMaxIter = 3
+		if maxIter > interviewMaxIter {
+			maxIter = interviewMaxIter
+		}
+	}
+
+	for iteration < maxIter {
 		iteration++
 
 		logger.DebugCF("agent", "LLM iteration",
 			map[string]interface{}{
 				"agent_id":  agent.ID,
 				"iteration": iteration,
-				"max":       agent.MaxIterations,
+				"max":       maxIter,
 			})
 
 		// Build tool definitions
@@ -770,7 +836,7 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, agent *AgentInstance,
 			al.bus.PublishOutbound(bus.OutboundMessage{
 				Channel:  opts.Channel,
 				ChatID:   opts.ChatID,
-				Content:  fmt.Sprintf("🔧 %s (%d/%d)", strings.Join(toolNames, ", "), iteration, agent.MaxIterations),
+				Content:  fmt.Sprintf("🔧 %s (%d/%d)", strings.Join(toolNames, ", "), iteration, maxIter),
 				IsStatus: true,
 			})
 		}
@@ -825,7 +891,14 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, agent *AgentInstance,
 				}
 			}
 
-			toolResult := agent.Tools.ExecuteWithContext(ctx, tc.Name, tc.Arguments, opts.Channel, opts.ChatID, asyncCallback)
+			// Block non-allowed tools during plan interview mode.
+			// Only read-type tools and MEMORY.md writes are permitted.
+			var toolResult *tools.ToolResult
+			if isPlanPreExecution(agent.ContextBuilder.GetPlanStatus()) && !isToolAllowedDuringInterview(tc.Name, tc.Arguments) {
+				toolResult = tools.ErrorResult("Interview mode: only read tools and MEMORY.md edits are allowed. Focus on asking questions and updating the plan.")
+			} else {
+				toolResult = agent.Tools.ExecuteWithContext(ctx, tc.Name, tc.Arguments, opts.Channel, opts.ChatID, asyncCallback)
+			}
 
 			// Send ForUser content to user immediately if not Silent
 			if !toolResult.Silent && toolResult.ForUser != "" && opts.SendResponse {
@@ -884,7 +957,7 @@ func (al *AgentLoop) runLLMIteration(ctx context.Context, agent *AgentInstance,
 
 	// If max iterations exhausted with tool calls still pending,
 	// make one final LLM call without tools to force a text response.
-	if finalContent == "" && iteration >= agent.MaxIterations {
+	if finalContent == "" && iteration >= maxIter {
 		logger.WarnCF("agent", "Max iterations reached, forcing final response without tools",
 			map[string]interface{}{
 				"agent_id":  agent.ID,
@@ -1465,13 +1538,20 @@ func (al *AgentLoop) handlePlanCommand(args []string) (string, bool) {
 		if !agent.ContextBuilder.HasActivePlan() {
 			return "No active plan.", true
 		}
-		if agent.ContextBuilder.GetPlanStatus() != "interviewing" {
+		status := agent.ContextBuilder.GetPlanStatus()
+		if status == "executing" {
 			return "Plan is already executing.", true
 		}
+		if status != "interviewing" && status != "review" {
+			return fmt.Sprintf("Cannot start from status %q.", status), true
+		}
+		if agent.ContextBuilder.GetTotalPhases() == 0 {
+			return "Cannot start: no phases defined yet. Complete the interview first.", true
+		}
 		if err := agent.ContextBuilder.SetPlanStatus("executing"); err != nil {
 			return fmt.Sprintf("Error: %v", err), true
 		}
-		return "Plan status changed to executing.", true
+		return "Plan approved. Executing.", true
 
 	case "next":
 		if !agent.ContextBuilder.HasActivePlan() {
@@ -1495,6 +1575,32 @@ func (al *AgentLoop) handlePlanCommand(args []string) (string, bool) {
 	}
 }
 
+// isPlanPreExecution returns true if the plan is in a pre-execution state
+// (interviewing or review) where tool restrictions and iteration caps apply.
+func isPlanPreExecution(status string) bool {
+	return status == "interviewing" || status == "review"
+}
+
+// isToolAllowedDuringInterview checks whether a tool call is permitted while the
+// plan is in a pre-execution state. Read-type tools are always allowed. Write-type
+// tools (edit_file, append_file, write_file) are only allowed when targeting MEMORY.md.
+func isToolAllowedDuringInterview(toolName string, args map[string]interface{}) bool {
+	// Read-type tools: always allowed
+	switch toolName {
+	case "read_file", "list_dir", "web_search", "web_fetch":
+		return true
+	}
+
+	// Write-type tools: allowed only when targeting MEMORY.md
+	switch toolName {
+	case "edit_file", "append_file", "write_file":
+		path, _ := args["path"].(string)
+		return strings.HasSuffix(path, "MEMORY.md")
+	}
+
+	return false
+}
+
 // expandPlanCommand detects "/plan <task>" (new plan start) and:
 //   - writes the interview seed to MEMORY.md
 //   - rewrites the message content for the LLM

pkg/agent/loop_test.go

@@ -995,18 +995,60 @@ func TestPlanCommand_Start(t *testing.T) {
 	al, cleanup := newTestAgentLoop(t)
 	defer cleanup()
 
-	// Create interviewing plan
+	agent := al.registry.GetDefaultAgent()
+
+	// Create interviewing plan with phases (start requires phases)
+	plan := "# Active Plan\n\n> Task: Test task\n> Status: interviewing\n> Phase: 1\n\n## Phase 1: Setup\n- [ ] Step one\n\n## Context\n"
+	_ = agent.ContextBuilder.WriteMemory(plan)
+
+	// Transition to executing via /plan start
+	response, _ := al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan start"})
+	if !strings.Contains(response, "approved") {
+		t.Errorf("expected 'approved', got %q", response)
+	}
+
+	if status := agent.ContextBuilder.GetPlanStatus(); status != "executing" {
+		t.Errorf("expected 'executing', got %q", status)
+	}
+}
+
+func TestPlanCommand_StartFromReview(t *testing.T) {
+	al, cleanup := newTestAgentLoop(t)
+	defer cleanup()
+
+	agent := al.registry.GetDefaultAgent()
+
+	// Create a plan in review status
+	plan := "# Active Plan\n\n> Task: Test task\n> Status: review\n> Phase: 1\n\n## Phase 1: Setup\n- [ ] Step one\n\n## Context\n"
+	_ = agent.ContextBuilder.WriteMemory(plan)
+
+	// Approve via /plan start
+	response, _ := al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan start"})
+	if !strings.Contains(response, "approved") {
+		t.Errorf("expected 'approved', got %q", response)
+	}
+
+	if status := agent.ContextBuilder.GetPlanStatus(); status != "executing" {
+		t.Errorf("expected 'executing', got %q", status)
+	}
+}
+
+func TestPlanCommand_StartNoPhases(t *testing.T) {
+	al, cleanup := newTestAgentLoop(t)
+	defer cleanup()
+
+	// Create interviewing plan without phases
 	al.expandPlanCommand(bus.InboundMessage{Content: "/plan Test task"})
 
-	// Transition to executing
+	// Should be blocked because no phases exist
 	response, _ := al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan start"})
-	if !strings.Contains(response, "executing") {
+	if !strings.Contains(response, "no phases") {
-		t.Errorf("expected 'executing', got %q", response)
+		t.Errorf("expected 'no phases' error, got %q", response)
 	}
 
 	agent := al.registry.GetDefaultAgent()
-	if status := agent.ContextBuilder.GetPlanStatus(); status != "executing" {
+	if status := agent.ContextBuilder.GetPlanStatus(); status != "interviewing" {
-		t.Errorf("expected 'executing', got %q", status)
+		t.Errorf("expected status to remain 'interviewing', got %q", status)
 	}
 }
 
@@ -1014,8 +1056,11 @@ func TestPlanCommand_StartAlreadyExecuting(t *testing.T) {
 	al, cleanup := newTestAgentLoop(t)
 	defer cleanup()
 
-	// Create interviewing plan then start
+	agent := al.registry.GetDefaultAgent()
-	al.expandPlanCommand(bus.InboundMessage{Content: "/plan Test task"})
+
+	// Create interviewing plan with phases, then start
+	plan := "# Active Plan\n\n> Task: Test task\n> Status: interviewing\n> Phase: 1\n\n## Phase 1: Setup\n- [ ] Step one\n\n## Context\n"
+	_ = agent.ContextBuilder.WriteMemory(plan)
 	al.handleCommand(context.Background(), bus.InboundMessage{Content: "/plan start"})
 
 	// Try start again

pkg/agent/memory.go

@@ -375,7 +375,11 @@ func (ms *MemoryStore) GetInterviewContext() string {
 	sb.WriteString("- Environment (OS, language, runtime versions)\n")
 	sb.WriteString("- Tooling preferences (test framework, linter, formatter, CI)\n")
 	sb.WriteString("- Key commands the user already runs (build, test, deploy)\n")
-	sb.WriteString("When ready, organize into 2-5 phases with 3-5 steps each.\n")
+	sb.WriteString("\n### Rules\n")
+	sb.WriteString("- After each answer, use edit_file to append findings to the ## Context section of memory/MEMORY.md.\n")
+	sb.WriteString("- When you have enough information, use edit_file to write ## Phase, ## Commands, and ## Context sections into memory/MEMORY.md.\n")
+	sb.WriteString("- Organize into 2-5 phases with 3-5 steps each.\n")
+	sb.WriteString("- After writing Phases, set Status to executing. The system will handle the rest.\n")
 	sb.WriteString("\n### Target Format\n")
 	sb.WriteString("```\n")
 	sb.WriteString("## Phase 1: <title>\n")
@@ -392,6 +396,20 @@ func (ms *MemoryStore) GetInterviewContext() string {
 	return sb.String()
 }
 
+// GetReviewContext returns context for injection during the review phase.
+// Shows the full plan and instructs the AI to wait for user approval.
+func (ms *MemoryStore) GetReviewContext() string {
+	content := ms.ReadLongTerm()
+
+	var sb strings.Builder
+	sb.WriteString("## Active Plan (awaiting approval)\n\n")
+	sb.WriteString(content)
+	sb.WriteString("\n\nThe plan is awaiting user approval.\n")
+	sb.WriteString("- If the user requests changes, update memory/MEMORY.md via edit_file.\n")
+	sb.WriteString("- Do NOT change Status yourself. The user will run /plan start to approve.\n")
+	return sb.String()
+}
+
 // GetPlanContext returns context for injection during the executing phase.
 // Only the current phase is shown in detail; completed phases are compressed
 // to one-line summaries; future phases are omitted.
@@ -572,9 +590,12 @@ func (ms *MemoryStore) GetMemoryContext() string {
 	if longTerm != "" {
 		if ms.HasActivePlan() {
 			status := ms.GetPlanStatus()
-			if status == "interviewing" {
+			switch status {
+			case "interviewing":
 				parts = append(parts, ms.GetInterviewContext())
-			} else {
+			case "review":
+				parts = append(parts, ms.GetReviewContext())
+			default:
 				parts = append(parts, ms.GetPlanContext())
 			}
 		} else {