From 85db6895e3c1fece040cb7765a04ea01ea0306e1 Mon Sep 17 00:00:00 2001 From: chrismuzyn Date: Thu, 28 May 2026 22:18:31 +0000 Subject: [PATCH] make sure empty strings don't get passed to the openapi sdk which don't allow env var fallback --- tests/test_model_registry.py | 46 ++++++++++++++++++++++++++++ tests/test_providers.py | 18 +++++++++++ turnstone/core/model_registry.py | 2 +- turnstone/core/providers/__init__.py | 18 ++++++++--- 4 files changed, 79 insertions(+), 5 deletions(-) diff --git a/tests/test_model_registry.py b/tests/test_model_registry.py index 68d44d2d..cefe0fc4 100644 --- a/tests/test_model_registry.py +++ b/tests/test_model_registry.py @@ -802,6 +802,52 @@ class TestLoadModelRegistryWithDB: reg = load_model_registry("http://x/v1", "x", "x", storage=storage) assert reg.has_alias("default") + def test_db_model_empty_api_key_falls_back_to_cli(self) -> None: + """DB model with empty api_key inherits the CLI/api_key fallback.""" + storage = _MockStorage( + [ + { + "alias": "cloud", + "model": "gpt-5", + "provider": "openai", + "base_url": "https://api.openai.com/v1", + "api_key": "", + "context_window": 32768, + "capabilities": "{}", + "enabled": True, + } + ] + ) + with patch("turnstone.core.model_registry.load_config", return_value={}): + reg = load_model_registry( + "http://x/v1", "cli-fallback-key", "x", storage=storage + ) + cfg = reg.get_config("cloud") + assert cfg.api_key == "cli-fallback-key" + + def test_db_model_explicit_api_key_overrides_cli(self) -> None: + """DB model with its own api_key uses it, not the CLI fallback.""" + storage = _MockStorage( + [ + { + "alias": "cloud", + "model": "gpt-5", + "provider": "openai", + "base_url": "https://api.openai.com/v1", + "api_key": "sk-db-specific", + "context_window": 32768, + "capabilities": "{}", + "enabled": True, + } + ] + ) + with patch("turnstone.core.model_registry.load_config", return_value={}): + reg = load_model_registry( + "http://x/v1", "cli-fallback-key", "x", storage=storage + ) + cfg = reg.get_config("cloud") + assert cfg.api_key == "sk-db-specific" + # --------------------------------------------------------------------------- # _resolve_env_vars diff --git a/tests/test_providers.py b/tests/test_providers.py index 2cef427f..2fec0879 100644 --- a/tests/test_providers.py +++ b/tests/test_providers.py @@ -1366,6 +1366,24 @@ class TestProviderFactory: ) assert client is mock_openai_cls.return_value + @patch("openai.OpenAI") + def test_create_client_empty_api_key_passes_none(self, mock_openai_cls: MagicMock) -> None: + from turnstone.core.providers import create_client + + mock_openai_cls.return_value = MagicMock() + create_client("openai", base_url="http://localhost:8000/v1", api_key="") + mock_openai_cls.assert_called_once_with( + base_url="http://localhost:8000/v1", api_key=None + ) + + @patch("openai.OpenAI") + def test_create_client_empty_api_key_no_base_url(self, mock_openai_cls: MagicMock) -> None: + from turnstone.core.providers import create_client + + mock_openai_cls.return_value = MagicMock() + create_client("openai", base_url="", api_key="") + mock_openai_cls.assert_called_once_with(api_key=None) + def test_create_client_unknown(self) -> None: from turnstone.core.providers import create_client diff --git a/turnstone/core/model_registry.py b/turnstone/core/model_registry.py index 9965cb4f..ec222948 100644 --- a/turnstone/core/model_registry.py +++ b/turnstone/core/model_registry.py @@ -418,7 +418,7 @@ def load_model_registry( configs[alias] = ModelConfig( alias=alias, base_url=row_base_url, - api_key=_resolve_env_vars(row.get("api_key", "")), + api_key=_resolve_env_vars(row.get("api_key") or api_key), model=row_model, context_window=row_ctx, provider=row_provider, diff --git a/turnstone/core/providers/__init__.py b/turnstone/core/providers/__init__.py index 474d6cd8..7b9c40fa 100644 --- a/turnstone/core/providers/__init__.py +++ b/turnstone/core/providers/__init__.py @@ -112,7 +112,15 @@ def create_provider( def create_client(provider_name: str, *, base_url: str, api_key: str) -> Any: - """Create an SDK client for the given provider.""" + """Create an SDK client for the given provider. + + An empty *api_key* is converted to ``None`` so that the underlying + SDK can fall back to its own environment-variable lookup (e.g. + ``OPENAI_API_KEY``, ``ANTHROPIC_API_KEY``). Passing an empty + string would short-circuit the SDK's env-var check and raise a + "Missing credentials" error even when the env var is set. + """ + resolved_key: str | None = api_key if api_key else None if provider_name in ("openai", "openai-compatible", "google", "xai"): from openai import OpenAI @@ -123,13 +131,15 @@ def create_client(provider_name: str, *, base_url: str, api_key: str) -> Any: elif not base_url and provider_name == "xai": base_url = XAI_DEFAULT_BASE_URL if base_url: - return OpenAI(base_url=base_url, api_key=api_key) - return OpenAI(api_key=api_key) + return OpenAI(base_url=base_url, api_key=resolved_key) + return OpenAI(api_key=resolved_key) if provider_name == "anthropic": from turnstone.core.providers._anthropic import _ensure_anthropic anthropic = _ensure_anthropic() - kwargs: dict[str, str] = {"api_key": api_key} + kwargs: dict[str, str] = {} + if resolved_key is not None: + kwargs["api_key"] = resolved_key if base_url and base_url != "https://api.anthropic.com": kwargs["base_url"] = base_url return anthropic.Anthropic(**kwargs)