mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-24 00:52:24 +00:00
fix(chatgpt): route the chat transformation through DBAuthenticator too
Reported symptom: "Test model" in the UI hangs indefinitely, a new
device code appears in the proxy logs, and the whole UI is frozen.
Root cause: the ChatGPT **chat** transformation still used the
filesystem-only ``Authenticator`` unconditionally. When the UI's
``/health/test_connection`` runs in the default ``mode="chat"``:
UI → /health/test_connection (sync wait)
→ litellm.ahealth_check(mode="chat")
→ litellm.acompletion(...)
→ ChatGPTConfig._get_openai_compatible_provider_info(api_key="oauth:X")
→ self.authenticator.get_access_token() # filesystem authenticator
→ _login_device_code() # no tokens on disk → prints a new
# device code and polls for up to 15
# minutes, blocking the request
``ChatGPTResponsesAPIConfig`` already had the ``resolve_authenticator``
dispatch (the PR's original scope), so the responses-mode path was
fine. The chat path was overlooked.
Fix:
- Apply ``resolve_authenticator`` to both call-sites in
``ChatGPTConfig`` (``_get_openai_compatible_provider_info`` and
``validate_environment``). When ``api_key`` carries the ``oauth:``
prefix, both methods now reach for the DB-backed authenticator and
the stored credential — no device-code prompt, no 15-minute hang.
- Extend ``resolve_authenticator`` to the 3-arg shape
``(api_key, litellm_params, fallback)`` so it matches the Copilot
resolver and covers the chat call-site that has ``api_key`` but not
``litellm_params``. Updated the single existing call-site in
``responses/transformation.py`` and the existing unit tests.
- Add a regression test asserting that ``ChatGPTConfig`` never calls
the filesystem authenticator when ``api_key`` is ``oauth:<name>``
(fs_auth is a MagicMock that raises on any attribute access).
170 tests pass; Black + Ruff clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
9167f9725b
commit
581b3e9692
5 changed files with 126 additions and 23 deletions
|
|
@ -10,6 +10,7 @@ from ..common_utils import (
|
|||
ensure_chatgpt_session_id,
|
||||
get_chatgpt_default_headers,
|
||||
)
|
||||
from ..db_authenticator import resolve_authenticator
|
||||
from .streaming_utils import ChatGPTToolCallNormalizer
|
||||
|
||||
|
||||
|
|
@ -30,9 +31,10 @@ class ChatGPTConfig(OpenAIConfig):
|
|||
api_key: Optional[str],
|
||||
custom_llm_provider: str,
|
||||
) -> Tuple[Optional[str], Optional[str], str]:
|
||||
dynamic_api_base = self.authenticator.get_api_base()
|
||||
authenticator = resolve_authenticator(api_key, None, self.authenticator)
|
||||
dynamic_api_base = authenticator.get_api_base()
|
||||
try:
|
||||
dynamic_api_key = self.authenticator.get_access_token()
|
||||
dynamic_api_key = authenticator.get_access_token()
|
||||
except GetAccessTokenError as e:
|
||||
raise AuthenticationError(
|
||||
model=model,
|
||||
|
|
@ -55,7 +57,10 @@ class ChatGPTConfig(OpenAIConfig):
|
|||
headers, model, messages, optional_params, litellm_params, api_key, api_base
|
||||
)
|
||||
|
||||
account_id = self.authenticator.get_account_id()
|
||||
authenticator = resolve_authenticator(
|
||||
api_key, litellm_params, self.authenticator
|
||||
)
|
||||
account_id = authenticator.get_account_id()
|
||||
session_id = ensure_chatgpt_session_id(litellm_params)
|
||||
default_headers = get_chatgpt_default_headers(
|
||||
api_key or "", account_id, session_id
|
||||
|
|
|
|||
|
|
@ -218,23 +218,33 @@ async def persist_credential_to_db(item: CredentialItem) -> None:
|
|||
|
||||
|
||||
def resolve_authenticator(
|
||||
api_key: Optional[str],
|
||||
litellm_params: Any,
|
||||
fallback: Authenticator,
|
||||
) -> Authenticator:
|
||||
"""
|
||||
If ``litellm_params.api_key`` starts with ``oauth:``, returns a
|
||||
:class:`DBAuthenticator` for the named credential. Otherwise returns
|
||||
the given fallback (typically the filesystem :class:`Authenticator`).
|
||||
If ``api_key`` (or ``litellm_params.api_key``) starts with ``oauth:``,
|
||||
returns a :class:`DBAuthenticator` for the named credential. Otherwise
|
||||
returns the given fallback (typically the filesystem
|
||||
:class:`Authenticator`).
|
||||
|
||||
Two sources are checked because the chat transformation's
|
||||
``_get_openai_compatible_provider_info`` call-site has ``api_key`` but
|
||||
not ``litellm_params``, while ``validate_environment`` on both chat
|
||||
and responses has one or both.
|
||||
"""
|
||||
if litellm_params is None:
|
||||
return fallback
|
||||
api_key = (
|
||||
litellm_params.get("api_key")
|
||||
if isinstance(litellm_params, dict)
|
||||
else getattr(litellm_params, "api_key", None)
|
||||
)
|
||||
if isinstance(api_key, str) and api_key.startswith(OAUTH_CREDENTIAL_API_KEY_PREFIX):
|
||||
return DBAuthenticator(
|
||||
credential_name=api_key[len(OAUTH_CREDENTIAL_API_KEY_PREFIX) :]
|
||||
)
|
||||
candidates = [api_key]
|
||||
if litellm_params is not None:
|
||||
if isinstance(litellm_params, dict):
|
||||
candidates.append(litellm_params.get("api_key"))
|
||||
else:
|
||||
candidates.append(getattr(litellm_params, "api_key", None))
|
||||
|
||||
for candidate in candidates:
|
||||
if isinstance(candidate, str) and candidate.startswith(
|
||||
OAUTH_CREDENTIAL_API_KEY_PREFIX
|
||||
):
|
||||
return DBAuthenticator(
|
||||
credential_name=candidate[len(OAUTH_CREDENTIAL_API_KEY_PREFIX) :]
|
||||
)
|
||||
return fallback
|
||||
|
|
|
|||
|
|
@ -43,7 +43,7 @@ class ChatGPTResponsesAPIConfig(OpenAIResponsesAPIConfig):
|
|||
model: str,
|
||||
litellm_params: Optional[GenericLiteLLMParams],
|
||||
) -> dict:
|
||||
authenticator = resolve_authenticator(litellm_params, self.authenticator)
|
||||
authenticator = resolve_authenticator(None, litellm_params, self.authenticator)
|
||||
try:
|
||||
access_token = authenticator.get_access_token()
|
||||
except GetAccessTokenError as e:
|
||||
|
|
|
|||
|
|
@ -0,0 +1,75 @@
|
|||
"""
|
||||
Regression test: when the ChatGPT *chat* transformation runs with
|
||||
``api_key="oauth:<name>"``, it must route through ``DBAuthenticator``.
|
||||
|
||||
The filesystem ``Authenticator`` would otherwise trigger the 15-minute
|
||||
device-code flow on the server thread (no tokens on disk) — manifesting
|
||||
as a hung "Test model" in the UI.
|
||||
"""
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from litellm.llms.chatgpt.authenticator import Authenticator
|
||||
from litellm.llms.chatgpt.chat.transformation import ChatGPTConfig
|
||||
from litellm.llms.chatgpt.db_authenticator import (
|
||||
OAUTH_CREDENTIAL_API_KEY_PREFIX,
|
||||
DBAuthenticator,
|
||||
)
|
||||
|
||||
|
||||
class TestChatTransformationDispatch:
|
||||
def test_get_openai_compatible_provider_info_uses_db_authenticator(self):
|
||||
config = ChatGPTConfig()
|
||||
fs_auth = MagicMock(spec=Authenticator)
|
||||
fs_auth.get_access_token.side_effect = AssertionError(
|
||||
"Filesystem authenticator must not be called for oauth: prefix"
|
||||
)
|
||||
fs_auth.get_api_base.side_effect = AssertionError(
|
||||
"Filesystem authenticator must not be called for oauth: prefix"
|
||||
)
|
||||
config.authenticator = fs_auth
|
||||
|
||||
with (
|
||||
patch.object(
|
||||
DBAuthenticator,
|
||||
"get_api_base",
|
||||
return_value="https://chatgpt.com/backend-api/codex",
|
||||
),
|
||||
patch.object(
|
||||
DBAuthenticator, "get_access_token", return_value="db-access-token"
|
||||
),
|
||||
):
|
||||
base, key, _ = config._get_openai_compatible_provider_info(
|
||||
model="chatgpt/gpt-5.3-codex",
|
||||
api_base=None,
|
||||
api_key=f"{OAUTH_CREDENTIAL_API_KEY_PREFIX}my-creds",
|
||||
custom_llm_provider="chatgpt",
|
||||
)
|
||||
assert key == "db-access-token"
|
||||
assert base == "https://chatgpt.com/backend-api/codex"
|
||||
|
||||
def test_validate_environment_uses_db_authenticator(self):
|
||||
config = ChatGPTConfig()
|
||||
fs_auth = MagicMock(spec=Authenticator)
|
||||
fs_auth.get_account_id.side_effect = AssertionError(
|
||||
"Filesystem authenticator must not be called for oauth: prefix"
|
||||
)
|
||||
config.authenticator = fs_auth
|
||||
|
||||
with (
|
||||
patch.object(DBAuthenticator, "get_account_id", return_value="acct-db"),
|
||||
patch(
|
||||
"litellm.llms.openai.openai.OpenAIConfig.validate_environment",
|
||||
return_value={},
|
||||
),
|
||||
):
|
||||
headers = config.validate_environment(
|
||||
headers={},
|
||||
model="chatgpt/gpt-5.3-codex",
|
||||
messages=[{"role": "user", "content": "hi"}],
|
||||
optional_params={},
|
||||
litellm_params={},
|
||||
api_key=f"{OAUTH_CREDENTIAL_API_KEY_PREFIX}my-creds",
|
||||
api_base=None,
|
||||
)
|
||||
assert headers.get("ChatGPT-Account-Id") == "acct-db"
|
||||
|
|
@ -14,27 +14,40 @@ class TestResolveAuthenticator:
|
|||
def test_plain_api_key_returns_fallback(self):
|
||||
fallback = MagicMock(spec=Authenticator)
|
||||
resolved = resolve_authenticator(
|
||||
GenericLiteLLMParams(api_key="sk-plain"), fallback
|
||||
None, GenericLiteLLMParams(api_key="sk-plain"), fallback
|
||||
)
|
||||
assert resolved is fallback
|
||||
|
||||
def test_none_litellm_params_returns_fallback(self):
|
||||
def test_none_both_returns_fallback(self):
|
||||
fallback = MagicMock(spec=Authenticator)
|
||||
assert resolve_authenticator(None, fallback) is fallback
|
||||
assert resolve_authenticator(None, None, fallback) is fallback
|
||||
|
||||
def test_oauth_prefix_returns_db_authenticator(self):
|
||||
def test_oauth_prefix_in_litellm_params_returns_db_authenticator(self):
|
||||
fallback = MagicMock(spec=Authenticator)
|
||||
resolved = resolve_authenticator(
|
||||
None,
|
||||
GenericLiteLLMParams(api_key=f"{OAUTH_CREDENTIAL_API_KEY_PREFIX}my-creds"),
|
||||
fallback,
|
||||
)
|
||||
assert isinstance(resolved, DBAuthenticator)
|
||||
assert resolved.credential_name == "my-creds"
|
||||
|
||||
def test_oauth_prefix_in_api_key_arg_returns_db_authenticator(self):
|
||||
"""Chat transformation's ``_get_openai_compatible_provider_info``
|
||||
call-site passes ``api_key`` directly without ``litellm_params``."""
|
||||
fallback = MagicMock(spec=Authenticator)
|
||||
resolved = resolve_authenticator(
|
||||
f"{OAUTH_CREDENTIAL_API_KEY_PREFIX}my-creds", None, fallback
|
||||
)
|
||||
assert isinstance(resolved, DBAuthenticator)
|
||||
assert resolved.credential_name == "my-creds"
|
||||
|
||||
def test_oauth_prefix_with_empty_suffix(self):
|
||||
fallback = MagicMock(spec=Authenticator)
|
||||
resolved = resolve_authenticator(
|
||||
GenericLiteLLMParams(api_key=OAUTH_CREDENTIAL_API_KEY_PREFIX), fallback
|
||||
None,
|
||||
GenericLiteLLMParams(api_key=OAUTH_CREDENTIAL_API_KEY_PREFIX),
|
||||
fallback,
|
||||
)
|
||||
assert isinstance(resolved, DBAuthenticator)
|
||||
assert resolved.credential_name == ""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue