Add deterministic completion policy

internal/agent/completion_policy.go

@@ -0,0 +1,86 @@
+package agent
+
+// CompletionDecision is the deterministic outcome of evaluating a text-only
+// assistant turn. Uncertain means the loop needs one bounded follow-up action
+// before it can finalize the run.
+type CompletionDecision string
+
+const (
+	CompletionUncertain  CompletionDecision = "uncertain"
+	CompletionComplete   CompletionDecision = "complete"
+	CompletionIncomplete CompletionDecision = "incomplete"
+)
+
+type completionAction string
+
+const (
+	completionActionNone          completionAction = ""
+	completionActionContinue      completionAction = "continue"
+	completionActionSemanticCheck completionAction = "semantic_check"
+)
+
+type completionEvaluation struct {
+	Decision CompletionDecision
+	Action   completionAction
+	Reason   string
+}
+
+// completionPolicy owns the small amount of state needed to keep uncertain
+// decisions bounded. requireSemanticCheck is set only for run profiles that
+// already opted into self-correction; ordinary completion remains entirely
+// local and adds no model call.
+type completionPolicy struct {
+	continueNudges         int
+	requireSemanticCheck   bool
+	semanticCheckRequested bool
+}
+
+func newCompletionPolicy(requireSemanticCheck bool) *completionPolicy {
+	return &completionPolicy{requireSemanticCheck: requireSemanticCheck}
+}
+
+func (policy *completionPolicy) evaluate(text string, planPending bool) completionEvaluation {
+	// A direct admission is strong local evidence and takes precedence over all
+	// weaker signals, avoiding both continuation nudges and semantic checks.
+	if reason := selfReportedIncompletion(text); reason != "" {
+		return completionEvaluation{Decision: CompletionIncomplete, Reason: reason}
+	}
+
+	// A continuation cue is strong unfinished evidence; a pending plan is only
+	// weak evidence because bookkeeping can be stale. Both get a bounded chance
+	// to continue, but only a persisted cue is ultimately incomplete.
+	cue := endsWithContinuationCue(text)
+	if cue || planPending {
+		if policy.continueNudges < maxContinueNudges {
+			policy.continueNudges++
+			reason := "your message ended mid-step"
+			if !cue {
+				reason = "pending plan items remain — finish them, or mark them complete with update_plan if you are done"
+			}
+			return completionEvaluation{
+				Decision: CompletionUncertain,
+				Action:   completionActionContinue,
+				Reason:   reason,
+			}
+		}
+		if cue {
+			return completionEvaluation{
+				Decision: CompletionIncomplete,
+				Reason:   "your message ended mid-step",
+			}
+		}
+	}
+
+	// Profiles with self-correction enabled require one task-grounded semantic
+	// check. It is requested at most once per run; the next locally complete turn
+	// is accepted without another model call.
+	if policy.requireSemanticCheck && !policy.semanticCheckRequested {
+		policy.semanticCheckRequested = true
+		return completionEvaluation{
+			Decision: CompletionUncertain,
+			Action:   completionActionSemanticCheck,
+		}
+	}
+
+	return completionEvaluation{Decision: CompletionComplete, Action: completionActionNone}
+}

internal/agent/completion_policy_test.go

@@ -0,0 +1,64 @@
+package agent
+
+import "testing"
+
+func TestCompletionPolicyLocalEvidenceDecidesWithoutSemanticCheck(t *testing.T) {
+	policy := newCompletionPolicy(false)
+
+	complete := policy.evaluate("Done. All required checks pass.", false)
+	if complete.Decision != CompletionComplete {
+		t.Fatalf("confident completion decision = %q, want %q", complete.Decision, CompletionComplete)
+	}
+
+	incomplete := policy.evaluate("I couldn't verify the result, so this is my best guess.", false)
+	if incomplete.Decision != CompletionIncomplete {
+		t.Fatalf("admitted failure decision = %q, want %q", incomplete.Decision, CompletionIncomplete)
+	}
+}
+
+func TestCompletionPolicyPreservesBoundedPlanStallProtection(t *testing.T) {
+	policy := newCompletionPolicy(false)
+	for attempt := 0; attempt < maxContinueNudges; attempt++ {
+		got := policy.evaluate("Let me inspect the remaining configuration:", true)
+		if got.Decision != CompletionUncertain || got.Action != completionActionContinue {
+			t.Fatalf("attempt %d = (%q, %q), want uncertain continue", attempt+1, got.Decision, got.Action)
+		}
+	}
+
+	got := policy.evaluate("Let me inspect the remaining configuration:", true)
+	if got.Decision != CompletionIncomplete {
+		t.Fatalf("decision after nudge budget = %q, want %q", got.Decision, CompletionIncomplete)
+	}
+}
+
+func TestCompletionPolicyTreatsPendingPlanAsWeakEvidence(t *testing.T) {
+	policy := newCompletionPolicy(false)
+	for attempt := 0; attempt < maxContinueNudges; attempt++ {
+		got := policy.evaluate("All set.", true)
+		if got.Decision != CompletionUncertain || got.Action != completionActionContinue {
+			t.Fatalf("attempt %d = (%q, %q), want uncertain continue", attempt+1, got.Decision, got.Action)
+		}
+	}
+
+	got := policy.evaluate("All set.", true)
+	if got.Decision != CompletionComplete {
+		t.Fatalf("stale plan decision after nudge budget = %q, want %q", got.Decision, CompletionComplete)
+	}
+}
+
+func TestCompletionPolicyAllowsExactlyOneRequiredSemanticCheck(t *testing.T) {
+	policy := newCompletionPolicy(true)
+
+	first := policy.evaluate("Implemented and tested.", false)
+	if first.Decision != CompletionUncertain || first.Action != completionActionSemanticCheck {
+		t.Fatalf("first decision = (%q, %q), want uncertain semantic check", first.Decision, first.Action)
+	}
+
+	second := policy.evaluate("PASS. The result meets the task criterion.", false)
+	if second.Decision != CompletionComplete {
+		t.Fatalf("post-check decision = %q, want %q", second.Decision, CompletionComplete)
+	}
+	if second.Action == completionActionSemanticCheck {
+		t.Fatal("semantic check was requested more than once")
+	}
+}

internal/agent/loop.go

@@ -170,13 +170,10 @@
 	// loaded tool's full schema; it lives only for the run (v1 within-run scope).
 	loaded := map[string]bool{}
 
-	// continueNudges counts how many times the headless completion gate
-	// (Options.RequireCompletionSignal) has re-prompted a no-tool-call turn that
-	// stopped with work still unfinished. Bounded by maxContinueNudges.
-	continueNudges := 0
-	// acceptanceRequested records that the one-time task-grounded acceptance check
-	// has already been demanded this run, so it fires at most once.
-	acceptanceRequested := false
+	// The feature-gated completion policy keeps local completion decisions and its
+	// bounded follow-up state out of the central loop. Self-correcting profiles
+	// opt into the one permitted task-grounded semantic check.
+	completionPolicy := newCompletionPolicy(options.SelfCorrect != nil)
 
 	// toolDefCache memoizes each tool's rendered JSON-schema definition across
 	// turns (a tool's advertised schema is stable for the run), so partitionTools
@@ -521,68 +518,32 @@
 			// model's final answer ONLY when the work is actually done. Default off
 			// (RequireCompletionSignal), so interactive runs stay byte-identical.
 			if options.RequireCompletionSignal {
-				// (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
-				// continue-nudges) and reports the accurate reason.
-				if reason := selfReportedIncompletion(collected.Text); reason != "" {
+				evaluation := completionPolicy.evaluate(collected.Text, guards.pendingPlanItems())
+				switch evaluation.Decision {
+				case CompletionIncomplete:
 					result.Incomplete = true
-					result.IncompleteReason = reason
+					result.IncompleteReason = evaluation.Reason
 					result.FinalAnswer = collected.Text
 					result.Messages = copyMessages(messages)
 					return result, nil
-				}
-
-				// (2) The model stopped without a tool call while work may be unfinished:
-				//   - a continuation cue ("…Let me check the config:") is an unambiguous
-				//     mid-step stop;
-				//   - pending update_plan items are a WEAK, ambiguous signal (the model may
-				//     have finished without re-marking the last step).
-				// Nudge to continue (bounded). After the budget: a persisted continuation
-				// cue finalizes INCOMPLETE; pending-plan WITHOUT a cue does NOT (that would
-				// 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++
+				case CompletionUncertain:
+					switch evaluation.Action {
+					case completionActionContinue:
 						options.Trace.Counter(trace.CounterCompletionNudges, 1)
-						reason := "your message ended mid-step"
-						if !cue {
-							reason = "pending plan items remain — finish them, or mark them complete with update_plan if you are done"
-						}
 						messages = append(messages, zeroruntime.Message{
 							Role:    zeroruntime.MessageRoleUser,
-							Content: continueNudge(reason),
+							Content: continueNudge(evaluation.Reason),
+						})
+					case completionActionSemanticCheck:
+						options.Trace.Counter(trace.CounterAcceptanceChecks, 1)
+						messages = append(messages, zeroruntime.Message{
+							Role:    zeroruntime.MessageRoleUser,
+							Content: acceptanceVerificationNudge(),
 						})
-						continue
-					}
-					if cue {
-						result.Incomplete = true
-						result.IncompleteReason = "your message ended mid-step"
-						result.FinalAnswer = collected.Text
-						result.Messages = copyMessages(messages)
-						return result, nil
 					}
-					// pending-plan only, budget spent: trust the model's completion claim
-					// over stale plan bookkeeping; fall through.
-				}
-
-				// (3) Task-grounded acceptance: before accepting a "done" turn as success,
-				// require ONE acceptance check grounded in the task's stated criterion
-				// (only when self-correct is on). Rejects "well-formed == correct",
-				// "existing-tests-pass == objective met", and "result == baseline" false
-				// successes. Bounded to a single pass; a genuine post-check completion
-				// (no admission, no cue) then finalizes as success on the next turn.
-				if options.SelfCorrect != nil && !acceptanceRequested {
-					acceptanceRequested = true
-					options.Trace.Counter(trace.CounterAcceptanceChecks, 1)
-					messages = append(messages, zeroruntime.Message{
-						Role:    zeroruntime.MessageRoleUser,
-						Content: acceptanceVerificationNudge(),
-					})
 					continue
+				case CompletionComplete:
+					// Local evidence is sufficient; proceed to final diagnostics.
 				}
 			}
 			// Finalization diagnostics gate: edits from this run may still have
@@ -2742,7 +2703,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

@@ -315,8 +315,11 @@
 	// 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.
+	// finalizes as INCOMPLETE (Result.Incomplete) rather than success. When the
+	// run's profile also enables SelfCorrect, an otherwise-complete turn gets one
+	// task-grounded semantic check before success; profiles without SelfCorrect
+	// add no model call. Default false leaves the loop byte-identical, so the
+	// interactive TUI is unaffected.
 	RequireCompletionSignal bool
 
 	runPermissions *permissionRunState
@@ -348,7 +351,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 != ""
 }