From 7020e1e5f75fe6f98480753825589345d027b3c7 Mon Sep 17 00:00:00 2001 From: tin-berri Date: Tue, 23 Jun 2026 09:11:49 -0700 Subject: [PATCH 1/3] feat(mcp): add resolve_credentials dispatch skeleton (#31056) PR3 of the MCP v2 outbound-credential migration, stacked on the typed vocabulary. Adds resolver.py: UpstreamCredentialProvider.resolve_credentials dispatches on the declared AuthConfig variant with one arm per mode, a wildcard-free match plus an assert_never tail so a missing arm fails basedpyright's exhaustiveness gate. Every arm is a not_implemented stub returning a typed CredError; each mode's real body and seam land in follow-up PRs. Pure v2, no v1 imports, nothing wired onto a request path. --- .../outbound_credentials/__init__.py | 15 ++-- .../outbound_credentials/resolver.py | 70 +++++++++++++++++++ .../outbound_credentials/test_resolver.py | 57 +++++++++++++++ 3 files changed, 137 insertions(+), 5 deletions(-) create mode 100644 litellm/proxy/_experimental/mcp_server/outbound_credentials/resolver.py create mode 100644 tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_resolver.py diff --git a/litellm/proxy/_experimental/mcp_server/outbound_credentials/__init__.py b/litellm/proxy/_experimental/mcp_server/outbound_credentials/__init__.py index b357504979d..73166a45d6e 100644 --- a/litellm/proxy/_experimental/mcp_server/outbound_credentials/__init__.py +++ b/litellm/proxy/_experimental/mcp_server/outbound_credentials/__init__.py @@ -1,16 +1,20 @@ """Typed upstream-credential resolution for MCP servers. -This subpackage houses the typed credential vocabulary and (in a later PR) the -``resolve_credentials`` dispatch. A server declares one per-mode config from the -``AuthConfig`` discriminated union; failures are modeled as values via :mod:`.result` -(``Result[T, CredError]``) rather than raised, so every seam is total. Nothing here is -wired onto a live request path yet. +This subpackage houses the typed credential vocabulary and the ``resolve_credentials`` +dispatch. A server declares one per-mode config from the ``AuthConfig`` discriminated union; +``UpstreamCredentialProvider.resolve_credentials`` selects one arm and returns an ``httpx.Auth`` +or a typed ``CredError``. Failures are modeled as values via :mod:`.result` (``Result[T, +CredError]``) rather than raised, so every seam is total. Nothing here is wired onto a live +request path yet. """ from litellm.proxy._experimental.mcp_server.outbound_credentials.httpx_auth import ( NoOpAuth, StaticHeaderAuth, ) +from litellm.proxy._experimental.mcp_server.outbound_credentials.resolver import ( + UpstreamCredentialProvider, +) from litellm.proxy._experimental.mcp_server.outbound_credentials.result import ( Error, Ok, @@ -45,6 +49,7 @@ __all__ = [ "Result", "NoOpAuth", "StaticHeaderAuth", + "UpstreamCredentialProvider", "AuthSpecKind", "CredError", "Subject", diff --git a/litellm/proxy/_experimental/mcp_server/outbound_credentials/resolver.py b/litellm/proxy/_experimental/mcp_server/outbound_credentials/resolver.py new file mode 100644 index 00000000000..7bcdb3e6529 --- /dev/null +++ b/litellm/proxy/_experimental/mcp_server/outbound_credentials/resolver.py @@ -0,0 +1,70 @@ +"""The one credential resolver: dispatch on the declared mode, fail closed. + +`resolve_credentials` selects exactly one arm off the server's typed `config` and either +produces an `httpx.Auth` or returns a typed `CredError`. The `match` is over the `AuthConfig` +variant, so each arm receives its own fully-typed config with no field-presence inference and +no precedence cascade. It is wildcard-free with an `assert_never` tail, so adding a mode without +an arm fails the type gate (basedpyright `reportMatchNotExhaustive`); a bypassed gate fails loudly +at runtime instead of returning `None`. + +This skeleton ships every arm as a `not_implemented` stub. Each mode's real body, with its +injected seam, lands in its own follow-up PR; until then the arm returns a typed error rather +than silently producing no credential. Pure v2: no imports from v1. +""" + +from __future__ import annotations + +import httpx +from typing_extensions import assert_never + +from litellm.proxy._experimental.mcp_server.outbound_credentials.result import ( + Error, + Result, +) +from litellm.proxy._experimental.mcp_server.outbound_credentials.types import ( + ApiKeyConfig, + AuthorizationCodeConfig, + AuthSpecKind, + AwsSigV4Config, + ClientCredentialsConfig, + CredError, + NoneConfig, + PassthroughConfig, + ServerSpec, + Subject, + TokenExchangeConfig, +) + + +class UpstreamCredentialProvider: + """Produces the one `httpx.Auth` for a `(subject, upstream)` pair, per declared mode. + + Collaborators (the per-mode credential stores and token fetchers) are injected as each arm + is built; the skeleton needs none, since every arm is a stub. + """ + + async def resolve_credentials( + self, subject: Subject, server: ServerSpec + ) -> Result[httpx.Auth, CredError]: + match server.config: + case NoneConfig(): + return _not_implemented(AuthSpecKind.none) + case ApiKeyConfig(): + return _not_implemented(AuthSpecKind.api_key) + case PassthroughConfig(): + return _not_implemented(AuthSpecKind.passthrough) + case ClientCredentialsConfig(): + return _not_implemented(AuthSpecKind.client_credentials) + case TokenExchangeConfig(): + return _not_implemented(AuthSpecKind.token_exchange) + case AuthorizationCodeConfig(): + return _not_implemented(AuthSpecKind.authorization_code) + case AwsSigV4Config(): + return _not_implemented(AuthSpecKind.aws_sigv4) + assert_never(server.config) + + +def _not_implemented(kind: AuthSpecKind) -> Result[httpx.Auth, CredError]: + return Error( + CredError.of_not_implemented(f"{kind.value}: resolver arm not implemented yet") + ) diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_resolver.py b/tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_resolver.py new file mode 100644 index 00000000000..7885617aa46 --- /dev/null +++ b/tests/test_litellm/proxy/_experimental/mcp_server/outbound_credentials/test_resolver.py @@ -0,0 +1,57 @@ +"""Tests for the resolver dispatch skeleton. + +Every mode must reach its own arm and, until that arm is built, return a typed +`not_implemented` CredError rather than silently producing no credential. Parametrizing over +one config per mode also guards reachability: if a `case` were dropped, that mode would fall to +the `assert_never` tail and raise here instead of returning the stub. +""" + +import pytest +from pydantic import SecretStr + +from litellm.proxy._experimental.mcp_server.outbound_credentials import ( + ApiKeyConfig, + AuthorizationCodeConfig, + AuthSpecKind, + AwsSigV4Config, + ClientCredentialsConfig, + Error, + NoneConfig, + PassthroughConfig, + ServerSpec, + SharedKey, + Subject, + TokenExchangeConfig, + UpstreamCredentialProvider, +) + +_ONE_CONFIG_PER_MODE = [ + (AuthSpecKind.none, NoneConfig()), + (AuthSpecKind.api_key, ApiKeyConfig(key_source=SharedKey(value=SecretStr("k")))), + (AuthSpecKind.passthrough, PassthroughConfig()), + (AuthSpecKind.client_credentials, ClientCredentialsConfig()), + (AuthSpecKind.token_exchange, TokenExchangeConfig()), + (AuthSpecKind.authorization_code, AuthorizationCodeConfig()), + (AuthSpecKind.aws_sigv4, AwsSigV4Config(region="us-east-1")), +] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("kind, config", _ONE_CONFIG_PER_MODE) +async def test_every_mode_reaches_its_arm_and_returns_not_implemented(kind, config): + spec = ServerSpec( + server_id="s", resource="https://upstream.example.com", config=config + ) + subject = Subject(tenant_id="", subject_id="") + + result = await UpstreamCredentialProvider().resolve_credentials(subject, spec) + + assert isinstance(result, Error) + assert result.error.tag == "not_implemented" + assert kind.value in result.error.summary + + +def test_all_seven_modes_are_covered(): + # Guards that the parametrization (and therefore the dispatch) spans every AuthSpecKind, so a + # newly added mode without a test row is caught here rather than slipping through. + assert {kind for kind, _ in _ONE_CONFIG_PER_MODE} == set(AuthSpecKind) From 02f63d20bb563f7ebbd5396d4be85db66a4f26e4 Mon Sep 17 00:00:00 2001 From: milan-berri Date: Tue, 23 Jun 2026 19:16:15 +0200 Subject: [PATCH 2/3] fix(router): guard num_retries=None in async_function_with_retries (#30036) When num_retries reaches async_function_with_retries as None - e.g. a caller passes num_retries=None explicitly (dict.get() does not fall back on an existing None value), an auto_router/complexity_router path does not propagate it, or Router.update_settings(num_retries=None) is used - the comparison `if num_retries > 0:` raised: TypeError: '>' not supported between instances of 'NoneType' and 'int' This only surfaced when the underlying call failed with a retryable error (rate limit / connection / 5xx), so the real upstream error was masked by a confusing TypeError. Normalise an explicit num_retries=None to the router default in _update_kwargs_before_fallbacks (falling back to 0 when the router default is itself None, and preserving an explicit 0), and keep the matching guard at the single pop site in async_function_with_retries as the safety net for paths that bypass the setter. Adds regression tests to test_router_per_deployment_num_retries.py. Relates to #23316, #25889, #23699, #28126 Co-authored-by: Cursor Co-authored-by: yuneng-jiang --- litellm/router.py | 19 ++- .../test_router_per_deployment_num_retries.py | 133 +++++++++++++++++- 2 files changed, 149 insertions(+), 3 deletions(-) diff --git a/litellm/router.py b/litellm/router.py index acce1c58a8b..c687b2d9f67 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -3134,7 +3134,18 @@ class Router: - litellm_trace_id - metadata """ - kwargs["num_retries"] = kwargs.get("num_retries", self.num_retries) + # Normalise an explicit num_retries=None to the router default here (dict.get() + # only falls back when the key is absent, not when its value is None), then to 0 + # if the router default is itself None - mirroring the guard in + # async_function_with_retries, which remains the safety net for paths that bypass + # this setter. + _req_num_retries = kwargs.get("num_retries") + if _req_num_retries is not None: + kwargs["num_retries"] = _req_num_retries + else: + kwargs["num_retries"] = ( + self.num_retries if self.num_retries is not None else 0 + ) kwargs.setdefault("litellm_trace_id", str(uuid.uuid4())) model_group_alias: Optional[str] = None if self._get_model_from_alias(model=model): @@ -6931,7 +6942,11 @@ class Router: "model_group_retry_policy", self.model_group_retry_policy ) model_group: Optional[str] = kwargs.get("model") - num_retries = kwargs.pop("num_retries") + num_retries = kwargs.pop("num_retries", None) + if num_retries is None: + # Fall back to the router setting (then 0) so the comparisons below never + # hit `None > int`, which would mask the real upstream error with a TypeError. + num_retries = self.num_retries if self.num_retries is not None else 0 ## ADD MODEL GROUP SIZE TO METADATA - used for model_group_rate_limit_error tracking _metadata: dict = kwargs.get("litellm_metadata", kwargs.get("metadata")) or {} diff --git a/tests/test_litellm/test_router_per_deployment_num_retries.py b/tests/test_litellm/test_router_per_deployment_num_retries.py index 154ba579e4e..af2372616a6 100644 --- a/tests/test_litellm/test_router_per_deployment_num_retries.py +++ b/tests/test_litellm/test_router_per_deployment_num_retries.py @@ -4,8 +4,9 @@ GitHub Issue: #18968 - Per-deployment max_retries/num_retries in litellm_params """ import pytest -from unittest.mock import MagicMock, patch +from unittest.mock import patch +import litellm from litellm import Router @@ -188,3 +189,133 @@ class TestPerDeploymentNumRetries: # Verify num_retries was converted from string to int assert exc.num_retries == 6 + + +class TestNumRetriesNoneGuard: + """ + Regression tests for the num_retries=None TypeError in async_function_with_retries. + + When num_retries reaches async_function_with_retries as None - e.g. a caller passes + num_retries=None explicitly (dict.get() does not fall back on an existing None value), + an auto_router/complexity_router path does not propagate it, or + Router.update_settings(num_retries=None) is used - AND the underlying call fails with a + retryable error, the comparison `if num_retries > 0:` raised: + + TypeError: '>' not supported between instances of 'NoneType' and 'int' + + This masked the real upstream error (rate limit / connection / 5xx) behind a TypeError. + Related issues: #23316, #25889, #23699, #28126. + """ + + @staticmethod + def _mock_router(num_retries=2): + return Router( + model_list=[ + { + "model_name": "mock-model", + "litellm_params": { + "model": "gpt-4o-mini", + "mock_response": "ok", + }, + } + ], + num_retries=num_retries, + ) + + def test_update_kwargs_normalises_explicit_none_to_router_default(self): + """ + _update_kwargs_before_fallbacks must normalise an explicit num_retries=None to + the router default (not leave it as None), while preserving an explicit 0. + """ + router = self._mock_router(num_retries=4) + + # explicit None -> router default + kwargs = {"num_retries": None} + router._update_kwargs_before_fallbacks(model="mock-model", kwargs=kwargs) + assert kwargs["num_retries"] == 4 + + # explicit 0 is preserved (retries stay disabled) + kwargs = {"num_retries": 0} + router._update_kwargs_before_fallbacks(model="mock-model", kwargs=kwargs) + assert kwargs["num_retries"] == 0 + + # absent -> router default (unchanged behaviour) + kwargs = {} + router._update_kwargs_before_fallbacks(model="mock-model", kwargs=kwargs) + assert kwargs["num_retries"] == 4 + + # explicit None with router default also None -> 0 (mirrors the downstream guard) + router.num_retries = None # simulate update_settings(num_retries=None) (#28126) + kwargs = {"num_retries": None} + router._update_kwargs_before_fallbacks(model="mock-model", kwargs=kwargs) + assert kwargs["num_retries"] == 0 + + @pytest.mark.asyncio + async def test_acompletion_num_retries_none_does_not_raise_typeerror(self): + """ + Per-request num_retries=None + a retryable error must NOT raise TypeError. + The router falls back to its configured num_retries and retries the (transient) + error, so the request succeeds. + """ + router = self._mock_router(num_retries=2) + with patch("asyncio.sleep", return_value=None): + response = await router.acompletion( + model="mock-model", + messages=[{"role": "user", "content": "hi"}], + num_retries=None, # the trigger + mock_testing_rate_limit_error=True, # retryable error path + ) + assert response.choices[0].message.content == "ok" + + @pytest.mark.asyncio + async def test_async_function_with_retries_none_falls_back_to_zero(self): + """ + When both the per-request value AND the router-level setting are None + (e.g. after Router.update_settings(num_retries=None), #28126), num_retries must + fall back to 0 and the real retryable error must surface - not a TypeError. + """ + router = self._mock_router(num_retries=0) + router.num_retries = None # simulate update_settings(num_retries=None) + + async def failing_fn(*args, **kwargs): + raise litellm.RateLimitError( + message="boom", model="mock-model", llm_provider="openai" + ) + + with patch("asyncio.sleep", return_value=None): + with pytest.raises(litellm.RateLimitError): + await router.async_function_with_retries( + original_function=failing_fn, + model="mock-model", + messages=[{"role": "user", "content": "hi"}], + num_retries=None, + ) + + @pytest.mark.asyncio + async def test_async_function_with_retries_none_falls_back_to_router_default(self): + """ + A None per-request num_retries falls back to the router-level setting, so retries + still happen (original_function is invoked more than once) before the real error + is raised - proving None did not silently disable retries or crash. + """ + router = self._mock_router(num_retries=3) + calls = {"n": 0} + + async def failing_fn(*args, **kwargs): + calls["n"] += 1 + raise litellm.InternalServerError( + message="boom", model="mock-model", llm_provider="openai" + ) + + with patch("asyncio.sleep", return_value=None): + with pytest.raises(litellm.InternalServerError): + await router.async_function_with_retries( + original_function=failing_fn, + model="mock-model", + messages=[{"role": "user", "content": "hi"}], + metadata={}, # populated by acompletion in the real path; log_retry needs it + num_retries=None, + ) + + # 1 initial attempt + at least 1 retry -> proves None fell back to a positive int + assert calls["n"] >= 2 From a5b75e8bab4b0ed6fda317c1fd3884bfe747666a Mon Sep 17 00:00:00 2001 From: yuneng-jiang Date: Tue, 23 Jun 2026 10:33:31 -0700 Subject: [PATCH 3/3] fix(ui): keep team Organization optional for proxy admins in single-org setups (#30861) The Create Team form auto-selected, disabled, and required the Organization field whenever exactly one organization existed, regardless of role. For a proxy admin the organization is optional, so single-org setups could not create a standalone team even though the field is presented as optional. Gate the single-org preselect, the disabled state, and the restrictive help text on the org-admin role so they apply only to org admins, who must scope a team to their organization. Proxy admins now keep an optional, clearable, empty organization field regardless of how many organizations exist, matching the multi-org behavior. The /team/new endpoint already accepts a null organization, so this was a UI-only restriction. --- .../src/components/OldTeams.test.tsx | 49 +++++++++++++++++++ .../src/components/OldTeams.tsx | 11 +++-- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/ui/litellm-dashboard/src/components/OldTeams.test.tsx b/ui/litellm-dashboard/src/components/OldTeams.test.tsx index d777ba1b0dc..0b6e5786aaf 100644 --- a/ui/litellm-dashboard/src/components/OldTeams.test.tsx +++ b/ui/litellm-dashboard/src/components/OldTeams.test.tsx @@ -1097,3 +1097,52 @@ describe("OldTeams - delete team warning copy", () => { ); }); }); + +describe("OldTeams - LIT-2530 organization stays optional for proxy admin with a single org", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockTeamInfoView.mockClear(); + vi.mocked(fetchAvailableModelsForTeamOrKey).mockResolvedValue(["gpt-4"]); + vi.mocked(fetchMCPAccessGroups).mockResolvedValue([]); + vi.mocked(getGuardrailsList).mockResolvedValue({ guardrails: [] }); + vi.mocked(teamListCall).mockResolvedValue({ teams: [], total: 0, page: 1, page_size: 100, total_pages: 1 }); + vi.mocked(teamCreateCall).mockResolvedValue({ + team_id: "new-team-1", + team_alias: "No Org Team", + models: ["gpt-4"], + organization_id: null, + keys: [], + members_with_roles: [], + spend: 0, + }); + mockUseOrganizations.mockReturnValue({ + data: [{ organization_id: "org-1", organization_alias: "Org 1", models: [], members: [] }], + }); + }); + + it("creates a team with no organization when exactly one organization exists", async () => { + renderWithQueryClient(); + + const createButton = screen.getAllByRole("button", { name: /create team/i })[0]; + act(() => { + fireEvent.click(createButton); + }); + + await waitFor(() => { + expect(screen.getByLabelText(/team name/i)).toBeInTheDocument(); + }); + + fireEvent.change(screen.getByLabelText(/team name/i), { target: { value: "No Org Team" } }); + fireEvent.change(screen.getByTestId("create-team-models-select"), { target: { value: "gpt-4" } }); + + const submitButtons = screen.getAllByRole("button", { name: /create team/i }); + fireEvent.click(submitButtons[submitButtons.length - 1]); + + await waitFor(() => { + expect(teamCreateCall).toHaveBeenCalledWith( + "test-token", + expect.objectContaining({ team_alias: "No Org Team", organization_id: null }), + ); + }); + }); +}); diff --git a/ui/litellm-dashboard/src/components/OldTeams.tsx b/ui/litellm-dashboard/src/components/OldTeams.tsx index adfec4bdf6a..be9015e3730 100644 --- a/ui/litellm-dashboard/src/components/OldTeams.tsx +++ b/ui/litellm-dashboard/src/components/OldTeams.tsx @@ -262,14 +262,15 @@ const Teams: React.FC = ({ accessToken, userID, userRole, premiumUser useEffect(() => { if (isTeamModalVisible) { const adminOrgs = getAdminOrganizations(userRole, userID, organizations); + const isOrgAdmin = userRole !== "Admin"; - // If there's exactly one organization the user is admin for, preselect it - if (adminOrgs.length === 1) { + // Org admins must scope a team to an org, so with exactly one we preselect it. + // Proxy admins can create org-less teams, so the field stays optional regardless of org count. + if (isOrgAdmin && adminOrgs.length === 1) { const org = adminOrgs[0]; form.setFieldValue("organization_id", org.organization_id); setCurrentOrgForCreateTeam(org); } else { - // Reset the organization selection for multiple orgs form.setFieldValue("organization_id", currentOrg?.organization_id || null); setCurrentOrgForCreateTeam(currentOrg); } @@ -1132,7 +1133,7 @@ const Teams: React.FC = ({ accessToken, userID, userRole, premiumUser : [] } help={ - isSingleOrg + isOrgAdmin && isSingleOrg ? "You can only create teams within this organization" : isOrgAdmin ? "required" @@ -1142,7 +1143,7 @@ const Teams: React.FC = ({ accessToken, userID, userRole, premiumUser