diff --git a/cmd/gateway_consumer_handlers.go b/cmd/gateway_consumer_handlers.go index 09be273f..315daa30 100644 --- a/cmd/gateway_consumer_handlers.go +++ b/cmd/gateway_consumer_handlers.go @@ -405,6 +405,34 @@ func handleTeammateMessage( _ = teamStore.RenewTaskLock(ctx, teamTaskID, teamID) slog.Info("post-turn: task submitted for review", "task_id", teamTaskID) + case outcome.Result != nil && outcome.Result.LoopKilled: + // Loop detector killed the run → auto-fail instead of auto-complete. + // The agent was terminated (not stuck in_progress) — mark as failed + // so the leader sees clear failure signal and can retry directly. + failMsg := outcome.Result.Content + if failMsg == "" { + failMsg = "Agent run terminated by loop detector" + } + if err := teamStore.FailTask(ctx, teamTaskID, teamID, failMsg); err != nil { + slog.Warn("auto-fail: FailTask error (loop kill)", "task_id", teamTaskID, "error", err) + } else { + bus.BroadcastForTenant(msgBus, protocol.EventTeamTaskFailed, store.TenantIDFromContext(ctx), protocol.TeamTaskEventPayload{ + TeamID: teamID.String(), + TaskID: teamTaskID.String(), + TaskNumber: taskNumber, + Subject: taskSubject, + Status: store.TeamTaskStatusFailed, + Reason: "loop_detector_kill", + Channel: taskChannel, + ChatID: taskChatID, + Timestamp: now, + ActorType: "system", + ActorID: "loop_detector", + }) + } + slog.Warn("post-turn: loop detector killed member run", + "task_id", teamTaskID, "agent", toAgent) + default: // Agent turn ended without terminal action — auto-complete. // Covers: Progressed, Commented, Claimed, or no flags at all. diff --git a/internal/agent/loop_finalize.go b/internal/agent/loop_finalize.go index d5a1f112..ef6d59a3 100644 --- a/internal/agent/loop_finalize.go +++ b/internal/agent/loop_finalize.go @@ -204,5 +204,6 @@ func (l *Loop) finalizeRun( Deliverables: rs.deliverables, BlockReplies: rs.blockReplies, LastBlockReply: rs.lastBlockReply, + LoopKilled: rs.loopKilled, } } diff --git a/internal/agent/loop_tools.go b/internal/agent/loop_tools.go index a05d8eac..7a08bd62 100644 --- a/internal/agent/loop_tools.go +++ b/internal/agent/loop_tools.go @@ -39,7 +39,7 @@ func (l *Loop) processToolResult( // Record for loop detection. argsHash := rs.loopDetector.record(registryName, tc.Arguments) rs.loopDetector.recordResult(argsHash, result.ForLLM) - rs.loopDetector.recordMutation(registryName) + rs.loopDetector.recordMutation(registryName, tc.Arguments) if result.Async { rs.asyncToolCalls = append(rs.asyncToolCalls, tc.Name) @@ -120,6 +120,7 @@ func (l *Loop) processToolResult( if level == "critical" { slog.Warn("tool loop critical", "agent", l.id, "tool", registryName, "message", msg) rs.finalContent = "I was unable to complete this task — I got stuck repeatedly calling " + registryName + " without making progress. Please try rephrasing your request." + rs.loopKilled = true return toolMsg, nil, toolResultBreak } slog.Warn("tool loop warning", "agent", l.id, "tool", registryName, "message", msg) @@ -134,6 +135,7 @@ func (l *Loop) processToolResult( slog.Warn("tool loop critical: same result", "tool", registryName, "agent", l.id, "run", req.RunID) rs.finalContent = msg + rs.loopKilled = true return toolMsg, nil, toolResultBreak } warningMsgs = append(warningMsgs, providers.Message{Role: "user", Content: msg}) @@ -153,8 +155,11 @@ func (l *Loop) checkReadOnlyStreak(rs *runState, req *RunRequest) (warningMsg *p } if level == "critical" { slog.Warn("tool loop critical: read-only streak", - "streak", rs.loopDetector.readOnlyStreak, "agent", l.id, "run", req.RunID) + "streak", rs.loopDetector.readOnlyStreak, + "unique", rs.loopDetector.readOnlyUnique, + "agent", l.id, "run", req.RunID) rs.finalContent = msg + rs.loopKilled = true return nil, true } slog.Warn("tool loop warning: read-only streak", diff --git a/internal/agent/loop_types.go b/internal/agent/loop_types.go index 830a6d81..30ff8397 100644 --- a/internal/agent/loop_types.go +++ b/internal/agent/loop_types.go @@ -465,6 +465,7 @@ type RunResult struct { Deliverables []string `json:"deliverables,omitempty"` // actual content from tool outputs (for team task results) BlockReplies int `json:"blockReplies,omitempty"` // number of block.reply events emitted LastBlockReply string `json:"lastBlockReply,omitempty"` // last block reply content (for dedup) + LoopKilled bool `json:"loopKilled,omitempty"` // true when run was terminated by loop detector } // MediaResult represents a media file produced by a tool during the agent run. @@ -514,4 +515,8 @@ type runState struct { skillNudge70Sent bool skillNudge90Sent bool skillPostscriptSent bool + + // Loop detector kill flag — set when any detector triggers critical level. + // Propagated to RunResult.LoopKilled so the consumer can auto-fail team tasks. + loopKilled bool } diff --git a/internal/agent/toolloop.go b/internal/agent/toolloop.go index f86e8add..94592366 100644 --- a/internal/agent/toolloop.go +++ b/internal/agent/toolloop.go @@ -15,9 +15,18 @@ const ( toolLoopCriticalThreshold = 5 // force stop the iteration loop // Read-only streak: consecutive non-mutating tool calls without any write/edit. + // Stuck mode (uniqueness ratio ≤ 0.6): original thresholds. readOnlyStreakWarning = 8 readOnlyStreakCritical = 12 + // Exploration mode (uniqueness ratio > 0.6): relaxed thresholds. + // Agents exploring unique files should not be killed as early as stuck loops. + readOnlyExplorationWarning = 24 + readOnlyExplorationCritical = 36 + + // Uniqueness ratio threshold: above this = exploration, below = stuck. + readOnlyUniquenessThreshold = 0.6 + // Same-result: same tool returning identical results with different args. sameResultWarning = 4 sameResultCritical = 6 @@ -26,19 +35,34 @@ const ( // 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. +// team_tasks is excluded: action-level classification in recordMutation. var mutatingTools = map[string]bool{ "write_file": true, "edit": true, "edit_file": true, - "spawn": true, "team_tasks": true, "message": true, + "spawn": true, "message": true, "create_image": true, "create_video": true, "create_audio": true, "tts": true, "cron": true, "publish_skill": true, "sessions_send": true, } +// teamTasksReadOnlyActions are team_tasks actions that don't indicate real progress. +// They increment the read-only streak like read_file/list_files. +var teamTasksReadOnlyActions = map[string]bool{ + "list": true, "get": true, "search": true, +} + +// teamTasksNeutralActions are team_tasks actions that are heartbeat/status only. +// They neither reset nor increment the read-only streak (like exec). +var teamTasksNeutralActions = map[string]bool{ + "progress": 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 - readOnlyStreak int // consecutive non-mutating, non-exec tool calls + readOnlyStreak int // consecutive non-mutating, non-exec tool calls + readOnlyUnique int // unique args hashes in current streak + seenReadArgs map[string]bool // tracks unique read arg hashes for uniqueness ratio } type toolCallRecord struct { @@ -119,9 +143,28 @@ func (s *toolLoopState) detect(toolName string, argsHash string) (level, message // recordMutation updates the read-only streak based on tool type. // Mutating tools reset the streak; exec/bash/mcp are neutral (ambiguous); all others increment. -func (s *toolLoopState) recordMutation(toolName string) { +// team_tasks is classified by action: read-only (list/get/search), neutral (progress), +// or mutating (create/complete/cancel/comment/etc.). +func (s *toolLoopState) recordMutation(toolName string, args map[string]any) { + // team_tasks: action-level classification instead of blanket mutating. + if toolName == "team_tasks" { + action, _ := args["action"].(string) + switch { + case teamTasksReadOnlyActions[action]: + s.incrementReadOnly(toolName, args) + case teamTasksNeutralActions[action]: + // Heartbeat — no effect on streak. + case action == "": + // Missing action arg — treat as neutral to avoid crash. + default: + // All other actions (create, complete, cancel, comment, etc.) = mutating. + s.resetStreak() + } + return + } + if mutatingTools[toolName] { - s.readOnlyStreak = 0 + s.resetStreak() return } // exec/bash: ambiguous (could be ls or rm). @@ -130,23 +173,69 @@ func (s *toolLoopState) recordMutation(toolName string) { if toolName == "exec" || toolName == "bash" || strings.HasPrefix(toolName, "mcp_") { return } + s.incrementReadOnly(toolName, args) +} + +// resetStreak clears the read-only streak and uniqueness tracking. +func (s *toolLoopState) resetStreak() { + s.readOnlyStreak = 0 + s.readOnlyUnique = 0 + s.seenReadArgs = nil +} + +// incrementReadOnly increments the read-only streak and tracks uniqueness. +func (s *toolLoopState) incrementReadOnly(toolName string, args map[string]any) { s.readOnlyStreak++ + argsHash := hashToolCall(toolName, args) + if s.seenReadArgs == nil { + s.seenReadArgs = make(map[string]bool) + } + if !s.seenReadArgs[argsHash] { + s.seenReadArgs[argsHash] = true + s.readOnlyUnique++ + } } // detectReadOnlyStreak checks for long runs of read-only tool calls -// without any write/edit action. Returns level and message. +// without any write/edit action. Uses uniqueness ratio to distinguish +// exploration (many unique files) from stuck loops (re-reading same files). +// +// Stuck mode (ratio ≤ 0.6): warn at 8, kill at 12. +// Exploration mode (ratio > 0.6): warn at 24, kill at 36. func (s *toolLoopState) detectReadOnlyStreak() (level, message string) { + if s.readOnlyStreak < readOnlyStreakWarning { + return "", "" + } + + uniqueRatio := float64(s.readOnlyUnique) / float64(s.readOnlyStreak) + + if uniqueRatio > readOnlyUniquenessThreshold { + // Exploration mode: agent is reading unique files. Relaxed thresholds. + if s.readOnlyStreak >= readOnlyExplorationCritical { + return "critical", fmt.Sprintf( + "CRITICAL: %d consecutive read-only tool calls (%d unique files). "+ + "Stopping — write your findings before reading more.", s.readOnlyStreak, s.readOnlyUnique) + } + if s.readOnlyStreak >= readOnlyExplorationWarning { + return "warning", fmt.Sprintf( + "[System: You have read %d files (%d unique). "+ + "Summarize what you have learned so far using write_file, then continue reading if needed.]", + s.readOnlyStreak, s.readOnlyUnique) + } + return "", "" + } + + // Stuck mode: low uniqueness — agent is re-reading same files. Original thresholds. 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) + "CRITICAL: %d consecutive read-only tool calls (only %d unique). "+ + "Stopping — you are re-reading the same files without making progress.", s.readOnlyStreak, s.readOnlyUnique) } 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) + "[System: WARNING — You have made %d consecutive read-only tool calls (only %d unique files). "+ + "Stop re-reading and take action with what you have — use edit or write_file, "+ + "or respond to the user if you are stuck.]", s.readOnlyStreak, s.readOnlyUnique) } return "", "" } diff --git a/internal/agent/toolloop_test.go b/internal/agent/toolloop_test.go index 630510aa..2300976b 100644 --- a/internal/agent/toolloop_test.go +++ b/internal/agent/toolloop_test.go @@ -1,6 +1,11 @@ package agent -import "testing" +import ( + "fmt" + "testing" +) + +// ===== Layer 1: Same-args loop detection ===== func TestToolLoopDetection_NoLoop(t *testing.T) { var s toolLoopState @@ -102,14 +107,407 @@ func TestStableJSON(t *testing.T) { } } -// --- Read-only streak detection --- +// ===== Group A: Uniqueness-aware read-only streak detection ===== + +func TestReadOnlyStreak_ExplorationMode_NoKill(t *testing.T) { + // A1: 12 unique reads ��� ratio=1.0 ��� exploration mode → NO trigger + var s toolLoopState + for i := range 12 { + s.recordMutation("read_file", map[string]any{"path": fmt.Sprintf("/file%d.txt", i)}) + } + level, _ := s.detectReadOnlyStreak() + if level == "critical" { + t.Fatal("12 unique reads should NOT trigger critical in exploration mode") + } + // Should also not warn (exploration warning at 24) + if level != "" { + t.Fatalf("12 unique reads should not trigger any level, got %q", level) + } +} + +func TestReadOnlyStreak_StuckMode_Kill(t *testing.T) { + // A2: Same file read 12 times → ratio=1/12 → stuck mode → KILL + var s toolLoopState + for range 12 { + s.recordMutation("read_file", map[string]any{"path": "/same-file.txt"}) + } + level, _ := s.detectReadOnlyStreak() + if level != "critical" { + t.Fatalf("12 reads of same file should trigger critical, got %q", level) + } +} + +func TestReadOnlyStreak_LowUniqueness_Kill(t *testing.T) { + // A3: 3 unique files read 4 times each (12 total) → ratio=0.25 → stuck → KILL + var s toolLoopState + files := []string{"/a.txt", "/b.txt", "/c.txt"} + for i := range 12 { + s.recordMutation("read_file", map[string]any{"path": files[i%3]}) + } + level, _ := s.detectReadOnlyStreak() + if level != "critical" { + t.Fatalf("low uniqueness (3/12) should trigger critical, got %q", level) + } +} + +func TestReadOnlyStreak_BoundaryRatio_Stuck(t *testing.T) { + // A4: 7 unique out of 12 = 0.583 → below 0.6 → stuck mode → critical at 12 + var s toolLoopState + // First 7 unique + for i := range 7 { + s.recordMutation("read_file", map[string]any{"path": fmt.Sprintf("/unique%d.txt", i)}) + } + // Next 5 repeat first file + for range 5 { + s.recordMutation("read_file", map[string]any{"path": "/unique0.txt"}) + } + if s.readOnlyStreak != 12 { + t.Fatalf("expected streak=12, got %d", s.readOnlyStreak) + } + if s.readOnlyUnique != 7 { + t.Fatalf("expected unique=7, got %d", s.readOnlyUnique) + } + level, _ := s.detectReadOnlyStreak() + if level != "critical" { + t.Fatalf("7/12 unique (0.583) should be stuck mode → critical, got %q", level) + } +} + +func TestReadOnlyStreak_BoundaryRatio_Exploration(t *testing.T) { + // A5: 8 unique out of 12 = 0.667 → above 0.6 → exploration mode → no trigger at 12 + var s toolLoopState + // First 8 unique + for i := range 8 { + s.recordMutation("read_file", map[string]any{"path": fmt.Sprintf("/unique%d.txt", i)}) + } + // Next 4 repeat first file + for range 4 { + s.recordMutation("read_file", map[string]any{"path": "/unique0.txt"}) + } + if s.readOnlyStreak != 12 { + t.Fatalf("expected streak=12, got %d", s.readOnlyStreak) + } + if s.readOnlyUnique != 8 { + t.Fatalf("expected unique=8, got %d", s.readOnlyUnique) + } + level, _ := s.detectReadOnlyStreak() + if level != "" { + t.Fatalf("8/12 unique (0.667) should be exploration mode → no trigger at 12, got %q", level) + } +} + +func TestReadOnlyStreak_ExplorationWarning(t *testing.T) { + // A6: 24 unique reads �� exploration mode → warning at 24 + var s toolLoopState + for i := range 24 { + s.recordMutation("read_file", map[string]any{"path": fmt.Sprintf("/file%d.txt", i)}) + } + level, _ := s.detectReadOnlyStreak() + if level != "warning" { + t.Fatalf("24 unique reads should trigger exploration warning, got %q", level) + } +} + +func TestReadOnlyStreak_ExplorationKill(t *testing.T) { + // A7: 36 unique reads → exploration mode → critical at 36 + var s toolLoopState + for i := range 36 { + s.recordMutation("read_file", map[string]any{"path": fmt.Sprintf("/file%d.txt", i)}) + } + level, _ := s.detectReadOnlyStreak() + if level != "critical" { + t.Fatalf("36 unique reads should trigger exploration critical, got %q", level) + } +} + +func TestReadOnlyStreak_MutationResetsUniqueness(t *testing.T) { + // A8: 10 unique reads → edit (resets) → 12 unique reads → exploration, no kill + var s toolLoopState + for i := range 10 { + s.recordMutation("read_file", map[string]any{"path": fmt.Sprintf("/file%d.txt", i)}) + } + // Mutation resets everything + s.recordMutation("edit", nil) + if s.readOnlyStreak != 0 { + t.Fatalf("expected streak=0 after edit, got %d", s.readOnlyStreak) + } + if s.readOnlyUnique != 0 { + t.Fatalf("expected unique=0 after edit, got %d", s.readOnlyUnique) + } + if s.seenReadArgs != nil { + t.Fatal("expected seenReadArgs=nil after edit") + } + // 12 more unique reads → exploration mode + for i := range 12 { + s.recordMutation("read_file", map[string]any{"path": fmt.Sprintf("/new%d.txt", i)}) + } + level, _ := s.detectReadOnlyStreak() + if level == "critical" { + t.Fatal("12 unique reads after mutation reset should NOT trigger critical") + } +} + +func TestReadOnlyStreak_ZeroStreak(t *testing.T) { + // A9: Division-by-zero guard — fresh state �� no panic + var s toolLoopState + level, _ := s.detectReadOnlyStreak() + if level != "" { + t.Fatalf("expected no detection on zero streak, got %q", level) + } +} + +// ===== Group B: Action-aware team_tasks ===== + +func TestReadOnlyStreak_TeamTasksProgressNeutral(t *testing.T) { + // B1: team_tasks(progress) is neutral — streak unchanged + var s toolLoopState + for range 5 { + s.recordMutation("read_file", map[string]any{"path": "/file.txt"}) + } + if s.readOnlyStreak != 5 { + t.Fatalf("expected streak=5, got %d", s.readOnlyStreak) + } + s.recordMutation("team_tasks", map[string]any{"action": "progress", "percent": 50}) + if s.readOnlyStreak != 5 { + t.Fatalf("team_tasks(progress) should be neutral, streak should stay 5, got %d", s.readOnlyStreak) + } +} + +func TestReadOnlyStreak_TeamTasksListReadOnly(t *testing.T) { + // B2: team_tasks(list) is read-only — streak increments + var s toolLoopState + for range 5 { + s.recordMutation("read_file", map[string]any{"path": "/file.txt"}) + } + s.recordMutation("team_tasks", map[string]any{"action": "list"}) + if s.readOnlyStreak != 6 { + t.Fatalf("team_tasks(list) should increment streak to 6, got %d", s.readOnlyStreak) + } +} + +func TestReadOnlyStreak_TeamTasksSearchReadOnly(t *testing.T) { + // B3: team_tasks(search) is read-only — streak increments + var s toolLoopState + for range 5 { + s.recordMutation("read_file", map[string]any{"path": "/file.txt"}) + } + s.recordMutation("team_tasks", map[string]any{"action": "search", "query": "test"}) + if s.readOnlyStreak != 6 { + t.Fatalf("team_tasks(search) should increment streak to 6, got %d", s.readOnlyStreak) + } +} + +func TestReadOnlyStreak_TeamTasksGetReadOnly(t *testing.T) { + // B3b: team_tasks(get) is read-only — streak increments + var s toolLoopState + for range 5 { + s.recordMutation("read_file", map[string]any{"path": "/file.txt"}) + } + s.recordMutation("team_tasks", map[string]any{"action": "get", "task_id": "abc"}) + if s.readOnlyStreak != 6 { + t.Fatalf("team_tasks(get) should increment streak to 6, got %d", s.readOnlyStreak) + } +} + +func TestReadOnlyStreak_TeamTasksCreateMutating(t *testing.T) { + // B4: team_tasks(create) is mutating — streak resets to 0 + var s toolLoopState + for range 5 { + s.recordMutation("read_file", map[string]any{"path": "/file.txt"}) + } + s.recordMutation("team_tasks", map[string]any{"action": "create", "subject": "test task"}) + if s.readOnlyStreak != 0 { + t.Fatalf("team_tasks(create) should reset streak to 0, got %d", s.readOnlyStreak) + } +} + +func TestReadOnlyStreak_TeamTasksCompleteMutating(t *testing.T) { + // B5: team_tasks(complete) is mutating — streak resets to 0 + var s toolLoopState + for range 5 { + s.recordMutation("read_file", map[string]any{"path": "/file.txt"}) + } + s.recordMutation("team_tasks", map[string]any{"action": "complete", "result": "done"}) + if s.readOnlyStreak != 0 { + t.Fatalf("team_tasks(complete) should reset streak to 0, got %d", s.readOnlyStreak) + } +} + +func TestReadOnlyStreak_TeamTasksCommentMutating(t *testing.T) { + // B6: team_tasks(comment) is mutating — streak resets to 0 + var s toolLoopState + for range 5 { + s.recordMutation("read_file", map[string]any{"path": "/file.txt"}) + } + s.recordMutation("team_tasks", map[string]any{"action": "comment", "text": "found something"}) + if s.readOnlyStreak != 0 { + t.Fatalf("team_tasks(comment) should reset streak to 0, got %d", s.readOnlyStreak) + } +} + +func TestReadOnlyStreak_TeamTasksNoAction(t *testing.T) { + // B7: team_tasks with no action arg → neutral (no crash) + var s toolLoopState + for range 5 { + s.recordMutation("read_file", map[string]any{"path": "/file.txt"}) + } + // Empty args — should not panic + s.recordMutation("team_tasks", map[string]any{}) + if s.readOnlyStreak != 5 { + t.Fatalf("team_tasks with no action should be neutral, streak should stay 5, got %d", s.readOnlyStreak) + } + // Nil args — should not panic + s.recordMutation("team_tasks", nil) + if s.readOnlyStreak != 5 { + t.Fatalf("team_tasks with nil args should be neutral, streak should stay 5, got %d", s.readOnlyStreak) + } +} + +func TestReadOnlyStreak_TeamTasksUnknownAction(t *testing.T) { + // B8: Unknown action → mutating (safe default) + var s toolLoopState + for range 5 { + s.recordMutation("read_file", map[string]any{"path": "/file.txt"}) + } + s.recordMutation("team_tasks", map[string]any{"action": "some_future_action"}) + if s.readOnlyStreak != 0 { + t.Fatalf("team_tasks with unknown action should reset streak to 0, got %d", s.readOnlyStreak) + } +} + +// ===== Group C: Trace replay (integration-style) ===== + +func TestReadOnlyStreak_TraceReplay_Issue506(t *testing.T) { + // C1: Exact trace scenario from issue #506. + // Agent explores monorepo: 12 unique reads + 1 team_tasks(progress). + // Should NOT trigger any critical kill. + var s toolLoopState + + // Span 2: read_file(SKILL.md) + s.recordMutation("read_file", map[string]any{"path": "/app/data/skills-store/builder-test-fixer/1/SKILL.md"}) + level, _ := s.detectReadOnlyStreak() + if level == "critical" { + t.Fatal("should not trigger critical at span 2") + } + + // Span 4: team_tasks(progress, 10%) + s.recordMutation("team_tasks", map[string]any{"action": "progress", "percent": float64(10), "text": "started"}) + if s.readOnlyStreak != 1 { + t.Fatalf("team_tasks(progress) should be neutral, streak should stay 1, got %d", s.readOnlyStreak) + } + + // Span 6: read_file(plan.md) + s.recordMutation("read_file", map[string]any{"path": "/app/workspace/teams/019d2ad3/order-booking-optimization-plan-2026-03-26.md"}) + // Span 7: read_file(qa-review.md) + s.recordMutation("read_file", map[string]any{"path": "/app/workspace/teams/019d2ad3/order-booking-phase-2-qa-review-2026-03-26.md"}) + // Span 8: read_file(risk-review.md) + s.recordMutation("read_file", map[string]any{"path": "/app/workspace/teams/019d2ad3/order-booking-optimization-risk-review-2026-03-26.md"}) + // Span 9: list_files(teams/...) + s.recordMutation("list_files", map[string]any{"path": "/app/workspace/teams/019d2ad3"}) + // Span 10: list_files(.) + s.recordMutation("list_files", map[string]any{"path": "."}) + // Span 12: list_files(order-booking/) + s.recordMutation("list_files", map[string]any{"path": "order-booking"}) + + // At this point: streak=8 (warning threshold in stuck mode, but exploration mode). + // Verify warning (if any) is not critical. + level, _ = s.detectReadOnlyStreak() + if level == "critical" { + t.Fatal("should not trigger critical at streak=8 in exploration mode") + } + + // Span 13: read_file(api/package.json) + s.recordMutation("read_file", map[string]any{"path": "order-booking/apps/api/package.json"}) + // Span 14: read_file(web/package.json) + s.recordMutation("read_file", map[string]any{"path": "order-booking/apps/web/package.json"}) + // Span 15: read_file(package.json) + s.recordMutation("read_file", map[string]any{"path": "order-booking/package.json"}) + // Span 16: read_file(database/package.json) + s.recordMutation("read_file", map[string]any{"path": "order-booking/packages/database/package.json"}) + + // Count: 1 (SKILL.md) + 0 (progress neutral) + 3 (plan/qa/risk) + 3 (list_files) + 4 (pkg.jsons) = 11 reads + if s.readOnlyStreak != 11 { + t.Fatalf("expected streak=11, got %d", s.readOnlyStreak) + } + if s.readOnlyUnique != 11 { + t.Fatalf("expected unique=11, got %d", s.readOnlyUnique) + } + level, _ = s.detectReadOnlyStreak() + if level == "critical" { + t.Fatal("REGRESSION: 11 unique reads should NOT trigger critical — this was the #506 false positive") + } + + // Span 17: read_file(types/package.json) + s.recordMutation("read_file", map[string]any{"path": "order-booking/packages/types/package.json"}) + // Span 18: read_file(turbo.json) + s.recordMutation("read_file", map[string]any{"path": "order-booking/turbo.json"}) + + // streak=13, all unique. In old code this would have been killed at 12. + // With uniqueness-aware detection, exploration mode continues. + if s.readOnlyStreak != 13 { + t.Fatalf("expected streak=13, got %d", s.readOnlyStreak) + } + level, _ = s.detectReadOnlyStreak() + if level == "critical" { + t.Fatal("13 unique reads should not trigger critical in exploration mode") + } + if level != "" { + t.Fatalf("13 unique reads in exploration mode should have no trigger, got %q", level) + } +} + +func TestReadOnlyStreak_TraceReplay_StuckLoop(t *testing.T) { + // C2: Agent reads same 2 files alternating 12 times → stuck → KILL + var s toolLoopState + for i := range 12 { + file := "/a.txt" + if i%2 == 1 { + file = "/b.txt" + } + s.recordMutation("read_file", map[string]any{"path": file}) + } + // ratio = 2/12 = 0.167 → stuck mode + if s.readOnlyUnique != 2 { + t.Fatalf("expected unique=2, got %d", s.readOnlyUnique) + } + level, _ := s.detectReadOnlyStreak() + if level != "critical" { + t.Fatalf("stuck loop (2 files alternating) should trigger critical at 12, got %q", level) + } +} + +func TestReadOnlyStreak_ExactBoundaryRatio(t *testing.T) { + // E4: Exactly 0.6 ratio → stuck mode (strict greater-than check) + // 6 unique out of 10 = 0.6 → NOT > 0.6 → stuck mode → warning at 10 + var s toolLoopState + for i := range 6 { + s.recordMutation("read_file", map[string]any{"path": fmt.Sprintf("/u%d.txt", i)}) + } + for range 4 { + s.recordMutation("read_file", map[string]any{"path": "/u0.txt"}) + } + if s.readOnlyStreak != 10 { + t.Fatalf("expected streak=10, got %d", s.readOnlyStreak) + } + if s.readOnlyUnique != 6 { + t.Fatalf("expected unique=6, got %d", s.readOnlyUnique) + } + level, _ := s.detectReadOnlyStreak() + // ratio=0.6, NOT > 0.6, so stuck mode. Streak=10 >= warning=8 → warning. + if level != "warning" { + t.Fatalf("exact 0.6 ratio should be stuck mode → warning at 10, got %q", level) + } +} + +// ===== Existing tests — updated for new recordMutation(toolName, args) signature ===== func TestReadOnlyStreak_Warning(t *testing.T) { var s toolLoopState for range readOnlyStreakWarning { - s.recordMutation("read_file") + s.recordMutation("read_file", nil) } level, _ := s.detectReadOnlyStreak() + // With nil args, all hashes are identical → unique=1 → ratio=1/8=0.125 → stuck mode if level != "warning" { t.Fatalf("expected warning after %d read-only calls, got %q", readOnlyStreakWarning, level) } @@ -118,9 +516,10 @@ func TestReadOnlyStreak_Warning(t *testing.T) { func TestReadOnlyStreak_Critical(t *testing.T) { var s toolLoopState for range readOnlyStreakCritical { - s.recordMutation("list_files") + s.recordMutation("list_files", nil) } level, _ := s.detectReadOnlyStreak() + // nil args → unique=1 → ratio=1/12=0.083 → stuck mode → critical if level != "critical" { t.Fatalf("expected critical after %d read-only calls, got %q", readOnlyStreakCritical, level) } @@ -130,16 +529,16 @@ func TestReadOnlyStreak_ResetByMutation(t *testing.T) { var s toolLoopState // 7 read-only calls → below warning for range 7 { - s.recordMutation("read_file") + s.recordMutation("read_file", nil) } // 1 edit resets streak - s.recordMutation("edit") + s.recordMutation("edit", nil) 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") + s.recordMutation("read_file", nil) } level, _ := s.detectReadOnlyStreak() if level != "" { @@ -151,16 +550,16 @@ func TestReadOnlyStreak_ExecNeutral(t *testing.T) { var s toolLoopState // 5 reads → streak = 5 for range 5 { - s.recordMutation("read_file") + s.recordMutation("read_file", nil) } // exec does not reset or increment - s.recordMutation("exec") + s.recordMutation("exec", nil) 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") + s.recordMutation("list_files", nil) } if s.readOnlyStreak != 10 { t.Fatalf("expected streak 10, got %d", s.readOnlyStreak) @@ -175,20 +574,20 @@ func TestReadOnlyStreak_MCPNeutral(t *testing.T) { var s toolLoopState // 5 reads → streak = 5 for range 5 { - s.recordMutation("read_file") + s.recordMutation("read_file", nil) } // MCP tools should not reset or increment (same as exec) - s.recordMutation("mcp_gmail__query_gmail_emails") + s.recordMutation("mcp_gmail__query_gmail_emails", nil) if s.readOnlyStreak != 5 { t.Fatalf("expected streak 5 after mcp tool, got %d", s.readOnlyStreak) } - s.recordMutation("mcp_gmail__get_gmail_email") + s.recordMutation("mcp_gmail__get_gmail_email", nil) if s.readOnlyStreak != 5 { t.Fatalf("expected streak 5 after second mcp tool, got %d", s.readOnlyStreak) } - // 7 more reads → streak = 12, should hit critical + // 7 more reads → streak = 12, should hit critical (stuck mode since nil args → unique=1) for range 7 { - s.recordMutation("list_files") + s.recordMutation("list_files", nil) } if s.readOnlyStreak != 12 { t.Fatalf("expected streak 12, got %d", s.readOnlyStreak) @@ -199,7 +598,7 @@ func TestReadOnlyStreak_MCPOnlyNeverTriggers(t *testing.T) { var s toolLoopState // 20 consecutive MCP tool calls → streak should stay 0 for range 20 { - s.recordMutation("mcp_gmail__query_gmail_emails") + s.recordMutation("mcp_gmail__query_gmail_emails", nil) } if s.readOnlyStreak != 0 { t.Fatalf("expected streak 0 after 20 mcp-only calls, got %d", s.readOnlyStreak) @@ -210,7 +609,7 @@ func TestReadOnlyStreak_MCPOnlyNeverTriggers(t *testing.T) { } } -// --- Same-result cross-args detection --- +// ===== Layer 2: Same-result cross-args detection ===== func TestSameResult_Warning(t *testing.T) { var s toolLoopState @@ -271,3 +670,60 @@ func TestHashToolCall(t *testing.T) { t.Fatal("different tools should have different hashes") } } + +// ===== Group D: Interaction between uniqueness and team_tasks ===== + +func TestReadOnlyStreak_ProgressBetweenReads(t *testing.T) { + // D1: reads → team_tasks(progress) → more reads → streak continues + var s toolLoopState + for i := range 5 { + s.recordMutation("read_file", map[string]any{"path": fmt.Sprintf("/file%d.txt", i)}) + } + s.recordMutation("team_tasks", map[string]any{"action": "progress", "percent": 30}) + // Streak should stay at 5 (progress is neutral) + if s.readOnlyStreak != 5 { + t.Fatalf("expected streak=5 after progress, got %d", s.readOnlyStreak) + } + for i := range 5 { + s.recordMutation("read_file", map[string]any{"path": fmt.Sprintf("/more%d.txt", i)}) + } + // Streak=10, unique=10 → exploration mode, below warning + if s.readOnlyStreak != 10 { + t.Fatalf("expected streak=10, got %d", s.readOnlyStreak) + } + if s.readOnlyUnique != 10 { + t.Fatalf("expected unique=10, got %d", s.readOnlyUnique) + } +} + +func TestReadOnlyStreak_ListBetweenReads(t *testing.T) { + // D2: reads → team_tasks(list) → more reads → streak continues (list is read-only) + var s toolLoopState + for i := range 5 { + s.recordMutation("read_file", map[string]any{"path": fmt.Sprintf("/file%d.txt", i)}) + } + s.recordMutation("team_tasks", map[string]any{"action": "list"}) + // team_tasks(list) is read-only → streak=6 + if s.readOnlyStreak != 6 { + t.Fatalf("expected streak=6 after team_tasks(list), got %d", s.readOnlyStreak) + } +} + +func TestReadOnlyStreak_CreateBetweenReads(t *testing.T) { + // D3: reads → team_tasks(create) → more reads → new streak from 0 + var s toolLoopState + for i := range 5 { + s.recordMutation("read_file", map[string]any{"path": fmt.Sprintf("/file%d.txt", i)}) + } + s.recordMutation("team_tasks", map[string]any{"action": "create", "subject": "task"}) + if s.readOnlyStreak != 0 { + t.Fatalf("expected streak=0 after create, got %d", s.readOnlyStreak) + } + // New reads start fresh + for range 3 { + s.recordMutation("read_file", map[string]any{"path": "/new.txt"}) + } + if s.readOnlyStreak != 3 { + t.Fatalf("expected streak=3, got %d", s.readOnlyStreak) + } +}