Feature request: improve agent looping detection #3479
pshickeydev
started this conversation in
Ideas
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
A common frustration I have with the agentic paradigm is an agent trying the same unsuccessful approach to a problem. Crush already detects loops, but this doesn't catch more subtle cases like a UI fix not working which doesn't surface an error code. I'm not super clear on a great solution to this problem and want to open a discussion for other input. I had Kimi K3 investigate to kick things off and this may be possible to some extent with user defined hooks. I'll include that below for reference.
Loop-Detection Investigation & Plan
What already exists
Crush already has one loop detector, added in
af86738e(fix #2130/#2214):internal/agent/loop_detection.go—hasRepeatedToolCalls(steps, windowSize=10, maxRepeats=5)hashes each step's tool interactions (tool name + input + output, SHA-256) and returns true when any identical signature appears >5 times within the last 10 steps.StopConditionatinternal/agent/agent.go:1058-1060. When it fires, the stream simply ends; the last assistant message getsFinishReasonToolUse→ treated as end-of-turn. There is no user-visible signal that a loop was detected — the turn just stops, indistinguishable from the agent finishing.Why it misses the frustrating scenarios
The current detector is exact-match only. The common stuck cases escape it:
editwith tweakedold_string) produces a different signature every time. Same forbashretries with reordered flags.Intervention points available
StopWhenconditionsagent.go:1036-1061[]fantasy.StepResulthistory (calls, inputs, outputs); stop the turn. Cheap, deterministic.OnStepFinishagent.go:983PrepareStepagent.go:808-879prepared.Messagesbefore each model call — e.g. a system nudge "you've attempted X 3 times, change approach" — instead of hard-stopping.hookedTool.Runhooked_tool.go:54-100internal/hooks/(onlyEventPreToolUseexists)FinishReason+ UI bannerinternal/message/content.go:36-48loop_detected) would let the TUI show why the turn stopped.Proposed design (layered)
1. Extend detection beyond exact-match
Replace/augment
loop_detection.gowith escalating signature levels:Coarser signatures get lower thresholds to compensate for higher false-positive risk.
2. Escalating response instead of binary stop
Via
PrepareStepmessage injection:3. Surface it to the user
FinishReasonLoopDetected(or useAddFinishwith a message) and render a banner in the TUI, so a stopped turn explains itself. Currently the stop is silent.4. Configurability
Both config formats, deep-merged through the same pipeline.
crush.json(existing format, unchanged schema path):{ "options": { "loop_detection": { "enabled": true, "window_size": 10, "max_repeats": 5, "soft_nudge_threshold": 3, "detect_cycles": true } } }crushrc(Bash format, via theoptionbuiltin ininternal/shellconfig/options.go):Implementation follows the existing
shellconfigpatterns:LoopDetection *LoopDetectionConfigjson:"loop_detection,omitempty"`` to the options struct ininternal/config/config.go(with jsonschema descriptions so it appears in the JSON schema), keeping `enabled` as a positive user-facing key mapped to a negatively-stored field only if the config convention requires it (see `disable_metrics` vs `metrics` at `internal/shellconfig/options.go:22-24`).optionSpecs(internal/shellconfig/options.go:180):loop-detectionandloop-detection-cyclesasoptBool;loop-detection-window-size,loop-detection-max-repeats, andloop-detection-soft-nudge-thresholdneed a newoptIntkind (the spec table currently has only string/bool/list atoptions.go:155-159) so values are validated and stored as numbers, not strings.loop_detectionmap underoptions, the wayattribution-*keys write into a nested map viachildMap(options.go:87-107) — either via achildfield onoptionSpecor as special cases inhandleOption.internal/shellconfig/options_test.gocases for each new key (including bool default-to-true shorthand and int validation errors), mirroring the existing table tests.5. Optional: PostToolUse hook event
Today only
EventPreToolUseexists, which can't see tool outcomes. APostToolUseevent would let users implement their own detection with full call+result context.Suggested starting point
The highest-value, lowest-risk change is (3) surfacing the existing detector plus (2) the soft-nudge injection via
PrepareStep— both reuse existing mechanisms with no new infrastructure. Level-(b)/(c) signatures and cycle detection are incremental extensions to the well-testedhasRepeatedToolCalls.All reactions