From 0f160e9042a31547a69a3f78ffa1fbe847ac0c37 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Thu, 29 Oct 2026 10:20:52 -0700 Subject: [PATCH] fix(vector stores): confirm a provider delete when a concurrent delete already removed the managed row The base managed resource delete returns None for a missing row, as its docstring says, so the route answers the provider's confirmation instead of 500. The managed files chat guard checks the message as an object, which keeps the runtime check without a redundant isinstance for the type gate --- .../proxy/hooks/managed_files.py | 2 +- .../base_managed_resource.py | 2 +- .../test_vector_store_endpoints.py | 53 +++++++++++++------ 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py index 171be00ef6b..6b4c0519b27 100644 --- a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py +++ b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py @@ -1735,7 +1735,7 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): """ file_ids = [] for message in messages: - if isinstance(message, dict) and message.get("role") == "user": + if isinstance(cast(object, message), dict) and message.get("role") == "user": content = message.get("content") if content: if isinstance(content, str): diff --git a/litellm/llms/base_llm/managed_resources/base_managed_resource.py b/litellm/llms/base_llm/managed_resources/base_managed_resource.py index 2663a3c42e7..b36770876d5 100644 --- a/litellm/llms/base_llm/managed_resources/base_managed_resource.py +++ b/litellm/llms/base_llm/managed_resources/base_managed_resource.py @@ -302,7 +302,7 @@ class BaseManagedResource(ABC, Generic[ResourceObjectType]): initial_value: Final = await table.find_first(where={"unified_resource_id": unified_resource_id}) if initial_value is None: - raise Exception(f"LiteLLM Managed {self.resource_type} with id={unified_resource_id} not found") + return None # Delete from cache await self.internal_usage_cache.async_set_cache( diff --git a/tests/unit/proxy/vector_store_endpoints/test_vector_store_endpoints.py b/tests/unit/proxy/vector_store_endpoints/test_vector_store_endpoints.py index 3a0b453a2d3..ba204eb2913 100644 --- a/tests/unit/proxy/vector_store_endpoints/test_vector_store_endpoints.py +++ b/tests/unit/proxy/vector_store_endpoints/test_vector_store_endpoints.py @@ -3434,11 +3434,13 @@ class _ManagedStoreTable: return self.rows.pop(where["unified_resource_id"]) -def test_proxy_answers_by_managed_vector_store_id_and_forgets_the_store_once_the_provider_deletes_it(monkeypatch): +_PROVIDER_STORE_ROUTE = "https://api.openai.com/v1/vector_stores/vs_provider" +_PROVIDER_DELETED_STORE = {"id": "vs_provider", "object": "vector_store.deleted", "deleted": True} + + +def _proxy_serving_one_managed_vector_store(monkeypatch): from types import SimpleNamespace - import httpx - import respx from fastapi.testclient import TestClient from litellm_enterprise.proxy.hooks.managed_vector_stores import PROXY_LiteLLMManagedVectorStores @@ -3474,21 +3476,24 @@ def test_proxy_answers_by_managed_vector_store_id_and_forgets_the_store_once_the table.rows[unified_id] = _ManagedStoreRow(provider_store) monkeypatch.setitem(proxy_server.proxy_logging_obj.proxy_hook_mapping, "managed_vector_stores", hook) monkeypatch.setattr(litellm, "callbacks", [hook]) - headers = {"Authorization": "Bearer sk-master-test"} - client = TestClient(app) + return TestClient(app, headers={"Authorization": "Bearer sk-master-test"}), table, unified_id, provider_store + + +def test_proxy_answers_by_managed_vector_store_id_and_forgets_the_store_once_the_provider_deletes_it(monkeypatch): + import httpx + import respx + + client, _, unified_id, provider_store = _proxy_serving_one_managed_vector_store(monkeypatch) with respx.mock(assert_all_called=False) as upstream: - provider_route = "https://api.openai.com/v1/vector_stores/vs_provider" - upstream.get(provider_route).mock(return_value=httpx.Response(200, json=provider_store)) - upstream.post(provider_route).mock(return_value=httpx.Response(200, json={**provider_store, "name": "renamed"})) - upstream.delete(provider_route).mock( - return_value=httpx.Response(200, json={"id": "vs_provider", "object": "vector_store.deleted", "deleted": True}) - ) + upstream.get(_PROVIDER_STORE_ROUTE).mock(return_value=httpx.Response(200, json=provider_store)) + upstream.post(_PROVIDER_STORE_ROUTE).mock(return_value=httpx.Response(200, json={**provider_store, "name": "renamed"})) + upstream.delete(_PROVIDER_STORE_ROUTE).mock(return_value=httpx.Response(200, json=_PROVIDER_DELETED_STORE)) - retrieved = client.get(f"/v1/vector_stores/{unified_id}", headers=headers) - updated = client.post(f"/v1/vector_stores/{unified_id}", json={"name": "renamed"}, headers=headers) - deleted = client.delete(f"/v1/vector_stores/{unified_id}", headers=headers) - retrieved_after_delete = client.get(f"/v1/vector_stores/{unified_id}", headers=headers) + retrieved = client.get(f"/v1/vector_stores/{unified_id}") + updated = client.post(f"/v1/vector_stores/{unified_id}", json={"name": "renamed"}) + deleted = client.delete(f"/v1/vector_stores/{unified_id}") + retrieved_after_delete = client.get(f"/v1/vector_stores/{unified_id}") upstream_requests = [(call.request.method, call.request.url.path) for call in upstream.calls] assert retrieved.status_code == 200, retrieved.text @@ -3503,3 +3508,21 @@ def test_proxy_answers_by_managed_vector_store_id_and_forgets_the_store_once_the ("POST", "/v1/vector_stores/vs_provider"), ("DELETE", "/v1/vector_stores/vs_provider"), ] + + +def test_proxy_confirms_a_managed_vector_store_delete_when_a_concurrent_delete_already_forgot_the_store(monkeypatch): + import httpx + import respx + + client, table, unified_id, _ = _proxy_serving_one_managed_vector_store(monkeypatch) + + def provider_deletes_after_a_concurrent_delete_forgot_the_store(request): + table.rows.pop(unified_id) + return httpx.Response(200, json=_PROVIDER_DELETED_STORE) + + with respx.mock(assert_all_called=False) as upstream: + upstream.delete(_PROVIDER_STORE_ROUTE).mock(side_effect=provider_deletes_after_a_concurrent_delete_forgot_the_store) + deleted = client.delete(f"/v1/vector_stores/{unified_id}") + + assert deleted.status_code == 200, deleted.text + assert (deleted.json()["id"], deleted.json()["deleted"]) == (unified_id, True)