fix(agent): cap same-entry credential refreshes so fallback can activate (#26080)
A persistent upstream 401 on a single-entry OAuth pool (common for Claude Max subscribers) made the credential-pool recovery spin forever: try_refresh_current() re-mints a fresh token and reports success on every 401, so recover_with_credential_pool returned True and the retry loop continue'd without ever incrementing retry_count or reaching the auth-failover block. The configured fallback_model never activated and the agent appeared to hang. Cap consecutive successful same-entry refreshes (keyed by provider + pool-entry id) at 2; once exceeded, treat the credential as unrecoverable and return not-recovered so the loop falls through to _try_activate_fallback. The 429/billing paths already rotate-or-fall-through correctly (mark_exhausted_and_rotate returns None on a single entry), so only the auth-refresh branch needed the cap. Co-authored-by: Hermes Agent <hermes@nousresearch.com>
This commit is contained in:
parent
fae920642a
commit
32732a8f83
3 changed files with 92 additions and 0 deletions
|
|
@ -42,6 +42,14 @@ from utils import base_url_host_matches, base_url_hostname, env_var_enabled, ato
|
|||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
# Max consecutive successful credential-pool token refreshes of the SAME entry
|
||||
# on a persistent auth failure before we give up and let the fallback chain
|
||||
# activate. A single-entry OAuth pool can re-mint a fresh token indefinitely
|
||||
# even when the upstream keeps rejecting it, so without this cap the retry loop
|
||||
# spins forever and never reaches ``_try_activate_fallback``. See #26080.
|
||||
_MAX_AUTH_REFRESH_ATTEMPTS = 2
|
||||
|
||||
|
||||
def _ra():
|
||||
"""Lazy ``run_agent`` reference for test-patch routing."""
|
||||
import run_agent
|
||||
|
|
@ -775,6 +783,30 @@ def recover_with_credential_pool(
|
|||
return False, has_retried_429
|
||||
refreshed = pool.try_refresh_current()
|
||||
if refreshed is not None:
|
||||
# ``try_refresh_current()`` re-mints a fresh OAuth token and reports
|
||||
# success even when the upstream keeps rejecting it — a single-entry
|
||||
# pool (common for OAuth/Max subscribers) has nothing to rotate to,
|
||||
# so a bare "refreshed → retry" loop spins forever on the same dead
|
||||
# token and the configured fallback never activates. Cap consecutive
|
||||
# same-entry refreshes and fall through to fallback once exceeded.
|
||||
# See #26080.
|
||||
refreshed_id = getattr(refreshed, "id", None)
|
||||
if refreshed_id is not None:
|
||||
refresh_counts = getattr(agent, "_auth_pool_refresh_counts", None)
|
||||
if refresh_counts is None:
|
||||
refresh_counts = {}
|
||||
agent._auth_pool_refresh_counts = refresh_counts
|
||||
refresh_key = (agent.provider, refreshed_id)
|
||||
refresh_counts[refresh_key] = refresh_counts.get(refresh_key, 0) + 1
|
||||
if refresh_counts[refresh_key] > _MAX_AUTH_REFRESH_ATTEMPTS:
|
||||
_ra().logger.warning(
|
||||
"Credential auth failure persists after %s refreshes for "
|
||||
"pool entry %s — treating as unrecoverable and allowing "
|
||||
"fallback to activate.",
|
||||
refresh_counts[refresh_key] - 1,
|
||||
refreshed_id,
|
||||
)
|
||||
return False, has_retried_429
|
||||
_ra().logger.info(f"Credential auth failure — refreshed pool entry {getattr(refreshed, 'id', '?')}")
|
||||
agent._swap_credential(refreshed)
|
||||
return True, has_retried_429
|
||||
|
|
|
|||
|
|
@ -588,6 +588,13 @@ def run_conversation(
|
|||
compression_attempts = 0
|
||||
_turn_exit_reason = "unknown" # Diagnostic: why the loop ended
|
||||
|
||||
# Per-turn tally of consecutive successful credential-pool token refreshes,
|
||||
# keyed by (provider, pool-entry-id). A persistent upstream 401 lets
|
||||
# ``try_refresh_current()`` "succeed" forever on a single-entry OAuth pool,
|
||||
# so this tally caps same-entry refreshes and lets the fallback chain take
|
||||
# over instead of spinning. Reset here so each turn starts fresh. See #26080.
|
||||
agent._auth_pool_refresh_counts = {}
|
||||
|
||||
# Optional opt-in runtime: if api_mode == codex_app_server, hand the
|
||||
# turn to the codex app-server subprocess (terminal/file ops/patching
|
||||
# all run inside Codex). Default Hermes path is bypassed entirely.
|
||||
|
|
|
|||
|
|
@ -5359,6 +5359,59 @@ class TestCredentialPoolRecovery:
|
|||
assert recovered is True
|
||||
agent._swap_credential.assert_called_once_with(refreshed_entry)
|
||||
|
||||
def test_recover_with_pool_401_same_entry_refreshes_stop_after_two(self, agent):
|
||||
"""Repeated same-entry auth refreshes must eventually fall through.
|
||||
|
||||
A single-entry OAuth pool re-mints a fresh token on every 401, so
|
||||
``try_refresh_current()`` reports success forever. The cap (#26080)
|
||||
must let the third consecutive same-entry refresh fall through
|
||||
(return not-recovered) so the fallback chain can activate instead of
|
||||
looping on the same dead credential.
|
||||
"""
|
||||
refreshed_entry = SimpleNamespace(label="primary", id="abc")
|
||||
|
||||
class _Pool:
|
||||
def try_refresh_current(self):
|
||||
return refreshed_entry
|
||||
|
||||
agent._credential_pool = _Pool()
|
||||
agent._swap_credential = MagicMock()
|
||||
agent._auth_pool_refresh_counts = {}
|
||||
|
||||
first = agent._recover_with_credential_pool(status_code=401, has_retried_429=False)
|
||||
second = agent._recover_with_credential_pool(status_code=401, has_retried_429=False)
|
||||
third = agent._recover_with_credential_pool(status_code=401, has_retried_429=False)
|
||||
|
||||
assert first == (True, False)
|
||||
assert second == (True, False)
|
||||
# Third same-entry refresh exceeds the cap → not recovered, fall through.
|
||||
assert third == (False, False)
|
||||
assert agent._swap_credential.call_count == 2
|
||||
|
||||
def test_recover_with_pool_401_cap_is_per_entry(self, agent):
|
||||
"""Rotating to a different entry resets the per-entry refresh tally."""
|
||||
entry_a = SimpleNamespace(label="primary", id="aaa")
|
||||
entry_b = SimpleNamespace(label="secondary", id="bbb")
|
||||
sequence = [entry_a, entry_a, entry_b, entry_b]
|
||||
|
||||
class _Pool:
|
||||
def try_refresh_current(self):
|
||||
return sequence.pop(0)
|
||||
|
||||
agent._credential_pool = _Pool()
|
||||
agent._swap_credential = MagicMock()
|
||||
agent._auth_pool_refresh_counts = {}
|
||||
|
||||
# Two refreshes of entry_a, then two of entry_b — neither hits the cap,
|
||||
# so all four recover. The (provider, id) key isolates the tallies.
|
||||
results = [
|
||||
agent._recover_with_credential_pool(status_code=401, has_retried_429=False)
|
||||
for _ in range(4)
|
||||
]
|
||||
|
||||
assert all(r == (True, False) for r in results)
|
||||
assert agent._swap_credential.call_count == 4
|
||||
|
||||
def test_recover_with_pool_rotates_on_401_when_refresh_fails(self, agent):
|
||||
"""401 with failed refresh should rotate to next credential."""
|
||||
next_entry = SimpleNamespace(label="secondary", id="def")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue