diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c77c0d9..0ef4a3ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ ## [Unreleased] +### Fixed + +- Browser chat fallback now merges `fallback_providers` with the legacy `fallback_model` in Hermes CLI/gateway order, so WebUI can continue to a later non-Codex fallback when the primary Codex provider path fails. + ## [v0.51.139] — 2026-05-25 — Release DK (stage-batch21 — 5-PR tier-2 batch) ### Added diff --git a/api/streaming.py b/api/streaming.py index 9abaf85e..2dc2819f 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -4207,28 +4207,50 @@ def _run_agent_streaming( except Exception as _ts_err: print(f"[webui] WARNING: failed to read per-session toolsets for {session_id}: {_ts_err}", flush=True) - # Fallback model from profile config (e.g. for rate-limit recovery) - _fallback = _cfg.get('fallback_model') or _cfg.get('fallback_providers') or None + # Fallback model chain from profile config (e.g. for rate-limit or + # provider recovery). Match Hermes CLI/gateway semantics: + # fallback_providers entries are tried first, then legacy + # fallback_model entries are appended unless they duplicate an + # earlier provider/model/base_url route. + def _fallback_entries(_raw): + if isinstance(_raw, dict): + _items = [_raw] + elif isinstance(_raw, list): + _items = _raw + else: + return [] + _entries = [] + for _entry in _items: + if not isinstance(_entry, dict): + continue + _provider = str(_entry.get('provider') or '').strip() + _model = str(_entry.get('model') or '').strip() + if not _provider or not _model: + continue + _entries.append({ + 'model': _model, + 'provider': _provider, + 'base_url': _entry.get('base_url'), + 'api_key': _entry.get('api_key'), + 'key_env': _entry.get('key_env'), + }) + return _entries + + _fallback_chain = [] + _fallback_seen = set() _fallback_resolved = None - if _fallback: - # Normalize: support both single dict (legacy) and list (chained fallback). - # Use the first valid entry as the fallback passed to AIAgent. - _fb_entry = None - if isinstance(_fallback, list): - for _entry in _fallback: - if isinstance(_entry, dict) and _entry.get('model'): - _fb_entry = _entry - break - elif isinstance(_fallback, dict) and _fallback.get('model'): - _fb_entry = _fallback - if _fb_entry: - _fallback_resolved = { - 'model': _fb_entry.get('model', ''), - 'provider': _fb_entry.get('provider', ''), - 'base_url': _fb_entry.get('base_url'), - 'api_key': _fb_entry.get('api_key'), - 'key_env': _fb_entry.get('key_env'), - } + for _fallback_key in ('fallback_providers', 'fallback_model'): + for _fb_entry in _fallback_entries(_cfg.get(_fallback_key)): + _identity = ( + str(_fb_entry.get('provider') or '').strip().lower(), + str(_fb_entry.get('model') or '').strip().lower(), + str(_fb_entry.get('base_url') or '').strip().rstrip('/').lower(), + ) + if _identity in _fallback_seen: + continue + _fallback_seen.add(_identity) + _fallback_chain.append(_fb_entry) + _fallback_resolved = _fallback_chain or None # Build kwargs defensively — guard newer params so the WebUI # degrades gracefully when run against an older hermes-agent build. diff --git a/tests/test_pr1339_fallback_providers_list.py b/tests/test_pr1339_fallback_providers_list.py index c0a804f5..bc8adb64 100644 --- a/tests/test_pr1339_fallback_providers_list.py +++ b/tests/test_pr1339_fallback_providers_list.py @@ -1,14 +1,13 @@ -"""Test for PR #1339 — streaming.py must support both single-dict `fallback_model` -and list-form `fallback_providers` config without crashing on `.get()`. +"""Tests for WebUI fallback config handling in streaming.py. Before the fix, when config had `fallback_providers: [{provider, model, ...}, ...]`, streaming.py read it as if it were a dict and called `.get('model', '')` on a list, which would raise `AttributeError: 'list' object has no attribute 'get'`. -The fix makes streaming.py handle both legacy dict form and new list form, picking -the first entry from the list when given a list. +WebUI must now also mirror Hermes CLI/gateway fallback-chain semantics: +`fallback_providers` entries are tried first, then legacy `fallback_model` +entries are appended when they do not duplicate an earlier route. """ -import re from pathlib import Path STREAMING_PY = Path(__file__).resolve().parent.parent / "api" / "streaming.py" @@ -18,7 +17,7 @@ def _extract_fallback_block(): """Return the source range that handles fallback_model/fallback_providers.""" src = STREAMING_PY.read_text(encoding="utf-8") # Locate the resolved-fallback region - idx = src.find("# Fallback model from profile config") + idx = src.find("# Fallback model chain from profile config") assert idx != -1, "Fallback block marker not found in streaming.py" end = src.find("# Build kwargs defensively", idx) assert end != -1, "End-of-block marker not found" @@ -36,31 +35,30 @@ def test_fallback_handles_both_dict_and_list_config(): ) -def test_fallback_list_iteration_picks_first_valid_entry(): - """When given a list, code must pick the first valid dict entry, not call .get on the list.""" +def test_fallback_list_iteration_builds_chain_before_legacy_fallback(): + """List-form fallback_providers must stay ahead of legacy fallback_model.""" block = _extract_fallback_block() # Must isinstance-check before calling .get - assert "isinstance(_fallback, list)" in block, ( + assert "isinstance(_raw, list)" in block, ( "Must detect list-form fallback_providers explicitly to avoid AttributeError" ) - assert "isinstance(_fallback, dict)" in block or "isinstance(_fallback,dict)" in block, ( + assert "isinstance(_raw, dict)" in block or "isinstance(_raw,dict)" in block, ( "Must keep legacy single-dict path explicitly" ) + assert "for _fallback_key in ('fallback_providers', 'fallback_model')" in block, ( + "WebUI must merge fallback_providers first, then append legacy fallback_model" + ) + assert "_fallback_resolved = _fallback_chain or None" in block, ( + "WebUI must pass the full fallback chain to AIAgent, not only the first entry" + ) - # No bare _fallback.get() — every .get() on _fallback must be guarded by an isinstance(_fallback, dict) check. - # We verify this structurally: every line containing `_fallback.get(` must be inside or preceded by an isinstance(_fallback, dict) gate. - lines = block.split("\n") - in_dict_block = False - for i, line in enumerate(lines): - if "isinstance(_fallback, dict)" in line: - in_dict_block = True - if "_fallback.get(" in line and not in_dict_block: - # Look back up to 3 lines for the isinstance gate on the same elif/if - window = "\n".join(lines[max(0, i - 3): i + 1]) - assert "isinstance(_fallback, dict)" in window, ( - f"Line {i} calls _fallback.get() without a nearby isinstance(_fallback, dict) gate:\n{line}" - ) + assert "_cfg.get(_fallback_key)" in block, ( + "Fallback entries should be read through the ordered key loop" + ) + assert "_fallback.get(" not in block, ( + "Do not call .get() on a value that may be a list" + ) def test_fallback_resolved_initialized_to_none(): @@ -75,16 +73,24 @@ def test_fallback_resolved_initialized_to_none(): def test_fallback_resolved_preserves_credential_hints(): """Fallback entries must keep credential hints for AIAgent fallback activation.""" block = _extract_fallback_block() - resolved_start = block.find("_fallback_resolved = {") - assert resolved_start != -1, "_fallback_resolved dict not found" + resolved_start = block.find("_entries.append({") + assert resolved_start != -1, "fallback entry normalization dict not found" resolved_end = block.find("}", resolved_start) resolved_dict = block[resolved_start:resolved_end] - assert "'api_key': _fb_entry.get('api_key')" in resolved_dict, ( + assert "'api_key': _entry.get('api_key')" in resolved_dict, ( "WebUI must preserve fallback_model/fallback_providers api_key so " "AIAgent._try_activate_fallback can authenticate the fallback." ) - assert "'key_env': _fb_entry.get('key_env')" in resolved_dict, ( + assert "'key_env': _entry.get('key_env')" in resolved_dict, ( "WebUI must preserve fallback_model/fallback_providers key_env so " "AIAgent._try_activate_fallback can resolve env-backed fallback keys." ) + + +def test_fallback_chain_deduplicates_routes(): + """Duplicate provider/model/base_url entries must not be passed twice.""" + block = _extract_fallback_block() + assert "_fallback_seen = set()" in block + assert "_identity in _fallback_seen" in block + assert "_fallback_seen.add(_identity)" in block