Fix WebUI fallback provider chain merging

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

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.

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