From 24ad97af310f7de9f1dd0c16b97a950be8c50267 Mon Sep 17 00:00:00 2001 From: viettranx Date: Thu, 19 Mar 2026 23:34:34 +0700 Subject: [PATCH] feat(agent): add read-only streak + same-result loop detection Detect agents stuck in read-only loops (consecutive non-mutating tool calls) and same-result patterns (identical outputs from different args). Warns at threshold, force-stops at critical. --- internal/agent/loop.go | 59 ++++++++++++++++ internal/agent/toolloop.go | 81 +++++++++++++++++++++- internal/agent/toolloop_test.go | 116 ++++++++++++++++++++++++++++++++ 3 files changed, 255 insertions(+), 1 deletion(-) diff --git a/internal/agent/loop.go b/internal/agent/loop.go index 6e441d6e..3c2f38e6 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -928,6 +928,7 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error) // Record result for loop detection. loopDetector.recordResult(argsHash, result.ForLLM) + loopDetector.recordMutation(tc.Name) if result.Async { asyncToolCalls = append(asyncToolCalls, tc.Name) @@ -1006,6 +1007,32 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error) slog.Warn("tool loop warning", "agent", l.id, "tool", tc.Name, "message", msg) messages = append(messages, providers.Message{Role: "user", Content: msg}) } + + // Check for read-only streak (no write/edit actions). + if level, msg := loopDetector.detectReadOnlyStreak(); level != "" { + if level == "critical" { + slog.Warn("tool loop critical: read-only streak", + "streak", loopDetector.readOnlyStreak, "agent", l.id, "run", req.RunID) + finalContent = msg + break + } + slog.Warn("tool loop warning: read-only streak", + "streak", loopDetector.readOnlyStreak, "agent", l.id, "run", req.RunID) + messages = append(messages, providers.Message{Role: "user", Content: msg}) + } + + // Check for same tool returning identical results with different args. + if rh := hashResult(result.ForLLM); rh != "" { + if level, msg := loopDetector.detectSameResult(tc.Name, rh); level != "" { + if level == "critical" { + slog.Warn("tool loop critical: same result", + "tool", tc.Name, "agent", l.id, "run", req.RunID) + finalContent = msg + break + } + messages = append(messages, providers.Message{Role: "user", Content: msg}) + } + } } else { // Multiple tools: parallel execution via goroutines. // Tool instances are immutable (context-based) so concurrent access is safe. @@ -1095,6 +1122,7 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error) // Record for loop detection. argsHash := loopDetector.record(r.tc.Name, r.tc.Arguments) loopDetector.recordResult(argsHash, r.result.ForLLM) + loopDetector.recordMutation(r.tc.Name) if r.result.Async { asyncToolCalls = append(asyncToolCalls, r.tc.Name) @@ -1173,7 +1201,38 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error) slog.Warn("tool loop warning", "agent", l.id, "tool", r.tc.Name, "message", msg) messages = append(messages, providers.Message{Role: "user", Content: msg}) } + + // Check for same tool returning identical results with different args. + if rh := hashResult(r.result.ForLLM); rh != "" { + if level, msg := loopDetector.detectSameResult(r.tc.Name, rh); level != "" { + if level == "critical" { + slog.Warn("tool loop critical: same result", + "tool", r.tc.Name, "agent", l.id, "run", req.RunID) + finalContent = msg + loopStuck = true + break + } + messages = append(messages, providers.Message{Role: "user", Content: msg}) + } + } } + + // Check read-only streak after processing all parallel results. + if !loopStuck { + if level, msg := loopDetector.detectReadOnlyStreak(); level != "" { + if level == "critical" { + slog.Warn("tool loop critical: read-only streak", + "streak", loopDetector.readOnlyStreak, "agent", l.id, "run", req.RunID) + finalContent = msg + loopStuck = true + } else { + slog.Warn("tool loop warning: read-only streak", + "streak", loopDetector.readOnlyStreak, "agent", l.id, "run", req.RunID) + messages = append(messages, providers.Message{Role: "user", Content: msg}) + } + } + } + if loopStuck { break } diff --git a/internal/agent/toolloop.go b/internal/agent/toolloop.go index 2c2cc76c..a8c6f62e 100644 --- a/internal/agent/toolloop.go +++ b/internal/agent/toolloop.go @@ -13,12 +13,32 @@ const ( toolLoopHistorySize = 30 toolLoopWarningThreshold = 3 // inject warning into conversation toolLoopCriticalThreshold = 5 // force stop the iteration loop + + // Read-only streak: consecutive non-mutating tool calls without any write/edit. + readOnlyStreakWarning = 8 + readOnlyStreakCritical = 12 + + // Same-result: same tool returning identical results with different args. + sameResultWarning = 4 + sameResultCritical = 6 ) +// mutatingTools are tools that indicate real progress (write/create/action). +// exec is excluded: ambiguous (could be ls or rm). It neither resets nor +// increments the read-only streak. +var mutatingTools = map[string]bool{ + "write_file": true, "edit": true, "edit_file": true, + "spawn": true, "team_tasks": true, "message": true, + "create_image": true, "create_video": true, "create_audio": true, + "tts": true, "cron": true, "publish_skill": true, + "sessions_send": true, +} + // toolLoopState tracks recent tool calls within a single agent run // to detect infinite loops (same tool + same args + same result). type toolLoopState struct { - history []toolCallRecord + history []toolCallRecord + readOnlyStreak int // consecutive non-mutating, non-exec tool calls } type toolCallRecord struct { @@ -97,6 +117,65 @@ func (s *toolLoopState) detect(toolName string, argsHash string) (level, message return "", "" } +// recordMutation updates the read-only streak based on tool type. +// Mutating tools reset the streak; exec is neutral (ambiguous); all others increment. +func (s *toolLoopState) recordMutation(toolName string) { + if mutatingTools[toolName] { + s.readOnlyStreak = 0 + return + } + if toolName == "exec" || toolName == "bash" { + return // ambiguous — neither reset nor increment + } + s.readOnlyStreak++ +} + +// detectReadOnlyStreak checks for long runs of read-only tool calls +// without any write/edit action. Returns level and message. +func (s *toolLoopState) detectReadOnlyStreak() (level, message string) { + if s.readOnlyStreak >= readOnlyStreakCritical { + return "critical", fmt.Sprintf( + "CRITICAL: %d consecutive read-only tool calls without any write/edit action. "+ + "Stopping to prevent runaway loop.", s.readOnlyStreak) + } + if s.readOnlyStreak >= readOnlyStreakWarning { + return "warning", fmt.Sprintf( + "[System: WARNING — You have made %d consecutive read-only tool calls (list_files, read_file, find, etc.) "+ + "without writing or editing any file. If you already have the information you need, use the edit or "+ + "write_file tool to take action. If you cannot proceed, respond directly to the user explaining "+ + "what is blocking you.]", s.readOnlyStreak) + } + return "", "" +} + +// detectSameResult checks if the same tool returned identical results multiple +// times with different arguments. This catches loops where the agent varies +// args slightly but gets no new information. +func (s *toolLoopState) detectSameResult(toolName, resultHash string) (level, message string) { + if resultHash == "" { + return "", "" + } + var count int + for _, rec := range s.history { + if rec.toolName == toolName && rec.resultHash == resultHash { + count++ + } + } + if count >= sameResultCritical { + return "critical", fmt.Sprintf( + "CRITICAL: %s returned identical results %d times (with different arguments). "+ + "Stopping to prevent runaway loop.", toolName, count) + } + if count >= sameResultWarning { + return "warning", fmt.Sprintf( + "[System: WARNING — %s has returned the same result %d times with different arguments. "+ + "The information is already in your context. Stop re-reading and take action — "+ + "use edit/write_file to modify files, or respond to the user if you are stuck.]", + toolName, count) + } + return "", "" +} + // hashToolCall produces a deterministic hash of tool name + arguments. func hashToolCall(toolName string, args map[string]any) string { s := toolName + ":" + stableJSON(args) diff --git a/internal/agent/toolloop_test.go b/internal/agent/toolloop_test.go index 6aeeac59..3eca5758 100644 --- a/internal/agent/toolloop_test.go +++ b/internal/agent/toolloop_test.go @@ -102,6 +102,122 @@ func TestStableJSON(t *testing.T) { } } +// --- Read-only streak detection --- + +func TestReadOnlyStreak_Warning(t *testing.T) { + var s toolLoopState + for range readOnlyStreakWarning { + s.recordMutation("read_file") + } + level, _ := s.detectReadOnlyStreak() + if level != "warning" { + t.Fatalf("expected warning after %d read-only calls, got %q", readOnlyStreakWarning, level) + } +} + +func TestReadOnlyStreak_Critical(t *testing.T) { + var s toolLoopState + for range readOnlyStreakCritical { + s.recordMutation("list_files") + } + level, _ := s.detectReadOnlyStreak() + if level != "critical" { + t.Fatalf("expected critical after %d read-only calls, got %q", readOnlyStreakCritical, level) + } +} + +func TestReadOnlyStreak_ResetByMutation(t *testing.T) { + var s toolLoopState + // 7 read-only calls → below warning + for range 7 { + s.recordMutation("read_file") + } + // 1 edit resets streak + s.recordMutation("edit") + if s.readOnlyStreak != 0 { + t.Fatalf("expected streak 0 after edit, got %d", s.readOnlyStreak) + } + // 7 more reads → streak = 7, still below warning + for range 7 { + s.recordMutation("read_file") + } + level, _ := s.detectReadOnlyStreak() + if level != "" { + t.Fatalf("expected no detection at streak 7, got %q", level) + } +} + +func TestReadOnlyStreak_ExecNeutral(t *testing.T) { + var s toolLoopState + // 5 reads → streak = 5 + for range 5 { + s.recordMutation("read_file") + } + // exec does not reset or increment + s.recordMutation("exec") + if s.readOnlyStreak != 5 { + t.Fatalf("expected streak 5 after exec, got %d", s.readOnlyStreak) + } + // 5 more reads → streak = 10 + for range 5 { + s.recordMutation("list_files") + } + if s.readOnlyStreak != 10 { + t.Fatalf("expected streak 10, got %d", s.readOnlyStreak) + } + level, _ := s.detectReadOnlyStreak() + if level != "warning" { + t.Fatalf("expected warning at streak 10, got %q", level) + } +} + +// --- Same-result cross-args detection --- + +func TestSameResult_Warning(t *testing.T) { + var s toolLoopState + sameResult := "directory listing output" + for i := range sameResultWarning { + args := map[string]any{"path": string(rune('a' + i))} + h := s.record("list_files", args) + s.recordResult(h, sameResult) + } + rh := hashResult(sameResult) + level, _ := s.detectSameResult("list_files", rh) + if level != "warning" { + t.Fatalf("expected warning after %d same-result calls, got %q", sameResultWarning, level) + } +} + +func TestSameResult_Critical(t *testing.T) { + var s toolLoopState + sameResult := "directory listing output" + for i := range sameResultCritical { + args := map[string]any{"path": string(rune('a' + i))} + h := s.record("list_files", args) + s.recordResult(h, sameResult) + } + rh := hashResult(sameResult) + level, _ := s.detectSameResult("list_files", rh) + if level != "critical" { + t.Fatalf("expected critical after %d same-result calls, got %q", sameResultCritical, level) + } +} + +func TestSameResult_DifferentResults(t *testing.T) { + var s toolLoopState + // Same tool, same args pattern, but different results each time → no detection + for i := range 8 { + args := map[string]any{"path": string(rune('a' + i))} + h := s.record("list_files", args) + s.recordResult(h, "result "+string(rune('a'+i))) + } + rh := hashResult("result a") // check against the first result + level, _ := s.detectSameResult("list_files", rh) + if level != "" { + t.Fatalf("expected no detection for different results, got %q", level) + } +} + func TestHashToolCall(t *testing.T) { // Same input → same hash h1 := hashToolCall("list_files", map[string]any{"path": "."})