From e73a73758d8f6260e4d65345cf7fa0b542bcf2f4 Mon Sep 17 00:00:00 2001 From: Hamza Imran Date: Wed, 21 Oct 2026 03:06:50 +0500 Subject: [PATCH] fix(agent): detect non-consecutive tool call loops (fixes #72) --- agentkit/core/agent.py | 79 ++++++++++++----- agentkit/core/errors.py | 9 +- docs/PROGRESS.md | 8 ++ tests/unit/test_agent_loop_detector.py | 117 +++++++++++++++++++++++++ 4 files changed, 189 insertions(+), 24 deletions(-) diff --git a/agentkit/core/agent.py b/agentkit/core/agent.py index 2c95115..398bc0e 100644 --- a/agentkit/core/agent.py +++ b/agentkit/core/agent.py @@ -51,6 +51,16 @@ class AgentConfig(BaseModel): gt=0, description="Halt run if identical tool calls are repeated consecutively.", ) + max_duplicate_tool_call_cycles: int | None = Field( + default=None, + gt=0, + description="Halt run if a repeating cycle of tool calls is detected this many times. Defaults to max_consecutive_duplicate_tool_calls.", + ) + max_tool_cycle_length: int = Field( + default=5, + gt=0, + description="Maximum cycle length (in tool calls) inspected by the loop detector.", + ) system_prompt: str = Field( default=DEFAULT_SYSTEM_PROMPT, description="System directive prepended to every run.", @@ -192,8 +202,7 @@ async def _finalize_and_return(result: RunResult) -> RunResult: total_input_tokens = 0 total_output_tokens = 0 steps_count = 0 - last_tool_signature: tuple[str, str] | None = None - consecutive_duplicate_count = 0 + tool_call_signatures: list[tuple[str, str]] = [] try: async with asyncio.timeout(self.config.run_timeout_s): @@ -218,33 +227,59 @@ async def _finalize_and_return(result: RunResult) -> RunResult: for call in response.tool_calls: args_json = json.dumps(call.arguments, sort_keys=True, default=str) sig = (call.name, args_json) - if sig == last_tool_signature: - consecutive_duplicate_count += 1 - else: - last_tool_signature = sig - consecutive_duplicate_count = 1 - - if ( - consecutive_duplicate_count - >= self.config.max_consecutive_duplicate_tool_calls - ): + tool_call_signatures.append(sig) + + # Check for repeating cycle or consecutive duplicates + detected_loop: tuple[int, int] | None = None + max_cycle_len = min( + self.config.max_tool_cycle_length, + len(tool_call_signatures), + ) + for plen in range(1, max_cycle_len + 1): + threshold = ( + self.config.max_consecutive_duplicate_tool_calls + if plen == 1 + else ( + self.config.max_duplicate_tool_call_cycles + or self.config.max_consecutive_duplicate_tool_calls + ) + ) + window_len = plen * threshold + if len(tool_call_signatures) >= window_len: + suffix = tool_call_signatures[-window_len:] + pattern = suffix[:plen] + if pattern * threshold == suffix: + detected_loop = (plen, threshold) + break + + if detected_loop is not None: + plen, count = detected_loop duration_ms = max( 0, int((time.perf_counter() - start_time) * 1000) ) - logger.warning( - "Detected duplicate tool call loop for '%s' (%d consecutive calls)", - call.name, - consecutive_duplicate_count, - ) + if plen == 1: + fail_msg = ( + f"Detected duplicate tool call loop: '{call.name}' called " + f"{count} consecutive times with identical arguments: {call.arguments}" + ) + else: + pattern_names = [ + name + for name, _ in tool_call_signatures[ + -plen * count : -plen * (count - 1) + ] + ] + fail_msg = ( + f"Detected duplicate tool call loop: repeating cycle of {plen} tool calls " + f"({' -> '.join(pattern_names)}) repeated {count} times" + ) + logger.warning("%s", fail_msg) res = RunResult( run_id=run_id, session_id=session_id, goal=goal, status=RunStatus.FAILED, - failure_reason=( - f"Detected duplicate tool call loop: '{call.name}' called " - f"{consecutive_duplicate_count} consecutive times with identical arguments: {call.arguments}" - ), + failure_reason=fail_msg, steps_count=steps_count, total_input_tokens=total_input_tokens, total_output_tokens=total_output_tokens, @@ -253,7 +288,7 @@ async def _finalize_and_return(result: RunResult) -> RunResult: await _finalize_and_return(res) if raise_on_failure: raise DuplicateToolCallLoopError( - call.name, consecutive_duplicate_count + call.name, count, cycle_length=plen ) return res diff --git a/agentkit/core/errors.py b/agentkit/core/errors.py index b8dd81b..cbaaf46 100644 --- a/agentkit/core/errors.py +++ b/agentkit/core/errors.py @@ -41,13 +41,18 @@ def __init__(self, timeout_s: int | float) -> None: class DuplicateToolCallLoopError(AgentKitError): """Raised when an agent run enters a repetitive loop calling identical tools with identical arguments.""" - def __init__(self, tool_name: str, count: int) -> None: + def __init__(self, tool_name: str, count: int, cycle_length: int = 1) -> None: + if cycle_length > 1: + msg = f"Detected duplicate tool call loop: repeating cycle of {cycle_length} tool calls repeated {count} times." + else: + msg = f"Detected duplicate tool call loop: '{tool_name}' called {count} consecutive times with identical arguments." super().__init__( - f"Detected duplicate tool call loop: '{tool_name}' called {count} consecutive times with identical arguments.", + msg, code="DUPLICATE_TOOL_CALL_LOOP", ) self.tool_name = tool_name self.count = count + self.cycle_length = cycle_length class AuthenticationError(AgentKitError): diff --git a/docs/PROGRESS.md b/docs/PROGRESS.md index 2c7b115..9d4b74b 100644 --- a/docs/PROGRESS.md +++ b/docs/PROGRESS.md @@ -61,6 +61,7 @@ This document tracks execution progress across all backlog tasks, including pull | AUD-09 | AUD | Live End-to-End Infrastructure Verification & Documentation Proof | `docs/e2e-verification-report` | #53 | 3 | Done | Standardized model, verified live stack, and documented in E2E_REPORT.md | | AUD-10 | AUD | Rate Limiter Outage Fallback & Provider-Valid Session History | `fix/ratelimit-and-session-repair` | #56 | 3 | Done | In-memory fallback on Redis connection failures; synthetic tool results repair aborted sessions | | M13-T06 | M13 | Whole-Project Audit Remediation | `fix/audit-hardening` | #47 | 3 | Done | Fail-closed config and LLM resolution, SSRF post-connect check, atomic fallback | +| AUDIT-15 | AUD | Detect Non-Consecutive Repeating Tool Call Loops | `fix/duplicate-tool-call-detector` | #83 | 1 | Done | Detect repeating cycles of tool calls (A,B,A,B,...); configurable cycle length and threshold | --- @@ -407,3 +408,10 @@ This document tracks execution progress across all backlog tasks, including pull - **PR**: #47 - **Commits**: 3 - **Notes**: Remediated 17 findings from a full-codebase audit, rebased onto the AUD-01/AUD-04/AUD-05 follow-ups. Critical: removed the `ak_test_key_12345` default from `Settings.API_KEYS` and made the field required; replaced the `FakeLLMClient` fallback in `get_llm_client` with a `ServiceUnavailableError` (503) boundary around provider construction; kept the AUD-04 factory-level `RetryingLLMClient` wrapping and made it configurable through new `LLM_MAX_RETRIES`/`LLM_RETRY_BASE_DELAY_S`/`LLM_RETRY_MAX_DELAY_S` settings; made the per-request `Agent` assembly's override explicit as `app.state.agent_override`; forwarded `get_run_trace`/`list_runs` through `TieredMemoryStore`; moved `http_fetch` DNS off the event loop with `asyncio.to_thread`, added a post-connect `server_addr` check against the validated addresses to close the DNS-rebinding TOCTOU window, and replaced full-body buffering with a byte-budgeted stream; replaced the rate limiter's non-scripting fallback with `WATCH`/`MULTI`/`EXEC`, leaving the AUD-10 in-memory fallback for Redis outages intact. Moderate: corrected the `RedisMemoryStore.truncate_messages` off-by-one that returned `max_messages + 1` entries when a system prompt was present; switched generated `tool_call_id`s to full UUIDs; wrapped response parsing in both LLM adapters inside the error boundary so malformed responses and malformed tool-argument JSON raise `LLMProviderError`; guarded `RetryingLLMClient.__getattr__` against recursion on a partially constructed instance; sanitized `/health` details so the unauthenticated endpoint no longer echoes driver error strings; made `sql_readonly` mask string literals and quoted identifiers before its keyword scan, allow `#>`/`#>>`/`#-`, apply the row limit in SQL and push `statement_timeout` to PostgreSQL. Minor: stopped logging raw run goals at INFO (length only, content at DEBUG); replaced `split(";")` in `scripts/seed.py` with a statement-aware splitter handling literals, comments and dollar-quotes; bounded calculator exponentiation by result bit length. Added `tests/unit/test_audit_regressions.py` and a real-Redis concurrency regression test pinning the fallback fix. Recorded in ADR-009, ADR-010, ADR-011. Suite: 298 passed, 25 skipped; coverage 92.86% on core/tools/llm; `ruff check`, `ruff format --check` and `mypy agentkit tests` all clean. + + +### AUDIT-15: Detect Non-Consecutive Repeating Tool Call Loops +- **Status**: Done +- **PR**: #83 +- **Commits**: 1 +- **Notes**: In `agentkit/core/agent.py`, enhanced duplicate tool call detection from only tracking consecutive calls to inspecting sequence history for repeating multi-call cycles (`max_tool_cycle_length=5`, configurable `max_duplicate_tool_call_cycles`). Updated `DuplicateToolCallLoopError` in `agentkit/core/errors.py` with `cycle_length` support while maintaining full backwards compatibility. Added unit tests in `tests/unit/test_agent_loop_detector.py` covering alternating 2-tool loops, 3-tool repeating cycles, and custom repetition thresholds. diff --git a/tests/unit/test_agent_loop_detector.py b/tests/unit/test_agent_loop_detector.py index fbed2ca..2731b59 100644 --- a/tests/unit/test_agent_loop_detector.py +++ b/tests/unit/test_agent_loop_detector.py @@ -254,3 +254,120 @@ def echo_tool(v: int) -> int: interrupted = next(m for m in messages if m.tool_call_id == "c_batch_2") assert "aborted" in interrupted.content.lower() + + +@pytest.mark.asyncio +async def test_repeating_alternating_pattern_detected_as_loop() -> None: + """AUDIT-15: Repeating cycle of non-consecutive tool calls terminates run before max_steps.""" + registry = ToolRegistry() + + @tool(registry=registry) + def calc(n: int) -> int: + """Calculate.""" + return n * 10 + + # Pattern: calc(1), calc(2), calc(1), calc(2), calc(1), calc(2) + responses = [ + LLMResponse(tool_calls=[ToolCall(id="c1", name="calc", arguments={"n": 1})]), + LLMResponse(tool_calls=[ToolCall(id="c2", name="calc", arguments={"n": 2})]), + LLMResponse(tool_calls=[ToolCall(id="c3", name="calc", arguments={"n": 1})]), + LLMResponse(tool_calls=[ToolCall(id="c4", name="calc", arguments={"n": 2})]), + LLMResponse(tool_calls=[ToolCall(id="c5", name="calc", arguments={"n": 1})]), + LLMResponse(tool_calls=[ToolCall(id="c6", name="calc", arguments={"n": 2})]), + ] + llm = FakeLLMClient(responses=responses) + + agent = Agent( + llm=llm, + registry=registry, + config=AgentConfig( + max_steps=20, + max_consecutive_duplicate_tool_calls=3, + ), + ) + + result = await agent.run(goal="Alternating loop test") + + assert result.status == RunStatus.FAILED + assert result.steps_count == 6 + assert "repeating cycle of 2 tool calls" in (result.failure_reason or "") + assert "repeated 3 times" in (result.failure_reason or "") + + +@pytest.mark.asyncio +async def test_repeating_alternating_pattern_raises_when_requested() -> None: + """AUDIT-15: Repeating cycle raises DuplicateToolCallLoopError when raise_on_failure=True.""" + registry = ToolRegistry() + + @tool(registry=registry) + def step_fn(x: int) -> int: + return x + + responses = [ + LLMResponse(tool_calls=[ToolCall(id="c1", name="step_fn", arguments={"x": 1})]), + LLMResponse(tool_calls=[ToolCall(id="c2", name="step_fn", arguments={"x": 2})]), + LLMResponse(tool_calls=[ToolCall(id="c3", name="step_fn", arguments={"x": 1})]), + LLMResponse(tool_calls=[ToolCall(id="c4", name="step_fn", arguments={"x": 2})]), + ] + llm = FakeLLMClient(responses=responses) + + agent = Agent( + llm=llm, + registry=registry, + config=AgentConfig( + max_steps=10, + max_duplicate_tool_call_cycles=2, + ), + ) + + with pytest.raises(DuplicateToolCallLoopError) as exc_info: + await agent.run(goal="Test raise on cycle", raise_on_failure=True) + + assert exc_info.value.tool_name == "step_fn" + assert exc_info.value.count == 2 + assert exc_info.value.cycle_length == 2 + + +@pytest.mark.asyncio +async def test_multi_tool_cycle_detected() -> None: + """AUDIT-15: Repeating cycle of 3 distinct tools is detected and halted.""" + registry = ToolRegistry() + + @tool(registry=registry) + def tool_alpha(x: int) -> int: + return x + + @tool(registry=registry) + def tool_beta(x: int) -> int: + return x + + @tool(registry=registry) + def tool_gamma(x: int) -> int: + return x + + # 2 repetitions of cycle (alpha -> beta -> gamma) + responses = [ + LLMResponse(tool_calls=[ToolCall(id="c1", name="tool_alpha", arguments={"x": 1})]), + LLMResponse(tool_calls=[ToolCall(id="c2", name="tool_beta", arguments={"x": 1})]), + LLMResponse(tool_calls=[ToolCall(id="c3", name="tool_gamma", arguments={"x": 1})]), + LLMResponse(tool_calls=[ToolCall(id="c4", name="tool_alpha", arguments={"x": 1})]), + LLMResponse(tool_calls=[ToolCall(id="c5", name="tool_beta", arguments={"x": 1})]), + LLMResponse(tool_calls=[ToolCall(id="c6", name="tool_gamma", arguments={"x": 1})]), + ] + llm = FakeLLMClient(responses=responses) + + agent = Agent( + llm=llm, + registry=registry, + config=AgentConfig( + max_steps=10, + max_duplicate_tool_call_cycles=2, + max_tool_cycle_length=5, + ), + ) + + result = await agent.run(goal="3-cycle test") + assert result.status == RunStatus.FAILED + assert result.steps_count == 6 + assert "repeating cycle of 3 tool calls" in (result.failure_reason or "") + assert "tool_alpha -> tool_beta -> tool_gamma" in (result.failure_reason or "")