test(agent): pin the auth-refresh retry boundary and the pool-gate split

Review feedback on the fall-through fix: the accept boundary and the
retry-fails-with-a-connection-error path through the credential rung were
reachable but untested, and two comments described behavior the code does
not have. Tests and comments only, no behavior change.
This commit is contained in:
Synero
2026-09-14 03:11:26 +00:00
committed by Teknium
parent 64a4687153
commit 1f88ebf3ec
2 changed files with 78 additions and 6 deletions
+10 -2
View File
@@ -7033,10 +7033,17 @@ def _ladder_credential_rungs(
step, lambda exc: _credential_rung_accepts(exc) or _is_connection_error(exc))
if first_err is None:
return resp, None
# ``first_err`` is now the retry's own failure, not the original auth error: the
# pool gate below and the ladder tail's eviction check both read this narrowed
# value. An unclaimed failure (e.g. a 500) re-raised out of ``_rung`` above
# instead, since the provider-fallback rung only acts on ``_FALLBACK_REASONS``.
pool_provider = _recoverable_pool_provider(resolved_provider, client, main_runtime=route.main_runtime)
# Capture the exact key used so recovery finds the right pool entry even if another
# process rotated the pool meanwhile (current() would be None).
_client_api_key = str(getattr(client, "api_key", "") or "")
# Gate on the narrowed error: a connection failure from the retry above arrives here
# unaccepted on purpose (a fresh key cannot fix an unreachable endpoint), so rotation
# is skipped and ``first_err`` is handed to the provider-fallback chain as-is.
if pool_provider and _credential_rung_accepts(first_err):
recovery_err = first_err
# Skip the extra retry for clear payment/quota errors — the endpoint won't accept
@@ -7192,8 +7199,9 @@ def _aux_recovery_ladder(
return resp
# Connection/timeout errors poison the cached client (closed transport, half-read
# stream); evict so the next aux call rebuilds a fresh one.
# Drop it from the cache regardless of whether we found a fallback above so the next auxiliary call
# rebuilds a fresh client instead of reusing the dead one. See issue #23432.
# Reached only when no fallback answered, so the next auxiliary call rebuilds a fresh
# client instead of reusing the dead one. ``first_err`` is the narrowed error from the
# rungs above, not necessarily the original one. See issue #23432.
# Mirror the sync path: drop poisoned clients on connection/timeout so the next aux call rebuilds. See
# issue #23432.
if _is_connection_error(first_err):
@@ -11,8 +11,12 @@ bare ``yield`` inside a ``return`` statement:
return (yield _LadderStep(
"retry_same_provider", ...)), None # generic credential rung
``_rung()`` exists to convert a retry failure into ``(None, exc)`` so the caller can
fall through to the next rung. Used this way no exception is caught: when the
``_rung()`` exists to convert a retry failure into ``(None, exc)`` -- but only when the
rung's accept predicate claims the error -- so the caller can fall through to the next
rung. An unclaimed failure (a 500, a malformed response) re-raises on purpose, since
``_ladder_provider_fallback`` only acts on the reasons in ``_FALLBACK_REASONS``; that
boundary is pinned by ``test_post_refresh_retry_reraises_non_recoverable_error``.
Used this way no exception is caught: when the
refreshed client also fails (e.g. an out-of-credit 404 on a stale Nous runtime token),
the error escapes ``_aux_recovery_ladder`` and ``_ladder_provider_fallback`` never
runs -- the configured ``auxiliary.<task>.fallback_chain`` is silently skipped. The
@@ -92,9 +96,9 @@ def hermetic(monkeypatch):
chain_calls = []
def _fake_provider_fallback(first_err, route):
"""Stands in for the last rung: a generator that performs no steps."""
chain_calls.append(first_err)
if False: # pragma: no cover - makes this a generator function
yield None
yield from ()
return "chain-response"
monkeypatch.setattr(aux, "_recoverable_pool_provider", lambda *a, **kw: None)
@@ -278,3 +282,63 @@ def test_auth_refresh_retry_failure_reaches_the_configured_chain_over_http(
"401, then the refreshed retry fails on credits, then the configured chain: %r"
% (seen,)
)
def test_post_refresh_retry_reraises_non_recoverable_error(monkeypatch, hermetic):
"""A retry failure the rung does not claim re-raises instead of voiding the ladder.
``_rung()`` only converts a failure into ``(None, exc)`` when the rung's accept
predicate claims it. The post-refresh retry claims credential and connection
failures; anything else (a 500, a malformed response) re-raises on purpose, because
``_ladder_provider_fallback`` only acts on the reasons in ``_FALLBACK_REASONS``.
Pinning it here turns the boundary that lives in the lambda into a documented
contract: the retry's own error surfaces, and no fallback is pretended.
"""
monkeypatch.setattr(aux, "_refresh_nous_auxiliary_client",
lambda **kwargs: (_FakeClient(), AUX_MODEL))
ladder = _ladder()
performed = []
def perform(step):
performed.append(step.kind)
raise _ApiError("Error code: 500 - Internal Server Error", status_code=500)
with pytest.raises(_ApiError) as excinfo:
aux._drive_ladder(ladder, perform)
assert "500" in str(excinfo.value), "the retry's own failure must surface"
assert performed == ["call"], "the post-refresh retry is the only request"
assert hermetic == [], "an unclaimed failure has no reason to consult the chain"
def test_post_refresh_retry_connection_error_skips_pool_rotation(monkeypatch, hermetic):
"""A connection failure from the refresh retry skips rotation and reaches the chain.
The retry predicate accepts connection errors, the pool gate below it does not: a
fresh key cannot fix an unreachable endpoint, so the narrowed ``first_err`` goes
straight to the provider fallback. The ladder tail then evicts the poisoned route
client based on that narrowed error, not on the original one.
"""
monkeypatch.setattr(aux, "_auth_refresh_provider_for_route", lambda *a, **kw: "codex")
monkeypatch.setattr(aux, "_refresh_provider_credentials", lambda *a, **kw: True)
monkeypatch.setattr(aux, "_evict_cached_clients", lambda *a, **kw: None)
rotations = []
monkeypatch.setattr(aux, "_recoverable_pool_provider", lambda *a, **kw: "openrouter")
monkeypatch.setattr(aux, "_recover_provider_pool",
lambda *a, **kw: rotations.append(a) or True)
ladder = _ladder(
base_info="https://openrouter.ai/api/v1", resolved_provider="openrouter")
retry_error = ConnectionError("Connection refused by the endpoint")
performed = []
def perform(step):
performed.append(step.kind)
raise retry_error
assert aux._drive_ladder(ladder, perform) == "chain-response"
assert performed == ["retry_same_provider"], "only the post-refresh retry is attempted"
assert rotations == [], "a connection error must not rotate credentials"
assert hermetic and hermetic[0] is retry_error, (
"the chain sees the narrowed connection error, not the original auth error"
)