mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 06:17:41 +00:00
feat: Add tool loop detection, negative context injection, and fix Docker workspace permissions
- Add tool loop detection (toolloop.go): tracks repeated no-progress tool calls using SHA256 hashing of args+results. Warning at 5 identical calls, force stop at 10. Prevents Gemini models from burning tokens in infinite loops. - Inject AVAILABILITY.md negative context when agent has no team/delegation targets, so models don't waste iterations probing unavailable capabilities. - Fix Dockerfile: create /app/.goclaw directory so Docker volume initializes with correct goclaw:goclaw ownership instead of root:root. - Update collapseToolCallsWithoutSig comments for clarity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
1 parent
40425d3ad2
commit
42263e5cc5
9 files changed
+331
-13
No files matched your search
+1
-1
@@ -52,7 +52,7 @@ COPY docker-entrypoint.sh /app/docker-entrypoint.sh
|
||||
RUN chmod +x /app/docker-entrypoint.sh
|
||||
|
||||
# Create data directories (owned by goclaw user)
|
||||
RUN mkdir -p /app/workspace /app/data /app/sessions /app/skills /app/tsnet-state \
|
||||
RUN mkdir -p /app/workspace /app/data /app/sessions /app/skills /app/tsnet-state /app/.goclaw \
|
||||
&& chown -R goclaw:goclaw /app
|
||||
|
||||
# Default environment
|
||||
|
||||
@@ -494,6 +494,7 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error)
|
||||
})
|
||||
|
||||
// 4. Run LLM iteration loop
|
||||
var loopDetector toolLoopState // detects repeated no-progress tool calls
|
||||
var totalUsage providers.Usage
|
||||
iteration := 0
|
||||
var finalContent string
|
||||
@@ -600,11 +601,16 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error)
|
||||
argsJSON, _ := json.Marshal(tc.Arguments)
|
||||
slog.Info("tool call", "agent", l.id, "tool", tc.Name, "args_len", len(argsJSON))
|
||||
|
||||
argsHash := loopDetector.record(tc.Name, tc.Arguments)
|
||||
|
||||
toolSpanStart := time.Now().UTC()
|
||||
result := l.tools.ExecuteWithContext(ctx, tc.Name, tc.Arguments, req.Channel, req.ChatID, req.PeerKind, req.SessionKey, nil)
|
||||
|
||||
l.emitToolSpan(ctx, toolSpanStart, tc.Name, tc.ID, string(argsJSON), result)
|
||||
|
||||
// Record result for loop detection.
|
||||
loopDetector.recordResult(argsHash, result.ForLLM)
|
||||
|
||||
if result.Async {
|
||||
asyncToolCalls = append(asyncToolCalls, tc.Name)
|
||||
}
|
||||
@@ -640,6 +646,18 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error)
|
||||
}
|
||||
messages = append(messages, toolMsg)
|
||||
pendingMsgs = append(pendingMsgs, toolMsg)
|
||||
|
||||
// Check for tool call loop after recording result.
|
||||
if level, msg := loopDetector.detect(tc.Name, argsHash); level != "" {
|
||||
if level == "critical" {
|
||||
slog.Warn("tool loop critical", "agent", l.id, "tool", tc.Name, "message", msg)
|
||||
finalContent = "I was unable to complete this task — I got stuck repeatedly calling " + tc.Name + " without making progress. Please try rephrasing your request."
|
||||
break
|
||||
}
|
||||
// Warning: inject message so model knows to change strategy.
|
||||
slog.Warn("tool loop warning", "agent", l.id, "tool", tc.Name, "message", msg)
|
||||
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.
|
||||
@@ -693,9 +711,14 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error)
|
||||
})
|
||||
|
||||
// 5. Process results sequentially: emit events, append messages, save to session
|
||||
var loopStuck bool
|
||||
for _, r := range collected {
|
||||
l.emitToolSpan(ctx, r.spanStart, r.tc.Name, r.tc.ID, r.argsJSON, r.result)
|
||||
|
||||
// Record for loop detection.
|
||||
argsHash := loopDetector.record(r.tc.Name, r.tc.Arguments)
|
||||
loopDetector.recordResult(argsHash, r.result.ForLLM)
|
||||
|
||||
if r.result.Async {
|
||||
asyncToolCalls = append(asyncToolCalls, r.tc.Name)
|
||||
}
|
||||
@@ -731,6 +754,21 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error)
|
||||
}
|
||||
messages = append(messages, toolMsg)
|
||||
pendingMsgs = append(pendingMsgs, toolMsg)
|
||||
|
||||
// Check for tool call loop.
|
||||
if level, msg := loopDetector.detect(r.tc.Name, argsHash); level != "" {
|
||||
if level == "critical" {
|
||||
slog.Warn("tool loop critical", "agent", l.id, "tool", r.tc.Name, "message", msg)
|
||||
finalContent = "I was unable to complete this task — I got stuck repeatedly calling " + r.tc.Name + " without making progress. Please try rephrasing your request."
|
||||
loopStuck = true
|
||||
break
|
||||
}
|
||||
slog.Warn("tool loop warning", "agent", l.id, "tool", r.tc.Name, "message", msg)
|
||||
messages = append(messages, providers.Message{Role: "user", Content: msg})
|
||||
}
|
||||
}
|
||||
if loopStuck {
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -102,6 +102,7 @@ func NewManagedResolver(deps ResolverDeps) ResolverFunc {
|
||||
// Inject DELEGATION.md from delegation links (only if not already present in DB).
|
||||
// Uses DELEGATION.md (not AGENTS.md) to avoid collision with per-user AGENTS.md
|
||||
// which contains workspace instructions for open agents.
|
||||
hasDelegation := false
|
||||
if deps.AgentLinkStore != nil {
|
||||
hasDelegationMD := false
|
||||
for _, cf := range contextFiles {
|
||||
@@ -118,23 +119,28 @@ func NewManagedResolver(deps ResolverDeps) ResolverFunc {
|
||||
targets := filterManualLinks(allTargets)
|
||||
if len(targets) > 0 && len(targets) <= 15 {
|
||||
// Static list: all targets directly
|
||||
hasDelegation = true
|
||||
contextFiles = append(contextFiles, bootstrap.ContextFile{
|
||||
Path: bootstrap.DelegationFile,
|
||||
Content: buildDelegateAgentsMD(targets),
|
||||
})
|
||||
} else if len(targets) > 15 {
|
||||
// Too many targets: instruct agent to use delegate_search tool
|
||||
hasDelegation = true
|
||||
contextFiles = append(contextFiles, bootstrap.ContextFile{
|
||||
Path: bootstrap.DelegationFile,
|
||||
Content: buildDelegateSearchInstruction(len(targets)),
|
||||
})
|
||||
}
|
||||
}
|
||||
} else {
|
||||
hasDelegation = true
|
||||
}
|
||||
}
|
||||
|
||||
// Inject TEAM.md for all team members (lead + members) so every agent
|
||||
// knows the team workflow: create/claim/complete tasks via team_tasks tool.
|
||||
hasTeam := false
|
||||
if deps.TeamStore != nil {
|
||||
hasTeamMD := false
|
||||
for _, cf := range contextFiles {
|
||||
@@ -146,15 +152,34 @@ func NewManagedResolver(deps ResolverDeps) ResolverFunc {
|
||||
if !hasTeamMD {
|
||||
if team, err := deps.TeamStore.GetTeamForAgent(ctx, ag.ID); err == nil && team != nil {
|
||||
if members, err := deps.TeamStore.ListMembers(ctx, team.ID); err == nil {
|
||||
hasTeam = true
|
||||
contextFiles = append(contextFiles, bootstrap.ContextFile{
|
||||
Path: bootstrap.TeamFile,
|
||||
Content: buildTeamMD(team, members, ag.ID),
|
||||
})
|
||||
}
|
||||
}
|
||||
} else {
|
||||
hasTeam = true
|
||||
}
|
||||
}
|
||||
|
||||
// Inject negative context so the model doesn't waste iterations probing
|
||||
// unavailable capabilities (team_tasks, delegate_search, etc.).
|
||||
if !hasTeam || !hasDelegation {
|
||||
var notes []string
|
||||
if !hasTeam {
|
||||
notes = append(notes, "You are NOT part of any team. Do not use team_tasks or team_message tools.")
|
||||
}
|
||||
if !hasDelegation {
|
||||
notes = append(notes, "You have NO delegation targets. Do not use delegate or delegate_search tools.")
|
||||
}
|
||||
contextFiles = append(contextFiles, bootstrap.ContextFile{
|
||||
Path: "AVAILABILITY.md",
|
||||
Content: strings.Join(notes, "\n"),
|
||||
})
|
||||
}
|
||||
|
||||
contextWindow := ag.ContextWindow
|
||||
if contextWindow <= 0 {
|
||||
contextWindow = 200000
|
||||
|
||||
@@ -104,15 +104,15 @@ func buildProjectContextSection(files []bootstrap.ContextFile) []string {
|
||||
for _, f := range files {
|
||||
base := filepath.Base(f.Path)
|
||||
|
||||
// During bootstrap (first run), skip delegation/team files — they add noise
|
||||
// During bootstrap (first run), skip delegation/team/availability files — they add noise
|
||||
// and waste tokens when the agent should only be introducing itself.
|
||||
if hasBootstrap && (base == bootstrap.DelegationFile || base == bootstrap.TeamFile) {
|
||||
if hasBootstrap && (base == bootstrap.DelegationFile || base == bootstrap.TeamFile || base == "AVAILABILITY.md") {
|
||||
continue
|
||||
}
|
||||
|
||||
// Virtual files (DELEGATION.md, TEAM.md) are system-injected, not on disk.
|
||||
// Virtual files (DELEGATION.md, TEAM.md, AVAILABILITY.md) are system-injected, not on disk.
|
||||
// Render with <system_context> so the LLM doesn't try to read/write them as files.
|
||||
if base == bootstrap.DelegationFile || base == bootstrap.TeamFile {
|
||||
if base == bootstrap.DelegationFile || base == bootstrap.TeamFile || base == "AVAILABILITY.md" {
|
||||
lines = append(lines,
|
||||
fmt.Sprintf("<system_context name=%q>", base),
|
||||
f.Content,
|
||||
|
||||
@@ -0,0 +1,137 @@
|
||||
package agent
|
||||
|
||||
import (
|
||||
"crypto/sha256"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"sort"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// Tool loop detection thresholds (per-run, not per-session).
|
||||
const (
|
||||
toolLoopHistorySize = 30
|
||||
toolLoopWarningThreshold = 5 // inject warning into conversation
|
||||
toolLoopCriticalThreshold = 10 // force stop the iteration loop
|
||||
)
|
||||
|
||||
// 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
|
||||
}
|
||||
|
||||
type toolCallRecord struct {
|
||||
toolName string
|
||||
argsHash string
|
||||
resultHash string // empty until result is recorded
|
||||
}
|
||||
|
||||
// record adds a tool call to history and returns its argsHash.
|
||||
func (s *toolLoopState) record(toolName string, args map[string]interface{}) string {
|
||||
h := hashToolCall(toolName, args)
|
||||
s.history = append(s.history, toolCallRecord{
|
||||
toolName: toolName,
|
||||
argsHash: h,
|
||||
})
|
||||
if len(s.history) > toolLoopHistorySize {
|
||||
s.history = s.history[len(s.history)-toolLoopHistorySize:]
|
||||
}
|
||||
return h
|
||||
}
|
||||
|
||||
// recordResult updates the most recent matching record with the result hash.
|
||||
func (s *toolLoopState) recordResult(argsHash, resultContent string) {
|
||||
rh := hashResult(resultContent)
|
||||
// Walk backward to find the latest record with matching argsHash and no result yet.
|
||||
for i := len(s.history) - 1; i >= 0; i-- {
|
||||
rec := &s.history[i]
|
||||
if rec.argsHash == argsHash && rec.resultHash == "" {
|
||||
rec.resultHash = rh
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// detect checks for repeated no-progress tool calls.
|
||||
// Returns level ("warning", "critical", or "") and a human-readable message.
|
||||
func (s *toolLoopState) detect(toolName string, argsHash string) (level, message string) {
|
||||
if len(s.history) < toolLoopWarningThreshold {
|
||||
return "", ""
|
||||
}
|
||||
|
||||
// Count records with identical argsHash AND identical non-empty resultHash.
|
||||
// This ensures we only flag true no-progress loops (same input → same output).
|
||||
var noProgressCount int
|
||||
var lastResultHash string
|
||||
|
||||
for i := len(s.history) - 1; i >= 0; i-- {
|
||||
rec := s.history[i]
|
||||
if rec.argsHash != argsHash {
|
||||
continue
|
||||
}
|
||||
if rec.resultHash == "" {
|
||||
continue // incomplete record, skip
|
||||
}
|
||||
if lastResultHash == "" {
|
||||
lastResultHash = rec.resultHash
|
||||
}
|
||||
if rec.resultHash == lastResultHash {
|
||||
noProgressCount++
|
||||
}
|
||||
}
|
||||
|
||||
if noProgressCount >= toolLoopCriticalThreshold {
|
||||
return "critical", fmt.Sprintf(
|
||||
"CRITICAL: %s has been called %d times with identical arguments and results. "+
|
||||
"Stopping to prevent runaway loop.", toolName, noProgressCount)
|
||||
}
|
||||
|
||||
if noProgressCount >= toolLoopWarningThreshold {
|
||||
return "warning", fmt.Sprintf(
|
||||
"[System: WARNING — %s has been called %d times with the same arguments and identical results. "+
|
||||
"This is not making progress. Try a completely different approach, use different tools, "+
|
||||
"or respond directly to the user with what you know.]", toolName, noProgressCount)
|
||||
}
|
||||
|
||||
return "", ""
|
||||
}
|
||||
|
||||
// hashToolCall produces a deterministic hash of tool name + arguments.
|
||||
func hashToolCall(toolName string, args map[string]interface{}) string {
|
||||
s := toolName + ":" + stableJSON(args)
|
||||
h := sha256.Sum256([]byte(s))
|
||||
return fmt.Sprintf("%x", h[:16]) // 32 hex chars, enough for dedup
|
||||
}
|
||||
|
||||
// hashResult produces a hash of tool result content.
|
||||
func hashResult(content string) string {
|
||||
h := sha256.Sum256([]byte(content))
|
||||
return fmt.Sprintf("%x", h[:16])
|
||||
}
|
||||
|
||||
// stableJSON serializes a value with sorted keys for deterministic hashing.
|
||||
func stableJSON(v interface{}) string {
|
||||
switch val := v.(type) {
|
||||
case map[string]interface{}:
|
||||
keys := make([]string, 0, len(val))
|
||||
for k := range val {
|
||||
keys = append(keys, k)
|
||||
}
|
||||
sort.Strings(keys)
|
||||
parts := make([]string, len(keys))
|
||||
for i, k := range keys {
|
||||
parts[i] = fmt.Sprintf("%q:%s", k, stableJSON(val[k]))
|
||||
}
|
||||
return "{" + strings.Join(parts, ",") + "}"
|
||||
case []interface{}:
|
||||
parts := make([]string, len(val))
|
||||
for i, elem := range val {
|
||||
parts[i] = stableJSON(elem)
|
||||
}
|
||||
return "[" + strings.Join(parts, ",") + "]"
|
||||
default:
|
||||
b, _ := json.Marshal(v)
|
||||
return string(b)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,117 @@
|
||||
package agent
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestToolLoopDetection_NoLoop(t *testing.T) {
|
||||
var s toolLoopState
|
||||
|
||||
// 4 identical calls with same result → below threshold, no detection
|
||||
for i := 0; i < 4; i++ {
|
||||
h := s.record("list_files", map[string]interface{}{"path": "."})
|
||||
s.recordResult(h, "access denied")
|
||||
level, _ := s.detect("list_files", h)
|
||||
if level != "" {
|
||||
t.Fatalf("iteration %d: expected no detection, got %q", i, level)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolLoopDetection_Warning(t *testing.T) {
|
||||
var s toolLoopState
|
||||
|
||||
var lastLevel string
|
||||
for i := 0; i < toolLoopWarningThreshold; i++ {
|
||||
h := s.record("list_files", map[string]interface{}{"path": "."})
|
||||
s.recordResult(h, "access denied")
|
||||
lastLevel, _ = s.detect("list_files", h)
|
||||
}
|
||||
if lastLevel != "warning" {
|
||||
t.Fatalf("expected warning after %d calls, got %q", toolLoopWarningThreshold, lastLevel)
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolLoopDetection_Critical(t *testing.T) {
|
||||
var s toolLoopState
|
||||
|
||||
var lastLevel string
|
||||
for i := 0; i < toolLoopCriticalThreshold; i++ {
|
||||
h := s.record("list_files", map[string]interface{}{"path": "."})
|
||||
s.recordResult(h, "access denied")
|
||||
lastLevel, _ = s.detect("list_files", h)
|
||||
}
|
||||
if lastLevel != "critical" {
|
||||
t.Fatalf("expected critical after %d calls, got %q", toolLoopCriticalThreshold, lastLevel)
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolLoopDetection_DifferentArgs(t *testing.T) {
|
||||
var s toolLoopState
|
||||
|
||||
// Same tool but different args each time → no detection
|
||||
for i := 0; i < 15; i++ {
|
||||
args := map[string]interface{}{"path": string(rune('a' + i))}
|
||||
h := s.record("list_files", args)
|
||||
s.recordResult(h, "access denied")
|
||||
level, _ := s.detect("list_files", h)
|
||||
if level != "" {
|
||||
t.Fatalf("iteration %d: expected no detection for different args, got %q", i, level)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolLoopDetection_DifferentResults(t *testing.T) {
|
||||
var s toolLoopState
|
||||
|
||||
// Same args but different results each time → progress, no detection
|
||||
for i := 0; i < 15; i++ {
|
||||
h := s.record("web_fetch", map[string]interface{}{"url": "https://example.com"})
|
||||
s.recordResult(h, "result content "+string(rune('a'+i)))
|
||||
level, _ := s.detect("web_fetch", h)
|
||||
if level != "" {
|
||||
t.Fatalf("iteration %d: expected no detection for different results, got %q", i, level)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestToolLoopDetection_MixedTools(t *testing.T) {
|
||||
var s toolLoopState
|
||||
|
||||
// Alternate between two tools with same result → each tool only hit ~half
|
||||
for i := 0; i < 15; i++ {
|
||||
toolName := "list_files"
|
||||
if i%2 == 1 {
|
||||
toolName = "read_file"
|
||||
}
|
||||
h := s.record(toolName, map[string]interface{}{"path": "."})
|
||||
s.recordResult(h, "error")
|
||||
level, _ := s.detect(toolName, h)
|
||||
// Each tool is only called ~7-8 times, should at most warn
|
||||
if level == "critical" {
|
||||
t.Fatalf("iteration %d: unexpected critical for alternating tools", i)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestStableJSON(t *testing.T) {
|
||||
// Same keys in different order → same hash
|
||||
a := stableJSON(map[string]interface{}{"b": 2, "a": 1})
|
||||
b := stableJSON(map[string]interface{}{"a": 1, "b": 2})
|
||||
if a != b {
|
||||
t.Fatalf("stableJSON not deterministic: %q != %q", a, b)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHashToolCall(t *testing.T) {
|
||||
// Same input → same hash
|
||||
h1 := hashToolCall("list_files", map[string]interface{}{"path": "."})
|
||||
h2 := hashToolCall("list_files", map[string]interface{}{"path": "."})
|
||||
if h1 != h2 {
|
||||
t.Fatal("hashToolCall not deterministic")
|
||||
}
|
||||
|
||||
// Different tool → different hash
|
||||
h3 := hashToolCall("read_file", map[string]interface{}{"path": "."})
|
||||
if h1 == h3 {
|
||||
t.Fatal("different tools should have different hashes")
|
||||
}
|
||||
}
|
||||
@@ -188,9 +188,10 @@ func (p *OpenAIProvider) ChatStream(ctx context.Context, req ChatRequest, onChun
|
||||
}
|
||||
|
||||
func (p *OpenAIProvider) buildRequestBody(model string, req ChatRequest, stream bool) map[string]interface{} {
|
||||
// Gemini 2.5+: collapse old tool_call cycles missing thought_signature into plain text.
|
||||
// Old session messages stored before the thought_signature fix don't have it,
|
||||
// and Gemini rejects them with HTTP 400. Collapse preserves context as text.
|
||||
// Gemini 2.5+: collapse tool_call cycles missing thought_signature.
|
||||
// Gemini requires thought_signature echoed back on every tool_call; models that
|
||||
// don't return it (e.g. gemini-3-flash) will cause HTTP 400 if sent as-is.
|
||||
// Tool results are folded into plain user messages to preserve context.
|
||||
inputMessages := req.Messages
|
||||
if strings.Contains(strings.ToLower(p.name), "gemini") {
|
||||
inputMessages = collapseToolCallsWithoutSig(inputMessages)
|
||||
|
||||
@@ -3,8 +3,8 @@ package providers
|
||||
import "strings"
|
||||
|
||||
// collapseToolCallsWithoutSig rewrites tool_call cycles that lack thought_signature
|
||||
// (required by Gemini 2.5+). Old session history stored before the thought_signature
|
||||
// capture fix doesn't have it, and Gemini rejects those messages with HTTP 400.
|
||||
// (required by Gemini 2.5+). Gemini requires thought_signature echoed back on every
|
||||
// tool_call; models that don't return it cause HTTP 400 if sent as-is.
|
||||
//
|
||||
// The assistant's tool_calls are stripped, and the corresponding tool-result messages
|
||||
// are folded into a single user message with the tool output content. This preserves
|
||||
|
||||
@@ -21,9 +21,9 @@ type Provider interface {
|
||||
|
||||
// ChatRequest contains the input for a Chat/ChatStream call.
|
||||
type ChatRequest struct {
|
||||
Messages []Message `json:"messages"`
|
||||
Tools []ToolDefinition `json:"tools,omitempty"`
|
||||
Model string `json:"model,omitempty"`
|
||||
Messages []Message `json:"messages"`
|
||||
Tools []ToolDefinition `json:"tools,omitempty"`
|
||||
Model string `json:"model,omitempty"`
|
||||
Options map[string]interface{} `json:"options,omitempty"`
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user