From fbe26c3a80cfd1413e98249b582f319b38f410ef Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Sun, 6 Sep 2026 07:28:09 +0000 Subject: [PATCH] fix(e2e): refuse a read-back that no replica serves A read-back over an empty replica mapping satisfied every predicate and returned as if it had converged, so it would have asserted nothing and passed. No wiring can produce that today, since the replica list always falls back to at least one URL, but a helper whose whole job is proving a write reached every replica should not have a shape that passes vacuously. --- .../proxy/_experimental/mcp_server/toolset_db.py | 5 ++--- tests/e2e/proxy_client.py | 7 +++++-- tests/e2e/test_e2e_http.py | 2 +- tests/e2e/test_proxy_client.py | 13 +++++++++++-- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/toolset_db.py b/litellm/proxy/_experimental/mcp_server/toolset_db.py index f13b1c72558..750ff2428a4 100644 --- a/litellm/proxy/_experimental/mcp_server/toolset_db.py +++ b/litellm/proxy/_experimental/mcp_server/toolset_db.py @@ -132,9 +132,8 @@ async def update_mcp_toolset( data: UpdateMCPToolsetRequest, touched_by: str, ) -> MCPToolset | None: - """A partial update: a field the caller left out keeps its stored value and a - field sent as null is cleared, where a null ``tools`` is an empty list and a null - ``toolset_name`` is ignored because a toolset always has a name.""" + """A partial update: absent keeps, null clears, except that a toolset always has a + name, so a null ``toolset_name`` is ignored.""" data_dict: Final = data.model_dump(exclude_unset=True, exclude={"toolset_id"}) if "tools" in data_dict: data_dict["tools"] = json.dumps(data_dict["tools"] or ()) diff --git a/tests/e2e/proxy_client.py b/tests/e2e/proxy_client.py index 47c5d0abaff..ea6cc191e63 100644 --- a/tests/e2e/proxy_client.py +++ b/tests/e2e/proxy_client.py @@ -542,8 +542,11 @@ class ProxyClient: """The replicas that serve `path`: every data-plane replica for an LLM route, and for a management route the control-plane replicas, which in a split deployment is the one service that serves it (the data-plane replicas trim - management routes) and in a monolith is every replica.""" - return self.control_replicas if is_control_plane_path(path) else self.replicas + management routes) and in a monolith is every replica. Never empty: a + read-back against no replica would assert nothing and pass.""" + replicas: Final = self.control_replicas if is_control_plane_path(path) else self.replicas + assert replicas, f"no replica is configured to serve {path}, so a read-back there would prove nothing" + return replicas def read_back_everywhere[R: BaseModel]( self, path: str, response_type: type[R], *, settled: Callable[[R], bool] diff --git a/tests/e2e/test_e2e_http.py b/tests/e2e/test_e2e_http.py index 6ab35dd6cd5..d0807226627 100644 --- a/tests/e2e/test_e2e_http.py +++ b/tests/e2e/test_e2e_http.py @@ -177,7 +177,7 @@ _JSON: Final[TypeAdapter[object]] = TypeAdapter(object) @dataclass class FakeJsonResponse: - """The `_classify` view of a response: a status, the raw body bytes, and the + """The `classify` view of a response: a status, the raw body bytes, and the parse that would raise on an empty one.""" status_code: int diff --git a/tests/e2e/test_proxy_client.py b/tests/e2e/test_proxy_client.py index 88931525c12..474aaf210c2 100644 --- a/tests/e2e/test_proxy_client.py +++ b/tests/e2e/test_proxy_client.py @@ -13,10 +13,9 @@ from __future__ import annotations from collections.abc import Iterable, Mapping from dataclasses import dataclass from itertools import chain, repeat -from typing import Final +from typing import Final, cast import pytest - from e2e_config import parse_replica_urls from e2e_http import Success from models import ModelListEntry, ModelsListResponse @@ -25,14 +24,17 @@ from proxy_client import ( ModelsPoller, NeverConvergedOn, NotServableOn, + ProxyClient, ReplicaRead, Servable, await_everywhere, await_servable_everywhere, build_proxy_client, ) +from transport import Transport MODEL: Final = "gpt-under-test" +_NO_TRANSPORTS: Final = cast(Transport, None) TIMEOUT: Final = 10.0 INTERVAL: Final = 2.0 @@ -156,3 +158,10 @@ class TestReplicasFor: replica_urls=("http://pod-1", "http://pod-2"), ) assert set(client.replicas_for("/v1/mcp/server/abc")) == {"http://pod-1", "http://pod-2"} + + def test_a_route_no_replica_serves_is_refused_rather_than_read_back_vacuously(self) -> None: + """A read-back over zero replicas would satisfy every predicate and assert + nothing, so asking for one fails instead of passing silently.""" + client: Final = ProxyClient(transport=_NO_TRANSPORTS, replicas={}, control_replicas={}) + with pytest.raises(AssertionError, match="no replica is configured"): + _ = client.replicas_for("/v1/models")