fix(agent): prevent plan execution from stalling after update_plan

internal/agent/completion_gate_test.go

@@ -2,6 +2,7 @@ package agent
 
 import (
 	"context"
+	"errors"
 	"strings"
 	"testing"
 
@@ -126,9 +127,107 @@ func TestCompletionGateContinuesOnCueThenSucceeds(t *testing.T) {
 	}
 }
 
-// With the gate OFF (the interactive/TUI default), a continuation-cue turn is
-// accepted as the final answer exactly as before — guaranteeing no behavior
-// change for non-headless callers.
+// Issue #666: the plan-aware gate applies even when RequireCompletionSignal is
+// off (interactive/TUI). A model that creates a plan then stalls on a
+// continuation-cue text turn must be re-prompted and eventually INCOMPLETE, not
+// accepted as success while the plan panel stays on step one.
+func TestPlanPendingGateAppliesWithoutRequireCompletionSignal(t *testing.T) {
+	cue := "Now I need to configure the SSH server. Let me check the current SSH configuration:"
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewUpdatePlanTool())
+
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
+		planTurn("in_progress", "pending", "pending"),
+		textTurn(cue), textTurn(cue), textTurn(cue), textTurn(cue), textTurn(cue),
+	}}
+
+	result, err := Run(context.Background(), "set up a git server", provider, Options{
+		Registry: registry,
+		MaxTurns: 10,
+		// RequireCompletionSignal deliberately off — interactive/TUI default.
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !result.Incomplete {
+		t.Fatalf("expected Incomplete=true (plan stalled without headless gate), got false; final=%q turns=%d", result.FinalAnswer, result.Turns)
+	}
+	if len(provider.requests) != 1+maxContinueNudges+1 {
+		t.Fatalf("expected %d provider turns (1 plan + %d nudges + 1 final), got %d",
+			1+maxContinueNudges+1, maxContinueNudges, len(provider.requests))
+	}
+	if !someRequestContains(provider.requests, continueNudgeMarker) {
+		t.Fatalf("expected a continue nudge (%q) to be injected", continueNudgeMarker)
+	}
+}
+
+// A pending plan must nudge non-colon continuation announcements too — not only
+// colon-terminated cues — so the first mid-step text turn cannot finalize while
+// steps remain. A later clean final answer is accepted after the bounded nudges.
+func TestPlanPendingNudgesNonColonContinuationText(t *testing.T) {
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewUpdatePlanTool())
+	midStep := "Let me inspect the configuration."
+	final := "Provider profiles are loaded from config and merged with defaults."
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
+		planTurn("in_progress", "pending", "pending"),
+		textTurn(midStep),
+		textTurn(final), textTurn(final), textTurn(final), textTurn(final),
+	}}
+
+	result, err := Run(context.Background(), "inspect and configure", provider, Options{
+		Registry: registry,
+		MaxTurns: 10,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if result.Incomplete {
+		t.Fatalf("expected success after nudge + final answer, got Incomplete (%q)", result.IncompleteReason)
+	}
+	if result.FinalAnswer != final {
+		t.Fatalf("final answer = %q, want %q", result.FinalAnswer, final)
+	}
+	if len(provider.requests) != 1+1+maxContinueNudges {
+		t.Fatalf("expected %d turns (plan + mid-step + %d nudged finals), got %d",
+			1+1+maxContinueNudges, maxContinueNudges, len(provider.requests))
+	}
+	if !someRequestContains(provider.requests, continueNudgeMarker) {
+		t.Fatalf("expected a continue nudge after the mid-step text with pending plan")
+	}
+}
+
+// After bounded nudges, a confident final answer with stale plan bookkeeping must
+// still succeed on the interactive/TUI path (no infinite loop, no false incomplete).
+func TestPlanPendingAcceptsFinalAnswerAfterBoundedNudgesWithoutHeadlessGate(t *testing.T) {
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewUpdatePlanTool())
+	done := "All provider profiles are documented above."
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
+		planTurn("in_progress", "pending"),
+		textTurn(done), textTurn(done), textTurn(done), textTurn(done),
+	}}
+
+	result, err := Run(context.Background(), "explain provider loading", provider, Options{
+		Registry: registry,
+		MaxTurns: 10,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if result.Incomplete {
+		t.Fatalf("stale plan alone must not force Incomplete on TUI path; reason=%q", result.IncompleteReason)
+	}
+	if result.FinalAnswer != done {
+		t.Fatalf("final answer = %q, want %q", result.FinalAnswer, done)
+	}
+	if !someRequestContains(provider.requests, continueNudgeMarker) {
+		t.Fatalf("expected at least one continue nudge before accepting completion")
+	}
+}
+
+// With the gate OFF and NO pending plan, a continuation-cue turn is still
+// accepted as the final answer — no behavior change for short single-step tasks.
 func TestCompletionGateOffPreservesLegacyBehavior(t *testing.T) {
 	cue := "Let me check the config:"
 	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
@@ -177,6 +276,44 @@ func TestContinuationCueMatching(t *testing.T) {
 	}
 }
 
+// Cancellation during a plan-pending stall must return immediately without extra
+// nudges or incomplete synthesis.
+func TestRunCancellationDuringPlanPendingStopsImmediately(t *testing.T) {
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewUpdatePlanTool())
+	ctx, cancel := context.WithCancel(context.Background())
+	_, err := Run(ctx, "do work", cancelMidStreamProvider{cancel: cancel}, Options{
+		Registry: registry,
+	})
+	if !errors.Is(err, context.Canceled) {
+		t.Fatalf("expected context.Canceled, got %v", err)
+	}
+}
+
+// Repeated identical update_plan calls must still hit the max-turns ceiling rather
+// than looping silently forever.
+func TestRunRepeatedIdenticalUpdatePlanHitsMaxTurns(t *testing.T) {
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewUpdatePlanTool())
+	same := toolTurn("plan", "update_plan", `{"plan":[{"content":"step","status":"in_progress"}]}`)
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{same, same, same, textTurn("halted summary")}}
+
+	result, err := Run(context.Background(), "keep planning", provider, Options{
+		Registry: registry,
+		MaxTurns: 3,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	// MaxTurns=3 tool turns, then finalAnswerAfterMaxTurns issues one more request.
+	if len(provider.requests) != 4 {
+		t.Fatalf("expected 3 tool turns + 1 max-turns final-answer call, got %d", len(provider.requests))
+	}
+	if result.FinalAnswer == "" {
+		t.Fatal("expected a max-turns final answer")
+	}
+}
+
 // review #4: a run that loops to the MaxTurns ceiling (always calling a tool, so it
 // never reaches the no-tool-call gate) was reported as success. Under the headless
 // gate, a max-turns cutoff is INCOMPLETE — the agent was stopped mid-run, not done.

internal/agent/guardrails.go

@@ -508,9 +508,19 @@ func (state *guardState) observeTurn(collected zeroruntime.CollectedStream) (sto
 	hasVisibleText := strings.TrimSpace(collected.Text) != ""
 	hasReasoning := collected.HasReasoning || len(collected.ReasoningBlocks) > 0
 
-	if hasToolCalls || hasVisibleText || hasReasoning {
+	productive := hasToolCalls || hasVisibleText
+	switch {
+	case productive:
 		state.emptyTurns = 0
-	} else {
+	case hasReasoning && state.planItemsPending > 0:
+		// Reasoning alone does not prove progress while plan steps remain — weaker
+		// models (e.g. HY3 via OpenAI-compatible gateways) can stream long
+		// reasoning-only turns without tools or visible text, burning the turn
+		// budget while the plan panel stays on the first in_progress step.
+		state.emptyTurns++
+	case hasReasoning:
+		state.emptyTurns = 0
+	default:
 		state.emptyTurns++
 	}
 	if hasToolCalls && !hasVisibleText {

internal/agent/guardrails_test.go

@@ -30,6 +30,16 @@ func reasoningTurn(content string) []zeroruntime.StreamEvent {
 	}
 }
 
+// multiReasoningTurn streams several reasoning chunks inside ONE provider turn.
+func multiReasoningTurn(chunks ...string) []zeroruntime.StreamEvent {
+	events := make([]zeroruntime.StreamEvent, 0, len(chunks)+1)
+	for _, chunk := range chunks {
+		events = append(events, zeroruntime.StreamEvent{Type: zeroruntime.StreamEventReasoning, Content: chunk})
+	}
+	events = append(events, zeroruntime.StreamEvent{Type: zeroruntime.StreamEventDone})
+	return events
+}
+
 // toolTurn produces a turn that calls a named tool with the given args JSON.
 func toolTurn(callID string, toolName string, args string) []zeroruntime.StreamEvent {
 	return []zeroruntime.StreamEvent{
@@ -109,6 +119,95 @@ func TestRunResetsEmptyTurnCounterOnVisibleOutput(t *testing.T) {
 	}
 }
 
+func TestRunStopsReasoningOnlyTurnsWhilePlanPending(t *testing.T) {
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewUpdatePlanTool())
+
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
+		toolTurn("plan", "update_plan", `{"plan":[{"content":"step 1","status":"in_progress"},{"content":"step 2","status":"pending"}]}`),
+		reasoningTurn("thinking 1"),
+		reasoningTurn("thinking 2"),
+		reasoningTurn("thinking 3"),
+		textTurn("should never reach here"),
+	}}
+
+	result, err := Run(context.Background(), "multi-step task", provider, Options{
+		Registry: registry,
+		MaxTurns: 12,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(provider.requests) != maxEmptyTurns+1 {
+		t.Fatalf("expected %d turns (1 plan + %d reasoning-only stalls), got %d", maxEmptyTurns+1, maxEmptyTurns, len(provider.requests))
+	}
+	if !result.Incomplete {
+		t.Fatalf("expected Incomplete=true after reasoning-only stall with pending plan, got false")
+	}
+	if !strings.Contains(result.FinalAnswer, noOutputStopMarker) {
+		t.Fatalf("expected no-output stop answer, got %q", result.FinalAnswer)
+	}
+}
+
+func TestRunMultiChunkReasoningCountsAsOneNonProductiveTurnWhilePlanPending(t *testing.T) {
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewUpdatePlanTool())
+
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
+		toolTurn("plan", "update_plan", `{"plan":[{"content":"step 1","status":"in_progress"},{"content":"step 2","status":"pending"}]}`),
+		multiReasoningTurn("chunk-1 ", "chunk-2 ", "chunk-3 ", "chunk-4 ", "chunk-5 "),
+		multiReasoningTurn("more-1 ", "more-2 ", "more-3 "),
+		multiReasoningTurn("final-1 ", "final-2 "),
+		textTurn("should never reach here"),
+	}}
+
+	result, err := Run(context.Background(), "multi-step task", provider, Options{
+		Registry: registry,
+		MaxTurns: 12,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(provider.requests) != maxEmptyTurns+1 {
+		t.Fatalf("expected %d turns (1 plan + %d multi-chunk reasoning stalls), got %d",
+			maxEmptyTurns+1, maxEmptyTurns, len(provider.requests))
+	}
+	if !result.Incomplete {
+		t.Fatalf("expected Incomplete=true, got false")
+	}
+}
+
+func TestRunReasoningThenToolCallSucceedsWithPendingPlan(t *testing.T) {
+	root := t.TempDir()
+	writeAgentTestFile(t, root+"/notes.txt", "alpha")
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewUpdatePlanTool())
+
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
+		toolTurn("plan", "update_plan", `{"plan":[{"content":"step 1","status":"in_progress"},{"content":"step 2","status":"pending"}]}`),
+		multiReasoningTurn("thinking ", "more thinking "),
+		toolTurn("read", "read_file", `{"path":"notes.txt"}`),
+		toolTurn("plan2", "update_plan", `{"plan":[{"content":"step 1","status":"completed"},{"content":"step 2","status":"completed"}]}`),
+		textTurn("Both steps are done."),
+	}}
+
+	result, err := Run(context.Background(), "multi-step task", provider, Options{
+		Registry: registry,
+		MaxTurns: 12,
+		Cwd:      root,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if result.Incomplete {
+		t.Fatalf("run should succeed after productive tool call, got Incomplete (%q)", result.IncompleteReason)
+	}
+	if result.FinalAnswer != "Both steps are done." {
+		t.Fatalf("final answer = %q", result.FinalAnswer)
+	}
+}
+
 func TestRunResetsEmptyTurnCounterOnReasoning(t *testing.T) {
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
@@ -293,7 +392,7 @@ func TestRunDoesNotInjectNotCalledReminderWhenPlanUsed(t *testing.T) {
 
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
-			toolTurn("call-1", "update_plan", `{"plan":[{"content":"step one"}]}`),
+			toolTurn("call-1", "update_plan", `{"plan":[{"content":"step one","status":"completed"}]}`),
 			toolTurn("call-2", "read_file", `{"path":"notes.txt"}`),
 			textTurn("done"),
 		},
@@ -329,6 +428,12 @@ func TestRunInjectsStalePlanReminderAfterManyToolCalls(t *testing.T) {
 	for i := 0; i < staleToolCallThreshold+2; i++ {
 		turns = append(turns, toolTurn("call", "read_file", `{"path":"notes.txt"}`))
 	}
+	// The plan-aware completion gate re-prompts up to maxContinueNudges when the
+	// model stops with text while plan items remain; supply those turns before
+	// the final answer.
+	for i := 0; i < maxContinueNudges; i++ {
+		turns = append(turns, textTurn("done."))
+	}
 	turns = append(turns, textTurn("done"))
 
 	provider := &mockProvider{turns: turns}
@@ -363,6 +468,9 @@ func TestRunStalePlanReminderIsOneShotPerInterval(t *testing.T) {
 	for i := 0; i < staleToolCallThreshold*2; i++ {
 		turns = append(turns, toolTurn("call", "read_file", `{"path":"notes.txt"}`))
 	}
+	for i := 0; i < maxContinueNudges; i++ {
+		turns = append(turns, textTurn("done."))
+	}
 	turns = append(turns, textTurn("done"))
 
 	provider := &mockProvider{turns: turns}

internal/agent/loop.go

@@ -416,6 +416,10 @@
 			// counted toward the runaway cap so we stop before burning maxTurns.
 			if guards.observeTurn(collected) {
 				result.FinalAnswer = noOutputStopAnswer(result.Turns)
+				if guards.pendingPlanItems() {
+					result.Incomplete = true
+					result.IncompleteReason = "stopped after non-productive turns while plan items remained"
+				}
 				result.Messages = copyMessages(messages)
 				return result, nil
 			}
@@ -429,10 +433,13 @@
 				})
 				continue
 			}
-			// Completion gate (headless): a turn with text but no tool call is the
-			// model's final answer ONLY when the work is actually done. Default off
-			// (RequireCompletionSignal), so interactive runs stay byte-identical.
-			if options.RequireCompletionSignal {
+			// Completion gate: a turn with text but no tool call is the model's
+			// final answer ONLY when the work is actually done. Headless exec opts in
+			// via RequireCompletionSignal; interactive runs also apply the gate while
+			// update_plan still has pending/in_progress items so a plan cannot stall
+			// silently after the first step is marked active.
+			planPending := guards.pendingPlanItems()
+			if options.RequireCompletionSignal || planPending {
 				// (1) Self-report downgrade (strongest, unambiguous): the model's own
 				// final message admits it guessed / could not meet the objective. Checked
 				// FIRST so an admitted-impossible task is downgraded immediately (no wasted
@@ -455,7 +462,6 @@
 				// false-fail a completed run with stale bookkeeping) — fall through to the
 				// acceptance check / success.
 				cue := endsWithContinuationCue(collected.Text)
-				planPending := guards.pendingPlanItems()
 				if cue || planPending {
 					if continueNudges < maxContinueNudges {
 						continueNudges++
@@ -2614,7 +2620,7 @@
 // through tool_search. Non-deferred tools (including tool_search) are always
 // exposed. The exposed slice is alpha-sorted by name, matching the legacy order
 // so the inactive path is stable.
 func partitionTools(registry *tools.Registry, permissionMode PermissionMode, options Options, loaded map[string]bool) ([]zeroruntime.ToolDefinition, string) {
 	return partitionToolsCached(registry, permissionMode, options, loaded, nil)
 }
 

internal/agent/types.go

@@ -296,13 +296,13 @@
 
 	// RequireCompletionSignal gates run completion for HEADLESS exec. Without it,
 	// any assistant turn that produces text but no tool call is accepted as the
-	// final answer. With it, a no-tool-call turn is NOT treated as "done" while
-	// work clearly remains — pending update_plan items, or a message that ends on a
-	// continuation cue ("…Let me check the config:"). The loop then nudges the
-	// model to continue instead, bounded by maxContinueNudges (and still by
-	// MaxTurns and the run deadline); if the model keeps stalling, the run
-	// finalizes as INCOMPLETE (Result.Incomplete) rather than success. Default
-	// false leaves the loop byte-identical, so the interactive TUI is unaffected.
+	// final answer — except while update_plan still has pending/in_progress items,
+	// when the plan-aware completion gate always applies (interactive and headless).
+	// With RequireCompletionSignal, a no-tool-call turn is also gated on
+	// continuation cues ("…Let me check the config:"). The loop nudges the model to
+	// continue instead, bounded by maxContinueNudges (and still by MaxTurns and the
+	// run deadline); if the model keeps stalling, the run finalizes as INCOMPLETE
+	// (Result.Incomplete) rather than success.
 	RequireCompletionSignal bool
 
 	runPermissions *permissionRunState
@@ -334,7 +334,7 @@
 // Truncated reports whether the final response ended abnormally (cut off at the
 // output token cap or withheld by a content filter) rather than completing
 // naturally. Callers can use it to warn the user that FinalAnswer is incomplete.
 func (result Result) Truncated() bool {
 	return result.FinishReason != ""
 }