From b3ac8d7f1f9dbe79ac7c7a7f430e22870a4cfae4 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Sun, 7 Jun 2026 18:00:27 -0700 Subject: [PATCH] fix(oneshot): honor fallback_providers chain during worker startup, not just runtime MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #6. Problem Worker startup calls `resolve_runtime_provider` to acquire credentials for the primary provider. If that raises AuthError (xAI OAuth token expired, Anthropic logged out, Codex revoked), the worker crashes before AIAgent's runtime fallback loop ever gets a chance — even though the user has explicitly configured a fallback chain for exactly this case. Observed in the v6.6 incident 2026-06-07: xAI OAuth token went missing mid-session and every subsequent worker crashed at startup despite having `fallback_providers: [openai-codex/gpt-5.5, xai-oauth/grok-4.3]` configured. Solution New helper `_resolve_runtime_with_fallback` wraps the primary-resolution call. On AuthError, iterates the configured fallback chain (read once from `get_fallback_chain(cfg)`) until one succeeds. If all fail, re-raises the LAST AuthError so cli.py's exit handling can surface it. Three safety bounds preserved (informed by code-review): 1. **Explicit CLI pin** — `hermes -z --model X --provider Y ...` should NOT silently downgrade. When `model` OR `provider` was a non-empty CLI arg, the helper re-raises primary AuthError verbatim, no fallback attempt. 2. **Rate-limit AuthError on primary** — falling through to other providers would burn their quota in milliseconds (the "quota amplification" footgun). Detected via existing `is_rate_limited_auth_error()` — re-raise immediately; existing rate-limit handling (cli.py exit 75) gets the task requeued. 3. **Remaining-chain handoff to AIAgent** — when fallback lands on chain entry [N], AIAgent's runtime fallback loop should only see entries AFTER N (not the dead primary, not the entry we just used). The helper now returns `(runtime, effective_model, landed_at_index, remaining_chain)` and the caller passes `remaining` to AIAgent's `fallback_model`. Implementation - `hermes_cli/oneshot.py:33-110` — new helper (testable at module level). - `hermes_cli/oneshot.py:439-460` — call site updated; reads chain once, detects explicit_pin from CLI args, passes remaining_chain to AIAgent. - AIAgent receives the correctly-sliced chain via `fallback_model=_fb`, preserving existing runtime-fallback semantics for mid-conversation failures. Tests (9/9 passing) — tests/cli/test_oneshot_runtime_fallback.py - primary succeeds → no fallback attempted, full chain preserved for AIAgent - primary fails, first fallback succeeds → effective_model advances, remaining_chain sliced correctly - two failures → third succeeds, slicing correct - all fail → LAST AuthError propagates (not primary's) - empty chain → primary error verbatim - fallback without model → effective_model preserved - explicit_pin=True → no fallback, primary error verbatim - rate-limit AuthError → no fallback, primary error verbatim - same provider in chain → no infinite loop, advances to next entry Code-review pre-merge: reviewer caught silent-downgrade regression, stale chain handoff, and quota-amplification footgun. All three addressed. Follow-up (separate issues, not blocking) - Consider applying the same pattern to `gateway/run.py:_resolve_runtime_agent_kwargs` and `cli.py:4881-4914` for a consistent worker-startup contract across surfaces. - Optional: emit a metric/heartbeat counter when fallback fires so we can detect "constantly failing primary" silently. Co-Authored-By: Claude Opus 4.7 (1M context) --- hermes_cli/oneshot.py | 152 ++++++++++- tests/cli/test_oneshot_runtime_fallback.py | 281 +++++++++++++++++++++ 2 files changed, 426 insertions(+), 7 deletions(-) create mode 100644 tests/cli/test_oneshot_runtime_fallback.py diff --git a/hermes_cli/oneshot.py b/hermes_cli/oneshot.py index f66d71c62e6d..f94ef0f41bed 100644 --- a/hermes_cli/oneshot.py +++ b/hermes_cli/oneshot.py @@ -27,9 +27,131 @@ from contextlib import redirect_stderr, redirect_stdout from typing import Optional +from hermes_cli.auth import AuthError from hermes_cli.fallback_config import get_fallback_chain +def _resolve_runtime_with_fallback( + *, + resolve_runtime_provider, + effective_provider: Optional[str], + effective_model: str, + explicit_base_url: Optional[str], + fallback_chain: list, + logger: logging.Logger, + explicit_pin: bool = False, +) -> tuple[dict, str, int, list]: + """Resolve runtime credentials with fallback-chain tolerance. + + Tries the primary provider first. On AuthError, iterates the configured + fallback chain (typically Claude → Codex → Grok 4.3 per team policy) until + one succeeds. Raises the LAST AuthError if every fallback also fails. + + This closes hermes-agent#6: a single provider's auth failure (e.g. xAI OAuth + token expired) used to crash worker startup before AIAgent's runtime + fallback loop could ever try the next provider. The fallback chain was + configured precisely for this case but was only honored AFTER successful + credential resolution, not during it. + + Special cases: + * ``explicit_pin=True`` — the caller pinned model AND/OR provider on + the CLI (e.g. ``hermes -z --model grok-4.3 --provider xai-oauth``). + Silent downgrade would surprise them, so we re-raise the primary + AuthError verbatim with no fallback attempt. + * Rate-limit AuthError on the primary — falling through wastes the + quota of every other configured provider in ~milliseconds (the + "quota amplification" footgun). Re-raise primary error verbatim; + existing rate-limit handling (cli.py exit code 75) takes over. + + Returns: + (runtime_dict, effective_model, landed_at_index, remaining_chain) + + ``landed_at_index`` is -1 if primary succeeded, else the 0-based + index into the original ``fallback_chain`` that resolved. Callers + should pass ``remaining_chain`` (the entries AFTER the landed-on + one) to AIAgent's ``fallback_model`` to avoid AIAgent re-attempting + the already-dead primary or the entry we just used. + """ + # Lazy-import to avoid a hard dependency at module load time (the rate-limit + # detector lives in hermes_cli.auth alongside AuthError). + try: + from hermes_cli.auth import is_rate_limited_auth_error + except ImportError: # pragma: no cover - defensive only + is_rate_limited_auth_error = lambda _exc: False # noqa: E731 + + try: + runtime = resolve_runtime_provider( + requested=effective_provider, + target_model=effective_model or None, + explicit_base_url=explicit_base_url, + ) + return runtime, effective_model, -1, list(fallback_chain) + except AuthError as primary_exc: + if explicit_pin: + # User explicitly pinned model/provider on the CLI — they would + # rather see the failure than get silently downgraded to another + # provider. Preserve that contract. + logger.warning( + "primary provider %r auth failed and caller pinned model/provider; " + "not attempting fallback (use auto-detection to enable fallback)", + effective_provider, + ) + raise + if is_rate_limited_auth_error(primary_exc): + # Rate limits are recoverable on the same provider after a cooldown. + # Falling through to the chain would burn quotas across every + # provider in milliseconds — the "quota amplification" footgun. + # Existing rate-limit handling (cli.py:16128 → exit code 75) gets + # this task requeued; let it do its job. + logger.warning( + "primary provider %r is rate-limited; not attempting fallback " + "(letting rate-limit retry path handle it)", + effective_provider, + ) + raise + if not fallback_chain: + # Nothing to fall back to — propagate original error verbatim. + raise + logger.warning( + "primary provider %r auth failed during worker startup (%s); " + "trying %d fallback provider(s) before giving up", + effective_provider, primary_exc, len(fallback_chain), + ) + last_exc: Exception = primary_exc + for fb_idx, fb in enumerate(fallback_chain): + fb_provider = (fb.get("provider") or "").strip() or None + fb_model = (fb.get("model") or "").strip() or None + fb_base_url = (fb.get("base_url") or "").strip() or None + try: + runtime = resolve_runtime_provider( + requested=fb_provider, + target_model=fb_model, + explicit_base_url=fb_base_url, + ) + # Successful fallback — update effective_model so AIAgent sees + # the right model for the provider we landed on. + new_effective_model = fb_model if fb_model else effective_model + logger.info( + "worker startup recovered: fallback[%d] %s/%s healthy", + fb_idx, fb_provider or "auto", fb_model or "", + ) + # Slice the chain so AIAgent's own runtime fallback loop only + # sees entries AFTER the one we just landed on. Avoids + # re-attempting the dead primary OR the entry we just used. + remaining = list(fallback_chain[fb_idx + 1:]) + return runtime, new_effective_model, fb_idx, remaining + except AuthError as fb_exc: + logger.debug( + "fallback[%d] %s/%s auth failed: %s", + fb_idx, fb_provider or "auto", fb_model or "", fb_exc, + ) + last_exc = fb_exc + continue + # All fallbacks exhausted. Re-raise the last AuthError; cli.py's + # generic exit handling will surface it as a non-zero exit. + raise last_exc + + def _normalize_toolsets(toolsets: object = None) -> list[str] | None: if not toolsets: return None @@ -314,10 +436,24 @@ def _run_agent( if detected: effective_provider, effective_model = detected - runtime = resolve_runtime_provider( - requested=effective_provider, - target_model=effective_model or None, - explicit_base_url=explicit_base_url_from_alias, + # Hoist the fallback chain once and reuse — avoids two reads of cfg and any + # TOCTOU window if cfg were mutated between calls. + _fb_chain = get_fallback_chain(cfg) or [] + # Caller pinned model/provider explicitly on the CLI (e.g. + # `hermes -z --model grok-4.3 --provider xai-oauth ...`) → silent + # downgrade to a fallback would surprise them. Detect from the original + # args, not the resolved effective_* values. + _explicit_pin = bool((model or "").strip() or (provider or "").strip()) + runtime, effective_model, _landed_idx, _remaining_fb = ( + _resolve_runtime_with_fallback( + resolve_runtime_provider=resolve_runtime_provider, + effective_provider=effective_provider, + effective_model=effective_model, + explicit_base_url=explicit_base_url_from_alias, + fallback_chain=_fb_chain, + logger=logging.getLogger(__name__), + explicit_pin=_explicit_pin, + ) ) # Pull in explicit toolsets when provided; otherwise use whatever the user @@ -328,9 +464,11 @@ def _run_agent( toolsets_list = sorted(_get_platform_tools(cfg, "cli")) session_db = _create_session_db_for_oneshot() - # Read the effective fallback chain from profile config so oneshot workers - # honour the same merge semantics as interactive CLI and gateway sessions. - _fb = get_fallback_chain(cfg) + # If we landed on a fallback during startup resolution, hand AIAgent only + # the entries AFTER the one we used — avoids re-trying the dead primary + # or the entry we just succeeded with when AIAgent's runtime fallback + # loop kicks in mid-conversation. + _fb = _remaining_fb agent = AIAgent( api_key=runtime.get("api_key"), diff --git a/tests/cli/test_oneshot_runtime_fallback.py b/tests/cli/test_oneshot_runtime_fallback.py new file mode 100644 index 000000000000..8ebee737fb31 --- /dev/null +++ b/tests/cli/test_oneshot_runtime_fallback.py @@ -0,0 +1,281 @@ +"""Tests for hermes_cli.oneshot._resolve_runtime_with_fallback. + +Closes hermes-agent#6: when the worker's primary provider auth fails during +startup (e.g. xAI OAuth token expired, Anthropic logged out, Codex revoked), +the configured fallback_providers chain should be tried before the worker +gives up. The fallback chain was previously only honored AFTER successful +credential resolution, not during initial startup. + +Three safety bounds preserved: +1. Explicit CLI pin (--model/--provider) → no silent downgrade, raise. +2. Rate-limit AuthError on primary → don't burn the chain, let rate-limit retry. +3. Successful fallback → AIAgent only sees the REMAINING chain entries (no + re-attempt of dead primary or already-used entry). +""" +from __future__ import annotations + +import logging + +import pytest + +from hermes_cli.auth import AuthError +from hermes_cli.oneshot import _resolve_runtime_with_fallback + + +def _runtime(provider: str, model: str = "x", api_key: str = "k") -> dict: + return { + "provider": provider, + "model": model, + "api_key": api_key, + "base_url": "https://example.test", + "api_mode": "openai_chat", + } + + +def test_primary_succeeds_no_fallback_attempted() -> None: + calls: list[tuple] = [] + + def fake_resolve(*, requested, target_model, explicit_base_url): + calls.append((requested, target_model, explicit_base_url)) + return _runtime(provider=requested or "auto", model=target_model or "x") + + runtime, model, landed_idx, remaining = _resolve_runtime_with_fallback( + resolve_runtime_provider=fake_resolve, + effective_provider="anthropic", + effective_model="claude-opus-4-7", + explicit_base_url=None, + fallback_chain=[{"provider": "xai-oauth", "model": "grok-4.3"}], + logger=logging.getLogger("test"), + ) + assert runtime["provider"] == "anthropic" + assert model == "claude-opus-4-7" + assert landed_idx == -1, "primary succeeded → landed_idx must be -1" + assert remaining == [{"provider": "xai-oauth", "model": "grok-4.3"}], ( + "primary succeeded → remaining chain unchanged for AIAgent's runtime loop" + ) + assert len(calls) == 1, "fallback should not have been touched" + + +def test_primary_auth_fails_first_fallback_succeeds() -> None: + calls: list[tuple] = [] + + def fake_resolve(*, requested, target_model, explicit_base_url): + calls.append((requested, target_model, explicit_base_url)) + if requested == "xai-oauth": + raise AuthError( + "xAI OAuth state is missing access_token", + provider="xai-oauth", + code="xai_auth_missing_access_token", + ) + return _runtime(provider=requested or "auto", model=target_model or "x") + + runtime, model, landed_idx, remaining = _resolve_runtime_with_fallback( + resolve_runtime_provider=fake_resolve, + effective_provider="xai-oauth", + effective_model="grok-4.3", + explicit_base_url=None, + fallback_chain=[ + {"provider": "openai-codex", "model": "gpt-5.5"}, + {"provider": "anthropic", "model": "claude-opus-4-7"}, + ], + logger=logging.getLogger("test"), + ) + assert runtime["provider"] == "openai-codex" + assert model == "gpt-5.5", "effective_model should advance to fallback's model" + assert landed_idx == 0 + assert remaining == [{"provider": "anthropic", "model": "claude-opus-4-7"}], ( + "AIAgent should only see fallbacks AFTER the one we just used" + ) + assert len(calls) == 2, "primary tried, then first fallback succeeded" + + +def test_primary_and_first_fallback_fail_second_fallback_succeeds() -> None: + calls: list[str] = [] + + def fake_resolve(*, requested, target_model, explicit_base_url): + calls.append(requested or "auto") + if requested in ("xai-oauth", "openai-codex"): + raise AuthError(f"{requested} auth dead", provider=requested) + return _runtime(provider=requested or "auto", model=target_model or "x") + + runtime, model, landed_idx, remaining = _resolve_runtime_with_fallback( + resolve_runtime_provider=fake_resolve, + effective_provider="xai-oauth", + effective_model="grok-4.3", + explicit_base_url=None, + fallback_chain=[ + {"provider": "openai-codex", "model": "gpt-5.5"}, + {"provider": "anthropic", "model": "claude-opus-4-7"}, + ], + logger=logging.getLogger("test"), + ) + assert runtime["provider"] == "anthropic" + assert model == "claude-opus-4-7" + assert calls == ["xai-oauth", "openai-codex", "anthropic"] + assert landed_idx == 1 + assert remaining == [], "landed on last entry → nothing left for AIAgent" + + +def test_all_providers_fail_last_auth_error_propagates() -> None: + last_seen = {"provider": None} + + def fake_resolve(*, requested, target_model, explicit_base_url): + last_seen["provider"] = requested + raise AuthError(f"{requested} auth dead", provider=requested or "auto", code="xx") + + with pytest.raises(AuthError) as excinfo: + _resolve_runtime_with_fallback( + resolve_runtime_provider=fake_resolve, + effective_provider="xai-oauth", + effective_model="grok-4.3", + explicit_base_url=None, + fallback_chain=[ + {"provider": "openai-codex", "model": "gpt-5.5"}, + {"provider": "anthropic", "model": "claude-opus-4-7"}, + ], + logger=logging.getLogger("test"), + ) + # The raised exception should be from the LAST attempted provider so + # downstream consumers see the final failure mode (not the original primary). + assert last_seen["provider"] == "anthropic" + assert "anthropic" in str(excinfo.value) + + +def test_empty_fallback_chain_propagates_primary_error_verbatim() -> None: + primary_err = AuthError("primary dead", provider="xai-oauth", code="dead") + + def fake_resolve(*, requested, target_model, explicit_base_url): + raise primary_err + + with pytest.raises(AuthError) as excinfo: + _resolve_runtime_with_fallback( + resolve_runtime_provider=fake_resolve, + effective_provider="xai-oauth", + effective_model="grok-4.3", + explicit_base_url=None, + fallback_chain=[], + logger=logging.getLogger("test"), + ) + assert excinfo.value is primary_err, "no fallback configured → original error must surface unchanged" + + +def test_fallback_without_explicit_model_keeps_primary_model() -> None: + """A fallback entry that omits 'model' should leave effective_model alone.""" + def fake_resolve(*, requested, target_model, explicit_base_url): + if requested == "xai-oauth": + raise AuthError("dead", provider="xai-oauth") + return _runtime(provider=requested or "auto", model=target_model or "x") + + runtime, model, _, _ = _resolve_runtime_with_fallback( + resolve_runtime_provider=fake_resolve, + effective_provider="xai-oauth", + effective_model="grok-4.3", + explicit_base_url=None, + fallback_chain=[{"provider": "openai-codex"}], # no model key + logger=logging.getLogger("test"), + ) + assert runtime["provider"] == "openai-codex" + assert model == "grok-4.3", "effective_model unchanged when fallback has no model" + + +# === Safety-bound tests (closes P0 review findings) === + + +def test_explicit_pin_does_not_fall_back_even_with_chain() -> None: + """User pinned --model/--provider on CLI → silent downgrade would surprise. + Re-raise primary AuthError verbatim with no fallback attempt. + """ + primary_err = AuthError( + "xAI OAuth missing access_token", provider="xai-oauth", + code="xai_auth_missing_access_token", + ) + calls: list[str] = [] + + def fake_resolve(*, requested, target_model, explicit_base_url): + calls.append(requested or "auto") + raise primary_err + + with pytest.raises(AuthError) as excinfo: + _resolve_runtime_with_fallback( + resolve_runtime_provider=fake_resolve, + effective_provider="xai-oauth", + effective_model="grok-4.3", + explicit_base_url=None, + fallback_chain=[ + {"provider": "openai-codex", "model": "gpt-5.5"}, + ], + logger=logging.getLogger("test"), + explicit_pin=True, + ) + assert excinfo.value is primary_err + assert calls == ["xai-oauth"], "explicit_pin must skip fallback chain entirely" + + +def test_rate_limit_auth_error_on_primary_does_not_burn_chain() -> None: + """Rate-limit on primary should re-raise immediately — falling through + would burn the quota of every other configured provider in milliseconds. + Existing rate-limit handling (cli.py exit code 75) gets the task requeued. + """ + # Construct a real rate-limit AuthError that is_rate_limited_auth_error + # will recognize — needs `code=CODEX_RATE_LIMITED_CODE` and + # `relogin_required=False` per auth.py:746-750. + from hermes_cli.auth import CODEX_RATE_LIMITED_CODE + primary_err = AuthError( + "rate limit exceeded", + provider="openai-codex", + code=CODEX_RATE_LIMITED_CODE, + relogin_required=False, + ) + calls: list[str] = [] + + def fake_resolve(*, requested, target_model, explicit_base_url): + calls.append(requested or "auto") + raise primary_err + + with pytest.raises(AuthError) as excinfo: + _resolve_runtime_with_fallback( + resolve_runtime_provider=fake_resolve, + effective_provider="openai-codex", + effective_model="gpt-5.5", + explicit_base_url=None, + fallback_chain=[ + {"provider": "xai-oauth", "model": "grok-4.3"}, + {"provider": "anthropic", "model": "claude-opus-4-7"}, + ], + logger=logging.getLogger("test"), + ) + assert excinfo.value is primary_err + assert calls == ["openai-codex"], ( + "rate-limit on primary must skip fallback chain entirely — let rate-limit retry handle it" + ) + + +def test_same_provider_in_chain_no_infinite_loop() -> None: + """If the fallback chain (perhaps misconfigured) contains the same + provider that failed primary, the loop must not retry it forever. Each + chain entry is attempted at most once; a second AuthError on the same + provider just advances to the next entry. + """ + call_count: dict[str, int] = {} + + def fake_resolve(*, requested, target_model, explicit_base_url): + call_count[requested or "auto"] = call_count.get(requested or "auto", 0) + 1 + if requested == "xai-oauth": + raise AuthError("xai dead", provider="xai-oauth") + return _runtime(provider=requested or "auto", model=target_model or "x") + + runtime, _, landed_idx, _ = _resolve_runtime_with_fallback( + resolve_runtime_provider=fake_resolve, + effective_provider="xai-oauth", + effective_model="grok-4.3", + explicit_base_url=None, + fallback_chain=[ + {"provider": "xai-oauth", "model": "grok-4.3"}, # same as primary + {"provider": "openai-codex", "model": "gpt-5.5"}, + ], + logger=logging.getLogger("test"), + ) + assert runtime["provider"] == "openai-codex" + assert landed_idx == 1, "should have skipped misconfigured same-provider entry" + assert call_count.get("xai-oauth") == 2, "primary + chain[0] both tried, then stopped" + assert call_count.get("openai-codex") == 1