mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
fix(auth): enforce complete fallback routes
Skip provider-only setup fallbacks, keep fallback selection explicit for resumed sessions, preserve configured primary identity for cron drift checks, and make the auth lost-update regression deterministic.
This commit is contained in:
parent
f68fd80f41
commit
679487b807
5 changed files with 123 additions and 44 deletions
|
|
@ -2935,6 +2935,7 @@ def run_job(
|
|||
|
||||
# Load config.yaml for model, reasoning, prefill, toolsets, provider routing
|
||||
_cfg = {}
|
||||
_model_cfg = {}
|
||||
try:
|
||||
import yaml
|
||||
_cfg_path = str(_get_hermes_home() / "config.yaml")
|
||||
|
|
@ -3037,7 +3038,16 @@ def run_job(
|
|||
_guard_job_credential_exfil(job)
|
||||
|
||||
primary_model_for_drift = model
|
||||
primary_provider_for_drift = (job.get("provider") or "").strip().lower() or None
|
||||
configured_provider_for_drift = (
|
||||
str(_model_cfg.get("provider") or "").strip().lower()
|
||||
if isinstance(_model_cfg, dict)
|
||||
else ""
|
||||
)
|
||||
primary_provider_for_drift = (
|
||||
str(job.get("provider") or "").strip().lower()
|
||||
or configured_provider_for_drift
|
||||
or None
|
||||
)
|
||||
try:
|
||||
# Do not inject HERMES_INFERENCE_PROVIDER here. resolve_runtime_provider()
|
||||
# already prefers persisted config over stale shell/env overrides when
|
||||
|
|
@ -3070,9 +3080,9 @@ def run_job(
|
|||
if not isinstance(entry, dict):
|
||||
continue
|
||||
fb_provider = str(entry.get("provider") or "").strip()
|
||||
if not fb_provider:
|
||||
fb_model = str(entry.get("model") or "").strip()
|
||||
if not fb_provider or not fb_model:
|
||||
continue
|
||||
fb_model = str(entry.get("model") or "").strip() or None
|
||||
try:
|
||||
fb_kwargs = {
|
||||
"requested": fb_provider,
|
||||
|
|
@ -3083,13 +3093,12 @@ def run_job(
|
|||
if entry.get("api_key"):
|
||||
fb_kwargs["explicit_api_key"] = entry["api_key"]
|
||||
runtime = resolve_runtime_provider(**fb_kwargs)
|
||||
if fb_model:
|
||||
model = fb_model
|
||||
model = fb_model
|
||||
logger.info(
|
||||
"Job '%s': fallback resolved to %s%s",
|
||||
"Job '%s': fallback resolved to %s model %s",
|
||||
job_id,
|
||||
runtime.get("provider"),
|
||||
f" model {fb_model}" if fb_model else "",
|
||||
fb_model,
|
||||
)
|
||||
break
|
||||
except Exception as fb_exc:
|
||||
|
|
|
|||
|
|
@ -213,6 +213,7 @@ def test_global_write_through_preserves_concurrent_root_update(
|
|||
)
|
||||
|
||||
helper_loaded = threading.Event()
|
||||
helper_has_target_lock = threading.Event()
|
||||
allow_helper_save = threading.Event()
|
||||
writer_started = threading.Event()
|
||||
writer_done = threading.Event()
|
||||
|
|
@ -221,11 +222,17 @@ def test_global_write_through_preserves_concurrent_root_update(
|
|||
def paused_helper_load(path=None):
|
||||
store = real_auth_load(path)
|
||||
if threading.current_thread().name == "profile-write-through":
|
||||
target_holder = A._auth_lock_holder_for(root_path)
|
||||
if getattr(target_holder, "depth", 0) > 0:
|
||||
helper_has_target_lock.set()
|
||||
helper_loaded.set()
|
||||
assert allow_helper_save.wait(timeout=5)
|
||||
return store
|
||||
|
||||
monkeypatch.setattr(A, "_load_auth_store", paused_helper_load)
|
||||
# The pre-fix implementation imported the loader directly; patch both
|
||||
# bindings so reverting the safe helper still exercises the stale ordering.
|
||||
monkeypatch.setattr(CP, "_load_auth_store", paused_helper_load)
|
||||
|
||||
def profile_write_through():
|
||||
CP._write_through_provider_state_to_global_root(
|
||||
|
|
@ -255,9 +262,12 @@ def test_global_write_through_preserves_concurrent_root_update(
|
|||
writer = threading.Thread(target=concurrent_codex_login, name="concurrent-login")
|
||||
writer.start()
|
||||
assert writer_started.wait(timeout=5)
|
||||
# Before the fix the writer completes while the stale helper is paused.
|
||||
# After the fix it blocks on the root lock until the helper saves and exits.
|
||||
writer_done.wait(timeout=0.2)
|
||||
# A fixed helper already owns the target lock, so the writer will merge
|
||||
# after release. A reverted unlocked helper must first let the competing
|
||||
# login finish; only then do we release its stale save. This makes the
|
||||
# losing pre-fix ordering deterministic rather than scheduler-dependent.
|
||||
if not helper_has_target_lock.is_set():
|
||||
assert writer_done.wait(timeout=5)
|
||||
allow_helper_save.set()
|
||||
helper.join(timeout=5)
|
||||
writer.join(timeout=5)
|
||||
|
|
|
|||
|
|
@ -2011,6 +2011,7 @@ class TestRunJobConfigEnvVarExpansion:
|
|||
" default: gpt-5.6-sol\n"
|
||||
" provider: openai-codex\n"
|
||||
"fallback_providers:\n"
|
||||
" - provider: anthropic\n"
|
||||
" - provider: openrouter\n"
|
||||
" model: z-ai/glm-5.2\n",
|
||||
encoding="utf-8",
|
||||
|
|
@ -2023,12 +2024,14 @@ class TestRunJobConfigEnvVarExpansion:
|
|||
"model_snapshot": "gpt-5.6-sol",
|
||||
}
|
||||
fake_db = MagicMock()
|
||||
requested = []
|
||||
|
||||
def resolve_runtime(**kwargs):
|
||||
requested.append(kwargs.get("requested"))
|
||||
if kwargs.get("requested") in (None, "openai-codex"):
|
||||
raise AuthError(
|
||||
"No Codex credentials stored", provider="openai-codex"
|
||||
)
|
||||
# Cron must retain the configured primary provider for drift
|
||||
# comparison even when older/custom AuthError sites omit it.
|
||||
raise AuthError("No Codex credentials stored")
|
||||
assert kwargs["requested"] == "openrouter"
|
||||
assert kwargs["target_model"] == "z-ai/glm-5.2"
|
||||
return {**self._RUNTIME, "provider": "openrouter"}
|
||||
|
|
@ -2049,6 +2052,7 @@ class TestRunJobConfigEnvVarExpansion:
|
|||
|
||||
assert success is True
|
||||
assert error is None
|
||||
assert requested == [None, "openrouter"]
|
||||
kwargs = mock_agent_cls.call_args.kwargs
|
||||
assert kwargs["provider"] == "openrouter"
|
||||
assert kwargs["model"] == "z-ai/glm-5.2"
|
||||
|
|
|
|||
|
|
@ -9528,11 +9528,12 @@ class TestResolveRuntimeWithFallback:
|
|||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
lambda **kw: expected,
|
||||
)
|
||||
runtime, fallback_model = server._resolve_runtime_with_fallback(
|
||||
resolution = server._resolve_runtime_with_fallback(
|
||||
{"requested": "openai"}
|
||||
)
|
||||
assert runtime == expected
|
||||
assert fallback_model is None
|
||||
assert resolution.runtime == expected
|
||||
assert resolution.selected_model is None
|
||||
assert resolution.used_fallback is False
|
||||
|
||||
def test_auth_error_tries_fallback_chain(self, monkeypatch):
|
||||
"""On AuthError from primary, walk fallback_providers chain."""
|
||||
|
|
@ -9554,11 +9555,47 @@ class TestResolveRuntimeWithFallback:
|
|||
"_load_fallback_model",
|
||||
lambda: [{"provider": "deepseek", "model": "deepseek-v4-pro"}],
|
||||
)
|
||||
runtime, fallback_model = server._resolve_runtime_with_fallback(
|
||||
resolution = server._resolve_runtime_with_fallback(
|
||||
{"requested": "openai-codex"},
|
||||
)
|
||||
assert runtime == fallback_runtime
|
||||
assert fallback_model == "deepseek-v4-pro"
|
||||
assert resolution.runtime == fallback_runtime
|
||||
assert resolution.selected_model == "deepseek-v4-pro"
|
||||
assert resolution.used_fallback is True
|
||||
|
||||
def test_auth_error_skips_provider_only_fallback(self, monkeypatch):
|
||||
"""Auth fallback requires one complete provider/model pair."""
|
||||
from hermes_cli.auth import AuthError
|
||||
|
||||
requested = []
|
||||
fallback_runtime = {"provider": "openrouter", "api_key": "fb-tok"}
|
||||
|
||||
def fake_resolve(**kwargs):
|
||||
requested.append(kwargs.get("requested"))
|
||||
if kwargs.get("requested") == "openai-codex":
|
||||
raise AuthError("No Codex credentials stored")
|
||||
return fallback_runtime
|
||||
|
||||
monkeypatch.setattr(
|
||||
"hermes_cli.runtime_provider.resolve_runtime_provider",
|
||||
fake_resolve,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
server,
|
||||
"_load_fallback_model",
|
||||
lambda: [
|
||||
{"provider": "anthropic"},
|
||||
{"provider": "openrouter", "model": "z-ai/glm-5.2"},
|
||||
],
|
||||
)
|
||||
|
||||
resolution = server._resolve_runtime_with_fallback(
|
||||
{"requested": "openai-codex"}
|
||||
)
|
||||
|
||||
assert requested == ["openai-codex", "openrouter"]
|
||||
assert resolution.runtime == fallback_runtime
|
||||
assert resolution.selected_model == "z-ai/glm-5.2"
|
||||
assert resolution.used_fallback is True
|
||||
|
||||
def test_auth_error_all_fallbacks_fail_raises(self, monkeypatch):
|
||||
"""When all fallbacks also fail, re-raise the original AuthError."""
|
||||
|
|
@ -9606,11 +9643,12 @@ class TestResolveRuntimeWithFallback:
|
|||
{"provider": "anthropic", "model": "claude-sonnet-4-6"},
|
||||
],
|
||||
)
|
||||
runtime, fallback_model = server._resolve_runtime_with_fallback(
|
||||
resolution = server._resolve_runtime_with_fallback(
|
||||
{"requested": "openai-codex"},
|
||||
)
|
||||
assert runtime == fallback_runtime
|
||||
assert fallback_model == "claude-sonnet-4-6"
|
||||
assert resolution.runtime == fallback_runtime
|
||||
assert resolution.selected_model == "claude-sonnet-4-6"
|
||||
assert resolution.used_fallback is True
|
||||
|
||||
def test_make_agent_uses_fallback_on_auth_error(self, monkeypatch):
|
||||
"""Integration: _make_agent falls back to configured fallback
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ import time
|
|||
import uuid
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
from typing import Any, Optional
|
||||
from typing import Any, NamedTuple, Optional
|
||||
|
||||
from hermes_constants import (
|
||||
get_hermes_home,
|
||||
|
|
@ -4471,34 +4471,46 @@ def _schedule_mcp_late_refresh(sid: str, agent) -> None:
|
|||
).start()
|
||||
|
||||
|
||||
class _RuntimeFallbackResolution(NamedTuple):
|
||||
runtime: dict
|
||||
selected_model: str | None
|
||||
used_fallback: bool
|
||||
|
||||
|
||||
def _resolve_runtime_with_fallback(
|
||||
resolve_kwargs: dict | None = None,
|
||||
) -> tuple[dict, str | None]:
|
||||
"""Resolve a runtime and the fallback model selected after auth failure.
|
||||
) -> _RuntimeFallbackResolution:
|
||||
"""Resolve the primary runtime or one complete provider/model fallback.
|
||||
|
||||
A fallback entry is one provider/model pair. Returning the model alongside
|
||||
the runtime prevents callers from accidentally pairing the fallback
|
||||
provider with the unavailable primary model.
|
||||
Setup-time auth fallback only accepts entries with both fields. Provider-
|
||||
only entries are skipped so the unavailable primary model can never leak
|
||||
into a different runtime. ``used_fallback`` remains explicit rather than
|
||||
overloading a nullable model as control flow.
|
||||
"""
|
||||
from hermes_cli.auth import AuthError
|
||||
from hermes_cli.runtime_provider import resolve_runtime_provider
|
||||
|
||||
kwargs = resolve_kwargs or {}
|
||||
try:
|
||||
return resolve_runtime_provider(**kwargs), None
|
||||
return _RuntimeFallbackResolution(
|
||||
resolve_runtime_provider(**kwargs),
|
||||
None,
|
||||
False,
|
||||
)
|
||||
except AuthError as primary_exc:
|
||||
fb_chain = _load_fallback_model() or []
|
||||
for entry in fb_chain:
|
||||
if not isinstance(entry, dict):
|
||||
continue
|
||||
fb_provider = (entry.get("provider") or "").strip()
|
||||
if not fb_provider:
|
||||
fb_provider = str(entry.get("provider") or "").strip()
|
||||
fb_model = str(entry.get("model") or "").strip()
|
||||
if not fb_provider or not fb_model:
|
||||
continue
|
||||
fb_model = (entry.get("model") or "").strip() or None
|
||||
try:
|
||||
fb_kwargs: dict = {"requested": fb_provider}
|
||||
if fb_model:
|
||||
fb_kwargs["target_model"] = fb_model
|
||||
fb_kwargs: dict = {
|
||||
"requested": fb_provider,
|
||||
"target_model": fb_model,
|
||||
}
|
||||
if entry.get("base_url"):
|
||||
fb_kwargs["explicit_base_url"] = entry["base_url"]
|
||||
if entry.get("api_key"):
|
||||
|
|
@ -4507,12 +4519,12 @@ def _resolve_runtime_with_fallback(
|
|||
import logging
|
||||
|
||||
logging.getLogger(__name__).warning(
|
||||
"Primary auth failed (%s), falling back to %s%s",
|
||||
"Primary auth failed (%s), falling back to %s model %s",
|
||||
primary_exc,
|
||||
fb_provider,
|
||||
f" model {fb_model}" if fb_model else "",
|
||||
fb_model,
|
||||
)
|
||||
return runtime, fb_model
|
||||
return _RuntimeFallbackResolution(runtime, fb_model, True)
|
||||
except Exception:
|
||||
continue
|
||||
raise
|
||||
|
|
@ -4613,9 +4625,12 @@ def _make_agent(
|
|||
resolve_kwargs["explicit_base_url"] = override_base_url
|
||||
resolve_kwargs["requested"] = requested_provider
|
||||
resolve_kwargs["target_model"] = model or None
|
||||
runtime, auth_fallback_model = _resolve_runtime_with_fallback(resolve_kwargs)
|
||||
if auth_fallback_model:
|
||||
model = auth_fallback_model
|
||||
resolution = _resolve_runtime_with_fallback(resolve_kwargs)
|
||||
runtime = resolution.runtime
|
||||
if resolution.used_fallback:
|
||||
if not resolution.selected_model:
|
||||
raise RuntimeError("Auth fallback resolved without a model")
|
||||
model = resolution.selected_model
|
||||
else:
|
||||
# The switch already resolved concrete credentials/endpoint; honor
|
||||
# persisted overrides only while using that original runtime. They
|
||||
|
|
@ -4632,12 +4647,15 @@ def _make_agent(
|
|||
model = model_override
|
||||
if provider_override:
|
||||
requested_provider = provider_override
|
||||
runtime, auth_fallback_model = _resolve_runtime_with_fallback({
|
||||
resolution = _resolve_runtime_with_fallback({
|
||||
"requested": requested_provider,
|
||||
"target_model": model or None,
|
||||
})
|
||||
if auth_fallback_model:
|
||||
model = auth_fallback_model
|
||||
runtime = resolution.runtime
|
||||
if resolution.used_fallback:
|
||||
if not resolution.selected_model:
|
||||
raise RuntimeError("Auth fallback resolved without a model")
|
||||
model = resolution.selected_model
|
||||
_pr = _load_provider_routing()
|
||||
return AIAgent(
|
||||
model=model,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue