fix: prevent infinite loop in performAgentChain on repeating tool calls#178
fix: prevent infinite loop in performAgentChain on repeating tool calls#178mason5052 wants to merge 2 commits intovxcontrol:masterfrom
Conversation
The performAgentChain loop has no iteration cap, allowing infinite loops when a model repeatedly calls the same tool. The repeating detector returns a message (not an error), so the loop never breaks. Add two safety mechanisms: - Hard cap of 100 iterations on the main agent chain loop - Escalation to error after 5 consecutive repeating detections (8 total identical calls: 3 before first detection + 5 detections) The soft "please try another tool" response is preserved for the first 4 detections, giving the LLM a chance to course-correct before aborting. Closes vxcontrol#175 Signed-off-by: mason5052 <ehehwnwjs5052@gmail.com>
There was a problem hiding this comment.
Pull request overview
Prevents runaway performAgentChain executions in backend/pkg/providers/performer.go by adding hard stops for excessive looping and repeated tool-call patterns, addressing resource exhaustion described in #175.
Changes:
- Add a maximum iteration cap to the main
performAgentChainloop. - Escalate repeating tool-call detections from a soft warning to a terminating error after a configured threshold.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
backend/pkg/providers/performer.go
Outdated
| if len(detector.funcCalls) >= RepeatingToolCallThreshold+maxRepeatingDetectionsBeforeErr-1 { | ||
| errMsg := fmt.Sprintf("tool '%s' repeated %d times consecutively, aborting chain", funcName, len(detector.funcCalls)) | ||
| logger.WithField("repeat_count", len(detector.funcCalls)).Error(errMsg) | ||
| return "", fmt.Errorf("%s", errMsg) |
There was a problem hiding this comment.
Similar to the iteration-cap error, fmt.Errorf("%s", errMsg) is redundant formatting. Consider using errors.New(errMsg) (or fmt.Errorf(errMsg)) to keep error construction idiomatic.
| return "", fmt.Errorf("%s", errMsg) | |
| return "", errors.New(errMsg) |
There was a problem hiding this comment.
Agreed. Switched to errors.New(errMsg) in the follow-up commit (917b371).
backend/pkg/providers/performer.go
Outdated
| if iteration >= maxAgentChainIterations { | ||
| msg := fmt.Sprintf("agent chain exceeded maximum iterations (%d)", maxAgentChainIterations) | ||
| logger.WithField("iteration", iteration).Error(msg) | ||
| return fmt.Errorf("%s", msg) |
There was a problem hiding this comment.
fmt.Errorf("%s", msg) is redundant and loses the opportunity to use the more idiomatic errors.New(msg) (or fmt.Errorf(msg) when no formatting is needed). Consider switching to one of those to keep error creation consistent and avoid unnecessary formatting.
| return fmt.Errorf("%s", msg) | |
| return errors.New(msg) |
There was a problem hiding this comment.
Good catch. Switched to errors.New(msg) in the follow-up commit (917b371). Since msg is already a fully formatted string, errors.New is the right choice here.
| maxAgentChainIterations = 100 | ||
| maxRepeatingDetectionsBeforeErr = 5 | ||
| delayBetweenRetries = 5 * time.Second |
There was a problem hiding this comment.
The constant name maxRepeatingDetectionsBeforeErr is a bit ambiguous because the code escalates on the Nth repeating detection (via the -1 in the threshold math), not after N detections. Consider renaming it to something like “...ToAbort/ToError” or adjusting the condition so the name matches the behavior.
There was a problem hiding this comment.
Good point on the naming ambiguity. Renamed to maxSoftDetectionsBeforeAbort = 4 and removed the -1 from the condition. Now the math is straightforward: RepeatingToolCallThreshold(3) + maxSoftDetectionsBeforeAbort(4) = 7, and the name accurately describes the behavior (4 soft detection responses before abort). Fixed in 917b371.
backend/pkg/providers/performer.go
Outdated
| if len(detector.funcCalls) >= RepeatingToolCallThreshold+maxRepeatingDetectionsBeforeErr-1 { | ||
| errMsg := fmt.Sprintf("tool '%s' repeated %d times consecutively, aborting chain", funcName, len(detector.funcCalls)) | ||
| logger.WithField("repeat_count", len(detector.funcCalls)).Error(errMsg) |
There was a problem hiding this comment.
With RepeatingToolCallThreshold = 3 (helpers.go), the current condition aborts when len(detector.funcCalls) >= 3 + maxRepeatingDetectionsBeforeErr - 1, i.e. on the 7th consecutive identical call when maxRepeatingDetectionsBeforeErr is 5. The PR description mentions “8 total identical calls”; if that’s the intended behavior, the threshold math here needs to be adjusted (or the description updated).
There was a problem hiding this comment.
You're right -- the description mentioned "8 total identical calls" which was inaccurate. The abort triggers on the 7th consecutive identical call. Timeline: calls 1-2 build up, call 3 triggers first detection, calls 3-6 get soft warnings (4 soft detections), call 7 aborts. Updated the constant naming and math in 917b371 to make this clearer, and will update the PR description.
- Rename maxRepeatingDetectionsBeforeErr to maxSoftDetectionsBeforeAbort
for clarity (name now matches behavior: 4 soft warnings before abort)
- Adjust threshold value from 5 to 4 and remove -1 from condition
(same runtime behavior: abort on 7th consecutive identical call)
- Use errors.New() instead of fmt.Errorf("%s", msg) for non-formatted
error strings (more idiomatic Go)
Signed-off-by: mason5052 <ehehwnwjs5052@gmail.com>
Description of the Change
Problem
The
performAgentChainloop inperformer.gois an unboundedfor {}with no iteration cap. When a model repeatedly calls the same tool, therepeatingDetectorfires and returns a message (nilerror), so the loop never breaks. This can result in 4,800+ iterations in a single session, consuming resources indefinitely.Closes #175
Solution
Add two safety mechanisms:
Iteration cap (
maxAgentChainIterations = 100): Hard limit on the main loop. Normal pentest flows use far fewer iterations; 100 is generous while preventing runaway loops.Repeating escalation (
maxSoftDetectionsBeforeAbort = 4): After 4 consecutive soft detection warnings (7 total identical calls), escalate from a soft message to an actual error that terminates the chain. The existing soft response is preserved for the first 4 detections, giving the LLM a chance to course-correct.Type of Change
Areas Affected
Testing and Verification
Test Configuration
Test Steps
maxRetriesToCallSimpleChain, etc.)for iteration := 0; ; iteration++preserves all existing loop semanticslen(detector.funcCalls)is accessible (same package) and correctly tracks consecutive identical calls (resets on different calls perhelpers.go:detect())RepeatingToolCallThreshold(3) + maxSoftDetectionsBeforeAbort(4) = 7, so error fires on the 7th consecutive identical callSecurity Considerations
No security impact. This is a resource exhaustion prevention fix.
Checklist
go fmtandgo vet