-
Notifications
You must be signed in to change notification settings - Fork 5
feat: Agent stall detection and recovery (#399) #423
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
02dcc9c
2df0ced
1384e0b
339a681
b7f4844
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| import json | ||
| import logging | ||
| import os | ||
| import threading | ||
| from pathlib import Path | ||
| from datetime import datetime, timezone | ||
| from typing import TYPE_CHECKING, Callable, Optional | ||
|
|
@@ -22,6 +23,7 @@ | |
| from codeframe.core.context import ContextLoader, TaskContext | ||
| from codeframe.core.events import EventType | ||
| from codeframe.core.fix_tracker import EscalationDecision, FixAttemptTracker, FixOutcome | ||
| from codeframe.core.stall_monitor import StallEvent, StallMonitor | ||
| from codeframe.core.models import AgentPhase, CompletionEvent, ErrorEvent, ProgressEvent | ||
| from codeframe.core.quick_fixes import apply_quick_fix, find_quick_fix | ||
| from codeframe.core.tools import AGENT_TOOLS, execute_tool | ||
|
|
@@ -107,6 +109,7 @@ def __init__( | |
| llm_provider: LLMProvider, | ||
| max_iterations: int = 30, | ||
| max_verification_retries: int = 5, | ||
| stall_timeout_s: float = 300, | ||
| event_publisher: Optional[EventPublisher] = None, | ||
| dry_run: bool = False, | ||
| verbose: bool = False, | ||
|
|
@@ -119,6 +122,7 @@ def __init__( | |
| self.llm_provider = llm_provider | ||
| self.max_iterations = max_iterations | ||
| self.max_verification_retries = max_verification_retries | ||
| self._stall_timeout_s = stall_timeout_s | ||
| self.event_publisher = event_publisher | ||
| self.dry_run = dry_run | ||
| self.verbose = verbose | ||
|
|
@@ -129,6 +133,14 @@ def __init__( | |
| self.fix_tracker = FixAttemptTracker() | ||
| self.blocker_id: Optional[str] = None | ||
|
|
||
| # Stall detection | ||
| self._stall_triggered = threading.Event() | ||
| self._stall_event: Optional[StallEvent] = None | ||
| self._stall_monitor = StallMonitor( | ||
| stall_timeout_s=stall_timeout_s, | ||
| on_stall=self._on_stall, | ||
| ) | ||
|
|
||
| # Token budget tracking for conversation compaction | ||
| self._context_window_size: int = DEFAULT_CONTEXT_WINDOW | ||
| self._compaction_threshold: float = self._read_compaction_threshold() | ||
|
|
@@ -187,45 +199,49 @@ def run(self, task_id: str) -> AgentStatus: | |
|
|
||
| system_prompt = self._build_system_prompt(context) | ||
|
|
||
| status = self._react_loop(system_prompt) | ||
| if status == AgentStatus.FAILED: | ||
| self._emit(EventType.AGENT_FAILED, { | ||
| "task_id": task_id, | ||
| "reason": "max_iterations_reached", | ||
| }) | ||
| self._emit_stream_error(task_id, "max_iterations_reached") | ||
| return status | ||
| self._stall_monitor.start(task_id) | ||
| try: | ||
| status = self._react_loop(system_prompt) | ||
| if status == AgentStatus.FAILED: | ||
| self._emit(EventType.AGENT_FAILED, { | ||
| "task_id": task_id, | ||
| "reason": "max_iterations_reached", | ||
| }) | ||
| self._emit_stream_error(task_id, "max_iterations_reached") | ||
| return status | ||
|
|
||
| if status == AgentStatus.BLOCKED: | ||
| self._emit(EventType.AGENT_FAILED, { | ||
| "task_id": task_id, | ||
| "reason": "blocked", | ||
| }) | ||
| self._emit_stream_error(task_id, "blocked") | ||
| return AgentStatus.BLOCKED | ||
| if status == AgentStatus.BLOCKED: | ||
| self._emit(EventType.AGENT_FAILED, { | ||
| "task_id": task_id, | ||
| "reason": "blocked", | ||
| }) | ||
| self._emit_stream_error(task_id, "blocked") | ||
| return AgentStatus.BLOCKED | ||
|
|
||
| # Final verification with retry | ||
| passed, reason = self._run_final_verification(system_prompt) | ||
| if passed: | ||
| self._verbose_print(f"[ReactAgent] Task {task_id} completed: {AgentStatus.COMPLETED.name}") | ||
| self._emit(EventType.AGENT_COMPLETED, {"task_id": task_id}) | ||
| self._emit_stream_completion(task_id) | ||
| return AgentStatus.COMPLETED | ||
| # Final verification with retry | ||
| passed, reason = self._run_final_verification(system_prompt) | ||
| if passed: | ||
| self._verbose_print(f"[ReactAgent] Task {task_id} completed: {AgentStatus.COMPLETED.name}") | ||
| self._emit(EventType.AGENT_COMPLETED, {"task_id": task_id}) | ||
| self._emit_stream_completion(task_id) | ||
| return AgentStatus.COMPLETED | ||
|
|
||
| if reason == "escalated_to_blocker" or reason == "stall_detected": | ||
| self._emit(EventType.AGENT_FAILED, { | ||
| "task_id": task_id, | ||
| "reason": "blocked", | ||
| }) | ||
| self._emit_stream_error(task_id, "blocked") | ||
| return AgentStatus.BLOCKED | ||
|
|
||
| if reason == "escalated_to_blocker": | ||
| self._emit(EventType.AGENT_FAILED, { | ||
| "task_id": task_id, | ||
| "reason": "blocked", | ||
| "reason": "verification_failed", | ||
| }) | ||
| self._emit_stream_error(task_id, "blocked") | ||
| return AgentStatus.BLOCKED | ||
|
|
||
| self._emit(EventType.AGENT_FAILED, { | ||
| "task_id": task_id, | ||
| "reason": "verification_failed", | ||
| }) | ||
| self._emit_stream_error(task_id, "verification_failed") | ||
| return AgentStatus.FAILED | ||
| self._emit_stream_error(task_id, "verification_failed") | ||
| return AgentStatus.FAILED | ||
| finally: | ||
| self._stall_monitor.stop() | ||
| except Exception: | ||
| logger.exception("ReactAgent.run() failed for task %s", task_id) | ||
| self._emit(EventType.AGENT_FAILED, { | ||
|
|
@@ -262,6 +278,20 @@ def _react_loop(self, system_prompt: str) -> AgentStatus: | |
| prompt_summary = system_prompt[:200] | ||
|
|
||
| while iterations < self.max_iterations: | ||
| # Check for stall before each iteration | ||
| if self._stall_triggered.is_set(): | ||
| stall_ctx = "" | ||
| if self._stall_event: | ||
| stall_ctx = ( | ||
| f"Agent stalled: no tool call for {self._stall_event.elapsed_s:.0f}s " | ||
| f"(timeout: {self._stall_event.stall_timeout_s}s)" | ||
| ) | ||
| self._create_text_blocker( | ||
| stall_ctx or "Agent stalled with no tool activity", | ||
| "stall_detected", | ||
| ) | ||
| return AgentStatus.BLOCKED | ||
|
|
||
| self._verbose_print(f"[ReactAgent] Iteration {iterations + 1}/{self.max_iterations}") | ||
| self._emit(EventType.AGENT_ITERATION_STARTED, { | ||
| "task_id": self._current_task_id, | ||
|
|
@@ -337,6 +367,11 @@ def _react_loop(self, system_prompt: str) -> AgentStatus: | |
|
|
||
| result = self._execute_tool_with_lint(tc) | ||
|
|
||
| if not result.is_error: | ||
| self._stall_monitor.notify_tool_executed( | ||
| self._current_task_id, iterations, | ||
| ) | ||
|
Comment on lines
+370
to
+373
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Failing tool calls still count as activity. The feature defines a stall as "no tool calls", but the timer is only reset for non-error results here. If the agent is actively iterating on failing Minimal fix result = self._execute_tool_with_lint(tc)
-
- if not result.is_error:
- self._stall_monitor.notify_tool_executed(
- self._current_task_id, iterations,
- )
+ self._stall_monitor.notify_tool_executed(
+ self._current_task_id,
+ iterations,
+ )🤖 Prompt for AI Agents
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is intentional design. A stall means the agent is not doing anything (e.g., LLM API hangs or agent loops without acting). When the agent is actively executing tools — even failing ones — it IS making progress (discovering errors, trying fixes). The existing fix_tracker and escalation system handles repeated failures. Stall detection is specifically for the case where nothing happens at all.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
✏️ Learnings added
🧠 Learnings used |
||
|
|
||
| self._emit(EventType.AGENT_TOOL_RESULT, { | ||
| "task_id": self._current_task_id, | ||
| "tool_call_id": result.tool_call_id, | ||
|
|
@@ -437,6 +472,9 @@ def _run_final_verification( | |
| max_fix_turns = 5 # LLM turns per retry attempt | ||
|
|
||
| for attempt in range(1 + self.max_verification_retries): | ||
| if self._stall_triggered.is_set(): | ||
| return (False, "stall_detected") | ||
|
|
||
| self._verbose_print("[ReactAgent] Running final verification...") | ||
| self._emit_progress( | ||
| AgentPhase.VERIFYING, | ||
|
|
@@ -912,6 +950,25 @@ def _emit_stream_error(self, task_id: str, reason: str) -> None: | |
| except Exception: | ||
| logger.debug("Failed to close task stream", exc_info=True) | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # Stall detection | ||
| # ------------------------------------------------------------------ | ||
|
|
||
| def _on_stall(self, event: StallEvent) -> None: | ||
| """Callback invoked by StallMonitor when inactivity is detected.""" | ||
| self._stall_event = event | ||
| self._stall_triggered.set() | ||
| self._verbose_print( | ||
| f"[ReactAgent] Stall detected: no tool call for {event.elapsed_s:.0f}s " | ||
| f"(timeout={event.stall_timeout_s}s, iterations={event.iterations_completed})" | ||
| ) | ||
| self._emit(EventType.AGENT_STALL_DETECTED, { | ||
| "task_id": event.task_id, | ||
| "elapsed_s": event.elapsed_s, | ||
| "stall_timeout_s": event.stall_timeout_s, | ||
| "iterations_completed": event.iterations_completed, | ||
| }) | ||
|
|
||
| # ------------------------------------------------------------------ | ||
| # Blocker creation helpers | ||
| # ------------------------------------------------------------------ | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CLI default currently overrides workspace config.
Both commands give
stall_timeouta concrete default and always pass it downstream, so omitting--stall-timeoutis indistinguishable from explicitly choosing300. That makes.codeframe/config.yaml'sagent_budget.stall_timeout_sineffective on the main CLI execution paths. Use an "unset" sentinel at the CLI boundary and let core resolve config/defaults.Also applies to: 2068-2070, 2872-2876, 2965-2975
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Valid concern. The CLI default pattern matches how
--engineworks today (also has a concrete default that overrides config). For now this is acceptable — the config precedence layer (issue #399 step 6 in the plan) was intentionally deferred as YAGNI. If users actually set stall_timeout_s in config and expect it to be respected from CLI, we can add sentinel-based detection in a follow-up.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@frankbria, understood — the concrete default matches the existing--engineprecedence pattern and the config-first resolution is a planned future concern. I'll note this for future reviews.Would you like me to open a follow-up issue to track the sentinel-based CLI/config precedence work (config → CLI default → explicit CLI flag) when the time comes?
✏️ Learnings added
🧠 Learnings used