Merge remote-tracking branch 'origin/litellm_internal_staging' into litellm_fix_budget_window_delete

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",

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")
+    )

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 {}

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)

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

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(<OldTeams accessToken="test-token" userID="user-123" userRole="Admin" />);
+
+    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 }),
+      );
+    });
+  });
+});

ui/litellm-dashboard/src/components/OldTeams.tsx

@@ -262,14 +262,15 @@ const Teams: React.FC<TeamProps> = ({ 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<TeamProps> = ({ 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<TeamProps> = ({ accessToken, userID, userRole, premiumUser
                       <Select
                         showSearch
                         allowClear={!isOrgAdmin}
-                        disabled={isSingleOrg}
+                        disabled={isOrgAdmin && isSingleOrg}
                         placeholder={hasNoOrgs ? "No organizations available" : "Search or select an Organization"}
                         onChange={(value) => {
                           form.setFieldValue("organization_id", value);