Merge 2cfadcf3cb into f445e466b4

litellm/proxy/proxy_server.py

@@ -22,6 +22,8 @@ from collections.abc import (
     Awaitable,
     Callable,
     Collection,
+    Iterable,
+    Iterator,
     Mapping,
     MutableMapping,
     Sequence,
@@ -19026,6 +19028,26 @@ async def delete_config_general_settings(
     return response
 
 
+_CALLBACK_LIST_KEYS: Final = ("success_callback", "failure_callback", "callbacks")
+
+
+def _configured_callback_names(value: object) -> tuple[JsonValue, ...]:
+    if isinstance(value, str):
+        return (value,)
+    if isinstance(value, (list, tuple, dict)):
+        return tuple(cast(Iterable[JsonValue], value))  # cast-ok: config.yaml values are untyped
+    return ()
+
+
+def _remaining_callback_names(settings: Mapping[str, object]) -> Iterator[JsonValue]:
+    for key in _CALLBACK_LIST_KEYS:
+        yield from _configured_callback_names(settings.get(key))
+
+
+def _is_callback_name(entry: object, callback_name: str) -> bool:
+    return isinstance(entry, str) and entry.lower() == callback_name
+
+
 @router.post(
     "/config/callback/delete",
     tags=["config.yaml"],
@@ -19061,34 +19083,49 @@ async def delete_callback(
 
     try:
         # Get current configuration
-        config: Final = await proxy_config.get_config()
+        config: Final[dict[str, object]] = cast(  # cast-ok: get_config returns an untyped config dict
+            dict[str, object],
+            await proxy_config.get_config(),
+        )
         callback_name: Final = data.callback_name.lower()
 
         # Check if callback exists in current configuration
-        litellm_settings: Final = config.get("litellm_settings", {})
-        success_callbacks: Final = litellm_settings.get("success_callback", [])
+        litellm_settings: Final[Mapping[str, object]] = cast(  # cast-ok: the config dict's values are untyped
+            Mapping[str, object],
+            config.get("litellm_settings", {}),
+        )
+        configured_lists: Final = {
+            key: _configured_callback_names(litellm_settings.get(key)) for key in _CALLBACK_LIST_KEYS
+        }
+        matching_keys: Final = tuple(
+            key
+            for key, names in configured_lists.items()
+            if any(_is_callback_name(entry, callback_name) for entry in names)
+        )
 
-        if callback_name not in success_callbacks:
+        if not matching_keys:
             raise HTTPException(
                 status_code=404,
                 detail={"error": f"Callback '{callback_name}' not found in active configuration"},
             )
 
-        before_success_callbacks: Final = list(success_callbacks)
-
-        # Remove callback from success_callback list
-        success_callbacks.remove(callback_name)
-        config.setdefault("litellm_settings", {})["success_callback"] = success_callbacks
+        before_callbacks: Final[dict[str, JsonValue]] = {key: list(configured_lists[key]) for key in matching_keys}
+        after_callbacks: Final[dict[str, JsonValue]] = {
+            key: [entry for entry in configured_lists[key] if not _is_callback_name(entry, callback_name)]
+            for key in matching_keys
+        }
+        updated_settings: Final = {**litellm_settings, **after_callbacks}
+        updated_config: Final = {**config, "litellm_settings": updated_settings}
 
         # Save the updated configuration
-        await proxy_config.save_config(new_config=config)
+        await proxy_config.save_config(new_config=updated_config)
 
         asyncio.create_task(
             create_config_audit_log(
                 "litellm_settings",
                 "deleted",
-                {"success_callback": before_success_callbacks},
-                {"success_callback": success_callbacks},
+                before_callbacks,
+                after_callbacks,
                 user_api_key_dict,
             )
         )
@@ -19099,7 +19136,7 @@ async def delete_callback(
         return {
             "message": f"Successfully deleted callback: {callback_name}",
             "removed_callback": callback_name,
-            "remaining_callbacks": success_callbacks,
+            "remaining_callbacks": list(_remaining_callback_names(updated_settings)),
             "deleted_at": datetime.now().isoformat(),
         }
 

tests/e2e/coverage_registry/mgmt.yaml

@@ -71,6 +71,7 @@
 - {id: mgmt.budget.list_v1.happy_path, module: mgmt, tier: P1, surface: api, assertions: [happy_path], source: "management_v1/budgets.py:129", rationale: "Budget enumeration the Budgets page can page, sort and filter"}
 - {id: mgmt.budget.list_v1.admin_only, module: mgmt, tier: P1, surface: api, assertions: [admin_only], source: "management_v1/budgets.py:129", rationale: "A caller without admin view is refused, not served an empty page"}
 - {id: mgmt.callback.list.happy_path, module: mgmt, tier: P2, surface: api, assertions: [happy_path], source: "callback_management_endpoints.py", rationale: "Callback config (smoke)"}
+- {id: mgmt.callback.delete.removes_configured_callback, module: mgmt, tier: P2, surface: api, assertions: [removes_configured_callback], source: "proxy_server.py delete_callback", fail_before_fix: proven, rationale: "A callback listed by GET /get/config/callbacks from success_callback, failure_callback or callbacks must be removable via POST /config/callback/delete; the delete only searched success_callback, so the other two returned 404 (LIT-8444)"}
 - {id: mgmt.cost_tracking.estimate.happy_path, module: mgmt, tier: P2, surface: api, assertions: [happy_path], source: "cost_tracking_settings.py", rationale: "Cost estimate (smoke)"}
 - {id: mgmt.router_settings.update.happy_path, module: mgmt, tier: P2, surface: api, assertions: [happy_path], source: "router_settings_endpoints.py", rationale: "Router config (smoke)"}
 - {id: mgmt.jwt_key_mapping.new.happy_path, module: mgmt, tier: P2, surface: api, assertions: [happy_path], source: "jwt_key_mapping_endpoints.py", rationale: "JWT->key mapping (smoke)"}

tests/e2e/logging/logging_client.py

@@ -17,35 +17,40 @@ import base64
 import json
 import os
 import time
+from collections.abc import Callable
 from dataclasses import dataclass
-from typing import Callable, Literal
+from typing import Literal
 
 import pytest
-from pydantic import BaseModel, ConfigDict, Field, JsonValue, TypeAdapter, ValidationError
-
 from e2e_config import POLL_INTERVAL, POLL_TIMEOUT, settle_propagation
-from proxy_client import ProxyClient
 from e2e_http import (
     URL,
     AuthHeaders,
-    require_successful_call,
     NoBody,
+    Result,
     StreamingResponse,
     Success,
     get,
+    require_successful_call,
     unwrap,
 )
 from models import (
     AnthropicMessagesBody,
+    CallbackDeleteBody,
+    CallbackDeleteResponse,
     ChatBody,
     ChatMessage,
     ChatResponse,
     ChatTool,
     ChatToolFunction,
+    ConfigCallbacksResponse,
+    ConfigUpdateCallbacksBody,
+    ConfigUpdateResponse,
     KeyGenerateBody,
     KeyLoggingCallback,
     KeyLoggingCallbackVars,
     KeyMetadata,
+    LitellmCallbackSettings,
     LiteLLMParamsBody,
     OrgDeleteBody,
     OrgNewBody,
@@ -58,6 +63,8 @@ from models import (
     UserNewBody,
     UserNewResponse,
 )
+from proxy_client import ProxyClient
+from pydantic import BaseModel, ConfigDict, Field, JsonValue, TypeAdapter, ValidationError
 
 # Deliberately invalid *upstream provider* key for failure-path tests.
 # Not a LiteLLM virtual key; OpenAI must reject it after the proxy accepts the call.
@@ -488,6 +495,46 @@ class LoggingClient:
     def delete_model(self, model_id: str) -> None:
         self.proxy.delete_model(model_id)
 
+    def set_litellm_callbacks(self, settings: LitellmCallbackSettings) -> ConfigUpdateResponse:
+        return unwrap(
+            self.proxy.transport.post(
+                "/config/update",
+                headers=self.proxy.transport.master,
+                json=ConfigUpdateCallbacksBody(litellm_settings=settings),
+                response_type=ConfigUpdateResponse,
+            )
+        )
+
+    def clear_litellm_callbacks(self, *keys: Literal["callbacks", "failure_callback"]) -> None:
+        _ = self.set_litellm_callbacks(
+            LitellmCallbackSettings(
+                callbacks=[] if "callbacks" in keys else None,
+                failure_callback=[] if "failure_callback" in keys else None,
+            )
+        )
+
+    def config_callback_names(self) -> set[str]:
+        return {
+            entry.name
+            for entry in unwrap(
+                self.proxy.transport.get(
+                    "/get/config/callbacks",
+                    headers=self.proxy.transport.master,
+                    params=NoBody(),
+                    response_type=ConfigCallbacksResponse,
+                )
+            ).callbacks
+            if not entry.read_only
+        }
+
+    def delete_config_callback(self, callback_name: str) -> Result[CallbackDeleteResponse]:
+        return self.proxy.transport.post(
+            "/config/callback/delete",
+            headers=self.proxy.transport.master,
+            json=CallbackDeleteBody(callback_name=callback_name),
+            response_type=CallbackDeleteResponse,
+        )
+
     def chat(self, key: str, model: str, text: str) -> ChatResponse:
         return unwrap(
             self.proxy.chat(

tests/e2e/logging/test_callback_delete_e2e.py

@@ -0,0 +1,78 @@
+"""A callback listed by GET /get/config/callbacks must be deletable with POST /config/callback/delete."""
+
+from collections.abc import Callable
+
+import pytest
+from e2e_http import unwrap
+from lifecycle import ResourceManager
+from logging_client import LoggingClient
+from models import LitellmCallbackSettings
+
+pytestmark = pytest.mark.e2e
+
+CALLBACK_NAME = "langsmith"
+
+
+class TestCallbackDelete:
+    def _assert_listed_then_deleted(
+        self,
+        client: LoggingClient,
+        resources: ResourceManager,
+        settings: LitellmCallbackSettings,
+        cleanup: Callable[[], object],
+    ) -> None:
+        response = client.set_litellm_callbacks(settings)
+        assert "success" in response.message.lower(), (
+            f"/config/update reported {response.message!r}, expected a success message"
+        )
+        resources.defer(cleanup)
+
+        names = client.config_callback_names()
+        assert CALLBACK_NAME in names, (
+            f"GET /get/config/callbacks does not list {CALLBACK_NAME!r} after it was "
+            f"configured via /config/update; listed {sorted(names)}"
+        )
+
+        deleted = unwrap(client.delete_config_callback(CALLBACK_NAME))
+        assert deleted.removed_callback == CALLBACK_NAME, (
+            f"/config/callback/delete removed {deleted.removed_callback!r}, expected {CALLBACK_NAME!r}"
+        )
+
+        names_after = client.config_callback_names()
+        assert CALLBACK_NAME not in names_after, (
+            f"GET /get/config/callbacks still lists {CALLBACK_NAME!r} after a successful "
+            f"delete; listed {sorted(names_after)}"
+        )
+
+    @pytest.mark.covers("mgmt.callback.delete.removes_configured_callback", exercised_on=[])
+    def test_callback_under_callbacks_key_is_listed_and_deletable(
+        self, client: LoggingClient, resources: ResourceManager
+    ) -> None:
+        self._assert_listed_then_deleted(
+            client,
+            resources,
+            LitellmCallbackSettings(callbacks=[CALLBACK_NAME]),
+            lambda: client.clear_litellm_callbacks("callbacks"),
+        )
+
+    @pytest.mark.covers("mgmt.callback.delete.removes_configured_callback", exercised_on=[])
+    def test_callback_under_failure_callback_key_is_listed_and_deletable(
+        self, client: LoggingClient, resources: ResourceManager
+    ) -> None:
+        self._assert_listed_then_deleted(
+            client,
+            resources,
+            LitellmCallbackSettings(failure_callback=[CALLBACK_NAME]),
+            lambda: client.clear_litellm_callbacks("failure_callback"),
+        )
+
+    @pytest.mark.covers("mgmt.callback.delete.removes_configured_callback", exercised_on=[])
+    def test_callback_under_success_callback_key_is_deletable(
+        self, client: LoggingClient, resources: ResourceManager
+    ) -> None:
+        self._assert_listed_then_deleted(
+            client,
+            resources,
+            LitellmCallbackSettings(success_callback=[CALLBACK_NAME]),
+            lambda: client.delete_config_callback(CALLBACK_NAME),
+        )

tests/e2e/models.py

@@ -1187,6 +1187,55 @@ class ConfigFieldList(RootModel[tuple[ConfigField, ...]]):
     """GET /config/list answers with a bare array of general_settings fields."""
 
 
+class LitellmCallbackSettings(BaseModel):
+    """The litellm_settings callback lists /config/update merges. Unset keys are
+    left out of the request so the proxy keeps whatever the stored row holds."""
+
+    model_config = ConfigDict(extra="ignore")
+    success_callback: list[str] | None = None
+    failure_callback: list[str] | None = None
+    callbacks: list[str] | None = None
+
+
+class ConfigUpdateCallbacksBody(BaseModel):
+    """POST /config/update body: the same shape the Admin UI's Logging page
+    sends when an admin adds a callback (litellm_settings plus, in the UI's
+    case, environment_variables the test does not need)."""
+
+    litellm_settings: LitellmCallbackSettings
+
+
+class ConfigUpdateResponse(BaseModel):
+    model_config = ConfigDict(extra="ignore")
+    message: str
+
+
+class ConfigCallbackEntry(BaseModel):
+    """One row of GET /get/config/callbacks `callbacks`. `type` is which
+    configured list the entry came from (success, failure, success_and_failure)."""
+
+    model_config = ConfigDict(extra="ignore")
+    name: str
+    type: str | None = None
+    read_only: bool | None = None
+
+
+class ConfigCallbacksResponse(BaseModel):
+    model_config = ConfigDict(extra="ignore")
+    callbacks: list[ConfigCallbackEntry] = []
+
+
+class CallbackDeleteBody(BaseModel):
+    callback_name: str
+
+
+class CallbackDeleteResponse(BaseModel):
+    model_config = ConfigDict(extra="ignore")
+    message: str
+    removed_callback: str
+    remaining_callbacks: list[str] = []
+
+
 class CostMapEntry(BaseModel):
     model_config = ConfigDict(extra="ignore")
     litellm_provider: str | None = None

tests/unit/proxy/proxy_server/test_routes_config.py

@@ -14,10 +14,14 @@ Routes covered:
 from __future__ import annotations
 
 import asyncio
+import contextlib
 import json
+from collections.abc import Callable
 from unittest.mock import AsyncMock, MagicMock
 
+import httpx
 import pytest
+from fastapi.testclient import TestClient
 
 from .conftest import VOLATILE_KEYS, normalize
 
@@ -944,6 +948,129 @@ def test_config_callback_delete_not_found(client, auth_as, mock_prisma, monkeypa
     assert "langfuse" in str(response.json()).lower() or "not found" in str(response.json()).lower()
 
 
+def _delete_callback_roundtrip(
+    client: TestClient,
+    auth_as: Callable[..., contextlib.AbstractContextManager[None]],
+    mock_prisma: MagicMock,
+    monkeypatch: pytest.MonkeyPatch,
+    litellm_settings: dict[str, object],
+    callback_name: str,
+) -> tuple[httpx.Response, dict[str, object]]:
+    """POST /config/callback/delete as admin against a stubbed config, and
+    return (response, saved litellm_settings) so each case pins the full body
+    plus the exact config persisted."""
+    from litellm.proxy import proxy_server as ps
+    from litellm.proxy._types import LitellmUserRoles
+
+    _install_litellm_config(mock_prisma)
+    monkeypatch.setattr(ps, "prisma_client", mock_prisma)
+    monkeypatch.setattr(ps, "store_model_in_db", True)
+
+    fake_proxy_config = MagicMock()
+    fake_proxy_config.get_config = AsyncMock(return_value={"litellm_settings": litellm_settings})
+    fake_proxy_config.save_config = AsyncMock()
+    fake_proxy_config.add_deployment = AsyncMock()
+    monkeypatch.setattr(ps, "proxy_config", fake_proxy_config)
+
+    with auth_as(LitellmUserRoles.PROXY_ADMIN):
+        response = client.post("/config/callback/delete", json={"callback_name": callback_name})
+    saved = fake_proxy_config.save_config.await_args.kwargs["new_config"]["litellm_settings"]
+    return response, saved
+
+
+def test_config_callback_delete_from_failure_callback(
+    client: TestClient,
+    auth_as: Callable[..., contextlib.AbstractContextManager[None]],
+    mock_prisma: MagicMock,
+    monkeypatch: pytest.MonkeyPatch,
+):
+    """A callback configured only under litellm_settings.failure_callback is
+    listed by GET /get/config/callbacks, so the delete route must remove it."""
+    response, saved = _delete_callback_roundtrip(
+        client,
+        auth_as,
+        mock_prisma,
+        monkeypatch,
+        {"success_callback": ["slack"], "failure_callback": ["datadog", "sentry"]},
+        "datadog",
+    )
+    assert response.status_code == 200
+    assert response.json()["removed_callback"] == "datadog"
+    assert saved == {"success_callback": ["slack"], "failure_callback": ["sentry"]}
+    assert response.json()["remaining_callbacks"] == ["slack", "sentry"]
+
+
+def test_config_callback_delete_from_callbacks_list(
+    client: TestClient,
+    auth_as: Callable[..., contextlib.AbstractContextManager[None]],
+    mock_prisma: MagicMock,
+    monkeypatch: pytest.MonkeyPatch,
+):
+    """A callback configured only under litellm_settings.callbacks is deleted
+    while unrelated lists pass through untouched."""
+    response, saved = _delete_callback_roundtrip(
+        client,
+        auth_as,
+        mock_prisma,
+        monkeypatch,
+        {"success_callback": ["langfuse"], "callbacks": ["datadog"], "drop_params": True},
+        "datadog",
+    )
+    assert response.status_code == 200
+    assert response.json()["removed_callback"] == "datadog"
+    assert saved == {"success_callback": ["langfuse"], "callbacks": [], "drop_params": True}
+    assert response.json()["remaining_callbacks"] == ["langfuse"]
+
+
+def test_config_callback_delete_from_multiple_lists(
+    client: TestClient,
+    auth_as: Callable[..., contextlib.AbstractContextManager[None]],
+    mock_prisma: MagicMock,
+    monkeypatch: pytest.MonkeyPatch,
+):
+    """A name present in both success_callback and callbacks is removed from
+    both, and remaining_callbacks reports the leftovers of every list."""
+    response, saved = _delete_callback_roundtrip(
+        client,
+        auth_as,
+        mock_prisma,
+        monkeypatch,
+        {
+            "success_callback": ["datadog", "slack"],
+            "failure_callback": ["sentry"],
+            "callbacks": ["langfuse", "datadog"],
+        },
+        "datadog",
+    )
+    assert response.status_code == 200
+    assert saved == {
+        "success_callback": ["slack"],
+        "failure_callback": ["sentry"],
+        "callbacks": ["langfuse"],
+    }
+    assert response.json()["remaining_callbacks"] == ["slack", "sentry", "langfuse"]
+
+
+def test_config_callback_delete_case_insensitive(
+    client: TestClient,
+    auth_as: Callable[..., contextlib.AbstractContextManager[None]],
+    mock_prisma: MagicMock,
+    monkeypatch: pytest.MonkeyPatch,
+):
+    """A mixed-case request name still matches the configured lowercase entry."""
+    response, saved = _delete_callback_roundtrip(
+        client,
+        auth_as,
+        mock_prisma,
+        monkeypatch,
+        {"callbacks": ["datadog", "otel"]},
+        "DataDog",
+    )
+    assert response.status_code == 200
+    assert response.json()["removed_callback"] == "datadog"
+    assert saved == {"callbacks": ["otel"]}
+
+
 # ---------------------------------------------------------------------------
 # GET /get/config/callbacks
 # ---------------------------------------------------------------------------