mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 10:18:42 +00:00
fix(agent): uniqueness-aware loop detector + auto-fail killed team tasks (#506)
Read-only streak detector was a dumb counter that killed agents exploring unique files (12 reads = kill regardless of uniqueness). Team tasks were then falsely auto-completed with the kill message as "result." Changes: - Add uniqueness ratio to read-only streak: exploration mode (>60% unique) uses relaxed thresholds (warn=24, kill=36) vs stuck mode (warn=8, kill=12) - Classify team_tasks by action: list/get/search=read-only, progress=neutral, create/complete/cancel/comment/etc.=mutating - Add LoopKilled flag to RunResult, propagated through all 3 kill paths - Consumer routes LoopKilled to FailTask (not CompleteTask) with clear signal - 40 tests covering uniqueness detection, team_tasks classification, exact #506 trace replay, boundary values, and edge cases
This commit is contained in:
1 parent
8b5a05a4b7
commit
40e32cf239
6 files changed
+614
-30
No files matched your search
@@ -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.
|
||||
|
||||
@@ -204,5 +204,6 @@ func (l *Loop) finalizeRun(
|
||||
Deliverables: rs.deliverables,
|
||||
BlockReplies: rs.blockReplies,
|
||||
LastBlockReply: rs.lastBlockReply,
|
||||
LoopKilled: rs.loopKilled,
|
||||
}
|
||||
}
|
||||
@@ -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",
|
||||
|
||||
@@ -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
|
||||
}
|
||||
+100
-11
@@ -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 "", ""
|
||||
}
|
||||
|
||||
+473
-17
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user