fix(fallback): resolve api_key_env in fallback chain entries (carve-out of #22665)

wesleysimplicio · teknium1 · commit 6ddc48b058a3 · 2026-05-09T17:53:56.000-07:00
Fallback chain entries with 'api_key_env: ENV_VAR_NAME' weren't being resolved by either the init-time fallback path (line ~1660) or the runtime _try_activate_fallback path (line ~8045). Only literal 'api_key' was honored; the snake_case 'api_key_env' alias documented elsewhere in the config was silently dropped, so a 'provider: custom' fallback with base_url + api_key_env worked as primary but failed as fallback with 'no endpoint credentials found' / 401. Adds 'or fb.get("api_key_env")' to the existing 'key_env' lookup in both call sites, with empty-string-to-None coercion so unset env vars don't poison the resolver. Salvage of #22665's fallback portion. The original PR also bundled gateway-degrade-on-no-adapters changes (those land via the carve-out in #22853 which is the same code) and run_agent.py memory-nudge counter hydration (issue #22357 territory, not mentioned in the title). Drops both bundled pieces; keeps just the api_key_env fix. Closes #5392.
diff --git a/run_agent.py b/run_agent.py
@@ -1665,10 +1665,15 @@ def __init__(
                             _fb_entries = [fallback_model]
                         _fb_resolved = False
                         for _fb in _fb_entries:
+                            _fb_explicit_key = (_fb.get("api_key") or "").strip() or None
+                            if not _fb_explicit_key:
+                                _fb_key_env = (_fb.get("key_env") or _fb.get("api_key_env") or "").strip()
+                                if _fb_key_env:
+                                    _fb_explicit_key = os.getenv(_fb_key_env, "").strip() or None
                             _fb_client, _fb_model = resolve_provider_client(
                                 _fb["provider"], model=_fb["model"], raw_codex=True,
                                 explicit_base_url=_fb.get("base_url"),
-                                explicit_api_key=_fb.get("api_key"),
+                                explicit_api_key=_fb_explicit_key,
                             )
                             if _fb_client is not None:
                                 self.provider = _fb["provider"]
@@ -8116,7 +8121,9 @@ def _try_activate_fallback(self, reason: "FailoverReason | None" = None) -> bool
             fb_base_url_hint = (fb.get("base_url") or "").strip() or None
             fb_api_key_hint = (fb.get("api_key") or "").strip() or None
             if not fb_api_key_hint:
-                fb_key_env = (fb.get("key_env") or "").strip()
+                # key_env and api_key_env are both documented aliases (see
+                # _normalize_custom_provider_entry in hermes_cli/config.py).
+                fb_key_env = (fb.get("key_env") or fb.get("api_key_env") or "").strip()
                 if fb_key_env:
                     fb_api_key_hint = os.getenv(fb_key_env, "").strip() or None
             # For Ollama Cloud endpoints, pull OLLAMA_API_KEY from env
diff --git a/tests/run_agent/test_fallback_model.py b/tests/run_agent/test_fallback_model.py
@@ -405,3 +405,107 @@ def test_provider_resolves(self, provider, env_var, base_url_fragment):
             assert agent.client is mock_client
             assert agent.model == "test-model"
             assert agent.provider == provider
+
+
+# =============================================================================
+# api_key_env / key_env resolution in fallback entries (#5392)
+# =============================================================================
+
+class TestFallbackKeyEnvResolution:
+    """Verify that api_key_env and key_env are both resolved from the
+    environment and forwarded to resolve_provider_client as explicit_api_key.
+
+    Before the fix, _try_activate_fallback only checked ``key_env`` and ignored
+    the ``api_key_env`` alias documented in the custom_providers config schema.
+    The init-time fallback path never resolved either field.
+    """
+
+    def test_api_key_env_resolved_at_runtime_fallback(self, monkeypatch):
+        """api_key_env in fallback entry must be read from env and passed
+        as explicit_api_key to resolve_provider_client (#5392)."""
+        monkeypatch.setenv("MY_GOOGLE_KEY", "google-secret-from-env")
+
+        agent = _make_agent(
+            fallback_model={
+                "provider": "custom",
+                "model": "gemini-flash",
+                "base_url": "https://generativelanguage.googleapis.com/v1beta/openai",
+                "api_key_env": "MY_GOOGLE_KEY",
+            },
+        )
+        captured = {}
+
+        def _fake_resolve(provider, model=None, raw_codex=False,
+                          explicit_base_url=None, explicit_api_key=None, **kw):
+            captured["explicit_api_key"] = explicit_api_key
+            captured["explicit_base_url"] = explicit_base_url
+            mock = MagicMock()
+            mock.api_key = explicit_api_key or "no-key"
+            mock.base_url = explicit_base_url or "https://example.com/v1"
+            return mock, model
+
+        with patch("agent.auxiliary_client.resolve_provider_client", side_effect=_fake_resolve):
+            result = agent._try_activate_fallback()
+
+        assert result is True
+        assert captured["explicit_api_key"] == "google-secret-from-env", (
+            "api_key_env value was not resolved and forwarded as explicit_api_key"
+        )
+        assert captured["explicit_base_url"] == "https://generativelanguage.googleapis.com/v1beta/openai"
+
+    def test_key_env_still_works_at_runtime_fallback(self, monkeypatch):
+        """key_env (canonical form) must still be resolved correctly."""
+        monkeypatch.setenv("MY_PROVIDER_KEY", "secret-via-key-env")
+
+        agent = _make_agent(
+            fallback_model={
+                "provider": "custom",
+                "model": "my-model",
+                "base_url": "https://api.example.com/v1",
+                "key_env": "MY_PROVIDER_KEY",
+            },
+        )
+        captured = {}
+
+        def _fake_resolve(provider, model=None, raw_codex=False,
+                          explicit_base_url=None, explicit_api_key=None, **kw):
+            captured["explicit_api_key"] = explicit_api_key
+            mock = MagicMock()
+            mock.api_key = explicit_api_key or "no-key"
+            mock.base_url = explicit_base_url or "https://api.example.com/v1"
+            return mock, model
+
+        with patch("agent.auxiliary_client.resolve_provider_client", side_effect=_fake_resolve):
+            result = agent._try_activate_fallback()
+
+        assert result is True
+        assert captured["explicit_api_key"] == "secret-via-key-env"
+
+    def test_api_key_env_unset_does_not_crash(self, monkeypatch):
+        """When api_key_env refers to an unset variable, explicit_api_key is None
+        (not an empty string) so the provider can fall through to its default."""
+        monkeypatch.delenv("ABSENT_KEY_VAR", raising=False)
+
+        agent = _make_agent(
+            fallback_model={
+                "provider": "openrouter",
+                "model": "some/model",
+                "api_key_env": "ABSENT_KEY_VAR",
+            },
+        )
+        captured = {}
+
+        def _fake_resolve(provider, model=None, raw_codex=False,
+                          explicit_base_url=None, explicit_api_key=None, **kw):
+            captured["explicit_api_key"] = explicit_api_key
+            mock = MagicMock()
+            mock.api_key = "fallback-default"
+            mock.base_url = "https://openrouter.ai/api/v1"
+            return mock, model
+
+        with patch("agent.auxiliary_client.resolve_provider_client", side_effect=_fake_resolve):
+            agent._try_activate_fallback()
+
+        assert captured["explicit_api_key"] is None, (
+            "Unset api_key_env should yield None, not empty string"
+        )
