fix: OAuth flag stale after refresh/fallback, memory nudge never fires, dead code
- Update _is_anthropic_oauth in _try_refresh_anthropic_client_credentials() when token type changes during credential refresh - Set _is_anthropic_oauth in _try_activate_fallback() Anthropic path - Move _turns_since_memory and _iters_since_skill init to __init__ so nudge counters accumulate across run_conversation() calls in CLI mode - Remove unreachable retry_count >= max_retries block after raise Adds 7 regression tests. Salvaged from PR #1797 by @0xbyt4.
This commit is contained in:
17
run_agent.py
17
run_agent.py
@@ -738,6 +738,8 @@ class AIAgent:
|
||||
self._user_profile_enabled = False
|
||||
self._memory_nudge_interval = 10
|
||||
self._memory_flush_min_turns = 6
|
||||
self._turns_since_memory = 0
|
||||
self._iters_since_skill = 0
|
||||
if not skip_memory:
|
||||
try:
|
||||
from hermes_cli.config import load_config as _load_mem_config
|
||||
@@ -2951,6 +2953,9 @@ class AIAgent:
|
||||
return False
|
||||
|
||||
self._anthropic_api_key = new_token
|
||||
# Update OAuth flag — token type may have changed (API key ↔ OAuth)
|
||||
from agent.anthropic_adapter import _is_oauth_token
|
||||
self._is_anthropic_oauth = _is_oauth_token(new_token)
|
||||
return True
|
||||
|
||||
def _anthropic_messages_create(self, api_kwargs: dict):
|
||||
@@ -3342,11 +3347,12 @@ class AIAgent:
|
||||
|
||||
if fb_api_mode == "anthropic_messages":
|
||||
# Build native Anthropic client instead of using OpenAI client
|
||||
from agent.anthropic_adapter import build_anthropic_client, resolve_anthropic_token
|
||||
from agent.anthropic_adapter import build_anthropic_client, resolve_anthropic_token, _is_oauth_token
|
||||
effective_key = fb_client.api_key or resolve_anthropic_token() or ""
|
||||
self._anthropic_api_key = effective_key
|
||||
self._anthropic_base_url = getattr(fb_client, "base_url", None)
|
||||
self._anthropic_client = build_anthropic_client(effective_key, self._anthropic_base_url)
|
||||
self._is_anthropic_oauth = _is_oauth_token(effective_key)
|
||||
self.client = None
|
||||
self._client_kwargs = {}
|
||||
else:
|
||||
@@ -4831,8 +4837,9 @@ class AIAgent:
|
||||
self._incomplete_scratchpad_retries = 0
|
||||
self._codex_incomplete_retries = 0
|
||||
self._last_content_with_tools = None
|
||||
self._turns_since_memory = 0
|
||||
self._iters_since_skill = 0
|
||||
# NOTE: _turns_since_memory and _iters_since_skill are NOT reset here.
|
||||
# They are initialized in __init__ and must persist across run_conversation
|
||||
# calls so that nudge logic accumulates correctly in CLI mode.
|
||||
self.iteration_budget = IterationBudget(self.max_iterations)
|
||||
|
||||
# Initialize conversation (copy to avoid mutating the caller's list)
|
||||
@@ -5850,10 +5857,6 @@ class AIAgent:
|
||||
self._client_log_context(),
|
||||
api_error,
|
||||
)
|
||||
if retry_count >= max_retries:
|
||||
self._vprint(f"{self.log_prefix}⚠️ API call failed after {retry_count} attempts: {str(api_error)[:100]}")
|
||||
self._vprint(f"{self.log_prefix}⏳ Final retry in {wait_time}s...")
|
||||
|
||||
# Sleep in small increments so we can respond to interrupts quickly
|
||||
# instead of blocking the entire wait_time in one sleep() call
|
||||
sleep_end = time.time() + wait_time
|
||||
|
||||
@@ -2717,3 +2717,135 @@ class TestNormalizeCodexDictArguments:
|
||||
msg, _ = agent._normalize_codex_response(response)
|
||||
tc = msg.tool_calls[0]
|
||||
assert tc.function.arguments == args_str
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# OAuth flag and nudge counter fixes (salvaged from PR #1797)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestOAuthFlagAfterCredentialRefresh:
|
||||
"""_is_anthropic_oauth must update when token type changes during refresh."""
|
||||
|
||||
def test_oauth_flag_updates_api_key_to_oauth(self, agent):
|
||||
"""Refreshing from API key to OAuth token must set flag to True."""
|
||||
agent.api_mode = "anthropic_messages"
|
||||
agent._anthropic_api_key = "sk-ant-api-old"
|
||||
agent._anthropic_client = MagicMock()
|
||||
agent._is_anthropic_oauth = False
|
||||
|
||||
with (
|
||||
patch("agent.anthropic_adapter.resolve_anthropic_token",
|
||||
return_value="sk-ant-setup-oauth-token"),
|
||||
patch("agent.anthropic_adapter.build_anthropic_client",
|
||||
return_value=MagicMock()),
|
||||
):
|
||||
result = agent._try_refresh_anthropic_client_credentials()
|
||||
|
||||
assert result is True
|
||||
assert agent._is_anthropic_oauth is True
|
||||
|
||||
def test_oauth_flag_updates_oauth_to_api_key(self, agent):
|
||||
"""Refreshing from OAuth to API key must set flag to False."""
|
||||
agent.api_mode = "anthropic_messages"
|
||||
agent._anthropic_api_key = "sk-ant-setup-old"
|
||||
agent._anthropic_client = MagicMock()
|
||||
agent._is_anthropic_oauth = True
|
||||
|
||||
with (
|
||||
patch("agent.anthropic_adapter.resolve_anthropic_token",
|
||||
return_value="sk-ant-api03-new-key"),
|
||||
patch("agent.anthropic_adapter.build_anthropic_client",
|
||||
return_value=MagicMock()),
|
||||
):
|
||||
result = agent._try_refresh_anthropic_client_credentials()
|
||||
|
||||
assert result is True
|
||||
assert agent._is_anthropic_oauth is False
|
||||
|
||||
|
||||
class TestFallbackSetsOAuthFlag:
|
||||
"""_try_activate_fallback must set _is_anthropic_oauth for Anthropic fallbacks."""
|
||||
|
||||
def test_fallback_to_anthropic_oauth_sets_flag(self, agent):
|
||||
agent._fallback_activated = False
|
||||
agent._fallback_model = {"provider": "anthropic", "model": "claude-sonnet-4-6"}
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.base_url = "https://api.anthropic.com/v1"
|
||||
mock_client.api_key = "sk-ant-setup-oauth-token"
|
||||
|
||||
with (
|
||||
patch("agent.auxiliary_client.resolve_provider_client",
|
||||
return_value=(mock_client, None)),
|
||||
patch("agent.anthropic_adapter.build_anthropic_client",
|
||||
return_value=MagicMock()),
|
||||
patch("agent.anthropic_adapter.resolve_anthropic_token",
|
||||
return_value=None),
|
||||
):
|
||||
result = agent._try_activate_fallback()
|
||||
|
||||
assert result is True
|
||||
assert agent._is_anthropic_oauth is True
|
||||
|
||||
def test_fallback_to_anthropic_api_key_clears_flag(self, agent):
|
||||
agent._fallback_activated = False
|
||||
agent._fallback_model = {"provider": "anthropic", "model": "claude-sonnet-4-6"}
|
||||
|
||||
mock_client = MagicMock()
|
||||
mock_client.base_url = "https://api.anthropic.com/v1"
|
||||
mock_client.api_key = "sk-ant-api03-regular-key"
|
||||
|
||||
with (
|
||||
patch("agent.auxiliary_client.resolve_provider_client",
|
||||
return_value=(mock_client, None)),
|
||||
patch("agent.anthropic_adapter.build_anthropic_client",
|
||||
return_value=MagicMock()),
|
||||
patch("agent.anthropic_adapter.resolve_anthropic_token",
|
||||
return_value=None),
|
||||
):
|
||||
result = agent._try_activate_fallback()
|
||||
|
||||
assert result is True
|
||||
assert agent._is_anthropic_oauth is False
|
||||
|
||||
|
||||
class TestMemoryNudgeCounterPersistence:
|
||||
"""_turns_since_memory must persist across run_conversation calls."""
|
||||
|
||||
def test_counters_initialized_in_init(self):
|
||||
"""Counters must exist on the agent after __init__."""
|
||||
with patch("run_agent.get_tool_definitions", return_value=[]):
|
||||
a = AIAgent(
|
||||
model="test", api_key="test-key", provider="openrouter",
|
||||
skip_context_files=True, skip_memory=True,
|
||||
)
|
||||
assert hasattr(a, "_turns_since_memory")
|
||||
assert hasattr(a, "_iters_since_skill")
|
||||
assert a._turns_since_memory == 0
|
||||
assert a._iters_since_skill == 0
|
||||
|
||||
def test_counters_not_reset_in_preamble(self):
|
||||
"""The run_conversation preamble must not zero the nudge counters."""
|
||||
import inspect
|
||||
src = inspect.getsource(AIAgent.run_conversation)
|
||||
# The preamble resets many fields (retry counts, budget, etc.)
|
||||
# before the main loop. Find that reset block and verify our
|
||||
# counters aren't in it. The reset block ends at iteration_budget.
|
||||
preamble_end = src.index("self.iteration_budget = IterationBudget")
|
||||
preamble = src[:preamble_end]
|
||||
assert "self._turns_since_memory = 0" not in preamble
|
||||
assert "self._iters_since_skill = 0" not in preamble
|
||||
|
||||
|
||||
class TestDeadRetryCode:
|
||||
"""Unreachable retry_count >= max_retries after raise must not exist."""
|
||||
|
||||
def test_no_unreachable_max_retries_after_backoff(self):
|
||||
import inspect
|
||||
source = inspect.getsource(AIAgent.run_conversation)
|
||||
occurrences = source.count("if retry_count >= max_retries:")
|
||||
assert occurrences == 2, (
|
||||
f"Expected 2 occurrences of 'if retry_count >= max_retries:' "
|
||||
f"but found {occurrences}"
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user