From 441042e9f84697a1e7fd4147aa47a5186c2ef3a9 Mon Sep 17 00:00:00 2001 From: Cloudymap1e <83044824+Cloudymap1e@users.noreply.github.com> Date: Wed, 5 Aug 2026 01:33:46 +0800 Subject: [PATCH] fix(review): propagate configured fallback chains --- agent/background_review.py | 12 ++++ agent/curator.py | 4 ++ hermes_cli/fallback_config.py | 34 ++++++++++ tests/agent/test_curator.py | 42 ++++++++++++ tests/hermes_cli/test_fallback_config.py | 43 +++++++++++- .../test_background_review_cache_parity.py | 2 + .../test_background_review_cost_controls.py | 66 +++++++++++++++++++ 7 files changed, 202 insertions(+), 1 deletion(-) diff --git a/agent/background_review.py b/agent/background_review.py index 01820cfffdf22..c2648e162f10b 100644 --- a/agent/background_review.py +++ b/agent/background_review.py @@ -68,11 +68,20 @@ def _resolve_review_runtime(agent: Any) -> Dict[str, Any]: "max_tokens": getattr(agent, "max_tokens", None), "command": getattr(agent, "acp_command", None), "args": list(getattr(agent, "acp_args", []) or []), + "fallback_model": copy.deepcopy(getattr(agent, "_fallback_chain", None)) or None, "routed": False, } try: from hermes_cli.config import load_config_readonly + from hermes_cli.fallback_config import get_auxiliary_fallback_chain cfg = load_config_readonly() + fallback_model = get_auxiliary_fallback_chain(cfg, "background_review") or None + # An explicitly configured task/global chain takes precedence. When + # config supplies no chain, retain the live parent agent's already- + # resolved fallback chain instead of silently stripping failover from + # the background-review fork (#78371). + if fallback_model: + parent["fallback_model"] = fallback_model except Exception: return parent aux = cfg.get("auxiliary", {}) if isinstance(cfg.get("auxiliary"), dict) else {} @@ -104,6 +113,7 @@ def _resolve_review_runtime(agent: Any) -> Dict[str, Any]: "max_tokens": rp.get("max_output_tokens"), "command": rp.get("command"), "args": list(rp.get("args") or []), + "fallback_model": parent["fallback_model"], "routed": True, } except Exception as e: @@ -733,6 +743,8 @@ def _bg_review_auto_deny(command, description, **kwargs): if isinstance(_rt.get("command"), str) and _rt["command"]: _fork_kwargs["acp_command"] = _rt["command"] _fork_kwargs["acp_args"] = _rt.get("args") or [] + if _rt.get("fallback_model"): + _fork_kwargs["fallback_model"] = _rt["fallback_model"] # Match parent's reasoning config so the fork's ``thinking`` / # ``output_config`` are byte-identical in the request body — # Anthropic's cache key is namespaced by ``thinking`` presence. diff --git a/agent/curator.py b/agent/curator.py index ab0adddae118d..f855aec3e93cd 100644 --- a/agent/curator.py +++ b/agent/curator.py @@ -1874,10 +1874,13 @@ def _run_llm_review(prompt: str) -> Dict[str, Any]: _acp_command = None _acp_args = None _model_name = "" + _fallback_model: Any = None try: from hermes_cli.config import load_config_readonly + from hermes_cli.fallback_config import get_auxiliary_fallback_chain from hermes_cli.runtime_provider import resolve_runtime_provider _cfg = load_config_readonly() + _fallback_model = get_auxiliary_fallback_chain(_cfg, "curator") or None _binding = _resolve_review_runtime(_cfg) _provider, _model_name = _binding.provider, _binding.model _rp = resolve_runtime_provider( @@ -1922,6 +1925,7 @@ def _run_llm_review(prompt: str) -> Dict[str, Any]: api_mode=_api_mode, credential_pool=_credential_pool, request_overrides=_request_overrides, + fallback_model=_fallback_model, **_agent_kwargs, enabled_toolsets=["skills", "terminal"], # Umbrella-building over a large skill collection is worth a diff --git a/hermes_cli/fallback_config.py b/hermes_cli/fallback_config.py index 2cce7a2d373e4..ff93b57b4c56a 100644 --- a/hermes_cli/fallback_config.py +++ b/hermes_cli/fallback_config.py @@ -99,3 +99,37 @@ def get_fallback_chain(config: dict[str, Any] | None) -> list[dict[str, Any]]: chain.append(entry) return chain + + +def get_auxiliary_fallback_chain( + config: dict[str, Any] | None, + task: str, +) -> list[dict[str, Any]]: + """Return a task's fallback chain followed by the global fallback chain. + + Auxiliary task entries take precedence. Global fallbacks remain the final + safety net, with duplicate provider/model/base-url routes removed while + preserving the first occurrence and returning fresh dict copies. + """ + + config = config or {} + auxiliary = config.get("auxiliary") + if not isinstance(auxiliary, dict): + auxiliary = {} + task_config = auxiliary.get(task) + if not isinstance(task_config, dict): + task_config = {} + + chain: list[dict[str, Any]] = [] + seen: set[tuple[str, str, str]] = set() + entries = [ + *_iter_fallback_entries(task_config.get("fallback_chain")), + *get_fallback_chain(config), + ] + for entry in entries: + identity = _entry_identity(entry) + if identity in seen: + continue + seen.add(identity) + chain.append(dict(entry)) + return chain diff --git a/tests/agent/test_curator.py b/tests/agent/test_curator.py index 1650c976197ed..0832065e020b4 100644 --- a/tests/agent/test_curator.py +++ b/tests/agent/test_curator.py @@ -654,6 +654,48 @@ def close(self): assert captured["kwargs"]["request_overrides"] == fake_overrides +def test_review_fork_forwards_task_then_global_fallback_chain(curator_env, monkeypatch): + curator = curator_env["curator"] + import importlib + importlib.reload(curator) + + task_fallback = {"provider": "custom", "model": "curator-backup"} + global_fallback = {"provider": "openrouter", "model": "openai/gpt-5.5"} + cfg = { + "model": {"provider": "custom", "default": "curator-primary"}, + "auxiliary": {"curator": {"fallback_chain": [task_fallback]}}, + "fallback_providers": [global_fallback], + } + captured = {} + + class _StubAgent: + def __init__(self, **kwargs): + captured.update(kwargs) + self._session_messages = [] + + def run_conversation(self, **_kwargs): + return {"final_response": "ok"} + + def close(self): + pass + + monkeypatch.setattr("hermes_cli.config.load_config_readonly", lambda: cfg) + monkeypatch.setattr( + "hermes_cli.runtime_provider.resolve_runtime_provider", + lambda **_kwargs: { + "provider": "custom", + "model": "curator-primary", + "api_mode": "chat_completions", + }, + ) + monkeypatch.setattr("run_agent.AIAgent", _StubAgent) + + result = curator._run_llm_review("review") + + assert result["error"] is None + assert captured.get("fallback_model") == [task_fallback, global_fallback] + + def test_review_fork_uses_runtime_model_and_output_cap(curator_env, monkeypatch): curator = curator_env["curator"] import importlib diff --git a/tests/hermes_cli/test_fallback_config.py b/tests/hermes_cli/test_fallback_config.py index 3d6da5604b2fa..488231f25f9c6 100644 --- a/tests/hermes_cli/test_fallback_config.py +++ b/tests/hermes_cli/test_fallback_config.py @@ -1,7 +1,48 @@ """Tests for hermes_cli/fallback_config.py — fallback entry API-key resolution.""" from agent.secret_scope import reset_secret_scope, set_secret_scope -from hermes_cli.fallback_config import resolve_entry_api_key +from hermes_cli.fallback_config import ( + get_auxiliary_fallback_chain, + resolve_entry_api_key, +) + + +def test_auxiliary_fallback_chain_precedes_and_deduplicates_global_chain(): + task_entry = { + "provider": "custom", + "model": "review-backup", + "base_url": "https://review.example/v1/", + } + global_entry = { + "provider": "openrouter", + "model": "openai/gpt-5.5", + } + config = { + "auxiliary": { + "curator": { + "fallback_chain": [task_entry, {**task_entry, "base_url": "https://review.example/v1"}], + }, + }, + "fallback_providers": [global_entry], + } + + assert get_auxiliary_fallback_chain(config, "curator") == [ + {**task_entry, "base_url": "https://review.example/v1"}, + global_entry, + ] + + +def test_auxiliary_fallback_chain_ignores_empty_and_malformed_entries(): + config = { + "auxiliary": { + "curator": { + "fallback_chain": [None, "bad", {}, {"provider": "custom"}], + }, + }, + "fallback_providers": [42, {"model": "missing-provider"}], + } + + assert get_auxiliary_fallback_chain(config, "curator") == [] class TestResolveEntryApiKey: diff --git a/tests/run_agent/test_background_review_cache_parity.py b/tests/run_agent/test_background_review_cache_parity.py index da5f1dcb61404..bbd288cc16e78 100644 --- a/tests/run_agent/test_background_review_cache_parity.py +++ b/tests/run_agent/test_background_review_cache_parity.py @@ -306,6 +306,7 @@ def test_routed_review_fork_does_not_inherit_reasoning_config(): "max_tokens": None, "command": None, "args": [], + "fallback_model": [{"provider": "openrouter", "model": "review-backup"}], "routed": True, } @@ -320,6 +321,7 @@ def test_routed_review_fork_does_not_inherit_reasoning_config(): ) init_kwargs = captured.get("init_kwargs", {}) + assert init_kwargs["fallback_model"] == routed_runtime["fallback_model"] assert "reasoning_config" not in init_kwargs, ( f"Routed review fork was passed the parent's reasoning_config " f"({init_kwargs.get('reasoning_config')!r}). On the routed path the " diff --git a/tests/run_agent/test_background_review_cost_controls.py b/tests/run_agent/test_background_review_cost_controls.py index e8a8df20cbc6a..bae05f47dce37 100644 --- a/tests/run_agent/test_background_review_cost_controls.py +++ b/tests/run_agent/test_background_review_cost_controls.py @@ -32,6 +32,7 @@ def __init__(self, provider="openai-codex", model="gpt-5.5"): self._credential_pool: Any = None self.request_overrides = {} self.max_tokens: int | None = None + self._fallback_chain: list[dict[str, str]] = [] def _current_main_runtime(self): return { @@ -88,6 +89,71 @@ def test_unrouted_runtime_keeps_parent_pool_and_overrides(): assert rt["max_tokens"] == 4096 +def test_review_runtime_resolves_task_then_global_fallback_chain(): + agent = _FakeAgent() + task_fallback = {"provider": "custom", "model": "review-backup"} + global_fallback = {"provider": "openrouter", "model": "openai/gpt-5.5"} + cfg = { + "auxiliary": { + "background_review": { + "provider": "auto", + "model": "", + "fallback_chain": [task_fallback], + }, + }, + "fallback_providers": [global_fallback], + } + + with patch("hermes_cli.config.load_config_readonly", return_value=cfg): + rt = br._resolve_review_runtime(agent) + + assert rt["fallback_model"] == [task_fallback, global_fallback] + + +def test_review_runtime_keeps_live_parent_chain_when_config_has_no_chain(): + agent = _FakeAgent() + parent_fallback = {"provider": "custom", "model": "parent-backup"} + agent._fallback_chain = [parent_fallback] + + with patch("hermes_cli.config.load_config_readonly", return_value={}): + rt = br._resolve_review_runtime(agent) + + assert rt["fallback_model"] == [parent_fallback] + + +def test_routed_review_runtime_keeps_live_parent_chain_when_config_has_no_chain(): + agent = _FakeAgent() + parent_fallback = {"provider": "custom", "model": "parent-backup"} + agent._fallback_chain = [parent_fallback] + cfg = { + "auxiliary": { + "background_review": { + "provider": "openrouter", + "model": "google/gemini-3-flash-preview", + }, + }, + } + fake_rp = { + "provider": "openrouter", + "model": "google/gemini-3-flash-preview", + "api_key": "or-key", + "base_url": "https://openrouter.ai/api/v1", + "api_mode": "chat_completions", + } + + with ( + patch("hermes_cli.config.load_config_readonly", return_value=cfg), + patch( + "hermes_cli.runtime_provider.resolve_runtime_provider", + return_value=fake_rp, + ), + ): + rt = br._resolve_review_runtime(agent) + + assert rt["routed"] is True + assert rt["fallback_model"] == [parent_fallback] + + def test_routing_same_model_as_parent_is_not_routed(): agent = _FakeAgent(provider="openrouter", model="anthropic/claude-opus-4.8") cfg = {"auxiliary": {"background_review": {