feat(agentloop): Self-Reflection-Turn vor Completion (issue #152)

cmd/sin-code/internal/agentloop/loop.go

@@ -78,6 +78,20 @@ type StopDecision struct {
 // A nil StopGate preserves the legacy behavior exactly.
 type StopGate func(ctx context.Context, snap StopSnapshot) StopDecision
 
+// Reflection is the worker's self-critique of a proposed completion. Issues
+// non-empty means the agent found problems in its own work and should fix
+// them before the stop-gate is consulted.
+type Reflection struct {
+	Issues []string
+	Notes  string
+}
+
+// Reflector performs a self-critique pass on a proposed completion. Returning
+// a Reflection with non-empty Issues forces one more work turn. A nil
+// Reflector disables the reflection step (legacy behavior).
+// Issue #152.
+type Reflector func(ctx context.Context, snap StopSnapshot) Reflection
+
 type Loop struct {
 	Gate      *verify.Gate
 	LocalTool LocalToolFunc
@@ -119,6 +133,13 @@ type Loop struct {
 	// crosses this fraction of MaxTokens (e.g. 0.8). Useful for alerting.
 	BudgetWarnRatio float64
 
+	// Reflector, if set, runs a self-critique pass right BEFORE the stop-gate.
+	// If it returns issues, the loop injects them and continues working — a
+	// cheap quality lift that reduces stop-gate rejections. Runs at most once
+	// per proposed completion to avoid infinite self-doubt loops.
+	// Issue #152.
+	Reflector Reflector
+
 	// AllowContinuation switches the maxTurns outcome from a hard error to a
 	// checkpointed, resumable Result (Continuation=true). Daemons set this so
 	// a long task is re-enqueued and resumed rather than abandoned; one-shot
@@ -283,6 +304,7 @@ func (l *Loop) Run(ctx context.Context, sess *session.Session, prompt string) (*
 	stallCount := 0
 	totalTokens := 0      // issue #151: cumulative tokens across the run
 	warnedBudget := false // fires hooks.BudgetWarn once per run
+	reflectedThisProposal := false
 	toolsSeen := map[string]bool{}
 	var toolsUsed []string
 
@@ -386,6 +408,37 @@ func (l *Loop) Run(ctx context.Context, sess *session.Session, prompt string) (*
 			})
 			l.record(ctx, ledger.TypeVerifyPass, map[string]any{"mode": string(res.Mode)}, "verification passed ("+string(res.Mode)+")")
 
+			// Self-reflection: one cheap self-critique pass before the
+			// independent stop-gate. Reset the flag whenever the worker did
+			// real work (tool calls) in between, so each fresh proposal gets
+			// exactly one reflection. Issue #152.
+			if l.Reflector != nil && !reflectedThisProposal {
+				reflectedThisProposal = true
+				ref := l.Reflector(ctx, StopSnapshot{
+					Prompt: prompt, FinalOutput: resp.Text, Turns: turn + 1,
+					ToolsUsed: toolsUsed, VerifyPassed: res.Passed, SessionID: sess.ID,
+				})
+				if len(ref.Issues) > 0 {
+					l.fire(ctx, hooks.ReflectIssues, "", map[string]any{"issues": ref.Issues})
+					l.record(ctx, ledger.TypeReflection,
+						map[string]any{"issues": ref.Issues},
+						"self-reflection found issues; continuing")
+					var b strings.Builder
+					b.WriteString("SELF-REVIEW found issues to fix before completing:\n")
+					for i, is := range ref.Issues {
+						fmt.Fprintf(&b, "  %d. %s\n", i+1, is)
+					}
+					if strings.TrimSpace(ref.Notes) != "" {
+						b.WriteString("Notes: " + ref.Notes + "\n")
+					}
+					msgs = append(msgs, session.Message{Role: "user", Content: b.String()})
+					if err := sess.SaveHistory(msgs); err != nil {
+						return nil, err
+					}
+					continue
+				}
+			}
+
 			// Stop-gate: completion authority is decoupled from the worker.
 			// The verify-gate passing is necessary but not sufficient — an
 			// independent evaluator confirms the goal contract is satisfied
@@ -483,6 +536,9 @@ func (l *Loop) Run(ctx context.Context, sess *session.Session, prompt string) (*
 		}
 
 		for _, tc := range resp.ToolCalls {
+			// Real work happened in this turn — reset the reflection
+			// flag so a fresh proposal can be re-evaluated.
+			reflectedThisProposal = false
 			if !toolsSeen[tc.Name] {
 				toolsSeen[tc.Name] = true
 				toolsUsed = append(toolsUsed, tc.Name)

cmd/sin-code/internal/agentloop/loop_reflect_test.go

@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: MIT
+// Purpose: tests for issue #152 — self-reflection pass before the
+// stop-gate. When the Reflector returns issues, the loop continues
+// working instead of consulting the gate.
+package agentloop
+
+import (
+	"context"
+	"testing"
+
+	"github.com/OpenSIN-Code/SIN-Code/cmd/sin-code/internal/session"
+)
+
+// Reflector=nil preserves legacy behavior (no reflection).
+func TestReflector_DisabledWhenNil(t *testing.T) {
+	s := setupSession(t)
+	gateCalls := 0
+	loop := &Loop{
+		Gate: passGate(),
+		// Reflector: nil (default)
+		StopGate: func(ctx context.Context, snap StopSnapshot) StopDecision {
+			gateCalls++
+			return StopDecision{Complete: true}
+		},
+		Completion: func(ctx context.Context, msgs []session.Message, tools []ToolSpec) (*Completion, error) {
+			return &Completion{Text: "ok", Raw: session.Message{Role: "assistant", Content: "ok"}}, nil
+		},
+	}
+	_, err := loop.Run(context.Background(), s, "x")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if gateCalls != 1 {
+		t.Errorf("expected 1 stop-gate call, got %d", gateCalls)
+	}
+}
+
+// Reflector returns issues -> loop injects them and forces another turn.
+func TestReflector_ForcesAnotherTurn(t *testing.T) {
+	s := setupSession(t)
+	reflections := 0
+	completionCalls := 0
+	gateCalls := 0
+	loop := &Loop{
+		Gate: passGate(),
+		Reflector: func(ctx context.Context, snap StopSnapshot) Reflection {
+			reflections++
+			// Return issues once, then accept. The Reflector runs at
+			// most once per proposal — the second time the worker
+			// proposes completion, reflectedThisProposal is true and
+			// the Reflector is skipped.
+			if reflections == 1 {
+				return Reflection{Issues: []string{"missing test"}, Notes: "add a unit test"}
+			}
+			return Reflection{}
+		},
+		StopGate: func(ctx context.Context, snap StopSnapshot) StopDecision {
+			gateCalls++
+			return StopDecision{Complete: true}
+		},
+		Completion: func(ctx context.Context, msgs []session.Message, tools []ToolSpec) (*Completion, error) {
+			completionCalls++
+			return &Completion{Text: "done", Raw: session.Message{Role: "assistant", Content: "done"}}, nil
+		},
+	}
+	_, err := loop.Run(context.Background(), s, "x")
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Reflector runs ONCE (returns issues, the loop continues).
+	// On the second turn, reflectedThisProposal is still true, so
+	// the Reflector is skipped — straight to stop-gate.
+	if reflections != 1 {
+		t.Errorf("expected 1 reflection call (one per proposal), got %d", reflections)
+	}
+	if completionCalls != 2 {
+		t.Errorf("expected 2 completion calls (one per turn), got %d", completionCalls)
+	}
+	if gateCalls != 1 {
+		t.Errorf("expected 1 stop-gate call (after second completion), got %d", gateCalls)
+	}
+}
+
+// Reflector with no issues on first call -> straight to stop-gate.
+func TestReflector_NoIssuesProceeds(t *testing.T) {
+	s := setupSession(t)
+	reflections := 0
+	gateCalls := 0
+	loop := &Loop{
+		Gate: passGate(),
+		Reflector: func(ctx context.Context, snap StopSnapshot) Reflection {
+			reflections++
+			return Reflection{} // no issues
+		},
+		StopGate: func(ctx context.Context, snap StopSnapshot) StopDecision {
+			gateCalls++
+			return StopDecision{Complete: true}
+		},
+		Completion: func(ctx context.Context, msgs []session.Message, tools []ToolSpec) (*Completion, error) {
+			return &Completion{Text: "ok", Raw: session.Message{Role: "assistant", Content: "ok"}}, nil
+		},
+	}
+	_, err := loop.Run(context.Background(), s, "x")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if reflections != 1 {
+		t.Errorf("expected 1 reflection call, got %d", reflections)
+	}
+	if gateCalls != 1 {
+		t.Errorf("expected 1 stop-gate call, got %d", gateCalls)
+	}
+}

cmd/sin-code/internal/hooks/hooks.go

@@ -56,6 +56,9 @@ const (
 	// Token budget lifecycle (issue #151).
 	BudgetWarn      = "budget.warn"
 	BudgetExhausted = "budget.exhausted"
+	// ReflectIssues fires when the self-reflection pass finds problems the
+	// worker must fix before completion is evaluated.
+	ReflectIssues = "reflect.issues"
 
 	AgentSpawn       = "agent.spawn"
 	AgentComplete    = "agent.complete"

cmd/sin-code/internal/ledger/store.go

@@ -40,6 +40,9 @@ const (
 	// TypeTokenBudgetExhausted is recorded when cumulative token usage exceeds
 	// MaxTokens and the run stops spending (issue #151).
 	TypeTokenBudgetExhausted EntryType = "token_budget_exhausted"
+	// TypeReflection records a self-critique pass that found issues and forced
+	// another work turn before stop-gate evaluation.
+	TypeReflection EntryType = "reflection"
 )
 
 // Entry is one row in the ledger.