refactor(fallback): single owner for backend identity and failure-scoped skips

Every fallback/dedup/skip decision asks one question — 'is this candidate
the same backend as the one that failed, along the axis that failure
invalidated?' — but it was re-implemented inline at six sites across four
subsystems, each comparing whatever string was locally convenient. Each
incident fixed one site while the others kept the bug: #22548, #70893,
#59561, #72468, #62984/#54250/#57584.

agent/backend_identity.py now owns the concept: BackendIdentity (provider /
model / base_url axes), FailureScope (MODEL / CREDENTIAL / ENDPOINT — each
failure class invalidates a different axis), and should_skip_candidate().
Unknown axes never manufacture a skip (over-skipping strands failover; a
wrong try costs one RTT).

Migrated sites:
- chat_completion_helpers.try_activate_fallback: replaces the provider+model
  early-exit (the #62984 bug: ignored base_url, stranding multi-endpoint
  pools) AND _fallback_entry_is_same_backend_by_base_url (deleted)
- auxiliary_client._try_configured_fallback_chain +
  _try_main_agent_model_fallback: replace label/model comparisons; auth and
  payment map to CREDENTIAL scope, keeping the #59561 carve-out
- hermes_cli/fallback_cmd add: primary-match + duplicate checks now identity-
  aware (#54250/#57584): same provider+model on a different explicit
  base_url is a pool entry, not a duplicate

_mark_provider_unhealthy stays label-keyed deliberately: its only triggers
are confirmed 402s, which ARE credential-scoped.

Owner-level tests pin each incident's semantics by number; sabotage-verified
(removing the base_url axis fails the #62984 test).
This commit is contained in:
teknium1 2026-07-26 23:21:26 -07:00 committed by Teknium
parent c7b75a7849
commit 39b5965569
5 changed files with 446 additions and 77 deletions

View file

@ -4230,10 +4230,20 @@ def _try_main_agent_model_fallback(
if not main_provider or not main_model or main_provider.lower() in {"auto", ""}:
return None, None, ""
skip = (failed_provider or "").lower().strip()
# Identity + scope semantics owned by agent.backend_identity (#72468):
# model-scoped failures skip only the exact deployment that failed;
# provider-wide failures (no failed_model) skip the credential surface.
from agent.backend_identity import (
BackendIdentity,
FailureScope,
should_skip_candidate,
)
skip_model = (failed_model or "").strip().lower() or None
if main_provider.lower() == skip and (
skip_model is None or main_model.lower() == skip_model
if should_skip_candidate(
BackendIdentity.build(provider=main_provider, model=main_model),
BackendIdentity.build(provider=failed_provider, model=skip_model),
FailureScope.MODEL if skip_model else FailureScope.CREDENTIAL,
):
# The thing that failed IS the main model (or the failure was
# provider-wide) — nothing to fall back to.
@ -4384,8 +4394,24 @@ def _try_configured_fallback_chain(
if not chain or not isinstance(chain, list):
return None, None, ""
skip = failed_provider.lower().strip()
skip_model = (failed_model or "").strip().lower() or None
# Identity + scope semantics owned by agent.backend_identity (#59561,
# #72468): a failed_model means the failure was model-scoped (timeout /
# connection / rate limit) — only the exact deployment is skipped; no
# failed_model means provider-wide (auth/payment) — the whole credential
# surface is skipped.
from agent.backend_identity import (
BackendIdentity,
FailureScope,
should_skip_candidate,
)
failed_ident = BackendIdentity.build(
provider=failed_provider, model=skip_model,
)
failure_scope = (
FailureScope.MODEL if skip_model else FailureScope.CREDENTIAL
)
tried = []
min_ctx = _task_minimum_context_length(task)
@ -4396,8 +4422,14 @@ def _try_configured_fallback_chain(
if not fb_provider:
continue
fb_model_raw = str(entry.get("model", "")).strip()
if fb_provider.lower() == skip and (
skip_model is None or fb_model_raw.lower() == skip_model
if should_skip_candidate(
BackendIdentity.build(
provider=fb_provider,
model=fb_model_raw,
base_url=str(entry.get("base_url") or ""),
),
failed_ident,
failure_scope,
):
continue
fb_model = fb_model_raw or None

204
agent/backend_identity.py Normal file
View file

@ -0,0 +1,204 @@
"""Single owner for backend identity and failure-scoped skip decisions.
Every fallback / dedup / skip / quarantine decision in Hermes ultimately asks
one question: **"is this candidate the same backend as the one that failed,
along the axis that failure invalidated?"** Before this module, that
question was re-implemented inline at six call sites across four subsystems,
each comparing whatever string was locally convenient (provider label,
provider+model, base_url+model, ...). Each incident fixed one site while the
others kept the bug: #22548 (same-shim aliases), #70893 (xai-oauth vs xai —
same host, distinct credential), #59561 (aux chain skipped sibling models),
#72468 (aux main-model safety net, same bug three weeks later), #62984 /
#54250 / #57584 (dedup ignoring base_url strands multi-endpoint pools).
The root insight: "provider" conflates three independent identity axes, and
each failure class invalidates a different one:
* **credential surface** auth 401 / payment 402 kill everything sharing the
credential (every model, every host reached with that key/token).
* **endpoint** DNS failure / connection refused kill everything behind the
URL, regardless of model or credential.
* **model deployment** timeout / overload / rate limit / model-incompatible
kill ONE model's deployment. A sibling model behind the same URL is an
independent deployment (real incident: aux ``glm-5.2`` hung and timed out
while main ``macaron-v1-venti`` on the identical endpoint was serving
448K-token turns).
Call sites should build :class:`BackendIdentity` values, classify the failure
with :func:`classify_failure_scope`, and ask :func:`should_skip_candidate`.
Do not re-implement any comparison inline extend THIS module instead.
"""
from __future__ import annotations
import logging
from dataclasses import dataclass
from enum import Enum
from typing import Optional
logger = logging.getLogger(__name__)
class FailureScope(Enum):
"""Which identity axis a failure invalidates."""
#: Timeout, overload/429, connection blip, model-incompatible, invalid
#: response: evidence against ONE model deployment only.
MODEL = "model"
#: Auth 401 / payment 402: evidence against the shared credential —
#: every model reached with it is equally dead.
CREDENTIAL = "credential"
#: DNS / connection-refused / unreachable host: evidence against the
#: endpoint — every model behind the URL is equally dead.
ENDPOINT = "endpoint"
#: Reason strings already used by auxiliary_client's except-chain, mapped to
#: scopes. Unknown reasons default to MODEL — the least-invalidating scope —
#: so an unrecognized failure never over-skips viable candidates.
_REASON_SCOPES = {
"auth error": FailureScope.CREDENTIAL,
"payment error": FailureScope.CREDENTIAL,
"rate limit": FailureScope.MODEL,
"model incompatible with route": FailureScope.MODEL,
"invalid provider response": FailureScope.MODEL,
"connection error": FailureScope.MODEL,
"timeout": FailureScope.MODEL,
}
def classify_failure_scope(reason: Optional[str]) -> FailureScope:
"""Map a human-readable failure reason to the identity axis it kills."""
return _REASON_SCOPES.get((reason or "").strip().lower(), FailureScope.MODEL)
def _norm_provider(value: Optional[str]) -> str:
return (value or "").strip().lower()
def _norm_model(value: Optional[str]) -> str:
return (value or "").strip().lower()
def _norm_base_url(value: Optional[str]) -> str:
return (value or "").strip().rstrip("/").lower()
@dataclass(frozen=True)
class BackendIdentity:
"""Normalized identity of one (provider, model, endpoint) deployment.
Empty fields mean "unknown" comparisons treat an unknown axis as
non-distinguishing (it can neither prove sameness nor difference on its
own; the remaining axes decide).
"""
provider: str = ""
model: str = ""
base_url: str = ""
@classmethod
def build(
cls,
provider: Optional[str] = None,
model: Optional[str] = None,
base_url: Optional[str] = None,
) -> "BackendIdentity":
return cls(
provider=_norm_provider(provider),
model=_norm_model(model),
base_url=_norm_base_url(base_url),
)
def _both_first_class(a: BackendIdentity, b: BackendIdentity) -> bool:
"""True when both providers are distinct registered first-class providers.
Two different registry providers have distinct credential surfaces even
when they share an inference host (xai-oauth vs xai, openai-codex vs
openai-api) #70893. Custom/shim aliases are NOT in the registry, so
two aliases pointing at one URL still count as the same backend (#22548).
"""
if not a.provider or not b.provider or a.provider == b.provider:
return False
try:
from hermes_cli.auth import PROVIDER_REGISTRY
return a.provider in PROVIDER_REGISTRY and b.provider in PROVIDER_REGISTRY
except Exception:
return False
def same_credential_surface(a: BackendIdentity, b: BackendIdentity) -> bool:
"""Do two identities share the credential a 401/402 just invalidated?
Conservative on purpose: an unprovable axis must answer "different"
(try the candidate worst case one wasted RTT) rather than "same"
(skip worst case stranded failover). Two distinct custom labels at
one URL may carry different per-entry api_keys, so a shared URL alone
never proves a shared credential; it is only used as a weak signal
when a provider label is missing entirely.
"""
if a.provider and b.provider:
# Same label = same configured credential. Different labels =
# different credential config (first-class registry providers
# explicitly so — #70893; custom entries can each carry their own
# api_key, so sameness is unprovable and we must not skip).
return a.provider == b.provider
# Provider unknown on a side: same explicit URL is the best signal left.
return bool(a.base_url and a.base_url == b.base_url)
def same_endpoint(a: BackendIdentity, b: BackendIdentity) -> bool:
"""Do two identities sit behind the endpoint that just went unreachable?"""
if a.base_url and b.base_url:
return a.base_url == b.base_url
# An unknown base_url inherits the provider default → same provider
# label implies the same default endpoint.
return bool(a.provider and a.provider == b.provider)
def same_deployment(a: BackendIdentity, b: BackendIdentity) -> bool:
"""Are these the exact same model deployment (the thing a timeout kills)?
Provider+model must match; the base_url axis distinguishes only when BOTH
sides carry an explicit URL (#62984: same provider+model on two different
explicit URLs is two deployments a pool). A side with an unknown URL
inherits the provider default and cannot prove difference.
"""
if not (a.provider and b.provider and a.provider == b.provider):
# Same-host different-label shims: same URL + same model IS the same
# deployment even when the alias labels differ (#22548) — unless both
# labels are first-class registry providers (#70893).
if (
a.base_url
and a.base_url == b.base_url
and a.model
and a.model == b.model
and not _both_first_class(a, b)
):
return True
return False
if not (a.model and b.model and a.model == b.model):
return False
if a.base_url and b.base_url and a.base_url != b.base_url:
return False # distinct explicit endpoints — a pool, not a dup
return True
def should_skip_candidate(
candidate: BackendIdentity,
failed: BackendIdentity,
scope: FailureScope = FailureScope.MODEL,
) -> bool:
"""THE skip predicate: would trying ``candidate`` just repeat the failure?
True when the candidate is the same backend as ``failed`` along the axis
``scope`` says the failure invalidated. Every fallback/dedup/skip site
must call this instead of comparing labels inline.
"""
if scope is FailureScope.CREDENTIAL:
return same_credential_surface(candidate, failed)
if scope is FailureScope.ENDPOINT:
return same_endpoint(candidate, failed)
return same_deployment(candidate, failed)

View file

@ -1544,45 +1544,6 @@ def _fallback_entry_key(fb: dict) -> tuple[str, str, str]:
)
def _fallback_entry_is_same_backend_by_base_url(
*,
current_provider: str,
fb_provider: str,
current_base_url: str,
fb_base_url: str,
current_model: str,
fb_model: str,
) -> bool:
"""True when base_url+model identity means the fallback is the same backend.
Issue #22548: two ``custom_providers`` aliases that point at the same shim
URL with the same model must be skipped, or failover loops on the dead
backend. First-class providers that share a host while using different
auth (``xai-oauth`` vs ``xai``, ``openai-codex`` vs ``openai-api``) are
distinct credential surfaces skipping them strands configured failover
when primary and fallback reuse the same model slug on that host.
"""
if not (
fb_base_url
and current_base_url
and fb_base_url == current_base_url
and fb_model == current_model
):
return False
if fb_provider == current_provider:
return True
try:
from hermes_cli.auth import PROVIDER_REGISTRY
# Both sides are registered first-class providers → different auth
# identities even when the inference host matches. Allow failover.
if current_provider in PROVIDER_REGISTRY and fb_provider in PROVIDER_REGISTRY:
return False
except Exception:
pass
return True
def _fallback_entry_unavailable_without_network(agent, fb: dict) -> Optional[str]:
"""Return a skip reason for fallback entries known to be unusable locally."""
fb_provider = (fb.get("provider") or "").strip().lower()
@ -1668,33 +1629,28 @@ def try_activate_fallback(agent, reason: "FailoverReason | None" = None) -> bool
)
return agent._try_activate_fallback(reason)
# Skip entries that resolve to the current (provider, model) — falling
# back to the same backend that just failed loops the failure. Compare
# base_url too so two distinct custom_providers entries pointing at the
# same shim/proxy URL also dedup. See issue #22548. Do NOT treat
# first-class providers that share a host (xai-oauth vs xai) as the same
# backend — they use different credentials.
current_provider = (getattr(agent, "provider", "") or "").strip().lower()
current_model = (getattr(agent, "model", "") or "").strip()
current_base_url = str(getattr(agent, "base_url", "") or "").rstrip("/").lower()
fb_base_url_for_dedup = (fb.get("base_url") or "").strip().rstrip("/").lower()
if fb_provider == current_provider and fb_model == current_model:
# Skip entries that resolve to the same backend that just failed —
# falling back to it loops the failure. Identity semantics (which axes
# distinguish two backends, shim aliases, first-class credential
# surfaces, multi-endpoint pools) are owned by agent.backend_identity —
# see #22548, #70893, #62984. Do not re-implement comparisons here.
from agent.backend_identity import BackendIdentity, should_skip_candidate
current_ident = BackendIdentity.build(
provider=getattr(agent, "provider", ""),
model=getattr(agent, "model", ""),
base_url=str(getattr(agent, "base_url", "") or ""),
)
fb_ident = BackendIdentity.build(
provider=fb_provider,
model=fb_model,
base_url=(fb.get("base_url") or ""),
)
if should_skip_candidate(fb_ident, current_ident):
logger.warning(
"Fallback skip: chain entry %s/%s matches current provider/model",
fb_provider, fb_model,
)
return agent._try_activate_fallback(reason)
if _fallback_entry_is_same_backend_by_base_url(
current_provider=current_provider,
fb_provider=fb_provider,
current_base_url=current_base_url,
fb_base_url=fb_base_url_for_dedup,
current_model=current_model,
fb_model=fb_model,
):
logger.warning(
"Fallback skip: chain entry base_url %s matches current backend",
fb_base_url_for_dedup,
"Fallback skip: chain entry %s/%s resolves to the same backend "
"as the current one (%s)",
fb_provider, fb_model, current_ident.base_url or current_ident.provider,
)
return agent._try_activate_fallback(reason)

View file

@ -184,10 +184,26 @@ def cmd_fallback_add(args) -> None:
return
# Picker picked the same thing that's already the primary → nothing changed,
# and there's nothing useful to add as a fallback to itself.
# and there's nothing useful to add as a fallback to itself. Identity
# semantics owned by agent.backend_identity (#54250/#57584/#62984): same
# provider+model on a DIFFERENT explicit base_url is a different backend
# (multi-endpoint pool) and is a legitimate fallback.
from agent.backend_identity import BackendIdentity, same_deployment
new_ident = BackendIdentity.build(
provider=new_entry.get("provider"),
model=new_entry.get("model"),
base_url=new_entry.get("base_url"),
)
primary_entry = _extract_fallback_from_model_cfg(model_before)
if primary_entry and primary_entry["provider"] == new_entry["provider"] \
and primary_entry["model"] == new_entry["model"]:
if primary_entry and same_deployment(
BackendIdentity.build(
provider=primary_entry.get("provider"),
model=primary_entry.get("model"),
base_url=primary_entry.get("base_url"),
),
new_ident,
):
_restore_model_cfg(model_before)
_restore_auth_active_provider(active_provider_before)
print()
@ -205,10 +221,17 @@ def cmd_fallback_add(args) -> None:
final_cfg = load_config()
chain = _read_chain(final_cfg)
# Reject exact-duplicate fallback entries.
# Reject exact-duplicate fallback entries (same deployment; a different
# explicit base_url is a different endpoint and NOT a duplicate).
for existing in chain:
if existing.get("provider") == new_entry["provider"] \
and existing.get("model") == new_entry["model"]:
if same_deployment(
BackendIdentity.build(
provider=existing.get("provider"),
model=existing.get("model"),
base_url=existing.get("base_url"),
),
new_ident,
):
print()
print(f" {_format_entry(new_entry)} is already in the fallback chain — skipped.")
return

View file

@ -0,0 +1,154 @@
"""Owner-level tests for agent.backend_identity.
This module is the single owner of the "same backend?" question tests
live HERE, against the predicate, not against each call site (see
references/never-patch-predicates.md in hermes-agent-dev). Each test names
the incident whose semantics it pins.
"""
from unittest.mock import patch
from agent.backend_identity import (
BackendIdentity,
FailureScope,
classify_failure_scope,
same_credential_surface,
same_deployment,
same_endpoint,
should_skip_candidate,
)
def _id(provider="", model="", base_url=""):
return BackendIdentity.build(provider=provider, model=model, base_url=base_url)
class TestClassifyFailureScope:
def test_auth_and_payment_are_credential_scoped(self):
assert classify_failure_scope("auth error") is FailureScope.CREDENTIAL
assert classify_failure_scope("payment error") is FailureScope.CREDENTIAL
def test_model_scoped_reasons(self):
for reason in (
"rate limit",
"timeout",
"connection error",
"model incompatible with route",
"invalid provider response",
):
assert classify_failure_scope(reason) is FailureScope.MODEL, reason
def test_unknown_reason_defaults_to_least_invalidating_scope(self):
"""Never over-skip on a reason string we don't recognize."""
assert classify_failure_scope("weird new error") is FailureScope.MODEL
assert classify_failure_scope(None) is FailureScope.MODEL
assert classify_failure_scope("") is FailureScope.MODEL
class TestSameDeployment:
def test_incident_59561_72468_sibling_model_same_provider_is_different(self):
"""aux glm-5.2 timing out says nothing about main macaron on the
same custom endpoint sibling models are independent deployments."""
failed = _id("custom", "zai-org/glm-5.2")
sibling = _id("custom", "mindai/macaron-v1-venti")
assert not same_deployment(sibling, failed)
assert not should_skip_candidate(sibling, failed, FailureScope.MODEL)
def test_exact_same_deployment_is_skipped(self):
failed = _id("custom", "zai-org/glm-5.2")
same = _id("custom", "ZAI-ORG/GLM-5.2") # case-insensitive
assert same_deployment(same, failed)
assert should_skip_candidate(same, failed, FailureScope.MODEL)
def test_incident_62984_same_model_different_explicit_url_is_a_pool(self):
"""Several LM Studio endpoints serving one model = a pool, not dups."""
a = _id("custom", "qwen-3", "http://box1:1234/v1")
b = _id("custom", "qwen-3", "http://box2:1234/v1")
assert not same_deployment(a, b)
assert not should_skip_candidate(a, b, FailureScope.MODEL)
def test_unknown_url_inherits_provider_default_and_dedups(self):
"""An entry without base_url inherits the provider default — it
cannot prove it is a different endpoint (#62984 semantics)."""
a = _id("openrouter", "z-ai/glm-4.7")
b = _id("openrouter", "z-ai/glm-4.7", "https://openrouter.ai/api/v1")
assert same_deployment(a, b)
def test_incident_22548_shim_aliases_same_url_same_model_are_same(self):
"""Two custom_providers aliases at one shim URL with one model."""
a = _id("claude-cli", "claude-opus-4.7", "http://127.0.0.1:7891/v1")
b = _id("claude-cli-alt", "claude-opus-4.7", "http://127.0.0.1:7891/v1")
assert same_deployment(a, b)
def test_incident_70893_first_class_pair_same_host_is_not_same(self):
"""xai-oauth vs xai share api.x.ai but are distinct backends."""
fake_registry = {"xai": object(), "xai-oauth": object()}
with patch("hermes_cli.auth.PROVIDER_REGISTRY", fake_registry):
a = _id("xai-oauth", "grok-4.5", "https://api.x.ai/v1")
b = _id("xai", "grok-4.5", "https://api.x.ai/v1")
assert not same_deployment(a, b)
assert not should_skip_candidate(a, b, FailureScope.MODEL)
class TestSameCredentialSurface:
def test_same_provider_label_shares_credential(self):
"""Auth/payment failure on a provider kills every model on it —
including the main model (the #59561/#72468 carve-out)."""
failed = _id("custom", "zai-org/glm-5.2")
sibling = _id("custom", "mindai/macaron-v1-venti")
assert same_credential_surface(sibling, failed)
assert should_skip_candidate(sibling, failed, FailureScope.CREDENTIAL)
def test_incident_70893_distinct_first_class_providers_differ(self):
fake_registry = {"xai": object(), "xai-oauth": object()}
with patch("hermes_cli.auth.PROVIDER_REGISTRY", fake_registry):
a = _id("xai", "grok-4.5", "https://api.x.ai/v1")
b = _id("xai-oauth", "grok-4.5", "https://api.x.ai/v1")
assert not same_credential_surface(a, b)
assert not should_skip_candidate(a, b, FailureScope.CREDENTIAL)
def test_distinct_custom_labels_same_url_not_assumed_shared(self):
"""Two custom entries can carry their own api_key each — sameness is
unprovable, so failover must be allowed (conservative direction)."""
a = _id("proxy-a", "m", "http://gw:9000/v1")
b = _id("proxy-b", "m", "http://gw:9000/v1")
assert not same_credential_surface(a, b)
def test_missing_provider_falls_back_to_url_signal(self):
a = _id("", "m", "http://gw:9000/v1")
b = _id("proxy-b", "m2", "http://gw:9000/v1")
assert same_credential_surface(a, b)
class TestSameEndpoint:
def test_same_explicit_url_is_same_endpoint(self):
a = _id("a", "m1", "http://host:8000/v1/")
b = _id("b", "m2", "http://HOST:8000/v1") # trailing slash + case
assert same_endpoint(a, b)
assert should_skip_candidate(a, b, FailureScope.ENDPOINT)
def test_different_urls_are_different_endpoints(self):
assert not same_endpoint(
_id("a", "m", "http://h1/v1"), _id("a2", "m", "http://h2/v1")
)
def test_unknown_url_falls_back_to_provider_default(self):
assert same_endpoint(_id("openrouter", "m1"), _id("openrouter", "m2"))
assert not same_endpoint(_id("openrouter", "m"), _id("nous", "m"))
class TestUnknownAxesNeverStrand:
"""The failure mode that produced this module: over-skipping. An
unprovable axis must never manufacture a skip."""
def test_empty_candidate_never_skipped(self):
failed = _id("custom", "glm", "http://h/v1")
for scope in FailureScope:
assert not should_skip_candidate(_id(), failed, scope), scope
def test_model_scope_requires_model_evidence(self):
# Failed side has no model recorded → cannot prove the candidate is
# the same deployment → do not skip.
failed = _id("custom")
candidate = _id("custom", "some-model")
assert not should_skip_candidate(candidate, failed, FailureScope.MODEL)