diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 501e49b54d..ed7c911b45 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -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: diff --git a/run_agent.py b/run_agent.py index 6c13f737c8..d649addce7 100644 --- a/run_agent.py +++ b/run_agent.py @@ -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, *, diff --git a/tests/test_agent_code_skew.py b/tests/test_agent_code_skew.py new file mode 100644 index 0000000000..be61a86e99 --- /dev/null +++ b/tests/test_agent_code_skew.py @@ -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")