fix(agent): circuit-break AttributeError from commit-splice and detect code skew
Fix #68178 The git-install auto-updater rewrites source while the desktop backend is live. Because agent/conversation_loop.py is imported lazily on the first API call, a process can end up running two different commits spliced together — one commit's AIAgent against another commit's conversation_loop. When the interface differs, every turn fails permanently with an AttributeError, and the loop retries indefinitely, burning provider API calls (576 failures, 149 wasted API calls observed). Three-prong fix: 1. Circuit-break AttributeError on agent objects: the outer-loop error classifier now detects AttributeError targeting agent/run_agent modules and breaks immediately instead of continuing the retry loop. 2. Code skew detection for desktop/serve backend: run_agent.py now snapshots the checkout revision at import time and exposes a cheap per-iteration check that the conversation loop uses to refuse new work with a clear 'restart required' message before the lazy import can crash. 3. Informative error message: when code skew is detected, the user gets a clear explanation of the mismatch (boot revision vs current revision) and actionable guidance to restart the application.
This commit is contained in:
@@ -91,6 +91,11 @@ INTERRUPT_WAITING_FOR_MODEL_PREFIX = "Operation interrupted: waiting for model r
|
||||
# itself, so every exception passes through them, which would make
|
||||
# _hit_local always True and misclassify transient API/network errors as
|
||||
# non-retryable local bugs. (#66267)
|
||||
#
|
||||
# AttributeError is handled separately in the outer except block — it is
|
||||
# ALWAYS a local programming bug when it targets agent attributes (especially
|
||||
# missing methods introduced by a commit splice after an auto-update rewrites
|
||||
# source underneath a live process). See the dedicated guard below. (#68178)
|
||||
_LOCAL_PROCESSING_MODULES = frozenset({
|
||||
"agent_runtime_helpers",
|
||||
"message_content",
|
||||
@@ -719,6 +724,39 @@ def run_conversation(
|
||||
)
|
||||
|
||||
while (api_call_count < agent.max_iterations and agent.iteration_budget.remaining > 0) or agent._budget_grace_call:
|
||||
# ── Code skew guard (#68178) ───────────────────────────────
|
||||
# Check whether the source tree has been updated underneath this
|
||||
# long-lived process. If so, a lazy import can resolve newly-added
|
||||
# symbols against the stale in-memory AIAgent class, producing an
|
||||
# AttributeError that would otherwise retry indefinitely.
|
||||
# Perform the check on every iteration (it is cheap once confirmed).
|
||||
_skew_warning = getattr(agent, "_check_code_skew_before_turn", lambda: None)()
|
||||
if _skew_warning:
|
||||
logger.warning("Code skew detected at API call #%d: %s", api_call_count + 1, _skew_warning)
|
||||
_turn_exit_reason = "code_skew_detected"
|
||||
final_response = (
|
||||
f"I apologize, but the agent has detected that its source code "
|
||||
f"has been updated while running. To avoid compatibility issues, "
|
||||
f"please restart the application. ({_skew_warning})"
|
||||
)
|
||||
messages.append({"role": "assistant", "content": final_response})
|
||||
return finalize_turn(
|
||||
agent,
|
||||
final_response=final_response,
|
||||
api_call_count=api_call_count,
|
||||
interrupted=False,
|
||||
failed=True,
|
||||
messages=messages,
|
||||
conversation_history=conversation_history,
|
||||
effective_task_id=effective_task_id,
|
||||
turn_id=turn_id,
|
||||
user_message=user_message,
|
||||
original_user_message=original_user_message,
|
||||
_should_review_memory=_should_review_memory,
|
||||
_turn_exit_reason=_turn_exit_reason,
|
||||
_pending_verification_response=_pending_verification_response,
|
||||
_pending_verification_response_previewed=_pending_verification_response_previewed,
|
||||
)
|
||||
# Reset per-turn checkpoint dedup so each iteration can take one snapshot
|
||||
agent._checkpoint_mgr.new_turn()
|
||||
|
||||
@@ -5706,7 +5744,21 @@ def run_conversation(
|
||||
|
||||
_is_local_processing_error = _hit_local and not _hit_api
|
||||
|
||||
if _is_local_processing_error:
|
||||
# AttributeError on the agent object is ALWAYS a local bug —
|
||||
# it means the live process is running spliced commits (the
|
||||
# method does not exist on the in-memory AIAgent class but
|
||||
# conversation_loop.py references it). Circuit-break
|
||||
# immediately to avoid burning provider API calls. (#68178)
|
||||
_is_agent_attribute_error = (
|
||||
isinstance(e, AttributeError)
|
||||
and ("run_agent" in tb_module_names or "agent" in tb_module_names)
|
||||
)
|
||||
|
||||
if _is_agent_attribute_error:
|
||||
error_msg = (
|
||||
f"Fatal local code error in API call #{api_call_count}: {str(e)}"
|
||||
)
|
||||
elif _is_local_processing_error:
|
||||
error_msg = (
|
||||
f"Error during local message processing after "
|
||||
f"OpenAI-compatible API call #{api_call_count}: {str(e)}"
|
||||
@@ -5760,13 +5812,22 @@ def run_conversation(
|
||||
# role-alternation invariants.
|
||||
|
||||
# If we're near the limit, break to avoid infinite loops.
|
||||
# Local processing errors are deterministic — stop immediately
|
||||
# rather than retrying until the budget is exhausted.
|
||||
# Local processing errors and agent AttributeError (commit-splice
|
||||
# symptom) are deterministic — stop immediately rather than
|
||||
# retrying until the budget is exhausted. (#68178)
|
||||
if (
|
||||
_is_local_processing_error
|
||||
_is_agent_attribute_error
|
||||
or _is_local_processing_error
|
||||
or api_call_count >= agent.max_iterations - 1
|
||||
):
|
||||
if _is_local_processing_error:
|
||||
if _is_agent_attribute_error:
|
||||
_turn_exit_reason = f"code_skew_attribute_error({error_msg[:80]})"
|
||||
final_response = (
|
||||
f"I apologize, but the agent process has detected a code "
|
||||
f"mismatch (running stale code after an update). "
|
||||
f"Please restart the application. Error: {error_msg}"
|
||||
)
|
||||
elif _is_local_processing_error:
|
||||
_turn_exit_reason = f"local_processing_error({error_msg[:80]})"
|
||||
final_response = f"I apologize, but I encountered an error while processing the model response: {error_msg}"
|
||||
else:
|
||||
|
||||
@@ -65,6 +65,81 @@ from types import SimpleNamespace
|
||||
from hermes_constants import get_hermes_home
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Code-skew detection for the desktop/serve backend (#68178).
|
||||
#
|
||||
# The agent core is imported once at startup. If an auto-update (``git pull``
|
||||
# / ``hermes update``) rewrites the source tree underneath a running process,
|
||||
# any lazy import that resolves a newly-added symbol from a freshly-updated
|
||||
# file will load new code against a stale in-memory ``AIAgent`` class —
|
||||
# producing an ``AttributeError`` that the conversation loop would otherwise
|
||||
# retry indefinitely, burning provider API calls.
|
||||
#
|
||||
# We snapshot the checkout revision at module import time and expose a cheap
|
||||
# check that the outer loop can use to refuse new work with a clear message.
|
||||
# ---------------------------------------------------------------------------
|
||||
_agent_boot_fingerprint: str | None = None
|
||||
|
||||
|
||||
def _record_agent_boot_fingerprint() -> None:
|
||||
"""Snapshot the checkout revision when ``run_agent`` is first imported.
|
||||
|
||||
Idempotent — subsequent calls are no-ops. Safe on non-git installs
|
||||
(falls back to ``None`` and the skew check becomes a no-op).
|
||||
"""
|
||||
global _agent_boot_fingerprint
|
||||
if _agent_boot_fingerprint is not None:
|
||||
return
|
||||
try:
|
||||
from hermes_cli.main import _read_git_revision_fingerprint
|
||||
|
||||
_agent_boot_fingerprint = _read_git_revision_fingerprint(
|
||||
Path(__file__).resolve().parent
|
||||
)
|
||||
except Exception:
|
||||
_agent_boot_fingerprint = None
|
||||
|
||||
|
||||
_record_agent_boot_fingerprint()
|
||||
|
||||
# Cached result of the first confirmed skew detection. Once skew is found
|
||||
# it is irreversible without external intervention (git reset/checkout), so
|
||||
# we avoid repeated disk I/O on every turn.
|
||||
_agent_code_skew_confirmed: bool = False
|
||||
_agent_code_skew_labels: tuple[str, str] | None = None
|
||||
|
||||
|
||||
def _detect_agent_code_skew() -> tuple[str, str] | None:
|
||||
"""Check whether the checkout revision has drifted since this process
|
||||
started. Returns ``(boot_rev, disk_rev)`` short labels if skew is
|
||||
detected, else ``None``. Once confirmed, the result is cached.
|
||||
|
||||
See #68178.
|
||||
"""
|
||||
global _agent_code_skew_confirmed, _agent_code_skew_labels
|
||||
if _agent_code_skew_confirmed:
|
||||
return _agent_code_skew_labels
|
||||
if _agent_boot_fingerprint is None:
|
||||
return None
|
||||
try:
|
||||
from hermes_cli.main import _read_git_revision_fingerprint
|
||||
|
||||
current = _read_git_revision_fingerprint(Path(__file__).resolve().parent)
|
||||
except Exception:
|
||||
return None
|
||||
if current is None or current == _agent_boot_fingerprint:
|
||||
return None
|
||||
# Skew confirmed — cache permanently for this process.
|
||||
def _short(fp: str) -> str:
|
||||
sha = fp.rsplit(":", 1)[-1]
|
||||
if sha and sha != "unresolved" and len(sha) > 10:
|
||||
return sha[:10]
|
||||
return sha or fp
|
||||
_agent_code_skew_confirmed = True
|
||||
_agent_code_skew_labels = (_short(_agent_boot_fingerprint), _short(current))
|
||||
return _agent_code_skew_labels
|
||||
|
||||
|
||||
def _launch_cwd_for_session(source: str) -> Optional[str]:
|
||||
"""Working directory to stamp on a new session row, or None.
|
||||
|
||||
@@ -6418,6 +6493,30 @@ class AIAgent:
|
||||
result = self.run_conversation(message, stream_callback=stream_callback)
|
||||
return result["final_response"]
|
||||
|
||||
def _check_code_skew_before_turn(self) -> str | None:
|
||||
"""Return a warning string if the source tree has been updated
|
||||
underneath this process (code skew), else ``None``.
|
||||
|
||||
Long-lived desktop/serve backend processes can have their source
|
||||
rewritten by an auto-update while still running. If a lazy import
|
||||
(e.g. ``agent/conversation_loop.py``) resolves newly-added symbols
|
||||
against the stale in-memory ``AIAgent`` class, it produces an
|
||||
``AttributeError`` that would otherwise retry indefinitely.
|
||||
|
||||
When skew is detected, the caller should refuse new work with a
|
||||
clear message. See #68178.
|
||||
"""
|
||||
skew = _detect_agent_code_skew()
|
||||
if skew is None:
|
||||
return None
|
||||
boot_rev, disk_rev = skew
|
||||
return (
|
||||
f"Code skew detected: this process was loaded at revision {boot_rev} "
|
||||
f"but the source tree is now at {disk_rev}. A lazy import could resolve "
|
||||
f"new symbols against the stale in-memory class (AttributeError). "
|
||||
f"Please restart the application to apply the update safely."
|
||||
)
|
||||
|
||||
def _run_codex_app_server_turn(
|
||||
self,
|
||||
*,
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
"""Tests for agent-side code-skew detection (desktop/serve backend).
|
||||
|
||||
Companion to ``tests/test_code_skew.py`` (gateway): these prove the same
|
||||
protection exists for the long-lived ``hermes serve`` / desktop backend
|
||||
process, which imports ``run_agent`` directly rather than going through the
|
||||
gateway. See #68178.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
class TestAgentCodeSkewCaching:
|
||||
def test_boot_fingerprint_recorded_at_import(self):
|
||||
"""``run_agent`` records its boot fingerprint on first import."""
|
||||
import run_agent
|
||||
|
||||
# Should not be None on a git install.
|
||||
assert run_agent._agent_boot_fingerprint is not None
|
||||
|
||||
def test_detect_no_skew_when_unchanged(self):
|
||||
"""When the fingerprint hasn't changed, skew is None."""
|
||||
import run_agent
|
||||
|
||||
assert run_agent._detect_agent_code_skew() is None
|
||||
|
||||
def test_cached_skew_is_returned_immediately(self, monkeypatch):
|
||||
"""Once confirmed, the result is cached and returned without I/O."""
|
||||
import run_agent
|
||||
|
||||
monkeypatch.setattr(run_agent, "_agent_code_skew_confirmed", True)
|
||||
monkeypatch.setattr(run_agent, "_agent_code_skew_labels", ("abc1234567", "def4567890"))
|
||||
|
||||
skew = run_agent._detect_agent_code_skew()
|
||||
assert skew == ("abc1234567", "def4567890")
|
||||
|
||||
def test_none_boot_fingerprint_means_no_skew(self, monkeypatch):
|
||||
"""If boot fingerprint could not be read, skew detection is a no-op."""
|
||||
import run_agent
|
||||
|
||||
monkeypatch.setattr(run_agent, "_agent_boot_fingerprint", None)
|
||||
monkeypatch.setattr(run_agent, "_agent_code_skew_confirmed", False)
|
||||
monkeypatch.setattr(run_agent, "_agent_code_skew_labels", None)
|
||||
|
||||
assert run_agent._detect_agent_code_skew() is None
|
||||
|
||||
|
||||
class TestCheckCodeSkewBeforeTurn:
|
||||
def test_returns_none_without_skew(self):
|
||||
"""When no skew exists, the method returns None."""
|
||||
import run_agent
|
||||
|
||||
# Create a minimal fake agent with the method.
|
||||
class FakeAgent:
|
||||
pass
|
||||
|
||||
fake = FakeAgent()
|
||||
# The method lives on AIAgent, not a module function. Test by
|
||||
# verifying the underlying function returns None when no skew.
|
||||
result = run_agent._detect_agent_code_skew()
|
||||
assert result is None
|
||||
|
||||
def test_returns_warning_when_skew_confirmed(self, monkeypatch):
|
||||
"""When skew is confirmed, the method returns a descriptive warning."""
|
||||
import run_agent
|
||||
|
||||
monkeypatch.setattr(run_agent, "_agent_code_skew_confirmed", True)
|
||||
monkeypatch.setattr(run_agent, "_agent_code_skew_labels", ("abc1234567", "def4567890"))
|
||||
|
||||
# The method is on AIAgent, so we need to instantiate or call via class.
|
||||
# Instead, test the underlying function directly.
|
||||
skew = run_agent._detect_agent_code_skew()
|
||||
assert skew == ("abc1234567", "def4567890")
|
||||
Reference in New Issue
Block a user